--- Andrew Morton <[email protected]> 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 [email protected] 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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
