[U-Boot] [PATCH 1/3] arm: add CONFIG_MACH_TYPE option and documentation

[U-Boot] [PATCH 1/3] arm: add CONFIG_MACH_TYPE option and documentation

[Igor Grinberg]

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

[U-Boot] [PATCH 2/3] arm: nvidia and smdk6400: use common code for machine type

[Igor Grinberg]

NVIDIA boards and Samsung SMDK6400 already use a local variant of
CONFIG_MACH_TYPE option.
Switch to use the new common code.

Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
---
 board/nvidia/common/board.c       |    2 --
 board/samsung/smdk6400/smdk6400.c |    1 -
 include/configs/smdk6400.h        |    2 +-
 3 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/board/nvidia/common/board.c b/board/nvidia/common/board.c
index 3d6c248..7c838e8 100644
--- a/board/nvidia/common/board.c
+++ b/board/nvidia/common/board.c
@@ -205,8 +205,6 @@ int board_init(void)
 {
 	/* boot param addr */
 	gd->bd->bi_boot_params = (NV_PA_SDRAM_BASE + 0x100);
-	/* board id for Linux */
-	gd->bd->bi_arch_number = CONFIG_MACH_TYPE;
 
 	return 0;
 }
diff --git a/board/samsung/smdk6400/smdk6400.c b/board/samsung/smdk6400/smdk6400.c
index 13c7ed5..c40d1f9 100644
--- a/board/samsung/smdk6400/smdk6400.c
+++ b/board/samsung/smdk6400/smdk6400.c
@@ -72,7 +72,6 @@ int board_init(void)
 	/* Enable WAIT */
 	SROM_BW_REG |= 4 | 8 | 1;
 
-	gd->bd->bi_arch_number = MACH_TYPE;
 	gd->bd->bi_boot_params = PHYS_SDRAM_1 + 0x100;
 
 	return 0;
diff --git a/include/configs/smdk6400.h b/include/configs/smdk6400.h
index c9acf58..d175ed6 100644
--- a/include/configs/smdk6400.h
+++ b/include/configs/smdk6400.h
@@ -65,7 +65,7 @@
 /*
  * Architecture magic and machine type
  */
-#define MACH_TYPE		1270
+#define CONFIG_MACH_TYPE		1270
 
 #define CONFIG_DISPLAY_CPUINFO
 #define CONFIG_DISPLAY_BOARDINFO
-- 
1.7.3.4

[U-Boot] [PATCH 3/3] arm: omap: innovator: use common code for machine type

[Igor Grinberg]

Innovator and H2 boards used machine_is_* macros for setting the machine
type. These macros are expanded in compile time and thus leaves
unreachable code (though gcc might optimize it).
Switch them to use common code for machine type setting.

Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
---
 board/ti/omap1610inn/omap1610innovator.c |    7 -------
 include/configs/omap1610h2.h             |    3 ++-
 include/configs/omap1610inn.h            |    3 ++-
 3 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/board/ti/omap1610inn/omap1610innovator.c b/board/ti/omap1610inn/omap1610innovator.c
index 44818bb..16e8237 100644
--- a/board/ti/omap1610inn/omap1610innovator.c
+++ b/board/ti/omap1610inn/omap1610innovator.c
@@ -63,13 +63,6 @@ static inline void delay (unsigned long loops)
 
 int board_init (void)
 {
-	if (machine_is_omap_h2())
-		gd->bd->bi_arch_number = MACH_TYPE_OMAP_H2;
-	else if (machine_is_omap_innovator())
-		gd->bd->bi_arch_number = MACH_TYPE_OMAP_INNOVATOR;
-	else
-		gd->bd->bi_arch_number = MACH_TYPE_OMAP_GENERIC;
-
 	/* adress of boot parameters */
 	gd->bd->bi_boot_params = 0x10000100;
 
diff --git a/include/configs/omap1610h2.h b/include/configs/omap1610h2.h
index cb2a07f..9fa3317 100644
--- a/include/configs/omap1610h2.h
+++ b/include/configs/omap1610h2.h
@@ -34,7 +34,8 @@
 #define CONFIG_OMAP		1	/* in a TI OMAP core */
 #define CONFIG_OMAP1610		1	/* which is in a 1610 */
 #define CONFIG_H2_OMAP1610	1	/* on an H2 Board */
-#define CONFIG_MACH_OMAP_H2		/* Select board mach-type */
+
+#define CONFIG_MACH_TYPE	MACH_TYPE_OMAP_H2
 
 /* input clock of PLL */
 /* the OMAP1610 H2 has 12MHz input clock */
diff --git a/include/configs/omap1610inn.h b/include/configs/omap1610inn.h
index e82b4b2..e3f3487 100644
--- a/include/configs/omap1610inn.h
+++ b/include/configs/omap1610inn.h
@@ -34,7 +34,8 @@
 #define CONFIG_OMAP	1			/* in a TI OMAP core    */
 #define CONFIG_OMAP1610	1		/* which is in a 1610  */
 #define CONFIG_INNOVATOROMAP1610	1	/*  a Innovator Board  */
-#define CONFIG_MACH_OMAP_INNOVATOR	/* Select board mach-type */
+
+#define CONFIG_MACH_TYPE	MACH_TYPE_OMAP_INNOVATOR
 
 /* input clock of PLL */
 /* the OMAP1610 Innovator has 12MHz input clock */
-- 
1.7.3.4

[U-Boot] [PATCH 1/3] arm: add CONFIG_MACH_TYPE option and documentation

[Christopher Harvey]

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

[U-Boot] [PATCH 1/3] arm: add CONFIG_MACH_TYPE option and documentation

[Albert ARIBAUD]

Hi Harvey,

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 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? 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,
-- 
Albert.

[U-Boot] [PATCH 1/3] arm: add CONFIG_MACH_TYPE option and documentation

[Albert ARIBAUD]

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 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? 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,
-- 
Albert.

[U-Boot] [PATCH 1/3] arm: add CONFIG_MACH_TYPE option and documentation

[Wolfgang Denk]

Dear Christopher Harvey,

In message <20110704210619.GA3218@harvey-pc.matrox.com> you wrote:
>
> 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

Yes, this is a feature. It comes in handy in a number of cases.

> set this to? If not, then maybe it should be a required value.  I've

Why?

> submitted some patches that deal with the same sort of issue, so I'm
> interested in seeing that happens to this one.

Sorry, I can't follow...

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The rule on staying alive as a program manager is to give 'em a  num-
ber or give 'em a date, but never give 'em both at once.

[U-Boot] [PATCH 1/3] arm: add CONFIG_MACH_TYPE option and documentation

[charvey at matrox.com]

On Tue, Jul 05, 2011 at 12:16:12AM +0200, Wolfgang Denk wrote:
> Dear Christopher Harvey,
> 
> In message <20110704210619.GA3218@harvey-pc.matrox.com> you wrote:
> >
> > 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
> 
> Yes, this is a feature. It comes in handy in a number of cases.
> 
> > set this to? If not, then maybe it should be a required value.  I've
> 
> Why?

Because if every machine sets an essentially static value at runtime
then it would be a nice compile-time check to do. But, there is no
point since the bi_arch_number isn't fixed for each u-boot
configuration.

> 
> > submitted some patches that deal with the same sort of issue, so I'm
> > interested in seeing that happens to this one.
> 
> Sorry, I can't follow...

I was refering to this patch:
http://patchwork.ozlabs.org/patch/103149/
which is similar. 

> 
> Best regards,
> 
> Wolfgang Denk
> 
> -- 
> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
> The rule on staying alive as a program manager is to give 'em a  num-
> ber or give 'em a date, but never give 'em both at once.

-Chris

[U-Boot] [PATCH 1/3] arm: add CONFIG_MACH_TYPE option and documentation

[Igor Grinberg]

On 07/05/11 17:08, charvey at matrox.com wrote:

> On Tue, Jul 05, 2011 at 12:16:12AM +0200, Wolfgang Denk wrote:
>> Dear Christopher Harvey,
>>
>> In message <20110704210619.GA3218@harvey-pc.matrox.com> you wrote:
>>> 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
>> Yes, this is a feature. It comes in handy in a number of cases.
>>
>>> set this to? If not, then maybe it should be a required value.  I've
>> Why?
> Because if every machine sets an essentially static value at runtime
> then it would be a nice compile-time check to do. But, there is no
> point since the bi_arch_number isn't fixed for each u-boot
> configuration.

Right.

>>> submitted some patches that deal with the same sort of issue, so I'm
>>> interested in seeing that happens to this one.
>> Sorry, I can't follow...
> I was refering to this patch:
> http://patchwork.ozlabs.org/patch/103149/
> which is similar. 

No, you are wrong! It is not similar even a bit! It does completely different thing.
Your patch: warns about machine type not set.
My patch: just adds a configuration _option_ which you can use, but you don't have to.
See... It is not the same!



-- 
Regards,
Igor.

[U-Boot] [PATCH 1/3] arm: add CONFIG_MACH_TYPE option and documentation

[Igor Grinberg]

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

-- 
Regards,
Igor.

[U-Boot] [PATCH 1/3] arm: add CONFIG_MACH_TYPE option and documentation

[Albert ARIBAUD]

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

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.

Amicalement,
-- 
Albert.

[U-Boot] [PATCH 1/3] arm: add CONFIG_MACH_TYPE option and documentation

[Igor Grinberg]

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 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
>
> 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 at lists.denx.de/msg52349.html
and in this thread:
http://www.mail-archive.com/u-boot at lists.denx.de/msg52396.html



-- 
Regards,
Igor.

[U-Boot] [PATCH 1/3] arm: add CONFIG_MACH_TYPE option and documentation

[Albert ARIBAUD]

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 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
>>
>> 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.html, 
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?)

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

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

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

