archive mirror
 help / color / mirror / Atom feed
From: "Uwe Kleine-König" <>
To: Geert Uytterhoeven <>
	Thierry Reding <>,
	Lee Jones <>,
Subject: Re: [PATCH v2] pwm: Ensure for legacy drivers that pwm->state stays consistent
Date: Wed, 30 Jun 2021 08:48:26 +0200	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <alpine.DEB.2.22.394.2106292138100.1194476@ramsan.of.borg>

[-- 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 <>
> 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,

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,

Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | |

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

  reply	other threads:[~2021-06-30  6:48 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <>
     [not found] ` <>
2021-06-29 19:44   ` [PATCH v2] pwm: Ensure for legacy drivers that pwm->state stays consistent Geert Uytterhoeven
2021-06-30  6:48     ` Uwe Kleine-König [this message]
2021-06-30 10:22       ` Geert Uytterhoeven
2021-06-30 16:21         ` Uwe Kleine-König
2021-06-30 17:08           ` Geert Uytterhoeven
2021-06-30 17:18           ` Thierry Reding
2021-06-29 19:52   ` Geert Uytterhoeven

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).