Hi,
not suggesting the diff below, or anything, as i'm not familiar w/that
device_lookup() refcounting, nor have time to find out about it right now
by opening subr_autoconf.c, but is it even necessary, at all?
to unref while erroring out like that? or acceptable, but w/only devices
like gpio which don't move around often? :)
-Artturi
diff --git a/sys/dev/gpio/gpio.c b/sys/dev/gpio/gpio.c
index 0aa7ed5c69b..71b83e44227 100644
--- a/sys/dev/gpio/gpio.c
+++ b/sys/dev/gpio/gpio.c
@@ -254,8 +254,10 @@ gpioopen(dev_t dev, int flag, int mode, struct proc *p)
if (sc == NULL)
return (ENXIO);
- if (sc->sc_opened)
+ if (sc->sc_opened) {
+ device_unref((struct device *)sc);
return (EBUSY);
+ }
sc->sc_opened = 1;
return (0);
@@ -272,6 +274,7 @@ gpioclose(dev_t dev, int flag, int mode, struct proc *p)
sc->sc_opened = 0;
+ device_unref((struct device *)sc);
return (0);
}
@@ -300,6 +303,7 @@ gpioioctl(dev_t dev, u_long cmd, caddr_t data, int flag,
struct proc *p)
struct gpio_pin_set *set;
struct device *dv;
int pin, value, flags, npins, found;
+ int rv = 0;
sc = (struct gpio_softc *)device_lookup(&gpio_cd, minor(dev));
if (sc == NULL)
@@ -324,47 +328,65 @@ gpioioctl(dev_t dev, u_long cmd, caddr_t data, int flag,
struct proc *p)
if (op->gp_name[0] != '\0') {
pin = gpio_pinbyname(sc, op->gp_name);
- if (pin == -1)
- return (EINVAL);
+ if (pin == -1) {
+ rv = EINVAL;
+ goto _fail_unref;
+ }
} else
pin = op->gp_pin;
- if (pin < 0 || pin >= sc->sc_npins)
- return (EINVAL);
+ if (pin < 0 || pin >= sc->sc_npins) {
+ rv = EINVAL;
+ goto _fail_unref;
+ }
if (!(sc->sc_pins[pin].pin_flags & GPIO_PIN_SET) &&
- securelevel > 0)
- return (EPERM);
+ securelevel > 0) {
+ rv = EPERM;
+ goto _fail_unref;
+ }
/* return read value */
op->gp_value = gpiobus_pin_read(gc, pin);
break;
case GPIOPINWRITE:
- if ((flag & FWRITE) == 0)
- return (EBADF);
+ if ((flag & FWRITE) == 0) {
+ rv = EBADF;
+ goto _fail_unref;
+ }
op = (struct gpio_pin_op *)data;
if (op->gp_name[0] != '\0') {
pin = gpio_pinbyname(sc, op->gp_name);
- if (pin == -1)
- return (EINVAL);
+ if (pin == -1) {
+ rv = EINVAL;
+ goto _fail_unref;
+ }
} else
pin = op->gp_pin;
- if (pin < 0 || pin >= sc->sc_npins)
- return (EINVAL);
+ if (pin < 0 || pin >= sc->sc_npins) {
+ rv = EINVAL;
+ goto _fail_unref;
+ }
- if (sc->sc_pins[pin].pin_mapped)
- return (EBUSY);
+ if (sc->sc_pins[pin].pin_mapped) {
+ rv = EBUSY;
+ goto _fail_unref;
+ }
if (!(sc->sc_pins[pin].pin_flags & GPIO_PIN_SET) &&
- securelevel > 0)
- return (EPERM);
+ securelevel > 0) {
+ rv = EPERM;
+ goto _fail_unref;
+ }
value = op->gp_value;
- if (value != GPIO_PIN_LOW && value != GPIO_PIN_HIGH)
- return (EINVAL);
+ if (value != GPIO_PIN_LOW && value != GPIO_PIN_HIGH) {
+ rv = EINVAL;
+ goto _fail_unref;
+ }
gpiobus_pin_write(gc, pin, value);
/* return old value */
@@ -373,27 +395,37 @@ gpioioctl(dev_t dev, u_long cmd, caddr_t data, int flag,
struct proc *p)
sc->sc_pins[pin].pin_state = value;
break;
case GPIOPINTOGGLE:
- if ((flag & FWRITE) == 0)
- return (EBADF);
+ if ((flag & FWRITE) == 0) {
+ rv = EBADF;
+ goto _fail_unref;
+ }
op = (struct gpio_pin_op *)data;
if (op->gp_name[0] != '\0') {
pin = gpio_pinbyname(sc, op->gp_name);
- if (pin == -1)
- return (EINVAL);
+ if (pin == -1) {
+ rv = EINVAL;
+ goto _fail_unref;
+ }
} else
pin = op->gp_pin;
- if (pin < 0 || pin >= sc->sc_npins)
- return (EINVAL);
+ if (pin < 0 || pin >= sc->sc_npins) {
+ rv = EINVAL;
+ goto _fail_unref;
+ }
- if (sc->sc_pins[pin].pin_mapped)
- return (EBUSY);
+ if (sc->sc_pins[pin].pin_mapped) {
+ rv = EBUSY;
+ goto _fail_unref;
+ }
if (!(sc->sc_pins[pin].pin_flags & GPIO_PIN_SET) &&
- securelevel > 0)
- return (EPERM);
+ securelevel > 0) {
+ rv = EPERM;
+ goto _fail_unref;
+ }
value = (sc->sc_pins[pin].pin_state == GPIO_PIN_LOW ?
GPIO_PIN_HIGH : GPIO_PIN_LOW);
@@ -404,8 +436,10 @@ gpioioctl(dev_t dev, u_long cmd, caddr_t data, int flag,
struct proc *p)
sc->sc_pins[pin].pin_state = value;
break;
case GPIOATTACH:
- if (securelevel > 0)
- return (EPERM);
+ if (securelevel > 0) {
+ rv = EPERM;
+ goto _fail_unref;
+ }
attach = (struct gpio_attach *)data;
bzero(&ga, sizeof(ga));
@@ -424,8 +458,10 @@ gpioioctl(dev_t dev, u_long cmd, caddr_t data, int flag,
struct proc *p)
}
break;
case GPIODETACH:
- if (securelevel > 0)
- return (EPERM);
+ if (securelevel > 0) {
+ rv = EPERM;
+ goto _fail_unref;
+ }
attach = (struct gpio_attach *)data;
LIST_FOREACH(gdev, &sc->sc_devs, sc_next) {
@@ -440,23 +476,32 @@ gpioioctl(dev_t dev, u_long cmd, caddr_t data, int flag,
struct proc *p)
}
break;
case GPIOPINSET:
- if (securelevel > 0)
- return (EPERM);
+ if (securelevel > 0) {
+ rv = EPERM;
+ goto _fail_unref;
+ }
set = (struct gpio_pin_set *)data;
if (set->gp_name[0] != '\0') {
pin = gpio_pinbyname(sc, set->gp_name);
- if (pin == -1)
- return (EINVAL);
+ if (pin == -1) {
+ rv = EINVAL;
+ goto _fail_unref;
+ }
} else
pin = set->gp_pin;
- if (pin < 0 || pin >= sc->sc_npins)
- return (EINVAL);
+ if (pin < 0 || pin >= sc->sc_npins) {
+ rv = EINVAL;
+ goto _fail_unref;
+ }
flags = set->gp_flags;
/* check that the controller supports all requested flags */
- if ((flags & sc->sc_pins[pin].pin_caps) != flags)
- return (ENODEV);
+ if ((flags & sc->sc_pins[pin].pin_caps) != flags) {
+ rv = ENODEV;
+ goto _fail_unref;
+ }
+
flags = set->gp_flags | GPIO_PIN_SET;
set->gp_caps = sc->sc_pins[pin].pin_caps;
@@ -488,24 +533,33 @@ gpioioctl(dev_t dev, u_long cmd, caddr_t data, int flag,
struct proc *p)
}
break;
case GPIOPINUNSET:
- if (securelevel > 0)
- return (EPERM);
+ if (securelevel > 0) {
+ rv = EPERM;
+ goto _fail_unref;
+ }
set = (struct gpio_pin_set *)data;
if (set->gp_name[0] != '\0') {
pin = gpio_pinbyname(sc, set->gp_name);
- if (pin == -1)
- return (EINVAL);
+ if (pin == -1) {
+ rv = EINVAL;
+ goto _fail_unref;
+ }
} else
pin = set->gp_pin;
- if (pin < 0 || pin >= sc->sc_npins)
- return (EINVAL);
- if (sc->sc_pins[pin].pin_mapped)
- return (EBUSY);
- if (!(sc->sc_pins[pin].pin_flags & GPIO_PIN_SET))
- return (EINVAL);
-
+ if (pin < 0 || pin >= sc->sc_npins){
+ rv = EINVAL;
+ goto _fail_unref;
+ }
+ if (sc->sc_pins[pin].pin_mapped) {
+ rv EBUSY;
+ goto _fail_unref;
+ }
+ if (!(sc->sc_pins[pin].pin_flags & GPIO_PIN_SET)) {
+ rv = EINVAL;
+ goto _fail_unref;
+ }
LIST_FOREACH(nm, &sc->sc_names, gp_next) {
if (nm->gp_pin == pin) {
LIST_REMOVE(nm, gp_next);
@@ -516,8 +570,10 @@ gpioioctl(dev_t dev, u_long cmd, caddr_t data, int flag,
struct proc *p)
sc->sc_pins[pin].pin_flags &= ~GPIO_PIN_SET;
break;
default:
- return (ENOTTY);
+ rv = ENOTTY;
+ break;
}
-
- return (0);
+_fail_unref:
+ device_unref((struct device *)sc);
+ return (rv);
}
