2016-03-08 11:47:26

by Venkat Reddy Talla

Subject: [PATCH 1/1] extcon: gpio: queue work only if debounce is NZ

Use queue_delayed_work only when debounce time is
required to read gpio state properly, read the gpio
state and set the extcon cable state in IRQ handler
context if gpio settling time is not needed.

Signed-off-by: Venkat Reddy Talla <[email protected]>
---
drivers/extcon/extcon-gpio.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/extcon/extcon-gpio.c b/drivers/extcon/extcon-gpio.c
index 279ff8f..1beb086 100644
--- a/drivers/extcon/extcon-gpio.c
+++ b/drivers/extcon/extcon-gpio.c
@@ -56,8 +56,12 @@ static irqreturn_t gpio_irq_handler(int irq, void *dev_id)
{
struct gpio_extcon_data *data = dev_id;

- queue_delayed_work(system_power_efficient_wq, &data->work,
+ if (data->debounce_jiffies)
+ queue_delayed_work(system_power_efficient_wq, &data->work,
data->debounce_jiffies);
+ else
+ gpio_extcon_work(&data->work.work);
+
return IRQ_HANDLED;
}

--
2.1.4

2016-03-08 15:00:39

by Guenter Roeck

Subject: Re: [PATCH 1/1] extcon: gpio: queue work only if debounce is NZ

On Tue, Mar 08, 2016 at 05:15:53PM +0530, Venkat Reddy Talla wrote:
> Use queue_delayed_work only when debounce time is
> required to read gpio state properly, read the gpio
> state and set the extcon cable state in IRQ handler
> context if gpio settling time is not needed.
>
> Signed-off-by: Venkat Reddy Talla <[email protected]>
> ---
> drivers/extcon/extcon-gpio.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/extcon/extcon-gpio.c b/drivers/extcon/extcon-gpio.c
> index 279ff8f..1beb086 100644
> --- a/drivers/extcon/extcon-gpio.c
> +++ b/drivers/extcon/extcon-gpio.c
> @@ -56,8 +56,12 @@ static irqreturn_t gpio_irq_handler(int irq, void *dev_id)
> {
> struct gpio_extcon_data *data = dev_id;
>
> - queue_delayed_work(system_power_efficient_wq, &data->work,
> + if (data->debounce_jiffies)
> + queue_delayed_work(system_power_efficient_wq, &data->work,
> data->debounce_jiffies);
> + else
> + gpio_extcon_work(&data->work.work);
> +

The interrupt is requested with devm_request_any_context_irq(), meaning
the handler may run in hard interrupt context. gpio_extcon_work() calls
gpiod_get_value_cansleep(), which may not be a good idea in hard interrupt
context.

Guenter

> return IRQ_HANDLED;
> }
>
> --
> 2.1.4
>
>