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

[Wolfgang Denk]

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.

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

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

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

I can't parse this.  All we need is the ability to directly call the
underlying flash driver.  We don't need all the layers that MTD adds
on top of that.

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

You write "can be minimal".  Has this already been implemented, or
is this just a promise for some (far) future add-on?

What is the (expected) size impact?

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

This is a benefit for MTD users, agreed.  For the ENV_IS_IN_FLASH
case, it replaces a simple call to memcpy(), at least for the boot
time case (and for SPL).  I don't see this as a benefit at all.

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

Yes, I understand this argument.

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

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
A committee is a life form with six or more legs and no brain.
                              -- Lazarus Long, "Time Enough For Love"