Re: [U-Boot] [PATCH 5/5] Warn when the machine ID isn't passed to an ARM kernel and u-boot is compiled in debug mode. The kernel cannot boot without it.

2011-07-05 Thread Igor Grinberg


On 07/05/11 13:04, Wolfgang Denk wrote:

> Dear Igor Grinberg,
>
> In message <4e12bf5d.6080...@compulab.co.il> you wrote:
>>> +   debug("\nWarning: machid not set! Linux will not finish 
>>> booting.\n\n");
>>> +   }
>>> +#endif
>> Is there a reason to close this in ifdef DEBUG? and also use debug()?
>> I would print this in any case, because machid must be set for Linux to boot
>> properly.
>> This message will not hurt anyone (just add ~50 bytes, this is not an spl 
>> code)
>> and if someone hacks Linux to boot in any case (without checking the machid),
>> then he can also hack U-Boot and remove the message (if it bothers him).
> I think "Warning: machid not set" should be sufficient.

Agreed!

> Note: we should't print excessive newlines. No \n at the begin of the
> string, only one at the end.
>
> Best regards,
>
> Wolfgang Denk
>

-- 
Regards,
Igor.

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 5/5] Warn when the machine ID isn't passed to an ARM kernel and u-boot is compiled in debug mode. The kernel cannot boot without it.

2011-07-05 Thread Wolfgang Denk
Dear Igor Grinberg,

In message <4e12bf5d.6080...@compulab.co.il> you wrote:
>
> > +   debug("\nWarning: machid not set! Linux will not finish 
> > booting.\n\n");
> > +   }
> > +#endif
> 
> Is there a reason to close this in ifdef DEBUG? and also use debug()?
> I would print this in any case, because machid must be set for Linux to boot
> properly.
> This message will not hurt anyone (just add ~50 bytes, this is not an spl 
> code)
> and if someone hacks Linux to boot in any case (without checking the machid),
> then he can also hack U-Boot and remove the message (if it bothers him).

I think "Warning: machid not set" should be sufficient.

Note: we should't print excessive newlines. No \n at the begin of the
string, only one at the end.

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: w...@denx.de
"We don't have to protect the environment -- the Second Coming is  at
hand."   - James Watt
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 5/5] Warn when the machine ID isn't passed to an ARM kernel and u-boot is compiled in debug mode. The kernel cannot boot without it.

2011-07-05 Thread Igor Grinberg
On 07/04/11 20:45, Christopher Harvey wrote:

