[coreboot] [Patch] Flashrom: board enable for Soyo SY-6BA+III.

Uwe Hermann uwe at hermann-uwe.de
Thu Jun 18 13:54:32 CEST 2009


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 <libv at skynet.be>

With the changes below:
Acked-by: Uwe Hermann <uwe at hermann-uwe.de>


> + * 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




More information about the coreboot mailing list