Re: [PATCH] watchdog: wdat_wdt: map registers that fall inside ACPI NVS

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Calling `devm_ioremap_resource()` triggers an unconditional `dev_err()` when it fails with `-EBUSY`, causing confusing and spurious errors in the kernel log on systems with ACPI NVS.
--

--- Patch [1]: [PATCH] watchdog: wdat_wdt: map registers that fall inside ACPI NVS ---
commit fe186a236b35b9dc11c6a76632ffa54de7cc01ba
Author: Renê de Souza Pinto <[email protected]>

watchdog: wdat_wdt: map registers that fall inside ACPI NVS

This commit fixes an issue where the WDAT watchdog fails to probe on systems 
where its registers reside in ACPI NVS memory (like the OnLogic Karbon 524)
by falling back to devm_memremap() when devm_ioremap_resource() returns -EBUSY.

> diff --git a/drivers/watchdog/wdat_wdt.c b/drivers/watchdog/wdat_wdt.c
> index dd3c2d69c9df1..a56224cf8d22d 100644
> --- a/drivers/watchdog/wdat_wdt.c
> +++ b/drivers/watchdog/wdat_wdt.c

[ ... ]

> @@ -309,6 +311,33 @@ static struct watchdog_ops wdat_wdt_ops = {
>  	.set_timeout = wdat_wdt_set_timeout,
>  };
>  
> +static void __iomem *wdat_wdt_map_mem(struct device *dev, struct resource *res)
> +{
> +	resource_size_t size = resource_size(res);
> +	void __iomem *reg;
> +	void *addr;
> +
> +	/* Try standard reservation + ioremap first */
> +	reg = devm_ioremap_resource(dev, res);
> +	if (!IS_ERR(reg) || PTR_ERR(reg) != -EBUSY)
> +		return reg;

[Severity: Medium]
Since devm_ioremap_resource() internally calls dev_err() when it fails,
will this unconditional call log a spurious error before the driver has
a chance to execute the expected ACPI NVS fallback?

Could this be avoided by checking region_intersects() for 
IORES_DESC_ACPI_NV_STORAGE before attempting the reservation, rather than
relying on devm_ioremap_resource() to fail first?

> +
> +	/* If busy, check if conflict is due to ACPI NVS memory */
> +	if (region_intersects(res->start, size, IORESOURCE_MEM,
> +			      IORES_DESC_ACPI_NV_STORAGE) != REGION_INTERSECTS)
> +		return reg; /* Real conflict, return -EBUSY */
> +
> +	dev_warn(dev, "%pR is inside ACPI NVS, mapping without reservation\n", res);
> +
> +	addr = devm_memremap(dev, res->start, size, MEMREMAP_WB);
> +	if (IS_ERR(addr)) {
> +		dev_err(dev, "failed to map resource %pR\n", res);
> +		return IOMEM_ERR_PTR(PTR_ERR(addr));
> +	}
> +
> +	return (void __iomem __force *)addr;
> +}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1