Coreboot-v2 patch rom names

[Myles Watson]

This patch changes all rom names that aren't coreboot.rom in Config.lb files.

I think that since the directory specifies the architecture and the
board, it is redundant information to name it something else, and it
makes it more difficult to automate the build process (buildrom).

In buildrom we should just use Config-options.lb files instead of
patching or keeping our own.  It just adds more to maintain, with very
little benefit.  The correct place for Config.lb files is in the
coreboot-v2 tree.

The next patch would add Config-lab.lb files for each architecture
supported by buildrom.  Another patch would change buildrom to stop
patching Config.lb files.  There is already a CBV2_CONFIG variable
that would work nicely for selecting the correct file.

Comments?

Signed-off-by: Myles Watson

[Jordan Crouse]

On 05/02/08 14:20 -0700, Myles Watson wrote:
> This patch changes all rom names that aren't coreboot.rom in Config.lb files.
>
> I think that since the directory specifies the architecture and the
> board, it is redundant information to name it something else, and it
> makes it more difficult to automate the build process (buildrom).
>
> In buildrom we should just use Config-options.lb files instead of
> patching or keeping our own.  It just adds more to maintain, with very
> little benefit.  The correct place for Config.lb files is in the
> coreboot-v2 tree.
>
> The next patch would add Config-lab.lb files for each architecture
> supported by buildrom.  Another patch would change buildrom to stop
> patching Config.lb files.  There is already a CBV2_CONFIG variable
> that would work nicely for selecting the correct file.
>
> Comments?
>
> Signed-off-by: Myles Watson

Acked-by: Jordan Crouse

Are you going to patch buildrom to remove the ROM name stuff too?  If so,
I ack it now if I don't see it until later.

Jordan

[Peter Stuge]

On Tue, Feb 05, 2008 at 02:20:06PM -0700, Myles Watson wrote:
> The correct place for Config.lb files is in the coreboot-v2 tree.

So obvious when you write it out. :)
> The next patch would add Config-lab.lb files for each architecture
> supported by buildrom.

I like it!

//Peter

[Ward Vandewege]

On Tue, Feb 05, 2008 at 02:20:06PM -0700, Myles Watson wrote:
> This patch changes all rom names that aren't coreboot.rom in Config.lb files.
>
> I think that since the directory specifies the architecture and the
> board, it is redundant information to name it something else, and it
> makes it more difficult to automate the build process (buildrom).

Great.
> In buildrom we should just use Config-options.lb files instead of
> patching or keeping our own.  It just adds more to maintain, with very
> little benefit.  The correct place for Config.lb files is in the
> coreboot-v2 tree.
>
> The next patch would add Config-lab.lb files for each architecture
> supported by buildrom.

That would work - but what about other payloads? Maybe we could use a naming
scheme like

Config-buildrom-$(PAYLOAD).lb

and then make buildrom look for such a file, perhaps falling back to the
generic Config.lb file if it does not exist?

Thanks,
Ward.

[Myles Watson]

> -----Original Message-----
> From: Ward Vandewege [mailto:[e-mail]]
> Sent: Wednesday, February 06, 2008 11:34 AM
> To: Myles Watson
> Cc: Coreboot
> Subject: Re: [coreboot] Coreboot-v2 patch rom names
>
> On Tue, Feb 05, 2008 at 02:20:06PM -0700, Myles Watson wrote:
> > This patch changes all rom names that aren't coreboot.rom in Config.lb
>
> files.
> > I think that since the directory specifies the architecture and the
> > board, it is redundant information to name it something else, and it
> > makes it more difficult to automate the build process (buildrom).
>
> Great.
> > In buildrom we should just use Config-options.lb files instead of
> > patching or keeping our own.  It just adds more to maintain, with very
> > little benefit.  The correct place for Config.lb files is in the
> > coreboot-v2 tree.
> >
> > The next patch would add Config-lab.lb files for each architecture
> > supported by buildrom.
>
> That would work - but what about other payloads? Maybe we could use a
> naming
> scheme like
>
> Config-buildrom-$(PAYLOAD).lb
>
> and then make buildrom look for such a file, perhaps falling back to the
> generic Config.lb file if it does not exist?

That could work.  Right now the Config-lab.lb for each architecture is a
fallback image with compression enabled.  Maybe Config-lab.lb is not the
right name.  Any payload that doesn't use compression will need to use the
Config.lb, and any payload that uses compression should use Config-lab.lb

In some cases the ROM size is larger for the Config-lab.lb as well.

I thought about naming them Config-ROM_SIZE-COMPRESSION.lb:
    Config-1M-lzma.lb or 
    Config-512K-none.lb

But it seemed uglier.

The two attached patches implement the switch, the buildrom patch depends on
the Coreboot patch being applied first to create a revision 3091.  I tested
it by not updating the revision, then manually doing "svn up", applying the
patch to Coreboot, and rebuilding.

There is a lot of cleaning up that could be done in the Config.lb files, but
I only touched the ones that buildrom uses, so I wouldn't break anything
else.

Suggestions are welcome.

Myles

Signed-off-by: Myles Watson
> Thanks,
> Ward.

[Ward Vandewege]

On Wed, Feb 06, 2008 at 12:22:11PM -0700, Myles Watson wrote:
> > That would work - but what about other payloads? Maybe we could use a
> > naming scheme like
> >
> > Config-buildrom-$(PAYLOAD).lb
> >
> > and then make buildrom look for such a file, perhaps falling back to the
> > generic Config.lb file if it does not exist?
>
> That could work.  Right now the Config-lab.lb for each architecture is a
> fallback image with compression enabled.  Maybe Config-lab.lb is not the
> right name. Any payload that doesn't use compression will need to use the
> Config.lb, and any payload that uses compression should use Config-lab.lb
>
> In some cases the ROM size is larger for the Config-lab.lb as well.

About that - eventually I'd like to have the ROM size an option that can be
set in the buildrom menus. Each payload/board combination should default to
something reasonable (say, 1MB for an LAB payload), but it should be
configurable.
> I thought about naming them Config-ROM_SIZE-COMPRESSION.lb:
>    Config-1M-lzma.lb or 
>    Config-512K-none.lb
>
> But it seemed uglier.

Yeah. I don't really like that.
> The two attached patches implement the switch, the buildrom patch depends on
> the Coreboot patch being applied first to create a revision 3091.  I tested
> it by not updating the revision, then manually doing "svn up", applying the
> patch to Coreboot, and rebuilding.
>
> There is a lot of cleaning up that could be done in the Config.lb files, but
> I only touched the ones that buildrom uses, so I wouldn't break anything
> else.
>
> Suggestions are welcome.
>
> Myles
>
> Signed-off-by: Myles Watson

This looks good to me.

Acked-by: Ward Vandewege

Thanks,
Ward.

[Myles Watson]

> > In some cases the ROM size is larger for the Config-lab.lb as well.
>
> About that - eventually I'd like to have the ROM size an option that can
> be
> set in the buildrom menus. Each payload/board combination should default
> to
> something reasonable (say, 1MB for an LAB payload), but it should be
> configurable.

I agree.  I just don't know what the best way to do it is.  My fear is that
we'll have so many Config.lb files that we'll forget to maintain them.
> > I thought about naming them Config-ROM_SIZE-COMPRESSION.lb:
> >    Config-1M-lzma.lb or
> >    Config-512K-none.lb
> >
> > But it seemed uglier.
>
> Yeah. I don't really like that.
> > Myles
> >
> > Signed-off-by: Myles Watson
>
> This looks good to me.

Thanks,
v2 Rev 3092 and buildrom Rev 106

Myles
> Acked-by: Ward Vandewege
>
> Thanks,
> Ward.