[PATCH] RFC: input: Extend matrix-keypad device tree binding

[PATCH] RFC: input: Extend matrix-keypad device tree binding

[Simon Glass]

Some matrix keypad drivers can support different numbers of rows and
columns. Add a generic binding for these.

Also define properties for the repeat rate, to allow a sane baseline to
be set up for the driver.

Implementation note:

In order to implement this binding in the kernel, we will need to modify
matrix_keypad_() to look up the number of rows and cols in
the keymap. Perhaps this could be done by passing 0 for these parameters?
Many of the parameters can already be set to NULL. Ick.

For the key repeat feature, we need to set this after the input device
is registered. So we would need to add a matrix_keypad_setup_input() or
similar to be called by the driver after input_register_device(). I am
less keen on that idea, and less again on the alternative of perhaps
matrix_keypad_register_device() which does input_register_device() and
then sets up the key repeat. Thoughts?

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 .../devicetree/bindings/input/matrix-keymap.txt    |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/Documentation/devicetree/bindings/input/matrix-keymap.txt b/Documentation/devicetree/bindings/input/matrix-keymap.txt
index 3cd8b98..b953d27 100644
--- a/Documentation/devicetree/bindings/input/matrix-keymap.txt
+++ b/Documentation/devicetree/bindings/input/matrix-keymap.txt
@@ -9,6 +9,16 @@ Required properties:
 	row << 24 | column << 16 | key-code
 
 Optional properties:
+Properties for the number of rows and columns are optional because some
+drivers will use fixed values for these.
+- keypad,num-rows: Number of row lines connected to the keypad controller.
+- keypad,num-columns: Number of column lines connected to the keypad
+  controller.
+- keypad,repeat-delay-ms: Number of milliseconds of delay before the first
+  keyboard repeat when a key is held down.
+- keypad,repeat-rate-ms: Number of milliseconds between each subsequent
+  keyboard repeat when a key is held down.
+
 Some users of this binding might choose to specify secondary keymaps for
 cases where there is a modifier key such as a Fn key. Proposed names
 for said properties are "linux,fn-keymap" or with another descriptive
@@ -17,3 +27,7 @@ word for the modifier other from "Fn".
 Example:
 	linux,keymap = < 0x00030012
 			 0x0102003a >;
+	keypad,num-rows = <2>;
+	keypad,num-columns = <8>;
+	keypad,repeat-delay-ms = <600>;
+	keypad,repeat-rate-ms = <60>;
-- 
1.7.7.3

Re: [PATCH] RFC: input: Extend matrix-keypad device tree binding

[Stephen Warren]

On 12/20/2012 01:23 PM, Simon Glass wrote:
> Some matrix keypad drivers can support different numbers of rows and
> columns. Add a generic binding for these.

Oh, I thought num-rows/columns were already there!

Anyway, looks fine to me, so,
Reviewed-by: Stephen Warren <swarren@nvidia.com>

Re: [PATCH] RFC: input: Extend matrix-keypad device tree binding

[Dmitry Torokhov]

Hi Simon,

On Thu, Dec 20, 2012 at 12:23:58PM -0800, Simon Glass wrote:
> 
> For the key repeat feature, we need to set this after the input device
> is registered. So we would need to add a matrix_keypad_setup_input() or
> similar to be called by the driver after input_register_device(). I am
> less keen on that idea, and less again on the alternative of perhaps
> matrix_keypad_register_device() which does input_register_device() and
> then sets up the key repeat. Thoughts?

No, we already have default rate and delay. Unless you can prove that
random firmware writer's idea of appropriate delay and rate is better
then current default - for everyone - and then can successfully argue
that that obviously best delay/rate combo should not replace the current
one but stay only in DT bindings, let's keep relying on users adjusting
their own preferences from respective desktop environments/console/etc.

Thanks.

-- 
Dmitry

Re: [PATCH] RFC: input: Extend matrix-keypad device tree binding

[Simon Glass]

Hi Dmitry,

On Thu, Dec 20, 2012 at 1:13 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> Hi Simon,
>
> On Thu, Dec 20, 2012 at 12:23:58PM -0800, Simon Glass wrote:
>>
>> For the key repeat feature, we need to set this after the input device
>> is registered. So we would need to add a matrix_keypad_setup_input() or
>> similar to be called by the driver after input_register_device(). I am
>> less keen on that idea, and less again on the alternative of perhaps
>> matrix_keypad_register_device() which does input_register_device() and
>> then sets up the key repeat. Thoughts?
>
> No, we already have default rate and delay. Unless you can prove that
> random firmware writer's idea of appropriate delay and rate is better
> then current default - for everyone - and then can successfully argue
> that that obviously best delay/rate combo should not replace the current
> one but stay only in DT bindings, let's keep relying on users adjusting
> their own preferences from respective desktop environments/console/etc.

Seems reasonable. My only comment on this is that the device tree
comes from kernel, not firmware. This lets us configure an embedded
system easily (where the user may not have access to repeat rate
preferences).

Grant are you OK with me just dropping the repeat settings, and
keeping the other two? If so I will respin the patch.

Regards,
Simon

>
> Thanks.
>
> --
> Dmitry

Re: [PATCH] RFC: input: Extend matrix-keypad device tree binding

[Rob Herring]

On 12/20/2012 03:41 PM, Simon Glass wrote:
> Hi Dmitry,
> 
> On Thu, Dec 20, 2012 at 1:13 PM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
>> Hi Simon,
>>
>> On Thu, Dec 20, 2012 at 12:23:58PM -0800, Simon Glass wrote:
>>>
>>> For the key repeat feature, we need to set this after the input device
>>> is registered. So we would need to add a matrix_keypad_setup_input() or
>>> similar to be called by the driver after input_register_device(). I am
>>> less keen on that idea, and less again on the alternative of perhaps
>>> matrix_keypad_register_device() which does input_register_device() and
>>> then sets up the key repeat. Thoughts?
>>
>> No, we already have default rate and delay. Unless you can prove that
>> random firmware writer's idea of appropriate delay and rate is better
>> then current default - for everyone - and then can successfully argue
>> that that obviously best delay/rate combo should not replace the current
>> one but stay only in DT bindings, let's keep relying on users adjusting
>> their own preferences from respective desktop environments/console/etc.
> 
> Seems reasonable. My only comment on this is that the device tree
> comes from kernel, not firmware. This lets us configure an embedded
> system easily (where the user may not have access to repeat rate
> preferences).

Maybe for your case, but in general the DT is not tied to the kernel.

> Grant are you OK with me just dropping the repeat settings, and
> keeping the other two? If so I will respin the patch.
> 

One comment is these properties are already defined for omap. You should
delete them from omap-keypad.txt and refer to this.

Rob

> Regards,
> Simon
> 
>>
>> Thanks.
>>
>> --
>> Dmitry
> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss
>