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

[Ian Lepore]

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

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/nxprtc.c

Modified: head/sys/dev/iicbus/nxprtc.c
==============================================================================
--- head/sys/dev/iicbus/nxprtc.c	Wed Jan 24 03:09:41 2018	(r328311)
+++ head/sys/dev/iicbus/nxprtc.c	Wed Jan 24 03:09:56 2018	(r328312)
@@ -198,6 +198,15 @@ struct nxprtc_softc {
 #define	SC_F_CPOL	(1 << 0)	/* Century bit means 19xx */
 
 /*
+ * 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)
+
+/*
  * We use the compat_data table to look up hint strings in the non-FDT case, so
  * define the struct locally when we don't get it from ofw_bus_subr.h.
  */
@@ -230,14 +239,14 @@ static int
 read_reg(struct nxprtc_softc *sc, uint8_t reg, uint8_t *val)
 {
 
-	return (iicdev_readfrom(sc->dev, reg, val, sizeof(*val), IIC_WAIT));
+	return (iicdev_readfrom(sc->dev, reg, val, sizeof(*val), WAITFLAGS));
 }
 
 static int
 write_reg(struct nxprtc_softc *sc, uint8_t reg, uint8_t val)
 {
 
-	return (iicdev_writeto(sc->dev, reg, &val, sizeof(val), IIC_WAIT));
+	return (iicdev_writeto(sc->dev, reg, &val, sizeof(val), WAITFLAGS));
 }
 
 static int
@@ -264,7 +273,7 @@ read_timeregs(struct nxprtc_softc *sc, struct time_reg
 				continue;
 		}
 		if ((err = iicdev_readfrom(sc->dev, sc->secaddr, tregs,
-		    sizeof(*tregs), IIC_WAIT)) != 0)
+		    sizeof(*tregs), WAITFLAGS)) != 0)
 			break;
 	} while (sc->use_timer && tregs->sec != sec);
 
@@ -294,7 +303,7 @@ write_timeregs(struct nxprtc_softc *sc, struct time_re
 {
 
 	return (iicdev_writeto(sc->dev, sc->secaddr, tregs,
-	    sizeof(*tregs), IIC_WAIT));
+	    sizeof(*tregs), WAITFLAGS));
 }
 
 static int
@@ -557,14 +566,15 @@ nxprtc_gettime(device_t dev, struct timespec *ts)
 	 * bit is not set in the control reg.  The latter can happen if there
 	 * was an error when setting the time.
 	 */
-	if ((err = read_timeregs(sc, &tregs, &tmrcount)) != 0) {
-		device_printf(dev, "cannot read RTC time\n");
-		return (err);
+	if ((err = iicbus_request_bus(sc->busdev, sc->dev, IIC_WAIT)) == 0) {
+		if ((err = read_timeregs(sc, &tregs, &tmrcount)) == 0) {
+			err = read_reg(sc, PCF85xx_R_CS1, &cs1);
+		}
+		iicbus_release_bus(sc->busdev, sc->dev);
 	}
-	if ((err = read_reg(sc, PCF85xx_R_CS1, &cs1)) != 0) {
-		device_printf(dev, "cannot read RTC time\n");
+	if (err != 0)
 		return (err);
-	}
+
 	if ((tregs.sec & PCF85xx_B_SECOND_OS) || (cs1 & PCF85xx_B_CS1_STOP)) {
 		device_printf(dev, "RTC clock not running\n");
 		return (EINVAL); /* hardware is good, time is not. */