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

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

Posted by Chandana Kishori Chiluveru 4 days, 10 hours ago

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>
---
 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..8f09eaf4e714 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(6, 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] dmaengine: qcom: gpi: Fix SE ID and SE Protocol fields widths in SCRATCH_0

Posted by Konrad Dybcio 4 days, 9 hours ago

On 7/20/26 5:27 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>
> ---
>  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..8f09eaf4e714 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(6, 4)

There exist protocols, such as QSPI with id=9, so this surely can't
be right (unless there's another bitfield for that?)

Konrad

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

Posted by Chandana Kishori Chiluveru 3 days, 14 hours ago

HI Konrad

On 7/20/26 08:47, Konrad Dybcio wrote:
> On 7/20/26 5:27 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>
>> ---
>>   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..8f09eaf4e714 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(6, 4)
> There exist protocols, such as QSPI with id=9, so this surely can't

Its my bad, The PROTO field should be [8:4]. I will correct this in the 
next patch

> be right (unless there's another bitfield for that?)
>
> Konrad
>