Re: [PATCH v9 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>,
	<sebin.francis@ti.com>,  <u-kumar1@ti.com>,  <a-dutta@ti.com>
Subject: Re: [PATCH v9 16/18] mtd: spinand: negotiate optimal controller operating point before dirmap creation
Date: Fri, 04 Sep 2026 18:23:36 +0200	[thread overview]
Message-ID: <8733vp0yuv.fsf@bootlin.com> (raw)
In-Reply-To: <20260825171737.776052-17-s-k6@ti.com> (Santhosh Kumar K.'s message of "Tue, 25 Aug 2026 22:47:35 +0530")

Hello Santhosh,

> +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;

I believe you almost addressed the concerns raised in v7 about
cont_read_possible, except what you do here is still a bit dangerous
since you could re-enable continuous reads on no_cs_assertion capable
controllers. I believe you should clear the flag only, never risk to set
it, something like:

if (!templates->cont_read_cache)
    spinand->cont_read_possible = false;

With this fixed,

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

I believe I will take this patch only after Mark takes the batch before,
which looks mostly good for my non-spi-expert eyes. Either Mark shares
an immutable tag, or I wait one cycle.

Thanks,
Miquèl

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