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:

quoted

On 8.11.2018 20:18, Uwe Kleine-König wrote:

quoted

On Thu, Nov 08, 2018 at 03:21:44PM +0000, Vokáč Michal wrote:

quoted

Hi Uwe,

On 7.11.2018 16:01, Uwe Kleine-König wrote:

quoted

quoted

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 :(

quoted

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.

quoted

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.

quoted

  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.

quoted

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/  |