Re: [PATCH 1/2] mtd: nand: raw: gpmi: new bch geometry settings

[Sean Nyekjaer <sean@geanix.com>]

On Wed, May 26, 2021 at 05:31:23PM +0200, Miquel Raynal wrote:
> Hi Han,
> 

[ ... ]

> 
> I understand that (2) might be ideal to meet but is breaking all the
> boards that use this driver really worth the trouble?
> 
> Short answer: no. So we need to adapt the calculation for new
> boards/new flash chips/certain geometries at most.
> 
> > > > The new implementation might get weak ecc than legacy way in some cases but it
> > > > is safety guaranteed.  
> > > 
> > > What does "safety guaranteed" means?  
> > 
> > set minimum ecc required by nand chip at least meet all requirements
> > 
> > >   
> > > > This reminds me the gpmi raw access mode changes in kernel 3.19, it also changes
> > > > the driver behaviors and makes totally different output compared with older
> > > > versions. I know changes bring mess but we have to accept it at some point
> > > > rather than keep compromising to the wrong way.  
> > > 
> > > How is this an argument? I am usually in favor of moving forward when
> > > there is a real justification, but this does not seem the case, unless
> > > I am understanding it all the wrong way.
> > >   
> > > > The change has been in NXP kernel fork for a while, so quite a few customers are
> > > > using this bch geometry settings. I hope it can be upstreamed, any other things
> > > > I can do may mitigate the imapact?  
> > > 
> > > You are well aware of the upstreaming process, trying to merge
> > > something locally, making it used and then complaining because not
> > > upstreaming it would break your customers really is your own
> > > responsibility.  
> > 
> > Sorry I understand I should try upstreaming it early, so I am still looking for
> > a chance to avoid further divergence.
> > 
> > > 
> > > IMHO the solutions are:
> > > - the current (mainline) default will remain the standard for
> > >   geometries which are already widely supported
> > > - if there are new geometries that must be supported and do not fit
> > >   because of the "legacy" logic, then you may detect that and try
> > >   to fallback to the "modern" way of calculating the ECC
> > >   parameters (or even jump directly to the modern way if the geometry
> > >   really is not currently supported officially)
> > > - if your customers want a specific chunk size/strength when
> > >   rebasing on top of a mainline kernel there are DT properties which do
> > >   that anyway
> > > - follow Sean advice: introduce a property requesting to use the
> > >   'modern' or 'legacy' logic (with a better name than modern) but first
> > >   check with Rob that this if valid.
> 
> Another hint: please check the core helpers and use them instead of
> trying to re-invent the wheel: normally just describing the engine
> capabilities and calling a single helper should do the trick. But this
> 'new' calculation should only apply to eg. MLC devices or devices with
> specific geometries, not to all devices.
> 
> Thanks,
> Miquèl

Hi Han,

Is this something you are working on?
If not I really think we need to revert the changes to u-boot, to allign
vanilla u-boot and kernel.

/Sean