diff mbox series

[2/3] dt-bindings: pinctrl: rockchip: Add io domain properties

Message ID 20230904115816.1237684-3-s.hauer@pengutronix.de (mailing list archive)
State New, archived
Headers show
Series Make Rockchip IO domains dependency from other devices explicit | expand

Commit Message

Sascha Hauer Sept. 4, 2023, 11:58 a.m. UTC
Add rockchip,io-domains property to the Rockchip pinctrl driver. This
list of phandles points to the IO domain device(s) the pins of the
pinctrl driver are supplied from.

Also a rockchip,io-domain-boot-on property is added to pin groups
which can be used for pin groups which themselves are needed to access
the regulators an IO domain is driven from.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 .../bindings/pinctrl/rockchip,pinctrl.yaml          | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

Comments

Robin Murphy Sept. 5, 2023, 9:03 a.m. UTC | #1
On 2023-09-04 12:58, Sascha Hauer wrote:
> Add rockchip,io-domains property to the Rockchip pinctrl driver. This
> list of phandles points to the IO domain device(s) the pins of the
> pinctrl driver are supplied from.
> 
> Also a rockchip,io-domain-boot-on property is added to pin groups
> which can be used for pin groups which themselves are needed to access
> the regulators an IO domain is driven from.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>   .../bindings/pinctrl/rockchip,pinctrl.yaml          | 13 ++++++++++++-
>   1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.yaml
> index 10c335efe619e..92075419d29cf 100644
> --- a/Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.yaml
> +++ b/Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.yaml
> @@ -62,6 +62,11 @@ properties:
>         Required for at least rk3188 and rk3288. On the rk3368 this should
>         point to the PMUGRF syscon.
>   
> +  rockchip,io-domains:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    description:
> +      Phandles to io domains
> +
>     "#address-cells":
>       enum: [1, 2]
>   
> @@ -137,7 +142,13 @@ additionalProperties:
>               - description:
>                   The phandle of a node contains the generic pinconfig options
>                   to use as described in pinctrl-bindings.txt.
> -
> +      rockchip,io-domain-boot-on:

I don't think "on" is a particularly descriptive or useful property name 
for something that has no "off" state. Furthermore it's no help at all 
if the DT consumer *is* the bootloader that's expected to configure this 
in the first place. IMO it would seem a lot more sensible to have an 
integer (or enum) property which describes the actual value for the 
initial I/O domain setting. Then Linux can choose to assume the presence 
of the property at all implies that the bootloader should have set it up 
already, but also has the option of actively enforcing it as well if we 
want to.

> +        type: boolean
> +        description:
> +          If true assume that the io domain needed for this pin group has been
> +          configured correctly by the bootloader. This is needed to break cyclic
> +          dependencies introduced when a io domain needs a regulator that can be
> +          accessed through pins configured here.

