[U-Boot] [PATCH v3 13/28] mtd: ensure MTD is compiled when ENV_IS_IN_FLASH is selected

[Miquel Raynal]

Hi Wolfgang,

Wolfgang Denk <wd at denx.de> wrote on Thu, 06 Dec 2018 10:32:14 +0100:

> Dear Boris,
> 
> In message <20181206100016.706330ba at bbrezillon> 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.  
> >
> > 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.

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.

> 
> > 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

> 
> > > 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.

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

[...]

> > > 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.

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


Thanks,
Miquèl