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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6077c959-f566-d399-d2be-8460eb063415@sberdevices.ru>
Date:   Tue, 30 May 2023 14:19:08 +0300
From:   Arseniy Krasnov <avkrasnov@...rdevices.ru>
To:     Miquel Raynal <miquel.raynal@...tlin.com>
CC:     Liang Yang <liang.yang@...ogic.com>,
        Richard Weinberger <richard@....at>,
        Vignesh Raghavendra <vigneshr@...com>,
        Neil Armstrong <neil.armstrong@...aro.org>,
        Kevin Hilman <khilman@...libre.com>,
        Jerome Brunet <jbrunet@...libre.com>,
        Martin Blumenstingl <martin.blumenstingl@...glemail.com>,
        Yixun Lan <yixun.lan@...ogic.com>,
        Jianxin Pan <jianxin.pan@...ogic.com>, <oxffffaa@...il.com>,
        <kernel@...rdevices.ru>, <linux-mtd@...ts.infradead.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <linux-amlogic@...ts.infradead.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4 1/5] mtd: rawnand: meson: fix command sequence for
 read/write



On 26.05.2023 20:22, Miquel Raynal wrote:
> Hi Arseniy,
> 
> avkrasnov@...rdevices.ru 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@...rdevices.ru 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