ARM: imx: enhance support for the cm-fx6 module
cover 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
PATCH 1/3 ARM: configs: cm_fx6: improve default environment
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.
--- include/configs/cm_fx6.h | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)
include/configs/cm_fx6.h +7 -5
@@ -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 at 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;" \
Signature · 1 line
2.8.3
PATCH 1/3 ARM: configs: cm_fx6: improve default environment
Quoted 10 lines · Christopher Spinrath
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 <[email protected]>
Signature · 2 lines
Regards, Igor.
PATCH 1/3 ARM: configs: cm_fx6: improve default environment
In reply to Christopher SpinrathQuoted 8 lines · Christopher Spinrath
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.
Quoted 6 lines
Signed-off-by Christopher Spinrath <[email protected]>--- include/configs/cm_fx6.h | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)
PATCH 2/3 ARM: board: cm_fx6: fixup mtd partitions in the fdt
In reply to Christopher SpinrathThe 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.
--- board/compulab/cm_fx6/cm_fx6.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)
board/compulab/cm_fx6/cm_fx6.c +15 -1
@@ -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 at 02100000/usdhc at 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
Signature · 1 line
2.8.3
PATCH 2/3 ARM: board: cm_fx6: fixup mtd partitions in the fdt
Hi Christopher,
Quoted 27 lines · Christopher Spinrath
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 <[email protected]>--- board/compulab/cm_fx6/cm_fx6.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)board/compulab/cm_fx6/cm_fx6.c +1
-0@@ -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>
board/compulab/cm_fx6/cm_fx6.c line 32
Quoted 4 lines
board/compulab/cm_fx6/cm_fx6.c +1
-0@@ -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?
Quoted 35 lines
board/compulab/cm_fx6/cm_fx6.c +9
-1#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 at 02100000/usdhc at 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",
board/compulab/cm_fx6/cm_fx6.c int ft_board_setup(void *blob, bd_t *bd) · line 631
Quoted 6 lines
board/compulab/cm_fx6/cm_fx6.c +3
-0@@ -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?
Quoted 5 lines
board/compulab/cm_fx6/cm_fx6.c +1
-0+ return 0; }#endif
Signature · 2 lines
Regards, Igor.
PATCH 2/3 ARM: board: cm_fx6: fixup mtd partitions in the fdt
In reply to Christopher SpinrathQuoted 18 lines · Christopher Spinrath
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 <[email protected]>--- board/compulab/cm_fx6/cm_fx6.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)board/compulab/cm_fx6/cm_fx6.c +0
-0
[...]
board/compulab/cm_fx6/cm_fx6.c int cm_fx6_setup_ecspi(void) { return 0; } · line 589
Quoted 3 lines
board/compulab/cm_fx6/cm_fx6.c +3
-0+#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?
Quoted 3 lines
board/compulab/cm_fx6/cm_fx6.c +3
-0+}; +#endif +
[...]
Signature · 2 lines
Regards, Igor.
PATCH 2/3 ARM: board: cm_fx6: fixup mtd partitions in the fdt
Quoted 22 lines · Igor Grinberg
Quoted 18 lines · Christopher Spinrath
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 <[email protected]>--- board/compulab/cm_fx6/cm_fx6.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)board/compulab/cm_fx6/cm_fx6.c +0
-0[...]
Quoted 5 lines
board/compulab/cm_fx6/cm_fx6.c int cm_fx6_setup_ecspi(void) { return 0; } · line 589
Quoted 3 lines
board/compulab/cm_fx6/cm_fx6.c +3
-0+#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.
Quoted 11 lines
Quoted 4 lines
board/compulab/cm_fx6/cm_fx6.c +3
-0+}; +#endif +[...]
Signature · 2 lines
Regards, Igor.
A message that is not in the archive (sent off-list or not yet received): [email protected]
PATCH 2/3 ARM: board: cm_fx6: fixup mtd partitions in the fdt
Hi Nikita,
Quoted 29 lines · Nikita Kiryanov
Quoted 22 lines · Igor Grinberg
Quoted 18 lines · Christopher Spinrath
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 <[email protected]>--- board/compulab/cm_fx6/cm_fx6.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)board/compulab/cm_fx6/cm_fx6.c +0
-0[...]
Quoted 5 lines
board/compulab/cm_fx6/cm_fx6.c int cm_fx6_setup_ecspi(void) { return 0; } · line 589
Quoted 3 lines
board/compulab/cm_fx6/cm_fx6.c +3
-0+#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?
Quoted 5 lines
Yes, CM-FX6 is using M25PX16 and SST25VF016B, both of which are supported by the
m25p80.cdriver. 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.
Linux is going to use "st,m25p", "jedec,spi-nor" as compatible list (currently queued for inclusion in v4.8:
https://git.kernel.org/cgit/linux/kernel/git/arm/arm-soc.git/tree/arch/arm/boot/dts/imx6q-cm-fx6.dts?h=next/dt#n123
).
I have chosen "st,m25p" here to cover both the mainline and CompuLab's device trees (I have seen some where "jedec,spi-nor" is not in the list). However, if you prefer I will switch to "jedec,spi-nor" (excluding some device trees) in v2.
Thanks,
Christopher
Quoted 12 lines
Quoted 12 lines
Quoted 5 lines
board/compulab/cm_fx6/cm_fx6.c +3
-0+}; +#endif +[...]
Signature · 2 lines
Regards, Igor.
PATCH 2/3 ARM: board: cm_fx6: fixup mtd partitions in the fdt
Hi Christopher,
Quoted 43 lines · Christopher Spinrath
Hi Nikita,
Quoted 29 lines · Nikita Kiryanov
Quoted 22 lines · Igor Grinberg
Quoted 18 lines · Christopher Spinrath
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 <[email protected]>--- board/compulab/cm_fx6/cm_fx6.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)board/compulab/cm_fx6/cm_fx6.c +0
-0[...]
Quoted 5 lines
board/compulab/cm_fx6/cm_fx6.c int cm_fx6_setup_ecspi(void) { return 0; } · line 589
Quoted 3 lines
board/compulab/cm_fx6/cm_fx6.c +3
-0+#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?
Quoted 5 lines
Yes, CM-FX6 is using M25PX16 and SST25VF016B, both of which are supported by the
m25p80.cdriver. 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.Linux is going to use "st,m25p", "jedec,spi-nor" as compatible list (currently queued for inclusion in v4.8:
https://git.kernel.org/cgit/linux/kernel/git/arm/arm-soc.git/tree/arch/arm/boot/dts/imx6q-cm-fx6.dts?h=next/dt#n123
).
Quoted 4 lines
I have chosen "st,m25p" here to cover both the mainline and CompuLab's device trees (I have seen some where "jedec,spi-nor" is not in the list). However, if you prefer I will switch to "jedec,spi-nor" (excluding some device trees) in v2.
Does it have to be an "or" situation? m25p is necessary to serve older CM-FX6 kernels, but it is not supported in the mainline kernel, so the correct course of actions seems to be to use both "st,m25p" and "jedec,spi-nor".
A message that is not in the archive (sent off-list or not yet received): [email protected]
PATCH 2/3 ARM: board: cm_fx6: fixup mtd partitions in the fdt
Hi Nikita,
Quoted 46 lines · Nikita Kiryanov
Hi Christopher,
Quoted 43 lines · Christopher Spinrath
Hi Nikita,
Quoted 29 lines · Nikita Kiryanov
Quoted 22 lines · Igor Grinberg
Quoted 18 lines · Christopher Spinrath
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 <[email protected]>--- board/compulab/cm_fx6/cm_fx6.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)board/compulab/cm_fx6/cm_fx6.c +0
-0[...]
Quoted 5 lines
board/compulab/cm_fx6/cm_fx6.c int cm_fx6_setup_ecspi(void) { return 0; } · line 589
Quoted 3 lines
board/compulab/cm_fx6/cm_fx6.c +3
-0+#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?
Quoted 5 lines
Yes, CM-FX6 is using M25PX16 and SST25VF016B, both of which are supported by the
m25p80.cdriver. 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.Linux is going to use "st,m25p", "jedec,spi-nor" as compatible list (currently queued for inclusion in v4.8:
https://git.kernel.org/cgit/linux/kernel/git/arm/arm-soc.git/tree/arch/arm/boot/dts/imx6q-cm-fx6.dts?h=next/dt#n123
).
Quoted 8 lines
Quoted 4 lines
I have chosen "st,m25p" here to cover both the mainline and CompuLab's device trees (I have seen some where "jedec,spi-nor" is not in the list). However, if you prefer I will switch to "jedec,spi-nor" (excluding some device trees) in v2.
Does it have to be an "or" situation? m25p is necessary to serve older CM-FX6 kernels, but it is not supported in the mainline kernel, so the correct course of actions seems to be to use both "st,m25p" and "jedec,spi-nor".
This has nothing to do with the kernel itself; the compatible is only used to find the correct dt node describing the flash chip and as I said in my previous email, the upstream device tree node is going to have the st,m25p compatible string.
On the other hand, m25p is not documented but it used to be in upstream device trees. Since it has been removed from most of them recently, I agree that having both compatibles is a good idea. From a quick look at the source code it should be possible. I will verify this for v2.
Thanks,
Christopher
A message that is not in the archive (sent off-list or not yet received): [email protected]
PATCH 2/3 ARM: board: cm_fx6: fixup mtd partitions in the fdt
Hi Igor,
Quoted 28 lines · Igor Grinberg
Hi Christopher,
Quoted 25 lines · Christopher Spinrath
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 <[email protected]>--- board/compulab/cm_fx6/cm_fx6.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)board/compulab/cm_fx6/cm_fx6.c +1
-0@@ -12,6 +12,7 @@#include <dm.h> #include <fsl_esdhc.h> #include <miiphy.h> +#include <mtd_node.h> #include <netdev.h> #include <errno.h>
Quoted 9 lines
board/compulab/cm_fx6/cm_fx6.c line 32
Quoted 6 lines
board/compulab/cm_fx6/cm_fx6.c +1
-0#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?
The MTD_DEV_TYPE_NOR constant is defined in this header. I agree that it is a bit ugly but this header is used for the same purpose in other board files, too (e.g.board/pdm360ng/pdm360ng.c).
Quoted 38 lines
Quoted 38 lines
board/compulab/cm_fx6/cm_fx6.c +9
-1#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 at 02100000/usdhc at 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); }
Quoted 8 lines
board/compulab/cm_fx6/cm_fx6.c int ft_board_setup(void *blob, bd_t *bd) · line 631
Quoted 3 lines
board/compulab/cm_fx6/cm_fx6.c +3
-0+#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 ininclude/fdt_support.hfor this one?
Sure. Is the header the correct place for this or should I add a #else case in the .c file?
Cheers,
Christopher
Quoted 6 lines
Quoted 5 lines
board/compulab/cm_fx6/cm_fx6.c +1
-0+ return 0; }#endif
PATCH 2/3 ARM: board: cm_fx6: fixup mtd partitions in the fdt
Hi Christopher,
Quoted 24 lines · Christopher Spinrath
Hi Igor,
Quoted 21 lines · Igor Grinberg
Hi Christopher,
Quoted 18 lines · Christopher Spinrath
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 <[email protected]>--- board/compulab/cm_fx6/cm_fx6.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)board/compulab/cm_fx6/cm_fx6.c +0
-0
[...]
Quoted 11 lines
Quoted 8 lines
board/compulab/cm_fx6/cm_fx6.c line 32
Quoted 5 lines
board/compulab/cm_fx6/cm_fx6.c +1
-0@@ -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?
The
MTD_DEV_TYPE_NORconstant is defined in this header. I agree that it is a bit ugly but this header is used for the same purpose in other board files, too (e.g.board/pdm360ng/pdm360ng.c).
I see.
I don't feel right to request this in scope of these patches, but if you can take care of that one (even in a follow up patch) - it would be awesome.
[...]
Quoted 12 lines
Quoted 4 lines
Quoted 4 lines
board/compulab/cm_fx6/cm_fx6.c +0
-0@@ -616,6 +626,10 @@ int ft_board_setup(void *blob, bd_t *bd) NULL, 0, 1); }Quoted 8 lines
board/compulab/cm_fx6/cm_fx6.c int ft_board_setup(void *blob, bd_t *bd) · line 631
Quoted 3 lines
board/compulab/cm_fx6/cm_fx6.c +3
-0+#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 ininclude/fdt_support.hfor this one?
Quoted 2 lines
Sure. Is the header the correct place for this or should I add a #else case in the .c file?
IMO, the header file is better for stubbing things out. It does not require you to add .c into compilation. There are also already several examples inside this header.
Signature · 2 lines
Regards, Igor.
A message that is not in the archive (sent off-list or not yet received): [email protected]
PATCH 2/3 ARM: board: cm_fx6: fixup mtd partitions in the fdt
Hi Igor,
Quoted 38 lines · Igor Grinberg
Hi Christopher,
Quoted 24 lines · Christopher Spinrath
Hi Igor,
Quoted 21 lines · Igor Grinberg
Hi Christopher,
Quoted 18 lines · Christopher Spinrath
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 <[email protected]>--- board/compulab/cm_fx6/cm_fx6.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)board/compulab/cm_fx6/cm_fx6.c +0
-0[...]
Quoted 8 lines
Quoted 8 lines
board/compulab/cm_fx6/cm_fx6.c line 32
Quoted 5 lines
board/compulab/cm_fx6/cm_fx6.c +1
-0@@ -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?
Quoted 9 lines
Quoted 3 lines
The
MTD_DEV_TYPE_NORconstant is defined in this header. I agree that it is a bit ugly but this header is used for the same purpose in other board files, too (e.g.board/pdm360ng/pdm360ng.c).I see.
I don't feel right to request this in scope of these patches, but if you can take care of that one (even in a follow up patch) - it would be awesome.
I prefer to defer it, since it touches another subsystem (jffs).
Quoted 14 lines
[...]
Quoted 12 lines
Quoted 12 lines
board/compulab/cm_fx6/cm_fx6.c int ft_board_setup(void *blob, bd_t *bd) · line 631
Quoted 7 lines
board/compulab/cm_fx6/cm_fx6.c +3
-0@@ -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 ininclude/fdt_support.hfor this one?
Quoted 7 lines
Quoted 2 lines
Sure. Is the header the correct place for this or should I add a #else case in the .c file?
IMO, the header file is better for stubbing things out. It does not require you to add .c into compilation. There are also already several examples inside this header.
Ok.
Thanks,
Christopher
PATCH 3/3 ARM: configs: cm_fx6: add mtd support
In reply to Christopher SpinrathThe 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).
--- include/configs/cm_fx6.h | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)
include/configs/cm_fx6.h +18 -1
@@ -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 at 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
Signature · 1 line
2.8.3
PATCH 3/3 ARM: configs: cm_fx6: add mtd support
Quoted 20 lines · Christopher Spinrath
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 <[email protected]>--- include/configs/cm_fx6.h | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)include/configs/cm_fx6.h +0
-0
[...]
include/configs/cm_fx6.h line 177
Quoted 6 lines
include/configs/cm_fx6.h +1
-1@@ -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?
Quoted 4 lines
include/configs/cm_fx6.h +0
-0/* SPI */ #define CONFIG_SPI
Signature · 2 lines
Regards, Igor.
A message that is not in the archive (sent off-list or not yet received): [email protected]
PATCH 3/3 ARM: configs: cm_fx6: add mtd support
Hi Igor,
Quoted 28 lines · Igor Grinberg
Quoted 20 lines · Christopher Spinrath
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 <[email protected]>--- include/configs/cm_fx6.h | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)include/configs/cm_fx6.h +0
-0[...]
Quoted 4 lines
include/configs/cm_fx6.h +0
-0@@ -157,7 +174,7 @@"run setupnandboot;" \"run nandboot;"
Quoted 6 lines
include/configs/cm_fx6.h line 177
Quoted 2 lines
include/configs/cm_fx6.h +1
-1-#define CONFIG_PREBOOT "usb start" +#define CONFIG_PREBOOT "usb start;sf probe"Probably, this is really needed.
Care to explain?
The sf probe command probes for the spi flash and registers (on success) the device as nor0. This device is used by mtdparts (cf. the mtdids variable; it maps the U-Boot name nor0 to the kernel name spi0.0) and the mtd fixup code in patch 2 (cf. the nodes array; it specifies the compatible of the flash chip of type NOR #0, i.e. nor0).
Without this all mtdparts commands will fail and the fixup code won't work because there is nor0 device.
Cheers,
Christopher
Quoted 5 lines
Quoted 4 lines
include/configs/cm_fx6.h +0
-0/* SPI */ #define CONFIG_SPI
PATCH 3/3 ARM: configs: cm_fx6: add mtd support
Quoted 37 lines · Christopher Spinrath
Hi Igor,
Quoted 28 lines · Igor Grinberg
Quoted 20 lines · Christopher Spinrath
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 <[email protected]>--- include/configs/cm_fx6.h | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)include/configs/cm_fx6.h +0
-0[...]
Quoted 4 lines
include/configs/cm_fx6.h +0
-0@@ -157,7 +174,7 @@"run setupnandboot;" \"run nandboot;" Quoted 6 lines
include/configs/cm_fx6.h line 177
Quoted 2 lines
include/configs/cm_fx6.h +1
-1-#define CONFIG_PREBOOT "usb start" +#define CONFIG_PREBOOT "usb start;sf probe"Probably, this is really needed.
Care to explain?
Quoted 8 lines
The sf probe command probes for the spi flash and registers (on success) the device as nor0. This device is used by mtdparts (cf. the mtdids variable; it maps the U-Boot name nor0 to the kernel name spi0.0) and the mtd fixup code in patch 2 (cf. the nodes array; it specifies the compatible of the flash chip of type NOR #0, i.e. nor0).
Without this all mtdparts commands will fail and the fixup code won't work because there is nor0 device.
Thanks for the explanation!
That sounds to me like this should go away once we have the DM in place for spi flash and MTD (added Simon to Cc).
Meanwhile, may be a short notice in the commit message?
Signature · 2 lines
Regards, Igor.
A message that is not in the archive (sent off-list or not yet received): [email protected]
PATCH 3/3 ARM: configs: cm_fx6: add mtd support
Hi Igor,
Quoted 44 lines · Igor Grinberg
Quoted 43 lines · Christopher Spinrath
Hi Igor,
Quoted 28 lines · Igor Grinberg
Quoted 20 lines · Christopher Spinrath
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 <[email protected]>--- include/configs/cm_fx6.h | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)include/configs/cm_fx6.h +0
-0[...]
Quoted 4 lines
include/configs/cm_fx6.h +0
-0@@ -157,7 +174,7 @@"run setupnandboot;" \"run nandboot;" Quoted 6 lines
include/configs/cm_fx6.h line 177
Quoted 2 lines
include/configs/cm_fx6.h +1
-1-#define CONFIG_PREBOOT "usb start" +#define CONFIG_PREBOOT "usb start;sf probe"Probably, this is really needed.
Care to explain?The sf probe command probes for the spi flash and registers (on success) the device as nor0. This device is used by mtdparts (cf. the mtdids variable; it maps the U-Boot name nor0 to the kernel name spi0.0) and the mtd fixup code in patch 2 (cf. the nodes array; it specifies the compatible of the flash chip of type NOR #0, i.e. nor0).
Quoted 8 lines
Quoted 2 lines
Without this all mtdparts commands will fail and the fixup code won't work because there is nor0 device.
Thanks for the explanation!
That sounds to me like this should go away once we have the DM in place for spi flash and MTD (added Simon to Cc).
Meanwhile, may be a short notice in the commit message?
Sure, I will add it.
Thanks,
Christopher
cover ARM: imx: enhance support for the cm-fx6 module
In reply to Christopher SpinrathHi Christopher,
Thanks for doing this work.
Quoted 18 lines · Christopher Spinrath
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).
Quoted 1 line
IMHO defining the mtd partitioning has the following (dis-)advantages:
You mean defining it in U-Boot instead of upstream DT.
Quoted 4 lines
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.
Quoted 2 lines
- 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.
Quoted 4 lines
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.
Quoted 4 lines
- 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.
Quoted 3 lines
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.
Quoted 6 lines
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.
Quoted 13 lines
- 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
Signature · 2 lines
Regards, Igor.
A message that is not in the archive (sent off-list or not yet received): [email protected]
cover ARM: imx: enhance support for the cm-fx6 module
Hi Igor,
thanks for your review!
Quoted 23 lines · Igor Grinberg
Hi Christopher,
Thanks for doing this work.
Quoted 18 lines · Christopher Spinrath
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).
Quoted 4 lines
Quoted 1 line
IMHO defining the mtd partitioning has the following (dis-)advantages:
You mean defining it in U-Boot instead of upstream DT.
Yes.
Quoted 5 lines
Quoted 4 lines
Advantages:
- It is easier for the user to change the partitioning (e.g. to use the unsued 1MB free space).
Quoted 8 lines
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.
Quoted 7 lines
Quoted 2 lines
- 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.
Quoted 8 lines
Quoted 4 lines
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.
Quoted 7 lines
Quoted 4 lines
- 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.
Quoted 9 lines
Quoted 3 lines
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
Quoted 23 lines
Quoted 6 lines
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.
Quoted 13 lines
- 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