Re: [PATCH v3 04/13] spi: spi-mem: add spi_mem_apply_base_freq_cap()

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hi Santhosh,

On 04/06/2026 at 17:40:26 +0530, Santhosh Kumar K <s-k6@ti.com> wrote:

> Hello Miquel,
>
> On 28/05/26 14:13, Miquel Raynal wrote:
>> Hi Santhosh,
>> 
>>> --- a/drivers/spi/spi-mem.c
>>> +++ b/drivers/spi/spi-mem.c
>>> @@ -398,7 +398,11 @@ int spi_mem_exec_op(struct spi_mem *mem, const struct spi_mem_op *op)
>>>   	u8 *tmpbuf;
>>>   	int ret;
>>>   -	/* Make sure the operation frequency is correct before going
>>> futher */
>>> +	/*
>>> +	 * Ops not configured for maximum speed are limited to the conservative
>>> +	 * base speed; spi_mem_adjust_op_freq() then caps to the device maximum.
>>> +	 */
>>> +	spi_mem_apply_base_freq_cap(mem, (struct spi_mem_op *)op);
>>>   	spi_mem_adjust_op_freq(mem, (struct spi_mem_op *)op);
>> There are many more spi_mem_adjust_op_freq() calls in the core where
>> we would
>> not apply the base frequency. Aren't we missing these places? Wouldn't it
>> be more appropriate to call spi_mem_apply_base_freq_cap() at the beginning
>> of spi_mem_adjust_op_freq() ?
>
> Moving spi_mem_apply_base_freq() into spi_mem_adjust_op_freq() would
> affect operation selection.

Yes, and that is precisely what we want.

> spi_mem_adjust_op_freq() is used during variant evaluation through
> spinand_select_op_variant() and spi_mem_calc_op_duration().

And this is a one time specific initialization path. There we need more
logic, probably.

> Applying the
> cap there would cause all candidates to be evaluated at base_speed_hz
> rather than the actual max_speed_hz. As a result, variant selection
> could converge on a different operation than the one that would be
> optimal.

But this is only a single place. Whereas in your implementation you push
the complexity in every place using the spi mem interface, I don't think
this is wise as you will likely end up with many unseen corner cases.

Thanks,
Miquèl