[LinuxBIOS] [PATCH] Flashrom -- Add W83627THF and K8N-Neo3 support
Uwe Hermann
uwe at hermann-uwe.de
Mon Oct 8 00:42:30 CEST 2007
Hi,
some comments below:
On Sat, Oct 06, 2007 at 07:15:13PM -0700, David Hendricks wrote:
> Index: board_enable.c
> ===================================================================
> --- board_enable.c (revision 2763)
> +++ board_enable.c (working copy)
> @@ -32,43 +32,43 @@
> /*
> * Helper functions for many Winbond Super I/Os of the W836xx range.
> */
> -#define W836_INDEX 0x2E
> -#define W836_DATA 0x2F
> +#define W836_INDEX_2E 0x2E
> +#define W836_INDEX_4E 0x4E
Hm, not sure the #define makes much sense anymore if the value is
encoded in the name anyway(?) I think we can drop the #defines
(but the code should still be able to cope with both 0x2e and 0x4e).
>
> /* Enter extended functions */
> -static void w836xx_ext_enter(void)
> +static void w836xx_ext_enter(unsigned char index)
'unsigned char' -> 'uint16_t'
Maybe also 'index' -> 'port'.
> {
> - outb(0x87, W836_INDEX);
> - outb(0x87, W836_INDEX);
> + outb(0x87, index);
> + outb(0x87, index);
> }
>
> /* Leave extended functions */
> -static void w836xx_ext_leave(void)
> +static void w836xx_ext_leave(unsigned char index)
> {
> - outb(0xAA, W836_INDEX);
> + outb(0xAA, index);
> }
>
> /* General functions for reading/writing Winbond Super I/Os. */
> -static unsigned char wbsio_read(unsigned char index)
> +static unsigned char wbsio_read(unsigned char index, unsigned char reg)
index -> uint16_t
reg -> uint8_t
> {
> - outb(index, W836_INDEX);
> - return inb(W836_DATA);
> + outb(reg, index);
> + return inb(index+1);
> }
>
> -static void wbsio_write(unsigned char index, unsigned char data)
> +static void wbsio_write(unsigned char index, unsigned char reg, unsigned char data)
Same here. data is uint8_t.
> {
> - outb(index, W836_INDEX);
> - outb(data, W836_DATA);
> + outb(reg, index);
> + outb(data, index+1);
> }
>
> -static void wbsio_mask(unsigned char index, unsigned char data,
> +static void wbsio_mask(unsigned char index, unsigned char reg, unsigned char data,
> unsigned char mask)
Same here.
> {
> unsigned char tmp;
>
> - outb(index, W836_INDEX);
> - tmp = inb(W836_DATA) & ~mask;
> - outb(tmp | (data & mask), W836_DATA);
> + outb(reg, index);
> + tmp = inb(index+1) & ~mask;
> + outb(tmp | (data & mask), index+1);
> }
>
> /**
> @@ -80,35 +80,69 @@
> */
> static int w83627hf_gpio24_raise(const char *name)
> {
> - w836xx_ext_enter();
> + w836xx_ext_enter(W836_INDEX_2E);
This won't work if the Super I/O config port is a 0x4e, correct?
Can the code be made generic enough to handle both cases?
> /* Is this the w83627hf? */
> - if (wbsio_read(0x20) != 0x52) { /* SIO device ID register */
> + if (wbsio_read(W836_INDEX_2E, 0x20) != 0x52) { /* SIO device ID register */
SIO -> Super I/O. "SIO" might be confused with "serial I/O" or so.
> fprintf(stderr, "\nERROR: %s: W83627HF: Wrong ID: 0x%02X.\n",
> - name, wbsio_read(0x20));
> - w836xx_ext_leave();
> + name, wbsio_read(W836_INDEX_2E, 0x20));
> + w836xx_ext_leave(W836_INDEX_2E);
> return -1;
> }
>
> /* PIN89S: WDTO/GP24 multiplex -> GPIO24 */
> - wbsio_mask(0x2B, 0x10, 0x10);
> + wbsio_mask(W836_INDEX_2E, 0x2B, 0x10, 0x10);
>
> - wbsio_write(0x07, 0x08); /* Select logical device 8: GPIO port 2 */
> + wbsio_write(W836_INDEX_2E, 0x07, 0x08); /* Select logical device 8: GPIO port 2 */
>
> - wbsio_mask(0x30, 0x01, 0x01); /* Activate logical device. */
> + wbsio_mask(W836_INDEX_2E, 0x30, 0x01, 0x01); /* Activate logical device. */
>
> - wbsio_mask(0xF0, 0x00, 0x10); /* GPIO24 -> output */
> + wbsio_mask(W836_INDEX_2E, 0xF0, 0x00, 0x10); /* GPIO24 -> output */
>
> - wbsio_mask(0xF2, 0x00, 0x10); /* Clear GPIO24 inversion */
> + wbsio_mask(W836_INDEX_2E, 0xF2, 0x00, 0x10); /* Clear GPIO24 inversion */
>
> - wbsio_mask(0xF1, 0x10, 0x10); /* Raise GPIO24 */
> + wbsio_mask(W836_INDEX_2E, 0xF1, 0x10, 0x10); /* Raise GPIO24 */
Just use 0x2e here (not W836_INDEX_2E).
> - w836xx_ext_leave();
> + w836xx_ext_leave(W836_INDEX_2E);
>
> return 0;
> }
>
> /**
> + * Winbond W83627THF: GPIO 4, bit 4
> + *
> + * Suited for:
> + * - MSI K8N-NEO3
> + */
> +static int w83627thf_gpio4_4_raise(const char *name)
> +{
> + w836xx_ext_enter(W836_INDEX_4E);
Same here, this is 0x4e specific and won't work for other boards which
have the (same) Super I/O at 0x2e?
I'd pass the port as a parameter to the function. The respective
mainboard-specific function should contain the specific 0x2e or 0x4e.
> + /* Is this the w83627thf? */
> + if (wbsio_read(W836_INDEX_4E, 0x20) != 0x82) { /* SIO device ID register */
> + fprintf(stderr, "\nERROR: %s: W83627THF: Wrong ID: 0x%02X.\n",
> + name, wbsio_read(W836_INDEX_4E, 0x20));
> + w836xx_ext_leave(W836_INDEX_4E);
> + return -1;
> + }
> +
> + /* PINxxxxS: GPIO4/bit 4 multiplex -> GPIOXXX */
> +
> + wbsio_write(W836_INDEX_4E, 0x07, 0x09); /* Select logical device 9: GPIO port 4 */
> +
> + wbsio_mask(W836_INDEX_4E, 0x30, 0x02, 0x02); /* Activate logical device. */
> +
> + wbsio_mask(W836_INDEX_4E, 0xF4, 0x00, 0x10); /* GPIO4 bit 4 -> output */
> +
> + wbsio_mask(W836_INDEX_4E, 0xF6, 0x00, 0x10); /* Clear GPIO4 bit 4 inversion */
> +
> + wbsio_mask(W836_INDEX_4E, 0xF5, 0x10, 0x10); /* Raise GPIO4 bit 4 */
> +
> + w836xx_ext_leave(W836_INDEX_4E);
> +
> + return 0;
> +}
> +
> +/**
> * Suited for VIAs EPIA M and MII, and maybe other CLE266 based EPIAs.
> *
> * We don't need to do this when using LinuxBIOS, GPIO15 is never lowered there.
> @@ -165,12 +199,12 @@
> pci_write_byte(dev, 0x59, val);
>
> /* Raise ROM MEMW# line on Winbond w83697 SuperIO */
> - w836xx_ext_enter();
> + w836xx_ext_enter(W836_INDEX_2E);
>
> - if (!(wbsio_read(0x24) & 0x02)) /* flash rom enabled? */
> - wbsio_mask(0x24, 0x08, 0x08); /* enable MEMW# */
> + if (!(wbsio_read(W836_INDEX_2E, 0x24) & 0x02)) /* flash rom enabled? */
> + wbsio_mask(W836_INDEX_2E, 0x24, 0x08, 0x08); /* enable MEMW# */
>
> - w836xx_ext_leave();
> + w836xx_ext_leave(W836_INDEX_2E);
>
> return 0;
> }
> @@ -316,6 +350,8 @@
> struct board_pciid_enable board_pciid_enables[] = {
> {0x1022, 0x7468, 0x0000, 0x0000, 0x0000, 0x0000, 0x0000, 0x0000,
> "iwill", "dk8_htx", "IWILL DK8-HTX", w83627hf_gpio24_raise},
> + {0x10de, 0x005e, 0x0000, 0x0000, 0x0000, 0x0000, 0x0000, 0x0000,
> + "MSI", "K8N-NEO3", "MSI K8N-NEO3", w83627thf_gpio4_4_raise},
Small spelling fixes:
"msi", "k8n-neo3", "MSI K8N Neo3", w83627thf_gpio4_4_raise},
Are you sure this is not the MSI K8N Neo3-F (attached -F)?
Uwe.
--
http://www.hermann-uwe.de | http://www.holsham-traders.de
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20071008/40dc062c/attachment.sig>
More information about the coreboot
mailing list