diff mbox series

[1/6] dt-bindings: regulator: pca9450: Document new usage of sd-vsel-gpios

Message ID 20230213155833.1644366-2-frieder@fris.de
State New
Headers
Series Use correct LDO5 control registers for PCA9450 |

Commit Message

Frieder Schrempf Feb. 13, 2023, 3:58 p.m. UTC 
  From: Frieder Schrempf <frieder.schrempf@kontron.de>

The sd-vsel-gpios property is abandoned in its current meaning as an
output. We now use it to specify an optional signal that can be
evaluated by the driver in order to retrieve the current status
of the SD_VSEL signal that is used to select the control register
of LDO5.

Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
---
 .../regulator/nxp,pca9450-regulator.yaml      | 23 ++++++++++++++-----
 1 file changed, 17 insertions(+), 6 deletions(-)

Comments

Rob Herring Feb. 15, 2023, 8:02 p.m. UTC | #1 
On Mon, Feb 13, 2023 at 04:58:19PM +0100, Frieder Schrempf wrote:
> From: Frieder Schrempf <frieder.schrempf@kontron.de>
> 
> The sd-vsel-gpios property is abandoned in its current meaning as an
> output. We now use it to specify an optional signal that can be
> evaluated by the driver in order to retrieve the current status
> of the SD_VSEL signal that is used to select the control register
> of LDO5.
> 
> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
> ---
>  .../regulator/nxp,pca9450-regulator.yaml      | 23 ++++++++++++++-----
>  1 file changed, 17 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml b/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml
> index 835b53302db8..c86534538a4e 100644
> --- a/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml
> +++ b/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml
> @@ -40,8 +40,24 @@ properties:
>      description: |
>        list of regulators provided by this controller
>  
> +    properties:
> +      LDO5:
> +        type: object
> +        $ref: regulator.yaml#
> +        description:
> +          Properties for single LDO5 regulator.
> +
> +        properties:
> +          sd-vsel-gpios:

It is a pin on the device, right? Then it belongs in the device node as 
it was.

Can't the direction of the signal tell you how it is used? Assuming the 
pin is bidirectional?

The binding should support any possible way the device is wired, not 
just what's been seen so far on some boards.

Rob
Marek Vasut Feb. 16, 2023, 1:27 a.m. UTC | #2 
On 2/15/23 21:02, Rob Herring wrote:
> On Mon, Feb 13, 2023 at 04:58:19PM +0100, Frieder Schrempf wrote:
>> From: Frieder Schrempf <frieder.schrempf@kontron.de>
>>
>> The sd-vsel-gpios property is abandoned in its current meaning as an
>> output. We now use it to specify an optional signal that can be
>> evaluated by the driver in order to retrieve the current status
>> of the SD_VSEL signal that is used to select the control register
>> of LDO5.
>>
>> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
>> ---
>>   .../regulator/nxp,pca9450-regulator.yaml      | 23 ++++++++++++++-----
>>   1 file changed, 17 insertions(+), 6 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml b/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml
>> index 835b53302db8..c86534538a4e 100644
>> --- a/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml
>> +++ b/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml
>> @@ -40,8 +40,24 @@ properties:
>>       description: |
>>         list of regulators provided by this controller
>>   
>> +    properties:
>> +      LDO5:
>> +        type: object
>> +        $ref: regulator.yaml#
>> +        description:
>> +          Properties for single LDO5 regulator.
>> +
>> +        properties:
>> +          sd-vsel-gpios:
> 
> It is a pin on the device, right? Then it belongs in the device node as
> it was.
> 
> Can't the direction of the signal tell you how it is used? Assuming the
> pin is bidirectional?

The pin is input to the PMIC, it is unidirection, i.e.

SoC(output)---->(input)PMIC

> The binding should support any possible way the device is wired, not
> just what's been seen so far on some boards.

The usage is always the above as far as I can tell.
Rob Herring Feb. 16, 2023, 2:30 a.m. UTC | #3 
On Wed, Feb 15, 2023 at 7:27 PM Marek Vasut <marex@denx.de> wrote:
>
> On 2/15/23 21:02, Rob Herring wrote:
> > On Mon, Feb 13, 2023 at 04:58:19PM +0100, Frieder Schrempf wrote:
> >> From: Frieder Schrempf <frieder.schrempf@kontron.de>
> >>
> >> The sd-vsel-gpios property is abandoned in its current meaning as an
> >> output. We now use it to specify an optional signal that can be
> >> evaluated by the driver in order to retrieve the current status
> >> of the SD_VSEL signal that is used to select the control register
> >> of LDO5.
> >>
> >> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
> >> ---
> >>   .../regulator/nxp,pca9450-regulator.yaml      | 23 ++++++++++++++-----
> >>   1 file changed, 17 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml b/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml
> >> index 835b53302db8..c86534538a4e 100644
> >> --- a/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml
> >> +++ b/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml
> >> @@ -40,8 +40,24 @@ properties:
> >>       description: |
> >>         list of regulators provided by this controller
> >>
> >> +    properties:
> >> +      LDO5:
> >> +        type: object
> >> +        $ref: regulator.yaml#
> >> +        description:
> >> +          Properties for single LDO5 regulator.
> >> +
> >> +        properties:
> >> +          sd-vsel-gpios:
> >
> > It is a pin on the device, right? Then it belongs in the device node as
> > it was.
> >
> > Can't the direction of the signal tell you how it is used? Assuming the
> > pin is bidirectional?
>
> The pin is input to the PMIC, it is unidirection, i.e.
>
> SoC(output)---->(input)PMIC
>
> > The binding should support any possible way the device is wired, not
> > just what's been seen so far on some boards.
>
> The usage is always the above as far as I can tell.

