patch: v3 device tree cleanup/fixup

[ron minnich]

Marc reviewed the v3 device tree code and we developed the set of
cleanups/fixes.

Attached.

ron

[Carl-Daniel Hailfinger]

On 27.10.2008 20:10, ron minnich wrote:
> Marc reviewed the v3 device tree code and we developed the set of
> cleanups/fixes.
>
> Fixup device tree code. Add/change methods as needed. This should help serengeti.
>
> Signed-off-by: Ronald G. Minnich

If you can get Marc to ack this, it is also
Acked-by: Carl-Daniel Hailfinger

I'd appreciate related fixups in the v3 design doc (or do we have to
rewrite that one anyway)?

One comment below, though it does not refer directly to your patch.

Regards,
Carl-Daniel
> Index: device/pci_rom.c
>
> --- device/pci_rom.c	(revision 953)
> +++ device/pci_rom.c	(working copy)
> @@ -108,7 +108,8 @@
>    rom_data = (struct pci_data *)((unsigned char *)rom_header +
>    			       le32_to_cpu(rom_header->data));

Am I missing something here or is the line above missing somebounds check?
> printk(BIOS_SPEW, "PCI ROM Image, Vendor %04x, Device %04x,\n",
> printk(BIOS_SPEW, "PCI ROM Image, @%p, Vendor %04x, Device %04x,\n",
> &rom_data->vendor,
>  rom_data->vendor, rom_data->device);
> if (dev->id.pci.vendor != rom_data->vendor || dev->id.pci.device != rom_data->device) {
> printk(BIOS_ERR,

[ron minnich]

On Mon, Oct 27, 2008 at 12:25 PM, Carl-Daniel Hailfinger
 wrote:
> On 27.10.2008 20:10, ron minnich wrote:
> > Marc reviewed the v3 device tree code and we developed the set of
> > cleanups/fixes.
> >
> > Fixup device tree code. Add/change methods as needed. This should help serengeti.
> >
> > Signed-off-by: Ronald G. Minnich
>
> If you can get Marc to ack this, it is also
> Acked-by: Carl-Daniel Hailfinger
>
> I'd appreciate related fixups in the v3 design doc (or do we have to
> rewrite that one anyway)?
>
> One comment below, though it does not refer directly to your patch.
>
> Regards,
> Carl-Daniel
> > Index: device/pci_rom.c
> >
> > --- device/pci_rom.c  (revision 953)
> > +++ device/pci_rom.c  (working copy)
> > @@ -108,7 +108,8 @@
> >       rom_data = (struct pci_data *)((unsigned char *)rom_header +
> >                                      le32_to_cpu(rom_header->data));
>
> Am I missing something here or is the line above missing somebounds check?

good catch. Bug has been there forever. That is a 16-bit field but still ...

Let's get a fix in for that one.

ron

[Corey Osgood]

On Mon, Oct 27, 2008 at 3:10 PM, ron minnich  wrote:
> Marc reviewed the v3 device tree code and we developed the set of
> cleanups/fixes.
>
> Attached.
>
> ron

I like it, but I'm sure Carl-Daniel and/or Peter will weigh in.

Acked-by: Corey Osgood

One small request, could you guys take a few minutes and make a wiki page
detailing the different stages and phases, especially what order the
different phases are run, and perhaps a short blurb as to what, in general,
each should to? Some are obvious, but others not so much.

Thanks,
Corey

[Marc Jones]

Corey Osgood wrote:
> On Mon, Oct 27, 2008 at 3:10 PM, ron minnich <[e-mail] 
> > wrote:
> Marc reviewed the v3 device tree code and we developed the set of
> cleanups/fixes.
>
> Attached.
>
> ron
>
>
> I like it, but I'm sure Carl-Daniel and/or Peter will weigh in.
>
> Acked-by: Corey Osgood <[e-mail] 
> >
>
> One small request, could you guys take a few minutes and make a wiki 
> page detailing the different stages and phases, especially what order 
> the different phases are run, and perhaps a short blurb as to what, in 
> general, each should to? Some are obvious, but others not so much.
>
> Thanks,
> Corey

I will add what I know now to the v3 page.
http://www.coreboot.org/Coreboot_v3

[ron minnich]

I greatly appreciate the quick turnaround.

OK, committed, anyone who can, please test!

Committed revision 954.

This is a *big* improvement, although it really was not a lot of lines
of code. I'm really happy with it.

ron

[Marc Jones]

Corey Osgood wrote:
> I like it, but I'm sure Carl-Daniel and/or Peter will weigh in.
>
> Acked-by: Corey Osgood <[e-mail] 
> >
>
> One small request, could you guys take a few minutes and make a wiki 
> page detailing the different stages and phases, especially what order 
> the different phases are run, and perhaps a short blurb as to what, in 
> general, each should to? Some are obvious, but others not so much.

http://www.coreboot.org/Coreboot_v3#coreboot_V3_in_Stages_and_Phases

Feel free to correct it.

Marc

[Peter Stuge]

Marc Jones wrote:
> > One small request, could you guys take a few minutes and make a
> > wiki page detailing the different stages and phases, especially
> > what order the different phases are run
> http://www.coreboot.org/Coreboot_v3#coreboot_V3_in_Stages_and_Phases

It's already written up pretty well by Ron in doc/design/newboot.lyx.

make doc gets you the pdf.

Some changes are looming though. I've also been thinking about making
stage/phase ordering clearer, possibly by removing/renaming one of
the concepts. Stage 1.1 or stage 1 substage 1 or something.

//Peter

[Marc Jones]

Peter Stuge wrote:
> Marc Jones wrote:
> > > One small request, could you guys take a few minutes and make a
> > > wiki page detailing the different stages and phases, especially
> > > what order the different phases are run
> > http://www.coreboot.org/Coreboot_v3#coreboot_V3_in_Stages_and_Phases
>
> It's already written up pretty well by Ron in doc/design/newboot.lyx.
>
> make doc gets you the pdf.
>
> Some changes are looming though. I've also been thinking about making
> stage/phase ordering clearer, possibly by removing/renaming one of
> the concepts. Stage 1.1 or stage 1 substage 1 or something.

True, It would be good if we could automate that to the wiki. It would 
also be nice to get a doxygen webpage up for v3.

Marc

[Marc Jones]

ron minnich wrote:
> Marc reviewed the v3 device tree code and we developed the set of
> cleanups/fixes.
>
> Attached.
>
> ron

Acked-by: Marc Jones