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

[Peng Fan <peng.fan@oss.nxp.com>]

On Sun, Oct 11, 2026 at 12:04:24PM +0000, sashiko-bot@kernel.org wrote:
>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);

I missed to update error path (:

Regards
Peng

>
>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
>