Re: [PATCH v9 16/18] mtd: spinand: negotiate optimal controller operating point before dirmap creation

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unconditional overwrite of `spinand->cont_read_possible` bypasses system capability checks, risking unsupported continuous reads.
--

commit 6afbb44eacdd7b8d06d61615ac789bcc3b8a5186
Author: Santhosh Kumar K [off-list ref]

mtd: spinand: negotiate optimal controller operating point before dirmap creation

This commit introduces a mechanism to calibrate the PHY before creating the
dirmap so the validated max frequency is embedded into the dirmap operations.
It adds a ranked fallback iteration to find optimal variants if the
pre-selected one fails, updating op templates and continuous read capabilities
accordingly.

[ ... ]

[ ... ]

+static bool spinand_try_ranked_variant(struct spinand_device *spinand,
+				       struct spi_mem *mem,
+				       enum spinand_bus_interface iface,
+				       u32 *tried_mask)
+{
+	const struct spinand_op_variants *variants = spinand->all_read_variants;
+	struct spinand_mem_ops *templates = iface == ODTR ?
+		&spinand->odtr_op_templates : &spinand->ssdr_op_templates;
+	const struct spi_mem_op *best;
+	int ret;
+
+	if (!variants)
+		return false;
+
+	while ((best = spinand_op_find_best_variant(spinand, variants, iface,
+						    *tried_mask, NULL))) {
+		*tried_mask |= BIT(best - variants->ops);
+		spinand->max_read_op = *best;
+		spinand->max_read_op.max_freq = 0;
+		spinand->max_write_op.max_freq = 0;
+		ret = spi_mem_execute_tuning(mem, &spinand->max_read_op,
+					     &spinand->max_write_op);
+		if (ret && ret != -EOPNOTSUPP)
+			dev_dbg(&mem->spi->dev, "%s optimization failed: %d\n",
+				iface == ODTR ? "ODTR" : "SSDR", ret);
+		if (!ret && spinand->max_read_op.max_freq) {
+			templates->read_cache = best;
+			if (spinand->all_cont_read_variants) {
+				templates->cont_read_cache =
+					spinand_find_cont_read_variant(spinand, best, iface);
+				spinand->cont_read_possible =
+					!!templates->cont_read_cache;

[Severity: High]
Does this code unconditionally overwrite the system capability flag for
continuous reads?

Earlier in the initialization sequence, spinand_cont_read_init() may have
cleared spinand->cont_read_possible due to system-level constraints, such as
pipelined ECC requirements or lack of controller support. By setting it here
based solely on the existence of an op template, it appears we might re-enable
continuous reads on hardware that cannot actually support it.

Could this lead to I/O timeouts or potential data corruption when the subsystem
attempts unsupported continuous read operations?

+			}
+			return true;
+		}
+	}
+	return false;
+}

[ ... ]

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