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

From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Tudor Ambarus <tudor.ambarus@linaro.org>
Cc: Richard Weinberger <richard@nod.at>,
	 Vignesh Raghavendra <vigneshr@ti.com>,
	 Pratyush Yadav <pratyush@kernel.org>,
	 Michael Walle <michael@walle.cc>,
	 linux-mtd@lists.infradead.org,  Mark Brown <broonie@kernel.org>,
	 linux-spi@vger.kernel.org,  Steam Lin <stlin2@winbond.com>,
	 Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	Sanjay R Mehta <sanju.mehta@amd.com>,  Han Xu <han.xu@nxp.com>,
	 Conor Dooley <conor.dooley@microchip.com>,
	 Daire McNamara <daire.mcnamara@microchip.com>,
	 Matthias Brugger <matthias.bgg@gmail.com>,
	 AngeloGioacchino Del Regno
	<angelogioacchino.delregno@collabora.com>,
	 Haibo Chen <haibo.chen@nxp.com>,
	 Yogesh Gaur <yogeshgaur.83@gmail.com>,
	 Heiko Stuebner <heiko@sntech.de>,
	 Michal Simek <michal.simek@amd.com>
Subject: Re: [PATCH 23/24] mtd: spinand: winbond: Add comment about naming
Date: Fri, 13 Dec 2024 13:25:05 +0100	[thread overview]
Message-ID: <87wmg3kbxa.fsf@bootlin.com> (raw)
In-Reply-To: <71d0cf83-6866-46be-b76f-291a8a6a1346@linaro.org> (Tudor Ambarus's message of "Mon, 11 Nov 2024 14:38:53 +0000")

On 11/11/2024 at 14:38:53 GMT, Tudor Ambarus <tudor.ambarus@linaro.org> 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@bootlin.com>
>> ---
>>  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

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