Re: [PATCH 4/10] Support for kernel+fs images

[David M. <Dav...@se...>]

Jivin Paul Mundt lays it down ...
> On Thu, Jul 26, 2007 at 04:21:01PM +1000, David McCullough wrote:
> > Jivin Paul Mundt lays it down ...
> > > On Thu, Jul 26, 2007 at 03:27:13PM +1000, David McCullough wrote:
> > > > Add support for copying our filesystem out of the way when images are
> > > > constructed by concatenating the root filesystem to the end of a linux.bin
> > > > which has the BSS removed (to save space).
> > >
> > > That's an interesting abuse :-)
> > 
> > It's a very old thing,  our images from the beginning of time have been
> > like this.  If you don't like it,  no problems, you can get around it
> > by making images with the BSS included in the linux.bin.
>
> Besides the watchdog cruft, I don't particularly have a problem with it.
> I'll certainly accept patches for the functionality, although it will be
> for 2.6.24.

No problems,  I'll go through and clean as much as this as I can and
keep dropping bits on you.  Is the format ok ?  Anything perferred ?

> > > CONFIG_SH_ROMBOOT is not added by any of your patches, likewise for
> > > _mem_start and _rom_store. Presumably this should be sent as a follow up
> > > patch if you want this functionality included.
> > 
> > My BAD,  the CONFIG_SH_ROMBOOT was contributed by someone else and since
> > we don't use it,  I didn't need all the other bits to make it go.
> > 
> > I'll fix this if you think the "strange romfs thing" is acceptable in
> > some shape or form.
>
> If there are sufficient users of this feature, then I don't have a
> problem with merging it. If you aren't using romboot at all, then just
> drop that from the update.

I have the code needed to make it work,  but we haven't used it at all.
I can contact the guy who sent it in and see if they would like to push
it in themselves.

> > > > +	/* copy backwards to avoid writing over ourselves */
> > > > +	arch_service_watchdog();
> > > > +	while (dp >= ((unsigned char *) (&_end[0]))) {
> > > > +		*dp-- = *sp--;
> > > > +		if ((((unsigned long) dp) & 0x7ffff) == 0)
> > > > +			arch_service_watchdog();
> > > > +	}
> > > > +	arch_service_watchdog();
> > > > +}
> > > 
> > > Can you not simply disable the watchdog in your loader and start it up
> > > from the board code? Having to do dummy reads from the thing in this
> > > convoluted manner is rather undesirable.
> > 
> > The watchdog is on from poweron, it cannot be disabled in any possible
> > way.  You have at most 1 second (or less on some versions) to service it or
> > it resets the hardware.
>
> Then perhaps we should just be abstracting out the romfs copy. ie, have
> the generic copy_romfs() (possibly renamed to something more reasonable)
> take care of extracting all of the various image details, and then kick
> the addresses down to a platform-specific symbol to do the actual copy.
> This too can be a weak symbol, where the default implementation simply
> does the copy without the watchdog noise.
> 
> This way you can keep all of your platform-specific cruft isolated
> without making a compromise on functionality.

That I can do. So putting all the special cases into the boards/snapgear
area is the preferred option ?  I'll have a go and see what it looks
like.

Hopefully some more patches soon ;-)

Thanks,
Davidm

-- 
David McCullough,  dav...@se...,   Ph:+61 734352815
Secure Computing - SnapGear  http://www.uCdot.org http://www.cyberguard.com