Subject: Re: [PATCH] ARM: dts: add touchkey nodes for midas

From: Paul Kocialkowski <[email-masked]>
Date: Wed, 7 Aug 2019 09:09:32 +0200

Hi,

On Tue 06 Aug 19, 18:57, Denis 'GNUtoo' Carikli wrote:
> From: Simon Shields <[email-masked]>
> 
> this patch adds the fixed VTOUCH_3.3V regulator and configures
> the touchkey node + i2c-gpio node.

Thanks for the patch, see some comments below.

> Signed-off-by: Simon Shields <[email-masked]>
> [email-masked]: Fixed keycodes.
> Signed-off-by: Denis 'GNUtoo' Carikli <[email-masked]>
> ---
>  arch/arm/boot/dts/exynos4412-galaxy-s3.dtsi |  4 +++
>  arch/arm/boot/dts/exynos4412-midas.dtsi     | 29 +++++++++++++++++++++
>  arch/arm/boot/dts/exynos4412-n710x.dts      |  4 +++
>  3 files changed, 37 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/exynos4412-galaxy-s3.dtsi b/arch/arm/boot/dts/exynos4412-galaxy-s3.dtsi
> index ce87d2ff27aa..e71f103ab940 100644
> --- a/arch/arm/boot/dts/exynos4412-galaxy-s3.dtsi
> +++ b/arch/arm/boot/dts/exynos4412-galaxy-s3.dtsi
> @@ -166,5 +166,9 @@
>  &s5c73m3 {
>  	standby-gpios = <&gpm0 1 GPIO_ACTIVE_LOW>;   /* ISP_STANDBY */
>  	vdda-supply = <&ldo17_reg>;
> +};
> +
> +&touchkey_reg {
> +	gpio = <&gpm0 0 GPIO_ACTIVE_HIGH>;
>  	status = "okay";

It looks like status = "okay" was initially found on the s5c73m3 node. With this
change, it's no longer the case so the camera node will remain disabled.

So you probably need to duplicate status = "okay", so that it's both on
touchkey_reg and s5c73m3.

>  };
> diff --git a/arch/arm/boot/dts/exynos4412-midas.dtsi b/arch/arm/boot/dts/exynos4412-midas.dtsi
> index 83be3a797411..797e8de40580 100644
> --- a/arch/arm/boot/dts/exynos4412-midas.dtsi
> +++ b/arch/arm/boot/dts/exynos4412-midas.dtsi
> @@ -13,6 +13,7 @@
>  #include "exynos4412.dtsi"
>  #include "exynos4412-ppmu-common.dtsi"
>  #include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/input/input.h>
>  #include <dt-bindings/interrupt-controller/irq.h>
>  #include <dt-bindings/clock/maxim,max77686.h>
>  #include <dt-bindings/pinctrl/samsung.h>
> @@ -92,6 +93,15 @@
>  		enable-active-high;
>  	};
>  
> +	touchkey_reg: voltage-regulator-4 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "VTOUCH_3.3V";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		enable-active-high;
> +		status = "disabled";
> +	};
> +
>  	gpio-keys {
>  		compatible = "gpio-keys";
>  		pinctrl-names = "default";
> @@ -197,6 +207,25 @@
>  		};
>  	};
>  
> +	i2c_touchkey: i2c-gpio-4 {

Any reason why this node is not marked as disabled here although the regulator
it depends on is?

> +		compatible = "i2c-gpio";
> +		sda-gpios = <&gpl0 2 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> +		scl-gpios = <&gpl0 1 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> +		i2c-gpio,delay-us = <2>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		touchkey@20 {
> +			compatible = "cypress,midas-touchkey";
> +			reg = <0x20>;
> +			vdd-supply = <&touchkey_reg>;
> +			vcc-supply = <&ldo5_reg>;
> +			interrupt-parent = <&gpj0>;
> +			interrupts = <3 IRQ_TYPE_EDGE_FALLING>;
> +			linux,keycodes = <KEY_BACK KEY_MENU>;
> +		};
> +	};
> +
>  	i2c-mhl {
>  		compatible = "i2c-gpio";
>  		gpios = <&gpf0 4 GPIO_ACTIVE_HIGH>, <&gpf0 6 GPIO_ACTIVE_HIGH>;
> diff --git a/arch/arm/boot/dts/exynos4412-n710x.dts b/arch/arm/boot/dts/exynos4412-n710x.dts
> index fe2bfd76cc4e..6acb19d2bae2 100644
> --- a/arch/arm/boot/dts/exynos4412-n710x.dts
> +++ b/arch/arm/boot/dts/exynos4412-n710x.dts
> @@ -71,5 +71,9 @@
>  &s5c73m3 {
>  	standby-gpios = <&gpm0 6 GPIO_ACTIVE_LOW>;   /* ISP_STANDBY */
>  	vdda-supply = <&cam_vdda_reg>;
> +};
> +
> +&touchkey_reg {
> +	gpio = <&gpm0 5 GPIO_ACTIVE_HIGH>;
>  	status = "okay";

And ditto about duplicating status.

Cheers,

Paul

>  };
> -- 
> 2.22.0

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
_______________________________________________
linux-arm-kernel mailing list
[email-masked]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel