Add locking to the core iicbus(4) drivers:

- Add an sx lock to the iic(4) driver to serialize open(), close(), read(),
  and write and to protect sc_addr and sc_count in the softc.
- Use cdev->si_drv1 instead of using the minor number of the cdev to
  lookup the softc via newbus in iic(4).
- Store the device_t in the softc to avoid a similar detour via minor
  numbers in iic(4).
- Only add at most one instance of iic(4) and iicsmb(4) to each iicbus(4)
  instance, and do it in the child driver.
- Add a mutex to the iicbus(4) softc to synchronize the request/release bus
  stuff.
- Use __BUS_ACCESSOR() for IICBUS_ACCESSOR() instead of rolling our own.
- Add a mutex to the iicsmb(4) softc to protect softc state updated in the
  interrupt handler.
- Remove Giant from all the smbus methods in iicsmb(4) now that all the
  iicbus(4) backend is locked.

5 files changed, 128 insertions, 106 deletions

diff --git a/sys/dev/iicbus/iic.c b/sys/dev/iicbus/iic.c
index 57b0717..6d3cb63 100644
--- a/sys/dev/iicbus/iic.c
+++ b/sys/dev/iicbus/iic.c
@@ -27,14 +27,16 @@
  *
  */
 #include <sys/param.h>
+#include <sys/bus.h>
+#include <sys/conf.h>
+#include <sys/fcntl.h>
+#include <sys/lock.h>
 #include <sys/kernel.h>
-#include <sys/systm.h>
 #include <sys/malloc.h>
 #include <sys/module.h>
-#include <sys/bus.h>
-#include <sys/conf.h>
+#include <sys/sx.h>
+#include <sys/systm.h>
 #include <sys/uio.h>
-#include <sys/fcntl.h>
 
 #include <dev/iicbus/iiconf.h>
 #include <dev/iicbus/iicbus.h>
@@ -46,6 +48,7 @@
 
 struct iic_softc {
 
+	device_t sc_dev;
 	u_char sc_addr;			/* 7 bit address on iicbus */
 	int sc_count;			/* >0 if device opened */
 
@@ -53,13 +56,11 @@ struct iic_softc {
 	char sc_inbuf[BUFSIZE];		/* input buffer */
 
 	struct cdev *sc_devnode;
+	struct sx sc_lock;
 };
 
-#define IIC_SOFTC(unit) \
-	((struct iic_softc *)devclass_get_softc(iic_devclass, (unit)))
-
-#define IIC_DEVICE(unit) \
-	(devclass_get_device(iic_devclass, (unit)))
+#define	IIC_LOCK(sc)			sx_xlock(&(sc)->sc_lock)
+#define	IIC_UNLOCK(sc)			sx_xunlock(&(sc)->sc_lock)
 
 static int iic_probe(device_t);
 static int iic_attach(device_t);
@@ -95,7 +96,7 @@ static	d_ioctl_t	iicioctl;
 
 static struct cdevsw iic_cdevsw = {
 	.d_version =	D_VERSION,
-	.d_flags =	D_NEEDGIANT,
+	.d_flags =	D_TRACKCLOSE,
 	.d_open =	iicopen,
 	.d_close =	iicclose,
 	.d_read =	iicread,
@@ -107,7 +108,9 @@ static struct cdevsw iic_cdevsw = {
 static void
 iic_identify(driver_t *driver, device_t parent)
 {
-	BUS_ADD_CHILD(parent, 0, "iic", -1);
+
+	if (device_find_child(parent, "iic", -1) == NULL)
+		BUS_ADD_CHILD(parent, 0, "iic", -1);
 }
 
 static int
@@ -122,9 +125,18 @@ iic_attach(device_t dev)
 {
 	struct iic_softc *sc = (struct iic_softc *)device_get_softc(dev);
 
+	sc->sc_dev = dev;
+	sx_init(&sc->sc_lock, "iic");
 	sc->sc_devnode = make_dev(&iic_cdevsw, device_get_unit(dev),
 			UID_ROOT, GID_WHEEL,
 			0600, "iic%d", device_get_unit(dev));
+	if (sc->sc_devnode == NULL) {
+		device_printf(dev, "failed to create character device\n");
+		sx_destroy(&sc->sc_lock);
+		return (ENXIO);
+	}
+	sc->sc_devnode->si_drv1 = sc;
+
 	return (0);
 }
 
@@ -135,6 +147,7 @@ iic_detach(device_t dev)
 
 	if (sc->sc_devnode)
 		destroy_dev(sc->sc_devnode);
+	sx_destroy(&sc->sc_lock);
 
 	return (0);
 }
@@ -142,15 +155,16 @@ iic_detach(device_t dev)
 static int
 iicopen(struct cdev *dev, int flags, int fmt, struct thread *td)
 {
-	struct iic_softc *sc = IIC_SOFTC(minor(dev));
+	struct iic_softc *sc = dev->si_drv1;
 
-	if (!sc)
-		return (EINVAL);
-
-	if (sc->sc_count > 0)
+	IIC_LOCK(sc);
+	if (sc->sc_count > 0) {
+		IIC_UNLOCK(sc);
 		return (EBUSY);
+	}
 
 	sc->sc_count++;
+	IIC_UNLOCK(sc);
 
 	return (0);
 }
@@ -158,18 +172,20 @@ iicopen(struct cdev *dev, int flags, int fmt, struct thread *td)
 static int
 iicclose(struct cdev *dev, int flags, int fmt, struct thread *td)
 {
-	struct iic_softc *sc = IIC_SOFTC(minor(dev));
+	struct iic_softc *sc = dev->si_drv1;
 
-	if (!sc)
-		return (EINVAL);
-
-	if (!sc->sc_count)
+	IIC_LOCK(sc);
+	if (!sc->sc_count) {
+		/* XXX: I don't think this can happen. */
+		IIC_UNLOCK(sc);
 		return (EINVAL);
+	}
 
 	sc->sc_count--;
 
 	if (sc->sc_count < 0)
 		panic("%s: iic_count < 0!", __func__);
+	IIC_UNLOCK(sc);
 
 	return (0);
 }
@@ -177,18 +193,28 @@ iicclose(struct cdev *dev, int flags, int fmt, struct thread *td)
 static int
 iicwrite(struct cdev *dev, struct uio * uio, int ioflag)
 {
-	device_t iicdev = IIC_DEVICE(minor(dev));
-	struct iic_softc *sc = IIC_SOFTC(minor(dev));
+	struct iic_softc *sc = dev->si_drv1;
+	device_t iicdev = sc->sc_dev;
 	int sent, error, count;
 
-	if (!sc || !iicdev || !sc->sc_addr)
+	IIC_LOCK(sc);
+	if (!sc->sc_addr) {
+		IIC_UNLOCK(sc);
 		return (EINVAL);
+	}
 
-	if (sc->sc_count == 0)
+	if (sc->sc_count == 0) {
+		/* XXX: I don't think this can happen. */
+		IIC_UNLOCK(sc);
 		return (EINVAL);
+	}
 
-	if ((error = iicbus_request_bus(device_get_parent(iicdev), iicdev, IIC_DONTWAIT)))
+	error = iicbus_request_bus(device_get_parent(iicdev), iicdev,
+	    IIC_DONTWAIT);
+	if (error) {
+		IIC_UNLOCK(sc);
 		return (error);
+	}
 
 	count = min(uio->uio_resid, BUFSIZE);
 	uiomove(sc->sc_buffer, count, uio);
@@ -197,47 +223,63 @@ iicwrite(struct cdev *dev, struct uio * uio, int ioflag)
 					sc->sc_buffer, count, &sent);
 
 	iicbus_release_bus(device_get_parent(iicdev), iicdev);
+	IIC_UNLOCK(sc);
 
-	return(error);
+	return (error);
 }
 
 static int
 iicread(struct cdev *dev, struct uio * uio, int ioflag)
 {
-	device_t iicdev = IIC_DEVICE(minor(dev));
-	struct iic_softc *sc = IIC_SOFTC(minor(dev));
+	struct iic_softc *sc = dev->si_drv1;
+	device_t iicdev = sc->sc_dev;
 	int len, error = 0;
 	int bufsize;
 
-	if (!sc || !iicdev || !sc->sc_addr)
+	IIC_LOCK(sc);
+	if (!sc->sc_addr) {
+		IIC_UNLOCK(sc);
 		return (EINVAL);
+	}
 
-	if (sc->sc_count == 0)
+	if (sc->sc_count == 0) {
+		/* XXX: I don't think this can happen. */
+		IIC_UNLOCK(sc);
 		return (EINVAL);
+	}
 
-	if ((error = iicbus_request_bus(device_get_parent(iicdev), iicdev, IIC_DONTWAIT)))
+	error = iicbus_request_bus(device_get_parent(iicdev), iicdev,
+	    IIC_DONTWAIT);
+	if (error) {
+		IIC_UNLOCK(sc);
 		return (error);
+	}
 
 	/* max amount of data to read */
 	len = min(uio->uio_resid, BUFSIZE);
 
-	if ((error = iicbus_block_read(device_get_parent(iicdev), sc->sc_addr,
-					sc->sc_inbuf, len, &bufsize)))
+	error = iicbus_block_read(device_get_parent(iicdev), sc->sc_addr,
+	    sc->sc_inbuf, len, &bufsize);
+	if (error) {
+		IIC_UNLOCK(sc);
 		return (error);
+	}
 
 	if (bufsize > uio->uio_resid)
 		panic("%s: too much data read!", __func__);
 
 	iicbus_release_bus(device_get_parent(iicdev), iicdev);
 
-	return (uiomove(sc->sc_inbuf, bufsize, uio));
+	error = uiomove(sc->sc_inbuf, bufsize, uio);
+	IIC_UNLOCK(sc);
+	return (error);
 }
 
 static int
 iicioctl(struct cdev *dev, u_long cmd, caddr_t data, int flags, struct thread *td)
 {
-	device_t iicdev = IIC_DEVICE(minor(dev));
-	struct iic_softc *sc = IIC_SOFTC(minor(dev));
+	struct iic_softc *sc = dev->si_drv1;
+	device_t iicdev = sc->sc_dev;
 	device_t parent = device_get_parent(iicdev);
 	struct iiccmd *s = (struct iiccmd *)data;
 	struct iic_rdwr_data *d = (struct iic_rdwr_data *)data;
@@ -246,15 +288,13 @@ iicioctl(struct cdev *dev, u_long cmd, caddr_t data, int flags, struct thread *t
 	char *buf = NULL;
 	void **usrbufs = NULL;
 
-	if (!sc)
-		return (EINVAL);
-
 	if ((error = iicbus_request_bus(parent, iicdev,
 	    (flags & O_NONBLOCK) ? IIC_DONTWAIT : (IIC_WAIT | IIC_INTR))))
 		return (error);
 
 	switch (cmd) {
 	case I2CSTART:
+		IIC_LOCK(sc);
 		error = iicbus_start(parent, s->slave, 0);
 
 		/*
@@ -264,6 +304,7 @@ iicioctl(struct cdev *dev, u_long cmd, caddr_t data, int flags, struct thread *t
 		 */
 		if (!error)
 			sc->sc_addr = s->slave;
+		IIC_UNLOCK(sc);
 
 		break;
 
diff --git a/sys/dev/iicbus/iicbus.c b/sys/dev/iicbus/iicbus.c
index 8594b8d..de64874 100644
--- a/sys/dev/iicbus/iicbus.c
+++ b/sys/dev/iicbus/iicbus.c
@@ -34,8 +34,10 @@ __FBSDID("$FreeBSD$");
 #include <sys/param.h>
 #include <sys/systm.h>
 #include <sys/kernel.h>
+#include <sys/lock.h>
 #include <sys/malloc.h>
 #include <sys/module.h>
+#include <sys/mutex.h>
 #include <sys/bus.h> 
 
 #include <dev/iicbus/iiconf.h>
@@ -90,6 +92,7 @@ iicbus_attach(device_t dev)
 	struct iicbus_softc *sc = IICBUS_SOFTC(dev);
 
 	sc->dev = dev;
+	mtx_init(&sc->lock, "iicbus", NULL, MTX_DEF);
 	iicbus_reset(dev, IIC_FASTEST, 0, NULL);
 
 	/* device probing is meaningless since the bus is supposed to be
@@ -108,13 +111,8 @@ iicbus_attach(device_t dev)
 	}
 	printf("\n");
 #endif
-	/* Always attach the iicsmb children */
-	BUS_ADD_CHILD(dev, 0, "iicsmb", -1);
-	/* attach any known device */
-	BUS_ADD_CHILD(dev, 0, "iic", -1);
-	/* Attach the wired devices via hints */
+	bus_generic_probe(dev);
 	bus_enumerate_hinted_children(dev);
-	/* Now probe and attach them */
 	bus_generic_attach(dev);
         return (0);
 }
@@ -122,9 +120,11 @@ iicbus_attach(device_t dev)
 static int
 iicbus_detach(device_t dev)
 {
+	struct iicbus_softc *sc = IICBUS_SOFTC(dev);
 
 	iicbus_reset(dev, IIC_FASTEST, 0, NULL);
 	bus_generic_detach(dev);
+	mtx_destroy(&sc->lock);
 	return (0);
 }
   
@@ -265,6 +265,4 @@ driver_t iicbus_driver = {
 
 devclass_t iicbus_devclass;
 
-DRIVER_MODULE(iicbus, envctrl, iicbus_driver, iicbus_devclass, 0, 0);
-DRIVER_MODULE(iicbus, iicbb, iicbus_driver, iicbus_devclass, 0, 0);
 MODULE_VERSION(iicbus, IICBUS_MODVER);
diff --git a/sys/dev/iicbus/iicbus.h b/sys/dev/iicbus/iicbus.h
index 55f3b93..0698d5c 100644
--- a/sys/dev/iicbus/iicbus.h
+++ b/sys/dev/iicbus/iicbus.h
@@ -29,6 +29,9 @@
 #ifndef __IICBUS_H
 #define __IICBUS_H
 
+#include <sys/_lock.h>
+#include <sys/_mutex.h>
+
 #define IICBUS_IVAR(d) (struct iicbus_ivar *) device_get_ivars(d)
 #define IICBUS_SOFTC(d) (struct iicbus_softc *) device_get_softc(d)
 
@@ -38,6 +41,7 @@ struct iicbus_softc
 	device_t owner;		/* iicbus owner device structure */
 	u_char started;		/* address of the 'started' slave
 				 * 0 if no start condition succeeded */
+	struct mtx lock;
 };
 
 struct iicbus_ivar
@@ -50,15 +54,14 @@ enum {
 };
 
 #define IICBUS_ACCESSOR(A, B, T)					\
-__inline static int							\
-iicbus_get_ ## A(device_t dev, T *t)					\
-{									\
-	return BUS_READ_IVAR(device_get_parent(dev), dev,		\
-	    IICBUS_IVAR_ ## B, (uintptr_t *) t);			\
-}
+	__BUS_ACCESSOR(iicbus, A, IICBUS, B, T)
 	
 IICBUS_ACCESSOR(addr,		ADDR,		uint32_t)
 
+#define	IICBUS_LOCK(sc)			mtx_lock(&(sc)->lock)
+#define	IICBUS_UNLOCK(sc)      		mtx_unlock(&(sc)->lock)
+#define	IICBUS_ASSERT_LOCKED(sc)       	mtx_assert(&(sc)->lock, MA_OWNED)
+
 extern int iicbus_generic_intr(device_t dev, int event, char *buf);
 
 extern driver_t iicbus_driver;
diff --git a/sys/dev/iicbus/iiconf.c b/sys/dev/iicbus/iiconf.c
index 7401820..bf7f453 100644
--- a/sys/dev/iicbus/iiconf.c
+++ b/sys/dev/iicbus/iiconf.c
@@ -29,8 +29,10 @@ __FBSDID("$FreeBSD$");
 
 #include <sys/param.h>
 #include <sys/systm.h>
+#include <sys/lock.h>
 #include <sys/malloc.h>
 #include <sys/module.h>
+#include <sys/mutex.h>
 #include <sys/bus.h>
 
 #include <dev/iicbus/iiconf.h>
@@ -57,13 +59,14 @@ iicbus_poll(struct iicbus_softc *sc, int how)
 {
 	int error;
 
+	IICBUS_ASSERT_LOCKED(sc);
 	switch (how) {
-	case (IIC_WAIT | IIC_INTR):
-		error = tsleep(sc, IICPRI|PCATCH, "iicreq", 0);
+	case IIC_WAIT | IIC_INTR:
+		error = mtx_sleep(sc, &sc->lock, IICPRI|PCATCH, "iicreq", 0);
 		break;
 
-	case (IIC_WAIT | IIC_NOINTR):
-		error = tsleep(sc, IICPRI, "iicreq", 0);
+	case IIC_WAIT | IIC_NOINTR:
+		error = mtx_sleep(sc, &sc->lock, IICPRI, "iicreq", 0);
 		break;
 
 	default:
@@ -85,9 +88,10 @@ int
 iicbus_request_bus(device_t bus, device_t dev, int how)
 {
 	struct iicbus_softc *sc = (struct iicbus_softc *)device_get_softc(bus);
-	int s, error = 0;
+	int error = 0;
 
 	/* first, ask the underlying layers if the request is ok */
+	IICBUS_LOCK(sc);
 	do {
 		error = IICBUS_CALLBACK(device_get_parent(bus),
 						IIC_REQUEST_BUS, (caddr_t)&how);
@@ -96,15 +100,13 @@ iicbus_request_bus(device_t bus, device_t dev, int how)
 	} while (error == EWOULDBLOCK);
 
 	while (!error) {
-		s = splhigh();	
 		if (sc->owner && sc->owner != dev) {
-			splx(s);
 
 			error = iicbus_poll(sc, how);
 		} else {
 			sc->owner = dev;
 
-			splx(s);
+			IICBUS_UNLOCK(sc);
 			return (0);
 		}
 
@@ -113,6 +115,7 @@ iicbus_request_bus(device_t bus, device_t dev, int how)
 			IICBUS_CALLBACK(device_get_parent(bus), IIC_RELEASE_BUS,
 					(caddr_t)&how);
 	}
+	IICBUS_UNLOCK(sc);
 
 	return (error);
 }
@@ -126,7 +129,7 @@ int
 iicbus_release_bus(device_t bus, device_t dev)
 {
 	struct iicbus_softc *sc = (struct iicbus_softc *)device_get_softc(bus);
-	int s, error;
+	int error;
 
 	/* first, ask the underlying layers if the release is ok */
 	error = IICBUS_CALLBACK(device_get_parent(bus), IIC_RELEASE_BUS, NULL);
@@ -134,17 +137,18 @@ iicbus_release_bus(device_t bus, device_t dev)
 	if (error)
 		return (error);
 
-	s = splhigh();
+	IICBUS_LOCK(sc);
+
 	if (sc->owner != dev) {
-		splx(s);
+		IICBUS_UNLOCK(sc);
 		return (EACCES);
 	}
 
-	sc->owner = 0;
-	splx(s);
+	sc->owner = NULL;
 
 	/* wakeup waiting processes */
 	wakeup(sc);
+	IICBUS_UNLOCK(sc);
 
 	return (0);
 }
@@ -334,7 +338,7 @@ iicbus_block_read(device_t bus, u_char slave, char *buf, int len, int *read)
 }
 
 /*
- * iicbus_trasnfer()
+ * iicbus_transfer()
  *
  * Do an aribtrary number of transfers on the iicbus.  We pass these
  * raw requests to the bridge driver.  If the bridge driver supports
@@ -364,7 +368,7 @@ iicbus_transfer_gen(device_t dev, struct iic_msg *msgs, uint32_t nmsgs)
 
 	device_get_children(dev, &children, &nkid);
 	if (nkid != 1)
-		return EIO;
+		return (EIO);
 	bus = children[0];
 	free(children, M_TEMP);
 	for (i = 0, error = 0; i < nmsgs && error == 0; i++) {
diff --git a/sys/dev/iicbus/iicsmb.c b/sys/dev/iicbus/iicsmb.c
index a8a95ae..196735c 100644
--- a/sys/dev/iicbus/iicsmb.c
+++ b/sys/dev/iicbus/iicsmb.c
@@ -75,6 +75,7 @@ struct iicsmb_softc {
 	char low;			/* low byte received first */
 	char high;			/* high byte */
 
+	struct mtx lock;
 	device_t smbus;
 };
 
@@ -139,7 +140,9 @@ static driver_t iicsmb_driver = {
 static void
 iicsmb_identify(driver_t *driver, device_t parent)
 {
-	BUS_ADD_CHILD(parent, 0, "iicsmb", -1);
+
+	if (device_find_child(parent, "iicsmb", -1) == NULL)
+		BUS_ADD_CHILD(parent, 0, "iicsmb", -1);
 }
 
 static int
@@ -154,6 +157,8 @@ iicsmb_attach(device_t dev)
 {
 	struct iicsmb_softc *sc = (struct iicsmb_softc *)device_get_softc(dev);
 
+	mtx_init(&sc->lock, "iicsmb", NULL, MTX_DEF);
+
 	sc->smbus = device_add_child(dev, "smbus", -1);
 
 	/* probe and attach the smbus */
@@ -171,6 +176,7 @@ iicsmb_detach(device_t dev)
 	if (sc->smbus) {
 		device_delete_child(dev, sc->smbus);
 	}
+	mtx_destroy(&sc->lock);
 
 	return (0);
 }
@@ -185,6 +191,7 @@ iicsmb_intr(device_t dev, int event, char *buf)
 {
 	struct iicsmb_softc *sc = (struct iicsmb_softc *)device_get_softc(dev);
 
+	mtx_lock(&sc->lock);
 	switch (event) {
 	case INTR_GENERAL:
 	case INTR_START:
@@ -243,6 +250,7 @@ end:
 	default:
 		panic("%s: unknown event (%d)!", __func__, event);
 	}
+	mtx_unlock(&sc->lock);
 
 	return;
 }
@@ -258,16 +266,12 @@ iicsmb_callback(device_t dev, int index, void *data)
 	case SMB_REQUEST_BUS:
 		/* request underlying iicbus */
 		how = *(int *)data;
-		mtx_lock(&Giant);
 		error = iicbus_request_bus(parent, dev, how);
-		mtx_unlock(&Giant);
 		break;
 
 	case SMB_RELEASE_BUS:
 		/* release underlying iicbus */
-		mtx_lock(&Giant);
 		error = iicbus_release_bus(parent, dev);
-		mtx_unlock(&Giant);
 		break;
 
 	default:
@@ -283,7 +287,6 @@ iicsmb_quick(device_t dev, u_char slave, int how)
 	device_t parent = device_get_parent(dev);
 	int error;
 
-	mtx_lock(&Giant);
 	switch (how) {
 	case SMB_QWRITE:
 		error = iicbus_start(parent, slave & ~LSB, IICBUS_TIMEOUT);
@@ -300,7 +303,6 @@ iicsmb_quick(device_t dev, u_char slave, int how)
 
 	if (!error)
 		error = iicbus_stop(parent);
-	mtx_unlock(&Giant);
 		
 	return (error);
 }
@@ -311,7 +313,6 @@ iicsmb_sendb(device_t dev, u_char slave, char byte)
 	device_t parent = device_get_parent(dev);
 	int error, sent;
 
-	mtx_lock(&Giant);
 	error = iicbus_start(parent, slave & ~LSB, IICBUS_TIMEOUT);
 
 	if (!error) {
@@ -319,7 +320,6 @@ iicsmb_sendb(device_t dev, u_char slave, char byte)
 
 		iicbus_stop(parent);
 	}
-	mtx_unlock(&Giant);
 
 	return (error);
 }
@@ -330,7 +330,6 @@ iicsmb_recvb(device_t dev, u_char slave, char *byte)
 	device_t parent = device_get_parent(dev);
 	int error, read;
 
-	mtx_lock(&Giant);
 	error = iicbus_start(parent, slave | LSB, 0);
 
 	if (!error) {
@@ -338,7 +337,6 @@ iicsmb_recvb(device_t dev, u_char slave, char *byte)
 
 		iicbus_stop(parent);
 	}
-	mtx_unlock(&Giant);
 
 	return (error);
 }
@@ -349,7 +347,6 @@ iicsmb_writeb(device_t dev, u_char slave, char cmd, char byte)
 	device_t parent = device_get_parent(dev);
 	int error, sent;
 
-	mtx_lock(&Giant);
 	error = iicbus_start(parent, slave & ~LSB, 0);
 
 	if (!error) {
@@ -358,7 +355,6 @@ iicsmb_writeb(device_t dev, u_char slave, char cmd, char byte)
 
 		iicbus_stop(parent);
 	}
-	mtx_unlock(&Giant);
 
 	return (error);
 }
@@ -372,7 +368,6 @@ iicsmb_writew(device_t dev, u_char slave, char cmd, short word)
 	char low = (char)(word & 0xff);
 	char high = (char)((word & 0xff00) >> 8);
 
-	mtx_lock(&Giant);
 	error = iicbus_start(parent, slave & ~LSB, 0);
 
 	if (!error) {
@@ -382,7 +377,6 @@ iicsmb_writew(device_t dev, u_char slave, char cmd, short word)
 
 		iicbus_stop(parent);
 	}
-	mtx_unlock(&Giant);
 
 	return (error);
 }
@@ -393,11 +387,8 @@ iicsmb_readb(device_t dev, u_char slave, char cmd, char *byte)
 	device_t parent = device_get_parent(dev);
 	int error, sent, read;
 
-	mtx_lock(&Giant);
-	if ((error = iicbus_start(parent, slave & ~LSB, IICBUS_TIMEOUT))) {
-		mtx_unlock(&Giant);
+	if ((error = iicbus_start(parent, slave & ~LSB, IICBUS_TIMEOUT)))
 		return (error);
-	}
 
 	if ((error = iicbus_write(parent, &cmd, 1, &sent, IICBUS_TIMEOUT)))
 		goto error;
@@ -410,7 +401,6 @@ iicsmb_readb(device_t dev, u_char slave, char cmd, char *byte)
 
 error:
 	iicbus_stop(parent);
-	mtx_unlock(&Giant);
 	return (error);
 }
 
@@ -424,11 +414,8 @@ iicsmb_readw(device_t dev, u_char slave, char cmd, short *word)
 	int error, sent, read;
 	char buf[2];
 
-	mtx_lock(&Giant);
-	if ((error = iicbus_start(parent, slave & ~LSB, IICBUS_TIMEOUT))) {
-		mtx_unlock(&Giant);
+	if ((error = iicbus_start(parent, slave & ~LSB, IICBUS_TIMEOUT)))
 		return (error);
-	}
 
 	if ((error = iicbus_write(parent, &cmd, 1, &sent, IICBUS_TIMEOUT)))
 		goto error;
@@ -444,7 +431,6 @@ iicsmb_readw(device_t dev, u_char slave, char cmd, short *word)
 
 error:
 	iicbus_stop(parent);
-	mtx_unlock(&Giant);
 	return (error);
 }
 
@@ -455,11 +441,8 @@ iicsmb_pcall(device_t dev, u_char slave, char cmd, short sdata, short *rdata)
 	int error, sent, read;
 	char buf[2];
 
-	mtx_lock(&Giant);
-	if ((error = iicbus_start(parent, slave & ~LSB, IICBUS_TIMEOUT))) {
-		mtx_unlock(&Giant);
+	if ((error = iicbus_start(parent, slave & ~LSB, IICBUS_TIMEOUT)))
 		return (error);
-	}
 
 	if ((error = iicbus_write(parent, &cmd, 1, &sent, IICBUS_TIMEOUT)))
 		goto error;
@@ -482,7 +465,6 @@ iicsmb_pcall(device_t dev, u_char slave, char cmd, short sdata, short *rdata)
 
 error:
 	iicbus_stop(parent);
-	mtx_unlock(&Giant);
 	return (error);
 }
 
@@ -492,7 +474,6 @@ iicsmb_bwrite(device_t dev, u_char slave, char cmd, u_char count, char *buf)
 	device_t parent = device_get_parent(dev);
 	int error, sent;
 
-	mtx_lock(&Giant);
 	if ((error = iicbus_start(parent, slave & ~LSB, IICBUS_TIMEOUT)))
 		goto error;
 
@@ -506,7 +487,6 @@ iicsmb_bwrite(device_t dev, u_char slave, char cmd, u_char count, char *buf)
 		goto error;
 
 error:
-	mtx_unlock(&Giant);
 	return (error);
 }
 
@@ -516,11 +496,8 @@ iicsmb_bread(device_t dev, u_char slave, char cmd, u_char *count, char *buf)
 	device_t parent = device_get_parent(dev);
 	int error, sent, read;
 
-	mtx_lock(&Giant);
-	if ((error = iicbus_start(parent, slave & ~LSB, IICBUS_TIMEOUT))) {
-		mtx_unlock(&Giant);
+	if ((error = iicbus_start(parent, slave & ~LSB, IICBUS_TIMEOUT)))
 		return (error);
-	}
 
 	if ((error = iicbus_write(parent, &cmd, 1, &sent, IICBUS_TIMEOUT)))
 		goto error;
@@ -535,7 +512,6 @@ iicsmb_bread(device_t dev, u_char slave, char cmd, u_char *count, char *buf)
 
 error:
 	iicbus_stop(parent);
-	mtx_unlock(&Giant);
 	return (error);
 }