Re: [PATCH] gpiolib: request fixes

From: David Brownell
Date: Fri Dec 26 2008 - 11:05:12 EST


On Thursday 25 December 2008, Magnus Damm wrote:
> From: Magnus Damm <damm@xxxxxxxxxx>
>
> 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 <damm@xxxxxxxxxx>
> ---
>
> drivers/gpio/gpiolib.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> --- 0001/drivers/gpio/gpiolib.c
> +++ work/drivers/gpio/gpiolib.c 2008-12-26 13:09:50.000000000 +0900
> @@ -789,6 +789,7 @@ int gpio_request(unsigned gpio, const ch
> } else {
> status = -EBUSY;
> module_put(chip->owner);
> + goto done;

Right, good catch.


> }
>
> 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. :)


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


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/