From: Stefan Wahren <stefan.wahren@i2se.com>
To: Victorien Vedrine <victorien.vedrine@ophrys.net>,
linux-kernel@vger.kernel.org
Cc: linux-clk@vger.kernel.org, sboyd@codeaurora.org,
mturquette@baylibre.com, shawn.guo@linaro.org,
"Fabio.Estevam@freescale.com" <Fabio.Estevam@freescale.com>,
Marek Vasut <marex@denx.de>, Shawn Guo <shawnguo@kernel.org>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v2] clk:mxs: Fix bug on frequency divider
Date: Fri, 18 Sep 2015 09:21:17 +0200 [thread overview]
Message-ID: <55FBBB6D.4010805@i2se.com> (raw)
In-Reply-To: <1441010707-17178-1-git-send-email-victorien.vedrine@ophrys.net>
+Shawn's new address
+linux-arm-kernel
> On drivers/clk/mxs/clk-frac.c, the function clk_frac_round_rate returned a bad
> result. The division before multiplication computes a wrong value ; the
> calculation is inverted to fix the problem. The second issue is that the exact
> rate have decimals and they are truncate. The consequence is that the function
> clk_frac_set_rate (which use the result of clk_frac_round_rate) computes a
> wrong value for the register (the rate generated can be closer to the desired
> rate). The correction is : if there is decimal to the result, it is rounded to
> the next larger integer.
> On drivers/clk/mxs/clk-frac.c, the function clk_frac_recalc_rate returned
> a bad result. The multiplication is made before the division to compute a
> correct value.
>
> Signed-off-by: Victorien Vedrine <victorien.vedrine@ophrys.net>
> ---
> drivers/clk/mxs/clk-frac.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/clk/mxs/clk-frac.c b/drivers/clk/mxs/clk-frac.c
> index e6aa6b5..b3fa7fa 100644
> --- a/drivers/clk/mxs/clk-frac.c
> +++ b/drivers/clk/mxs/clk-frac.c
> @@ -42,11 +42,13 @@ static unsigned long clk_frac_recalc_rate(struct clk_hw *hw,
> {
> struct clk_frac *frac = to_clk_frac(hw);
> u32 div;
> + u64 tmp_rate;
>
> div = readl_relaxed(frac->reg) >> frac->shift;
> div &= (1 << frac->width) - 1;
>
> - return (parent_rate >> frac->width) * div;
> + tmp_rate = (u64)parent_rate * div;
> + return tmp_rate >> frac->width;
> }
>
> static long clk_frac_round_rate(struct clk_hw *hw, unsigned long rate,
> @@ -55,7 +57,7 @@ static long clk_frac_round_rate(struct clk_hw *hw, unsigned long rate,
> struct clk_frac *frac = to_clk_frac(hw);
> unsigned long parent_rate = *prate;
> u32 div;
> - u64 tmp;
> + u64 tmp, tmp_rate, result;
>
> if (rate > parent_rate)
> return -EINVAL;
> @@ -68,7 +70,11 @@ static long clk_frac_round_rate(struct clk_hw *hw, unsigned long rate,
> if (!div)
> return -EINVAL;
>
> - return (parent_rate >> frac->width) * div;
> + tmp_rate = (u64)parent_rate * div;
> + result = tmp_rate >> frac->width;
> + if ((result << frac->width) < tmp_rate)
> + result += 1;
> + return result;
> }
>
> static int clk_frac_set_rate(struct clk_hw *hw, unsigned long rate,
WARNING: multiple messages have this Message-ID (diff)
From: stefan.wahren@i2se.com (Stefan Wahren)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2] clk:mxs: Fix bug on frequency divider
Date: Fri, 18 Sep 2015 09:21:17 +0200 [thread overview]
Message-ID: <55FBBB6D.4010805@i2se.com> (raw)
In-Reply-To: <1441010707-17178-1-git-send-email-victorien.vedrine@ophrys.net>
+Shawn's new address
+linux-arm-kernel
> On drivers/clk/mxs/clk-frac.c, the function clk_frac_round_rate returned a bad
> result. The division before multiplication computes a wrong value ; the
> calculation is inverted to fix the problem. The second issue is that the exact
> rate have decimals and they are truncate. The consequence is that the function
> clk_frac_set_rate (which use the result of clk_frac_round_rate) computes a
> wrong value for the register (the rate generated can be closer to the desired
> rate). The correction is : if there is decimal to the result, it is rounded to
> the next larger integer.
> On drivers/clk/mxs/clk-frac.c, the function clk_frac_recalc_rate returned
> a bad result. The multiplication is made before the division to compute a
> correct value.
>
> Signed-off-by: Victorien Vedrine <victorien.vedrine@ophrys.net>
> ---
> drivers/clk/mxs/clk-frac.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/clk/mxs/clk-frac.c b/drivers/clk/mxs/clk-frac.c
> index e6aa6b5..b3fa7fa 100644
> --- a/drivers/clk/mxs/clk-frac.c
> +++ b/drivers/clk/mxs/clk-frac.c
> @@ -42,11 +42,13 @@ static unsigned long clk_frac_recalc_rate(struct clk_hw *hw,
> {
> struct clk_frac *frac = to_clk_frac(hw);
> u32 div;
> + u64 tmp_rate;
>
> div = readl_relaxed(frac->reg) >> frac->shift;
> div &= (1 << frac->width) - 1;
>
> - return (parent_rate >> frac->width) * div;
> + tmp_rate = (u64)parent_rate * div;
> + return tmp_rate >> frac->width;
> }
>
> static long clk_frac_round_rate(struct clk_hw *hw, unsigned long rate,
> @@ -55,7 +57,7 @@ static long clk_frac_round_rate(struct clk_hw *hw, unsigned long rate,
> struct clk_frac *frac = to_clk_frac(hw);
> unsigned long parent_rate = *prate;
> u32 div;
> - u64 tmp;
> + u64 tmp, tmp_rate, result;
>
> if (rate > parent_rate)
> return -EINVAL;
> @@ -68,7 +70,11 @@ static long clk_frac_round_rate(struct clk_hw *hw, unsigned long rate,
> if (!div)
> return -EINVAL;
>
> - return (parent_rate >> frac->width) * div;
> + tmp_rate = (u64)parent_rate * div;
> + result = tmp_rate >> frac->width;
> + if ((result << frac->width) < tmp_rate)
> + result += 1;
> + return result;
> }
>
> static int clk_frac_set_rate(struct clk_hw *hw, unsigned long rate,
next prev parent reply other threads:[~2015-09-18 7:21 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-07-31 16:24 [PATCH] clk:mxs: Fix bug on frequency divider Victorien Vedrine
2015-07-31 17:34 ` Stephen Boyd
2015-08-19 10:29 ` victorien.vedrine
2015-08-31 8:45 ` [PATCH v2] " Victorien Vedrine
2015-09-16 14:16 ` Stefan Wahren
2015-09-18 7:21 ` Stefan Wahren [this message]
2015-09-18 7:21 ` Stefan Wahren
2015-09-24 12:17 ` Shawn Guo
2015-09-24 12:17 ` Shawn Guo
2015-10-01 22:25 ` Stephen Boyd
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=55FBBB6D.4010805@i2se.com \
--to=stefan.wahren@i2se.com \
--cc=Fabio.Estevam@freescale.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-clk@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=marex@denx.de \
--cc=mturquette@baylibre.com \
--cc=sboyd@codeaurora.org \
--cc=shawn.guo@linaro.org \
--cc=shawnguo@kernel.org \
--cc=victorien.vedrine@ophrys.net \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.