[PATCH v2] i2c: imx-lpi2c: fix resource leaks switching to devm_dma_request_chan()

Carlos Song (OSS) posted 1 patch 4 days, 13 hours ago
drivers/i2c/busses/i2c-imx-lpi2c.c | 53 ++++++++++++++++++------------
1 file changed, 32 insertions(+), 21 deletions(-)
[PATCH v2] i2c: imx-lpi2c: fix resource leaks switching to devm_dma_request_chan()
Posted by Carlos Song (OSS) 4 days, 13 hours ago
From: Carlos Song <carlos.song@nxp.com>

The LPI2C driver requests DMA channels using dma_request_chan(), but
never releases them in lpi2c_imx_remove(), resulting in DMA channel
leaks every time the driver is unloaded.

Additionally, when lpi2c_dma_init() successfully requests the TX DMA
channel but fails to request the RX DMA channel, the probe falls back
to PIO mode and completes successfully. Since probe succeeds, the devres
framework will not trigger any cleanup, leaving the TX DMA channel and
the memory allocated for the dma structure held for the lifetime of the
device even though DMA is never used.

Switch to devm_dma_request_chan() to let the device core manage DMA
channel lifetime automatically. Wrap all allocations within a devres
group so that devres_release_group() can release all partially acquired
resources when DMA init fails and probe continues in PIO mode.

Fixes: a09c8b3f9047 ("i2c: imx-lpi2c: add eDMA mode support for LPI2C")
Cc: stable@vger.kernel.org
Signed-off-by: Carlos Song <carlos.song@nxp.com>
---
Change for v2:
  - Wrap all allocations in lpi2c_dma_init() within a devres group so
    that devres_release_group() releases all partially acquired resources
    (dma structure memory, TX DMA channel) when DMA init fails and probe
    continues in PIO mode. Without this, a successful TX channel request
    followed by a failed RX channel request would leave the TX channel
    and dma structure held for the lifetime of the device.
---
 drivers/i2c/busses/i2c-imx-lpi2c.c | 53 ++++++++++++++++++------------
 1 file changed, 32 insertions(+), 21 deletions(-)

diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c b/drivers/i2c/busses/i2c-imx-lpi2c.c
index 6e298424de5e..dedcc24e63ec 100644
--- a/drivers/i2c/busses/i2c-imx-lpi2c.c
+++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
@@ -1383,55 +1383,66 @@ static int lpi2c_imx_init_recovery_info(struct lpi2c_imx_struct *lpi2c_imx,
 	return 0;
 }
 
-static void dma_exit(struct device *dev, struct lpi2c_imx_dma *dma)
-{
-	if (dma->chan_rx)
-		dma_release_channel(dma->chan_rx);
-
-	if (dma->chan_tx)
-		dma_release_channel(dma->chan_tx);
-
-	devm_kfree(dev, dma);
-}
-
 static int lpi2c_dma_init(struct device *dev, dma_addr_t phy_addr)
 {
 	struct lpi2c_imx_struct *lpi2c_imx = dev_get_drvdata(dev);
 	struct lpi2c_imx_dma *dma;
+	void *group;
 	int ret;
 
-	dma = devm_kzalloc(dev, sizeof(*dma), GFP_KERNEL);
-	if (!dma)
+	/*
+	 * Open a devres group so that all resources allocated within
+	 * this function can be released together if DMA init fails but
+	 * probe continues in PIO mode.
+	 */
+	group = devres_open_group(dev, NULL, GFP_KERNEL);
+	if (!group)
 		return -ENOMEM;
 
+	dma = devm_kzalloc(dev, sizeof(*dma), GFP_KERNEL);
+	if (!dma) {
+		ret = -ENOMEM;
+		goto release_group;
+	}
+
 	dma->phy_addr = phy_addr;
 
 	/* Prepare for TX DMA: */
-	dma->chan_tx = dma_request_chan(dev, "tx");
+	dma->chan_tx = devm_dma_request_chan(dev, "tx");
 	if (IS_ERR(dma->chan_tx)) {
 		ret = PTR_ERR(dma->chan_tx);
 		if (ret != -ENODEV && ret != -EPROBE_DEFER)
 			dev_err(dev, "can't request DMA tx channel (%d)\n", ret);
-		dma->chan_tx = NULL;
-		goto dma_exit;
+		goto release_group;
 	}
 
 	/* Prepare for RX DMA: */
-	dma->chan_rx = dma_request_chan(dev, "rx");
+	dma->chan_rx = devm_dma_request_chan(dev, "rx");
 	if (IS_ERR(dma->chan_rx)) {
 		ret = PTR_ERR(dma->chan_rx);
 		if (ret != -ENODEV && ret != -EPROBE_DEFER)
 			dev_err(dev, "can't request DMA rx channel (%d)\n", ret);
-		dma->chan_rx = NULL;
-		goto dma_exit;
+		goto release_group;
 	}
 
+	/*
+	 * DMA init succeeded. Remove the group marker but keep all resources
+	 * bound to the device, they will be freed at device removal.
+	 */
+	devres_remove_group(dev, group);
+
 	lpi2c_imx->can_use_dma = true;
 	lpi2c_imx->dma = dma;
 	return 0;
 
-dma_exit:
-	dma_exit(dev, dma);
+release_group:
+	/*
+	 * DMA init failed. Release ALL resources allocated inside this
+	 * group (dma memory, TX channel if already acquired, etc.) so
+	 * that a successful PIO-mode probe does not hold unused resources
+	 * for the entire device lifetime.
+	 */
+	devres_release_group(dev, group);
 	return ret;
 }
 
-- 
2.43.0
Re: [PATCH v2] i2c: imx-lpi2c: fix resource leaks switching to devm_dma_request_chan()
Posted by Frank Li 4 days, 5 hours ago
On Wed, May 20, 2026 at 05:33:23PM +0800, Carlos Song (OSS) wrote:
> From: Carlos Song <carlos.song@nxp.com>
>
> The LPI2C driver requests DMA channels using dma_request_chan(), but
> never releases them in lpi2c_imx_remove(), resulting in DMA channel
> leaks every time the driver is unloaded.
>
> Additionally, when lpi2c_dma_init() successfully requests the TX DMA
> channel but fails to request the RX DMA channel, the probe falls back
> to PIO mode and completes successfully. Since probe succeeds, the devres
> framework will not trigger any cleanup, leaving the TX DMA channel and
> the memory allocated for the dma structure held for the lifetime of the
> device even though DMA is never used.
>
> Switch to devm_dma_request_chan() to let the device core manage DMA
> channel lifetime automatically. Wrap all allocations within a devres
> group so that devres_release_group() can release all partially acquired
> resources when DMA init fails and probe continues in PIO mode.
>
> Fixes: a09c8b3f9047 ("i2c: imx-lpi2c: add eDMA mode support for LPI2C")
> Cc: stable@vger.kernel.org
> Signed-off-by: Carlos Song <carlos.song@nxp.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> Change for v2:
>   - Wrap all allocations in lpi2c_dma_init() within a devres group so
>     that devres_release_group() releases all partially acquired resources
>     (dma structure memory, TX DMA channel) when DMA init fails and probe
>     continues in PIO mode. Without this, a successful TX channel request
>     followed by a failed RX channel request would leave the TX channel
>     and dma structure held for the lifetime of the device.
> ---
>  drivers/i2c/busses/i2c-imx-lpi2c.c | 53 ++++++++++++++++++------------
>  1 file changed, 32 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c b/drivers/i2c/busses/i2c-imx-lpi2c.c
> index 6e298424de5e..dedcc24e63ec 100644
> --- a/drivers/i2c/busses/i2c-imx-lpi2c.c
> +++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
> @@ -1383,55 +1383,66 @@ static int lpi2c_imx_init_recovery_info(struct lpi2c_imx_struct *lpi2c_imx,
>  	return 0;
>  }
>
> -static void dma_exit(struct device *dev, struct lpi2c_imx_dma *dma)
> -{
> -	if (dma->chan_rx)
> -		dma_release_channel(dma->chan_rx);
> -
> -	if (dma->chan_tx)
> -		dma_release_channel(dma->chan_tx);
> -
> -	devm_kfree(dev, dma);
> -}
> -
>  static int lpi2c_dma_init(struct device *dev, dma_addr_t phy_addr)
>  {
>  	struct lpi2c_imx_struct *lpi2c_imx = dev_get_drvdata(dev);
>  	struct lpi2c_imx_dma *dma;
> +	void *group;
>  	int ret;
>
> -	dma = devm_kzalloc(dev, sizeof(*dma), GFP_KERNEL);
> -	if (!dma)
> +	/*
> +	 * Open a devres group so that all resources allocated within
> +	 * this function can be released together if DMA init fails but
> +	 * probe continues in PIO mode.
> +	 */
> +	group = devres_open_group(dev, NULL, GFP_KERNEL);
> +	if (!group)
>  		return -ENOMEM;
>
> +	dma = devm_kzalloc(dev, sizeof(*dma), GFP_KERNEL);
> +	if (!dma) {
> +		ret = -ENOMEM;
> +		goto release_group;
> +	}
> +
>  	dma->phy_addr = phy_addr;
>
>  	/* Prepare for TX DMA: */
> -	dma->chan_tx = dma_request_chan(dev, "tx");
> +	dma->chan_tx = devm_dma_request_chan(dev, "tx");
>  	if (IS_ERR(dma->chan_tx)) {
>  		ret = PTR_ERR(dma->chan_tx);
>  		if (ret != -ENODEV && ret != -EPROBE_DEFER)
>  			dev_err(dev, "can't request DMA tx channel (%d)\n", ret);
> -		dma->chan_tx = NULL;
> -		goto dma_exit;
> +		goto release_group;
>  	}
>
>  	/* Prepare for RX DMA: */
> -	dma->chan_rx = dma_request_chan(dev, "rx");
> +	dma->chan_rx = devm_dma_request_chan(dev, "rx");
>  	if (IS_ERR(dma->chan_rx)) {
>  		ret = PTR_ERR(dma->chan_rx);
>  		if (ret != -ENODEV && ret != -EPROBE_DEFER)
>  			dev_err(dev, "can't request DMA rx channel (%d)\n", ret);
> -		dma->chan_rx = NULL;
> -		goto dma_exit;
> +		goto release_group;
>  	}
>
> +	/*
> +	 * DMA init succeeded. Remove the group marker but keep all resources
> +	 * bound to the device, they will be freed at device removal.
> +	 */
> +	devres_remove_group(dev, group);
> +
>  	lpi2c_imx->can_use_dma = true;
>  	lpi2c_imx->dma = dma;
>  	return 0;
>
> -dma_exit:
> -	dma_exit(dev, dma);
> +release_group:
> +	/*
> +	 * DMA init failed. Release ALL resources allocated inside this
> +	 * group (dma memory, TX channel if already acquired, etc.) so
> +	 * that a successful PIO-mode probe does not hold unused resources
> +	 * for the entire device lifetime.
> +	 */
> +	devres_release_group(dev, group);
>  	return ret;
>  }
>
> --
> 2.43.0
>