Re: [PATCH] OMAP: Keypad driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, 22 Jul 2006 01:26:14 -0700
"Komal Shah" <[email protected]> wrote:

> Please review the attached TI OMAP Keypad driver patch
> and consider it for -mm submission.

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. 

- Use INTF_TRIGGER_FALLING (etc), not the now-deprecated SA_TRIGGER_FALLING.

- The changelog needs work.  What's an 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.

-
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]
  Powered by Linux