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

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

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
>
  

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

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
  

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