Re: [PATCH v6 04/17] spi: spi-mem: teach spi_mem_adjust_op_freq() about post-config ops

Hello Santhosh,

On 23/07/2026 at 11:34:42 GMT, [email protected] wrote:

> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] Using strict equality `op->max_freq == post_config_max_speed_hz` as a sentinel is brittle, breaks high-speed modes, and can inappropriately elevate frequencies of un-tuned operations.
> - [High] Ops running at the post-configuration frequency bypass the `per_op_freq` capability check, potentially causing unsupported controller behavior.
> --
>
> --- Patch [4]: [PATCH v6 04/17] spi: spi-mem: teach spi_mem_adjust_op_freq() about post-config ops ---
> commit 929442442d981a80e44b25e60b6fa3ecd9dc8a62
> Author: Santhosh Kumar K <[email protected]>
>
> spi: spi-mem: teach spi_mem_adjust_op_freq() about post-config ops
>
> This commit extends spi_mem_adjust_op_freq() with a bypass for post-config 
> operations. If an operation's maximum frequency matches the configured
> post-configuration maximum speed, it remains uncapped, while all other
> operations are capped to the base maximum speed.
>
>> diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
>> index a88b9f0383564..e20eca1b82457 100644
>> --- a/drivers/spi/spi-mem.c
>> +++ b/drivers/spi/spi-mem.c
>> @@ -591,9 +591,18 @@ EXPORT_SYMBOL_GPL(spi_mem_adjust_op_size);
> [ ... ]
>>  void spi_mem_adjust_op_freq(struct spi_mem *mem, struct spi_mem_op *op)
>>  {
>> +	if (mem->spi->post_config_max_speed_hz &&
>> +	    op->max_freq == mem->spi->post_config_max_speed_hz)
>> +		return;
>
> [Severity: High]
> Does this strict equality check reliably identify post-configuration ops?
>
> If a hardware-defined operation limit exceeds the board's tuned rate, the
> exact equality check will fail, demoting the operation to the untuned base
> rate and wasting the PHY tuning.
>
> Conversely, if an untuned operation happens to have a datasheet limit that
> is identical to the tuned board limit, could it bypass the base rate cap
> and execute at an unsafe elevated frequency?

I am not sure this issue is relevant.

> [Severity: High]
> Additionally, does bypassing the cap for post-configuration speeds break the
> frequency capability check in spi_mem_default_supports_op()?
>
> In spi_mem_default_supports_op(), the capability check is guarded by:
>
> 	if (op->max_freq &&
> 	    op->max_freq < mem->spi->max_speed_hz) {
> 		if (!spi_mem_controller_is_capable(ctlr, per_op_freq))
> 			return false;
> 	}
>
> Because post-config speeds are typically greater than max_speed_hz, the check
> op->max_freq < mem->spi->max_speed_hz evaluates to false.
>
> Does this cause controllers that lack per-operation frequency switching to skip
> the verification entirely and incorrectly accept operations requiring dynamic
> clock rates?

However this one is plausible. Could you please verify? You can just
drop the boolean enabling the multi frequency capability in the spi
controller driver and you'll see how it behaves.

Thanks,
Miquèl