Date: Sat, 1 Nov 2014 09:56:23 +0000 From: "Bjoern A. Zeeb" <bz@FreeBSD.org> To: Luiz Otavio O Souza <loos@FreeBSD.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r273917 - head/sys/dev/gpio Message-ID: <B3986980-D53E-4015-9EF1-4B317ADC6955@FreeBSD.org> In-Reply-To: <201410311915.s9VJFEDZ003525@svn.freebsd.org> References: <201410311915.s9VJFEDZ003525@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 31 Oct 2014, at 19:15 , Luiz Otavio O Souza <loos@FreeBSD.org> wrote: > Author: loos > Date: Fri Oct 31 19:15:14 2014 > New Revision: 273917 > URL: https://svnweb.freebsd.org/changeset/base/273917 >=20 > Log: > Fix the gpiobus locking by using a more sane model where it isn't = necessary > hold the gpiobus lock between the gpio calls. >=20 > gpiobus_acquire_lock() now accepts a third parameter which tells = gpiobus > what to do when the bus is already busy. >=20 > When GPIOBUS_WAIT wait is used, the calling thread will be put to = sleep > until the bus became free. >=20 > With GPIOBUS_DONTWAIT the calling thread will receive EWOULDBLOCK = right > away and then it can act upon. >=20 > 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 >=20 > 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 >=20 > Modified: head/sys/dev/gpio/gpiobus.c > = =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D > --- 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); > } >=20 > -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; >=20 > sc =3D device_get_softc(busdev); > GPIOBUS_ASSERT_UNLOCKED(sc); > GPIOBUS_LOCK(sc); > -} > - > -static void > -gpiobus_unlock_bus(device_t busdev) > -{ > - struct gpiobus_softc *sc; > - > - sc =3D device_get_softc(busdev); > - GPIOBUS_ASSERT_LOCKED(sc); > + if (sc->sc_owner !=3D NULL) { > + if (how =3D=3D GPIOBUS_DONTWAIT) { > + GPIOBUS_UNLOCK(sc); > + return (EWOULDBLOCK); > + } > + while (sc->sc_owner !=3D NULL) > + mtx_sleep(sc, &sc->sc_mtx, 0, "gpiobuswait", 0); > + } > + sc->sc_owner =3D child; > GPIOBUS_UNLOCK(sc); > -} >=20 > -static void > -gpiobus_acquire_bus(device_t busdev, device_t child) > -{ > - struct gpiobus_softc *sc; > - > - sc =3D device_get_softc(busdev); > - GPIOBUS_ASSERT_LOCKED(sc); > - > - if (sc->sc_owner) > - panic("gpiobus: cannot serialize the access to = device."); > - sc->sc_owner =3D child; > + return (0); > } >=20 > static void > @@ -365,14 +352,15 @@ gpiobus_release_bus(device_t busdev, dev > struct gpiobus_softc *sc; >=20 > sc =3D device_get_softc(busdev); > - GPIOBUS_ASSERT_LOCKED(sc); > - > - if (!sc->sc_owner) > + GPIOBUS_ASSERT_UNLOCKED(sc); > + GPIOBUS_LOCK(sc); > + if (sc->sc_owner =3D=3D NULL) > panic("gpiobus: releasing unowned bus."); > if (sc->sc_owner !=3D child) > panic("gpiobus: you don't own the bus. game over."); > - > sc->sc_owner =3D NULL; > + wakeup(sc); > + GPIOBUS_UNLOCK(sc); > } >=20 > static int > @@ -469,8 +457,6 @@ static device_method_t gpiobus_methods[] > DEVMETHOD(bus_hinted_child, gpiobus_hinted_child), >=20 > /* 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), >=20 > Modified: head/sys/dev/gpio/gpiobus_if.m > = =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D > --- 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; >=20 > # > -# 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; > }; >=20 > # >=20 > Modified: head/sys/dev/gpio/gpiobusvar.h > = =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D > --- 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) >=20 > +#define GPIOBUS_WAIT 1 > +#define GPIOBUS_DONTWAIT 2 > + > struct gpiobus_softc > { > struct mtx sc_mtx; /* bus mutex */ >=20 > Modified: head/sys/dev/gpio/gpioiic.c > = =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D > --- 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 =3D device_get_softc(dev); > - int error =3D 0; > + int error, how; >=20 > + how =3D GPIOBUS_DONTWAIT; > + if (data !=3D NULL && (int)*data =3D=3D IIC_WAIT) > + how =3D GPIOBUS_WAIT; > + error =3D 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 =3D 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 =3D EINVAL; >=20 > Modified: head/sys/dev/gpio/gpioled.c > = =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D > --- 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=20 > gpioled_control(void *priv, int onoff) > { > - struct gpioled_softc *sc =3D priv; > + int error; > + struct gpioled_softc *sc; > + > + sc =3D (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,=20 > - onoff ? GPIO_PIN_HIGH : GPIO_PIN_LOW); > + error =3D GPIOBUS_ACQUIRE_BUS(sc->sc_busdev, sc->sc_dev, > + GPIOBUS_DONTWAIT); > + if (error !=3D 0) { > + GPIOLED_UNLOCK(sc); > + return; > + } > + error =3D GPIOBUS_PIN_SETFLAGS(sc->sc_busdev, sc->sc_dev, > + GPIOLED_PIN, GPIO_PIN_OUTPUT); > + if (error =3D=3D 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); > } >=20 >=20 =97=20 Bjoern A. Zeeb "Come on. Learn, goddamn it.", WarGames, 1983
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?B3986980-D53E-4015-9EF1-4B317ADC6955>