svn commit: r273917 - head/sys/dev/gpio
Bjoern A. Zeeb
bz at FreeBSD.org
Sat Nov 1 09:56:43 UTC 2014
On 31 Oct 2014, at 19:15 , Luiz Otavio O Souza <loos at 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
More information about the svn-src-head
mailing list