Skip to contentSkip to conversation

gpiolib: request fixes

PATCH NAK Magnus Damm 2 participants 2 messages Dec 26, 2008 GPIO

lore
Share & tools
lore

Magnus Damm

author
#0 lore raw

PATCH 1/1 gpiolib: request fixes

From: Magnus Damm <[email protected]>

Fix request related issues in gpiolib such as:
- fix request-already-requested handling in gpio_request()
- clear FLAG_REQUESTED on request error in gpio_direction_input()
- clear FLAG_REQUESTED on request error in gpio_direction_output()

Signed-off-by Magnus Damm <[email protected]>

---

 drivers/gpio/gpiolib.c |    3 +++
 1 file changed, 3 insertions(+)
drivers/gpio/gpiolib.c +3 -0
@@ -789,6 +789,7 @@ int gpio_request(unsigned gpio, const ch
 	} else {
 		status = -EBUSY;
 		module_put(chip->owner);
+		goto done;
 	}
 
 	if (chip->request) {

@@ -924,6 +925,7 @@ int gpio_direction_input(unsigned gpio)
 			/* and it's not available to anyone else ...
 			 * gpio_request() is the fully clean solution.
 			 */
+			clear_bit(FLAG_REQUESTED, &desc->flags);
 			goto lose;
 		}
 	}

@@ -977,6 +979,7 @@ int gpio_direction_output(unsigned gpio,
 			/* and it's not available to anyone else ...
 			 * gpio_request() is the fully clean solution.
 			 */
+			clear_bit(FLAG_REQUESTED, &desc->flags);
 			goto lose;
 		}
 	}
NAK
#1 lore raw
Quoted 16 lines · Magnus Damm

From: Magnus Damm <[email protected]>

Fix request related issues in gpiolib such as:
- fix request-already-requested handling in gpio_request()
- clear FLAG_REQUESTED on request error in gpio_direction_input()
- clear FLAG_REQUESTED on request error in gpio_direction_output()

Signed-off-by Magnus Damm <[email protected]>

---

 drivers/gpio/gpiolib.c |    3 +++
 1 file changed, 3 insertions(+)

drivers/gpio/gpiolib.c int gpio_request(unsigned gpio, const ch · line 792

Quoted 4 lines
drivers/gpio/gpiolib.c +1 -0
@@ -789,6 +789,7 @@ int gpio_request(unsigned gpio, const ch
 	} else {
 		status = -EBUSY;
 		module_put(chip->owner);
+		goto done;

Right, good catch.

drivers/gpio/gpiolib.c int gpio_direction_input(unsigned gpio) · line 928

Quoted 8 lines
drivers/gpio/gpiolib.c +1 -0
 	}
 
 	if (chip->request) {

@@ -924,6 +925,7 @@ int gpio_direction_input(unsigned gpio)
 			/* and it's not available to anyone else ...
 			 * gpio_request() is the fully clean solution.
 			 */
+			clear_bit(FLAG_REQUESTED, &desc->flags);

NAK, this is insufficient ... it would need to drop the module refcount and null the label too. Plus this invalidates the comment. (Same below.)

However a basic premise is that drivers should now be avoiding this legacy autorequest stuff, using gpio_request() instead.

Hence the comments here, below, and at ensure_requested() to note the lack of cleanup if these legacy paths lose: small incentives to "do the right thing". I'd rather see the work go into making callers stop using autorequest; not making that mechanism work better.

It may be time to make ensure_requested() use WARN(), which will create a lot more noise than the current message ... a larger incentive. :)

Quoted 13 lines
drivers/gpio/gpiolib.c +1 -0
 			goto lose;
 		}
 	}

@@ -977,6 +979,7 @@ int gpio_direction_output(unsigned gpio,
 			/* and it's not available to anyone else ...
 			 * gpio_request() is the fully clean solution.
 			 */
+			clear_bit(FLAG_REQUESTED, &desc->flags);
 			goto lose;
 		}
 	}