On 21/07/05 12:46PM, Sean Nyekjaer wrote: > 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. I will send patches for both kernel and u-boot to use legacy bch scheme by default, and add some code to treat few MLC nand chips as corner cases. > > /Sean