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

[Thierry Reding <thierry.reding@gmail.com>]

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

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@pengutronix.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@pengutronix.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

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