Date: Mon, 07 Sep 2015 16:18:40 -0600 From: Ian Lepore <ian@freebsd.org> To: Luiz Otavio O Souza <loos@FreeBSD.org> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r287542 - head/sys/dev/rccgpio Message-ID: <1441664320.68284.76.camel@freebsd.org> In-Reply-To: <201509072159.t87LxBsw097287@repo.freebsd.org> References: <201509072159.t87LxBsw097287@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, 2015-09-07 at 21:59 +0000, Luiz Otavio O Souza wrote: > Author: loos > Date: Mon Sep 7 21:59:11 2015 > New Revision: 287542 > URL: https://svnweb.freebsd.org/changeset/base/287542 > > Log: > Fix off-by-one bugs. > > While here, only set the GPIO pin state for output pins. > It's not a good idea to forbid setting output state on an input pin, because it's a technique that's often used to preset the drive state of a pin before changing it to be an output pin. That way a pin that powers on to a default state of input-pulled-high can be set to drive high before making it output, and you avoid any transitions on the pin (which might be important if the pin is connected to, say, the power-control input of a PMIC). Most hardware allows setting the output-state register bits for a pin even if the pin is currently assigned as input. I guess if the hardware has no way to do that, then returning EINVAL might make sense. -- Ian > Pointy hat to: loos > Sponsored by: Rubicon Communications (Netgate) > > Modified: > head/sys/dev/rccgpio/rccgpio.c > > Modified: head/sys/dev/rccgpio/rccgpio.c > ============================================================================== > --- head/sys/dev/rccgpio/rccgpio.c Mon Sep 7 20:55:14 2015 (r287541) > +++ head/sys/dev/rccgpio/rccgpio.c Mon Sep 7 21:59:11 2015 (r287542) > @@ -126,7 +126,7 @@ rcc_gpio_pin_getcaps(device_t dev, uint3 > struct rcc_gpio_softc *sc; > > sc = device_get_softc(dev); > - if (pin > sc->sc_gpio_npins) > + if (pin >= sc->sc_gpio_npins) > return (EINVAL); > > *caps = rcc_pins[pin].caps; > @@ -140,7 +140,7 @@ rcc_gpio_pin_getflags(device_t dev, uint > struct rcc_gpio_softc *sc; > > sc = device_get_softc(dev); > - if (pin > sc->sc_gpio_npins) > + if (pin >= sc->sc_gpio_npins) > return (EINVAL); > > /* Flags cannot be changed. */ > @@ -155,7 +155,7 @@ rcc_gpio_pin_getname(device_t dev, uint3 > struct rcc_gpio_softc *sc; > > sc = device_get_softc(dev); > - if (pin > sc->sc_gpio_npins) > + if (pin >= sc->sc_gpio_npins) > return (EINVAL); > > memcpy(name, rcc_pins[pin].name, GPIOMAXNAME); > @@ -169,7 +169,7 @@ rcc_gpio_pin_setflags(device_t dev, uint > struct rcc_gpio_softc *sc; > > sc = device_get_softc(dev); > - if (pin > sc->sc_gpio_npins) > + if (pin >= sc->sc_gpio_npins) > return (EINVAL); > > /* Flags cannot be changed - risk of short-circuit!!! */ > @@ -183,7 +183,10 @@ rcc_gpio_pin_set(device_t dev, uint32_t > struct rcc_gpio_softc *sc; > > sc = device_get_softc(dev); > - if (pin > sc->sc_gpio_npins) > + if (pin >= sc->sc_gpio_npins) > + return (EINVAL); > + > + if ((rcc_pins[pin].caps & GPIO_PIN_OUTPUT) == 0) > return (EINVAL); > > RCC_GPIO_LOCK(sc); > @@ -204,7 +207,7 @@ rcc_gpio_pin_get(device_t dev, uint32_t > uint32_t value; > > sc = device_get_softc(dev); > - if (pin > sc->sc_gpio_npins) > + if (pin >= sc->sc_gpio_npins) > return (EINVAL); > > RCC_GPIO_LOCK(sc); > @@ -224,7 +227,10 @@ rcc_gpio_pin_toggle(device_t dev, uint32 > struct rcc_gpio_softc *sc; > > sc = device_get_softc(dev); > - if (pin > sc->sc_gpio_npins) > + if (pin >= sc->sc_gpio_npins) > + return (EINVAL); > + > + if ((rcc_pins[pin].caps & GPIO_PIN_OUTPUT) == 0) > return (EINVAL); > > RCC_GPIO_LOCK(sc); >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1441664320.68284.76.camel>