This is describing a Linux implementation detail, not the binding 
itself. There's no technical reason a DT consumer couldn't already 
figure this much out from the existing topology (by observing that the 
pinctrl consumer is a grandparent of the I/O domain's supply).

Thanks,
Robin.

>   examples:
>     - |
>       #include <dt-bindings/interrupt-controller/arm-gic.h>
Rob Herring (Arm) Sept. 5, 2023, 6:14 p.m. UTC | #2
On Mon, Sep 04, 2023 at 01:58:15PM +0200, Sascha Hauer wrote:
> Add rockchip,io-domains property to the Rockchip pinctrl driver. This
> list of phandles points to the IO domain device(s) the pins of the
> pinctrl driver are supplied from.

Is there an actual need for multiple IO devices with multiple pinctrl 
blocks? If not, you don't need a property, just lookup the IO domain 
node by compatible.

> 
> Also a rockchip,io-domain-boot-on property is added to pin groups
> which can be used for pin groups which themselves are needed to access
> the regulators an IO domain is driven from.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  .../bindings/pinctrl/rockchip,pinctrl.yaml          | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.yaml
> index 10c335efe619e..92075419d29cf 100644
> --- a/Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.yaml
> +++ b/Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.yaml
> @@ -62,6 +62,11 @@ properties:
>        Required for at least rk3188 and rk3288. On the rk3368 this should
>        point to the PMUGRF syscon.
>  
> +  rockchip,io-domains:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    description:
> +      Phandles to io domains
> +
>    "#address-cells":
>      enum: [1, 2]
>  
> @@ -137,7 +142,13 @@ additionalProperties:
>              - description:
>                  The phandle of a node contains the generic pinconfig options
>                  to use as described in pinctrl-bindings.txt.
> -
> +      rockchip,io-domain-boot-on:
> +        type: boolean
> +        description:
> +          If true assume that the io domain needed for this pin group has been
> +          configured correctly by the bootloader. This is needed to break cyclic
> +          dependencies introduced when a io domain needs a regulator that can be
> +          accessed through pins configured here.
>  examples:
>    - |
>      #include <dt-bindings/interrupt-controller/arm-gic.h>
> -- 
> 2.39.2
>
Sascha Hauer Sept. 6, 2023, 7:21 a.m. UTC | #3
On Tue, Sep 05, 2023 at 10:03:20AM +0100, Robin Murphy wrote:
> On 2023-09-04 12:58, Sascha Hauer wrote:
> > Add rockchip,io-domains property to the Rockchip pinctrl driver. This
> > list of phandles points to the IO domain device(s) the pins of the
> > pinctrl driver are supplied from.
> > 
> > Also a rockchip,io-domain-boot-on property is added to pin groups
> > which can be used for pin groups which themselves are needed to access
> > the regulators an IO domain is driven from.
> > 
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> >   .../bindings/pinctrl/rockchip,pinctrl.yaml          | 13 ++++++++++++-
> >   1 file changed, 12 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.yaml
> > index 10c335efe619e..92075419d29cf 100644
> > --- a/Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.yaml
> > +++ b/Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.yaml
> > @@ -62,6 +62,11 @@ properties:
> >         Required for at least rk3188 and rk3288. On the rk3368 this should
> >         point to the PMUGRF syscon.
> > +  rockchip,io-domains:
> > +    $ref: /schemas/types.yaml#/definitions/phandle-array
> > +    description:
> > +      Phandles to io domains
> > +
> >     "#address-cells":
> >       enum: [1, 2]
> > @@ -137,7 +142,13 @@ additionalProperties:
> >               - description:
> >                   The phandle of a node contains the generic pinconfig options
> >                   to use as described in pinctrl-bindings.txt.
> > -
> > +      rockchip,io-domain-boot-on:
> 
> I don't think "on" is a particularly descriptive or useful property name for
> something that has no "off" state.

In fact it has an "off" state. A IO Domain can be disabled in the SoC
registers and also the corresponding regulator can be disabled.

> Furthermore it's no help at all if the DT
> consumer *is* the bootloader that's expected to configure this in the first
> place. IMO it would seem a lot more sensible to have an integer (or enum)
> property which describes the actual value for the initial I/O domain
> setting.

I agree though that a particular setting instead of a boolean is better
and could help the bootloader.

> Then Linux can choose to assume the presence of the property at all
> implies that the bootloader should have set it up already, but also has the
> option of actively enforcing it as well if we want to.

Ok.

Thanks,
 Sascha
Quentin Schulz Sept. 6, 2023, 8:20 a.m. UTC | #4
Sascha, Robin,

On 9/5/23 11:03, Robin Murphy wrote:
> [You don't often get email from robin.murphy@arm.com. Learn why this is 
> important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> On 2023-09-04 12:58, Sascha Hauer wrote:
>> Add rockchip,io-domains property to the Rockchip pinctrl driver. This
>> list of phandles points to the IO domain device(s) the pins of the
>> pinctrl driver are supplied from.
>>
>> Also a rockchip,io-domain-boot-on property is added to pin groups
>> which can be used for pin groups which themselves are needed to access
>> the regulators an IO domain is driven from.
>>
>> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>> ---
>>   .../bindings/pinctrl/rockchip,pinctrl.yaml          | 13 ++++++++++++-
>>   1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git 
>> a/Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.yaml 
>> b/Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.yaml
>> index 10c335efe619e..92075419d29cf 100644
>> --- a/Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.yaml
>> +++ b/Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.yaml
>> @@ -62,6 +62,11 @@ properties:
>>         Required for at least rk3188 and rk3288. On the rk3368 this 
>> should
>>         point to the PMUGRF syscon.
>>
>> +  rockchip,io-domains:
>> +    $ref: /schemas/types.yaml#/definitions/phandle-array
>> +    description:
>> +      Phandles to io domains
>> +
>>     "#address-cells":
>>       enum: [1, 2]
>>
>> @@ -137,7 +142,13 @@ additionalProperties:
>>               - description:
>>                   The phandle of a node contains the generic pinconfig 
>> options
>>                   to use as described in pinctrl-bindings.txt.
>> -
>> +      rockchip,io-domain-boot-on:
> 
> I don't think "on" is a particularly descriptive or useful property name
> for something that has no "off" state. Furthermore it's no help at all
> if the DT consumer *is* the bootloader that's expected to configure this
> in the first place. IMO it would seem a lot more sensible to have an
> integer (or enum) property which describes the actual value for the
> initial I/O domain setting. Then Linux can choose to assume the presence
> of the property at all implies that the bootloader should have set it up
> already, but also has the option of actively enforcing it as well if we
> want to.
> 

This is actually highly misleading. Whether the bootloader handles IO 
domains for pinctrl or not absolutely doesn't matter to the kernel since 
the kernel is required to handle IO domain for pinctrl as well. They're 
not exclusive, they are not complementary.

The point is that the voltage of the IO domain **can** change at 
runtime, at any point in time. We could theoretically have the 
bootloader require the regulator to be running at 1.8V and then the 
kernel at 3.3V, both should work (can't think of anything that would 
work like that but why not, the kernel IO domain driver is supposed to 
handle that within the kernel runtime even).

The issue here is that we want to avoid a cyclic dependency, where the 
pinctrl is needed for the IO domain regulator that we're trying to add 
as a dependency of the same pinctrl. There needs to be either some smart 
detection or a property to specify that the IO domain dependency needs 
to be ignored. This seems unfortunately to be for working around how 
Linux handles dependencies between devices and doesn't allow cyclic 
dependencies. At the same time, I do not know if there's anyway to not 
work around it?

>> +        type: boolean
>> +        description:
>> +          If true assume that the io domain needed for this pin group 
>> has been
>> +          configured correctly by the bootloader. This is needed to 
>> break cyclic
>> +          dependencies introduced when a io domain needs a regulator 
>> that can be
>> +          accessed through pins configured here.
> 
> This is describing a Linux implementation detail, not the binding
> itself. There's no technical reason a DT consumer couldn't already
> figure this much out from the existing topology (by observing that the
> pinctrl consumer is a grandparent of the I/O domain's supply).
> 

I am guessing you're suggesting to have some complex handling in the 
driver to detect those cyclic dependencies and ignore the IO domain 
dependency for the pinctrl pins where this happens?

This can actually be quite difficult to detect reliably:
We need to go through the phandle in pinctrl to the IO domain DT node, 
then check all phandles there to other DT node (likely regulators), then 
we need to look into the pinctrl-0 (actually, the one for "default" 
maybe, but what about the other states of pinctrl?) phandles and then 
parse the pinctrt DT nodes to see if they're pointing to the same DT 
node as the one we're trying to use. Here, we also do not know if the 
regulator DT node has other dependencies that needs to be accounted for.

I haven't put too much thoughts into it so maybe it's easier/harder than 
what I'm saying here (or maybe I'm completely off too...).

One of the issues we're having here too is that we lose granularity. 
There are multiple domains inside an IO domain device and here we make 
the whole pinctrl device depend on all domains from one IO domain device 
(there can be multiple ones) while it is factually (on the HW level) 
only dependent on one domain. Considering (if I remember correctly) 
Heiko highly suggested we think about adding child nodes to the IO 
domain devices to have a DT node per domain in the IO domain device, how 
would this work with the suggested DT binding?

Cheers,
Quentin
Quentin Schulz Sept. 6, 2023, 8:27 a.m. UTC | #5
Hi Rob,

On 9/5/23 20:14, Rob Herring wrote:
> On Mon, Sep 04, 2023 at 01:58:15PM +0200, Sascha Hauer wrote:
>> Add rockchip,io-domains property to the Rockchip pinctrl driver. This
>> list of phandles points to the IO domain device(s) the pins of the
>> pinctrl driver are supplied from.
> 
> Is there an actual need for multiple IO devices with multiple pinctrl
> blocks? If not, you don't need a property, just lookup the IO domain
> node by compatible.
> 

Yes. There can be multiple IO domain devices on Rockchip SoCs and we 
typically have only one pinctrl controller. Each pinctrl "pin" (for a 
lack of the appropriate term to use here) belongs to one domain of one 
of the IO domain controller/device.

However what I don't like here is that we do not explicit this link 
between the pinctrl "pin" and the IO domain controller it belongs to 
(even less so on which domain of the IO domain controller, which to be 
fair we do not represent in the DT at the moment except through a 
phandle property in the IO domain controller, c.f. 
https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/power/rockchip-io-domain.yaml#L84).

Cheers,
Quentin
Sascha Hauer Sept. 6, 2023, 10:19 a.m. UTC | #6
On Wed, Sep 06, 2023 at 10:20:26AM +0200, Quentin Schulz wrote:
> Sascha, Robin,
> 
> On 9/5/23 11:03, Robin Murphy wrote:
> > [You don't often get email from robin.murphy@arm.com. Learn why this is
> > important at https://aka.ms/LearnAboutSenderIdentification ]
> > 
> > > +        type: boolean
> > > +        description:
> > > +          If true assume that the io domain needed for this pin
> > > group has been
> > > +          configured correctly by the bootloader. This is needed to
> > > break cyclic
> > > +          dependencies introduced when a io domain needs a
> > > regulator that can be
> > > +          accessed through pins configured here.
> > 
> > This is describing a Linux implementation detail, not the binding
> > itself. There's no technical reason a DT consumer couldn't already
> > figure this much out from the existing topology (by observing that the
> > pinctrl consumer is a grandparent of the I/O domain's supply).
> > 
> 
> I am guessing you're suggesting to have some complex handling in the driver
> to detect those cyclic dependencies and ignore the IO domain dependency for
> the pinctrl pins where this happens?

I haven't read this as a suggestion, but only as an argument to make it
clear that I should describe the binding rather than anticipating
how it should be used.

I may have misunderstood it though.

> One of the issues we're having here too is that we lose granularity. There
> are multiple domains inside an IO domain device and here we make the whole
> pinctrl device depend on all domains from one IO domain device (there can be
> multiple ones) while it is factually (on the HW level) only dependent on one
> domain. Considering (if I remember correctly) Heiko highly suggested we
> think about adding child nodes to the IO domain devices to have a DT node
> per domain in the IO domain device, how would this work with the suggested
> DT binding?

I started implementing that. I have moved the IO domains into subnodes
of the IO domain controller and started adding phandles from the pin
groups in rk3568-pinctrl.dtsi to the corresponding IO domains. After a
couple of hours I had phandles for around a quarter of the existing
groups of only one SoC, so doing this for all SoCs would really be a
cumbersome job.

In the end I realized this doesn't solve any problem. Also adding the
properties I suggested doesn't prevent us from adding the more specific
dependencies from the pins to their actual IO domains later.

Sascha
Robin Murphy Sept. 7, 2023, 4:35 p.m. UTC | #7
On 2023-09-06 08:21, Sascha Hauer wrote:
> On Tue, Sep 05, 2023 at 10:03:20AM +0100, Robin Murphy wrote:
>> On 2023-09-04 12:58, Sascha Hauer wrote:
>>> Add rockchip,io-domains property to the Rockchip pinctrl driver. This
>>> list of phandles points to the IO domain device(s) the pins of the
>>> pinctrl driver are supplied from.
>>>
>>> Also a rockchip,io-domain-boot-on property is added to pin groups
>>> which can be used for pin groups which themselves are needed to access
>>> the regulators an IO domain is driven from.
>>>
>>> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>>> ---
>>>    .../bindings/pinctrl/rockchip,pinctrl.yaml          | 13 ++++++++++++-
>>>    1 file changed, 12 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.yaml
>>> index 10c335efe619e..92075419d29cf 100644
>>> --- a/Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.yaml
>>> +++ b/Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.yaml
>>> @@ -62,6 +62,11 @@ properties:
>>>          Required for at least rk3188 and rk3288. On the rk3368 this should
>>>          point to the PMUGRF syscon.
>>> +  rockchip,io-domains:
>>> +    $ref: /schemas/types.yaml#/definitions/phandle-array
>>> +    description:
>>> +      Phandles to io domains
>>> +
>>>      "#address-cells":
>>>        enum: [1, 2]
>>> @@ -137,7 +142,13 @@ additionalProperties:
>>>                - description:
>>>                    The phandle of a node contains the generic pinconfig options
>>>                    to use as described in pinctrl-bindings.txt.
>>> -
>>> +      rockchip,io-domain-boot-on:
>>
>> I don't think "on" is a particularly descriptive or useful property name for
>> something that has no "off" state.
> 
> In fact it has an "off" state. A IO Domain can be disabled in the SoC
> registers

Oh, is that a thing on newer SoCs? At least in the RK3399 TRM the only 
I/O-domain-related control I can find is the 1.8V/3.0V logic level 
threshold in GRF_IO_VSEL (plus the one outlier in PMUGRF_SOC_CON0).

> and also the corresponding regulator can be disabled.

...which is clearly a property of the regulator, not of its consumers ;)

However it's also not a meaningful state in this context anyway, since 
if the supply was actually off, and thus we were unable to communicate 
with the PMIC to turn it on... oh dear.

Cheers,
Robin.

>> Furthermore it's no help at all if the DT
>> consumer *is* the bootloader that's expected to configure this in the first
>> place. IMO it would seem a lot more sensible to have an integer (or enum)
>> property which describes the actual value for the initial I/O domain
>> setting.
> 
> I agree though that a particular setting instead of a boolean is better
> and could help the bootloader.
> 
>> Then Linux can choose to assume the presence of the property at all
>> implies that the bootloader should have set it up already, but also has the
>> option of actively enforcing it as well if we want to.
> 
> Ok.
> 
> Thanks,
>   Sascha
>
Robin Murphy Sept. 7, 2023, 4:47 p.m. UTC | #8
On 06/09/2023 11:19 am, Sascha Hauer wrote:
> On Wed, Sep 06, 2023 at 10:20:26AM +0200, Quentin Schulz wrote:
>> Sascha, Robin,
>>
>> On 9/5/23 11:03, Robin Murphy wrote:
>>> [You don't often get email from robin.murphy@arm.com. Learn why this is
>>> important at https://aka.ms/LearnAboutSenderIdentification ]
>>>
>>>> +ᅵᅵᅵᅵᅵᅵᅵ type: boolean
>>>> +ᅵᅵᅵᅵᅵᅵᅵ description:
>>>> +ᅵᅵᅵᅵᅵᅵᅵᅵᅵ If true assume that the io domain needed for this pin
>>>> group has been
>>>> +ᅵᅵᅵᅵᅵᅵᅵᅵᅵ configured correctly by the bootloader. This is needed to
>>>> break cyclic
>>>> +ᅵᅵᅵᅵᅵᅵᅵᅵᅵ dependencies introduced when a io domain needs a
>>>> regulator that can be
>>>> +ᅵᅵᅵᅵᅵᅵᅵᅵᅵ accessed through pins configured here.
>>>
>>> This is describing a Linux implementation detail, not the binding
>>> itself. There's no technical reason a DT consumer couldn't already
>>> figure this much out from the existing topology (by observing that the
>>> pinctrl consumer is a grandparent of the I/O domain's supply).
>>>
>>
>> I am guessing you're suggesting to have some complex handling in the driver
>> to detect those cyclic dependencies and ignore the IO domain dependency for
>> the pinctrl pins where this happens?
> 
> I haven't read this as a suggestion, but only as an argument to make it
> clear that I should describe the binding rather than anticipating
> how it should be used.
> 
> I may have misunderstood it though.

Indeed it was more about the definition itself - an extra property isn't 
*needed* to break the cycle since the cycle is already fully described 
in DT, so anyone who can parse parents and phandles already has 
sufficient information to detect it and break it at any point they 
choose. However, as mentioned subsequently, breaking the cycle alone 
isn't enough to guarantee that things will actually work in general.

AFAICS what we fundamentally need to know is the initial voltage of the 
supply regulator, to be able to short-circuit requiring the I/O domain 
in order to query it from the regulator itself, and instead just 
initialise the I/O domain directly. However that would still represent a 
bunch of fiddly extra DT parsing, so for practical purposes it seems 
reasonable to then short-cut that into directly describing the initial 
setting of the I/O domain on the node itself, such that the consumer of 
the binding can easily handle it all in a self-contained manner.

Cheers,
Robin

>> One of the issues we're having here too is that we lose granularity. There
>> are multiple domains inside an IO domain device and here we make the whole
>> pinctrl device depend on all domains from one IO domain device (there can be
>> multiple ones) while it is factually (on the HW level) only dependent on one
>> domain. Considering (if I remember correctly) Heiko highly suggested we
>> think about adding child nodes to the IO domain devices to have a DT node
>> per domain in the IO domain device, how would this work with the suggested
>> DT binding?
> 
> I started implementing that. I have moved the IO domains into subnodes
> of the IO domain controller and started adding phandles from the pin
> groups in rk3568-pinctrl.dtsi to the corresponding IO domains. After a
> couple of hours I had phandles for around a quarter of the existing
> groups of only one SoC, so doing this for all SoCs would really be a
> cumbersome job.
> 
> In the end I realized this doesn't solve any problem. Also adding the
> properties I suggested doesn't prevent us from adding the more specific
> dependencies from the pins to their actual IO domains later.
> 
> Sascha
>
Sascha Hauer Sept. 8, 2023, 7:20 a.m. UTC | #9
On Thu, Sep 07, 2023 at 05:35:26PM +0100, Robin Murphy wrote:
> On 2023-09-06 08:21, Sascha Hauer wrote:
> > On Tue, Sep 05, 2023 at 10:03:20AM +0100, Robin Murphy wrote:
> > > On 2023-09-04 12:58, Sascha Hauer wrote:
> > > > Add rockchip,io-domains property to the Rockchip pinctrl driver. This
> > > > list of phandles points to the IO domain device(s) the pins of the
> > > > pinctrl driver are supplied from.
> > > > 
> > > > Also a rockchip,io-domain-boot-on property is added to pin groups
> > > > which can be used for pin groups which themselves are needed to access
> > > > the regulators an IO domain is driven from.
> > > > 
> > > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > > > ---
> > > >    .../bindings/pinctrl/rockchip,pinctrl.yaml          | 13 ++++++++++++-
> > > >    1 file changed, 12 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.yaml
> > > > index 10c335efe619e..92075419d29cf 100644
> > > > --- a/Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.yaml
> > > > +++ b/Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.yaml
> > > > @@ -62,6 +62,11 @@ properties:
> > > >          Required for at least rk3188 and rk3288. On the rk3368 this should
> > > >          point to the PMUGRF syscon.
> > > > +  rockchip,io-domains:
> > > > +    $ref: /schemas/types.yaml#/definitions/phandle-array
> > > > +    description:
> > > > +      Phandles to io domains
> > > > +
> > > >      "#address-cells":
> > > >        enum: [1, 2]
> > > > @@ -137,7 +142,13 @@ additionalProperties:
> > > >                - description:
> > > >                    The phandle of a node contains the generic pinconfig options
> > > >                    to use as described in pinctrl-bindings.txt.
> > > > -
> > > > +      rockchip,io-domain-boot-on:
> > > 
> > > I don't think "on" is a particularly descriptive or useful property name for
> > > something that has no "off" state.
> > 
> > In fact it has an "off" state. A IO Domain can be disabled in the SoC
> > registers
> 
> Oh, is that a thing on newer SoCs? At least in the RK3399 TRM the only
> I/O-domain-related control I can find is the 1.8V/3.0V logic level threshold
> in GRF_IO_VSEL (plus the one outlier in PMUGRF_SOC_CON0).

I didn't realize that it's new, the RK3568 is the first Rockchip SoC I
work on, but yes, on RK3568 we have three bits per domain. One bit is to
enable 1.8V, one to enable 2.5V and one for 3.3V. I would assume that
clearing all bits means disable, and whatever strange things may happen
when multiple bits are set...

Sascha
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.yaml
index 10c335efe619e..92075419d29cf 100644
--- a/Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.yaml
@@ -62,6 +62,11 @@  properties:
       Required for at least rk3188 and rk3288. On the rk3368 this should
       point to the PMUGRF syscon.
 
+  rockchip,io-domains:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    description:
+      Phandles to io domains
+
   "#address-cells":
     enum: [1, 2]
 
@@ -137,7 +142,13 @@  additionalProperties:
             - description:
                 The phandle of a node contains the generic pinconfig options
                 to use as described in pinctrl-bindings.txt.
-
+      rockchip,io-domain-boot-on:
+        type: boolean
+        description:
+          If true assume that the io domain needed for this pin group has been
+          configured correctly by the bootloader. This is needed to break cyclic
+          dependencies introduced when a io domain needs a regulator that can be
+          accessed through pins configured here.
 examples:
   - |
     #include <dt-bindings/interrupt-controller/arm-gic.h>