RE: pwm: rcar: improve calculation of divider

From: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
To: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	"thierry.reding@gmail.com" <thierry.reding@gmail.com>,
	"linux-pwm@vger.kernel.org" <linux-pwm@vger.kernel.org>,
	"linux-renesas-soc@vger.kernel.org" 
	<linux-renesas-soc@vger.kernel.org>,
	"kernel@pengutronix.de" <kernel@pengutronix.de>
Subject: RE: pwm: rcar: improve calculation of divider
Date: Wed, 12 Dec 2018 03:13:54 +0000	[thread overview]
Message-ID: <OSBPR01MB2293D189DCEA1EFB89A1AC34D8A70@OSBPR01MB2293.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <20181210080413.6367nhxhoj6ecqhl@pengutronix.de>

Hi Uwe,

> From: Uwe Kleine-König, Sent: Monday, December 10, 2018 5:04 PM
> 
> Hello,
> 
> On Mon, Dec 10, 2018 at 05:09:31AM +0000, Yoshihiro Shimoda wrote:
> > Thank you very much for your patches!
> > I tested patches and both codes work correctly.
> 
> \o/, that's actually better than I expected :-)
> 
> > > > The value of div should be calculatable without a loop. Something like:
> > > >
> > > >    divider = NSEC_PER_SEC * RCAR_PWM_MAX_CYCLE;
> > > >    tmp = (unsigned long long)period_ns * clk_rate + (divider - 1);
> > > >    do_div(tmp, divider);
> >
> > This should be do_div64_u64() because the divider is 1,023,000,000,000 (over 32-bits).
> 
> Yes, I think Laurent did this part right.
> 
> > > >    div = ilog2(tmp - 1) + 1;
> > > >
> > > > You might want to check if my maths are right, I didn't test.
> > >
> > > I've noticed the same, and wrote the following patch last week, also untested.
> > > I was planning to give it a try before sending it out, but as you've noticed
> > > the same issue, here's the code if anyone wants to give it a try before I can.
> > > Our calculations are similar, the main difference is the last line, and I
> > > think yours read better.
> >
> > So, I'd like to apply Uwe's code to mainline. Uwe, may I send such a patch
> > with your author and Singed-off-by?
> 
> Please no, I cannot sing good enough for this :-)
> 
> Honestly: If you take the authorship and write something like "Algorithm
> suggested by Uwe Kleine-König and Laurent Pinchart" that's IMHO fine.

Thank you for your reply. I got it. I'll make a patch with such comment :)

Best regards,
Yoshihiro Shimoda

> Best regards
> Uwe
> 
> --
> Pengutronix e.K.                           | Uwe Kleine-König            |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |