[U-Boot] env_mmc: Fix crashing bug encountered after enabling ARM relocation

[U-Boot] env_mmc: Fix crashing bug encountered after enabling ARM relocation

[Steve Sakoman]

The crash was occuring in env_relocate because it was being called prior
to mmc_initialize.  This patch moves the MMC initialization earlier in
the init process.

This patch also cleans up the env_relocate_spec code in env_mmc.c

Developed jointly with Stefano Babic

Signed-off-by: Steve Sakoman <steve.sakoman@linaro.org>
---

diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
index 5f2dfd0..0e2f129 100644
--- a/arch/arm/lib/board.c
+++ b/arch/arm/lib/board.c
@@ -779,6 +779,11 @@ void board_init_r (gd_t *id, ulong dest_addr)
 	onenand_init();
 #endif
 
+#ifdef CONFIG_GENERIC_MMC
+       puts ("MMC:   ");
+       mmc_initialize (bd);
+#endif
+
 #ifdef CONFIG_HAS_DATAFLASH
 	AT91F_DataflashInit();
 	dataflash_print_info();
@@ -854,11 +859,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/common/env_mmc.c b/common/env_mmc.c
index cc288d4..d443ff5 100644
--- a/common/env_mmc.c
+++ b/common/env_mmc.c
@@ -129,18 +129,21 @@ inline int read_env(struct mmc *mmc, unsigned long size,
 void env_relocate_spec(void)
 {
 #if !defined(ENV_IS_EMBEDDED)
+       char buf[CONFIG_ENV_SIZE];
+
 	struct mmc *mmc = find_mmc_device(CONFIG_SYS_MMC_ENV_DEV);
 
-	if (init_mmc_for_env(mmc))
+	if (init_mmc_for_env(mmc)) {
+		use_default();
 		return;
+	}
 
-	if (read_env(mmc, CONFIG_ENV_SIZE, CONFIG_ENV_OFFSET, env_ptr))
-		return use_default();
-
-	if (crc32(0, env_ptr->data, ENV_SIZE) != env_ptr->crc)
-		return use_default();
+	if (read_env(mmc, CONFIG_ENV_SIZE, CONFIG_ENV_OFFSET, buf)) {
+		use_default();
+		return;
+	}
 
-	gd->env_valid = 1;
+	env_import(buf, 1);
 #endif
 }
 

[U-Boot] env_mmc: Fix crashing bug encountered after enabling ARM relocation

[Paulraj, Sandeep]

> 
> The crash was occuring in env_relocate because it was being called prior
> to mmc_initialize.  This patch moves the MMC initialization earlier in
> the init process.
> 
> This patch also cleans up the env_relocate_spec code in env_mmc.c
> 
> Developed jointly with Stefano Babic

Fine; but is a Signed-off-by or Tested-by/Acked-by required from Stefano then?

Also this patch touch ARM and MMC code so can't add to u-boot-ti unless Wolfgang is ok.

> 
> Signed-off-by: Steve Sakoman <steve.sakoman@linaro.org>
> ---
> 
> diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
> index 5f2dfd0..0e2f129 100644
> --- a/arch/arm/lib/board.c
> +++ b/arch/arm/lib/board.c
> @@ -779,6 +779,11 @@ void board_init_r (gd_t *id, ulong dest_addr)
>  	onenand_init();
>  #endif
> 
> +#ifdef CONFIG_GENERIC_MMC
> +       puts ("MMC:   ");
> +       mmc_initialize (bd);
> +#endif
> +
>  #ifdef CONFIG_HAS_DATAFLASH
>  	AT91F_DataflashInit();
>  	dataflash_print_info();
> @@ -854,11 +859,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/common/env_mmc.c b/common/env_mmc.c
> index cc288d4..d443ff5 100644
> --- a/common/env_mmc.c
> +++ b/common/env_mmc.c
> @@ -129,18 +129,21 @@ inline int read_env(struct mmc *mmc, unsigned long
> size,
>  void env_relocate_spec(void)
>  {
>  #if !defined(ENV_IS_EMBEDDED)
> +       char buf[CONFIG_ENV_SIZE];
> +
>  	struct mmc *mmc = find_mmc_device(CONFIG_SYS_MMC_ENV_DEV);
> 
> -	if (init_mmc_for_env(mmc))
> +	if (init_mmc_for_env(mmc)) {
> +		use_default();
>  		return;
> +	}
> 
> -	if (read_env(mmc, CONFIG_ENV_SIZE, CONFIG_ENV_OFFSET, env_ptr))
> -		return use_default();
> -
> -	if (crc32(0, env_ptr->data, ENV_SIZE) != env_ptr->crc)
> -		return use_default();
> +	if (read_env(mmc, CONFIG_ENV_SIZE, CONFIG_ENV_OFFSET, buf)) {
> +		use_default();
> +		return;
> +	}
> 
> -	gd->env_valid = 1;
> +	env_import(buf, 1);
>  #endif
>  }

[U-Boot] env_mmc: Fix crashing bug encountered after enabling ARM relocation

[stefano babic]

Am 09.10.2010 23:49, schrieb Paulraj, Sandeep:
> 
>>
>> The crash was occuring in env_relocate because it was being called prior
>> to mmc_initialize.  This patch moves the MMC initialization earlier in
>> the init process.
>>
>> This patch also cleans up the env_relocate_spec code in env_mmc.c
>>
>> Developed jointly with Stefano Babic
> 
> Fine; but is a Signed-off-by or Tested-by/Acked-by required from Stefano then?

Steve can add my acked-by, as I discussed with him how to fix the issue.

Acked-by: Stefano Babic <sbabic@denx.de>

> 
> Also this patch touch ARM and MMC code so can't add to u-boot-ti unless Wolfgang is ok.

Agree. The patch touches both ARM code and common code, and Wolfgang is
maintainer for both of them.

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
=====================================================================

[U-Boot] env_mmc: Fix crashing bug encountered after enabling ARM relocation

[Sergei Shtylyov]

Hello.

On 09-10-2010 1:49, Steve Sakoman wrote:

> The crash was occuring in env_relocate because it was being called prior
> to mmc_initialize.  This patch moves the MMC initialization earlier in
> the init process.

> This patch also cleans up the env_relocate_spec code in env_mmc.c

> Developed jointly with Stefano Babic

> Signed-off-by: Steve Sakoman<steve.sakoman@linaro.org>
> ---

> diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
> index 5f2dfd0..0e2f129 100644
> --- a/arch/arm/lib/board.c
> +++ b/arch/arm/lib/board.c
> @@ -779,6 +779,11 @@ void board_init_r (gd_t *id, ulong dest_addr)
>   	onenand_init();
>   #endif
>
> +#ifdef CONFIG_GENERIC_MMC
> +       puts ("MMC:   ");
> +       mmc_initialize (bd);

    This would cause checkpatch.pl to complain -- no spaces allowed before (.

WBR, Sergei

[U-Boot] env_mmc: Fix crashing bug encountered after enabling ARM relocation

[Steve Sakoman]

On Sun, Oct 10, 2010 at 4:46 AM, Sergei Shtylyov <sshtylyov@mvista.com> wrote:
> Hello.
>
> On 09-10-2010 1:49, Steve Sakoman wrote:
>
>> The crash was occuring in env_relocate because it was being called prior
>> to mmc_initialize. ?This patch moves the MMC initialization earlier in
>> the init process.
>
>> This patch also cleans up the env_relocate_spec code in env_mmc.c
>
>> Developed jointly with Stefano Babic
>
>> Signed-off-by: Steve Sakoman<steve.sakoman@linaro.org>
>> ---
>
>> diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
>> index 5f2dfd0..0e2f129 100644
>> --- a/arch/arm/lib/board.c
>> +++ b/arch/arm/lib/board.c
>> @@ -779,6 +779,11 @@ void board_init_r (gd_t *id, ulong dest_addr)
>> ? ? ? onenand_init();
>> ? #endif
>>
>> +#ifdef CONFIG_GENERIC_MMC
>> + ? ? ? puts ("MMC: ? ");
>> + ? ? ? mmc_initialize (bd);
>
> ? ?This would cause checkpatch.pl to complain -- no spaces allowed before (.

I  will resubmit a V2 with that change as well as Stefano's Signed-off-by

Steve

[U-Boot] env_mmc: Fix crashing bug encountered after enabling ARM relocation

[Sergei Shtylyov]

Hello.

On 10-10-2010 17:42, Steve Sakoman wrote:

>>> The crash was occuring in env_relocate because it was being called prior
>>> to mmc_initialize.  This patch moves the MMC initialization earlier in
>>> the init process.

>>> This patch also cleans up the env_relocate_spec code in env_mmc.c

>>> Developed jointly with Stefano Babic

>>> Signed-off-by: Steve Sakoman<steve.sakoman@linaro.org>
>>> ---

>>> diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
>>> index 5f2dfd0..0e2f129 100644
>>> --- a/arch/arm/lib/board.c
>>> +++ b/arch/arm/lib/board.c
>>> @@ -779,6 +779,11 @@ void board_init_r (gd_t *id, ulong dest_addr)
>>>        onenand_init();
>>>    #endif
>>>
>>> +#ifdef CONFIG_GENERIC_MMC
>>> +       puts ("MMC:   ");
>>> +       mmc_initialize (bd);
>>
>>     This would cause checkpatch.pl to complain -- no spaces allowed before (.

> I  will resubmit a V2 with that change as well as Stefano's Signed-off-by

    You've posted V2 with Stefano's ACK... and this line unchanged. :-)

> Steve

WBR, Sergei

[U-Boot] env_mmc: Fix crashing bug encountered after enabling ARM relocation

[Steve Sakoman]

On Sun, Oct 10, 2010 at 7:52 AM, Sergei Shtylyov <sshtylyov@mvista.com> wrote:
> Hello.
>
> On 10-10-2010 17:42, Steve Sakoman wrote:
>
>>>> The crash was occuring in env_relocate because it was being called prior
>>>> to mmc_initialize. ?This patch moves the MMC initialization earlier in
>>>> the init process.
>
>>>> This patch also cleans up the env_relocate_spec code in env_mmc.c
>
>>>> Developed jointly with Stefano Babic
>
>>>> Signed-off-by: Steve Sakoman<steve.sakoman@linaro.org>
>>>> ---
>
>>>> diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
>>>> index 5f2dfd0..0e2f129 100644
>>>> --- a/arch/arm/lib/board.c
>>>> +++ b/arch/arm/lib/board.c
>>>> @@ -779,6 +779,11 @@ void board_init_r (gd_t *id, ulong dest_addr)
>>>> ? ? ? onenand_init();
>>>> ? #endif
>>>>
>>>> +#ifdef CONFIG_GENERIC_MMC
>>>> + ? ? ? puts ("MMC: ? ");
>>>> + ? ? ? mmc_initialize (bd);
>>>
>>> ? ?This would cause checkpatch.pl to complain -- no spaces allowed before
>>> (.
>
>> I ?will resubmit a V2 with that change as well as Stefano's Signed-off-by
>
> ? You've posted V2 with Stefano's ACK... and this line unchanged. :-)

???

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

It looks changed to me!

Steve

[U-Boot] env_mmc: Fix crashing bug encountered after enabling ARM relocation

[Sergei Shtylyov]

On 10-10-2010 19:47, Steve Sakoman wrote:

>>>>> The crash was occuring in env_relocate because it was being called prior
>>>>> to mmc_initialize.  This patch moves the MMC initialization earlier in
>>>>> the init process.

>>>>> This patch also cleans up the env_relocate_spec code in env_mmc.c

>>>>> Developed jointly with Stefano Babic

>>>>> Signed-off-by: Steve Sakoman<steve.sakoman@linaro.org>
>>>>> ---

>>>>> diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
>>>>> index 5f2dfd0..0e2f129 100644
>>>>> --- a/arch/arm/lib/board.c
>>>>> +++ b/arch/arm/lib/board.c
>>>>> @@ -779,6 +779,11 @@ void board_init_r (gd_t *id, ulong dest_addr)
>>>>>        onenand_init();
>>>>>    #endif
>>>>>
>>>>> +#ifdef CONFIG_GENERIC_MMC
>>>>> +       puts ("MMC:   ");
>>>>> +       mmc_initialize (bd);

>>>>     This would cause checkpatch.pl to complain -- no spaces allowed before
>>>> (.

>>> I  will resubmit a V2 with that change as well as Stefano's Signed-off-by

>>    You've posted V2 with Stefano's ACK... and this line unchanged. :-)

> ???

> +#ifdef CONFIG_GENERIC_MMC
> +       puts("MMC:   ");
> +       mmc_initialize (bd);
> +#endif
> +

> It looks changed to me!

    Indeed, I have ovelooked that first line under #ifdef was changed. But not 
the second. ;-)

> Steve

WBR, Sergei

[U-Boot] env_mmc: Fix crashing bug encountered after enabling ARM relocation

[Steve Sakoman]

On Sun, Oct 10, 2010 at 8:53 AM, Sergei Shtylyov <sshtylyov@mvista.com> wrote:
> On 10-10-2010 19:47, Steve Sakoman wrote:
>
>>>>>> The crash was occuring in env_relocate because it was being called
>>>>>> prior
>>>>>> to mmc_initialize.  This patch moves the MMC initialization earlier in
>>>>>> the init process.
>
>>>>>> This patch also cleans up the env_relocate_spec code in env_mmc.c
>
>>>>>> Developed jointly with Stefano Babic
>
>>>>>> Signed-off-by: Steve Sakoman<steve.sakoman@linaro.org>
>>>>>> ---
>
>>>>>> diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
>>>>>> index 5f2dfd0..0e2f129 100644
>>>>>> --- a/arch/arm/lib/board.c
>>>>>> +++ b/arch/arm/lib/board.c
>>>>>> @@ -779,6 +779,11 @@ void board_init_r (gd_t *id, ulong dest_addr)
>>>>>>       onenand_init();
>>>>>>   #endif
>>>>>>
>>>>>> +#ifdef CONFIG_GENERIC_MMC
>>>>>> +       puts ("MMC:   ");
>>>>>> +       mmc_initialize (bd);
>
>>>>>    This would cause checkpatch.pl to complain -- no spaces allowed
>>>>> before
>>>>> (.
>
>>>> I  will resubmit a V2 with that change as well as Stefano's
>>>> Signed-off-by
>
>>>   You've posted V2 with Stefano's ACK... and this line unchanged. :-)
>
>> ???
>
>> +#ifdef CONFIG_GENERIC_MMC
>> +       puts("MMC:   ");
>> +       mmc_initialize (bd);
>> +#endif
>> +
>
>> It looks changed to me!
>
>   Indeed, I have ovelooked that first line under #ifdef was changed. But not
> the second. ;-)

Sigh.  You are correct!

I hate these older files that were done when "space before paren" was
the rule.  You have to be really careful when cutting and pasting, and
even then you end up with a source file that is totally inconsistent
in spacing unless you go and fix the entire file!

I will submit a v3.

Steve