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

PATCH Superseded Sebastian Raase 4 participants 8 messages Mar 14, 2024 – Mar 15, 2024

Subsystems (3)

Device tree bindings Arm Qualcomm mailing list Arm Qualcomm support

lore

Share & tools

lore

Krzysztof Kozlowski

2024-03-14 20:37 UTC #1 lore raw

In reply to Sebastian Raase

Quoted 16 lines · 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 <[email protected]>
---
 arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)
arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi +0 -0
index 87d0293c728d..5eedca6f288f 100644

arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi cam_vana_rear_vreg: cam-vana-rear-regulator { · line 94

Quoted 5 lines

arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi +2 -0

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

arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi ts_lcd_id_active: ts-lcd-id-active-state { · line 640

Quoted 8 lines

arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi +1 -0

 		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

Sebastian Raase

author

2024-03-14 20:40 UTC #0 lore raw

PATCH 1/1 arm64: dts: qcom: sdm630-nile: add pinctrl for camera key

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 <[email protected]>

---
 arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)

arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi +9 -0

index 87d0293c728d..5eedca6f288f 100644
@@ -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";

Signature · 1 line

2.42.0

Sebastian Raase

author

2024-03-14 23:20 UTC #4 lore raw

PATCH v2 1/1 arm64: dts: qcom: sdm630-nile: add pinctrl for camera key

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 <[email protected]>

---
 arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)

arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi +9 -0

index 87d0293c728d..823c21d5ee59 100644
@@ -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";

Signature · 1 line

2.42.0

Sebastian Raase

2024-03-14 23:23 UTC #2 lore raw

In reply to Krzysztof Kozlowski

Hi Krzysztof,

Quoted 18 lines · On Thu, 14 Mar 2024 21:37:30 +0100 Krzysztof Kozlowski…

Quoted 17 lines · 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 <[email protected]>
---
 arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)
arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi +0 -0
index 87d0293c728d..5eedca6f288f 100644
@@ -90,6 +90,8 @@ cam_vana_rear_vreg: cam-vana-rear-regulator {
 

Quoted 6 lines

arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi cam_vana_rear_vreg: cam-vana-rear-regulator { · line 94
Quoted 4 lines
arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi +2 -0
 	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?

Quoted 11 lines

arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi ts_lcd_id_active: ts-lcd-id-active-state { · line 640
Quoted 8 lines
arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi +1 -0
 		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:

Quoted 5 lines

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.

Quoted 2 lines

Best regards,
Krzysztof

Best Regards,
Sebastian

Konrad Dybcio

Reviewed-by on v2 1/1

2024-03-14 23:49 UTC #5 lore raw

In reply to Sebastian Raase

Quoted 16 lines · 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 <[email protected]>
---
  arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi | 9 +++++++++
  1 file changed, 9 insertions(+)
arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi +0 -0
index 87d0293c728d..823c21d5ee59 100644

arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi cam_vana_rear_vreg: cam-vana-rear-regulator { · line 94

Quoted 5 lines

arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi +2 -0

@@ -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 <[email protected]>

Konrad

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

Krzysztof Kozlowski

2024-03-15 08:32 UTC #6 lore raw

Quoted 20 lines · Konrad Dybcio

Quoted 17 lines · 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 <[email protected]>
---
  arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi | 9 +++++++++
  1 file changed, 9 insertions(+)
arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi +0 -0
index 87d0293c728d..823c21d5ee59 100644
@@ -90,6 +90,8 @@ cam_vana_rear_vreg: cam-vana-rear-regulator {
 

Quoted 7 lines

arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi cam_vana_rear_vreg: cam-vana-rear-regulator { · line 94
Quoted 4 lines
arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi +2 -0
  	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

Krzysztof Kozlowski

2024-03-15 08:33 UTC #3 lore raw

In reply to Sebastian Raase

Quoted 8 lines · Sebastian Raase

Quoted 6 lines
arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi cam_vana_rear_vreg: cam-vana-rear-regulator { · line 94
Quoted 4 lines
arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi +2 -0
 	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

Sebastian Raase

author

2024-03-15 09:04 UTC #7 lore raw

In reply to Krzysztof Kozlowski

Hi Krzysztof,

Quoted 27 lines · Konrad Dybcio

Quoted 18 lines · Sebastian Raase
Quoted 17 lines
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 <[email protected]>
---
  arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi | 9 +++++++++
  1 file changed, 9 insertions(+)
arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi +0 -0
index 87d0293c728d..823c21d5ee59 100644
@@ -90,6 +90,8 @@ cam_vana_rear_vreg: cam-vana-rear-regulator {
 
Quoted 7 lines
arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi cam_vana_rear_vreg: cam-vana-rear-regulator { · line 94
Quoted 4 lines
arch/arm64/boot/dts/qcom/sdm630-sony-xperia-nile.dtsi +2 -0
  	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.

Quoted 3 lines

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.

Quoted 2 lines

Best regards,
Krzysztof

Best Regards,
Sebastian