Hi Abraham, On Wed, Apr 14, 2010 at 6:40 AM, Arce, Abraham <x0066660@xxxxxx> wrote: > Keyboard controller for OMAP4 with built-in scanning algorithm. > The following implementations are used: > > - matrix_keypac.c logic > - hwmod framework Do we have hwmod framework mainlined in the kernel? > > +config KEYBOARD_OMAP4 > + tristate "TI OMAP4 keypad support" > + depends on (ARCH_OMAP4) No need of brackets. Could you please provide "default y/n" option too? > + * linux/drivers/input/keyboard/omap4-keypad.c Better not to include file paths. > + > +struct omap_device_pm_latency omap_keyboard_latency[] = { static? > + { > + .deactivate_func = omap_device_idle_hwmods, > + .activate_func = omap_device_enable_hwmods, > + .flags = OMAP_DEVICE_LATENCY_AUTO_ADJUST, > + }, > +}; > +static irqreturn_t omap_keypad_threaded(int irq, void *dev_id) > +{ > + struct omap_keypad *keypad_data = dev_id; > + struct input_dev *input = keypad_data->input; > + unsigned char key_state[8]; > + int col, row, code; > + u32 *new_state = (u32 *) key_state; > + > + *new_state = __raw_readl(keypad_data->base + > + OMAP4_KBD_FULLCODE31_0); > + *(new_state + 1) = __raw_readl(keypad_data->base + > + OMAP4_KBD_FULLCODE63_32); > + > + for (col = 0; col < keypad_data->cols; col++) { > + for (row = 0; row < keypad_data->rows; row++) { > + code = MATRIX_SCAN_CODE(row, col, > + keypad_data->row_shift); > + if (code < 0) { > + printk(KERN_INFO "Spurious key %d-%d\n", > + col, row); > + continue; > + } > + input_report_key(input, keypad_data->keycodes[code], > + key_state[col] & (1 << row)); > + } > + } where is input_sync(..)? > + > + pdata = kzalloc(sizeof(struct matrix_keypad_platform_data), > + GFP_KERNEL); Please check error return here, because kzalloc can fail. > + > + od = omap_device_build(name, id, oh, pdata, > + sizeof(struct matrix_keypad_platform_data), > + omap_keyboard_latency, > + ARRAY_SIZE(omap_keyboard_latency), 1); > + WARN(IS_ERR(od), "Could not build omap_device for %s %s\n", > + name, oh_name); > + > + /* platform data */ > + pdata = pdev->dev.platform_data; > + if (!pdata) { > + dev_err(&pdev->dev, "no platform data defined\n"); > + kfree(pdata); > + return -EINVAL; > + } > + > + /* keymap data */ > + keymap_data = pdata->keymap_data; > + if (!keymap_data) { > + dev_err(&pdev->dev, "no keymap data defined\n"); > + kfree(keymap_data); not freeing kfree(pdata)? Please check error return path again in this driver. > + > + __set_bit(EV_REP, input_dev->evbit); > + __set_bit(KEY_OK, input_dev->keybit); > + __set_bit(EV_KEY, input_dev->evbit); > + > + input_set_capability(input_dev, EV_MSC, MSC_SCAN); > + input_set_drvdata(input_dev, keypad_data); > + > + status = input_register_device(keypad_data->input); > + if (status < 0) { > + dev_err(&pdev->dev, "failed to register input device\n"); > + goto failed_free_device; > + } > + > + omap_keypad_config(keypad_data); > + > + platform_set_drvdata(pdev, keypad_data); > + > + return 0; > + > +failed_free_device: > + input_unregister_device(keypad_data->input); add keypad_data->input = NULL. -- ---Trilok Soni http://triloksoni.wordpress.com http://www.linkedin.com/in/triloksoni -- 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