svn commit: r287542 - head/sys/dev/rccgpio

[Ian Lepore]

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);
>