Re: [PATCH 06/18] arm64: dts: ti: k3-j722s-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 channel leak in ti_csi2rx_runtime_resume() on error path.
- [High] DMA channel leak in ti_csi2rx_probe() on context initialization failure.
--

commit 93bee806331f1aba1613005a42f8e9a3e3a302f3
Author: Vaishnav Achath <vaishnav.a@ti.com>

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

This commit adds additional DMA channels to the J721E CSI2RX SHIM layer to
enable multistream support for CSI2RX. It increases the number of DMA channels
per node from one to four.

> diff --git a/arch/arm64/boot/dts/ti/k3-j722s-main.dtsi b/arch/arm64/boot/dts/ti/k3-j722s-main.dtsi
> index 5a5b9a1bfd9ea..199c8652bcde2 100644
> --- a/arch/arm64/boot/dts/ti/k3-j722s-main.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-j722s-main.dtsi
> @@ -173,8 +173,9 @@ ti_csi2rx1: ticsi2rx@30122000 {
>  		ranges;
>  		#address-cells = <2>;
>  		#size-cells = <2>;
> -		dmas = <&main_bcdma_csi 0 0x5100 0>;
> -		dma-names = "rx0";
> +		dmas = <&main_bcdma_csi 0 0x5100 0>, <&main_bcdma_csi 0 0x5101 0>,
> +		       <&main_bcdma_csi 0 0x5102 0>, <&main_bcdma_csi 0 0x5103 0>;
> +		dma-names = "rx0", "rx1", "rx2", "rx3";

[Severity: High]
This isn't a bug introduced by this patch, but adding multiple DMA channels
exposes a dormant hardware DMA channel leak in the CSI2RX driver.

If ti_csi2rx_init_dma() fails for a context where i > 0 during runtime resume,
the function returns immediately without releasing the channels requested in
earlier loop iterations:

j721e-csi2rx.c:ti_csi2rx_runtime_resume() {
	for (unsigned int i = 0; i < csi->num_ctx; i++) {
		ret = ti_csi2rx_init_dma(&csi->ctx[i]);
		if (ret)
			return ret;
	}
	...
}

Does this code need an error path to release the successfully requested
channels when a subsequent iteration fails?

[Severity: High]
This is a pre-existing issue, but increasing the number of DMA channels here
also exposes another channel leak in the probe error path.

If ti_csi2rx_init_ctx() fails for a context i > 0, the driver attempts to
clean up previous contexts:

j721e-csi2rx.c:ti_csi2rx_probe() {
	for (i = 0; i < csi->num_ctx; i++) {
		csi->ctx[i].idx = i;
		csi->ctx[i].csi = csi;
		ret = ti_csi2rx_init_ctx(&csi->ctx[i]);
		if (ret)
			goto err_ctx;
	}
	...
}

However, the cleanup skips releasing the channel because the device's PM
runtime status is still suspended:

j721e-csi2rx.c:ti_csi2rx_cleanup_ctx() {
	if (!pm_runtime_status_suspended(ctx->csi->dev))
		dma_release_channel(ctx->dma.chan);
	...
}

Since pm_runtime_set_active() hasn't been called yet during early probe, the
condition prevents dma_release_channel() from being executed. Could this
permanently leak the previously initialized hardware DMA channels?

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

[ ... ]

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