From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: MIME-Version: 1.0 In-Reply-To: <20160304112736.660e4223.drivshin.allworx@gmail.com> References: <1454128014-22866-1-git-send-email-drivshin.allworx@gmail.com> <20160226203100.303fe192.drivshin.allworx@gmail.com> <20160304151948.GF26400@ulmo.nvidia.com> <20160304112736.660e4223.drivshin.allworx@gmail.com> Date: Fri, 4 Mar 2016 10:29:07 -0600 Message-ID: Subject: Re: [PATCH 0/4] pwm: omap-dmtimer: fix period/duty_cycle calculation From: Adam Ford Content-Type: multipart/alternative; boundary=001a114b17ec39fe08052d3b9d02 List-ID: To: "David Rivshin (Allworx)" Cc: NeilBrown , Tony Lindgren , linux-omap@vger.kernel.org, Neil Armstrong , Joachim Eastwood , linux-arm-kernel@lists.infradead.org, Thierry Reding , Grant Erickson , linux-pwm@vger.kernel.org --001a114b17ec39fe08052d3b9d02 Content-Type: text/plain; charset=UTF-8 I am OK with it. 0% vs 1% is not perceivable and neither is 99% vs 100%. Thanks! Adam On Mar 4, 2016 10:27 AM, "David Rivshin (Allworx)" < drivshin.allworx@gmail.com> wrote: > On Fri, 4 Mar 2016 16:19:48 +0100 > Thierry Reding wrote: > > > On Fri, Feb 26, 2016 at 08:31:00PM -0500, David Rivshin (Allworx) wrote: > > > On Fri, 29 Jan 2016 23:26:50 -0500 > > > "David Rivshin (Allworx)" wrote: > > > > > > > From: David Rivshin > > > > > > > > When using a short PWM period (approaching the min of 2/clk_rate), > > > > pwm-omap-dmtimer does not produce accurate results. In the worst > case a > > > > requested period of 2/clk_rate would result in a real period of > 4/clk_rate > > > > instead. This is a series includes a fix for that problem, as well as > > > > other related improvements, and is based on the current > linux-pwm/for-next > > > > tip. > > > > > > > > I have tested on a Sitara AM335x platform, using a scope to verify > the > > > > output with a variety of periods and duty cycles. This includes a PWM > > > > rate up clk_rate/2 with 50% duty cycle (e.g. generating fclk/2) with > > > > both 32768Hz and 24MHz fclks. I do not have an OMAP4 board to test > with, > > > > although appropriate sections in the the reference manuals appear > > > > substantially the same, so I believe the changes are equally correct > > > > there. > > > > > > > > Note that the OMAP4 TRMs do effectively state that the maximum PWM > > > > rate is clk_rate/4, so at very fast PWM rates the behavior may not be > > > > as reliable as I observed with Sitara. Although I suspect that it's > > > > the same module and will also work, at least under some > circumstances. > > > > If anyone with OMAP4 hardware and a scope is so inclined, I would be > > > > curious to know the results. > > > > > > > > David Rivshin (4): > > > > pwm: omap-dmtimer: fix inaccurate period/duty_cycle calculation > > > > pwm: omap-dmtimer: add sanity checking for load and match values > > > > pwm: omap-dmtimer: round load and match values rather than truncate > > > > pwm: omap-dmtimer: add dev_dbg() message for effective period and > duty > > > > cycle > > > > > > > > drivers/pwm/pwm-omap-dmtimer.c | 71 > ++++++++++++++++++++++++++++++++---------- > > > > 1 file changed, 55 insertions(+), 16 deletions(-) > > > > > > > > > > Hi Thierry, > > > > > > Gentle ping. It does not look like you've taken this series, and I > > > wanted to make sure you're not waiting on something from me. It would > > > be nice to get at least the first patch into 4.5, if possible. > > > > I've applied patches 1 and 3, and I'm planning on sending out a pull > > request for inclusion in v4.5-rc7 later on. > > Thanks! > > > Patches 2 and 4 didn't seem ready/critical, so let's finish those up > > for v4.6-rc1. > > I know there was a lot of discussion on 4, but I'm not sure what the > concern is on patch 2. Is there something specific you're thinking of? > > FYI, I know that Adam Ford is using this driver as the backend for > a pwm-backlight control. Without patch 2 this driver will not configure > the HW in a legal way at 0 or 100% duty cycle. However, I forget what > the practical effect of that is, and Adam seemed to indicate it was OK > for his purposes. > > --001a114b17ec39fe08052d3b9d02 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable

