[PATCH] arm64: dts: qcom: sdm630-nile: add pinctrl for camera key

[PATCH] arm64: dts: qcom: sdm630-nile: add pinctrl for camera key

[Sebastian Raase]

Add pinctrl configuration for gpio-keys. Without this,
camera button half-presses are not detected.

Tested on discovery and pioneer.

Fixes: e781633b6067 ("arm64: dts: qcom: Add support for Sony Xperia XA2/Plus/Ultra (Nile platform)")
Signed-off-by: Sebastian Raase <linux@sraa.de>
---
 arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi b/arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi
index 87d0293c728d..5eedca6f288f 100644
--- a/arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi
@@ -90,6 +90,8 @@ cam_vana_rear_vreg: cam-vana-rear-regulator {
 
 	gpio-keys {
 		compatible = "gpio-keys";
+		pinctrl-names = "default";
+		pinctrl-0 = <&gpio_keys_default>;
 
 		key-camera-focus {
 			label = "Camera Focus";
@@ -635,6 +637,13 @@ ts_lcd_id_active: ts-lcd-id-active-state {
 		bias-disable;
 	};
 
+	gpio_keys_default: gpio-keys-default {
+		pins = "gpio64", "gpio113";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-pull-up;
+	};
+
 	imx300_vana_default: imx300-vana-default-state {
 		pins = "gpio50";
 		function = "gpio";
-- 
2.42.0

Re: [PATCH] arm64: dts: qcom: sdm630-nile: add pinctrl for camera key

[Krzysztof Kozlowski]

On 14/03/2024 21:00, Sebastian Raase wrote:
> Add pinctrl configuration for gpio-keys. Without this,
> camera button half-presses are not detected.
> 
> Tested on discovery and pioneer.
> 
> Fixes: e781633b6067 ("arm64: dts: qcom: Add support for Sony Xperia XA2/Plus/Ultra (Nile platform)")
> Signed-off-by: Sebastian Raase <linux@sraa.de>
> ---
>  arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi b/arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi
> index 87d0293c728d..5eedca6f288f 100644
> --- a/arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi
> @@ -90,6 +90,8 @@ cam_vana_rear_vreg: cam-vana-rear-regulator {
>  
>  	gpio-keys {
>  		compatible = "gpio-keys";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&gpio_keys_default>;

A nit: Please reverse the order: pinctrl-0 then names
>  
>  		key-camera-focus {
>  			label = "Camera Focus";
> @@ -635,6 +637,13 @@ ts_lcd_id_active: ts-lcd-id-active-state {
>  		bias-disable;
>  	};
>  
> +	gpio_keys_default: gpio-keys-default {

Looks missing suffix (state) and test, so:

It does not look like you tested the DTS against bindings. Please run
`make dtbs_check W=1` (see
Documentation/devicetree/bindings/writing-schema.rst or
https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
for instructions).

Best regards,
Krzysztof

Re: [PATCH] arm64: dts: qcom: sdm630-nile: add pinctrl for camera key

[Sebastian Raase]

Hi Krzysztof,

On Thu, 14 Mar 2024 21:37:30 +0100
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:

> On 14/03/2024 21:00, Sebastian Raase wrote:
> > Add pinctrl configuration for gpio-keys. Without this,
> > camera button half-presses are not detected.
> > 
> > Tested on discovery and pioneer.
> > 
> > Fixes: e781633b6067 ("arm64: dts: qcom: Add support for Sony Xperia XA2/Plus/Ultra (Nile platform)")
> > Signed-off-by: Sebastian Raase <linux@sraa.de>
> > ---
> >  arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi b/arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi
> > index 87d0293c728d..5eedca6f288f 100644
> > --- a/arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi
> > @@ -90,6 +90,8 @@ cam_vana_rear_vreg: cam-vana-rear-regulator {
> >  
> >  	gpio-keys {
> >  		compatible = "gpio-keys";
> > +		pinctrl-names = "default";
> > +		pinctrl-0 = <&gpio_keys_default>;  
> 
> A nit: Please reverse the order: pinctrl-0 then names

All existing pinctrl definitions use pinctrl-names first, so I followed the existing file. Is this still okay?

> >  
> >  		key-camera-focus {
> >  			label = "Camera Focus";
> > @@ -635,6 +637,13 @@ ts_lcd_id_active: ts-lcd-id-active-state {
> >  		bias-disable;
> >  	};
> >  
> > +	gpio_keys_default: gpio-keys-default {  
> 
> Looks missing suffix (state) and test, so:
> 
> It does not look like you tested the DTS against bindings. Please run
> `make dtbs_check W=1` (see
> Documentation/devicetree/bindings/writing-schema.rst or
> https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
> for instructions).

I did add the suffix and ran the test, and the warning is gone now. Sent an updated patch.

> Best regards,
> Krzysztof

Best Regards,
Sebastian

Re: [PATCH] arm64: dts: qcom: sdm630-nile: add pinctrl for camera key

[Krzysztof Kozlowski]

On 15/03/2024 00:23, Sebastian Raase wrote:
>>>  	gpio-keys {
>>>  		compatible = "gpio-keys";
>>> +		pinctrl-names = "default";
>>> +		pinctrl-0 = <&gpio_keys_default>;  
>>
>> A nit: Please reverse the order: pinctrl-0 then names
> 
> All existing pinctrl definitions use pinctrl-names first, so I followed the existing file. Is this still okay?

No, not all. If you are sending v2, why you cannot fix this in new code?

Best regards,
Krzysztof

[PATCH v2] arm64: dts: qcom: sdm630-nile: add pinctrl for camera key

[Sebastian Raase]

Add pinctrl configuration for gpio-keys. Without this,
camera button half-presses are not detected.

Tested on discovery and pioneer.

Fixes: e781633b6067 ("arm64: dts: qcom: Add support for Sony Xperia XA2/Plus/Ultra (Nile platform)")
Signed-off-by: Sebastian Raase <linux@sraa.de>
---
 arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi b/arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi
index 87d0293c728d..823c21d5ee59 100644
--- a/arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi
@@ -90,6 +90,8 @@ cam_vana_rear_vreg: cam-vana-rear-regulator {
 
 	gpio-keys {
 		compatible = "gpio-keys";
+		pinctrl-names = "default";
+		pinctrl-0 = <&gpio_keys_default>;
 
 		key-camera-focus {
 			label = "Camera Focus";
@@ -635,6 +637,13 @@ ts_lcd_id_active: ts-lcd-id-active-state {
 		bias-disable;
 	};
 
+	gpio_keys_default: gpio-keys-default-state {
+		pins = "gpio64", "gpio113";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-pull-up;
+	};
+
 	imx300_vana_default: imx300-vana-default-state {
 		pins = "gpio50";
 		function = "gpio";
-- 
2.42.0

Re: [PATCH v2] arm64: dts: qcom: sdm630-nile: add pinctrl for camera key

[Konrad Dybcio]

On 3/15/24 00:20, Sebastian Raase wrote:
> Add pinctrl configuration for gpio-keys. Without this,
> camera button half-presses are not detected.
> 
> Tested on discovery and pioneer.
> 
> Fixes: e781633b6067 ("arm64: dts: qcom: Add support for Sony Xperia XA2/Plus/Ultra (Nile platform)")
> Signed-off-by: Sebastian Raase <linux@sraa.de>
> ---
>   arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi | 9 +++++++++
>   1 file changed, 9 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi b/arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi
> index 87d0293c728d..823c21d5ee59 100644
> --- a/arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi
> @@ -90,6 +90,8 @@ cam_vana_rear_vreg: cam-vana-rear-regulator {
>   
>   	gpio-keys {
>   		compatible = "gpio-keys";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&gpio_keys_default>;

It's fine to keep the "non-preferred" order, I'll probably send some
changes to nile and fix up the style while at it in the near future.

Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Konrad

P.S.
For your next submissions, please send the follow-up revisions in
new mail threads

Re: [PATCH v2] arm64: dts: qcom: sdm630-nile: add pinctrl for camera key

[Krzysztof Kozlowski]

On 15/03/2024 00:49, Konrad Dybcio wrote:
> 
> 
> On 3/15/24 00:20, Sebastian Raase wrote:
>> Add pinctrl configuration for gpio-keys. Without this,
>> camera button half-presses are not detected.
>>
>> Tested on discovery and pioneer.
>>
>> Fixes: e781633b6067 ("arm64: dts: qcom: Add support for Sony Xperia XA2/Plus/Ultra (Nile platform)")
>> Signed-off-by: Sebastian Raase <linux@sraa.de>
>> ---
>>   arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi b/arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi
>> index 87d0293c728d..823c21d5ee59 100644
>> --- a/arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi
>> @@ -90,6 +90,8 @@ cam_vana_rear_vreg: cam-vana-rear-regulator {
>>   
>>   	gpio-keys {
>>   		compatible = "gpio-keys";
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&gpio_keys_default>;
> 
> It's fine to keep the "non-preferred" order, I'll probably send some
> changes to nile and fix up the style while at it in the near future.

Sebastian,
Although it is also preferred to implement feedback the reviewer is
asking. I don't understand why you insisted to keep the other order.

Best regards,
Krzysztof

Re: [PATCH v2] arm64: dts: qcom: sdm630-nile: add pinctrl for camera key

[Sebastian Raase]

Hi Krzysztof,

> On 15/03/2024 00:49, Konrad Dybcio wrote:
> > On 3/15/24 00:20, Sebastian Raase wrote:  
> >> Add pinctrl configuration for gpio-keys. Without this,
> >> camera button half-presses are not detected.
> >>
> >> Tested on discovery and pioneer.
> >>
> >> Fixes: e781633b6067 ("arm64: dts: qcom: Add support for Sony Xperia XA2/Plus/Ultra (Nile platform)")
> >> Signed-off-by: Sebastian Raase <linux@sraa.de>
> >> ---
> >>   arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi | 9 +++++++++
> >>   1 file changed, 9 insertions(+)
> >>
> >> diff --git a/arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi b/arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi
> >> index 87d0293c728d..823c21d5ee59 100644
> >> --- a/arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi
> >> +++ b/arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi
> >> @@ -90,6 +90,8 @@ cam_vana_rear_vreg: cam-vana-rear-regulator {
> >>   
> >>   	gpio-keys {
> >>   		compatible = "gpio-keys";
> >> +		pinctrl-names = "default";
> >> +		pinctrl-0 = <&gpio_keys_default>;  
> > 
> > It's fine to keep the "non-preferred" order, I'll probably send some
> > changes to nile and fix up the style while at it in the near future.  
> 
> Sebastian,
> Although it is also preferred to implement feedback the reviewer is
> asking. I don't understand why you insisted to keep the other order.

I simply followed the existing style and did not know any better.
Since you called it a "nit", I wrongly assumed that was acceptable.

Fixed in v3.

> Best regards,
> Krzysztof

Best Regards,
Sebastian