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

[Miquel Raynal]

Hi Michal,

michal.simek at amd.com wrote on Fri, 22 Sep 2023 11:16:20 +0200:

> On 9/22/23 11:14, Miquel Raynal wrote:
> > Hi Michal,
> > 
> > michal.simek at 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 at 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 at 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 at amd.com>
> >>>>>> Cc: stable at vger.kernel.org
> >>>>>> Fixes: 88ffef1b65cf ("mtd: rawnand: arasan: Support the hardware BCH ECC engine")
> >>>>>> Signed-off-by: Miquel Raynal <miquel.raynal at 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 at 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.

Perfect. Thanks a lot!

Miquèl