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

Arce, Abraham
2010-04-16T00:04:11Z

Hi Trilok!

> >  - matrix_keypac.c logic
> >  - hwmod framework
>
> Do we have hwmod framework mainlined in the kernel?

Not yet but wanted to gather initial comments to be ready once framework is pushed

>
> >
> > +config KEYBOARD_OMAP4
> > +        tristate "TI OMAP4 keypad support"
> > +        depends on (ARCH_OMAP4)
>
> No need of brackets.

Removed

>
> Could you please provide "default y/n" option too?

Added

>
> > + * linux/drivers/input/keyboard/omap4-keypad.c
>
> Better not to include file paths.

Removed path... only filename

>
> > +
> > +struct omap_device_pm_latency omap_keyboard_latency[] = {
>
> static?

Not sure... I'll check

<snip>

> > +                       input_report_key(input, keypad_data->keycodes[code],
> > +                                       key_state[col] & (1 << row));
> > +               }
> > +       }
>
> where is input_sync(..)?

Added

>
> > +
> > +       pdata = kzalloc(sizeof(struct matrix_keypad_platform_data),
> > +                               GFP_KERNEL);
>
> Please check error return here, because kzalloc can fail.

Added

<snip>

> > +
> > +       /* 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.

I'll check error return in all code

<snip>

> > +
> > +failed_free_device:
> > +       input_unregister_device(keypad_data->input);
>
> add keypad_data->input = NULL.

Added

Best Regards
Abraham
--
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