Hello Michal, On Fri, Nov 09, 2018 at 05:55:55PM +0100, Uwe Kleine-König wrote: > On Fri, Nov 09, 2018 at 02:24:42PM +0000, Vokáč Michal wrote: > > On 8.11.2018 20:18, Uwe Kleine-König wrote: > > > On Thu, Nov 08, 2018 at 03:21:44PM +0000, Vokáč Michal wrote: > > >> Hi Uwe, > > >> > > >> On 7.11.2018 16:01, Uwe Kleine-König wrote: > > >>>> Interesting idea. I just wonder why nobody else did not come up with such > > >>>> a simple solution before. > > >>> > > >>> I think I mentioned it already in this thread, but it went unnoticed :-) > > >> > > >> I meant it like "How happened this was not invented years ago, when people > > >> first noticed the issue with using inverted PWM for backlight on i.MX6." > > >> In our project, this issue dates back to 2015 :( > > >> > > >>> Then the patch isn't correct yet. The idea is always keep the hardware > > >>> running and only disable it if it's uninverted. > > >> > > >> OK, I got the point. > > >> > > >>> In imx_pwm_probe it's not yet known what the polarity is supposed to be, > > >>> right? > > >> > > >> Not really. It can already be known but currently there is no way how to > > >> pass the information to the probe function. I, as a creator of the device > > >> (and author of a DTS file) know that the circuit needs inverted PWM signal. > > >> And I know that the circuit needs to be disabled until I say it can be > > >> enabled. How I say that can warry. It may be default brightness level > 0 > > >> in DTS file or from a userspace program using PWM sysfs interface. > > >> > > >>> So the right thing to do there is to not touch the configuration > > >>> of the pwm. I think all states that are problematic then are also > > >>> problematic with the gpio/pinmux approach. > > >> > > >> I think my use-case I already presented before is an example where > > >> involving pinctrl solves the problem while the "leave PWM enabled > > >> for inverted users" does not. That is all the time between > > >> imx_pwm_probe() and imx_pwm_apply_v2(). > > > > > > You're doing in probe: > > > > > > if (pwm_is_running()): > > > mux(pin, function=pwm) > > > else: > > > gpio_set_value(gpio, 0) > > > mux(pin, function=gpio) > > > > > > This gives you the right level assuming the gpio specification uses the > > > right flag (GPIO_ACTIVE_HIGH or GPIO_ACTIVE_LOW). > > > > Agree. > > > > > Taking your example with the backlight device you specify an "init" and > > > a "default" pinctrl and only "default" contains the muxing for the PWM > > > pin everything should be as smooth as necessary: The pwm is only muxed > > > when the backlight device is successfully bound. > > > > Have you tried that Uwe? The bad news is I tested that before and now > > again and it does not work like that. We already discussed that earlier. > > The key is that the pinmux setting for the PWM pin should be part of the > bl pinctrl, not the pwm pinctrl. Then "default" is only setup when the > bl device is successfully bound which is after the bl's .probe callback > called pwm_apply(). Did my mail clear up my suggestion? Do you agree it should work like this (or even did you test that, as I did not)? Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ |
