Reliability fixes:

- don't force enabling host controller
- don't start new transfer if bus is busy
- kill transfer on timeout

Some ideas from kettenis@; ok deraadt@.

2 files changed, 78 insertions, 40 deletions

diff --git a/sys/dev/pci/ichiic.c b/sys/dev/pci/ichiic.c
index bf4e7435070..707311a14c0 100644
--- a/sys/dev/pci/ichiic.c
+++ b/sys/dev/pci/ichiic.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ichiic.c,v 1.7 2006/01/02 08:11:25 brad Exp $	*/
+/*	$OpenBSD: ichiic.c,v 1.8 2006/01/05 08:16:22 grange Exp $	*/
 
 /*
  * Copyright (c) 2005 Alexander Yurchenko <grange@openbsd.org>
@@ -116,6 +116,15 @@ ichiic_attach(struct device *parent, struct device *self, void *aux)
 	pci_intr_handle_t ih;
 	const char *intrstr = NULL;
 
+	/* Read configuration */
+	conf = pci_conf_read(pa->pa_pc, pa->pa_tag, ICH_SMB_HOSTC);
+	DPRINTF((": conf 0x%x", conf));
+
+	if ((conf & ICH_SMB_HOSTC_HSTEN) == 0) {
+		printf(": SMBus disabled\n");
+		return;
+	}
+
 	/* Map I/O space */
 	if (pci_mapreg_map(pa, ICH_SMB_BASE, PCI_MAPREG_TYPE_IO, 0,
 	    &sc->sc_iot, &sc->sc_ioh, NULL, &iosize, 0)) {
@@ -123,10 +132,6 @@ ichiic_attach(struct device *parent, struct device *self, void *aux)
 		return;
 	}
 
-	/* Read configuration */
-	conf = pci_conf_read(pa->pa_pc, pa->pa_tag, ICH_SMB_HOSTC);
-	DPRINTF((": conf 0x%x", conf));
-
 	if (conf & ICH_SMB_HOSTC_SMIEN) {
 		printf(": SMI");
 		sc->sc_poll = 1;
@@ -149,10 +154,6 @@ ichiic_attach(struct device *parent, struct device *self, void *aux)
 		printf(": %s", intrstr);
 	}
 
-	/* Enable controller */
-	pci_conf_write(pa->pa_pc, pa->pa_tag, ICH_SMB_HOSTC,
-	    conf | ICH_SMB_HOSTC_HSTEN);
-
 	printf("\n");
 
 	/* Attach I2C bus */
@@ -204,10 +205,15 @@ ichiic_i2c_exec(void *cookie, i2c_op_t op, i2c_addr_t addr,
 	u_int8_t ctl, st;
 	int retries;
 
-	DPRINTF(("%s: exec op %d, addr 0x%x, cmdlen %d, len %d, "
-	    "flags 0x%x, status 0x%b\n", sc->sc_dev.dv_xname, op, addr,
-	    cmdlen, len, flags, bus_space_read_1(sc->sc_iot, sc->sc_ioh,
-	    ICH_SMB_HS), ICH_SMB_HS_BITS));
+	DPRINTF(("%s: exec: op %d, addr 0x%x, cmdlen %d, len %d, flags 0x%x\n",
+	    sc->sc_dev.dv_xname, op, addr, cmdlen, len, flags));
+
+	/* Check if there's a transfer already running */
+	st = bus_space_read_1(sc->sc_iot, sc->sc_ioh, ICH_SMB_HS);
+	DPRINTF(("%s: exec: st 0x%b\n", sc->sc_dev.dv_xname, st,
+	    ICH_SMB_HS_BITS));
+	if (st & ICH_SMB_HS_BUSY)
+		return (1);
 
 	if (cold || sc->sc_poll)
 		flags |= I2C_F_POLL;
@@ -268,22 +274,35 @@ ichiic_i2c_exec(void *cookie, i2c_op_t op, i2c_addr_t addr,
 				break;
 			DELAY(ICHIIC_DELAY);
 		}
-		if (st & ICH_SMB_HS_BUSY) {
-			printf("%s: timeout, status 0x%b\n",
-			    sc->sc_dev.dv_xname, st, ICH_SMB_HS_BITS);
-			return (1);
-		}
+		if (st & ICH_SMB_HS_BUSY)
+			goto timeout;
 		ichiic_intr(sc);
 	} else {
 		/* Wait for interrupt */
 		if (tsleep(sc, PRIBIO, "iicexec", ICHIIC_TIMEOUT * hz))
-			return (1);
-	}	
+			goto timeout;
+	}
 
 	if (sc->sc_i2c_xfer.error)
 		return (1);
 
 	return (0);
+
+timeout:
+	/*
+	 * Transfer timeout. Kill the transaction and clear status bits.
+	 */
+	printf("%s: timeout, status 0x%b\n", sc->sc_dev.dv_xname, st,
+	    ICH_SMB_HS_BITS);
+	bus_space_write_1(sc->sc_iot, sc->sc_ioh, ICH_SMB_HC,
+	    ICH_SMB_HC_KILL);
+	DELAY(ICHIIC_DELAY);
+	st = bus_space_read_1(sc->sc_iot, sc->sc_ioh, ICH_SMB_HS);
+	if ((st & ICH_SMB_HS_FAILED) == 0)
+		printf("%s: transaction abort failed, status 0x%b\n",
+		    sc->sc_dev.dv_xname, st, ICH_SMB_HS_BITS);
+	bus_space_write_1(sc->sc_iot, sc->sc_ioh, ICH_SMB_HS, st);
+	return (1);
 }
 
 int
diff --git a/sys/dev/pci/piixpm.c b/sys/dev/pci/piixpm.c
index 44ad8e4d843..898658c1484 100644
--- a/sys/dev/pci/piixpm.c
+++ b/sys/dev/pci/piixpm.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: piixpm.c,v 1.11 2006/01/03 23:24:06 grange Exp $	*/
+/*	$OpenBSD: piixpm.c,v 1.12 2006/01/05 08:16:22 grange Exp $	*/
 
 /*
  * Copyright (c) 2005 Alexander Yurchenko <grange@openbsd.org>
@@ -110,6 +110,15 @@ piixpm_attach(struct device *parent, struct device *self, void *aux)
 	pci_intr_handle_t ih;
 	const char *intrstr = NULL;
 
+	/* Read configuration */
+	conf = pci_conf_read(pa->pa_pc, pa->pa_tag, PIIX_SMB_HOSTC);
+	DPRINTF((": conf 0x%x", conf));
+
+	if ((conf & PIIX_SMB_HOSTC_HSTEN) == 0) {
+		printf(": SMBus disabled\n");
+		return;
+	}
+
 	/* Map I/O space */
 	sc->sc_iot = pa->pa_iot;
 	base = pci_conf_read(pa->pa_pc, pa->pa_tag, PIIX_SMB_BASE);
@@ -119,10 +128,6 @@ piixpm_attach(struct device *parent, struct device *self, void *aux)
 		return;
 	}
 
-	/* Read configuration */
-	conf = pci_conf_read(pa->pa_pc, pa->pa_tag, PIIX_SMB_HOSTC);
-	DPRINTF((": conf 0x%x", conf));
-
 	if ((conf & PIIX_SMB_HOSTC_INTMASK) == PIIX_SMB_HOSTC_SMI) {
 		printf(": SMI");
 		sc->sc_poll = 1;
@@ -147,10 +152,6 @@ piixpm_attach(struct device *parent, struct device *self, void *aux)
 		sc->sc_poll = 1;
 	}
 
-	/* Enable controller */
-	pci_conf_write(pa->pa_pc, pa->pa_tag, PIIX_SMB_HOSTC,
-	    conf | PIIX_SMB_HOSTC_HSTEN);
-
 	printf("\n");
 
 	/* Attach I2C bus */
@@ -202,10 +203,15 @@ piixpm_i2c_exec(void *cookie, i2c_op_t op, i2c_addr_t addr,
 	u_int8_t ctl, st;
 	int retries;
 
-	DPRINTF(("%s: exec op %d, addr 0x%x, cmdlen %d, len %d, "
-	    "flags 0x%x, status 0x%b\n", sc->sc_dev.dv_xname, op, addr,
-	    cmdlen, len, flags, bus_space_read_1(sc->sc_iot, sc->sc_ioh,
-	    PIIX_SMB_HS), PIIX_SMB_HS_BITS));
+	DPRINTF(("%s: exec: op %d, addr 0x%x, cmdlen %d, len %d, flags 0x%x\n",
+	    sc->sc_dev.dv_xname, op, addr, cmdlen, len, flags));
+
+	/* Check if there's a transfer already running */
+	st = bus_space_read_1(sc->sc_iot, sc->sc_ioh, PIIX_SMB_HS);
+	DPRINTF(("%s: exec: st 0x%b\n", sc->sc_dev.dv_xname, st,
+	    PIIX_SMB_HS_BITS));
+	if (st & PIIX_SMB_HS_BUSY)
+		return (1);
 
 	if (cold || sc->sc_poll)
 		flags |= I2C_F_POLL;
@@ -266,22 +272,35 @@ piixpm_i2c_exec(void *cookie, i2c_op_t op, i2c_addr_t addr,
 				break;
 			DELAY(PIIXPM_DELAY);
 		}
-		if (st & PIIX_SMB_HS_BUSY) {
-			printf("%s: timeout, status 0x%b\n",
-			    sc->sc_dev.dv_xname, st, PIIX_SMB_HS_BITS);
-			return (1);
-		}
+		if (st & PIIX_SMB_HS_BUSY)
+			goto timeout;
 		piixpm_intr(sc);
 	} else {
 		/* Wait for interrupt */
 		if (tsleep(sc, PRIBIO, "iicexec", PIIXPM_TIMEOUT * hz))
-			return (1);
-	}	
+			goto timeout;
+	}
 
 	if (sc->sc_i2c_xfer.error)
 		return (1);
 
 	return (0);
+
+timeout:
+	/*
+	 * Transfer timeout. Kill the transaction and clear status bits.
+	 */
+	printf("%s: timeout, status 0x%b\n", sc->sc_dev.dv_xname, st,
+	    PIIX_SMB_HS_BITS);
+	bus_space_write_1(sc->sc_iot, sc->sc_ioh, PIIX_SMB_HC,
+	    PIIX_SMB_HC_KILL);
+	DELAY(PIIXPM_DELAY);
+	st = bus_space_read_1(sc->sc_iot, sc->sc_ioh, PIIX_SMB_HS);
+	if ((st & PIIX_SMB_HS_FAILED) == 0)
+		printf("%s: transaction abort failed, status 0x%b\n",
+		    sc->sc_dev.dv_xname, st, PIIX_SMB_HS_BITS);
+	bus_space_write_1(sc->sc_iot, sc->sc_ioh, PIIX_SMB_HS, st);
+	return (1);
 }
 
 int