From owner-svn-src-all@FreeBSD.ORG Fri Oct 31 19:15:16 2014 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 30E5BFF2; Fri, 31 Oct 2014 19:15:16 +0000 (UTC) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:1900:2254:2068::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 1CBE2DF; Fri, 31 Oct 2014 19:15:16 +0000 (UTC) Received: from svn.freebsd.org ([127.0.1.70]) by svn.freebsd.org (8.14.9/8.14.9) with ESMTP id s9VJFFbX003535; Fri, 31 Oct 2014 19:15:15 GMT (envelope-from loos@FreeBSD.org) Received: (from loos@localhost) by svn.freebsd.org (8.14.9/8.14.9/Submit) id s9VJFEDZ003525; Fri, 31 Oct 2014 19:15:14 GMT (envelope-from loos@FreeBSD.org) Message-Id: <201410311915.s9VJFEDZ003525@svn.freebsd.org> X-Authentication-Warning: svn.freebsd.org: loos set sender to loos@FreeBSD.org using -f From: Luiz Otavio O Souza Date: Fri, 31 Oct 2014 19:15:14 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r273917 - head/sys/dev/gpio X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 31 Oct 2014 19:15:16 -0000 Author: loos Date: Fri Oct 31 19:15:14 2014 New Revision: 273917 URL: https://svnweb.freebsd.org/changeset/base/273917 Log: Fix the gpiobus locking by using a more sane model where it isn't necessary hold the gpiobus lock between the gpio calls. gpiobus_acquire_lock() now accepts a third parameter which tells gpiobus what to do when the bus is already busy. When GPIOBUS_WAIT wait is used, the calling thread will be put to sleep until the bus became free. With GPIOBUS_DONTWAIT the calling thread will receive EWOULDBLOCK right away and then it can act upon. This fixes the gpioiic(4) locking issues that arises when doing multiple concurrent access on the bus. Modified: head/sys/dev/gpio/gpiobus.c head/sys/dev/gpio/gpiobus_if.m head/sys/dev/gpio/gpiobusvar.h head/sys/dev/gpio/gpioiic.c head/sys/dev/gpio/gpioled.c Modified: head/sys/dev/gpio/gpiobus.c ============================================================================== --- head/sys/dev/gpio/gpiobus.c Fri Oct 31 18:53:16 2014 (r273916) +++ head/sys/dev/gpio/gpiobus.c Fri Oct 31 19:15:14 2014 (r273917) @@ -53,9 +53,7 @@ static void gpiobus_hinted_child(device_ /* * GPIOBUS interface */ -static void gpiobus_lock_bus(device_t); -static void gpiobus_unlock_bus(device_t); -static void gpiobus_acquire_bus(device_t, device_t); +static int gpiobus_acquire_bus(device_t, device_t, int); static void gpiobus_release_bus(device_t, device_t); static int gpiobus_pin_setflags(device_t, device_t, uint32_t, uint32_t); static int gpiobus_pin_getflags(device_t, device_t, uint32_t, uint32_t*); @@ -326,37 +324,26 @@ gpiobus_hinted_child(device_t bus, const device_delete_child(bus, child); } -static void -gpiobus_lock_bus(device_t busdev) +static int +gpiobus_acquire_bus(device_t busdev, device_t child, int how) { struct gpiobus_softc *sc; sc = device_get_softc(busdev); GPIOBUS_ASSERT_UNLOCKED(sc); GPIOBUS_LOCK(sc); -} - -static void -gpiobus_unlock_bus(device_t busdev) -{ - struct gpiobus_softc *sc; - - sc = device_get_softc(busdev); - GPIOBUS_ASSERT_LOCKED(sc); + if (sc->sc_owner != NULL) { + if (how == GPIOBUS_DONTWAIT) { + GPIOBUS_UNLOCK(sc); + return (EWOULDBLOCK); + } + while (sc->sc_owner != NULL) + mtx_sleep(sc, &sc->sc_mtx, 0, "gpiobuswait", 0); + } + sc->sc_owner = child; GPIOBUS_UNLOCK(sc); -} -static void -gpiobus_acquire_bus(device_t busdev, device_t child) -{ - struct gpiobus_softc *sc; - - sc = device_get_softc(busdev); - GPIOBUS_ASSERT_LOCKED(sc); - - if (sc->sc_owner) - panic("gpiobus: cannot serialize the access to device."); - sc->sc_owner = child; + return (0); } static void @@ -365,14 +352,15 @@ gpiobus_release_bus(device_t busdev, dev struct gpiobus_softc *sc; sc = device_get_softc(busdev); - GPIOBUS_ASSERT_LOCKED(sc); - - if (!sc->sc_owner) + GPIOBUS_ASSERT_UNLOCKED(sc); + GPIOBUS_LOCK(sc); + if (sc->sc_owner == NULL) panic("gpiobus: releasing unowned bus."); if (sc->sc_owner != child) panic("gpiobus: you don't own the bus. game over."); - sc->sc_owner = NULL; + wakeup(sc); + GPIOBUS_UNLOCK(sc); } static int @@ -469,8 +457,6 @@ static device_method_t gpiobus_methods[] DEVMETHOD(bus_hinted_child, gpiobus_hinted_child), /* GPIO protocol */ - DEVMETHOD(gpiobus_lock_bus, gpiobus_lock_bus), - DEVMETHOD(gpiobus_unlock_bus, gpiobus_unlock_bus), DEVMETHOD(gpiobus_acquire_bus, gpiobus_acquire_bus), DEVMETHOD(gpiobus_release_bus, gpiobus_release_bus), DEVMETHOD(gpiobus_pin_getflags, gpiobus_pin_getflags), Modified: head/sys/dev/gpio/gpiobus_if.m ============================================================================== --- head/sys/dev/gpio/gpiobus_if.m Fri Oct 31 18:53:16 2014 (r273916) +++ head/sys/dev/gpio/gpiobus_if.m Fri Oct 31 19:15:14 2014 (r273917) @@ -32,25 +32,12 @@ INTERFACE gpiobus; # -# Lock the gpio bus -# -METHOD void lock_bus { - device_t busdev; -}; - -# -# Unlock the gpio bus -# -METHOD void unlock_bus { - device_t busdev; -}; - -# # Dedicate the gpio bus control for a child # -METHOD void acquire_bus { +METHOD int acquire_bus { device_t busdev; device_t dev; + int how; }; # Modified: head/sys/dev/gpio/gpiobusvar.h ============================================================================== --- head/sys/dev/gpio/gpiobusvar.h Fri Oct 31 18:53:16 2014 (r273916) +++ head/sys/dev/gpio/gpiobusvar.h Fri Oct 31 19:15:14 2014 (r273917) @@ -56,6 +56,9 @@ #define GPIOBUS_ASSERT_LOCKED(_sc) mtx_assert(&_sc->sc_mtx, MA_OWNED) #define GPIOBUS_ASSERT_UNLOCKED(_sc) mtx_assert(&_sc->sc_mtx, MA_NOTOWNED) +#define GPIOBUS_WAIT 1 +#define GPIOBUS_DONTWAIT 2 + struct gpiobus_softc { struct mtx sc_mtx; /* bus mutex */ Modified: head/sys/dev/gpio/gpioiic.c ============================================================================== --- head/sys/dev/gpio/gpioiic.c Fri Oct 31 18:53:16 2014 (r273916) +++ head/sys/dev/gpio/gpioiic.c Fri Oct 31 19:15:14 2014 (r273917) @@ -154,18 +154,18 @@ static int gpioiic_callback(device_t dev, int index, caddr_t data) { struct gpioiic_softc *sc = device_get_softc(dev); - int error = 0; + int error, how; + how = GPIOBUS_DONTWAIT; + if (data != NULL && (int)*data == IIC_WAIT) + how = GPIOBUS_WAIT; + error = 0; switch (index) { case IIC_REQUEST_BUS: - GPIOBUS_LOCK_BUS(sc->sc_busdev); - GPIOBUS_ACQUIRE_BUS(sc->sc_busdev, sc->sc_dev); - GPIOBUS_UNLOCK_BUS(sc->sc_busdev); + error = GPIOBUS_ACQUIRE_BUS(sc->sc_busdev, sc->sc_dev, how); break; case IIC_RELEASE_BUS: - GPIOBUS_LOCK_BUS(sc->sc_busdev); GPIOBUS_RELEASE_BUS(sc->sc_busdev, sc->sc_dev); - GPIOBUS_UNLOCK_BUS(sc->sc_busdev); break; default: error = EINVAL; Modified: head/sys/dev/gpio/gpioled.c ============================================================================== --- head/sys/dev/gpio/gpioled.c Fri Oct 31 18:53:16 2014 (r273916) +++ head/sys/dev/gpio/gpioled.c Fri Oct 31 19:15:14 2014 (r273917) @@ -79,16 +79,23 @@ static int gpioled_detach(device_t); static void gpioled_control(void *priv, int onoff) { - struct gpioled_softc *sc = priv; + int error; + struct gpioled_softc *sc; + + sc = (struct gpioled_softc *)priv; GPIOLED_LOCK(sc); - GPIOBUS_LOCK_BUS(sc->sc_busdev); - GPIOBUS_ACQUIRE_BUS(sc->sc_busdev, sc->sc_dev); - GPIOBUS_PIN_SETFLAGS(sc->sc_busdev, sc->sc_dev, GPIOLED_PIN, - GPIO_PIN_OUTPUT); - GPIOBUS_PIN_SET(sc->sc_busdev, sc->sc_dev, GPIOLED_PIN, - onoff ? GPIO_PIN_HIGH : GPIO_PIN_LOW); + error = GPIOBUS_ACQUIRE_BUS(sc->sc_busdev, sc->sc_dev, + GPIOBUS_DONTWAIT); + if (error != 0) { + GPIOLED_UNLOCK(sc); + return; + } + error = GPIOBUS_PIN_SETFLAGS(sc->sc_busdev, sc->sc_dev, + GPIOLED_PIN, GPIO_PIN_OUTPUT); + if (error == 0) + GPIOBUS_PIN_SET(sc->sc_busdev, sc->sc_dev, GPIOLED_PIN, + onoff ? GPIO_PIN_HIGH : GPIO_PIN_LOW); GPIOBUS_RELEASE_BUS(sc->sc_busdev, sc->sc_dev); - GPIOBUS_UNLOCK_BUS(sc->sc_busdev); GPIOLED_UNLOCK(sc); }