On Wed, Nov 26, 2014 at 06:12:42PM +0100, Uwe Kleine-König wrote: > Hello, > > two years ago I addressed the issue that on i.MX28 a led connected to a > pwm-mxs device doesn't go off when brightness is set to 0. I tried again > later to find an acceptable solution. > (Links to respective threads: > http://thread.gmane.org/gmane.linux.ports.arm.kernel/282593/focus=282596) > http://thread.gmane.org/gmane.linux.kernel/1381289 > ) > > The technical side is well understood: The problem is that leds-pwm does > the following: > > pwm_config(led_dat->pwm, new_duty, led_dat->period); > > if (new_duty == 0) > pwm_disable(led_dat->pwm); > else > pwm_enable(led_dat->pwm); > > On i.MX28 reconfiguring the duty cycle only takes effect after the > current period is over. That means that if the previous configuration > was duty=period (i.e. "on") the likely outcome of the above sequence > with new_duty=0 is that setting the duty cycle to 0 is scheduled but the > current period never ends because the clock is stopped. So the led keeps > being on. > > Now we need a settlement in which way this should be fixed. > There are several possibilities, roughly in order of my preference: > a) Declare the pin state after pwm_disable as undefined > In this case the leds-pwm driver must not call pwm_disable when > expecting a certain brightness (here: off). > A nice follow-up is then to patch the drivers that work the way the > leds-pwm author expected to call their equivalent of pwm_disable > themselves on new_duty=0. > b) Let pwm_config return at once, implement the waiting in a new call > pwm_sync() that either busy-loops or sleeps until the configuration > is > c) Make the call to pwm_config for pwm-mxs block until the new period > started. > d) Make the call to pwm_config for pwm-mxs sleep until the new period > started. > (Not sure this works nicely because IIRC there is no irq available > that triggers accordingly.) > e) In pwm_disable detect that there is a reconfiguration pending and > block accordingly. > f) Extend the PWM-API to differentiate the two variants of pwm_config > E.g. make pwm_config sleep/busy-looping until the requested config is > active plus a new function pwm_config_schedule. > > Back then Sascha and Matt also prefered a). The obvious downside of a) > is that most people expect pwm_disable implying a constant 0. The > obvious advantage of a) is that it's the simplest alternative and also > works best when there are board specific inverters involved or the pwm > supports built-in inversion. I'm still in favor for a). IMO it's the only sane way to solve problems with inverted PWMs. A PWM in disabled state has an undefined output on some hardwares and we don't even axactly know which value a disabled PWM should have. Using a duty cycle of 0% or 100% makes the desired output very clear. Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | -- To unsubscribe from this list: send the line "unsubscribe linux-leds" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
