Re: [RFC/PATCHv2 2/4] arm: omap: gpio: implement set_debounce method

[Grazvydas Ignotas <notasas@gmail.com>]

On Wed, Mar 31, 2010 at 6:35 PM, Felipe Balbi <felipe.balbi@nokia.com> wrote:
> OMAP support debouncing of gpio lines, implement
> the method using gpiolib.
>
> Signed-off-by: Felipe Balbi <felipe.balbi@nokia.com>
> ---
>  arch/arm/plat-omap/gpio.c |   68 +++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 68 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/plat-omap/gpio.c b/arch/arm/plat-omap/gpio.c
> index 76a347b..345ed2c 100644
> --- a/arch/arm/plat-omap/gpio.c
> +++ b/arch/arm/plat-omap/gpio.c
> @@ -612,6 +612,59 @@ do {       \
>        __raw_writel(l, base + reg); \
>  } while(0)
>
> +/**
> + * _set_gpio_debounce - low level gpio debounce time
> + * @bank: the gpio bank we're acting upon
> + * @gpio: the gpio number on this @gpio
> + * @debounce: debounce time to use
> + *
> + * OMAP's debounce time is in 31us steps so we need
> + * to convert and round up to the closest unit.
> + */
> +static void _set_gpio_debounce(struct gpio_bank *bank, unsigned gpio,
> +               unsigned debounce)
> +{
> +       void __iomem            *reg = bank->base;
> +       u32                     val;
> +       u32                     l;
> +
> +       if (debounce < 32)
> +               debounce = 0x01;
> +       else if (debounce > 7936)
> +               debounce = 0xff;
> +       else
> +               debounce = (debounce / 0x1f) - 1;
> +
> +       l = 1 << get_gpio_index(gpio);
> +
> +       if (cpu_is_omap44xx())
> +               reg += OMAP4_GPIO_DEBOUNCINGTIME;
> +       else
> +               reg += OMAP24XX_GPIO_DEBOUNCE_VAL;
> +
> +       __raw_writel(debounce, reg);
> +
> +       reg = bank->base;
> +       if (cpu_is_omap44xx())
> +               reg += OMAP4_GPIO_DEBOUNCENABLE;
> +       else
> +               reg += OMAP24XX_GPIO_DEBOUNCE_EN;
> +
> +       val = __raw_readl(reg);
> +
> +       if (debounce) {
> +               val |= l;
> +               if (cpu_is_omap34xx() || cpu_is_omap44xx())
> +                       clk_enable(bank->dbck);
> +       } else {
> +               val &= ~l;
> +               if (cpu_is_omap34xx() || cpu_is_omap44xx())
> +                       clk_disable(bank->dbck);
> +       }

Hmh, dbck is shared by the whole GPIO bank, so what happens if someone
calls _set_gpio_debounce(bank, 1, 310) and then
_set_gpio_debounce(bank, 2, 0)? This should leave debounce enabled for
GPIO1, but you'll disable dbck on second call. GPIOs 0-31 share the
same bank.

There is also an issue if somebody calls _set_gpio_debounce(bank, 1,
310) and _set_gpio_debounce(bank, 2, 620), the second call will
override debounce setting of GPIO1 (as it's shared by the whole bank).
This might be not what the user intended, would be useful to detect
this and warn the user.
--
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