lkml.org 
[lkml]   [2014]   [Sep]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRe: [PATCH 3/4 v4] GPIO: gpio-dwapb: Support Debounce
From
Date
On Tue, 2014-09-16 at 02:22 -0700, Weike Chen wrote:
> This patch enables 'debounce' for the designware GPIO, and
> it is based on Josef Ahmad's previous work.
>
> Reviewed-by: Hock Leong Kweh <hock.leong.kweh@intel.com>
> Reviewed-by: Shevchenko, Andriy <andriy.shevchenko@intel.com>

This line is wrong. How come?

Couple of minor comments below, and after addressing them, please, use
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> Signed-off-by: Weike Chen <alvin.chen@intel.com>
> ---
> drivers/gpio/gpio-dwapb.c | 28 ++++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
> diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c
> index 534945c..9a76e3c 100644
> --- a/drivers/gpio/gpio-dwapb.c
> +++ b/drivers/gpio/gpio-dwapb.c
> @@ -37,6 +37,7 @@
> #define GPIO_INTTYPE_LEVEL 0x38
> #define GPIO_INT_POLARITY 0x3c
> #define GPIO_INTSTATUS 0x40
> +#define GPIO_PORTA_DEBOUNCE 0x48
> #define GPIO_PORTA_EOI 0x4c
> #define GPIO_EXT_PORTA 0x50
> #define GPIO_EXT_PORTB 0x54
> @@ -235,6 +236,29 @@ static int dwapb_irq_set_type(struct irq_data *d, u32 type)
> return 0;
> }
>
> +static int dwapb_gpio_set_debounce(struct gpio_chip *gc,
> + unsigned offset, unsigned debounce)
> +{
> + struct bgpio_chip *bgc = to_bgpio_chip(gc);
> + struct dwapb_gpio_port *port =
> + container_of(bgc, struct dwapb_gpio_port, bgc);

Does it make sense to introduce an inline helper like

static inline struct dwapb_gpio_port *to_dwapb_gpio_port(struct
bgpio_chip *gc)
{...}

?

There is also another place where it could be used.

> + struct dwapb_gpio *gpio = port->gpio;
> + unsigned long flags, val_deb;
> + unsigned long mask = bgc->pin2mask(bgc, offset);
> +
> + spin_lock_irqsave(&bgc->lock, flags);
> +
> + val_deb = dwapb_read(gpio, GPIO_PORTA_DEBOUNCE);
> + if (debounce)
> + dwapb_write(gpio, GPIO_PORTA_DEBOUNCE, mask | val_deb);

May you put value on the first place? Like 'val_deb | mask'.

> + else
> + dwapb_write(gpio, GPIO_PORTA_DEBOUNCE, ~mask & val_deb);

Ditto.

> +

> + spin_unlock_irqrestore(&bgc->lock, flags);
> +
> + return 0;
> +}
> +
> static irqreturn_t dwapb_irq_handler_mfd(int irq, void *dev_id)
> {
> u32 worked;
> @@ -373,6 +397,10 @@ static int dwapb_gpio_add_port(struct dwapb_gpio *gpio,
> port->bgc.gc.ngpio = pp->ngpio;
> port->bgc.gc.base = pp->gpio_base;
>
> + /* Only port A support debounce */
> + if (pp->idx == 0)
> + port->bgc.gc.set_debounce = dwapb_gpio_set_debounce;
> +
> if (pp->irq)
> dwapb_configure_irqs(gpio, port, pp);
>


--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy


\
 
 \ /
  Last update: 2014-09-16 11:21    [W:0.291 / U:2.024 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site