LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Guru Das Srinagesh <gurus@codeaurora.org>
To: David Laight <David.Laight@ACULAB.COM>
Cc: "linux-pwm@vger.kernel.org" <linux-pwm@vger.kernel.org>,
	"Thierry Reding" <thierry.reding@gmail.com>,
	"Uwe Kleine-König" <uwe@kleine-koenig.org>,
	"Subbaraman Narayanamurthy" <subbaram@codeaurora.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"Michael Turquette" <mturquette@baylibre.com>,
	"Stephen Boyd" <sboyd@kernel.org>,
	"linux-clk@vger.kernel.org" <linux-clk@vger.kernel.org>
Subject: Re: [PATCH v8 01/12] clk: pwm: Use 64-bit division function
Date: Thu, 12 Mar 2020 12:09:12 -0700	[thread overview]
Message-ID: <20200312190859.GA19605@codeaurora.org> (raw)
In-Reply-To: <fea86a43b28f4493abe0826654369513@AcuMS.aculab.com>

On Thu, Mar 12, 2020 at 09:14:09AM +0000, David Laight wrote:
> From: Guru Das Srinagesh
> > Sent: 12 March 2020 02:10
> > On Wed, Mar 11, 2020 at 04:58:24PM +0000, David Laight wrote:
> > > From: Guru Das Srinagesh
> > > > Sent: 11 March 2020 01:41
> > > >
> > > > Since the PWM framework is switching struct pwm_args.period's datatype
> > > > to u64, prepare for this transition by using div64_u64 to handle a
> > > > 64-bit divisor.
> > > >
> ...
> > > > --- a/drivers/clk/clk-pwm.c
> > > > +++ b/drivers/clk/clk-pwm.c
> > > > @@ -89,7 +89,7 @@ static int clk_pwm_probe(struct platform_device *pdev)
> > > >  	}
> > > >
> > > >  	if (of_property_read_u32(node, "clock-frequency", &clk_pwm->fixed_rate))
> > > > -		clk_pwm->fixed_rate = NSEC_PER_SEC / pargs.period;
> > > > +		clk_pwm->fixed_rate = div64_u64(NSEC_PER_SEC, pargs.period);
> > >
> > > That cannot be needed, a 32 bit division is fine.
> > 
> > Could you please explain why? I think the use of this function is
> > warranted in order to handle the division properly with a 64-bit
> > divisor.
> ...
> > > I'd assign pargs.period to an 'unsigned int' variable
> > > prior to the division (I hate casts - been bitten by them in the past.).
> > 
> > Wouldn't this truncate the 64-bit value? The intention behind this patch
> > is to allow the processing of 64-bit values in full.
> 
> You are dividing a 32bit constant by a value.
> If pargs.period is greater than 2^32 the result is zero.

Thanks for the explanation. 

> I think you divide by 'fixed_rate' a bit later on - better not be zero.

Good point, but this issue exists with or without this patch, and fixing
it is beyond this patch's scope.

Just to check if this patch can be dropped, I tested out compilation
with this patch reverted and there were no errors, so I'm leaning
towards dropping this patch unless you have any further comments on how
to proceed.

Thank you.

Guru Das.

  reply	other threads:[~2020-03-12 19:09 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <cover.1583889178.git.gurus@codeaurora.org>
2020-03-11  1:41 ` Guru Das Srinagesh
2020-03-11 16:58   ` David Laight
2020-03-12  2:09     ` Guru Das Srinagesh
2020-03-12  9:14       ` David Laight
2020-03-12 19:09         ` Guru Das Srinagesh [this message]
2020-03-19 20:53           ` Guru Das Srinagesh
2020-04-09  2:40         ` Guru Das Srinagesh
2020-03-11  1:41 ` [PATCH v8 02/12] drm/i915: Use 64-bit division macro Guru Das Srinagesh
2020-03-11  1:41 ` [PATCH v8 03/12] hwmon: pwm-fan: " Guru Das Srinagesh
2020-03-11 15:08   ` Guenter Roeck
2020-03-11  1:41 ` [PATCH v8 04/12] ir-rx51: " Guru Das Srinagesh
2020-03-11  2:11   ` Sean Young
2020-03-11  1:41 ` [PATCH v8 05/12] pwm: clps711x: " Guru Das Srinagesh
2020-03-11  1:41 ` [PATCH v8 06/12] pwm: pwm-imx-tpm: " Guru Das Srinagesh
2020-03-11  1:41 ` [PATCH v8 07/12] pwm: imx27: Use 64-bit division macro and function Guru Das Srinagesh
2020-03-11  1:41 ` [PATCH v8 08/12] pwm: sifive: Use 64-bit division macro Guru Das Srinagesh
2020-03-11  1:41 ` [PATCH v8 09/12] pwm: stm32-lp: Use %llu format specifier for period Guru Das Srinagesh
2020-03-11  1:41 ` [PATCH v8 10/12] pwm: sun4i: Use 64-bit division function Guru Das Srinagesh
2020-03-11  3:35   ` Chen-Yu Tsai
2020-03-11  1:41 ` [PATCH v8 11/12] backlight: pwm_bl: " Guru Das Srinagesh
2020-03-11 10:05   ` Daniel Thompson
2020-03-11  1:41 ` [PATCH v8 12/12] pwm: core: Convert period and duty cycle to u64 Guru Das Srinagesh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200312190859.GA19605@codeaurora.org \
    --to=gurus@codeaurora.org \
    --cc=David.Laight@ACULAB.COM \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pwm@vger.kernel.org \
    --cc=mturquette@baylibre.com \
    --cc=sboyd@kernel.org \
    --cc=subbaram@codeaurora.org \
    --cc=thierry.reding@gmail.com \
    --cc=uwe@kleine-koenig.org \
    --subject='Re: [PATCH v8 01/12] clk: pwm: Use 64-bit division function' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).