Re: [PATCHv2 2/3] watchdog: sama5d4: use platform_get_irq_optional()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The interrupt handler relies on platform drvdata but is registered before drvdata is set, which can lead to a NULL pointer dereference.
--

commit 6a0bceec2bdd17bb8ca5d9b9518a193041169703
Author: Rosen Penev <rosenp@gmail.com>

watchdog: sama5d4: use platform_get_irq_optional()

This commit replaces irq_of_parse_and_map() with platform_get_irq_optional()
to avoid needing irq_dispose_mapping() on failure. It also adds handling for
-EPROBE_DEFER.

> diff --git a/drivers/watchdog/sama5d4_wdt.c b/drivers/watchdog/sama5d4_wdt.c
> index 13e72918338a2..e12499547d735 100644
> --- a/drivers/watchdog/sama5d4_wdt.c
> +++ b/drivers/watchdog/sama5d4_wdt.c

[ ... ]

> @@ -286,8 +285,11 @@ static int sama5d4_wdt_probe(struct platform_device *pdev)
>  		return ret;
>  
>  	if (wdt->need_irq) {
> -		irq = irq_of_parse_and_map(dev->of_node, 0);
> -		if (!irq) {
> +		irq = platform_get_irq_optional(pdev, 0);
> +		if (irq == -EPROBE_DEFER)
> +			return irq;
> +
> +		if (irq < 0) {
>  			dev_warn(dev, "failed to get IRQ from DT\n");
>  			wdt->need_irq = false;
>  		}

[Severity: High]
This is a pre-existing issue, but since the interrupt is registered
with IRQF_SHARED shortly after this, could the handler fire before
platform_set_drvdata() is called?

In sama5d4_wdt_probe(), the interrupt is registered like this:

	ret = devm_request_irq(dev, irq, sama5d4_wdt_irq_handler,
			       IRQF_SHARED | IRQF_IRQPOLL |
			       IRQF_NO_SUSPEND, pdev->name, pdev);

Because it is shared, another device on the same IRQ line could trigger
an interrupt immediately. The handler then retrieves the wdt pointer
using platform_get_drvdata():

drivers/watchdog/sama5d4_wdt.c:sama5d4_wdt_irq_handler() {
	struct sama5d4_wdt *wdt = platform_get_drvdata(dev_id);
	...
	if (wdt->sam9x60_support)
		...
}

However, platform_set_drvdata() is only called at the very end of
sama5d4_wdt_probe(). Would this result in a NULL pointer dereference
if the interrupt triggers early? Should platform_set_drvdata() be
moved before the interrupt is requested?

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