MTD defconfigs/Kconfigs/Makefiles heavy cleanup

MTD must be enabled when there is a SPI NOR flash using the SPI_FLASH_MTD config entry.

Signed-off-by Miquel Raynal <[email protected]>

UBI must be enabled when using fastmap, reflect this is defconfigs.

Signed-off-by Miquel Raynal <[email protected]>

MTD must be enabled when UBI is used, this is mandatory and will later be reflected thanks to a "depends on" in Kconfig. But first, defconfigs needs to be fixed.

Signed-off-by Miquel Raynal <[email protected]>

UBI must be enabled when CMD_UBI is used, this is mandatory and will later be reflected thanks to a "depends on" in Kconfig. But first, defconfigs needs to be fixed.

Signed-off-by Miquel Raynal <[email protected]>

UBI must be enabled when the environment is in UBI.

Signed-off-by Miquel Raynal <[email protected]>

Raw NAND support must be enabled when the environment is in NAND.

Signed-off-by Miquel Raynal <[email protected]>

MTD support must be enabled when the environment is in NOR.

Signed-off-by Miquel Raynal <[email protected]>

---
 configs/armadillo-800eva_defconfig | 1 +
 1 file changed, 1 insertion(+)

configs/armadillo-800eva_defconfig +0 -0

index b1d923c069..45736a9b01 100644

configs/armadillo-800eva_defconfig +0 -0

 CONFIG_SH_ETHER=y
 CONFIG_SCIF_CONSOLE=y
 CONFIG_OF_LIBFDT=y

MTD support must be enabled when the environment is in NOR.

Dear Miquel,

Quoted 1 line · In message <[email protected]> you wrote:

MTD support must be enabled when the environment is in NOR.

Naked-by Wolfgang Denk <[email protected]>

Environment in NOR must not, I repeat: must not ever depend on MTD!

For more than 19 years we have been using environment in NOR flash without the need for MTD support, which makes a lot of sense especially on smaller systems where resources are low and MTD is an expensive, but not really needed feature.

It is not acceptable to create a dependency that costs so much.

Hi Wolfgang,

Wolfgang Denk <[email protected]> wrote on Wed, 05 Dec 2018 13:06:10 +0100:

Quoted 7 lines

Dear Miquel,

Quoted 1 line · In message <[email protected]> you wrote:

MTD support must be enabled when the environment is in NOR.

Naked-by Wolfgang Denk <[email protected]>

Quoted 8 lines

Environment in NOR must not, I repeat: must not ever depend on MTD!

For more than 19 years we have been using environment in NOR flash without the need for MTD support, which makes a lot of sense especially on smaller systems where resources are low and MTD is an expensive, but not really needed feature.

It is not acceptable to create a dependency that costs so much.

I took a rather small configuration: stm32f429-discovery_defconfig which uses CONFIG_MTD_NOR_FLASH. Without CONFIG_MTD, u-boot.bin is 209416 bytes. With CONFIG_MTD, u-boot.bin is 214540 bytes. This is an additional 5124 bytes which represent about 2% of the entire size.

I am talking about U-Boot only, there is a CONFIG_SPL_MTD_SUPPORT option that must be enabled to compile MTD in the SPL so the change I propose do not enlarge the SPL.

Today, there are multiple boards having more than one type of MTD device (eg. raw NAND and SPI-NOR). In this case you need to compile two commands which interface only with the subsystem they have been written for. We propose to kill this approach and instead use an 'mtd' command which shares the same code for all MTD devices: less duplication, more users, and in the end, a reduced size. And I am not event talking about all the code that has been added over the years to "avoid using MTD" which enlarges the binary as well.

The current situation is unmaintainable. Any change in U-Boot under the drivers/mtd/ directory results in hours of debugging to fix broken dependencies and crappy configurations. It took me one week to port the SPI-mem/SPI-NAND layers (which rely on MTD) and to have a working 'mtd' command. It took me and Boris almost 4 weeks to fix the fallouts. Now, either we keep U-Boot MTD subsystem maintainable and in sync as much as possible with Linux to continue to benefit from evolutions/drivers/fixes at the cost of a little overhead, or we continue in the current path, with the results we know.

Moving towards a unified MTD layer to replace all the historic solutions is definitely the right thing to do. Miquèl already mentioned the reasons in detail...

I took a rather small configuration: stm32f429-discovery_defconfig which uses CONFIG_MTD_NOR_FLASH. Without CONFIG_MTD, u-boot.bin is 209416 bytes. With CONFIG_MTD, u-boot.bin is 214540 bytes. This is an additional 5124 bytes which represent about 2% of the entire size.

I am talking about U-Boot only, there is a CONFIG_SPL_MTD_SUPPORT option that must be enabled to compile MTD in the SPL so the change I propose do not enlarge the SPL.

Today, there are multiple boards having more than one type of MTD device (eg. raw NAND and SPI-NOR). In this case you need to compile two commands which interface only with the subsystem they have been written for. We propose to kill this approach and instead use an 'mtd' command which shares the same code for all MTD devices: less duplication, more users, and in the end, a reduced size. And I am not event talking about all the code that has been added over the years to "avoid using MTD" which enlarges the binary as well.

Dear Miquel,

Quoted 5 lines · In message <20181205153218.36f6ed4e@xps13> you wrote:

I took a rather small configuration: stm32f429-discovery_defconfig which uses CONFIG_MTD_NOR_FLASH. Without CONFIG_MTD, u-boot.bin is 209416 bytes. With CONFIG_MTD, u-boot.bin is 214540 bytes. This is an additional 5124 bytes which represent about 2% of the entire size.

So it's a 5 k code increase for zero benefit.

Quoted 4 lines

I am talking about U-Boot only, there is a CONFIG_SPL_MTD_SUPPORT option that must be enabled to compile MTD in the SPL so the change I propose do not enlarge the SPL.

This is even worse - the SPL is frequently at the size limits.

Please understand that there is a fundamental difference between parallel NOR flash and things like NAND, SPI NOR etc. - the latter are storage devices, which are usually handled as block device or similar, i. e. you always need a driver to read data. Parallel NOR is not storage, it is _memory_, which is directly mapped int o memory. You can execute code from it.

Reading data from parallel NOR comes at zero overhead (except maybe for setting up the memory controller to map the NOR into the address space).

Adding a dependency on the MTD layer is broken.

Quoted 8 lines

Today, there are multiple boards having more than one type of MTD device (eg. raw NAND and SPI-NOR). In this case you need to compile two commands which interface only with the subsystem they have been written for. We propose to kill this approach and instead use an 'mtd' command which shares the same code for all MTD devices: less duplication, more users, and in the end, a reduced size. And I am not event talking about all the code that has been added over the years to "avoid using MTD" which enlarges the binary as well.

This is perfectly fine. But you must not break fundamental features that have been there ever since the beginning of the project.

Simple things - like reading the environment from parallel NOR (which boils down to calling ofa memcpy()) must not be bloated with unneeded layers like MTD - even if these are needed and useful elsewhere.

Please do not understand me wrong: I fully agree with the MTD rework in general, and I'm happy to see it happen. But please do it in a way not to break existing basic use cases.

Quoted 7 lines

Quoted 5 lines

I took a rather small configuration: stm32f429-discovery_defconfig which uses CONFIG_MTD_NOR_FLASH. Without CONFIG_MTD, u-boot.bin is 209416 bytes. With CONFIG_MTD, u-boot.bin is 214540 bytes. This is an additional 5124 bytes which represent about 2% of the entire size.

So it's a 5 k code increase for zero benefit.

There's no short term benefit, but the long term benefit is ease better maintainability.

I think you missed the *no*. There's no overhead at all for the SPL. Either the platform was already enabling CONFIG_SPL_MTD_SUPPORT and the MTD code was already compiled in before Miquel's patch, or this option is disabled, and the SPL still does *not* embed the MTD layer.

Quoted 6 lines

Please understand that there is a fundamental difference between parallel NOR flash and things like NAND, SPI NOR etc. - the latter are storage devices, which are usually handled as block device or similar, i. e. you always need a driver to read data. Parallel NOR is not storage, it is _memory_, which is directly mapped int o memory. You can execute code from it.

That's only partially true. Yes, you can read from a parallel NORs like if it was memory because memory controllers embedded in SoCs provide your a direct mmio mapping. That's not true for write accesses,

as NORs need to be erased before you can write on it. The MTD layer is here to abstract that, such that flash users only have to know how to manipulate an MTD device instead of having one backend per flash-type (actually it's even one per-flash-type+mem-bus).

The overhead can be minimal if we want (we can have a tiny MTD layer)

current:
flash_read()

proposed:

	mtd_read()
		mtd_to_flash_read_wrapper()
			flash_read()

The benefit, well, a single entry point for all kind of mtd accesses.

That means we can have an MTD backend for env retrieval instead of one per flash type. Same goes for DFU backends, and maybe even for SPL boots.
To sum-up: less code to maintain. And I wouldn't be surprized if we were actually reducing the image footprint in some cases (when support for several flash types is enabled).

Quoted 3 lines

Please do not understand me wrong: I fully agree with the MTD rework in general, and I'm happy to see it happen. But please do it in a way not to break existing basic use cases.

Okay, maybe we can put aside the parallel NOR case for now, but I do think even parallel NOR accesses would benefit from the MTD layer.

Dear Boris,

Quoted 10 lines · In message <20181206100016.706330ba@bbrezillon> you wrote:

Quoted 7 lines

Quoted 5 lines

I took a rather small configuration: stm32f429-discovery_defconfig which uses CONFIG_MTD_NOR_FLASH. Without CONFIG_MTD, u-boot.bin is 209416 bytes. With CONFIG_MTD, u-boot.bin is 214540 bytes. This is an additional 5124 bytes which represent about 2% of the entire size.

So it's a 5 k code increase for zero benefit.

There's no short term benefit, but the long term benefit is ease better maintainability.

For MTD, yes. But a user of environment in NOR flash does not necessarily want MTD,

Parallel NOR flash is a traditional memory device - you find it frequently on older, resource-restricted systems. It is not so polular any more today - SPI NOR is much cheaper, comea with a smaller footprint of thr PCB and is much easier to route.

And on these older systems a 5 k code increase is often critical.

Especially if this should also affect the SPL.

Quoted 5 lines

I think you missed the *no*. There's no overhead at all for the SPL. Either the platform was already enabling CONFIG_SPL_MTD_SUPPORT and the MTD code was already compiled in before Miquel's patch, or this option is disabled, and the SPL still does *not* embed the MTD layer.

So parallel NOR is still supported in the SPL _without_ MTD? Then why cannot we have the same support in U-Boot, too?

Quoted 8 lines

Quoted 7 lines

Please understand that there is a fundamental difference between parallel NOR flash and things like NAND, SPI NOR etc. - the latter are storage devices, which are usually handled as block device or similar, i. e. you always need a driver to read data. Parallel NOR is not storage, it is _memory_, which is directly mapped int o memory. You can execute code from it.

Quoted 3 lines

That's only partially true. Yes, you can read from a parallel NORs like if it was memory because memory controllers embedded in SoCs provide your a direct mmio mapping. That's not true for write accesses,

Right, but especially in the SPL read access is sufficient.

Quoted 6 lines

Quoted 3 lines

Please do not understand me wrong: I fully agree with the MTD rework in general, and I'm happy to see it happen. But please do it in a way not to break existing basic use cases.

Okay, maybe we can put aside the parallel NOR case for now, but I do think even parallel NOR accesses would benefit from the MTD layer.

I agree that it benefits form an architectural and maintenance point of view. But it adds a new dependency with more than insignificant impact to systems where this has not been before, and where resources are tight.

Hi Wolfgang,

Wolfgang Denk <[email protected]> wrote on Thu, 06 Dec 2018 10:32:14 +0100:

Quoted 22 lines

Dear Boris,

Quoted 10 lines · In message <20181206100016.706330ba@bbrezillon> you wrote:

Quoted 7 lines

Quoted 5 lines

I took a rather small configuration: stm32f429-discovery_defconfig which uses CONFIG_MTD_NOR_FLASH. Without CONFIG_MTD, u-boot.bin is 209416 bytes. With CONFIG_MTD, u-boot.bin is 214540 bytes. This is an additional 5124 bytes which represent about 2% of the entire size.

So it's a 5 k code increase for zero benefit.

There's no short term benefit, but the long term benefit is ease better maintainability.

For MTD, yes. But a user of environment in NOR flash does not necessarily want MTD,

Parallel NOR flash is a traditional memory device - you find it frequently on older, resource-restricted systems. It is not so polular any more today - SPI NOR is much cheaper, comea with a smaller footprint of thr PCB and is much easier to route.

Quoted 3 lines

And on these older systems a 5 k code increase is often critical.

Especially if this should also affect the SPL.

Please, stop repeating the SPL is a problem. I am not forcing MTD to be compiled in the SPL. You overlooked that point in our last three e-mails: MTD is *not* built in the SPL unless specifically requested. CONFIG_MTD has absolutely no impact on the SPL.

Now, let's talk about U-Boot size. I went through U-Boot MTD implementation to see where this 5k overhead was coming from. The explanation is short: nothing. My observation was wrong because of commands being automatically compiled ("default y"). By the way this is the kind of approach I want to get rid of because it is fundamentally wrong.

Here is a brief summary, still with stm32f429-discovery_defconfig:
* before my series (MTD_NOR_FLASH + CMD_FLASH)
* after my series without MTD (CONFIG_MTD_NOR_FLASH + CMD_FLASH)
* after my series with MTD (MTD_NOR_FLASH + CMD_FLASH + MTD)

In all cases, u-boot.bin is 214540 bytes. I suppose if you don't use anything from MTD, there is no additional symbol, the binary will not be enlarged. And when you use it, it is hard to tell how much bytes were added by MTD and how much bytes have been added by the feature using MTD.

Quoted 7 lines

Quoted 4 lines

I think you missed the *no*. There's no overhead at all for the SPL. Either the platform was already enabling CONFIG_SPL_MTD_SUPPORT and the MTD code was already compiled in before Miquel's patch, or this option is disabled, and the SPL still does *not* embed the MTD layer.

So parallel NOR is still supported in the SPL _without_ MTD? Then why cannot we have the same support in U-Boot, too?

Code duplication, maintainability, single interface, (hopefully) single command, etc

Quoted 7 lines

Quoted 7 lines

Quoted 6 lines

Please understand that there is a fundamental difference between parallel NOR flash and things like NAND, SPI NOR etc. - the latter are storage devices, which are usually handled as block device or similar, i. e. you always need a driver to read data. Parallel NOR is not storage, it is _memory_, which is directly mapped int o memory. You can execute code from it.

Quoted 5 lines

Quoted 3 lines

That's only partially true. Yes, you can read from a parallel NORs like if it was memory because memory controllers embedded in SoCs provide your a direct mmio mapping. That's not true for write accesses,

Right, but especially in the SPL read access is sufficient.

100% agreed, the SPL just needs read access. But we are talking about U-Boot itself here.

[...]

Quoted 4 lines

Quoted 4 lines

Quoted 3 lines

Please do not understand me wrong: I fully agree with the MTD rework in general, and I'm happy to see it happen. But please do it in a way not to break existing basic use cases.

Quoted 7 lines

Quoted 2 lines

Okay, maybe we can put aside the parallel NOR case for now, but I do think even parallel NOR accesses would benefit from the MTD layer.

I agree that it benefits form an architectural and maintenance point of view. But it adds a new dependency with more than insignificant impact to systems where this has not been before, and where resources are tight.

Quoted 26 lines

Hi Wolfgang,

Wolfgang Denk <[email protected]> wrote on Thu, 06 Dec 2018 10:32:14 +0100:

Quoted 22 lines

Dear Boris,

Quoted 10 lines · In message <20181206100016.706330ba@bbrezillon> you wrote:

Quoted 7 lines

Quoted 5 lines

I took a rather small configuration: stm32f429-discovery_defconfig which uses CONFIG_MTD_NOR_FLASH. Without CONFIG_MTD, u-boot.bin is 209416 bytes. With CONFIG_MTD, u-boot.bin is 214540 bytes. This is an additional 5124 bytes which represent about 2% of the entire size.

So it's a 5 k code increase for zero benefit.

There's no short term benefit, but the long term benefit is ease better maintainability.

For MTD, yes. But a user of environment in NOR flash does not necessarily want MTD,

Parallel NOR flash is a traditional memory device - you find it frequently on older, resource-restricted systems. It is not so polular any more today - SPI NOR is much cheaper, comea with a smaller footprint of thr PCB and is much easier to route.

Quoted 8 lines

Quoted 3 lines

And on these older systems a 5 k code increase is often critical.

Especially if this should also affect the SPL.

Please, stop repeating the SPL is a problem. I am not forcing MTD to be compiled in the SPL. You overlooked that point in our last three e-mails: MTD is *not* built in the SPL unless specifically requested. CONFIG_MTD has absolutely no impact on the SPL.

Right, and this is the important part that you are indeed getting right. We must be careful about the SPL case. You are, so good, lets move on.

Quoted 12 lines

Now, let's talk about U-Boot size. I went through U-Boot MTD implementation to see where this 5k overhead was coming from. The explanation is short: nothing. My observation was wrong because of commands being automatically compiled ("default y"). By the way this is the kind of approach I want to get rid of because it is fundamentally wrong.

Here is a brief summary, still with stm32f429-discovery_defconfig:
* before my series (MTD_NOR_FLASH + CMD_FLASH)
* after my series without MTD (CONFIG_MTD_NOR_FLASH + CMD_FLASH)
* after my series with MTD (MTD_NOR_FLASH + CMD_FLASH + MTD)

Quoted 5 lines

In all cases, u-boot.bin is 214540 bytes. I suppose if you don't use anything from MTD, there is no additional symbol, the binary will not be enlarged. And when you use it, it is hard to tell how much bytes were added by MTD and how much bytes have been added by the feature using MTD.

So when you aren't calling into the new code it gets discarded as expected, good. Thanks for digging.

Quoted 10 lines

Quoted 7 lines

Quoted 4 lines

I think you missed the *no*. There's no overhead at all for the SPL. Either the platform was already enabling CONFIG_SPL_MTD_SUPPORT and the MTD code was already compiled in before Miquel's patch, or this option is disabled, and the SPL still does *not* embed the MTD layer.

So parallel NOR is still supported in the SPL _without_ MTD? Then why cannot we have the same support in U-Boot, too?

Code duplication, maintainability, single interface, (hopefully) single command, etc

Right. And to be clear, in the main U-Boot case while we don't grow the binary for fun, we do grow the overall binary (for some board or another) with nearly every push because we're quite often doing "lets fix $X" or "let's add $Y" and rarely "Drop $X as it's linked but unused". Just like DM is resulting in an overall larger binary that's a more consistent and maintainable codebase, this too will result in some things growing in binary size. And we should review the framework with an eye towards "can we do this in a more simple or size-courteous way?" size growth in and of itself isn't a fatal problem.

Quoted 13 lines

Quoted 12 lines

Quoted 10 lines

Quoted 6 lines

Please understand that there is a fundamental difference between parallel NOR flash and things like NAND, SPI NOR etc. - the latter are storage devices, which are usually handled as block device or similar, i. e. you always need a driver to read data. Parallel NOR is not storage, it is _memory_, which is directly mapped int o memory. You can execute code from it.

That's only partially true. Yes, you can read from a parallel NORs like if it was memory because memory controllers embedded in SoCs provide your a direct mmio mapping. That's not true for write accesses,

Right, but especially in the SPL read access is sufficient.

Quoted 2 lines

100% agreed, the SPL just needs read access. But we are talking about U-Boot itself here.

So, new but related question. Are we dropping the write path in the case of SPL? The only case I can think of where SPL might be doing an MTD write would be bootcount stored in env, so we should be able to drop those paths in the normal case.

Quoted 7 lines

[...]

Quoted 4 lines

Quoted 4 lines

Quoted 3 lines

Please do not understand me wrong: I fully agree with the MTD rework in general, and I'm happy to see it happen. But please do it in a way not to break existing basic use cases.

Quoted 7 lines

Quoted 7 lines

Quoted 2 lines

Okay, maybe we can put aside the parallel NOR case for now, but I do think even parallel NOR accesses would benefit from the MTD layer.

I agree that it benefits form an architectural and maintenance point of view. But it adds a new dependency with more than insignificant impact to systems where this has not been before, and where resources are tight.

Frankly I'm concerned that we must have a lot of these boards that aren't actually functional today, or are overwriting something else in memory. Using buildman and the stm32f429-discovery with buildman, I see our growth from v2018.01 to v2018.11 is:
all +48781 bss -628 data +5592 rodata +3079 text +40738

Of course, that's not all inertia growth, there's lots of intentional new board-specific code. So lets try taurus (slightly reformatted output):
all +42136 bss +13344 data +4512 rodata +936 text +23344 spl/u-boot-spl:all +126 spl/u-boot-spl:bss +24 spl/u-boot-spl:rodata +38 spl/u-boot-spl:text +64

Here the board hasn't opted out of the EFI subsystem which has seen a good deal of desired growth (fixes and features).

So, if we're going to start actively worrying about resource constrained boards not fitting within resource _now_ step 1 is to identify them and step 2 is to make sure they're making use of the hooks we have to make exceeding a specific size a link time error. Step 3 will be dealing with however many boards we have that are failing today.

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
https://lists.denx.de/listinfo/u-boot

Am 06.12.2018 um 16:59 schrieb Tom Rini:

Quoted 5 lines · Miquel Raynal

Quoted 4 lines

Hi Wolfgang,

Wolfgang Denk <[email protected]> wrote on Thu, 06 Dec 2018 10:32:14 +0100:

Quoted 14 lines

Quoted 14 lines

Quoted 13 lines

Quoted 11 lines

Quoted 6 lines

Please understand that there is a fundamental difference between parallel NOR flash and things like NAND, SPI NOR etc. - the latter are storage devices, which are usually handled as block device or similar, i. e. you always need a driver to read data.  Parallel NOR is not storage, it is _memory_, which is directly mapped int o memory. You can execute code from it.

That's only partially true. Yes, you can read from a parallel NORs like
if it was memory because memory controllers embedded in SoCs provide your a direct mmio mapping. That's not true for write accesses,

Right, but especially in the SPL read access is sufficient.

Quoted 7 lines

Quoted 2 lines

100% agreed, the SPL just needs read access. But we are talking about U-Boot itself here.

So, new but related question.  Are we dropping the write path in the case of SPL?  The only case I can think of where SPL might be doing an MTD write would be bootcount stored in env, so we should be able to drop those paths in the normal case.

Based on my tests with Vignesh's series to port SF to MTD, I don't think we drop the write part in SPL. I also brought that up in that thread.

However, my (preliminary) test results of his work showed that SPL size is actually reduced when using his new 'spi-nor-tiny' code. And that's without trying to reduce MTD code size.

Quoted 26 lines

Hi Wolfgang,

Wolfgang Denk <[email protected]> wrote on Thu, 06 Dec 2018 10:32:14 +0100:

Quoted 22 lines

Dear Boris,

Quoted 10 lines · In message <20181206100016.706330ba@bbrezillon> you wrote:

Quoted 7 lines

Quoted 5 lines

I took a rather small configuration: stm32f429-discovery_defconfig which uses CONFIG_MTD_NOR_FLASH. Without CONFIG_MTD, u-boot.bin is 209416 bytes. With CONFIG_MTD, u-boot.bin is 214540 bytes. This is an additional 5124 bytes which represent about 2% of the entire size.

So it's a 5 k code increase for zero benefit.

There's no short term benefit, but the long term benefit is ease better maintainability.

For MTD, yes. But a user of environment in NOR flash does not necessarily want MTD,

Parallel NOR flash is a traditional memory device - you find it frequently on older, resource-restricted systems. It is not so polular any more today - SPI NOR is much cheaper, comea with a smaller footprint of thr PCB and is much easier to route.

Quoted 8 lines

Quoted 3 lines

And on these older systems a 5 k code increase is often critical.

Especially if this should also affect the SPL.

Please, stop repeating the SPL is a problem. I am not forcing MTD to be compiled in the SPL. You overlooked that point in our last three e-mails: MTD is *not* built in the SPL unless specifically requested. CONFIG_MTD has absolutely no impact on the SPL.

Right, and this is the important part that you are indeed getting right. We must be careful about the SPL case. You are, so good, lets move on.

Quoted 12 lines

Now, let's talk about U-Boot size. I went through U-Boot MTD implementation to see where this 5k overhead was coming from. The explanation is short: nothing. My observation was wrong because of commands being automatically compiled ("default y"). By the way this is the kind of approach I want to get rid of because it is fundamentally wrong.

Here is a brief summary, still with stm32f429-discovery_defconfig:
* before my series (MTD_NOR_FLASH + CMD_FLASH)
* after my series without MTD (CONFIG_MTD_NOR_FLASH + CMD_FLASH)
* after my series with MTD (MTD_NOR_FLASH + CMD_FLASH + MTD)

Quoted 5 lines

In all cases, u-boot.bin is 214540 bytes. I suppose if you don't use anything from MTD, there is no additional symbol, the binary will not be enlarged. And when you use it, it is hard to tell how much bytes were added by MTD and how much bytes have been added by the feature using MTD.

So when you aren't calling into the new code it gets discarded as expected, good. Thanks for digging.

Quoted 10 lines

Quoted 7 lines

Quoted 4 lines

I think you missed the *no*. There's no overhead at all for the SPL. Either the platform was already enabling CONFIG_SPL_MTD_SUPPORT and the MTD code was already compiled in before Miquel's patch, or this option is disabled, and the SPL still does *not* embed the MTD layer.

So parallel NOR is still supported in the SPL _without_ MTD? Then why cannot we have the same support in U-Boot, too?

Code duplication, maintainability, single interface, (hopefully) single command, etc

Right. And to be clear, in the main U-Boot case while we don't grow the binary for fun, we do grow the overall binary (for some board or another) with nearly every push because we're quite often doing "lets fix $X" or "let's add $Y" and rarely "Drop $X as it's linked but unused". Just like DM is resulting in an overall larger binary that's a more consistent and maintainable codebase, this too will result in some things growing in binary size. And we should review the framework with an eye towards "can we do this in a more simple or size-courteous way?" size growth in and of itself isn't a fatal problem.

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

Quoted 6 lines

Please understand that there is a fundamental difference between parallel NOR flash and things like NAND, SPI NOR etc. - the latter are storage devices, which are usually handled as block device or similar, i. e. you always need a driver to read data. Parallel NOR is not storage, it is _memory_, which is directly mapped int o memory. You can execute code from it.

Quoted 8 lines

Quoted 5 lines

Quoted 3 lines

That's only partially true. Yes, you can read from a parallel NORs like if it was memory because memory controllers embedded in SoCs provide your a direct mmio mapping. That's not true for write accesses,

Right, but especially in the SPL read access is sufficient.

100% agreed, the SPL just needs read access. But we are talking about U-Boot itself here.

So, new but related question. Are we dropping the write path in the case of SPL? The only case I can think of where SPL might be doing an MTD write would be bootcount stored in env, so we should be able to drop those paths in the normal case.

Quoted 6 lines

[...]

Quoted 4 lines

Quoted 4 lines

Quoted 3 lines

Please do not understand me wrong: I fully agree with the MTD rework in general, and I'm happy to see it happen. But please do it in a way not to break existing basic use cases.

Quoted 7 lines

Quoted 7 lines

Quoted 2 lines

Okay, maybe we can put aside the parallel NOR case for now, but I do think even parallel NOR accesses would benefit from the MTD layer.

I agree that it benefits form an architectural and maintenance point of view. But it adds a new dependency with more than insignificant impact to systems where this has not been before, and where resources are tight.

Frankly I'm concerned that we must have a lot of these boards that aren't actually functional today, or are overwriting something else in memory. Using buildman and the stm32f429-discovery with buildman, I see our growth from v2018.01 to v2018.11 is:
all +48781 bss -628 data +5592 rodata +3079 text +40738

Of course, that's not all inertia growth, there's lots of intentional new board-specific code. So lets try taurus (slightly reformatted output):
all +42136 bss +13344 data +4512 rodata +936 text +23344 spl/u-boot-spl:all +126 spl/u-boot-spl:bss +24 spl/u-boot-spl:rodata +38 spl/u-boot-spl:text +64

Here the board hasn't opted out of the EFI subsystem which has seen a good deal of desired growth (fixes and features).

So, if we're going to start actively worrying about resource constrained boards not fitting within resource _now_ step 1 is to identify them and step 2 is to make sure they're making use of the hooks we have to make exceeding a specific size a link time error. Step 3 will be dealing with however many boards we have that are failing today.

Hi Tom, Wolfgang,

So what's our next move? I think Wolfgang had two points:
* NOR (and everything related to NOR) should not depend on MTD.
* Same for NAND.

For NAND this is already too late, NAND code already relies on MTD so I am just cleaning Makefiles/Kconfig/defconfigs to avoid the "xxx:
missing symbol" error. Now, what about NOR? We know that the overhead, today, is null, so do you allow me to make NOR depend on MTD or are you still opposed to that? I think U-Boot maintenance and future changes would be really eased.

Do you see another significant impact that must prevent NOR flashes to depend on MTD?

In some files, options of CMD_NAND are selected but not the command itself. Fix this inconsistency.

Signed-off-by Miquel Raynal <[email protected]>

MTD support must be enabled when using mtdparts. Indeed, functions like get_mtd_info(), get_mtd_device() and put_mtd_device() are in drivers/mtd/mtd_uboot.c and are built only with CONFIG_MTD.

Signed-off-by Miquel Raynal <[email protected]>

Let's be consistent and always declare CONFIG_MTD from the defconfig file when needed.

Signed-off-by Miquel Raynal <[email protected]>

---
 configs/socfpga_stratix10_defconfig       | 1 +
 configs/turris_mox_defconfig              | 1 +
 include/configs/mvebu_armada-37xx.h       | 1 -
 include/configs/socfpga_stratix10_socdk.h | 1 -
 4 files changed, 2 insertions(+), 2 deletions(-)

configs/socfpga_stratix10_defconfig +1 -0

index 5f3d733a8b..292dbd6973 100644
@@ -38,6 +38,7 @@ CONFIG_DM_I2C=y
 CONFIG_SYS_I2C_DW=y
 CONFIG_DM_MMC=y
 CONFIG_MMC_DW=y
+CONFIG_MTD=y
 CONFIG_SPI_FLASH=y
 CONFIG_SPI_FLASH_BAR=y
 CONFIG_SPI_FLASH_SPANSION=y

configs/turris_mox_defconfig +1 -0

index 749ed31acd..13e2af7e1b 100644
@@ -42,6 +42,7 @@ CONFIG_DM_MMC=y
 CONFIG_MMC_SDHCI=y
 CONFIG_MMC_SDHCI_SDMA=y
 CONFIG_MMC_SDHCI_XENON=y
+CONFIG_MTD=y
 CONFIG_SPI_FLASH=y
 CONFIG_SPI_FLASH_MACRONIX=y
 CONFIG_SPI_FLASH_SPANSION=y

include/configs/mvebu_armada-37xx.h +0 -1

index f93ab0f830..640267c9c2 100644
@@ -64,7 +64,6 @@
 #define CONFIG_SF_DEFAULT_SPEED		1000000
 #define CONFIG_SF_DEFAULT_MODE		SPI_MODE_0
 #define CONFIG_ENV_SPI_MODE		CONFIG_SF_DEFAULT_MODE
-#define CONFIG_MTD			/* needed for mtdparts commands */
 #define CONFIG_MTD_PARTITIONS		/* required for UBI partition support */
 
 /* Environment in SPI NOR flash */

include/configs/socfpga_stratix10_socdk.h +0 -0

index 22e1dc84a1..967784e379 100644

include/configs/socfpga_stratix10_socdk.h +0 -0

 #define MTDIDS_DEFAULT			"nor0=ff705000.spi.0"
 #endif /* CONFIG_SPL_BUILD */

Quoted 4 lines · On Wed, 5 Dec 2018 00:57:02 +0100 Miquel Raynal <[email protected]>…

Let's be consistent and always declare CONFIG_MTD from the defconfig file when needed.

Signed-off-by Miquel Raynal <[email protected]>

Reviewed-by Boris Brezillon <[email protected]>

One comment below.

Quoted 50 lines

---
 configs/socfpga_stratix10_defconfig       | 1 +
 configs/turris_mox_defconfig              | 1 +
 include/configs/mvebu_armada-37xx.h       | 1 -
 include/configs/socfpga_stratix10_socdk.h | 1 -
 4 files changed, 2 insertions(+), 2 deletions(-)

configs/socfpga_stratix10_defconfig +1 -0

index 5f3d733a8b..292dbd6973 100644
@@ -38,6 +38,7 @@ CONFIG_DM_I2C=y
 CONFIG_SYS_I2C_DW=y
 CONFIG_DM_MMC=y
 CONFIG_MMC_DW=y
+CONFIG_MTD=y
 CONFIG_SPI_FLASH=y
 CONFIG_SPI_FLASH_BAR=y
 CONFIG_SPI_FLASH_SPANSION=y

configs/turris_mox_defconfig +1 -0

index 749ed31acd..13e2af7e1b 100644
@@ -42,6 +42,7 @@ CONFIG_DM_MMC=y
 CONFIG_MMC_SDHCI=y
 CONFIG_MMC_SDHCI_SDMA=y
 CONFIG_MMC_SDHCI_XENON=y
+CONFIG_MTD=y
 CONFIG_SPI_FLASH=y
 CONFIG_SPI_FLASH_MACRONIX=y
 CONFIG_SPI_FLASH_SPANSION=y

include/configs/mvebu_armada-37xx.h +0 -1

index f93ab0f830..640267c9c2 100644
@@ -64,7 +64,6 @@
 #define CONFIG_SF_DEFAULT_SPEED		1000000
 #define CONFIG_SF_DEFAULT_MODE		SPI_MODE_0
 #define CONFIG_ENV_SPI_MODE		CONFIG_SF_DEFAULT_MODE
-#define CONFIG_MTD			/* needed for mtdparts commands */
 #define CONFIG_MTD_PARTITIONS		/* required for UBI partition support */
 
 /* Environment in SPI NOR flash */

include/configs/socfpga_stratix10_socdk.h +0 -0

index 22e1dc84a1..967784e379 100644
@@ -76,7 +76,6 @@
 #endif /* CONFIG_ENV_IS_IN_SPI_FLASH */
 

include/configs/socfpga_stratix10_socdk.h line 79

Quoted 3 lines

include/configs/socfpga_stratix10_socdk.h +0 -1

 #ifndef CONFIG_SPL_BUILD
-#define CONFIG_MTD
 #define CONFIG_MTD_PARTITIONS  

Do you get rid of CONFIG_MTD_PARTITIONS at some point?

include/configs/socfpga_stratix10_socdk.h line 81

Quoted 3 lines

include/configs/socfpga_stratix10_socdk.h +0 -0

 #define MTDIDS_DEFAULT			"nor0=ff705000.spi.0"
 #endif /* CONFIG_SPL_BUILD */  

Quoted 4 lines · On Wed, 5 Dec 2018 00:57:02 +0100 Miquel Raynal <[email protected]>…

Let's be consistent and always declare CONFIG_MTD from the defconfig file when needed.

Signed-off-by Miquel Raynal <[email protected]>

Reviewed-by Boris Brezillon <[email protected]>

One comment below.

Quoted 50 lines

---
 configs/socfpga_stratix10_defconfig       | 1 +
 configs/turris_mox_defconfig              | 1 +
 include/configs/mvebu_armada-37xx.h       | 1 -
 include/configs/socfpga_stratix10_socdk.h | 1 -
 4 files changed, 2 insertions(+), 2 deletions(-)

configs/socfpga_stratix10_defconfig +1 -0

index 5f3d733a8b..292dbd6973 100644
@@ -38,6 +38,7 @@ CONFIG_DM_I2C=y
 CONFIG_SYS_I2C_DW=y
 CONFIG_DM_MMC=y
 CONFIG_MMC_DW=y
+CONFIG_MTD=y
 CONFIG_SPI_FLASH=y
 CONFIG_SPI_FLASH_BAR=y
 CONFIG_SPI_FLASH_SPANSION=y

configs/turris_mox_defconfig +1 -0

index 749ed31acd..13e2af7e1b 100644
@@ -42,6 +42,7 @@ CONFIG_DM_MMC=y
 CONFIG_MMC_SDHCI=y
 CONFIG_MMC_SDHCI_SDMA=y
 CONFIG_MMC_SDHCI_XENON=y
+CONFIG_MTD=y
 CONFIG_SPI_FLASH=y
 CONFIG_SPI_FLASH_MACRONIX=y
 CONFIG_SPI_FLASH_SPANSION=y

include/configs/mvebu_armada-37xx.h +0 -1

index f93ab0f830..640267c9c2 100644
@@ -64,7 +64,6 @@
 #define CONFIG_SF_DEFAULT_SPEED		1000000
 #define CONFIG_SF_DEFAULT_MODE		SPI_MODE_0
 #define CONFIG_ENV_SPI_MODE		CONFIG_SF_DEFAULT_MODE
-#define CONFIG_MTD			/* needed for mtdparts commands */
 #define CONFIG_MTD_PARTITIONS		/* required for UBI partition support */
 
 /* Environment in SPI NOR flash */

include/configs/socfpga_stratix10_socdk.h +0 -0

index 22e1dc84a1..967784e379 100644
@@ -76,7 +76,6 @@
 #endif /* CONFIG_ENV_IS_IN_SPI_FLASH */
 

include/configs/socfpga_stratix10_socdk.h line 79

Quoted 3 lines

include/configs/socfpga_stratix10_socdk.h +0 -1

 #ifndef CONFIG_SPL_BUILD
-#define CONFIG_MTD
 #define CONFIG_MTD_PARTITIONS  

Do you get rid of CONFIG_MTD_PARTITIONS at some point?

include/configs/socfpga_stratix10_socdk.h line 81

Quoted 3 lines

include/configs/socfpga_stratix10_socdk.h +0 -0

 #define MTDIDS_DEFAULT			"nor0=ff705000.spi.0"
 #endif /* CONFIG_SPL_BUILD */  

Due to previous Makefile organization, the raw NAND subdirectory was not compiled in if CMD_NAND was not enabled. Because the Denali driver does not compile with these boards (undefined environment offset), remove the dependency within the defconfig over the controller driver (was ignored anyway in the past).

Signed-off-by Miquel Raynal <[email protected]>

---
 configs/k2g_evm_defconfig    | 1 -
 configs/k2g_hs_evm_defconfig | 1 -
 2 files changed, 2 deletions(-)

configs/k2g_evm_defconfig +0 -1

index dee00ec8d4..b474781ccb 100644
@@ -41,7 +41,6 @@ CONFIG_MMC_OMAP_HS=y
 CONFIG_MTD=y
 CONFIG_MTD_UBI=y
 CONFIG_MTD_RAW_NAND=y
-CONFIG_NAND_DAVINCI=y
 CONFIG_DM_SPI_FLASH=y
 CONFIG_SPI_FLASH=y
 CONFIG_SPI_FLASH_BAR=y

configs/k2g_hs_evm_defconfig +0 -1

index b8c395d78d..a50a7abf30 100644
@@ -34,7 +34,6 @@ CONFIG_MMC_OMAP_HS=y
 CONFIG_MTD=y
 CONFIG_MTD_UBI=y
 CONFIG_MTD_RAW_NAND=y
-CONFIG_NAND_DAVINCI=y
 CONFIG_DM_SPI_FLASH=y
 CONFIG_SPI_FLASH=y
 CONFIG_SPI_FLASH_BAR=y

Quoted 5 lines · On Wed, 5 Dec 2018 00:57:03 +0100 Miquel Raynal <[email protected]>…

Due to previous Makefile organization, the raw NAND subdirectory was not compiled in if CMD_NAND was not enabled. Because the Denali driver does not compile with these boards (undefined environment offset), remove the dependency within the defconfig over the controller driver (was ignored anyway in the past).

Maybe this should be moved before the Makefile re-organization if this the build of those defconfigs.

While the right Kconfig entries were selected, because of the missing CMD_NAND symbol the raw NAND core was never compiled. Remove it from the defconfigs otherwise the build will fail.

Because of the include's game, when some files are compiled for a SPI NAND device, no warning appears. But when it is for a raw NAND device, GCC complains. Fix these warning by including <common.h>.

Signed-off-by Miquel Raynal <[email protected]>

---
 drivers/mtd/nand/bbt.c  | 1 +
 drivers/mtd/nand/core.c | 1 +
 2 files changed, 2 insertions(+)

drivers/mtd/nand/bbt.c +1 -0

index 7e0ad3190c..f3d05e6757 100644
@@ -9,6 +9,7 @@
 
 #define pr_fmt(fmt)	"nand-bbt: " fmt
 
+#include <common.h>
 #include <linux/mtd/nand.h>
 #ifndef __UBOOT__
 #include <linux/slab.h>

drivers/mtd/nand/core.c +1 -0

index 0b793695cc..3abaef23c5 100644
@@ -9,6 +9,7 @@
 
 #define pr_fmt(fmt)	"nand: " fmt
 
+#include <common.h>
 #ifndef __UBOOT__
 #include <linux/module.h>
 #endif

CONFIG_DFU_NAND needs the NAND core being compiled.

Also fix the colibri_vf defconfig to reflect this dependency.

Signed-off-by Miquel Raynal <[email protected]>

---
 configs/colibri_vf_defconfig | 1 +
 drivers/dfu/Kconfig          | 1 +
 2 files changed, 2 insertions(+)

configs/colibri_vf_defconfig +1 -0

index 305dfcccb5..73b7055de1 100644
@@ -50,6 +50,7 @@ CONFIG_FSL_ESDHC=y
 CONFIG_NAND_VF610_NFC=y
 CONFIG_SYS_NAND_VF610_NFC_60_ECC_BYTES=y
 CONFIG_MTD=y
+CONFIG_MTD_RAW_NAND=y
 CONFIG_MTD_UBI=y
 CONFIG_MTD_UBI_FASTMAP=y
 CONFIG_PHYLIB=y

drivers/dfu/Kconfig +1 -0

index 4692736c9d..5ee260913c 100644
@@ -31,6 +31,7 @@ config DFU_MMC
 config DFU_NAND
 	bool "NAND back end for DFU"
 	depends on CMD_MTDPARTS
+	depends on MTD_RAW_NAND
 	help
 	  This option enables using DFU to read and write to NAND based
 	  storage.

A NOR flash needs the MTD core, ensure this dependency is met by adding a "depends on" in Kconfig. This is fine since defconfigs have been fixed.

Signed-off-by Miquel Raynal <[email protected]>

---
 drivers/mtd/Kconfig | 1 +
 1 file changed, 1 insertion(+)

drivers/mtd/Kconfig +0 -0

index 345046c2a6..0832f5b411 100644

Quoted 14 lines · On Wed, 5 Dec 2018 00:57:07 +0100 Miquel Raynal <[email protected]>…

A NOR flash needs the MTD core, ensure this dependency is met by adding a "depends on" in Kconfig. This is fine since defconfigs have been fixed.

Signed-off-by Miquel Raynal <[email protected]>

---
 drivers/mtd/Kconfig | 1 +
 1 file changed, 1 insertion(+)

drivers/mtd/Kconfig +0 -0

index 345046c2a6..0832f5b411 100644

drivers/mtd/Kconfig config DM_MTD · line 25

Quoted 7 lines

drivers/mtd/Kconfig +1 -0

@@ -19,6 +19,7 @@ config DM_MTD
 
 config MTD_NOR_FLASH
 	bool "Enable parallel NOR flash support"
+	depends on MTD
 	help
 	  Enable support for parallel NOR flash.
 

Don't know if this applies here, but if you have almost all options in Kconfig that depend on MTD, you can add an

if MTD
endif

section.

A NOR flash needs the MTD core, ensure this dependency is met by adding a "depends on" in Kconfig. This is fine since defconfigs have been fixed.

Now that the defconfigs have been fixed, remove the dependencies on commands in Kconfig.

The necessary symbols are enabled so this will
not break anything.

Signed-off-by Miquel Raynal <[email protected]>

---
 drivers/mtd/nand/Makefile | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

drivers/mtd/nand/Makefile +1 -3

index 99ca69ef95..e5849dc02a 100644
@@ -2,7 +2,5 @@
 
 nandcore-objs := core.o bbt.o
 obj-$(CONFIG_MTD_NAND_CORE) += nandcore.o
-ifneq (,$(findstring y,$(CONFIG_CMD_NAND)$(CONFIG_CMD_MTD)))
-obj-y += raw/
-endif
+obj-$(CONFIG_MTD_RAW_NAND) += raw/
 obj-$(CONFIG_MTD_SPI_NAND) += spi/

Now that the defconfigs have been fixed, remove the dependencies on commands in Kconfig.

The necessary symbols are enabled so this will
not break anything.

Signed-off-by Miquel Raynal <[email protected]>

---
 drivers/mtd/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/mtd/Makefile +1 -1

index 486634b562..68b1029734 100644
@@ -24,5 +24,5 @@ ifeq ($(CONFIG_SPL_BUILD)$(CONFIG_TPL_BUILD),)
 obj-y += nand/
 obj-y += onenand/
 obj-y += spi/
-obj-$(CONFIG_CMD_UBI) += ubi/
+obj-$(CONFIG_MTD_UBI) += ubi/
 endif

Guard mtdparts options with an if/endif statement in Kconfig. Also move the Kconfig entry of the option right after the entry of the command.

Signed-off-by Miquel Raynal <[email protected]>

---
 cmd/Kconfig | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

cmd/Kconfig +0 -0

index bf9cc0c52d..cf58174013 100644

cmd/Kconfig +8 -10

+	help
+	  This enables the 'spread' sub-command of the mtdparts command.
+	  This command will modify the existing mtdparts variable by increasing
+	  the size of the partitions such that 1) each partition's net size is
+	  at least as large as the size specified in the mtdparts variable and
+	  2) each partition starts on a good block.
+endif
+
 config MTDIDS_DEFAULT
 	string "Default MTD IDs"
 	depends on MTD_PARTITIONS || CMD_MTDPARTS || CMD_NAND || CMD_FLASH

@@ -1755,16 +1766,6 @@ config MTDPARTS_DEFAULT
 	  Defines a default MTD partitioning scheme in the Linux MTD command
 	  line partitions format
 
-config CMD_MTDPARTS_SPREAD
-	bool "Padd partition size to take account of bad blocks"
-	depends on CMD_MTDPARTS
-	help
-	  This enables the 'spread' sub-command of the mtdparts command.
-	  This command will modify the existing mtdparts variable by increasing
-	  the size of the partitions such that 1) each partition's net size is
-	  at least as large as the size specified in the mtdparts variable and
-	  2) each partition starts on a good block.
-
 config CMD_REISER
 	bool "reiser - Access to reiserfs filesystems"
 	help

The 'sf' command is not supposed to rely on the MTD stack, but both 'sf' and 'nand' commands use helpers located in mtd_uboot.c. Despite their location, these functions do not depend at all on the MTD stack.

This file (drivers/mtd/mtd_uboot.c) is only compiled if CONFIG_MTD is selected, which is incoherent with the current situation. Solve this

by mobing these three functions (which are only used by the above two

commands) out of mtd_uboot.c and put them in a C file only compiled with cmd/sf.c and cmd/nand.c.

Signed-off-by Miquel Raynal <[email protected]>

---
 cmd/Makefile            |  4 +-
 cmd/legacy-mtd-utils.c  | 99 +++++++++++++++++++++++++++++++++++++++++
 cmd/legacy-mtd-utils.h  | 14 ++++++
 cmd/nand.c              |  2 +
 cmd/sf.c                |  2 +
 drivers/mtd/mtd_uboot.c | 94 --------------------------------------
 include/linux/mtd/mtd.h |  6 ---
 7 files changed, 119 insertions(+), 102 deletions(-)
 create mode 100644 cmd/legacy-mtd-utils.c
 create mode 100644 cmd/legacy-mtd-utils.h

cmd/Makefile +1 -1

index 0534ddc679..54c54c062d 100644
@@ -94,7 +94,7 @@ obj-$(CONFIG_CMD_MMC_SPI) += mmc_spi.o
 obj-$(CONFIG_MP) += mp.o
 obj-$(CONFIG_CMD_MTD) += mtd.o
 obj-$(CONFIG_CMD_MTDPARTS) += mtdparts.o
-obj-$(CONFIG_CMD_NAND) += nand.o
+obj-$(CONFIG_CMD_NAND) += nand.o legacy-mtd-utils.o
 obj-$(CONFIG_CMD_NET) += net.o
 obj-$(CONFIG_CMD_ONENAND) += onenand.o
 obj-$(CONFIG_CMD_OSD) += osd.o

ifeq (,$(findstring y,$(CONFIG_CMD_NAND)$(CONFIG_CMD_SF)))

Hello,

On Wed, 5 Dec 2018 11:37:54 +0100, Boris Brezillon wrote:

Quoted 1 line

ifeq (,$(findstring y,$(CONFIG_CMD_NAND)$(CONFIG_CMD_SF)))

This could be "simplified" as:

ifneq ($(CONFIG_CMD_NAND)$(CONFIG_CMD_SF),)

if either of the options is 'y', the string is non-empty, so the condition will be true.

Defconfigs have been fixed, now we can add proper dependencies in Kconfig. SPI FLASH is still not dependent on MTD (deeper rework needed).

Signed-off-by Miquel Raynal <[email protected]>

---
 cmd/Kconfig | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

cmd/Kconfig +5 -1

index cf58174013..7c166a07e6 100644
@@ -679,6 +679,7 @@ config CMD_FDC
 config CMD_FLASH
 	bool "flinfo, erase, protect"
 	default y
+	depends on MTD
 	help
 	  NOR flash support.
 	    flinfo - print FLASH memory information

@@ -868,6 +869,7 @@ config CMD_MMC_SWRITE
 
 config CMD_MTD
 	bool "mtd"
+	depends on MTD
 	select MTD_PARTITIONS
 	help
 	  MTD commands support.

@@ -875,6 +877,7 @@ config CMD_MTD
 config CMD_NAND
 	bool "nand"
 	default y if NAND_SUNXI
+	depends on MTD_RAW_NAND
 	help
 	  NAND support.
 

@@ -915,6 +918,7 @@ config CMD_MMC_SPI
 
 config CMD_ONENAND
 	bool "onenand - access to onenand device"
+	depends on MTD
 	help
 	  OneNAND is a brand of NAND ('Not AND' gate) flash which provides
 	  various useful features. This command allows reading, writing,

