linux-kernel - Re: [PATCH v3 7/9] arm64: dts: qcom: sm6115: Add basic soc dtsi

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 11 Sep 2022 15:03:58 +0300
From:   Iskren Chernev <iskren.chernev@...il.com>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Rob Herring <robh+dt@...nel.org>
Cc:     phone-devel@...r.kernel.org, ~postmarketos/upstreaming@...ts.sr.ht,
        linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
        Andy Gross <agross@...nel.org>,
        Konrad Dybcio <konrad.dybcio@...ainline.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 7/9] arm64: dts: qcom: sm6115: Add basic soc dtsi



On 9/11/22 13:26, Krzysztof Kozlowski wrote:
> On 11/09/2022 12:22, Iskren Chernev wrote:
>>         basic-state { // this matches the first state in oneOf
>>             pins: "gpio1";
>>             funciton: "normal";
>>         };
>>
>>         nested-state {
>>             some-pins { // this matches the second state in oneOf
>>                 pins: "gpio1";
>>                 funciton: "normal";
>>             };
>>             other-pins {
>>                 pins: "gpio2"
>>                 funciton: "normal";
>>             };
>>         }
>>
>>         // but also, matching second state in oneOf
>>         nested-basic-state {
>>             pinconf {
>>                 pins: "gpio1";
>>                 funciton: "normal";
>>             };
>>         };
>>     };
>>
>> So I'm saying, we should either choose basic-state and nested-state, in which
>> case we don't need the "^pinconf$" variant, or we can have nested-state and
>> nested-basic-state, in which case we don't need the 1st case of the oneOf.
>
> Ah, I get it.
>
>>
>> Otherwise people have to choose between basic-state and nested-basic-state,
>> which are equivalent in semantics.
>
> Yeah, I can drop pinconf. I put it in the PMIC because it was used, but
> I don't find it for TLMM pinctrl nodes.

Frankly I'm not sure which is better, to drop pinconf, or to use it (and drop
basic case). You probably have more experience and taste regarding that.
Another thing is that you normally specify one pin at a time in the nested
case, so having -pins is a bit confusing. Maybe it should allow -pin and -pins.

I understand that you technically can't change existing bindings (because
they're immutable), but at least for the future you can pick something that
will stand, so I wouldn't be too concerned about existing ones :)

>>
>> On a tangent -- why specifying the .* regex of pinctrl subnodes has effect on
>> pinctrl references in other nodes. I.e I don't understand why this fix fixes
>> the issue (but it does).
>
> Because it works on DTB and finds linux,phandle. This might be some bug
> in dtschema, but anyway better to have a bit stricter patterns in bindings.

I see, the phandle node appears only when you use a reference, not when you
define it... there should be a way to handle that more precisely. phandle is
a very special case. Also `additional-properties` is smart enough to allow it.

>
> Best regards,
> Krzysztof