> Signed-off-by: Christopher Harvey 
> ---
>  arch/arm/lib/board.c |4 
>  arch/arm/lib/bootm.c |6 ++
>  2 files changed, 10 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
> index 169dfeb..dbb835a 100644
> --- a/arch/arm/lib/board.c
> +++ b/arch/arm/lib/board.c
> @@ -404,6 +404,10 @@ void board_init_f (ulong bootflag)
>   post_bootmode_init();
>   post_run (NULL, POST_ROM | post_bootmode_get(0));
>  #endif
> + /* 0x is used to mark is value as "unset".
> +Hopefully there will never be this many machines. 
> +Can't use 0 since 0 is already used as a mach-type. */
> + gd->bd->bi_arch_number = 0x; 
>  
>   gd->bd->bi_baudrate = gd->baudrate;
>   /* Ram ist board specific, so move it to board code ... */
> diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
> index 802e833..70b3b76 100644
> --- a/arch/arm/lib/bootm.c
> +++ b/arch/arm/lib/bootm.c
> @@ -113,6 +113,12 @@ int do_bootm_linux(int flag, int argc, char *argv[], 
> bootm_headers_t *images)
>   printf ("Using machid 0x%x from environment\n", machid);
>   }
>  
> +#ifdef DEBUG
> + if(machid==0x) {

this one lacks some white spaces:
if (machid == 0x) {
and I agree with Wolfgang, the illegal value should be a self describing define 
instead.

> + debug("\nWarning: machid not set! Linux will not finish 
> booting.\n\n");
> + }
> +#endif

Is there a reason to close this in ifdef DEBUG? and also use debug()?
I would print this in any case, because machid must be set for Linux to boot
properly.
This message will not hurt anyone (just add ~50 bytes, this is not an spl code)
and if someone hacks Linux to boot in any case (without checking the machid),
then he can also hack U-Boot and remove the message (if it bothers him).


-- 
Regards,
Igor.

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 5/5] Warn when the machine ID isn't passed to an ARM kernel and u-boot is compiled in debug mode. The kernel cannot boot without it.

2011-07-05 Thread Igor Grinberg


On 07/04/11 23:32, Christopher Harvey wrote:
> On Mon, Jul 04, 2011 at 04:13:49PM -0400, Jason wrote:
>> On Mon, Jul 04, 2011 at 02:55:54PM -0400, Christopher Harvey wrote:
>>> On Mon, Jul 04, 2011 at 02:08:44PM -0400, Jason wrote:
 On Mon, Jul 04, 2011 at 01:45:41PM -0400, Christopher Harvey wrote:
> +Hopefully there will never be this many machines. 
> +Can't use 0 since 0 is already used as a mach-type. */
> + gd->bd->bi_arch_number = 0x; 
>  
>   gd->bd->bi_baudrate = gd->baudrate;
>   /* Ram ist board specific, so move it to board code ... */
> diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
> index 802e833..70b3b76 100644
> --- a/arch/arm/lib/bootm.c
> +++ b/arch/arm/lib/bootm.c
> @@ -113,6 +113,12 @@ int do_bootm_linux(int flag, int argc, char *argv[], 
> bootm_headers_t *images)
>   printf ("Using machid 0x%x from environment\n", machid);
>   }
>  
> +#ifdef DEBUG
> + if(machid==0x) {
> + debug("\nWarning: machid not set! Linux will not finish 
> booting.\n\n");
 s/finish/start/ ;-)

>>> I'll have to disagree here.  Linux will decompress and some functions
>>> will run but it will eventually stop, hence will not finish.
>> On further investigation, you're right, it doesn't finish
>> starting/booting.  Sorry for the noise.

To remove all doubts, why not make it:
Warning: machid not set! Linux will not be able to boot properly!

 Also, shouldn't the compile fail in this case (#error)?  Or, at least 
 #warn?

>>> The compiler can't know what machid will be at runtime. Maybe a "would
>>> you like to continue?" prompt could work.
>> Since the kernel throws a nice fat error message when the MACH_TYPE
>> doesn't match what it was compiled for, I don't see the point to adding
>> another message at the same point in the development process.
> I didn't see that message. Do you know what lines of code in the
> kernel print it? Or maybe just the message itself? 
> If the kernel can check the value why would it need to be passed 
> in the first place?
>
>> Perhaps use the constant CONFIG_MACH_TYPE, set to 0x.  Each
>> board config file sets it to MACH_TYPE_WHATEVER and then you could
>> do:
>>
>> #if CONFIG_MACH_TYPE == 0x
>> #warning "Machine type not set!  Linux will not finish booting!"
>> #endif
>>
>> You could use -Werror to fail on such things.  DBGFLAGS in ./config.mk
>> might be a good place.
>>
>> If the maintainers choose to move to a menuconfig style configuration
>> system, this logic could be handled in there (invalid config file).
> Right now CONFIG_MACH_TYPE is only used in a few boards and isn't used
> in core u-boot code, so I ignored it. I would agree that perhaps
> adding a CONFIG_MACH_TYPE to u-boot would be a more elegant solution
> than checking to make sure that it is a valid value before boot, but
> that would be another patch.

There is a patch ("arm: add CONFIG_MACH_TYPE option and documentation") pending
that adds CONFIG_MACH_TYPE (in case Jason missed it) as the formal config 
option.
But, again, it can be _runtime_ configurable, so you _can't_ fail the 
compilation.


-- 
Regards,
Igor.

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 5/5] Warn when the machine ID isn't passed to an ARM kernel and u-boot is compiled in debug mode. The kernel cannot boot without it.

2011-07-05 Thread Igor Grinberg
On 07/05/11 00:24, Jason wrote:

> On Mon, Jul 04, 2011 at 04:32:35PM -0400, Christopher Harvey wrote:
>> On Mon, Jul 04, 2011 at 04:13:49PM -0400, Jason wrote:
>>> On Mon, Jul 04, 2011 at 02:55:54PM -0400, Christopher Harvey wrote:
 On Mon, Jul 04, 2011 at 02:08:44PM -0400, Jason wrote:
