[U-Boot] __mmc_get_env_addr() is not being called?
Hi,
I'm running v2011.12-rc1 (with one missing usdhc patch applied) on mx6qarm2 board, and seeing a problem. When I install u-boot on a blank SD card, it boots fine with messge "*** Warning - bad CRC, using default environment" seen as expected. But once I run command 'save' to write envs to the card, the card stop booting.
I tracked the issue a little bit, and found __mmc_get_env_addr() does not get called at all. Therefore when I run 'save' for the first time, the envs are written to offset 0 of the card, which in turn overwrites the boot image.
I have not figured out why it does. But the following change fixes the problem for me.
diff --git a/common/env_mmc.c b/common/env_mmc.c index 8441c77..3832fe4 100644 --- a/common/env_mmc.c +++ b/common/env_mmc.c @@ -46,13 +46,11 @@ DECLARE_GLOBAL_DATA_PTR; #define CONFIG_ENV_OFFSET 0 #endif
-static int __mmc_get_env_addr(struct mmc *mmc, u32 *env_addr) +static int mmc_get_env_addr(struct mmc *mmc, u32 *env_addr) { *env_addr = CONFIG_ENV_OFFSET; return 0; } -int mmc_get_env_addr(struct mmc *mmc, u32 *env_addr) - __attribute__((weak, alias("__mmc_get_env_addr")));
Any comment on why the change fixes the problem is appreciated.
On 16/12/2011 06:54, Shawn Guo wrote:
Hi,
Hi Shawn,
I'm running v2011.12-rc1 (with one missing usdhc patch applied) on mx6qarm2 board, and seeing a problem. When I install u-boot on a blank SD card, it boots fine with messge "*** Warning - bad CRC, using default environment" seen as expected. But once I run command 'save' to write envs to the card, the card stop booting.
I tracked the issue a little bit, and found __mmc_get_env_addr() does not get called at all. Therefore when I run 'save' for the first time, the envs are written to offset 0 of the card, which in turn overwrites the boot image.
I have not figured out why it does. But the following change fixes the problem for me.
diff --git a/common/env_mmc.c b/common/env_mmc.c index 8441c77..3832fe4 100644 --- a/common/env_mmc.c +++ b/common/env_mmc.c @@ -46,13 +46,11 @@ DECLARE_GLOBAL_DATA_PTR; #define CONFIG_ENV_OFFSET 0 #endif
-static int __mmc_get_env_addr(struct mmc *mmc, u32 *env_addr) +static int mmc_get_env_addr(struct mmc *mmc, u32 *env_addr) { *env_addr = CONFIG_ENV_OFFSET; return 0; } -int mmc_get_env_addr(struct mmc *mmc, u32 *env_addr)
__attribute__((weak, alias("__mmc_get_env_addr")));Any comment on why the change fixes the problem is appreciated.
The function is declared weak, this means that a specific architecture could implement its own version. The reason why it does not work is that there is a specific implementation in board/freescale/sdhc_boot.c, and the mmc_get_env_addr() in this file is taken instead of the default when CONFIG_ENV_IS_IN_MMC (as in our case) is set. Instead of your proposal, we should surround mmc_get_env_addr() in sdhc_boot.c with something like #ifndef CONFIG_MX6Q, or check if we can reuse this function, that really does not use CONFIG_ENV_OFFSET at all.
I put Jason (board maintainer) in CC - Jason, what do mind ? Do you get the same issue when you tested the board ?
Best regards, Stefano Babic
2011/12/16 Stefano Babic sbabic@denx.de:
On 16/12/2011 06:54, Shawn Guo wrote:
Hi,
Hi Shawn,
I'm running v2011.12-rc1 (with one missing usdhc patch applied) on mx6qarm2 board, and seeing a problem. When I install u-boot on a blank SD card, it boots fine with messge "*** Warning - bad CRC, using default environment" seen as expected. But once I run command 'save' to write envs to the card, the card stop booting.
I tracked the issue a little bit, and found __mmc_get_env_addr() does not get called at all. Therefore when I run 'save' for the first time, the envs are written to offset 0 of the card, which in turn overwrites the boot image.
I have not figured out why it does. But the following change fixes the problem for me.
diff --git a/common/env_mmc.c b/common/env_mmc.c index 8441c77..3832fe4 100644 --- a/common/env_mmc.c +++ b/common/env_mmc.c @@ -46,13 +46,11 @@ DECLARE_GLOBAL_DATA_PTR; #define CONFIG_ENV_OFFSET 0 #endif
-static int __mmc_get_env_addr(struct mmc *mmc, u32 *env_addr) +static int mmc_get_env_addr(struct mmc *mmc, u32 *env_addr) { *env_addr = CONFIG_ENV_OFFSET; return 0; } -int mmc_get_env_addr(struct mmc *mmc, u32 *env_addr)
- __attribute__((weak, alias("__mmc_get_env_addr")));
Any comment on why the change fixes the problem is appreciated.
The function is declared weak, this means that a specific architecture could implement its own version. The reason why it does not work is that there is a specific implementation in board/freescale/sdhc_boot.c, and the mmc_get_env_addr() in this file is taken instead of the default when CONFIG_ENV_IS_IN_MMC (as in our case) is set. Instead of your proposal, we should surround mmc_get_env_addr() in sdhc_boot.c with something like #ifndef CONFIG_MX6Q, or check if we can reuse this function, that really does not use CONFIG_ENV_OFFSET at all.
I put Jason (board maintainer) in CC - Jason, what do mind ? Do you get the same issue when you tested the board ?
Stefano, I saw the issue. Fabio's patch can fix the i.mx28evk issue, but can't fix the following platforms:
11 144 include/configs/efikamx.h <<CONFIG_FSL_ESDHC>> #define CONFIG_FSL_ESDHC 12 86 include/configs/mx51evk.h <<CONFIG_FSL_ESDHC>> #define CONFIG_FSL_ESDHC 13 58 include/configs/mx53ard.h <<CONFIG_FSL_ESDHC>> #define CONFIG_FSL_ESDHC 14 68 include/configs/mx53evk.h <<CONFIG_FSL_ESDHC>> #define CONFIG_FSL_ESDHC 15 51 include/configs/mx53loco.h <<CONFIG_FSL_ESDHC>> #define CONFIG_FSL_ESDHC 16 58 include/configs/mx53smd.h <<CONFIG_FSL_ESDHC>> #define CONFIG_FSL_ESDHC 17 48 include/configs/mx6qarm2.h <<CONFIG_FSL_ESDHC>> #define CONFIG_FSL_ESDHC 18 48 include/configs/mx6qsabrelite.h <<CONFIG_FSL_ESDHC>> #define CONFIG_FSL_ESDHC 20 105 include/configs/vision2.h <<CONFIG_FSL_ESDHC>> #define CONFIG_FSL_ESDHC
The commit 97039ab98 (env_mmc: Allow board code to override the environment address) break the i.mx5/6 boards support.
BR, Jason
Best regards, Stefano Babic
--
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: office@denx.de ===================================================================== _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
On Fri, Dec 16, 2011 at 3:54 AM, Shawn Guo shawn.guo@freescale.com wrote:
Hi,
I'm running v2011.12-rc1 (with one missing usdhc patch applied) on mx6qarm2 board, and seeing a problem. When I install u-boot on a blank SD card, it boots fine with messge "*** Warning - bad CRC, using default environment" seen as expected. But once I run command 'save' to write envs to the card, the card stop booting.
This is the same behavior I saw on mx28evk as well and I sent this patch to fix it: http://lists.denx.de/pipermail/u-boot/2011-November/111448.html
I tracked the issue a little bit, and found __mmc_get_env_addr() does not get called at all. Therefore when I run 'save' for the first time, the envs are written to offset 0 of the card, which in turn overwrites the boot image.
I have not figured out why it does. But the following change fixes the problem for me.
diff --git a/common/env_mmc.c b/common/env_mmc.c index 8441c77..3832fe4 100644 --- a/common/env_mmc.c +++ b/common/env_mmc.c @@ -46,13 +46,11 @@ DECLARE_GLOBAL_DATA_PTR; #define CONFIG_ENV_OFFSET 0 #endif
-static int __mmc_get_env_addr(struct mmc *mmc, u32 *env_addr) +static int mmc_get_env_addr(struct mmc *mmc, u32 *env_addr) { *env_addr = CONFIG_ENV_OFFSET; return 0; } -int mmc_get_env_addr(struct mmc *mmc, u32 *env_addr)
- __attribute__((weak, alias("__mmc_get_env_addr")));
Any comment on why the change fixes the problem is appreciated.
The weak function was introduced by this commit: http://git.denx.de/?p=u-boot.git;a=commitdiff;h=97039ab98c551c7860bc0977d684...
which breaks non CONFIG_FSL_ESDHC users.
Regards,
Fabio Estevam
On 16/12/2011 10:33, Fabio Estevam wrote:
The weak function was introduced by this commit: http://git.denx.de/?p=u-boot.git;a=commitdiff;h=97039ab98c551c7860bc0977d684...
which breaks non CONFIG_FSL_ESDHC users.
Ok, I see - the patch is more related to Freescale SOCs as to the MMC, and it is sometimes unknown who whould pick it up.
Andy, not CONFIG_FSL_ESDHC means IMX users - if you do not complain, I merge it into u-boot-imx.
Thanks, Stefano
On Dec 16, 2011, at 3:42 AM, Stefano Babic wrote:
On 16/12/2011 10:33, Fabio Estevam wrote:
The weak function was introduced by this commit: http://git.denx.de/?p=u-boot.git;a=commitdiff;h=97039ab98c551c7860bc0977d684...
which breaks non CONFIG_FSL_ESDHC users.
Ok, I see - the patch is more related to Freescale SOCs as to the MMC, and it is sometimes unknown who whould pick it up.
Andy, not CONFIG_FSL_ESDHC means IMX users - if you do not complain, I merge it into u-boot-imx.
Thanks, Stefano
If I'm reading the patch correct it will break things (or I'm not sure which patch you're referring to). How would the PPC specific version ever get called?
- k
On 18/12/2011 18:56, Kumar Gala wrote:
On Dec 16, 2011, at 3:42 AM, Stefano Babic wrote:
On 16/12/2011 10:33, Fabio Estevam wrote:
The weak function was introduced by this commit: http://git.denx.de/?p=u-boot.git;a=commitdiff;h=97039ab98c551c7860bc0977d684...
which breaks non CONFIG_FSL_ESDHC users.
Ok, I see - the patch is more related to Freescale SOCs as to the MMC, and it is sometimes unknown who whould pick it up.
Andy, not CONFIG_FSL_ESDHC means IMX users - if you do not complain, I merge it into u-boot-imx.
Thanks, Stefano
If I'm reading the patch correct it will break things (or I'm not sure which patch you're referring to). How would the PPC specific version ever get called?
Yes, I was too optimistic with the patch, I have myself seen that the patch then break PowerPC boards.
Reading sdhc_boot.c, this file is not strictly related to PowerPC or IMX hardware, but where u-boot image is stored on PowerPC code.
This is like a specific feature, but there is not a specific CONFIG_ for it. In fact, in board/freescale/common/Makefile the file is always compiled if CONFIG_RAMBOOT_PBL (not significant for imx) is not set if the environment is in MMC:
ifndef CONFIG_RAMBOOT_PBL COBJS-$(CONFIG_ENV_IS_IN_MMC) += sdhc_boot.o endif
What about to have a specific CONFIG_FSL_* for this file ? It can be defined for PowerPC boards, and it will not for IMX. Any thoughts ?
Stefano
On Mon, Dec 19, 2011 at 2:37 PM, Stefano Babic sbabic@denx.de wrote:
On 18/12/2011 18:56, Kumar Gala wrote:
On Dec 16, 2011, at 3:42 AM, Stefano Babic wrote:
On 16/12/2011 10:33, Fabio Estevam wrote:
The weak function was introduced by this commit: http://git.denx.de/?p=u-boot.git;a=commitdiff;h=97039ab98c551c7860bc0977d684...
which breaks non CONFIG_FSL_ESDHC users.
Ok, I see - the patch is more related to Freescale SOCs as to the MMC, and it is sometimes unknown who whould pick it up.
Andy, not CONFIG_FSL_ESDHC means IMX users - if you do not complain, I merge it into u-boot-imx.
Thanks, Stefano
If I'm reading the patch correct it will break things (or I'm not sure which patch you're referring to). How would the PPC specific version ever get called?
Yes, I was too optimistic with the patch, I have myself seen that the patch then break PowerPC boards.
Reading sdhc_boot.c, this file is not strictly related to PowerPC or IMX hardware, but where u-boot image is stored on PowerPC code.
This is like a specific feature, but there is not a specific CONFIG_ for it. In fact, in board/freescale/common/Makefile the file is always compiled if CONFIG_RAMBOOT_PBL (not significant for imx) is not set if the environment is in MMC:
ifndef CONFIG_RAMBOOT_PBL COBJS-$(CONFIG_ENV_IS_IN_MMC) += sdhc_boot.o endif
What about to have a specific CONFIG_FSL_* for this file ? It can be defined for PowerPC boards, and it will not for IMX. Any thoughts ?
I agree with this option. Also Cced: Chang-Ming.Huang@freescale.com
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: office@denx.de =====================================================================
participants (6)
-
Fabio Estevam -
Jason Hui -
Jason Liu -
Kumar Gala -
Shawn Guo -
Stefano Babic