Re: [PATCH] gpio:Fix incorrect variable assignment in the function __gpiod_request
From: Alexandre Courbot <hidden>
Date: 2015-08-18 07:34:23
Also in:
lkml
Your commit subject is missing a space after "gpio:" On Tue, Aug 18, 2015 at 2:07 AM, Nicholas Krause [off-list ref] wrote:
This fixes the incorrect variable assignment in the function __gpiod_request of not assigning the variable status to the return value of the function gpiod_get_direction before this function return's to it's respective caller due to the fact that the function gpiod_get_direction can fail and thus callers of the function __gpiod_request should be correctly returned the error by this call to correctly be alerted of failed calls to the function __gpiod_request.
That's a very long commit message for such a simple change. And all in one sentence! "Assign the return value of gpiod_get_direction() into the status variable to propagate errors" would have been both more concise and informative.
quoted hunk
Signed-off-by: Nicholas Krause <redacted> --- drivers/gpio/gpiolib.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index bf4bd1d..bd848df 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c@@ -821,7 +821,7 @@ static int __gpiod_request(struct gpio_desc *desc, const char *label) if (chip->get_direction) { /* chip->get_direction may sleep */ spin_unlock_irqrestore(&gpio_lock, flags); - gpiod_get_direction(desc); + status = gpiod_get_direction(desc);
This is incorrect. Look at the documentation for gpiod_get_direction():
Return GPIOF_DIR_IN or GPIOF_DIR_OUT, or an error code in
case of error.
So even in case of success, this function may return non-zero, which
will trigger error handling in some callers, e.g. gpiod_request():
if (status)
gpiod_dbg(desc, "%s: status %d\n", __func__, status);
Also, even though you return an error code if gpiod_get_direction()
failed, you omitted to clean the GPIO state (what is done if
chip->request() fails), making that GPIO unusuable forever.
Finally, should errors in gpiod_get_direction() be fatal? We call it
here only to update the kernel state of the GPIO, it is not really
fatal if it failed here.