Re: [PATCH v6 10/17] spi: cadence-quadspi: refactor direct read path for PHY support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] cqspi_memcpy_fromio() directly casts an arbitrary destination pointer to u16* and dereferences it, triggering unaligned access exceptions on strict-alignment architectures.
- [High] cqspi_memcpy_fromio() triggers a hardware alignment fault (Data Abort) by issuing __raw_readw() on odd device memory addresses.
- [High] cqspi_memcpy_fromio() delegates bulk copies to memcpy_fromio(), which can fall back to 8-bit I/O reads and violate the strict 16-bit AHB requirement for 8D-8D-8D mode.
--

--- Patch [10]: [PATCH v6 10/17] spi: cadence-quadspi: refactor direct read path for PHY support ---
commit f23703adf58b1895572efb17e7d5f736dc1bf89f
Author: Santhosh Kumar K <s-k6@ti.com>
Subject: spi: cadence-quadspi: refactor direct read path for PHY support

This commit extracts the DMA transfer code from cqspi_direct_read_execute()
into a new cqspi_direct_read_dma() helper. It also adds
cqspi_memcpy_fromio() to handle non-DMA transfers, attempting to guarantee
2-byte-aligned I/O accesses for 8D-8D-8D mode.

> diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
> index 5070ffc53d5e8..322e95b3bc17d 100644
> --- a/drivers/spi/spi-cadence-quadspi.c
> +++ b/drivers/spi/spi-cadence-quadspi.c

[ ... ]

> @@ -1580,8 +1577,61 @@ static int cqspi_direct_read_execute(struct cqspi_flash_pdata *f_pdata,
>  	return ret;
>  }
>  
> +static void cqspi_memcpy_fromio(const struct spi_mem_op *op, void *to,
> +				const void __iomem *from, size_t count)
> +{
> +	if (op->data.buswidth == 8 && op->data.dtr) {
> +		unsigned long from_addr = (unsigned long)from;
> +
> +		/* Handle unaligned start with 2-byte read */
> +		if (count && !IS_ALIGNED(from_addr, 4)) {
> +			*(u16 *)to = __raw_readw(from);

[Severity: High]
Is it safe to cast 'to' directly to a u16 pointer?

If the destination buffer 'to' is not 2-byte aligned, won't this pointer
dereference cause an unaligned access exception on strict-alignment
architectures? Should put_unaligned() be used here instead?

[Severity: High]
Will __raw_readw() cause a hardware alignment fault (Data Abort) on
architectures like ARM if 'from' is an odd address?

Since IS_ALIGNED(from_addr, 4) only checks for 4-byte alignment, an odd
'from' address will enter this block and trigger a 16-bit I/O read on an
unaligned device memory address. Should the driver enforce even address
offsets for these operations?

> +			from += 2;
> +			to += 2;
> +			count -= 2;
> +		}
> +
> +		/* Use 4-byte reads for aligned bulk (no readq for 32-bit) */
> +		if (count >= 4) {
> +			size_t len = round_down(count, 4);
> +
> +			memcpy_fromio(to, from, len);

[Severity: High]
Could memcpy_fromio() fall back to 8-bit I/O reads on some architectures
(like ARM32), violating the controller's strict >=16-bit AHB requirement
for 8D-8D-8D mode?

To guarantee the AHB transaction size irrespective of the architecture's
memcpy_fromio() implementation, would it be better to use explicit 32-bit
or 16-bit I/O accessors (like an explicit __raw_readl() loop with
put_unaligned())?

> +			from += len;
> +			to += len;
> +			count -= len;
> +		}
> +
> +		/* Handle remaining 2 bytes */
> +		if (count)
> +			*(u16 *)to = __raw_readw(from);

[Severity: High]
Similar to the issue above, does this need put_unaligned() to prevent
unaligned memory access exceptions if 'to' is not aligned?

> +
> +		return;
> +	}
> +
> +	memcpy_fromio(to, from, count);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260723110219.3180747-1-s-k6@ti.com?part=10