From: Geert Uytterhoeven <geert@linux-m68k.org>
To: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
Cc: Linux-Renesas <linux-renesas-soc@vger.kernel.org>,
Linux PWM List <linux-pwm@vger.kernel.org>,
Thierry Reding <thierry.reding@gmail.com>,
Lee Jones <lee.jones@linaro.org>,
Sascha Hauer <kernel@pengutronix.de>
Subject: Re: [PATCH v2] pwm: Ensure for legacy drivers that pwm->state stays consistent
Date: Wed, 30 Jun 2021 12:22:22 +0200 [thread overview]
Message-ID: <CAMuHMdV0w8mxxLVT4abrD-FgqS9Kg4BLomDQbXS_csdMyqhypQ@mail.gmail.com> (raw)
In-Reply-To: <20210630064826.4u2p37tlbriiwtsn@pengutronix.de>
Hi Uwe,
On Wed, Jun 30, 2021 at 8:48 AM Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
> 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?
tpu_pwm_config() is no longer called:
renesas-tpu-pwm e6600000.pwm: tpu_pwm_set_polarity:334: channel
2, polarity = 1
-renesas-tpu-pwm e6600000.pwm: tpu_pwm_config:257: channel = 2,
duty_ns = 0, period_ns = 33333
-renesas-tpu-pwm e6600000.pwm: tpu_pwm_config:257: channel = 2,
duty_ns = 33333, period_ns = 33333
renesas-tpu-pwm e6600000.pwm: tpu_pwm_enable:346: channel 2
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
next prev parent reply other threads:[~2021-06-30 10:22 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-11 16:04 [PATCH] pwm: Ensure for legacy drivers that pwm->state stays consistent Uwe Kleine-König
2021-05-01 16:09 ` [PATCH v2] " Uwe Kleine-König
2021-06-28 11:46 ` Thierry Reding
2021-06-28 17:24 ` Uwe Kleine-König
2021-06-29 19:44 ` Geert Uytterhoeven
2021-06-30 6:48 ` Uwe Kleine-König
2021-06-30 10:22 ` Geert Uytterhoeven [this message]
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:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAMuHMdV0w8mxxLVT4abrD-FgqS9Kg4BLomDQbXS_csdMyqhypQ@mail.gmail.com \
--to=geert@linux-m68k.org \
--cc=kernel@pengutronix.de \
--cc=lee.jones@linaro.org \
--cc=linux-pwm@vger.kernel.org \
--cc=linux-renesas-soc@vger.kernel.org \
--cc=thierry.reding@gmail.com \
--cc=u.kleine-koenig@pengutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* 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 read-only IMAP folder(s) and NNTP newsgroup(s).