[PATCH v5] Input: sysrq - DT binding for key sequence

2013-07-17 Thread mathieu . poirier 
From: Mathieu J. Poirier mathieu.poir...@linaro.org

Adding a simple device tree binding for the specification of key sequences.
Definition of the keys found in the sequence are located in
'include/uapi/linux/input.h'.

For the sysrq driver, holding the sequence of keys down for a specific amount 
of time
will reset the system.

Signed-off-by: Mathieu Poirier mathieu.poir...@linaro.org
---
changes for v5:
  - Corrected error in binding definition.
  - Using helper macro to fetch the key sequence.
  - Removing white space.
---
 .../devicetree/bindings/input/input-reset.txt  | 34 
 drivers/tty/sysrq.c| 37 ++
 2 files changed, 71 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/input-reset.txt

diff --git a/Documentation/devicetree/bindings/input/input-reset.txt 
b/Documentation/devicetree/bindings/input/input-reset.txt
new file mode 100644
index 000..c69390c
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/input-reset.txt
@@ -0,0 +1,34 @@
+Input: sysrq reset sequence
+
+A simple binding to represent a set of keys as described in
+include/uapi/linux/input.h.  This is to communicate a
+sequence of keys to the sysrq driver.  Upon holding the keys
+for a specified amount of time (if specified) the system is
+sync'ed and reset.
+
+Key sequences are global to the system but all the keys in a
+set must be coming from the same input device.
+
+The /chosen node should contain a 'linux,sysrq-reset-seq' child
+node to define a set of keys.
+
+Required property:
+sysrq-reset-seq: array of keycodes
+
+Optional property:
+timeout-ms: duration keys must be pressed together in milliseconds
+before generating a sysrq
+
+Example:
+
+ chosen {
+linux,sysrq-reset-seq {
+keyset = 0x03
+  0x04
+  0x0a;
+timeout-ms = 3000;
+};
+ };
+
+Would represent KEY_2, KEY_3 and KEY_9.
+
diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
index b51c154..ed8f00f 100644
--- a/drivers/tty/sysrq.c
+++ b/drivers/tty/sysrq.c
@@ -44,6 +44,7 @@
 #include linux/uaccess.h
 #include linux/moduleparam.h
 #include linux/jiffies.h
+#include linux/of.h
 
 #include asm/ptrace.h
 #include asm/irq_regs.h
@@ -671,6 +672,34 @@ static void sysrq_detect_reset_sequence(struct sysrq_state 
*state,
}
 }
 
+static void sysrq_of_get_keyreset_config(void)
+{
+   u32 key;
+   struct device_node *np;
+   struct property *prop;
+   const __be32 *p;
+
+   np = of_find_node_by_path(/chosen/linux,sysrq-reset-seq);
+   if (!np) {
+   pr_debug(No sysrq node found);
+   return;
+   }
+
+   /* reset in case a __weak definition was present */
+   sysrq_reset_seq_len = 0;
+
+   of_property_for_each_u32(np, keyset, prop, p, key) {
+   if ((key == KEY_RESERVED || key  KEY_MAX) ||
+   (sysrq_reset_seq_len == SYSRQ_KEY_RESET_MAX))
+   break;
+
+   sysrq_reset_seq[sysrq_reset_seq_len++] = (unsigned short)key;
+   }
+
+   /* get reset timeout if any */
+   of_property_read_u32(np, timeout-ms, sysrq_reset_downtime_ms);
+}
+
 static void sysrq_reinject_alt_sysrq(struct work_struct *work)
 {
struct sysrq_state *sysrq =
@@ -903,6 +932,7 @@ static inline void sysrq_register_handler(void)
int error;
int i;
 
+   /* first check if a __weak interface was instantiated */
for (i = 0; i  ARRAY_SIZE(sysrq_reset_seq); i++) {
key = platform_sysrq_reset_seq[i];
if (key == KEY_RESERVED || key  KEY_MAX)
@@ -911,6 +941,13 @@ static inline void sysrq_register_handler(void)
sysrq_reset_seq[sysrq_reset_seq_len++] = key;
}
 
+   /*
+* DT configuration takes precedence over anything
+* that would have been defined via the __weak
+* interface
+*/
+   sysrq_of_get_keyreset_config();
+
error = input_register_handler(sysrq_handler);
if (error)
pr_err(Failed to register input handler, error %d, error);
-- 
1.8.1.2

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH v4] Input: sysrq - DT binding for key sequence

2013-07-16 Thread Mathieu Poirier 
On 15 July 2013 20:43, Rob Herring robherri...@gmail.com wrote:

 On 07/15/2013 10:36 AM, mathieu.poir...@linaro.org wrote:
  From: Mathieu J. Poirier mathieu.poir...@linaro.org
 
  Adding a simple device tree binding for the specification of key
 sequences.
  Definition of the keys found in the sequence are located in
  'include/uapi/linux/input.h'.
 
  For the sysrq driver, holding the sequence of keys down for a specific
 amount of time
  will reset the system.
 
  Signed-off-by: Mathieu Poirier mathieu.poir...@linaro.org
  ---
  changes for v4:
- Moved seach of reset sequence nodes to a single call.
  ---
   .../devicetree/bindings/input/input-reset.txt  | 34 ++
   drivers/tty/sysrq.c| 54
 ++
   2 files changed, 88 insertions(+)
   create mode 100644
 Documentation/devicetree/bindings/input/input-reset.txt
 
  diff --git a/Documentation/devicetree/bindings/input/input-reset.txt
 b/Documentation/devicetree/bindings/input/input-reset.txt
  new file mode 100644
  index 000..79504af
  --- /dev/null
  +++ b/Documentation/devicetree/bindings/input/input-reset.txt
  @@ -0,0 +1,34 @@
  +Input: sysrq reset sequence
  +
  +A simple binding to represent a set of keys as described in
  +include/uapi/linux/input.h.  This is to communicate a
  +sequence of keys to the sysrq driver.  Upon holding the keys
  +for a specified amount of time (if specified) the system is
  +sync'ed and reset.
  +
  +Key sequences are global to the system but all the keys in a
  +set must be coming from the same input device.
  +
  +The /chosen node should contain a 'linux,sysrq-reset-seq' child
  +node to define a set of keys.
  +
  +Required property:
  +sysrq-reset-seq: array of keycodes
  +
  +Optional property:
  +timeout-ms: duration keys must be pressed together in microseconds

 milliseconds (ms) or microseconds (us)?