> On Mon, Jul 04, 2011 at 01:45:41PM -0400, Christopher Harvey wrote:
>> +   Hopefully there will never be this many machines. 
>> +   Can't use 0 since 0 is already used as a mach-type. */
>> +gd->bd->bi_arch_number = 0x; 
>>  
>>  gd->bd->bi_baudrate = gd->baudrate;
>>  /* Ram ist board specific, so move it to board code ... */
>> diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
>> index 802e833..70b3b76 100644
>> --- a/arch/arm/lib/bootm.c
>> +++ b/arch/arm/lib/bootm.c
>> @@ -113,6 +113,12 @@ int do_bootm_linux(int flag, int argc, char 
>> *argv[], bootm_headers_t *images)
>>  printf ("Using machid 0x%x from environment\n", machid);
>>  }
>>  
>> +#ifdef DEBUG
>> +if(machid==0x) {
>> +debug("\nWarning: machid not set! Linux will not finish 
>> booting.\n\n");
> s/finish/start/ ;-)
>
 I'll have to disagree here.  Linux will decompress and some functions
 will run but it will eventually stop, hence will not finish.
>>> On further investigation, you're right, it doesn't finish
>>> starting/booting.  Sorry for the noise.
>>>
> Also, shouldn't the compile fail in this case (#error)?  Or, at least 
> #warn?
>
 The compiler can't know what machid will be at runtime. Maybe a "would
 you like to continue?" prompt could work.
>>> Since the kernel throws a nice fat error message when the MACH_TYPE
>>> doesn't match what it was compiled for, I don't see the point to adding
>>> another message at the same point in the development process.
>> I didn't see that message. Do you know what lines of code in the
>> kernel print it? Or maybe just the message itself? 
> In init/main.c
>   start_kernel() calls
>   setup_arch()
>
> In arch/arm/kernel/setup.c
>   setup_arch() calls
>   setup_machine_tags() which calls
>   dump_machine_table()
>
> when the value in r1 doesn't match any of the mach-types the kernel was
> compiled for.

If you don't have the earlyprintk enabled, will this still be seen?
I don't think so...

So, I think there is a point to add a warning message.

-- 
Regards,
Igor.

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 5/5] Warn when the machine ID isn't passed to an ARM kernel and u-boot is compiled in debug mode. The kernel cannot boot without it.

2011-07-04 Thread Jason
On Mon, Jul 04, 2011 at 04:32:35PM -0400, Christopher Harvey wrote:
> On Mon, Jul 04, 2011 at 04:13:49PM -0400, Jason wrote:
> > On Mon, Jul 04, 2011 at 02:55:54PM -0400, Christopher Harvey wrote:
> > > On Mon, Jul 04, 2011 at 02:08:44PM -0400, Jason wrote:
> > > > On Mon, Jul 04, 2011 at 01:45:41PM -0400, Christopher Harvey wrote:
> > > > > +Hopefully there will never be this many machines. 
> > > > > +Can't use 0 since 0 is already used as a mach-type. */
> > > > > + gd->bd->bi_arch_number = 0x; 
> > > > >  
> > > > >   gd->bd->bi_baudrate = gd->baudrate;
> > > > >   /* Ram ist board specific, so move it to board code ... */
> > > > > diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
> > > > > index 802e833..70b3b76 100644
> > > > > --- a/arch/arm/lib/bootm.c
> > > > > +++ b/arch/arm/lib/bootm.c
> > > > > @@ -113,6 +113,12 @@ int do_bootm_linux(int flag, int argc, char 
> > > > > *argv[], bootm_headers_t *images)
> > > > >   printf ("Using machid 0x%x from environment\n", machid);
> > > > >   }
> > > > >  
> > > > > +#ifdef DEBUG
> > > > > + if(machid==0x) {
> > > > > + debug("\nWarning: machid not set! Linux will not finish 
> > > > > booting.\n\n");
> > > > 
> > > > s/finish/start/ ;-)
> > > >
> > > I'll have to disagree here.  Linux will decompress and some functions
> > > will run but it will eventually stop, hence will not finish.
> > 
> > On further investigation, you're right, it doesn't finish
> > starting/booting.  Sorry for the noise.
> > 
> > > > Also, shouldn't the compile fail in this case (#error)?  Or, at least 
> > > > #warn?
> > > >
> > > The compiler can't know what machid will be at runtime. Maybe a "would
> > > you like to continue?" prompt could work.
> > 
> > Since the kernel throws a nice fat error message when the MACH_TYPE
> > doesn't match what it was compiled for, I don't see the point to adding
> > another message at the same point in the development process.
> 
> I didn't see that message. Do you know what lines of code in the
> kernel print it? Or maybe just the message itself? 

In init/main.c
start_kernel() calls
setup_arch()

In arch/arm/kernel/setup.c
setup_arch() calls
setup_machine_tags() which calls
dump_machine_table()

when the value in r1 doesn't match any of the mach-types the kernel was
compiled for.

> If the kernel can check the value why would it need to be passed 
> in the first place?

Because the kernel has no way of easily determining which arm board it's
running on without this feature.

hth,

Jason.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 5/5] Warn when the machine ID isn't passed to an ARM kernel and u-boot is compiled in debug mode. The kernel cannot boot without it.

2011-07-04 Thread Christopher Harvey
On Mon, Jul 04, 2011 at 04:13:49PM -0400, Jason wrote:
> On Mon, Jul 04, 2011 at 02:55:54PM -0400, Christopher Harvey wrote:
> > On Mon, Jul 04, 2011 at 02:08:44PM -0400, Jason wrote:
> > > On Mon, Jul 04, 2011 at 01:45:41PM -0400, Christopher Harvey wrote:
> > > > +  Hopefully there will never be this many machines. 
> > > > +  Can't use 0 since 0 is already used as a mach-type. */
> > > > +   gd->bd->bi_arch_number = 0x; 
> > > >  
> > > > gd->bd->bi_baudrate = gd->baudrate;
> > > > /* Ram ist board specific, so move it to board code ... */
> > > > diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
> > > > index 802e833..70b3b76 100644
> > > > --- a/arch/arm/lib/bootm.c
> > > > +++ b/arch/arm/lib/bootm.c
> > > > @@ -113,6 +113,12 @@ int do_bootm_linux(int flag, int argc, char 
> > > > *argv[], bootm_headers_t *images)
> > > > printf ("Using machid 0x%x from environment\n", machid);
> > > > }
> > > >  
> > > > +#ifdef DEBUG
> > > > +   if(machid==0x) {
> > > > +   debug("\nWarning: machid not set! Linux will not finish 
> > > > booting.\n\n");
> > > 
> > > s/finish/start/ ;-)
> > >
> > I'll have to disagree here.  Linux will decompress and some functions
> > will run but it will eventually stop, hence will not finish.
> 
> On further investigation, you're right, it doesn't finish
> starting/booting.  Sorry for the noise.
> 
> > > Also, shouldn't the compile fail in this case (#error)?  Or, at least 
> > > #warn?
> > >
> > The compiler can't know what machid will be at runtime. Maybe a "would
> > you like to continue?" prompt could work.
> 
> Since the kernel throws a nice fat error message when the MACH_TYPE
> doesn't match what it was compiled for, I don't see the point to adding
> another message at the same point in the development process.

I didn't see that message. Do you know what lines of code in the
kernel print it? Or maybe just the message itself? 
If the kernel can check the value why would it need to be passed 
in the first place?

> Perhaps use the constant CONFIG_MACH_TYPE, set to 0x.  Each
> board config file sets it to MACH_TYPE_WHATEVER and then you could
> do:
> 
> #if CONFIG_MACH_TYPE == 0x
> #warning "Machine type not set!  Linux will not finish booting!"
> #endif
> 
> You could use -Werror to fail on such things.  DBGFLAGS in ./config.mk
> might be a good place.
> 
> If the maintainers choose to move to a menuconfig style configuration
> system, this logic could be handled in there (invalid config file).

Right now CONFIG_MACH_TYPE is only used in a few boards and isn't used
in core u-boot code, so I ignored it. I would agree that perhaps
adding a CONFIG_MACH_TYPE to u-boot would be a more elegant solution
than checking to make sure that it is a valid value before boot, but
that would be another patch.

>  
> > > Please take comments with a grain of salt, I'm asking, not telling.  I'm
> > > fairly new to this as well.
> > >
> > I'm happy to clarify. 
> 
> Thanks for exercising my brain before I seek out the beer and
> explosives. ;-)
> 
> Jason.

-Chris
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 5/5] Warn when the machine ID isn't passed to an ARM kernel and u-boot is compiled in debug mode. The kernel cannot boot without it.

2011-07-04 Thread Jason
On Mon, Jul 04, 2011 at 02:55:54PM -0400, Christopher Harvey wrote:
> On Mon, Jul 04, 2011 at 02:08:44PM -0400, Jason wrote:
> > On Mon, Jul 04, 2011 at 01:45:41PM -0400, Christopher Harvey wrote:
> > > +Hopefully there will never be this many machines. 
> > > +Can't use 0 since 0 is already used as a mach-type. */
> > > + gd->bd->bi_arch_number = 0x; 
> > >  
> > >   gd->bd->bi_baudrate = gd->baudrate;
> > >   /* Ram ist board specific, so move it to board code ... */
> > > diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
> > > index 802e833..70b3b76 100644
> > > --- a/arch/arm/lib/bootm.c
> > > +++ b/arch/arm/lib/bootm.c
> > > @@ -113,6 +113,12 @@ int do_bootm_linux(int flag, int argc, char *argv[], 
> > > bootm_headers_t *images)
> > >   printf ("Using machid 0x%x from environment\n", machid);
> > >   }
> > >  
> > > +#ifdef DEBUG
> > > + if(machid==0x) {
> > > + debug("\nWarning: machid not set! Linux will not finish 
> > > booting.\n\n");
> > 
> > s/finish/start/ ;-)
> >
> I'll have to disagree here.  Linux will decompress and some functions
> will run but it will eventually stop, hence will not finish.

On further investigation, you're right, it doesn't finish
starting/booting.  Sorry for the noise.

> > Also, shouldn't the compile fail in this case (#error)?  Or, at least #warn?
> >
> The compiler can't know what machid will be at runtime. Maybe a "would
> you like to continue?" prompt could work.

Since the kernel throws a nice fat error message when the MACH_TYPE
doesn't match what it was compiled for, I don't see the point to adding
another message at the same point in the development process.

Perhaps use the constant CONFIG_MACH_TYPE, set to 0x.  Each
board config file sets it to MACH_TYPE_WHATEVER and then you could
do:

#if CONFIG_MACH_TYPE == 0x
#warning "Machine type not set!  Linux will not finish booting!"
#endif

You could use -Werror to fail on such things.  DBGFLAGS in ./config.mk
might be a good place.

If the maintainers choose to move to a menuconfig style configuration
system, this logic could be handled in there (invalid config file).
 
> > Please take comments with a grain of salt, I'm asking, not telling.  I'm
> > fairly new to this as well.
> >
> I'm happy to clarify. 

Thanks for exercising my brain before I seek out the beer and
explosives. ;-)

Jason.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 5/5] Warn when the machine ID isn't passed to an ARM kernel and u-boot is compiled in debug mode. The kernel cannot boot without it.

2011-07-04 Thread Wolfgang Denk
Dear Christopher Harvey,

In message <20110704185554.gh3...@harvey-pc.matrox.com> you wrote:
> On Mon, Jul 04, 2011 at 02:08:44PM -0400, Jason wrote:
> > On Mon, Jul 04, 2011 at 01:45:41PM -0400, Christopher Harvey wrote:
> > > Signed-off-by: Christopher Harvey 
> > > ---
> > >  arch/arm/lib/board.c |4 
> > >  arch/arm/lib/bootm.c |6 ++
> > >  2 files changed, 10 insertions(+), 0 deletions(-)
> > > 
> > > diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
> > > index 169dfeb..dbb835a 100644
> > > --- a/arch/arm/lib/board.c
> > > +++ b/arch/arm/lib/board.c
> > > @@ -404,6 +404,10 @@ void board_init_f (ulong bootflag)
> > >   post_bootmode_init();
> > >   post_run (NULL, POST_ROM | post_bootmode_get(0));
> > >  #endif
> > > + /* 0x is used to mark is value as "unset".
> > 
> > s/mark is/mark a/
> 
> Yes, what I meant was:
> 0x is used to mark a value as "unset".

But this is wrong.  It is not unset (= undefined), it is set.
More specifically, it is set to an illegal value.


> > Also, shouldn't the compile fail in this case (#error)?  Or, at least #warn?
> The compiler can't know what machid will be at runtime. Maybe a "would
> you like to continue?" prompt could work.

No. Just print a warning message, and continue.

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: w...@denx.de
To know how another being, another creature feels -  that  is  impos-
sible.  - Terry Pratchett, _The Dark Side of the Sun_
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 5/5] Warn when the machine ID isn't passed to an ARM kernel and u-boot is compiled in debug mode. The kernel cannot boot without it.

2011-07-04 Thread Wolfgang Denk
Dear Christopher Harvey,

In message <20110704174541.gf3...@harvey-pc.matrox.com> you wrote:
> Signed-off-by: Christopher Harvey 
> ---
>  arch/arm/lib/board.c |4 
>  arch/arm/lib/bootm.c |6 ++
>  2 files changed, 10 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
> index 169dfeb..dbb835a 100644
> --- a/arch/arm/lib/board.c
> +++ b/arch/arm/lib/board.c
> @@ -404,6 +404,10 @@ void board_init_f (ulong bootflag)
>   post_bootmode_init();
>   post_run (NULL, POST_ROM | post_bootmode_get(0));
>  #endif
> + /* 0x is used to mark is value as "unset".
> +Hopefully there will never be this many machines. 
> +Can't use 0 since 0 is already used as a mach-type. */
> + gd->bd->bi_arch_number = 0x; 

Incorrect multiline comment style.

Instead of using hardwired magic numbers, please use a #define'd
constant (add to arch/arm/include/asm/u-boot.h close to where
bi_arch_number gets defined).

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: w...@denx.de
[Braddock:] Mr. Churchill, you are drunk.
[Churchill:] And you madam, are ugly.  But I shall be sober tomorrow.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 5/5] Warn when the machine ID isn't passed to an ARM kernel and u-boot is compiled in debug mode. The kernel cannot boot without it.

2011-07-04 Thread Christopher Harvey
On Mon, Jul 04, 2011 at 02:08:44PM -0400, Jason wrote:
> On Mon, Jul 04, 2011 at 01:45:41PM -0400, Christopher Harvey wrote:
> > Signed-off-by: Christopher Harvey 
> > ---
> >  arch/arm/lib/board.c |4 
> >  arch/arm/lib/bootm.c |6 ++
> >  2 files changed, 10 insertions(+), 0 deletions(-)
> > 
> > diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
> > index 169dfeb..dbb835a 100644
> > --- a/arch/arm/lib/board.c
> > +++ b/arch/arm/lib/board.c
> > @@ -404,6 +404,10 @@ void board_init_f (ulong bootflag)
> > post_bootmode_init();
> > post_run (NULL, POST_ROM | post_bootmode_get(0));
> >  #endif
> > +   /* 0x is used to mark is value as "unset".
> 
> s/mark is/mark a/

Yes, what I meant was:
0x is used to mark a value as "unset".

> 
> > +  Hopefully there will never be this many machines. 
> > +  Can't use 0 since 0 is already used as a mach-type. */
> > +   gd->bd->bi_arch_number = 0x; 
> >  
> > gd->bd->bi_baudrate = gd->baudrate;
> > /* Ram ist board specific, so move it to board code ... */
> > diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
> > index 802e833..70b3b76 100644
> > --- a/arch/arm/lib/bootm.c
> > +++ b/arch/arm/lib/bootm.c
> > @@ -113,6 +113,12 @@ int do_bootm_linux(int flag, int argc, char *argv[], 
> > bootm_headers_t *images)
> > printf ("Using machid 0x%x from environment\n", machid);
> > }
> >  
> > +#ifdef DEBUG
> > +   if(machid==0x) {
> > +   debug("\nWarning: machid not set! Linux will not finish 
> > booting.\n\n");
> 
> s/finish/start/ ;-)
I'll have to disagree here.  Linux will decompress and some functions
will run but it will eventually stop, hence will not finish.
> 
> Also, shouldn't the compile fail in this case (#error)?  Or, at least #warn?
The compiler can't know what machid will be at runtime. Maybe a "would
you like to continue?" prompt could work.
> 
> > +   }
> > +#endif
> > +
> > show_boot_progress (15);
> >  
> >  #ifdef CONFIG_OF_LIBFDT
> > -- 
> > 1.7.3.4
> 
> Please take comments with a grain of salt, I'm asking, not telling.  I'm
> fairly new to this as well.
I'm happy to clarify. 
> 
> thx,
> 
> Jason.

thanks,
-Chris
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 5/5] Warn when the machine ID isn't passed to an ARM kernel and u-boot is compiled in debug mode. The kernel cannot boot without it.

2011-07-04 Thread Jason
On Mon, Jul 04, 2011 at 01:45:41PM -0400, Christopher Harvey wrote:
> Signed-off-by: Christopher Harvey 
> ---
>  arch/arm/lib/board.c |4 
>  arch/arm/lib/bootm.c |6 ++
>  2 files changed, 10 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
> index 169dfeb..dbb835a 100644
> --- a/arch/arm/lib/board.c
> +++ b/arch/arm/lib/board.c
> @@ -404,6 +404,10 @@ void board_init_f (ulong bootflag)
>   post_bootmode_init();
>   post_run (NULL, POST_ROM | post_bootmode_get(0));
>  #endif
> + /* 0x is used to mark is value as "unset".

s/mark is/mark a/

> +Hopefully there will never be this many machines. 
> +Can't use 0 since 0 is already used as a mach-type. */
> + gd->bd->bi_arch_number = 0x; 
>  
>   gd->bd->bi_baudrate = gd->baudrate;
>   /* Ram ist board specific, so move it to board code ... */
> diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
> index 802e833..70b3b76 100644
> --- a/arch/arm/lib/bootm.c
> +++ b/arch/arm/lib/bootm.c
> @@ -113,6 +113,12 @@ int do_bootm_linux(int flag, int argc, char *argv[], 
> bootm_headers_t *images)
>   printf ("Using machid 0x%x from environment\n", machid);
>   }
>  
> +#ifdef DEBUG
> + if(machid==0x) {
> + debug("\nWarning: machid not set! Linux will not finish 
> booting.\n\n");

s/finish/start/ ;-)

Also, shouldn't the compile fail in this case (#error)?  Or, at least #warn?

> + }
> +#endif
> +
>   show_boot_progress (15);
>  
>  #ifdef CONFIG_OF_LIBFDT
> -- 
> 1.7.3.4

Please take comments with a grain of salt, I'm asking, not telling.  I'm
fairly new to this as well.

thx,

Jason.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH 5/5] Warn when the machine ID isn't passed to an ARM kernel and u-boot is compiled in debug mode. The kernel cannot boot without it.

2011-07-04 Thread Christopher Harvey
Signed-off-by: Christopher Harvey 
---
 arch/arm/lib/board.c |4 
 arch/arm/lib/bootm.c |6 ++
 2 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
index 169dfeb..dbb835a 100644
--- a/arch/arm/lib/board.c
+++ b/arch/arm/lib/board.c
@@ -404,6 +404,10 @@ void board_init_f (ulong bootflag)
post_bootmode_init();
post_run (NULL, POST_ROM | post_bootmode_get(0));
 #endif
+   /* 0x is used to mark is value as "unset".
+  Hopefully there will never be this many machines. 
+  Can't use 0 since 0 is already used as a mach-type. */
+   gd->bd->bi_arch_number = 0x; 
 
gd->bd->bi_baudrate = gd->baudrate;
/* Ram ist board specific, so move it to board code ... */
diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
index 802e833..70b3b76 100644
--- a/arch/arm/lib/bootm.c
+++ b/arch/arm/lib/bootm.c
@@ -113,6 +113,12 @@ int do_bootm_linux(int flag, int argc, char *argv[], 
bootm_headers_t *images)
printf ("Using machid 0x%x from environment\n", machid);
}
 
+#ifdef DEBUG
+   if(machid==0x) {
+   debug("\nWarning: machid not set! Linux will not finish 
booting.\n\n");
+   }
+#endif
+
show_boot_progress (15);
 
 #ifdef CONFIG_OF_LIBFDT
-- 
1.7.3.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot