Re: [PATCH v10 04/19] spi: spi-mem: teach spi_mem_adjust_op_freq() about post-config ops
From: sashiko-bot@kernel.org
Date: 2026-09-29 06:51:44
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 [off-list ref] 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.
quoted hunk ↗ jump to hunk
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. [ ... ]
quoted hunk ↗ jump to hunk
@@ -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