[U-Boot] [PATCH 00/11] NAND/MMC environment in SPL, Falcon Mode enhancements
Hey all,
This series fixes a few bugs / issues, and then sets things up so that we can use MMC (or NAND) for environment in SPL, in addition to NOR flash or simply built-in. The end result is that Falcon Mode becomes much more configurable via the environment, and thus from within Linux.
Currently our limit is too small to allow for /dev/mmcblk0boot0 to work, for example. Expand to 32 for future needs.
Signed-off-by: Tom Rini trini@ti.com --- tools/env/fw_env.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c index 577ce2d..bf5c552 100644 --- a/tools/env/fw_env.c +++ b/tools/env/fw_env.c @@ -40,7 +40,7 @@ _min1 < _min2 ? _min1 : _min2; })
struct envdev_s { - char devname[16]; /* Device name */ + char devname[32]; /* Device name */ ulong devoff; /* Device offset */ ulong env_size; /* environment size */ ulong erase_size; /* device erase size */
Dear Tom Rini,
In message 1380227287-26057-2-git-send-email-trini@ti.com you wrote:
Currently our limit is too small to allow for /dev/mmcblk0boot0 to work, for example. Expand to 32 for future needs.
Signed-off-by: Tom Rini trini@ti.com
tools/env/fw_env.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c index 577ce2d..bf5c552 100644 --- a/tools/env/fw_env.c +++ b/tools/env/fw_env.c @@ -40,7 +40,7 @@ _min1 < _min2 ? _min1 : _min2; })
struct envdev_s {
- char devname[16]; /* Device name */
- char devname[32]; /* Device name */
Do we really need a static size here? Can we not auto-adjust to the needed size, say by dynamically allocating the buffer?
Best regards,
Wolfgang Denk
On Sat, Oct 05, 2013 at 10:02:12PM +0200, Wolfgang Denk wrote:
Dear Tom Rini,
In message 1380227287-26057-2-git-send-email-trini@ti.com you wrote:
Currently our limit is too small to allow for /dev/mmcblk0boot0 to work, for example. Expand to 32 for future needs.
Signed-off-by: Tom Rini trini@ti.com
tools/env/fw_env.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c index 577ce2d..bf5c552 100644 --- a/tools/env/fw_env.c +++ b/tools/env/fw_env.c @@ -40,7 +40,7 @@ _min1 < _min2 ? _min1 : _min2; })
struct envdev_s {
- char devname[16]; /* Device name */
- char devname[32]; /* Device name */
Do we really need a static size here? Can we not auto-adjust to the needed size, say by dynamically allocating the buffer?
Doesn't look like it, without a big change to the parsing code.
Dear Tom,
In message 20131006205527.GP15917@bill-the-cat you wrote:
Do we really need a static size here? Can we not auto-adjust to the needed size, say by dynamically allocating the buffer?
Doesn't look like it, without a big change to the parsing code.
I don't think this requires a big change. Eventually all it takes is changing the sscanf() call in get_config() to use a format "%ms" instead of plain "%s"; form the sscanf() man page:
· An optional 'm' character. This is used with string conversions (%s, %c, %[), and relieves the caller of the need to allocate a corresponding buffer to hold the input: instead, scanf() allocates a buffer of sufficient size, and assigns the address of this buffer to the corresponding pointer argument, which should be a pointer to a char * variable (this variable does not need to be initialized before the call). The caller should subsequently free(3) this buffer when it is no longer required.
OK, the struct should then of course contain a const char pointer instead of the buffer itself, but that's also a trivial change.
Best regards,
Wolfgang Denk
On Mon, Oct 07, 2013 at 07:47:13AM +0200, Wolfgang Denk wrote:
Dear Tom,
In message 20131006205527.GP15917@bill-the-cat you wrote:
Do we really need a static size here? Can we not auto-adjust to the needed size, say by dynamically allocating the buffer?
Doesn't look like it, without a big change to the parsing code.
I don't think this requires a big change. Eventually all it takes is changing the sscanf() call in get_config() to use a format "%ms" instead of plain "%s"; form the sscanf() man page:
?? An optional 'm' character. This is used with string conversions (%s, %c, %[), and relieves the caller of the need to allocate a corresponding buffer to hold the input: instead, scanf() allocates a buffer of sufficient size, and assigns the address of this buffer to the corresponding pointer argument, which should be a pointer to a char * variable (this variable does not need to be initialized before the call). The caller should subsequently free(3) this buffer when it is no longer required.OK, the struct should then of course contain a const char pointer instead of the buffer itself, but that's also a trivial change.
Well, that's what I get for looking at the code, and not checking man pages. Agreed, I'll re-work this part.
Switch the case of non-redundant non-embedded environment to use malloc to allocate buffers, rather than place them on the stack, like the redundant case does.
Signed-off-by: Tom Rini trini@ti.com --- common/env_mmc.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/common/env_mmc.c b/common/env_mmc.c index 65aafa9..0ab11d3 100644 --- a/common/env_mmc.c +++ b/common/env_mmc.c @@ -274,11 +274,18 @@ err: void env_relocate_spec(void) { #if !defined(ENV_IS_EMBEDDED) - ALLOC_CACHE_ALIGN_BUFFER(char, buf, CONFIG_ENV_SIZE); struct mmc *mmc = find_mmc_device(CONFIG_SYS_MMC_ENV_DEV); + env_t *ep; u32 offset; int ret;
+ ep = (env_t *)malloc(CONFIG_ENV_SIZE); + if (ep == NULL) { + puts("Can't allocate buffers for environment\n"); + ret = 1; + goto err; + } + if (init_mmc_for_env(mmc)) { ret = 1; goto err; @@ -289,12 +296,12 @@ void env_relocate_spec(void) goto fini; }
- if (read_env(mmc, CONFIG_ENV_SIZE, offset, buf)) { + if (read_env(mmc, CONFIG_ENV_SIZE, offset, ep)) { ret = 1; goto fini; }
- env_import(buf, 1); + env_import((char *)ep, 1); ret = 0;
fini: @@ -302,6 +309,8 @@ fini: err: if (ret) set_default_env(NULL); + + free(ep); #endif } #endif /* CONFIG_ENV_OFFSET_REDUND */
Dear Tom Rini,
In message 1380227287-26057-3-git-send-email-trini@ti.com you wrote:
Switch the case of non-redundant non-embedded environment to use malloc to allocate buffers, rather than place them on the stack, like the redundant case does.
What exactly would be the benefit of this change? It just adds code size and execution time and makes the code more complex, without any appearent advanteages?
Best regards,
Wolfgang Denk
On Sat, Oct 05, 2013 at 10:00:13PM +0200, Wolfgang Denk wrote:
Dear Tom Rini,
In message 1380227287-26057-3-git-send-email-trini@ti.com you wrote:
Switch the case of non-redundant non-embedded environment to use malloc to allocate buffers, rather than place them on the stack, like the redundant case does.
What exactly would be the benefit of this change? It just adds code size and execution time and makes the code more complex, without any appearent advanteages?
The main advantage is that we can use this code in environments with less than CONFIG_ENV_SIZE worth of stack available. Arguably it makes the behaviour and code paths similar for redundant and non-redundant cases (but someone posted a patch to make the redundant case use the stack).
Dear Tom,
In message 20131006204046.GN15917@bill-the-cat you wrote:
In message 1380227287-26057-3-git-send-email-trini@ti.com you wrote:
Switch the case of non-redundant non-embedded environment to use malloc to allocate buffers, rather than place them on the stack, like the redundant case does.
What exactly would be the benefit of this change? It just adds code size and execution time and makes the code more complex, without any appearent advanteages?
The main advantage is that we can use this code in environments with less than CONFIG_ENV_SIZE worth of stack available. Arguably it makes the behaviour and code paths similar for redundant and non-redundant cases (but someone posted a patch to make the redundant case use the stack).
Instead of stack size you pay for that with additional size of the malloc() arena. As I just explained in the other mail [1], this is even worse to deal with and will result in less memoy available for other purposes.
And yes, we definitely should use the stack for the redundant case as well. Do you want me to submit such a patch?
[1] http://article.gmane.org/gmane.comp.boot-loaders.u-boot/171217
Best regards,
Wolfgang Denk
Inside of SPL we only concern ourself with one MMC device, so instead of being able to use CONFIG_SYS_MMC_ENV_DEV we need to use 0 in SPL. Switch the code to use a 'dev' variable to facilitate this.
Signed-off-by: Tom Rini trini@ti.com --- common/env_mmc.c | 37 ++++++++++++++++++++++++++++++------- 1 file changed, 30 insertions(+), 7 deletions(-)
diff --git a/common/env_mmc.c b/common/env_mmc.c index 0ab11d3..ad3ff45 100644 --- a/common/env_mmc.c +++ b/common/env_mmc.c @@ -64,6 +64,13 @@ int env_init(void)
static int init_mmc_for_env(struct mmc *mmc) { +#ifdef CONFIG_SYS_MMC_ENV_PART + int dev = CONFIG_SYS_MMC_ENV_DEV; +#ifdef CONFIG_SPL_BUILD + dev = 0; +#endif +#endif + if (!mmc) { puts("No MMC card found\n"); return -1; @@ -76,8 +83,7 @@ static int init_mmc_for_env(struct mmc *mmc)
#ifdef CONFIG_SYS_MMC_ENV_PART if (CONFIG_SYS_MMC_ENV_PART != mmc->part_num) { - if (mmc_switch_part(CONFIG_SYS_MMC_ENV_DEV, - CONFIG_SYS_MMC_ENV_PART)) { + if (mmc_switch_part(dev, CONFIG_SYS_MMC_ENV_PART)) { puts("MMC partition switch failed\n"); return -1; } @@ -90,9 +96,12 @@ static int init_mmc_for_env(struct mmc *mmc) static void fini_mmc_for_env(struct mmc *mmc) { #ifdef CONFIG_SYS_MMC_ENV_PART + int dev = CONFIG_SYS_MMC_ENV_DEV; +#ifdef CONFIG_SPL_BUILD + dev = 0; +#endif if (CONFIG_SYS_MMC_ENV_PART != mmc->part_num) - mmc_switch_part(CONFIG_SYS_MMC_ENV_DEV, - mmc->part_num); + mmc_switch_part(dev, mmc->part_num); #endif }
@@ -174,11 +183,15 @@ static inline int read_env(struct mmc *mmc, unsigned long size, unsigned long offset, const void *buffer) { uint blk_start, blk_cnt, n; + int dev = CONFIG_SYS_MMC_ENV_DEV; +#ifdef CONFIG_SPL_BUILD + dev = 0; +#endif
blk_start = ALIGN(offset, mmc->read_bl_len) / mmc->read_bl_len; blk_cnt = ALIGN(size, mmc->read_bl_len) / mmc->read_bl_len;
- n = mmc->block_dev.block_read(CONFIG_SYS_MMC_ENV_DEV, blk_start, + n = mmc->block_dev.block_read(dev, blk_start, blk_cnt, (uchar *)buffer);
return (n == blk_cnt) ? 0 : -1; @@ -188,13 +201,18 @@ static inline int read_env(struct mmc *mmc, unsigned long size, void env_relocate_spec(void) { #if !defined(ENV_IS_EMBEDDED) - struct mmc *mmc = find_mmc_device(CONFIG_SYS_MMC_ENV_DEV); + struct mmc *mmc; u32 offset1, offset2; int read1_fail = 0, read2_fail = 0; int crc1_ok = 0, crc2_ok = 0; env_t *ep, *tmp_env1, *tmp_env2; int ret; + int dev = CONFIG_SYS_MMC_ENV_DEV; +#ifdef CONFIG_SPL_BUILD + dev = 0; +#endif
+ mmc = find_mmc_device(dev); tmp_env1 = (env_t *)malloc(CONFIG_ENV_SIZE); tmp_env2 = (env_t *)malloc(CONFIG_ENV_SIZE); if (tmp_env1 == NULL || tmp_env2 == NULL) { @@ -274,11 +292,16 @@ err: void env_relocate_spec(void) { #if !defined(ENV_IS_EMBEDDED) - struct mmc *mmc = find_mmc_device(CONFIG_SYS_MMC_ENV_DEV); + struct mmc *mmc; env_t *ep; u32 offset; int ret; + int dev = CONFIG_SYS_MMC_ENV_DEV; +#ifdef CONFIG_SPL_BUILD + dev = 0; +#endif
+ mmc = find_mmc_device(dev); ep = (env_t *)malloc(CONFIG_ENV_SIZE); if (ep == NULL) { puts("Can't allocate buffers for environment\n");
This mainly converts the am335x_spl_bch driver to the "normal" format which means a slight change to nand_info within the driver.
Cc: Scott Wood scottwood@freescale.com Signed-off-by: Tom Rini trini@ti.com --- drivers/mtd/nand/am335x_spl_bch.c | 54 ++++++++++++++++++------------------- include/configs/ti_armv7_common.h | 1 + spl/Makefile | 1 + 3 files changed, 29 insertions(+), 27 deletions(-)
diff --git a/drivers/mtd/nand/am335x_spl_bch.c b/drivers/mtd/nand/am335x_spl_bch.c index c84851b..bd89b06 100644 --- a/drivers/mtd/nand/am335x_spl_bch.c +++ b/drivers/mtd/nand/am335x_spl_bch.c @@ -16,7 +16,7 @@ #include <linux/mtd/nand_ecc.h>
static int nand_ecc_pos[] = CONFIG_SYS_NAND_ECCPOS; -static nand_info_t mtd; +nand_info_t nand_info[1]; static struct nand_chip nand_chip;
#define ECCSTEPS (CONFIG_SYS_NAND_PAGE_SIZE / \ @@ -30,12 +30,12 @@ static struct nand_chip nand_chip; static int nand_command(int block, int page, uint32_t offs, u8 cmd) { - struct nand_chip *this = mtd.priv; + struct nand_chip *this = nand_info[0].priv; int page_addr = page + block * CONFIG_SYS_NAND_PAGE_COUNT; void (*hwctrl)(struct mtd_info *mtd, int cmd, unsigned int ctrl) = this->cmd_ctrl;
- while (!this->dev_ready(&mtd)) + while (!this->dev_ready(&nand_info[0])) ;
/* Emulate NAND_CMD_READOOB */ @@ -45,11 +45,11 @@ static int nand_command(int block, int page, uint32_t offs, }
/* Begin command latch cycle */ - hwctrl(&mtd, cmd, NAND_CTRL_CLE | NAND_CTRL_CHANGE); + hwctrl(&nand_info[0], cmd, NAND_CTRL_CLE | NAND_CTRL_CHANGE);
if (cmd == NAND_CMD_RESET) { - hwctrl(&mtd, NAND_CMD_NONE, NAND_NCE | NAND_CTRL_CHANGE); - while (!this->dev_ready(&mtd)) + hwctrl(&nand_info[0], NAND_CMD_NONE, NAND_NCE | NAND_CTRL_CHANGE); + while (!this->dev_ready(&nand_info[0])) ; return 0; } @@ -60,35 +60,35 @@ static int nand_command(int block, int page, uint32_t offs,
/* Set ALE and clear CLE to start address cycle */ /* Column address */ - hwctrl(&mtd, offs & 0xff, + hwctrl(&nand_info[0], offs & 0xff, NAND_CTRL_ALE | NAND_CTRL_CHANGE); /* A[7:0] */ - hwctrl(&mtd, (offs >> 8) & 0xff, NAND_CTRL_ALE); /* A[11:9] */ + hwctrl(&nand_info[0], (offs >> 8) & 0xff, NAND_CTRL_ALE); /* A[11:9] */ /* Row address */ - hwctrl(&mtd, (page_addr & 0xff), NAND_CTRL_ALE); /* A[19:12] */ - hwctrl(&mtd, ((page_addr >> 8) & 0xff), + hwctrl(&nand_info[0], (page_addr & 0xff), NAND_CTRL_ALE); /* A[19:12] */ + hwctrl(&nand_info[0], ((page_addr >> 8) & 0xff), NAND_CTRL_ALE); /* A[27:20] */ #ifdef CONFIG_SYS_NAND_5_ADDR_CYCLE /* One more address cycle for devices > 128MiB */ - hwctrl(&mtd, (page_addr >> 16) & 0x0f, + hwctrl(&nand_info[0], (page_addr >> 16) & 0x0f, NAND_CTRL_ALE); /* A[31:28] */ #endif - hwctrl(&mtd, NAND_CMD_NONE, NAND_NCE | NAND_CTRL_CHANGE); + hwctrl(&nand_info[0], NAND_CMD_NONE, NAND_NCE | NAND_CTRL_CHANGE);
if (cmd == NAND_CMD_READ0) { /* Latch in address */ - hwctrl(&mtd, NAND_CMD_READSTART, + hwctrl(&nand_info[0], NAND_CMD_READSTART, NAND_CTRL_CLE | NAND_CTRL_CHANGE); - hwctrl(&mtd, NAND_CMD_NONE, NAND_NCE | NAND_CTRL_CHANGE); + hwctrl(&nand_info[0], NAND_CMD_NONE, NAND_NCE | NAND_CTRL_CHANGE);
/* * Wait a while for the data to be ready */ - while (!this->dev_ready(&mtd)) + while (!this->dev_ready(&nand_info[0])) ; } else if (cmd == NAND_CMD_RNDOUT) { - hwctrl(&mtd, NAND_CMD_RNDOUTSTART, NAND_CTRL_CLE | + hwctrl(&nand_info[0], NAND_CMD_RNDOUTSTART, NAND_CTRL_CLE | NAND_CTRL_CHANGE); - hwctrl(&mtd, NAND_CMD_NONE, NAND_NCE | NAND_CTRL_CHANGE); + hwctrl(&nand_info[0], NAND_CMD_NONE, NAND_NCE | NAND_CTRL_CHANGE); }
return 0; @@ -96,7 +96,7 @@ static int nand_command(int block, int page, uint32_t offs,
static int nand_is_bad_block(int block) { - struct nand_chip *this = mtd.priv; + struct nand_chip *this = nand_info[0].priv;
nand_command(block, 0, CONFIG_SYS_NAND_BAD_BLOCK_POS, NAND_CMD_READOOB); @@ -117,7 +117,7 @@ static int nand_is_bad_block(int block)
static int nand_read_page(int block, int page, void *dst) { - struct nand_chip *this = mtd.priv; + struct nand_chip *this = nand_info[0].priv; u_char ecc_calc[ECCTOTAL]; u_char ecc_code[ECCTOTAL]; u_char oob_data[CONFIG_SYS_NAND_OOBSIZE]; @@ -133,15 +133,15 @@ static int nand_read_page(int block, int page, void *dst) nand_command(block, page, 0, NAND_CMD_READ0);
for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize) { - this->ecc.hwctl(&mtd, NAND_ECC_READ); + this->ecc.hwctl(&nand_info[0], NAND_ECC_READ); nand_command(block, page, data_pos, NAND_CMD_RNDOUT);
- this->read_buf(&mtd, p, eccsize); + this->read_buf(&nand_info[0], p, eccsize);
nand_command(block, page, oob_pos, NAND_CMD_RNDOUT);
- this->read_buf(&mtd, oob, eccbytes); - this->ecc.calculate(&mtd, p, &ecc_calc[i]); + this->read_buf(&nand_info[0], oob, eccbytes); + this->ecc.calculate(&nand_info[0], p, &ecc_calc[i]);
data_pos += eccsize; oob_pos += eccbytes; @@ -160,7 +160,7 @@ static int nand_read_page(int block, int page, void *dst) * from correct_data(). We just hope that all possible errors * are corrected by this routine. */ - this->ecc.correct(&mtd, p, &ecc_code[i], &ecc_calc[i]); + this->ecc.correct(&nand_info[0], p, &ecc_code[i], &ecc_calc[i]); }
return 0; @@ -206,13 +206,13 @@ void nand_init(void) /* * Init board specific nand support */ - mtd.priv = &nand_chip; + nand_info[0].priv = &nand_chip; nand_chip.IO_ADDR_R = nand_chip.IO_ADDR_W = (void __iomem *)CONFIG_SYS_NAND_BASE; board_nand_init(&nand_chip);
if (nand_chip.select_chip) - nand_chip.select_chip(&mtd, 0); + nand_chip.select_chip(&nand_info[0], 0);
/* NAND chip may require reset after power-on */ nand_command(0, 0, 0, NAND_CMD_RESET); @@ -222,5 +222,5 @@ void nand_init(void) void nand_deselect(void) { if (nand_chip.select_chip) - nand_chip.select_chip(&mtd, -1); + nand_chip.select_chip(&nand_info[0], -1); } diff --git a/include/configs/ti_armv7_common.h b/include/configs/ti_armv7_common.h index e89e874..32110f6 100644 --- a/include/configs/ti_armv7_common.h +++ b/include/configs/ti_armv7_common.h @@ -244,6 +244,7 @@ #define CONFIG_SPL_NAND_BASE #define CONFIG_SPL_NAND_DRIVERS #define CONFIG_SPL_NAND_ECC +#define CONFIG_SPL_MTD_SUPPORT #define CONFIG_SYS_NAND_U_BOOT_START CONFIG_SYS_TEXT_BASE #define CONFIG_SYS_NAND_U_BOOT_OFFS 0x80000 #endif diff --git a/spl/Makefile b/spl/Makefile index 174d0a7..fa642ec 100644 --- a/spl/Makefile +++ b/spl/Makefile @@ -89,6 +89,7 @@ LIBS-$(CONFIG_SPL_FAT_SUPPORT) += fs/fat/libfat.o LIBS-$(CONFIG_SPL_LIBGENERIC_SUPPORT) += lib/libgeneric.o LIBS-$(CONFIG_SPL_POWER_SUPPORT) += drivers/power/libpower.o LIBS-$(CONFIG_SPL_NAND_SUPPORT) += drivers/mtd/nand/libnand.o +LIBS-$(CONFIG_SPL_MTD_SUPPORT) += drivers/mtd/libmtd.o LIBS-$(CONFIG_SPL_ONENAND_SUPPORT) += drivers/mtd/onenand/libonenand.o LIBS-$(CONFIG_SPL_DMA_SUPPORT) += drivers/dma/libdma.o LIBS-$(CONFIG_SPL_POST_MEM_SUPPORT) += post/drivers/memory.o
On Thu, 2013-09-26 at 16:28 -0400, Tom Rini wrote:
This mainly converts the am335x_spl_bch driver to the "normal" format which means a slight change to nand_info within the driver.
Cc: Scott Wood scottwood@freescale.com Signed-off-by: Tom Rini trini@ti.com
drivers/mtd/nand/am335x_spl_bch.c | 54 ++++++++++++++++++------------------- include/configs/ti_armv7_common.h | 1 + spl/Makefile | 1 + 3 files changed, 29 insertions(+), 27 deletions(-)
Acked-by: Scott Wood scottwood@freescale.com
with one caveat:
diff --git a/include/configs/ti_armv7_common.h b/include/configs/ti_armv7_common.h index e89e874..32110f6 100644 --- a/include/configs/ti_armv7_common.h +++ b/include/configs/ti_armv7_common.h @@ -244,6 +244,7 @@ #define CONFIG_SPL_NAND_BASE #define CONFIG_SPL_NAND_DRIVERS #define CONFIG_SPL_NAND_ECC +#define CONFIG_SPL_MTD_SUPPORT #define CONFIG_SYS_NAND_U_BOOT_START CONFIG_SYS_TEXT_BASE #define CONFIG_SYS_NAND_U_BOOT_OFFS 0x80000 #endif diff --git a/spl/Makefile b/spl/Makefile index 174d0a7..fa642ec 100644 --- a/spl/Makefile +++ b/spl/Makefile @@ -89,6 +89,7 @@ LIBS-$(CONFIG_SPL_FAT_SUPPORT) += fs/fat/libfat.o LIBS-$(CONFIG_SPL_LIBGENERIC_SUPPORT) += lib/libgeneric.o LIBS-$(CONFIG_SPL_POWER_SUPPORT) += drivers/power/libpower.o LIBS-$(CONFIG_SPL_NAND_SUPPORT) += drivers/mtd/nand/libnand.o +LIBS-$(CONFIG_SPL_MTD_SUPPORT) += drivers/mtd/libmtd.o LIBS-$(CONFIG_SPL_ONENAND_SUPPORT) += drivers/mtd/onenand/libonenand.o LIBS-$(CONFIG_SPL_DMA_SUPPORT) += drivers/dma/libdma.o LIBS-$(CONFIG_SPL_POST_MEM_SUPPORT) += post/drivers/memory.o
This needs to be documented in the README.
-Scott
Cc: Scott Wood scottwood@freescale.com Signed-off-by: Tom Rini trini@ti.com --- drivers/mtd/nand/Makefile | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile index 366dee6..bb5b29d 100644 --- a/drivers/mtd/nand/Makefile +++ b/drivers/mtd/nand/Makefile @@ -24,6 +24,7 @@ COBJS-$(CONFIG_SPL_NAND_LOAD) += nand_spl_load.o COBJS-$(CONFIG_SPL_NAND_ECC) += nand_ecc.o COBJS-$(CONFIG_SPL_NAND_BASE) += nand_base.o COBJS-$(CONFIG_SPL_NAND_INIT) += nand.o +COBJS-$(CONFIG_ENV_IS_IN_NAND) += nand_util.o
else # not spl
On Thu, 2013-09-26 at 16:28 -0400, Tom Rini wrote:
Cc: Scott Wood scottwood@freescale.com Signed-off-by: Tom Rini trini@ti.com
drivers/mtd/nand/Makefile | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile index 366dee6..bb5b29d 100644 --- a/drivers/mtd/nand/Makefile +++ b/drivers/mtd/nand/Makefile @@ -24,6 +24,7 @@ COBJS-$(CONFIG_SPL_NAND_LOAD) += nand_spl_load.o COBJS-$(CONFIG_SPL_NAND_ECC) += nand_ecc.o COBJS-$(CONFIG_SPL_NAND_BASE) += nand_base.o COBJS-$(CONFIG_SPL_NAND_INIT) += nand.o +COBJS-$(CONFIG_ENV_IS_IN_NAND) += nand_util.o
Not all NAND SPLs will want this. Define a new CONFIG_SPL_* symbol, or perhaps it could just use CONFIG_SPL_NAND_BASE.
-Scott
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 09/26/2013 04:42 PM, Scott Wood wrote:
On Thu, 2013-09-26 at 16:28 -0400, Tom Rini wrote:
Cc: Scott Wood scottwood@freescale.com Signed-off-by: Tom Rini trini@ti.com
drivers/mtd/nand/Makefile | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile index 366dee6..bb5b29d 100644 --- a/drivers/mtd/nand/Makefile +++ b/drivers/mtd/nand/Makefile @@ -24,6 +24,7 @@ COBJS-$(CONFIG_SPL_NAND_LOAD) += nand_spl_load.o COBJS-$(CONFIG_SPL_NAND_ECC) += nand_ecc.o COBJS-$(CONFIG_SPL_NAND_BASE) += nand_base.o COBJS-$(CONFIG_SPL_NAND_INIT) += nand.o +COBJS-$(CONFIG_ENV_IS_IN_NAND) += nand_util.o
Not all NAND SPLs will want this. Define a new CONFIG_SPL_* symbol, or perhaps it could just use CONFIG_SPL_NAND_BASE.
Then they won't get it. Without CONFIG_SPL_ENV_SUPPORT that all gets discarded. And talked about before adding CONFIG_SPL_ENV_... and I didn't want to go down that path. If you feel strongly about not building the object we can guard this with CONFIG_SPL_ENV_SUPPORT checks perhaps?
- -- Tom
On Thu, 2013-09-26 at 16:45 -0400, Tom Rini wrote:
On 09/26/2013 04:42 PM, Scott Wood wrote:
On Thu, 2013-09-26 at 16:28 -0400, Tom Rini wrote:
Cc: Scott Wood scottwood@freescale.com Signed-off-by: Tom Rini trini@ti.com
drivers/mtd/nand/Makefile | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile index 366dee6..bb5b29d 100644 --- a/drivers/mtd/nand/Makefile +++ b/drivers/mtd/nand/Makefile @@ -24,6 +24,7 @@ COBJS-$(CONFIG_SPL_NAND_LOAD) += nand_spl_load.o COBJS-$(CONFIG_SPL_NAND_ECC) += nand_ecc.o COBJS-$(CONFIG_SPL_NAND_BASE) += nand_base.o COBJS-$(CONFIG_SPL_NAND_INIT) += nand.o +COBJS-$(CONFIG_ENV_IS_IN_NAND) += nand_util.o
Not all NAND SPLs will want this. Define a new CONFIG_SPL_* symbol, or perhaps it could just use CONFIG_SPL_NAND_BASE.
Then they won't get it. Without CONFIG_SPL_ENV_SUPPORT that all gets discarded.
Anonymous strings won't.
And talked about before adding CONFIG_SPL_ENV_... and I didn't want to go down that path. If you feel strongly about not building the object we can guard this with CONFIG_SPL_ENV_SUPPORT checks perhaps?
And then we get a bigger mess if something wants nand_util for some other reason...
I think the entire way SPL has been structured is a mistake, and it should have been viewed as just multiple U-Boot configs that happen to be automatically built and concatenated together under one user-visible target name. Then, if existing config mechanisms are not fine grained enough, make them finer grained. Bring in kconfig to sort out the dependency mess.
-Scott
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 09/26/2013 04:51 PM, Scott Wood wrote:
On Thu, 2013-09-26 at 16:45 -0400, Tom Rini wrote:
On 09/26/2013 04:42 PM, Scott Wood wrote:
On Thu, 2013-09-26 at 16:28 -0400, Tom Rini wrote:
Cc: Scott Wood scottwood@freescale.com Signed-off-by: Tom Rini trini@ti.com
drivers/mtd/nand/Makefile | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile index 366dee6..bb5b29d 100644 --- a/drivers/mtd/nand/Makefile +++ b/drivers/mtd/nand/Makefile @@ -24,6 +24,7 @@ COBJS-$(CONFIG_SPL_NAND_LOAD) += nand_spl_load.o COBJS-$(CONFIG_SPL_NAND_ECC) += nand_ecc.o COBJS-$(CONFIG_SPL_NAND_BASE) += nand_base.o COBJS-$(CONFIG_SPL_NAND_INIT) += nand.o +COBJS-$(CONFIG_ENV_IS_IN_NAND) += nand_util.o
Not all NAND SPLs will want this. Define a new CONFIG_SPL_* symbol, or perhaps it could just use CONFIG_SPL_NAND_BASE.
Then they won't get it. Without CONFIG_SPL_ENV_SUPPORT that all gets discarded.
Anonymous strings won't.
Ah that's right, curses...
And talked about before adding CONFIG_SPL_ENV_... and I didn't want to go down that path. If you feel strongly about not building the object we can guard this with CONFIG_SPL_ENV_SUPPORT checks perhaps?
And then we get a bigger mess if something wants nand_util for some other reason...
I think the entire way SPL has been structured is a mistake, and it should have been viewed as just multiple U-Boot configs that happen to be automatically built and concatenated together under one user-visible target name. Then, if existing config mechanisms are not fine grained enough, make them finer grained. Bring in kconfig to sort out the dependency mess.
Mid-term, yes. I would like to talk about Kconfig more at ELC-E and get a feel from the community on how much time people have to devote to such big efforts.
- -- Tom
Previously, we forced a "no environment" choice on network using SPL. Now we allow all users to set where they want to look for their environment. This means we have to set CONFIG_SPL_ENV_SUPPORT now for ti_armv7_common.h.
Signed-off-by: Tom Rini trini@ti.com --- common/Makefile | 4 +--- include/configs/ti_armv7_common.h | 1 + 2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/common/Makefile b/common/Makefile index 288690b..4568388 100644 --- a/common/Makefile +++ b/common/Makefile @@ -197,17 +197,15 @@ COBJS-$(CONFIG_ENV_IS_IN_FLASH) += env_flash.o COBJS-$(CONFIG_SPL_YMODEM_SUPPORT) += xyzModem.o COBJS-$(CONFIG_SPL_NET_SUPPORT) += miiphyutil.o # environment +ifeq ($(CONFIG_SPL_ENV_SUPPORT),y) COBJS-$(CONFIG_SPL_ENV_SUPPORT) += env_attr.o COBJS-$(CONFIG_SPL_ENV_SUPPORT) += env_flags.o COBJS-$(CONFIG_SPL_ENV_SUPPORT) += env_callback.o -ifneq ($(CONFIG_SPL_NET_SUPPORT),y) COBJS-$(CONFIG_ENV_IS_NOWHERE) += env_nowhere.o COBJS-$(CONFIG_ENV_IS_IN_MMC) += env_mmc.o COBJS-$(CONFIG_ENV_IS_IN_NAND) += env_nand.o COBJS-$(CONFIG_ENV_IS_IN_SPI_FLASH) += env_sf.o COBJS-$(CONFIG_ENV_IS_IN_FLASH) += env_flash.o -else -COBJS-y += env_nowhere.o endif endif # core command diff --git a/include/configs/ti_armv7_common.h b/include/configs/ti_armv7_common.h index 32110f6..d15850b 100644 --- a/include/configs/ti_armv7_common.h +++ b/include/configs/ti_armv7_common.h @@ -202,6 +202,7 @@ #define CONFIG_SPL_FAT_LOAD_PAYLOAD_NAME "u-boot.img"
#ifdef CONFIG_SPL_OS_BOOT +#define CONFIG_SPL_ENV_SUPPORT /* For 'boot_os' and similar */ #define CONFIG_SYS_SPL_ARGS_ADDR (CONFIG_SYS_SDRAM_BASE + 0x100)
/* FAT */
We use the same variable as a3m071 in the environment to determine if we should boot into Linux or U-Boot. This is useful on boards like Beaglebone Black or AM335x GP EVM where we have persistent storage for the environment.
Signed-off-by: Tom Rini trini@ti.com --- board/ti/am335x/board.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/board/ti/am335x/board.c b/board/ti/am335x/board.c index cc04426..0cfd048 100644 --- a/board/ti/am335x/board.c +++ b/board/ti/am335x/board.c @@ -26,6 +26,7 @@ #include <i2c.h> #include <miiphy.h> #include <cpsw.h> +#include <environment.h> #include "board.h"
DECLARE_GLOBAL_DATA_PTR; @@ -232,7 +233,17 @@ static struct emif_regs ddr3_evm_emif_reg_data = { int spl_start_uboot(void) { /* break into full u-boot on 'c' */ - return (serial_tstc() && serial_getc() == 'c'); + if (serial_tstc() && serial_getc() == 'c') + return 1; + +#ifdef CONFIG_SPL_ENV_SUPPORT + env_init(); + env_relocate_spec(); + if (getenv_yesno("boot_os") != 1) + return 1; +#endif + + return 0; } #endif
Signed-off-by: Tom Rini trini@ti.com --- README | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/README b/README index 20ba650..ce8fb37 100644 --- a/README +++ b/README @@ -3031,6 +3031,10 @@ FIT uImage format: supports MMC, NAND and YMODEM loading of U-Boot and NAND NAND loading of the Linux Kernel.
+ CONFIG_SPL_OS_BOOT + Enable booting directly to an OS from SPL. + See also: doc/README.falcon + CONFIG_SPL_DISPLAY_PRINT For ARM, enable an optional function to print more information about the running system.
Signed-off-by: Tom Rini trini@ti.com --- doc/README.falcon | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/doc/README.falcon b/doc/README.falcon index 6357b1e..bccf6c9 100644 --- a/doc/README.falcon +++ b/doc/README.falcon @@ -80,6 +80,15 @@ spl_start_uboot() : required Returns "0" if SPL should start the kernel, "1" if U-Boot must be started.
+Environment variables +--------------------- + +A board may chose to look at the environment for decisions about falcon +mode. In this case the following variables may be supported: + +boot_os : Set to yes/Yes/true/True/1 to enable booting to OS, + any other value to fall back to U-Boot (including + unset)
Using spl command -----------------
This change makes the behaviour slightly more rebust and will match other implementations which can use getenv_yesno directly.
Cc: Stefan Roese sr@denx.de Signed-off-by: Tom Rini trini@ti.com --- board/a3m071/a3m071.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/board/a3m071/a3m071.c b/board/a3m071/a3m071.c index 7aeefb2..b96ba81 100644 --- a/board/a3m071/a3m071.c +++ b/board/a3m071/a3m071.c @@ -412,7 +412,8 @@ int spl_start_uboot(void)
env_init(); getenv_f("boot_os", s, sizeof(s)); - if ((s != NULL) && (strcmp(s, "yes") == 0)) + if ((s != NULL) && (*s == '1' || *s == 'y' || *s == 'Y' || + *s == 't' || *s == 'T')) return 0;
return 1;
On 26.09.2013 22:28, Tom Rini wrote:
This change makes the behaviour slightly more rebust and will match other implementations which can use getenv_yesno directly.
Cc: Stefan Roese sr@denx.de Signed-off-by: Tom Rini trini@ti.com
I see the benefit by this generalization. So:
Acked-by: Stefan Roese sr@denx.de
Thanks, Stefan
We add two new environment variables, falcon_args_file and falcon_image_file, which when set will override the compiled in default values for falcon mode.
Signed-off-by: Tom Rini trini@ti.com --- common/spl/spl_mmc.c | 28 +++++++++++++++++++++++++++- doc/README.falcon | 4 ++++ 2 files changed, 31 insertions(+), 1 deletion(-)
diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c index fc2f226..5405bbc 100644 --- a/common/spl/spl_mmc.c +++ b/common/spl/spl_mmc.c @@ -100,7 +100,33 @@ end: static int mmc_load_image_fat_os(struct mmc *mmc) { int err; - +#if defined(CONFIG_SPL_ENV_SUPPORT) && defined(CONFIG_SPL_OS_BOOT) + char *file; + + file = getenv("falcon_args_file"); + if (file) { + err = file_fat_read(file, (void *)CONFIG_SYS_SPL_ARGS_ADDR, 0); + if (err <= 0) { + printf("spl: error reading image %s, err - %d, falling back to default\n", + file, err); + goto defaults; + } + file = getenv("falcon_image_file"); + if (file) { + err = mmc_load_image_fat(mmc, file); + if (err != 0) { + puts("spl: falling back to default\n"); + goto defaults; + } + + return 0; + } else + puts("spl: falcon_image_file not set in environment, falling back to default\n"); + } else + puts("spl: falcon_args_file not set in environment, falling back to default\n"); + +defaults: +#endif err = file_fat_read(CONFIG_SPL_FAT_LOAD_ARGS_NAME, (void *)CONFIG_SYS_SPL_ARGS_ADDR, 0); if (err <= 0) { diff --git a/doc/README.falcon b/doc/README.falcon index bccf6c9..82a254b 100644 --- a/doc/README.falcon +++ b/doc/README.falcon @@ -89,6 +89,10 @@ mode. In this case the following variables may be supported: boot_os : Set to yes/Yes/true/True/1 to enable booting to OS, any other value to fall back to U-Boot (including unset) +falcon_args_file : Filename to load as the 'args' portion of falcon mode + rather than the hard-coded value. +falcon_image_file : Filename to load as the OS image portion of falcon + mode rather than the hard-coded value.
Using spl command -----------------
participants (4)
-
Scott Wood -
Stefan Roese -
Tom Rini -
Wolfgang Denk