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

[Santhosh Kumar K]

Hello Miquel,

Apologies for the delayed response.

On 02/07/26 19:11, Miquel Raynal wrote:
> On 18/06/2026 at 13:07:22 +0530, Santhosh Kumar K <s-k6@xxxxxx> 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@xxxxxx>
> > ---
> >   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.

Agreed!

> > + * @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.

Yeah, thanks for pointing out. I'll use 'enum' in v5.

Thanks,
Santhosh.

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