All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v1 0/3] Add support for saveing env variable to SDCard
@ 2009-09-11  3:40 Mingkai Hu
  2009-09-11  3:40 ` [U-Boot] [PATCH v1 1/3] Make mmc init come before env_relocate Mingkai Hu
  0 siblings, 1 reply; 14+ messages in thread
From: Mingkai Hu @ 2009-09-11  3:40 UTC (permalink / raw)
  To: u-boot

[PATCH v1 1/3] Make mmc init come before env_relocate
[PATCH v1 2/3] Add support for save environment variable to MMC/SD card
[PATCH v1 3/3] mpc8536: Get the address of env on the SDCard

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH v1 1/3] Make mmc init come before env_relocate
  2009-09-11  3:40 [U-Boot] [PATCH v1 0/3] Add support for saveing env variable to SDCard Mingkai Hu
@ 2009-09-11  3:40 ` Mingkai Hu
  2009-09-11  3:40   ` [U-Boot] [PATCH v1 2/3] Add support for save environment variable to MMC/SD card Mingkai Hu
  2009-09-22 18:57   ` [U-Boot] [PATCH v1 1/3] Make mmc init come before env_relocate Wolfgang Denk
  0 siblings, 2 replies; 14+ messages in thread
From: Mingkai Hu @ 2009-09-11  3:40 UTC (permalink / raw)
  To: u-boot

If the environment variables are saved on the MMC/SD card,
env_relocat can't relocate env from MMC/SD card without mmc init.

Signed-off-by: Mingkai Hu <Mingkai.hu@freescale.com>
---
 lib_ppc/board.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/lib_ppc/board.c b/lib_ppc/board.c
index e8509ee..d5329b7 100644
--- a/lib_ppc/board.c
+++ b/lib_ppc/board.c
@@ -824,6 +824,12 @@ void board_init_r (gd_t *id, ulong dest_addr)
 	nand_init();		/* go init the NAND */
 #endif
 
+#ifdef CONFIG_GENERIC_MMC
+	WATCHDOG_RESET ();
+	puts ("MMC:  ");
+	mmc_initialize (bd);
+#endif
+
 	/* relocate environment function pointers etc. */
 	env_relocate ();
 
@@ -990,12 +996,6 @@ void board_init_r (gd_t *id, ulong dest_addr)
 	scsi_init ();
 #endif
 
-#ifdef CONFIG_GENERIC_MMC
-	WATCHDOG_RESET ();
-	puts ("MMC:  ");
-	mmc_initialize (bd);
-#endif
-
 #if defined(CONFIG_CMD_DOC)
 	WATCHDOG_RESET ();
 	puts ("DOC:   ");
-- 
1.6.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH v1 2/3] Add support for save environment variable to MMC/SD card
  2009-09-11  3:40 ` [U-Boot] [PATCH v1 1/3] Make mmc init come before env_relocate Mingkai Hu
@ 2009-09-11  3:40   ` Mingkai Hu
  2009-09-11  3:40     ` [U-Boot] [PATCH v1 3/3] mpc8536: Get the address of env on the SDCard Mingkai Hu
  2009-09-22 18:54     ` [U-Boot] [PATCH v1 2/3] Add support for save environment variable to MMC/SD card Wolfgang Denk
  2009-09-22 18:57   ` [U-Boot] [PATCH v1 1/3] Make mmc init come before env_relocate Wolfgang Denk
  1 sibling, 2 replies; 14+ messages in thread
From: Mingkai Hu @ 2009-09-11  3:40 UTC (permalink / raw)
  To: u-boot

Whether booting from MMC/SD card or not, the environment variables
can be saved on it, this patch add the operation support.

Signed-off-by: Mingkai Hu <Mingkai.hu@freescale.com>
---
 common/Makefile     |    1 +
 common/cmd_nvedit.c |    3 +-
 common/env_sdcard.c |  135 +++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 138 insertions(+), 1 deletions(-)
 create mode 100644 common/env_sdcard.c

diff --git a/common/Makefile b/common/Makefile
index 3781738..ce6a7da 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -61,6 +61,7 @@ COBJS-$(CONFIG_ENV_IS_IN_NAND) += env_nand.o
 COBJS-$(CONFIG_ENV_IS_IN_NVRAM) += env_nvram.o
 COBJS-$(CONFIG_ENV_IS_IN_ONENAND) += env_onenand.o
 COBJS-$(CONFIG_ENV_IS_IN_SPI_FLASH) += env_sf.o
+COBJS-$(CONFIG_ENV_IS_IN_SDCARD) += env_sdcard.o
 COBJS-$(CONFIG_ENV_IS_NOWHERE) += env_nowhere.o
 
 # command
diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index 2186205..83969ef 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -60,9 +60,10 @@ DECLARE_GLOBAL_DATA_PTR;
     !defined(CONFIG_ENV_IS_IN_NVRAM)	&& \
     !defined(CONFIG_ENV_IS_IN_ONENAND)	&& \
     !defined(CONFIG_ENV_IS_IN_SPI_FLASH)	&& \
+    !defined(CONFIG_ENV_IS_IN_SDCARD)	&& \
     !defined(CONFIG_ENV_IS_NOWHERE)
 # error Define one of CONFIG_ENV_IS_IN_{EEPROM|FLASH|DATAFLASH|ONENAND|\
-SPI_FLASH|MG_DISK|NVRAM|NOWHERE}
+SPI_FLASH|MG_DISK|NVRAM|SDCARD|NOWHERE}
 #endif
 
 #define XMK_STR(x)	#x
diff --git a/common/env_sdcard.c b/common/env_sdcard.c
new file mode 100644
index 0000000..ce73358
--- /dev/null
+++ b/common/env_sdcard.c
@@ -0,0 +1,135 @@
+/*
+ * Copyright (C) 2009 Freescale Semiconductor, Inc.
+ *
+ * Changelog:
+ * July 2008, Intial version.
+ * June 2009, align to the MMC framework.
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+#include <common.h>
+#include <mmc.h>
+
+#include <environment.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+/* references to names in env_common.c */
+extern uchar default_environment[];
+extern int mmc_get_env_addr(int dev, u32 *env_addr);
+
+char *env_name_spec = "SD CARD";
+env_t *env_ptr;
+
+uchar env_get_char_spec(int index)
+{
+	return *((uchar *)(gd->env_addr + index));
+}
+
+static int readenv(int dev, size_t offset, u_char *buf)
+{
+	int ret;
+	struct mmc *mmc = find_mmc_device(dev);
+
+	mmc_init(mmc);
+
+	ret = mmc_read(mmc, offset, buf, CONFIG_ENV_SIZE);
+	if (ret)
+		return 1;
+
+	return 0;
+}
+
+static int writeenv(int dev, size_t offset, u_char *buf)
+{
+	int env_blknr, env_blkcnt, n;
+	uint blklen;
+	struct mmc *mmc = find_mmc_device(dev);
+
+	mmc_init(mmc);
+
+	blklen = mmc->write_bl_len;
+	env_blknr = offset / blklen;
+	env_blkcnt = CONFIG_ENV_SIZE / blklen;
+
+	n = mmc->block_dev.block_write(dev, env_blknr, env_blkcnt, buf);
+	if (n != env_blkcnt)
+		return 1;
+
+	return 0;
+}
+
+int saveenv(void)
+{
+	int ret;
+	int dev = 0;
+	u32 env_addr;
+
+	ret = mmc_get_env_addr(dev, &env_addr);
+	if (ret) {
+		puts("FAILED!\n");
+		return 1;
+	}
+
+	ret = writeenv(dev, env_addr, (u_char *) env_ptr);
+	if (ret)
+		return 1;
+
+	puts("done\n");
+	gd->env_valid = 1;
+
+	return ret;
+}
+
+void env_relocate_spec(void)
+{
+	int ret;
+	int dev = 0;
+	u32 env_addr;
+
+	ret = mmc_get_env_addr(dev, &env_addr);
+	if (ret)
+		goto err_read;
+
+	ret = readenv(dev, env_addr, (u_char *) env_ptr);
+	if (ret)
+		goto err_read;
+
+	if (crc32(0, env_ptr->data, ENV_SIZE) != env_ptr->crc)
+		goto err_crc;
+
+	gd->env_valid = 1;
+
+	return;
+
+err_read:
+err_crc:
+	puts("*** Warning - bad CRC, using default environment\n\n");
+
+	set_default_env();
+}
+
+int env_init(void)
+{
+	/* eSDHC isn't usable before relocation */
+	gd->env_addr  = (ulong)&default_environment[0];
+	gd->env_valid = 1;
+
+	return 0;
+}
-- 
1.6.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH v1 3/3] mpc8536: Get the address of env on the SDCard
  2009-09-11  3:40   ` [U-Boot] [PATCH v1 2/3] Add support for save environment variable to MMC/SD card Mingkai Hu
@ 2009-09-11  3:40     ` Mingkai Hu
  2009-09-22 18:59       ` Wolfgang Denk
  2009-09-22 18:54     ` [U-Boot] [PATCH v1 2/3] Add support for save environment variable to MMC/SD card Wolfgang Denk
  1 sibling, 1 reply; 14+ messages in thread
From: Mingkai Hu @ 2009-09-11  3:40 UTC (permalink / raw)
  To: u-boot

Both the save env and load env operation will call this function
to get the address of env on the SDCard, so the user can control
where to put the env freely.

Also enable the functionlity of saving env variable to SDCard on
mpc8536

Signed-off-by: Mingkai Hu <Mingkai.hu@freescale.com>
---
 board/freescale/mpc8536ds/mpc8536ds.c |   46 +++++++++++++++++++++++++++++++++
 include/configs/MPC8536DS.h           |    5 +++-
 2 files changed, 50 insertions(+), 1 deletions(-)

diff --git a/board/freescale/mpc8536ds/mpc8536ds.c b/board/freescale/mpc8536ds/mpc8536ds.c
index da72916..85a3dc3 100644
--- a/board/freescale/mpc8536ds/mpc8536ds.c
+++ b/board/freescale/mpc8536ds/mpc8536ds.c
@@ -38,6 +38,8 @@
 #include <tsec.h>
 #include <netdev.h>
 #include <sata.h>
+#include <mmc.h>
+#include <malloc.h>
 
 #include "../common/pixis.h"
 #include "../common/sgmii_riser.h"
@@ -681,3 +683,47 @@ void ft_board_setup(void *blob, bd_t *bd)
 #endif
 }
 #endif
+
+#if defined(CONFIG_MMC)
+/*
+ * The environment variables are written to just after the u-boot image
+ * on SDCard, so we must read the MBR to get the start address and code
+ * length of the u-boot image, then calculate the address of the env.
+ */
+#define ESDHC_BOOT_IMAGE_SIZE	0x48
+#define ESDHC_BOOT_IMAGE_ADDR	0x50
+
+int mmc_get_env_addr(int dev, u32 *env_addr)
+{
+	int ret;
+	u8 *tmp_buf;
+	u32 blklen, code_offset, code_len;
+	struct mmc *mmc = find_mmc_device(dev);
+
+	mmc_init(mmc);
+
+	blklen = mmc->read_bl_len;
+	tmp_buf = malloc(blklen);
+	if (!tmp_buf)
+		return 1;
+
+	/* read out the first block, get the config data information */
+	ret = mmc_read(mmc, 0, tmp_buf, blklen);
+	if (ret) {
+		free(tmp_buf);
+		return 1;
+	}
+
+	/* Get the Source Address, from offset 0x50 */
+	code_offset = *(u32 *)(tmp_buf + ESDHC_BOOT_IMAGE_ADDR);
+
+	/* Get the code size from offset 0x48 */
+	code_len = *(u32 *)(tmp_buf + ESDHC_BOOT_IMAGE_SIZE);
+
+	*env_addr = code_offset + code_len;
+
+	free(tmp_buf);
+
+	return 0;
+}
+#endif
diff --git a/include/configs/MPC8536DS.h b/include/configs/MPC8536DS.h
index 580d66f..6ffa894 100644
--- a/include/configs/MPC8536DS.h
+++ b/include/configs/MPC8536DS.h
@@ -633,7 +633,10 @@ extern unsigned long get_board_ddr_clk(unsigned long dummy);
 	#define CONFIG_ENV_IS_IN_NAND	1
 	#define CONFIG_ENV_SIZE		CONFIG_SYS_NAND_BLOCK_SIZE
 	#define CONFIG_ENV_OFFSET	((512 * 1024) + CONFIG_SYS_NAND_BLOCK_SIZE)
-#elif defined(CONFIG_RAMBOOT_SPIFLASH) || defined(CONFIG_RAMBOOT_SDCARD)
+#elif defined(CONFIG_RAMBOOT_SDCARD)
+        #define CONFIG_ENV_IS_IN_SDCARD         1
+        #define CONFIG_ENV_SIZE                 0x2000
+#elif defined(CONFIG_RAMBOOT_SPIFLASH)
 	#define CONFIG_ENV_IS_NOWHERE	1	/* Store ENV in memory only */
 	#define CONFIG_ENV_ADDR		(CONFIG_SYS_MONITOR_BASE - 0x1000)
 	#define CONFIG_ENV_SIZE		0x2000
-- 
1.6.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH v1 2/3] Add support for save environment variable to MMC/SD card
  2009-09-11  3:40   ` [U-Boot] [PATCH v1 2/3] Add support for save environment variable to MMC/SD card Mingkai Hu
  2009-09-11  3:40     ` [U-Boot] [PATCH v1 3/3] mpc8536: Get the address of env on the SDCard Mingkai Hu
@ 2009-09-22 18:54     ` Wolfgang Denk
  2009-09-23  4:46       ` Hu Mingkai-B21284
  1 sibling, 1 reply; 14+ messages in thread
From: Wolfgang Denk @ 2009-09-22 18:54 UTC (permalink / raw)
  To: u-boot

Dear Mingkai Hu,

In message <1252640445-7890-3-git-send-email-Mingkai.hu@freescale.com> you wrote:
> Whether booting from MMC/SD card or not, the environment variables
> can be saved on it, this patch add the operation support.
> 
> Signed-off-by: Mingkai Hu <Mingkai.hu@freescale.com>
...
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -61,6 +61,7 @@ COBJS-$(CONFIG_ENV_IS_IN_NAND) += env_nand.o
>  COBJS-$(CONFIG_ENV_IS_IN_NVRAM) += env_nvram.o
>  COBJS-$(CONFIG_ENV_IS_IN_ONENAND) += env_onenand.o
>  COBJS-$(CONFIG_ENV_IS_IN_SPI_FLASH) += env_sf.o
> +COBJS-$(CONFIG_ENV_IS_IN_SDCARD) += env_sdcard.o
>  COBJS-$(CONFIG_ENV_IS_NOWHERE) += env_nowhere.o

Please keep the list sorted.

>  # command
> diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
> index 2186205..83969ef 100644
> --- a/common/cmd_nvedit.c
> +++ b/common/cmd_nvedit.c
> @@ -60,9 +60,10 @@ DECLARE_GLOBAL_DATA_PTR;
>      !defined(CONFIG_ENV_IS_IN_NVRAM)	&& \
>      !defined(CONFIG_ENV_IS_IN_ONENAND)	&& \
>      !defined(CONFIG_ENV_IS_IN_SPI_FLASH)	&& \
> +    !defined(CONFIG_ENV_IS_IN_SDCARD)	&& \

Ditto.

>  # error Define one of CONFIG_ENV_IS_IN_{EEPROM|FLASH|DATAFLASH|ONENAND|\
> -SPI_FLASH|MG_DISK|NVRAM|NOWHERE}
> +SPI_FLASH|MG_DISK|NVRAM|SDCARD|NOWHERE}

Please take the opportunity to sort this list, too. Thanks.

...
> +int env_init(void)
> +{
> +	/* eSDHC isn't usable before relocation */
> +	gd->env_addr  = (ulong)&default_environment[0];
> +	gd->env_valid = 1;

Argh... Does that mean that your environment suddenly changes while
running? That you start running from the default environment (which
cannot be changed) and then switch to a the real, changable
environment?

This is going to cause a hell of confusion to users who for example
want to define a different console baud rate or such...

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
It is surely a great calamity for  a  human  being  to  have  no  ob-
sessions.                                                - Robert Bly

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH v1 1/3] Make mmc init come before env_relocate
  2009-09-11  3:40 ` [U-Boot] [PATCH v1 1/3] Make mmc init come before env_relocate Mingkai Hu
  2009-09-11  3:40   ` [U-Boot] [PATCH v1 2/3] Add support for save environment variable to MMC/SD card Mingkai Hu
@ 2009-09-22 18:57   ` Wolfgang Denk
  2009-09-23  2:57     ` Hu Mingkai-B21284
                       ` (2 more replies)
  1 sibling, 3 replies; 14+ messages in thread
From: Wolfgang Denk @ 2009-09-22 18:57 UTC (permalink / raw)
  To: u-boot

Dear Mingkai Hu,

In message <1252640445-7890-2-git-send-email-Mingkai.hu@freescale.com> you wrote:
> If the environment variables are saved on the MMC/SD card,
> env_relocat can't relocate env from MMC/SD card without mmc init.
> 
> Signed-off-by: Mingkai Hu <Mingkai.hu@freescale.com>

I'm biased. I understand that you do this because you need it for the
next patch, which reads the environment from MMC card. But then MMC is
just one out of many storage devices, and with the same right we would
have to move the SCSI or DoC or S-ATA initialization up, because we
could implement storing the environment on such devices, too.

This has not been done so far, because it suffers from the fundamental
problem your code is suffering from, too: you cannot access the
environment early enough, so for example your board boots with a
hard-wird, unchangable console baud rate, despite that you suggest to
the user he could change it.

I don't like that, and therefore tend to NAK the whole approach.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Don't hit a man when he's down - kick him; it's easier.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH v1 3/3] mpc8536: Get the address of env on the SDCard
  2009-09-11  3:40     ` [U-Boot] [PATCH v1 3/3] mpc8536: Get the address of env on the SDCard Mingkai Hu
@ 2009-09-22 18:59       ` Wolfgang Denk
  2009-09-23  2:53         ` Hu Mingkai-B21284
  0 siblings, 1 reply; 14+ messages in thread
From: Wolfgang Denk @ 2009-09-22 18:59 UTC (permalink / raw)
  To: u-boot

Dear Mingkai Hu,

In message <1252640445-7890-4-git-send-email-Mingkai.hu@freescale.com> you wrote:
> Both the save env and load env operation will call this function
> to get the address of env on the SDCard, so the user can control
> where to put the env freely.
> 
> Also enable the functionlity of saving env variable to SDCard on
> mpc8536
> 
> Signed-off-by: Mingkai Hu <Mingkai.hu@freescale.com>
...
> +#if defined(CONFIG_MMC)
> +/*
> + * The environment variables are written to just after the u-boot image
> + * on SDCard, so we must read the MBR to get the start address and code
> + * length of the u-boot image, then calculate the address of the env.

This seems to be broken by design. If your U-Boot image grows, it
would overwrite the environment? Or am I missing something?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
If A equals success, then the formula is A = X + Y + Z. X is work.  Y
is play. Z is keep your mouth shut.                 - Albert Einstein

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH v1 3/3] mpc8536: Get the address of env on the SDCard
  2009-09-22 18:59       ` Wolfgang Denk
@ 2009-09-23  2:53         ` Hu Mingkai-B21284
  0 siblings, 0 replies; 14+ messages in thread
From: Hu Mingkai-B21284 @ 2009-09-23  2:53 UTC (permalink / raw)
  To: u-boot


> -----Original Message-----
> From: Wolfgang Denk [mailto:wd at denx.de] 
> Sent: Wednesday, September 23, 2009 3:00 AM
> To: Hu Mingkai-B21284
> Cc: u-boot at lists.denx.de; Wood Scott-B07421; Gala Kumar-B11780
> Subject: Re: [U-Boot] [PATCH v1 3/3] mpc8536: Get the address 
> of env on the SDCard
> 
> This seems to be broken by design. If your U-Boot image 
> grows, it would overwrite the environment? Or am I missing something?
> 

Yes, if the U-Boot image grows, it would overwrite the env saved last
time.
Maybe we should put it the to haed of the U-Boot image.

Thanks,
Mingkai

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH v1 1/3] Make mmc init come before env_relocate
  2009-09-22 18:57   ` [U-Boot] [PATCH v1 1/3] Make mmc init come before env_relocate Wolfgang Denk
@ 2009-09-23  2:57     ` Hu Mingkai-B21284
  2009-09-23  7:28     ` Konrad Mattheis
  2009-09-23 17:59     ` Scott Wood
  2 siblings, 0 replies; 14+ messages in thread
From: Hu Mingkai-B21284 @ 2009-09-23  2:57 UTC (permalink / raw)
  To: u-boot

 

> -----Original Message-----
> From: Wolfgang Denk [mailto:wd at denx.de] 
> Sent: Wednesday, September 23, 2009 2:58 AM
> To: Hu Mingkai-B21284
> Cc: u-boot at lists.denx.de; Wood Scott-B07421; Gala Kumar-B11780
> Subject: Re: [U-Boot] [PATCH v1 1/3] Make mmc init come 
> before env_relocate
> 
> This has not been done so far, because it suffers from the 
> fundamental problem your code is suffering from, too: you 
> cannot access the environment early enough, so for example 
> your board boots with a hard-wird, unchangable console baud 
> rate, despite that you suggest to the user he could change it.
>
Got it.
 
> I don't like that, and therefore tend to NAK the whole approach.
>
No problem, after all this patchset is not matured enough.

Thanks,
Mingkai

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH v1 2/3] Add support for save environment variable to MMC/SD card
  2009-09-22 18:54     ` [U-Boot] [PATCH v1 2/3] Add support for save environment variable to MMC/SD card Wolfgang Denk
