* Re: [PATCH 2/2] bus: Remove redundant dev_err()/dev_err_probe()
2026-07-10 11:09 ` [PATCH 2/2] bus: Remove redundant dev_err()/dev_err_probe() Pan Chuang
@ 2026-07-10 11:19 ` sashiko-bot
0 siblings, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-07-10 11:19 UTC (permalink / raw)
To: Pan Chuang; +Cc: linux-sunxi
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
Pre-existing issues:
- [High] Unhandled negative error code from platform_get_irq() in omap_l3_noc is passed directly to devm_request_irq(), masking the original error and breaking probe deferral.
- [High] IRQ is requested and enabled before hardware clock and reset initialization in sunxi-rsb, risking synchronous bus aborts.
- [High] The IRQ handler in sunxi-rsb remains registered after the device's clock is disabled during driver removal, risking a fatal crash on spurious interrupts.
- [Medium] Use of a static struct pointer in the probe function of omap_l3_noc leads to a race condition and data corruption on concurrent device probes.
--
--- Patch [2]: [PATCH 2/2] bus: Remove redundant dev_err()/dev_err_probe() ---
commit 78c3bde11117dc0a88b0af99a88dd1bbb4e58713
Author: Pan Chuang <panchuang@vivo.com>
bus: Remove redundant dev_err()/dev_err_probe()
Since commit 55b48e23f5c4 ("genirq/devres: Add error handling in
devm_request_*_irq()"), devm_request_irq() automatically logs
detailed error messages on failure. Remove the now-redundant
driver-specific dev_err() and dev_err_probe() calls.
> diff --git a/drivers/bus/omap_l3_noc.c b/drivers/bus/omap_l3_noc.c
> index eb1ba6319fdaf..45f3f793976c5 100644
> --- a/drivers/bus/omap_l3_noc.c
> +++ b/drivers/bus/omap_l3_noc.c
> @@ -278,17 +278,12 @@ static int omap_l3_probe(struct platform_device *pdev)
> l3->debug_irq = platform_get_irq(pdev, 0);
> ret = devm_request_irq(l3->dev, l3->debug_irq, l3_interrupt_handler,
> IRQF_NO_THREAD, "l3-dbg-irq", l3);
[Severity: Medium]
This isn't a bug introduced by this patch, but could the use of the l3 pointer
here lead to cross-linked interrupts or data corruption on concurrent probes?
Looking at omap_l3_probe(), the l3 pointer is declared as a static struct:
static int omap_l3_probe(struct platform_device *pdev)
{
...
static struct omap_l3 *l3;
...
l3 = devm_kzalloc(&pdev->dev, sizeof(*l3), GFP_KERNEL);
If two instances are probed concurrently, one thread's allocation could
overwrite the other's before it is passed to devm_request_irq().
[Severity: High]
This is also a pre-existing issue, but does passing an unhandled negative
error code from platform_get_irq() directly into devm_request_irq() mask
probe deferral errors?
If platform_get_irq() returns -EPROBE_DEFER, passing it as an unsigned int
to devm_request_irq() will result in a permanent -EINVAL failure. Since
this patch causes devm_request_irq() to log the -EINVAL on failure, this
could also unnecessarily log errors in the kernel during probe deferral.
> - if (ret) {
> - dev_err(l3->dev, "request_irq failed for %d\n",
> - l3->debug_irq);
> + if (ret)
> return ret;
> - }
>
> l3->app_irq = platform_get_irq(pdev, 1);
> ret = devm_request_irq(l3->dev, l3->app_irq, l3_interrupt_handler,
> IRQF_NO_THREAD, "l3-app-irq", l3);
> - if (ret)
> - dev_err(l3->dev, "request_irq failed for %d\n", l3->app_irq);
>
> return ret;
> }
> diff --git a/drivers/bus/sunxi-rsb.c b/drivers/bus/sunxi-rsb.c
> index daf0ea563e477..671c9b33e783a 100644
> --- a/drivers/bus/sunxi-rsb.c
> +++ b/drivers/bus/sunxi-rsb.c
> @@ -787,8 +787,7 @@ static int sunxi_rsb_probe(struct platform_device *pdev)
>
> ret = devm_request_irq(dev, irq, sunxi_rsb_irq, 0, RSB_CTRL_NAME, rsb);
> if (ret)
> - return dev_err_probe(dev, ret,
> - "can't register interrupt handler irq %d\n", irq);
> + return ret;
>
> ret = sunxi_rsb_hw_init(rsb);
[Severity: High]
This is a pre-existing issue, but does registering the IRQ handler before
sunxi_rsb_hw_init() enables the clock and clears the reset risk a
synchronous external abort?
If an interrupt is already pending from a previous boot stage, the
sunxi_rsb_irq handler will execute immediately. Because the clocks are
gated and the device is in reset, reading the device registers inside the
handler could crash the system.
[Severity: High]
This is another pre-existing issue related to the devm_request_irq() usage.
Does the IRQ handler remain registered after the device's clock is manually
disabled during driver removal?
Because sunxi_rsb_remove() manually disables the hardware by calling
sunxi_rsb_hw_exit(), but relies on devres to free the interrupt handler
afterward, a spurious interrupt firing in that window could cause a crash
when the handler attempts to access gated device registers.
> if (ret)
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260710110930.462109-1-panchuang@vivo.com?part=2
^ permalink raw reply [flat|nested] 3+ messages in thread