Skip site navigation (1)Skip section navigation (2) 
Date:      Wed, 26 Jul 2017 21:06:27 +0000 (UTC)
From:      Ian Lepore <ian@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r321584 - head/sys/dev/iicbus
Message-ID:  <201707262106.v6QL6RaR087702@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: ian
Date: Wed Jul 26 21:06:26 2017
New Revision: 321584
URL: https://svnweb.freebsd.org/changeset/base/321584

Log:
  Add support for tracking nested calls to iicbus_request/release_bus().
  
  Usually it is sufficient to use iicbus_transfer_excl(), or one of the
  higher-level convenience functions that use it, to reserve the bus for the
  duration of each register access.  Occasionally it is important that a
  series of accesses or read-modify-write operations must be done without any
  other intervening access to the device, to prevent corrupting state.
  
  Without support for nested request/release, slave device drivers would have
  to stop using high-level convenience functions and resort to working with
  arrays of iic_msg structs just for a few operations (often involving
  one-time device setup or infrequent configuration changes).
  
  The changes here appear large from a glance at the diff, but in fact they're
  nearly trivial, and the large diff is because of changes in indentation and
  the re-wrapping of comments caused by that.  One notable change is that
  iicbus_release_bus() now ignores the IICBUS_CALLBACK(IIC_RELEASE_BUS) return
  value.  The old error handling left the bus in a kind of limbo state where
  it was still owned at the iicbus layer, but drivers rarely check the return
  of the release call, and it's unclear what they would do to recover from an
  error return anyway.  No existing low-level drivers return any kind of error
  from IIC_RELEASE_BUS except one EINVAL for "you don't own the bus", to which
  the right response is probably to carry on with the process of releasing the
  reference to the bus anyway.

Modified:
  head/sys/dev/iicbus/iicbus.h
  head/sys/dev/iicbus/iiconf.c

Modified: head/sys/dev/iicbus/iicbus.h
==============================================================================
--- head/sys/dev/iicbus/iicbus.h	Wed Jul 26 20:40:24 2017	(r321583)
+++ head/sys/dev/iicbus/iicbus.h	Wed Jul 26 21:06:26 2017	(r321584)
@@ -39,6 +39,7 @@ struct iicbus_softc
 {
 	device_t dev;		/* Myself */
 	device_t owner;		/* iicbus owner device structure */
+	u_int owncount;		/* iicbus ownership nesting count */
 	u_char started;		/* address of the 'started' slave
 				 * 0 if no start condition succeeded */
 	u_char strict;		/* deny operations that violate the

Modified: head/sys/dev/iicbus/iiconf.c
==============================================================================
--- head/sys/dev/iicbus/iiconf.c	Wed Jul 26 20:40:24 2017	(r321583)
+++ head/sys/dev/iicbus/iiconf.c	Wed Jul 26 21:06:26 2017	(r321584)
@@ -113,26 +113,30 @@ iicbus_request_bus(device_t bus, device_t dev, int how
 
 	IICBUS_LOCK(sc);
 
-	while ((error == 0) && (sc->owner != NULL))
+	while (error == 0 && sc->owner != NULL && sc->owner != dev)
 		error = iicbus_poll(sc, how);
 
 	if (error == 0) {
-		sc->owner = dev;
-		/* 
-		 * Drop the lock around the call to the bus driver. 
-		 * This call should be allowed to sleep in the IIC_WAIT case.
-		 * Drivers might also need to grab locks that would cause LOR
-		 * if our lock is held.
-		 */
-		IICBUS_UNLOCK(sc);
-		/* Ask the underlying layers if the request is ok */
-		error = IICBUS_CALLBACK(device_get_parent(bus),
-		    IIC_REQUEST_BUS, (caddr_t)&how);
-		IICBUS_LOCK(sc);
-
-		if (error != 0) {
-			sc->owner = NULL;
-			wakeup_one(sc);
+		++sc->owncount;
+		if (sc->owner == NULL) {
+			sc->owner = dev;
+			/* 
+			 * Drop the lock around the call to the bus driver, it
+			 * should be allowed to sleep in the IIC_WAIT case.
+			 * Drivers might also need to grab locks that would
+			 * cause a LOR if our lock is held.
+			 */
+			IICBUS_UNLOCK(sc);
+			/* Ask the underlying layers if the request is ok */
+			error = IICBUS_CALLBACK(device_get_parent(bus),
+			    IIC_REQUEST_BUS, (caddr_t)&how);
+			IICBUS_LOCK(sc);
+	
+			if (error != 0) {
+				sc->owner = NULL;
+				sc->owncount = 0;
+				wakeup_one(sc);
+			}
 		}
 	}
 
@@ -150,7 +154,6 @@ int
 iicbus_release_bus(device_t bus, device_t dev)
 {
 	struct iicbus_softc *sc = (struct iicbus_softc *)device_get_softc(bus);
-	int error;
 
 	IICBUS_LOCK(sc);
 
@@ -159,26 +162,16 @@ iicbus_release_bus(device_t bus, device_t dev)
 		return (IIC_EBUSBSY);
 	}
 
-	/* 
-	 * Drop the lock around the call to the bus driver. 
-	 * This call should be allowed to sleep in the IIC_WAIT case.
-	 * Drivers might also need to grab locks that would cause LOR
-	 * if our lock is held.
-	 */
-	IICBUS_UNLOCK(sc);
-	/* Ask the underlying layers if the release is ok */
-	error = IICBUS_CALLBACK(device_get_parent(bus), IIC_RELEASE_BUS, NULL);
-
-	if (error == 0) {
+	if (--sc->owncount == 0) {
+		/* Drop the lock while informing the low-level driver. */
+		IICBUS_UNLOCK(sc);
+		IICBUS_CALLBACK(device_get_parent(bus), IIC_RELEASE_BUS, NULL);
 		IICBUS_LOCK(sc);
 		sc->owner = NULL;
-
-		/* wakeup a waiting thread */
 		wakeup_one(sc);
-		IICBUS_UNLOCK(sc);
 	}
-
-	return (error);
+	IICBUS_UNLOCK(sc);
+	return (0);
 }
 
 /*

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201707262106.v6QL6RaR087702>