@ 2009-09-23  4:46       ` Hu Mingkai-B21284
  0 siblings, 0 replies; 14+ messages in thread
From: Hu Mingkai-B21284 @ 2009-09-23  4:46 UTC (permalink / raw)
  To: u-boot

 

> -----Original Message-----
> From: Wolfgang Denk [mailto:wd at denx.de] 
> Sent: Wednesday, September 23, 2009 2:54 AM
> To: Hu Mingkai-B21284
> Cc: u-boot at lists.denx.de; Wood Scott-B07421; Gala Kumar-B11780
> Subject: Re: [U-Boot] [PATCH v1 2/3] Add support for save 
> environment variable to MMC/SD card
> 
> Dear Mingkai Hu,
> 
> In message 
> <1252640445-7890-3-git-send-email-Mingkai.hu@freescale.com> you wrote:
> > Whether booting from MMC/SD card or not, the environment 
> variables can 
> > be saved on it, this patch add the operation support.
> > 
> > Signed-off-by: Mingkai Hu <Mingkai.hu@freescale.com>
> ...
> > --- a/common/Makefile
> > +++ b/common/Makefile
> > @@ -61,6 +61,7 @@ COBJS-$(CONFIG_ENV_IS_IN_NAND) += env_nand.o
> >  COBJS-$(CONFIG_ENV_IS_IN_NVRAM) += env_nvram.o
> >  COBJS-$(CONFIG_ENV_IS_IN_ONENAND) += env_onenand.o
> >  COBJS-$(CONFIG_ENV_IS_IN_SPI_FLASH) += env_sf.o
> > +COBJS-$(CONFIG_ENV_IS_IN_SDCARD) += env_sdcard.o
> >  COBJS-$(CONFIG_ENV_IS_NOWHERE) += env_nowhere.o
> 
> Please keep the list sorted.
> 
OK.

> >  # command
> > diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c index 
> > 2186205..83969ef 100644
> > --- a/common/cmd_nvedit.c
> > +++ b/common/cmd_nvedit.c
> > @@ -60,9 +60,10 @@ DECLARE_GLOBAL_DATA_PTR;
> >      !defined(CONFIG_ENV_IS_IN_NVRAM)	&& \
> >      !defined(CONFIG_ENV_IS_IN_ONENAND)	&& \
> >      !defined(CONFIG_ENV_IS_IN_SPI_FLASH)	&& \
> > +    !defined(CONFIG_ENV_IS_IN_SDCARD)	&& \
> 
> Ditto.
> 
OK.

> >  # error Define one of 
> > CONFIG_ENV_IS_IN_{EEPROM|FLASH|DATAFLASH|ONENAND|\
> > -SPI_FLASH|MG_DISK|NVRAM|NOWHERE}
> > +SPI_FLASH|MG_DISK|NVRAM|SDCARD|NOWHERE}
> 
> Please take the opportunity to sort this list, too. Thanks.
> 
OK.

> ...
> > +int env_init(void)
> > +{
> > +	/* eSDHC isn't usable before relocation */
> > +	gd->env_addr  = (ulong)&default_environment[0];
> > +	gd->env_valid = 1;
> 
> Argh... Does that mean that your environment suddenly changes 
> while running? That you start running from the default 
> environment (which cannot be changed) and then switch to a 
> the real, changable environment?
> 
Yes. I refered to env_sf.c file, maybe it has the same issue.

> This is going to cause a hell of confusion to users who for 
> example want to define a different console baud rate or such...
> 
It's a problem.

Thanks,
Mingkai

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH v1 1/3] Make mmc init come before env_relocate
  2009-09-22 18:57   ` [U-Boot] [PATCH v1 1/3] Make mmc init come before env_relocate Wolfgang Denk
  2009-09-23  2:57     ` Hu Mingkai-B21284
@ 2009-09-23  7:28     ` Konrad Mattheis
  2009-09-23 12:43       ` Wolfgang Denk
  2009-09-23 17:59     ` Scott Wood
  2 siblings, 1 reply; 14+ messages in thread
From: Konrad Mattheis @ 2009-09-23  7:28 UTC (permalink / raw)
  To: u-boot

Dear Wolfgang Denk,

> I'm biased. I understand that you do this because you need it for the
> next patch, which reads the environment from MMC card. But then MMC is
> just one out of many storage devices, and with the same right we would
> h ave to move the SCSI or DoC or S-ATA initialization up, because we
> could implement storing the environment on such devices, too.