Amicalement,
-- 
Albert.

[U-Boot] [PATCH 1/3] arm: add CONFIG_MACH_TYPE option and documentation

[Igor Grinberg]

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 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
>>>
>>> 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.html, 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!



-- 
Regards,
Igor.

[U-Boot] [PATCH 1/3] arm: add CONFIG_MACH_TYPE option and documentation

[Albert ARIBAUD]

Le 07/07/2011 18:51, Igor Grinberg a ?crit :

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

I see your point.

Now the issue I foresee is that this commonalization has benefits only 
for boards which currently set their bi_arch_number in board_init_f(), 
but has no incentive -- that's a code that will be used only in a few 
places and could stay that way for quite long, because boards that will 
not adhere to it will still build unchanged.

IOW, there is no benefit for e.g. ED Mini V2, to use CONFIG_MACH_TYPE, 
so why would it? Thus instead of simplifying and commonalizing, this 
feature will *add* to the code base complexity.

Unless the goal is to add this macro *and* change all related board 
codes in the same patchset? I don't see it as feasible either.

Any suggestion for ensuring adoption of the feature wherever it can be used?

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

Agreed.

Amicalement,
-- 
Albert.

[U-Boot] [PATCH 1/3] arm: add CONFIG_MACH_TYPE option and documentation

[Igor Grinberg]

On 07/07/11 20:46, Albert ARIBAUD wrote:
> Le 07/07/2011 18:51, Igor Grinberg a ?crit :
>
>>>> 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...
>
> I see your point.
>
> Now the issue I foresee is that this commonalization has benefits only for boards which currently set their bi_arch_number in board_init_f(),

Vast majority of boards set their bi_arch_number in board_init().
I went through all the boards and there is only one that set it in board_early_init_f()
and several that do this in checkboard().

This makes me think of v2 of this patch which will set the bi_arch_number in board_init_f()
just before the init_sequence[] array is run.

> but has no incentive -- that's a code that will be used only in a few places and could stay that way for quite long, because boards that will not adhere to it will still build unchanged.

Well, I don't like to force people do something by breaking their builds...
In general, I think that any change should not break any existing code (at least not on purpose).
At least, this is how it works in Linux.

>
> IOW, there is no benefit for e.g. ED Mini V2, to use CONFIG_MACH_TYPE, so why would it? Thus instead of simplifying and commonalizing, this feature will *add* to the code base complexity.
>
> Unless the goal is to add this macro *and* change all related board codes in the same patchset? I don't see it as feasible either.

Well, I can do the change board/* wide, but it will take some time to accomplish.
Also, we still don't have an exact list of boards for removal, so I'd like to wait until
the removal takes place, so there will be less boards to consider.

>
> Any suggestion for ensuring adoption of the feature wherever it can be used?

Currently, I can think of:
1) Changing all relevant boards to use it - will eliminate "bad" examples.
2) Pointing to the use of CONFIG_MACH_TYPE during code review.
3) Introduce one more config option, like CONFIG_DYNAMIC_MACH_TYPE
and change the patch to something like:
#ifndef CONFIG_DYNAMIC_MACH_TYPE
bi_arch_number = CONFIG_MACH_TYPE;
#endif

If we decide to go for 3), it would integrate nicely with Christopher's patch.


-- 
Regards,
Igor.

[U-Boot] [PATCH 1/3] arm: add CONFIG_MACH_TYPE option and documentation

[Igor Grinberg]

Hi Albert,

On 07/08/11 00:06, Igor Grinberg wrote:
> On 07/07/11 20:46, Albert ARIBAUD wrote:
>> Le 07/07/2011 18:51, Igor Grinberg a ?crit :
>>
>>>>> 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...
>> I see your point.
>>
>> Now the issue I foresee is that this commonalization has benefits only for boards which currently set their bi_arch_number in board_init_f(),
> Vast majority of boards set their bi_arch_number in board_init().
> I went through all the boards and there is only one that set it in board_early_init_f()
> and several that do this in checkboard().
>
> This makes me think of v2 of this patch which will set the bi_arch_number in board_init_f()
> just before the init_sequence[] array is run.
>
>> but has no incentive -- that's a code that will be used only in a few places and could stay that way for quite long, because boards that will not adhere to it will still build unchanged.
> Well, I don't like to force people do something by breaking their builds...
> In general, I think that any change should not break any existing code (at least not on purpose).
> At least, this is how it works in Linux.
>
>> IOW, there is no benefit for e.g. ED Mini V2, to use CONFIG_MACH_TYPE, so why would it? Thus instead of simplifying and commonalizing, this feature will *add* to the code base complexity.
>>
>> Unless the goal is to add this macro *and* change all related board codes in the same patchset? I don't see it as feasible either.
> Well, I can do the change board/* wide, but it will take some time to accomplish.
> Also, we still don't have an exact list of boards for removal, so I'd like to wait until
> the removal takes place, so there will be less boards to consider.
>
>> Any suggestion for ensuring adoption of the feature wherever it can be used?
> Currently, I can think of:
> 1) Changing all relevant boards to use it - will eliminate "bad" examples.
> 2) Pointing to the use of CONFIG_MACH_TYPE during code review.
> 3) Introduce one more config option, like CONFIG_DYNAMIC_MACH_TYPE
> and change the patch to something like:
> #ifndef CONFIG_DYNAMIC_MACH_TYPE
> bi_arch_number = CONFIG_MACH_TYPE;
> #endif
>
> If we decide to go for 3), it would integrate nicely with Christopher's patch.

So, what will it be?
If it will be 1 and 2, then I'd like to get this patch in, so I can start working on 1.

If it will be 3, then I want to make the change and resubmit,
hoping for current merge window...



-- 
Regards,
Igor.

[U-Boot] [PATCH 1/3] arm: add CONFIG_MACH_TYPE option and documentation

[Albert ARIBAUD]

Hi Igor,

Le 13/07/2011 07:52, Igor Grinberg a ?crit :
> Hi Albert,
>
> On 07/08/11 00:06, Igor Grinberg wrote:
>> On 07/07/11 20:46, Albert ARIBAUD wrote:
>>> Le 07/07/2011 18:51, Igor Grinberg a ?crit :
>>>
>>>>>> 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...
>>> I see your point.
>>>
>>> Now the issue I foresee is that this commonalization has benefits only for boards which currently set their bi_arch_number in board_init_f(),
>> Vast majority of boards set their bi_arch_number in board_init().
>> I went through all the boards and there is only one that set it in board_early_init_f()
>> and several that do this in checkboard().
>>
>> This makes me think of v2 of this patch which will set the bi_arch_number in board_init_f()
>> just before the init_sequence[] array is run.
>>
>>> but has no incentive -- that's a code that will be used only in a few places and could stay that way for quite long, because boards that will not adhere to it will still build unchanged.
>> Well, I don't like to force people do something by breaking their builds...
>> In general, I think that any change should not break any existing code (at least not on purpose).
>> At least, this is how it works in Linux.
>>
>>> IOW, there is no benefit for e.g. ED Mini V2, to use CONFIG_MACH_TYPE, so why would it? Thus instead of simplifying and commonalizing, this feature will *add* to the code base complexity.
>>>
>>> Unless the goal is to add this macro *and* change all related board codes in the same patchset? I don't see it as feasible either.
>> Well, I can do the change board/* wide, but it will take some time to accomplish.
>> Also, we still don't have an exact list of boards for removal, so I'd like to wait until
>> the removal takes place, so there will be less boards to consider.
>>
>>> Any suggestion for ensuring adoption of the feature wherever it can be used?
>> Currently, I can think of:
>> 1) Changing all relevant boards to use it - will eliminate "bad" examples.
>> 2) Pointing to the use of CONFIG_MACH_TYPE during code review.
>> 3) Introduce one more config option, like CONFIG_DYNAMIC_MACH_TYPE
>> and change the patch to something like:
>> #ifndef CONFIG_DYNAMIC_MACH_TYPE
>> bi_arch_number = CONFIG_MACH_TYPE;
>> #endif
>>
>> If we decide to go for 3), it would integrate nicely with Christopher's patch.
>
> So, what will it be?
> If it will be 1 and 2, then I'd like to get this patch in, so I can start working on 1.
>
> If it will be 3, then I want to make the change and resubmit,
> hoping for current merge window...

I don't think two macros are needed for this. Either the board config 
file targets a single Linux machine ID and it defines CONFIG_MACH_TYPE, 
or it targets several and somewhere in the board init code it sets 
bi_arch_number to one of some MACH_TYPE_xxxx values without defining 
CONFIG_MACH_TYPE, so this one macro is enough and ...DYNAMIC... would be 
redundant.

Solution 1 would be the most correct IMO but is a lot of work for you as 
a submitter -- to be clear, I understand it as changing *every* board 
that sets bi_arch_number in board code to setting it in (lib and) config 
file. As much as I like it, I myself would hesitate to take on such an 
effort, so I will not force it upon you.

Pragmatism against perfection: let's go for 2, then. Change the boards 
you intended to change, and from now on reviewers for any change to a 
board should point out the move to CONFIG_MACH_TYPE as mandatory.

Amicalement,
-- 
Albert.

[U-Boot] [PATCH 1/3] arm: add CONFIG_MACH_TYPE option and documentation

[Albert ARIBAUD]

Hi again Igor,

Le 14/07/2011 16:10, Albert ARIBAUD a ?crit :

> Pragmatism against perfection: let's go for 2, then. Change the boards
> you intended to change, and from now on reviewers for any change to a
> board should point out the move to CONFIG_MACH_TYPE as mandatory.

Forgot to mention:

That makes patches 2/3 and 3/3 ok for acceptance, but can you post a V2 
of patch 1/3 in which documentation of CONFIG_MACH_TYPE describes it as 
mandatory (and forbids direct setting of bi_arch_number) for board 
configs with a single machine-type?

Thanks in advance,

Amicalement,
-- 
Albert.

[U-Boot] [PATCH 1/3] arm: add CONFIG_MACH_TYPE option and documentation

[Igor Grinberg]

On 07/14/11 17:20, Albert ARIBAUD wrote:

> Hi again Igor,
>
> Le 14/07/2011 16:10, Albert ARIBAUD a ?crit :
>
>> Pragmatism against perfection: let's go for 2, then. Change the boards
>> you intended to change, and from now on reviewers for any change to a
>> board should point out the move to CONFIG_MACH_TYPE as mandatory.
>
> Forgot to mention:
>
> That makes patches 2/3 and 3/3 ok for acceptance, but can you post a V2 of patch 1/3 in which documentation of CONFIG_MACH_TYPE describes it as mandatory (and forbids direct setting of bi_arch_number) for board configs with a single machine-type?

Ok.

I have to make it v2 anyway, because the setting of bi_arch_number in the board.c
has to be moved to board_init_f() - before the init_sequence[] array is run.



-- 
Regards,
Igor.

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

[Igor Grinberg]

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 ();
-- 
1.7.3.4

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

[Igor Grinberg]

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 ();

-- 
Regards,
Igor.

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

[Albert ARIBAUD]

Hi Igor,

Le 17/07/2011 08:56, Igor Grinberg a ?crit :
> Hi Albert,
>
>
> Do you plan on including this in the current merge window?

Done. The other two will be going in as well as they were initially 
submitted before the merge window closure.

Amicalement,
-- 
Albert.

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

[Albert ARIBAUD]

Hi Igor,

Le 14/07/2011 17:45, Igor Grinberg a ?crit :
> 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()

Applied to u-boot-arm/master, thanks!

Amicalement,
-- 
Albert.

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

[Chander Kashyap]

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 at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>

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

[Igor Grinberg]

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


-- 
Regards,
Igor.

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

[Chander Kashyap]

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



-- 
with warm regards,
Chander Kashyap

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

[Igor Grinberg]

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




-- 
Regards,
Igor.

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

[Chander Kashyap]

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



-- 
with warm regards,
Chander Kashyap

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

[Igor Grinberg]

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 at 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?


-- 
Regards,
Igor.

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

[Albert ARIBAUD]

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 at 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,
-- 
Albert.

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

[Chander Kashyap]

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



-- 
with warm regards,
Chander Kashyap