Re: [RFC] [PATCH v3 1/4] OMAP4: Keyboard controller support

Felipe Balbi
2010-06-01T04:43:19Z

On Mon, May 31, 2010 at 11:44:02PM +0200, ext Arce, Abraham wrote:
>diff --git a/arch/arm/plat-omap/include/plat/omap4-keypad.h b/arch/arm/plat-omap/include/plat/omap4-keypad.h
>new file mode 100644
>index 0000000..7a6ce70
>--- /dev/null
>+++ b/arch/arm/plat-omap/include/plat/omap4-keypad.h
>@@ -0,0 +1,23 @@
>+#ifndef ARCH_ARM_PLAT_OMAP4_KEYPAD_H
>+#define ARCH_ARM_PLAT_OMAP4_KEYPAD_H
>+
>+#include <linux/input/matrix_keypad.h>
>+
>+struct omap4_keypad_platform_data {
>+ const struct matrix_keymap_data *keymap_data;
>+
>+ u8 rows;
>+ u8 cols;

rows and cols should be passed by struct matryx_keymap_data

>+ u16 irq;
>+ void __iomem *base;

base and irq are to be passed by struct resource

>+ int (*device_enable) (struct platform_device *pdev);
>+ int (*device_shutdown) (struct platform_device *pdev);
>+ int (*device_idle) (struct platform_device *pdev);

why are you passing these three here ?? I would expect those to be
driver specific not board specific.

>+ error = request_irq(keypad_data->irq, omap4_keypad_interrupt,
>+ IRQF_TRIGGER_FALLING,
>+ "omap4-keypad", keypad_data);
>+ if (error) {
>+ dev_err(&pdev->dev, "failed to register interrupt\n");
>+ goto err_free_input;
>+ }
>+
>+ error = input_register_device(keypad_data->input);

register the input device before enabling the irq line please, you might
end up with 'spurious' irqs happening during probe otherwise.

--
balbi

DefectiveByDesign.org