Re: [PATCH v1] mtd: rawnand: meson: fix ready/busy command

From: Arseniy Krasnov <avkrasnov@sberdevices.ru>
To: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: <oxffffaa@gmail.com>, <kernel@sberdevices.ru>,
	<stable@vger.kernel.org>, <linux-mtd@lists.infradead.org>,
	<linux-arm-kernel@lists.infradead.org>,
	<linux-amlogic@lists.infradead.org>,
	<linux-kernel@vger.kernel.org>,
	Liang Yang <liang.yang@amlogic.com>,
	Richard Weinberger <richard@nod.at>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Neil Armstrong <neil.armstrong@linaro.org>,
	Jerome Brunet <jbrunet@baylibre.com>,
	Martin Blumenstingl <martin.blumenstingl@googlemail.com>,
	Kevin Hilman <khilman@baylibre.com>,
	Jianxin Pan <jianxin.pan@amlogic.com>
Subject: Re: [PATCH v1] mtd: rawnand: meson: fix ready/busy command
Date: Mon, 5 Jun 2023 11:27:34 +0300	[thread overview]
Message-ID: <9bea333a-e5fc-af89-29a0-fa01236488a3@sberdevices.ru> (raw)
In-Reply-To: <20230605101739.069b98af@xps-13>

On 05.06.2023 11:17, Miquel Raynal wrote:
> Hi Arseniy,
> 
> avkrasnov@sberdevices.ru wrote on Mon, 5 Jun 2023 09:39:40 +0300:
> 
>> Hello Miquel!
>>
>> I exclude this patch from the recent Meson patchset, as it is fix and not related to another patches.
>> Also I think that I can split Meson patchset (from links below) in the following way:
>> 1) Patch/patchset for OOB layout
>> 2) Patchset for "nand-rb" logic (meson_nand.c + DT bindings)
>> These two can also go independently:
>> 3) https://lore.kernel.org/linux-mtd/20230601061850.3907800-6-AVKrasnov@sberdevices.ru/
>> 4) https://lore.kernel.org/linux-mtd/20230601061850.3907800-7-AVKrasnov@sberdevices.ru/
> 
> LGTM.

I think we don't need this patch with new RB_INT value. I dive into conversations with Liang Yang - author of this driver. He said,
that this define is "special" polling mode. Now I'm a little bit more experienced in NAND area, so IIUC we have two basic waiting modes:
1) Software, by sending status command, then polling reply from controller, and then finally send READ0 to exit status reading mode (nand_soft_waitrdy()).
2) By RB pin and interrupt - in this mode we send vendor specific command to the controller and it triggers interrupt on RB pin update.
   In this case RB pin must be connected to the controller. This mode is already implemented by Liang Yang.

Now, command with this new define works in the following way: we send status command, by instead of reading reply from it by software,
controller itself checks RB pin and status ready bit in IO bus. Driver just waits on completion. After waiting we need to send
READ0 command again to make controller leave status reading mode. I think this is like "intermediate" mode between software polling
and hardware interrupt - there is no need to spin in loop, waiting for status ready bit, but still need to leave status mode by sending
READ0 command. I'm not sure that we need this as third mode of waiting for command is done. May be at least not in this work on Meson driver,
so I guess to drop this patch at  this moment and add 'nand_soft_waitrdy()' support.

Thanks, Arseniy

> 
>>
>> What do You think?
>>
>> Thanks, Arseniy
>>
>> On 05.06.2023 09:10, Arseniy Krasnov wrote:
>>> Fix the ready/busy command value.
>>>
>>> Fixes: 8fae856c5350 ("mtd: rawnand: meson: add support for Amlogic NAND flash controller")
>>> Cc: stable@vger.kernel.org
>>> Suggested-by: Liang Yang <liang.yang@amlogic.com>
>>> Signed-off-by: Arseniy Krasnov <AVKrasnov@sberdevices.ru>
>>> ---
>>>  drivers/mtd/nand/raw/meson_nand.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/mtd/nand/raw/meson_nand.c b/drivers/mtd/nand/raw/meson_nand.c
>>> index 074e14225c06..9dd4a676497b 100644
>>> --- a/drivers/mtd/nand/raw/meson_nand.c
>>> +++ b/drivers/mtd/nand/raw/meson_nand.c
>>> @@ -37,7 +37,7 @@
>>>  #define NFC_CMD_SCRAMBLER_ENABLE	BIT(19)
>>>  #define NFC_CMD_SCRAMBLER_DISABLE	0
>>>  #define NFC_CMD_SHORTMODE_DISABLE	0
>>> -#define NFC_CMD_RB_INT		BIT(14)
>>> +#define NFC_CMD_RB_INT		((0xb << 10) | BIT(18) | BIT(16))
>>>  
>>>  #define NFC_CMD_GET_SIZE(x)	(((x) >> 22) & GENMASK(4, 0))
>>>    
> 
> 
> Thanks,
> Miquèl