linux-kernel - Re: [PATCH] arm64: dts: qcom: sm6125-seine: Clean up gpio-keys (volume down)

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]

Date:   Mon, 19 Dec 2022 11:00:10 +0100
From:   Konrad Dybcio <konrad.dybcio@...aro.org>
To:     Marijn Suijten <marijn.suijten@...ainline.org>,
        phone-devel@...r.kernel.org, ~postmarketos/upstreaming@...ts.sr.ht,
        AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@...ainline.org>,
        Konrad Dybcio <konrad.dybcio@...ainline.org>,
        Martin Botka <martin.botka@...ainline.org>,
        Jami Kettunen <jami.kettunen@...ainline.org>,
        Andy Gross <agross@...nel.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] arm64: dts: qcom: sm6125-seine: Clean up gpio-keys
 (volume down)



On 18.12.2022 11:18, Marijn Suijten wrote:
> On 2022-12-17 16:04:17, Konrad Dybcio wrote:
>> On 17.12.2022 11:04, Marijn Suijten wrote:
>>> [..]
>>> @@ -270,6 +270,16 @@ &sdhc_1 {
>>>  
>>>  &tlmm {
>>>  	gpio-reserved-ranges = <22 2>, <28 6>;
>>> +
>>> +	gpio_keys_state: gpio-keys-state {
>>> +		key-volume-down-pins {
>> I see no need for defining a wrapper node.
>> The other changes look good!
> 
> I did the same for sm6350-lena, which we should flatten out then too.
> 
> For these uses I'm not sure when it's clearer/better to use:
> 
>     thing@x {
>         pinctrl-0 = <&thing_state>;
>         ...
>     };
> 
>     thing_state: thing-state {
>         specific-pin {
>             ...
>         };
> 
>         other-specific-pin ...
>         ...
>     };
> 
> Or separate out the pins with their own state and instead use:
> 
>     thing@x {
>         pinctrl-0 = <&specific_pin1_state &specific_pin2_state>;
>         ...
>     };
> 
> If I had to guess the former groups related pins together (as we finally
> do now for SDC...) which should all be toggled at once.  In this
> specific gpio-keys case, irrespective of whether it has one or more
> keys, the pins aren't related apart from representing keys, and should
> thus better be individual pinctrl nodes and individually referenced in
> pinctrl-X.
> 
> Did I sympathize that correctly?
I think so.

> 
> (side-note: the SDC pinctrl groups typically get extended with a
>  card-detect pin in board DTS or in some likely-erroneous cases directly
>  in SoC DTSI.  This may also count as unrelated pins being grouped
>  together only because that is how the hardware/DTS node consumes them,
>  but it is rather concise/readable/convenient though...)
8450 has:

pinctrl-0 = <&sdc2_default_state &sdc2_card_det_n>;

which seems like a sane application of what you described.

Konrad
> 
> - Marijn