Re: [PATCH v9 04/18] 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 2 potential issue(s) to consider:
- [High] Standard SPI memory operations are erroneously rejected on controllers lacking the `per_op_freq` capability if the device tree sets `post_config_max_speed_hz` equal to `max_speed_hz`.
- [Medium] `post_config_max_speed_hz` is not clamped to the controller's maximum speed limit, leading to incorrect theoretical operation duration estimates.
--

commit 77b46afe94c20b6f95b9ec99806440ce9c5f6df0
Author: Santhosh Kumar K <s-k6@ti.com>

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
ops and extends spi_mem_default_supports_op() to require the per_op_freq
controller capability for these operations.

> diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
> index 5f973ebfb8b6e..cd4bc4c914e7f 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)) {

[Severity: High]
Will this erroneously reject standard SPI memory operations on controllers
lacking the per_op_freq capability if the device tree sets
post_config_max_speed_hz equal to max_speed_hz?

If the device tree specifies a spi-max-post-config-frequency-hz identical to
spi-max-frequency, standard generic operations initially have op->max_freq = 0,
which spi_mem_adjust_op_freq() clamps to max_speed_hz.

Subsequently, spi_mem_default_supports_op() checks if
op->max_freq == mem->spi->post_config_max_speed_hz to identify
post-configuration operations. When the max limits are equal, this condition
triggers for all standard operations, strictly demanding the per_op_freq
capability. Controllers without this capability will reject the standard
operations.

Does this overloaded use of the max_freq integer act as an operation type flag
that leads to this conflict?

>  		if (!spi_mem_controller_is_capable(ctlr, per_op_freq))
>  			return false;
>  	}
> @@ -623,9 +624,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]
Will this early return fail to clamp post_config_max_speed_hz to the
controller's maximum speed limit?

If the device tree specifies a spi-max-post-config-frequency-hz that exceeds
the SPI controller's absolute max_speed_hz, the post_config_max_speed_hz value
is left unbounded.

Because spi_mem_calc_op_duration() calculates time based on this unbounded
value, it could lead to incorrect theoretical operation duration estimates and
prefer operations that appear fast theoretically but are physically clamped to
a slower speed later.

>  	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/20260825171737.776052-1-s-k6@ti.com?part=4