Re: [PATCH v3 05/13] 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 1 potential issue(s) to consider:
- [High] spi_mem_execute_tuning() invokes the controller's execute_tuning callback without acquiring the SPI bus locks or waking the controller via Runtime PM.
--

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

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

This commit adds a new execute_tuning callback to spi_controller_mem_ops
and a corresponding spi_mem_execute_tuning() wrapper to allow drivers to
trigger tuning procedures for high-speed SPI memory operations.

> diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
> index 37d10c4e614b2..98339ac8dafa2 100644
> --- a/drivers/spi/spi-mem.c
> +++ b/drivers/spi/spi-mem.c

[ ... ]

> +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;
> +
> +	return ctlr->mem_ops->execute_tuning(mem, read_op, write_op);

[Severity: High]
Does this code safely access the controller hardware without holding the SPI
bus locks or waking the controller via Runtime PM?

In the spi-mem subsystem, operations that interact directly with controller
hardware via ctlr->mem_ops typically must be wrapped with
spi_mem_access_start() and spi_mem_access_end(). These wrappers ensure
mutual exclusion by acquiring bus_lock_mutex and io_mutex, and power on the
controller via pm_runtime_resume_and_get().

Since spi_mem_execute_tuning() bypasses these wrappers, could the
controller's execute_tuning callback perform direct register accesses
without holding the bus locks? This might allow concurrent SPI operations to
other devices on the same bus to interfere. 

Additionally, if tuning is executed outside of the initial device probe
context, could the controller be runtime-suspended, causing external aborts
or crashes when its registers are accessed?

It appears there might be a deeper architectural conflict here. If
spi_mem_execute_tuning() were to correctly acquire the locks, and a
controller's execute_tuning implementation called spi_mem_exec_op()
internally, wouldn't that attempt to acquire the same non-recursive mutexes
and cause a deadlock?

> +}
> +EXPORT_SYMBOL_GPL(spi_mem_execute_tuning);

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