| Message ID | 1358963812-19947-2-git-send-email-florian.vaussard@epfl.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On 01/23/2013 06:56 PM, Florian Vaussard wrote: > Convert the on-board LED connected to the TWL4030 (LEDB) to use > pwm-leds. > > Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch> > --- > arch/arm/boot/dts/omap3-overo.dtsi | 9 +++++---- > 1 files changed, 5 insertions(+), 4 deletions(-) > > diff --git a/arch/arm/boot/dts/omap3-overo.dtsi b/arch/arm/boot/dts/omap3-overo.dtsi > index 89808ce..800be29 100644 > --- a/arch/arm/boot/dts/omap3-overo.dtsi > +++ b/arch/arm/boot/dts/omap3-overo.dtsi > @@ -14,12 +14,13 @@ > /include/ "omap3.dtsi" > > / { > - leds { > - compatible = "gpio-leds"; > + pwmleds { > + compatible = "pwm-leds"; > + > overo { > label = "overo:blue:COM"; > - gpios = <&twl_gpio 19 0>; > - linux,default-trigger = "mmc0"; You can keep the default trigger for the pwm-leds as well. The best way to test this is on top of linux-next which already have the leds-pwm DT bindings. > + pwms = <&twl_pwmled 1 7812500>; > + max-brightness = <127>; > }; > }; > }; >
Hi Peter, >> >> diff --git a/arch/arm/boot/dts/omap3-overo.dtsi b/arch/arm/boot/dts/omap3-overo.dtsi >> index 89808ce..800be29 100644 >> --- a/arch/arm/boot/dts/omap3-overo.dtsi >> +++ b/arch/arm/boot/dts/omap3-overo.dtsi >> @@ -14,12 +14,13 @@ >> /include/ "omap3.dtsi" >> >> / { >> - leds { >> - compatible = "gpio-leds"; >> + pwmleds { >> + compatible = "pwm-leds"; >> + >> overo { >> label = "overo:blue:COM"; >> - gpios = <&twl_gpio 19 0>; >> - linux,default-trigger = "mmc0"; > > You can keep the default trigger for the pwm-leds as well. > The best way to test this is on top of linux-next which already have the > leds-pwm DT bindings. > I did it at first, but the led API executes in atomic context, where the pwm-twl-led driver uses i2c communication. Setting a trigger will result in a kernel panic. I am working on a patch for pwm-twl-led to defer using a workqueue right now. Cheers, Florian
On 01/24/2013 04:42 PM, Florian Vaussard wrote: > I did it at first, but the led API executes in atomic context, where the > pwm-twl-led driver uses i2c communication. Setting a trigger will result in a > kernel panic. Now that you mentioned it, this might be true. > I am working on a patch for pwm-twl-led to defer using a workqueue right now. Great! The only thing I worry about is the latency we are going to get with the workqueue.
>> I did it at first, but the led API executes in atomic context, where the >> pwm-twl-led driver uses i2c communication. Setting a trigger will result in a >> kernel panic. > > Now that you mentioned it, this might be true. > [<c0013204>] (unwind_backtrace+0x0/0xec) from [<c00348ac>] (warn_slowpath_common+0x4c/0x64) [<c00348ac>] (warn_slowpath_common+0x4c/0x64) from [<c00348e0>] (warn_slowpath_null+0x1c/0x24) [<c00348e0>] (warn_slowpath_null+0x1c/0x24) from [<c054d384>] (__mutex_lock_slowpath+0x6c/0x26c) [<c054d384>] (__mutex_lock_slowpath+0x6c/0x26c) from [<c054d590>] (mutex_lock+0xc/0x20) [<c054d590>] (mutex_lock+0xc/0x20) from [<c02d740c>] (regmap_bulk_write+0x48/0x138) [<c02d740c>] (regmap_bulk_write+0x48/0x138) from [<c02de2c0>] (twl_i2c_write+0xa4/0xf0) [<c02de2c0>] (twl_i2c_write+0xa4/0xf0) from [<c0299e34>] (twl4030_pwmled_config+0x70/0x9c) [<c0299e34>] (twl4030_pwmled_config+0x70/0x9c) from [<c029875c>] (pwm_config+0x5c/0x6c) [<c029875c>] (pwm_config+0x5c/0x6c) from [<c039dc04>] (led_pwm_set+0x28/0x64) [<c039dc04>] (led_pwm_set+0x28/0x64) from [<c039e27c>] (led_heartbeat_function+0x10c/0x134) [<c039e27c>] (led_heartbeat_function+0x10c/0x134) from [<c004359c>] (call_timer_fn+0x90/0x178) [<c004359c>] (call_timer_fn+0x90/0x178) from [<c0043994>] (run_timer_softirq+0x250/0x2c8) [<c0043994>] (run_timer_softirq+0x250/0x2c8) from [<c003cf78>] (__do_softirq+0xf8/0x248) [<c003cf78>] (__do_softirq+0xf8/0x248) from [<c003d154>] (irq_exit+0x44/0x98) [<c003d154>] (irq_exit+0x44/0x98) from [<c000e338>] (handle_IRQ+0x68/0x8c) [<c000e338>] (handle_IRQ+0x68/0x8c) from [<c000870c>] (omap3_intc_handle_irq+0x58/0x70) [<c000870c>] (omap3_intc_handle_irq+0x58/0x70) from [<c054f8c0>] (__irq_svc+0x40/0x70) Exception stack(0xc077df60 to 0xc077dfa8) :-) >> I am working on a patch for pwm-twl-led to defer using a workqueue right now. > > Great! > The only thing I worry about is the latency we are going to get with the > workqueue. > If the latency becomes critical, we can create our own workqueue. Do we merge anyway this patchset, or do we wait until the trigger has been fixed? Florian
On 01/24/2013 05:50 PM, Florian Vaussard wrote: >>> I did it at first, but the led API executes in atomic context, where the >>> pwm-twl-led driver uses i2c communication. Setting a trigger will result in a >>> kernel panic. >> >> Now that you mentioned it, this might be true. >> > > [<c0013204>] (unwind_backtrace+0x0/0xec) from [<c00348ac>] > (warn_slowpath_common+0x4c/0x64) > [<c00348ac>] (warn_slowpath_common+0x4c/0x64) from [<c00348e0>] > (warn_slowpath_null+0x1c/0x24) > [<c00348e0>] (warn_slowpath_null+0x1c/0x24) from [<c054d384>] > (__mutex_lock_slowpath+0x6c/0x26c) > [<c054d384>] (__mutex_lock_slowpath+0x6c/0x26c) from [<c054d590>] > (mutex_lock+0xc/0x20) > [<c054d590>] (mutex_lock+0xc/0x20) from [<c02d740c>] > (regmap_bulk_write+0x48/0x138) > [<c02d740c>] (regmap_bulk_write+0x48/0x138) from [<c02de2c0>] > (twl_i2c_write+0xa4/0xf0) > [<c02de2c0>] (twl_i2c_write+0xa4/0xf0) from [<c0299e34>] > (twl4030_pwmled_config+0x70/0x9c) > [<c0299e34>] (twl4030_pwmled_config+0x70/0x9c) from [<c029875c>] > (pwm_config+0x5c/0x6c) > [<c029875c>] (pwm_config+0x5c/0x6c) from [<c039dc04>] (led_pwm_set+0x28/0x64) > [<c039dc04>] (led_pwm_set+0x28/0x64) from [<c039e27c>] > (led_heartbeat_function+0x10c/0x134) > [<c039e27c>] (led_heartbeat_function+0x10c/0x134) from [<c004359c>] > (call_timer_fn+0x90/0x178) > [<c004359c>] (call_timer_fn+0x90/0x178) from [<c0043994>] > (run_timer_softirq+0x250/0x2c8) > [<c0043994>] (run_timer_softirq+0x250/0x2c8) from [<c003cf78>] > (__do_softirq+0xf8/0x248) > [<c003cf78>] (__do_softirq+0xf8/0x248) from [<c003d154>] (irq_exit+0x44/0x98) > [<c003d154>] (irq_exit+0x44/0x98) from [<c000e338>] (handle_IRQ+0x68/0x8c) > [<c000e338>] (handle_IRQ+0x68/0x8c) from [<c000870c>] > (omap3_intc_handle_irq+0x58/0x70) > [<c000870c>] (omap3_intc_handle_irq+0x58/0x70) from [<c054f8c0>] > (__irq_svc+0x40/0x70) > Exception stack(0xc077df60 to 0xc077dfa8) > > :-) > >>> I am working on a patch for pwm-twl-led to defer using a workqueue right now. >> >> Great! >> The only thing I worry about is the latency we are going to get with the >> workqueue. >> > > If the latency becomes critical, we can create our own workqueue. Hrm, when we handled the led via gpio-leds it was also going through the same path at the end, via i2c to twl4030. I think the fix for this is going to be needed in the pwm core level. Just need to look at the gpio code to have similar handling of might_sleep interfaces. > Do we merge anyway this patchset, or do we wait until the trigger has been fixed? I think it can go and later when we have the fix for the slow path you can add the default trigger. Reviewed-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
Hi >> >>>> I am working on a patch for pwm-twl-led to defer using a workqueue right now. >>> >>> Great! >>> The only thing I worry about is the latency we are going to get with the >>> workqueue. >>> >> >> If the latency becomes critical, we can create our own workqueue. > > Hrm, when we handled the led via gpio-leds it was also going through the same > path at the end, via i2c to twl4030. > I think the fix for this is going to be needed in the pwm core level. Just > need to look at the gpio code to have similar handling of might_sleep interfaces. > You are right. But then the pwm core must provide a way to know if the pwm access function are callable from atomic context or not (the gpio framework provides gpio_cansleep()). This implies a good amount of changes to the pwm framework, and currently we are the only driver using non-atomic access. I will take a closer look to the complexity of this solution tomorrow. Florian
On 01/24/2013 10:14 PM, Florian Vaussard wrote: > You are right. But then the pwm core must provide a way to know if the pwm > access function are callable > from atomic context or not (the gpio framework provides gpio_cansleep()). > This implies a good amount of changes to the pwm framework, and currently we > are the only driver using non-atomic access. We have two drivers at the moment: pwm-twl and pwm-twl-led. However new out of SoC PWM drivers might come (for example for palmas). So it worth take a look at some generic implementation.
On 01/25/2013 09:29 AM, Peter Ujfalusi wrote: > On 01/24/2013 10:14 PM, Florian Vaussard wrote: >> You are right. But then the pwm core must provide a way to know if the pwm >> access function are callable >> from atomic context or not (the gpio framework provides gpio_cansleep()). >> This implies a good amount of changes to the pwm framework, and currently we >> are the only driver using non-atomic access. > > We have two drivers at the moment: pwm-twl and pwm-twl-led. However new out of > SoC PWM drivers might come (for example for palmas). So it worth take a look > at some generic implementation. OK. So I have the series. I need to add few more things but pwm-leds on BeagleBoard works fine when I put the default_trigger for the pmustat LED to be mmc0. It is blinking happily ;) I'll CC you with the patches when I send them.
>> >> We have two drivers at the moment: pwm-twl and pwm-twl-led. However new out of >> SoC PWM drivers might come (for example for palmas). So it worth take a look >> at some generic implementation. > > OK. So I have the series. I need to add few more things but pwm-leds on > BeagleBoard works fine when I put the default_trigger for the pmustat LED to > be mmc0. It is blinking happily ;) > I'll CC you with the patches when I send them. > I sent a patchset 2 hours ago with you in CC, you haven't received them? Cheers, Florian
On 01/25/2013 01:21 PM, Florian Vaussard wrote: >>> >>> We have two drivers at the moment: pwm-twl and pwm-twl-led. However new out of >>> SoC PWM drivers might come (for example for palmas). So it worth take a look >>> at some generic implementation. >> >> OK. So I have the series. I need to add few more things but pwm-leds on >> BeagleBoard works fine when I put the default_trigger for the pmustat LED to >> be mmc0. It is blinking happily ;) >> I'll CC you with the patches when I send them. >> > > I sent a patchset 2 hours ago with you in CC, you haven't received them? I have not noticed them. Going through them right now.
diff --git a/arch/arm/boot/dts/omap3-overo.dtsi b/arch/arm/boot/dts/omap3-overo.dtsi index 89808ce..800be29 100644 --- a/arch/arm/boot/dts/omap3-overo.dtsi +++ b/arch/arm/boot/dts/omap3-overo.dtsi @@ -14,12 +14,13 @@ /include/ "omap3.dtsi" / { - leds { - compatible = "gpio-leds"; + pwmleds { + compatible = "pwm-leds"; + overo { label = "overo:blue:COM"; - gpios = <&twl_gpio 19 0>; - linux,default-trigger = "mmc0"; + pwms = <&twl_pwmled 1 7812500>; + max-brightness = <127>; }; }; };
Convert the on-board LED connected to the TWL4030 (LEDB) to use pwm-leds. Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch> --- arch/arm/boot/dts/omap3-overo.dtsi | 9 +++++---- 1 files changed, 5 insertions(+), 4 deletions(-)