Hi Abraham,
On Wed, Apr 14, 2010 at 6:40 AM, Arce, Abraham <[email protected]> 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-omap" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
