On 22/11/2019 09:55, Herbert Xu wrote: > On Fri, Nov 15, 2019 at 10:31:16AM +0800, Chuhong Yuan wrote: >> This driver forgets to kill tasklet when probe fails and remove. >> Add the calls to fix it. >> >> Signed-off-by: Chuhong Yuan <hslester96@xxxxxxxxx> > > Yes this driver does look buggy but I think your patch isn't > enough. > >> diff --git a/drivers/crypto/picoxcell_crypto.c b/drivers/crypto/picoxcell_crypto.c >> index 3cbefb41b099..8d7c6bb2876e 100644 >> --- a/drivers/crypto/picoxcell_crypto.c >> +++ b/drivers/crypto/picoxcell_crypto.c >> @@ -1755,6 +1755,7 @@ static int spacc_probe(struct platform_device *pdev) >> if (!ret) >> return 0; >> >> + tasklet_kill(&engine->complete); > > The tasklet is schedule by the IRQ handler so you should not kill > it until the IRQ handler has been unregistered. > > This driver is also buggy because it registers the IRQ handler > before initialising the tasklet. You must always be prepared for > spurious IRQs. IOW, as soon as you register the IRQ handler you > must be prepared for it to be called. > >> @@ -1771,6 +1772,7 @@ static int spacc_remove(struct platform_device *pdev) >> struct spacc_alg *alg, *next; >> struct spacc_engine *engine = platform_get_drvdata(pdev); >> >> + tasklet_kill(&engine->complete); > > Ditto. > > However, the IRQ handler is registered through devm which makes it > hard to kill the tasklet after unregistering it. We should probably > convert it to a normal request_irq so we can control how it's > unregistered. Or inversely, registering the tasklet_kill() through devm, so that it is called *after* the ISR unregistration. Regards.
