Re: [RFC PATCH 17/27] mtd: nand: ecc: Let the software Hamming ECC engine be unselected

From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Boris Brezillon <bbrezillon@kernel.org>
Cc: Mason Yang <masonccyang@mxic.com.tw>, Vignesh R <vigneshr@ti.com>,
	Tudor Ambarus <Tudor.Ambarus@microchip.com>,
	Julien Su <juliensu@mxic.com.tw>,
	Richard Weinberger <richard@nod.at>,
	Schrempf Frieder <frieder.schrempf@kontron.de>,
	Marek Vasut <marek.vasut@gmail.com>,
	linux-mtd@lists.infradead.org,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	Brian Norris <computersforpeace@gmail.com>,
	David Woodhouse <dwmw2@infradead.org>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [RFC PATCH 17/27] mtd: nand: ecc: Let the software Hamming ECC engine be unselected
Date: Thu, 21 Feb 2019 14:46:38 +0100	[thread overview]
Message-ID: <20190221144638.5f3ee481@xps13> (raw)
In-Reply-To: <20190221144148.6313d2ba@kernel.org>

Hi Boris,

Boris Brezillon <bbrezillon@kernel.org> wrote on Thu, 21 Feb 2019
14:41:48 +0100:

> On Thu, 21 Feb 2019 14:35:39 +0100
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> > Hi Boris,
> > 
> > Boris Brezillon <bbrezillon@kernel.org> wrote on Thu, 21 Feb 2019
> > 14:20:02 +0100:
> >   
> > > On Thu, 21 Feb 2019 13:57:56 +0100
> > > Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > >     
> > > > There is no reason to always embed the software Hamming ECC engine
> > > > implementation. By default it is, but we can let the user decide.
> > > > 
> > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > > ---
> > > >  drivers/mtd/nand/ecc/Kconfig               | 10 +++++-
> > > >  drivers/mtd/nand/raw/Kconfig               |  2 +-
> > > >  include/linux/mtd/nand-sw-hamming-engine.h | 38 ++++++++++++++++++++++
> > > >  3 files changed, 48 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/mtd/nand/ecc/Kconfig b/drivers/mtd/nand/ecc/Kconfig
> > > > index e0106b3a7ec1..ff20e621ffef 100644
> > > > --- a/drivers/mtd/nand/ecc/Kconfig
> > > > +++ b/drivers/mtd/nand/ecc/Kconfig
> > > > @@ -1,7 +1,15 @@
> > > >  menu "ECC engine support"
> > > >  
> > > >  config MTD_NAND_ECC_SW_HAMMING
> > > > -	tristate
> > > > +	tristate "Software Hamming ECC engine"
> > > > +	default y      
> > > 
> > > Same as for the NAND_CORE stuff, let users this option when they need
> > > it instead of having a "default y". Haven't made my mind yet on whether
> > > this option should be visible to users or not. I guess it could be with
> > > the new infrastructure, but it's probably too early in the patch series
> > > to change that.    
> > 
> > This one is different.
> > 
> > Before the series: the software Hamming ECC algorithm is part of the
> > 'NAND package'. There is no way to ignore it, it *will* be part of your
> > binary (or module).
> > 
> > After the series: I just give the user the possibility to deselect
> > this option. But having 'default y' is mandatory here to avoid breaking
> > current defconfigs.  
> 
> Okay, then maybe
> 
> 	default y if MTD_RAW_NAND

Right, this is more accurate.

> 
> And I think you should select the option in MTD_NAND_NDFC instead of
> adding a depends on. 

Ack.

> 
> >   
> > >     
> > > > +	help
> > > > +	  This enables support for software Hamming error
> > > > +	  correction. This correction can correct up to 1 bit error
> > > > +	  per chunk and detect up to 2 bit errors. While it used to be
> > > > +	  widely used with old parts, newer NAND chips usually require
> > > > +	  more strength correction and in this case BCH or RS will be
> > > > +	  preferred.
> > > >        
> > 
> > 
> > Thanks,
> > Miquèl  
> 

Thanks,
Miquèl

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