Re: [PATCH v4 1/5] mtd: rawnand: meson: fix command sequence for read/write

[Arseniy Krasnov]

On 26.05.2023 20:22, Miquel Raynal wrote:
> Hi Arseniy,
> 
> avkrasnov@xxxxxxxxxxxxxx wrote on Wed, 24 May 2023 12:05:47 +0300:
> 
>> On 23.05.2023 12:12, Arseniy Krasnov wrote:
>>> Hello Miquel, Liang
>>>
>>> On 22.05.2023 18:05, Miquel Raynal wrote:  
>>>> Hi Arseniy,
>>>>
>>>> AVKrasnov@xxxxxxxxxxxxxx wrote on Mon, 15 May 2023 12:44:35 +0300:
>>>>  
>>>>> This fixes read/write functionality by:
>>>>> 1) Changing NFC_CMD_RB_INT bit value.  
>>>>
>>>> I guess this is a separate fix
>>>>  
>>>
>>> Ok, I'll move it to separate patch
>>>   
>>>>> 2) Adding extra NAND_CMD_STATUS command on each r/w request.  
>>>>
>>>> Is this really needed? Looks like you're delaying the next op only. Is
>>>> using a delay enough? If yes, then it's probably the wrong approach.  
>>
>> Hi Miquel, small update, I found some details from @Liang's message in v1 talks from the last month:
>>
>> *
>> After sending NAND_CMD_READ0, address, NAND_CMD_READSTART and read status(NAND_CMD_STATUS = 0x70) commands, it should send
>> NAND_CMD_READ0 command for exiting the read status mode from the datasheet from NAND device.
> 
> That is true.
> 
>> but previous meson_nfc_queue_rb()
>> only checks the Ready/Busy pin and it doesn't send read status(NAND_CMD_STATUS = 0x70) command.
>> i think there is something wrong with the Ready/Busy pin(please check the hardware whether this
>> Ready/Busy pin is connected with SOC) or the source code. i have the board without Ready/Busy pin and prefer to use the
>> nfc command called RB_IO6. it sends NAND_CMD_STATUS command and checks bit6 of the status register of NAND device from the
>> data bus and generate IRQ if ready.
>> *
>>
>> I guess, that sequence of commands from this patch is described in datasheet (unfortunately I don't have it and relied on the old driver).
>> Yesterday I tried to remove sending of NAND_CMD_STATUS from this patch, but it broke current driver - i had ECC errors, so it looks like
>> "shot in the dark" situation, to understand this logic.
> 
> When an operation on the NAND array happens (eg. read, prog, erase),
> you need to wait "some time" before accessing the internal sram or even
> the chip which is "busy" until it gets "ready" again. You can probe the
> ready/busy pin (that's the hardware way, fast and reliable) or you can
> poll a status with NAND_CMD_STATUS. The chips are designed so they can
> actually process that command while they are doing time consuming tasks
> to update the host. But IIRC every byte read will return the status
> until you send READ0 again, which means "I'm done with the status
> read" somehow.
> 
> Please see nand_soft_waitrdy() in order to understand how this is
> supposed to work. You can even use that helper (which is exported)
> instead of open-coding it in your driver. See atmel or sunxi
> implementations for instance.
> 
> As using the native RB pin is better, you would need to identify
> whether you have one or not at probe time and then either poll the
> relevant bit of your controller if there is one, or fallback to the
> soft read (which should fallback on exec_op in the end).

Thanks for this information! I'll use 'nand_soft_waitrdy()' at least, because i guess that
there is no RB pin on my device.

Thanks, Arseniy

> 
> Thanks,
> Miquèl