* make ledtrig-gpio useable with GPIO drivers requiring threaded irqs
@ 2014-09-09 7:40 Lothar Waßmann
2014-09-09 7:40 ` [PATCH 1/2] leds: trigger: gpio: fix warning in gpio trigger for gpios whose accessor function may sleep Lothar Waßmann
2014-09-09 7:40 ` [PATCH 2/2] leds: trigger: gpio: make ledtrig-gpio useable with GPIO drivers requiring threaded irqs Lothar Waßmann
0 siblings, 2 replies; 7+ messages in thread
From: Lothar Waßmann @ 2014-09-09 7:40 UTC (permalink / raw)
To: linux-leds; +Cc: linux-kernel, Richard Purdie, Bryan Wu
These patches make it possible to use the ledtrig-gpio driver with
GPIO drivers that require threaded IRQs (like the PCA953x I2C driver).
It has been tested with a PCA9554 chip on an i.MX28 module.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] leds: trigger: gpio: fix warning in gpio trigger for gpios whose accessor function may sleep
2014-09-09 7:40 make ledtrig-gpio useable with GPIO drivers requiring threaded irqs Lothar Waßmann
@ 2014-09-09 7:40 ` Lothar Waßmann
2014-09-12 0:13 ` Bryan Wu
2014-09-09 7:40 ` [PATCH 2/2] leds: trigger: gpio: make ledtrig-gpio useable with GPIO drivers requiring threaded irqs Lothar Waßmann
1 sibling, 1 reply; 7+ messages in thread
From: Lothar Waßmann @ 2014-09-09 7:40 UTC (permalink / raw)
To: linux-leds; +Cc: linux-kernel, Richard Purdie, Bryan Wu, Lothar Waßmann
When using a GPIO driver whose accessor functions may sleep (e.g. an
I2C GPIO extender like PCA9554) the following warning is issued:
WARNING: CPU: 0 PID: 665 at drivers/gpio/gpiolib.c:2274 gpiod_get_raw_value+0x3c/0x48()
Modules linked in:
CPU: 0 PID: 665 Comm: kworker/0:2 Not tainted 3.16.0-karo+ #115
Workqueue: events gpio_trig_work
[<c00142cc>] (unwind_backtrace) from [<c00118f8>] (show_stack+0x10/0x14)
[<c00118f8>] (show_stack) from [<c001bf10>] (warn_slowpath_common+0x64/0x84)
[<c001bf10>] (warn_slowpath_common) from [<c001bf4c>] (warn_slowpath_null+0x1c/0x24)
[<c001bf4c>] (warn_slowpath_null) from [<c020a1b8>] (gpiod_get_raw_value+0x3c/0x48)
[<c020a1b8>] (gpiod_get_raw_value) from [<c02f68a0>] (gpio_trig_work+0x1c/0xb0)
[<c02f68a0>] (gpio_trig_work) from [<c0030c1c>] (process_one_work+0x144/0x38c)
[<c0030c1c>] (process_one_work) from [<c0030ef8>] (worker_thread+0x60/0x5cc)
[<c0030ef8>] (worker_thread) from [<c0036dd4>] (kthread+0xb4/0xd0)
[<c0036dd4>] (kthread) from [<c000f0f0>] (ret_from_fork+0x14/0x24)
---[ end trace cd51a1dad8b86c9c ]---
Fix this by using the _cansleep() variant of gpio_get_value().
Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
---
drivers/leds/trigger/ledtrig-gpio.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/leds/trigger/ledtrig-gpio.c b/drivers/leds/trigger/ledtrig-gpio.c
index 35812e3..c86c418 100644
--- a/drivers/leds/trigger/ledtrig-gpio.c
+++ b/drivers/leds/trigger/ledtrig-gpio.c
@@ -48,7 +48,7 @@ static void gpio_trig_work(struct work_struct *work)
if (!gpio_data->gpio)
return;
- tmp = gpio_get_value(gpio_data->gpio);
+ tmp = gpio_get_value_cansleep(gpio_data->gpio);
if (gpio_data->inverted)
tmp = !tmp;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] leds: trigger: gpio: make ledtrig-gpio useable with GPIO drivers requiring threaded irqs
2014-09-09 7:40 make ledtrig-gpio useable with GPIO drivers requiring threaded irqs Lothar Waßmann
2014-09-09 7:40 ` [PATCH 1/2] leds: trigger: gpio: fix warning in gpio trigger for gpios whose accessor function may sleep Lothar Waßmann
@ 2014-09-09 7:40 ` Lothar Waßmann
2014-09-12 0:32 ` Bryan Wu
1 sibling, 1 reply; 7+ messages in thread
From: Lothar Waßmann @ 2014-09-09 7:40 UTC (permalink / raw)
To: linux-leds; +Cc: linux-kernel, Richard Purdie, Bryan Wu, Lothar Waßmann
When trying to use the LED GPIO trigger with e.g. the PCA953x GPIO
driver, request_irq() fails with -EINVAL, because the GPIO driver
requires a nested interrupt handler.
Use request_any_context_irq() to be able to use any GPIO driver as LED
trigger.
Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
---
drivers/leds/trigger/ledtrig-gpio.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/leds/trigger/ledtrig-gpio.c b/drivers/leds/trigger/ledtrig-gpio.c
index c86c418..b4168a7 100644
--- a/drivers/leds/trigger/ledtrig-gpio.c
+++ b/drivers/leds/trigger/ledtrig-gpio.c
@@ -161,10 +161,10 @@ static ssize_t gpio_trig_gpio_store(struct device *dev,
return n;
}
- ret = request_irq(gpio_to_irq(gpio), gpio_trig_irq,
+ ret = request_any_context_irq(gpio_to_irq(gpio), gpio_trig_irq,
IRQF_SHARED | IRQF_TRIGGER_RISING
| IRQF_TRIGGER_FALLING, "ledtrig-gpio", led);
- if (ret) {
+ if (ret < 0) {
dev_err(dev, "request_irq failed with error %d\n", ret);
} else {
if (gpio_data->gpio != 0)
@@ -172,7 +172,7 @@ static ssize_t gpio_trig_gpio_store(struct device *dev,
gpio_data->gpio = gpio;
}
- return ret ? ret : n;
+ return ret < 0 ? ret : n;
}
static DEVICE_ATTR(gpio, 0644, gpio_trig_gpio_show, gpio_trig_gpio_store);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] leds: trigger: gpio: fix warning in gpio trigger for gpios whose accessor function may sleep
2014-09-09 7:40 ` [PATCH 1/2] leds: trigger: gpio: fix warning in gpio trigger for gpios whose accessor function may sleep Lothar Waßmann
@ 2014-09-12 0:13 ` Bryan Wu
0 siblings, 0 replies; 7+ messages in thread
From: Bryan Wu @ 2014-09-12 0:13 UTC (permalink / raw)
To: Lothar Waßmann; +Cc: Linux LED Subsystem, lkml, Richard Purdie
On Tue, Sep 9, 2014 at 12:40 AM, Lothar Waßmann <LW@karo-electronics.de> wrote:
> When using a GPIO driver whose accessor functions may sleep (e.g. an
> I2C GPIO extender like PCA9554) the following warning is issued:
> WARNING: CPU: 0 PID: 665 at drivers/gpio/gpiolib.c:2274 gpiod_get_raw_value+0x3c/0x48()
> Modules linked in:
> CPU: 0 PID: 665 Comm: kworker/0:2 Not tainted 3.16.0-karo+ #115
> Workqueue: events gpio_trig_work
> [<c00142cc>] (unwind_backtrace) from [<c00118f8>] (show_stack+0x10/0x14)
> [<c00118f8>] (show_stack) from [<c001bf10>] (warn_slowpath_common+0x64/0x84)
> [<c001bf10>] (warn_slowpath_common) from [<c001bf4c>] (warn_slowpath_null+0x1c/0x24)
> [<c001bf4c>] (warn_slowpath_null) from [<c020a1b8>] (gpiod_get_raw_value+0x3c/0x48)
> [<c020a1b8>] (gpiod_get_raw_value) from [<c02f68a0>] (gpio_trig_work+0x1c/0xb0)
> [<c02f68a0>] (gpio_trig_work) from [<c0030c1c>] (process_one_work+0x144/0x38c)
> [<c0030c1c>] (process_one_work) from [<c0030ef8>] (worker_thread+0x60/0x5cc)
> [<c0030ef8>] (worker_thread) from [<c0036dd4>] (kthread+0xb4/0xd0)
> [<c0036dd4>] (kthread) from [<c000f0f0>] (ret_from_fork+0x14/0x24)
> ---[ end trace cd51a1dad8b86c9c ]---
>
> Fix this by using the _cansleep() variant of gpio_get_value().
>
Good catch, I will merge this.
Thanks,
-Bryan
> Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
> ---
> drivers/leds/trigger/ledtrig-gpio.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/leds/trigger/ledtrig-gpio.c b/drivers/leds/trigger/ledtrig-gpio.c
> index 35812e3..c86c418 100644
> --- a/drivers/leds/trigger/ledtrig-gpio.c
> +++ b/drivers/leds/trigger/ledtrig-gpio.c
> @@ -48,7 +48,7 @@ static void gpio_trig_work(struct work_struct *work)
> if (!gpio_data->gpio)
> return;
>
> - tmp = gpio_get_value(gpio_data->gpio);
> + tmp = gpio_get_value_cansleep(gpio_data->gpio);
> if (gpio_data->inverted)
> tmp = !tmp;
>
> --
> 1.7.10.4
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] leds: trigger: gpio: make ledtrig-gpio useable with GPIO drivers requiring threaded irqs
2014-09-09 7:40 ` [PATCH 2/2] leds: trigger: gpio: make ledtrig-gpio useable with GPIO drivers requiring threaded irqs Lothar Waßmann
@ 2014-09-12 0:32 ` Bryan Wu
2014-09-12 7:09 ` Lothar Waßmann
0 siblings, 1 reply; 7+ messages in thread
From: Bryan Wu @ 2014-09-12 0:32 UTC (permalink / raw)
To: Lothar Waßmann; +Cc: Linux LED Subsystem, lkml, Richard Purdie
On Tue, Sep 9, 2014 at 12:40 AM, Lothar Waßmann <LW@karo-electronics.de> wrote:
> When trying to use the LED GPIO trigger with e.g. the PCA953x GPIO
> driver, request_irq() fails with -EINVAL, because the GPIO driver
> requires a nested interrupt handler.
>
> Use request_any_context_irq() to be able to use any GPIO driver as LED
> trigger.
>
Hmmm, what about use request_thread_irq() and put the gpio_trig_work()
in as the thread_func.
Felipe, can you take a look at this?
Also in the first patch:
Actually in gpio_trig_irq(), it said:
/* just schedule_work since gpio_get_value can sleep */
schedule_work(&gpio_data->work);
Then that means we need to call gpio_get_value_can_sleep() in the
gpio_trig_work() instead of gpio_get_value(), right?
Thanks,
-Bryan
> Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
> ---
> drivers/leds/trigger/ledtrig-gpio.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/leds/trigger/ledtrig-gpio.c b/drivers/leds/trigger/ledtrig-gpio.c
> index c86c418..b4168a7 100644
> --- a/drivers/leds/trigger/ledtrig-gpio.c
> +++ b/drivers/leds/trigger/ledtrig-gpio.c
> @@ -161,10 +161,10 @@ static ssize_t gpio_trig_gpio_store(struct device *dev,
> return n;
> }
>
> - ret = request_irq(gpio_to_irq(gpio), gpio_trig_irq,
> + ret = request_any_context_irq(gpio_to_irq(gpio), gpio_trig_irq,
> IRQF_SHARED | IRQF_TRIGGER_RISING
> | IRQF_TRIGGER_FALLING, "ledtrig-gpio", led);
> - if (ret) {
> + if (ret < 0) {
> dev_err(dev, "request_irq failed with error %d\n", ret);
> } else {
> if (gpio_data->gpio != 0)
> @@ -172,7 +172,7 @@ static ssize_t gpio_trig_gpio_store(struct device *dev,
> gpio_data->gpio = gpio;
> }
>
> - return ret ? ret : n;
> + return ret < 0 ? ret : n;
> }
> static DEVICE_ATTR(gpio, 0644, gpio_trig_gpio_show, gpio_trig_gpio_store);
>
> --
> 1.7.10.4
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] leds: trigger: gpio: make ledtrig-gpio useable with GPIO drivers requiring threaded irqs
2014-09-12 0:32 ` Bryan Wu
@ 2014-09-12 7:09 ` Lothar Waßmann
2014-09-12 18:57 ` Bryan Wu
0 siblings, 1 reply; 7+ messages in thread
From: Lothar Waßmann @ 2014-09-12 7:09 UTC (permalink / raw)
To: Bryan Wu; +Cc: Linux LED Subsystem, lkml, Richard Purdie
Hi,
Bryan Wu wrote:
> On Tue, Sep 9, 2014 at 12:40 AM, Lothar Waßmann <LW@karo-electronics.de> wrote:
> > When trying to use the LED GPIO trigger with e.g. the PCA953x GPIO
> > driver, request_irq() fails with -EINVAL, because the GPIO driver
> > requires a nested interrupt handler.
> >
> > Use request_any_context_irq() to be able to use any GPIO driver as LED
> > trigger.
> >
>
> Hmmm, what about use request_thread_irq() and put the gpio_trig_work()
> in as the thread_func.
>
> Felipe, can you take a look at this?
>
> Also in the first patch:
> Actually in gpio_trig_irq(), it said:
> /* just schedule_work since gpio_get_value can sleep */
> schedule_work(&gpio_data->work);
>
> Then that means we need to call gpio_get_value_can_sleep() in the
> gpio_trig_work() instead of gpio_get_value(), right?
>
That's exactly what my first patch does!
Lothar Waßmann
--
___________________________________________________________
Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
www.karo-electronics.de | info@karo-electronics.de
___________________________________________________________
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] leds: trigger: gpio: make ledtrig-gpio useable with GPIO drivers requiring threaded irqs
2014-09-12 7:09 ` Lothar Waßmann
@ 2014-09-12 18:57 ` Bryan Wu
0 siblings, 0 replies; 7+ messages in thread
From: Bryan Wu @ 2014-09-12 18:57 UTC (permalink / raw)
To: Lothar Waßmann; +Cc: Linux LED Subsystem, lkml, Richard Purdie
On Fri, Sep 12, 2014 at 12:09 AM, Lothar Waßmann <LW@karo-electronics.de> wrote:
> Hi,
>
> Bryan Wu wrote:
>> On Tue, Sep 9, 2014 at 12:40 AM, Lothar Waßmann <LW@karo-electronics.de> wrote:
>> > When trying to use the LED GPIO trigger with e.g. the PCA953x GPIO
>> > driver, request_irq() fails with -EINVAL, because the GPIO driver
>> > requires a nested interrupt handler.
>> >
>> > Use request_any_context_irq() to be able to use any GPIO driver as LED
>> > trigger.
>> >
>>
>> Hmmm, what about use request_thread_irq() and put the gpio_trig_work()
>> in as the thread_func.
>>
>> Felipe, can you take a look at this?
>>
>> Also in the first patch:
>> Actually in gpio_trig_irq(), it said:
>> /* just schedule_work since gpio_get_value can sleep */
>> schedule_work(&gpio_data->work);
>>
>> Then that means we need to call gpio_get_value_can_sleep() in the
>> gpio_trig_work() instead of gpio_get_value(), right?
>>
> That's exactly what my first patch does!
>
Yeah, exactly. I'm just curious about the comment here.
-Bryan
>
> Lothar Waßmann
> --
> ___________________________________________________________
>
> Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
> Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
> Geschäftsführer: Matthias Kaussen
> Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
>
> www.karo-electronics.de | info@karo-electronics.de
> ___________________________________________________________
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-09-12 18:57 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-09 7:40 make ledtrig-gpio useable with GPIO drivers requiring threaded irqs Lothar Waßmann
2014-09-09 7:40 ` [PATCH 1/2] leds: trigger: gpio: fix warning in gpio trigger for gpios whose accessor function may sleep Lothar Waßmann
2014-09-12 0:13 ` Bryan Wu
2014-09-09 7:40 ` [PATCH 2/2] leds: trigger: gpio: make ledtrig-gpio useable with GPIO drivers requiring threaded irqs Lothar Waßmann
2014-09-12 0:32 ` Bryan Wu
2014-09-12 7:09 ` Lothar Waßmann
2014-09-12 18:57 ` Bryan Wu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.