| Message ID | 20181011093516.12835-1-miquel.raynal@bootlin.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Jagannadha Sutradharudu Teki |
| Headers | show |
| Series | [U-Boot] mtd: nand: let the raw NAND devices be compiled upon selection | expand |
On Thu, Oct 11, 2018 at 3:05 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > Today way is to rely on CMD_NAND to be selected and from the root > Makefile compile what is in drivers/mtd/nand/raw. > > While this will work most of the time with decent configurations, it > is better to also compile this subsystem upon simple request in the > configuration. Otherwise, a user not selecting CMD_NAND but selecting > NAND and any of the controller drivers will not see their build. Fix > this weird situation by adding a single line in the nand/ directory > Kconfig file. > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > --- > drivers/mtd/nand/Makefile | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile > index a358bc680e..2c33447995 100644 > --- a/drivers/mtd/nand/Makefile > +++ b/drivers/mtd/nand/Makefile > @@ -2,4 +2,5 @@ > > nandcore-objs := core.o bbt.o > obj-$(CONFIG_MTD_NAND_CORE) += nandcore.o > +obj-$(CONFIG_NAND) += raw/ But we even source raw Kconfig path, isn't it? and also how about remove this from root Makefile?
Hi Jagan, Jagan Teki <jagan@amarulasolutions.com> wrote on Tue, 23 Oct 2018 16:40:05 +0530: > On Thu, Oct 11, 2018 at 3:05 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > > Today way is to rely on CMD_NAND to be selected and from the root > > Makefile compile what is in drivers/mtd/nand/raw. > > > > While this will work most of the time with decent configurations, it > > is better to also compile this subsystem upon simple request in the > > configuration. Otherwise, a user not selecting CMD_NAND but selecting > > NAND and any of the controller drivers will not see their build. Fix > > this weird situation by adding a single line in the nand/ directory > > Kconfig file. > > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > > --- > > drivers/mtd/nand/Makefile | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile > > index a358bc680e..2c33447995 100644 > > --- a/drivers/mtd/nand/Makefile > > +++ b/drivers/mtd/nand/Makefile > > @@ -2,4 +2,5 @@ > > > > nandcore-objs := core.o bbt.o > > obj-$(CONFIG_MTD_NAND_CORE) += nandcore.o > > +obj-$(CONFIG_NAND) += raw/ > > But we even source raw Kconfig path, isn't it? and also how about > remove this from root Makefile? What do you mean by "we even source raw Kconfig path"? It was not sourced before this modification unless CMD_NAND was selected. I agree we should also remove this link between a path being compiled and a CMD_* symbol in the root Makefile. It will probably produce a lot of errors though. I'll try when I'll have some time. Thanks, Miquèl
On Mon, Oct 29, 2018 at 2:37 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > Hi Jagan, > > Jagan Teki <jagan@amarulasolutions.com> wrote on Tue, 23 Oct 2018 > 16:40:05 +0530: > > > On Thu, Oct 11, 2018 at 3:05 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > > > > Today way is to rely on CMD_NAND to be selected and from the root > > > Makefile compile what is in drivers/mtd/nand/raw. > > > > > > While this will work most of the time with decent configurations, it > > > is better to also compile this subsystem upon simple request in the > > > configuration. Otherwise, a user not selecting CMD_NAND but selecting > > > NAND and any of the controller drivers will not see their build. Fix > > > this weird situation by adding a single line in the nand/ directory > > > Kconfig file. > > > > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > > > --- > > > drivers/mtd/nand/Makefile | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile > > > index a358bc680e..2c33447995 100644 > > > --- a/drivers/mtd/nand/Makefile > > > +++ b/drivers/mtd/nand/Makefile > > > @@ -2,4 +2,5 @@ > > > > > > nandcore-objs := core.o bbt.o > > > obj-$(CONFIG_MTD_NAND_CORE) += nandcore.o > > > +obj-$(CONFIG_NAND) += raw/ > > > > But we even source raw Kconfig path, isn't it? and also how about > > remove this from root Makefile? > > What do you mean by "we even source raw Kconfig path"? It was not > sourced before this modification unless CMD_NAND was selected. > > I agree we should also remove this link between a path being > compiled and a CMD_* symbol in the root Makefile. It will probably > produce a lot of errors though. I'll try when I'll have some time. OK, but If it can't fire any issue as of now, may be we do full cleanup later.
Hi Jagan, Miquel Raynal <miquel.raynal@bootlin.com> wrote on Mon, 29 Oct 2018 10:07:28 +0100: > Hi Jagan, > > Jagan Teki <jagan@amarulasolutions.com> wrote on Tue, 23 Oct 2018 > 16:40:05 +0530: > > > On Thu, Oct 11, 2018 at 3:05 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > > > > Today way is to rely on CMD_NAND to be selected and from the root > > > Makefile compile what is in drivers/mtd/nand/raw. > > > > > > While this will work most of the time with decent configurations, it > > > is better to also compile this subsystem upon simple request in the > > > configuration. Otherwise, a user not selecting CMD_NAND but selecting > > > NAND and any of the controller drivers will not see their build. Fix > > > this weird situation by adding a single line in the nand/ directory > > > Kconfig file. > > > > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > > > --- > > > drivers/mtd/nand/Makefile | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile > > > index a358bc680e..2c33447995 100644 > > > --- a/drivers/mtd/nand/Makefile > > > +++ b/drivers/mtd/nand/Makefile > > > @@ -2,4 +2,5 @@ > > > > > > nandcore-objs := core.o bbt.o > > > obj-$(CONFIG_MTD_NAND_CORE) += nandcore.o > > > +obj-$(CONFIG_NAND) += raw/ > > > > But we even source raw Kconfig path, isn't it? and also how about > > remove this from root Makefile? > > What do you mean by "we even source raw Kconfig path"? It was not > sourced before this modification unless CMD_NAND was selected. > > I agree we should also remove this link between a path being > compiled and a CMD_* symbol in the root Makefile. It will probably > produce a lot of errors though. I'll try when I'll have some time. Yet another patch you ignored. All these are real fixes, not just some kind of sandbox leisure, please consider this change. Thanks, Miquèl
On Mon, Apr 08, 2019 at 11:52:31AM +0200, Miquel Raynal wrote: > Hi Jagan, > > Miquel Raynal <miquel.raynal@bootlin.com> wrote on Mon, 29 Oct 2018 > 10:07:28 +0100: > > > Hi Jagan, > > > > Jagan Teki <jagan@amarulasolutions.com> wrote on Tue, 23 Oct 2018 > > 16:40:05 +0530: > > > > > On Thu, Oct 11, 2018 at 3:05 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > > > > > > Today way is to rely on CMD_NAND to be selected and from the root > > > > Makefile compile what is in drivers/mtd/nand/raw. > > > > > > > > While this will work most of the time with decent configurations, it > > > > is better to also compile this subsystem upon simple request in the > > > > configuration. Otherwise, a user not selecting CMD_NAND but selecting > > > > NAND and any of the controller drivers will not see their build. Fix > > > > this weird situation by adding a single line in the nand/ directory > > > > Kconfig file. > > > > > > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > > > > --- > > > > drivers/mtd/nand/Makefile | 1 + > > > > 1 file changed, 1 insertion(+) > > > > > > > > diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile > > > > index a358bc680e..2c33447995 100644 > > > > --- a/drivers/mtd/nand/Makefile > > > > +++ b/drivers/mtd/nand/Makefile > > > > @@ -2,4 +2,5 @@ > > > > > > > > nandcore-objs := core.o bbt.o > > > > obj-$(CONFIG_MTD_NAND_CORE) += nandcore.o > > > > +obj-$(CONFIG_NAND) += raw/ > > > > > > But we even source raw Kconfig path, isn't it? and also how about > > > remove this from root Makefile? > > > > What do you mean by "we even source raw Kconfig path"? It was not > > sourced before this modification unless CMD_NAND was selected. > > > > I agree we should also remove this link between a path being > > compiled and a CMD_* symbol in the root Makefile. It will probably > > produce a lot of errors though. I'll try when I'll have some time. > > Yet another patch you ignored. All these are real fixes, not just some > kind of sandbox leisure, please consider this change. Yes, what's going on here? Thanks!
Hi Miquel, On Mon, Apr 8, 2019 at 3:22 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > Hi Jagan, > > Miquel Raynal <miquel.raynal@bootlin.com> wrote on Mon, 29 Oct 2018 > 10:07:28 +0100: > > > Hi Jagan, > > > > Jagan Teki <jagan@amarulasolutions.com> wrote on Tue, 23 Oct 2018 > > 16:40:05 +0530: > > > > > On Thu, Oct 11, 2018 at 3:05 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > > > > > > Today way is to rely on CMD_NAND to be selected and from the root > > > > Makefile compile what is in drivers/mtd/nand/raw. > > > > > > > > While this will work most of the time with decent configurations, it > > > > is better to also compile this subsystem upon simple request in the > > > > configuration. Otherwise, a user not selecting CMD_NAND but selecting > > > > NAND and any of the controller drivers will not see their build. Fix > > > > this weird situation by adding a single line in the nand/ directory > > > > Kconfig file. > > > > > > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > > > > --- > > > > drivers/mtd/nand/Makefile | 1 + > > > > 1 file changed, 1 insertion(+) > > > > > > > > diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile > > > > index a358bc680e..2c33447995 100644 > > > > --- a/drivers/mtd/nand/Makefile > > > > +++ b/drivers/mtd/nand/Makefile > > > > @@ -2,4 +2,5 @@ > > > > > > > > nandcore-objs := core.o bbt.o > > > > obj-$(CONFIG_MTD_NAND_CORE) += nandcore.o > > > > +obj-$(CONFIG_NAND) += raw/ > > > > > > But we even source raw Kconfig path, isn't it? and also how about > > > remove this from root Makefile? > > > > What do you mean by "we even source raw Kconfig path"? It was not > > sourced before this modification unless CMD_NAND was selected. > > > > I agree we should also remove this link between a path being > > compiled and a CMD_* symbol in the root Makefile. It will probably > > produce a lot of errors though. I'll try when I'll have some time. > > Yet another patch you ignored. All these are real fixes, not just some > kind of sandbox leisure, please consider this change. 1) Did I ignore any patches from you? 2) This change I just commented before and I think the same is handling on your cleanup series, If I'm not wrong? Jagan.
Jagan, Jagan Teki <jagan@amarulasolutions.com> wrote on Mon, 8 Apr 2019 17:28:18 +0530: > Hi Miquel, > > On Mon, Apr 8, 2019 at 3:22 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > > Hi Jagan, > > > > Miquel Raynal <miquel.raynal@bootlin.com> wrote on Mon, 29 Oct 2018 > > 10:07:28 +0100: > > > > > Hi Jagan, > > > > > > Jagan Teki <jagan@amarulasolutions.com> wrote on Tue, 23 Oct 2018 > > > 16:40:05 +0530: > > > > > > > On Thu, Oct 11, 2018 at 3:05 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > > > > > > > > Today way is to rely on CMD_NAND to be selected and from the root > > > > > Makefile compile what is in drivers/mtd/nand/raw. > > > > > > > > > > While this will work most of the time with decent configurations, it > > > > > is better to also compile this subsystem upon simple request in the > > > > > configuration. Otherwise, a user not selecting CMD_NAND but selecting > > > > > NAND and any of the controller drivers will not see their build. Fix > > > > > this weird situation by adding a single line in the nand/ directory > > > > > Kconfig file. > > > > > > > > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > > > > > --- > > > > > drivers/mtd/nand/Makefile | 1 + > > > > > 1 file changed, 1 insertion(+) > > > > > > > > > > diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile > > > > > index a358bc680e..2c33447995 100644 > > > > > --- a/drivers/mtd/nand/Makefile > > > > > +++ b/drivers/mtd/nand/Makefile > > > > > @@ -2,4 +2,5 @@ > > > > > > > > > > nandcore-objs := core.o bbt.o > > > > > obj-$(CONFIG_MTD_NAND_CORE) += nandcore.o > > > > > +obj-$(CONFIG_NAND) += raw/ > > > > > > > > But we even source raw Kconfig path, isn't it? and also how about > > > > remove this from root Makefile? > > > > > > What do you mean by "we even source raw Kconfig path"? It was not > > > sourced before this modification unless CMD_NAND was selected. > > > > > > I agree we should also remove this link between a path being > > > compiled and a CMD_* symbol in the root Makefile. It will probably > > > produce a lot of errors though. I'll try when I'll have some time. > > > > Yet another patch you ignored. All these are real fixes, not just some > > kind of sandbox leisure, please consider this change. > > 1) Did I ignore any patches from you? * [PATCH] mtd: nand: let the raw NAND devices be compiled upon selection -> 11/10/2018: Sent on the ML -> 23/10/2018-05/11/2018: you want something cleaner, I tell you I will do it soon but for now the issue should be fixed. -> 08/04/2019: Ping from me because the other series is still not applied, the quick fix neither, the bug is still there. * [PATCH v4 00/25] MTD defconfigs/Kconfigs/Makefiles heavy cleanup -> 09/12/2018: v4 sent on the ML with all comments addressed -> 20/02/2019: Ping from Vignesh -> 20/02/2019: Ping from me -> 06/03/2019: Ping from me -> 17/03/2019: Ping from me -> 17/03/2019: Ping from Tom -> 19/03/2019: You answer: "Yes, it's late from my end, will have a look at and comment asap." -> 08/04/2019: Ping from me * [PATCH 1/2] mtd: fix mtd_oobavail() incoherent returned value * [PATCH 2/2] mtd: fix Coverity integer handling issue -> 16/10/2018: Coverity issues reported by Tom privately -> 18/11/2018: Fixes sent on the ML (both for Linux and U-Boot). For your defense, you were not in Cc of the patches but they are marked 'New' in Patchwork and assigned to you. -> 03/12/2018: Fixes applied in Linux -> 08/04/2018: Ping from me > 2) This change I just commented before and I think the same is > handling on your cleanup series, If I'm not wrong? When there is a bug, the usual handling procedure is: 1/ fix the bug so that users are not bothered, 2/ if the fix is not elegant enough then a rework of the faulty section is engaged. History: We discovered after applying my "SPI-NAND support (third batch)" 13th version from the 01/10/2018 that under certain circumstances the raw NAND core could be missing because of the game of the select/depends on properties. It was a compile-time issue only. Because I want the selection to be easier for the user, I fixed it with the current patch. This is the quick fix sent the 11/10/2018 addressing situation 1/. We agreed that it could be useful to rework the entire Makefile/Kconfig soup in the MTD layer. So this is why I wrote on my spare time and send the full cleanup series. This is the deepest rework sent the 29/10/2018 (v2), the 05/12/2018 (v3) and the 09/12/2018 (v4) addressing situation 2/. Regards, Miquèl
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile index a358bc680e..2c33447995 100644 --- a/drivers/mtd/nand/Makefile +++ b/drivers/mtd/nand/Makefile @@ -2,4 +2,5 @@ nandcore-objs := core.o bbt.o obj-$(CONFIG_MTD_NAND_CORE) += nandcore.o +obj-$(CONFIG_NAND) += raw/ obj-$(CONFIG_MTD_SPI_NAND) += spi/
Today way is to rely on CMD_NAND to be selected and from the root Makefile compile what is in drivers/mtd/nand/raw. While this will work most of the time with decent configurations, it is better to also compile this subsystem upon simple request in the configuration. Otherwise, a user not selecting CMD_NAND but selecting NAND and any of the controller drivers will not see their build. Fix this weird situation by adding a single line in the nand/ directory Kconfig file. Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> --- drivers/mtd/nand/Makefile | 1 + 1 file changed, 1 insertion(+)