Re: [PATCH 2/4] dmaengine: qcom: gpi: Calculate UART TRE count per direction

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] dmaengine: qcom: gpi: endianness bug via u32 cast of config struct
--

commit cef09069b78a3ad6018236a1002aee6d9919181a
Author: Naresh Maramaina <naresh.maramaina@oss.qualcomm.com>

dmaengine: qcom: gpi: Calculate UART TRE count per direction

This commit refactors the TRE-count calculation by moving it into a
protocol-specific helper function. It addresses the fact that UART RX and TX
track configuration updates independently and may require different numbers
of TREs, while aiming to preserve the existing behavior for SPI and I2C.

> diff --git a/drivers/dma/qcom/gpi.c b/drivers/dma/qcom/gpi.c
> index fb26a286e5df4..f72b80bf5553c 100644
> --- a/drivers/dma/qcom/gpi.c
> +++ b/drivers/dma/qcom/gpi.c
> @@ -1882,6 +1882,35 @@ gpi_create_serial_tre(struct gchan *chan, struct gpi_desc *desc,
>  	return tre_idx;
>  }
>  
> +/* Calculate the number of TREs required for this transfer. */
> +static u32 gpi_get_nr_tre(struct gchan *gchan,
> +			  enum dma_transfer_direction direction)
> +{
> +	const struct gpi_serial_config *serial;
> +	u32 nr_tre = MAX_TRE;
> +	u8 set_config = true;
> +
> +	switch (gchan->protocol) {
> +	case QCOM_GPI_SPI:
> +	case QCOM_GPI_I2C:
> +		set_config = *(u32 *)gchan->config;

[Severity: Medium]
Will this 32-bit pointer cast extract the correct byte on Big Endian
architectures?

Due to Big Endian byte ordering, casting the configuration structure to a
u32 pointer and truncating the result to a u8 will extract the 4th byte in
memory (e.g., data_pol_high for SPI or high_count for I2C) rather than the
1st byte (set_config). This can result in an incorrect TRE count calculation,
leading to DMA transfer failures, incorrect transfer sizes, or ring buffer
overflows.

While Qualcomm platforms typically run little-endian kernels, the Linux
kernel on ARM/ARM64 can be built with CONFIG_CPU_BIG_ENDIAN, and this driver
lacks a !CPU_BIG_ENDIAN dependency.

Would it be better to use proper struct field access (as done for UART in
this patch) or cast to a u8 pointer to ensure portability across different
architectures?

> +
> +		if (direction == DMA_DEV_TO_MEM)
> +			return 1;
> +		break;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009-add-gpi-dma-support-for-uart-v1-0-c69e728df686@oss.qualcomm.com?part=2