[coreboot] v3 Tyan s2892

[Myles Watson]

On Mon, Jan 5, 2009 at 4:07 PM, Peter Stuge <peter at stuge.se> wrote:
> Myles Watson wrote:
>> This patch adds initial support for mainboard/tyan/s2892.  It
>> compiles, but is untested in hardware.
>
> I think this needs a little polishing.

> Does Kconfig allow choices to span multiple files? If so it would be
> nice to not have to repeat the Mainboard model choice for each
> vendor.

I don't know how to do that.  It would affect all vendors currently in
v3, not just the one I'm adding, though.

>> +++ svn/mainboard/tyan/s2892/stage1.c
> ..
>> +void ck804_enable_rom(void);
>
> This prototype shouldn't be needed here right?

It won't compile without it.  It's possible that the sources could be
rearranged so that it doesn't need to be there.

> What ROM is it anyway?

The flashROM where coreboot is stored.  Sorry I'm not sure of the
correct terminology there.

>
>> +#define SERIAL_DEV W83627HF_SP1
>> +#define SERIAL_IOBASE 0x3f8
>
> Isn't at least the iobase in a CONFIG_ already?
No.  It's in the dts only, which isn't available yet.

>> +void mainboard_pre_payload(void)
>> +{
>> +     banner(BIOS_DEBUG, "mainboard_pre_payload: done");
>> +}
>
> Can we remove this function from mainboard sources when it doesn't do
> anything like here?

This function is only used for geode right now, but in geode it's used
to disable caching of the ROM.  I think that's up in the air right
now.

>
>> +++ svn/mainboard/tyan/s2892/initram.c
> ..
>> +/* this code is very mainboard dependent, sadly. */
> ..
>> +/**
>> + * read a byte from spd.
>> + * @param device device to read from
>> + * @param address address in the spd ROM
>> + * @return the value of the byte at that address.
>> + */
>> +u8 spd_read_byte(u16 device, u8 address)
>> +{
>> +     int smbus_read_byte(u16 device, u16 address);
>> +     return smbus_read_byte(device, address);
>> +}
>
> I hope you see the irony above.
Yes.  I was hoping to unify the code when we had two (maybe three)
working k8 boards in v3.  I still haven't figured out v2's execution
path.  cache_as_ram_auto.c vs. auto.c vs. ...

I'd love help here.

>> +
>> +/**
>> +  * main for initram for the AMD Serengeti
>
> No, this is for s2892. If this code is copypaste, it can not go in.
> I would like us to reuse more.

Me too.  See above.

Thanks again,
Myles