Felipe, Thanks for your comments...
[..] > >+#include <plat/omap_hwmod.h> > >+#include <plat/omap_device.h> > > should the platform_driver know about hwmod and omap_device ? Paul ? > Kevin ? Working on these changes... > > >+struct omap_keypad { > >+ > > unnecessary blank line. Removed > > >+ struct platform_device *pdev; > > generaly we save the struct device *. > > >+ struct omap_hwmod *oh; > >+ const struct matrix_keypad_platform_data *pdata; > > you should not save the platform_data here. Generaly you should only use > that to initialize fields on your structure. Remember that a > platform_data will be declared as __initdata. Removed > > >+/* Interrupt thread primary handler, called in hard interrupt context */ > >+ > >+static irqreturn_t omap_keypad_interrupt(int irq, void *dev_id) > >+{ > >+ struct omap_keypad *keypad_data = dev_id; > >+ > >+ /* disable interrupts */ > >+ __raw_writel(OMAP4_DEF_IRQDISABLE, keypad_data->base + > >+ OMAP4_KBD_IRQENABLE); > >+ > >+ /* wake up handler thread */ > >+ return IRQ_WAKE_THREAD; > >+ > >+} > >+ > >+/* Interrupt thread handler thread */ > >+ > >+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); > > use dev_dbg() Added > > >+ continue; > >+ } > >+ input_report_key(input, keypad_data->keycodes[code], > >+ key_state[col] & (1 << row)); > > missing input_sync() Added > > >+static int __devinit omap_keypad_probe(struct platform_device *pdev) > >+{ > > [..] > > >+ length = snprintf(oh_name, hw_mod_name_len, "keyboard"); > >+ WARN(length >= hw_mod_name_len, > >+ "String buffer overflow in keyboard device setup\n"); > > you're using snprintf() this WARN() shouldn't happen even, remove it. Code now in board file, removed > > >+ > >+ oh = omap_hwmod_lookup(oh_name); > >+ if (!oh) > >+ pr_err("Could not look up %s\n", oh_name); > > I think omap_hwmod and omap_device shouldn't be here, but at least use > dev_err(); Moving to board file... > > >+ > >+ pdata = kzalloc(sizeof(struct matrix_keypad_platform_data), > >+ GFP_KERNEL); > > this should come from platform code. Working on these changes... I'll have a function in 4430sdp board file that will setup hwmod and omap device called sdp4430_keypad_init() { ... omap_hwmod_lookup(oh_name); kzalloc(sizeof(struct matrix_keypad_platform_data), GFP_KERNEL); omap_device_build(name, id, oh, pdata, ... } and then called within omap_4430sdp_init() > > >+ > >+ 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); > > this too. Moving to board file... > > >+ 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); > > registering and configuring should be done before requesting the irq. Changed 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