[coreboot] [PATCH] workaround v2 VIA ROMCC breakage

[Eric W. Biederman]

Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net> writes:

> On 17.10.2008 01:14, Eric W. Biederman wrote:
>> Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net> 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