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

From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Santhosh Kumar K <s-k6@ti.com>
Cc: <broonie@kernel.org>,  <robh@kernel.org>,  <krzk+dt@kernel.org>,
	<conor+dt@kernel.org>,  <richard@nod.at>,  <vigneshr@ti.com>,
	<pratyush@kernel.org>,  <mwalle@kernel.org>,
	<takahiro.kuwano@infineon.com>,  <linux-spi@vger.kernel.org>,
	<devicetree@vger.kernel.org>,  <linux-kernel@vger.kernel.org>,
	<linux-mtd@lists.infradead.org>,  <praneeth@ti.com>,
	 <u-kumar1@ti.com>, <a-dutta@ti.com>
Subject: Re: [PATCH v7 16/18] mtd: spinand: negotiate optimal controller operating point before dirmap creation
Date: Thu, 13 Aug 2026 10:35:52 +0200	[thread overview]
Message-ID: <87bjb6l8nr.fsf@bootlin.com> (raw)
In-Reply-To: <20260811183313.1550425-17-s-k6@ti.com> (Santhosh Kumar K.'s message of "Wed, 12 Aug 2026 00:03:11 +0530")

Hi Santhosh,

I have only one comment on the spinand bits.

> +/*
> + * spinand_try_ranked_variant() - Try controller optimization on variants in
> + *				  performance order.
> + * @spinand:    SPI NAND device
> + * @mem:        SPI memory device
> + * @iface:      bus interface to iterate (ODTR or SSDR)
> + * @tried_mask: bitmask of already-tried variant indices; updated on each try
> + *
> + * Iterates the full read variant list in descending performance order,
> + * skipping variants in @tried_mask, and calls execute_tuning on each until
> + * one succeeds. Ranked iteration finds the best available variant without
> + * re-trying already-attempted ones.
> + *
> + * On success, sets spinand->max_read_op and updates the matching
> + * odtr_op_templates.read_cache or ssdr_op_templates.read_cache.
> + */
> +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;
> +		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) {
> +			if (iface == ODTR)
> +				spinand->odtr_op_templates.read_cache = best;
> +			else
> +				spinand->ssdr_op_templates.read_cache = best;
> +			spinand->cont_read_possible = false;

Why do you disable continuous reads? I know it is not the same as the
read template, but it only differs by a few dummy cycles, so everything
should work as expected. I believe without complexifying much the logic
we should be able to support it.

Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/