On 31 Oct 2014, at 19:15 , Luiz Otavio O Souza <l...@freebsd.org> wrote:
> 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. I guess it was this commit that broke things: /scratch/tmp/bz/head.svn/sys/dev/gpio/gpioled.c: In function 'gpioled_control': /scratch/tmp/bz/head.svn/sys/dev/gpio/gpioled.c:88: error: 'GPIOBUS_DONTWAIT' undeclared (first use in this function) /scratch/tmp/bz/head.svn/sys/dev/gpio/gpioled.c:88: error: (Each undeclared identifier is reported only once /scratch/tmp/bz/head.svn/sys/dev/gpio/gpioled.c:88: error: for each function it appears in.) --- gpioled.o --- *** [gpioled.o] Error code 1 bmake: stopped in /storage/head/obj/mips.mips/scratch/tmp/bz/head.svn/sys/AP121 --- buildkernel --- *** [buildkernel] Error code 1 > > 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); > } > > — Bjoern A. Zeeb "Come on. Learn, goddamn it.", WarGames, 1983 _______________________________________________ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
