[PATCH v2] dmaengine: qcom: gpi: Fix SE ID and SE Protocol fields widths in SCRATCH_0

[PATCH v2] dmaengine: qcom: gpi: Fix SE ID and SE Protocol fields widths in SCRATCH_0

[Chandana Kishori Chiluveru]

GPII_n_CH_k_SCRATCH_0_SEID is currently defined as GENMASK(2, 0), only
3 bits wide. Serial engine indices >= 8 get silently truncated by
FIELD_PREP()/u32_encode_bits() when written into the scratch register,
causing the GPI channel to bind to the wrong serial engine. The
channel allocate command still succeeds (it doesn't require the
target SE to respond), but the subsequent channel start command times
out waiting for hardware to complete the handoff to the (incorrectly
bound) SE, and gpi_send_cmd() returns -EIO leading to GSI usecase failures
for SE8 and SE9 serial engines.

Older documentation, such as the SDM845 hardware programming guide,
describes the SE ID field as bits [2:0]. More recent QUP hardware
documentation defines the field as bits [3:0]. Use the wider definition
to match current hardware documentation while remaining compatible with
existing platforms and enabling serial engine IDs greater than 7.

Also update the SE Protocol field definition to match the documented
register layout and keep both fields correctly packed.

Fixes: 5d0c3533a19f ("dmaengine: qcom: Add GPI dma driver")
Signed-off-by: Chandana Kishori Chiluveru <chandana.chiluveru@oss.qualcomm.com>
---
Changes in v2:
- Updated the protocol field value properly
- Link to v1: https://patch.msgid.link/20260720-fix_se_id_and_se_protocol_fields-v1-1-e41dbb107d97@oss.qualcomm.com
---
 drivers/dma/qcom/gpi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/qcom/gpi.c b/drivers/dma/qcom/gpi.c
