[PATCH] ARM: dts: Add atmel trackpad node to 5250 cros

[PATCH] ARM: dts: Add atmel trackpad node to 5250 cros

[Arun Kumar K]

The newer versions of exynos5250 based Snow boards have
atmel trackpad. Updating relevant nodes for the same.

Signed-off-by: Arun Kumar K <arun.kk@samsung.com>
---
 arch/arm/boot/dts/exynos5250-cros-common.dtsi |   24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/arch/arm/boot/dts/exynos5250-cros-common.dtsi b/arch/arm/boot/dts/exynos5250-cros-common.dtsi
index 2c1560d..658f086 100644
--- a/arch/arm/boot/dts/exynos5250-cros-common.dtsi
+++ b/arch/arm/boot/dts/exynos5250-cros-common.dtsi
@@ -28,6 +28,13 @@
 			samsung,pin-pud = <0>;
 		};
 
+		trackpad_irq: trackpad-irq {
+			samsung,pins = "gpx1-2";
+			samsung,pin-function = <0>;
+			samsung,pin-pud = <0>;
+			samsung,pin-drv = <0>;
+		};
+
 		max77686_irq: max77686-irq {
 			samsung,pins = "gpx3-2";
 			samsung,pin-function = <0>;
@@ -191,6 +198,9 @@
 		samsung,i2c-sda-delay = <100>;
 		samsung,i2c-max-bus-freq = <378000>;
 
+		pinctrl-names = "default";
+		pinctrl-0 = <&i2c1_bus &trackpad_irq>;
+
 		trackpad {
 			reg = <0x67>;
 			compatible = "cypress,cyapa";
@@ -198,6 +208,20 @@
 			interrupt-parent = <&gpx1>;
 			wakeup-source;
 		};
+		trackpad-alt {
+			reg = <0x4b>;
+			compatible = "atmel,atmel_mxt_tp";
+			interrupts = <2 0>;
+			interrupt-parent = <&gpx1>;
+			wakeup-source;
+		};
+		trackpad-bootloader {
+			reg = <0x25>;
+			compatible = "atmel,atmel_mxt_tp";
+			interrupts = <2 0>;
+			interrupt-parent = <&gpx1>;
+			wakeup-source;
+		};
 	};
 
 	i2c@12C80000 {
-- 
1.7.9.5

Re: [PATCH] ARM: dts: Add atmel trackpad node to 5250 cros

[Tomasz Figa]

Hi Arun,

On 14.04.2014 08:35, Arun Kumar K wrote:
> The newer versions of exynos5250 based Snow boards have
> atmel trackpad. Updating relevant nodes for the same.
>
> Signed-off-by: Arun Kumar K <arun.kk@samsung.com>
> ---
>   arch/arm/boot/dts/exynos5250-cros-common.dtsi |   24 ++++++++++++++++++++++++
>   1 file changed, 24 insertions(+)
>
> diff --git a/arch/arm/boot/dts/exynos5250-cros-common.dtsi b/arch/arm/boot/dts/exynos5250-cros-common.dtsi
> index 2c1560d..658f086 100644
> --- a/arch/arm/boot/dts/exynos5250-cros-common.dtsi
> +++ b/arch/arm/boot/dts/exynos5250-cros-common.dtsi
> @@ -28,6 +28,13 @@
>   			samsung,pin-pud = <0>;
>   		};
>
> +		trackpad_irq: trackpad-irq {
> +			samsung,pins = "gpx1-2";
> +			samsung,pin-function = <0>;
> +			samsung,pin-pud = <0>;
> +			samsung,pin-drv = <0>;
> +		};
> +
>   		max77686_irq: max77686-irq {
>   			samsung,pins = "gpx3-2";
>   			samsung,pin-function = <0>;
> @@ -191,6 +198,9 @@
>   		samsung,i2c-sda-delay = <100>;
>   		samsung,i2c-max-bus-freq = <378000>;
>
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&i2c1_bus &trackpad_irq>;

Please add &trackpad_irq to trackpad node instead. Pinctrl properties of 
i2c node should contain only i2c-related pins.

> +
>   		trackpad {
>   			reg = <0x67>;
>   			compatible = "cypress,cyapa";
> @@ -198,6 +208,20 @@
>   			interrupt-parent = <&gpx1>;
>   			wakeup-source;
>   		};
> +		trackpad-alt {

Style: Please keep one blank line between two nodes.

> +			reg = <0x4b>;
> +			compatible = "atmel,atmel_mxt_tp";
> +			interrupts = <2 0>;
> +			interrupt-parent = <&gpx1>;
> +			wakeup-source;
> +		};
> +		trackpad-bootloader {

Ditto.

> +			reg = <0x25>;
> +			compatible = "atmel,atmel_mxt_tp";
> +			interrupts = <2 0>;
> +			interrupt-parent = <&gpx1>;
> +			wakeup-source;
> +		};

Hmm, why are there 3 different nodes here? Could you explain what one by 
one for what hardware they are?

Best regards,
Tomasz

Re: [PATCH] ARM: dts: Add atmel trackpad node to 5250 cros

[Doug Anderson]

Tomasz,

On Thu, Apr 24, 2014 at 9:57 AM, Tomasz Figa <t.figa@samsung.com> wrote:
> Hi Arun,
>
>
> On 14.04.2014 08:35, Arun Kumar K wrote:
>>
>> The newer versions of exynos5250 based Snow boards have
>> atmel trackpad. Updating relevant nodes for the same.
>>
>> Signed-off-by: Arun Kumar K <arun.kk@samsung.com>
>> ---
>>   arch/arm/boot/dts/exynos5250-cros-common.dtsi |   24
>> ++++++++++++++++++++++++
>>   1 file changed, 24 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/exynos5250-cros-common.dtsi
>> b/arch/arm/boot/dts/exynos5250-cros-common.dtsi
>> index 2c1560d..658f086 100644
>> --- a/arch/arm/boot/dts/exynos5250-cros-common.dtsi
>> +++ b/arch/arm/boot/dts/exynos5250-cros-common.dtsi
>> @@ -28,6 +28,13 @@
>>                         samsung,pin-pud = <0>;
>>                 };
>>
>> +               trackpad_irq: trackpad-irq {
>> +                       samsung,pins = "gpx1-2";
>> +                       samsung,pin-function = <0>;
>> +                       samsung,pin-pud = <0>;
>> +                       samsung,pin-drv = <0>;
>> +               };
>> +
>>                 max77686_irq: max77686-irq {
>>                         samsung,pins = "gpx3-2";
>>                         samsung,pin-function = <0>;
>> @@ -191,6 +198,9 @@
>>                 samsung,i2c-sda-delay = <100>;
>>                 samsung,i2c-max-bus-freq = <378000>;
>>
>> +               pinctrl-names = "default";
>> +               pinctrl-0 = <&i2c1_bus &trackpad_irq>;
>
>
> Please add &trackpad_irq to trackpad node instead. Pinctrl properties of i2c
> node should contain only i2c-related pins.

This was a workaround for a limitation of the i2c subsystem which need
to be fixed before atmel support can land upstream.

The problem is that there are two sources for the trackpad on this
hardware.  They are pin compatible, so some boards may have one
trackpad and other (mostly newer) boards may have the other.

The i2c bus is probable, so the theory is that you can just specify
both of trackpads and one will fail to be found.  The problem (at
least on 3.8) was that the pinctrl subsystem held onto the pins.  :(


I just tried now to see if I could reproduce this on ToT.  ...and it
turns out that the atmel_mxt_ts doesn't support device tree upstream.
Right at the top of mxt_probe I see:

if (!pdata)
  return -EINVAL;

I actually find a patch (from you!) that adds some device tree support
at <https://patchwork.kernel.org/patch/2393631/>.  ...though
interestingly enough our current tree somehow gets by without
specifying all of those properties.  +Benson may be able to give more
details?


Until atmel device tree support lands, though, I think this patch is stalled.

>> +                       reg = <0x25>;
>> +                       compatible = "atmel,atmel_mxt_tp";
>> +                       interrupts = <2 0>;
>> +                       interrupt-parent = <&gpx1>;
>> +                       wakeup-source;
>> +               };
>
>
> Hmm, why are there 3 different nodes here? Could you explain what one by one
> for what hardware they are?

cyapa: the Cypress trackpad that shipped with original exynos5250-snow

atmel: some newer exynos5250-snow ship with this

atmel-bootloader: appears to be a hack to get around the fact that you
can only specify one i2c address through the device tree.  I'm told
that sometimes this device shows up at a different address.  On x86
machines with this they use "i2c_new_probed_device" which allows you
to pass in a list of addresses, it seems.  I'm not sure how this
should really translate to the device tree.


-Doug