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.