@@ -1733,7 +1737,7 @@ config CMD_JFFS2
 
 config CMD_MTDPARTS
 	bool "MTD partition support"
-	select MTD if (CMD_NAND || NAND)
+	depends on MTD
 	help
 	  MTD partitioning tool support.
 	  It is strongly encouraged to avoid using this command

cmd/Kconfig +1 -1

 	help
 	  Defines a default MTD IDs list for use with MTD partitions in the
 	  Linux MTD command line partitions format.
 
 config MTDPARTS_DEFAULT
 	string "Default MTD partition scheme"
-	depends on MTD_PARTITIONS || CMD_MTDPARTS || CMD_NAND || CMD_FLASH
+	depends on MTD || SPI_FLASH
 	help
 	  Defines a default MTD partitioning scheme in the Linux MTD command
 	  line partitions format

Quoted 55 lines · On Wed, 5 Dec 2018 00:57:12 +0100 Miquel Raynal <[email protected]>…

Defconfigs have been fixed, now we can add proper dependencies in Kconfig. SPI FLASH is still not dependent on MTD (deeper rework needed).

Signed-off-by Miquel Raynal <[email protected]>

---
 cmd/Kconfig | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

cmd/Kconfig +5 -1

index cf58174013..7c166a07e6 100644
@@ -679,6 +679,7 @@ config CMD_FDC
 config CMD_FLASH
 	bool "flinfo, erase, protect"
 	default y
+	depends on MTD
 	help
 	  NOR flash support.
 	    flinfo - print FLASH memory information

@@ -868,6 +869,7 @@ config CMD_MMC_SWRITE
 
 config CMD_MTD
 	bool "mtd"
+	depends on MTD
 	select MTD_PARTITIONS
 	help
 	  MTD commands support.

@@ -875,6 +877,7 @@ config CMD_MTD
 config CMD_NAND
 	bool "nand"
 	default y if NAND_SUNXI
+	depends on MTD_RAW_NAND
 	help
 	  NAND support.
 

@@ -915,6 +918,7 @@ config CMD_MMC_SPI
 
 config CMD_ONENAND
 	bool "onenand - access to onenand device"
+	depends on MTD
 	help
 	  OneNAND is a brand of NAND ('Not AND' gate) flash which provides
 	  various useful features. This command allows reading, writing,

@@ -1733,7 +1737,7 @@ config CMD_JFFS2
 
 config CMD_MTDPARTS
 	bool "MTD partition support"
-	select MTD if (CMD_NAND || NAND)
+	depends on MTD
 	help
 	  MTD partitioning tool support.
 	  It is strongly encouraged to avoid using this command

@@ -1754,14 +1758,14 @@ endif
 

cmd/Kconfig endif · line 1761

