[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.
--
Regards,
Shawn
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] __mmc_get_env_addr() is not being called?
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: off...@denx.de
=
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] __mmc_get_env_addr() is not being called?
On Fri, Dec 16, 2011 at 3:54 AM, Shawn Guo 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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] __mmc_get_env_addr() is not being called?
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: off...@denx.de = ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] __mmc_get_env_addr() is not being called?
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 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] __mmc_get_env_addr() is not being called?
2011/12/16 Stefano Babic :
> 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:
11144 include/configs/efikamx.h <>
#define CONFIG_FSL_ESDHC
12 86 include/configs/mx51evk.h <>
#define CONFIG_FSL_ESDHC
13 58 include/configs/mx53ard.h <>
#define CONFIG_FSL_ESDHC
14 68 include/configs/mx53evk.h <>
#define CONFIG_FSL_ESDHC
15 51 include/configs/mx53loco.h <>
#define CONFIG_FSL_ESDHC
16 58 include/configs/mx53smd.h <>
#define CONFIG_FSL_ESDHC
17 48 include/configs/mx6qarm2.h <>
#define CONFIG_FSL_ESDHC
18 48 include/configs/mx6qsabrelite.h <>
#define CONFIG_FSL_ESDHC
20105 include/configs/vision2.h <>
#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: 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
Re: [U-Boot] __mmc_get_env_addr() is not being called?
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: off...@denx.de = ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] __mmc_get_env_addr() is not being called?
On Mon, Dec 19, 2011 at 2:37 PM, Stefano Babic 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.hu...@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: off...@denx.de > = ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
