Hi Igor,

thanks for your review!

On 06/22/2016 05:46 PM, Igor Grinberg wrote:

Hi Christopher,

Thanks for doing this work.

On 06/19/2016 06:44 PM, Christopher Spinrath wrote:

...

Hi,

this series aims at enhancing support for the cm-fx6 module. In particular, with respect to the upstream linux kernel.

The first patch improves the default environment. It is non-functional but makes it more convenient to adapt certain settings.

The later two patches add mtd partition support for the on-board spi flash chip. They pick up the discussion about specifying a default partitioning in the device tree from here [1]. In short: adding the default partitioning to the device tree was rejected by the linux/ device tree community during mainlining large parts of the device tree. It was proposed to implement the partition/mtd handling in u-boot. On the other hand, it was argued that the flash chip becomes some kind of "black-box" since there will be no partition labeling (in particular, with old u-boot versions).

IMHO defining the mtd partitioning has the following (dis-)advantages:

You mean defining it in U-Boot instead of upstream DT.

Yes.

...

Advantages:

• It is easier for the user to change the partitioning (e.g. to use the unsued 1MB free space).

I know it says reserved, but that is not exactly unused... It is intended to hold a splash screen of up to 1MB size and may be other things (like dtb, etc.). By the time the partitioning layout was defined, it was still unclear what requirements of that additional space will be. So it was called reserved to provide a kind of warning to the users as it might be used at some point.

Good to know. So this would be a use case for this series. People can rename the last partition to e.g. uboot-splash.

...

• The flash ship is used entirely for u-boot.

Close, but not precise... The spi flash chip is used for all boot purposes, so it might be beyond U-Boot.

Ok. But still it is U-Boot (or another bootloader) that relies on the memory layout.

...

So it is quite natural that u-boot manages it. Also, moving the partition table to it allows us to change the layout in future versions of u-boot (almost independently of the kernel - there are still non-device tree kernels).

Please don't change the layout as it will break backwards compatibility. Also, there is not much you can change.

I have no intention do so but users may want to change it (and now, can do so easily). For example, choose a better name for the "reserved" partition.

...

• U-Boot becomes the single point of definition for all device tree kernels. Otherwise, each kernel (vendor vs. upstream + version) would ship its own partitioning.

True.

...

Moreover, u-boot has to know something about the partitioning, too, because it has to know where the environment is saved.

It does, although the env address is hardcoded, but it should not be changed. The reason for providing a partition for it is purely for Linux to able to change the environment variables.

Indeed, but moving both definitions into one project reduces the risk of having conflicts and allows (advanced) users to change it by adapting only one project (e.g. another bootloader may use another partitioning).

Cheers, Christopher

...

Disadvantages:

• Users of the upstream linux kernel have to use a recent u-boot version to avoid the "black box" effect. A concrete impact is that the update routine (described/proposed by CompuLab) does not work out of the box with older u-boot versions.

True.

...

• Updating u-boot is something users might not want or miss to do.

However, I think nowadays it is ok to demand a recent u-boot in combination with the upstream kernel. The cm-fx6 wouldn't be the first board doing so. Hence, I propose these patches here.

Cheers, Christopher

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-June/434562.html