* [U-Boot] __mmc_get_env_addr() is not being called? @ 2011-12-16 5:54 Shawn Guo 2011-12-16 9:31 ` Stefano Babic 2011-12-16 9:33 ` Fabio Estevam 0 siblings, 2 replies; 8+ messages in thread From: Shawn Guo @ 2011-12-16 5:54 UTC (permalink / raw) To: u-boot 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. -- Regards, Shawn ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [U-Boot] __mmc_get_env_addr() is not being called? 2011-12-16 5:54 [U-Boot] __mmc_get_env_addr() is not being called? Shawn Guo @ 2011-12-16 9:31 ` Stefano Babic 2011-12-19 5:21 ` Jason Liu 2011-12-16 9:33 ` Fabio Estevam 1 sibling, 1 reply; 8+ messages in thread From: Stefano Babic @ 2011-12-16 9:31 UTC (permalink / raw) To: u-boot 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 -- ===================================================================== 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 at denx.de ===================================================================== ^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] __mmc_get_env_addr() is not being called? 2011-12-16 9:31 ` Stefano Babic @ 2011-12-19 5:21 ` Jason Liu 0 siblings, 0 replies; 8+ messages in thread From: Jason Liu @ 2011-12-19 5:21 UTC (permalink / raw) To: u-boot 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 at denx.de > ===================================================================== > _______________________________________________ > U-Boot mailing list > U-Boot at lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot ^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] __mmc_get_env_addr() is not being called? 2011-12-16 5:54 [U-Boot] __mmc_get_env_addr() is not being called? Shawn Guo 2011-12-16 9:31 ` Stefano Babic @ 2011-12-16 9:33 ` Fabio Estevam 2011-12-16 9:42 ` Stefano Babic 1 sibling, 1 reply; 8+ messages in thread From: Fabio Estevam @ 2011-12-16 9:33 UTC (permalink / raw) To: 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=97039ab98c551c7860bc0977d684ef686159e0d7 which breaks non CONFIG_FSL_ESDHC users. Regards, Fabio Estevam ^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] __mmc_get_env_addr() is not being called? 2011-12-16 9:33 ` Fabio Estevam @ 2011-12-16 9:42 ` Stefano Babic 2011-12-18 17:56 ` Kumar Gala 0 siblings, 1 reply; 8+ messages in thread From: Stefano Babic @ 2011-12-16 9:42 UTC (permalink / raw) To: u-boot 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=97039ab98c551c7860bc0977d684ef686159e0d7 > > 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 -- ===================================================================== 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 at denx.de ===================================================================== ^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] __mmc_get_env_addr() is not being called? 2011-12-16 9:42 ` Stefano Babic @ 2011-12-18 17:56 ` Kumar Gala 2011-12-19 6:37 ` Stefano Babic 0 siblings, 1 reply; 8+ messages in thread From: Kumar Gala @ 2011-12-18 17:56 UTC (permalink / raw) To: u-boot 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=97039ab98c551c7860bc0977d684ef686159e0d7 >> >> 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] __mmc_get_env_addr() is not being called? 2011-12-18 17:56 ` Kumar Gala @ 2011-12-19 6:37 ` Stefano Babic 2011-12-19 6:53 ` Jason Hui 0 siblings, 1 reply; 8+ messages in thread From: Stefano Babic @ 2011-12-19 6:37 UTC (permalink / raw) To: u-boot 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=97039ab98c551c7860bc0977d684ef686159e0d7 >>> >>> 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 -- ===================================================================== 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 at denx.de ===================================================================== ^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] __mmc_get_env_addr() is not being called? 2011-12-19 6:37 ` Stefano Babic @ 2011-12-19 6:53 ` Jason Hui 0 siblings, 0 replies; 8+ messages in thread From: Jason Hui @ 2011-12-19 6:53 UTC (permalink / raw) To: u-boot 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=97039ab98c551c7860bc0977d684ef686159e0d7 >>>> >>>> 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 at 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 at denx.de > ===================================================================== ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2011-12-19 6:53 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-12-16 5:54 [U-Boot] __mmc_get_env_addr() is not being called? Shawn Guo 2011-12-16 9:31 ` Stefano Babic 2011-12-19 5:21 ` Jason Liu 2011-12-16 9:33 ` Fabio Estevam 2011-12-16 9:42 ` Stefano Babic 2011-12-18 17:56 ` Kumar Gala 2011-12-19 6:37 ` Stefano Babic 2011-12-19 6:53 ` Jason Hui
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox