Re: [PATCH v4 10/16] 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] Casting an arbitrary `void *` buffer pointer to `u16 *` and dereferencing it causes unaligned access faults on strict-alignment architectures.
- [High] Performing 16-bit unaligned reads from memory-mapped I/O if the flash offset is odd causes a Data Abort.
- [High] Silent degradation to 1-byte I/O reads by memcpy_fromio() violates 8D-8D-8D access size requirements.
--

commit d928e9450946bfcaed1dc0c5349081ffad136425
Author: Santhosh Kumar K <s-k6@ti.com>

spi: cadence-quadspi: refactor direct read path for PHY support

Extract the DMA transfer code from cqspi_direct_read_execute() into a
new cqspi_direct_read_dma() helper. Add cqspi_memcpy_fromio() to handle
non-DMA transfers, with 2-byte-aligned I/O accesses for 8D-8D-8D mode.
Transfers shorter than CQSPI_PHY_MIN_DIRECT_READ_LEN bytes always use
the memcpy path; longer transfers use DMA when available.

> diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
> index c94bb8ad5d1dd..5f992fee790a4 100644
> --- a/drivers/spi/spi-cadence-quadspi.c
> +++ b/drivers/spi/spi-cadence-quadspi.c
[ ... ]
> @@ -1506,8 +1503,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]
Will this __raw_readw() cause a Data Abort on ARM architectures if the
flash offset is odd?

The code assumes from_addr is 2-byte aligned, but userspace MTD reads
can specify arbitrary odd offsets. If from_addr is odd,
!IS_ALIGNED(from_addr, 4) evaluates to true and we execute __raw_readw()
on an odd I/O memory address.

Unaligned accesses to device memory strictly cause Alignment Faults on
ARM, even if the underlying hardware ignores the LSB.

Also, can casting the destination buffer to a u16 pointer and dereferencing
it cause unaligned access faults?

The destination buffer, derived from op->data.buf.in, might be unaligned.
Casting an unaligned void pointer to a u16 pointer and dereferencing it
causes an unaligned store exception on strict-alignment architectures.
Does this need to use put_unaligned() instead?

> +			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]
Does memcpy_fromio() guarantee the 2-byte minimum I/O access size required
by the 8D-8D-8D hardware?

If the generic memcpy_fromio() fallback is used by the architecture, or
if the destination buffer is unaligned on architectures like ARM64,
memcpy_fromio() will silently degrade to byte-wise reads.

Issuing 1-byte read requests on the AHB bus violates the hardware's 2-byte
minimum access requirement in 8D-8D-8D mode. Should this use an explicit
loop with ioread32_rep() or __raw_readl() to guarantee the minimum access
size?

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

[Severity: High]
As with the unaligned start handling above, does this unaligned void
pointer cast risk unaligned access faults on strict-alignment architectures?

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

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