Re: [PATCH 3/6] arm64: dts: qcom: sc8280xp-lenovo-thinkpad-x13s: set GPI DMA channels

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Incorrect channel reference in error cleanup path of gpi_ch_init causes resource leaks and hardware state inconsistency.
--

commit de0b4a765a18a5d7af04a98b890d09dd31cb2362
Author: Icenowy Zheng <zhengxingda@iscas.ac.cn>

arm64: dts: qcom: sc8280xp-lenovo-thinkpad-x13s: set GPI DMA channels

This commit adds GPI interface availability information to the device tree by
setting the dma-channel-mask and marking the status as okay. This ensures
that HLOS does not touch restricted GPI interfaces, preventing system hangs.

> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
> index d84ca010ab9dc..119eeb1560e16 100644
> --- a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
> +++ b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
> @@ -704,14 +704,17 @@ &dispcc0 {
>  };
>  
>  &gpi_dma0 {
> +	dma-channel-mask = <0x3>;

[Severity: High]
This is a pre-existing issue, but since this patch enables GPI DMA interfaces
that might fail to initialize, it could expose an error cleanup bug in the
GPI driver itself.

Looking at the error paths in drivers/dma/qcom/gpi.c:gpi_ch_init():

    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);
        }
        i = 2;
    error_alloc_chan:
        for (i = i - 1; i >= 0; i--)
            gpi_reset_chan(gchan, GPI_CH_CMD_DE_ALLOC);

Are these loops referencing the correct channel during cleanup?

By using the fixed function argument gchan instead of the iterated channel
&gpii->gchan[i] in gpi_send_cmd() and gpi_reset_chan(), it appears this path
would repeatedly issue reset and de-alloc commands to the same fixed channel
while entirely skipping the cleanup for the successfully allocated or started
channels.

Could this leak the previously initialized DMA channels and leave their
hardware state active, potentially causing hardware lockups?

>  	status = "okay";
>  };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260602081451.3808833-1-zhengxingda@iscas.ac.cn?part=3