On Sat, Jan 05, 2013 at 04:50:58PM +0530, Laxman Dewangan wrote:HI Dmitry,devm_request_irq() has a potential of creating a race condition, but it
Thanks for quick review.
I will take care of your comment in next version. Some have my answer.
On Saturday 05 January 2013 01:36 PM, Dmitry Torokhov wrote:Hi Laxman,Ok, mostly errors are in negative and hence this change, I will
On Sat, Jan 05, 2013 at 01:15:08PM +0530, Laxman Dewangan wrote:Use devm_* for memory, clock, input device allocation. This reducesWhy is this change? As far as I can see tegra_kbd_setup_keymap() never
code for freeing these resources.
err = tegra_kbd_setup_keymap(kbc);
- if (err) {
+ if (err < 0) {
returns positive values.
revert it and will keep original.
I understand from Mark B on different patches that usingdev_err(&pdev->dev, "failed to setup keymap\n");Neither request_irq(). BTW, why not devm_request_irq?
- goto err_put_clk;
+ return err;
}
__set_bit(EV_REP, input_dev->evbit);
@@ -790,15 +784,15 @@ static int tegra_kbc_probe(struct platform_device *pdev)
err = request_irq(kbc->irq, tegra_kbc_isr,
IRQF_NO_SUSPEND | IRQF_TRIGGER_HIGH, pdev->name, kbc);
- if (err) {
+ if (err < 0) {
devm_request_irq() can create race condition when removing device.
Interrupt can occur when device resource release is in process and
so it can cause isr call which can use the freed pointer.
devm_request_irq() should be avoided.
depents on the driver. In this particular case tegra driver ensures that
interrupts are inhibited when input device is unregistered by providing
tegra_kbc_close() method, so in this particular case it is safe to
use devm_request_irq().
Also, when using managed input devices, the unregistering and final
freeing is a 2-step process, so even in absence of close() method, if
initialization sequence was:
devm_input_allocate_device()
...
devm_request_irq()
...
input_unregister_device()
then order of freeing resources (behind the scenes) will be
devm_input_device_unregister();
/* input device is still present in memory and can
* handle input_event() calls.
*/
free_irq();
devm_input_device_release();
So using managed request_irq() _together_ with managed input devices is
OK.