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

[Iskren Chernev]

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