Re: [RFC PATCH 02/27] mtd: nand: Compile in the NAND core by default

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

Hi Boris,

Boris Brezillon <bbrezillon@kernel.org> wrote on Thu, 21 Feb 2019
13:08:15 +0100:

> On Thu, 21 Feb 2019 12:46:28 +0100
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> > Hi Boris,
> > 
> > Boris Brezillon <bbrezillon@kernel.org> wrote on Thu, 21 Feb 2019
> > 12:14:37 +0100:
> >   
> > > On Thu, 21 Feb 2019 12:06:10 +0100
> > > Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > >     
> > > > Hi Boris,
> > > > 
> > > > Boris Brezillon <bbrezillon@kernel.org> wrote on Thu, 21 Feb 2019
> > > > 11:55:54 +0100:
> > > >       
> > > > > On Thu, 21 Feb 2019 11:01:51 +0100
> > > > > Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > > > >         
> > > > > > Force the NAND core be compiled-in when using any kind of NAND.          
> > > > > 
> > > > > Why?        
> > > > 
> > > > Because all the logic that comes later in the series relies on this
> > > > change. I need the NAND core to be compiled-in,      
> > > 
> > > Hm, not exactly compiled-in as it can still be selected as a module.    
> > 
> > Yes, do you think it is a problem?  
> 
> Yes, it is, at least for the SPI NAND case (and soon the RAW NAND case
> too) since you remove the select but don't add a depends on. If you
> select NAND_CORE as a module and SPI_NAND compiled-in => BOOM!

I am not entirely sure but I think SPI NAND cannot be compiled-in if it
is "under" a menu that has been selected as a module. But anyway, it's
just a matter of seeing things, and the onenand argument is valid to
me. I will change it to the below proposal you accepted.

> 
> >   
> > >     
> > > > as well the ECC engine
> > > > core, as well as everything that is shared between raw NAND and
> > > > SPI-NAND which I move in the NAND core.      
> > > 
> > > Yes, the core is required for SPI NAND and soon will be for RAW NAND,
> > > but I still don't see the problem with the "select NAND_CORE" approach,
> > > sorry.
> > >     
> > > >       
> > > > >         
> > > > > > 
> > > > > > Also remove the redundant dependencies on MTD which is enforced by the
> > > > > > game of the if/endif blocs.
> > > > > > 
> > > > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > > > > ---
> > > > > >  drivers/mtd/nand/Kconfig         | 12 ++++++++++--
> > > > > >  drivers/mtd/nand/onenand/Kconfig |  1 -
> > > > > >  drivers/mtd/nand/raw/Kconfig     |  1 -
> > > > > >  drivers/mtd/nand/spi/Kconfig     |  1 -
> > > > > >  4 files changed, 10 insertions(+), 5 deletions(-)
> > > > > > 
> > > > > > diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
> > > > > > index 495751ed3fd7..e8d26a715922 100644
> > > > > > --- a/drivers/mtd/nand/Kconfig
> > > > > > +++ b/drivers/mtd/nand/Kconfig
> > > > > > @@ -1,6 +1,14 @@
> > > > > > -config MTD_NAND_CORE
> > > > > > -	tristate
> > > > > > +menuconfig MTD_NAND_CORE
> > > > > > +	tristate "NAND"          
> > > > > 
> > > > > Definitely not something we want to expose in menuconfig.        
> > > > 
> > > > I considered the NAND core as essential when using raw NAND and
> > > > SPI-NAND.      
> > > 
> > > It is needed for SPI NAND, and will be for raw NAND for sure, but I
> > > still see no reasons for turning it into a user-visible option.
> > >     
> > > > Hence, turning it into a menuconfig option make the whole
> > > > NAND subsystem available (or not) and will appear in a different
> > > > "menuconfig page", which really enhances the readability.      
> > > 
> > > That's a different thing, and I have no problem with adding an extra
> > > level in the Kconfig hierarchy.    
> > 
> > Then why not using this symbol? I don't get why you are opposed.  
> 
> Because, not only you're forcing onenand users to pull the nand core
> code in, which is not needed until the conversion to the generic NAND
> layer is done, but you're actually forcing that for all existing
> defconfigs because of your "default y" approach. Also, I don't see
> what's the benefit of letting users know about this intermediate
> framework when all they care about is supporting a specific class of
> devices.
> 
> > 
> > So you would like a config NAND_CORE (invisible) and a visible
> > menuconfig NAND? And all entries in the NAND menu selecting NAND_CORE?  
> 
> Not all entries, just SPI NAND, and soon, raw NAND. And yes, I'd prefer
> that.


Thanks,
Miquèl

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel