Linus, The patch looks good, except for one issue (noted inline).
Reviewed-by: Frank Rowand <[email protected]> -Frank > -------- Original Message -------- > Subject: [PATCH] gpio: improve error path in gpiolib > Date: Fri, 30 Aug 2013 09:46:35 +0200 > From: Linus Walleij <[email protected]> > To: [email protected] > CC: Alexandre Courbot <[email protected]>, Linus Walleij > <[email protected]>, Frank Rowand <[email protected]>, > Tim Bird <[email protected]> > > At several places the gpiolib will proceed to handle a GPIO > descriptor even if it's ->chip member is NULL and no gpiochip > is associated. > > Fix this by checking that both the descriptor cookie *and* > the chip pointer are valid. > > Also bail out earlier with more specific diagnostic messages > on missing operations for setting as input/output or debounce. > > Suggested-by: Alexandre Courbot <[email protected]> > Cc: Frank Rowand <[email protected]> > Cc: Tim Bird <[email protected]> > Signed-off-by: Linus Walleij <[email protected]> > --- > drivers/gpio/gpiolib.c | 36 ++++++++++++++++++++++-------------- > 1 file changed, 22 insertions(+), 14 deletions(-) > > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c > index d6413b2..f041f9e 100644 > --- a/drivers/gpio/gpiolib.c > +++ b/drivers/gpio/gpiolib.c > @@ -1398,7 +1398,7 @@ static int gpiod_request(struct gpio_desc *desc, > const char *label) > int status = -EPROBE_DEFER; > unsigned long flags; > > - if (!desc) { > + if (!desc || !desc->chip) { > pr_warn("%s: invalid GPIO\n", __func__); > return -EINVAL; > } > @@ -1406,8 +1406,6 @@ static int gpiod_request(struct gpio_desc *desc, > const char *label) > spin_lock_irqsave(&gpio_lock, flags); > > chip = desc->chip; > - if (chip == NULL) > - goto done; > > if (!try_module_get(chip->owner)) > goto done; > @@ -1630,16 +1628,20 @@ static int gpiod_direction_input(struct > gpio_desc *desc) > int status = -EINVAL; > int offset; > > - if (!desc) { > + if (!desc || !desc->chip) { > pr_warn("%s: invalid GPIO\n", __func__); > return -EINVAL; > } > > + chip = desc->chip; > + if (!chip->get || !chip->direction_input) { > + pr_warn("%s: missing get() or direction_input() operations\n", > + __func__); > + return -EIO; > + } > + > spin_lock_irqsave(&gpio_lock, flags); > > - chip = desc->chip; > - if (!chip || !chip->get || !chip->direction_input) > - goto fail; > status = gpio_ensure_requested(desc); > if (status < 0) > goto fail; > @@ -1691,7 +1693,7 @@ static int gpiod_direction_output(struct gpio_desc > *desc, int value) > int status = -EINVAL; > int offset; > > - if (!desc) { > + if (!desc || !desc->chip) { > pr_warn("%s: invalid GPIO\n", __func__); > return -EINVAL; > } > @@ -1704,11 +1706,15 @@ static int gpiod_direction_output(struct > gpio_desc *desc, int value) > if (!value && test_bit(FLAG_OPEN_SOURCE, &desc->flags)) > return gpiod_direction_input(desc); > > + chip = desc->chip; > + if (!chip->set || !chip->direction_output) { > + pr_warn("%s: missing set() or direction_output() operations\n", > + __func__); > + return -EIO; > + } > + > spin_lock_irqsave(&gpio_lock, flags); > > - chip = desc->chip; > - if (!chip || !chip->set || !chip->direction_output) > - goto fail; > status = gpio_ensure_requested(desc); > if (status < 0) > goto fail; > @@ -1765,7 +1771,7 @@ static int gpiod_set_debounce(struct gpio_desc > *desc, unsigned debounce) > int status = -EINVAL; > int offset; > > - if (!desc) { > + if (!desc || !desc->chip) { > pr_warn("%s: invalid GPIO\n", __func__); > return -EINVAL; > } > @@ -1773,8 +1779,10 @@ static int gpiod_set_debounce(struct gpio_desc > *desc, unsigned debounce) > spin_lock_irqsave(&gpio_lock, flags); > > chip = desc->chip; > - if (!chip || !chip->set || !chip->set_debounce) > - goto fail; > + if (!chip->set || !chip->set_debounce) { > + pr_warn("%s: missing set() or set_debounce() operations\n", > + __func__); There should be a "return -EIO" here also. Otherwise the NULL chip->set_debounce() gets called a few lines below. > + } > > status = gpio_ensure_requested(desc); > if (status < 0) > -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
