| Message ID | 20190221100216.25255-3-miquel.raynal@bootlin.com (mailing list archive) |
|---|---|
| State | RFC |
| Headers | show |
| Series | Introduce the generic ECC engine abstraction | expand |
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? > > 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. > + default y I don't see what's the problem with letting SPINAND, RAWNAND and ONENAND select this options if they actually need it. > + help > + Embed the NAND core. This is the generic part between raw > + NAND, SPI NAND and Onenand. > + > +if MTD_NAND_CORE > > source "drivers/mtd/nand/onenand/Kconfig" > source "drivers/mtd/nand/raw/Kconfig" > source "drivers/mtd/nand/spi/Kconfig" > + > +endif > diff --git a/drivers/mtd/nand/onenand/Kconfig b/drivers/mtd/nand/onenand/Kconfig > index 9dc15748947b..c168f3b4b296 100644 > --- a/drivers/mtd/nand/onenand/Kconfig > +++ b/drivers/mtd/nand/onenand/Kconfig > @@ -1,6 +1,5 @@ > menuconfig MTD_ONENAND > tristate "OneNAND Device Support" > - depends on MTD > depends on HAS_IOMEM > help > This enables support for accessing all type of OneNAND flash > diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig > index 510a6b32820d..ebb8a3da9fa5 100644 > --- a/drivers/mtd/nand/raw/Kconfig > +++ b/drivers/mtd/nand/raw/Kconfig > @@ -11,7 +11,6 @@ config MTD_NAND_ECC_SW_HAMMING_SMC > > menuconfig MTD_RAW_NAND > tristate "Raw/Parallel NAND Device Support" > - depends on MTD > select MTD_NAND_ECC_SW_HAMMING > help > This enables support for accessing all type of raw/parallel > diff --git a/drivers/mtd/nand/spi/Kconfig b/drivers/mtd/nand/spi/Kconfig > index 7c37d2929b68..7631050610ab 100644 > --- a/drivers/mtd/nand/spi/Kconfig > +++ b/drivers/mtd/nand/spi/Kconfig > @@ -1,6 +1,5 @@ > menuconfig MTD_SPI_NAND > tristate "SPI NAND device Support" > - select MTD_NAND_CORE > depends on SPI_MASTER > select SPI_MEM > help
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, 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. > > > > > 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. 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. > > > + default y > > I don't see what's the problem with letting SPINAND, RAWNAND and > ONENAND select this options if they actually need it. See the reason above, hence having it "default y" is needed to do not break existing defconfigs. Thanks, Miquèl
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. > 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. > > > > > > + default y > > > > I don't see what's the problem with letting SPINAND, RAWNAND and > > ONENAND select this options if they actually need it. > > See the reason above, hence having it "default y" is needed to do not > break existing defconfigs. Not if you make RAW_NAND select NAND_CORE.
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? > > > 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. So you would like a config NAND_CORE (invisible) and a visible menuconfig NAND? And all entries in the NAND menu selecting NAND_CORE? > > > > > > > > > > + default y > > > > > > I don't see what's the problem with letting SPINAND, RAWNAND and > > > ONENAND select this options if they actually need it. > > > > See the reason above, hence having it "default y" is needed to do not > > break existing defconfigs. > > Not if you make RAW_NAND select NAND_CORE. Thanks, Miquèl
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! > > > > > > 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.
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
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" + default y + help + Embed the NAND core. This is the generic part between raw + NAND, SPI NAND and Onenand. + +if MTD_NAND_CORE source "drivers/mtd/nand/onenand/Kconfig" source "drivers/mtd/nand/raw/Kconfig" source "drivers/mtd/nand/spi/Kconfig" + +endif diff --git a/drivers/mtd/nand/onenand/Kconfig b/drivers/mtd/nand/onenand/Kconfig index 9dc15748947b..c168f3b4b296 100644 --- a/drivers/mtd/nand/onenand/Kconfig +++ b/drivers/mtd/nand/onenand/Kconfig @@ -1,6 +1,5 @@ menuconfig MTD_ONENAND tristate "OneNAND Device Support" - depends on MTD depends on HAS_IOMEM help This enables support for accessing all type of OneNAND flash diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig index 510a6b32820d..ebb8a3da9fa5 100644 --- a/drivers/mtd/nand/raw/Kconfig +++ b/drivers/mtd/nand/raw/Kconfig @@ -11,7 +11,6 @@ config MTD_NAND_ECC_SW_HAMMING_SMC menuconfig MTD_RAW_NAND tristate "Raw/Parallel NAND Device Support" - depends on MTD select MTD_NAND_ECC_SW_HAMMING help This enables support for accessing all type of raw/parallel diff --git a/drivers/mtd/nand/spi/Kconfig b/drivers/mtd/nand/spi/Kconfig index 7c37d2929b68..7631050610ab 100644 --- a/drivers/mtd/nand/spi/Kconfig +++ b/drivers/mtd/nand/spi/Kconfig @@ -1,6 +1,5 @@ menuconfig MTD_SPI_NAND tristate "SPI NAND device Support" - select MTD_NAND_CORE depends on SPI_MASTER select SPI_MEM help
Force the NAND core be compiled-in when using any kind of NAND. 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(-)