| Message ID | 1356035039-21653-1-git-send-email-sjg@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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> -- 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
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.
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 -- 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
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 > -- 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 --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>;
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(-)