[U-Boot] [PATCH 0/3] ARM: imx: enhance support for the cm-fx6 module
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:
Advantages: - It is easier for the user to change the partitioning (e.g. to use the unsued 1MB free space).
- The flash ship is used entirely for u-boot. 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).
- 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. Moreover, u-boot has to know something about the partitioning, too, because it has to know where the environment is saved.
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.
- 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
Currently, entire script segments have to be changed in the default environment to change the kernel image location or to append kernel cmdline parameters. In the later case this has to be changed for every possible boot device.
Introduce new variables for kernel image locations and boot device independent kernel parameters to make it easier to change these settings.
Signed-off-by: Christopher Spinrath christopher.spinrath@rwth-aachen.de --- include/configs/cm_fx6.h | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/include/configs/cm_fx6.h b/include/configs/cm_fx6.h index 1f20ec3..f054ca8 100644 --- a/include/configs/cm_fx6.h +++ b/include/configs/cm_fx6.h @@ -69,6 +69,8 @@ "stderr=serial,vga\0" \ "panel=HDMI\0" \ "autoload=no\0" \ + "uImage=uImage-cm-fx6\0" \ + "zImage=zImage-cm-fx6\0" \ "kernel=uImage-cm-fx6\0" \ "script=boot.scr\0" \ "dtb=cm-fx6.dtb\0" \ @@ -81,10 +83,10 @@ "video_dvi=mxcfb0:dev=dvi,1280x800M-32@50,if=RGB32\0" \ "doboot=bootm ${loadaddr}\0" \ "doloadfdt=false\0" \ - "setboottypez=setenv kernel zImage-cm-fx6;" \ + "setboottypez=setenv kernel ${zImage};" \ "setenv doboot bootz ${loadaddr} - ${fdtaddr};" \ "setenv doloadfdt true;\0" \ - "setboottypem=setenv kernel uImage-cm-fx6;" \ + "setboottypem=setenv kernel ${uImage};" \ "setenv doboot bootm ${loadaddr};" \ "setenv doloadfdt false;\0"\ "mmcroot=/dev/mmcblk0p2 rw rootwait\0" \ @@ -92,13 +94,13 @@ "nandroot=/dev/mtdblock4 rw\0" \ "nandrootfstype=ubifs\0" \ "mmcargs=setenv bootargs console=${console} root=${mmcroot} " \ - "${video}\0" \ + "${video} ${extrabootargs}\0" \ "sataargs=setenv bootargs console=${console} root=${sataroot} " \ - "${video}\0" \ + "${video} ${extrabootargs}\0" \ "nandargs=setenv bootargs console=${console} " \ "root=${nandroot} " \ "rootfstype=${nandrootfstype} " \ - "${video}\0" \ + "${video} ${extrabootargs}\0" \ "nandboot=if run nandloadkernel; then " \ "run nandloadfdt;" \ "run setboottypem;" \
On 06/19/2016 06:44 PM, Christopher Spinrath wrote:
Currently, entire script segments have to be changed in the default environment to change the kernel image location or to append kernel cmdline parameters. In the later case this has to be changed for every possible boot device.
Introduce new variables for kernel image locations and boot device independent kernel parameters to make it easier to change these settings.
Signed-off-by: Christopher Spinrath christopher.spinrath@rwth-aachen.de
Reviewed-by: Igor Grinberg grinberg@compulab.co.il
On Sun, Jun 19, 2016 at 05:44:54PM +0200, Christopher Spinrath wrote:
Currently, entire script segments have to be changed in the default environment to change the kernel image location or to append kernel cmdline parameters. In the later case this has to be changed for every possible boot device.
Introduce new variables for kernel image locations and boot device independent kernel parameters to make it easier to change these settings.
Reviewed-by: Nikita Kiryanov nikita@compulab.co.il
Signed-off-by: Christopher Spinrath christopher.spinrath@rwth-aachen.de
include/configs/cm_fx6.h | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)
The cm-fx6 module has an on-board st,m25p compatible spi flash chip used for u-boot (binary & environment). Overwrite the partitions in the device tree by the partition table provided in the mtdparts environment variable, if it is set.
This allows to specify a kernel independent partitioning in the environment and provides a convient way for the user to adapt the partition table.
Signed-off-by: Christopher Spinrath christopher.spinrath@rwth-aachen.de --- board/compulab/cm_fx6/cm_fx6.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/board/compulab/cm_fx6/cm_fx6.c b/board/compulab/cm_fx6/cm_fx6.c index 712057a..81a7ae2 100644 --- a/board/compulab/cm_fx6/cm_fx6.c +++ b/board/compulab/cm_fx6/cm_fx6.c @@ -12,6 +12,7 @@ #include <dm.h> #include <fsl_esdhc.h> #include <miiphy.h> +#include <mtd_node.h> #include <netdev.h> #include <errno.h> #include <usb.h> @@ -28,6 +29,7 @@ #include <asm/io.h> #include <asm/gpio.h> #include <dm/platform_data/serial_mxc.h> +#include <jffs2/load_kernel.h> #include "common.h" #include "../common/eeprom.h" #include "../common/common.h" @@ -581,6 +583,13 @@ int cm_fx6_setup_ecspi(void) { return 0; }
#ifdef CONFIG_OF_BOARD_SETUP #define USDHC3_PATH "/soc/aips-bus@02100000/usdhc@02198000/" + +#ifdef CONFIG_FDT_FIXUP_PARTITIONS +struct node_info nodes[] = { + { "st,m25p", MTD_DEV_TYPE_NOR, }, +}; +#endif + int ft_board_setup(void *blob, bd_t *bd) { u32 baseboard_rev; @@ -589,6 +598,8 @@ int ft_board_setup(void *blob, bd_t *bd) char baseboard_name[16]; int err;
+ fdt_shrink_to_minimum(blob); /* Make room for new properties */ + /* MAC addr */ if (eth_getenv_enetaddr("ethaddr", enetaddr)) { fdt_find_and_setprop(blob, @@ -607,7 +618,6 @@ int ft_board_setup(void *blob, bd_t *bd) return 0; /* Assume not an early revision SB-FX6m baseboard */
if (!strncmp("SB-FX6m", baseboard_name, 7) && baseboard_rev <= 120) { - fdt_shrink_to_minimum(blob); /* Make room for new properties */ nodeoffset = fdt_path_offset(blob, USDHC3_PATH); fdt_delprop(blob, nodeoffset, "cd-gpios"); fdt_find_and_setprop(blob, USDHC3_PATH, "broken-cd", @@ -616,6 +626,10 @@ int ft_board_setup(void *blob, bd_t *bd) NULL, 0, 1); }
+#ifdef CONFIG_FDT_FIXUP_PARTITIONS + fdt_fixup_mtdparts(blob, nodes, ARRAY_SIZE(nodes)); +#endif + return 0; } #endif
Hi Christopher,
On 06/19/2016 06:44 PM, Christopher Spinrath wrote:
The cm-fx6 module has an on-board st,m25p compatible spi flash chip used for u-boot (binary & environment). Overwrite the partitions in the device tree by the partition table provided in the mtdparts environment variable, if it is set.
This allows to specify a kernel independent partitioning in the environment and provides a convient way for the user to adapt the partition table.
Signed-off-by: Christopher Spinrath christopher.spinrath@rwth-aachen.de
board/compulab/cm_fx6/cm_fx6.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/board/compulab/cm_fx6/cm_fx6.c b/board/compulab/cm_fx6/cm_fx6.c index 712057a..81a7ae2 100644 --- a/board/compulab/cm_fx6/cm_fx6.c +++ b/board/compulab/cm_fx6/cm_fx6.c @@ -12,6 +12,7 @@ #include <dm.h> #include <fsl_esdhc.h> #include <miiphy.h> +#include <mtd_node.h> #include <netdev.h> #include <errno.h> #include <usb.h> @@ -28,6 +29,7 @@ #include <asm/io.h> #include <asm/gpio.h> #include <dm/platform_data/serial_mxc.h> +#include <jffs2/load_kernel.h>
Why is this needed?
#include "common.h" #include "../common/eeprom.h" #include "../common/common.h" @@ -581,6 +583,13 @@ int cm_fx6_setup_ecspi(void) { return 0; }
#ifdef CONFIG_OF_BOARD_SETUP #define USDHC3_PATH "/soc/aips-bus@02100000/usdhc@02198000/"
+#ifdef CONFIG_FDT_FIXUP_PARTITIONS +struct node_info nodes[] = {
- { "st,m25p", MTD_DEV_TYPE_NOR, },
+}; +#endif
int ft_board_setup(void *blob, bd_t *bd) { u32 baseboard_rev; @@ -589,6 +598,8 @@ int ft_board_setup(void *blob, bd_t *bd) char baseboard_name[16]; int err;
- fdt_shrink_to_minimum(blob); /* Make room for new properties */
- /* MAC addr */ if (eth_getenv_enetaddr("ethaddr", enetaddr)) { fdt_find_and_setprop(blob,
@@ -607,7 +618,6 @@ int ft_board_setup(void *blob, bd_t *bd) return 0; /* Assume not an early revision SB-FX6m baseboard */
if (!strncmp("SB-FX6m", baseboard_name, 7) && baseboard_rev <= 120) {
fdt_shrink_to_minimum(blob); /* Make room for new properties */@@ -616,6 +626,10 @@ int ft_board_setup(void *blob, bd_t *bd) NULL, 0, 1); }
+#ifdef CONFIG_FDT_FIXUP_PARTITIONS
- fdt_fixup_mtdparts(blob, nodes, ARRAY_SIZE(nodes));
+#endif
I really dislike the ifdeffery inside functions. Care to introduce a stub for the !CONFIG_FDT_FIXUP_PARTITIONS case in include/fdt_support.h for this one?
- return 0;
} #endif
On 06/19/2016 06:44 PM, Christopher Spinrath wrote:
The cm-fx6 module has an on-board st,m25p compatible spi flash chip used for u-boot (binary & environment). Overwrite the partitions in the device tree by the partition table provided in the mtdparts environment variable, if it is set.
This allows to specify a kernel independent partitioning in the environment and provides a convient way for the user to adapt the partition table.
Signed-off-by: Christopher Spinrath christopher.spinrath@rwth-aachen.de
board/compulab/cm_fx6/cm_fx6.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/board/compulab/cm_fx6/cm_fx6.c b/board/compulab/cm_fx6/cm_fx6.c index 712057a..81a7ae2 100644 --- a/board/compulab/cm_fx6/cm_fx6.c +++ b/board/compulab/cm_fx6/cm_fx6.c
[...]
+#ifdef CONFIG_FDT_FIXUP_PARTITIONS +struct node_info nodes[] = {
- { "st,m25p", MTD_DEV_TYPE_NOR, },
Nikita, is this enough for all flashes we assemble on cm-fx6?
+}; +#endif
[...]
On Wed, Jun 22, 2016 at 07:17:53PM +0300, Igor Grinberg wrote:
On 06/19/2016 06:44 PM, Christopher Spinrath wrote:
The cm-fx6 module has an on-board st,m25p compatible spi flash chip used for u-boot (binary & environment). Overwrite the partitions in the device tree by the partition table provided in the mtdparts environment variable, if it is set.
This allows to specify a kernel independent partitioning in the environment and provides a convient way for the user to adapt the partition table.
Signed-off-by: Christopher Spinrath christopher.spinrath@rwth-aachen.de
board/compulab/cm_fx6/cm_fx6.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/board/compulab/cm_fx6/cm_fx6.c b/board/compulab/cm_fx6/cm_fx6.c index 712057a..81a7ae2 100644 --- a/board/compulab/cm_fx6/cm_fx6.c +++ b/board/compulab/cm_fx6/cm_fx6.c
[...]
+#ifdef CONFIG_FDT_FIXUP_PARTITIONS +struct node_info nodes[] = {
- { "st,m25p", MTD_DEV_TYPE_NOR, },
Nikita, is this enough for all flashes we assemble on cm-fx6?
Yes, CM-FX6 is using M25PX16 and SST25VF016B, both of which are supported by the m25p80.c driver. However, on the mainline branch I don't see "m25p" in the list of device ids, and IIRC the request is to favor "jedec,spi-nor" as compatible string over device specific ones.
+}; +#endif
[...]
-- Regards, Igor.
The cm-fx6 module has an on-board spi flash chip. Enable mtd support and the mtdparts command. Also define a default partitioning, add it to the default environment, and enable support to overwrite the partitioning defined in a device tree by it.
These changes move the effective default partitioning from the device tree shipped with the vendor kernels to u-boot which becomes the single point of definition for the partitioning for all device tree based kernels (in particular, for the upstream linux kernel which does not have a default partitioning defined in its device tree).
Signed-off-by: Christopher Spinrath christopher.spinrath@rwth-aachen.de --- include/configs/cm_fx6.h | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/include/configs/cm_fx6.h b/include/configs/cm_fx6.h index f054ca8..c839b03 100644 --- a/include/configs/cm_fx6.h +++ b/include/configs/cm_fx6.h @@ -18,6 +18,7 @@ #define CONFIG_MACH_TYPE 4273
/* CMD */ +#define CONFIG_CMD_MTDPARTS
/* MMC */ #define CONFIG_SYS_FSL_USDHC_NUM 3 @@ -53,6 +54,20 @@ #define CONFIG_SF_DEFAULT_SPEED 25000000 #define CONFIG_SF_DEFAULT_MODE (SPI_MODE_0)
+/* MTD support */ +#ifndef CONFIG_SPL_BUILD +#define CONFIG_FDT_FIXUP_PARTITIONS +#define CONFIG_MTD_DEVICE +#define CONFIG_MTD_PARTITIONS +#define CONFIG_SPI_FLASH_MTD +#endif + +#define MTDIDS_DEFAULT "nor0=spi0.0" +#define MTDPARTS_DEFAULT "mtdparts=spi0.0:" \ + "768k(uboot)," \ + "256k(uboot-environment)," \ + "-(reserved)" + /* Environment */ #define CONFIG_ENV_IS_IN_SPI_FLASH #define CONFIG_ENV_SPI_MAX_HZ CONFIG_SF_DEFAULT_SPEED @@ -83,6 +98,8 @@ "video_dvi=mxcfb0:dev=dvi,1280x800M-32@50,if=RGB32\0" \ "doboot=bootm ${loadaddr}\0" \ "doloadfdt=false\0" \ + "mtdids=" MTDIDS_DEFAULT "\0" \ + "mtdparts=" MTDPARTS_DEFAULT "\0" \ "setboottypez=setenv kernel ${zImage};" \ "setenv doboot bootz ${loadaddr} - ${fdtaddr};" \ "setenv doloadfdt true;\0" \ @@ -157,7 +174,7 @@ "run setupnandboot;" \ "run nandboot;"
-#define CONFIG_PREBOOT "usb start" +#define CONFIG_PREBOOT "usb start;sf probe"
/* SPI */ #define CONFIG_SPI
On 06/19/2016 06:44 PM, Christopher Spinrath wrote:
The cm-fx6 module has an on-board spi flash chip. Enable mtd support and the mtdparts command. Also define a default partitioning, add it to the default environment, and enable support to overwrite the partitioning defined in a device tree by it.
These changes move the effective default partitioning from the device tree shipped with the vendor kernels to u-boot which becomes the single point of definition for the partitioning for all device tree based kernels (in particular, for the upstream linux kernel which does not have a default partitioning defined in its device tree).
Signed-off-by: Christopher Spinrath christopher.spinrath@rwth-aachen.de
include/configs/cm_fx6.h | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/include/configs/cm_fx6.h b/include/configs/cm_fx6.h index f054ca8..c839b03 100644 --- a/include/configs/cm_fx6.h +++ b/include/configs/cm_fx6.h
[...]
@@ -157,7 +174,7 @@ "run setupnandboot;" \ "run nandboot;"
-#define CONFIG_PREBOOT "usb start" +#define CONFIG_PREBOOT "usb start;sf probe"
Probably, this is really needed. Care to explain?
/* SPI */ #define CONFIG_SPI
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.
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.
- 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.
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.
- 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.
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
participants (3)
-
Christopher Spinrath -
Igor Grinberg -
Nikita Kiryanov