Le 05/07/2011 00:03, Albert ARIBAUD a écrit :

Hi Harvey,

Sorry Christopher, I mixed up.

Le 04/07/2011 23:06, Christopher Harvey a écrit :

...

On Mon, Jul 04, 2011 at 12:00:19PM +0300, Igor Grinberg wrote:

...

CONFIG_MACH_TYPE can be used to set the machine type number in the common arm code instead of setting it in the board code.

Signed-off-by: Igor Grinberggrinberg@compulab.co.il

README | 12 ++++++++++++ arch/arm/lib/board.c | 5 +++++ 2 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/README b/README index 446966d..a9ccb0a 100644 --- a/README +++ b/README @@ -442,6 +442,18 @@ 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]
  

• This option can 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/).
  

• If this option is not defined, then your board code
  

• will have to set this up like:
  

• gd->bd->bi_arch_number =<mach type>;
  

• Note: This option is not suitable if you have multiple
  

• boards supported in a single configuration file and the
  

• machine type is runtime discoverable.
  

• • 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..ee77d05 100644 --- a/arch/arm/lib/board.c +++ b/arch/arm/lib/board.c @@ -451,6 +451,11 @@ void board_init_r (gd_t *id, ulong dest_addr)

monitor_flash_len = _end_ofs;
debug ("monitor flash len: %08lX\n", monitor_flash_len);

+#ifdef CONFIG_MACH_TYPE

• bd->bi_arch_number = CONFIG_MACH_TYPE; /* board id for Linux */

+#endif

• board_init(); /* Setup chipselects */

  #ifdef CONFIG_SERIAL_MULTI

-- 1.7.3.4

I'm curious, is it a feature that bd->bi_arch_number can be set at runtime? Do any boards actually make a decision about what value to set this to? If not, then maybe it should be a required value. I've submitted some patches that deal with the same sort of issue, so I'm interested in seeing that happens to this one.

Some boards indeed have a feature to set the mach_type at runtime, for example to run both the mainline linux kernel and a manufacturer one (manufacturers tend to use/expect fancy machine types).

Amicalement,

Amicalement,

On 07/05/11 00:06, Christopher Harvey wrote:

On Mon, Jul 04, 2011 at 12:00:19PM +0300, Igor Grinberg wrote:

...

CONFIG_MACH_TYPE can be used to set the machine type number in the common arm code instead of setting it in the board code.

Signed-off-by: Igor Grinberg grinberg@compulab.co.il

README | 12 ++++++++++++ arch/arm/lib/board.c | 5 +++++ 2 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/README b/README index 446966d..a9ccb0a 100644 --- a/README +++ b/README @@ -442,6 +442,18 @@ 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]
  

• This option can 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/).
  

• If this option is not defined, then your board code
  

• will have to set this up like:
  

• gd->bd->bi_arch_number = <mach type>;
  

• Note: This option is not suitable if you have multiple
  

• boards supported in a single configuration file and the
  

• machine type is runtime discoverable.
  

• 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..ee77d05 100644 --- a/arch/arm/lib/board.c +++ b/arch/arm/lib/board.c @@ -451,6 +451,11 @@ void board_init_r (gd_t *id, ulong dest_addr)

monitor_flash_len = _end_ofs; debug ("monitor flash len: %08lX\n", monitor_flash_len);

+#ifdef CONFIG_MACH_TYPE

• bd->bi_arch_number = CONFIG_MACH_TYPE; /* board id for Linux */

+#endif

• board_init(); /* Setup chipselects */

#ifdef CONFIG_SERIAL_MULTI

1.7.3.4

I'm curious, is it a feature that bd->bi_arch_number can be set at runtime?

Yes, it is a feature, as Wolfgang already said.

Do any boards actually make a decision about what value to set this to?

Currently, at least, 2 boards (cm_t35 and keymile) use this feature. This does not mean that in the future there will be no more boards using it.

If not, then maybe it should be a required value. I've submitted some patches that deal with the same sort of issue, so I'm interested in seeing that happens to this one.

If you're talking about this: "Make u-boot a bit easier for newbies to port", then no you are wrong, this patch has nothing to do with your patch, it does completely another thing (see the subject).

On 07/06/11 21:53, Albert ARIBAUD wrote:

Hi Igor,

Le 04/07/2011 11:00, Igor Grinberg a écrit :

...

CONFIG_MACH_TYPE can be used to set the machine type number in the common arm code instead of setting it in the board code.

Signed-off-by: Igor Grinberggrinberg@compulab.co.il

README | 12 ++++++++++++ arch/arm/lib/board.c | 5 +++++ 2 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/README b/README index 446966d..a9ccb0a 100644 --- a/README +++ b/README @@ -442,6 +442,18 @@ 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]
  

•    This option can 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/).
  

•    If this option is not defined, then your board code
  

•    will have to set this up like:
  

•    gd->bd->bi_arch_number =<mach type>;
  

•    Note: This option is not suitable if you have multiple
  

•    boards supported in a single configuration file and the
  

•    machine type is runtime discoverable.
  

• • 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..ee77d05 100644 --- a/arch/arm/lib/board.c +++ b/arch/arm/lib/board.c @@ -451,6 +451,11 @@ void board_init_r (gd_t *id, ulong dest_addr)

  monitor_flash_len = _end_ofs;
  debug ("monitor flash len: %08lX\n", monitor_flash_len);

+#ifdef CONFIG_MACH_TYPE

• bd->bi_arch_number = CONFIG_MACH_TYPE; /* board id for Linux */

+#endif

• board_init();    /* Setup chipselects */
  
  

  #ifdef CONFIG_SERIAL_MULTI

I don't really see the added value of having this configuration option. It is used in only one place for one line of code, which is a sign to me that it does not bring any substantial benefits.

Well, this is something that came up when I tried to get rid of machine_is_* macros (patch 3/3), so we could change/cut/simplify the overgrown mach-types.h if we want/need to...

I think there is an added value, may be it is hard to see it right now. If we have this option and it is documented, then any new board can use it instead of thinking (although it is simple) where and how to dereference the bi_arch_number. Also, it can come in handy, in the configuration system (I think it is called Kbuild?), so the value can be chosen from the list or something.

The history of how this option was born is here: http://www.mail-archive.com/u-boot@lists.denx.de/msg52349.html and in this thread: http://www.mail-archive.com/u-boot@lists.denx.de/msg52396.html

On 07/07/11 19:07, Albert ARIBAUD wrote:

Hi Igor,

Le 06/07/2011 22:05, Igor Grinberg a écrit :

...

On 07/06/11 21:53, Albert ARIBAUD wrote:

...

Hi Igor,

Le 04/07/2011 11:00, Igor Grinberg a écrit :

...

CONFIG_MACH_TYPE can be used to set the machine type number in the common arm code instead of setting it in the board code.

Signed-off-by: Igor Grinberggrinberg@compulab.co.il

README | 12 ++++++++++++ arch/arm/lib/board.c | 5 +++++ 2 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/README b/README index 446966d..a9ccb0a 100644 --- a/README +++ b/README @@ -442,6 +442,18 @@ 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]
  

•    This option can 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/).
  

•    If this option is not defined, then your board code
  

•    will have to set this up like:
  

•    gd->bd->bi_arch_number =<mach type>;
  

•    Note: This option is not suitable if you have multiple
  

•    boards supported in a single configuration file and the
  

•    machine type is runtime discoverable.
  

• • 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..ee77d05 100644 --- a/arch/arm/lib/board.c +++ b/arch/arm/lib/board.c @@ -451,6 +451,11 @@ void board_init_r (gd_t *id, ulong dest_addr)

   monitor_flash_len = _end_ofs;
   debug ("monitor flash len: %08lX\n", monitor_flash_len);

+#ifdef CONFIG_MACH_TYPE

• bd->bi_arch_number = CONFIG_MACH_TYPE; /* board id for Linux */

+#endif

•  board_init();    /* Setup chipselects */
  
  

  #ifdef CONFIG_SERIAL_MULTI

I don't really see the added value of having this configuration option. It is used in only one place for one line of code, which is a sign to me that it does not bring any substantial benefits.

Well, this is something that came up when I tried to get rid of machine_is_* macros (patch 3/3), so we could change/cut/simplify the overgrown mach-types.h if we want/need to...

You've read and responded to this thread: http://old.nabble.com/-U-Boot--Update-and-Cut-down-mach-types-td31432177.htm..., so you know that mach-types.h should not be changed, cut or simplified, but should only be generated from the mach-types list pulled from the ARM Linux machine repository just like Linux' mach-types.h is generated (though we use the full list whereas Linux uses a reduced list, because U-Boot supports some ARM machines which Linux does not). If your goal is reducing the size of mach-types.h, there is a branch in u-boot-ti that is about this (Sandeep: has it been posted as patches for review?)

Right, I've just tried to explain where it was originated from.

It helps to simplify the mach_type logic for omap1610innovator and omap1610h2. Also, variants of it used locally by some other boards (e.g. all nvidia boards, smdk6400). Don't you think that generalizing this stuff is an added value? Instead of having multiple and undocumented variants of the same feature, we'd better provide a common and documented one, no?

...

I think there is an added value, may be it is hard to see it right now.

If it is hard to see, then maybe it is too small a value to be worth the effort -- that's what I am trying to sort out.

I've done it already... The effort is already made... Is there any other effort that I'm not aware of (besides of picking it up and merging)?

...

If we have this option and it is documented, then any new board can use it instead of thinking (although it is simple) where and how to dereference the bi_arch_number.

Not sure I get you there. Can you elaborate on a more precise example that would show the benefits of it?

For example, if you think of Christopher's patch (ARM: Warn when the machine ID isn't set.), If you need Christopher's patch, then there are cases when the machid is not set, right? When someone gets this warning, he thinks: "Ah, I forgot the machid!" and then goes to fix the code, but again he thinks, where is the best place to put it? For us, it is trivial, that it should be in board_init() function, but for newbies, it is not that trivial. With this patch, you get the explanation and also a place to put the machid definition. With this patch, you just define the configuration "variable" and the whole thing will be done for you. Another example would be the board/nvidia/*, the code is shared as much as possible, and the mach_type is set in the common code. That is something I would expect to be done for all ARM boards, not just for nvidia...

...

Also, it can come in handy, in the configuration system (I think it is called Kbuild?), so the value can be chosen from the list or something.

AFAICT, the Linux build system does fine with the generated mach-type.

Here, I'm not talking about the mach_types, but about the config option. It can be set by the build system and no need to do it in board code!

Hi Albert,

Do you plan on including this in the current merge window?

On 07/14/11 18:45, Igor Grinberg 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

• for (init_fnc_ptr = init_sequence; *init_fnc_ptr; ++init_fnc_ptr) { if ((*init_fnc_ptr)() != 0) { hang ();

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

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.

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?

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.**ilgrinberg@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.**ilgrinberg@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.**ilgrinberg@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.**ilgrinberg@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-boothttp://lists.denx.de/mailman/listinfo/u-boot >> >> > From d8df2f0ca9f08470c0cb88307fea4a**66f41147a5 Mon Sep 17 > 00:00:00 2001 > From: Chander Kashyap<chander.kashyap@**linaro.orgchander.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.orgchander.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.