* 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
* 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
* [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 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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).