index a5055a6273af..fc12cf9122f9 100644
--- a/drivers/dma/qcom/gpi.c
+++ b/drivers/dma/qcom/gpi.c
@@ -207,8 +207,8 @@ enum CNTXT_OFFS {
 
 /* Scratch registers */
 #define GPII_n_CH_k_SCRATCH_0_OFFS(n, k)	(0x20060 + (0x4000 * (n)) + (0x80 * (k)))
-#define GPII_n_CH_k_SCRATCH_0_SEID		GENMASK(2, 0)
-#define GPII_n_CH_k_SCRATCH_0_PROTO		GENMASK(7, 4)
+#define GPII_n_CH_k_SCRATCH_0_SEID		GENMASK(3, 0)
+#define GPII_n_CH_k_SCRATCH_0_PROTO		GENMASK(8, 4)
 #define GPII_n_CH_k_SCRATCH_0_PAIR		GENMASK(20, 16)
 #define GPII_n_CH_k_SCRATCH_0(pair, proto, seid)		\
 			     (FIELD_PREP(GPII_n_CH_k_SCRATCH_0_PAIR, pair)	| \

---
base-commit: 34cf6dafc47441dfb6b356a095b89c3585a93714
change-id: 20260710-fix_se_id_and_se_protocol_fields-d7f0fe616596

Best regards,
--  
Chandana Kishori Chiluveru <chandana.chiluveru@oss.qualcomm.com>

Re: [PATCH v2] dmaengine: qcom: gpi: Fix SE ID and SE Protocol fields widths in SCRATCH_0

[sashiko-bot]

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

Pre-existing issues:
- [High] Missing write memory barrier between copying TREs to the DMA ring buffer and ringing the hardware doorbell.
- [High] Missing read memory barrier after reading the event ring read pointer and before reading the event descriptor from DMA memory.
--

--- Patch [1]: [PATCH v2] dmaengine: qcom: gpi: Fix SE ID and SE Protocol fields widths in SCRATCH_0 ---
commit 57c9b90268b2dd84de12a839b2d8897b5356eee0
Author: Chandana Kishori Chiluveru <chandana.chiluveru@oss.qualcomm.com>

dmaengine: qcom: gpi: Fix SE ID and SE Protocol fields widths in SCRATCH_0

This commit fixes silently truncated serial engine indices when written into
the scratch register. It widens the GPII_n_CH_k_SCRATCH_0_SEID mask to
GENMASK(3, 0) and adjusts the GPII_n_CH_k_SCRATCH_0_PROTO mask to GENMASK(8, 4)
to match the documented register layout.

Link: https://patch.msgid.link/20260720-fix_se_id_and_se_protocol_fields-v1-1-e41dbb107d97@oss.qualcomm.com

> diff --git a/drivers/dma/qcom/gpi.c b/drivers/dma/qcom/gpi.c
> index a5055a6273af6..fc12cf9122f9a 100644
> --- a/drivers/dma/qcom/gpi.c
> +++ b/drivers/dma/qcom/gpi.c
> @@ -207,8 +207,8 @@ enum CNTXT_OFFS {
>  
>  /* Scratch registers */
>  #define GPII_n_CH_k_SCRATCH_0_OFFS(n, k)	(0x20060 + (0x4000 * (n)) + (0x80 * (k)))
> -#define GPII_n_CH_k_SCRATCH_0_SEID		GENMASK(2, 0)
> -#define GPII_n_CH_k_SCRATCH_0_PROTO		GENMASK(7, 4)
> +#define GPII_n_CH_k_SCRATCH_0_SEID		GENMASK(3, 0)
> +#define GPII_n_CH_k_SCRATCH_0_PROTO		GENMASK(8, 4)
>  #define GPII_n_CH_k_SCRATCH_0_PAIR		GENMASK(20, 16)

While reviewing this driver, I noticed a couple of potential memory ordering
concerns in other parts of the file.

[Severity: High]
This is a pre-existing issue, but was not introduced by this patch. Is it
possible for the hardware to fetch uninitialized descriptors here?

In gpi_issue_pending(), the code queues descriptors via gpi_queue_xfer()
(which performs a memcpy()) and then rings the doorbell:

drivers/dma/qcom/gpi.c:gpi_issue_pending() {
    ...
	gpi_desc->db = ch_ring->wp;
	gpi_write_ch_db(gchan, &gchan->ch_ring, gpi_desc->db);
    ...
}

Because gpi_write_ch_db() uses writel_relaxed() without a preceding
dma_wmb(), weakly ordered architectures like ARM64 might reorder the
doorbell IO write ahead of the descriptor memory writes. Could this allow
the DMA controller to fetch stale or uninitialized descriptors from memory?


[Severity: High]
This is a pre-existing issue, but was not introduced by this patch. Can the
CPU speculatively prefetch a stale event descriptor?

In gpi_process_events(), the read pointer is fetched from the device via
readl_relaxed(), but there is no subsequent dma_rmb() before reading the
event descriptor from DMA memory:

drivers/dma/qcom/gpi.c:gpi_process_events() {
    ...
	cntxt_rp = gpi_read_reg(gpii, gpii->ev_ring_rp_lsb_reg);
	rp = to_virtual(ev_ring, cntxt_rp);

	do {
		while (rp != ev_ring->rp) {
			gpi_event = ev_ring->rp;
			chid = gpi_event->xfer_compl_event.chid;
    ...
}

Since control dependencies do not enforce memory ordering on ARM64, the
memory read of gpi_event->xfer_compl_event.chid can be reordered ahead of
the register read. Could a dma_rmb() be needed after reading the RP register
to prevent processing a stale event descriptor?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721-fix_se_id_and_se_protocol_fields-v2-1-ee1d63d6c549@oss.qualcomm.com?part=1

Re: [PATCH v2] dmaengine: qcom: gpi: Fix SE ID and SE Protocol fields widths in SCRATCH_0

[Konrad Dybcio]

On 7/21/26 1:15 PM, Chandana Kishori Chiluveru wrote:
> GPII_n_CH_k_SCRATCH_0_SEID is currently defined as GENMASK(2, 0), only
> 3 bits wide. Serial engine indices >= 8 get silently truncated by
> FIELD_PREP()/u32_encode_bits() when written into the scratch register,
> causing the GPI channel to bind to the wrong serial engine. The
> channel allocate command still succeeds (it doesn't require the
> target SE to respond), but the subsequent channel start command times
> out waiting for hardware to complete the handoff to the (incorrectly
> bound) SE, and gpi_send_cmd() returns -EIO leading to GSI usecase failures
> for SE8 and SE9 serial engines.
> 
> Older documentation, such as the SDM845 hardware programming guide,
> describes the SE ID field as bits [2:0]. More recent QUP hardware
> documentation defines the field as bits [3:0]. Use the wider definition
> to match current hardware documentation while remaining compatible with
> existing platforms and enabling serial engine IDs greater than 7.
> 
> Also update the SE Protocol field definition to match the documented
> register layout and keep both fields correctly packed.
> 
> Fixes: 5d0c3533a19f ("dmaengine: qcom: Add GPI dma driver")
> Signed-off-by: Chandana Kishori Chiluveru <chandana.chiluveru@oss.qualcomm.com>
> ---

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad

Re: [PATCH v2] dmaengine: qcom: gpi: Fix SE ID and SE Protocol fields widths in SCRATCH_0

[Mukesh Savaliya]

On 7/21/2026 4:45 PM, Chandana Kishori Chiluveru wrote:
> GPII_n_CH_k_SCRATCH_0_SEID is currently defined as GENMASK(2, 0), only
> 3 bits wide. Serial engine indices >= 8 get silently truncated by
> FIELD_PREP()/u32_encode_bits() when written into the scratch register,
> causing the GPI channel to bind to the wrong serial engine. The
> channel allocate command still succeeds (it doesn't require the
> target SE to respond), but the subsequent channel start command times
> out waiting for hardware to complete the handoff to the (incorrectly
> bound) SE, and gpi_send_cmd() returns -EIO leading to GSI usecase failures
> for SE8 and SE9 serial engines.
> 
> Older documentation, such as the SDM845 hardware programming guide,
> describes the SE ID field as bits [2:0]. More recent QUP hardware
> documentation defines the field as bits [3:0]. Use the wider definition
> to match current hardware documentation while remaining compatible with
> existing platforms and enabling serial engine IDs greater than 7.
> 
> Also update the SE Protocol field definition to match the documented
> register layout and keep both fields correctly packed.
> 
> Fixes: 5d0c3533a19f ("dmaengine: qcom: Add GPI dma driver")
> Signed-off-by: Chandana Kishori Chiluveru <chandana.chiluveru@oss.qualcomm.com>
> ---
> Changes in v2:
> - Updated the protocol field value properly
> - Link to v1: https://patch.msgid.link/20260720-fix_se_id_and_se_protocol_fields-v1-1-e41dbb107d97@oss.qualcomm.com
> ---
>   drivers/dma/qcom/gpi.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Mukesh Kumar Savaliya <mukesh.savaliya@oss.qualcomm.com>