Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20201207193629.493241-1-clemens.gruber@pqgruber.com>
 <20201207220025.42b6g76wq7ph5nvb@pengutronix.de> <X863KNo0IaekkU7q@workstation.tuxnet>
 <20201208091033.bxzrlad7mjbe3dsp@pengutronix.de> <X89RgpTb3sBBI++w@workstation.tuxnet>
 <X8+DI7ZN7mXtsxv9@ulmo>
In-Reply-To: <X8+DI7ZN7mXtsxv9@ulmo>
From:   Sven Van Asbroeck <thesven73@gmail.com>
Date:   Tue, 8 Dec 2020 09:44:42 -0500
Message-ID: <CAGngYiXgVbEXj-yR=DTeA4pO-N3=WhiHjQhknFsbfXBeD_yRbw@mail.gmail.com>
Subject: Re: [PATCH v4 1/4] pwm: pca9685: Switch to atomic API
To:     Thierry Reding <thierry.reding@gmail.com>
Cc:     Clemens Gruber <clemens.gruber@pqgruber.com>,
        =?UTF-8?Q?Uwe_Kleine=2DK=C3=B6nig?= 
        <u.kleine-koenig@pengutronix.de>, linux-pwm@vger.kernel.org,
        Lee Jones <lee.jones@linaro.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Mika Westerberg <mika.westerberg@linux.intel.com>,
        David Jander <david@protonic.nl>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Uwe, Thierry,

On Tue, Dec 8, 2020 at 4:10 AM Uwe Kleine-K=C3=B6nig
<u.kleine-koenig@pengutronix.de> wrote:
>
> If this is already in the old code, this probably warrants a separate
> fix, and yes, I consider this a severe bug. (Consider one channel
> driving a motor and reconfiguring an LED modifies the motor's speed.)
>

I think you are 100% correct, this would be a severe bug. I have only used
this chip to drive LEDs, where the actual period is not that important. But
for motor control, it's a different story.

Basically you are suggesting: the period (prescaler) can only be changed if=
f
its use-count is 1.

This however brings up a whole load of additional questions: consider the c=
ase
where the chip outputs are also used in gpio mode. the gpio functionality
only sets "full on" and "full off" bits. On a scope, a gpio output will loo=
k
identical, no matter the value of the period. So when a gpio output is in u=
se,
does it increment the prescaler use-count ?

Example:
1. output 1: set pwm mode (enabled=3Dtrue, duty_cycle=3D50%, period=3D1/200=
Hz)
2. output 2: set led mode (full-on bit set)
3. output 1: change period(enabled=3Dtrue, duty_cycle=3D50%, period=3D1/100=
Hz)

Do we have to make (3) fail? I would say no: although output 2 is in use,
it's not actually using the prescaler. Changing prescale won't modify
output 2 in any way.

Which brings us to an even trickier question: what happens if a pwm output
is set to 0% or 100% duty cycle? In that case, it'll behave like a gpio out=
put.
So when it's enabled, it does not use the prescaler.
But! what happens if we now set that output to a different duty cycle?

Example:
1. output 1: set pwm mode (enabled=3Dtrue, duty_cycle=3D50%,  period=3D1/20=
0Hz)
2. output 2: set pwm mode (enabled=3Dtrue, duty_cycle=3D100%, period=3D1/40=
0Hz)
  fail? no, because it's not actually using the period (it's full on)
3. output 2: set pwm mode (enabled=3Dtrue, duty_cycle=3D100%, period=3D1/20=
0Hz)
  fail? no, because it's not actually using the period (it's full on)
4. output 1: set pwm mode (enabled=3Dtrue, duty_cycle=3D50%,  period=3D1/40=
0Hz)
  fail? no, because only output 1 is using the prescaler
5. output 2: set pwm mode (enabled=3Dtrue, duty_cycle=3D50%, period=3D1/400=
Hz)
  fail? no, because output 2 is not changing the prescaler
6. output 2: set pwm mode (enabled=3Dtrue, duty_cycle=3D50%, period=3D1/200=
Hz)
  fail? yes, because output 2 is changing prescaler and it's already in use

IMHO all this can get very complicated and tricky.

We can of course make this much simpler by assumung that gpio or on/off pwm=
s
are actually using the prescaler. But then we'd be limiting this chip's
functionality.