Re: [PATCH 11/9] move twl4030-gpio to drivers/gpio

[David Brownell <david-b@pacbell.net>]

On Saturday 27 September 2008, Felipe Balbi wrote:
> We need something like the patch below (I can't find a substitute
> to twl4030_set_gpio_edge_ctrl and twl4030_set_gpio_debounce):

Something like that, yes.  I have other patches queued up though.

And meanwhile, your build seems goofed ... fix that first.

For the moment I'm waiting until HSMMC works again in the OMAP
tree before I submit patches for any of those issues ... and
some of them imply updates from upstream.  Like these two:

  http://marc.info/?l=linux-kernel&m=122239480403730&w=2
  http://marc.info/?l=linux-kernel&m=122239472903349&w=2

For the "edge control" thing, I have a patch pushing that into
the standard IRQF_TRIGGER_{RISING,FALLING} and set_irq_type()
calls ... there was never a need for a twl-specific call here.

For debouncing, see this LKML RFC:

  http://marc.info/?l=linux-kernel&m=122228602206108&w=2

I have a twl4030 patch using that mechanism, and that patch
covers four processor families.  (OMAP2, OMAP3, AT91, AVR32.)
The OMAP1 MPUIO lines have debouncing too, but it's ganged
(all sixteen MPUIO lines get debounced, or none do).  I know
of a few more chips with built-in debouncing... so the RFC is
IMO more "are there blocking problems doing it this way" than
"should it be done at all".

- Dave



> ==== patch 1 (desired) ====
> 
> diff --git a/arch/arm/mach-omap2/hsmmc.c b/arch/arm/mach-omap2/hsmmc.c
> index 32b517b..3ca43aa 100644
> --- a/arch/arm/mach-omap2/hsmmc.c
> +++ b/arch/arm/mach-omap2/hsmmc.c
> @@ -16,6 +16,7 @@
>  #include <linux/interrupt.h>
>  #include <linux/delay.h>
>  #include <linux/i2c/twl4030.h>
> +#include <linux/gpio.h>
>  #include <mach/hardware.h>
>  #include <mach/mmc.h>
>  #include <mach/board.h>
> @@ -48,7 +49,7 @@
>  
>  static int hsmmc_card_detect(int irq)
>  {
> -       return twl4030_get_gpio_datain(irq - TWL4030_GPIO_IRQ_BASE);
> +       return gpio_get_value(irq - TWL4030_GPIO_IRQ_BASE);

That's a non-portable version of irq_to_gpio() ... and one
which should use gpio_get_value_cansleep().  My patches use
a simpler scheme, which among other things lets other GPIOs
be used for card detect.

(I understand that not all OMAP3 customers want to use twl4030
for their power management ... and in any case, having this kind
of needless dependency between drivers is a Bad Thing.)


>  }
>  
>  /*
> @@ -61,7 +62,7 @@ static int hsmmc_late_init(struct device *dev)
>         /*
>          * Configure TWL4030 GPIO parameters for MMC hotplug irq
>          */
> -       ret = twl4030_request_gpio(MMC1_CD_IRQ);
> +       ret = gpio_request(MMC1_CD_IRQ, "mmc_card_detect");

Another inappropriate dependency, removed in my patches.

Note also that the TWL call currently does stuff gpio_request()
can't ... so this would be incorrect right now.  (It's the right
direction obviously, but fixing that needs my other patches.)

>         if (ret)
>                 goto err;
>  
> @@ -88,11 +89,7 @@ err:
>  
>  static void hsmmc_cleanup(struct device *dev)
>  {
> -       int ret = 0;
> -
> -       ret = twl4030_free_gpio(MMC1_CD_IRQ);
> -       if (ret)
> -               dev_err(dev, "Failed to configure TWL4030 GPIO IRQ\n");
> +       gpio_free(MMC1_CD_IRQ);

(That's the same inappropriate dependency.)


>  }
>  
>  #ifdef CONFIG_PM
> 


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