Re: [PATCH v5 10/17] rtc: rzn1: Replace remove callback with devm_add_action_or_reset()

[Wolfram Sang <wsa+renesas@sang-engineering.com>]

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


> > +	ret = devm_add_action_or_reset(&pdev->dev, rzn1_rtc_disable_hardware, &pdev->dev);
> > +	if (ret)
> > +		return ret;
> > +
> 
> [Severity: High]
> Since devres executes teardown actions in LIFO order, will registering the
> hardware disable action here early in rzn1_rtc_probe cause it to run last
> during driver unbind?
> 
> [ ... ]
> 
> > @@ -468,14 +480,12 @@ static int rzn1_rtc_probe(struct platform_device *pdev)
> >  	ret = devm_request_irq(&pdev->dev, irq, rzn1_rtc_alarm_irq, 0, "RZN1 RTC Alarm", rtc);
> >  	if (ret) {
> >  		dev_err(&pdev->dev, "RTC alarm interrupt not available\n");
> > -		goto dis_runtime_pm;
> > +		return ret;
> >  	}
> 
> [Severity: High]
> If devm_request_irq is called after the hardware disable action is
> registered, won't the IRQ handlers be freed before the hardware is disabled
> during teardown?
> 
> If an RTC alarm or 1-second interrupt is active when the driver unbinds,
> could an interrupt fire in the devres teardown window where the handler is
> gone but the hardware is still active? This might trigger the generic IRQ
> subsystem's spurious interrupt detector, which could permanently disable the
> IRQ line.

This needs to be checked, I am afraid. It sounds reasonable.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]