| Message ID | 20200324123518.239768-4-rajatja@google.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,1/5] input/serio/i8042: Attach fwnode to serio i8042 kbd device | expand |
Hi Rajat, On Tue, Mar 24, 2020 at 05:35:17AM -0700, Rajat Jain wrote: > Allow the firmware to specify the mapping between the physical > code and the linux keycode. This takes the form of a "keymap" > property which is an array of u32 values, each value specifying > mapping for a key. > > Signed-off-by: Rajat Jain <rajatja@google.com> > --- > v2: Remove the Change-Id from the commit log > > drivers/input/keyboard/atkbd.c | 39 ++++++++++++++++++++++++++++++++-- > 1 file changed, 37 insertions(+), 2 deletions(-) > > diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c > index 7623eebef2593..c8017a5707581 100644 > --- a/drivers/input/keyboard/atkbd.c > +++ b/drivers/input/keyboard/atkbd.c > @@ -66,6 +66,9 @@ MODULE_PARM_DESC(terminal, "Enable break codes on an IBM Terminal keyboard conne > > #define MAX_FUNCTION_ROW_KEYS 24 > > +#define PHYSCODE(keymap) ((keymap >> 16) & 0xFFFF) > +#define KEYCODE(keymap) (keymap & 0xFFFF) > + > /* > * Scancode to keycode tables. These are just the default setting, and > * are loadable via a userland utility. > @@ -236,6 +239,9 @@ struct atkbd { > > u16 function_row_physmap[MAX_FUNCTION_ROW_KEYS]; > int num_function_row_keys; > + > + unsigned short fw_keymap[ATKBD_KEYMAP_SIZE]; > + bool use_fw_keymap; Why do we need to keep firmware-provided keymap in atkbd instance? It is not going anywhere and can be accessed via device_property_* API whenever we decide to refresh the keymap. > }; > > /* > @@ -1045,7 +1051,10 @@ static void atkbd_set_keycode_table(struct atkbd *atkbd) > memset(atkbd->keycode, 0, sizeof(atkbd->keycode)); > bitmap_zero(atkbd->force_release_mask, ATKBD_KEYMAP_SIZE); > > - if (atkbd->translated) { > + if (atkbd->use_fw_keymap) { > + memcpy(atkbd->keycode, atkbd->fw_keymap, > + sizeof(atkbd->keycode)); > + } else if (atkbd->translated) { > for (i = 0; i < 128; i++) { > scancode = atkbd_unxlate_table[i]; > atkbd->keycode[i] = atkbd_set2_keycode[scancode]; > @@ -1163,7 +1172,9 @@ static void atkbd_parse_fwnode_data(struct serio *serio) > { > struct atkbd *atkbd = serio_get_drvdata(serio); > struct device *dev = &serio->dev; > - int n; > + int i, n; > + u32 *ptr; > + u16 physcode, keycode; > > if (!dev_fwnode(dev)) > return; > @@ -1176,6 +1187,30 @@ static void atkbd_parse_fwnode_data(struct serio *serio) > atkbd->num_function_row_keys = n; > dev_info(dev, "FW reported %d function-row key locations\n", n); > } > + > + /* Parse "keymap" property */ > + n = device_property_count_u32(dev, "keymap"); > + if (n > 0 && n <= ATKBD_KEYMAP_SIZE) { > + > + ptr = kcalloc(n, sizeof(u32), GFP_KERNEL); > + if (!ptr) > + return; > + > + if (device_property_read_u32_array(dev, "keymap", ptr, n)) { > + dev_err(dev, "problem parsing FW keymap property\n"); > + kfree(ptr); > + return; > + } > + > + for (i = 0; i < n; i++) { > + physcode = PHYSCODE(ptr[i]); > + keycode = KEYCODE(ptr[i]); > + atkbd->fw_keymap[physcode] = keycode; > + } > + dev_info(dev, "Using FW keymap (%d keys)\n", n); This should be dev_dbg(). > + atkbd->use_fw_keymap = true; > + kfree(ptr); > + } > } > > /* > -- > 2.25.1.696.g5e7596f4ac-goog > Thanks.
On Thu, Mar 26, 2020 at 2:20 PM Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote: > > Hi Rajat, > > On Tue, Mar 24, 2020 at 05:35:17AM -0700, Rajat Jain wrote: > > Allow the firmware to specify the mapping between the physical > > code and the linux keycode. This takes the form of a "keymap" > > property which is an array of u32 values, each value specifying > > mapping for a key. > > > > Signed-off-by: Rajat Jain <rajatja@google.com> > > --- > > v2: Remove the Change-Id from the commit log > > > > drivers/input/keyboard/atkbd.c | 39 ++++++++++++++++++++++++++++++++-- > > 1 file changed, 37 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c > > index 7623eebef2593..c8017a5707581 100644 > > --- a/drivers/input/keyboard/atkbd.c > > +++ b/drivers/input/keyboard/atkbd.c > > @@ -66,6 +66,9 @@ MODULE_PARM_DESC(terminal, "Enable break codes on an IBM Terminal keyboard conne > > > > #define MAX_FUNCTION_ROW_KEYS 24 > > > > +#define PHYSCODE(keymap) ((keymap >> 16) & 0xFFFF) > > +#define KEYCODE(keymap) (keymap & 0xFFFF) > > + > > /* > > * Scancode to keycode tables. These are just the default setting, and > > * are loadable via a userland utility. > > @@ -236,6 +239,9 @@ struct atkbd { > > > > u16 function_row_physmap[MAX_FUNCTION_ROW_KEYS]; > > int num_function_row_keys; > > + > > + unsigned short fw_keymap[ATKBD_KEYMAP_SIZE]; > > + bool use_fw_keymap; > > Why do we need to keep firmware-provided keymap in atkbd instance? It is > not going anywhere and can be accessed via device_property_* API > whenever we decide to refresh the keymap. Done. I've sent out a new v3 patchset for review with this change. > > > }; > > > > /* > > @@ -1045,7 +1051,10 @@ static void atkbd_set_keycode_table(struct atkbd *atkbd) > > memset(atkbd->keycode, 0, sizeof(atkbd->keycode)); > > bitmap_zero(atkbd->force_release_mask, ATKBD_KEYMAP_SIZE); > > > > - if (atkbd->translated) { > > + if (atkbd->use_fw_keymap) { > > + memcpy(atkbd->keycode, atkbd->fw_keymap, > > + sizeof(atkbd->keycode)); > > + } else if (atkbd->translated) { > > for (i = 0; i < 128; i++) { > > scancode = atkbd_unxlate_table[i]; > > atkbd->keycode[i] = atkbd_set2_keycode[scancode]; > > @@ -1163,7 +1172,9 @@ static void atkbd_parse_fwnode_data(struct serio *serio) > > { > > struct atkbd *atkbd = serio_get_drvdata(serio); > > struct device *dev = &serio->dev; > > - int n; > > + int i, n; > > + u32 *ptr; > > + u16 physcode, keycode; > > > > if (!dev_fwnode(dev)) > > return; > > @@ -1176,6 +1187,30 @@ static void atkbd_parse_fwnode_data(struct serio *serio) > > atkbd->num_function_row_keys = n; > > dev_info(dev, "FW reported %d function-row key locations\n", n); > > } > > + > > + /* Parse "keymap" property */ > > + n = device_property_count_u32(dev, "keymap"); > > + if (n > 0 && n <= ATKBD_KEYMAP_SIZE) { > > + > > + ptr = kcalloc(n, sizeof(u32), GFP_KERNEL); > > + if (!ptr) > > + return; > > + > > + if (device_property_read_u32_array(dev, "keymap", ptr, n)) { > > + dev_err(dev, "problem parsing FW keymap property\n"); > > + kfree(ptr); > > + return; > > + } > > + > > + for (i = 0; i < n; i++) { > > + physcode = PHYSCODE(ptr[i]); > > + keycode = KEYCODE(ptr[i]); > > + atkbd->fw_keymap[physcode] = keycode; > > + } > > + dev_info(dev, "Using FW keymap (%d keys)\n", n); > > This should be dev_dbg(). Done. Thanks, Rajat > > > + atkbd->use_fw_keymap = true; > > + kfree(ptr); > > + } > > } > > > > /* > > -- > > 2.25.1.696.g5e7596f4ac-goog > > > > Thanks. > > -- > Dmitry
diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c index 7623eebef2593..c8017a5707581 100644 --- a/drivers/input/keyboard/atkbd.c +++ b/drivers/input/keyboard/atkbd.c @@ -66,6 +66,9 @@ MODULE_PARM_DESC(terminal, "Enable break codes on an IBM Terminal keyboard conne #define MAX_FUNCTION_ROW_KEYS 24 +#define PHYSCODE(keymap) ((keymap >> 16) & 0xFFFF) +#define KEYCODE(keymap) (keymap & 0xFFFF) + /* * Scancode to keycode tables. These are just the default setting, and * are loadable via a userland utility. @@ -236,6 +239,9 @@ struct atkbd { u16 function_row_physmap[MAX_FUNCTION_ROW_KEYS]; int num_function_row_keys; + + unsigned short fw_keymap[ATKBD_KEYMAP_SIZE]; + bool use_fw_keymap; }; /* @@ -1045,7 +1051,10 @@ static void atkbd_set_keycode_table(struct atkbd *atkbd) memset(atkbd->keycode, 0, sizeof(atkbd->keycode)); bitmap_zero(atkbd->force_release_mask, ATKBD_KEYMAP_SIZE); - if (atkbd->translated) { + if (atkbd->use_fw_keymap) { + memcpy(atkbd->keycode, atkbd->fw_keymap, + sizeof(atkbd->keycode)); + } else if (atkbd->translated) { for (i = 0; i < 128; i++) { scancode = atkbd_unxlate_table[i]; atkbd->keycode[i] = atkbd_set2_keycode[scancode]; @@ -1163,7 +1172,9 @@ static void atkbd_parse_fwnode_data(struct serio *serio) { struct atkbd *atkbd = serio_get_drvdata(serio); struct device *dev = &serio->dev; - int n; + int i, n; + u32 *ptr; + u16 physcode, keycode; if (!dev_fwnode(dev)) return; @@ -1176,6 +1187,30 @@ static void atkbd_parse_fwnode_data(struct serio *serio) atkbd->num_function_row_keys = n; dev_info(dev, "FW reported %d function-row key locations\n", n); } + + /* Parse "keymap" property */ + n = device_property_count_u32(dev, "keymap"); + if (n > 0 && n <= ATKBD_KEYMAP_SIZE) { + + ptr = kcalloc(n, sizeof(u32), GFP_KERNEL); + if (!ptr) + return; + + if (device_property_read_u32_array(dev, "keymap", ptr, n)) { + dev_err(dev, "problem parsing FW keymap property\n"); + kfree(ptr); + return; + } + + for (i = 0; i < n; i++) { + physcode = PHYSCODE(ptr[i]); + keycode = KEYCODE(ptr[i]); + atkbd->fw_keymap[physcode] = keycode; + } + dev_info(dev, "Using FW keymap (%d keys)\n", n); + atkbd->use_fw_keymap = true; + kfree(ptr); + } } /*
Allow the firmware to specify the mapping between the physical code and the linux keycode. This takes the form of a "keymap" property which is an array of u32 values, each value specifying mapping for a key. Signed-off-by: Rajat Jain <rajatja@google.com> --- v2: Remove the Change-Id from the commit log drivers/input/keyboard/atkbd.c | 39 ++++++++++++++++++++++++++++++++-- 1 file changed, 37 insertions(+), 2 deletions(-)