Messages in this thread | | | Date | Sat, 22 Jul 2006 03:13:40 -0700 (PDT) | From | Komal Shah <> | Subject | Re: [PATCH] OMAP: Keypad driver |
| |
--- Andrew Morton <akpm@osdl.org> wrote:
> > The code looks clean. > > - Could use setup_timer(). > > - Check the return value from device_create_file() (and any and all > such > similar functions), handle failure appropriately. > > - I don't think the tasklet stopping code is correct. > tasklet_disable() > will prevent the callback from being called. The del_timer_sync() > will > kill off the timer. But the just-killed timer handler might have > left > the tasklet scheduled, and although it will not call the handler, > the > high-level tasklet code can still execute, and will then start > playing > with now-freed data. A final tasklet_kill() should fix that up. > > - Perhaps the probe function is requesting the IRQ too early? The > IRQ > handler can perhaps be called while the hardware is still being set > up.
Ok. I will move it before omap_writew(0, OMAP_MPUIO_BASE + OMAP_MPUIO_KBD_MASKIT);
> > - Use INTF_TRIGGER_FALLING (etc), not the now-deprecated > SA_TRIGGER_FALLING.
Thanx for the review. I will update the driver and submit the patch.
> > - The changelog needs work. What's an OMAP? ;)
Ok. May changelog needs URL entry like http://linux.omap.com _or_ http://www.ti.com/omap :)
> > - Finally, by what authority does random_person@yahoo.com submit > Nokia > and TI's code?? Please review Section 11 of > Documentation/SubmittingPatches, seek and obtain signoffs from the > other > authors and then add your own, thanks.
I have CCed all the authors mentioned in the top of the driver file, and I thought they will just reply with Signed-off-by: line. I never added my name in Copyright, even when I did some re-organization of omap keypad driver, which I thought was not major change :)
---Komal Shah http://komalshah.blogspot.com/
__________________________________________________ Do You Yahoo!? Tired of spam? Yahoo! Mail has the best spam protection around http://mail.yahoo.com - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |