Re: [U-Boot] [PATCH] Save environment data to mmc.

2010-04-30 Thread Stefano Babic 

Hi Terry,

[Ty]: env_ptr is defined in env_mmc.c and the value is
 assigned in env_common.c. Correct me if I'm wrong.

No, you are right - env_relocate() called malloc and initialize the pointer.

Best regards,
Stefano

-- 
=
DENX Software Engineering GmbH, MD: Wolfgang Denk  Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: off...@denx.de
=
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH] Save environment data to mmc.

2010-04-29 Thread Terry Lv 
This patch is to save environment data to mmc card.
It uses interfaces defined in generic mmc.

Signed-off-by: Terry Lv r65...@freescale.com
---
 arch/arm/lib/board.c |   10 ++--
 arch/powerpc/lib/board.c |   12 ++--
 common/Makefile  |1 +
 common/cmd_nvedit.c  |1 +
 common/env_mmc.c |  154 ++
 5 files changed, 167 insertions(+), 11 deletions(-)
 create mode 100644 common/env_mmc.c

diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
index f5660a9..f62e0eb 100644
--- a/arch/arm/lib/board.c
+++ b/arch/arm/lib/board.c
@@ -347,6 +347,11 @@ void start_armboot (void)
dataflash_print_info();
 #endif
 
+#ifdef CONFIG_GENERIC_MMC
+   puts (MMC:   );
+   mmc_initialize (gd-bd);
+#endif
+
/* initialize environment */
env_relocate ();
 
@@ -419,11 +424,6 @@ extern void davinci_eth_set_mac_addr (const u_int8_t 
*addr);
board_late_init ();
 #endif
 
-#ifdef CONFIG_GENERIC_MMC
-   puts (MMC:   );
-   mmc_initialize (gd-bd);
-#endif
-
 #ifdef CONFIG_BITBANGMII
bb_miiphy_init();
 #endif
diff --git a/arch/powerpc/lib/board.c b/arch/powerpc/lib/board.c
index 7b09fb5..1008635 100644
--- a/arch/powerpc/lib/board.c
+++ b/arch/powerpc/lib/board.c
@@ -783,6 +783,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 ();
 
@@ -939,12 +945,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:   );
diff --git a/common/Makefile b/common/Makefile
index dbf7a05..2c37073 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -58,6 +58,7 @@ COBJS-$(CONFIG_ENV_IS_IN_FLASH) += env_embedded.o
 COBJS-$(CONFIG_ENV_IS_IN_NVRAM) += env_embedded.o
 COBJS-$(CONFIG_ENV_IS_IN_FLASH) += env_flash.o
 COBJS-$(CONFIG_ENV_IS_IN_MG_DISK) += env_mgdisk.o
+COBJS-$(CONFIG_ENV_IS_IN_MMC) += env_mmc.o
 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
diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index eb89e9e..78f75fb 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -59,6 +59,7 @@ DECLARE_GLOBAL_DATA_PTR;
 !defined(CONFIG_ENV_IS_IN_FLASH)\
 !defined(CONFIG_ENV_IS_IN_DATAFLASH)\
 !defined(CONFIG_ENV_IS_IN_MG_DISK)  \
+!defined(CONFIG_ENV_IS_IN_MMC)   \
 !defined(CONFIG_ENV_IS_IN_NAND) \
 !defined(CONFIG_ENV_IS_IN_NVRAM)\
 !defined(CONFIG_ENV_IS_IN_ONENAND)  \
diff --git a/common/env_mmc.c b/common/env_mmc.c
new file mode 100644
index 000..88db336
--- /dev/null
+++ b/common/env_mmc.c
@@ -0,0 +1,154 @@
+/*
+ * (C) Copyright 2008-2010 Freescale Semiconductor, Inc.
+ *
+ * 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
+ */
+
+/* #define DEBUG */
+
+#include common.h
+
+#include command.h
+#include environment.h
+#include linux/stddef.h
+#include malloc.h
+#include mmc.h
+
+/* references to names in env_common.c */
+extern uchar default_environment[];
+
+char *env_name_spec = MMC;
+
+#ifdef ENV_IS_EMBEDDED
+extern uchar environment[];
+env_t *env_ptr = (env_t *)(environment[0]);
+#else /* ! ENV_IS_EMBEDDED */
+env_t *env_ptr;
+#endif /* ENV_IS_EMBEDDED */
+
+/* local functions */
+#if !defined(ENV_IS_EMBEDDED)
+static void use_default(void);
+#endif
+
+DECLARE_GLOBAL_DATA_PTR;
+
+uchar env_get_char_spec(int index)
+{
+   return *((uchar *)(gd-env_addr + index));
+}
+
+int env_init(void)
+{
+   /* use default */
+   gd-env_addr = (ulong)default_environment[0];
+   gd-env_valid = 1;
+
+   return 0;
+}
+
+int init_mmc_for_env(struct mmc *mmc)
+{
+   if (!mmc) {
+   puts(No MMC card found\n);
+   return -1;
+   }
+
+   if (mmc_init(mmc)) {
+   puts(MMC init failed\n);
+

Re: [U-Boot] [PATCH] Save environment data to mmc.

2010-04-29 Thread Stefano Babic 
Terry Lv wrote:
 This patch is to save environment data to mmc card.
 It uses interfaces defined in generic mmc.

Hi Terry,

 
 Signed-off-by: Terry Lv r65...@freescale.com
 ---
  arch/arm/lib/board.c |   10 ++--
  arch/powerpc/lib/board.c |   12 ++--
  common/Makefile  |1 +
  common/cmd_nvedit.c  |1 +
  common/env_mmc.c |  154 
 ++
  5 files changed, 167 insertions(+), 11 deletions(-)
  create mode 100644 common/env_mmc.c

Could you set a version of your patch (something like [PATCH V*] in the
subject, so it is easier to track changes ? This is the third version,
but it is difficult to get it without searching in archive.

 
 diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
 index f5660a9..f62e0eb 100644
 --- a/arch/arm/lib/board.c
 +++ b/arch/arm/lib/board.c
 @@ -347,6 +347,11 @@ void start_armboot (void)
   dataflash_print_info();
  #endif
  
 +#ifdef CONFIG_GENERIC_MMC
 + puts (MMC:   );
 + mmc_initialize (gd-bd);
 +#endif
 +
   /* initialize environment */
   env_relocate ();
  
 @@ -419,11 +424,6 @@ extern void davinci_eth_set_mac_addr (const u_int8_t 
 *addr);
   board_late_init ();
  #endif
  
 -#ifdef CONFIG_GENERIC_MMC
 - puts (MMC:   );
 - mmc_initialize (gd-bd);
 -#endif
 -
  #ifdef CONFIG_BITBANGMII
   bb_miiphy_init();
  #endif

Because it is required to move the initialization of the mmc before
env_relocate(), we need probably to advise that there are some
consequences. If someone implements the mmc support in board_late_init()
(setting a pin multiplexer or something like that, for example), it does
not work. I think we should at least write a comment to advise that the
mmc/sd controller should work after board_init() is called.
However, after a quick check in the arm boards, I have not found a board
that is initializing the mmc controller in board_late_init(). Not sure
for powerpc.

 diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
 index eb89e9e..78f75fb 100644
 --- a/common/cmd_nvedit.c
 +++ b/common/cmd_nvedit.c
 @@ -59,6 +59,7 @@ DECLARE_GLOBAL_DATA_PTR;
  !defined(CONFIG_ENV_IS_IN_FLASH)  \
  !defined(CONFIG_ENV_IS_IN_DATAFLASH)  \
  !defined(CONFIG_ENV_IS_IN_MG_DISK)\
 +!defined(CONFIG_ENV_IS_IN_MMC)   \
  !defined(CONFIG_ENV_IS_IN_NAND)   \
  !defined(CONFIG_ENV_IS_IN_NVRAM)  \
  !defined(CONFIG_ENV_IS_IN_ONENAND)\

From the first version you remove the changes in the error string:

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

I know it is only an error message, but the change seems correct. I have
not found a comment against it ;)


 +#else /* ! ENV_IS_EMBEDDED */
 +env_t *env_ptr;
 +#endif /* ENV_IS_EMBEDDED */

You missed Andy's comment. You have to initialize env_ptr.

 + if (read_env(mmc, CONFIG_ENV_SIZE, CONFIG_ENV_OFFSET, env_ptr))
 + return use_default();

This is still broken, as found by Andy. I cannot find a line where
env_ptr is initialized and then you pass the pointer to the mmc read
function.

Best regards,
Stefano

-- 
=
DENX Software Engineering GmbH, MD: Wolfgang Denk  Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: off...@denx.de
=
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] Save environment data to mmc.

2010-04-29 Thread Lv Terry-R65388 
Hi Babic,

Pls check my comments with keyword [Ty].

Thanks for reviewing the patch.

Thanks~~

Yours
Terry

-Original Message-
From: Stefano Babic [mailto:sba...@denx.de] 
Sent: 2010年4月29日 22:34
To: Lv Terry-R65388
Cc: u-boot@lists.denx.de
Subject: Re: [U-Boot] [PATCH] Save environment data to mmc.

Terry Lv wrote:
 This patch is to save environment data to mmc card.
 It uses interfaces defined in generic mmc.

Hi Terry,

 
 Signed-off-by: Terry Lv r65...@freescale.com
 ---
  arch/arm/lib/board.c |   10 ++--
  arch/powerpc/lib/board.c |   12 ++--
  common/Makefile  |1 +
  common/cmd_nvedit.c  |1 +
  common/env_mmc.c |  154 
 ++
  5 files changed, 167 insertions(+), 11 deletions(-)  create mode 
 100644 common/env_mmc.c

Could you set a version of your patch (something like [PATCH V*] in the 
subject, so it is easier to track changes ? This is the third version, but it 
is difficult to get it without searching in archive.
[Ty]: OK, I will do that, thanks, :-)

 
 diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c index 
 f5660a9..f62e0eb 100644
 --- a/arch/arm/lib/board.c
 +++ b/arch/arm/lib/board.c
 @@ -347,6 +347,11 @@ void start_armboot (void)
   dataflash_print_info();
  #endif
  
 +#ifdef CONFIG_GENERIC_MMC
 + puts (MMC:   );
 + mmc_initialize (gd-bd);
 +#endif
 +
   /* initialize environment */
   env_relocate ();
  
 @@ -419,11 +424,6 @@ extern void davinci_eth_set_mac_addr (const u_int8_t 
 *addr);
   board_late_init ();
  #endif
  
 -#ifdef CONFIG_GENERIC_MMC
 - puts (MMC:   );
 - mmc_initialize (gd-bd);
 -#endif
 -
  #ifdef CONFIG_BITBANGMII
   bb_miiphy_init();
  #endif

Because it is required to move the initialization of the mmc before 
env_relocate(), we need probably to advise that there are some consequences. If 
someone implements the mmc support in board_late_init() (setting a pin 
multiplexer or something like that, for example), it does not work. I think we 
should at least write a comment to advise that the mmc/sd controller should 
work after board_init() is called.
However, after a quick check in the arm boards, I have not found a board that 
is initializing the mmc controller in board_late_init(). Not sure for powerpc.
[Ty]: I have moved mmc initialization in boards that uses generic mmc. I will 
add the comment, thanks.

 diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c index 
 eb89e9e..78f75fb 100644
 --- a/common/cmd_nvedit.c
 +++ b/common/cmd_nvedit.c
 @@ -59,6 +59,7 @@ DECLARE_GLOBAL_DATA_PTR;
  !defined(CONFIG_ENV_IS_IN_FLASH)  \
  !defined(CONFIG_ENV_IS_IN_DATAFLASH)  \
  !defined(CONFIG_ENV_IS_IN_MG_DISK)\
 +!defined(CONFIG_ENV_IS_IN_MMC)   \
  !defined(CONFIG_ENV_IS_IN_NAND)   \
  !defined(CONFIG_ENV_IS_IN_NVRAM)  \
  !defined(CONFIG_ENV_IS_IN_ONENAND)\

From the first version you remove the changes in the error string:

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

I know it is only an error message, but the change seems correct. I have not 
found a comment against it ;)
[Ty]: I will add this.


 +#else /* ! ENV_IS_EMBEDDED */
 +env_t *env_ptr;
 +#endif /* ENV_IS_EMBEDDED */

You missed Andy's comment. You have to initialize env_ptr.
[Ty]: This is a global variable. I think that compiler will set it to zero for 
me. Correct me if I'm wrong. Anyway, I will set it to NULL.

 + if (read_env(mmc, CONFIG_ENV_SIZE, CONFIG_ENV_OFFSET, env_ptr))
 + return use_default();

This is still broken, as found by Andy. I cannot find a line where env_ptr is 
initialized and then you pass the pointer to the mmc read function.
[Ty]: env_ptr is defined in env_mmc.c and the value is assigned in 
env_common.c. Correct me if I'm wrong.

Best regards,
Stefano

--
=
DENX Software Engineering GmbH, MD: Wolfgang Denk  Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: off...@denx.de 
=

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH] Save environment data to mmc.

2010-04-28 Thread Terry Lv 
This patch is to save environment data to mmc card.
It uses interfaces defined in generic mmc.

Signed-off-by: Terry Lv r65...@freescale.com
---
 arch/arm/lib/board.c |   10 ++--
 arch/powerpc/lib/board.c |   12 ++--
 common/Makefile  |1 +
 common/cmd_nvedit.c  |1 +
 common/env_mmc.c |  160 ++
 5 files changed, 173 insertions(+), 11 deletions(-)
 create mode 100644 common/env_mmc.c

diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
index f5660a9..f62e0eb 100644
--- a/arch/arm/lib/board.c
+++ b/arch/arm/lib/board.c
@@ -347,6 +347,11 @@ void start_armboot (void)
dataflash_print_info();
 #endif
 
+#ifdef CONFIG_GENERIC_MMC
+   puts (MMC:   );
+   mmc_initialize (gd-bd);
+#endif
+
/* initialize environment */
env_relocate ();
 
@@ -419,11 +424,6 @@ extern void davinci_eth_set_mac_addr (const u_int8_t 
*addr);
board_late_init ();
 #endif
 
-#ifdef CONFIG_GENERIC_MMC
-   puts (MMC:   );
-   mmc_initialize (gd-bd);
-#endif
-
 #ifdef CONFIG_BITBANGMII
bb_miiphy_init();
 #endif
diff --git a/arch/powerpc/lib/board.c b/arch/powerpc/lib/board.c
index 7b09fb5..1008635 100644
--- a/arch/powerpc/lib/board.c
+++ b/arch/powerpc/lib/board.c
@@ -783,6 +783,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 ();
 
@@ -939,12 +945,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:   );
diff --git a/common/Makefile b/common/Makefile
index dbf7a05..2c37073 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -58,6 +58,7 @@ COBJS-$(CONFIG_ENV_IS_IN_FLASH) += env_embedded.o
 COBJS-$(CONFIG_ENV_IS_IN_NVRAM) += env_embedded.o
 COBJS-$(CONFIG_ENV_IS_IN_FLASH) += env_flash.o
 COBJS-$(CONFIG_ENV_IS_IN_MG_DISK) += env_mgdisk.o
+COBJS-$(CONFIG_ENV_IS_IN_MMC) += env_mmc.o
 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
diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index eb89e9e..78f75fb 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -59,6 +59,7 @@ DECLARE_GLOBAL_DATA_PTR;
 !defined(CONFIG_ENV_IS_IN_FLASH)\
 !defined(CONFIG_ENV_IS_IN_DATAFLASH)\
 !defined(CONFIG_ENV_IS_IN_MG_DISK)  \
+!defined(CONFIG_ENV_IS_IN_MMC)   \
 !defined(CONFIG_ENV_IS_IN_NAND) \
 !defined(CONFIG_ENV_IS_IN_NVRAM)\
 !defined(CONFIG_ENV_IS_IN_ONENAND)  \
diff --git a/common/env_mmc.c b/common/env_mmc.c
new file mode 100644
index 000..202370e
--- /dev/null
+++ b/common/env_mmc.c
@@ -0,0 +1,160 @@
+/*
+ * (C) Copyright 2008-2010 Freescale Semiconductor, Inc.
+
+ * (C) Copyright 2000-2006
+ * Wolfgang Denk, DENX Software Engineering, w...@denx.de.
+ *
+ * (C) Copyright 2001 Sysgo Real-Time Solutions, GmbH www.elinos.com
+ * Andreas Heppel ahep...@sysgo.de
+
+ * 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
+ */
+
+/* #define DEBUG */
+
+#include common.h
+
+#include command.h
+#include environment.h
+#include linux/stddef.h
+#include malloc.h
+#include mmc.h
+
+/* references to names in env_common.c */
+extern uchar default_environment[];
+
+char *env_name_spec = MMC;
+
+#ifdef ENV_IS_EMBEDDED
+extern uchar environment[];
+env_t *env_ptr = (env_t *)(environment[0]);
+#else /* ! ENV_IS_EMBEDDED */
+env_t *env_ptr;
+#endif /* ENV_IS_EMBEDDED */
+
+/* local functions */
+#if !defined(ENV_IS_EMBEDDED)
+static void use_default(void);
+#endif
+
+DECLARE_GLOBAL_DATA_PTR;
+
+#define BLOCK_ALIGN(x) ((x  (0x200 - 1)) \
+? ((x  9) + 1) : (x  9))
+
+uchar env_get_char_spec(int index)
+{
+   return *((uchar *)(gd-env_addr + index));
+}
+
+int env_init(void)
+{
+   /* use default */
+   gd-env_addr =

Re: [U-Boot] [PATCH] Save environment data to mmc.

2010-04-28 Thread Andy Fleming 
On Wed, Apr 28, 2010 at 2:52 AM, Terry Lv r65...@freescale.com wrote:
 This patch is to save environment data to mmc card.
 It uses interfaces defined in generic mmc.

 Signed-off-by: Terry Lv r65...@freescale.com


 diff --git a/common/env_mmc.c b/common/env_mmc.c
 new file mode 100644
 index 000..202370e
 --- /dev/null
 +++ b/common/env_mmc.c
 @@ -0,0 +1,160 @@
 +/*
 + * (C) Copyright 2008-2010 Freescale Semiconductor, Inc.
 +
 + * (C) Copyright 2000-2006
 + * Wolfgang Denk, DENX Software Engineering, w...@denx.de.
 + *
 + * (C) Copyright 2001 Sysgo Real-Time Solutions, GmbH www.elinos.com
 + * Andreas Heppel ahep...@sysgo.de

Are these non-Freescale copyrights here for a reason, or were they
just copied from somewhere?

 +#ifdef ENV_IS_EMBEDDED
 +extern uchar environment[];
 +env_t *env_ptr = (env_t *)(environment[0]);
 +#else /* ! ENV_IS_EMBEDDED */
 +env_t *env_ptr;
 +#endif /* ENV_IS_EMBEDDED */

You should probably initialize env_ptr to NULL, here.

 +
 +DECLARE_GLOBAL_DATA_PTR;
 +
 +#define BLOCK_ALIGN(x) ((x  (0x200 - 1)) \
 +                        ? ((x  9) + 1) : (x  9))

No need to reinvent the wheel, here.  This is the same as:

ALIGN(x, 0x200);

See: include/common.h

Also, are you sure the block size is 0x200?  That's a very common
block size, but the MMC spec allows a whole bunch of alternate ones.
You should use the actual value of mmc-read_bl_len and
mmc-write_bl_len;


 +int env_init(void)
 +{
 +       /* use default */
 +       gd-env_addr = (ulong)default_environment[0];
 +       gd-env_valid = 1;
 +
 +       return 0;
 +}
 +
 +inline int init_mmc_for_env(struct mmc *mmc)

Why is this inlined?  Don't inline a function if there's no good reason.

 +{
 +       if (!mmc) {
 +               puts(No MMC card found\n);
 +               return -1;
 +       }
 +
 +       if (mmc_init(mmc)) {
 +               puts(MMC init failed\n);
 +               return  -1;
 +       }
 +
 +       return 0;
 +}

 +inline int write_env(struct mmc *mmc, unsigned long size,
 +                                               unsigned long offset, const 
 void *buffer)
 +{
 +       uint blk_start = 0, blk_cnt = 0, n = 0;

Don't initialize variables to zero right before assigning them.

 +
 +       blk_start = BLOCK_ALIGN(offset);
 +       blk_cnt   = BLOCK_ALIGN(size);
 +
 +       n = mmc-block_dev.block_write(CONFIG_SYS_MMC_ENV_DEV, blk_start , 
 blk_cnt, (u_char *)buffer);
 +
 +       return (n == blk_cnt) ? 0 : -1;
 +}
 +
 +int saveenv(void)
 +{
 +       struct mmc *mmc = find_mmc_device(CONFIG_SYS_MMC_ENV_DEV);
 +
 +       if (init_mmc_for_env(mmc))
 +               return 1;
 +
 +       printf(Writing to MMC(%d)... , CONFIG_SYS_MMC_ENV_DEV);
 +       if (write_env(mmc, CONFIG_ENV_SIZE, \

You don't need that \.  C does that for you automatically.



 +inline int read_env(struct mmc *mmc, unsigned long size,
 +                                               unsigned long offset, const 
 void *buffer)
 +{
 +       uint blk_start = 0, blk_cnt = 0, n = 0;

Again, don't bother zeroing out values like this.

 +
 +       blk_start = BLOCK_ALIGN(offset);
 +       blk_cnt   = BLOCK_ALIGN(size);
 +
 +       n = mmc-block_dev.block_read(CONFIG_SYS_MMC_ENV_DEV, blk_start, 
 blk_cnt, (uchar *)buffer);
 +
 +       return (n == blk_cnt) ? 0 : -1;
 +}
 +
 +void env_relocate_spec(void)
 +{
 +#if !defined(ENV_IS_EMBEDDED)
 +       struct mmc *mmc = find_mmc_device(CONFIG_SYS_MMC_ENV_DEV);
 +
 +       if (init_mmc_for_env(mmc))
 +               return;
 +
 +       if (read_env(mmc, CONFIG_ENV_SIZE, CONFIG_ENV_OFFSET, env_ptr))
 +               return use_default();


This is broken.  env_ptr hasn't been initialized to a value, yet!
This writes to random memory!  env_ptr needs to be malloc'ed.


Andy
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] Save environment data to mmc.

2010-03-03 Thread Lv Terry-R65388 
Hi Babic,

Thanks for reviewing the patch.

I will send out a new patch to fix the problems you point out.

For setting block numbers for MMC offset, I just don't want env mmc to 
be different. I want users to use it as similiar as other env devices.

For the initalization for ARM and PPC, It will be added for all 
architectures.

Thanks a lot ~~

Yours
Terry

-Original Message-
From: Stefano Babic [mailto:sba...@denx.de] 
Sent: 2010年3月3日 1:03
To: Lv Terry-R65388
Cc: u-boot@lists.denx.de
Subject: Re: [U-Boot] [PATCH] Save environment data to mmc.

Terry Lv wrote:

 diff --git a/common/env_mmc.c b/common/env_mmc.c

 +#include linux/stddef.h
 +#include malloc.h
 +#include mmc.h
 +
 +#if defined(CONFIG_CMD_ENV)  defined(CONFIG_CMD_MMC)

This seems not correct. If not explicitely set, we get a compiler error.
Assuming you has taken this check from env_nand.c, this should be:

#if defined(CONFIG_CMD_SAVEENV)  defined(CONFIG_CMD_MMC)

 +#define CMD_SAVEENV
 +#elif defined(CONFIG_ENV_OFFSET_REDUND) #error Cannot use 
 +CONFIG_ENV_OFFSET_REDUND without CONFIG_CMD_ENV  CONFIG_CMD_MMC

Line too long.

 +#endif
 +
 +#if defined(CONFIG_ENV_SIZE_REDUND)  (CONFIG_ENV_SIZE_REDUND  
 +CONFIG_ENV_SIZE)

Ditto.


 +
 +#ifdef CMD_SAVEENV
 +
 +inline int write_env(struct mmc *mmc, unsigned long size,
 + unsigned long offset, const 
 void *buffer)

Line too long.

 +{
 + uint blk_start = 0, blk_cnt = 0, n = 0;
 +
 + blk_start = (offset % 512) ? ((offset / 512) + 1) : (offset / 512);
 + blk_cnt = (size % 512) ? ((size / 512) + 1) : (size / 512);

The alignment to block size is repeated here and in the read function.
Should not better to set a macro (something like BLOCK_ALIGN) providing the 
required alignment ?

 +int saveenv(void)
 +{
 + struct mmc *mmc = find_mmc_device(0);

Why is the MMC device hard-coded ? At least should be configurable with a 
CONFIG_ option. There are boards with more than one MMC controller and you 
constraint to use always the first one.

 + blk_start = (offset % 512) ? ((offset / 512) + 1) : (offset / 512);

Already said, a macro is much more readable to perform alignment.

 diff --git a/include/environment.h b/include/environment.h

 +#if defined(CONFIG_ENV_IS_IN_MMC)
 +# ifndef CONFIG_ENV_OFFSET
 +#  error Need to define CONFIG_ENV_OFFSET when using CONFIG_ENV_IS_IN_MMC
 +# endif
 +# ifndef CONFIG_ENV_ADDR
 +#  define CONFIG_ENV_ADDR(CONFIG_ENV_OFFSET)
 +# endif
 +# ifndef CONFIG_ENV_OFFSET
 +#  define CONFIG_ENV_OFFSET (CONFIG_ENV_ADDR) # endif # ifdef 
 +CONFIG_ENV_OFFSET_REDUND #  define CONFIG_SYS_REDUNDAND_ENVIRONMENT # 
 +endif # ifdef CONFIG_ENV_IS_EMBEDDED
 +#  define ENV_IS_EMBEDDED1
 +# endif
 +#endif /* CONFIG_ENV_IS_IN_MMC */

You missed Wolfgang's comment. I think also that there is no reason to set 
offset for the MMC and block numbers makes more sense.

 +
  /* Embedded env is only supported for some flash types */  #ifdef 
 CONFIG_ENV_IS_EMBEDDED  # if !defined(CONFIG_ENV_IS_IN_FLASH)  \ 
 diff --git a/lib_arm/board.c b/lib_arm/board.c index 5e3d7f6..f846d0d 
 100644

 +#ifdef CONFIG_GENERIC_MMC
 + puts (MMC:   );
 + mmc_initialize (gd-bd);
 +#endif

 diff --git a/lib_ppc/board.c b/lib_ppc/board.c index 765f97a..9b3f84c 
 100644
 --- a/lib_ppc/board.c
 +++ b/lib_ppc/board.c
 @@ -776,6 +776,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

I am quite confused. You add the initialization only for ARM and PPC.
What about the other architectures ?

I tested your patch on mx51evk, environment is correctly read/written on the SD 
situated on the back.

Regards,
Stefano

--
=
DENX Software Engineering GmbH, MD: Wolfgang Denk  Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: off...@denx.de 
=

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] Save environment data to mmc.

2010-03-03 Thread Liu Hui-R64343 
Hi, Terry,

 -Original Message-
 From: u-boot-boun...@lists.denx.de 
 [mailto:u-boot-boun...@lists.denx.de] On Behalf Of Lv Terry-R65388
 Sent: 2010年3月4日 11:01
 To: Stefano Babic
 Cc: u-boot@lists.denx.de
 Subject: Re: [U-Boot] [PATCH] Save environment data to mmc.
 
 Hi Babic,
 
   Thanks for reviewing the patch.
 
   I will send out a new patch to fix the problems you point out.
 
   For setting block numbers for MMC offset, I just don't 
 want env mmc to be different. I want users to use it as 
 similiar as other env devices.
 
   For the initalization for ARM and PPC, It will be added 
 for all architectures.
 
   Thanks a lot ~~

Had better not put comments on the top of the email, need follow the community 
style. 

 
 Yours
 Terry
 
 -Original Message-
 From: Stefano Babic [mailto:sba...@denx.de]
 Sent: 2010年3月3日 1:03
 To: Lv Terry-R65388
 Cc: u-boot@lists.denx.de
 Subject: Re: [U-Boot] [PATCH] Save environment data to mmc.
 
 Terry Lv wrote:
 
  diff --git a/common/env_mmc.c b/common/env_mmc.c
 
  +#include linux/stddef.h
  +#include malloc.h
  +#include mmc.h
  +
  +#if defined(CONFIG_CMD_ENV)  defined(CONFIG_CMD_MMC)
 
 This seems not correct. If not explicitely set, we get a 
 compiler error.
 Assuming you has taken this check from env_nand.c, this should be:
 
 #if defined(CONFIG_CMD_SAVEENV)  defined(CONFIG_CMD_MMC)
 
  +#define CMD_SAVEENV
  +#elif defined(CONFIG_ENV_OFFSET_REDUND) #error Cannot use 
  +CONFIG_ENV_OFFSET_REDUND without CONFIG_CMD_ENV  CONFIG_CMD_MMC
 
 Line too long.
 
  +#endif
  +
  +#if defined(CONFIG_ENV_SIZE_REDUND)  (CONFIG_ENV_SIZE_REDUND 
  +CONFIG_ENV_SIZE)
 
 Ditto.
 
 
  +
  +#ifdef CMD_SAVEENV
  +
  +inline int write_env(struct mmc *mmc, unsigned long size,
  +   unsigned long 
 offset, const void *buffer)
 
 Line too long.
 
  +{
  +   uint blk_start = 0, blk_cnt = 0, n = 0;
  +
  +   blk_start = (offset % 512) ? ((offset / 512) + 1) : 
 (offset / 512);
  +   blk_cnt = (size % 512) ? ((size / 512) + 1) : (size / 512);
 
 The alignment to block size is repeated here and in the read function.
 Should not better to set a macro (something like BLOCK_ALIGN) 
 providing the required alignment ?
 
  +int saveenv(void)
  +{
  +   struct mmc *mmc = find_mmc_device(0);
 
 Why is the MMC device hard-coded ? At least should be 
 configurable with a CONFIG_ option. There are boards with 
 more than one MMC controller and you constraint to use always 
 the first one.
 
  +   blk_start = (offset % 512) ? ((offset / 512) + 1) : 
 (offset / 512);
 
 Already said, a macro is much more readable to perform alignment.
 
  diff --git a/include/environment.h b/include/environment.h
 
  +#if defined(CONFIG_ENV_IS_IN_MMC)
  +# ifndef CONFIG_ENV_OFFSET
  +#  error Need to define CONFIG_ENV_OFFSET when using 
 CONFIG_ENV_IS_IN_MMC
  +# endif
  +# ifndef CONFIG_ENV_ADDR
  +#  define CONFIG_ENV_ADDR  (CONFIG_ENV_OFFSET)
  +# endif
  +# ifndef CONFIG_ENV_OFFSET
  +#  define CONFIG_ENV_OFFSET (CONFIG_ENV_ADDR) # endif # ifdef 
  +CONFIG_ENV_OFFSET_REDUND #  define 
 CONFIG_SYS_REDUNDAND_ENVIRONMENT # 
  +endif # ifdef CONFIG_ENV_IS_EMBEDDED
  +#  define ENV_IS_EMBEDDED  1
  +# endif
  +#endif /* CONFIG_ENV_IS_IN_MMC */
 
 You missed Wolfgang's comment. I think also that there is no 
 reason to set offset for the MMC and block numbers makes more sense.
 
  +
   /* Embedded env is only supported for some flash types */  #ifdef 
  CONFIG_ENV_IS_EMBEDDED  # if !defined(CONFIG_ENV_IS_IN_FLASH)  \ 
  diff --git a/lib_arm/board.c b/lib_arm/board.c index 
 5e3d7f6..f846d0d
  100644
 
  +#ifdef CONFIG_GENERIC_MMC
  +   puts (MMC:   );
  +   mmc_initialize (gd-bd);
  +#endif
 
  diff --git a/lib_ppc/board.c b/lib_ppc/board.c index 
 765f97a..9b3f84c
  100644
  --- a/lib_ppc/board.c
  +++ b/lib_ppc/board.c
  @@ -776,6 +776,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
 
 I am quite confused. You add the initialization only for ARM and PPC.
 What about the other architectures ?
 
 I tested your patch on mx51evk, environment is correctly 
 read/written on the SD situated on the back.
 
 Regards,
 Stefano
 
 --
 =
 DENX Software Engineering GmbH, MD: Wolfgang Denk  Detlev Zundel
 HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
 Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: 
 off...@denx.de 
 =
 
 ___
 U-Boot mailing list
 U-Boot@lists.denx.de
 http://lists.denx.de/mailman/listinfo/u-boot
 
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH] Save environment data to mmc.

2009-11-18 Thread Terry Lv 
This patch is to save environment data to mmc card.
It uses interfaces defined in generic mmc.

Signed-off-by: Terry Lv r65...@freescale.com
---
 common/Makefile   |1 +
 common/cmd_nvedit.c   |3 +-
 common/env_mmc.c  |  166 +
 include/environment.h |   18 +
 lib_arm/board.c   |   10 ++--
 lib_ppc/board.c   |   12 ++--
 6 files changed, 198 insertions(+), 12 deletions(-)
 create mode 100644 common/env_mmc.c

diff --git a/common/Makefile b/common/Makefile
index a92a75f..5f953b1 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -54,6 +54,7 @@ COBJS-$(CONFIG_ENV_IS_IN_EEPROM) += env_eeprom.o
 COBJS-$(CONFIG_ENV_IS_EMBEDDED) += env_embedded.o
 COBJS-$(CONFIG_ENV_IS_IN_FLASH) += env_flash.o
 COBJS-$(CONFIG_ENV_IS_IN_MG_DISK) += env_mgdisk.o
+COBJS-$(CONFIG_ENV_IS_IN_MMC) += env_mmc.o
 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
diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index 9f8d531..9168241 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_MMC)  \
 !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|MMC|NOWHERE}
 #endif
 
 #define XMK_STR(x) #x
diff --git a/common/env_mmc.c b/common/env_mmc.c
new file mode 100644
index 000..5742db0
--- /dev/null
+++ b/common/env_mmc.c
@@ -0,0 +1,166 @@
+/*
+ * (C) Copyright 2008-2009 Freescale Semiconductor, Inc.
+
+ * (C) Copyright 2000-2006
+ * Wolfgang Denk, DENX Software Engineering, w...@denx.de.
+ *
+ * (C) Copyright 2001 Sysgo Real-Time Solutions, GmbH www.elinos.com
+ * Andreas Heppel ahep...@sysgo.de
+
+ * 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
+ */
+
+/* #define DEBUG */
+
+#include common.h
+
+#include command.h
+#include environment.h
+#include linux/stddef.h
+#include malloc.h
+#include mmc.h
+
+#if defined(CONFIG_CMD_ENV)  defined(CONFIG_CMD_MMC)
+#define CMD_SAVEENV
+#elif defined(CONFIG_ENV_OFFSET_REDUND)
+#error Cannot use CONFIG_ENV_OFFSET_REDUND without CONFIG_CMD_ENV  
CONFIG_CMD_MMC
+#endif
+
+#if defined(CONFIG_ENV_SIZE_REDUND)  (CONFIG_ENV_SIZE_REDUND  
CONFIG_ENV_SIZE)
+#error CONFIG_ENV_SIZE_REDUND should not be less then CONFIG_ENV_SIZE
+#endif
+
+/* references to names in env_common.c */
+extern uchar default_environment[];
+
+char *env_name_spec = MMC;
+
+#ifdef ENV_IS_EMBEDDED
+extern uchar environment[];
+env_t *env_ptr = (env_t *)(environment[0]);
+#else /* ! ENV_IS_EMBEDDED */
+env_t *env_ptr;
+#endif /* ENV_IS_EMBEDDED */
+
+/* local functions */
+#if !defined(ENV_IS_EMBEDDED)
+static void use_default(void);
+#endif
+
+DECLARE_GLOBAL_DATA_PTR;
+
+uchar env_get_char_spec(int index)
+{
+   return *((uchar *)(gd-env_addr + index));
+}
+
+int env_init(void)
+{
+   /* use default */
+   gd-env_addr = (ulong)default_environment[0];
+   gd-env_valid = 1;
+
+   return 0;
+}
+
+inline int init_mmc_for_env(struct mmc *mmc)
+{
+   if (!mmc) {
+   puts(No MMC card found\n);
+   return -1;
+   }
+
+   if (mmc_init(mmc)) {
+   puts(MMC init failed\n);
+   return  -1;
+   }
+
+   return 0;
+}
+
+#ifdef CMD_SAVEENV
+
+inline int write_env(struct mmc *mmc, unsigned long size,
+   unsigned long offset, const 
void *buffer)
+{
+   uint blk_start = 0, blk_cnt = 0, n = 0;
+
+   blk_start = (offset % 512) ? ((offset / 512) + 1) : (offset / 512);
+   blk_cnt = (size % 512) ? ((size / 512) + 1) : (size / 512);
+   n = mmc-block_dev.block_write(0, blk_start , blk_cnt, (u_char 
*)buffer);
+
+   return (n == blk_cnt) ? 0 : -1;
+}
+
+int saveenv(void)
+{
+   struct mmc *mmc = find_mmc_device(0);
+
+   if (init_mmc_for_env(mmc))
+   return 1;
+
+   puts(Writing to MMC... );
+   if (write_env(mmc,

Re: [U-Boot] [PATCH] Save environment data to mmc.

2009-11-05 Thread Wolfgang Denk 
Dear Terry Lv,

In message 12574070132761-git-send-email-r65...@freescale.com you wrote:
 This patch is to save environment data to mmc card.
 It uses interfaces defined in generic mmc.
...
 + if (!crc1_ok  !crc2_ok)
 + gd-env_valid = 0;
 + else if (crc1_ok  !crc2_ok)
 + gd-env_valid = 1;
 + else if (!crc1_ok  crc2_ok)
 + gd-env_valid = 2;
 + else {
 + /* both ok - check serial */
 + if (tmp_env1-flags == 255  tmp_env2-flags == 0)
 + gd-env_valid = 2;
 + else if (tmp_env2-flags == 255  tmp_env1-flags == 0)
 + gd-env_valid = 1;
 + else if (tmp_env1-flags  tmp_env2-flags)
 + gd-env_valid = 1;
 + else if (tmp_env2-flags  tmp_env1-flags)
 + gd-env_valid = 2;
 + else /* flags are equal - almost impossible */
 + gd-env_valid = 1;
 + }

Please check if this logic is really working on MMC - do you really erase
(i. e. fill with 0xFF bytes) the data blocks when erasing?

...
 + if (!mmc) {
 + puts(No MMC card found\n);
 + return;
 + }
 +
 + if (mmc_init(mmc)) {
 + puts(MMC init failed\n);
 + return 1;
 + }

This code repeats a number of times. Make it a (inline?) function?

 + tmp_env1 = (env_t *)malloc(CONFIG_ENV_SIZE);
 + if (!tmp_env1) {
 + puts(Not enough memory!\n);
 + goto use_default;
 + }

Please print where we are, and how many bytes you attempted to
allocate.

 + if (!tmp_env2) {
 + puts(Not enough memory!\n);
 + goto use_default;
 + }

Ditto . Again - repeated code!

 --- a/include/environment.h
 +++ b/include/environment.h
 @@ -94,6 +94,24 @@
  # endif
  #endif /* CONFIG_ENV_IS_IN_MG_DISK */
  
 +#if defined(CONFIG_ENV_IS_IN_MMC)
 +# ifndef CONFIG_ENV_OFFSET
 +#  error Need to define CONFIG_ENV_OFFSET when using CONFIG_ENV_IS_IN_MMC
 +# endif
 +# ifndef CONFIG_ENV_ADDR
 +#  define CONFIG_ENV_ADDR(CONFIG_ENV_OFFSET)
 +# endif
 +# ifndef CONFIG_ENV_OFFSET
 +#  define CONFIG_ENV_OFFSET (CONFIG_ENV_ADDR)
 +# endif
 +# ifdef CONFIG_ENV_OFFSET_REDUND
 +#  define CONFIG_SYS_REDUNDAND_ENVIRONMENT
 +# endif
 +# ifdef CONFIG_ENV_IS_EMBEDDED
 +#  define ENV_IS_EMBEDDED1
 +# endif
 +#endif /* CONFIG_ENV_IS_IN_MMC */

Does this make sense on MMC?

Would it not be better to define block numbers instead?

 +#ifdef CONFIG_GENERIC_MMC
 +puts (MMC:   );
 +mmc_initialize (gd-bd);
 +#endif

Indentation by TABs, please.

Are you sure movin the MMC initialization here has no negative impact
on any of the existing boards?

 --- a/lib_ppc/board.c
 +++ b/lib_ppc/board.c
 @@ -776,6 +776,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

Ditto, for both remarks.

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: w...@denx.de
The only way you could make a happy  marriage  is  by  cuttin'  their
heads  off  as  soon  as  they say `I do', yes? You can't make happi-
ness...   - Terry Pratchett, _Witches Abroad_
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] Save environment data to mmc.

2009-11-05 Thread Lv Terry-R65388 
 Hi Mike,

I will fix the two issues you pointed. Thanks a lot.

Actually, env_mmc.c is written based on env_nand.c in u-boot 1.3.3.

I have modified it according to the latest changes in environment code. 
But seems there's still a lot of issue. Sorry for that.

It works well in our project. So I want to add it to u-boot mainline.

Thanks~~

Yours
Terry

-Original Message-
From: Mike Frysinger [mailto:vap...@gentoo.org] 
Sent: 2009年11月5日 20:10
To: u-boot@lists.denx.de
Cc: Lv Terry-R65388
Subject: Re: [U-Boot] [PATCH] Save environment data to mmc.

On Thursday 05 November 2009 02:43:33 Terry Lv wrote:
 --- a/common/Makefile
 +++ b/common/Makefile
 @@ -58,6 +58,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_MMC) += env_mmc.o

this list is attempting to stay in order according to the CONFIG name, so you 
should add this above the NAND ENV entry

 --- /dev/null
 +++ b/common/env_mmc.c
 @@ -0,0 +1,359 @@
 +#if defined(CONFIG_ENV_IS_IN_MMC) /* Environment is in MMC Flash */

you dont need this line anymore as it's in the Makefile

 +#ifdef CONFIG_INFERNO
 +#error CONFIG_INFERNO not supported yet #endif

is this really needed ?

 +extern int default_environment_size;

this looks like another useless line copy  pasted from whatever file you based 
this on -mike
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH] Save environment data to mmc.

2009-11-04 Thread Terry Lv 
This patch is to save environment data to mmc card.

Signed-off-by: Terry Lv r65...@freescale.com
---
 common/cmd_nvedit.c   |3 +-
 common/env_mmc.c  |  376 +
 include/environment.h |   18 +++
 3 files changed, 396 insertions(+), 1 deletions(-)
 create mode 100644 common/env_mmc.c

diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index 9f8d531..9168241 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_MMC)  \
 !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|MMC|NOWHERE}
 #endif
 
 #define XMK_STR(x) #x
diff --git a/common/env_mmc.c b/common/env_mmc.c
new file mode 100644
index 000..a24d281
--- /dev/null
+++ b/common/env_mmc.c
@@ -0,0 +1,376 @@
+/*
+ * (C) Copyright 2008-2009 Freescale Semiconductor, Inc.
+
+ * (C) Copyright 2000-2006
+ * Wolfgang Denk, DENX Software Engineering, w...@denx.de.
+ *
+ * (C) Copyright 2001 Sysgo Real-Time Solutions, GmbH www.elinos.com
+ * Andreas Heppel ahep...@sysgo.de
+
+ * 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
+ */
+
+/* #define DEBUG */
+
+#include common.h
+
+#if defined(CONFIG_ENV_IS_IN_MMC) /* Environment is in MMC Flash */
+
+#include command.h
+#include environment.h
+#include linux/stddef.h
+#include malloc.h
+#include mmc.h
+
+#if defined(CONFIG_CMD_ENV)  defined(CONFIG_CMD_MMC)
+#define CMD_SAVEENV
+#elif defined(CONFIG_ENV_OFFSET_REDUND)
+#error Cannot use CONFIG_ENV_OFFSET_REDUND without CONFIG_CMD_ENV  
CONFIG_CMD_MMC
+#endif
+
+#if defined(CONFIG_ENV_SIZE_REDUND)  (CONFIG_ENV_SIZE_REDUND  
CONFIG_ENV_SIZE)
+#error CONFIG_ENV_SIZE_REDUND should not be less then CONFIG_ENV_SIZE
+#endif
+
+#ifdef CONFIG_INFERNO
+#error CONFIG_INFERNO not supported yet
+#endif
+
+/* references to names in env_common.c */
+extern uchar default_environment[];
+extern int default_environment_size;
+
+char *env_name_spec = MMC;
+
+#ifdef ENV_IS_EMBEDDED
+extern uchar environment[];
+env_t *env_ptr = (env_t *)(environment[0]);
+#else /* ! ENV_IS_EMBEDDED */
+env_t *env_ptr;
+#endif /* ENV_IS_EMBEDDED */
+
+/* local functions */
+#if !defined(ENV_IS_EMBEDDED)
+static void use_default(void);
+#endif
+
+DECLARE_GLOBAL_DATA_PTR;
+
+uchar env_get_char_spec(int index)
+{
+   return *((uchar *)(gd-env_addr + index));
+}
+
+
+/* this is called before nand_init()
+ * so we can't read Nand to validate env data.
+ * Mark it OK for now. env_relocate() in env_common.c
+ * will call our relocate function which will does
+ * the real validation.
+ *
+ * When using a NAND boot image (like sequoia_nand), the environment
+ * can be embedded or attached to the U-Boot image in NAND flash. This way
+ * the SPL loads not only the U-Boot image from NAND but also the
+ * environment.
+ */
+int env_init(void)
+{
+#if defined(CONFIG_IS_EMBEDDED)
+   size_t total;
+   int crc1_ok = 0, crc2_ok = 0;
+   env_t *tmp_env1, *tmp_env2;
+
+   total = CONFIG_ENV_SIZE;
+
+   tmp_env1 = env_ptr;
+   tmp_env2 = (env_t *)((ulong)env_ptr + CONFIG_ENV_SIZE);
+
+   crc1_ok = (crc32(0, tmp_env1-data, ENV_SIZE) == tmp_env1-crc);
+   crc2_ok = (crc32(0, tmp_env2-data, ENV_SIZE) == tmp_env2-crc);
+
+   if (!crc1_ok  !crc2_ok)
+   gd-env_valid = 0;
+   else if (crc1_ok  !crc2_ok)
+   gd-env_valid = 1;
+   else if (!crc1_ok  crc2_ok)
+   gd-env_valid = 2;
+   else {
+   /* both ok - check serial */
+   if (tmp_env1-flags == 255  tmp_env2-flags == 0)
+   gd-env_valid = 2;
+   else if (tmp_env2-flags == 255  tmp_env1-flags == 0)
+   gd-env_valid = 1;
+   else if (tmp_env1-flags  tmp_env2-flags)
+   gd-env_valid = 1;
+   else if (tmp_env2-flags  tmp_env1-flags)
+   gd-env_valid = 2;
+   else /* flags are equal - almost impossible */

Re: [U-Boot] [PATCH] Save environment data to mmc.

2009-11-04 Thread Lv Terry-R65388 
Sorry, this patch has something wrong.

I'll send out a new one for review.

Yours
Terry

-Original Message-
From: Lv Terry-R65388 
Sent: 2009年11月4日 17:52
To: u-boot@lists.denx.de
Cc: Lv Terry-R65388
Subject: [PATCH] Save environment data to mmc.

This patch is to save environment data to mmc card.

Signed-off-by: Terry Lv r65...@freescale.com
---
 common/cmd_nvedit.c   |3 +-
 common/env_mmc.c  |  376 +
 include/environment.h |   18 +++
 3 files changed, 396 insertions(+), 1 deletions(-)  create mode 100644 
common/env_mmc.c

diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c index 9f8d531..9168241 
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_MMC)  \
 !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|MMC|NOWHERE}
 #endif
 
 #define XMK_STR(x) #x
diff --git a/common/env_mmc.c b/common/env_mmc.c new file mode 100644 index 
000..a24d281
--- /dev/null
+++ b/common/env_mmc.c
@@ -0,0 +1,376 @@
+/*
+ * (C) Copyright 2008-2009 Freescale Semiconductor, Inc.
+
+ * (C) Copyright 2000-2006
+ * Wolfgang Denk, DENX Software Engineering, w...@denx.de.
+ *
+ * (C) Copyright 2001 Sysgo Real-Time Solutions, GmbH www.elinos.com
+ * Andreas Heppel ahep...@sysgo.de
+
+ * 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
+ */
+
+/* #define DEBUG */
+
+#include common.h
+
+#if defined(CONFIG_ENV_IS_IN_MMC) /* Environment is in MMC Flash */
+
+#include command.h
+#include environment.h
+#include linux/stddef.h
+#include malloc.h
+#include mmc.h
+
+#if defined(CONFIG_CMD_ENV)  defined(CONFIG_CMD_MMC) #define 
+CMD_SAVEENV #elif defined(CONFIG_ENV_OFFSET_REDUND) #error Cannot use 
+CONFIG_ENV_OFFSET_REDUND without CONFIG_CMD_ENV  CONFIG_CMD_MMC #endif
+
+#if defined(CONFIG_ENV_SIZE_REDUND)  (CONFIG_ENV_SIZE_REDUND  
+CONFIG_ENV_SIZE) #error CONFIG_ENV_SIZE_REDUND should not be less then 
+CONFIG_ENV_SIZE #endif
+
+#ifdef CONFIG_INFERNO
+#error CONFIG_INFERNO not supported yet #endif
+
+/* references to names in env_common.c */ extern uchar 
+default_environment[]; extern int default_environment_size;
+
+char *env_name_spec = MMC;
+
+#ifdef ENV_IS_EMBEDDED
+extern uchar environment[];
+env_t *env_ptr = (env_t *)(environment[0]); #else /* ! ENV_IS_EMBEDDED 
+*/ env_t *env_ptr; #endif /* ENV_IS_EMBEDDED */
+
+/* local functions */
+#if !defined(ENV_IS_EMBEDDED)
+static void use_default(void);
+#endif
+
+DECLARE_GLOBAL_DATA_PTR;
+
+uchar env_get_char_spec(int index)
+{
+   return *((uchar *)(gd-env_addr + index)); }
+
+
+/* this is called before nand_init()
+ * so we can't read Nand to validate env data.
+ * Mark it OK for now. env_relocate() in env_common.c
+ * will call our relocate function which will does
+ * the real validation.
+ *
+ * When using a NAND boot image (like sequoia_nand), the environment
+ * can be embedded or attached to the U-Boot image in NAND flash. This 
+way
+ * the SPL loads not only the U-Boot image from NAND but also the
+ * environment.
+ */
+int env_init(void)
+{
+#if defined(CONFIG_IS_EMBEDDED)
+   size_t total;
+   int crc1_ok = 0, crc2_ok = 0;
+   env_t *tmp_env1, *tmp_env2;
+
+   total = CONFIG_ENV_SIZE;
+
+   tmp_env1 = env_ptr;
+   tmp_env2 = (env_t *)((ulong)env_ptr + CONFIG_ENV_SIZE);
+
+   crc1_ok = (crc32(0, tmp_env1-data, ENV_SIZE) == tmp_env1-crc);
+   crc2_ok = (crc32(0, tmp_env2-data, ENV_SIZE) == tmp_env2-crc);
+
+   if (!crc1_ok  !crc2_ok)
+   gd-env_valid = 0;
+   else if (crc1_ok  !crc2_ok)
+   gd-env_valid = 1;
+   else if (!crc1_ok  crc2_ok)
+   gd-env_valid = 2;
+   else {
+   /* both ok - check serial */
+   if (tmp_env1-flags == 255  tmp_env2-flags == 0)
+   gd-env_valid = 2;
+   else if (tmp_env2-flags == 255  tmp_env1-flags == 0)
+   gd-env_valid

[U-Boot] [PATCH] Save environment data to mmc.

2009-11-04 Thread Terry Lv 
This patch is to save environment data to mmc card.
It uses interfaces defined in generic mmc.

Signed-off-by: Terry Lv r65...@freescale.com
---
 common/cmd_nvedit.c   |3 +-
 common/env_mmc.c  |  359 +
 include/environment.h |   18 +++
 lib_arm/board.c   |   10 +-
 lib_ppc/board.c   |   12 +-
 5 files changed, 390 insertions(+), 12 deletions(-)
 create mode 100644 common/env_mmc.c

diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index 9f8d531..9168241 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_MMC)  \
 !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|MMC|NOWHERE}
 #endif
 
 #define XMK_STR(x) #x
diff --git a/common/env_mmc.c b/common/env_mmc.c
new file mode 100644
index 000..9ef7c40
--- /dev/null
+++ b/common/env_mmc.c
@@ -0,0 +1,359 @@
+/*
+ * (C) Copyright 2008-2009 Freescale Semiconductor, Inc.
+
+ * (C) Copyright 2000-2006
+ * Wolfgang Denk, DENX Software Engineering, w...@denx.de.
+ *
+ * (C) Copyright 2001 Sysgo Real-Time Solutions, GmbH www.elinos.com
+ * Andreas Heppel ahep...@sysgo.de
+
+ * 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
+ */
+
+/* #define DEBUG */
+
+#include common.h
+
+#if defined(CONFIG_ENV_IS_IN_MMC) /* Environment is in MMC Flash */
+
+#include command.h
+#include environment.h
+#include linux/stddef.h
+#include malloc.h
+#include mmc.h
+
+#if defined(CONFIG_CMD_ENV)  defined(CONFIG_CMD_MMC)
+#define CMD_SAVEENV
+#elif defined(CONFIG_ENV_OFFSET_REDUND)
+#error Cannot use CONFIG_ENV_OFFSET_REDUND without CONFIG_CMD_ENV  
CONFIG_CMD_MMC
+#endif
+
+#if defined(CONFIG_ENV_SIZE_REDUND)  (CONFIG_ENV_SIZE_REDUND  
CONFIG_ENV_SIZE)
+#error CONFIG_ENV_SIZE_REDUND should not be less then CONFIG_ENV_SIZE
+#endif
+
+#ifdef CONFIG_INFERNO
+#error CONFIG_INFERNO not supported yet
+#endif
+
+/* references to names in env_common.c */
+extern uchar default_environment[];
+extern int default_environment_size;
+
+char *env_name_spec = MMC;
+
+#ifdef ENV_IS_EMBEDDED
+extern uchar environment[];
+env_t *env_ptr = (env_t *)(environment[0]);
+#else /* ! ENV_IS_EMBEDDED */
+env_t *env_ptr;
+#endif /* ENV_IS_EMBEDDED */
+
+/* local functions */
+#if !defined(ENV_IS_EMBEDDED)
+static void use_default(void);
+#endif
+
+DECLARE_GLOBAL_DATA_PTR;
+
+uchar env_get_char_spec(int index)
+{
+   return *((uchar *)(gd-env_addr + index));
+}
+
+int env_init(void)
+{
+#if defined(CONFIG_IS_EMBEDDED)
+   size_t total;
+   int crc1_ok = 0, crc2_ok = 0;
+   env_t *tmp_env1, *tmp_env2;
+
+   total = CONFIG_ENV_SIZE;
+
+   tmp_env1 = env_ptr;
+   tmp_env2 = (env_t *)((ulong)env_ptr + CONFIG_ENV_SIZE);
+
+   crc1_ok = (crc32(0, tmp_env1-data, ENV_SIZE) == tmp_env1-crc);
+   crc2_ok = (crc32(0, tmp_env2-data, ENV_SIZE) == tmp_env2-crc);
+
+   if (!crc1_ok  !crc2_ok)
+   gd-env_valid = 0;
+   else if (crc1_ok  !crc2_ok)
+   gd-env_valid = 1;
+   else if (!crc1_ok  crc2_ok)
+   gd-env_valid = 2;
+   else {
+   /* both ok - check serial */
+   if (tmp_env1-flags == 255  tmp_env2-flags == 0)
+   gd-env_valid = 2;
+   else if (tmp_env2-flags == 255  tmp_env1-flags == 0)
+   gd-env_valid = 1;
+   else if (tmp_env1-flags  tmp_env2-flags)
+   gd-env_valid = 1;
+   else if (tmp_env2-flags  tmp_env1-flags)
+   gd-env_valid = 2;
+   else /* flags are equal - almost impossible */
+   gd-env_valid = 1;
+   }
+
+   if (gd-env_valid == 1)
+   env_ptr = tmp_env1;
+   else if (gd-env_valid == 2)
+   env_ptr = tmp_env2;
+
+#else /* ENV_IS_EMBEDDED */
+   gd-env_addr  = (ulong)default_environment[0];
+   gd-env_valid = 1;
+
+#endif /* ENV_IS_EMBEDDED */
+
+

Re: [U-Boot] [PATCH] Save environment data to mmc.

2009-11-04 Thread Mike Frysinger 
On Wednesday 04 November 2009 05:02:44 Terry Lv wrote:
 This patch is to save environment data to mmc card.
 It uses interfaces defined in generic mmc.
 
 Signed-off-by: Terry Lv r65...@freescale.com
 ---
  common/cmd_nvedit.c   |3 +-
  common/env_mmc.c
  include/environment.h | 
  lib_arm/board.c   |   10 +-
  lib_ppc/board.c   |   12 +-
  5 files changed, 390 insertions(+), 12 deletions(-)
  create mode 100644 common/env_mmc.c

you never added env_mmc to the Makefile

 --- /dev/null
 +++ b/common/env_mmc.c
 +#if defined(CONFIG_ENV_IS_IN_MMC) /* Environment is in MMC Flash */

have this config check be in the Makefile, then it isnt needed in the source 
as the build system will only build it when needed
-mike


signature.asc
Description: This is a digitally signed message part.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot