[U-Boot] [PATCH 1/3] spl: mmc: fix switch statement
If CONFIG_SPL_LIBCOMMON_SUPPORT is not defined there is a lone case statement at the end of the switch leading to a compile error. Remove the offending case statement.
| common/spl/spl_mmc.c:339:7: error: label at end of compound statement
Signed-off-by: Max Krummenacher max.krummenacher@toradex.com ---
common/spl/spl_mmc.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c index c674e61..367b4e4 100644 --- a/common/spl/spl_mmc.c +++ b/common/spl/spl_mmc.c @@ -342,7 +342,6 @@ static int spl_mmc_load_image(struct spl_image_info *spl_image, return err;
break; - case MMCSD_MODE_UNDEFINED: #ifdef CONFIG_SPL_LIBCOMMON_SUPPORT default: puts("spl: mmc: wrong boot mode\n");
The ipu has two display interfaces. Make the used one a parameter in struct display_info_t instead of using unconditionally DI0. DI0 is the default setting.
Signed-off-by: Max Krummenacher max.krummenacher@toradex.com ---
arch/arm/imx-common/video.c | 2 +- arch/arm/include/asm/imx-common/video.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/arm/imx-common/video.c b/arch/arm/imx-common/video.c index fdc987f..549bf9d 100644 --- a/arch/arm/imx-common/video.c +++ b/arch/arm/imx-common/video.c @@ -34,7 +34,7 @@ int board_video_skip(void) }
if (i < display_count) { - ret = ipuv3_fb_init(&displays[i].mode, 0, + ret = ipuv3_fb_init(&displays[i].mode, displays[i].di ? 1 : 0, displays[i].pixfmt); if (!ret) { if (displays[i].enable) diff --git a/arch/arm/include/asm/imx-common/video.h b/arch/arm/include/asm/imx-common/video.h index cad5f86..941a031 100644 --- a/arch/arm/include/asm/imx-common/video.h +++ b/arch/arm/include/asm/imx-common/video.h @@ -12,6 +12,7 @@ struct display_info_t { int bus; int addr; int pixfmt; + int di; int (*detect)(struct display_info_t const *dev); void (*enable)(struct display_info_t const *dev); struct fb_videomode mode;
Synchronize CONFIG_SPL_PAD_TO (used at build time for u-boot-with-spl.imx) with CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR (used by SPL to read u-boot image).
Signed-off-by: Max Krummenacher max.krummenacher@toradex.com ---
include/configs/imx6_spl.h | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/include/configs/imx6_spl.h b/include/configs/imx6_spl.h index 76d1ca0..0b426bb 100644 --- a/include/configs/imx6_spl.h +++ b/include/configs/imx6_spl.h @@ -23,6 +23,8 @@ * which consists of a 4K header in front of us that contains the IVT, DCD * and some padding thus 'our' max size is really 0x00908000 - 0x00918000 * or 64KB + * - Padding between start of SPL(with IVT...) and U-Boot is 68KB, SPL starts + * at 1KB, U-Boot at 69kB into the storage media. */ #define CONFIG_SYS_THUMB_BUILD #define CONFIG_SPL_LDSCRIPT "arch/arm/cpu/armv7/omap-common/u-boot-spl.lds" @@ -38,6 +40,7 @@ /* MMC support */ #if defined(CONFIG_SPL_MMC_SUPPORT) #define CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR 138 /* offset 69KB */ +#define CONFIG_SPL_PAD_TO 0x11000 /* offset 68KB */ #define CONFIG_SYS_U_BOOT_MAX_SIZE_SECTORS 800 /* 400 KB */ #define CONFIG_SYS_MMCSD_FS_BOOT_PARTITION 1 #define CONFIG_SYS_MONITOR_LEN (CONFIG_SYS_U_BOOT_MAX_SIZE_SECTORS/2*1024)
On 10/15/2016 07:10 PM, Max Krummenacher wrote:
Synchronize CONFIG_SPL_PAD_TO (used at build time for u-boot-with-spl.imx) with CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR (used by SPL to read u-boot image).
Signed-off-by: Max Krummenacher max.krummenacher@toradex.com
include/configs/imx6_spl.h | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/include/configs/imx6_spl.h b/include/configs/imx6_spl.h index 76d1ca0..0b426bb 100644 --- a/include/configs/imx6_spl.h +++ b/include/configs/imx6_spl.h @@ -23,6 +23,8 @@
- which consists of a 4K header in front of us that contains the IVT, DCD
- and some padding thus 'our' max size is really 0x00908000 - 0x00918000
- or 64KB
- Padding between start of SPL(with IVT...) and U-Boot is 68KB, SPL starts
*/
- at 1KB, U-Boot at 69kB into the storage media.
#define CONFIG_SYS_THUMB_BUILD #define CONFIG_SPL_LDSCRIPT "arch/arm/cpu/armv7/omap-common/u-boot-spl.lds" @@ -38,6 +40,7 @@ /* MMC support */ #if defined(CONFIG_SPL_MMC_SUPPORT) #define CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR 138 /* offset 69KB */ +#define CONFIG_SPL_PAD_TO 0x11000 /* offset 68KB */
Does this mess up boards which can boot from both SD and other boot media (NAND, SPI NOR, PNOR...) ?
#define CONFIG_SYS_U_BOOT_MAX_SIZE_SECTORS 800 /* 400 KB */ #define CONFIG_SYS_MMCSD_FS_BOOT_PARTITION 1 #define CONFIG_SYS_MONITOR_LEN (CONFIG_SYS_U_BOOT_MAX_SIZE_SECTORS/2*1024)
Hi Marak
--- a/include/configs/imx6_spl.h +++ b/include/configs/imx6_spl.h @@ -23,6 +23,8 @@
- which consists of a 4K header in front of us that contains
the IVT, DCD
- and some padding thus 'our' max size is really 0x00908000 -
0x00918000
- or 64KB
- Padding between start of SPL(with IVT...) and U-Boot is
68KB, SPL starts
*/
- at 1KB, U-Boot at 69kB into the storage media.
#define CONFIG_SYS_THUMB_BUILD #define CONFIG_SPL_LDSCRIPT "arch/arm/cpu/armv7/omap -common/u-boot-spl.lds" @@ -38,6 +40,7 @@ /* MMC support */ #if defined(CONFIG_SPL_MMC_SUPPORT) #define CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR 138 /* offset 69KB */ +#define CONFIG_SPL_PAD_TO 0x11000 /* offset 68KB */
Does this mess up boards which can boot from both SD and other boot media (NAND, SPI NOR, PNOR...) ?
Good point. CONFIG_SPL_PAD_TO is used to create the combined SPL/U-Boot binary. While I have not found any use of this by any config which includes imx6_spl.h it might hinder future boards. So it's probably best to define CONFIG_SPL_PAD_TO in the individual board configs and skip this patch.
Max
On 10/15/2016 09:17 PM, Max Krummenacher wrote:
Hi Marak
--- a/include/configs/imx6_spl.h +++ b/include/configs/imx6_spl.h @@ -23,6 +23,8 @@
- which consists of a 4K header in front of us that contains
the IVT, DCD
- and some padding thus 'our' max size is really 0x00908000 -
0x00918000
- or 64KB
- Padding between start of SPL(with IVT...) and U-Boot is
68KB, SPL starts
*/
- at 1KB, U-Boot at 69kB into the storage media.
#define CONFIG_SYS_THUMB_BUILD #define CONFIG_SPL_LDSCRIPT "arch/arm/cpu/armv7/omap -common/u-boot-spl.lds" @@ -38,6 +40,7 @@ /* MMC support */ #if defined(CONFIG_SPL_MMC_SUPPORT) #define CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR 138 /* offset 69KB */ +#define CONFIG_SPL_PAD_TO 0x11000 /* offset 68KB */
Does this mess up boards which can boot from both SD and other boot media (NAND, SPI NOR, PNOR...) ?
Good point. CONFIG_SPL_PAD_TO is used to create the combined SPL/U-Boot binary. While I have not found any use of this by any config which includes imx6_spl.h it might hinder future boards. So it's probably best to define CONFIG_SPL_PAD_TO in the individual board configs and skip this patch.
So how did you come up with this patch ? What was the thought process?
Hi Marek
#if defined(CONFIG_SPL_MMC_SUPPORT) #define CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR 138 /* offset 69KB */ +#define CONFIG_SPL_PAD_TO 0x11000 /* offset 68KB */
Does this mess up boards which can boot from both SD and other boot media (NAND, SPI NOR, PNOR...) ?
Good point. CONFIG_SPL_PAD_TO is used to create the combined SPL/U-Boot binary. While I have not found any use of this by any config which includes imx6_spl.h it might hinder future boards. So it's probably best to define CONFIG_SPL_PAD_TO in the individual board configs and skip this patch.
So how did you come up with this patch ? What was the thought process?
I'm preparing patches for submission of new boards, Colibri iMX6, Apalis iMX6. They rely on this patchset https://www.mail-archive.com/u-boot@lists.denx.de/msg227944.html, so the board patches are not yet ready for submission.
I forward ported from a downstream 2015.04 U-Boot where 'make u-boot-with-spl.imx' used an offset of 68KB between SPL and U-Boot binary matching the SPL code which later loads U-Boot. With the current master it used an offset of 64KB resulting in an SPL not loading U-Boot. Probably the 4KB stem from now counting SPL_PAD_TO from the start of the IVT/DCD region and earlier counting from the start of SPL.
Max
On 10/17/2016 02:28 PM, Max Krummenacher wrote:
Hi Marek
#if defined(CONFIG_SPL_MMC_SUPPORT) #define CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR 138 /* offset 69KB */ +#define CONFIG_SPL_PAD_TO 0x11000 /* offset 68KB */
Does this mess up boards which can boot from both SD and other boot media (NAND, SPI NOR, PNOR...) ?
Good point. CONFIG_SPL_PAD_TO is used to create the combined SPL/U-Boot binary. While I have not found any use of this by any config which includes imx6_spl.h it might hinder future boards. So it's probably best to define CONFIG_SPL_PAD_TO in the individual board configs and skip this patch.
So how did you come up with this patch ? What was the thought process?
I'm preparing patches for submission of new boards, Colibri iMX6, Apalis iMX6. They rely on this patchset https://www.mail-archive.com/u-boot@lists.denx.de/msg227944.html, so the board patches are not yet ready for submission.
OT: Can you make those boards load u-boot.img from extfs partition instead ?
I forward ported from a downstream 2015.04 U-Boot where 'make u-boot-with-spl.imx' used an offset of 68KB between SPL and U-Boot binary matching the SPL code which later loads U-Boot. With the current master it used an offset of 64KB resulting in an SPL not loading U-Boot. Probably the 4KB stem from now counting SPL_PAD_TO from the start of the IVT/DCD region and earlier counting from the start of SPL.
I see, thanks for clarifying.
Max
2016-10-17 14:45 GMT+02:00 Marek Vasut marex@denx.de:
On 10/17/2016 02:28 PM, Max Krummenacher wrote:
Hi Marek
#if defined(CONFIG_SPL_MMC_SUPPORT) #define CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR 138 /* offset 69KB */ +#define CONFIG_SPL_PAD_TO 0x11000 /* offset 68KB */
Does this mess up boards which can boot from both SD and other boot media (NAND, SPI NOR, PNOR...) ?
Good point. CONFIG_SPL_PAD_TO is used to create the combined SPL/U-Boot binary. While I have not found any use of this by any config which includes imx6_spl.h it might hinder future boards. So it's probably best to define CONFIG_SPL_PAD_TO in the individual board configs and skip this patch.
So how did you come up with this patch ? What was the thought process?
I'm preparing patches for submission of new boards, Colibri iMX6, Apalis iMX6. They rely on this patchset https://www.mail-archive.com/u-boot@lists.denx.de/msg227944.html, so the board patches are not yet ready for submission.
OT: Can you make those boards load u-boot.img from extfs partition instead ?
I probably could but did not try.
Also it would change the way we use U-Boot across our module family and I'm worried that we more often need to recover over USB due to a corrupted filesystem. On top of that I made some measurements on loading the kernel from an ext2 instead of an fat fs and have seen differences bigger than 100ms. So parsing into a filesystem twice would probably add even more overhead.
Max
I forward ported from a downstream 2015.04 U-Boot where 'make u-boot-with-spl.imx' used an offset of 68KB between SPL and U-Boot binary matching the SPL code which later loads U-Boot. With the current master it used an offset of 64KB resulting in an SPL not loading U-Boot. Probably the 4KB stem from now counting SPL_PAD_TO from the start of the IVT/DCD region and earlier counting from the start of SPL.
I see, thanks for clarifying.
Max
-- Best regards, Marek Vasut
On 10/15/2016 07:10 PM, Max Krummenacher wrote:
If CONFIG_SPL_LIBCOMMON_SUPPORT is not defined there is a lone case statement at the end of the switch leading to a compile error. Remove the offending case statement.
| common/spl/spl_mmc.c:339:7: error: label at end of compound statement
Signed-off-by: Max Krummenacher max.krummenacher@toradex.com
common/spl/spl_mmc.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c index c674e61..367b4e4 100644 --- a/common/spl/spl_mmc.c +++ b/common/spl/spl_mmc.c @@ -342,7 +342,6 @@ static int spl_mmc_load_image(struct spl_image_info *spl_image, return err;
break;
- case MMCSD_MODE_UNDEFINED:
This patch is wrong -- in case CONFIG_SPL_LIBCOMMON_SUPPORT is enabled and mode is MMCSD_MODE_UNDEFINED, the message in the puts() below would be printed. After applying this change, the message won't be printed .
The fix is probably something like:
case foo: default: #ifdef CONFIG_BAR puts(); #endif break;
#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT default: puts("spl: mmc: wrong boot mode\n");
Hi Marek
Am Samstag, den 15.10.2016, 19:29 +0200 schrieb Marek Vasut:
On 10/15/2016 07:10 PM, Max Krummenacher wrote:
If CONFIG_SPL_LIBCOMMON_SUPPORT is not defined there is a lone case statement at the end of the switch leading to a compile error. Remove the offending case statement.
common/spl/spl_mmc.c:339:7: error: label at end of compound statement
Signed-off-by: Max Krummenacher max.krummenacher@toradex.com
common/spl/spl_mmc.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c index c674e61..367b4e4 100644 --- a/common/spl/spl_mmc.c +++ b/common/spl/spl_mmc.c @@ -342,7 +342,6 @@ static int spl_mmc_load_image(struct spl_image_info *spl_image, return err;
break;
- case MMCSD_MODE_UNDEFINED:
This patch is wrong -- in case CONFIG_SPL_LIBCOMMON_SUPPORT is enabled and mode is MMCSD_MODE_UNDEFINED, the message in the puts() below would be printed. After applying this change, the message won't be printed
I disagree.
With CONFIG_SPL_LIBCOMMON_SUPPORT we had something like this: switch(bar) { ... case foo: default: put("bla\n"); }
as 'case foo:' falls through into 'default:' removing the specific case does not change anything.
Regards Max
The fix is probably something like:
case foo: default: #ifdef CONFIG_BAR puts(); #endif break;
#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT default: puts("spl: mmc: wrong boot mode\n");
On 10/15/2016 09:18 PM, Max Krummenacher wrote:
Hi Marek
Am Samstag, den 15.10.2016, 19:29 +0200 schrieb Marek Vasut:
On 10/15/2016 07:10 PM, Max Krummenacher wrote:
If CONFIG_SPL_LIBCOMMON_SUPPORT is not defined there is a lone case statement at the end of the switch leading to a compile error. Remove the offending case statement.
common/spl/spl_mmc.c:339:7: error: label at end of compound statement
Signed-off-by: Max Krummenacher max.krummenacher@toradex.com
common/spl/spl_mmc.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c index c674e61..367b4e4 100644 --- a/common/spl/spl_mmc.c +++ b/common/spl/spl_mmc.c @@ -342,7 +342,6 @@ static int spl_mmc_load_image(struct spl_image_info *spl_image, return err;
break;
- case MMCSD_MODE_UNDEFINED:
This patch is wrong -- in case CONFIG_SPL_LIBCOMMON_SUPPORT is enabled and mode is MMCSD_MODE_UNDEFINED, the message in the puts() below would be printed. After applying this change, the message won't be printed
I disagree.
With CONFIG_SPL_LIBCOMMON_SUPPORT we had something like this: switch(bar) { ... case foo: default: put("bla\n"); }
as 'case foo:' falls through into 'default:' removing the specific case does not change anything.
Oh, that's a good point, I missed that and that's in fact pretty elegant fix. Thanks.
Acked-by: Marek Vasut marex@denx.de
[...]
On Sat, Oct 15, 2016 at 07:10:14PM +0200, Max Krummenacher wrote:
If CONFIG_SPL_LIBCOMMON_SUPPORT is not defined there is a lone case statement at the end of the switch leading to a compile error. Remove the offending case statement.
| common/spl/spl_mmc.c:339:7: error: label at end of compound statement
Signed-off-by: Max Krummenacher max.krummenacher@toradex.com
Reviewed-by: Tom Rini trini@konsulko.com
participants (3)
-
Marek Vasut -
Max Krummenacher -
Tom Rini