[U-Boot] [PATCH] 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(-)
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); + config->plain_keycode = plain_keycode; + /* Conversion error -> fail */ + if (!config->plain_keycode) + return -1; + + 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) { - 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; }
Hi Stephen,
On Thu, May 23, 2013 at 3:09 PM, Stephen Warren swarren@wwwdotorg.orgwrote:
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!
participants (3)
-
Simon Glass -
Stephen Warren -
Tom Rini