Re: [PATCH 2/3] mtd: rawnand: arasan: Ensure program page operations are successful

[Michal Simek <michal.simek@amd.com>]

On 9/22/23 11:14, Miquel Raynal wrote:
> Hi Michal,
> 
> michal.simek@amd.com wrote on Thu, 21 Sep 2023 12:25:10 +0200:
> 
>> On 9/12/23 16:17, Miquel Raynal wrote:
>>> Hi Michal,
>>>
>>> michal.simek@amd.com wrote on Tue, 12 Sep 2023 15:55:23 +0200:
>>>    
>>>> Hi Miquel,
>>>>
>>>> On 9/11/23 17:52, Miquel Raynal wrote:
>>>>> Hi Michal,
>>>>>
>>>>> miquel.raynal@bootlin.com wrote on Mon, 17 Jul 2023 21:42:20 +0200:
>>>>>     >>>> The NAND core complies with the ONFI specification, which itself
>>>>>> mentions that after any program or erase operation, a status check
>>>>>> should be performed to see whether the operation was finished *and*
>>>>>> successful.
>>>>>>
>>>>>> The NAND core offers helpers to finish a page write (sending the
>>>>>> "PAGE PROG" command, waiting for the NAND chip to be ready again, and
>>>>>> checking the operation status). But in some cases, advanced controller
>>>>>> drivers might want to optimize this and craft their own page write
>>>>>> helper to leverage additional hardware capabilities, thus not always
>>>>>> using the core facilities.
>>>>>>
>>>>>> Some drivers, like this one, do not use the core helper to finish a page
>>>>>> write because the final cycles are automatically managed by the
>>>>>> hardware. In this case, the additional care must be taken to manually
>>>>>> perform the final status check.
>>>>>>
>>>>>> Let's read the NAND chip status at the end of the page write helper and
>>>>>> return -EIO upon error.
>>>>>>
>>>>>> Cc: Michal Simek <michal.simek@amd.com>
>>>>>> Cc: stable@vger.kernel.org
>>>>>> Fixes: 88ffef1b65cf ("mtd: rawnand: arasan: Support the hardware BCH ECC engine")
>>>>>> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
>>>>>>
>>>>>> ---
>>>>>>
>>>>>> Hello Michal,
>>>>>>
>>>>>> I have not tested this, but based on a report on another driver, I
>>>>>> believe the status check is also missing here and could sometimes
>>>>>> lead to unnoticed partial writes.
>>>>>>
>>>>>> Please test on your side that everything still works and let me
>>>>>> know how it goes.
>>>>>
>>>>> Any news from the testing team about patches 2/3 and 3/3?
>>>>
>>>> I asked Amit to test and he didn't get back to me even I asked for it couple of times.
>>>
>>> Ok.
>>>    
>>>> Can you please tell me how to test it? I will setup HW myself and test it and get back to you.
>>>
>>> I believe setting up the board to use the hardware BCH engine and
>>> performing basic erase/write/read testing with a known file and check
>>> it still behaves correctly would work. You can also run
>>>
>>> 	nandbiterrs -i /dev/mtdx
>>>
>>> as a second step and verify there is no difference with and without the
>>> patch and finally check the impact:
>>>
>>> 	flash_speed -d -c 10 /dev/mtdx
>>> 	(be careful: this is a destructive operation)
>>
>> Testing team won't see any issue that's why feel free to add my
>> Acked-by: Michal Smek <michal.simek@amd.com>
> 
> I think you told me in the last e-mail you tested the pl353 patch, not
> the one for the Arasan controller. Shall I add your Acked-by here and
> your Tested-by in the other?

Yes exactly.
I tested pl353 myself. If that log looks good feel free to add my Tested-by tag.
And I got information from testing team that they tested Arasan one hence only 
Ack one.

Thanks,
Michal


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