Re: [PATCH] iio-trig-gpio:Remove redundant gpio_request

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Michael,

I'm wondering about this one.  Are you sure all platforms 
have the correct gpio setup when doing the request_irq?
(those that need it obviously, basically the question is whether
all platforms default to having gpio's as inputs).

If the do there is certainly a lot of redundant code about
based on a quick bit of grepping. (loads of it in arm/mach-pxa
for starters).  In that particular case all gpio's are configured
as inputs, but I'm not sure other platforms are so well behaved.


Unless I'm completely wrong that means we have run into a
rather general issue.

Also, if this is fine, the right fix is probably to have the interrupt as the
platform data parameter rather than the gpio.  It makes the code more general
and moves a lot of these direction issues etc into the board specific areas
rather than here.

Also, CC Greg KH on any patches like this which are fixes.

Jonathan

> Remove redundant gpio_request.
> The GPIO used as trigger IRQ, is also requested as gpio, but actually never read.
> On some platforms a GPIO requested as IRQ can't be requested as GPIO as well.
> 
> From: Michael Hennerich <Michael.Hennerich@xxxxxxxxxx>
> 
> Index: drivers/staging/iio/trigger/iio-trig-gpio.c
> ===================================================================
> --- drivers/staging/iio/trigger/iio-trig-gpio.c (revision 8368)
> +++ drivers/staging/iio/trigger/iio-trig-gpio.c (working copy)
> @@ -94,18 +94,11 @@
>                          IIO_TRIGGER_NAME_LENGTH,
>                          "gpiotrig%d",
>                          pdata[i]);
> -               ret = gpio_request(trig_info->gpio, trig->name);
> -               if (ret)
> -                       goto error_free_name;
> 
> -               ret = gpio_direction_input(trig_info->gpio);
> -               if (ret)
> -                       goto error_release_gpio;
> -
>                 irq = gpio_to_irq(trig_info->gpio);
>                 if (irq < 0) {
>                         ret = irq;
> -                       goto error_release_gpio;
> +                       goto error_free_name;
>                 }
> 
>                 ret = request_irq(irq, iio_gpio_trigger_poll,
> @@ -113,7 +106,7 @@
>                                   trig->name,
>                                   trig);
>                 if (ret)
> -                       goto error_release_gpio;
> +                       goto error_free_name;
> 
>                 ret = iio_trigger_register(trig);
>                 if (ret)
> @@ -127,8 +120,6 @@
>  /* First clean up the partly allocated trigger */
>  error_release_irq:
>         free_irq(irq, trig);
> -error_release_gpio:
> -       gpio_free(trig_info->gpio);
>  error_free_name:
>         kfree(trig->name);
>  error_free_trig_info:
> @@ -143,7 +134,6 @@
>                                  alloc_list) {
>                 trig_info = trig->private_data;
>                 free_irq(gpio_to_irq(trig_info->gpio), trig);
> -               gpio_free(trig_info->gpio);
>                 kfree(trig->name);
>                 kfree(trig_info);
>                 iio_trigger_unregister(trig);
> @@ -166,7 +156,6 @@
>                 trig_info = trig->private_data;
>                 iio_trigger_unregister(trig);
>                 free_irq(gpio_to_irq(trig_info->gpio), trig);
> -               gpio_free(trig_info->gpio);
>                 kfree(trig->name);
>                 kfree(trig_info);
>                 iio_put_trigger(trig);
> 
> 
> 
> ------------------------------------------------------------------
> ********* Analog Devices GmbH              Open Platform Solutions
> **  *****
> **     ** Wilhelm-Wagenfeld-Strasse 6
> **  ***** D-80807 Munich
> ********* Germany
> Registergericht München HRB 40368,  Geschäftsführer: Thomas Wessel, William A. Martin, Margaret K. Seif
> 

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

[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux