Re: [PATCH 03/18] arm64: dts: ti: k3-j721s2-main: Add multiple channels for CSI2RX DMA

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] DMA channels are permanently leaked during early probe failures and during module unbind due to a flawed PM runtime status check.
- [High] Missing `pm_runtime_disable()` in the `probe` function's error paths leads to an unbalanced and permanently enabled PM runtime state.
--

commit 79fa01b02ba17aabe215aeb8794e92dac90f80ad
Author: Vaishnav Achath <vaishnav.a@ti.com>

arm64: dts: ti: k3-j721s2-main: Add multiple channels for CSI2RX DMA

J721E CSI2RX SHIM layer can support up to 32 DMA channel contexts. Add
additional DMA channels to enable multistream support for CSI2RX.

> diff --git a/arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi b/arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi
> index 5cac119e4292a..f4b8713873b17 100644
> --- a/arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi
> @@ -1239,8 +1239,11 @@ ti_csi2rx0: ticsi2rx@4500000 {
>  		ranges;
>  		#address-cells = <2>;
>  		#size-cells = <2>;
> -		dmas = <&main_bcdma_csi 0 0x4940 0>;
> -		dma-names = "rx0";
> +		dmas = <&main_bcdma_csi 0 0x4940 0>, <&main_bcdma_csi 0 0x4941 0>,
> +		       <&main_bcdma_csi 0 0x4942 0>, <&main_bcdma_csi 0 0x4943 0>,
> +		       <&main_bcdma_csi 0 0x4944 0>, <&main_bcdma_csi 0 0x4945 0>,
> +		       <&main_bcdma_csi 0 0x4946 0>, <&main_bcdma_csi 0 0x4947 0>;
> +		dma-names = "rx0", "rx1", "rx2", "rx3", "rx4", "rx5", "rx6", "rx7";

[Severity: High]
This isn't a bug introduced by this patch, but increasing the channel
allocations from 1 to 8 will exhaust the system's DMA engine much faster due
to an existing channel leak in the driver.

During an early probe failure before pm_runtime_set_active() is called, the
device status defaults to RPM_SUSPENDED.

In drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c:ti_csi2rx_cleanup_ctx(),
the driver checks this status:

    if (!pm_runtime_status_suspended(ctx->csi->dev))
        dma_release_channel(ctx->dma.chan);

Because the status is already suspended, it wrongly assumes the channels were
freed by a suspend callback and skips releasing them.

This same leak also happens on module unbind. In ti_csi2rx_remove(), if the
device is active, the driver forces the state to suspended without disabling PM
runtime first:

    if (!pm_runtime_status_suspended(&pdev->dev))
        pm_runtime_set_suspended(&pdev->dev);

This alters the status without invoking the runtime_suspend callback that would
normally free the channels. Does it make sense to fix this PM state handling
so these newly added DMA channels aren't permanently leaked?

[Severity: High]
This is a pre-existing issue, but there is also a missing pm_runtime_disable()
in the probe error path that could cause PM state tracking issues when this
node fails to probe.

If ti_csi2rx_notifier_register() or devm_of_platform_populate() fails, the code
jumps to err_ctx:

drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c:ti_csi2rx_probe() {
    ...
err_ctx:
    while (i--)
        ti_csi2rx_cleanup_ctx(&csi->ctx[i]);
    ti_csi2rx_cleanup_v4l2(csi);
err_dma_chan:
    ...
}

This error path misses a call to pm_runtime_disable(), leaving the PM runtime
disable_depth unbalanced. When a subsequent probe attempt calls
pm_runtime_enable() again, could the disable_depth underflow and break power
management for the device?

>  		power-domains = <&k3_pds 38 TI_SCI_PD_EXCLUSIVE>;
>  		status = "disabled";

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260702093123.1048575-1-y-abhilashchandra@ti.com?part=3