[PATCH] workaround v2 VIA ROMCC breakage

[Carl-Daniel Hailfinger]

Workaround v2 VIA ROMCC breakage.

Someone please tell me why the network adapter has to be handled by
romcc compiled code.

Signed-off-by: Carl-Daniel Hailfinger

Index: LinuxBIOSv2-tmp2/src/southbridge/via/vt8237r/vt8237r.h
===================================================================
--- LinuxBIOSv2-tmp2/src/southbridge/via/vt8237r/vt8237r.h	(Revision 3660)
+++ LinuxBIOSv2-tmp2/src/southbridge/via/vt8237r/vt8237r.h	(Arbeitskopie)
@@ -29,7 +29,11 @@
 #define VT8237R_SMBUS_IO_BASE		0x400
 /* 0x0 disabled, 0x2 reserved, 0xf = IRQ15 */
 #define VT8237R_ACPI_IRQ		0x9
+#if defined(__GNUC__)
 #define VT8237S_SPI_MEM_BASE		0xfed02000ULL
+#else
+#define VT8237S_SPI_MEM_BASE		0xfed02000UL
+#endif
 #define VT8237R_HPET_ADDR		0xfed00000ULL
 #define VT8237R_APIC_BASE		0xfec00000ULL

Index: LinuxBIOSv2-tmp2/src/southbridge/via/vt8237r/vt8237r_early_smbus.c
===================================================================
--- LinuxBIOSv2-tmp2/src/southbridge/via/vt8237r/vt8237r_early_smbus.c	(Revision 3660)
+++ LinuxBIOSv2-tmp2/src/southbridge/via/vt8237r/vt8237r_early_smbus.c	(Arbeitskopie)
@@ -354,9 +354,11 @@
    pci_write_config32(dev, 0xbc,
    		   (VT8237S_SPI_MEM_BASE >> 8) | (tmp & 0xFF000000));

+#if 0
    /* Set SPI clock to 33MHz. */
    spireg = (u16 *) (VT8237S_SPI_MEM_BASE + 0x6c);
    (*spireg) &= 0xff00;
+#endif
 }

/*
@@ -413,6 +415,7 @@
    if (status == 3)
    	return 0;

+#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

rom_write = (u16 *) rom;
    checksum = 0;

[Rudolf Marek]

Hello,
> +#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.
> +#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.

Rudolf

[Carl-Daniel Hailfinger]

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.

Regards,
Carl-Daniel

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.

Thanks.
> 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.

I would need to get the preprocessed output of that chunk of code
to see how bad that would be to fix.
> > > +#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.

Yes.  That sounds like something straight forward.

What is the definition of NULL and the type of rom?

Again preprocessed source is best for debugging this kind of thing.

Eric

[Carl-Daniel Hailfinger]

On 16.10.2008 19:59, Eric W. Biederman wrote:
> Carl-Daniel Hailfinger  writes:
> > I'm copying you on this mail because we managed to have ROMCC segfault
> > reliably and choke on another piece of code.
>
> Thanks.
> > 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.
>
> I would need to get the preprocessed output of that chunk of code
> to see how bad that would be to fix.

Sure. Preprocessed output follows. ROMCC will segfault with the
following command line:
romcc -mcpu=c3 -O auto.E -o auto.inc

Thanks for looking into this!

Regards,
Carl-Daniel

[Carl-Daniel Hailfinger]

On 16.10.2008 19:59, 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.
>
> Thanks.
> > > > +#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.
>
> Yes.  That sounds like something straight forward.
>
> What is the definition of NULL and the type of rom?
>
> Again preprocessed source is best for debugging this kind of thing.

The error message is:
"arithmetic type expexted"

romcc -mcpu=c3 -O auto.E -o auto.inc

Input file is attached.

Regards,
Carl-Daniel

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.

Eric

[Carl-Daniel Hailfinger]

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.

Regards,
Carl-Daniel

[Corey Osgood]

On Thu, Oct 16, 2008 at 7:32 PM, Carl-Daniel Hailfinger <
[e-mail]> 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.

There's another issue: 0x<something>ULL in vt8237r.h causes ROMCC to die,
too (garbage at the end of integer constant, iirc). My proposal was going to
be to move the vt8237_early_network_init funciton along with the 3 #defines
that cause the problem to another file, and only have that file included by
the k8-based boards. I don't remember why I trashed it, probably because we
really should just get CAR going in v2 (do we have/need a disable_car for C7
in v2?)

-Corey

[Carl-Daniel Hailfinger]

On 17.10.2008 02:17, Corey Osgood wrote:
> On Thu, Oct 16, 2008 at 7:32 PM, Carl-Daniel Hailfinger <
> [e-mail]> 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.
>
> There's another issue: 0x<something>ULL in vt8237r.h causes ROMCC to die,
> too (garbage at the end of integer constant, iirc). My proposal was going to
> be to move the vt8237_early_network_init funciton along with the 3 #defines
> that cause the problem to another file, and only have that file included by
> the k8-based boards. I don't remember why I trashed it, probably because we
> really should just get CAR going in v2 (do we have/need a disable_car for C7
> in v2?)

I added a special case for ROMCC in the 0x...ULL constants in vt8237r.h.
Please see r3664.
The remaining segfault is being investigated by Eric.

Regards,
Carl-Daniel

Carl-Daniel Hailfinger  writes:
> I added a special case for ROMCC in the 0x...ULL constants in vt8237r.h.
> Please see r3664.
> The remaining segfault is being investigated by Eric.

It is reproducible, and it happens as the intermediate expression is being
translated into intermediate code so it should be to weird to track.  I'm
going to guess 4-5 hours to sort through it.  I will aim to look at it this
weekend.

Eric

[Carl-Daniel Hailfinger]

On 17.10.2008 02:50, Eric W. Biederman wrote:
> Carl-Daniel Hailfinger  writes:
> > I added a special case for ROMCC in the 0x...ULL constants in vt8237r.h.
> > Please see r3664.
> > The remaining segfault is being investigated by Eric.
>
> It is reproducible, and it happens as the intermediate expression is being
> translated into intermediate code so it should be to weird to track.  I'm
> going to guess 4-5 hours to sort through it.  I will aim to look at it this
> weekend.

Wow, that was unexpected. Thanks for investing your time!

Regards,
Carl-Daniel

Carl-Daniel Hailfinger  writes:
> On 17.10.2008 02:50, Eric W. Biederman wrote:
> > Carl-Daniel Hailfinger  writes:
> > > I added a special case for ROMCC in the 0x...ULL constants in vt8237r.h.
> > > Please see r3664.
> > > The remaining segfault is being investigated by Eric.
> >
> > It is reproducible, and it happens as the intermediate expression is being
> > translated into intermediate code so it should be to weird to track.  I'm
> > going to guess 4-5 hours to sort through it.  I will aim to look at it this
> > weekend.
>
> Wow, that was unexpected. Thanks for investing your time!

Yes. If the bug was down in the optimizer it would take to long to track.

Eric

[Corey Osgood]

On Thu, Oct 16, 2008 at 9:28 PM, Eric W. Biederman wrote:
> Carl-Daniel Hailfinger  writes:
> > On 17.10.2008 02:50, Eric W. Biederman wrote:
> > > Carl-Daniel Hailfinger  writes:
> > > > I added a special case for ROMCC in the 0x...ULL constants in
>
> vt8237r.h.
> > > > Please see r3664.
> > > > The remaining segfault is being investigated by Eric.
> > >
> > > It is reproducible, and it happens as the intermediate expression is
>
> being
> > > translated into intermediate code so it should be to weird to track.
>
> I'm
> > > going to guess 4-5 hours to sort through it.  I will aim to look at it
>
> this
> > > weekend.
> >
> > Wow, that was unexpected. Thanks for investing your time!
>
> Yes. If the bug was down in the optimizer it would take to long to track.
>
> Eric

The break is caused by vt8237_early_spi_init(), most likely by spireg. Patch
to disable that function in ROMCC is attached, I know that via epia-cn,
pc2500, and jetway j7f2/4 don't use SPI, but I can't remember what the other
C7/CN700 board is right this moment to check. Thanks Carl-Daniel for fixing
the rest of vt8237, and Eric for your time :)

-Corey

[Corey Osgood]

On Thu, Oct 16, 2008 at 10:10 PM, Corey Osgood wrote:
> On Thu, Oct 16, 2008 at 9:28 PM, Eric W. Biederman wrote:
> > Carl-Daniel Hailfinger  writes:
> > > On 17.10.2008 02:50, Eric W. Biederman wrote:
> > > > Carl-Daniel Hailfinger  writes:
> > > > > I added a special case for ROMCC in the 0x...ULL constants in
> >
> > vt8237r.h.
> > > > > Please see r3664.
> > > > > The remaining segfault is being investigated by Eric.
> > > >
> > > > It is reproducible, and it happens as the intermediate expression is
> >
> > being
> > > > translated into intermediate code so it should be to weird to track.
> >
> > I'm
> > > > going to guess 4-5 hours to sort through it.  I will aim to look at it
> >
> > this
> > > > weekend.
> > >
> > > Wow, that was unexpected. Thanks for investing your time!
> >
> > Yes. If the bug was down in the optimizer it would take to long to track.
> >
> > Eric
>
> The break is caused by vt8237_early_spi_init(), most likely by spireg.
> Patch to disable that function in ROMCC is attached, I know that via
> epia-cn, pc2500, and jetway j7f2/4 don't use SPI, but I can't remember what
> the other C7/CN700 board is right this moment to check.

Bcom WinNet P680, which doesn't use SPI either, so we should be all good to
disable that function as far as current boards go.

-Corey

[Peter Stuge]

Corey Osgood wrote:
> Final fix for C7 boards, which are still using ROMCC, to be able to
> build. As far as I know, no C7 boards currently in the tree use SPI
> flash.
>
> Signed-off-by: Corey Osgood

Acked-by: Peter Stuge

[Corey Osgood]

On Thu, Oct 16, 2008 at 10:25 PM, Peter Stuge  wrote:
> Corey Osgood wrote:
> > Final fix for C7 boards, which are still using ROMCC, to be able to
> > build. As far as I know, no C7 boards currently in the tree use SPI
> > flash.
> >
> > Signed-off-by: Corey Osgood
>
> Acked-by: Peter Stuge

Thanks, r3665. This isn't quite the end though, cn700 boards now need a
Config-abuild.lb, but this fixes the problem on a real build. I'll be
commiting those in a few minutes.

-Corey

[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

Carl-Daniel Hailfinger  writes:
> 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 immediate failure is fixable (comparison against NULL) it just was
never implemented in romcc and is pretty simple as far as that goes.
Since the problem runs deeper, there is an easy workaround for the
unimplemented feature and we don't run the code under romcc anyway
it doesn't look like it is worth fixing right now.

Eric

[ron minnich]

On Thu, Oct 16, 2008 at 9:36 AM, Rudolf Marek  wrote:
> 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.

FWIW, this is what stage2 is for in v3. Just this kind of thing.

ron