diff mbox

[2/2] Input: atmel_mxt_ts - Add keycodes array example

Message ID 1407372486-25881-2-git-send-email-javier.martinez@collabora.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Javier Martinez Canillas Aug. 7, 2014, 12:48 a.m. UTC
The Atmel maXTouch driver allows to dynamically define the
event keycodes set that the device is capable of. This may
not be evident when reading the DT binding document so add
an example to make it easier to understand how this works.

Also, the current documentation says that the array limit
is four entries but the driver dynamically allocates the
keymap array and does not limit the array size.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 Documentation/devicetree/bindings/input/atmel,maxtouch.txt | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comments

Nick Dyer Aug. 7, 2014, 12:38 p.m. UTC | #1
On 07/08/14 01:48, Javier Martinez Canillas wrote:
> The Atmel maXTouch driver allows to dynamically define the
> event keycodes set that the device is capable of. This may
> not be evident when reading the DT binding document so add
> an example to make it easier to understand how this works.
> 
> Also, the current documentation says that the array limit
> is four entries but the driver dynamically allocates the
> keymap array and does not limit the array size.

There is a physical limit to the number of GPIOs on the device. The number
4 is wrong, the protocol does allow for up to 8 GPIOs. But it is a hard limit.

> 
> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> ---
>  Documentation/devicetree/bindings/input/atmel,maxtouch.txt | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/input/atmel,maxtouch.txt b/Documentation/devicetree/bindings/input/atmel,maxtouch.txt
> index baef432..be50476 100644
> --- a/Documentation/devicetree/bindings/input/atmel,maxtouch.txt
> +++ b/Documentation/devicetree/bindings/input/atmel,maxtouch.txt
> @@ -11,7 +11,7 @@ Required properties:
>  
>  Optional properties for main touchpad device:
>  
> -- linux,gpio-keymap: An array of up to 4 entries indicating the Linux
> +- linux,gpio-keymap: An array of entries indicating the Linux
>      keycode generated by each GPIO. Linux keycodes are defined in
>      <dt-bindings/input/input.h>.
>  
> @@ -22,4 +22,10 @@ Example:
>  		reg = <0x4b>;
>  		interrupt-parent = <&gpio>;
>  		interrupts = <TEGRA_GPIO(W, 3) IRQ_TYPE_LEVEL_LOW>;
> +		linux,gpio-keymap = < BTN_LEFT
> +				      BTN_TOOL_FINGER
> +				      BTN_TOOL_DOUBLETAP
> +				      BTN_TOOL_TRIPLETAP
> +				      BTN_TOOL_QUADTAP
> +				      BTN_TOOL_QUINTTAP >;

I'm afraid you have misunderstood the impact of this change to the way that
the GPIOs coming in to the touch controller are mapped to key codes. Look
at the protocol guide for T19.

The DOUBLE/TRIPLE/QUAD/QUINTTAP stuff is filled in for us by the input core
when we use INPUT_MT_POINTER, anyway.
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Javier Martinez Canillas Aug. 8, 2014, 2:52 p.m. UTC | #2
Hello Nick,

On 08/07/2014 02:38 PM, Nick Dyer wrote:
>> 
>> Also, the current documentation says that the array limit
>> is four entries but the driver dynamically allocates the
>> keymap array and does not limit the array size.
> 
> There is a physical limit to the number of GPIOs on the device. The number
> 4 is wrong, the protocol does allow for up to 8 GPIOs. But it is a hard limit.
>

Thanks a lot for the explanation, then I guess s/4/8 is enough.

>> 
>> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
>> ---
>>  Documentation/devicetree/bindings/input/atmel,maxtouch.txt | 8 +++++++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>> 
>> diff --git a/Documentation/devicetree/bindings/input/atmel,maxtouch.txt b/Documentation/devicetree/bindings/input/atmel,maxtouch.txt
>> index baef432..be50476 100644
>> --- a/Documentation/devicetree/bindings/input/atmel,maxtouch.txt
>> +++ b/Documentation/devicetree/bindings/input/atmel,maxtouch.txt
>> @@ -11,7 +11,7 @@ Required properties:
>>  
>>  Optional properties for main touchpad device:
>>  
>> -- linux,gpio-keymap: An array of up to 4 entries indicating the Linux
>> +- linux,gpio-keymap: An array of entries indicating the Linux
>>      keycode generated by each GPIO. Linux keycodes are defined in
>>      <dt-bindings/input/input.h>.
>>  
>> @@ -22,4 +22,10 @@ Example:
>>  		reg = <0x4b>;
>>  		interrupt-parent = <&gpio>;
>>  		interrupts = <TEGRA_GPIO(W, 3) IRQ_TYPE_LEVEL_LOW>;
>> +		linux,gpio-keymap = < BTN_LEFT
>> +				      BTN_TOOL_FINGER
>> +				      BTN_TOOL_DOUBLETAP
>> +				      BTN_TOOL_TRIPLETAP
>> +				      BTN_TOOL_QUADTAP
>> +				      BTN_TOOL_QUINTTAP >;
> 
> I'm afraid you have misunderstood the impact of this change to the way that
> the GPIOs coming in to the touch controller are mapped to key codes. Look

