From: Paul Cercueil <paul@crapouillou.net>
To: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
Cc: Thierry Reding <thierry.reding@gmail.com>,
od@opendingux.net, linux-pwm@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-mips@vger.kernel.org,
stable@vger.kernel.org
Subject: Re: [PATCH 2/5] pwm: jz4740: Fix pin level of disabled TCU2 channels, part 2
Date: Tue, 29 Nov 2022 16:58:28 +0000 [thread overview]
Message-ID: <dfb368f51365ab068d477154c8051117bae197de.camel@crapouillou.net> (raw)
In-Reply-To: <20221129162447.sqa6veugc2xn6vui@pengutronix.de>
Le mardi 29 novembre 2022 à 17:24 +0100, Uwe Kleine-König a écrit :
> Hello Paul,
>
> On Tue, Nov 29, 2022 at 12:25:56PM +0000, Paul Cercueil wrote:
> > Hi Uwe,
> >
> > Le lun. 28 nov. 2022 à 15:39:11 +0100, Uwe Kleine-König
> > <u.kleine-koenig@pengutronix.de> a écrit :
> > > Hello,
> > >
> > > On Tue, Oct 25, 2022 at 11:10:46AM +0100, Paul Cercueil wrote:
> > > > > Note that for disabled PWMs there is no official guaranty
> > > > > about the pin
> > > > > state. So it would be ok (but admittedly not great) to
> > > > > simplify the
> > > > > driver and accept that the pinstate is active while the PWM
> > > > > is off.
> > > > > IMHO this is also better than a glitch.
> > > > >
> > > > > If a consumer wants the PWM to be in its inactive state, they
> > > > > should
> > > > > not disable it.
> > > >
> > > > Completely disagree. I absolutely do not want the backlight to
> > > > go full
> > > > bright mode when the PWM pin is disabled. And disabling the
> > > > backlight is a
> > > > thing (for screen blanking and during mode changes).
> > >
> > > For some hardwares there is no pretty choice. So the gist is: If
> > > the
> > > backlight driver wants to ensure that the PWM pin is driven to
> > > its
> > > inactive level, it should use:
> > >
> > > pwm_apply(pwm, { .period = ..., .duty_cycle = 0, .enabled
> > > = true });
> > >
> > > and better not
> > >
> > > pwm_apply(pwm, { ..., .enabled = false });
> >
> > Well that sounds pretty stupid to me; why doesn't the PWM subsystem
> > enforce
> > that the pins must be driven to their inactive level when the PWM
> > function
> > is disabled?
> >
> > Then for such hardware you describe, the corresponding PWM
> > driver could itself apply a duty_cycle = 0 if that's what it takes
> > to get an
> > inactive state.
>
> Let's assume we claim that on disable the pin is driven to the
> inactive level.
>
> The (bad) effect is that for a use case where the pin state doesn't
> matter (e.g. a backlight where the power regulator is off), the PWM
> keeps running even though it could be disabled and so save some
> power.
>
> So to make this use case properly supported, we need another flag in
> struct pwm_state that allows the consumer to tell the lowlevel driver
> that it's ok to disable the hardware even with the output being UB.
> Let's call this new flag "spam" and the pin is allowed to do whatever
> it
> wants with .spam = false.
>
> After that you can realize that applying any state with:
>
> .duty_cycle = A,
> .period = B,
> .polarity = C,
> .enabled = false,
> .spam = true,
>
> semantically (i.e. just looking at the output) has the same effect as
>
> .duty_cycle = 0,
> .period = $something,
> .polarity = C,
> .enabled = true,
> .spam = true,
>
> So having .enabled doesn't add to the expressiveness of pwm_apply(),
> because you can specify any configuration without having to resort to
> .enabled = false. So the enabled member of struct pwm_state can be
> dropped.
>
> Then we end up with the exact scenario we have now, just that the
> flag
> that specifies if the output should be held in the inactive state has
> a
> bad name.
If I follow you, then it means that the PWM backlight driver pwm_bl.c
should set state.enabled=true in pwm_backlight_power_off() to make sure
that the pin is inactive?
-Paul
next prev parent reply other threads:[~2022-11-29 17:01 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-24 20:52 [PATCH 0/5] pwm: jz4740: Fixes and some light changes Paul Cercueil
2022-10-24 20:52 ` [PATCH 1/5] pwm: jz4740: Fix pin level of disabled TCU2 channels, part 1 Paul Cercueil
2022-10-25 6:21 ` Uwe Kleine-König
2022-10-25 10:02 ` Paul Cercueil
2022-11-17 13:29 ` Uwe Kleine-König
2022-11-18 9:55 ` Paul Cercueil
2023-01-17 21:35 ` Uwe Kleine-König
2023-01-17 23:05 ` Paul Cercueil
2023-01-18 8:16 ` Uwe Kleine-König
2022-10-24 20:52 ` [PATCH 2/5] pwm: jz4740: Fix pin level of disabled TCU2 channels, part 2 Paul Cercueil
2022-10-25 6:44 ` Uwe Kleine-König
2022-10-25 10:10 ` Paul Cercueil
2022-11-28 14:39 ` Uwe Kleine-König
2022-11-29 12:16 ` Thierry Reding
2022-11-29 12:34 ` Paul Cercueil
2022-11-29 12:25 ` Paul Cercueil
2022-11-29 16:24 ` Uwe Kleine-König
2022-11-29 16:58 ` Paul Cercueil [this message]
2022-11-29 17:46 ` Uwe Kleine-König
2022-10-24 20:52 ` [PATCH 3/5] pwm: jz4740: Force dependency on Device Tree Paul Cercueil
2022-11-28 14:41 ` Uwe Kleine-König
2022-10-24 20:52 ` [PATCH 4/5] pwm: jz4740: Depend on MACH_INGENIC instead of MIPS Paul Cercueil
2022-10-25 10:32 ` Philippe Mathieu-Daudé
2022-11-15 10:40 ` Uwe Kleine-König
2022-10-24 20:52 ` [PATCH 5/5] pwm: jz4740: Use regmap_{set,clear}_bits Paul Cercueil
2022-10-25 10:46 ` Philippe Mathieu-Daudé
2022-11-15 10:51 ` Uwe Kleine-König
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=dfb368f51365ab068d477154c8051117bae197de.camel@crapouillou.net \
--to=paul@crapouillou.net \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mips@vger.kernel.org \
--cc=linux-pwm@vger.kernel.org \
--cc=od@opendingux.net \
--cc=stable@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 NNTP newsgroup(s).