Attachment 207593 Details for Bug 238037 – ig4 suspend/resume with other improvements

[patch] ig4 suspend/resume with other improvements

ig4.patch (text/plain), 40.94 KB, created by Vladimir Kondratyev on 2019-09-18 00:44:30 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Vladimir Kondratyev

Created: 2019-09-18 00:44:30 UTC

Size: 40.94 KB

patch

obsolete

>diff --git a/sys/dev/ichiic/ig4_acpi.c b/sys/dev/ichiic/ig4_acpi.c
>index f0b431fd1007..afb3908554ea 100644
>--- a/sys/dev/ichiic/ig4_acpi.c
>+++ b/sys/dev/ichiic/ig4_acpi.c
>@@ -69,18 +69,15 @@ static int
> ig4iic_acpi_probe(device_t dev)
> {
> 	ig4iic_softc_t *sc;
>-	char *hid;
> 	int rv;
> 
> 	sc = device_get_softc(dev);
> 	if (acpi_disabled("ig4iic"))
> 		return (ENXIO);
>-	rv = ACPI_ID_PROBE(device_get_parent(dev), dev, ig4iic_ids, &hid);
>+	rv = ACPI_ID_PROBE(device_get_parent(dev), dev, ig4iic_ids, NULL);
> 	if (rv > 0)
> 		return (rv);
> 
>-        if (strcmp("AMDI0010", hid) == 0)
>-                sc->access_intr_mask = 1;
> 	device_set_desc(dev, "Designware I2C Controller");
> 	return (rv);
> }
>@@ -148,22 +145,51 @@ ig4iic_acpi_detach(device_t dev)
> 	return (0);
> }
> 
>+static int
>+ig4iic_acpi_suspend(device_t dev)
>+{
>+	ig4iic_softc_t *sc = device_get_softc(dev);
>+
>+	return (ig4iic_suspend(sc));
>+}
>+
>+static int
>+ig4iic_acpi_resume(device_t dev)
>+{
>+	ig4iic_softc_t *sc  = device_get_softc(dev);
>+
>+	return (ig4iic_resume(sc));
>+}
>+
> static device_method_t ig4iic_acpi_methods[] = {
> 	/* Device interface */
> 	DEVMETHOD(device_probe, ig4iic_acpi_probe),
> 	DEVMETHOD(device_attach, ig4iic_acpi_attach),
> 	DEVMETHOD(device_detach, ig4iic_acpi_detach),
>+	DEVMETHOD(device_suspend, ig4iic_acpi_suspend),
>+	DEVMETHOD(device_resume, ig4iic_acpi_resume),
>+
>+	/* Bus interface */
>+	DEVMETHOD(bus_setup_intr, bus_generic_setup_intr),
>+	DEVMETHOD(bus_teardown_intr, bus_generic_teardown_intr),
>+	DEVMETHOD(bus_alloc_resource, bus_generic_alloc_resource),
>+	DEVMETHOD(bus_release_resource, bus_generic_release_resource),
>+	DEVMETHOD(bus_activate_resource, bus_generic_activate_resource),
>+	DEVMETHOD(bus_deactivate_resource, bus_generic_deactivate_resource),
>+	DEVMETHOD(bus_adjust_resource, bus_generic_adjust_resource),
>+	DEVMETHOD(bus_set_resource, bus_generic_rl_set_resource),
>+	DEVMETHOD(bus_get_resource, bus_generic_rl_get_resource),
> 
> 	/* iicbus interface */
> 	DEVMETHOD(iicbus_transfer, ig4iic_transfer),
> 	DEVMETHOD(iicbus_reset, ig4iic_reset),
>-	DEVMETHOD(iicbus_callback, iicbus_null_callback),
>+	DEVMETHOD(iicbus_callback, ig4iic_callback),
> 
> 	DEVMETHOD_END
> };
> 
> static driver_t ig4iic_acpi_driver = {
>-	"ig4iic_acpi",
>+	"ig4iic",
> 	ig4iic_acpi_methods,
> 	sizeof(struct ig4iic_softc),
> };
>diff --git a/sys/dev/ichiic/ig4_iic.c b/sys/dev/ichiic/ig4_iic.c
>index 54dba7b9657b..e14d89e8fc5d 100644
>--- a/sys/dev/ichiic/ig4_iic.c
>+++ b/sys/dev/ichiic/ig4_iic.c
>@@ -43,13 +43,17 @@ __FBSDID("$FreeBSD$");
>  * See ig4_var.h for locking semantics.
>  */
> 
>+#include "opt_acpi.h"
>+
> #include <sys/param.h>
> #include <sys/systm.h>
> #include <sys/kernel.h>
> #include <sys/module.h>
> #include <sys/errno.h>
>+#include <sys/kdb.h>
> #include <sys/lock.h>
> #include <sys/mutex.h>
>+#include <sys/proc.h>
> #include <sys/sx.h>
> #include <sys/syslog.h>
> #include <sys/bus.h>
>@@ -58,6 +62,12 @@ __FBSDID("$FreeBSD$");
> #include <machine/bus.h>
> #include <sys/rman.h>
> 
>+#ifdef DEV_ACPI
>+#include <contrib/dev/acpica/include/acpi.h>
>+#include <contrib/dev/acpica/include/accommon.h>
>+#include <dev/acpica/acpivar.h>
>+#endif
>+
> #include <dev/pci/pcivar.h>
> #include <dev/pci/pcireg.h>
> #include <dev/iicbus/iicbus.h>
>@@ -70,7 +80,78 @@ __FBSDID("$FreeBSD$");
> #define TRANS_PCALL	2
> #define TRANS_BLOCK	3
> 
>-static void ig4iic_start(void *xdev);
>+#define DO_POLL(sc)	(cold || kdb_active || SCHEDULER_STOPPED() || \
>+			    sc->poll || sc->resume)
>+
>+/*
>+ * Clock register values calculation formulas and timings are snarfed from
>+ * Linux driver.
>+ * *S_SCL_HCNT = IC clock rate * (tHIGH + SDA falling time) - 3
>+ * *S_SCL_LCNT = IC clock rate * (tLOW + SCL falling time) - 1
>+ * SDA_TX_HOLD = IC clock rate * SDA hold time
>+ * All results are rounded to nearest.
>+ *
>+ * tLOW and tHIGH periods of the SCL clock are taken from I2C specification:
>+ * Speed mode   STD     FAST    FAST+   HIGH
>+ * tHIGH	4.0 ns	0.6 ns	0.26 ns	0.06-0.12 ns
>+ * tLOW		4.7 ns	1.3 ns	0.5 ns	0.16-0.32 ns
>+ *
>+ * HIGH Speed mode tHIGH/tLOW values are depend on bus capacitance.
>+ */
>+static const struct ig4_cfg ig4iic_configs[] = {
>+	[IG4_HASWELL] = {
>+		.bus_speed = IG4_CTL_SPEED_FAST,
>+		.ss_scl_hcnt = 432,
>+		.ss_scl_lcnt = 507,
>+		.fs_scl_hcnt = 110,
>+		.fs_scl_lcnt = 160,
>+		.sda_tx_hold = 9,
>+		.txfifo_depth = 32,
>+		.rxfifo_depth = 32,
>+	},
>+	[IG4_ATOM] = {
>+		.bus_speed = IG4_CTL_SPEED_FAST,
>+		.ss_scl_hcnt = 512,
>+		.ss_scl_lcnt = 512,
>+		.fs_scl_hcnt = 85,
>+		.fs_scl_lcnt = 153,
>+		.sda_tx_hold = 6,
>+		.txfifo_depth = 32,
>+		.rxfifo_depth = 32,
>+	},
>+	[IG4_SKYLAKE] = {
>+		/*
>+		 * IC clock rate:	120 MHz
>+		 * SDA hold time:	230 ns
>+		 * SDA falling time:	300 ns
>+		 * SCL falling time:	300 ns
>+		 */
>+		.bus_speed = IG4_CTL_SPEED_FAST,
>+		.ss_scl_hcnt = 513,
>+		.ss_scl_lcnt = 599,
>+		.fs_scl_hcnt = 105,
>+		.fs_scl_lcnt = 191,
>+		.sda_tx_hold = 28,
>+		.txfifo_depth = 64,
>+		.rxfifo_depth = 64,
>+	},
>+	[IG4_APL] = {
>+		/*
>+		 * IC clock rate:	133 MHz
>+		 * SDA hold time:	207 ns
>+		 * SDA falling time:	171 ns
>+		 * SCL falling time:	208 ns
>+		 */
>+		.bus_speed = IG4_CTL_SPEED_FAST,
>+		.ss_scl_hcnt = 552,
>+		.ss_scl_lcnt = 652,
>+		.fs_scl_hcnt = 100,
>+		.fs_scl_lcnt = 200,
>+		.sda_tx_hold = 28,
>+	},
>+};
>+
>+static int ig4iic_set_config(ig4iic_softc_t *sc);
> static void ig4iic_intr(void *cookie);
> static void ig4iic_dump(ig4iic_softc_t *sc);
> 
>@@ -98,6 +179,64 @@ reg_read(ig4iic_softc_t *sc, uint32_t reg)
> 	return (value);
> }
> 
>+static void
>+set_intr_mask(ig4iic_softc_t *sc, uint32_t val)
>+{
>+	if (sc->intr_mask != val) {
>+		reg_write(sc, IG4_REG_INTR_MASK, val);
>+		sc->intr_mask = val;
>+	}
>+}
>+
>+static int
>+intrstat2iic(ig4iic_softc_t *sc, uint32_t val)
>+{
>+	uint32_t src;
>+
>+	if (val & IG4_INTR_RX_UNDER)
>+		reg_read(sc, IG4_REG_CLR_RX_UNDER);
>+	if (val & IG4_INTR_RX_OVER)
>+		reg_read(sc, IG4_REG_CLR_RX_OVER);
>+	if (val & IG4_INTR_TX_OVER)
>+		reg_read(sc, IG4_REG_CLR_TX_OVER);
>+
>+	if (val & IG4_INTR_TX_ABRT) {
>+		src = reg_read(sc, IG4_REG_TX_ABRT_SOURCE);
>+		reg_read(sc, IG4_REG_CLR_TX_ABORT);
>+		/* User-requested abort. Not really a error */
>+		if (src & IG4_ABRTSRC_TRANSFER)
>+			return (IIC_ESTATUS);
>+		/* Master has lost arbitration */
>+		if (src & IG4_ABRTSRC_ARBLOST)
>+			return (IIC_EBUSBSY);
>+		/* Did not receive an acknowledge from the remote slave */
>+		if (src & (IG4_ABRTSRC_TXNOACK_ADDR7 |
>+			   IG4_ABRTSRC_TXNOACK_ADDR10_1 |
>+			   IG4_ABRTSRC_TXNOACK_ADDR10_2 |
>+			   IG4_ABRTSRC_TXNOACK_DATA |
>+			   IG4_ABRTSRC_GENCALL_NOACK))
>+			return (IIC_ENOACK);
>+		/* Programming errors */
>+		if (src & (IG4_ABRTSRC_GENCALL_READ |
>+			   IG4_ABRTSRC_NORESTART_START |
>+			   IG4_ABRTSRC_NORESTART_10))
>+			return (IIC_ENOTSUPP);
>+		/* Other errors */
>+		if (src & IG4_ABRTSRC_ACKED_START)
>+			return (IIC_EBUSERR);
>+	}
>+	/*
>+	 * TX_OVER, RX_OVER and RX_UNDER are caused by wrong RX/TX FIFO depth
>+	 * detection or driver's read/write pipelining errors.
>+	 */
>+	if (val & (IG4_INTR_TX_OVER | IG4_INTR_RX_OVER))
>+		return (IIC_EOVERFLOW);
>+	if (val & IG4_INTR_RX_UNDER)
>+		return (IIC_EUNDERFLOW);
>+
>+	return (IIC_NOERR);
>+}
>+
> /*
>  * Enable or disable the controller and wait for the controller to acknowledge
>  * the state change.
>@@ -113,12 +252,9 @@ set_controller(ig4iic_softc_t *sc, uint32_t ctl)
> 	 * When the controller is enabled, interrupt on STOP detect
> 	 * or receive character ready and clear pending interrupts.
> 	 */
>-	if (ctl & IG4_I2C_ENABLE) {
>-		reg_write(sc, IG4_REG_INTR_MASK, IG4_INTR_STOP_DET |
>-						 IG4_INTR_RX_FULL);
>+	set_intr_mask(sc, 0);
>+	if (ctl & IG4_I2C_ENABLE)
> 		reg_read(sc, IG4_REG_CLR_INTR);
>-	} else
>-		reg_write(sc, IG4_REG_INTR_MASK, 0);
> 
> 	reg_write(sc, IG4_REG_I2C_EN, ctl);
> 	error = IIC_ETIMEOUT;
>@@ -129,19 +265,16 @@ set_controller(ig4iic_softc_t *sc, uint32_t ctl)
> 			error = 0;
> 			break;
> 		}
>-		if (cold)
>-			DELAY(1000);
>-		else
>-			mtx_sleep(sc, &sc->io_lock, 0, "i2cslv", 1);
>+		pause("i2cslv", 1);
> 	}
> 	return (error);
> }
> 
> /*
>- * Wait up to 25ms for the requested status using a 25uS polling loop.
>+ * Wait up to 25ms for the requested interrupt using a 25uS polling loop.
>  */
> static int
>-wait_status(ig4iic_softc_t *sc, uint32_t status)
>+wait_intr(ig4iic_softc_t *sc, uint32_t intr)
> {
> 	uint32_t v;
> 	int error;
>@@ -151,25 +284,18 @@ wait_status(ig4iic_softc_t *sc, uint32_t status)
> 
> 	error = IIC_ETIMEOUT;
> 
>-	for (;;) {
>+	while (error == IIC_ETIMEOUT) {
> 		/*
> 		 * Check requested status
> 		 */
>-		v = reg_read(sc, IG4_REG_I2C_STA);
>-		if (v & status) {
>-			error = 0;
>+		v = reg_read(sc, IG4_REG_RAW_INTR_STAT);
>+		error = intrstat2iic(sc, v & IG4_INTR_ERR_MASK);
>+		if (error)
> 			break;
>-		}
> 
>-		/*
>-		 * When waiting for receive data break-out if the interrupt
>-		 * loaded data into the FIFO.
>-		 */
>-		if (status & IG4_STATUS_RX_NOTEMPTY) {
>-			if (sc->rpos != sc->rnext) {
>-				error = 0;
>-				break;
>-			}
>+		if (v & intr) {
>+			error = 0;
>+			break;
> 		}
> 
> 		/*
>@@ -177,7 +303,7 @@ wait_status(ig4iic_softc_t *sc, uint32_t status)
> 		 * reset the timeout if we see a change in the transmit
> 		 * FIFO level as progress is being made.
> 		 */
>-		if (status & IG4_STATUS_TX_EMPTY) {
>+		if (intr & (IG4_INTR_TX_EMPTY | IG4_INTR_STOP_DET)) {
> 			v = reg_read(sc, IG4_REG_TXFLR) & IG4_FIFOLVL_MASK;
> 			if (txlvl != v) {
> 				txlvl = v;
>@@ -188,45 +314,37 @@ wait_status(ig4iic_softc_t *sc, uint32_t status)
> 		/*
> 		 * Stop if we've run out of time.
> 		 */
>-		if (count_us >= limit_us)
>+		if (count_us >= limit_us) {
>+			error = IIC_ETIMEOUT;
> 			break;
>+		}
> 
> 		/*
>-		 * When waiting for receive data let the interrupt do its
>-		 * work, otherwise poll with the lock held.
>+		 * When polling is not requested let the interrupt do its work.
> 		 */
>-		if (status & IG4_STATUS_RX_NOTEMPTY) {
>-			mtx_sleep(sc, &sc->io_lock, 0, "i2cwait",
>+		if (!DO_POLL(sc)) {
>+			mtx_lock(&sc->io_lock);
>+			sc->error = 0;
>+			set_intr_mask(sc, intr | IG4_INTR_ERR_MASK);
>+			error = mtx_sleep(sc, &sc->io_lock, 0, "i2cwait",
> 				  (hz + 99) / 100); /* sleep up to 10ms */
>+			if (error != 0)
>+				error = IIC_ETIMEOUT;
>+			else
>+				error = sc->error;
>+			set_intr_mask(sc, 0);
>+			mtx_unlock(&sc->io_lock);
> 			count_us += 10000;
> 		} else {
> 			DELAY(25);
> 			count_us += 25;
>+			error = IIC_ETIMEOUT;
> 		}
> 	}
> 
> 	return (error);
> }
> 
>-/*
>- * Read I2C data.  The data might have already been read by
>- * the interrupt code, otherwise it is sitting in the data
>- * register.
>- */
>-static uint8_t
>-data_read(ig4iic_softc_t *sc)
>-{
>-	uint8_t c;
>-
>-	if (sc->rpos == sc->rnext) {
>-		c = (uint8_t)reg_read(sc, IG4_REG_DATA_CMD);
>-	} else {
>-		c = sc->rbuf[sc->rpos & IG4_RBUFMASK];
>-		++sc->rpos;
>-	}
>-	return (c);
>-}
>-
> /*
>  * Set the slave address.  The controller must be disabled when
>  * changing the address.
>@@ -250,22 +368,8 @@ set_slave_addr(ig4iic_softc_t *sc, uint8_t slave)
> 
> 	/*
> 	 * Wait for TXFIFO to drain before disabling the controller.
>-	 *
>-	 * If a write message has not been completed it's really a
>-	 * programming error, but for now in that case issue an extra
>-	 * byte + STOP.
>-	 *
>-	 * If a read message has not been completed it's also a programming
>-	 * error, for now just ignore it.
> 	 */
>-	wait_status(sc, IG4_STATUS_TX_NOTFULL);
>-	if (sc->write_started) {
>-		reg_write(sc, IG4_REG_DATA_CMD, IG4_DATA_STOP);
>-		sc->write_started = 0;
>-	}
>-	if (sc->read_started)
>-		sc->read_started = 0;
>-	wait_status(sc, IG4_STATUS_TX_EMPTY);
>+	wait_intr(sc, IG4_INTR_TX_EMPTY);
> 
> 	set_controller(sc, 0);
> 	ctl = reg_read(sc, IG4_REG_CTL);
>@@ -288,48 +392,99 @@ set_slave_addr(ig4iic_softc_t *sc, uint8_t slave)
>  *				IICBUS API FUNCTIONS
>  */
> static int
>-ig4iic_xfer_start(ig4iic_softc_t *sc, uint16_t slave)
>+ig4iic_xfer_start(ig4iic_softc_t *sc, uint16_t slave, bool repeated_start)
> {
> 	set_slave_addr(sc, slave >> 1);
>+
>+	if (!repeated_start) {
>+		/*
>+		 * Clear any previous TX/RX FIFOs overflow/underflow bits
>+		 * and I2C bus STOP condition.
>+		 */
>+		reg_read(sc, IG4_REG_CLR_INTR);
>+	}
>+
> 	return (0);
> }
> 
>+static int
>+ig4iic_xfer_abort(ig4iic_softc_t *sc)
>+{
>+	int error;
>+
>+	/* Request send of STOP condition and flush of TX FIFO */
>+	set_controller(sc, IG4_I2C_ABORT | IG4_I2C_ENABLE);
>+	/*
>+	 * Wait for the TX_ABRT interrupt with ABRTSRC_TRANSFER
>+	 * bit set in TX_ABRT_SOURCE register.
>+	 */
>+	error = wait_intr(sc, IG4_INTR_STOP_DET);
>+	set_controller(sc, IG4_I2C_ENABLE);
>+
>+	return (error == IIC_ESTATUS ? 0 : error);
>+}
>+
>+/*
>+ * Amount of unread data before next burst to get better I2C bus utilization.
>+ * 2 bytes is enough in FAST mode. 8 bytes is better in FAST+ and HIGH modes.
>+ * Intel-recommended value is 16 for DMA transfers with 64-byte depth FIFOs.
>+ */
>+#define	IG4_FIFO_LOWAT	2
>+
> static int
> ig4iic_read(ig4iic_softc_t *sc, uint8_t *buf, uint16_t len,
>     bool repeated_start, bool stop)
> {
> 	uint32_t cmd;
>-	uint16_t i;
>+	int requested = 0;
>+	int received = 0;
>+	int burst, target, lowat = 0;
> 	int error;
> 
> 	if (len == 0)
> 		return (0);
> 
>-	cmd = IG4_DATA_COMMAND_RD;
>-	cmd |= repeated_start ? IG4_DATA_RESTART : 0;
>-	cmd |= stop && len == 1 ? IG4_DATA_STOP : 0;
>-
>-	/* Issue request for the first byte (could be last as well). */
>-	reg_write(sc, IG4_REG_DATA_CMD, cmd);
>-
>-	for (i = 0; i < len; i++) {
>-		/*
>-		 * Maintain a pipeline by queueing the allowance for the next
>-		 * read before waiting for the current read.
>-		 */
>-		cmd = IG4_DATA_COMMAND_RD;
>-		if (i < len - 1) {
>+	while (received < len) {
>+		burst = sc->cfg.txfifo_depth -
>+		    (reg_read(sc, IG4_REG_TXFLR) & IG4_FIFOLVL_MASK);
>+		if (burst <= 0) {
>+			error = wait_intr(sc, IG4_INTR_TX_EMPTY);
>+			if (error)
>+				break;
>+			burst = sc->cfg.txfifo_depth;
>+		}
>+		/* Ensure we have enough free space in RXFIFO */
>+		burst = MIN(burst, sc->cfg.rxfifo_depth - lowat);
>+		target = MIN(requested + burst, (int)len);
>+		while (requested < target) {
> 			cmd = IG4_DATA_COMMAND_RD;
>-			cmd |= stop && i == len - 2 ? IG4_DATA_STOP : 0;
>+			if (repeated_start && requested == 0)
>+				cmd |= IG4_DATA_RESTART;
>+			if (stop && requested == len - 1)
>+				cmd |= IG4_DATA_STOP;
> 			reg_write(sc, IG4_REG_DATA_CMD, cmd);
>+			requested++;
>+		}
>+		/* Leave some data queued to maintain the hardware pipeline */
>+		lowat = 0;
>+		if (requested != len && requested - received > IG4_FIFO_LOWAT)
>+			lowat = IG4_FIFO_LOWAT;
>+		/* After TXFLR fills up, clear it by reading available data */
>+		while (received < requested - lowat) {
>+			burst = MIN((int)len - received,
>+			    reg_read(sc, IG4_REG_RXFLR) & IG4_FIFOLVL_MASK);
>+			if (burst > 0) {
>+				while (burst--)
>+					buf[received++] = 0xFF &
>+					    reg_read(sc, IG4_REG_DATA_CMD);
>+			} else {
>+				error = wait_intr(sc, IG4_INTR_RX_FULL);
>+				if (error)
>+					goto out;
>+			}
> 		}
>-		error = wait_status(sc, IG4_STATUS_RX_NOTEMPTY);
>-		if (error)
>-			break;
>-		buf[i] = data_read(sc);
> 	}
>-
>-	(void)reg_read(sc, IG4_REG_TX_ABRT_SOURCE);
>+out:
> 	return (error);
> }
> 
>@@ -338,24 +493,41 @@ ig4iic_write(ig4iic_softc_t *sc, uint8_t *buf, uint16_t len,
>     bool repeated_start, bool stop)
> {
> 	uint32_t cmd;
>-	uint16_t i;
>+	int sent = 0;
>+	int burst, target;
> 	int error;
>+	bool lowat_set = false;
> 
> 	if (len == 0)
> 		return (0);
> 
>-	cmd = repeated_start ? IG4_DATA_RESTART : 0;
>-	for (i = 0; i < len; i++) {
>-		error = wait_status(sc, IG4_STATUS_TX_NOTFULL);
>-		if (error)
>-			break;
>-		cmd |= buf[i];
>-		cmd |= stop && i == len - 1 ? IG4_DATA_STOP : 0;
>-		reg_write(sc, IG4_REG_DATA_CMD, cmd);
>-		cmd = 0;
>+	while (sent < len) {
>+		burst = sc->cfg.txfifo_depth -
>+		    (reg_read(sc, IG4_REG_TXFLR) & IG4_FIFOLVL_MASK);
>+		target = MIN(sent + burst, (int)len);
>+		/* Leave some data queued to maintain the hardware pipeline */
>+		if (sent == 0 && target != len) {
>+			lowat_set = true;
>+			reg_write(sc, IG4_REG_TX_TL, IG4_FIFO_LOWAT);
>+		}
>+		while(sent < target) {
>+			cmd = buf[sent];
>+			if (repeated_start && sent == 0)
>+				cmd |= IG4_DATA_RESTART;
>+			if (stop && sent == len - 1)
>+				cmd |= IG4_DATA_STOP;
>+			reg_write(sc, IG4_REG_DATA_CMD, cmd);
>+			sent++;
>+		}
>+		if (sent < len) {
>+			error = wait_intr(sc, IG4_INTR_TX_EMPTY);
>+			if (error)
>+				break;
>+		}
> 	}
>+	if (lowat_set)
>+		reg_write(sc, IG4_REG_TX_TL, 0);
> 
>-	(void)reg_read(sc, IG4_REG_TX_ABRT_SOURCE);
> 	return (error);
> }
> 
>@@ -369,6 +541,7 @@ ig4iic_transfer(device_t dev, struct iic_msg *msgs, uint32_t nmsgs)
> 	int unit;
> 	bool rpstart;
> 	bool stop;
>+	bool allocated;
> 
> 	/*
> 	 * The hardware interface imposes limits on allowed I2C messages.
>@@ -429,8 +602,10 @@ ig4iic_transfer(device_t dev, struct iic_msg *msgs, uint32_t nmsgs)
> 		return (IIC_ENOTSUPP);
> 	}
> 
>-	sx_xlock(&sc->call_lock);
>-	mtx_lock(&sc->io_lock);
>+	/* Check if device is already allocated with iicbus_request_bus() */
>+	allocated = sx_xlocked(&sc->call_lock) != 0;
>+	if (!allocated)
>+		sx_xlock(&sc->call_lock);
> 
> 	/* Debugging - dump registers. */
> 	if (ig4_dump) {
>@@ -447,21 +622,11 @@ ig4iic_transfer(device_t dev, struct iic_msg *msgs, uint32_t nmsgs)
> 	 */
> 	reg_read(sc, IG4_REG_CLR_TX_ABORT);
> 
>-	/*
>-	 * Clean out any previously received data.
>-	 */
>-	if (sc->rpos != sc->rnext && bootverbose) {
>-		device_printf(sc->dev, "discarding %d bytes of spurious data\n",
>-		    sc->rnext - sc->rpos);
>-	}
>-	sc->rpos = 0;
>-	sc->rnext = 0;
>-
> 	rpstart = false;
> 	error = 0;
> 	for (i = 0; i < nmsgs; i++) {
> 		if ((msgs[i].flags & IIC_M_NOSTART) == 0) {
>-			error = ig4iic_xfer_start(sc, msgs[i].slave);
>+			error = ig4iic_xfer_start(sc, msgs[i].slave, rpstart);
> 		} else {
> 			if (!sc->slave_valid ||
> 			    (msgs[i].slave >> 1) != sc->last_slave) {
>@@ -482,14 +647,42 @@ ig4iic_transfer(device_t dev, struct iic_msg *msgs, uint32_t nmsgs)
> 		else
> 			error = ig4iic_write(sc, msgs[i].buf, msgs[i].len,
> 			    rpstart, stop);
>-		if (error != 0)
>+
>+		/* Wait for error or stop condition occurred on the I2C bus */
>+		if (stop && error == 0) {
>+			error = wait_intr(sc, IG4_INTR_STOP_DET);
>+			if (error == 0)
>+				reg_read(sc, IG4_REG_CLR_INTR);
>+		}
>+
>+		if (error != 0) {
>+			/*
>+			 * Send STOP condition if it's not done yet and flush
>+			 * both FIFOs. Do a controller soft reset if transfer
>+			 * abort is failed.
>+			 */
>+			if (((reg_read(sc, IG4_REG_RAW_INTR_STAT) &
>+			    (IG4_INTR_START_DET | IG4_INTR_STOP_DET))
>+			     == IG4_INTR_START_DET) &&
>+			    ig4iic_xfer_abort(sc) != 0) {
>+				device_printf(sc->dev, "Failed to abort "
>+				    "transfer. Do the controller reset.\n");
>+				ig4iic_set_config(sc);
>+			} else {
>+				while (reg_read(sc, IG4_REG_I2C_STA) &
>+				    IG4_STATUS_RX_NOTEMPTY)
>+					reg_read(sc, IG4_REG_DATA_CMD);
>+				reg_read(sc, IG4_REG_TX_ABRT_SOURCE);
>+				reg_read(sc, IG4_REG_CLR_INTR);
>+			}
> 			break;
>+		}
> 
> 		rpstart = !stop;
> 	}
> 
>-	mtx_unlock(&sc->io_lock);
>-	sx_unlock(&sc->call_lock);
>+	if (!allocated)
>+		sx_unlock(&sc->call_lock);
> 	return (error);
> }
> 
>@@ -497,9 +690,11 @@ int
> ig4iic_reset(device_t dev, u_char speed, u_char addr, u_char *oldaddr)
> {
> 	ig4iic_softc_t *sc = device_get_softc(dev);
>+	bool allocated;
> 
>-	sx_xlock(&sc->call_lock);
>-	mtx_lock(&sc->io_lock);
>+	allocated = sx_xlocked(&sc->call_lock) != 0;
>+	if (!allocated)
>+		sx_xlock(&sc->call_lock);
> 
> 	/* TODO handle speed configuration? */
> 	if (oldaddr != NULL)
>@@ -508,32 +703,193 @@ ig4iic_reset(device_t dev, u_char speed, u_char addr, u_char *oldaddr)
> 	if (addr == IIC_UNKNOWN)
> 		sc->slave_valid = false;
> 
>-	mtx_unlock(&sc->io_lock);
>-	sx_unlock(&sc->call_lock);
>+	if (!allocated)
>+		sx_unlock(&sc->call_lock);
> 	return (0);
> }
> 
>-/*
>- * Called from ig4iic_pci_attach/detach()
>- */
> int
>-ig4iic_attach(ig4iic_softc_t *sc)
>+ig4iic_callback(device_t dev, int index, caddr_t data)
>+{
>+	ig4iic_softc_t *sc = device_get_softc(dev);
>+	int error = 0;
>+	int how;
>+
>+	/*
>+	 * Unfortunately, iicbus_request_bus() can return error code in both
>+	 * formats, POSIX and IIC. Here we use EAGAIN instead of IIC_EBUSBSY
>+	 * to match other possibly buggy iicbus_callback implementations.
>+	 * As iicbus_poll() returns IIC_EBUSBSY, caller should check returned
>+	 * value for both EAGAIN and IIC_EBUSBSY. Other error codes are POSIX.
>+	 */
>+	switch (index) {
>+	case IIC_REQUEST_BUS:
>+		/* force polling if ig4iic is requested with IIC_DONTWAIT */
>+		how = *(int *)data;
>+		if ((how & IIC_WAIT) == 0) {
>+			if (sx_try_xlock(&sc->call_lock) == 0)
>+				error = EWOULDBLOCK;
>+			else
>+				sc->poll = true;
>+		} else
>+			sx_xlock(&sc->call_lock);
>+		break;
>+
>+	case IIC_RELEASE_BUS:
>+		sc->poll = false;
>+		sx_unlock(&sc->call_lock);
>+		break;
>+
>+	default:
>+		error = EINVAL;
>+	}
>+
>+	return (error);
>+}
>+
>+#ifdef DEV_ACPI
>+static int
>+ig4iic_acpi_params(ig4iic_softc_t *sc, char *method,
>+    uint16_t *scl_hcnt, uint16_t *scl_lcnt, uint16_t *sda_tx_hold)
> {
>+	ACPI_BUFFER buf;
>+	ACPI_HANDLE handle;
>+	ACPI_OBJECT *obj, *elems;
> 	int error;
>+
>+	handle = acpi_get_handle(sc->dev);
>+	if (handle == NULL)
>+		return (ENXIO);
>+
>+	buf.Pointer = NULL;
>+	buf.Length = ACPI_ALLOCATE_BUFFER;
>+
>+	if (ACPI_FAILURE(AcpiEvaluateObject(handle, method, NULL, &buf)))
>+		return (ENXIO);
>+
>+	error = ENXIO;
>+	obj = (ACPI_OBJECT *)buf.Pointer;
>+	if (obj->Type == ACPI_TYPE_PACKAGE && obj->Package.Count == 3) {
>+		elems = obj->Package.Elements;
>+		*scl_hcnt = elems[0].Integer.Value & IG4_SCL_CLOCK_MASK;
>+		*scl_lcnt = elems[1].Integer.Value & IG4_SCL_CLOCK_MASK;
>+		*sda_tx_hold = elems[2].Integer.Value & IG4_SDA_TX_HOLD_MASK;
>+		error = 0;
>+	}
>+
>+	AcpiOsFree(obj);
>+
>+	return (error);
>+}
>+#endif /* DEV_ACPI */
>+
>+static void
>+ig4iic_get_config(ig4iic_softc_t *sc)
>+{
>+	const struct ig4_cfg *cfg;
> 	uint32_t v;
>+#ifdef DEV_ACPI
>+	uint16_t sda_tx_hold;
>+#endif
> 
>-	mtx_init(&sc->io_lock, "IG4 I/O lock", NULL, MTX_DEF);
>-	sx_init(&sc->call_lock, "IG4 call lock");
>+	/* Fetch default hardware config from controller */
>+	sc->cfg.version = reg_read(sc, IG4_REG_COMP_VER);
>+	sc->cfg.bus_speed = reg_read(sc, IG4_REG_CTL) & IG4_CTL_SPEED_MASK;
>+	sc->cfg.ss_scl_hcnt =
>+	    reg_read(sc, IG4_REG_SS_SCL_HCNT) & IG4_SCL_CLOCK_MASK;
>+	sc->cfg.ss_scl_lcnt =
>+	    reg_read(sc, IG4_REG_SS_SCL_LCNT) & IG4_SCL_CLOCK_MASK;
>+	sc->cfg.fs_scl_hcnt =
>+	    reg_read(sc, IG4_REG_FS_SCL_HCNT) & IG4_SCL_CLOCK_MASK;
>+	sc->cfg.fs_scl_lcnt =
>+	    reg_read(sc, IG4_REG_FS_SCL_LCNT) & IG4_SCL_CLOCK_MASK;
>+	sc->cfg.sda_tx_hold =
>+	    reg_read(sc, IG4_REG_SDA_HOLD) & IG4_SDA_TX_HOLD_MASK;
>+
>+	if (sc->version == IG4_HASWELL || sc->version == IG4_ATOM) {
>+		/* REG_COMP_PARAM1 register is not documented in Intel specs */
>+		v = reg_read(sc, IG4_REG_COMP_PARAM1);
>+		if (IG4_PARAM1_TXFIFO_DEPTH(v) != 0)
>+			sc->cfg.txfifo_depth = IG4_PARAM1_TXFIFO_DEPTH(v);
>+		if (IG4_PARAM1_RXFIFO_DEPTH(v) != 0)
>+			sc->cfg.rxfifo_depth = IG4_PARAM1_RXFIFO_DEPTH(v);
>+	} else {
>+		/*
>+		 * Hardware does not allow FIFO Threshold Levels value to be
>+		 * set larger than the depth of the buffer. If an attempt is
>+		 * made to do that, the actual value set will be the maximum
>+		 * depth of the buffer.
>+		 */
>+		v = reg_read(sc, IG4_REG_TX_TL);
>+		reg_write(sc, IG4_REG_TX_TL, v | IG4_FIFO_MASK);
>+		sc->cfg.txfifo_depth =
>+		    (reg_read(sc, IG4_REG_TX_TL) & IG4_FIFO_MASK) + 1;
>+		reg_write(sc, IG4_REG_TX_TL, v);
>+		v = reg_read(sc, IG4_REG_RX_TL);
>+		reg_write(sc, IG4_REG_RX_TL, v | IG4_FIFO_MASK);
>+		sc->cfg.rxfifo_depth =
>+		    (reg_read(sc, IG4_REG_RX_TL) & IG4_FIFO_MASK) + 1;
>+		reg_write(sc, IG4_REG_RX_TL, v);
>+	}
>+
>+	/* Override hardware config with precalculated counter values */
>+	if (sc->version < nitems(ig4iic_configs)) {
>+		cfg = &ig4iic_configs[sc->version];
>+		if (cfg->bus_speed != 0)
>+			sc->cfg.bus_speed = cfg->bus_speed;
>+		if (cfg->ss_scl_hcnt != 0)
>+	                sc->cfg.ss_scl_hcnt = cfg->ss_scl_hcnt;
>+		if (cfg->ss_scl_lcnt != 0)
>+	                sc->cfg.ss_scl_lcnt = cfg->ss_scl_lcnt;
>+		if (cfg->fs_scl_hcnt != 0)
>+	                sc->cfg.fs_scl_hcnt = cfg->fs_scl_hcnt;
>+		if (cfg->fs_scl_lcnt != 0)
>+	                sc->cfg.fs_scl_lcnt = cfg->fs_scl_lcnt;
>+		if (cfg->sda_tx_hold != 0)
>+			sc->cfg.sda_tx_hold = cfg->sda_tx_hold;
>+		if (cfg->txfifo_depth != 0)
>+			sc->cfg.txfifo_depth = cfg->txfifo_depth;
>+		if (cfg->rxfifo_depth != 0)
>+			sc->cfg.rxfifo_depth = cfg->rxfifo_depth;
>+	}
>+
>+	if (sc->cfg.bus_speed != IG4_CTL_SPEED_STD)
>+		sc->cfg.bus_speed = IG4_CTL_SPEED_FAST;
>+
>+#ifdef DEV_ACPI
>+	/* Evaluate SSCN and FMCN ACPI methods to fetch timings */
>+	if (ig4iic_acpi_params(sc, "SSCN",
>+	    &sc->cfg.ss_scl_hcnt, &sc->cfg.ss_scl_lcnt, &sda_tx_hold) == 0 &&
>+	    sc->cfg.bus_speed == IG4_CTL_SPEED_STD &&
>+	    sda_tx_hold != 0)
>+		sc->cfg.sda_tx_hold = sda_tx_hold;
>+	if (ig4iic_acpi_params(sc, "FMCN",
>+	    &sc->cfg.fs_scl_hcnt, &sc->cfg.fs_scl_lcnt, &sda_tx_hold) == 0 &&
>+	    sc->cfg.bus_speed == IG4_CTL_SPEED_FAST &&
>+	    sda_tx_hold != 0)
>+		sc->cfg.sda_tx_hold = sda_tx_hold;
>+#endif
>+}
>+
>+static int
>+ig4iic_set_config(ig4iic_softc_t *sc)
>+{
>+	uint32_t v;
> 
> 	v = reg_read(sc, IG4_REG_DEVIDLE_CTRL);
> 	if (sc->version == IG4_SKYLAKE && (v & IG4_RESTORE_REQUIRED) ) {
> 		reg_write(sc, IG4_REG_DEVIDLE_CTRL, IG4_DEVICE_IDLE | IG4_RESTORE_REQUIRED);
> 		reg_write(sc, IG4_REG_DEVIDLE_CTRL, 0);
>+	}
> 
>+	if (sc->version == IG4_HASWELL || sc->version == IG4_ATOM) {
>+		reg_write(sc, IG4_REG_RESETS_HSW, IG4_RESETS_ASSERT_HSW);
>+		reg_write(sc, IG4_REG_RESETS_HSW, IG4_RESETS_DEASSERT_HSW);
>+	} else if (sc->version == IG4_SKYLAKE) {
> 		reg_write(sc, IG4_REG_RESETS_SKL, IG4_RESETS_ASSERT_SKL);
> 		reg_write(sc, IG4_REG_RESETS_SKL, IG4_RESETS_DEASSERT_SKL);
>-		DELAY(1000);
> 	}
>+	pause("i2crst", 1);
> 
> 	if (sc->version == IG4_ATOM)
> 		v = reg_read(sc, IG4_REG_COMP_TYPE);
>@@ -563,35 +919,24 @@ ig4iic_attach(ig4iic_softc_t *sc)
> 
> 	if (sc->version == IG4_HASWELL || sc->version == IG4_ATOM) {
> 		v = reg_read(sc, IG4_REG_COMP_VER);
>-		if (v < IG4_COMP_MIN_VER) {
>-			error = ENXIO;
>-			goto done;
>-		}
>+		if (v < IG4_COMP_MIN_VER)
>+			return(ENXIO);
> 	}
>-	v = reg_read(sc, IG4_REG_SS_SCL_HCNT);
>-	v = reg_read(sc, IG4_REG_SS_SCL_LCNT);
>-	v = reg_read(sc, IG4_REG_FS_SCL_HCNT);
>-	v = reg_read(sc, IG4_REG_FS_SCL_LCNT);
>-	v = reg_read(sc, IG4_REG_SDA_HOLD);
> 
>-	v = reg_read(sc, IG4_REG_SS_SCL_HCNT);
>-	reg_write(sc, IG4_REG_FS_SCL_HCNT, v);
>-	v = reg_read(sc, IG4_REG_SS_SCL_LCNT);
>-	reg_write(sc, IG4_REG_FS_SCL_LCNT, v);
>+	if (set_controller(sc, 0)) {
>+		device_printf(sc->dev, "controller error during attach-1\n");
>+		return (ENXIO);
>+	}
> 
>-	/*
>-	 * Program based on a 25000 Hz clock.  This is a bit of a
>-	 * hack (obviously).  The defaults are 400 and 470 for standard
>-	 * and 60 and 130 for fast.  The defaults for standard fail
>-	 * utterly (presumably cause an abort) because the clock time
>-	 * is ~18.8ms by default.  This brings it down to ~4ms (for now).
>-	 */
>-	reg_write(sc, IG4_REG_SS_SCL_HCNT, 100);
>-	reg_write(sc, IG4_REG_SS_SCL_LCNT, 125);
>-	reg_write(sc, IG4_REG_FS_SCL_HCNT, 100);
>-	reg_write(sc, IG4_REG_FS_SCL_LCNT, 125);
>-	if (sc->version == IG4_SKYLAKE)
>-		reg_write(sc, IG4_REG_SDA_HOLD, 28);
>+	reg_read(sc, IG4_REG_CLR_INTR);
>+	reg_write(sc, IG4_REG_INTR_MASK, 0);
>+	sc->intr_mask = 0;
>+
>+	reg_write(sc, IG4_REG_SS_SCL_HCNT, sc->cfg.ss_scl_hcnt);
>+	reg_write(sc, IG4_REG_SS_SCL_LCNT, sc->cfg.ss_scl_lcnt);
>+	reg_write(sc, IG4_REG_FS_SCL_HCNT, sc->cfg.fs_scl_hcnt);
>+	reg_write(sc, IG4_REG_FS_SCL_LCNT, sc->cfg.fs_scl_lcnt);
>+	reg_write(sc, IG4_REG_SDA_HOLD, sc->cfg.sda_tx_hold);
> 
> 	/*
> 	 * Use a threshold of 1 so we get interrupted on each character,
>@@ -600,13 +945,37 @@ ig4iic_attach(ig4iic_softc_t *sc)
> 	 *
> 	 * See ig4_var.h for details on interrupt handler synchronization.
> 	 */
>-	reg_write(sc, IG4_REG_RX_TL, 1);
>+	reg_write(sc, IG4_REG_RX_TL, 0);
>+	reg_write(sc, IG4_REG_TX_TL, 0);
> 
> 	reg_write(sc, IG4_REG_CTL,
> 		  IG4_CTL_MASTER |
> 		  IG4_CTL_SLAVE_DISABLE |
> 		  IG4_CTL_RESTARTEN |
>-		  IG4_CTL_SPEED_STD);
>+		  (sc->cfg.bus_speed & IG4_CTL_SPEED_MASK));
>+
>+	/* Force setting of the target address on the next transfer */
>+	sc->slave_valid = 0;
>+
>+	return (0);
>+}
>+
>+/*
>+ * Called from ig4iic_pci_attach/detach()
>+ */
>+int
>+ig4iic_attach(ig4iic_softc_t *sc)
>+{
>+	int error;
>+
>+	mtx_init(&sc->io_lock, "IG4 I/O lock", NULL, MTX_DEF);
>+	sx_init(&sc->call_lock, "IG4 call lock");
>+
>+	ig4iic_get_config(sc);
>+
>+	error = ig4iic_set_config(sc);
>+	if (error)
>+		goto done;
> 
> 	sc->iicbus = device_add_child(sc->dev, "iicbus", -1);
> 	if (sc->iicbus == NULL) {
>@@ -615,25 +984,16 @@ ig4iic_attach(ig4iic_softc_t *sc)
> 		goto done;
> 	}
> 
>-#if 0
>-	/*
>-	 * Don't do this, it blows up the PCI config
>-	 */
>-	if (sc->version == IG4_HASWELL || sc->version == IG4_ATOM) {
>-		reg_write(sc, IG4_REG_RESETS_HSW, IG4_RESETS_ASSERT_HSW);
>-		reg_write(sc, IG4_REG_RESETS_HSW, IG4_RESETS_DEASSERT_HSW);
>-	} else if (sc->version = IG4_SKYLAKE) {
>-		reg_write(sc, IG4_REG_RESETS_SKL, IG4_RESETS_ASSERT_SKL);
>-		reg_write(sc, IG4_REG_RESETS_SKL, IG4_RESETS_DEASSERT_SKL);
>-	}
>-#endif
>-
>-	mtx_lock(&sc->io_lock);
>-	if (set_controller(sc, 0))
>-		device_printf(sc->dev, "controller error during attach-1\n");
>-	if (set_controller(sc, IG4_I2C_ENABLE))
>+	if (set_controller(sc, IG4_I2C_ENABLE)) {
> 		device_printf(sc->dev, "controller error during attach-2\n");
>-	mtx_unlock(&sc->io_lock);
>+		error = ENXIO;
>+		goto done;
>+	}
>+	if (set_controller(sc, 0)) {
>+		device_printf(sc->dev, "controller error during attach-3\n");
>+		error = ENXIO;
>+		goto done;
>+	}
> 	error = bus_setup_intr(sc->dev, sc->intr_res, INTR_TYPE_MISC | INTR_MPSAFE,
> 			       NULL, ig4iic_intr, sc, &sc->intr_handle);
> 	if (error) {
>@@ -641,38 +1001,14 @@ ig4iic_attach(ig4iic_softc_t *sc)
> 			      "Unable to setup irq: error %d\n", error);
> 	}
> 
>-	sc->enum_hook.ich_func = ig4iic_start;
>-	sc->enum_hook.ich_arg = sc->dev;
>-
>-	/*
>-	 * We have to wait until interrupts are enabled. I2C read and write
>-	 * only works if the interrupts are available.
>-	 */
>-	if (config_intrhook_establish(&sc->enum_hook) != 0)
>-		error = ENOMEM;
>-	else
>-		error = 0;
>-
>-done:
>-	return (error);
>-}
>-
>-void
>-ig4iic_start(void *xdev)
>-{
>-	int error;
>-	ig4iic_softc_t *sc;
>-	device_t dev = (device_t)xdev;
>-
>-	sc = device_get_softc(dev);
>-
>-	config_intrhook_disestablish(&sc->enum_hook);
>-
> 	error = bus_generic_attach(sc->dev);
> 	if (error) {
> 		device_printf(sc->dev,
> 			      "failed to attach child: error %d\n", error);
> 	}
>+
>+done:
>+	return (error);
> }
> 
> int
>@@ -691,14 +1027,12 @@ ig4iic_detach(ig4iic_softc_t *sc)
> 		bus_teardown_intr(sc->dev, sc->intr_res, sc->intr_handle);
> 
> 	sx_xlock(&sc->call_lock);
>-	mtx_lock(&sc->io_lock);
> 
> 	sc->iicbus = NULL;
> 	sc->intr_handle = NULL;
> 	reg_write(sc, IG4_REG_INTR_MASK, 0);
> 	set_controller(sc, 0);
> 
>-	mtx_unlock(&sc->io_lock);
> 	sx_xunlock(&sc->call_lock);
> 
> 	mtx_destroy(&sc->io_lock);
>@@ -707,6 +1041,50 @@ ig4iic_detach(ig4iic_softc_t *sc)
> 	return (0);
> }
> 
>+int
>+ig4iic_suspend(ig4iic_softc_t *sc)
>+{
>+	int error;
>+
>+	/* suspend all children */
>+	error = bus_generic_suspend(sc->dev);
>+
>+	/*
>+	* set IG4_DEVICE_IDLE and IG4_RESTORE_REQUIRED
>+	* to place the device in the idle state, just to be safe
>+	*/
>+	if (sc->version == IG4_SKYLAKE) {
>+		sx_xlock(&sc->call_lock);
>+
>+		reg_write(sc, IG4_REG_DEVIDLE_CTRL,
>+		    IG4_DEVICE_IDLE | IG4_RESTORE_REQUIRED);
>+
>+		sx_xunlock(&sc->call_lock);
>+	}
>+
>+	return (error);
>+}
>+
>+int ig4iic_resume(ig4iic_softc_t *sc)
>+{
>+	int error;
>+
>+	sx_xlock(&sc->call_lock);
>+	/* Use polling as interrupts might not be available at this moment */
>+	sc->resume = true;
>+	if (ig4iic_set_config(sc))
>+		device_printf(sc->dev, "controller error during resume\n");
>+	sx_xunlock(&sc->call_lock);
>+
>+	error = bus_generic_resume(sc->dev);
>+
>+	sx_xlock(&sc->call_lock);
>+	sc->resume = false;
>+	sx_xunlock(&sc->call_lock);
>+
>+	return (error);
>+}
>+
> /*
>  * Interrupt Operation, see ig4_var.h for locking semantics.
>  */
>@@ -717,29 +1095,13 @@ ig4iic_intr(void *cookie)
> 	uint32_t status;
> 
> 	mtx_lock(&sc->io_lock);
>-/*	reg_write(sc, IG4_REG_INTR_MASK, IG4_INTR_STOP_DET);*/
>-	reg_read(sc, IG4_REG_CLR_INTR);
>-	status = reg_read(sc, IG4_REG_I2C_STA);
>-	while (status & IG4_STATUS_RX_NOTEMPTY) {
>-		sc->rbuf[sc->rnext & IG4_RBUFMASK] =
>-		    (uint8_t)reg_read(sc, IG4_REG_DATA_CMD);
>-		++sc->rnext;
>-		status = reg_read(sc, IG4_REG_I2C_STA);
>-	}
>-
>-	/* 
>-	 * Workaround to trigger pending interrupt if IG4_REG_INTR_STAT
>-	 * is changed after clearing it
>-	 */
>-	if (sc->access_intr_mask != 0) {
>-		status = reg_read(sc, IG4_REG_INTR_MASK);
>-		if (status != 0) {
>-			reg_write(sc, IG4_REG_INTR_MASK, 0);
>-			reg_write(sc, IG4_REG_INTR_MASK, status);
>-		}
>-	}
>-
>-	wakeup(sc);
>+	if (sc->intr_mask != 0) {
>+		status = reg_read(sc, IG4_REG_INTR_STAT);
>+		set_intr_mask(sc, 0);
>+		sc->error = intrstat2iic(sc, status);
>+		wakeup(sc);
>+	} else
>+		reg_write(sc, IG4_REG_INTR_MASK, 0);
> 	mtx_unlock(&sc->io_lock);
> }
> 
>@@ -773,10 +1135,8 @@ ig4iic_dump(ig4iic_softc_t *sc)
> 	REGDUMP(sc, IG4_REG_DMA_RDLR);
> 	REGDUMP(sc, IG4_REG_SDA_SETUP);
> 	REGDUMP(sc, IG4_REG_ENABLE_STATUS);
>-	if (sc->version == IG4_HASWELL || sc->version == IG4_ATOM) {
>-		REGDUMP(sc, IG4_REG_COMP_PARAM1);
>-		REGDUMP(sc, IG4_REG_COMP_VER);
>-	}
>+	REGDUMP(sc, IG4_REG_COMP_PARAM1);
>+	REGDUMP(sc, IG4_REG_COMP_VER);
> 	if (sc->version == IG4_ATOM) {
> 		REGDUMP(sc, IG4_REG_COMP_TYPE);
> 		REGDUMP(sc, IG4_REG_CLK_PARMS);
>diff --git a/sys/dev/ichiic/ig4_pci.c b/sys/dev/ichiic/ig4_pci.c
>index 8d7275ae57e6..b15950262909 100644
>--- a/sys/dev/ichiic/ig4_pci.c
>+++ b/sys/dev/ichiic/ig4_pci.c
>@@ -206,21 +206,51 @@ ig4iic_pci_detach(device_t dev)
> 	return (0);
> }
> 
>+static int
>+ig4iic_pci_suspend(device_t dev)
>+{
>+	ig4iic_softc_t *sc = device_get_softc(dev);
>+
>+	return (ig4iic_suspend(sc));
>+}
>+
>+static int
>+ig4iic_pci_resume(device_t dev)
>+{
>+	ig4iic_softc_t *sc  = device_get_softc(dev);
>+
>+	return (ig4iic_resume(sc));
>+}
>+
> static device_method_t ig4iic_pci_methods[] = {
> 	/* Device interface */
> 	DEVMETHOD(device_probe, ig4iic_pci_probe),
> 	DEVMETHOD(device_attach, ig4iic_pci_attach),
> 	DEVMETHOD(device_detach, ig4iic_pci_detach),
>+	DEVMETHOD(device_suspend, ig4iic_pci_suspend),
>+	DEVMETHOD(device_resume, ig4iic_pci_resume),
>+
>+	/* Bus interface */
>+	DEVMETHOD(bus_setup_intr, bus_generic_setup_intr),
>+	DEVMETHOD(bus_teardown_intr, bus_generic_teardown_intr),
>+	DEVMETHOD(bus_alloc_resource, bus_generic_alloc_resource),
>+	DEVMETHOD(bus_release_resource, bus_generic_release_resource),
>+	DEVMETHOD(bus_activate_resource, bus_generic_activate_resource),
>+	DEVMETHOD(bus_deactivate_resource, bus_generic_deactivate_resource),
>+	DEVMETHOD(bus_adjust_resource, bus_generic_adjust_resource),
>+	DEVMETHOD(bus_set_resource, bus_generic_rl_set_resource),
>+	DEVMETHOD(bus_get_resource, bus_generic_rl_get_resource),
> 
>+	/* iicbus interface */
> 	DEVMETHOD(iicbus_transfer, ig4iic_transfer),
> 	DEVMETHOD(iicbus_reset, ig4iic_reset),
>-	DEVMETHOD(iicbus_callback, iicbus_null_callback),
>+	DEVMETHOD(iicbus_callback, ig4iic_callback),
> 
> 	DEVMETHOD_END
> };
> 
> static driver_t ig4iic_pci_driver = {
>-	"ig4iic_pci",
>+	"ig4iic",
> 	ig4iic_pci_methods,
> 	sizeof(struct ig4iic_softc)
> };
>diff --git a/sys/dev/ichiic/ig4_reg.h b/sys/dev/ichiic/ig4_reg.h
>index 45bc6aaed55f..24543ffe9dbf 100644
>--- a/sys/dev/ichiic/ig4_reg.h
>+++ b/sys/dev/ichiic/ig4_reg.h
>@@ -112,7 +112,7 @@
> #define IG4_REG_SDA_SETUP	0x0094	/* RW	SDA Setup */
> #define IG4_REG_ACK_GENERAL_CALL 0x0098	/* RW	I2C ACK General Call */
> #define IG4_REG_ENABLE_STATUS	0x009C	/* RO	Enable Status */
>-/* Available at least on Atom SoCs and Haswell mobile. */
>+/* Available at least on Atom SoCs, Haswell mobile and some Skylakes. */
> #define IG4_REG_COMP_PARAM1	0x00F4	/* RO	Component Parameter */
> #define IG4_REG_COMP_VER	0x00F8	/* RO	Component Version */
> /* Available at least on Atom SoCs */
>@@ -154,9 +154,12 @@
> #define IG4_CTL_SLAVE_DISABLE	0x0040	/* snarfed from linux */
> #define IG4_CTL_RESTARTEN	0x0020	/* Allow Restart when master */
> #define IG4_CTL_10BIT		0x0010	/* ctlr accepts 10-bit addresses */
>+#define IG4_CTL_SPEED_MASK	0x0006	/* speed at which the I2C operates */
>+#define IG4_CTL_MASTER		0x0001	/* snarfed from linux */
>+
>+#define IG4_CTL_SPEED_HIGH	0x0006	/* snarfed from linux */
> #define IG4_CTL_SPEED_FAST	0x0004	/* snarfed from linux */
> #define IG4_CTL_SPEED_STD	0x0002	/* snarfed from linux */
>-#define IG4_CTL_MASTER		0x0001	/* snarfed from linux */
> 
> /*
>  * TAR_ADD - Target Address Register	22.2.2
>@@ -325,6 +328,9 @@
> #define IG4_INTR_RX_OVER	0x0002
> #define IG4_INTR_RX_UNDER	0x0001
> 
>+#define IG4_INTR_ERR_MASK	(IG4_INTR_TX_ABRT | IG4_INTR_TX_OVER | \
>+				 IG4_INTR_RX_OVER | IG4_INTR_RX_UNDER)
>+
> /*
>  * RX_TL	- (RW) Receive FIFO Threshold Register		22.2.11
>  * TX_TL	- (RW) Transmit FIFO Threshold Register		22.2.12
>@@ -377,7 +383,9 @@
>  * I2C_EN	- (RW) I2C Enable Register			22.2.22
>  *
>  *	ABORT		Software can abort an I2C transfer by setting this
>- *			bit.  Hardware will clear the bit once the STOP has
>+ *			bit. In response, the controller issues the STOP
>+ *			condition over the I2C bus, followed by TX FIFO flush.
>+ *			Hardware will clear the bit once the STOP has
>  *			been detected.  This bit can only be set while the
>  *			I2C interface is enabled.
>  *
>@@ -406,14 +414,14 @@
>  *	FIFOs.  Note that for some reason the mask is 9 bits instead of
>  *	the 8 bits the fill level controls.
>  */
>-#define IG4_FIFOLVL_MASK	0x001F
>+#define IG4_FIFOLVL_MASK	0x01FF
> 
> /*
>  * SDA_HOLD	- (RW) SDA Hold Time Length Register		22.2.26
>  *
>  *	Set the SDA hold time length register in I2C clocks.
>  */
>-#define IG4_SDA_HOLD_MASK	0x00FF
>+#define IG4_SDA_TX_HOLD_MASK	0x0000FFFF
> 
> /*
>  * TX_ABRT_SOURCE- (RO) Transmit Abort Source Register		22.2.27
>@@ -432,8 +440,8 @@
> #define IG4_ABRTSRC_NORESTART_10	0x00000400 /* RESTART disabled */
> #define IG4_ABRTSRC_NORESTART_START	0x00000200 /* RESTART disabled */
> #define IG4_ABRTSRC_ACKED_START		0x00000080 /* Improper acked START */
>-#define IG4_ABRTSRC_GENCALL_NOACK	0x00000020 /* Improper GENCALL */
>-#define IG4_ABRTSRC_GENCALL_READ	0x00000010 /* Nobody acked GENCALL */
>+#define IG4_ABRTSRC_GENCALL_READ	0x00000020 /* Improper GENCALL */
>+#define IG4_ABRTSRC_GENCALL_NOACK	0x00000010 /* Nobody acked GENCALL */
> #define IG4_ABRTSRC_TXNOACK_DATA	0x00000008 /* data phase no ACK */
> #define IG4_ABRTSRC_TXNOACK_ADDR10_2	0x00000004 /* addr10/1 phase no ACK */
> #define IG4_ABRTSRC_TXNOACK_ADDR10_1	0x00000002 /* addr10/2 phase no ACK */
>@@ -530,8 +538,8 @@
>  *
>  *	DATAW		- Indicates the internal bus width in bits.
>  */
>-#define IG4_PARAM1_TXFIFO_DEPTH(v)	(((v) >> 16) & 0xFF)
>-#define IG4_PARAM1_RXFIFO_DEPTH(v)	(((v) >> 8) & 0xFF)
>+#define IG4_PARAM1_TXFIFO_DEPTH(v)	((((v) >> 16) & 0xFF) + 1)
>+#define IG4_PARAM1_RXFIFO_DEPTH(v)	((((v) >> 8) & 0xFF) + 1)
> #define IG4_PARAM1_CONFIG_VALID		0x00000080
> #define IG4_PARAM1_CONFIG_HASDMA	0x00000040
> #define IG4_PARAM1_CONFIG_INTR_IO	0x00000020
>diff --git a/sys/dev/ichiic/ig4_var.h b/sys/dev/ichiic/ig4_var.h
>index 3250df5f7496..b1e4b3ee66a9 100644
>--- a/sys/dev/ichiic/ig4_var.h
>+++ b/sys/dev/ichiic/ig4_var.h
>@@ -43,15 +43,23 @@
> #include "pci_if.h"
> #include "iicbus_if.h"
> 
>-#define IG4_RBUFSIZE	128
>-#define IG4_RBUFMASK	(IG4_RBUFSIZE - 1)
>-
> enum ig4_op { IG4_IDLE, IG4_READ, IG4_WRITE };
> enum ig4_vers { IG4_HASWELL, IG4_ATOM, IG4_SKYLAKE, IG4_APL };
> 
>+struct ig4_cfg {
>+	uint32_t	version;
>+	uint32_t	bus_speed;
>+	uint16_t	ss_scl_hcnt;
>+	uint16_t	ss_scl_lcnt;
>+	uint16_t	fs_scl_hcnt;
>+	uint16_t	fs_scl_lcnt;
>+	uint16_t	sda_tx_hold;
>+	int		txfifo_depth;
>+	int		rxfifo_depth;
>+};
>+
> struct ig4iic_softc {
> 	device_t	dev;
>-	struct		intr_config_hook enum_hook;
> 	device_t	iicbus;
> 	struct resource	*regs_res;
> 	int		regs_rid;
>@@ -61,39 +69,28 @@ struct ig4iic_softc {
> 	int		intr_type;
> 	enum ig4_vers	version;
> 	enum ig4_op	op;
>+	struct ig4_cfg	cfg;
> 	int		cmd;
>-	int		rnext;
>-	int		rpos;
>-	char		rbuf[IG4_RBUFSIZE];
>+	uint32_t	intr_mask;
> 	int		error;
> 	uint8_t		last_slave;
> 	int		platform_attached : 1;
> 	int		use_10bit : 1;
> 	int		slave_valid : 1;
>-	int		read_started : 1;
>-	int		write_started : 1;
>-	int		access_intr_mask : 1;
>+	int		poll: 1;
>+	int		resume: 1;
> 
> 	/*
> 	 * Locking semantics:
> 	 *
> 	 * Functions implementing the icbus interface that interact
> 	 * with the controller acquire an exclusive lock on call_lock
>-	 * to prevent interleaving of calls to the interface and a lock on
>-	 * io_lock right afterwards, to synchronize controller I/O activity.
>-	 *
>-	 * The interrupt handler can only read data while no iicbus call
>-	 * is in progress or while io_lock is dropped during mtx_sleep in
>-	 * wait_status and set_controller. It is safe to drop io_lock in those
>-	 * places, because the interrupt handler only accesses those registers:
>-	 *
>-	 * - IG4_REG_I2C_STA  (I2C Status)
>-	 * - IG4_REG_DATA_CMD (Data Buffer and Command)
>-	 * - IG4_REG_CLR_INTR (Clear Interrupt)
>+	 * to prevent interleaving of calls to the interface.
> 	 *
>-	 * Locking outside of those places is required to make the content
>-	 * of rpos/rnext predictable (e.g. whenever data_read is called and in
>-	 * ig4iic_transfer).
>+	 * io_lock is used as condition variable to synchronize active process
>+	 * with the interrupt handler. It should not be used for tasks other
>+	 * than waiting for interrupt and passing parameters to and from
>+	 * it's handler.
> 	 */
> 	struct sx	call_lock;
> 	struct mtx	io_lock;
>@@ -104,9 +101,12 @@ typedef struct ig4iic_softc ig4iic_softc_t;
> /* Attach/Detach called from ig4iic_pci_*() */
> int ig4iic_attach(ig4iic_softc_t *sc);
> int ig4iic_detach(ig4iic_softc_t *sc);
>+int ig4iic_suspend(ig4iic_softc_t *sc);
>+int ig4iic_resume(ig4iic_softc_t *sc);
> 
> /* iicbus methods */
> extern iicbus_transfer_t ig4iic_transfer;
> extern iicbus_reset_t   ig4iic_reset;
>+extern iicbus_callback_t ig4iic_callback;
> 
> #endif /* _ICHIIC_IG4_VAR_H_ */
>diff --git a/sys/dev/iicbus/iicbus.c b/sys/dev/iicbus/iicbus.c
>index a4417bc36252..969f9d665a9c 100644
>--- a/sys/dev/iicbus/iicbus.c
>+++ b/sys/dev/iicbus/iicbus.c
>@@ -330,6 +330,8 @@ static device_method_t iicbus_methods[] = {
> 	DEVMETHOD(device_probe,		iicbus_probe),
> 	DEVMETHOD(device_attach,	iicbus_attach),
> 	DEVMETHOD(device_detach,	iicbus_detach),
>+	DEVMETHOD(device_suspend,	bus_generic_suspend),
>+	DEVMETHOD(device_resume,	bus_generic_resume),
> 
> 	/* bus interface */
> 	DEVMETHOD(bus_setup_intr,	bus_generic_setup_intr),

Actions: View | Diff

Attachments on bug 238037: 204527 | 205417 | 205432 | 205447 | 205461 | 205573 | 205867 | 207593