Re: [PATCH v5 15/17] mtd: spinand: negotiate optimal controller operating point before dirmap creation

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Ranked fallback optimization fails to downgrade continuous read cache variants, leading to execution of untuned operations at tuned high frequencies.
--

--- Patch [15]: [PATCH v5 15/17] mtd: spinand: negotiate optimal controller operating point before dirmap creation ---
commit 7c43cf072fbe0a771ec2adb617714b7d258f3161
Author: Santhosh Kumar K <s-k6@ti.com>

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

This commit moves dirmap creation to the probe function, after controller
optimization is completed. It introduces a ranked fallback mechanism that 
iterates through read variants if the pre-selected variant fails controller
tuning, applying the validated PHY frequency to the dirmap descriptors.

> diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
> index b6896760b0b87..58bf7f26b49ba 100644
> --- a/drivers/mtd/nand/spi/core.c
> +++ b/drivers/mtd/nand/spi/core.c

[ ... ]

> @@ -2050,6 +2055,175 @@ static void spinand_cleanup(struct spinand_device *spinand)
>  	kfree(spinand->scratchbuf);
>  }
>  
> +/*
> + * spinand_try_ranked_variant() - Try controller optimization on variants in

[ ... ]

> +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;
> +	const struct spi_mem_op *best;
> +	int ret;
> +
> +	if (!variants)
> +		return false;
> +
> +	while ((best = spinand_op_find_best_variant(spinand, variants, iface,
> +						    *tried_mask))) {
> +		*tried_mask |= BIT(best - variants->ops);
> +		spinand->max_read_op = *best;
> +		spinand->max_read_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) {
> +			if (iface == ODTR)
> +				spinand->odtr_op_templates.read_cache = best;
> +			else
> +				spinand->ssdr_op_templates.read_cache = best;

[Severity: High]
If the controller tuning fails for the best variant but succeeds for a slower
fallback variant, this code updates the read_cache to the fallback variant
but does not update or disable cont_read_cache.

Earlier in spinand_create_dirmap(), the untuned continuous read operation
is assigned the high PHY-validated frequency of this fallback variant:

spinand_create_dirmap() {
    ...
    if (secondary_op) {
        info.secondary_op_tmpl = *spinand->op_templates->cont_read_cache;
        ...
        info.secondary_op_tmpl.max_freq = spinand->max_read_op.max_freq;
    }
    ...
}

Could this result in executing rejected, untuned continuous read operations
at the high frequency of the fallback variant, potentially causing data
corruption?

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

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