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-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to