We don't need to allocate memory for keymap in matrix_keyboard_of_fill_keymap(),
as this would only be used by matrix_keyboard_of_free_keymap(). Instead create
another routine matrix_keypad_of_build_keymap() which reads directly the
property from struct device_node and builds keymap.
With this eariler routines matrix_keyboard_of_fill_keymap() and
matrix_keyboard_of_free_keymap() go away.
This patch also fixes tegra driver according to these changes.
Signed-off-by: Viresh Kumar <redacted>
---
drivers/input/keyboard/tegra-kbc.c | 48 ++++++++++--------
drivers/input/of_keymap.c | 94 ++++++++++++++++++++---------------
include/linux/input/matrix_keypad.h | 16 ++----
3 files changed, 85 insertions(+), 73 deletions(-)
@@ -26,62 +27,75 @@#include<linux/gfp.h>#include<linux/slab.h>-structmatrix_keymap_data*-matrix_keyboard_of_fill_keymap(structdevice_node*np,-constchar*propname)+/**+*matrix_keypad_of_build_keymap-convertplatformDTkeymapintomatrixkeymap+*@idev:pointertostructinput_dev;usedforgettingkeycode,keybitand+*keycodemax.+*@row_shift:numberofbitstoshiftrowvaluebytoadvancetothenext+*lineinthekeymap+*@propname:DeviceTreepropertynametobeusedforreadingkeymap.Ifpassed+*asNULL,"linux,keymap"isused.+*+*Thisfunctioncreatesanarrayofkeycodes,byreadingpropnamepropertyfrom+*devicetreepassed,thatissuitableforusinginastandardmatrixkeyboard+*driverthatusesrowandcolasindices.+*+*Expectationfromuserdriver:idevmustbeinitializedwithfollowingfields:+*dev.parent,keycode,keybitandkeycodemax.+*/+intmatrix_keypad_of_build_keymap(structinput_dev*idev,+unsignedintrow_shift,constchar*propname){-structmatrix_keymap_data*kd;-u32*keymap;-intproplen,i;+structdevice*dev=idev->dev.parent;+structdevice_node*np=dev->of_node;const__be32*prop;+unsignedintproplen,i,size,col_range=1<<row_shift;+unsignedshort*keycode;-if(!np)-returnNULL;+if(!np||!idev)+return-ENODEV;if(!propname)propname="linux,keymap";prop=of_get_property(np,propname,&proplen);-if(!prop)-returnNULL;+if(!prop){+dev_err(dev,"OF: %s property not defined in %s\n",propname,+np->full_name);+return-ENODEV;+}if(proplen%sizeof(u32)){-pr_warn("Malformed keymap property %s in %s\n",-propname,np->full_name);-returnNULL;+dev_warn(dev,"Malformed keycode property %s in %s\n",propname,+np->full_name);+return-EINVAL;}-kd=kzalloc(sizeof(*kd),GFP_KERNEL);-if(!kd)-returnNULL;--kd->keymap=keymap=kzalloc(proplen,GFP_KERNEL);-if(!kd->keymap){-kfree(kd);-returnNULL;+size=proplen/sizeof(u32);+if(size>idev->keycodemax){+dev_err(dev,"OF: %s overflow\n",propname);+return-EINVAL;}-kd->keymap_size=proplen/sizeof(u32);+keycode=idev->keycode;-for(i=0;i<kd->keymap_size;i++){-u32tmp=be32_to_cpup(prop+i);-intkey_code,row,col;+for(i=0;i<size;i++){+unsignedintkey=be32_to_cpup(prop+i);+unsignedintrow=KEY_ROW(key);+unsignedintcol=KEY_COL(key);+unsignedshortcode=KEY_VAL(key);-row=(tmp>>24)&0xff;-col=(tmp>>16)&0xff;-key_code=tmp&0xffff;-keymap[i]=KEY(row,col,key_code);-}--returnkd;-}-EXPORT_SYMBOL_GPL(matrix_keyboard_of_fill_keymap);+if(col>=col_range){+dev_err(dev,"OF: %s: column %x overflowed its range %d\n",+propname,col,col_range);+return-EINVAL;+}-voidmatrix_keyboard_of_free_keymap(conststructmatrix_keymap_data*kd)-{-if(kd){-kfree(kd->keymap);-kfree(kd);+keycode[MATRIX_SCAN_CODE(row,col,row_shift)]=code;+__set_bit(code,idev->keybit);}+__clear_bit(KEY_RESERVED,idev->keybit);++return0;}-EXPORT_SYMBOL_GPL(matrix_keyboard_of_free_keymap);+EXPORT_SYMBOL_GPL(matrix_keypad_of_build_keymap);
From: Stephen Warren <hidden> Date: 2012-03-29 15:44:35
On 03/29/2012 02:33 AM, Viresh Kumar wrote:
We don't need to allocate memory for keymap in matrix_keyboard_of_fill_keymap(),
as this would only be used by matrix_keyboard_of_free_keymap(). Instead create
another routine matrix_keypad_of_build_keymap() which reads directly the
property from struct device_node and builds keymap.
With this eariler routines matrix_keyboard_of_fill_keymap() and
matrix_keyboard_of_free_keymap() go away.
This patch also fixes tegra driver according to these changes.
That is checking the number of entries in the property, not the values
of the MATRIX_SCAN_CODE values derived from those entries. I'd say just
remove this check. See below.
+ keycode = idev->keycode;
+ for (i = 0; i < size; i++) {
+ unsigned int key = be32_to_cpup(prop + i);
+ unsigned int row = KEY_ROW(key);
+ unsigned int col = KEY_COL(key);
+ unsigned short code = KEY_VAL(key);
+ if (col >= col_range) {
+ dev_err(dev, "OF: %s: column %x overflowed its range %d\n",
+ propname, col, col_range);
+ return -EINVAL;
+ }
But, you also need to do something like:
scancode = MATRIX_SCAN_CODE(row, col, row_shift);
if (scancode >= idev->keycodemax) {
error out;
}
keycode[scancode] = code;
That is checking the number of entries in the property, not the values
of the MATRIX_SCAN_CODE values derived from those entries. I'd say just
remove this check. See below.
That is checking the number of entries in the property, not the values
of the MATRIX_SCAN_CODE values derived from those entries. I'd say just
remove this check. See below.
Stephen,
I have added a check on return value of MATRIX_SCAN_CODE(), but
would still keep above check. Number of keys passed should
also be less than keycodemax.
--
viresh
Hi Dmitry,
On Wed, May 9, 2012 at 10:58 AM, Dmitry Torokhov
[off-list ref] wrote:
Now that I got matrix_keypad_build_keymap() also handle DT case, how
about the patch below?
Input: spear-keyboard - add device tree bindings
From: Viresh Kumar <redacted>
This adds simple DT bindings for spear-keyboard controller.
Signed-off-by: Viresh Kumar <redacted>
Signed-off-by: Dmitry Torokhov <redacted>
---
drivers/input/keyboard/spear-keyboard.c | 71 ++++++++++++++++++++++++-------
1 files changed, 56 insertions(+), 15 deletions(-)
You missed following file in this patch, which was there in original patch.
Documentation/devicetree/bindings/input/spear-keyboard.txt
--
viresh
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, May 15, 2012 at 12:24:11PM +0530, viresh kumar wrote:
Hi Dmitry,
On Wed, May 9, 2012 at 10:58 AM, Dmitry Torokhov
[off-list ref] wrote:
quoted
Now that I got matrix_keypad_build_keymap() also handle DT case, how
about the patch below?
Input: spear-keyboard - add device tree bindings
From: Viresh Kumar <redacted>
This adds simple DT bindings for spear-keyboard controller.
Signed-off-by: Viresh Kumar <redacted>
Signed-off-by: Dmitry Torokhov <redacted>
---
drivers/input/keyboard/spear-keyboard.c | 71 ++++++++++++++++++++++++-------
1 files changed, 56 insertions(+), 15 deletions(-)
You missed following file in this patch, which was there in original patch.
Documentation/devicetree/bindings/input/spear-keyboard.txt
Oh, yes, sorry about that - that is danges of stgit - if patch does not
import cleanly for some reason (local changes in the tree for example)
and I have to massage and apply the patch then new files are not added
to git automatically and I sometimes forget about them. I'll ressurect
the doc change.
Thanks.
--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
We don't need to allocate memory for keymap in matrix_keyboard_of_fill_keymap(),
as this would only be used by matrix_keyboard_of_free_keymap(). Instead create
another routine matrix_keypad_of_build_keymap() which reads directly the
property from struct device_node and builds keymap.
With this eariler routines matrix_keyboard_of_fill_keymap() and
matrix_keyboard_of_free_keymap() go away.
This patch also fixes tegra driver according to these changes.
Signed-off-by: Viresh Kumar <redacted>
---
V4:
- check return value of MATRIX_SCAN_CODE() to guarantee that it doesn't overflow
keycodemax sized buffer.
drivers/input/keyboard/tegra-kbc.c | 48 ++++++++++--------
drivers/input/of_keymap.c | 98 ++++++++++++++++++++--------------
include/linux/input/matrix_keypad.h | 16 ++----
3 files changed, 89 insertions(+), 73 deletions(-)
@@ -26,62 +27,79 @@#include<linux/gfp.h>#include<linux/slab.h>-structmatrix_keymap_data*-matrix_keyboard_of_fill_keymap(structdevice_node*np,-constchar*propname)+/**+*matrix_keypad_of_build_keymap-convertplatformDTkeymapintomatrixkeymap+*@idev:pointertostructinput_dev;usedforgettingkeycode,keybitand+*keycodemax.+*@row_shift:numberofbitstoshiftrowvaluebytoadvancetothenext+*lineinthekeymap+*@propname:DeviceTreepropertynametobeusedforreadingkeymap.Ifpassed+*asNULL,"linux,keymap"isused.+*+*Thisfunctioncreatesanarrayofkeycodes,byreadingpropnamepropertyfrom+*devicetreepassed,thatissuitableforusinginastandardmatrixkeyboard+*driverthatusesrowandcolasindices.+*+*Expectationfromuserdriver:idevmustbeinitializedwithfollowingfields:+*dev.parent,keycode,keybitandkeycodemax.+*/+intmatrix_keypad_of_build_keymap(structinput_dev*idev,+unsignedintrow_shift,constchar*propname){-structmatrix_keymap_data*kd;-u32*keymap;-intproplen,i;+structdevice*dev=idev->dev.parent;+structdevice_node*np=dev->of_node;+unsignedshort*keycode;const__be32*prop;+unsignedintproplen,i,size,col_range=1<<row_shift,index;-if(!np)-returnNULL;+if(!np||!idev)+return-ENODEV;if(!propname)propname="linux,keymap";prop=of_get_property(np,propname,&proplen);-if(!prop)-returnNULL;+if(!prop){+dev_err(dev,"OF: %s property not defined in %s\n",propname,+np->full_name);+return-ENODEV;+}if(proplen%sizeof(u32)){-pr_warn("Malformed keymap property %s in %s\n",-propname,np->full_name);-returnNULL;+dev_warn(dev,"Malformed keycode property %s in %s\n",propname,+np->full_name);+return-EINVAL;}-kd=kzalloc(sizeof(*kd),GFP_KERNEL);-if(!kd)-returnNULL;--kd->keymap=keymap=kzalloc(proplen,GFP_KERNEL);-if(!kd->keymap){-kfree(kd);-returnNULL;+size=proplen/sizeof(u32);+if(size>idev->keycodemax){+dev_err(dev,"OF: %s size overflow\n",propname);+return-EINVAL;}-kd->keymap_size=proplen/sizeof(u32);+keycode=idev->keycode;+for(i=0;i<size;i++){+unsignedintkey=be32_to_cpup(prop+i);+unsignedintrow=KEY_ROW(key);+unsignedintcol=KEY_COL(key);+unsignedshortcode=KEY_VAL(key);-for(i=0;i<kd->keymap_size;i++){-u32tmp=be32_to_cpup(prop+i);-intkey_code,row,col;+if(col>=col_range){+dev_err(dev,"OF: %s: column %x overflowed its range %d\n",+propname,col,col_range);+return-EINVAL;+}-row=(tmp>>24)&0xff;-col=(tmp>>16)&0xff;-key_code=tmp&0xffff;-keymap[i]=KEY(row,col,key_code);+index=MATRIX_SCAN_CODE(row,col,row_shift);+if(index>idev->keycodemax){+dev_err(dev,"OF: %s index overflow\n",propname);+return-EINVAL;+}+keycode[index]=code;+__set_bit(code,idev->keybit);}+__clear_bit(KEY_RESERVED,idev->keybit);-returnkd;-}-EXPORT_SYMBOL_GPL(matrix_keyboard_of_fill_keymap);--voidmatrix_keyboard_of_free_keymap(conststructmatrix_keymap_data*kd)-{-if(kd){-kfree(kd->keymap);-kfree(kd);-}+return0;}-EXPORT_SYMBOL_GPL(matrix_keyboard_of_free_keymap);+EXPORT_SYMBOL_GPL(matrix_keypad_of_build_keymap);
We don't need to allocate memory for keymap in matrix_keyboard_of_fill_keymap(),
as this would only be used by matrix_keyboard_of_free_keymap(). Instead create
another routine matrix_keypad_of_build_keymap() which reads directly the
property from struct device_node and builds keymap.
With this eariler routines matrix_keyboard_of_fill_keymap() and
matrix_keyboard_of_free_keymap() go away.
This patch also fixes tegra driver according to these changes.
Signed-off-by: Viresh Kumar <redacted>
---
V4:
- check return value of MATRIX_SCAN_CODE() to guarantee that it doesn't overflow
keycodemax sized buffer.
- moved __devinit with the first line in function prototype
drivers/input/keyboard/tegra-kbc.c | 46 +++++++++-------
drivers/input/of_keymap.c | 98 ++++++++++++++++++++--------------
include/linux/input/matrix_keypad.h | 16 ++----
3 files changed, 88 insertions(+), 72 deletions(-)
@@ -26,62 +27,79 @@#include<linux/gfp.h>#include<linux/slab.h>-structmatrix_keymap_data*-matrix_keyboard_of_fill_keymap(structdevice_node*np,-constchar*propname)+/**+*matrix_keypad_of_build_keymap-convertplatformDTkeymapintomatrixkeymap+*@idev:pointertostructinput_dev;usedforgettingkeycode,keybitand+*keycodemax.+*@row_shift:numberofbitstoshiftrowvaluebytoadvancetothenext+*lineinthekeymap+*@propname:DeviceTreepropertynametobeusedforreadingkeymap.Ifpassed+*asNULL,"linux,keymap"isused.+*+*Thisfunctioncreatesanarrayofkeycodes,byreadingpropnamepropertyfrom+*devicetreepassed,thatissuitableforusinginastandardmatrixkeyboard+*driverthatusesrowandcolasindices.+*+*Expectationfromuserdriver:idevmustbeinitializedwithfollowingfields:+*dev.parent,keycode,keybitandkeycodemax.+*/+intmatrix_keypad_of_build_keymap(structinput_dev*idev,+unsignedintrow_shift,constchar*propname){-structmatrix_keymap_data*kd;-u32*keymap;-intproplen,i;+structdevice*dev=idev->dev.parent;+structdevice_node*np=dev->of_node;+unsignedshort*keycode;const__be32*prop;+unsignedintproplen,i,size,col_range=1<<row_shift,index;-if(!np)-returnNULL;+if(!np||!idev)+return-ENODEV;if(!propname)propname="linux,keymap";prop=of_get_property(np,propname,&proplen);-if(!prop)-returnNULL;+if(!prop){+dev_err(dev,"OF: %s property not defined in %s\n",propname,+np->full_name);+return-ENODEV;+}if(proplen%sizeof(u32)){-pr_warn("Malformed keymap property %s in %s\n",-propname,np->full_name);-returnNULL;+dev_warn(dev,"Malformed keycode property %s in %s\n",propname,+np->full_name);+return-EINVAL;}-kd=kzalloc(sizeof(*kd),GFP_KERNEL);-if(!kd)-returnNULL;--kd->keymap=keymap=kzalloc(proplen,GFP_KERNEL);-if(!kd->keymap){-kfree(kd);-returnNULL;+size=proplen/sizeof(u32);+if(size>idev->keycodemax){+dev_err(dev,"OF: %s size overflow\n",propname);+return-EINVAL;}-kd->keymap_size=proplen/sizeof(u32);+keycode=idev->keycode;+for(i=0;i<size;i++){+unsignedintkey=be32_to_cpup(prop+i);+unsignedintrow=KEY_ROW(key);+unsignedintcol=KEY_COL(key);+unsignedshortcode=KEY_VAL(key);-for(i=0;i<kd->keymap_size;i++){-u32tmp=be32_to_cpup(prop+i);-intkey_code,row,col;+if(col>=col_range){+dev_err(dev,"OF: %s: column %x overflowed its range %d\n",+propname,col,col_range);+return-EINVAL;+}-row=(tmp>>24)&0xff;-col=(tmp>>16)&0xff;-key_code=tmp&0xffff;-keymap[i]=KEY(row,col,key_code);+index=MATRIX_SCAN_CODE(row,col,row_shift);+if(index>idev->keycodemax){+dev_err(dev,"OF: %s index overflow\n",propname);+return-EINVAL;+}+keycode[index]=code;+__set_bit(code,idev->keybit);}+__clear_bit(KEY_RESERVED,idev->keybit);-returnkd;-}-EXPORT_SYMBOL_GPL(matrix_keyboard_of_fill_keymap);--voidmatrix_keyboard_of_free_keymap(conststructmatrix_keymap_data*kd)-{-if(kd){-kfree(kd->keymap);-kfree(kd);-}+return0;}-EXPORT_SYMBOL_GPL(matrix_keyboard_of_free_keymap);+EXPORT_SYMBOL_GPL(matrix_keypad_of_build_keymap);
From: Stephen Warren <hidden> Date: 2012-03-30 18:45:34
On 03/29/2012 09:40 PM, Viresh Kumar wrote:
We don't need to allocate memory for keymap in matrix_keyboard_of_fill_keymap(),
as this would only be used by matrix_keyboard_of_free_keymap(). Instead create
another routine matrix_keypad_of_build_keymap() which reads directly the
property from struct device_node and builds keymap.
With this eariler routines matrix_keyboard_of_fill_keymap() and
matrix_keyboard_of_free_keymap() go away.
This patch also fixes tegra driver according to these changes.
Signed-off-by: Viresh Kumar <redacted>
The error checking looks good now, so once the issues mentioned below
are fixed:
Acked-by: Stephen Warren <redacted>
Hi Dmitry,
This is an incremental patch over patch
"Input: of_keymap: Introduce matrix_keypad_of_build_keymap()"
sent earlier by me. Please squash it to that patch while applying.
This contains last few changes suggested by Stephen on V4 of this patch.
---
drivers/input/keyboard/tegra-kbc.c | 4 ++--
include/linux/input/matrix_keypad.h | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
Hi Viresh,
On Mon, Apr 02, 2012 at 09:31:51AM +0530, Viresh Kumar wrote:
quoted hunk
Hi Dmitry,
This is an incremental patch over patch
"Input: of_keymap: Introduce matrix_keypad_of_build_keymap()"
sent earlier by me. Please squash it to that patch while applying.
This contains last few changes suggested by Stephen on V4 of this patch.
---
drivers/input/keyboard/tegra-kbc.c | 4 ++--
include/linux/input/matrix_keypad.h | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
This is the full implementation and should stay __devinit. It's the stub
for !CONFIG_OF case that you want to mark "static inline". I'll fix it
up.
Thanks.
--
Dmitry