Quoted 4 lines

cmd/Kconfig +1 -1

 config MTDIDS_DEFAULT
 	string "Default MTD IDs"
-	depends on MTD_PARTITIONS || CMD_MTDPARTS || CMD_NAND || CMD_FLASH
+	depends on MTD || SPI_FLASH  

Can't we have MTD enabled without MTD_PARTITIONS? I guess we don't need to expose these options if MTD_PARTITIONS is disabled.

Quoted 5 lines

cmd/Kconfig +0 -0

 	help
 	  Defines a default MTD IDs list for use with MTD partitions in the
 	  Linux MTD command line partitions format.
 

cmd/Kconfig endif · line 1771

Quoted 7 lines

cmd/Kconfig +1 -1

 config MTDPARTS_DEFAULT
 	string "Default MTD partition scheme"
-	depends on MTD_PARTITIONS || CMD_MTDPARTS || CMD_NAND || CMD_FLASH
+	depends on MTD || SPI_FLASH
 	help
 	  Defines a default MTD partitioning scheme in the Linux MTD command
 	  line partitions format  

Hi Boris,

Boris Brezillon <[email protected]> wrote on Wed, 5 Dec 2018 11:42:08 +0100:

Quoted 58 lines

Quoted 55 lines · On Wed, 5 Dec 2018 00:57:12 +0100 Miquel Raynal <[email protected]>…

Defconfigs have been fixed, now we can add proper dependencies in Kconfig. SPI FLASH is still not dependent on MTD (deeper rework needed).

Signed-off-by Miquel Raynal <[email protected]>

---
 cmd/Kconfig | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

cmd/Kconfig +5 -1

index cf58174013..7c166a07e6 100644
@@ -679,6 +679,7 @@ config CMD_FDC
 config CMD_FLASH
 	bool "flinfo, erase, protect"
 	default y
+	depends on MTD
 	help
 	  NOR flash support.
 	    flinfo - print FLASH memory information

@@ -868,6 +869,7 @@ config CMD_MMC_SWRITE
 
 config CMD_MTD
 	bool "mtd"
+	depends on MTD
 	select MTD_PARTITIONS
 	help
 	  MTD commands support.

@@ -875,6 +877,7 @@ config CMD_MTD
 config CMD_NAND
 	bool "nand"
 	default y if NAND_SUNXI
+	depends on MTD_RAW_NAND
 	help
 	  NAND support.
 

@@ -915,6 +918,7 @@ config CMD_MMC_SPI
 
 config CMD_ONENAND
 	bool "onenand - access to onenand device"
+	depends on MTD
 	help
 	  OneNAND is a brand of NAND ('Not AND' gate) flash which provides
 	  various useful features. This command allows reading, writing,

@@ -1733,7 +1737,7 @@ config CMD_JFFS2
 
 config CMD_MTDPARTS
 	bool "MTD partition support"
-	select MTD if (CMD_NAND || NAND)
+	depends on MTD
 	help
 	  MTD partitioning tool support.
 	  It is strongly encouraged to avoid using this command

@@ -1754,14 +1758,14 @@ endif
 

Quoted 7 lines

cmd/Kconfig endif · line 1761

Quoted 4 lines

cmd/Kconfig +1 -1

 config MTDIDS_DEFAULT
 	string "Default MTD IDs"
-	depends on MTD_PARTITIONS || CMD_MTDPARTS || CMD_NAND || CMD_FLASH
+	depends on MTD || SPI_FLASH    

Can't we have MTD enabled without MTD_PARTITIONS? I guess we don't need to expose these options if MTD_PARTITIONS is disabled.

That's the theory. (Travis) Experience shows that adding a dependency on MTD_PARTITIONS when removing the dependency on the above commands is too restrictive and some header files using MTDIDS/MTDPARTS_DEFAULT will produce build issues. This is insane but I did not want to debug this issue and, anyway, it is harmless to have these strings defined.

Instead of compiling just a few files from the root Makefile, rework the MTD Makefile tree to take into account SPL's needs.