Config.lb includes

[Carl-Daniel Hailfinger]

We have 14 Config.lb files in the mainboard dir of our tree which all
contain exactly the same first section (comments and whitespace are
vastly different, but the code is identical):

Ron, you wrote that Config.lb can do include statements. I didn't find
any examples, but maybe I just looked at the wrong part of the code.
Tell me how to use the include statements and I'll refactor the section
below out of lots of boards.
--------------------------------------
##
## Compute the location and size of where this firmware image
## (coreboot plus bootloader) will live in the boot rom chip.
##
if USE_FAILOVER_IMAGE
        default ROM_SECTION_SIZE   = FAILOVER_SIZE
        default ROM_SECTION_OFFSET = ( ROM_SIZE - FAILOVER_SIZE )
else
    if USE_FALLBACK_IMAGE
        default ROM_SECTION_SIZE   = FALLBACK_SIZE
        default ROM_SECTION_OFFSET = ( ROM_SIZE - FALLBACK_SIZE -
FAILOVER_SIZE )
    else
        default ROM_SECTION_SIZE   = ( ROM_SIZE - FALLBACK_SIZE -
FAILOVER_SIZE )
        default ROM_SECTION_OFFSET = 0
    end
end

##
## Compute the start location and size size of
## The coreboot bootloader.
##
default PAYLOAD_SIZE             = ( ROM_SECTION_SIZE - ROM_IMAGE_SIZE )
default CONFIG_ROM_PAYLOAD_START = (0xffffffff - ROM_SIZE +
ROM_SECTION_OFFSET + 1)

##
## Compute where this copy of coreboot will start in the boot rom
##
default _ROMBASE      = ( CONFIG_ROM_PAYLOAD_START + PAYLOAD_SIZE )

##
## Compute a range of ROM that can cached to speed up coreboot,
## execution speed.
##
## XIP_ROM_SIZE must be a power of 2.
## XIP_ROM_BASE must be a multiple of XIP_ROM_SIZE
##
default XIP_ROM_SIZE=65536
#
if USE_FAILOVER_IMAGE
        default XIP_ROM_BASE = ( _ROMBASE - XIP_ROM_SIZE + ROM_IMAGE_SIZE)
else
    if USE_FALLBACK_IMAGE
        default XIP_ROM_BASE = ( _ROMBASE - XIP_ROM_SIZE +
ROM_IMAGE_SIZE + FAILOVER_SIZE)
    else
        default XIP_ROM_BASE = ( _ROMBASE - XIP_ROM_SIZE + ROM_IMAGE_SIZE)
    end
end
--------------------------------------

I'd like to place the section above in a generic
failover_size_calculation.lb file included by the respective targets.

Regards,
Carl-Daniel

[ron minnich]

basically, if you do this:

dir some-dir-name

and the tool finds a Config.lb in that directory, it includes it.

But I wonder if you could put those calculations in the
arch/i386/Config.lb file. In other words, is there some file that is a
good place for these common defines?

thanks

ron

[Peter Stuge]

ron minnich wrote:
> In other words, is there some file that is a good place for these
> common defines?

I think a new file with a descriptive name is a good idea, and I also
think it makes a lot of sense to include it from a file which is
already included everywhere.

What about the ppc mainboards? They can also do failover, right?

//Peter

[ron minnich]

On Sun, Apr 12, 2009 at 9:03 PM, Peter Stuge  wrote:
> ron minnich wrote:
> > In other words, is there some file that is a good place for these
> > common defines?
>
> I think a new file with a descriptive name is a good idea, and I also
> think it makes a lot of sense to include it from a file which is
> already included everywhere.
>
> What about the ppc mainboards? They can also do failover, right?

not sure. The failover stuff post-dates working versions of ppc mainboards ...

ron

[Carl-Daniel Hailfinger]

On 13.04.2009 02:13, ron minnich wrote:
> basically, if you do this:
>
> dir some-dir-name
>
> and the tool finds a Config.lb in that directory, it includes it.
>
> But I wonder if you could put those calculations in the
> arch/i386/Config.lb file. In other words, is there some file that is a
> good place for these common defines?

I'd like to factor these snippets out first, then decide where to put them.

Unfortunately, if I use the "dir" syntax, I have to create a directory
for every snippet. I tried to change config.g, but it's too difficult to
modify it to have another command which takes a file name instead of a
directory for the include operation.

Any help is appreciated.

Regards,
Carl-Daniel

[ron minnich]

On Fri, Apr 17, 2009 at 7:53 AM, Carl-Daniel Hailfinger
 wrote:
> Unfortunately, if I use the "dir" syntax, I have to create a directory
> for every snippet. I tried to change config.g, but it's too difficult to
> modify it to have another command which takes a file name instead of a
> directory for the include operation.
>
> Any help is appreciated.

I can add an include command if you want.

But I still think those make rules could go in an existing Config.lb.

ron

[Myles Watson]

On Fri, Apr 17, 2009 at 9:28 AM, ron minnich  wrote:
> On Fri, Apr 17, 2009 at 7:53 AM, Carl-Daniel Hailfinger
>  wrote:
> > Unfortunately, if I use the "dir" syntax, I have to create a directory
> > for every snippet. I tried to change config.g, but it's too difficult to
> > modify it to have another command which takes a file name instead of a
> > directory for the include operation.
> >
> > Any help is appreciated.
>
> I can add an include command if you want.

Maybe this one will be good enough while he's factoring things out.

I made it so it always includes from /config/shared.  I think it
should be easy to modify this rule for your needs.

Thanks,
Myles

[Myles Watson]

On Fri, Apr 17, 2009 at 9:50 AM, Myles Watson  wrote:
> On Fri, Apr 17, 2009 at 9:28 AM, ron minnich  wrote:
> > On Fri, Apr 17, 2009 at 7:53 AM, Carl-Daniel Hailfinger
> >  wrote:
> > > Unfortunately, if I use the "dir" syntax, I have to create a directory
> > > for every snippet. I tried to change config.g, but it's too difficult to
> > > modify it to have another command which takes a file name instead of a
> > > directory for the include operation.
> > >
> > > Any help is appreciated.
> >
> > I can add an include command if you want.
>
> Maybe this one will be good enough while he's factoring things out.
>
> I made it so it always includes from /config/shared.  I think it
> should be easy to modify this rule for your needs.

I forgot to include a usage note.  I tested it by adding:

sharedinclude "Config2.lb"

to src/northbridge/amd/amdk8/Config.lb

and it looks in

src/config/shared/Config2.lb

Thanks,
Myles

[ron minnich]

On Fri, Apr 17, 2009 at 9:16 AM, Myles Watson  wrote:
> I forgot to include a usage note.  I tested it by adding:
>
> sharedinclude "Config2.lb"
>
> to src/northbridge/amd/amdk8/Config.lb
>
> and it looks in
>
> src/config/shared/Config2.lb

We used to have these kinds of commands that sort of "built in" a
path. E.g. northbridge intel/xyz had an implicit path of
src/northbridge/intel/xyz.

We took them out at the request of a vendor. So, just checking, is
this really better than
include /config/shared/Config2.lb
?

Also, Config2? Is there a better name, since you did a good thing and
let us use any name we want?

thanks

ron

[Myles Watson]

On Fri, Apr 17, 2009 at 10:20 AM, ron minnich  wrote:
> On Fri, Apr 17, 2009 at 9:16 AM, Myles Watson  wrote:
> > I forgot to include a usage note.  I tested it by adding:
> >
> > sharedinclude "Config2.lb"
> >
> > to src/northbridge/amd/amdk8/Config.lb
> >
> > and it looks in
> >
> > src/config/shared/Config2.lb
>
> We used to have these kinds of commands that sort of "built in" a
> path. E.g. northbridge intel/xyz had an implicit path of
> src/northbridge/intel/xyz.
>
> We took them out at the request of a vendor. So, just checking, is
> this really better than
> include /config/shared/Config2.lb
> ?

Not really.  I guess I couldn't tell from Carl-Daniel's request what
exactly he wanted.  I thought this would be enough to let him make
something work.  I admit it was quick and dirty.  It could be easily
changed to take a path instead of a string.
> Also, Config2? Is there a better name, since you did a good thing and
> let us use any name we want?

It was just an example to make it obvious that it wasn't hardcoded to
Config.lb :)

Thanks for the review,
Myles

[ron minnich]

On Fri, Apr 17, 2009 at 9:25 AM, Myles Watson  wrote:
> Not really.  I guess I couldn't tell from Carl-Daniel's request what
> exactly he wanted.  I thought this would be enough to let him make
> something work.  I admit it was quick and dirty.  It could be easily
> changed to take a path instead of a string.

Since you are up to speed on the config tool, and know how to improve
it :-), I'd like to request a general purpose include command instead
(or see if there is already one in there I forgot about :-)

Then, once you have that, carl-daniel can figure out where he wants
this include file, and then we are done!

Thanks

ron

[Myles Watson]

On Fri, Apr 17, 2009 at 10:28 AM, ron minnich  wrote:
> On Fri, Apr 17, 2009 at 9:25 AM, Myles Watson  wrote:
> > Not really.  I guess I couldn't tell from Carl-Daniel's request what
> > exactly he wanted.  I thought this would be enough to let him make
> > something work.  I admit it was quick and dirty.  It could be easily
> > changed to take a path instead of a string.
>
> Since you are up to speed on the config tool, and know how to improve
> it :-), I'd like to request a general purpose include command instead
> (or see if there is already one in there I forgot about :-)

I didn't find one when I looked.  I think it would be hard to make one
that worked in targets/*/*/Config.lb files and src/.../Config.lb
files, but I guess no one has asked for that.

How general are you thinking?  I'm probably willing to do this:

include acpi.lb

include /acpi.lb

Where acpi.lb includes from the current directory and /acpi.lb
includes from /src/acpi.lb

Thanks,
Myles

[Stefan Reinauer]

Myles Watson schrieb:
> On Fri, Apr 17, 2009 at 10:28 AM, ron minnich  wrote:
> > On Fri, Apr 17, 2009 at 9:25 AM, Myles Watson  wrote:
> > > Not really.  I guess I couldn't tell from Carl-Daniel's request what
> > > exactly he wanted.  I thought this would be enough to let him make
> > > something work.  I admit it was quick and dirty.  It could be easily
> > > changed to take a path instead of a string.
> >
> > Since you are up to speed on the config tool, and know how to improve
> > it :-), I'd like to request a general purpose include command instead
> > (or see if there is already one in there I forgot about :-)
>
> I didn't find one when I looked.  I think it would be hard to make one
> that worked in targets/*/*/Config.lb files and src/.../Config.lb
> files, but I guess no one has asked for that.
>
> How general are you thinking?  I'm probably willing to do this:
>
> include acpi.lb
>
> include /acpi.lb
>
> Where acpi.lb includes from the current directory and /acpi.lb
> includes from /src/acpi.lb

Whats wrong with the "dir" command?

Especially since we're working on getting rid of Config.lb in favor of 
Kconfig..

[Myles Watson]

On Fri, Apr 17, 2009 at 11:15 AM, Stefan Reinauer  wrote:
> Myles Watson schrieb:
> > On Fri, Apr 17, 2009 at 10:28 AM, ron minnich  wrote:
> > > On Fri, Apr 17, 2009 at 9:25 AM, Myles Watson  wrote:
> > > > Not really.  I guess I couldn't tell from Carl-Daniel's request what
> > > > exactly he wanted.  I thought this would be enough to let him make
> > > > something work.  I admit it was quick and dirty.  It could be easily
> > > > changed to take a path instead of a string.
> > >
> > > Since you are up to speed on the config tool, and know how to improve
> > > it :-), I'd like to request a general purpose include command instead
> > > (or see if there is already one in there I forgot about :-)
> >
> > I didn't find one when I looked.  I think it would be hard to make one
> > that worked in targets/*/*/Config.lb files and src/.../Config.lb
> > files, but I guess no one has asked for that.
> >
> > How general are you thinking?  I'm probably willing to do this:
> >
> > include acpi.lb
> >
> > include /acpi.lb
> >
> > Where acpi.lb includes from the current directory and /acpi.lb
> > includes from /src/acpi.lb
>
> Whats wrong with the "dir" command?
>
> Especially since we're working on getting rid of Config.lb in favor of
> Kconfig..

I think the idea was a little more flexibility.  If it is going away
soon it should be fine to let cleanups happen.  Hopefully they'll
speed up the conversion to Kconfig when it happens.

This include is very simple and related to the dir command.

Implemented with relative and absolute paths.

Signed-off-by: Myles Watson

Thanks,
Myles

Myles

[ron minnich]

Acked-by: Ronald G. Minnich

[Myles Watson]

On Fri, Apr 17, 2009 at 11:52 AM, ron minnich  wrote:
> Acked-by: Ronald G. Minnich

Rev 4131.

Thanks,
Myles

[Carl-Daniel Hailfinger]

On 17.04.2009 19:38, Myles Watson wrote:
> On Fri, Apr 17, 2009 at 11:15 AM, Stefan Reinauer  wrote:
> > Myles Watson schrieb:
> > > On Fri, Apr 17, 2009 at 10:28 AM, ron minnich  wrote:
> > > > On Fri, Apr 17, 2009 at 9:25 AM, Myles Watson  wrote:
> > > > > Not really.  I guess I couldn't tell from Carl-Daniel's request what
> > > > > exactly he wanted.  I thought this would be enough to let him make
> > > > > something work.  I admit it was quick and dirty.  It could be easily
> > > > > changed to take a path instead of a string.
> > > >
> > > > Since you are up to speed on the config tool, and know how to improve
> > > > it :-), I'd like to request a general purpose include command instead
> > > > (or see if there is already one in there I forgot about :-)
> > >
> > > I didn't find one when I looked.  I think it would be hard to make one
> > > that worked in targets/*/*/Config.lb files and src/.../Config.lb
> > > files, but I guess no one has asked for that.
> > >
> > > How general are you thinking?  I'm probably willing to do this:
> > >
> > > include acpi.lb
> > >
> > > include /acpi.lb
> > >
> > > Where acpi.lb includes from the current directory and /acpi.lb
> > > includes from /src/acpi.lb
> >
> > Whats wrong with the "dir" command?

If you want to factor out some small chunks, having to create a separate
directory for each chunk is horrible.
> > Especially since we're working on getting rid of Config.lb in favor of
> > Kconfig..
>
> I think the idea was a little more flexibility.  If it is going away
> soon it should be fine to let cleanups happen.  Hopefully they'll
> speed up the conversion to Kconfig when it happens.
>
> This include is very simple and related to the dir command.
>
> Implemented with relative and absolute paths.
>
> Signed-off-by: Myles Watson

Awesome, thanks!

If this works, it is
Acked-by: Carl-Daniel Hailfinger

Regards,
Carl-Daniel

[ron minnich]

On Fri, Apr 17, 2009 at 9:34 AM, Myles Watson  wrote:
> How general are you thinking?  I'm probably willing to do this:
>
> include acpi.lb
>
> include /acpi.lb
>
> Where acpi.lb includes from the current directory and /acpi.lb

Good enough. Since Stefan points out that this is a short term issue.

ron