diff mbox

[U-Boot,v7,4/5] imx6: add spl in the header file

Message ID CAOMZO5Dzg5CFFoqB5fwD9U+9XX3WLNgBQUegP_wbzMSN2hATAA@mail.gmail.com
State Not Applicable
Headers show

Commit Message

Fabio Estevam Nov. 13, 2014, 12:49 p.m. UTC
On Thu, Nov 13, 2014 at 10:41 AM, Fabio Estevam <festevam@gmail.com> wrote:
> Hi John,
>
> On Thu, Nov 13, 2014 at 6:58 AM, Stefano Babic <sbabic@denx.de> wrote:
>
>>>  #define CONFIG_LOADADDR                        0x12000000
>>> +#ifndef CONFIG_SYS_TEXT_BASE
>>>  #define CONFIG_SYS_TEXT_BASE           0x17800000
>>> +#endif
>>>
>>
>> Why is that needed ? SPL does not use it, because it use
>> CONFIG_SPL_TEXT_BASE, and this is defined in imx6_spl.h. And you do not
>> redefine it, meaning that overwriting the value is not necessary at all,
>> as we expect.
>
> Stefano is right.
>
> You can simply remove this piece:
>
> --- a/include/configs/mx6sabre_common.h
> +++ b/include/configs/mx6sabre_common.h
> @@ -95,9 +95,6 @@
>  #define CONFIG_BOOTDELAY               1
>
>  #define CONFIG_LOADADDR                        0x12000000
> -#ifndef CONFIG_SYS_TEXT_BASE
> -#define CONFIG_SYS_TEXT_BASE           0x17800000
> -#endif

Sorry, I meant to remove only the ifdef:

http://lists.denx.de/mailman/listinfo/u-boot

Comments

Fabio Estevam Nov. 13, 2014, 2:39 p.m. UTC | #1
Hi Stefano,

On Thu, Nov 13, 2014 at 10:49 AM, Fabio Estevam <festevam@gmail.com> wrote:

> Sorry, I meant to remove only the ifdef:
>
> --- a/include/configs/mx6sabre_common.h
> +++ b/include/configs/mx6sabre_common.h
> @@ -95,9 +95,7 @@
>  #define CONFIG_BOOTDELAY               1
>
>  #define CONFIG_LOADADDR                        0x12000000
> -#ifndef CONFIG_SYS_TEXT_BASE
>  #define CONFIG_SYS_TEXT_BASE           0x17800000
> -#endif

Should John resend the series again with this change or can you remove
the ifndef manually while applying it?

Thanks,

Fabio Estevam
Stefano Babic Nov. 13, 2014, 2:42 p.m. UTC | #2
Hi Fabio, John,

On 13/11/2014 15:39, Fabio Estevam wrote:
> Hi Stefano,
> 
> On Thu, Nov 13, 2014 at 10:49 AM, Fabio Estevam <festevam@gmail.com> wrote:
> 
>> Sorry, I meant to remove only the ifdef:
>>
>> --- a/include/configs/mx6sabre_common.h
>> +++ b/include/configs/mx6sabre_common.h
>> @@ -95,9 +95,7 @@
>>  #define CONFIG_BOOTDELAY               1
>>
>>  #define CONFIG_LOADADDR                        0x12000000
>> -#ifndef CONFIG_SYS_TEXT_BASE
>>  #define CONFIG_SYS_TEXT_BASE           0x17800000
>> -#endif
> 
> Should John resend the series again with this change or can you remove
> the ifndef manually while applying it?
> 

We already agree about the changes. There is no need to resend the
patchset, I will fix it when I apply the patchset.

Best regards,
Stefano Babic
Fabio Estevam Nov. 13, 2014, 2:43 p.m. UTC | #3
On Thu, Nov 13, 2014 at 12:42 PM, Stefano Babic <sbabic@denx.de> wrote:

> We already agree about the changes. There is no need to resend the
> patchset, I will fix it when I apply the patchset.

Excellent! Thanks, Stefano
John Tobias Nov. 13, 2014, 3:47 p.m. UTC | #4
Hi Stefano,

On Thu, Nov 13, 2014 at 4:49 AM, Fabio Estevam <festevam@gmail.com> wrote:
> On Thu, Nov 13, 2014 at 10:41 AM, Fabio Estevam <festevam@gmail.com> wrote:
>> Hi John,
>>
>> On Thu, Nov 13, 2014 at 6:58 AM, Stefano Babic <sbabic@denx.de> wrote:
>>
>>>>  #define CONFIG_LOADADDR                        0x12000000
>>>> +#ifndef CONFIG_SYS_TEXT_BASE
>>>>  #define CONFIG_SYS_TEXT_BASE           0x17800000
>>>> +#endif
>>>>
>>>
>>> Why is that needed ? SPL does not use it, because it use
>>> CONFIG_SPL_TEXT_BASE, and this is defined in imx6_spl.h. And you do not
>>> redefine it, meaning that overwriting the value is not necessary at all,
>>> as we expect.

The CONFIG_SYS_TEXT_BASE was defined in imx6_spl.h and I was seeing
previously some warning related to the duplication of the said macro.

I thought it would be great to remove the warning messages so that is why
I added the macro.

Regards,

john


>>
>> Stefano is right.
>>
>> You can simply remove this piece:
>>
>> --- a/include/configs/mx6sabre_common.h
>> +++ b/include/configs/mx6sabre_common.h
>> @@ -95,9 +95,6 @@
>>  #define CONFIG_BOOTDELAY               1
>>
>>  #define CONFIG_LOADADDR                        0x12000000
>> -#ifndef CONFIG_SYS_TEXT_BASE
>> -#define CONFIG_SYS_TEXT_BASE           0x17800000
>> -#endif
>
> Sorry, I meant to remove only the ifdef:
>
> --- a/include/configs/mx6sabre_common.h
> +++ b/include/configs/mx6sabre_common.h
> @@ -95,9 +95,7 @@
>  #define CONFIG_BOOTDELAY               1
>
>  #define CONFIG_LOADADDR                        0x12000000
> -#ifndef CONFIG_SYS_TEXT_BASE
>  #define CONFIG_SYS_TEXT_BASE           0x17800000
> -#endif
John Tobias Nov. 13, 2014, 3:49 p.m. UTC | #5
Hi Fabio, Stefano,

Thanks for reviewing my work and for accepting it.

Regards,

John

On Thu, Nov 13, 2014 at 6:42 AM, Stefano Babic <sbabic@denx.de> wrote:
> Hi Fabio, John,
>
> On 13/11/2014 15:39, Fabio Estevam wrote:
>> Hi Stefano,
>>
>> On Thu, Nov 13, 2014 at 10:49 AM, Fabio Estevam <festevam@gmail.com> wrote:
>>
>>> Sorry, I meant to remove only the ifdef:
>>>
>>> --- a/include/configs/mx6sabre_common.h
>>> +++ b/include/configs/mx6sabre_common.h
>>> @@ -95,9 +95,7 @@
>>>  #define CONFIG_BOOTDELAY               1
>>>
>>>  #define CONFIG_LOADADDR                        0x12000000
>>> -#ifndef CONFIG_SYS_TEXT_BASE
>>>  #define CONFIG_SYS_TEXT_BASE           0x17800000
>>> -#endif
>>
>> Should John resend the series again with this change or can you remove
>> the ifndef manually while applying it?
>>
>
> We already agree about the changes. There is no need to resend the
> patchset, I will fix it when I apply the patchset.
>
> 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-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de
> =====================================================================
Stefano Babic Nov. 13, 2014, 4:29 p.m. UTC | #6
Hi John,

On 13/11/2014 16:47, John Tobias wrote:

> The CONFIG_SYS_TEXT_BASE was defined in imx6_spl.h and I was seeing
> previously some warning related to the duplication of the said macro.
> 
> I thought it would be great to remove the warning messages so that is why
> I added the macro.

Maybe it was in a development phase where, or it was added as extra
option by compiling. I confirm that it compile fine without warnings
after removing.

Best regards,
Stefano Babic
diff mbox

Patch

--- a/include/configs/mx6sabre_common.h
+++ b/include/configs/mx6sabre_common.h
@@ -95,9 +95,7 @@ 
 #define CONFIG_BOOTDELAY               1

 #define CONFIG_LOADADDR                        0x12000000
-#ifndef CONFIG_SYS_TEXT_BASE
 #define CONFIG_SYS_TEXT_BASE           0x17800000
-#endif
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de