[U-Boot] input: simplify key_matrix_decode_fdt()

From: Stephen Warren <swarren@nvidia.com>

We know the exact property names that the code wants to process. Look
these up directly with fdt_get_property(), rather than iterating over
all properties within the node, and checking each property's name, in
a convoluted fashion, against the expected name.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
Note: This depends on my previous patch "input: fix unaligned access
in key_matrix_decode_fdt()". While to two patches could be squashed
together, I'd prefer them to go in separately, since the former is a
bug-fix that makes the original code work again on ARMv7 at least, and
this patch is an unrelated refactoring.
---
 drivers/input/key_matrix.c |   66 +++++++++++++++++---------------------------
 1 file changed, 26 insertions(+), 40 deletions(-)

Hi Stephen,

On Thu, May 23, 2013 at 3:09 PM, Stephen Warren <swarren@wwwdotorg.org>wrote:

> From: Stephen Warren <swarren@nvidia.com>
>
> We know the exact property names that the code wants to process. Look
> these up directly with fdt_get_property(), rather than iterating over
> all properties within the node, and checking each property's name, in
> a convoluted fashion, against the expected name.
>

The original code dealt with ctrl and shift also - since removed. I think
it is good to simplify it.


>
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> ---
> Note: This depends on my previous patch "input: fix unaligned access
> in key_matrix_decode_fdt()". While to two patches could be squashed
> together, I'd prefer them to go in separately, since the former is a
> bug-fix that makes the original code work again on ARMv7 at least, and
> this patch is an unrelated refactoring.
>

Yes.


> ---
>  drivers/input/key_matrix.c |   66
> +++++++++++++++++---------------------------
>  1 file changed, 26 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/input/key_matrix.c b/drivers/input/key_matrix.c
> index c8b048e..ea05c77 100644
> --- a/drivers/input/key_matrix.c
> +++ b/drivers/input/key_matrix.c
> @@ -154,54 +154,40 @@ static uchar *create_keymap(struct key_matrix
> *config, u32 *data, int len,
>         return map;
>  }
>
> -int key_matrix_decode_fdt(struct key_matrix *config, const void *blob,
> -                         int node)
> +int key_matrix_decode_fdt(struct key_matrix *config, const void *blob,
> int node)
>  {
>         const struct fdt_property *prop;
> -       static const char prefix[] = "linux,";
> -       int plen = sizeof(prefix) - 1;
> -       int offset;
> -
> -       /* Check each property name for ones that we understand */
> -       for (offset = fdt_first_property_offset(blob, node);
> -                     offset > 0;
> -                     offset = fdt_next_property_offset(blob, offset)) {
> -               const char *name;
> -               int len;
> -
> -               prop = fdt_get_property_by_offset(blob, offset, NULL);
> -               name = fdt_string(blob, fdt32_to_cpu(prop->nameoff));
> -               len = strlen(name);
> -
> -               /* Name needs to match "1,<type>keymap" */
> -               debug("%s: property '%s'\n", __func__, name);
> -               if (strncmp(name, prefix, plen) ||
> -                               len < plen + 6 ||
> -                               strcmp(name + len - 6, "keymap"))
> -                       continue;
> +       int proplen;
> +       uchar *plain_keycode;
>
> -               len -= plen + 6;
> -               if (len == 0) {
> -                       config->plain_keycode = create_keymap(config,
> -                               (u32 *)prop->data, fdt32_to_cpu(prop->len),
> -                               KEY_FN, &config->fn_pos);
> -               } else if (0 == strncmp(name + plen, "fn-", len)) {
> -                       config->fn_keycode = create_keymap(config,
> -                               (u32 *)prop->data, fdt32_to_cpu(prop->len),
> -                               -1, NULL);
> -               } else {
> -                       debug("%s: unrecognised property '%s'\n", __func__,
> -                             name);
> -               }
> -       }
> -       debug("%s: Decoded key maps %p, %p from fdt\n", __func__,
> -             config->plain_keycode, config->fn_keycode);
> +       prop = fdt_get_property(blob, node, "linux,keymap", &proplen);
> +       /* Basic keymap is required */
> +       if (!prop)
> +               return -1;
>
> +       plain_keycode = create_keymap(config, (u32 *)prop->data,
> +               proplen, KEY_FN, &config->fn_pos);
>

Probably don't need plain_keycode variable at all.


> +       config->plain_keycode = plain_keycode;
> +       /* Conversion error -> fail */
> +       if (!config->plain_keycode)
> +               return -1;
>

Missing debug() here from old code


> +
> +       prop = fdt_get_property(blob, node, "linux,fn-keymap", &proplen);
> +       /* fn keymap is optional */
> +       if (!prop)
> +               goto done;
> +
> +       config->fn_keycode = create_keymap(config, (u32 *)prop->data,
> +               proplen, -1, NULL);
> +       /* Conversion error -> fail */
>         if (!config->plain_keycode) {
>

Should check config->fn_keycode here.


> -               debug("%s: cannot find keycode-plain map\n", __func__);
> +               free(plain_keycode);
>                 return -1;
>         }
>
> +done:
> +       debug("%s: Decoded key maps %p, %p from fdt\n", __func__,
> +             config->plain_keycode, config->fn_keycode);
>         return 0;
>  }
>
> --
> 1.7.10.4
>
>

Regards,
Simon

On 05/26/2013 01:31 PM, Simon Glass wrote:
> Hi Stephen,
> 
> On Thu, May 23, 2013 at 3:09 PM, Stephen Warren <swarren@wwwdotorg.org
> <mailto:swarren@wwwdotorg.org>> wrote:
> 
>     From: Stephen Warren <swarren@nvidia.com <mailto:swarren@nvidia.com>>
> 
>     We know the exact property names that the code wants to process. Look
>     these up directly with fdt_get_property(), rather than iterating over
>     all properties within the node, and checking each property's name, in
>     a convoluted fashion, against the expected name.
>     +       plain_keycode = create_keymap(config, (u32 *)prop->data,
>     +               proplen, KEY_FN, &config->fn_pos);
> 
> Probably don't need plain_keycode variable at all.

This is required because the variable is passed to free() later, and
config->plain_keycode is marked const, whereas free isn't prototyped to
take a const. I figured that it was simplest to use a separate variable
here rather than cast away the const when calling free(). Now, if C had
const_cast<>, then I would have made a different decision:-)

On 05/23/2013 04:09 PM, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
> 
> We know the exact property names that the code wants to process. Look
> these up directly with fdt_get_property(), rather than iterating over
> all properties within the node, and checking each property's name, in
> a convoluted fashion, against the expected name.

Tom, is this likely to be applied for the upcoming release, or would it
be deferred until the next? Thanks.

On Thu, May 23, 2013 at 12:09:57PM -0000, Stephen Warren wrote:

> From: Stephen Warren <swarren@nvidia.com>
> 
> We know the exact property names that the code wants to process. Look
> these up directly with fdt_get_property(), rather than iterating over
> all properties within the node, and checking each property's name, in
> a convoluted fashion, against the expected name.
> 
> Signed-off-by: Stephen Warren <swarren@nvidia.com>

Applied to u-boot/master, thanks!