[LinuxBIOS] [PATCH] VIA VT8237R support
Rudolf Marek
r.marek at assembler.cz
Sat Oct 27 22:45:57 CEST 2007
> On your special board or on the SB in general?
Asus put Gigabit Ethernet chip. So only on my board.
> I just recently noticed that it _does_ make a big difference whether the
> PCI ID lines are there, at least for the (ISA) bridge. If it's not
> listed (no matter if the function it calls is empty or not), the stuff
> "below" won't be initialized, in my case the superio with keyboard, com
> ports, parallel port and so on...
I have there the ISA bridge.
>
>
>>>> + /* address decoding */
>>>> + /* disable the BARs, we use std ports */
>>> Why? Just curious...
>> I just dropped that.
>
> The code or the comments? Why? Not needed?
Whole stuff. It will get the ports from resource init.
>
>>>> +#define DIMM_BASE (0x50<<1)
>>> Is this really needed in vt8237r_early_smbus.c? I don't think so.
>>> This address is dependent on the board anyway, correct?
>> hmm this is first DIMM address, which is allowed in general. No eeprom will
>> occupy lower (except for the write protect control)
>
> Hm, but you never use DIMM_BASE in the SB code. Why is it defined at all?
Yes I noticed that too, and dropped. I think there was some debug stuff which
was used by me or Corey.
>>>> + /* actual addr to reg format */
>>>> + dimm = (dimm << 1);
>>>> + dimm |= 1;
>>>> + outb(dimm, SMBXMITADD);
>>>> + outb(offset, SMBHSTCMD);
>>>> + outb(0x48, SMBHSTCTL);
>>> What is 0x48? What effect does it have (should be a comment)?
>> It tells to start the byte data transaction, whit start bit set.
>
> OK, make that a comment please.
I did
>
>
>>>> +struct southbridge_via_vt8237r_config {
>>>> +
>>>> + /* function enable bits, check vt8237r.c for details */
>>>> + unsigned int fn_ctrl_lo;
>>> u16?
>> If it works then why not. I was not sure if I can use it here.
>
> Probably needs to include arch/i386/include/stdint.h, so:
>
> #include <stdint.h>
>
> But I'm not so sure I like this anyway. It's ok for now, but
> these variables (ide0_enable etc) will become run-time changeable
> via CMOS settings in v3 (I hope!) so it's probably better to split
> this up per bit. I.e.
>
> enable_ac97
> enable_mc97
> enable_rtc
> enable_lan
> etc.
>
Ok later.
>
>>>> + unsigned int fn_ctrl_hi;
>>>> + int ide0_enable:1;
>>>> + int ide1_enable:1;
>>>> + /* 1 = 80-pin cable */
>>>> + int ide0_cable:1;
>>>> + int ide1_cable:1;
>>>> +};
>
>
>>>> + print_debug("B188 device dump\n");
>>> B188?
>> it is the device ID. I dont know what it does....
>
> Hm, ok.
Nope really :/
>
>
>>> ARRAY_SIZE (later).
>> do we have it in v2?
>
> We have since r2901 :)
>
> #include <stdlib.h>
ok.
>>>> + /* FIXME Map 4MB of FLASH into the address space,
>>>> + this should be in CAR call */
>>> Am I reading this correctly? This SB supports 4MB ROM chips then?
>> Yes - FFFFFFFFh to FFC00000h. But the memory space it decodes it strapped.
>> So perhaps for LB I should set in CAR to maximum value.
>
> Btw, do we have flashrom support for VT8237R already? Does it work on
> your board?
Yes it does work.
debian:/mnt/disc4/Moje_dilna/LinuxBIOSv2/targets/asus/a8v-e_se/a8v-e_se#
../../../../util/flashrom/flashrom -v linuxbios.rom
Calibrating delay loop... ok
Found LinuxBIOS table at: 00000530
vendor id: ASUS part id: A8V-E SE
Found chipset "VT8237": Enabling flash write... OK.
Pm49FL004 found at physical address: 0xfff80000
Flash part is Pm49FL004 (512 KB)
Flash image seems to be a legacy BIOS. Disabling checks.
Verifying flash - VERIFIED
>>> + /* pci_write_config8(dev, 0x41, 0x7f); */
>>>> +
>>>> + /* Set bit 6 of 0x40, because Award does it (IO recovery time)
>>> "because AWARD does it" no good...
>> Hmm this comment was not written by me, but this nbit should be set.
>
> OK, then replace the comment with something meaningful please.
ok
>
>
>>>> + /* enable HPET at: */
>>>> + pci_write_config32(dev, 0x68, (VT8237R_HPET_ADDR | 0x80));
>>> Maybe make it configurable via an hpet_enable variable?
>> could be, but this is the default HPET address for all x86.
>
> Yep. What I meant is to make it configurable whether to enable HPET or
> not (similar to ide0_enable etc).
aha ok. I think we can leave it now as it is.
>
>
>>> Hm, this is superio stuff usually. As the VT8237R integrates these kinds
>>> of functions I'm not sure how to procede. Extract the Superio-related
>>> stuff into src/superio/via/vt8237r/* or leave it here?
>>> Opinions?
>> it has just RTC and keyboard. No other stuff.
>
> Ah, ok. In that case that stuff is better placed here, indeed.
Ok.
Rudolf
More information about the coreboot
mailing list