Unfortunately there are no boards in mainline using this "linux,gpio-keymap"
property so I tried to figure out what the expected values were by reading the
driver. So is more than possible that I got them wrong.

By passing all these keycodes the touchpad worked as expected for me and the
driver did the same than the Chrome OS driver that has these keycodes hardcoded
when is_tp is true.

> at the protocol guide for T19.
> 

I don't have access to proper documentation and I wouldn't expect people to have
access to non-public docs in order to use a Device Tree binding.

That's why I wanted to document an example, so using this property could be
easier for others and they shouldn't have to look at the driver in order to
figure it out (and getting it wrong as you said :) )

So it would be great if you could provide an example on how this is supposed to
be used.

> The DOUBLE/TRIPLE/QUAD/QUINTTAP stuff is filled in for us by the input core
> when we use INPUT_MT_POINTER, anyway.
> 

Thanks for the hint, I didn't know that this was the case but I just looked at
input_mt_init_slots() [0] and indeed those are not needed.

Best regards,
Javier

[0]: http://lxr.free-electrons.com/source/drivers/input/input-mt.c#L69
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Javier Martinez Canillas Aug. 15, 2014, 12:01 p.m. UTC | #3
Hello Nick,

On 08/08/2014 04:52 PM, Javier Martinez Canillas wrote:
On 08/07/2014 02:38 PM, Nick Dyer wrote:
>> >>
>> 
>> I'm afraid you have misunderstood the impact of this change to the way that
>> the GPIOs coming in to the touch controller are mapped to key codes. Look
> 
> Unfortunately there are no boards in mainline using this "linux,gpio-keymap"
> property so I tried to figure out what the expected values were by reading the
> driver. So is more than possible that I got them wrong.
> 
> By passing all these keycodes the touchpad worked as expected for me and the
> driver did the same than the Chrome OS driver that has these keycodes hardcoded
> when is_tp is true.
> 
>> at the protocol guide for T19.
>> 
> 
> I don't have access to proper documentation and I wouldn't expect people to have
> access to non-public docs in order to use a Device Tree binding.
> 
> That's why I wanted to document an example, so using this property could be
> easier for others and they shouldn't have to look at the driver in order to
> figure it out (and getting it wrong as you said :) )
> 
> So it would be great if you could provide an example on how this is supposed to
> be used.
> 

Any comments on this? I would really appreciate if you can expand on how
this DT property is supposed to be used so I can re-spin the atmel support
patch for Peach boards.

Thanks a lot and best regards,
Javier
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/input/atmel,maxtouch.txt b/Documentation/devicetree/bindings/input/atmel,maxtouch.txt
index baef432..be50476 100644
--- a/Documentation/devicetree/bindings/input/atmel,maxtouch.txt
+++ b/Documentation/devicetree/bindings/input/atmel,maxtouch.txt
@@ -11,7 +11,7 @@  Required properties:
 
 Optional properties for main touchpad device:
 
-- linux,gpio-keymap: An array of up to 4 entries indicating the Linux
+- linux,gpio-keymap: An array of entries indicating the Linux
     keycode generated by each GPIO. Linux keycodes are defined in
     <dt-bindings/input/input.h>.
 
@@ -22,4 +22,10 @@  Example:
 		reg = <0x4b>;
 		interrupt-parent = <&gpio>;
 		interrupts = <TEGRA_GPIO(W, 3) IRQ_TYPE_LEVEL_LOW>;
+		linux,gpio-keymap = < BTN_LEFT
+				      BTN_TOOL_FINGER
+				      BTN_TOOL_DOUBLETAP
+				      BTN_TOOL_TRIPLETAP
+				      BTN_TOOL_QUADTAP
+				      BTN_TOOL_QUINTTAP >;
 	};