From owner-svn-src-all@freebsd.org Mon Sep 7 22:19:49 2015 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 993759CC30C for ; Mon, 7 Sep 2015 22:19:49 +0000 (UTC) (envelope-from ian@freebsd.org) Received: from outbound1b.ore.mailhop.org (outbound1b.ore.mailhop.org [54.200.247.200]) (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 7EDE61973 for ; Mon, 7 Sep 2015 22:19:48 +0000 (UTC) (envelope-from ian@freebsd.org) Received: from ilsoft.org (unknown [73.34.117.227]) by outbound1.ore.mailhop.org (Halon Mail Gateway) with ESMTPSA; Mon, 7 Sep 2015 22:18:59 +0000 (UTC) Received: from rev (rev [172.22.42.240]) by ilsoft.org (8.14.9/8.14.9) with ESMTP id t87MIeun056079; Mon, 7 Sep 2015 16:18:40 -0600 (MDT) (envelope-from ian@freebsd.org) Message-ID: <1441664320.68284.76.camel@freebsd.org> Subject: Re: svn commit: r287542 - head/sys/dev/rccgpio From: Ian Lepore To: Luiz Otavio O Souza Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Date: Mon, 07 Sep 2015 16:18:40 -0600 In-Reply-To: <201509072159.t87LxBsw097287@repo.freebsd.org> References: <201509072159.t87LxBsw097287@repo.freebsd.org> Content-Type: text/plain; charset="us-ascii" X-Mailer: Evolution 3.12.10 FreeBSD GNOME Team Port Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.20 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: Mon, 07 Sep 2015 22:19:49 -0000 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); >