Re: [PATCH 23/24] mtd: spinand: winbond: Add comment about naming

[Miquel Raynal <miquel.raynal@xxxxxxxxxxx>]

On 11/11/2024 at 14:38:53 GMT, Tudor Ambarus <tudor.ambarus@xxxxxxxxxx> wrote:

> On 10/25/24 5:15 PM, Miquel Raynal wrote:
>> Make the link between the core macros and the datasheet.
>> 
>> Signed-off-by: Miquel Raynal <miquel.raynal@xxxxxxxxxxx>
>> ---
>>  drivers/mtd/nand/spi/winbond.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>> 
>> diff --git a/drivers/mtd/nand/spi/winbond.c b/drivers/mtd/nand/spi/winbond.c
>> index 686e872fe0ff..9e2562805d23 100644
>> --- a/drivers/mtd/nand/spi/winbond.c
>> +++ b/drivers/mtd/nand/spi/winbond.c
>> @@ -18,6 +18,11 @@
>>  
>>  #define W25N04KV_STATUS_ECC_5_8_BITFLIPS	(3 << 4)
>>  
>> +/*
>> + * "X2" in the core is equivalent to "dual output" in the datasheets,
>> + * "X4" in the core is equivalent to "quad output" in the datasheets.
>> + */
>
> doesn't help great for an outsider like me. Is quad referring to cmd,
> addr or data? Or maybe of all? I need to read the code anyway.

I also don't like these terms. IIRC "output" is referring to the data cycles,
otherwise it means address (dummy) and data cycles.

In single, dual or quad mode the naming is unclear but "okay". But octal
DDR modes can require the opcode to be sent in octal mode as well, which
is new. If we support that, I'll take care of using a more
understandable naming for all macros like Xy-Xy-Xy, X being the
buswidth, y being S (sdr) or D (ddr) and the three members being
Command-Address-Data. I might even be tempted to include dummy cycles as
well, because it is important to be clear if eg. in octal mode "1" means
"1 cycle" or "8 cycles".

>> +
>>  static SPINAND_OP_VARIANTS(read_cache_dtr_variants,
>>  		SPINAND_PAGE_READ_FROM_CACHE_QUADIO_OP(0, 2, NULL, 0),
>>  		SPINAND_PAGE_READ_FROM_CACHE_X4_OP(0, 1, NULL, 0),

Thanks,
Miquèl