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]

Message-ID: <20221218101841.mjsmnwtfzbmazfys@SoMainline.org>
Date:   Sun, 18 Dec 2022 11:18:41 +0100
From:   Marijn Suijten <marijn.suijten@...ainline.org>
To:     Konrad Dybcio <konrad.dybcio@...aro.org>
Cc:     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 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?

(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...)

- Marijn