Re: question about drivers/i2c/busses/i2c-davinci.c

[Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 1227 bytes --]

On Sun, Jan 06, 2013 at 09:00:59PM +0100, Julia Lawall wrote:
> The function davinci_i2c_remove in drivers/i2c/busses/i2c-davinci.c
> contains the following code:
> 
>         put_device(&pdev->dev);
> 
>         clk_disable_unprepare(dev->clk);
>         clk_put(dev->clk);
>         dev->clk = NULL;
> 
>         davinci_i2c_write_reg(dev, DAVINCI_I2C_MDR_REG, 0);
> 	free_irq(dev->irq, dev);
> 
> Is there any danger in putting free_irq(dev->irq, dev); after
> put_device(&pdev->dev);, because the interrupt handler
> i2c_davinci_isr can eg refer to dev->dev.

Not having a clock doesn't sound exactly thrilling either when servicing
an interrupt. I've seen something like this in the remove path of
another driver today as well. I assume a lot of drivers might have such
issues. It is also one of the subtle issues with devm_request_irq. The
remove path can already render the ISR unusable/oopsable but devm will
free the interrupt only after remove has finished. Interrupts need to be
properly masked out before.

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]