lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5165d26f-d5fe-13e9-7940-b73e27b2bea7@arm.com>
Date:   Thu, 7 Sep 2023 17:35:26 +0100
From:   Robin Murphy <robin.murphy@....com>
To:     Sascha Hauer <s.hauer@...gutronix.de>
Cc:     linux-rockchip@...ts.infradead.org,
        Heiko Stuebner <heiko@...ech.de>,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        devicetree@...r.kernel.org, linux-gpio@...r.kernel.org,
        kernel@...gutronix.de,
        Quentin Schulz <quentin.schulz@...obroma-systems.com>,
        Michael Riesch <michael.riesch@...fvision.net>,
        Linus Walleij <linus.walleij@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>
Subject: Re: [PATCH 2/3] dt-bindings: pinctrl: rockchip: Add io domain
 properties

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@...gutronix.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
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