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

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 <[email protected]> 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 <[email protected]>
>> ---
>>   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