Re: (EXT) Re: [PATCH 1/4] pwm: pca9685: remove unused duty_cycle struct element

From: Clemens Gruber <clemens.gruber@pqgruber.com>
To: Sven Van Asbroeck <thesven73@gmail.com>
Cc: "Matthias Schiffer" <matthias.schiffer@ew.tq-group.com>,
	"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
	"Thierry Reding" <thierry.reding@gmail.com>,
	linux-pwm@vger.kernel.org,
	"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
	"Andy Shevchenko" <andy.shevchenko@gmail.com>
Subject: Re: (EXT) Re: [PATCH 1/4] pwm: pca9685: remove unused duty_cycle struct element
Date: Sat, 4 Apr 2020 19:35:46 +0200	[thread overview]
Message-ID: <20200404173546.GA55833@workstation.tuxnet> (raw)
In-Reply-To: <CAGngYiUe-tihBJUcXQ738_5aA9pzgp_-NSs4iCrz3eWO6rMukA@mail.gmail.com>

Hi,

On Fri, Apr 03, 2020 at 07:50:07PM -0400, Sven Van Asbroeck wrote:
> On Mon, Mar 30, 2020 at 11:12 AM Clemens Gruber
> <clemens.gruber@pqgruber.com> wrote:
> >
> > On Wed, Feb 26, 2020 at 06:03:02PM +0100, Matthias Schiffer wrote:
> > > As it turns out, this driver is broken in yet another way I didn't find
> > > before: For changing the global prescaler the chip needs to be put into
> > > sleep mode, but the driver doesn't follow the restart sequence
> > > described in the datasheet when waking it back up. In consequence,
> > > changing the period of one PWM does not only modify the period of all
> > > PWMs (which is bad enough, but can't be avoided with this hardware),
> > > but it also leaves all PWMs disabled...
> >
> > I am unable to reproduce this: If I set a specific duty cycle on a
> > channel and then change the period, the channel stays active.
> > I can see the brightness of an LED decrease if I increase the period.
> 
> What happens when pwm channels 0 and 1 are both enabled, and
> you change the pwm period of channel 0. Does channel 1 remain
> on?

Yes. Both channels remain on.

Let's say I configure a period of 5ms for both channels 0 and 1, as well
as a duty cycle of 4ms, meaning a relative duty cycle of 80%.
If I then increase the period of channel 0 to 10ms, there will be a
relative duty cycle of 40% on channel 0, but channel 1 will remain at a
relative duty cycle of 80%.
This is due to the relative nature of the internal ON/OFF times. For
the channel with the period change however, we recalculate the duty_ns
to period_ns ratio and reprogram the ON/OFF registers, because the user
might have already given us a different duty cycle in .config / .apply.

As the user is setting the duty cycle in nanoseconds, it makes sense
that the relative duty cycle decreases in an absolute period increase.
As for the behavior that the other channels remain at the same relative
duty cycle: Not sure how we can avoid this, other than reprogramming all
15 other channels if one of them is changed and that's not really
acceptable, I think.

Clemens