Re: [PATCH 04/18] arm64: dts: ti: k3-j784s4-j742s2-main-common: Add multiple channels for CSI2RX DMA

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] DMA channel leak on driver removal and probe failure due to incorrect PM runtime status checks.
--

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

arm64: dts: ti: k3-j784s4-j742s2-main-common: 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-j784s4-j742s2-main-common.dtsi b/arch/arm64/boot/dts/ti/k3-j784s4-j742s2-main-common.dtsi
> index c2636e624f18b..4b4545a5af192 100644
> --- a/arch/arm64/boot/dts/ti/k3-j784s4-j742s2-main-common.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-j784s4-j742s2-main-common.dtsi
> @@ -811,8 +811,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";
>  		power-domains = <&k3_pds 72 TI_SCI_PD_EXCLUSIVE>;
>  		status = "disabled";
>  

[Severity: High]
This isn't a bug introduced by this patch, but adding these multiple channels
amplifies a pre-existing DMA channel leak in the j721e-csi2rx driver.

When ti_csi2rx_remove() is called or during probe failure, are all of these
allocated DMA channels permanently leaked?

Looking at ti_csi2rx_remove() in
drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c, the device is
explicitly forced into a suspended state before cleaning up the contexts:

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

	for (unsigned int i = 0; i < csi->num_ctx; i++)
		ti_csi2rx_cleanup_ctx(&csi->ctx[i]);

When ti_csi2rx_cleanup_ctx() is called, it checks the suspend status:

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

Since the device was suspended just prior to this loop, won't this check
always evaluate to false and skip dma_release_channel() completely?

[ ... ]

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