[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20060722023935.aab52bd2.akpm@osdl.org>
Date: Sat, 22 Jul 2006 02:39:35 -0700
From: Andrew Morton <akpm@...l.org>
To: "Komal Shah" <komal_shah802003@...oo.com>
Cc: linux-input@...ey.karlin.mff.cuni.cz, dtor_core@...ritech.net,
tony@...mide.com, ext-timo.teras@...ia.com,
juha.yrjola@...idboot.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] OMAP: Keypad driver
On Sat, 22 Jul 2006 01:26:14 -0700
"Komal Shah" <komal_shah802003@...oo.com> 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 random_person@...oo.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.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists