Add locking to the various iicbus(4) bridge drivers:

- Just grab Giant in the ixp425_iic(4) driver since this driver uses
  a shared address/data register window pair to access the actual
  I2C registers.  None of the other ixp425 drivers lock access to these
  shared address/data registers yet and that would need to be done before
  this could use any meaningful locking.
- Add locking to the interrupt handler and 'iicbus_reset' methods of the
  at91_twi(4) driver.
- Add locking to the pcf(4) driver.  Other pcf(4) fixes include:
  - Don't needlessly zero the softc.
  - Use bus_foo rather than bus_space_foo and remove bus space tag and
    handle from softc.
- The lpbb(4) driver just grabs Giant for now.  This will be refined later
  when ppbus(4) is locked.
- As was done with smbus earlier, move the DRIVER_MODULE() lines to match
  the bus driver (either iicbus or iicbb) to the bridge driver into the
  bridge drivers.

Tested by:	sam (arm/ixp425)

5 files changed, 82 insertions, 34 deletions

diff --git a/sys/dev/pcf/envctrl.c b/sys/dev/pcf/envctrl.c
index 8c47958..7c1cb1b 100644
--- a/sys/dev/pcf/envctrl.c
+++ b/sys/dev/pcf/envctrl.c
@@ -37,13 +37,15 @@ __FBSDID("$FreeBSD$");
  */
 
 #include <sys/param.h>
-#include <sys/systm.h>
 #include <sys/bus.h>
 #include <sys/conf.h>
 #include <sys/kernel.h>
+#include <sys/lock.h>
 #include <sys/malloc.h>
 #include <sys/module.h>
+#include <sys/mutex.h>
 #include <sys/resource.h>
+#include <sys/systm.h>
 #include <sys/uio.h>
 
 #include <dev/ofw/ofw_bus.h>
@@ -107,7 +109,7 @@ envctrl_attach(device_t dev)
 	int rv = ENXIO;
 
 	sc = DEVTOSOFTC(dev);
-	bzero(sc, sizeof(struct pcf_softc));
+	mtx_init(&sc->pcf_lock, device_get_nameunit(dev), "pcf", MTX_DEF);
 
 	/* IO port is mandatory */
 	sc->res_ioport = bus_alloc_resource_any(dev, SYS_RES_MEMORY,
@@ -116,8 +118,6 @@ envctrl_attach(device_t dev)
 		device_printf(dev, "cannot reserve I/O port range\n");
 		goto error;
 	}
-	sc->bt_ioport = rman_get_bustag(sc->res_ioport);
-	sc->bh_ioport = rman_get_bushandle(sc->res_ioport);
 
 	sc->pcf_flags = device_get_flags(dev);
 
@@ -134,7 +134,7 @@ envctrl_attach(device_t dev)
 	pcf_rst_card(dev, IIC_FASTEST, PCF_DEFAULT_ADDR, NULL);
 
 	rv = bus_setup_intr(dev, sc->res_irq,
-			    INTR_TYPE_NET /* | INTR_ENTROPY */,
+			    INTR_TYPE_NET | INTR_MPSAFE /* | INTR_ENTROPY */,
 			    NULL, pcf_intr, sc, &sc->intr_cookie);
 	if (rv) {
 		device_printf(dev, "could not setup IRQ\n");
@@ -158,6 +158,7 @@ error:
 		bus_release_resource(dev, SYS_RES_MEMORY, sc->rid_ioport,
 				     sc->res_ioport);
 	}
+	mtx_destroy(&sc->pcf_lock);
 	return (rv);
 }
 
@@ -181,10 +182,12 @@ envctrl_detach(device_t dev)
 	}
 
 	bus_release_resource(dev, SYS_RES_MEMORY, sc->rid_ioport, sc->res_ioport);
+	mtx_destroy(&sc->pcf_lock);
 
 	return (0);
 }
 
 DRIVER_MODULE(envctrl, ebus, envctrl_driver, envctrl_devclass, 0, 0);
+DRIVER_MODULE(iicbus, envctrl, iicbus_driver, iicbus_devclass, 0, 0);
 MODULE_DEPEND(envctrl, iicbus, PCF_MINVER, PCF_PREFVER, PCF_MAXVER);
 MODULE_VERSION(envctrl, PCF_MODVER);
diff --git a/sys/dev/pcf/pcf.c b/sys/dev/pcf/pcf.c
index 0320958..e6c9967 100644
--- a/sys/dev/pcf/pcf.c
+++ b/sys/dev/pcf/pcf.c
@@ -29,10 +29,12 @@
 __FBSDID("$FreeBSD$");
 
 #include <sys/param.h>
-#include <sys/systm.h>
+#include <sys/bus.h>
+#include <sys/lock.h>
 #include <sys/kernel.h>
 #include <sys/module.h>
-#include <sys/bus.h>
+#include <sys/mutex.h>
+#include <sys/systm.h>
 
 #include <machine/bus.h>
 #include <machine/resource.h>
@@ -48,16 +50,18 @@ __FBSDID("$FreeBSD$");
 
 static int pcf_wait_byte(struct pcf_softc *pcf);
 static int pcf_noack(struct pcf_softc *pcf, int timeout);
+static void pcf_stop_locked(struct pcf_softc *pcf);
 
 /*
  * Polling mode for master operations wait for a new
- * byte incomming or outgoing
+ * byte incoming or outgoing
  */
-int
+static int
 pcf_wait_byte(struct pcf_softc *sc)
 {
 	int counter = TIMEOUT;
 
+	PCF_ASSERT_LOCKED(sc);
 	while (counter--) {
 
 		if ((pcf_get_S1(sc) & PIN) == 0)
@@ -71,11 +75,11 @@ pcf_wait_byte(struct pcf_softc *sc)
 	return (IIC_ETIMEOUT);
 }
 
-int
-pcf_stop(device_t dev)
+static void
+pcf_stop_locked(struct pcf_softc *sc)
 {
-	struct pcf_softc *sc = DEVTOSOFTC(dev);
 
+	PCF_ASSERT_LOCKED(sc);
 #ifdef PCFDEBUG
 	device_printf(dev, " >> stop\n");
 #endif
@@ -91,17 +95,15 @@ pcf_stop(device_t dev)
 
 		sc->pcf_started = 0;
 	}
-
-	return (0);
 }
 
-
-int
+static int
 pcf_noack(struct pcf_softc *sc, int timeout)
 {
 	int noack;
 	int k = timeout/10;
 
+	PCF_ASSERT_LOCKED(sc);
 	do {
 		noack = pcf_get_S1(sc) & LRB;
 		if (!noack)
@@ -118,6 +120,7 @@ pcf_repeated_start(device_t dev, u_char slave, int timeout)
 	struct pcf_softc *sc = DEVTOSOFTC(dev);
 	int error = 0;
 
+	PCF_LOCK(sc);
 #ifdef PCFDEBUG
 	device_printf(dev, " >> repeated start for slave %#x\n",
 		      (unsigned)slave);
@@ -142,10 +145,12 @@ pcf_repeated_start(device_t dev, u_char slave, int timeout)
 		goto error;
 	}
 
+	PCF_UNLOCK(sc);
 	return (0);
 
 error:
-	pcf_stop(dev);
+	pcf_stop_locked(sc);
+	PCF_UNLOCK(sc);
 	return (error);
 }
 
@@ -155,6 +160,7 @@ pcf_start(device_t dev, u_char slave, int timeout)
 	struct pcf_softc *sc = DEVTOSOFTC(dev);
 	int error = 0;
 
+	PCF_LOCK(sc);
 #ifdef PCFDEBUG
 	device_printf(dev, " >> start for slave %#x\n", (unsigned)slave);
 #endif
@@ -162,6 +168,7 @@ pcf_start(device_t dev, u_char slave, int timeout)
 #ifdef PCFDEBUG
 		printf("pcf: busy!\n");
 #endif
+		PCF_UNLOCK(sc);
 		return (IIC_EBUSBSY);
 	}
 
@@ -187,13 +194,30 @@ pcf_start(device_t dev, u_char slave, int timeout)
 		goto error;
 	}
 
+	PCF_UNLOCK(sc);
 	return (0);
 
 error:
-	pcf_stop(dev);
+	pcf_stop_locked(sc);
+	PCF_UNLOCK(sc);
 	return (error);
 }
 
+int
+pcf_stop(device_t dev)
+{
+	struct pcf_softc *sc = DEVTOSOFTC(dev);
+
+#ifdef PCFDEBUG
+	device_printf(dev, " >> stop\n");
+#endif
+	PCF_LOCK(sc);
+	pcf_stop_locked(sc);
+	PCF_UNLOCK(sc);
+
+	return (0);
+}
+
 void
 pcf_intr(void *arg)
 {
@@ -201,6 +225,7 @@ pcf_intr(void *arg)
 	char data, status, addr;
 	char error = 0;
 
+	PCF_LOCK(sc);
 	status = pcf_get_S1(sc);
 
 	if (status & PIN) {
@@ -288,6 +313,7 @@ pcf_intr(void *arg)
 		    }
 
 	} while ((pcf_get_S1(sc) & PIN) == 0);
+	PCF_UNLOCK(sc);
 
 	return;
 
@@ -296,6 +322,7 @@ error:
 	pcf_set_S1(sc, PIN|ESO|ENI|ACK);
 
 	sc->pcf_slave_mode = SLAVE_RECEIVER;
+	PCF_UNLOCK(sc);
 
 	return;
 }
@@ -305,6 +332,7 @@ pcf_rst_card(device_t dev, u_char speed, u_char addr, u_char *oldaddr)
 {
 	struct pcf_softc *sc = DEVTOSOFTC(dev);
 
+	PCF_LOCK(sc);
 	if (oldaddr)
 		*oldaddr = sc->pcf_addr;
 
@@ -343,6 +371,7 @@ pcf_rst_card(device_t dev, u_char speed, u_char addr, u_char *oldaddr)
 	pcf_set_S1(sc, PIN|ESO|ENI|ACK);
 
 	sc->pcf_slave_mode = SLAVE_RECEIVER;
+	PCF_UNLOCK(sc);
 
 	return (0);
 }
@@ -359,6 +388,7 @@ pcf_write(device_t dev, char *buf, int len, int *sent, int timeout /* us */)
 #endif
 
 	bytes = 0;
+	PCF_LOCK(sc);
 	while (len) {
 
 		pcf_set_S0(sc, *buf++);
@@ -379,6 +409,7 @@ pcf_write(device_t dev, char *buf, int len, int *sent, int timeout /* us */)
 
 error:
 	*sent = bytes;
+	PCF_UNLOCK(sc);
 
 #ifdef PCFDEBUG
 	device_printf(dev, " >> %d bytes written (%d)\n", bytes, error);
@@ -399,6 +430,7 @@ pcf_read(device_t dev, char *buf, int len, int *read, int last,
 	device_printf(dev, " << reading %d bytes\n", len);
 #endif
 
+	PCF_LOCK(sc);
 	/* trig the bus to get the first data byte in S0 */
 	if (len) {
 		if (len == 1 && last)
@@ -415,14 +447,14 @@ pcf_read(device_t dev, char *buf, int len, int *read, int last,
 
 		/* wait for trigged byte */
 		if ((error = pcf_wait_byte(sc))) {
-			pcf_stop(dev);
+			pcf_stop_locked(sc);
 			goto error;
 		}
 
 		if (len == 1 && last)
 			/* ok, last data byte already in S0, no I2C activity
 			 * on next pcf_get_S0() */
-			pcf_stop(dev);
+			pcf_stop_locked(sc);
 
 		else if (len == 2 && last)
 			/* next trigged byte with no ack */
@@ -437,6 +469,7 @@ pcf_read(device_t dev, char *buf, int len, int *read, int last,
 
 error:
 	*read = bytes;
+	PCF_UNLOCK(sc);
 
 #ifdef PCFDEBUG
 	device_printf(dev, " << %d bytes read (%d): %#x%s\n", bytes, error,
diff --git a/sys/dev/pcf/pcf_ebus.c b/sys/dev/pcf/pcf_ebus.c
index e53e6d4..ad0e36e 100644
--- a/sys/dev/pcf/pcf_ebus.c
+++ b/sys/dev/pcf/pcf_ebus.c
@@ -64,11 +64,13 @@ __FBSDID("$FreeBSD$");
  */
 
 #include <sys/param.h>
-#include <sys/systm.h>
 #include <sys/bus.h>
+#include <sys/lock.h>
 #include <sys/kernel.h>
 #include <sys/module.h>
+#include <sys/mutex.h>
 #include <sys/resource.h>
+#include <sys/systm.h>
 
 #include <dev/ofw/ofw_bus.h>
 #include <dev/ofw/openfirm.h>
@@ -142,7 +144,7 @@ pcf_ebus_attach(device_t dev)
 	uint64_t own_addr;
 
 	sc = DEVTOSOFTC(dev);
-	bzero(sc, sizeof(struct pcf_softc));
+	mtx_init(&sc->pcf_lock, device_get_nameunit(dev), "pcf", MTX_DEF);
 
 	/* get OFW node of the pcf */
 	if ((node = ofw_bus_get_node(dev)) <= 0) {
@@ -157,8 +159,6 @@ pcf_ebus_attach(device_t dev)
 		device_printf(dev, "cannot reserve I/O port range\n");
 		goto error;
 	}
-	sc->bt_ioport = rman_get_bustag(sc->res_ioport);
-	sc->bh_ioport = rman_get_bushandle(sc->res_ioport);
 
 	sc->pcf_flags = device_get_flags(dev);
 
@@ -219,6 +219,7 @@ error:
 		bus_release_resource(dev, SYS_RES_MEMORY, sc->rid_ioport,
 		    sc->res_ioport);
 	}
+	mtx_destroy(&sc->pcf_lock);
 	return (rv);
 }
 
@@ -245,6 +246,7 @@ pcf_ebus_detach(device_t dev)
 
 	bus_release_resource(dev, SYS_RES_MEMORY, sc->rid_ioport,
 	    sc->res_ioport);
+	mtx_destroy(&sc->pcf_lock);
 
 	return (0);
 }
diff --git a/sys/dev/pcf/pcf_isa.c b/sys/dev/pcf/pcf_isa.c
index 40064cc..3bb1ec6 100644
--- a/sys/dev/pcf/pcf_isa.c
+++ b/sys/dev/pcf/pcf_isa.c
@@ -36,11 +36,13 @@ __FBSDID("$FreeBSD$");
  */
 
 #include <sys/param.h>
-#include <sys/systm.h>
 #include <sys/bus.h>
+#include <sys/lock.h>
 #include <sys/kernel.h>
 #include <sys/module.h>
+#include <sys/mutex.h>
 #include <sys/resource.h>
+#include <sys/systm.h>
 
 #include <machine/bus.h>
 #include <machine/resource.h>
@@ -126,7 +128,7 @@ pcf_isa_attach(device_t dev)
 	int rv = ENXIO;
 
 	sc = DEVTOSOFTC(dev);
-	bzero(sc, sizeof(struct pcf_softc));
+	mtx_init(&sc->pcf_lock, device_get_nameunit(dev), "pcf", MTX_DEF);
 
 	/* IO port is mandatory */
 	sc->res_ioport = bus_alloc_resource_any(dev, SYS_RES_IOPORT,
@@ -135,8 +137,6 @@ pcf_isa_attach(device_t dev)
 		device_printf(dev, "cannot reserve I/O port range\n");
 		goto error;
 	}
-	sc->bt_ioport = rman_get_bustag(sc->res_ioport);
-	sc->bh_ioport = rman_get_bushandle(sc->res_ioport);
 
 	sc->pcf_flags = device_get_flags(dev);
 
@@ -179,6 +179,7 @@ error:
 		bus_release_resource(dev, SYS_RES_IOPORT, sc->rid_ioport,
 				     sc->res_ioport);
 	}
+	mtx_destroy(&sc->pcf_lock);
 	return (rv);
 }
 
@@ -202,6 +203,7 @@ pcf_isa_detach(device_t dev)
 	}
 
 	bus_release_resource(dev, SYS_RES_IOPORT, sc->rid_ioport, sc->res_ioport);
+	mtx_destroy(&sc->pcf_lock);
 
 	return (0);
 }
diff --git a/sys/dev/pcf/pcfvar.h b/sys/dev/pcf/pcfvar.h
index ed4d9d8..8e4d5d8 100644
--- a/sys/dev/pcf/pcfvar.h
+++ b/sys/dev/pcf/pcfvar.h
@@ -27,6 +27,9 @@
  * $FreeBSD$
  */
 
+#ifndef __PCFVAR_H__
+#define	__PCFVAR_H__
+
 #define IO_PCFSIZE	2
 
 #define TIMEOUT	9999					/* XXX */
@@ -63,19 +66,22 @@ struct pcf_softc {
 	int	pcf_slave_mode;		/* receiver or transmitter */
 	int	pcf_started;		/* 1 if start condition sent */
 
+	struct mtx pcf_lock;
 	device_t iicbus;		/* the corresponding iicbus */
 
 	/* Resource handling stuff. */
 	struct resource		*res_ioport;
 	int			rid_ioport;
-	bus_space_tag_t		bt_ioport;
-	bus_space_handle_t	bh_ioport;
 	struct resource		*res_irq;
 	int			rid_irq;
 	void			*intr_cookie;
 };
 #define DEVTOSOFTC(dev) ((struct pcf_softc *)device_get_softc(dev))
 
+#define	PCF_LOCK(sc)		mtx_lock(&(sc)->pcf_lock)
+#define	PCF_UNLOCK(sc)		mtx_unlock(&(sc)->pcf_lock)
+#define	PCF_ASSERT_LOCKED(sc)	mtx_assert(&(sc)->pcf_lock, MA_OWNED)
+
 /*
  * PCF8584 datasheet : when operate at 8 MHz or more, a minimun time of
  * 6 clocks cycles must be left between two consecutives access
@@ -92,7 +98,7 @@ static __inline void
 pcf_set_S0(struct pcf_softc *sc, int data)
 {
 
-	bus_space_write_1(sc->bt_ioport, sc->bh_ioport, 0, data);
+	bus_write_1(sc->res_ioport, 0, data);
 	pcf_nops();
 }
 
@@ -100,7 +106,7 @@ static __inline void
 pcf_set_S1(struct pcf_softc *sc, int data)
 {
 
-	bus_space_write_1(sc->bt_ioport, sc->bh_ioport, 1, data);
+	bus_write_1(sc->res_ioport, 1, data);
 	pcf_nops();
 }
 
@@ -109,7 +115,7 @@ pcf_get_S0(struct pcf_softc *sc)
 {
 	char data;
 
-	data = bus_space_read_1(sc->bt_ioport, sc->bh_ioport, 0);
+	data = bus_read_1(sc->res_ioport, 0);
 	pcf_nops();
 
 	return (data);
@@ -120,7 +126,7 @@ pcf_get_S1(struct pcf_softc *sc)
 {
 	char data;
 
-	data = bus_space_read_1(sc->bt_ioport, sc->bh_ioport, 1);
+	data = bus_read_1(sc->res_ioport, 1);
 	pcf_nops();
 
 	return (data);
@@ -138,3 +144,5 @@ extern driver_intr_t pcf_intr;
 #define PCF_MINVER	1
 #define PCF_MAXVER	1
 #define PCF_PREFVER	PCF_MODVER
+
+#endif /* !__PCFVAR_H__ */