On Thu, Jun 14, 2007 at 07:32:33PM +0200, Stefan Reinauer wrote:
- Uwe Hermann uwe@hermann-uwe.de [070607 20:35]:
Also, I tried to split up the ACPI/SMBus code into two functions, even though both devices have the same PCI ID. Is this supposed to work?
LinuxBIOS will only call one constructor per file.
One _list_ of constructors, right? But I guess it won't handle multiple constructors in that list which apply to the _same_ PCI IDs?
But anyway...
Can't you call both functions from the same constructor?
...this is the easiest solution, so I did it this way.
One issue needs to be resolved: Where/how do we set the config options from config.h? In the dts? For now they seem to default to 0 (off), thus the devices are not initialized (QEMU still boots for some reason).
See Ron's posting from these days.
Qemu needs no init. We do it for the sake of it and for the hope that it will some time emulate enough of the hw. (Its improving)
OK, then I'll leave the default values for another patch (or until we have real hardware using one of the 82371XX southbridges).
- /* TODO: ISA DMA init? */
}
DMA init is there why dont you just call it?
Done. I had some trouble in v2 with this, but that has other reasons, I guess.
- // TODO: Rename to ide_init() if it's not 82371EB specific?
it is specific. So remove TODO
I should have been more precise. It's specific to the 82371XX series, yes, but it's mostly the same for any of the 82371FB/SB/AB/EB/MB. Fixed now.
- if (conf->ide0_enable) {
/* Enable/disable UDMA/33 operation (primary IDE interface). */reg8 = pci_read_config8(dev, UDMACTL);if (conf->ide0_drive0_udma33_enable) {reg8 |= PSDE0;printk(BIOS_INFO, "Primary IDE, drive 0: UDMA/33 on\n");} else {reg8 &= ~(PSDE0);printk(BIOS_INFO, "Primary IDE, drive 0: UDMA/33 off\n");}if (conf->ide0_drive1_udma33_enable) {reg8 |= PSDE1;printk(BIOS_INFO, "Primary IDE, drive 1: UDMA/33 on\n");} else {reg8 &= ~(PSDE1);printk(BIOS_INFO, "Primary IDE, drive 1: UDMA/33 off\n");}Suggestion for code size: If you do
printk(BIOS_INFO, "%s IDE, drive %d: UDMA/33 %s\n", "Primary", 1, "on");
Without any other magic, the resulting code will be a lot smaller, since it only saves one main string, plus the words Primary/Secondary and on/off instead of a full string every time. gcc is great sometimes.
Nice, thanks.
- printk(BIOS_INFO, "SMBus controller enabled.\n");
This should be DEBUG.
OK, done. Shall we make _all_ "FOO init" messages DEBUG?
Attached is a heavily updated patch which now also supports pretty much _all_ of the 82371FB/SB/AB/EB/MB southbridges...
I also changed some 'int smbus_io = 0xfff0' (SMBus I/O Base) to a #define in i82371eb.h, I don't see a reason to make this a local variable. Is there any reason?
Uwe.