arm64: dts: qcom: sm8350-sagami: Rectify GPIO keys

Message ID 20221210133850.3664-1-konrad.dybcio@linaro.org
State New
Headers
Series arm64: dts: qcom: sm8350-sagami: Rectify GPIO keys |

Commit Message

Konrad Dybcio Dec. 10, 2022, 1:38 p.m. UTC
  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@linaro.org>
---
 .../qcom/sm8350-sony-xperia-sagami-pdx214.dts | 24 ++++++++++
 .../dts/qcom/sm8350-sony-xperia-sagami.dtsi   | 47 ++++++++++++++++++-
 2 files changed, 70 insertions(+), 1 deletion(-)
  

Comments

Marijn Suijten Dec. 13, 2022, 9:03 a.m. UTC | #1
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@linaro.org>

Tested-by: Marijn Suijten <marijn.suijten@somainline.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!

>  };
>  
>  &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@e1000000 {
>  	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
>
  
Konrad Dybcio Dec. 13, 2022, 10:23 a.m. UTC | #2
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@linaro.org>
> 
> Tested-by: Marijn Suijten <marijn.suijten@somainline.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@e1000000 {
>>  	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
>>
  
Marijn Suijten Dec. 13, 2022, 11:11 a.m. UTC | #3
On 2022-12-13 11:23:12, Konrad Dybcio wrote:
> 
> 
> 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@linaro.org>
> > 
> > Tested-by: Marijn Suijten <marijn.suijten@somainline.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

I have the device in my hand right now and, unless I take an angle
grinder to it, it has this button.

How else did you expect me to successfully test it?

Maybe you are confused with Xperia 1 IV, which does _not_ have the
button.  Nor does Xperia 1 II, for that matter.

- Marijn

> - 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
  
Konrad Dybcio Dec. 13, 2022, 11:20 a.m. UTC | #4
On 13.12.2022 12:11, Marijn Suijten wrote:
> On 2022-12-13 11:23:12, Konrad Dybcio wrote:
>>
>>
>> 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@linaro.org>
>>>
>>> Tested-by: Marijn Suijten <marijn.suijten@somainline.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
> 
> I have the device in my hand right now and, unless I take an angle
> grinder to it, it has this button.
> 
> How else did you expect me to successfully test it?
> 
> Maybe you are confused with Xperia 1 IV, which does _not_ have the
> button.  Nor does Xperia 1 II, for that matter.
Lol yes I did, will send a v2..

Konrad
> 
> - Marijn
> 
>> - 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
  

Patch

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;
+		};
+	};
 };
 
 &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;
+	};
+};
+
 &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@e1000000 {
 	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 {