Re: [PATCH v2] pwm: Ensure for legacy drivers that pwm->state stays consistent

["Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>]

[-- Attachment #1: Type: text/plain, Size: 2057 bytes --]

Hi Geert,

On Tue, Jun 29, 2021 at 09:44:38PM +0200, Geert Uytterhoeven wrote:
> On Sat, 1 May 2021, Uwe Kleine-König wrote:
> > Without this change it can happen that if changing the polarity succeeded
> > but changing duty_cycle and period failed pwm->state contains a mixture
> > between the old and the requested state.
> > 
> > So remember the initial state before starting to modify the configuration
> > and restore it when one of the required callback fails.
> > 
> > Compared to the previous implementation .disable() (if necessary) is called
> > earlier to prevent a glitch.
> > 
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> 
> Thanks for your patch, which is now commit d7bff84fe7ed8c3b ("pwm:
> Ensure for legacy drivers that pwm->state stays consistent") in
> pwm/for-next.
> 
> This commit broke the backlight on the Atmark Techno Armadillo 800 EVA
> board (arch/arm/boot/dts/r8a7740-armadillo800eva.dts), which now shows a
> black screen.  Reverting the commit fixes the problem.
> 
> Do you have an idea what is wrong, and how to fix it?

I starred at the patch for some time now and couldn't find a problem.
Looking at drivers/pwm/pwm-renesas-tpu.c I don't see something obvious.
(The .set_polarity callback is faulty as I doesn't commit the request to
hardware, but that shouldn't matter here.)

I guess the first request the backlight driver emits is

	.period = 33333,
	.duty_cycle = 33333,
	.enabled = true,
	.polarity = PWM_POLARITY_INVERSED,

which should result into the following driver calls (with and without
the breaking commit):

	tpu_pwm_set_polarity(chip, pwm, PWM_POLARITY_INVERSED);
	tpu_pwm_config(chip, pwm, 33333, 33333);
	tpu_pwm_enable(chip, pwm);

Can you confirm that?

Feel free to contact me via irc if you have questions/insights.

Thanks for your time to report the issue,
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]