svn commit: r328311 - head/sys/dev/iicbus

[Ian Lepore]

Author: ian
Date: Wed Jan 24 03:09:41 2018
New Revision: 328311
URL: https://svnweb.freebsd.org/changeset/base/328311

Log:
  Follow changes in r328307 by using new IIC_RECURSIVE flag.
  
  The driver now ensures only one thread at a time is running in the API
  functions (clock_gettime() and clock_settime()) by specifically requesting
  ownership of the i2c bus without using IIC_RECURSIVE, then it does all IO
  using IIC_RECURSIVE so that each individual IO operation doesn't try to
  re-acquire the bus.
  
  The other IO done by the driver happens at attach or intr_config_hooks time,
  when there can't be multiple threads running with the same device instance.
  So, the IIC_RECURSIVE flag can be safely ORed into the wait flags for all IO
  done by the driver, because it's all either done in a single-threaded
  environment, or protected within a block bounded by explict
  iicbus_acquire_bus() and iicbus_release_bus() calls.

Modified:
  head/sys/dev/iicbus/isl12xx.c

Modified: head/sys/dev/iicbus/isl12xx.c
==============================================================================
--- head/sys/dev/iicbus/isl12xx.c	Wed Jan 24 00:52:25 2018	(r328310)
+++ head/sys/dev/iicbus/isl12xx.c	Wed Jan 24 03:09:41 2018	(r328311)
@@ -137,18 +137,27 @@ static struct ofw_compat_data compat_data[] = {
 };
 #endif
 
+/*
+ * When doing i2c IO, indicate that we need to wait for exclusive bus ownership,
+ * but that we should not wait if we already own the bus.  This lets us put
+ * iicbus_acquire_bus() calls with a non-recursive wait at the entry of our API
+ * functions to ensure that only one client at a time accesses the hardware for
+ * the entire series of operations it takes to read or write the clock.
+ */
+#define	WAITFLAGS	(IIC_WAIT | IIC_RECURSIVE)
+
 static inline int
 isl12xx_read1(struct isl12xx_softc *sc, uint8_t reg, uint8_t *data) 
 {
 
-	return (iicdev_readfrom(sc->dev, reg, data, 1, IIC_WAIT));
+	return (iicdev_readfrom(sc->dev, reg, data, 1, WAITFLAGS));
 }
 
 static inline int
 isl12xx_write1(struct isl12xx_softc *sc, uint8_t reg, uint8_t val) 
 {
 
-	return (iicdev_writeto(sc->dev, reg, &val, 1, IIC_WAIT));
+	return (iicdev_writeto(sc->dev, reg, &val, 1, WAITFLAGS));
 }
 
 static void
@@ -229,17 +238,23 @@ isl12xx_gettime(device_t dev, struct timespec *ts)
 	int err;
 	uint8_t hourmask, sreg;
 
-	/* If power failed, we can't provide valid time. */
-	if ((err = isl12xx_read1(sc, ISL12XX_SR_REG, &sreg)) != 0)
+	/*
+	 * Read the status and time registers.
+	 */
+	if ((err = iicbus_request_bus(sc->busdev, sc->dev, IIC_WAIT)) == 0) {
+		if ((err = isl12xx_read1(sc, ISL12XX_SR_REG, &sreg)) == 0) {
+			err = iicdev_readfrom(sc->dev, ISL12XX_SC_REG, &tregs,
+			    sizeof(tregs), WAITFLAGS);
+		}
+		iicbus_release_bus(sc->busdev, sc->dev);
+	}
+	if (err != 0)
 		return (err);
+
+	/* If power failed, we can't provide valid time. */
 	if (sreg & ISL12XX_SR_RTCF)
 		return (EINVAL);
 
-	/* Read the bcd time registers. */
-	if ((err = iicdev_readfrom(sc->dev, ISL12XX_SC_REG, &tregs, sizeof(tregs),
-	    IIC_WAIT)) != 0)
-		return (EINVAL);
-
 	/* If chip is in AM/PM mode remember that for when we set time. */
 	if (tregs.hour & ISL12XX_24HR_FLAG) {
 		hourmask = ISL12xx_24HR_MASK;
@@ -319,7 +334,7 @@ isl12xx_settime(device_t dev, struct timespec *ts)
 		sreg |= ISL12XX_SR_WRTC | ISL12XX_SR_W0C_BITS;
 		if ((err = isl12xx_write1(sc, ISL12XX_SR_REG, sreg)) == 0) {
 			err = iicdev_writeto(sc->dev, ISL12XX_SC_REG, &tregs,
-			    sizeof(tregs), IIC_WAIT);
+			    sizeof(tregs), WAITFLAGS);
 			sreg &= ~ISL12XX_SR_WRTC;
 			isl12xx_write1(sc, ISL12XX_SR_REG, sreg);
 		}