Hi Janusz,
On Fri, Dec 17, 2010 at 11:32:04PM +0100, Janusz Krzysztofik wrote:
> Most keypad drivers make use of the <linux/input/matrix_keypad.h>
> defined macros, structures and inline functions.
>
> Convert omap-keypad driver to use those as well, as suggested by a
> compile time warning, hardcoded into the OMAP <palt/keypad.h>.
>
Looks nice, couple of comments.
>
> struct omap_kp_platform_data {
> int rows;
> int cols;
> - int *keymap;
> - unsigned int keymapsize;
> + const struct matrix_keymap_data *keymap_data;
> unsigned int rep:1;
> unsigned long delay;
> unsigned int dbounce:1;
Since you are updating all boards maybe also change rep and dbounce to
bool?
> --- linux-2.6.37-rc5/drivers/input/keyboard/omap-keypad.c.orig 2010-12-09 23:08:09.000000000 +0100
> +++ linux-2.6.37-rc5/drivers/input/keyboard/omap-keypad.c 2010-12-16 21:04:07.000000000 +0100
> @@ -65,7 +65,6 @@ struct omap_kp {
>
> static DECLARE_TASKLET_DISABLED(kp_tasklet, omap_kp_tasklet, 0);
>
> -static int *keymap;
> static unsigned int *row_gpios;
> static unsigned int *col_gpios;
>
> @@ -162,20 +161,11 @@ static void omap_kp_scan_keypad(struct o
> }
> }
>
> -static inline int omap_kp_find_key(int col, int row)
> -{
> - int i, key;
> -
> - key = KEY(col, row, 0);
> - for (i = 0; keymap[i] != 0; i++)
> - if ((keymap[i] & 0xff000000) == key)
> - return keymap[i] & 0x00ffffff;
> - return -1;
> -}
> -
> static void omap_kp_tasklet(unsigned long data)
> {
> struct omap_kp *omap_kp_data = (struct omap_kp *) data;
> + unsigned short *keycodes = omap_kp_data->input->keycode;
> + unsigned int row_shift = get_count_order(omap_kp_data->cols);
> unsigned char new_state[8], changed, key_down = 0;
> int col, row;
> int spurious = 0;
> @@ -199,7 +189,7 @@ static void omap_kp_tasklet(unsigned lon
> row, (new_state[col] & (1 << row)) ?
> "pressed" : "released");
> #else
> - key = omap_kp_find_key(col, row);
> + key = keycodes[MATRIX_SCAN_CODE(row, col, row_shift)];
> if (key < 0) {
> printk(KERN_WARNING
> "omap-keypad: Spurious key event %d-%d\n",
> @@ -298,16 +288,22 @@ static int __devinit omap_kp_probe(struc
> struct input_dev *input_dev;
> struct omap_kp_platform_data *pdata = pdev->dev.platform_data;
> int i, col_idx, row_idx, irq_idx, ret;
> + unsigned short *keycodes;
> + unsigned int row_shift;
>
> - if (!pdata->rows || !pdata->cols || !pdata->keymap) {
> - printk(KERN_ERR "No rows, cols or keymap from pdata\n");
> + if (!pdata->rows || !pdata->cols || !pdata->keymap_data) {
> + printk(KERN_ERR "No rows, cols or keymap_data from pdata\n");
> return -EINVAL;
> }
>
> omap_kp = kzalloc(sizeof(struct omap_kp), GFP_KERNEL);
> + row_shift = get_count_order(pdata->cols);
> + keycodes = kzalloc((pdata->rows << row_shift) * sizeof(*keycodes),
> + GFP_KERNEL);
> input_dev = input_allocate_device();
> - if (!omap_kp || !input_dev) {
> + if (!omap_kp || !keycodes || !input_dev) {
> kfree(omap_kp);
> + kfree(keycodes);
I do not see where you freeing keycodes in case something else fails nor
in omap_kp_remove(). But why don't you put keycodes at the end of struct
omap_kp and allocate it in one shot?
Thanks.
--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
