Re: [PATCH] Add support for Macronix NAND randomizer

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hi Mason,

masonccyang@mxic.com.tw wrote on Mon, 26 Aug 2019 10:52:31 +0800:

> Hi Miquel,
> > 
> > Mason Yang <masonccyang@mxic.com.tw> wrote on Tue, 20 Aug 2019 13:53:48
> > +0800:
> >   
> > > Macronix NANDs support randomizer operation for user data scrambled,
> > > which can be enabled with a SET_FEATURE.
> > > 
> > > User data written to the NAND device without randomizer is still   
> readable
> > > after randomizer function enabled.
> > > The penalty of randomizer are NOP = 1 instead of NOP = 4 and more time   
> period
> > 
> > please don't use 'NOP' here, use 'subpage accesses' instead, otherwise
> > people might not understand what it means while it has a real impact.
> >   
> 
> okay, understood. 
> will fix it by next submitting.
> 
> > > is needed in program operation and entering deep power-down mode.
> > > i.e., tPROG 300us to 340us(randomizer enabled)
> > > 
> > > If subpage write not available with hardware ECC, for example,
> > > NAND chip options NAND_NO_SUBPAGE_WRITE be set in driver and
> > > randomizer function is recommended for high-reliability.
> > > Driver checks byte 167 of Vendor Blocks in ONFI parameter page table
> > > to see if this high-reliability function is supported.
> > >   
> > 
> > You did not flagged this patch as a v2 and forgot about the changelog.  
> 
> will fix, thank you.
> 
> > You did not listen to our comments in the last version neither. I was
> > open to a solution with a specific DT property for warned users but I
> > don't see it coming.  
> 
> Sorry I missed the previous version of "read-retry and randomizer support" 
> patch. 
> Specific DT property is a good method to control it.
> 
> For more high-reliability concern, randomizer is recommended to enable by 
> default,
> but sub-page write is not allowed when randomizer is enabled.
> 
> Since most of HW ECC did not support sub-page write and we think driver to 
> check
> chip options flags is another simple and good way to enable randomizer.

Sorry but this is wrong. Several controllers and NAND chips support
subpages. And changing now the behavior with such chips would entirely
break the concerned setups (see Boris answer about UBI complaining if
the subpage size changes).

Thanks,
Miquèl