All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christopher Harvey <charvey@matrox.com>
To: u-boot@lists.denx.de
Subject: [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.
Date: Mon, 4 Jul 2011 16:32:35 -0400	[thread overview]
Message-ID: <20110704203235.GI3016@harvey-pc.matrox.com> (raw)
In-Reply-To: <20110704201349.GD30477@titan.lakedaemon.net>

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 = 0xffffffff; 
> > > >  
> > > >  	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==0xffffffff) {
> > > > +	        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 0xffffffff.  Each
> board config file sets it to MACH_TYPE_WHATEVER and then you could
> do:
> 
> #if CONFIG_MACH_TYPE == 0xffffffff
> #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

  reply	other threads:[~2011-07-04 20:32 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <cover.1309799825.git.charvey@matrox.com>
2011-07-04 17:43 ` [U-Boot] [PATCH 1/5] Added documentation for CONFIG_SYS_TEXT_BASE for ARM Christopher Harvey
2011-07-04 19:39   ` Wolfgang Denk
2011-07-04 17:43 ` [U-Boot] [PATCH 2/5] Added extra documentation about how the relocation address to RAM is picked " Christopher Harvey
2011-07-04 19:43   ` Wolfgang Denk
2011-07-06 20:58     ` Christopher Harvey
2011-07-06 21:29       ` Wolfgang Denk
2011-07-07 16:10   ` Albert ARIBAUD
2011-07-04 17:44 ` [U-Boot] [PATCH 3/5] Removed unused define, CONFIG_ARMV7 Christopher Harvey
2011-07-04 18:00   ` Jason
2011-07-04 18:46     ` Christopher Harvey
2011-07-04 19:47       ` Wolfgang Denk
2011-07-04 17:45 ` [U-Boot] [PATCH 4/5] Don't compile in large memory test function by default Christopher Harvey
2011-07-07 16:13   ` Albert ARIBAUD
2011-07-04 17:45 ` [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 Christopher Harvey
2011-07-04 18:08   ` Jason
2011-07-04 18:55     ` Christopher Harvey
2011-07-04 19:56       ` Wolfgang Denk
2011-07-04 20:13       ` Jason
2011-07-04 20:32         ` Christopher Harvey [this message]
2011-07-04 21:24           ` Jason
2011-07-05  7:21             ` Igor Grinberg
2011-07-05  7:31           ` Igor Grinberg
2011-07-04 19:53   ` Wolfgang Denk
2011-07-05  7:38   ` Igor Grinberg
2011-07-05 10:04     ` Wolfgang Denk
2011-07-05 10:46       ` Igor Grinberg

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110704203235.GI3016@harvey-pc.matrox.com \
    --to=charvey@matrox.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.