Re: [PATCH 15/26] dmaengine: sh-usb-dmac: Remove redundant dev_err()/dev_err_probe()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:

Pre-existing issues:
- [High] The driver unconditionally replaces the error code from `platform_get_irq_byname` with `-ENODEV`, breaking probe deferral.
- [High] Shared interrupt is requested before `vchan_init` initializes the channel's spinlock and lists.
- [High] Missing IRQ cleanup in the probe error path leaves shared interrupts registered while device clocks are disabled.
- [High] The driver fails to cancel the `vchan` tasklets during channel teardown and device removal, which can lead to a Use-After-Free if a tasklet executes after the device memory is freed.
- [High] If `pm_runtime_get_sync()` fails in `usb_dmac_alloc_chan_resources()`, the newly allocated DMA descriptors and the PM runtime usage counter are permanently leaked.
--

--- Patch [15]: [PATCH 15/26] dmaengine: sh-usb-dmac: Remove redundant dev_err()/dev_err_probe() ---
commit 26b5ad26e29c18063b36a1a20ac48c474a74b7b1
Author: Pan Chuang <panchuang@vivo.com>

dmaengine: sh-usb-dmac: Remove redundant dev_err()/dev_err_probe()

The devm_request_irq() now automatically logs detailed error messages on
failure. This eliminates the need for driver-specific dev_err() and
dev_err_probe() calls that previously printed generic messages.

> diff --git a/drivers/dma/sh/usb-dmac.c b/drivers/dma/sh/usb-dmac.c
> index 16509be0d3608..17b385d857930 100644
> --- a/drivers/dma/sh/usb-dmac.c
> +++ b/drivers/dma/sh/usb-dmac.c
> @@ -726,11 +726,8 @@ static int usb_dmac_chan_probe(struct usb_dmac *dmac,
>  
>  	ret = devm_request_irq(dmac->dev, uchan->irq, usb_dmac_isr_channel,
>  			       IRQF_SHARED, irqname, uchan);

[Severity: High]
This is a pre-existing issue, but is the shared interrupt requested too
early?

devm_request_irq() is called with IRQF_SHARED before vchan_init()
initializes the channel's spinlock and lists. If another device sharing
the IRQ line fires an interrupt immediately, the registered ISR
usb_dmac_isr_channel() will execute and unconditionally acquire the
uninitialized chan->vc.lock:

drivers/dma/sh/usb-dmac.c:usb_dmac_isr_channel() {
    ...
    spin_lock(&chan->vc.lock);
    ...
}

Could this lead to a lockdep splat or kernel panic on platforms where this
IRQ line is shared with another active device?

> -	if (ret) {
> -		dev_err(dmac->dev, "failed to request IRQ %u (%d)\n",
> -			uchan->irq, ret);
> +	if (ret)
>  		return ret;
> -	}
>  
>  	uchan->vc.desc_free = usb_dmac_virt_desc_free;
>  	vchan_init(&uchan->vc, &dmac->engine);

[Severity: High]
This is a pre-existing issue, but does the unconditional return of
-ENODEV earlier in usb_dmac_chan_probe() break probe deferral?

drivers/dma/sh/usb-dmac.c:usb_dmac_chan_probe() {
    ...
    uchan->irq = platform_get_irq_byname(pdev, pdev_irqname);
    if (uchan->irq < 0)
        return -ENODEV;
    ...
}

If the interrupt controller is probed after the DMA controller,
platform_get_irq_byname() would return -EPROBE_DEFER, which gets
incorrectly replaced with -ENODEV. Will this cause the driver to
permanently fail to probe?


[Severity: High]
This is a pre-existing issue, but does the probe error path leave shared
interrupts active while device clocks are disabled?

drivers/dma/sh/usb-dmac.c:usb_dmac_probe() {
    ...
error:
    of_dma_controller_free(pdev->dev.of_node);
error_pm:
    pm_runtime_put(&pdev->dev);
    pm_runtime_disable(&pdev->dev);
    return ret;
}

If dma_async_device_register() fails, the error path disables PM runtime.
However, the shared IRQs for successfully probed channels remain active
until devres cleans them up after the function returns. If a shared
interrupt fires in this window, could the ISR access device registers
with stopped clocks and cause an imprecise external abort?


[Severity: High]
This is a pre-existing issue, but does the channel teardown logic fail to
cancel the vchan tasklets?

drivers/dma/sh/usb-dmac.c:usb_dmac_chan_remove() {
    usb_dmac_chan_halt(uchan);
    devm_free_irq(dmac->dev, uchan->irq, uchan);
}

If the driver is unbound or fails during probe while a vchan tasklet is
scheduled, devres will free the DMA controller memory containing the
tasklet structure. If the tasklet subsequently executes, will it access
freed memory? Should tasklet_kill(&uchan->vc.task) be called here?


[Severity: High]
This is a pre-existing issue, but does this leak resources if
pm_runtime_get_sync() fails?

drivers/dma/sh/usb-dmac.c:usb_dmac_alloc_chan_resources() {
    ...
    return pm_runtime_get_sync(chan->device->dev);
}

If it returns a negative error, it leaves the PM runtime usage counter
incremented and the previously allocated DMA descriptors are not freed.
The dmaengine core abandons the channel without calling
device_free_chan_resources(), which appears to result in a permanent leak
preventing the device from entering runtime suspend.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709135846.97972-1-panchuang@vivo.com?part=15