I am OK with it. 0% vs 1% is not perceivable and neither is = 99% vs 100%.

Thanks!

Adam

On Mar 4, 2016 10:27 AM, "David Rivshin (Al= lworx)" <drivshin.all= worx@gmail.com> wrote:
On Fri, 4 Mar 2016 16:19:48 +0100
Thierry Reding <thierry.redi= ng@gmail.com> wrote:

> On Fri, Feb 26, 2016 at 08:31:00PM -0500, David Rivshin (Allworx) wrot= e:
> > On Fri, 29 Jan 2016 23:26:50 -0500
> > "David Rivshin (Allworx)" <drivshin.allworx@gmail.com> wrote:
> >
> > > From: David Rivshin <drivshin@allworx.com>
> > >
> > > When using a short PWM period (approaching the min of 2/clk_= rate),
> > > pwm-omap-dmtimer does not produce accurate results. In the w= orst case a
> > > requested period of 2/clk_rate would result in a real period= of 4/clk_rate
> > > instead. This is a series includes a fix for that problem, a= s well as
> > > other related improvements, and is based on the current linu= x-pwm/for-next
> > > tip.
> > >
> > > I have tested on a Sitara AM335x platform, using a scope to = verify the
> > > output with a variety of periods and duty cycles. This inclu= des a PWM
> > > rate up clk_rate/2 with 50% duty cycle (e.g. generating fclk= /2) with
> > > both 32768Hz and 24MHz fclks. I do not have an OMAP4 board t= o test with,
> > > although appropriate sections in the the reference manuals a= ppear
> > > substantially the same, so I believe the changes are equally= correct
> > > there.
> > >
> > > Note that the OMAP4 TRMs do effectively state that the maxim= um PWM
> > > rate is clk_rate/4, so at very fast PWM rates the behavior m= ay not be
> > > as reliable as I observed with Sitara. Although I suspect th= at it's
> > > the same module and will also work, at least under some circ= umstances.
> > > If anyone with OMAP4 hardware and a scope is so inclined, I = would be
> > > curious to know the results.
> > >
> > > David Rivshin (4):
> > >=C2=A0 =C2=A0pwm: omap-dmtimer: fix inaccurate period/duty_cy= cle calculation
> > >=C2=A0 =C2=A0pwm: omap-dmtimer: add sanity checking for load = and match values
> > >=C2=A0 =C2=A0pwm: omap-dmtimer: round load and match values r= ather than truncate
> > >=C2=A0 =C2=A0pwm: omap-dmtimer: add dev_dbg() message for eff= ective period and duty
> > >=C2=A0 =C2=A0 =C2=A0cycle
> > >
> > >=C2=A0 drivers/pwm/pwm-omap-dmtimer.c | 71 ++++++++++++++++++= ++++++++++++++----------
> > >=C2=A0 1 file changed, 55 insertions(+), 16 deletions(-)
> > >
> >
> > Hi Thierry,
> >
> > Gentle ping. It does not look like you've taken this series, = and I
> > wanted to make sure you're not waiting on something from me. = It would
> > be nice to get at least the first patch into 4.5, if possible. >
> I've applied patches 1 and 3, and I'm planning on sending out = a pull
> request for inclusion in v4.5-rc7 later on.

Thanks!

> Patches 2 and 4 didn't seem ready/critical, so let's finish th= ose up
> for v4.6-rc1.

I know there was a lot of discussion on 4, but I'm not sure what the concern is on patch 2. Is there something specific you're thinking of?<= br>
FYI, I know that Adam Ford is using this driver as the backend for
a pwm-backlight control. Without patch 2 this driver will not configure
the HW in a legal way at 0 or 100% duty cycle. However, I forget what
the practical effect of that is, and Adam seemed to indicate it was OK
for his purposes.

--001a114b17ec39fe08052d3b9d02--