On Thu, Jun 18, 2009 at 02:47:33AM +0200, Luc Verhaegen wrote: > Board enable for soyo sy-6ba+iii. > > This board enable drops GPO26 on the southbridge (PIIX4). Since there > was another board raising GPO22 on the same southbridge, a general > routine was created reducing both board enables to a single line. > > This routine does full gpo pin checking and filters out those which > are most likely in use already, enables dual-function gpo pins properly, > and then sets the relevant gpo line. It should be fully implemented for > all gpo setting on PIIX4, except when a supposed used gpo pin does happen > to be available for gpio. > > The board itself predates proper subsystem id usage and can therefor not > be autodetected. > > Signed-off-by: Luc Verhaegen <[email protected]>
With the changes below: Acked-by: Uwe Hermann <[email protected]> > + * Helper function to raise/drop a given gpo line on intel PIIX4* > +static int intel_piix4_gpo_set(int gpo, int raise) Rename to *_piix4x_* maybe, to make it clear that PIIX4E and PIIX4M are also supported. 'gpo' should be 'unsigned int' I guess, there are no negative GPOs. > + dev = pci_dev_find(0x8086, 0x7110); /* Intel PIIX4 ISA bridge */ "PIIX4{,E,M}" please, to be explicit about what is supported. > + if (!dev) { > + fprintf(stderr, "\nERROR: Intel PIIX4 ISA bridge not found.\n"); PIIX4{,E,M} > + /* sanity check */ > + if ((gpo < 0) || (gpo > 30)) { You can drop the < 0 check by making gpo 'unsigned int'. > + fprintf(stderr, "\nERROR: Intel PIIX4 has no GPO%d.\n", gpo); PIIX4{,E,M} > + tmp |= 0x10000000; > + break; > + case 24: /* RTCSS#/GPO24 */ > + tmp |= 0x20000000; > + break; > + case 25: /* RTCALE/GPO25 */ > + tmp |= 0x40000000; > + break; > + case 26: /* KBCSS#/GPO26 */ > + tmp |= 0x80000000; Please consider using tmp |= (1 << 31); etc. as that is (IMHO) much easier to read. In the 0x0x80000000 case you have to first count the zeros (and concentrate while doing so, in order not to count too many/few) and then fire up the HEX2BIN converter in your brain in order to figure out which bit this line is actually trying to set. The (1 << 31) form is much nicer on the eye and brain. > + break; > + default: > + fprintf(stderr, "\nERROR: Unsupported PIIX4 GPO%d.\n", > gpo); PIIX4{,E,M} > + dev = pci_dev_find(0x8086, 0x7113); /* Intel PIIX4 PM */ PIIX4{,E,M} > + if (!dev) { > + fprintf(stderr, "\nERROR: Intel PIIX4 PM not found.\n"); PIIX4{,E,M} > + if (raise) > + tmp |= 0x01 << gpo; > + else > + tmp |= ~(0x01 << gpo); > + OUTL(tmp, base + 0x34); > + > + return 0; > +} > + > +/** > * Suited for VIAs EPIA M and MII, and maybe other CLE266 based EPIAs. > * > * We don't need to do this when using coreboot, GPIO15 is never lowered > there. > @@ -413,18 +485,7 @@ > */ > static int board_epox_ep_bx3(const char *name) > { > - uint8_t tmp; > - > - /* Raise GPIO22. */ > - tmp = INB(0x4036); > - OUTB(tmp, 0xEB); > - > - tmp |= 0x40; > - > - OUTB(tmp, 0x4036); > - OUTB(tmp, 0xEB); > - > - return 0; > + return intel_piix4_gpo_set(22, 1); Nice! This also makes the board_epox_ep_bx3() more generic, as the 0x4036 address might not always be 0x4036 if $SOMETHING (BIOS? OS?) decideѕ to map it elsewhere, correct? Uwe. -- http://www.hermann-uwe.de | http://www.holsham-traders.de http://www.crazy-hacks.org | http://www.unmaintained-free-software.org -- coreboot mailing list: [email protected] http://www.coreboot.org/mailman/listinfo/coreboot
