On 13-06-27 10:28 AM, Dmitry Torokhov wrote:

Hi Mathieu,

On Thu, Jun 27, 2013 at 10:13:25AM -0600, mathieu.poirier@linaro.org wrote:

quoted

From: "Mathieu J. Poirier" <mathieu.poirier@linaro.org>

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

Signed-off-by: Mathieu Poirier <mathieu.poirier@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 ?

quoted

+		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.

quoted

+	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.