> This has not been done so far, because it suffers from the fundamental
> problem your code is suffering from, too: you cannot access the
> environment early enough, so for example your board boots with a
> hard-wird, unchangable console baud rate, despite that you suggest to
> the user he could change it.

>I don't like that, and therefore tend to NAK the whole approach.

I understand why you would like to NAK the approach, but this is not
only interesting for this type of ARM. And more and more ARM devices
allow direct boot from SD-Card,... and so everybody ask for that I need
another storage just for the environment variables. In the production
you also have to program this storage. So I think you will get
this question more and more. Or more and more patches that are not
in the mainline.

For me the question is if where a good way to implement this feature?
Do you have a good suggestion for solving the problem of initializations
steps?

I would really like to help to get this problem solved, because
I'm building at the moment an G20/G45 board that doing the same thing.

bye
Konrad Mattheis

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH v1 1/3] Make mmc init come before env_relocate
  2009-09-23  7:28     ` Konrad Mattheis
@ 2009-09-23 12:43       ` Wolfgang Denk
  2009-09-24  9:02         ` Konrad Mattheis
  0 siblings, 1 reply; 14+ messages in thread
From: Wolfgang Denk @ 2009-09-23 12:43 UTC (permalink / raw)
  To: u-boot

Dear Konrad Mattheis,

In message <F32E8C2599F37641BB4E3F3AC4A3C412016D265C4FE3@ws> you wrote:
> 
> >I don't like that, and therefore tend to NAK the whole approach.
> 
> I understand why you would like to NAK the approach, but this is not
> only interesting for this type of ARM. And more and more ARM devices
> allow direct boot from SD-Card,... and so everybody ask for that I need
> another storage just for the environment variables. In the production
> you also have to program this storage. So I think you will get
> this question more and more. Or more and more patches that are not
> in the mainline.

I understand the requirements, but I disagree with your conclusions,
and with the implementation.

Booting from some storage device (whatever it may be) is one thing,
but selecting the storage medium for the U-Boot environment is a
different thing.

> For me the question is if where a good way to implement this feature?
> Do you have a good suggestion for solving the problem of initializations
> steps?

Well, there are IMO two fundamental questions:

1) Where do we store the environment?

   Even if you load the U-Boot binary from - say - MMC/SDCard, there
   is most probably also another storage medium available on your
   board which may be easier to use (i. e. available before
   relocation to RAM) and thus more appropriate to use.

   Also, not storing the environment on changable media has the
   affect that it will allow for persistent settings even when you
   change the mdium you boot from. This may or may not be an
   advantage, depending on your requirements. My guess is that it
   would be an advantage in most applications.

2) How do we implement this?

   Assuming you load the U-Boot binary from - say - MMC/SDCard, you
   must already have some code running that can do this. Then why
   should that very same code not be able to load the envrionment from
   the same MMC/SDCard as well? In the worst case you could make it
   one big blob that gets loaded in one go.

   Why do you think we need U-Boot's MMC/SDCard drivers to perform the
   initial load of the environment from the external storage?


Thus I suggest two approaches to solve the problem, which both have
their specific features and advantages, and probably should be not
considered as alternatives, but used in combination, at least
optionally.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
We, the unwilling, led by the unknowing, are doing the impossible for
the ungrateful. We have done so much, for so long, with so little, we
are now qualified to do anything with nothing.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH v1 1/3] Make mmc init come before env_relocate
  2009-09-22 18:57   ` [U-Boot] [PATCH v1 1/3] Make mmc init come before env_relocate Wolfgang Denk
  2009-09-23  2:57     ` Hu Mingkai-B21284
  2009-09-23  7:28     ` Konrad Mattheis
@ 2009-09-23 17:59     ` Scott Wood
  2 siblings, 0 replies; 14+ messages in thread
From: Scott Wood @ 2009-09-23 17:59 UTC (permalink / raw)
  To: u-boot

On Tue, Sep 22, 2009 at 08:57:33PM +0200, Wolfgang Denk wrote:
> I'm biased. I understand that you do this because you need it for the
> next patch, which reads the environment from MMC card. But then MMC is
> just one out of many storage devices, and with the same right we would
> have to move the SCSI or DoC or S-ATA initialization up, because we
> could implement storing the environment on such devices, too.
> 
> This has not been done so far, because it suffers from the fundamental
> problem your code is suffering from, too: you cannot access the
> environment early enough, so for example your board boots with a
> hard-wird, unchangable console baud rate, despite that you suggest to
> the user he could change it.
> 
> I don't like that, and therefore tend to NAK the whole approach.

The same applies to NAND.  The CONFIG_NAND_ENV_DST mechanism makes it a
bit better (the first stage loader puts the environment in a designated
part of RAM); perhaps something similar could be done here?

There's still a problem with doing any console output during the first
stage loading itself (such as to indicate failures).  For that, the best
I can think of would be to have a CONFIG option for a hardcoded baud, and
if you leave it undefined you will get no output until the second stage.

-Scott

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH v1 1/3] Make mmc init come before env_relocate
  2009-09-23 12:43       ` Wolfgang Denk
