linux-kernel - Re: [PATCH 6/7] pwm: jz4740: Make PWM start with the active part

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Sat, 21 Sep 2019 00:52:54 +0200
From:   Thierry Reding <thierry.reding@...il.com>
To:     Uwe Kleine-König <u.kleine-koenig@...gutronix.de>
Cc:     Paul Cercueil <paul@...pouillou.net>, od@...c.me,
        linux-pwm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 6/7] pwm: jz4740: Make PWM start with the active part

On Mon, Aug 12, 2019 at 11:58:53PM +0200, Uwe Kleine-König wrote:
> On Mon, Aug 12, 2019 at 10:50:01PM +0200, Paul Cercueil wrote:
> > 
> > 
> > Le lun. 12 août 2019 à 7:55, Uwe =?iso-8859-1?q?Kleine-K=F6nig?=
> > <u.kleine-koenig@...gutronix.de> a écrit :
> > > On Fri, Aug 09, 2019 at 07:33:24PM +0200, Paul Cercueil wrote:
> > > > 
> > > > 
> > > >  Le ven. 9 août 2019 à 19:10, Uwe =?iso-8859-1?q?Kleine-K=F6nig?=
> > > >  <u.kleine-koenig@...gutronix.de> a écrit :
> > > >  > On Fri, Aug 09, 2019 at 02:30:30PM +0200, Paul Cercueil wrote:
> > > >  > >  The PWM will always start with the inactive part. To counter
> > > > that,
> > > >  > >  when PWM is enabled we switch the configured polarity, and use
> > > >  > >  'period - duty + 1' as the real duty.
> > > >  >
> > > >  > Where does the + 1 come from? This looks wrong. (So if duty=0 is
> > > >  > requested you use duty = period + 1?)
> > > > 
> > > >  You'd never request duty == 0, would you?
> > > > 
> > > >  Your duty must always be in the inclusive range [1, period]
> > > >  (hardware values, not ns). A duty of 0 is a hardware fault
> > > >  (on the jz4740 it is).
> > > 
> > > From the PWM framework's POV duty cycle = 0 is perfectly valid. Similar
> > > to duty == period. Not supporting dutz cycle 0 is another limitation of
> > > your PWM that should be documented.
> > > 
> > > For actual use cases of duty cycle = 0 see drivers/hwmon/pwm-fan.c or
> > > drivers/leds/leds-pwm.c.
> > 
> > Perfectly valid for the PWM framework, maybe; but what is the expected
> > output then? A constant inactive state?
> 
> Yes, a constant inactive state is expected. This is consistent and in a
> similar way when using duty == period an constant active output is
> expected.
> 
> > Then I guess I can just disable the PWM output in the driver when
> > configured with duty == 0.
> 
> Some time ago I argued with Thierry that we could drop the concept of
> enabled/disabled for a PWM because a disabled PWM is supposed to behave
> identically to duty=0. This is however only nearly true because with
> duty=0 the time the PWM is inactive still is a multiple of the period.
> 
> I tend to agree that disabling the PWM when duty=0 is requested is
> better than to fail the request (or configure for duty=1 $whateverunit).
> I'm looking forward to what Thierry's opinion is here.

Agreed. If in order to meet the expectations of duty == 0 you have to
disable the PWM, then that's what you should do.

Thierry

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)