Re: [PATCH] driver core: platform: Setup device MSI domain just before driver probe

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Incorrect parameter passed to `of_msi_configure()` causes silent MSI domain configuration failures for devices where `dev->of_node` is NULL.
- [Medium] Removing explicit `-EPROBE_DEFER` causes permanent probe failures when MSI controllers probe later.
--

commit 117de411608386f49450165e5a9b44f955e55db1
Author: Anup Patel <anup.patel@oss.qualcomm.com>
driver core: platform: Setup device MSI domain just before driver probe

This commit moves the configuration of the device MSI domain for platform
devices into the platform_dma_configure() function. It removes the duplicate
domain setup and EPROBE_DEFER handling from individual RISC-V MSI client
drivers.

> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index a19dd22deef2f..225e33080bc62 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -1470,8 +1470,12 @@ static int platform_dma_configure(struct device *dev)
>  	int ret = 0;
>  
>  	if (is_of_node(fwnode)) {
> +		if (!dev_get_msi_domain(dev))
> +			of_msi_configure(dev, dev->of_node);

[Severity: High]
Does this code silently fail to configure the MSI domain for devices where
dev->of_node is NULL?

If a device is instantiated such that its fwnode is a valid OF node but its
primary dev->of_node is not explicitly populated (like some dynamically
created devices or MFD children), dev->of_node will be NULL.

Should this use to_of_node(fwnode) instead of dev->of_node, similar to the
of_dma_configure() call right below it?

>  		ret = of_dma_configure(dev, to_of_node(fwnode), true);
>  	} else if (is_acpi_device_node(fwnode)) {

[ ... ]

> diff --git a/drivers/irqchip/irq-riscv-aplic-msi.c b/drivers/irqchip/irq-riscv-aplic-msi.c
> index fb8d1838609fb..2cf5d42cd66a8 100644
> --- a/drivers/irqchip/irq-riscv-aplic-msi.c
> +++ b/drivers/irqchip/irq-riscv-aplic-msi.c
[ ... ]
> @@ -245,32 +244,6 @@ int aplic_msi_setup(struct device *dev, void __iomem *regs)
>  	/* Setup global config and interrupt delivery */
>  	aplic_init_hw_global(priv, true);
>  
> -	/* Set the APLIC device MSI domain if not available */
> -	if (!dev_get_msi_domain(dev)) {
[ ... ]
> -		if (!dev_get_msi_domain(dev))
> -			return -EPROBE_DEFER;
> -	}

[Severity: Medium]
Will removing this -EPROBE_DEFER fallback cause permanent probe failures if
fw_devlink is disabled (e.g., via fw_devlink=off) or fails to fully resolve
dependencies?

If the client device probes before the IMSIC controller in such scenarios,
dev_get_msi_domain(dev) will be NULL, and subsequent calls like
msi_create_device_irq_domain() might return false.

Would it be safer to retain a way to gracefully defer the probe here rather
than failing permanently?

> -
>  	if (!msi_create_device_irq_domain(dev, MSI_DEFAULT_DOMAIN, &aplic_msi_template,
>  					  priv->nr_irqs + 1, priv, priv)) {
>  		dev_err(dev, "failed to create MSI irq domain\n");

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260526145240.1627200-1-anup.patel@oss.qualcomm.com?part=1