@ 2009-09-24  9:02         ` Konrad Mattheis
  0 siblings, 0 replies; 14+ messages in thread
From: Konrad Mattheis @ 2009-09-24  9:02 UTC (permalink / raw)
  To: u-boot

Dear Wofgang Denk,

> Well, there are IMO two fundamental questions:
> 
> 1) Where do we store the environment?
> 
>    Even if you load the U-Boot binary from - say - MMC/SDCard, there
>    is most probably also another storage medium available on your
>    board which may be easier to use (i. e. available before
>    relocation to RAM) and thus more appropriate to use.
>  
>    Also, not storing the environment on changable media has the
>    affect that it will allow for persistent settings even when you
>    change the mdium you boot from. This may or may not be an
>    advantage, depending on your requirements. My guess is that it
>    would be an advantage in most applications.

I understand the advantages for storing the settings on the board,
but I thing this problem is already perfectly solved. So for that
type of applications there is nothing to do.
But for the other type I'm searching for a good way. Some reasons
this way is also important:
	1. If I have two same hardware boards I just have to change
	   the storage card and the new hardware work in the same way
	   (this is very important for warranty or support problems)
      2. At mass production I don't need to program /flash something

> 2) How do we implement this?
> 
>    Assuming you load the U-Boot binary from - say - MMC/SDCard, you
>    must already have some code running that can do this. Then why
>    should that very same code not be able to load the envrionment from
>    the same MMC/SDCard as well? In the worst case you could make it
>    one big blob that gets loaded in one go.
> 
>    Why do you think we need U-Boot's MMC/SDCard drivers to perform the
>    initial load of the environment from the external storage?
> 
> 
> Thus I suggest two approaches to solve the problem, which both have
> their specific features and advantages, and probably should be not
> considered as alternatives, but used in combination, at least
> optionally.

For the implementation I have now this Idea:

The environment variables can be attached to the uboot-binary. This could be
in the begin or end of the file and both have some advantages and disadvantages,
but this we could discuss later.
At boot time the u-boot has the possibility to get this environment variables
without any driver, because there are in flash or copied into the ram.
So the u-boot just have to look at position if it find these variables
and then will use them.
If you want to change these variables you have two possibilities.
1. do it in u-boot, u-boot load the driver for the storage and then can write on
2. do in later high level linux/win (there it could be good to deliver an easy c code
   for the users)

With this solution you also solve the problem with Scott Wood solution,
because you already have the right env. variables at the start time.
So no problem with serial,...

Best regards,

Konrad Mattheis

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2009-09-24  9:02 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-09-11  3:40 [U-Boot] [PATCH v1 0/3] Add support for saveing env variable to SDCard Mingkai Hu
2009-09-11  3:40 ` [U-Boot] [PATCH v1 1/3] Make mmc init come before env_relocate Mingkai Hu
2009-09-11  3:40   ` [U-Boot] [PATCH v1 2/3] Add support for save environment variable to MMC/SD card Mingkai Hu
2009-09-11  3:40     ` [U-Boot] [PATCH v1 3/3] mpc8536: Get the address of env on the SDCard Mingkai Hu
2009-09-22 18:59       ` Wolfgang Denk
2009-09-23  2:53         ` Hu Mingkai-B21284
2009-09-22 18:54     ` [U-Boot] [PATCH v1 2/3] Add support for save environment variable to MMC/SD card Wolfgang Denk
2009-09-23  4:46       ` Hu Mingkai-B21284
2009-09-22 18:57   ` [U-Boot] [PATCH v1 1/3] Make mmc init come before env_relocate Wolfgang Denk
2009-09-23  2:57     ` Hu Mingkai-B21284
2009-09-23  7:28     ` Konrad Mattheis
2009-09-23 12:43       ` Wolfgang Denk
2009-09-24  9:02         ` Konrad Mattheis
2009-09-23 17:59     ` Scott Wood

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.