Re: [RFC] [PATCH 1/3] OMAP4: Keyboard Controller Support

[Trilok Soni <soni.trilok@gmail.com>]

Hi Abraham,

On Wed, Apr 14, 2010 at 6:40 AM, Arce, Abraham <x0066660@ti.com> 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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html