[PATCH v1] dmaengine: qcom: gpi: Fix channel cleanup in unwind path

[PATCH v1] dmaengine: qcom: gpi: Fix channel cleanup in unwind path

[Aniket Randive]

Fix three issues in the gpi_ch_init() error path.

Use the indexed channel in the error_start_chan and
error_alloc_chan unwind loops instead of the original gchan
pointer. Otherwise, each iteration operates on the same channel,
leaving sibling channels unreset and undeallocated on failure.

Clear pm_state after freeing the event ring in the error path.
gpi_alloc_ev_chan() sets pm_state to ACTIVE_STATE, but the error
path frees ev_ring without restoring pm_state. As a result,
gpi_free_chan_resources() may attempt to deallocate an already
freed event ring and issue a redundant EV_CMD_DEALLOC.

Also free ch_ring in gpi_alloc_chan_resources() if gpi_ch_init()
fails. The ring is allocated before calling gpi_ch_init(), so a
failure leaves it leaked without this free.

Fix all three issues by unwinding the correct channels, restoring
pm_state to DISABLE_STATE after freeing the event ring, and freeing
ch_ring on gpi_ch_init() failure.

Signed-off-by: Aniket Randive <aniket.randive@oss.qualcomm.com>
---
 drivers/dma/qcom/gpi.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/qcom/gpi.c b/drivers/dma/qcom/gpi.c
index a5055a6273af..c41bfac65ddf 100644
--- a/drivers/dma/qcom/gpi.c
+++ b/drivers/dma/qcom/gpi.c
@@ -1965,16 +1965,19 @@ static int gpi_ch_init(struct gchan *gchan)
 error_start_chan:
 	for (i = i - 1; i >= 0; i--) {
 		gpi_stop_chan(&gpii->gchan[i]);
-		gpi_send_cmd(gpii, gchan, GPI_CH_CMD_RESET);
+		gpi_send_cmd(gpii, &gpii->gchan[i], GPI_CH_CMD_RESET);
 	}
 	i = 2;
 error_alloc_chan:
 	for (i = i - 1; i >= 0; i--)
-		gpi_reset_chan(gchan, GPI_CH_CMD_DE_ALLOC);
+		gpi_reset_chan(&gpii->gchan[i], GPI_CH_CMD_DE_ALLOC);
 error_alloc_ev_ring:
 	gpi_disable_interrupts(gpii);
 error_config_int:
 	gpi_free_ring(&gpii->ev_ring, gpii);
+	write_lock_irq(&gpii->pm_lock);
+	gpii->pm_state = DISABLE_STATE;
+	write_unlock_irq(&gpii->pm_lock);
 exit_gpi_init:
 	return ret;
 }
@@ -2065,6 +2068,8 @@ static int gpi_alloc_chan_resources(struct dma_chan *chan)
 		goto xfer_alloc_err;
 
 	ret = gpi_ch_init(gchan);
+	if (ret)
+		gpi_free_ring(&gchan->ch_ring, gpii);
 
 	mutex_unlock(&gpii->ctrl_lock);
 
-- 
2.34.1

Re: [PATCH v1] dmaengine: qcom: gpi: Fix channel cleanup in unwind path

[Mukesh Savaliya]

On 8/3/2026 5:33 PM, Aniket Randive wrote:
> Fix three issues in the gpi_ch_init() error path.
> 
start here mentioning  the three problems first in generic way.
> Use the indexed channel in the error_start_chan and
> error_alloc_chan unwind loops instead of the original gchan
> pointer. Otherwise, each iteration operates on the same channel,
> leaving sibling channels unreset and undeallocated on failure.

meaning, channel cleanup not happened ? make it simpler anyway.
> 
> Clear pm_state after freeing the event ring in the error path.
> gpi_alloc_ev_chan() sets pm_state to ACTIVE_STATE, but the error
> path frees ev_ring without restoring pm_state. As a result,
> gpi_free_chan_resources() may attempt to deallocate an already
> freed event ring and issue a redundant EV_CMD_DEALLOC.
> 
Looking at your change, it looks we are cleaning up gpii iterating over 
each channel along with setting proper state.  Code seems simpler than 
the commit log, so request you to simplify.

> Also free ch_ring in gpi_alloc_chan_resources() if gpi_ch_init()
> fails. The ring is allocated before calling gpi_ch_init(), so a
> failure leaves it leaked without this free.
> 
> Fix all three issues by unwinding the correct channels, restoring
> pm_state to DISABLE_STATE after freeing the event ring, and freeing
> ch_ring on gpi_ch_init() failure.
> 
> Signed-off-by: Aniket Randive <aniket.randive@oss.qualcomm.com>
> ---
>   drivers/dma/qcom/gpi.c | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/dma/qcom/gpi.c b/drivers/dma/qcom/gpi.c

[...]

Re: [PATCH v1] dmaengine: qcom: gpi: Fix channel cleanup in unwind path

[Aniket RANDIVE]

Thanks Mukesh for the review.

I will update the commit message as per your suggestion.

Thanks,
Aniket

On 8/7/2026 5:10 PM, Mukesh Savaliya wrote:
> 
> 
> On 8/3/2026 5:33 PM, Aniket Randive wrote:
>> Fix three issues in the gpi_ch_init() error path.
>>
> start here mentioning  the three problems first in generic way.
>> Use the indexed channel in the error_start_chan and
>> error_alloc_chan unwind loops instead of the original gchan
>> pointer. Otherwise, each iteration operates on the same channel,
>> leaving sibling channels unreset and undeallocated on failure.
> 
> meaning, channel cleanup not happened ? make it simpler anyway.
>>
>> Clear pm_state after freeing the event ring in the error path.
>> gpi_alloc_ev_chan() sets pm_state to ACTIVE_STATE, but the error
>> path frees ev_ring without restoring pm_state. As a result,
>> gpi_free_chan_resources() may attempt to deallocate an already
>> freed event ring and issue a redundant EV_CMD_DEALLOC.
>>
> Looking at your change, it looks we are cleaning up gpii iterating over 
> each channel along with setting proper state.  Code seems simpler than 
> the commit log, so request you to simplify.
> 
>> Also free ch_ring in gpi_alloc_chan_resources() if gpi_ch_init()
>> fails. The ring is allocated before calling gpi_ch_init(), so a
>> failure leaves it leaked without this free.
>>
>> Fix all three issues by unwinding the correct channels, restoring
>> pm_state to DISABLE_STATE after freeing the event ring, and freeing
>> ch_ring on gpi_ch_init() failure.
>>
>> Signed-off-by: Aniket Randive <aniket.randive@oss.qualcomm.com>
>> ---
>>   drivers/dma/qcom/gpi.c | 9 +++++++--
>>   1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/dma/qcom/gpi.c b/drivers/dma/qcom/gpi.c
> 
> [...]
>