Re: [PATCH v4 09/16] spi: cadence-quadspi: skip DDR PHY tuning for 2-byte-address ops (i2383)

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Unchecked dereference of `cqspi->ddata` in `cqspi_mem_op_execute_tuning()` leads to a potential kernel oops.
--

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

spi: cadence-quadspi: skip DDR PHY tuning for 2-byte-address ops (i2383)

This commit introduces a CQSPI_NO_2BYTE_ADDR_PHY_DDR quirk to skip DDR PHY
tuning for 2-byte column addresses. This is done to address erratum i2383 on
AM654, which locks the address phase in PHY DDR mode for such operations.

[0] https://www.ti.com/lit/er/sprz544c/sprz544c.pdf

> diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
> index 05e1b7a2f3433..c94bb8ad5d1dd 100644
> --- a/drivers/spi/spi-cadence-quadspi.c
> +++ b/drivers/spi/spi-cadence-quadspi.c

[ ... ]

> @@ -3864,7 +3879,7 @@ static const struct cqspi_driver_platdata k2g_qspi = {
>  
>  static const struct cqspi_driver_platdata am654_ospi = {
>  	.hwcaps_mask = CQSPI_SUPPORTS_OCTAL | CQSPI_SUPPORTS_QUAD,
> -	.quirks = CQSPI_NEEDS_WR_DELAY,
> +	.quirks = CQSPI_NEEDS_WR_DELAY | CQSPI_NO_2BYTE_ADDR_PHY_DDR,
>  	.execute_tuning = cqspi_am654_ospi_execute_tuning,
>  };
>  

[Severity: Medium]
Looking at the tuning execution path that uses these callbacks, is there a
potential NULL pointer dereference in cqspi_mem_op_execute_tuning()?

drivers/spi/spi-cadence-quadspi.c:cqspi_mem_op_execute_tuning() {
...
	struct cqspi_st *cqspi =
		spi_controller_get_devdata(mem->spi->controller);

	if (!cqspi->ddata->execute_tuning)
		return -EOPNOTSUPP;

	return cqspi->ddata->execute_tuning(mem, read_op, write_op);
}

The cqspi->ddata pointer is initialized via of_device_get_match_data(), which
can return NULL if the driver is bound via sysfs to a device node without a
matching compatible string.

Other uses of ddata in the driver (like in cqspi_probe or cqspi_setup_flash)
include a NULL check before dereferencing it, proving that ddata == NULL is a
supported and handled state.

Should this tuning wrapper also include a defensive check for ddata == NULL?

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