Re: [PATCH v6 11/13] gpio: mxc: add dev_err_probe() for error paths to help diagnostics

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] gpio: mxc: race condition exposes uninitialized irq_domain to consumers
- [Low] gpio: mxc: missing dev_err_probe() on clk and irq_count error paths
--

commit 7b3e39d56ce16597ec573873f31010421e588a2f
Author: Peng Fan <peng.fan@nxp.com>

    gpio: mxc: add dev_err_probe() for error paths to help diagnostics

This commit replaces bare returns with dev_err_probe() across the probe error
paths for consistent diagnostics and deferred-probe support.

>     Replace bare return statements with dev_err_probe() across all probe
>     error paths for consistent diagnostics and deferred-probe support.

[Severity: Low]
Is this description accurate?

Looking at mxc_gpio_probe(), bare returns for platform_irq_count() and
devm_clk_get_optional_enabled() are still present:

	irq_count = platform_irq_count(pdev);
	if (irq_count < 0)
		return irq_count;

	...

	port->clk = devm_clk_get_optional_enabled(dev, NULL);
	if (IS_ERR(port->clk))
		return PTR_ERR(port->clk);

Unlike platform_get_irq() or devm_platform_ioremap_resource(), neither of
these functions logs errors or handles probe deferrals internally via
dev_err_probe().

If they fail or defer, the failure might propagate silently without being
recorded in the deferred-probe debugfs list.

Does the commit message need to be updated, or should dev_err_probe() be
added to these error paths?

> diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
> index 09acb186c84c1..2847304640f17 100644
> --- a/drivers/gpio/gpio-mxc.c
> +++ b/drivers/gpio/gpio-mxc.c
[ ... ]
> @@ -536,11 +536,11 @@ static int mxc_gpio_probe(struct platform_device *pdev)
>  
>  	err = devm_gpiochip_add_data(dev, &port->gen_gc.gc, port);
>  	if (err)
> -		return err;
> +		return dev_err_probe(dev, err, "Failed to add gpiochip data\n");

[Severity: Medium]
Could this expose an uninitialized irq_domain to consumers?

devm_gpiochip_add_data() exposes the GPIO chip to the system. If a consumer
immediately attempts to translate a GPIO to an IRQ via gpiod_to_irq(),
mxc_gpio_to_irq() is invoked.

This creates a narrow window where mxc_gpio_to_irq() can be called before
devm_irq_domain_instantiate() initializes port->domain.

If this happens, mxc_gpio_to_irq() will pass a NULL domain to
irq_find_mapping():

drivers/gpio/gpio-mxc.c:mxc_gpio_to_irq() {
	...
	return irq_find_mapping(port->domain, offset);
}

This can result in the consumer getting an incorrect IRQ mapping or a
permanent -ENXIO failure from gpiod_to_irq(), leading to a permanent
probe failure instead of -EPROBE_DEFER.

Could we instantiate the irq_domain before adding the GPIO chip?

>  
>  	irq_base = devm_irq_alloc_descs(dev, -1, 0, 32, numa_node_id());
>  	if (irq_base < 0)
> -		return irq_base;
> +		return dev_err_probe(dev, irq_base, "Failed to alloc irq desc\n");

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261011-gpio-mxc-cleanup-v6-0-cb47ce2ad758@nxp.com?part=11