diff mbox

[U-Boot,v2,1/3] arm: add CONFIG_MACH_TYPE setting and documentation

Message ID CANuQgHGOY58qNS2Pv3uKaiKCOYa1140YNwkWPER-LY9oKrrdYw@mail.gmail.com
State Not Applicable
Headers show

Commit Message

Chander Kashyap July 27, 2011, 10:31 a.m. UTC
dear Igor,


On 14 July 2011 21:15, Igor Grinberg <grinberg@compulab.co.il> wrote:
> CONFIG_MACH_TYPE is used to set the machine type number in the
> common arm code instead of setting it in the board code.
> Boards with dynamically discoverable machine types can still set the
> machine type number in the board code.
>
> Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
> ---
> v2:     Document the option as mandatory.
>        Move the bi_arch_number setting to board_init_f()
>
>  README               |   10 ++++++++++
>  arch/arm/lib/board.c |    4 ++++
>  2 files changed, 14 insertions(+), 0 deletions(-)
>
> diff --git a/README b/README
> index 446966d..0b6802d 100644
> --- a/README
> +++ b/README
> @@ -442,6 +442,16 @@ The following options need to be configured:
>                crash. This is needed for buggy hardware (uc101) where
>                no pull down resistor is connected to the signal IDE5V_DD7.
>
> +               CONFIG_MACH_TYPE        [relevant for ARM only][mandatory]
> +
> +               This setting is mandatory for all boards that have only one
> +               machine type and must be used to specify the machine type
> +               number as it appears in the ARM machine registry
> +               (see http://www.arm.linux.org.uk/developer/machines/).
> +               Only boards that have multiple machine types supported
> +               in a single configuration file and the machine type is
> +               runtime discoverable, do not have to use this setting.
> +
>  - vxWorks boot parameters:
>
>                bootvx constructs a valid bootline using the following
> diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
> index 169dfeb..9901694 100644
> --- a/arch/arm/lib/board.c
> +++ b/arch/arm/lib/board.c
> @@ -281,6 +281,10 @@ void board_init_f (ulong bootflag)
>
>        gd->mon_len = _bss_end_ofs;
>
> +#ifdef CONFIG_MACH_TYPE
> +       gd->bd->bi_arch_number = CONFIG_MACH_TYPE; /* board id for Linux */
> +#endif
> +
bd structure is not initialised by this time.
It leads to u-boot hanging for my board.
I fixed this problem but modifying it. Below is the patch attached for the same.

>        for (init_fnc_ptr = init_sequence; *init_fnc_ptr; ++init_fnc_ptr) {
>                if ((*init_fnc_ptr)() != 0) {
>                        hang ();
> --
> 1.7.3.4
>
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>

From d8df2f0ca9f08470c0cb88307fea4a66f41147a5 Mon Sep 17 00:00:00 2001
From: Chander Kashyap <chander.kashyap@linaro.org>
Date: Wed, 27 Jul 2011 15:10:59 +0530
Subject: [PATCH] ARM: Fix wrong initialisation of bi_arch_number

bi_arch_number is initialised using
@arch/arm/lib/board.c
\#ifdef CONFIG_MACH_TYPE
        gd->bd->bi_arch_number = CONFIG_MACH_TYPE; /* board id for Linux */
\#endif

bd structure is not intialized by this time.
This leads to u-boot hanging when CONFIG_MACH_TYPE is defined.

Signed-off-by: Chander Kashyap <chander.kashyap@linaro.org>
---
 arch/arm/lib/board.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

Comments

Igor Grinberg July 27, 2011, 1:04 p.m. UTC | #1
On 07/27/11 13:31, Chander Kashyap wrote:
> dear Igor,
>
>
> On 14 July 2011 21:15, Igor Grinberg <grinberg@compulab.co.il> wrote:
>> CONFIG_MACH_TYPE is used to set the machine type number in the
>> common arm code instead of setting it in the board code.
>> Boards with dynamically discoverable machine types can still set the
>> machine type number in the board code.
>>
>> Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
>> ---
>> v2:     Document the option as mandatory.
>>        Move the bi_arch_number setting to board_init_f()
>>
>>  README               |   10 ++++++++++
>>  arch/arm/lib/board.c |    4 ++++
>>  2 files changed, 14 insertions(+), 0 deletions(-)
>>
>> diff --git a/README b/README
>> index 446966d..0b6802d 100644
>> --- a/README
>> +++ b/README
>> @@ -442,6 +442,16 @@ The following options need to be configured:
>>                crash. This is needed for buggy hardware (uc101) where
>>                no pull down resistor is connected to the signal IDE5V_DD7.
>>
>> +               CONFIG_MACH_TYPE        [relevant for ARM only][mandatory]
>> +
>> +               This setting is mandatory for all boards that have only one
>> +               machine type and must be used to specify the machine type
>> +               number as it appears in the ARM machine registry
>> +               (see http://www.arm.linux.org.uk/developer/machines/).
>> +               Only boards that have multiple machine types supported
>> +               in a single configuration file and the machine type is
>> +               runtime discoverable, do not have to use this setting.
>> +
>>  - vxWorks boot parameters:
>>
>>                bootvx constructs a valid bootline using the following
>> diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
>> index 169dfeb..9901694 100644
>> --- a/arch/arm/lib/board.c
>> +++ b/arch/arm/lib/board.c
>> @@ -281,6 +281,10 @@ void board_init_f (ulong bootflag)
>>
>>        gd->mon_len = _bss_end_ofs;
>>
>> +#ifdef CONFIG_MACH_TYPE
>> +       gd->bd->bi_arch_number = CONFIG_MACH_TYPE; /* board id for Linux */
>> +#endif
>> +
> bd structure is not initialised by this time.
> It leads to u-boot hanging for my board.
> I fixed this problem but modifying it. Below is the patch attached for the same.

Then how does it work for boards setting the gd->bd->bi_arch_number
in board_early_init_f() function?

>>        for (init_fnc_ptr = init_sequence; *init_fnc_ptr; ++init_fnc_ptr) {
>>                if ((*init_fnc_ptr)() != 0) {
>>                        hang ();
>> --
>> 1.7.3.4
>>
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot@lists.denx.de
>> http://lists.denx.de/mailman/listinfo/u-boot
>>
> >From d8df2f0ca9f08470c0cb88307fea4a66f41147a5 Mon Sep 17 00:00:00 2001
> From: Chander Kashyap <chander.kashyap@linaro.org>
> Date: Wed, 27 Jul 2011 15:10:59 +0530
> Subject: [PATCH] ARM: Fix wrong initialisation of bi_arch_number
>
> bi_arch_number is initialised using
> @arch/arm/lib/board.c
> \#ifdef CONFIG_MACH_TYPE
>         gd->bd->bi_arch_number = CONFIG_MACH_TYPE; /* board id for Linux */
> \#endif
>
> bd structure is not intialized by this time.
> This leads to u-boot hanging when CONFIG_MACH_TYPE is defined.
>
> Signed-off-by: Chander Kashyap <chander.kashyap@linaro.org>
> ---
>  arch/arm/lib/board.c |    7 +++----
>  1 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
> index bcbf697..98a9bcc 100644
> --- a/arch/arm/lib/board.c
> +++ b/arch/arm/lib/board.c
> @@ -281,10 +281,6 @@ void board_init_f (ulong bootflag)
>
>  	gd->mon_len = _bss_end_ofs;
>
> -#ifdef CONFIG_MACH_TYPE
> -	gd->bd->bi_arch_number = CONFIG_MACH_TYPE; /* board id for Linux */
> -#endif
> -
>  	for (init_fnc_ptr = init_sequence; *init_fnc_ptr; ++init_fnc_ptr) {
>  		if ((*init_fnc_ptr)() != 0) {
>  			hang ();
> @@ -380,6 +376,9 @@ void board_init_f (ulong bootflag)
>  	gd->bd = bd;
>  	debug ("Reserving %zu Bytes for Board Info at: %08lx\n",
>  			sizeof (bd_t), addr_sp);
> +#ifdef CONFIG_MACH_TYPE
> +	gd->bd->bi_arch_number = CONFIG_MACH_TYPE; /* board id for Linux */
> +#endif

This is problematic...
There are boards that rely on this setting in early init function calls.
For them it should be set before the init_sequence array is run.
I will rethink this once again.

Thanks for testing...
Chander Kashyap July 28, 2011, 6:41 a.m. UTC | #2
Dear Igor,


On 27 July 2011 18:34, Igor Grinberg <grinberg@compulab.co.il> wrote:
> On 07/27/11 13:31, Chander Kashyap wrote:
>> dear Igor,
>>
>>
>> On 14 July 2011 21:15, Igor Grinberg <grinberg@compulab.co.il> wrote:
>>> CONFIG_MACH_TYPE is used to set the machine type number in the
>>> common arm code instead of setting it in the board code.
>>> Boards with dynamically discoverable machine types can still set the
>>> machine type number in the board code.
>>>
>>> Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
>>> ---
>>> v2:     Document the option as mandatory.
>>>        Move the bi_arch_number setting to board_init_f()
>>>
>>>  README               |   10 ++++++++++
>>>  arch/arm/lib/board.c |    4 ++++
>>>  2 files changed, 14 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/README b/README
>>> index 446966d..0b6802d 100644
>>> --- a/README
>>> +++ b/README
>>> @@ -442,6 +442,16 @@ The following options need to be configured:
>>>                crash. This is needed for buggy hardware (uc101) where
>>>                no pull down resistor is connected to the signal IDE5V_DD7.
>>>
>>> +               CONFIG_MACH_TYPE        [relevant for ARM only][mandatory]
>>> +
>>> +               This setting is mandatory for all boards that have only one
>>> +               machine type and must be used to specify the machine type
>>> +               number as it appears in the ARM machine registry
>>> +               (see http://www.arm.linux.org.uk/developer/machines/).
>>> +               Only boards that have multiple machine types supported
>>> +               in a single configuration file and the machine type is
>>> +               runtime discoverable, do not have to use this setting.
>>> +
>>>  - vxWorks boot parameters:
>>>
>>>                bootvx constructs a valid bootline using the following
>>> diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
>>> index 169dfeb..9901694 100644
>>> --- a/arch/arm/lib/board.c
>>> +++ b/arch/arm/lib/board.c
>>> @@ -281,6 +281,10 @@ void board_init_f (ulong bootflag)
>>>
>>>        gd->mon_len = _bss_end_ofs;
>>>
>>> +#ifdef CONFIG_MACH_TYPE
>>> +       gd->bd->bi_arch_number = CONFIG_MACH_TYPE; /* board id for Linux */
>>> +#endif
>>> +
>> bd structure is not initialised by this time.
>> It leads to u-boot hanging for my board.
>> I fixed this problem but modifying it. Below is the patch attached for the same.
>
> Then how does it work for boards setting the gd->bd->bi_arch_number
> in board_early_init_f() function?
can you please point out any board which sets in board_early_init_f() ?
>
>>>        for (init_fnc_ptr = init_sequence; *init_fnc_ptr; ++init_fnc_ptr) {
>>>                if ((*init_fnc_ptr)() != 0) {
>>>                        hang ();
>>> --
>>> 1.7.3.4
>>>
>>> _______________________________________________
>>> U-Boot mailing list
>>> U-Boot@lists.denx.de
>>> http://lists.denx.de/mailman/listinfo/u-boot
>>>
>> >From d8df2f0ca9f08470c0cb88307fea4a66f41147a5 Mon Sep 17 00:00:00 2001
>> From: Chander Kashyap <chander.kashyap@linaro.org>
>> Date: Wed, 27 Jul 2011 15:10:59 +0530
>> Subject: [PATCH] ARM: Fix wrong initialisation of bi_arch_number
>>
>> bi_arch_number is initialised using
>> @arch/arm/lib/board.c
>> \#ifdef CONFIG_MACH_TYPE
>>         gd->bd->bi_arch_number = CONFIG_MACH_TYPE; /* board id for Linux */
>> \#endif
>>
>> bd structure is not intialized by this time.
>> This leads to u-boot hanging when CONFIG_MACH_TYPE is defined.
>>
>> Signed-off-by: Chander Kashyap <chander.kashyap@linaro.org>
>> ---
>>  arch/arm/lib/board.c |    7 +++----
>>  1 files changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
>> index bcbf697..98a9bcc 100644
>> --- a/arch/arm/lib/board.c
>> +++ b/arch/arm/lib/board.c
>> @@ -281,10 +281,6 @@ void board_init_f (ulong bootflag)
>>
>>       gd->mon_len = _bss_end_ofs;
>>
>> -#ifdef CONFIG_MACH_TYPE
>> -     gd->bd->bi_arch_number = CONFIG_MACH_TYPE; /* board id for Linux */
>> -#endif
>> -
>>       for (init_fnc_ptr = init_sequence; *init_fnc_ptr; ++init_fnc_ptr) {
>>               if ((*init_fnc_ptr)() != 0) {
>>                       hang ();
>> @@ -380,6 +376,9 @@ void board_init_f (ulong bootflag)
>>       gd->bd = bd;
>>       debug ("Reserving %zu Bytes for Board Info at: %08lx\n",
>>                       sizeof (bd_t), addr_sp);
>> +#ifdef CONFIG_MACH_TYPE
>> +     gd->bd->bi_arch_number = CONFIG_MACH_TYPE; /* board id for Linux */
>> +#endif
>
> This is problematic...
> There are boards that rely on this setting in early init function calls.
> For them it should be set before the init_sequence array is run.
> I will rethink this once again.
as per my understanding board_init_f() is the first initialisation call.

>
> Thanks for testing...
>
>
> --
> Regards,
> Igor.
>
>
Igor Grinberg July 28, 2011, 7:59 a.m. UTC | #3
On 07/28/11 09:41, Chander Kashyap wrote:
> Dear Igor,
>
>
> On 27 July 2011 18:34, Igor Grinberg <grinberg@compulab.co.il> wrote:
>> On 07/27/11 13:31, Chander Kashyap wrote:
>>> dear Igor,
>>>
>>>
>>> On 14 July 2011 21:15, Igor Grinberg <grinberg@compulab.co.il> wrote:
>>>> CONFIG_MACH_TYPE is used to set the machine type number in the
>>>> common arm code instead of setting it in the board code.
>>>> Boards with dynamically discoverable machine types can still set the
>>>> machine type number in the board code.
>>>>
>>>> Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
>>>> ---
>>>> v2:     Document the option as mandatory.
>>>>        Move the bi_arch_number setting to board_init_f()
>>>>
>>>>  README               |   10 ++++++++++
>>>>  arch/arm/lib/board.c |    4 ++++
>>>>  2 files changed, 14 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/README b/README
>>>> index 446966d..0b6802d 100644
>>>> --- a/README
>>>> +++ b/README
>>>> @@ -442,6 +442,16 @@ The following options need to be configured:
>>>>                crash. This is needed for buggy hardware (uc101) where
>>>>                no pull down resistor is connected to the signal IDE5V_DD7.
>>>>
>>>> +               CONFIG_MACH_TYPE        [relevant for ARM only][mandatory]
>>>> +
>>>> +               This setting is mandatory for all boards that have only one
>>>> +               machine type and must be used to specify the machine type
>>>> +               number as it appears in the ARM machine registry
>>>> +               (see http://www.arm.linux.org.uk/developer/machines/).
>>>> +               Only boards that have multiple machine types supported
>>>> +               in a single configuration file and the machine type is
>>>> +               runtime discoverable, do not have to use this setting.
>>>> +
>>>>  - vxWorks boot parameters:
>>>>
>>>>                bootvx constructs a valid bootline using the following
>>>> diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
>>>> index 169dfeb..9901694 100644
>>>> --- a/arch/arm/lib/board.c
>>>> +++ b/arch/arm/lib/board.c
>>>> @@ -281,6 +281,10 @@ void board_init_f (ulong bootflag)
>>>>
>>>>        gd->mon_len = _bss_end_ofs;
>>>>
>>>> +#ifdef CONFIG_MACH_TYPE
>>>> +       gd->bd->bi_arch_number = CONFIG_MACH_TYPE; /* board id for Linux */
>>>> +#endif
>>>> +
>>> bd structure is not initialised by this time.
>>> It leads to u-boot hanging for my board.
>>> I fixed this problem but modifying it. Below is the patch attached for the same.
>> Then how does it work for boards setting the gd->bd->bi_arch_number
>> in board_early_init_f() function?
> can you please point out any board which sets in board_early_init_f() ?

board/esd/otc570/otc570.c

Also, I don't think we should restrict setting it to board_init() and later functions.

>>>>        for (init_fnc_ptr = init_sequence; *init_fnc_ptr; ++init_fnc_ptr) {
>>>>                if ((*init_fnc_ptr)() != 0) {
>>>>                        hang ();
>>>> --
>>>> 1.7.3.4
>>>>
>>>> _______________________________________________
>>>> U-Boot mailing list
>>>> U-Boot@lists.denx.de
>>>> http://lists.denx.de/mailman/listinfo/u-boot
>>>>
>>> >From d8df2f0ca9f08470c0cb88307fea4a66f41147a5 Mon Sep 17 00:00:00 2001
>>> From: Chander Kashyap <chander.kashyap@linaro.org>
>>> Date: Wed, 27 Jul 2011 15:10:59 +0530
>>> Subject: [PATCH] ARM: Fix wrong initialisation of bi_arch_number
>>>
>>> bi_arch_number is initialised using
>>> @arch/arm/lib/board.c
>>> \#ifdef CONFIG_MACH_TYPE
>>>         gd->bd->bi_arch_number = CONFIG_MACH_TYPE; /* board id for Linux */
>>> \#endif
>>>
>>> bd structure is not intialized by this time.
>>> This leads to u-boot hanging when CONFIG_MACH_TYPE is defined.
>>>
>>> Signed-off-by: Chander Kashyap <chander.kashyap@linaro.org>
>>> ---
>>>  arch/arm/lib/board.c |    7 +++----
>>>  1 files changed, 3 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
>>> index bcbf697..98a9bcc 100644
>>> --- a/arch/arm/lib/board.c
>>> +++ b/arch/arm/lib/board.c
>>> @@ -281,10 +281,6 @@ void board_init_f (ulong bootflag)
>>>
>>>       gd->mon_len = _bss_end_ofs;
>>>
>>> -#ifdef CONFIG_MACH_TYPE
>>> -     gd->bd->bi_arch_number = CONFIG_MACH_TYPE; /* board id for Linux */
>>> -#endif
>>> -
>>>       for (init_fnc_ptr = init_sequence; *init_fnc_ptr; ++init_fnc_ptr) {
>>>               if ((*init_fnc_ptr)() != 0) {
>>>                       hang ();
>>> @@ -380,6 +376,9 @@ void board_init_f (ulong bootflag)
>>>       gd->bd = bd;
>>>       debug ("Reserving %zu Bytes for Board Info at: %08lx\n",
>>>                       sizeof (bd_t), addr_sp);
>>> +#ifdef CONFIG_MACH_TYPE
>>> +     gd->bd->bi_arch_number = CONFIG_MACH_TYPE; /* board id for Linux */
>>> +#endif
>> This is problematic...
>> There are boards that rely on this setting in early init function calls.
>> For them it should be set before the init_sequence array is run.
>> I will rethink this once again.
> as per my understanding board_init_f() is the first initialisation call.

Yes, but there is the init_sequence[] array, which calls early board functions...
Also your proposed patch moves the initialization of bi_arch_number inside
#ifndef CONFIG_PRELOADER which is IMHO not right.
Chander Kashyap July 28, 2011, 8:19 a.m. UTC | #4
On 28 July 2011 13:29, Igor Grinberg <grinberg@compulab.co.il> wrote:
>
>
> On 07/28/11 09:41, Chander Kashyap wrote:
>> Dear Igor,
>>
>>
>> On 27 July 2011 18:34, Igor Grinberg <grinberg@compulab.co.il> wrote:
>>> On 07/27/11 13:31, Chander Kashyap wrote:
>>>> dear Igor,
>>>>
>>>>
>>>> On 14 July 2011 21:15, Igor Grinberg <grinberg@compulab.co.il> wrote:
>>>>> CONFIG_MACH_TYPE is used to set the machine type number in the
>>>>> common arm code instead of setting it in the board code.
>>>>> Boards with dynamically discoverable machine types can still set the
>>>>> machine type number in the board code.
>>>>>
>>>>> Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
>>>>> ---
>>>>> v2:     Document the option as mandatory.
>>>>>        Move the bi_arch_number setting to board_init_f()
>>>>>
>>>>>  README               |   10 ++++++++++
>>>>>  arch/arm/lib/board.c |    4 ++++
>>>>>  2 files changed, 14 insertions(+), 0 deletions(-)
>>>>>
>>>>> diff --git a/README b/README
>>>>> index 446966d..0b6802d 100644
>>>>> --- a/README
>>>>> +++ b/README
>>>>> @@ -442,6 +442,16 @@ The following options need to be configured:
>>>>>                crash. This is needed for buggy hardware (uc101) where
>>>>>                no pull down resistor is connected to the signal IDE5V_DD7.
>>>>>
>>>>> +               CONFIG_MACH_TYPE        [relevant for ARM only][mandatory]
>>>>> +
>>>>> +               This setting is mandatory for all boards that have only one
>>>>> +               machine type and must be used to specify the machine type
>>>>> +               number as it appears in the ARM machine registry
>>>>> +               (see http://www.arm.linux.org.uk/developer/machines/).
>>>>> +               Only boards that have multiple machine types supported
>>>>> +               in a single configuration file and the machine type is
>>>>> +               runtime discoverable, do not have to use this setting.
>>>>> +
>>>>>  - vxWorks boot parameters:
>>>>>
>>>>>                bootvx constructs a valid bootline using the following
>>>>> diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
>>>>> index 169dfeb..9901694 100644
>>>>> --- a/arch/arm/lib/board.c
>>>>> +++ b/arch/arm/lib/board.c
>>>>> @@ -281,6 +281,10 @@ void board_init_f (ulong bootflag)
>>>>>
>>>>>        gd->mon_len = _bss_end_ofs;
>>>>>
>>>>> +#ifdef CONFIG_MACH_TYPE
>>>>> +       gd->bd->bi_arch_number = CONFIG_MACH_TYPE; /* board id for Linux */
>>>>> +#endif
>>>>> +
>>>> bd structure is not initialised by this time.
>>>> It leads to u-boot hanging for my board.
>>>> I fixed this problem but modifying it. Below is the patch attached for the same.
>>> Then how does it work for boards setting the gd->bd->bi_arch_number
>>> in board_early_init_f() function?
>> can you please point out any board which sets in board_early_init_f() ?
>
> board/esd/otc570/otc570.c
>
> Also, I don't think we should restrict setting it to board_init() and later functions.
>
>>>>>        for (init_fnc_ptr = init_sequence; *init_fnc_ptr; ++init_fnc_ptr) {
>>>>>                if ((*init_fnc_ptr)() != 0) {
>>>>>                        hang ();
>>>>> --
>>>>> 1.7.3.4
>>>>>
>>>>> _______________________________________________
>>>>> U-Boot mailing list
>>>>> U-Boot@lists.denx.de
>>>>> http://lists.denx.de/mailman/listinfo/u-boot
>>>>>
>>>> >From d8df2f0ca9f08470c0cb88307fea4a66f41147a5 Mon Sep 17 00:00:00 2001
>>>> From: Chander Kashyap <chander.kashyap@linaro.org>
>>>> Date: Wed, 27 Jul 2011 15:10:59 +0530
>>>> Subject: [PATCH] ARM: Fix wrong initialisation of bi_arch_number
>>>>
>>>> bi_arch_number is initialised using
>>>> @arch/arm/lib/board.c
>>>> \#ifdef CONFIG_MACH_TYPE
>>>>         gd->bd->bi_arch_number = CONFIG_MACH_TYPE; /* board id for Linux */
>>>> \#endif
>>>>
>>>> bd structure is not intialized by this time.
>>>> This leads to u-boot hanging when CONFIG_MACH_TYPE is defined.
>>>>
>>>> Signed-off-by: Chander Kashyap <chander.kashyap@linaro.org>
>>>> ---
>>>>  arch/arm/lib/board.c |    7 +++----
>>>>  1 files changed, 3 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
>>>> index bcbf697..98a9bcc 100644
>>>> --- a/arch/arm/lib/board.c
>>>> +++ b/arch/arm/lib/board.c
>>>> @@ -281,10 +281,6 @@ void board_init_f (ulong bootflag)
>>>>
>>>>       gd->mon_len = _bss_end_ofs;
>>>>
>>>> -#ifdef CONFIG_MACH_TYPE
>>>> -     gd->bd->bi_arch_number = CONFIG_MACH_TYPE; /* board id for Linux */
>>>> -#endif
>>>> -
>>>>       for (init_fnc_ptr = init_sequence; *init_fnc_ptr; ++init_fnc_ptr) {
>>>>               if ((*init_fnc_ptr)() != 0) {
>>>>                       hang ();
>>>> @@ -380,6 +376,9 @@ void board_init_f (ulong bootflag)
>>>>       gd->bd = bd;
>>>>       debug ("Reserving %zu Bytes for Board Info at: %08lx\n",
>>>>                       sizeof (bd_t), addr_sp);
>>>> +#ifdef CONFIG_MACH_TYPE
>>>> +     gd->bd->bi_arch_number = CONFIG_MACH_TYPE; /* board id for Linux */
>>>> +#endif
>>> This is problematic...
>>> There are boards that rely on this setting in early init function calls.
>>> For them it should be set before the init_sequence array is run.
>>> I will rethink this once again.
>> as per my understanding board_init_f() is the first initialisation call.
>
> Yes, but there is the init_sequence[] array, which calls early board functions...
> Also your proposed patch moves the initialization of bi_arch_number inside
> #ifndef CONFIG_PRELOADER which is IMHO not right.
CONFIG_PRELOADER is only defined when building SPL.
>
>
>
>
> --
> Regards,
> Igor.
>
>
Igor Grinberg July 28, 2011, 8:58 a.m. UTC | #5
On 07/28/11 11:19, Chander Kashyap wrote:
> On 28 July 2011 13:29, Igor Grinberg <grinberg@compulab.co.il> wrote:
>>
>> On 07/28/11 09:41, Chander Kashyap wrote:
>>> Dear Igor,
>>>
>>>
>>> On 27 July 2011 18:34, Igor Grinberg <grinberg@compulab.co.il> wrote:
>>>> On 07/27/11 13:31, Chander Kashyap wrote:
>>>>> dear Igor,
>>>>>
>>>>>
>>>>> On 14 July 2011 21:15, Igor Grinberg <grinberg@compulab.co.il> wrote:
>>>>>> CONFIG_MACH_TYPE is used to set the machine type number in the
>>>>>> common arm code instead of setting it in the board code.
>>>>>> Boards with dynamically discoverable machine types can still set the
>>>>>> machine type number in the board code.
>>>>>>
>>>>>> Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
>>>>>> ---
>>>>>> v2:     Document the option as mandatory.
>>>>>>        Move the bi_arch_number setting to board_init_f()
>>>>>>
>>>>>>  README               |   10 ++++++++++
>>>>>>  arch/arm/lib/board.c |    4 ++++
>>>>>>  2 files changed, 14 insertions(+), 0 deletions(-)
>>>>>>
>>>>>> diff --git a/README b/README
>>>>>> index 446966d..0b6802d 100644
>>>>>> --- a/README
>>>>>> +++ b/README
>>>>>> @@ -442,6 +442,16 @@ The following options need to be configured:
>>>>>>                crash. This is needed for buggy hardware (uc101) where
>>>>>>                no pull down resistor is connected to the signal IDE5V_DD7.
>>>>>>
>>>>>> +               CONFIG_MACH_TYPE        [relevant for ARM only][mandatory]
>>>>>> +
>>>>>> +               This setting is mandatory for all boards that have only one
>>>>>> +               machine type and must be used to specify the machine type
>>>>>> +               number as it appears in the ARM machine registry
>>>>>> +               (see http://www.arm.linux.org.uk/developer/machines/).
>>>>>> +               Only boards that have multiple machine types supported
>>>>>> +               in a single configuration file and the machine type is
>>>>>> +               runtime discoverable, do not have to use this setting.
>>>>>> +
>>>>>>  - vxWorks boot parameters:
>>>>>>
>>>>>>                bootvx constructs a valid bootline using the following
>>>>>> diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
>>>>>> index 169dfeb..9901694 100644
>>>>>> --- a/arch/arm/lib/board.c
>>>>>> +++ b/arch/arm/lib/board.c
>>>>>> @@ -281,6 +281,10 @@ void board_init_f (ulong bootflag)
>>>>>>
>>>>>>        gd->mon_len = _bss_end_ofs;
>>>>>>
>>>>>> +#ifdef CONFIG_MACH_TYPE
>>>>>> +       gd->bd->bi_arch_number = CONFIG_MACH_TYPE; /* board id for Linux */
>>>>>> +#endif
>>>>>> +
>>>>> bd structure is not initialised by this time.
>>>>> It leads to u-boot hanging for my board.
>>>>> I fixed this problem but modifying it. Below is the patch attached for the same.
>>>> Then how does it work for boards setting the gd->bd->bi_arch_number
>>>> in board_early_init_f() function?
>>> can you please point out any board which sets in board_early_init_f() ?
>> board/esd/otc570/otc570.c
>>
>> Also, I don't think we should restrict setting it to board_init() and later functions.

I've looked into the code a bit more deeply...
Currently, I don't see how the bd initialization can be done earlier than it is right now,
to let boards use it in board_early_init_f() function and other early functions.
I have not found any other initialization of bd on that architecture,
so this makes the otc570 misuse the bd pointer
(unless 0 is a valid pointer on that architecture, but then it is a total mess...)

>>>>>>        for (init_fnc_ptr = init_sequence; *init_fnc_ptr; ++init_fnc_ptr) {
>>>>>>                if ((*init_fnc_ptr)() != 0) {
>>>>>>                        hang ();
>>>>>> --
>>>>>> 1.7.3.4
>>>>>>
>>>>>> _______________________________________________
>>>>>> U-Boot mailing list
>>>>>> U-Boot@lists.denx.de
>>>>>> http://lists.denx.de/mailman/listinfo/u-boot
>>>>>>
>>>>> >From d8df2f0ca9f08470c0cb88307fea4a66f41147a5 Mon Sep 17 00:00:00 2001
>>>>> From: Chander Kashyap <chander.kashyap@linaro.org>
>>>>> Date: Wed, 27 Jul 2011 15:10:59 +0530
>>>>> Subject: [PATCH] ARM: Fix wrong initialisation of bi_arch_number
>>>>>
>>>>> bi_arch_number is initialised using
>>>>> @arch/arm/lib/board.c
>>>>> \#ifdef CONFIG_MACH_TYPE
>>>>>         gd->bd->bi_arch_number = CONFIG_MACH_TYPE; /* board id for Linux */
>>>>> \#endif
>>>>>
>>>>> bd structure is not intialized by this time.
>>>>> This leads to u-boot hanging when CONFIG_MACH_TYPE is defined.
>>>>>
>>>>> Signed-off-by: Chander Kashyap <chander.kashyap@linaro.org>
>>>>> ---
>>>>>  arch/arm/lib/board.c |    7 +++----
>>>>>  1 files changed, 3 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
>>>>> index bcbf697..98a9bcc 100644
>>>>> --- a/arch/arm/lib/board.c
>>>>> +++ b/arch/arm/lib/board.c
>>>>> @@ -281,10 +281,6 @@ void board_init_f (ulong bootflag)
>>>>>
>>>>>       gd->mon_len = _bss_end_ofs;
>>>>>
>>>>> -#ifdef CONFIG_MACH_TYPE
>>>>> -     gd->bd->bi_arch_number = CONFIG_MACH_TYPE; /* board id for Linux */
>>>>> -#endif
>>>>> -
>>>>>       for (init_fnc_ptr = init_sequence; *init_fnc_ptr; ++init_fnc_ptr) {
>>>>>               if ((*init_fnc_ptr)() != 0) {
>>>>>                       hang ();
>>>>> @@ -380,6 +376,9 @@ void board_init_f (ulong bootflag)
>>>>>       gd->bd = bd;
>>>>>       debug ("Reserving %zu Bytes for Board Info at: %08lx\n",
>>>>>                       sizeof (bd_t), addr_sp);
>>>>> +#ifdef CONFIG_MACH_TYPE
>>>>> +     gd->bd->bi_arch_number = CONFIG_MACH_TYPE; /* board id for Linux */
>>>>> +#endif
>>>> This is problematic...
>>>> There are boards that rely on this setting in early init function calls.
>>>> For them it should be set before the init_sequence array is run.
>>>> I will rethink this once again.
>>> as per my understanding board_init_f() is the first initialisation call.
>> Yes, but there is the init_sequence[] array, which calls early board functions...
>> Also your proposed patch moves the initialization of bi_arch_number inside
>> #ifndef CONFIG_PRELOADER which is IMHO not right.
> CONFIG_PRELOADER is only defined when building SPL.

If I recall correctly there was an attempt to boot Linux straight from SPL code,
but I'm not sure...
Anyway, if we move the bi_arch_number initialization after the init_sequence[] array,
then it can be moved further till after the POST.
I'll send a patch for this in a minute.
Can you please test it?
Albert ARIBAUD Aug. 4, 2011, 12:05 p.m. UTC | #6
Hi Igor,

On 28/07/2011 10:58, Igor Grinberg wrote:
> On 07/28/11 11:19, Chander Kashyap wrote:
>> On 28 July 2011 13:29, Igor Grinberg<grinberg@compulab.co.il>  wrote:
>>>
>>> On 07/28/11 09:41, Chander Kashyap wrote:
>>>> Dear Igor,
>>>>
>>>>
>>>> On 27 July 2011 18:34, Igor Grinberg<grinberg@compulab.co.il>  wrote:
>>>>> On 07/27/11 13:31, Chander Kashyap wrote:
>>>>>> dear Igor,
>>>>>>
>>>>>>
>>>>>> On 14 July 2011 21:15, Igor Grinberg<grinberg@compulab.co.il>  wrote:
>>>>>>> CONFIG_MACH_TYPE is used to set the machine type number in the
>>>>>>> common arm code instead of setting it in the board code.
>>>>>>> Boards with dynamically discoverable machine types can still set the
>>>>>>> machine type number in the board code.
>>>>>>>
>>>>>>> Signed-off-by: Igor Grinberg<grinberg@compulab.co.il>
>>>>>>> ---
>>>>>>> v2:     Document the option as mandatory.
>>>>>>>         Move the bi_arch_number setting to board_init_f()
>>>>>>>
>>>>>>>   README               |   10 ++++++++++
>>>>>>>   arch/arm/lib/board.c |    4 ++++
>>>>>>>   2 files changed, 14 insertions(+), 0 deletions(-)
>>>>>>>
>>>>>>> diff --git a/README b/README
>>>>>>> index 446966d..0b6802d 100644
>>>>>>> --- a/README
>>>>>>> +++ b/README
>>>>>>> @@ -442,6 +442,16 @@ The following options need to be configured:
>>>>>>>                 crash. This is needed for buggy hardware (uc101) where
>>>>>>>                 no pull down resistor is connected to the signal IDE5V_DD7.
>>>>>>>
>>>>>>> +               CONFIG_MACH_TYPE        [relevant for ARM only][mandatory]
>>>>>>> +
>>>>>>> +               This setting is mandatory for all boards that have only one
>>>>>>> +               machine type and must be used to specify the machine type
>>>>>>> +               number as it appears in the ARM machine registry
>>>>>>> +               (see http://www.arm.linux.org.uk/developer/machines/).
>>>>>>> +               Only boards that have multiple machine types supported
>>>>>>> +               in a single configuration file and the machine type is
>>>>>>> +               runtime discoverable, do not have to use this setting.
>>>>>>> +
>>>>>>>   - vxWorks boot parameters:
>>>>>>>
>>>>>>>                 bootvx constructs a valid bootline using the following
>>>>>>> diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
>>>>>>> index 169dfeb..9901694 100644
>>>>>>> --- a/arch/arm/lib/board.c
>>>>>>> +++ b/arch/arm/lib/board.c
>>>>>>> @@ -281,6 +281,10 @@ void board_init_f (ulong bootflag)
>>>>>>>
>>>>>>>         gd->mon_len = _bss_end_ofs;
>>>>>>>
>>>>>>> +#ifdef CONFIG_MACH_TYPE
>>>>>>> +       gd->bd->bi_arch_number = CONFIG_MACH_TYPE; /* board id for Linux */
>>>>>>> +#endif
>>>>>>> +
>>>>>> bd structure is not initialised by this time.
>>>>>> It leads to u-boot hanging for my board.
>>>>>> I fixed this problem but modifying it. Below is the patch attached for the same.
>>>>> Then how does it work for boards setting the gd->bd->bi_arch_number
>>>>> in board_early_init_f() function?
>>>> can you please point out any board which sets in board_early_init_f() ?
>>> board/esd/otc570/otc570.c
>>>
>>> Also, I don't think we should restrict setting it to board_init() and later functions.
>
> I've looked into the code a bit more deeply...
> Currently, I don't see how the bd initialization can be done earlier than it is right now,
> to let boards use it in board_early_init_f() function and other early functions.
> I have not found any other initialization of bd on that architecture,
> so this makes the otc570 misuse the bd pointer
> (unless 0 is a valid pointer on that architecture, but then it is a total mess...)
>
>>>>>>>         for (init_fnc_ptr = init_sequence; *init_fnc_ptr; ++init_fnc_ptr) {
>>>>>>>                 if ((*init_fnc_ptr)() != 0) {
>>>>>>>                         hang ();
>>>>>>> --
>>>>>>> 1.7.3.4
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> U-Boot mailing list
>>>>>>> U-Boot@lists.denx.de
>>>>>>> http://lists.denx.de/mailman/listinfo/u-boot
>>>>>>>
>>>>>> > From d8df2f0ca9f08470c0cb88307fea4a66f41147a5 Mon Sep 17 00:00:00 2001
>>>>>> From: Chander Kashyap<chander.kashyap@linaro.org>
>>>>>> Date: Wed, 27 Jul 2011 15:10:59 +0530
>>>>>> Subject: [PATCH] ARM: Fix wrong initialisation of bi_arch_number
>>>>>>
>>>>>> bi_arch_number is initialised using
>>>>>> @arch/arm/lib/board.c
>>>>>> \#ifdef CONFIG_MACH_TYPE
>>>>>>          gd->bd->bi_arch_number = CONFIG_MACH_TYPE; /* board id for Linux */
>>>>>> \#endif
>>>>>>
>>>>>> bd structure is not intialized by this time.
>>>>>> This leads to u-boot hanging when CONFIG_MACH_TYPE is defined.
>>>>>>
>>>>>> Signed-off-by: Chander Kashyap<chander.kashyap@linaro.org>
>>>>>> ---
>>>>>>   arch/arm/lib/board.c |    7 +++----
>>>>>>   1 files changed, 3 insertions(+), 4 deletions(-)
>>>>>>
>>>>>> diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
>>>>>> index bcbf697..98a9bcc 100644
>>>>>> --- a/arch/arm/lib/board.c
>>>>>> +++ b/arch/arm/lib/board.c
>>>>>> @@ -281,10 +281,6 @@ void board_init_f (ulong bootflag)
>>>>>>
>>>>>>        gd->mon_len = _bss_end_ofs;
>>>>>>
>>>>>> -#ifdef CONFIG_MACH_TYPE
>>>>>> -     gd->bd->bi_arch_number = CONFIG_MACH_TYPE; /* board id for Linux */
>>>>>> -#endif
>>>>>> -
>>>>>>        for (init_fnc_ptr = init_sequence; *init_fnc_ptr; ++init_fnc_ptr) {
>>>>>>                if ((*init_fnc_ptr)() != 0) {
>>>>>>                        hang ();
>>>>>> @@ -380,6 +376,9 @@ void board_init_f (ulong bootflag)
>>>>>>        gd->bd = bd;
>>>>>>        debug ("Reserving %zu Bytes for Board Info at: %08lx\n",
>>>>>>                        sizeof (bd_t), addr_sp);
>>>>>> +#ifdef CONFIG_MACH_TYPE
>>>>>> +     gd->bd->bi_arch_number = CONFIG_MACH_TYPE; /* board id for Linux */
>>>>>> +#endif
>>>>> This is problematic...
>>>>> There are boards that rely on this setting in early init function calls.
>>>>> For them it should be set before the init_sequence array is run.
>>>>> I will rethink this once again.
>>>> as per my understanding board_init_f() is the first initialisation call.
>>> Yes, but there is the init_sequence[] array, which calls early board functions...
>>> Also your proposed patch moves the initialization of bi_arch_number inside
>>> #ifndef CONFIG_PRELOADER which is IMHO not right.
>> CONFIG_PRELOADER is only defined when building SPL.
>
> If I recall correctly there was an attempt to boot Linux straight from SPL code,
> but I'm not sure...
> Anyway, if we move the bi_arch_number initialization after the init_sequence[] array,
> then it can be moved further till after the POST.
> I'll send a patch for this in a minute.
> Can you please test it?

Should I revert this patch?

Amicalement,
Chander Kashyap Aug. 11, 2011, 4:16 a.m. UTC | #7
Hi Albert,


On 4 August 2011 17:35, Albert ARIBAUD <albert.u.boot@aribaud.net> wrote:

> Hi Igor,
>
>
> On 28/07/2011 10:58, Igor Grinberg wrote:
>
>> On 07/28/11 11:19, Chander Kashyap wrote:
>>
>>> On 28 July 2011 13:29, Igor Grinberg<grinberg@compulab.co.**il<grinberg@compulab.co.il>>
>>>  wrote:
>>>
>>>>
>>>> On 07/28/11 09:41, Chander Kashyap wrote:
>>>>
>>>>> Dear Igor,
>>>>>
>>>>>
>>>>> On 27 July 2011 18:34, Igor Grinberg<grinberg@compulab.co.**il<grinberg@compulab.co.il>>
>>>>>  wrote:
>>>>>
>>>>>> On 07/27/11 13:31, Chander Kashyap wrote:
>>>>>>
>>>>>>> dear Igor,
>>>>>>>
>>>>>>>
>>>>>>> On 14 July 2011 21:15, Igor Grinberg<grinberg@compulab.co.**il<grinberg@compulab.co.il>>
>>>>>>>  wrote:
>>>>>>>
>>>>>>>> CONFIG_MACH_TYPE is used to set the machine type number in the
>>>>>>>> common arm code instead of setting it in the board code.
>>>>>>>> Boards with dynamically discoverable machine types can still set the
>>>>>>>> machine type number in the board code.
>>>>>>>>
>>>>>>>> Signed-off-by: Igor Grinberg<grinberg@compulab.co.**il<grinberg@compulab.co.il>
>>>>>>>> >
>>>>>>>> ---
>>>>>>>> v2:     Document the option as mandatory.
>>>>>>>>        Move the bi_arch_number setting to board_init_f()
>>>>>>>>
>>>>>>>>  README               |   10 ++++++++++
>>>>>>>>  arch/arm/lib/board.c |    4 ++++
>>>>>>>>  2 files changed, 14 insertions(+), 0 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/README b/README
>>>>>>>> index 446966d..0b6802d 100644
>>>>>>>> --- a/README
>>>>>>>> +++ b/README
>>>>>>>> @@ -442,6 +442,16 @@ The following options need to be configured:
>>>>>>>>                crash. This is needed for buggy hardware (uc101)
>>>>>>>> where
>>>>>>>>                no pull down resistor is connected to the signal
>>>>>>>> IDE5V_DD7.
>>>>>>>>
>>>>>>>> +               CONFIG_MACH_TYPE        [relevant for ARM
>>>>>>>> only][mandatory]
>>>>>>>> +
>>>>>>>> +               This setting is mandatory for all boards that have
>>>>>>>> only one
>>>>>>>> +               machine type and must be used to specify the machine
>>>>>>>> type
>>>>>>>> +               number as it appears in the ARM machine registry
>>>>>>>> +               (see http://www.arm.linux.org.uk/**
>>>>>>>> developer/machines/<http://www.arm.linux.org.uk/developer/machines/>
>>>>>>>> ).
>>>>>>>> +               Only boards that have multiple machine types
>>>>>>>> supported
>>>>>>>> +               in a single configuration file and the machine type
>>>>>>>> is
>>>>>>>> +               runtime discoverable, do not have to use this
>>>>>>>> setting.
>>>>>>>> +
>>>>>>>>  - vxWorks boot parameters:
>>>>>>>>
>>>>>>>>                bootvx constructs a valid bootline using the
>>>>>>>> following
>>>>>>>> diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
>>>>>>>> index 169dfeb..9901694 100644
>>>>>>>> --- a/arch/arm/lib/board.c
>>>>>>>> +++ b/arch/arm/lib/board.c
>>>>>>>> @@ -281,6 +281,10 @@ void board_init_f (ulong bootflag)
>>>>>>>>
>>>>>>>>        gd->mon_len = _bss_end_ofs;
>>>>>>>>
>>>>>>>> +#ifdef CONFIG_MACH_TYPE
>>>>>>>> +       gd->bd->bi_arch_number = CONFIG_MACH_TYPE; /* board id for
>>>>>>>> Linux */
>>>>>>>> +#endif
>>>>>>>> +
>>>>>>>>
>>>>>>> bd structure is not initialised by this time.
>>>>>>> It leads to u-boot hanging for my board.
>>>>>>> I fixed this problem but modifying it. Below is the patch attached
>>>>>>> for the same.
>>>>>>>
>>>>>> Then how does it work for boards setting the gd->bd->bi_arch_number
>>>>>> in board_early_init_f() function?
>>>>>>
>>>>> can you please point out any board which sets in board_early_init_f() ?
>>>>>
>>>> board/esd/otc570/otc570.c
>>>>
>>>> Also, I don't think we should restrict setting it to board_init() and
>>>> later functions.
>>>>
>>>
>> I've looked into the code a bit more deeply...
>> Currently, I don't see how the bd initialization can be done earlier than
>> it is right now,
>> to let boards use it in board_early_init_f() function and other early
>> functions.
>> I have not found any other initialization of bd on that architecture,
>> so this makes the otc570 misuse the bd pointer
>> (unless 0 is a valid pointer on that architecture, but then it is a total
>> mess...)
>>
>>         for (init_fnc_ptr = init_sequence; *init_fnc_ptr; ++init_fnc_ptr)
>>>>>>>> {
>>>>>>>>                if ((*init_fnc_ptr)() != 0) {
>>>>>>>>                        hang ();
>>>>>>>> --
>>>>>>>> 1.7.3.4
>>>>>>>>
>>>>>>>> ______________________________**_________________
>>>>>>>> U-Boot mailing list
>>>>>>>> U-Boot@lists.denx.de
>>>>>>>> http://lists.denx.de/mailman/**listinfo/u-boot<http://lists.denx.de/mailman/listinfo/u-boot>
>>>>>>>>
>>>>>>>>  > From d8df2f0ca9f08470c0cb88307fea4a**66f41147a5 Mon Sep 17
>>>>>>> 00:00:00 2001
>>>>>>> From: Chander Kashyap<chander.kashyap@**linaro.org<chander.kashyap@linaro.org>
>>>>>>> >
>>>>>>> Date: Wed, 27 Jul 2011 15:10:59 +0530
>>>>>>> Subject: [PATCH] ARM: Fix wrong initialisation of bi_arch_number
>>>>>>>
>>>>>>> bi_arch_number is initialised using
>>>>>>> @arch/arm/lib/board.c
>>>>>>> \#ifdef CONFIG_MACH_TYPE
>>>>>>>         gd->bd->bi_arch_number = CONFIG_MACH_TYPE; /* board id for
>>>>>>> Linux */
>>>>>>> \#endif
>>>>>>>
>>>>>>> bd structure is not intialized by this time.
>>>>>>> This leads to u-boot hanging when CONFIG_MACH_TYPE is defined.
>>>>>>>
>>>>>>> Signed-off-by: Chander Kashyap<chander.kashyap@**linaro.org<chander.kashyap@linaro.org>
>>>>>>> >
>>>>>>> ---
>>>>>>>  arch/arm/lib/board.c |    7 +++----
>>>>>>>  1 files changed, 3 insertions(+), 4 deletions(-)
>>>>>>>
>>>>>>> diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
>>>>>>> index bcbf697..98a9bcc 100644
>>>>>>> --- a/arch/arm/lib/board.c
>>>>>>> +++ b/arch/arm/lib/board.c
>>>>>>> @@ -281,10 +281,6 @@ void board_init_f (ulong bootflag)
>>>>>>>
>>>>>>>       gd->mon_len = _bss_end_ofs;
>>>>>>>
>>>>>>> -#ifdef CONFIG_MACH_TYPE
>>>>>>> -     gd->bd->bi_arch_number = CONFIG_MACH_TYPE; /* board id for
>>>>>>> Linux */
>>>>>>> -#endif
>>>>>>> -
>>>>>>>       for (init_fnc_ptr = init_sequence; *init_fnc_ptr;
>>>>>>> ++init_fnc_ptr) {
>>>>>>>               if ((*init_fnc_ptr)() != 0) {
>>>>>>>                       hang ();
>>>>>>> @@ -380,6 +376,9 @@ void board_init_f (ulong bootflag)
>>>>>>>       gd->bd = bd;
>>>>>>>       debug ("Reserving %zu Bytes for Board Info at: %08lx\n",
>>>>>>>                       sizeof (bd_t), addr_sp);
>>>>>>> +#ifdef CONFIG_MACH_TYPE
>>>>>>> +     gd->bd->bi_arch_number = CONFIG_MACH_TYPE; /* board id for
>>>>>>> Linux */
>>>>>>> +#endif
>>>>>>>
>>>>>> This is problematic...
>>>>>> There are boards that rely on this setting in early init function
>>>>>> calls.
>>>>>> For them it should be set before the init_sequence array is run.
>>>>>> I will rethink this once again.
>>>>>>
>>>>> as per my understanding board_init_f() is the first initialisation
>>>>> call.
>>>>>
>>>> Yes, but there is the init_sequence[] array, which calls early board
>>>> functions...
>>>> Also your proposed patch moves the initialization of bi_arch_number
>>>> inside
>>>> #ifndef CONFIG_PRELOADER which is IMHO not right.
>>>>
>>> CONFIG_PRELOADER is only defined when building SPL.
>>>
>>
>> If I recall correctly there was an attempt to boot Linux straight from SPL
>> code,
>> but I'm not sure...
>> Anyway, if we move the bi_arch_number initialization after the
>> init_sequence[] array,
>> then it can be moved further till after the POST.
>> I'll send a patch for this in a minute.
>> Can you please test it?
>>
>
> Should I revert this patch?
>
Either revert this patch or apply new patch sent by Igor.

http://permalink.gmane.org/gmane.comp.boot-loaders.u-boot/104522


>
> Amicalement,
> --
> Albert.
>
diff mbox

Patch

diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
index bcbf697..98a9bcc 100644
--- a/arch/arm/lib/board.c
+++ b/arch/arm/lib/board.c
@@ -281,10 +281,6 @@  void board_init_f (ulong bootflag)

 	gd->mon_len = _bss_end_ofs;

-#ifdef CONFIG_MACH_TYPE
-	gd->bd->bi_arch_number = CONFIG_MACH_TYPE; /* board id for Linux */
-#endif
-
 	for (init_fnc_ptr = init_sequence; *init_fnc_ptr; ++init_fnc_ptr) {
 		if ((*init_fnc_ptr)() != 0) {
 			hang ();
@@ -380,6 +376,9 @@  void board_init_f (ulong bootflag)
 	gd->bd = bd;
 	debug ("Reserving %zu Bytes for Board Info at: %08lx\n",
 			sizeof (bd_t), addr_sp);
+#ifdef CONFIG_MACH_TYPE
+	gd->bd->bi_arch_number = CONFIG_MACH_TYPE; /* board id for Linux */
+#endif
 	addr_sp -= sizeof (gd_t);
 	id = (gd_t *) addr_sp;
 	debug ("Reserving %zu Bytes for Global Data at: %08lx\n",