Re: [PATCH v5 05/17] spi: spi-mem: add execute_tuning callback and spi_mem_execute_tuning()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] `spi_mem_execute_tuning()` invokes the hardware controller's tuning callback without locking the SPI bus or acquiring runtime PM references.
- [Medium] `spi_mem_execute_tuning()` fails to check for GPIO Chip Select usage, leading to hardware tuning without an asserted chip select.
--

--- Patch [5]: [PATCH v5 05/17] spi: spi-mem: add execute_tuning callback and spi_mem_execute_tuning() ---
commit 6a445325da668ad3d841d5bfb6bc2ba5a8bc4d99
Author: Santhosh Kumar K <s-k6@ti.com>

spi: spi-mem: add execute_tuning callback and spi_mem_execute_tuning()

Add an execute_tuning callback to struct spi_controller_mem_ops. The
callback receives a mandatory read op template and an optional write op
template. On success the controller sets op->max_freq in each provided
template to the validated clock rate.

> diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
> index e20eca1b82457..ae9e2d7ac6f3c 100644
> --- a/drivers/spi/spi-mem.c
> +++ b/drivers/spi/spi-mem.c

[ ... ]

> @@ -660,6 +660,37 @@ u64 spi_mem_calc_op_duration(struct spi_mem *mem, struct spi_mem_op *op)
>  }
>  EXPORT_SYMBOL_GPL(spi_mem_calc_op_duration);
>  
> +/**
> + * spi_mem_execute_tuning() - Execute controller tuning procedure
> + * @mem: the SPI memory device
> + * @read_op: read operation template (mandatory)
> + * @write_op: write operation template (optional, may be NULL)
> + *
> + * Requests the controller to perform tuning for high-speed operation
> + * using the provided op templates. On success the controller callback
> + * sets @read_op->max_freq (and @write_op->max_freq when non-NULL) to
> + * the validated clock rate.
> + *
> + * Return: 0 on success, -EINVAL if @mem or @read_op is NULL,
> + *         -EOPNOTSUPP if the controller doesn't support tuning,
> + *         or a negative error code on failure.
> + */
> +int spi_mem_execute_tuning(struct spi_mem *mem, struct spi_mem_op *read_op,
> +			   struct spi_mem_op *write_op)
> +{
> +	struct spi_controller *ctlr;
> +
> +	if (!mem || !read_op)
> +		return -EINVAL;
> +
> +	ctlr = mem->spi->controller;
> +	if (!ctlr->mem_ops || !ctlr->mem_ops->execute_tuning)
> +		return -EOPNOTSUPP;

[Severity: Medium]
Other spi-mem operations like spi_mem_exec_op() and spi_mem_poll_status()
verify that a GPIO chip select isn't being used by checking
!spi_get_csgpiod(mem->spi, 0) before delegating to hardware mem_ops.

Hardware sequencers typically only manage native chip selects. If a GPIO CS
is used, will this skip the check and attempt hardware tuning while the flash
device's chip select remains deasserted?

> +
> +	return ctlr->mem_ops->execute_tuning(mem, read_op, write_op);

[Severity: High]
Does this execute the hardware tuning callback without locking the SPI bus or
acquiring runtime PM references?

Other wrappers that invoke hardware controller callbacks directly (such as
spi_mem_exec_op() and spi_mem_poll_status()) are wrapped in
spi_mem_access_start() and spi_mem_access_end() to power up the hardware
and lock the bus against concurrent transfers.

Should spi_mem_access_start() be called here before execute_tuning()?

> +}
> +EXPORT_SYMBOL_GPL(spi_mem_execute_tuning);

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