Re: [PATCH 2/2] spi: cadence-xspi: support 4bytes sdma-io-width

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Mixing 32-bit and 8-bit stream accessors on the same SDMA port causes data corruption on big-endian systems and potential bus faults.
- [Low] Casting an __iomem pointer directly to uintptr_t drops the __iomem annotation and the alignment check on sdmabase is redundant.
--

commit e78d668225027044d0694ef2be9788b6fddb0ef3
Author: Jisheng Zhang <jszhang@kernel.org>

spi: cadence-xspi: support 4bytes sdma-io-width

This commit adds support for 4-byte SDMA I/O width to the Cadence XSPI
controller. It introduces new read and write functions that utilize 32-bit
I/O accessors when the buffer is aligned and the hardware width is configured
to 4 bytes.

> diff --git a/drivers/spi/spi-cadence-xspi.c b/drivers/spi/spi-cadence-xspi.c
> index 895b4b3276a56..c1fb749540e7a 100644
> --- a/drivers/spi/spi-cadence-xspi.c
> +++ b/drivers/spi/spi-cadence-xspi.c

[ ... ]

> @@ -578,6 +579,38 @@ static int cdns_xspi_controller_init(struct cdns_xspi_dev *cdns_xspi)
>  	return 0;
>  }
>  
> +static inline void cdns_xspi_sdma_read(struct cdns_xspi_dev *cdns_xspi, size_t len)
> +{
> +	void __iomem *src = cdns_xspi->sdmabase;
> +	void *buf = cdns_xspi->in_buffer;
> +	size_t offset = 0;
> +
> +	if (cdns_xspi->sdma_io_width == 4) {
> +		if (IS_ALIGNED((uintptr_t)src, 4) && IS_ALIGNED((uintptr_t)buf, 4)) {

This isn't a bug, but does casting the __iomem pointer to uintptr_t drop the
address space annotation and trigger a sparse warning?

Since sdmabase is mapped via devm_ioremap_resource, is the alignment check on
src redundant?

> +			ioread32_rep(src, buf, len >> 2);
> +			offset = len & ~0x3;
> +			len -= offset;
> +		}
> +	}
> +	ioread8_rep(src, (u8 *)buf + offset, len);

Can mixing 32-bit and 8-bit stream accessors on the same SDMA port cause data
corruption on big-endian architectures?

On big-endian systems, reading memory in 32-bit native words places bytes in a
different order than processing them sequentially in 8-bit chunks.

Additionally, if the hardware port is explicitly configured for 32-bit I/O,
could falling back to 8-bit accessors for remainders or unaligned buffers
trigger bus faults?

Using 32-bit stream accessors exclusively with a temporary 32-bit variable and
memcpy for unaligned buffers or remainders would resolve this.

> +}
> +
> +static inline void cdns_xspi_sdma_write(struct cdns_xspi_dev *cdns_xspi, size_t len)
> +{
> +	void __iomem *dst = cdns_xspi->sdmabase;
> +	const void *buf = cdns_xspi->out_buffer;
> +	size_t offset = 0;
> +
> +	if (cdns_xspi->sdma_io_width == 4) {
> +		if (IS_ALIGNED((uintptr_t)dst, 4) && IS_ALIGNED((uintptr_t)buf, 4)) {
> +			iowrite32_rep(dst, buf, len >> 2);
> +			offset = len & ~0x3;
> +			len -= offset;
> +		}
> +	}
> +	iowrite8_rep(dst, (const u8 *)buf + offset, len);

Does the same accessor mixing issue apply here when writing to the SDMA port?

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260511031732.3199-1-jszhang@kernel.org?part=2