[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <b26891a3-f784-a188-e7ef-422dda9ef771@linaro.org>
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
Powered by blists - more mailing lists