linux-kernel - Re: [PATCH] arm64: dts: qcom: sm8350-sagami: Rectify GPIO keys

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: <0c6bad50-500c-00b0-30c3-853b0c0a6d5e@linaro.org>
Date:   Tue, 13 Dec 2022 11:23:12 +0100
From:   Konrad Dybcio <konrad.dybcio@...aro.org>
To:     Marijn Suijten <marijn.suijten@...ainline.org>
Cc:     linux-arm-msm@...r.kernel.org, andersson@...nel.org,
        agross@...nel.org, krzysztof.kozlowski@...aro.org,
        patches@...aro.org, Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] arm64: dts: qcom: sm8350-sagami: Rectify GPIO keys



On 13.12.2022 10:03, Marijn Suijten wrote:
> On 2022-12-10 14:38:50, Konrad Dybcio wrote:
>> With enough pins set properly, the hardware buttons now also work
>> like a charm.
>>
>> Fixes: c2721b0c23d9 ("arm64: dts: qcom: Add support for Xperia 1 III / 5 III")
>> Signed-off-by: Konrad Dybcio <konrad.dybcio@...aro.org>
> 
> Tested-by: Marijn Suijten <marijn.suijten@...ainline.org> # On Xperia 5 III
> 
> However I also tested this on Xperia 1 III, and...
> 
>> ---
>>  .../qcom/sm8350-sony-xperia-sagami-pdx214.dts | 24 ++++++++++
>>  .../dts/qcom/sm8350-sony-xperia-sagami.dtsi   | 47 ++++++++++++++++++-
>>  2 files changed, 70 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami-pdx214.dts b/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami-pdx214.dts
>> index e6824c8c2774..6fa830bdc6bd 100644
>> --- a/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami-pdx214.dts
>> +++ b/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami-pdx214.dts
>> @@ -10,6 +10,20 @@
>>  / {
>>  	model = "Sony Xperia 5 III";
>>  	compatible = "sony,pdx214-generic", "qcom,sm8350";
>> +
>> +	gpio-keys {
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&focus_n &snapshot_n &vol_down_n &g_assist_n>;
>> +
>> +		key-google-assist {
>> +			label = "Google Assistant Key";
>> +			gpios = <&pm8350_gpios 9 GPIO_ACTIVE_LOW>;
>> +			linux,code = <KEY_LEFTMETA>;
>> +			debounce-interval = <15>;
>> +			linux,can-disable;
>> +			gpio-key,wakeup;
>> +		};
>> +	};
> 
> ... please move this to board DTS.  Xperia 1 III would also like to have
> it mapped.  Downstream DT indicates, and my local testing confirms, that
> it is identical to Xperia 5 III (i.e. common to the Sagami board).
> 
> The other buttons work great on both devices!
While I'd agree that having it like that would be nice for completeness,
it's:

- not necessary, as the button is not physically there, so the user will
never come into contact with it

- it will leak power, the plan is to park all unused pins after we get
"good enough" support for our devices (not now, so as not to mess with
ourselves in the dev process)

Konrad
> 
>>  };
>>  
>>  &framebuffer {
>> @@ -18,6 +32,16 @@ &framebuffer {
>>  	stride = <(1080 * 4)>;
>>  };
>>  
>> +&pm8350_gpios {
>> +	g_assist_n: g-assist-n-state {
>> +		pins = "gpio9";
>> +		function = "normal";
>> +		power-source = <1>;
>> +		bias-pull-up;
>> +		input-enable;
>> +	};
>> +};
>> +
> 
> (And this too, of course)
> 
> - Marijn
> 
>>  &pm8350b_gpios {
>>  	gpio-line-names = "NC", /* GPIO_1 */
>>  			  "NC",
>> diff --git a/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi b/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi
>> index d73e1f3fa501..557b4538a031 100644
>> --- a/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami.dtsi
>> @@ -49,7 +49,26 @@ framebuffer: framebuffer@...00000 {
>>  	gpio-keys {
>>  		compatible = "gpio-keys";
>>  
>> -		/* For reasons still unknown, GAssist key and Camera Focus/Shutter don't work.. */
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&focus_n &snapshot_n &vol_down_n>;
>> +
>> +		key-camera-focus {
>> +			label = "Camera Focus";
>> +			linux,code = <KEY_CAMERA_FOCUS>;
>> +			gpios = <&pm8350b_gpios 8 GPIO_ACTIVE_LOW>;
>> +			debounce-interval = <15>;
>> +			linux,can-disable;
>> +			gpio-key,wakeup;
>> +		};
>> +
>> +		key-camera-snapshot {
>> +			label = "Camera Snapshot";
>> +			linux,code = <KEY_CAMERA>;
>> +			gpios = <&pm8350b_gpios 5 GPIO_ACTIVE_LOW>;
>> +			debounce-interval = <15>;
>> +			linux,can-disable;
>> +			gpio-key,wakeup;
>> +		};
>>  
>>  		key-vol-down {
>>  			label = "Volume Down";
>> @@ -547,11 +566,37 @@ &pm8350_gpios {
>>  			  "PM8350_OPTION"; /* GPIO_10 */
>>  };
>>  
>> +&pm8350b_gpios {
>> +	snapshot_n: snapshot-n-state {
>> +		pins = "gpio5";
>> +		function = "normal";
>> +		power-source = <0>;
>> +		bias-pull-up;
>> +		input-enable;
>> +	};
>> +
>> +	focus_n: focus-n-state {
>> +		pins = "gpio8";
>> +		function = "normal";
>> +		power-source = <0>;
>> +		input-enable;
>> +		bias-pull-up;
>> +	};
>> +};
>> +
>>  &pmk8350_gpios {
>>  	gpio-line-names = "NC", /* GPIO_1 */
>>  			  "NC",
>>  			  "VOL_DOWN_N",
>>  			  "PMK8350_OPTION";
>> +
>> +	vol_down_n: vol-down-n-state {
>> +		pins = "gpio3";
>> +		function = "normal";
>> +		power-source = <0>;
>> +		bias-pull-up;
>> +		input-enable;
>> +	};
>>  };
>>  
>>  &pmk8350_rtc {
>> -- 
>> 2.38.1
>>