Right, milliseconds.


  +before generating a sysrq
  +
  +Example:
  +
  + chosen {
  +linux,sysrq-reset-seq {
  +keyset = 0x03
  +  0x04
  +  0x0a;
  +timeout-ms = 3000;
  +};
  + };
  +
  +Would represent KEY_2, KEY_3 and KEY_9.
  +
  diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
  index b51c154..01a8729 100644
  --- a/drivers/tty/sysrq.c
  +++ b/drivers/tty/sysrq.c
  @@ -44,6 +44,7 @@
   #include linux/uaccess.h
   #include linux/moduleparam.h
   #include linux/jiffies.h
  +#include linux/of.h
 
   #include asm/ptrace.h
   #include asm/irq_regs.h
  @@ -671,6 +672,50 @@ static void sysrq_detect_reset_sequence(struct
 sysrq_state *state,
}
   }
 
  +static void sysrq_of_get_keyreset_config(void)
  +{
  + unsigned short key;
  + struct device_node *np;
  + const struct property *prop;
  + const __be32 *val;
  + int count, i;
  +
  + np = of_find_node_by_path(/chosen/linux,sysrq-reset-seq);
  + if (!np) {
  + pr_debug(No sysrq node found);
  + goto out;
  + }
  +
  + prop = of_find_property(np, keyset, NULL);
  + if (!prop || !prop-value) {
  + pr_err(Invalid input keyset);
  + goto out;
  + }
  +
  + count = prop-length / sizeof(u32);
  + val = prop-value;

 None of the existing helpers to retrieve property arrays doesn't work here?


The problem here is that we never know how long the sequence will be.  As
such the 'of_property_read_uXY_array' functions won't work.  Or maybe
you're referring to another set of helpers... If so, please clarify.



  +
  + if (count  SYSRQ_KEY_RESET_MAX)
  + count = SYSRQ_KEY_RESET_MAX;
  +
  + /* reset in case a __weak definition was present */
  + sysrq_reset_seq_len = 0;
  +
  + for (i = 0; i  count; i++) {
  + key = (unsigned short)be32_to_cpup(val++);
  + if (key == KEY_RESERVED || key  KEY_MAX)
  + break;
  +
  + sysrq_reset_seq[sysrq_reset_seq_len++] = key;
  + }
  +
  + /* get reset timeout if any */
  + of_property_read_u32(np, timeout-ms, sysrq_reset_downtime_ms);
  +
  + out:
  + ;
  +}
  +
   static void sysrq_reinject_alt_sysrq(struct work_struct *work)
   {
struct sysrq_state *sysrq =
  @@ -688,6 +733,7 @@ static void sysrq_reinject_alt_sysrq(struct
 work_struct *work)
input_inject_event(handle, EV_KEY, KEY_SYSRQ, 1);
input_inject_event(handle, EV_SYN, SYN_REPORT, 1);
 
  +

 spurious ws change


input_inject_event(handle, EV_KEY, KEY_SYSRQ, 0);
input_inject_event(handle, EV_KEY, alt_code, 0);
input_inject_event(handle, EV_SYN, SYN_REPORT, 1);
  @@ -903,6 +949,7 @@ static inline void sysrq_register_handler(void)
int error;
int i;
 
  + /* first check if a __weak interface was instantiated */
for (i = 0; i  ARRAY_SIZE

[PATCH v4] Input: sysrq - DT binding for key sequence

2013-07-15 Thread mathieu . poirier 
From: Mathieu J. Poirier mathieu.poir...@linaro.org

Adding a simple device tree binding for the specification of key sequences.
Definition of the keys found in the sequence are located in
'include/uapi/linux/input.h'.

For the sysrq driver, holding the sequence of keys down for a specific amount 
of time
will reset the system.

Signed-off-by: Mathieu Poirier mathieu.poir...@linaro.org
---
changes for v4:
  - Moved seach of reset sequence nodes to a single call.
---
 .../devicetree/bindings/input/input-reset.txt  | 34 ++
 drivers/tty/sysrq.c| 54 ++
 2 files changed, 88 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/input-reset.txt

diff --git a/Documentation/devicetree/bindings/input/input-reset.txt 
b/Documentation/devicetree/bindings/input/input-reset.txt
new file mode 100644
index 000..79504af
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/input-reset.txt
@@ -0,0 +1,34 @@
+Input: sysrq reset sequence
+
+A simple binding to represent a set of keys as described in
+include/uapi/linux/input.h.  This is to communicate a
+sequence of keys to the sysrq driver.  Upon holding the keys
+for a specified amount of time (if specified) the system is
+sync'ed and reset.
+
+Key sequences are global to the system but all the keys in a
+set must be coming from the same input device.
+
+The /chosen node should contain a 'linux,sysrq-reset-seq' child
+node to define a set of keys.
+
+Required property:
+sysrq-reset-seq: array of keycodes
+
+Optional property:
+timeout-ms: duration keys must be pressed together in microseconds
+before generating a sysrq
+
+Example:
+
+ chosen {
+linux,sysrq-reset-seq {
+keyset = 0x03
+  0x04
+  0x0a;
+timeout-ms = 3000;
+};
+ };
+
+Would represent KEY_2, KEY_3 and KEY_9.
+
diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
index b51c154..01a8729 100644
--- a/drivers/tty/sysrq.c
+++ b/drivers/tty/sysrq.c
@@ -44,6 +44,7 @@
 #include linux/uaccess.h
 #include linux/moduleparam.h
 #include linux/jiffies.h
+#include linux/of.h
 
 #include asm/ptrace.h
 #include asm/irq_regs.h
@@ -671,6 +672,50 @@ static void sysrq_detect_reset_sequence(struct sysrq_state 
*state,
}
 }
 
+static void sysrq_of_get_keyreset_config(void)
+{
+   unsigned short key;
+   struct device_node *np;
+   const struct property *prop;
+   const __be32 *val;
+   int count, i;
+
+   np = of_find_node_by_path(/chosen/linux,sysrq-reset-seq);
+   if (!np) {
+   pr_debug(No sysrq node found);
+   goto out;
+   }
+
+   prop = of_find_property(np, keyset, NULL);
+   if (!prop || !prop-value) {
+   pr_err(Invalid input keyset);
+   goto out;
+   }
+
+   count = prop-length / sizeof(u32);
+   val = prop-value;
+
+   if (count  SYSRQ_KEY_RESET_MAX)
+   count = SYSRQ_KEY_RESET_MAX;
+
+   /* reset in case a __weak definition was present */
+   sysrq_reset_seq_len = 0;
+
+   for (i = 0; i  count; i++) {
+   key = (unsigned short)be32_to_cpup(val++);
+   if (key == KEY_RESERVED || key  KEY_MAX)
+   break;
+
+   sysrq_reset_seq[sysrq_reset_seq_len++] = key;
+   }
+
+   /* get reset timeout if any */
+   of_property_read_u32(np, timeout-ms, sysrq_reset_downtime_ms);
+
+ out:
+   ;
+}
+
 static void sysrq_reinject_alt_sysrq(struct work_struct *work)
 {
struct sysrq_state *sysrq =
@@ -688,6 +733,7 @@ static void sysrq_reinject_alt_sysrq(struct work_struct 
*work)
input_inject_event(handle, EV_KEY, KEY_SYSRQ, 1);
input_inject_event(handle, EV_SYN, SYN_REPORT, 1);
 
+
input_inject_event(handle, EV_KEY, KEY_SYSRQ, 0);
input_inject_event(handle, EV_KEY, alt_code, 0);
input_inject_event(handle, EV_SYN, SYN_REPORT, 1);
@@ -903,6 +949,7 @@ static inline void sysrq_register_handler(void)
int error;
int i;
 
+   /* first check if a __weak interface was instantiated */
for (i = 0; i  ARRAY_SIZE(sysrq_reset_seq); i++) {
key = platform_sysrq_reset_seq[i];
if (key == KEY_RESERVED || key  KEY_MAX)
@@ -911,6 +958,13 @@ static inline void sysrq_register_handler(void)
sysrq_reset_seq[sysrq_reset_seq_len++] = key;
}
 
+   /*
+* DT configuration takes precedence over anything
+* that would have been defined via the __weak
+* interface
+*/
+   sysrq_of_get_keyreset_config();
+
error = input_register_handler(sysrq_handler);
if (error)
pr_err(Failed to register input handler, error %d, error);
-- 
1.8.1.2

[PATCH v3] Input: sysrq - DT binding for key sequences

2013-07-12 Thread mathieu . poirier 
From: Mathieu J. Poirier mathieu.poir...@linaro.org

Adding a simple device tree binding for the specification of key sequences.
Definition of the keys found in the sequence are located in
'include/uapi/linux/input.h'.

For the sysrq driver, holding the sequence of keys down for a specific amount 
of time
will reset the system.

Signed-off-by: Mathieu Poirier mathieu.poir...@linaro.org
---
changes for v3:
  - Simplified binding definition.
  - Renamed binding and moved to chosen.
---
 .../devicetree/bindings/input/input-reset.txt  | 34 +
 drivers/tty/sysrq.c| 58 ++
 2 files changed, 92 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/input-reset.txt

diff --git a/Documentation/devicetree/bindings/input/input-reset.txt 
b/Documentation/devicetree/bindings/input/input-reset.txt
new file mode 100644
index 000..79504af
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/input-reset.txt
@@ -0,0 +1,34 @@
+Input: sysrq reset sequence
+
+A simple binding to represent a set of keys as described in
+include/uapi/linux/input.h.  This is to communicate a
+sequence of keys to the sysrq driver.  Upon holding the keys
+for a specified amount of time (if specified) the system is
+sync'ed and reset.
+
+Key sequences are global to the system but all the keys in a
+set must be coming from the same input device.
+
+The /chosen node should contain a 'linux,sysrq-reset-seq' child
+node to define a set of keys.
+
+Required property:
+sysrq-reset-seq: array of keycodes
+
+Optional property:
+timeout-ms: duration keys must be pressed together in microseconds
+before generating a sysrq
+
+Example:
+
+ chosen {
+linux,sysrq-reset-seq {
+keyset = 0x03
+  0x04
+  0x0a;
+timeout-ms = 3000;
+};
+ };
+
+Would represent KEY_2, KEY_3 and KEY_9.
+
diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
index b51c154..4b77adf 100644
--- a/drivers/tty/sysrq.c
+++ b/drivers/tty/sysrq.c
@@ -44,6 +44,7 @@
 #include linux/uaccess.h
 #include linux/moduleparam.h
 #include linux/jiffies.h
+#include linux/of.h
 
 #include asm/ptrace.h
 #include asm/irq_regs.h
@@ -671,6 +672,54 @@ static void sysrq_detect_reset_sequence(struct sysrq_state 
*state,
}
 }
 
+static void sysrq_of_get_keyreset_config(void)
+{
+   unsigned short key;
+   struct device_node *np;
+   const struct property *prop;
+   const __be32 *val;
+   int count, i;
+
+   np = of_find_node_by_path(/chosen);
+   if (!np)
+   goto out;
+
+   np = of_find_node_by_name(np, linux,sysrq-reset-seq);
+   if (!np) {
+   pr_debug(No sysrq node found);
+   goto out;
+   }
+
+   prop = of_find_property(np, keyset, NULL);
+   if (!prop || !prop-value) {
+   pr_err(Invalid input keyset);
+   goto out;
+   }
+
+   count = prop-length / sizeof(u32);
+   val = prop-value;
+
+   if (count  SYSRQ_KEY_RESET_MAX)
+   count = SYSRQ_KEY_RESET_MAX;
+
+   /* reset in case a __weak definition was present */
+   sysrq_reset_seq_len = 0;
+
+   for (i = 0; i  count; i++) {
+   key = (unsigned short)be32_to_cpup(val++);
+   if (key == KEY_RESERVED || key  KEY_MAX)
+   break;
+
+   sysrq_reset_seq[sysrq_reset_seq_len++] = key;
+   }
+
+   /* get reset timeout if any */
+   of_property_read_u32(np, timeout-ms, sysrq_reset_downtime_ms);
+
+ out:
+   ;
+}
+
 static void sysrq_reinject_alt_sysrq(struct work_struct *work)
 {
struct sysrq_state *sysrq =
@@ -688,6 +737,7 @@ static void sysrq_reinject_alt_sysrq(struct work_struct 
*work)
input_inject_event(handle, EV_KEY, KEY_SYSRQ, 1);
input_inject_event(handle, EV_SYN, SYN_REPORT, 1);
 
+
input_inject_event(handle, EV_KEY, KEY_SYSRQ, 0);
input_inject_event(handle, EV_KEY, alt_code, 0);
input_inject_event(handle, EV_SYN, SYN_REPORT, 1);
@@ -903,6 +953,7 @@ static inline void sysrq_register_handler(void)
int error;
int i;
 
+   /* first check if a __weak interface was instantiated */
for (i = 0; i  ARRAY_SIZE(sysrq_reset_seq); i++) {
key = platform_sysrq_reset_seq[i];
if (key == KEY_RESERVED || key  KEY_MAX)
@@ -911,6 +962,13 @@ static inline void sysrq_register_handler(void)
sysrq_reset_seq[sysrq_reset_seq_len++] = key;
}
 
+   /*
+* DT configuration takes precedence over anything
+* that would have been defined via the __weak
+* interface
+*/
+   sysrq_of_get_keyreset_config();
+
error = input_register_handler(sysrq_handler);
if (error)
pr_err(Failed

Re: [PATCH 2/2] Input: Adding DT support for keyreset tuneables

2013-07-10 Thread Mathieu Poirier 
On 10 July 2013 10:52, Dmitry Torokhov dmitry.torok...@gmail.com wrote:

 On Wed, Jul 10, 2013 at 04:14:57PM +0100, Grant Likely wrote:
  On Fri, 28 Jun 2013 07:19:06 -0600, Mathieu Poirier 
 mathieu.poir...@linaro.org wrote:
   On 13-06-28 12:09 AM, Dmitry Torokhov wrote:
I do not agree.  We want the binding to be generic and not tied
specifically to the keyreset functionality.  As such
 'input-keyset' or
'input-keychord' are more appropriate.
   
The binding is defined specifically for sysrq and specifically to
perform reset action.
   
Yes for now but as the examples in the binding show, it is easy to
envision how other drivers could use it.
   
I think you over-complicate things here. Unlike matrix-keypad
 binding,
where you have a common parsing code, here we have an individual
 driver.
I really do not see anyone else using such sequences or chords as
 such
processing should be done in userspace. Sysrq is quite an exception.
  
   To be honest I don't have a very strong opinion on the binding.  I made
   it as generic as possible on the guidance of the DT people.  Let's see
   what they think of it.
 
  Hi Mathieu,
 
  As per our conversation just now at Connect, the binding should probably
  look like this:
 
  Sysrq keyset binding:
 
  The /chosen node can contain a linux,input-keyset-sysrq child node to
  define a set of keys that will generate a sysrq when pressed together.

 Hmm, we would have only one such node, /sysrq, or /linux,sysrq,
 whatever. The sysrq setting is system-wide and applicable to all
 devices. Given that it is used only on mobile, where there not that
 many input devices (a few keys and touchscreen) I do not believe we
 should consider adding per-device settings.


Putting the binding in the chosen node is definitely a system wide
setting.  If I didn't interpret your comment properly, please get back to
me.



 
  Required properties:
  keyset: array of keycodes

 Please, let's call it 'key-reset-seq', because it is exactly the reset
 sequence. There won't be any additional sequences or chords as those
 should be handled in userspace, sysrq is a special case here.



I'm not strongly opinionated on that front but I think it should, at least,
include the some clue about the driver it ties to.  What do you think
about: linux,sysrq-reset-seq ?


  timeout-ms: duration keys must be pressed together in microseconds
  before generating a sysrq
 

 Thanks.

 --
 Dmitry

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 2/2] Input: Adding DT support for keyreset tuneables

2013-07-10 Thread Mathieu Poirier 
On 10 July 2013 16:20, Dmitry Torokhov dmitry.torok...@gmail.com wrote:

 On Wednesday, July 10, 2013 10:50:26 PM Grant Likely wrote:
  On Wed, Jul 10, 2013 at 5:52 PM, Dmitry Torokhov
 
  dmitry.torok...@gmail.com wrote:
   On Wed, Jul 10, 2013 at 04:14:57PM +0100, Grant Likely wrote:
   On Fri, 28 Jun 2013 07:19:06 -0600, Mathieu Poirier
 mathieu.poir...@linaro.org wrote:
On 13-06-28 12:09 AM, Dmitry Torokhov wrote:
 I do not agree.  We want the binding to be generic and not tied
 specifically to the keyreset functionality.  As such
 'input-keyset' or
 'input-keychord' are more appropriate.

 The binding is defined specifically for sysrq and specifically
 to
 perform reset action.

 Yes for now but as the examples in the binding show, it is easy
 to
 envision how other drivers could use it.

 I think you over-complicate things here. Unlike matrix-keypad
 binding,
 where you have a common parsing code, here we have an individual
 driver.
 I really do not see anyone else using such sequences or chords as
 such
 processing should be done in userspace. Sysrq is quite an
 exception.
   
To be honest I don't have a very strong opinion on the binding.  I
 made
it as generic as possible on the guidance of the DT people.  Let's
 see
what they think of it.
  
   Hi Mathieu,
  
   As per our conversation just now at Connect, the binding should
 probably
   look like this:
  
   Sysrq keyset binding:
  
   The /chosen node can contain a linux,input-keyset-sysrq child node to
   define a set of keys that will generate a sysrq when pressed together.
  
   Hmm, we would have only one such node, /sysrq, or /linux,sysrq,
   whatever. The sysrq setting is system-wide and applicable to all
   devices. Given that it is used only on mobile, where there not that
   many input devices (a few keys and touchscreen) I do not believe we
   should consider adding per-device settings.
 
  It's in /chosen, that isn't per-device.
 
   Required properties:
   keyset: array of keycodes
  
   Please, let's call it 'key-reset-seq', because it is exactly the reset
   sequence. There won't be any additional sequences or chords as those
   should be handled in userspace, sysrq is a special case here.
 
  This is absolutely a linux-specific binding. It encodes the Linux
  keycodes, and generates a linux meaning. I'm usually all about
  carrying the OS-independent banner when defining DT bindings, but in
  this case the linux prefix and sysrq reference is completely
  appropriate.

 OK, I have no idea what /chosen actually means. What I am trying to say
 that there should be either sysrq or linux,sysrq node and that is what
 sysrq driver will be looking for.



Chosen pertains to system wide parameters that aren't related to hw
specific devices.  Correct, the driver will look exactly for
linux,sysrq-reset-seq in the chosen node and nowhere else.



 The entire node is Linux-specific and therefore there is no point in
 marking only one of the properties (the key sequence) Linux-specific while
 leaving other ones generic.

 Thanks.

 --
 Dmitry

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 2/2] Input: Adding DT support for keyreset tuneables

2013-06-28 Thread Mathieu Poirier 
On 13-06-28 12:09 AM, Dmitry Torokhov wrote:
 On Thu, Jun 27, 2013 at 12:42:14PM -0600, Mathieu Poirier wrote:
 On 13-06-27 12:25 PM, Dmitry Torokhov wrote:
 On Thu, Jun 27, 2013 at 11:56:37AM -0600, Mathieu Poirier wrote:
 On 13-06-27 10:28 AM, Dmitry Torokhov wrote:
 Hi Mathieu,

 On Thu, Jun 27, 2013 at 10:13:25AM -0600, mathieu.poir...@linaro.org 
 wrote:
 From: Mathieu J. Poirier mathieu.poir...@linaro.org

 This patch adds the possibility to get the keyreset and timeout
 values from the device tree.

 Signed-off-by: Mathieu Poirier mathieu.poir...@linaro.org
 ---
  drivers/tty/sysrq.c |   54 
 +++
  1 file changed, 54 insertions(+)

 diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
 index b51c154..91d081c 100644
 --- a/drivers/tty/sysrq.c
 +++ b/drivers/tty/sysrq.c
 @@ -44,6 +44,7 @@
  #include linux/uaccess.h
  #include linux/moduleparam.h
  #include linux/jiffies.h
 +#include linux/of.h
  
  #include asm/ptrace.h
  #include asm/irq_regs.h
 @@ -671,6 +672,50 @@ static void sysrq_detect_reset_sequence(struct 
 sysrq_state *state,
  }
  }
  
 +static void sysrq_of_get_keyreset_config(void)
 +{
 +unsigned short key;
 +struct device_node *np;
 +const struct property *prop;
 +const __be32 *val;
 +int count, i;
 +
 +np = of_find_node_by_path(/sysrq);
 +if (!np) {
 +pr_info(No sysrq node found);

 I do not think this should be an info as majority would not have it
 defined I think.

 I fail to understand your point - could you please be more specific ?

 pr_info() will clutter everyone's dmesg because I expect majority of
 installs will not have this enabled. Please change to pr_debug or just
 drop it.

 Ah! Yes certainly.




 +goto out;
 +}
 +
 +prop = of_find_property(np, linux,input-keyset, NULL);

 Maybe linux,input-key*re*set?

 I do not agree.  We want the binding to be generic and not tied
 specifically to the keyreset functionality.  As such 'input-keyset' or
 'input-keychord' are more appropriate.

 The binding is defined specifically for sysrq and specifically to
 perform reset action.

 Yes for now but as the examples in the binding show, it is easy to
 envision how other drivers could use it.
 
 I think you over-complicate things here. Unlike matrix-keypad binding,
 where you have a common parsing code, here we have an individual driver.
 I really do not see anyone else using such sequences or chords as such
 processing should be done in userspace. Sysrq is quite an exception.

To be honest I don't have a very strong opinion on the binding.  I made
it as generic as possible on the guidance of the DT people.  Let's see
what they think of it.

 




 +if (!prop || !prop-value) {
 +pr_err(Invalid input-keyset);
 +goto out;
 +}
 +
 +count = prop-length / sizeof(u32);
 +val = prop-value;
 +
 +if (count  SYSRQ_KEY_RESET_MAX)
 +count = SYSRQ_KEY_RESET_MAX;
 +
 +/* reset in case a __weak definition was present */
 +sysrq_reset_seq_len = 0;

 Hmm, my preference for ordering would be software over firmware, so that
 user could override firmware data, if needed.

 The idea is to offer flexibility.  The same kernel can be used on
 multiple device.  As such DT information should be prioritised over a
 __weak symbol, otherwise it defeats the purpose.

 The weak symbol should defined in board data, so it won't get picked up
 on multiple boards. But I guess I do not care that much as indeed we can
 change the sequence from userspace so we won't be stuck with firmware
 choices.

 Humm... My reply wasn't clear enough.  I was thinking about the same
 board with different input device, i.e keypad or touchscreen.  In that
 case the board file would have been the same but not the keyreset
 sequence, which is exactly what the above ordering allows to do.
 
 That does not quite make sense as the only sequence we are parsing is
 the one in sysrq node, which is global.

Once again our opinion diverge.  It is entirely possible to use the same
kernel on different device (stemming from the same board file) with a
device tree to guide the configuration.

 
 Thanks.
 

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

[PATCH v2] Input: Add device tree bindings for input keys

2013-06-28 Thread mathieu . poirier 
From: Mathieu J. Poirier mathieu.poir...@linaro.org

This adds a simple device tree binding for the specification of
key sequences.  Definition of the keys found in the sequence are
located in 'include/uapi/linux/input.h'.

A keyset can be applied to a single device or the system as a
whole, depending on the associated driver.  An extention is also
provided for the definition of multiple keysets.

Lastly, the possibility to get keyreset timeout values from the
device tree based on the new binding is added.

Signed-off-by: Mathieu Poirier mathieu.poir...@linaro.org
---
changes for v2:
   - Moving to pr_debug when 'sysrq' node isn't found.
   - Merging binding description and code enhancement.
---
 .../devicetree/bindings/input/input-keyset.txt |   78 
 drivers/tty/sysrq.c|   54 ++
 2 files changed, 132 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/input-keyset.txt

diff --git a/Documentation/devicetree/bindings/input/input-keyset.txt 
b/Documentation/devicetree/bindings/input/input-keyset.txt
new file mode 100644
index 000..4f96299
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/input-keyset.txt
@@ -0,0 +1,78 @@
+Input: keyset
+
+A simple binding to represent a set of keys as described in
+include/uapi/linux/input.h.  This is targeted at devices that want
+to react to certain key combination being pressed or need to
+perform some configuration based on a set of input keys.
+
+It can also be used in a scenario where the system has a whole
+needs to respond to a specific sequence of keys.
+
+Required properties:
+ - linux,input-keyset: An array of 1-cell entries representing the
+   values associated with the KEY_xyz #defines found in input.h.
+
+Example1:
+Applicable to a specific device:
+
+matrix-keypad {
+compatible = gpio-matrix-keypad;
+debounce-delay-ms = 5;
+col-scan-delay-us = 2;
+
+row-gpios = gpio2 25 0
+ gpio2 26 0
+ gpio2 27 0;
+
+col-gpios = gpio2 21 0
+ gpio2 22 0
+ gpio2 23 0;
+
+linux,keymap = 0x0002
+0x0105
+0x0208
+0x00010003
+0x01010006
+0x02010009
+0x00020004
+0x01020007
+0x0202000a;
+
+linux,keyset = 0x04
+0x05
+0x0a;
+};
+
+Example2:
+Used as a system-wide parameter:
+
+   sysrq {
+   linux,input-keyset = 0x01
+ 0x0A
+ 0x19;
+   timout-ms = 3000;
+   };
+
+Would represent KEY_1, KEY_9 and KEY_P.
+
+Example3:
+Binding used when multiple declarations are needed:
+
+acme_keysets {
+keyset0 {
+linux,input-keyset = 0x01
+  0x02
+  0x03;
+};
+keyset1 {
+linux,input-keyset = 0x04
+  0x05
+  0x06;
+};
+keyset2 {
+linux,input-keyset = 0x07
+  0x08
+  0x09;
+};
+
+};
diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
index b51c154..9c20886 100644
--- a/drivers/tty/sysrq.c
+++ b/drivers/tty/sysrq.c
@@ -44,6 +44,7 @@
 #include linux/uaccess.h
 #include linux/moduleparam.h
 #include linux/jiffies.h
+#include linux/of.h
 
 #include asm/ptrace.h
 #include asm/irq_regs.h
@@ -671,6 +672,50 @@ static void sysrq_detect_reset_sequence(struct sysrq_state 
*state,
}
 }
 
+static void sysrq_of_get_keyreset_config(void)
+{
+   unsigned short key;
+   struct device_node *np;
+   const struct property *prop;
+   const __be32 *val;
+   int count, i;
+
+   np = of_find_node_by_path(/sysrq);
+   if (!np) {
+   pr_debug(No sysrq node found);
+   goto out;
+   }
+
+   prop = of_find_property(np, linux,input-keyset, NULL);
+   if (!prop || !prop-value) {
+   pr_err(Invalid input-keyset);
+   goto out;
+   }
+
+   count = prop-length / sizeof(u32);
+   val = prop-value;
+
+   if (count  SYSRQ_KEY_RESET_MAX)
+   count = SYSRQ_KEY_RESET_MAX;
+
+   /* reset in case a __weak definition was present */
+   sysrq_reset_seq_len = 0;
+
+   for (i = 0; i  count; i++) {
+   key = (unsigned short)be32_to_cpup(val++);
+   if (key == KEY_RESERVED || key  KEY_MAX)
+   break

[PATCH 2/2] Input: Adding DT support for keyreset tuneables

2013-06-27 Thread mathieu . poirier 
From: Mathieu J. Poirier mathieu.poir...@linaro.org

This patch adds the possibility to get the keyreset and timeout
values from the device tree.

Signed-off-by: Mathieu Poirier mathieu.poir...@linaro.org
---
 drivers/tty/sysrq.c |   54 +++
 1 file changed, 54 insertions(+)

diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
index b51c154..91d081c 100644
--- a/drivers/tty/sysrq.c
+++ b/drivers/tty/sysrq.c
@@ -44,6 +44,7 @@
 #include linux/uaccess.h
 #include linux/moduleparam.h
 #include linux/jiffies.h
+#include linux/of.h
 
 #include asm/ptrace.h
 #include asm/irq_regs.h
@@ -671,6 +672,50 @@ static void sysrq_detect_reset_sequence(struct sysrq_state 
*state,
}
 }
 
+static void sysrq_of_get_keyreset_config(void)
+{
+   unsigned short key;
+   struct device_node *np;
+   const struct property *prop;
+   const __be32 *val;
+   int count, i;
+
+   np = of_find_node_by_path(/sysrq);
+   if (!np) {
+   pr_info(No sysrq node found);
+   goto out;
+   }
+
+   prop = of_find_property(np, linux,input-keyset, NULL);
+   if (!prop || !prop-value) {
+   pr_err(Invalid input-keyset);
+   goto out;
+   }
+
+   count = prop-length / sizeof(u32);
+   val = prop-value;
+
+   if (count  SYSRQ_KEY_RESET_MAX)
+   count = SYSRQ_KEY_RESET_MAX;
+
+   /* reset in case a __weak definition was present */
+   sysrq_reset_seq_len = 0;
+
+   for (i = 0; i  count; i++) {
+   key = (unsigned short)be32_to_cpup(val++);
+   if (key == KEY_RESERVED || key  KEY_MAX)
+   break;
+
+   sysrq_reset_seq[sysrq_reset_seq_len++] = key;
+   }
+
+   /* get reset timeout if any */
+   of_property_read_u32(np, timeout-ms, sysrq_reset_downtime_ms);
+
+ out:
+   ;
+}
+
 static void sysrq_reinject_alt_sysrq(struct work_struct *work)
 {
struct sysrq_state *sysrq =
@@ -688,6 +733,7 @@ static void sysrq_reinject_alt_sysrq(struct work_struct 
*work)
input_inject_event(handle, EV_KEY, KEY_SYSRQ, 1);
input_inject_event(handle, EV_SYN, SYN_REPORT, 1);
 
+
input_inject_event(handle, EV_KEY, KEY_SYSRQ, 0);
input_inject_event(handle, EV_KEY, alt_code, 0);
input_inject_event(handle, EV_SYN, SYN_REPORT, 1);
@@ -903,6 +949,7 @@ static inline void sysrq_register_handler(void)
int error;
int i;
 
+   /* first check if a __weak interface was instantiated */
for (i = 0; i  ARRAY_SIZE(sysrq_reset_seq); i++) {
key = platform_sysrq_reset_seq[i];
if (key == KEY_RESERVED || key  KEY_MAX)
@@ -911,6 +958,13 @@ static inline void sysrq_register_handler(void)
sysrq_reset_seq[sysrq_reset_seq_len++] = key;
}
 
+   /*
+* DT configuration takes precedence over anything
+* that would have been defined via the __weak
+* interface
+*/
+   sysrq_of_get_keyreset_config();
+
error = input_register_handler(sysrq_handler);
if (error)
pr_err(Failed to register input handler, error %d, error);
-- 
1.7.9.5

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

[PATCH 1/2] Input: Add device tree bindings for input keys

2013-06-27 Thread mathieu . poirier 
From: Mathieu J. Poirier mathieu.poir...@linaro.org

This adds a simple device tree binding for the specification of
key sequences.  Definition of the keys found in the sequence are
located in 'include/uapi/linux/input.h'.

A keyset can be applied to a single device or the system as a
whole, depending on the associated driver.  An extention is also
provided for the definition of multiple keysets.

Signed-off-by: Mathieu Poirier mathieu.poir...@linaro.org
---
 .../devicetree/bindings/input/input-keyset.txt |   78 
 1 file changed, 78 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/input-keyset.txt

diff --git a/Documentation/devicetree/bindings/input/input-keyset.txt 
b/Documentation/devicetree/bindings/input/input-keyset.txt
new file mode 100644
index 000..4f96299
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/input-keyset.txt
@@ -0,0 +1,78 @@
+Input: keyset
+
+A simple binding to represent a set of keys as described in
+include/uapi/linux/input.h.  This is targeted at devices that want
+to react to certain key combination being pressed or need to
+perform some configuration based on a set of input keys.
+
+It can also be used in a scenario where the system has a whole
+needs to respond to a specific sequence of keys.
+
+Required properties:
+ - linux,input-keyset: An array of 1-cell entries representing the
+   values associated with the KEY_xyz #defines found in input.h.
+
+Example1:
+Applicable to a specific device:
+
+matrix-keypad {
+compatible = gpio-matrix-keypad;
+debounce-delay-ms = 5;
+col-scan-delay-us = 2;
+
+row-gpios = gpio2 25 0
+ gpio2 26 0
+ gpio2 27 0;
+
+col-gpios = gpio2 21 0
+ gpio2 22 0
+ gpio2 23 0;
+
+linux,keymap = 0x0002
+0x0105
+0x0208
+0x00010003
+0x01010006
+0x02010009
+0x00020004
+0x01020007
+0x0202000a;
+
+linux,keyset = 0x04
+0x05
+0x0a;
+};
+
+Example2:
+Used as a system-wide parameter:
+
+   sysrq {
+   linux,input-keyset = 0x01
+ 0x0A
+ 0x19;
+   timout-ms = 3000;
+   };
+
+Would represent KEY_1, KEY_9 and KEY_P.
+
+Example3:
+Binding used when multiple declarations are needed:
+
+acme_keysets {
+keyset0 {
+linux,input-keyset = 0x01
+  0x02
+  0x03;
+};
+keyset1 {
+linux,input-keyset = 0x04
+  0x05
+  0x06;
+};
+keyset2 {
+linux,input-keyset = 0x07
+  0x08
+  0x09;
+};
+
+};
-- 
1.7.9.5

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 2/2] Input: Adding DT support for keyreset tuneables

2013-06-27 Thread Mathieu Poirier 
On 13-06-27 10:28 AM, Dmitry Torokhov wrote:
 Hi Mathieu,
 
 On Thu, Jun 27, 2013 at 10:13:25AM -0600, mathieu.poir...@linaro.org wrote:
 From: Mathieu J. Poirier mathieu.poir...@linaro.org

 This patch adds the possibility to get the keyreset and timeout
 values from the device tree.

 Signed-off-by: Mathieu Poirier mathieu.poir...@linaro.org
 ---
  drivers/tty/sysrq.c |   54 
 +++
  1 file changed, 54 insertions(+)

 diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
 index b51c154..91d081c 100644
 --- a/drivers/tty/sysrq.c
 +++ b/drivers/tty/sysrq.c
 @@ -44,6 +44,7 @@
  #include linux/uaccess.h
  #include linux/moduleparam.h
  #include linux/jiffies.h
 +#include linux/of.h
  
  #include asm/ptrace.h
  #include asm/irq_regs.h
 @@ -671,6 +672,50 @@ static void sysrq_detect_reset_sequence(struct 
 sysrq_state *state,
  }
  }
  
 +static void sysrq_of_get_keyreset_config(void)
 +{
 +unsigned short key;
 +struct device_node *np;
 +const struct property *prop;
 +const __be32 *val;
 +int count, i;
 +
 +np = of_find_node_by_path(/sysrq);
 +if (!np) {
 +pr_info(No sysrq node found);
 
 I do not think this should be an info as majority would not have it
 defined I think.

I fail to understand your point - could you please be more specific ?

 
 +goto out;
 +}
 +
 +prop = of_find_property(np, linux,input-keyset, NULL);
 
 Maybe linux,input-key*re*set?

I do not agree.  We want the binding to be generic and not tied
specifically to the keyreset functionality.  As such 'input-keyset' or
'input-keychord' are more appropriate.

 
 +if (!prop || !prop-value) {
 +pr_err(Invalid input-keyset);
 +goto out;
 +}
 +
 +count = prop-length / sizeof(u32);
 +val = prop-value;
 +
 +if (count  SYSRQ_KEY_RESET_MAX)
 +count = SYSRQ_KEY_RESET_MAX;
 +
 +/* reset in case a __weak definition was present */
 +sysrq_reset_seq_len = 0;
 
 Hmm, my preference for ordering would be software over firmware, so that
 user could override firmware data, if needed.

The idea is to offer flexibility.  The same kernel can be used on
multiple device.  As such DT information should be prioritised over a
__weak symbol, otherwise it defeats the purpose.

Once the system is loaded user can still configure the keyreset
specifics by using the /sysfs interface.

 
 Please also add the documenation describing the binding.

The documentation describing the binding is in patch 1/2.  You suggest
that I add the documentation in this patch too ?

 
 Thanks.
 

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 2/2] Input: Adding DT support for keyreset tuneables

2013-06-27 Thread Mathieu Poirier 
On 13-06-27 12:25 PM, Dmitry Torokhov wrote:
 On Thu, Jun 27, 2013 at 11:56:37AM -0600, Mathieu Poirier wrote:
 On 13-06-27 10:28 AM, Dmitry Torokhov wrote:
 Hi Mathieu,

 On Thu, Jun 27, 2013 at 10:13:25AM -0600, mathieu.poir...@linaro.org wrote:
 From: Mathieu J. Poirier mathieu.poir...@linaro.org

 This patch adds the possibility to get the keyreset and timeout
 values from the device tree.

 Signed-off-by: Mathieu Poirier mathieu.poir...@linaro.org
 ---
  drivers/tty/sysrq.c |   54 
 +++
  1 file changed, 54 insertions(+)

 diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
 index b51c154..91d081c 100644
 --- a/drivers/tty/sysrq.c
 +++ b/drivers/tty/sysrq.c
 @@ -44,6 +44,7 @@
  #include linux/uaccess.h
  #include linux/moduleparam.h
  #include linux/jiffies.h
 +#include linux/of.h
  
  #include asm/ptrace.h
  #include asm/irq_regs.h
 @@ -671,6 +672,50 @@ static void sysrq_detect_reset_sequence(struct 
 sysrq_state *state,
}
  }
  
 +static void sysrq_of_get_keyreset_config(void)
 +{
 +  unsigned short key;
 +  struct device_node *np;
 +  const struct property *prop;
 +  const __be32 *val;
 +  int count, i;
 +
 +  np = of_find_node_by_path(/sysrq);
 +  if (!np) {
 +  pr_info(No sysrq node found);

 I do not think this should be an info as majority would not have it
 defined I think.

 I fail to understand your point - could you please be more specific ?
 
 pr_info() will clutter everyone's dmesg because I expect majority of
 installs will not have this enabled. Please change to pr_debug or just
 drop it.

Ah! Yes certainly.

 


 +  goto out;
 +  }
 +
 +  prop = of_find_property(np, linux,input-keyset, NULL);

 Maybe linux,input-key*re*set?

 I do not agree.  We want the binding to be generic and not tied
 specifically to the keyreset functionality.  As such 'input-keyset' or
 'input-keychord' are more appropriate.
 
 The binding is defined specifically for sysrq and specifically to
 perform reset action.

Yes for now but as the examples in the binding show, it is easy to
envision how other drivers could use it.

 


 +  if (!prop || !prop-value) {
 +  pr_err(Invalid input-keyset);
 +  goto out;
 +  }
 +
 +  count = prop-length / sizeof(u32);
 +  val = prop-value;
 +
 +  if (count  SYSRQ_KEY_RESET_MAX)
 +  count = SYSRQ_KEY_RESET_MAX;
 +
 +  /* reset in case a __weak definition was present */
 +  sysrq_reset_seq_len = 0;

 Hmm, my preference for ordering would be software over firmware, so that
 user could override firmware data, if needed.

 The idea is to offer flexibility.  The same kernel can be used on
 multiple device.  As such DT information should be prioritised over a
 __weak symbol, otherwise it defeats the purpose.
 
 The weak symbol should defined in board data, so it won't get picked up
 on multiple boards. But I guess I do not care that much as indeed we can
 change the sequence from userspace so we won't be stuck with firmware
 choices.

Humm... My reply wasn't clear enough.  I was thinking about the same
board with different input device, i.e keypad or touchscreen.  In that
case the board file would have been the same but not the keyreset
sequence, which is exactly what the above ordering allows to do.  But
it's ok, we agree.

 

 Once the system is loaded user can still configure the keyreset
 specifics by using the /sysfs interface.


 Please also add the documenation describing the binding.

 The documentation describing the binding is in patch 1/2.  You suggest
 that I add the documentation in this patch too ?
 
 I simply missed your other patch as it hit my mailbox after this one.
 But I would just combine the 2 together.

I will gladly do that.

 

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss