Re: [PATCH] rtc: ma35d1: fix unchecked IRQ error and IRQ-before-rtcdev ordering

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] rtc: devm_request_irq() called after devm_rtc_register_device()
- [Low] rtc: redundant dev_err_probe() on platform_get_irq() failure
--

commit 65db24f320f8394c839e4022831930f15fb56e95
Author: Cong Nguyen <congnt264@gmail.com>
rtc: ma35d1: fix unchecked IRQ error and IRQ-before-rtcdev ordering

This commit checks the return value of platform_get_irq() to prevent passing
a negative error code to devm_request_irq(). It also moves the IRQ
request to after the RTC device is registered, intending to fix the teardown
order.

Link: https://lore.kernel.org/r/20260914133532.CBFAA1F000FF@smtp.kernel.org

> diff --git a/drivers/rtc/rtc-ma35d1.c b/drivers/rtc/rtc-ma35d1.c
> --- a/drivers/rtc/rtc-ma35d1.c
> +++ b/drivers/rtc/rtc-ma35d1.c
> @@ -236,11 +236,8 @@ static int ma35d1_rtc_probe(struct platform_device *pdev)
>  	}
>  
>  	rtc->irq_num = platform_get_irq(pdev, 0);
> -
> -	ret = devm_request_irq(&pdev->dev, rtc->irq_num, ma35d1_rtc_interrupt,
> -			       IRQF_NO_SUSPEND, "ma35d1rtc", rtc);
> -	if (ret)
> -		return dev_err_probe(&pdev->dev, ret, "Failed to request rtc irq\n");
> +	if (rtc->irq_num < 0)
> +		return dev_err_probe(&pdev->dev, rtc->irq_num, "failed to get rtc irq\n");

[Severity: Low]
Does this cause a redundant error message to be printed? platform_get_irq()
already calls dev_err_probe() internally when it fails. Returning the error
code directly without another dev_err_probe() might be preferred.

[ ... ]

> @@ -258,6 +255,11 @@ static int ma35d1_rtc_probe(struct platform_device *pdev)
>  	if (ret)
>  		return dev_err_probe(&pdev->dev, ret, "Failed to register rtc device\n");
>  
> +	ret = devm_request_irq(&pdev->dev, rtc->irq_num, ma35d1_rtc_interrupt,
> +			       IRQF_NO_SUSPEND, "ma35d1rtc", rtc);
> +	if (ret)
> +		return dev_err_probe(&pdev->dev, ret, "Failed to request rtc irq\n");
> +
>  	return 0;
>  }

[Severity: High]
Does placing devm_request_irq() after devm_rtc_register_device() introduce a
race condition?

Since devres teardown is last-in, first-out, during an unbind or a probe
failure, the IRQ would be freed before the RTC device is unregistered.
This leaves the device exposed to userspace without an active interrupt
handler, potentially leading to missed alarms or unhandled interrupt storms.

Would it be better to place devm_request_irq() between
devm_rtc_allocate_device() and devm_rtc_register_device() in
ma35d1_rtc_probe()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260918161925.2639130-1-congnt264@gmail.com?part=1