This patch is saying the opposite though. Something else drives the
signal, but the signal is also routed to the SoC to sample the state.

Rob
Frieder Schrempf Feb. 16, 2023, 10:15 a.m. UTC | #4 
On 16.02.23 03:30, Rob Herring wrote:
> On Wed, Feb 15, 2023 at 7:27 PM Marek Vasut <marex@denx.de> wrote:
>>
>> On 2/15/23 21:02, Rob Herring wrote:
>>> On Mon, Feb 13, 2023 at 04:58:19PM +0100, Frieder Schrempf wrote:
>>>> From: Frieder Schrempf <frieder.schrempf@kontron.de>
>>>>
>>>> The sd-vsel-gpios property is abandoned in its current meaning as an
>>>> output. We now use it to specify an optional signal that can be
>>>> evaluated by the driver in order to retrieve the current status
>>>> of the SD_VSEL signal that is used to select the control register
>>>> of LDO5.
>>>>
>>>> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
>>>> ---
>>>>   .../regulator/nxp,pca9450-regulator.yaml      | 23 ++++++++++++++-----
>>>>   1 file changed, 17 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml b/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml
>>>> index 835b53302db8..c86534538a4e 100644
>>>> --- a/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml
>>>> +++ b/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml
>>>> @@ -40,8 +40,24 @@ properties:
>>>>       description: |
>>>>         list of regulators provided by this controller
>>>>
>>>> +    properties:
>>>> +      LDO5:
>>>> +        type: object
>>>> +        $ref: regulator.yaml#
>>>> +        description:
>>>> +          Properties for single LDO5 regulator.
>>>> +
>>>> +        properties:
>>>> +          sd-vsel-gpios:
>>>
>>> It is a pin on the device, right? Then it belongs in the device node as
>>> it was.

Physically it's a pin on the PCA9450 chip. If you look at the block
diagram in the datasheet [1] (page 3) you can see though, that the
SD_VSEL signal is routed to the LD05 regulator block inside the chip.
This makes me think that the signal is best described inside the LDO5 node.

>>>
>>> Can't the direction of the signal tell you how it is used? Assuming the
>>> pin is bidirectional?
>>
>> The pin is input to the PMIC, it is unidirection, i.e.
>>
>> SoC(output)---->(input)PMIC
>>
>>> The binding should support any possible way the device is wired, not
>>> just what's been seen so far on some boards.
>>
>> The usage is always the above as far as I can tell.

There is only one usage that is likely to occur and that is the one we
describe here.

There are other ways to wire up the signal of course and in some
unlikely event a hardware engineer might have the idea to hard-wire the
SD_VSEL to a fixed level or wire it up to a SoC pin that doesn't have
the VSELECT mux option.

But I don't really see a good reason for covering these cases in the
binding/driver if there are good chances we won't ever need them.

> This patch is saying the opposite though. Something else drives the
> signal, but the signal is also routed to the SoC to sample the state.

SoC                                  PMIC
+-----------------------+           +-------------------+
|                       |           |                   |
|                       |           |                   |
|  GPIO <----------+    |           |                   |
|                  |    |    SD_VSEL|   +-------+       |
|  USHC_VSELECT -->+------------------->| LDO5  |       |
|                       |           |   +-------+       |
|                       |           |                   |
+-----------------------+           +-------------------+

This is how the setup looks like. The SD_VSEL on the PMIC is always an
input. It's driven by the SoC's VSELECT signal (controlled by the USDHC
controller) and we use the SION bit in the IOMUX to internally loop back
the signal in order to sample it using the GPIO.

[1] https://www.nxp.com/docs/en/data-sheet/PCA9450DS.pdf
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml b/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml
index 835b53302db8..c86534538a4e 100644
--- a/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml
+++ b/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml
@@ -40,8 +40,24 @@  properties:
     description: |
       list of regulators provided by this controller
 
+    properties:
+      LDO5:
+        type: object
+        $ref: regulator.yaml#
+        description:
+          Properties for single LDO5 regulator.
+
+        properties:
+          sd-vsel-gpios:
+            description:
+              GPIO that can be used to read the current status of the SD_VSEL
+              signal in order for the driver to know if LDO5CTRL_L or LDO5CTRL_H
+              is used by the hardware.
+
+        unevaluatedProperties: false
+
     patternProperties:
-      "^LDO[1-5]$":
+      "^LDO[1-4]$":
         type: object
         $ref: regulator.yaml#
         description:
@@ -76,11 +92,6 @@  properties:
 
     additionalProperties: false
 
-  sd-vsel-gpios:
-    description: GPIO that is used to switch LDO5 between being configured by
-      LDO5CTRL_L or LDO5CTRL_H register. Use this if the SD_VSEL signal is
-      connected to a host GPIO.
-
   nxp,i2c-lt-enable:
     type: boolean
     description: