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