Le mar. 25 oct. 2022 à 08:21:29 +0200, Uwe Kleine-König
<u.kleine-koenig@xxxxxxxxxxxxxx> a écrit :
Hello,
On Mon, Oct 24, 2022 at 09:52:09PM +0100, Paul Cercueil wrote:
The "duty > cycle" trick to force the pin level of a disabled TCU2
channel would only work when the channel had been enabled
previously.
Address this issue by enabling the PWM mode in jz4740_pwm_disable
(I know, right) so that the "duty > cycle" trick works before
disabling
the PWM channel right after.
This issue went unnoticed, as the PWM pins on the majority of the
boards
tested would default to the inactive level once the corresponding
TCU
clock was enabled, so the first call to jz4740_pwm_disable() would
not
actually change the pin levels.
On the GCW Zero however, the PWM pin for the backlight (PWM1, which
is
a TCU2 channel) goes active as soon as the timer1 clock is enabled.
Since the jz4740_pwm_disable() function did not work on channels not
previously enabled, the backlight would shine at full brightness
from
the moment the backlight driver would probe, until the backlight
driver
tried to *enable* the PWM output.
With this fix, the PWM pins will be forced inactive as soon as
jz4740_pwm_apply() is called (and might be reconfigured to active if
dictated by the pwm_state). This means that there is still a tiny
time
frame between the .request() and .apply() callbacks where the PWM
pin
might be active. Sadly, there is no way to fix this issue: it is
impossible to write a PWM channel's registers if the corresponding
clock
is not enabled, and enabling the clock is what causes the PWM pin
to go
active.
There is a workaround, though, which complements this fix: simply
starting the backlight driver (or any PWM client driver) with a
"init"
pinctrl state that sets the pin as an inactive GPIO. Once the
driver is
probed and the pinctrl state switches to "default", the regular PWM
pin
configuration can be used as it will be properly driven.
Fixes: c2693514a0a1 ("pwm: jz4740: Obtain regmap from parent node")
Signed-off-by: Paul Cercueil <paul@xxxxxxxxxxxxxxx>
Cc: stable@xxxxxxxxxxxxxxx
OK, understood the issue. I think there is another similar issue: The
clk is get and enabled only in the .request() callback. The result is
(I
think---depends on a few further conditions) that if you have the
backlight driver as a module and the bootloader enables the backlight
to
show a splash screen, the backlight goes off because of the
clk_disable_unused initcall.
I will have to verify, but I'm pretty sure disabling the clock doesn't
change the pin level back to inactive.
-Paul
So the right thing to do is to get the clock in .probe(), and ensure
it
is kept on if the PWM is running already. Then you can also enable the
counter in .probe() and don't care for it in the enable and disable
functions.
The init pinctrl then has to be on the PWM then, but that's IMHO ok.
Best regards
Uwe
PS: While looking into the driver I noticed that .request() uses
dev_err_probe(). That's wrong, this function is only supposed to be
used
in .probe().
--
Pengutronix e.K. | Uwe Kleine-König
|
Industrial Linux Solutions |
https://www.pengutronix.de/ |
