[U-Boot] granularity of bootm subcommands

[U-Boot] granularity of bootm subcommands

[Kumar Gala]

So I proposed a set of subcommands that looked like:

   bootm start <args>
   bootm prep
   bootm load_os
   bootm load_fdt
   bootm load_initrd
   bootm jump
   bootm restore

is this the right granularity or should we go finer?

"prep" could be:

   bootm disable_interrupts
   bootm usb_stop
   bootm disable_caches

"load_os" stays as is.

"load_fdt" could be:

   bootm load_fdt
   bootm boardsetup_fdt
   bootm fdt_resize

"load_initrd" could be:
   bootm load_initrd
   bootm fdt_initrd_fixup

"jump" stays as is.

"restore" could be:
   bootm enable_interrupts
   bootm usb_start
   bootm enable_caches

Also I'm adding in:
   bootm cmdline
   bootm kbd (kernel bd_t)

Finally I'd like to change the subcommands to be prefixed with '_' to  
make parsing easier:
   bootm _start <args>
   bootm _prep
   bootm _load_os
   ...

- k

[U-Boot] granularity of bootm subcommands

[Jerry Van Baren]

Kumar Gala wrote:
> So I proposed a set of subcommands that looked like:
> 
>    bootm start <args>
>    bootm prep
>    bootm load_os
>    bootm load_fdt
>    bootm load_initrd
>    bootm jump
>    bootm restore
> 
> is this the right granularity or should we go finer?
> 
> "prep" could be:
> 
>    bootm disable_interrupts
>    bootm usb_stop

This is "usb stop"

>    bootm disable_caches

This is "icache off && dcache off"

> "load_os" stays as is.

Decompresses to a destination address?  I didn't see a gunzip/bz2 
command in my brief search.  I would rather have a "decompress" general 
purpose command (perhaps it is/should be part of the "image" (im*) 
command set?).

I'm speculating based on a very brief look at the code/structs that an 
image command could do a "load" on a kernel image and ramdisk image 
identically (filling out an image header struct as a side effect?).  For 
"FIT" images, there are potentially more pieces, but the concept would 
be the same.

I'm kinda hung up on how to use the image header struct.  I think we 
want to be able to reference values for scripting (e.g. fixing up fdt 
properties) which implies the values need to be put into env variables 
(ugh, lots of 'em) or we need a way for an "image" command (?) to pull 
values out of the image header struct?

<fill in clever ideas here>

> "load_fdt" could be:
> 
>    bootm load_fdt

This is probably "fdt move"? (which is a misnomer: it actually is a copy 
- my bad)  The fdt source and destination addresses need to be 
determined (source from the image or "fdt address", malloc/lmb the 
destination?).  I would advocate an "image" (iminfo?) command that sets, 
as a side effect env variables or the image header structure you 
identified previously.

Obviously, I have not thought through this very carefully.

>    bootm boardsetup_fdt

Use "fdt boardsetup"

>    bootm fdt_resize

"fdt move" with a size will do a resize.  Perhaps a new "fdt resize" 
command to make it more obvious.  Do we really need to resize, can we 
just make the fdt blob "big enough" (at the source or when we do the 
"fdt move") and not worry about it?  Note that I addressed this 
previously with the "-S" dtc flag (now deprecated, use -p instead) and 
forced the problem on whoever built the blob originally.  Not 100% 
satisfactory, but making it SEP is very effective. ;-)

> "load_initrd" could be:
>    bootm load_initrd

See "load_os" comments on it being an "image" command.

>    bootm fdt_initrd_fixup

This is fixing up the fdt blob to point to the initrd?  That should be 
doable with a couple of "fdt set <path> <value>" commands.

> "jump" stays as is.

YEAH!  The Core Functionality.

> "restore" could be:
>    bootm enable_interrupts
>    bootm usb_start

"usb reset" looks like it does this?

>    bootm enable_caches

icache on && dcache on

> Also I'm adding in:
>    bootm cmdline

Use "fdt set <path> $cmdline"?

>    bootm kbd (kernel bd_t)

I presume "kbd" is shorthand for "kernel bd", aka. legacy boot.  Maybe 
"bootm legacybd"?  (shortenable to "bootm legacy")

> Finally I'd like to change the subcommands to be prefixed with '_' to  
> make parsing easier:
>    bootm _start <args>
>    bootm _prep
>    bootm _load_os

Bleah, I dislike typing underscores, it requires me to use both hands 
and seriously impacts my ability to drink beer. :-P  Since these 
commands will all be scripted (normally), I can live with that.

Thanks,
gvb

[U-Boot] granularity of bootm subcommands

[Kumar Gala]

>>   bootm usb_stop
>
> This is "usb stop"

So this case is easy since its a compile time choice.

>>   bootm disable_caches
>
> This is "icache off && dcache off"

I assume you are suggesting new toplevel cache commands.

>> "load_os" stays as is.
>
> Decompresses to a destination address?  I didn't see a gunzip/bz2  
> command in my brief search.  I would rather have a "decompress"  
> general purpose command (perhaps it is/should be part of the  
> "image" (im*) command set?).

This is orthogonal to the bootm discussion.  We can refactor the code  
to allow for such a toplevel command and reuse the code.  But the  
bootm commands CAN NOT require control flow in the scripting lang.   
Thus the desire to decompress or not, and what type has to be hidden  
inside a command like "load_os".  I'm ok w/providing and optional  
argument to the command to override the desired dest addr.

>> "load_fdt" could be:
>>   bootm load_fdt
>
> This is probably "fdt move"? (which is a misnomer: it actually is a  
> copy - my bad)  The fdt source and destination addresses need to be  
> determined (source from the image or "fdt address", malloc/lmb the  
> destination?).  I would advocate an "image" (iminfo?) command that  
> sets, as a side effect env variables or the image header structure  
> you identified previously.
>
> Obviously, I have not thought through this very carefully.

Some of this can be dealt with in a second pass.

>>   bootm boardsetup_fdt
>
> Use "fdt boardsetup"

We can't use it directly.  Remember the unconditionality/control issues.

>>   bootm fdt_resize
>
> "fdt move" with a size will do a resize.  Perhaps a new "fdt resize"  
> command to make it more obvious.  Do we really need to resize, can  
> we just make the fdt blob "big enough" (at the source or when we do  
> the "fdt move") and not worry about it?  Note that I addressed this  
> previously with the "-S" dtc flag (now deprecated, use -p instead)  
> and forced the problem on whoever built the blob originally.  Not  
> 100% satisfactory, but making it SEP is very effective. ;-)

Andy providing a patch that makes -S/-p not needed anymore.  I'd  
prefer to keep that functionality.  So we should have resize support.

The key to remember is in the first pass we have to be able to do all  
the steps in order and it work regardless of the input differences..  
thus mimicking the current bootm functionality.  This means no  
(if..than..else) which means we can't use any of the current commands  
directly.

- k

[U-Boot] granularity of bootm subcommands

[Jerry Van Baren]

Kumar Gala wrote:
>>>   bootm usb_stop
>>
>> This is "usb stop"
> 
> So this case is easy since its a compile time choice.
> 
>>>   bootm disable_caches
>>
>> This is "icache off && dcache off"
> 
> I assume you are suggesting new toplevel cache commands.

No, existing: cmd_cache.c

>>> "load_os" stays as is.
>>
>> Decompresses to a destination address?  I didn't see a gunzip/bz2 
>> command in my brief search.  I would rather have a "decompress" 
>> general purpose command (perhaps it is/should be part of the "image" 
>> (im*) command set?).
> 
> This is orthogonal to the bootm discussion.  We can refactor the code to 
> allow for such a toplevel command and reuse the code.

Agreed on the orthogonality.  Last night I started looking at image.c to 
how much "bootm" functionality is implemented in image.c.  My thought is 
that the image manipulation functions (iminfo, imtest, imls) should be 
factored out of cmd_bootm.c into a new cmd_image.c (or something like 
that) and I'm thinking we need some new "image" commands to do the image 
processing for OS images (various types), ramdisk images, and ?others?.

> But the bootm commands CAN NOT require control flow in the scripting
> lang.  Thus the desire to decompress or not, and what type has to be
> hidden inside a command like "load_os".  I'm ok w/providing and
> optional argument to the command to override the desired dest addr.

Wolfgang advocated supporting the old parser.  Personally, I'm OK with 
requiring hush in order to enable the New Improved bootm behavior and 
leaving users of the old parser with the old bootm (arrrr, more 
#ifdefs).  I'm not sure we can get to a reasonable state without /some/ 
hush conditionals.

I think it would be foolish to cripple or kill a bootm refactor simply 
because we were unwilling to require hush.

>>> "load_fdt" could be:
>>>   bootm load_fdt
>>
>> This is probably "fdt move"? (which is a misnomer: it actually is a 
>> copy - my bad)  The fdt source and destination addresses need to be 
>> determined (source from the image or "fdt address", malloc/lmb the 
>> destination?).  I would advocate an "image" (iminfo?) command that 
>> sets, as a side effect env variables or the image header structure you 
>> identified previously.
>>
>> Obviously, I have not thought through this very carefully.
> 
> Some of this can be dealt with in a second pass.
> 
>>>   bootm boardsetup_fdt
>>
>> Use "fdt boardsetup"
> 
> We can't use it directly.  Remember the unconditionality/control issues.

Either conditionals or have two scripts, one with fdt and one without 
(which still is a conditional, just SEP to implement the conditional).

My drive is to get bootm to only boot stuff out of memory and I'm 
fixating on getting bootm out of the business of rearranging and fixing 
up stuff in memory.  IMHO there are or should be other commands to do 
all the rearranging and fixing up.  Well, as much as possible anyway.

>>>   bootm fdt_resize
>>
>> "fdt move" with a size will do a resize.  Perhaps a new "fdt resize" 
>> command to make it more obvious.  Do we really need to resize, can we 
>> just make the fdt blob "big enough" (at the source or when we do the 
>> "fdt move") and not worry about it?  Note that I addressed this 
>> previously with the "-S" dtc flag (now deprecated, use -p instead) and 
>> forced the problem on whoever built the blob originally.  Not 100% 
>> satisfactory, but making it SEP is very effective. ;-)
> 
> Andy providing a patch that makes -S/-p not needed anymore.  I'd prefer 
> to keep that functionality.  So we should have resize support.

Yes, but I would like it to be built into the fdt command, not the bootm 
command.

While autoresizing is cool, I struggle seeing why we need to work so 
hard.  Adding 4K padding isn't /that/ bad (ducks as Wolfgang swings). 
Doing two passes is ugly IMHO.  Oh well, another dog, another day.

> The key to remember is in the first pass we have to be able to do all 
> the steps in order and it work regardless of the input differences.. 
> thus mimicking the current bootm functionality.  This means no 
> (if..than..else) which means we can't use any of the current commands 
> directly.
> 
> - k

Hush little baby, don't say a word...
gvb

<http://kids.niehs.nih.gov/lyrics/hushbaby.htm>

[U-Boot] granularity of bootm subcommands

[Wolfgang Denk]

Dear Jerry Van Baren,

In message <489C7A65.50709@ge.com> you wrote:
>
> Wolfgang advocated supporting the old parser.  Personally, I'm OK with 
> requiring hush in order to enable the New Improved bootm behavior and 
> leaving users of the old parser with the old bootm (arrrr, more 
> #ifdefs).  I'm not sure we can get to a reasonable state without /some/ 
> hush conditionals.
> 
> I think it would be foolish to cripple or kill a bootm refactor simply 
> because we were unwilling to require hush.

I think that the default boot behaviour will not need *any* shell
commands, because we can implement this easily in C function calls.

It is just when a user wants to redefine boot behaviour that the shell
interface comes into play, and then it's the user's decision if he has
simple needs that can be implemented with the old parser, or if he
needs more scripting capabilities and has to enable hush.

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
Things that try to look like things often do look  more  like  things
than things. Well-known fact.       - Terry Pratchett, _Wyrd Sisters_

[U-Boot] granularity of bootm subcommands

[Scott Wood]

On Fri, Aug 08, 2008 at 08:37:47AM -0500, Kumar Gala wrote:
> Finally I'd like to change the subcommands to be prefixed with '_' to  
> make parsing easier:
>    bootm _start <args>
>    bootm _prep
>    bootm _load_os
>    ...

Ick.  How about using a different name than "bootm"?

-Scott

[U-Boot] granularity of bootm subcommands

[Jerry Van Baren]

Scott Wood wrote:
> On Fri, Aug 08, 2008 at 08:37:47AM -0500, Kumar Gala wrote:
>> Finally I'd like to change the subcommands to be prefixed with '_' to  
>> make parsing easier:
>>    bootm _start <args>
>>    bootm _prep
>>    bootm _load_os
>>    ...
> 
> Ick.  How about using a different name than "bootm"?
> 
> -Scott

Yeah, reusing "bootm" may be too clever.

What I had in mind for overloading bootm is to do strncmp() operations 
for each of the subcommands (and where none of the subcommands could be 
interpreted as hex numbers - i.e. have a letter g-z or "_" in it) and, 
if none of the valid subcommands passed, do a legacy implementation 
where the three parameters are parsed as hex (or "-" for the ramdisk).

Again, while I like the concept of using "bootm" since we are familiar 
with it (and I don't have a better name), I would not be adverse to 
using a different command name since the New Improved "bootm" will be 
substantially different than bootm-classic.

Best regards,
gvb