Re: [coreboot] [PATCH] workaround v2 VIA ROMCC breakage

[Carl-Daniel Hailfinger]

On 17.10.2008 01:32, Carl-Daniel Hailfinger wrote:
> On 17.10.2008 01:14, Eric W. Biederman wrote:
> > Carl-Daniel Hailfinger  writes:
> > > Hi Eric,
> > >
> > > I'm copying you on this mail because we managed to have ROMCC segfault
> > > reliably and choke on another piece of code.
> > >
> > > On 16.10.2008 18:36, Rudolf Marek wrote:
> > > > > +#if 0
> > > > >      /* Set SPI clock to 33MHz. */
> > > > >      spireg = (u16 *) (VT8237S_SPI_MEM_BASE + 0x6c);
> > > > >      (*spireg) &= 0xff00;
> > > > > +#endif
> > > >
> > > > This is OK because default is 16MHz, mtrr should handle caching for us.
> > >
> > > ROMCC segfaulted on the code above, so a small rewrite may also fix this
> > > problem. Or we could fix ROMCC.
> > > > > +#if 0
> > > > >      if (rom == NULL) {
> > > > >          print_err("No config data specified, using default MAC!\n");
> > > > >          n.mac_address[0] = 0x0;
> > > > > @@ -443,6 +446,7 @@
> > > > >          n.checksum = 0x0;
> > > > >          rom = &n;
> > > > >      }
> > > > > +#endif
> > > >
> > > > The reason why exactly this needs to be handled in rom stage is that
> > > > the shadow registers needs to be filled _before_ PCI reset, because
> > > > PCI reset will force the internal microcontroller to reload with this
> > > > configuration. Its not much documented, only in programming guide,
> > > > and there is just assembly code and some strange English ;) they
> > > > really mention that the controller should be reloaded when the device
> > > > enters D0 via PCIRST#. Dont know if for example some ->D3 and ->D0
> > > > transition will work or not.
> > >
> > > Oh my. That's quite strange hardware. Anyway, it seems that romcc mainly
> > > barfs on the
> > > if (rom == NULL)
> > > check due to unexpected typing. It should be possible to fix this.
> >
> > Likely.  I'm not certain because that function is never called in the output
> > you sent me.
> >
> > I was going to suggest changing it to just if (!rom) and then it started
> > complaining about addresses of auto variables not being supported.
> >
> > So ultimately putting #ifndef ROMCC around that whole function looks
> > like the right thing to do.
>
> That indeed sounds like a solution, thanks. The code in question is
> sometimes compiled with ROMCC and sometimes with GCC. Fortunately, the
> only case where the code is called is a single mainboard which uses CAR
> and GCC anyway.

The function is hidden from ROMCC since revision 3664.

That leaves us with the segfault problem, but I'm confident your
wizardry will fix that.

Regards,
Carl-Daniel