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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] spi-mem: falsely rejects normal ops when max_speed_hz matches post_config speed
- [Medium] spi-mem: premature high-speed execution due to numerical frequency match
- [Medium] spi-mem: missing hardware limit clamp for post-config operations
--

commit 597934102ffdf54ebc8e13a8b68844c8afbc350b
Author: Santhosh Kumar K <s-k6@ti.com>

spi: spi-mem: teach spi_mem_adjust_op_freq() about post-config ops

This patch extends spi_mem_adjust_op_freq() to handle post-config
operations by adding a bypass when op->max_freq matches
post_config_max_speed_hz. It also updates spi_mem_default_supports_op()
to require per-op frequency switching capability for such ops.

> diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
> index b94680b60af95..7916387a4f1d0 100644
> --- a/drivers/spi/spi-mem.c
> +++ b/drivers/spi/spi-mem.c
> @@ -206,7 +206,8 @@ bool spi_mem_default_supports_op(struct spi_mem *mem,
>  		return false;
>  
>  	if (op->max_freq &&
> -	    op->max_freq < mem->spi->max_speed_hz) {
> +	    (op->max_freq < mem->spi->max_speed_hz ||
> +	     op->max_freq == mem->spi->post_config_max_speed_hz)) {
>  		if (!spi_mem_controller_is_capable(ctlr, per_op_freq))
>  			return false;
>  	}

[Severity: High]
Could this falsely reject standard operations if a board's Device Tree
configures spi-max-frequency and spi-max-post-config-frequency-hz to the
same value?

If post_config_max_speed_hz equals max_speed_hz, spi_mem_adjust_op_freq()
will set an untuned operation's max_freq to max_speed_hz. When
spi_mem_default_supports_op() checks this, it sees op->max_freq ==
mem->spi->post_config_max_speed_hz and strictly requires the per_op_freq
capability.

Controllers lacking per_op_freq would fail this check and reject standard
operations, even though the pre-config and post-config frequencies are identical
and no per-op frequency switching is actually needed.

[ ... ]

> @@ -627,9 +628,18 @@ EXPORT_SYMBOL_GPL(spi_mem_adjust_op_size);
>   * Some chips have per-op frequency limitations and must adapt the maximum
>   * speed. This function allows SPI mem drivers to set @op->max_freq to the
>   * maximum supported value.
> + *
> + * When @mem->spi->post_config_max_speed_hz is set, ops with @op->max_freq
> + * equal to that value are treated as post-configuration ops (e.g. PHY-tuned)
> + * and are allowed to run at the full post-config rate. All other ops are
> + * capped to @mem->spi->max_speed_hz, the always-reachable base rate.
>   */
>  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: Medium]
Does this logic prematurely elevate standard untuned operations to run at high
post-config speeds just because their frequency happens to match numerically?

Since spi->post_config_max_speed_hz is unconditionally populated from the
Device Tree during probe:

drivers/spi/spi.c:of_spi_parse_dt() {
    ...
	if (!of_property_read_u32(nc, "spi-max-post-config-frequency-hz", &value))
		spi->post_config_max_speed_hz = value;
    ...
}

If a standard untuned operation specifies a max_freq limit that
serendipitously matches the DT-populated value, it will be treated as a
post-config op and bypass the base-rate safety cap. Could this result in bus
failures or data corruption for unrelated ops?

[Severity: Medium]
Does this early return also miss a hardware limit clamp for post-config
operations?

Unlike standard max_speed_hz, post_config_max_speed_hz isn't clamped to
the controller's maximum supported hardware speed (ctlr->max_speed_hz) during
device setup. If a misconfigured device tree specifies a
spi-max-post-config-frequency-hz that exceeds ctlr->max_speed_hz, this early
return bypasses the fallback clamping logic entirely. Could this cause the
hardware to receive a frequency request exceeding its absolute physical limits?

>  	if (!op->max_freq || op->max_freq > mem->spi->max_speed_hz)
>  		op->max_freq = mem->spi->max_speed_hz;
>  }

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