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

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

Hi Mathieu,

On Thu, Jun 27, 2013 at 10:13:25AM -0600, mathieu.poirier@linaro.org wrote:
> 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.

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

Maybe "linux,input-key*re*set"?

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

Please also add the documenation describing the binding.

Thanks.

-- 
Dmitry