From owner-svn-src-head@freebsd.org Wed Jul 26 21:06:28 2017 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 3914CDB04D4; Wed, 26 Jul 2017 21:06:28 +0000 (UTC) (envelope-from ian@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 0DA9276D7E; Wed, 26 Jul 2017 21:06:27 +0000 (UTC) (envelope-from ian@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id v6QL6RCt087704; Wed, 26 Jul 2017 21:06:27 GMT (envelope-from ian@FreeBSD.org) Received: (from ian@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id v6QL6RaR087702; Wed, 26 Jul 2017 21:06:27 GMT (envelope-from ian@FreeBSD.org) Message-Id: <201707262106.v6QL6RaR087702@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: ian set sender to ian@FreeBSD.org using -f From: Ian Lepore Date: Wed, 26 Jul 2017 21:06:27 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r321584 - head/sys/dev/iicbus X-SVN-Group: head X-SVN-Commit-Author: ian X-SVN-Commit-Paths: head/sys/dev/iicbus X-SVN-Commit-Revision: 321584 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 26 Jul 2017 21:06:28 -0000 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); } /*