Re: [PATCH v4 13/16] mtd: spinand: extract variant ranking logic into spinand_op_find_best()

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 v4 13/16] mtd: spinand: extract variant ranking logic into spinand_op_find_best()
Date: Thu, 02 Jul 2026 15:41:27 +0200	[thread overview]
Message-ID: <87v7axpkzc.fsf@bootlin.com> (raw)
In-Reply-To: <20260618073725.84733-14-s-k6@ti.com> (Santhosh Kumar K.'s message of "Thu, 18 Jun 2026 13:07:22 +0530")

On 18/06/2026 at 13:07:22 +0530, Santhosh Kumar K <s-k6@ti.com> wrote:

> spinand_select_op_variant() open-codes a loop that finds the fastest
> eligible op variant by transfer duration. Extract this into a shared
> helper spinand_op_find_best() that accepts a skip_mask bitmask of
> already-tried variant indices, enabling callers to iterate variants in
> ranked order while skipping previously attempted ones.
>
> spinand_select_op_variant() becomes a one-liner. No functional change.
>
> Signed-off-by: Santhosh Kumar K <s-k6@ti.com>
> ---
>  drivers/mtd/nand/spi/core.c | 32 +++++++++++++++++++++++++++-----
>  1 file changed, 27 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
> index f86786344d52..b678d0534297 100644
> --- a/drivers/mtd/nand/spi/core.c
> +++ b/drivers/mtd/nand/spi/core.c
> @@ -1541,9 +1541,22 @@ static int spinand_init_odtr_instruction_set(struct spinand_device *spinand)
>  	return 0;
>  }
>  
> +/*
> + * spinand_op_find_best() - Find the fastest eligible op variant.

Should definitely be named spinand_op_find_best_variant(). find_best is
way too wide.

> + * @spinand:    SPI NAND device
> + * @variants:   full variant list to search
> + * @odtr:       true to consider ODTR ops, false for SSDR ops
> + * @skip_mask:  bitmask of variant indices to skip (already tried)

The flag seems to be quite overlapping with the odtr flag. Shouldn't we
have a single way to filter operations?

Also, why are you switching from an interface enumeration to a boolean?
Some day we might need to support things like 4D-4D-4D. That is where an
enumeration is better than a boolean. If you want to drop the
enumeration, it shall be done everywhere else, but I don't think it is a
good approach.

> + * Iterates @variants, evaluates transfer duration for each eligible op, and
> + * returns a pointer to the fastest one not in @skip_mask.  Returns NULL when
> + * no eligible variant remains.  Used by both variant selection at init time
> + * (skip_mask == 0) and ranked PHY tuning iteration.
> + */

I haven't seen what is the use case of this mask yet, let's see in the
following patches.

Thanks,
Miquèl

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