[PATCH v3 0/2] clk: imx: fix AV PLL rate setting

[PATCH v3 0/2] clk: imx: fix AV PLL rate setting

[Emil Lundmark]

Hi,

I discovered a problem when trying to set the rate of the audio PLL
(pll4_post_div) on an i.MX6Q. The rate I wanted to set was 196.608 MHz, but
the actual rate I got was 192.000570 MHz. This patch series fixes this
issue and also improves the precision of the audio/video PLLs.

Changes since v2:
- Cast result of 'parent_rate * mfn / mfd' to unsigned long instead of u32.
- Clarify how this issue was discovered in the commit message.
- Rebased on Linux 4.8.

Changes since v1:
- Use correct subsystem name in commit summary.
- Use 'Fixes:' tag to indicate bug fix.
- Revise argument for choosing the denominator register value after
  comments from Lothar Waßmann.

v2: http://lists.infradead.org/pipermail/linux-arm-kernel/2016-October/460809.html
v1: http://lists.infradead.org/pipermail/linux-arm-kernel/2016-October/460350.html

Emil Lundmark (2):
  clk: imx: fix integer overflow in AV PLL round rate
  clk: imx: improve precision of AV PLL to 1 Hz

 drivers/clk/imx/clk-pllv3.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

-- 
2.7.4

[PATCH v3 0/2] clk: imx: fix AV PLL rate setting

[Emil Lundmark]

Hi,

I discovered a problem when trying to set the rate of the audio PLL
(pll4_post_div) on an i.MX6Q. The rate I wanted to set was 196.608 MHz, but
the actual rate I got was 192.000570 MHz. This patch series fixes this
issue and also improves the precision of the audio/video PLLs.

Changes since v2:
- Cast result of 'parent_rate * mfn / mfd' to unsigned long instead of u32.
- Clarify how this issue was discovered in the commit message.
- Rebased on Linux 4.8.

Changes since v1:
- Use correct subsystem name in commit summary.
- Use 'Fixes:' tag to indicate bug fix.
- Revise argument for choosing the denominator register value after
  comments from Lothar Wa?mann.

v2: http://lists.infradead.org/pipermail/linux-arm-kernel/2016-October/460809.html
v1: http://lists.infradead.org/pipermail/linux-arm-kernel/2016-October/460350.html

Emil Lundmark (2):
  clk: imx: fix integer overflow in AV PLL round rate
  clk: imx: improve precision of AV PLL to 1 Hz

 drivers/clk/imx/clk-pllv3.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

-- 
2.7.4

[PATCH v3 1/2] clk: imx: fix integer overflow in AV PLL round rate

[Emil Lundmark]

Since 'parent_rate * mfn' may overflow 32 bits, the result should be
stored using 64 bits.

The problem was discovered when trying to set the rate of the audio PLL
(pll4_post_div) on an i.MX6Q. The desired rate was 196.608 MHz, but
the actual rate returned was 192.000570 MHz. The round rate function should
have been able to return 196.608 MHz, i.e., the desired rate.

Fixes: ba7f4f557eb6 ("clk: imx: correct AV PLL rate formula")
Cc: Anson Huang <b20788@freescale.com>
Signed-off-by: Emil Lundmark <emil@limesaudio.com>
---
 drivers/clk/imx/clk-pllv3.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/imx/clk-pllv3.c b/drivers/clk/imx/clk-pllv3.c
index 19f9b622981a..7a6acc3e4a92 100644
--- a/drivers/clk/imx/clk-pllv3.c
+++ b/drivers/clk/imx/clk-pllv3.c
@@ -223,7 +223,7 @@ static unsigned long clk_pllv3_av_recalc_rate(struct clk_hw *hw,
 	temp64 *= mfn;
 	do_div(temp64, mfd);
 
-	return (parent_rate * div) + (u32)temp64;
+	return parent_rate * div + (unsigned long)temp64;
 }
 
 static long clk_pllv3_av_round_rate(struct clk_hw *hw, unsigned long rate,
@@ -247,7 +247,11 @@ static long clk_pllv3_av_round_rate(struct clk_hw *hw, unsigned long rate,
 	do_div(temp64, parent_rate);
 	mfn = temp64;
 
-	return parent_rate * div + parent_rate * mfn / mfd;
+	temp64 = (u64)parent_rate;
+	temp64 *= mfn;
+	do_div(temp64, mfd);
+
+	return parent_rate * div + (unsigned long)temp64;
 }
 
 static int clk_pllv3_av_set_rate(struct clk_hw *hw, unsigned long rate,
-- 
2.7.4

[PATCH v3 1/2] clk: imx: fix integer overflow in AV PLL round rate

[Emil Lundmark]

Since 'parent_rate * mfn' may overflow 32 bits, the result should be
stored using 64 bits.

The problem was discovered when trying to set the rate of the audio PLL
(pll4_post_div) on an i.MX6Q. The desired rate was 196.608 MHz, but
the actual rate returned was 192.000570 MHz. The round rate function should
have been able to return 196.608 MHz, i.e., the desired rate.

Fixes: ba7f4f557eb6 ("clk: imx: correct AV PLL rate formula")
Cc: Anson Huang <b20788@freescale.com>
Signed-off-by: Emil Lundmark <emil@limesaudio.com>
---
 drivers/clk/imx/clk-pllv3.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/imx/clk-pllv3.c b/drivers/clk/imx/clk-pllv3.c
index 19f9b622981a..7a6acc3e4a92 100644
--- a/drivers/clk/imx/clk-pllv3.c
+++ b/drivers/clk/imx/clk-pllv3.c
@@ -223,7 +223,7 @@ static unsigned long clk_pllv3_av_recalc_rate(struct clk_hw *hw,
 	temp64 *= mfn;
 	do_div(temp64, mfd);
 
-	return (parent_rate * div) + (u32)temp64;
+	return parent_rate * div + (unsigned long)temp64;
 }
 
 static long clk_pllv3_av_round_rate(struct clk_hw *hw, unsigned long rate,
@@ -247,7 +247,11 @@ static long clk_pllv3_av_round_rate(struct clk_hw *hw, unsigned long rate,
 	do_div(temp64, parent_rate);
 	mfn = temp64;
 
-	return parent_rate * div + parent_rate * mfn / mfd;
+	temp64 = (u64)parent_rate;
+	temp64 *= mfn;
+	do_div(temp64, mfd);
+
+	return parent_rate * div + (unsigned long)temp64;
 }
 
 static int clk_pllv3_av_set_rate(struct clk_hw *hw, unsigned long rate,
-- 
2.7.4

[PATCH v3 2/2] clk: imx: improve precision of AV PLL to 1 Hz

[Emil Lundmark]

The audio and video PLLs are designed to have a precision of 1 Hz if some
conditions are met. The current implementation only allows a precision that
depends on the rate of the parent clock. E.g., if the parent clock is 24
MHz, the precision will be 24 Hz; or more generally the precision will be

    p / 10^6 Hz

where p is the parent clock rate. This comes down to how the register
values for the PLL's fractional loop divider are chosen.

The clock rate calculation for the PLL is

    PLL output frequency = Fref * (DIV_SELECT + NUM / DENOM)

or with a shorter notation

    r = p * (d + a / b)

In addition to all variables being integers, we also have the following
conditions:

    27 <= d <= 54

    -2^29 <= a <= 2^29-1
     0    <  b <= 2^30-1
    |a| < b

Here, d, a and b are register values for the fractional loop divider. We
want to chose d, a and b such that f(p, r) = p, i.e. f is our round_rate
function. Currently, d and b are chosen as

    d = r / p
    b = 10^6

hence we get the poor precision. And a is defined in terms of r, d, p and
b:

    a = (r - d * p) * b / p

I propose that if p <= 2^30-1 (i.e., the max value for b), we chose b as

    b = p

We can do this since

    |a| < b

    |(r - d * p) * b / p| < b

    |r - d * p| < p

Which have two solutions, one of them is when p < 0, so we can skip that
one. The other is when p > 0 and

    p * (d - 1) < r < p * (d + 1)

Substitute d = r / p:

    (r - p) < r < (r + p)  <=>  p > 0

So, as long as p > 0, we can chose b = p. This is a good choise for b since

    a = (r - d * p) * b / p
      = (r - d * p) * p / p
      = r - d * p

    r = p * (d + a / b)
      = p * d + p * a / b
      = p * d + p * a / p
      = p * d + a

and if d = r / p:

    a = r - d * p
      = r - r / p * p
      = 0

    r = p * d + a
      = p * d + 0
      = p * r / p
      = r

I reckon this is the intention by the design of the clock rate formula.

Signed-off-by: Emil Lundmark <emil@limesaudio.com>
---
 drivers/clk/imx/clk-pllv3.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/clk/imx/clk-pllv3.c b/drivers/clk/imx/clk-pllv3.c
index 7a6acc3e4a92..ed3a2df536ea 100644
--- a/drivers/clk/imx/clk-pllv3.c
+++ b/drivers/clk/imx/clk-pllv3.c
@@ -234,6 +234,7 @@ static long clk_pllv3_av_round_rate(struct clk_hw *hw, unsigned long rate,
 	unsigned long max_rate = parent_rate * 54;
 	u32 div;
 	u32 mfn, mfd = 1000000;
+	u32 max_mfd = 0x3FFFFFFF;
 	u64 temp64;
 
 	if (rate > max_rate)
@@ -241,6 +242,9 @@ static long clk_pllv3_av_round_rate(struct clk_hw *hw, unsigned long rate,
 	else if (rate < min_rate)
 		rate = min_rate;
 
+	if (parent_rate <= max_mfd)
+		mfd = parent_rate;
+
 	div = rate / parent_rate;
 	temp64 = (u64) (rate - div * parent_rate);
 	temp64 *= mfd;
@@ -262,11 +266,15 @@ static int clk_pllv3_av_set_rate(struct clk_hw *hw, unsigned long rate,
 	unsigned long max_rate = parent_rate * 54;
 	u32 val, div;
 	u32 mfn, mfd = 1000000;
+	u32 max_mfd = 0x3FFFFFFF;
 	u64 temp64;
 
 	if (rate < min_rate || rate > max_rate)
 		return -EINVAL;
 
+	if (parent_rate <= max_mfd)
+		mfd = parent_rate;
+
 	div = rate / parent_rate;
 	temp64 = (u64) (rate - div * parent_rate);
 	temp64 *= mfd;
-- 
2.7.4

[PATCH v3 2/2] clk: imx: improve precision of AV PLL to 1 Hz

[Emil Lundmark]

The audio and video PLLs are designed to have a precision of 1 Hz if some
conditions are met. The current implementation only allows a precision that
depends on the rate of the parent clock. E.g., if the parent clock is 24
MHz, the precision will be 24 Hz; or more generally the precision will be

    p / 10^6 Hz

where p is the parent clock rate. This comes down to how the register
values for the PLL's fractional loop divider are chosen.

The clock rate calculation for the PLL is

    PLL output frequency = Fref * (DIV_SELECT + NUM / DENOM)

or with a shorter notation

    r = p * (d + a / b)

In addition to all variables being integers, we also have the following
conditions:

    27 <= d <= 54

    -2^29 <= a <= 2^29-1
     0    <  b <= 2^30-1
    |a| < b

Here, d, a and b are register values for the fractional loop divider. We
want to chose d, a and b such that f(p, r) = p, i.e. f is our round_rate
function. Currently, d and b are chosen as

    d = r / p
    b = 10^6

hence we get the poor precision. And a is defined in terms of r, d, p and
b:

    a = (r - d * p) * b / p

I propose that if p <= 2^30-1 (i.e., the max value for b), we chose b as

    b = p

We can do this since

    |a| < b

    |(r - d * p) * b / p| < b

    |r - d * p| < p

Which have two solutions, one of them is when p < 0, so we can skip that
one. The other is when p > 0 and

    p * (d - 1) < r < p * (d + 1)

Substitute d = r / p:

    (r - p) < r < (r + p)  <=>  p > 0

So, as long as p > 0, we can chose b = p. This is a good choise for b since

    a = (r - d * p) * b / p
      = (r - d * p) * p / p
      = r - d * p

    r = p * (d + a / b)
      = p * d + p * a / b
      = p * d + p * a / p
      = p * d + a

and if d = r / p:

    a = r - d * p
      = r - r / p * p
      = 0

    r = p * d + a
      = p * d + 0
      = p * r / p
      = r

I reckon this is the intention by the design of the clock rate formula.

Signed-off-by: Emil Lundmark <emil@limesaudio.com>
---
 drivers/clk/imx/clk-pllv3.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/clk/imx/clk-pllv3.c b/drivers/clk/imx/clk-pllv3.c
index 7a6acc3e4a92..ed3a2df536ea 100644
--- a/drivers/clk/imx/clk-pllv3.c
+++ b/drivers/clk/imx/clk-pllv3.c
@@ -234,6 +234,7 @@ static long clk_pllv3_av_round_rate(struct clk_hw *hw, unsigned long rate,
 	unsigned long max_rate = parent_rate * 54;
 	u32 div;
 	u32 mfn, mfd = 1000000;
+	u32 max_mfd = 0x3FFFFFFF;
 	u64 temp64;
 
 	if (rate > max_rate)
@@ -241,6 +242,9 @@ static long clk_pllv3_av_round_rate(struct clk_hw *hw, unsigned long rate,
 	else if (rate < min_rate)
 		rate = min_rate;
 
+	if (parent_rate <= max_mfd)
+		mfd = parent_rate;
+
 	div = rate / parent_rate;
 	temp64 = (u64) (rate - div * parent_rate);
 	temp64 *= mfd;
@@ -262,11 +266,15 @@ static int clk_pllv3_av_set_rate(struct clk_hw *hw, unsigned long rate,
 	unsigned long max_rate = parent_rate * 54;
 	u32 val, div;
 	u32 mfn, mfd = 1000000;
+	u32 max_mfd = 0x3FFFFFFF;
 	u64 temp64;
 
 	if (rate < min_rate || rate > max_rate)
 		return -EINVAL;
 
+	if (parent_rate <= max_mfd)
+		mfd = parent_rate;
+
 	div = rate / parent_rate;
 	temp64 = (u64) (rate - div * parent_rate);
 	temp64 *= mfd;
-- 
2.7.4

Re: [PATCH v3 1/2] clk: imx: fix integer overflow in AV PLL round rate

[Fabio Estevam]

On Wed, Oct 12, 2016 at 7:31 AM, Emil Lundmark <emil@limesaudio.com> wrote:
> Since 'parent_rate * mfn' may overflow 32 bits, the result should be
> stored using 64 bits.
>
> The problem was discovered when trying to set the rate of the audio PLL
> (pll4_post_div) on an i.MX6Q. The desired rate was 196.608 MHz, but
> the actual rate returned was 192.000570 MHz. The round rate function should
> have been able to return 196.608 MHz, i.e., the desired rate.
>
> Fixes: ba7f4f557eb6 ("clk: imx: correct AV PLL rate formula")
> Cc: Anson Huang <b20788@freescale.com>
> Signed-off-by: Emil Lundmark <emil@limesaudio.com>

Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>

[PATCH v3 1/2] clk: imx: fix integer overflow in AV PLL round rate

[Fabio Estevam]

On Wed, Oct 12, 2016 at 7:31 AM, Emil Lundmark <emil@limesaudio.com> wrote:
> Since 'parent_rate * mfn' may overflow 32 bits, the result should be
> stored using 64 bits.
>
> The problem was discovered when trying to set the rate of the audio PLL
> (pll4_post_div) on an i.MX6Q. The desired rate was 196.608 MHz, but
> the actual rate returned was 192.000570 MHz. The round rate function should
> have been able to return 196.608 MHz, i.e., the desired rate.
>
> Fixes: ba7f4f557eb6 ("clk: imx: correct AV PLL rate formula")
> Cc: Anson Huang <b20788@freescale.com>
> Signed-off-by: Emil Lundmark <emil@limesaudio.com>

Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>

Re: [PATCH v3 2/2] clk: imx: improve precision of AV PLL to 1 Hz

[Fabio Estevam]

On Wed, Oct 12, 2016 at 7:31 AM, Emil Lundmark <emil@limesaudio.com> wrote:
> The audio and video PLLs are designed to have a precision of 1 Hz if some
> conditions are met. The current implementation only allows a precision that
> depends on the rate of the parent clock. E.g., if the parent clock is 24
> MHz, the precision will be 24 Hz; or more generally the precision will be
...
> I reckon this is the intention by the design of the clock rate formula.
>
> Signed-off-by: Emil Lundmark <emil@limesaudio.com>

Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>

[PATCH v3 2/2] clk: imx: improve precision of AV PLL to 1 Hz

[Fabio Estevam]

On Wed, Oct 12, 2016 at 7:31 AM, Emil Lundmark <emil@limesaudio.com> wrote:
> The audio and video PLLs are designed to have a precision of 1 Hz if some
> conditions are met. The current implementation only allows a precision that
> depends on the rate of the parent clock. E.g., if the parent clock is 24
> MHz, the precision will be 24 Hz; or more generally the precision will be
...
> I reckon this is the intention by the design of the clock rate formula.
>
> Signed-off-by: Emil Lundmark <emil@limesaudio.com>

Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>

Re: [PATCH v3 0/2] clk: imx: fix AV PLL rate setting

[Shawn Guo]

On Wed, Oct 12, 2016 at 12:31:39PM +0200, Emil Lundmark wrote:
> Emil Lundmark (2):
>   clk: imx: fix integer overflow in AV PLL round rate
>   clk: imx: improve precision of AV PLL to 1 Hz

For both,

Acked-by: Shawn Guo <shawnguo@kernel.org>

[PATCH v3 0/2] clk: imx: fix AV PLL rate setting

[Shawn Guo]

On Wed, Oct 12, 2016 at 12:31:39PM +0200, Emil Lundmark wrote:
> Emil Lundmark (2):
>   clk: imx: fix integer overflow in AV PLL round rate
>   clk: imx: improve precision of AV PLL to 1 Hz

For both,

Acked-by: Shawn Guo <shawnguo@kernel.org>

Re: [PATCH v3 1/2] clk: imx: fix integer overflow in AV PLL round rate

[Stephen Boyd]

On 10/12, Emil Lundmark wrote:
> Since 'parent_rate * mfn' may overflow 32 bits, the result should be
> stored using 64 bits.
> 
> The problem was discovered when trying to set the rate of the audio PLL
> (pll4_post_div) on an i.MX6Q. The desired rate was 196.608 MHz, but
> the actual rate returned was 192.000570 MHz. The round rate function should
> have been able to return 196.608 MHz, i.e., the desired rate.
> 
> Fixes: ba7f4f557eb6 ("clk: imx: correct AV PLL rate formula")
> Cc: Anson Huang <b20788@freescale.com>
> Signed-off-by: Emil Lundmark <emil@limesaudio.com>
> ---

Applied to clk-next

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v3 1/2] clk: imx: fix integer overflow in AV PLL round rate

[Stephen Boyd]

On 10/12, Emil Lundmark wrote:
> Since 'parent_rate * mfn' may overflow 32 bits, the result should be
> stored using 64 bits.
> 
> The problem was discovered when trying to set the rate of the audio PLL
> (pll4_post_div) on an i.MX6Q. The desired rate was 196.608 MHz, but
> the actual rate returned was 192.000570 MHz. The round rate function should
> have been able to return 196.608 MHz, i.e., the desired rate.
> 
> Fixes: ba7f4f557eb6 ("clk: imx: correct AV PLL rate formula")
> Cc: Anson Huang <b20788@freescale.com>
> Signed-off-by: Emil Lundmark <emil@limesaudio.com>
> ---

Applied to clk-next

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v3 2/2] clk: imx: improve precision of AV PLL to 1 Hz

[Stephen Boyd]

On 10/12, Emil Lundmark wrote:
> The audio and video PLLs are designed to have a precision of 1 Hz if some
> conditions are met. The current implementation only allows a precision that
> depends on the rate of the parent clock. E.g., if the parent clock is 24
> MHz, the precision will be 24 Hz; or more generally the precision will be
> 
>     p / 10^6 Hz
> 
> where p is the parent clock rate. This comes down to how the register
> values for the PLL's fractional loop divider are chosen.
> 
> The clock rate calculation for the PLL is
> 
>     PLL output frequency = Fref * (DIV_SELECT + NUM / DENOM)
> 
> or with a shorter notation
> 
>     r = p * (d + a / b)
> 
> In addition to all variables being integers, we also have the following
> conditions:
> 
>     27 <= d <= 54
> 
>     -2^29 <= a <= 2^29-1
>      0    <  b <= 2^30-1
>     |a| < b
> 
> Here, d, a and b are register values for the fractional loop divider. We
> want to chose d, a and b such that f(p, r) = p, i.e. f is our round_rate
> function. Currently, d and b are chosen as
> 
>     d = r / p
>     b = 10^6
> 
> hence we get the poor precision. And a is defined in terms of r, d, p and
> b:
> 
>     a = (r - d * p) * b / p
> 
> I propose that if p <= 2^30-1 (i.e., the max value for b), we chose b as
> 
>     b = p
> 
> We can do this since
> 
>     |a| < b
> 
>     |(r - d * p) * b / p| < b
> 
>     |r - d * p| < p
> 
> Which have two solutions, one of them is when p < 0, so we can skip that
> one. The other is when p > 0 and
> 
>     p * (d - 1) < r < p * (d + 1)
> 
> Substitute d = r / p:
> 
>     (r - p) < r < (r + p)  <=>  p > 0
> 
> So, as long as p > 0, we can chose b = p. This is a good choise for b since
> 
>     a = (r - d * p) * b / p
>       = (r - d * p) * p / p
>       = r - d * p
> 
>     r = p * (d + a / b)
>       = p * d + p * a / b
>       = p * d + p * a / p
>       = p * d + a
> 
> and if d = r / p:
> 
>     a = r - d * p
>       = r - r / p * p
>       = 0
> 
>     r = p * d + a
>       = p * d + 0
>       = p * r / p
>       = r
> 
> I reckon this is the intention by the design of the clock rate formula.
> 
> Signed-off-by: Emil Lundmark <emil@limesaudio.com>
> ---

Applied to clk-next

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v3 2/2] clk: imx: improve precision of AV PLL to 1 Hz

[Stephen Boyd]

On 10/12, Emil Lundmark wrote:
> The audio and video PLLs are designed to have a precision of 1 Hz if some
> conditions are met. The current implementation only allows a precision that
> depends on the rate of the parent clock. E.g., if the parent clock is 24
> MHz, the precision will be 24 Hz; or more generally the precision will be
> 
>     p / 10^6 Hz
> 
> where p is the parent clock rate. This comes down to how the register
> values for the PLL's fractional loop divider are chosen.
> 
> The clock rate calculation for the PLL is
> 
>     PLL output frequency = Fref * (DIV_SELECT + NUM / DENOM)
> 
> or with a shorter notation
> 
>     r = p * (d + a / b)
> 
> In addition to all variables being integers, we also have the following
> conditions:
> 
>     27 <= d <= 54
> 
>     -2^29 <= a <= 2^29-1
>      0    <  b <= 2^30-1
>     |a| < b
> 
> Here, d, a and b are register values for the fractional loop divider. We
> want to chose d, a and b such that f(p, r) = p, i.e. f is our round_rate
> function. Currently, d and b are chosen as
> 
>     d = r / p
>     b = 10^6
> 
> hence we get the poor precision. And a is defined in terms of r, d, p and
> b:
> 
>     a = (r - d * p) * b / p
> 
> I propose that if p <= 2^30-1 (i.e., the max value for b), we chose b as
> 
>     b = p
> 
> We can do this since
> 
>     |a| < b
> 
>     |(r - d * p) * b / p| < b
> 
>     |r - d * p| < p
> 
> Which have two solutions, one of them is when p < 0, so we can skip that
> one. The other is when p > 0 and
> 
>     p * (d - 1) < r < p * (d + 1)
> 
> Substitute d = r / p:
> 
>     (r - p) < r < (r + p)  <=>  p > 0
> 
> So, as long as p > 0, we can chose b = p. This is a good choise for b since
> 
>     a = (r - d * p) * b / p
>       = (r - d * p) * p / p
>       = r - d * p
> 
>     r = p * (d + a / b)
>       = p * d + p * a / b
>       = p * d + p * a / p
>       = p * d + a
> 
> and if d = r / p:
> 
>     a = r - d * p
>       = r - r / p * p
>       = 0
> 
>     r = p * d + a
>       = p * d + 0
>       = p * r / p
>       = r
> 
> I reckon this is the intention by the design of the clock rate formula.
> 
> Signed-off-by: Emil Lundmark <emil@limesaudio.com>
> ---

Applied to clk-next

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v3 1/2] clk: imx: fix integer overflow in AV PLL round rate

[Fabio Estevam]

Hi Stephen,

On Thu, Oct 27, 2016 at 11:41 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 10/12, Emil Lundmark wrote:
>> Since 'parent_rate * mfn' may overflow 32 bits, the result should be
>> stored using 64 bits.
>>
>> The problem was discovered when trying to set the rate of the audio PLL
>> (pll4_post_div) on an i.MX6Q. The desired rate was 196.608 MHz, but
>> the actual rate returned was 192.000570 MHz. The round rate function should
>> have been able to return 196.608 MHz, i.e., the desired rate.
>>
>> Fixes: ba7f4f557eb6 ("clk: imx: correct AV PLL rate formula")
>> Cc: Anson Huang <b20788@freescale.com>
>> Signed-off-by: Emil Lundmark <emil@limesaudio.com>
>> ---
>
> Applied to clk-next

This one fixes a regression caused by ba7f4f557eb6 ("clk: imx: correct
AV PLL rate formula").

So it should go to clk-fixes instead with the stable tag:

Cc: <stable@vger.kernel.org> # 4.8.x

Thanks

[PATCH v3 1/2] clk: imx: fix integer overflow in AV PLL round rate

[Fabio Estevam]

Hi Stephen,

On Thu, Oct 27, 2016 at 11:41 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 10/12, Emil Lundmark wrote:
>> Since 'parent_rate * mfn' may overflow 32 bits, the result should be
>> stored using 64 bits.
>>
>> The problem was discovered when trying to set the rate of the audio PLL
>> (pll4_post_div) on an i.MX6Q. The desired rate was 196.608 MHz, but
>> the actual rate returned was 192.000570 MHz. The round rate function should
>> have been able to return 196.608 MHz, i.e., the desired rate.
>>
>> Fixes: ba7f4f557eb6 ("clk: imx: correct AV PLL rate formula")
>> Cc: Anson Huang <b20788@freescale.com>
>> Signed-off-by: Emil Lundmark <emil@limesaudio.com>
>> ---
>
> Applied to clk-next

This one fixes a regression caused by ba7f4f557eb6 ("clk: imx: correct
AV PLL rate formula").

So it should go to clk-fixes instead with the stable tag:

Cc: <stable@vger.kernel.org> # 4.8.x

Thanks

Re: [PATCH v3 1/2] clk: imx: fix integer overflow in AV PLL round rate

[Stephen Boyd]

On 10/28, Fabio Estevam wrote:
> On Thu, Oct 27, 2016 at 11:41 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> > On 10/12, Emil Lundmark wrote:
> >> Since 'parent_rate * mfn' may overflow 32 bits, the result should be
> >> stored using 64 bits.
> >>
> >> The problem was discovered when trying to set the rate of the audio PLL
> >> (pll4_post_div) on an i.MX6Q. The desired rate was 196.608 MHz, but
> >> the actual rate returned was 192.000570 MHz. The round rate function should
> >> have been able to return 196.608 MHz, i.e., the desired rate.
> >>
> >> Fixes: ba7f4f557eb6 ("clk: imx: correct AV PLL rate formula")
> >> Cc: Anson Huang <b20788@freescale.com>
> >> Signed-off-by: Emil Lundmark <emil@limesaudio.com>
> >> ---
> >
> > Applied to clk-next
> 
> This one fixes a regression caused by ba7f4f557eb6 ("clk: imx: correct
> AV PLL rate formula").
> 
> So it should go to clk-fixes instead with the stable tag:
> 
> Cc: <stable@vger.kernel.org> # 4.8.x
> 

The clk-fixes branch is for patches that fix problems in code
merged during the merge window as well as small one-liners and
things that are causing great pain for people on the latest
release. Given that this fixes a regression in v4.8 and we're
trying to stabilize v4.9 it looked like it could wait until
v4.10.

So is there something going on here where the pain is too much to
wait for the next merge window? If so the commit text should
mention something about what's causing that pain. Perhaps by
referencing the commit that merged outside of clk tree that
caused problems.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v3 1/2] clk: imx: fix integer overflow in AV PLL round rate

[Stephen Boyd]

On 10/28, Fabio Estevam wrote:
> On Thu, Oct 27, 2016 at 11:41 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> > On 10/12, Emil Lundmark wrote:
> >> Since 'parent_rate * mfn' may overflow 32 bits, the result should be
> >> stored using 64 bits.
> >>
> >> The problem was discovered when trying to set the rate of the audio PLL
> >> (pll4_post_div) on an i.MX6Q. The desired rate was 196.608 MHz, but
> >> the actual rate returned was 192.000570 MHz. The round rate function should
> >> have been able to return 196.608 MHz, i.e., the desired rate.
> >>
> >> Fixes: ba7f4f557eb6 ("clk: imx: correct AV PLL rate formula")
> >> Cc: Anson Huang <b20788@freescale.com>
> >> Signed-off-by: Emil Lundmark <emil@limesaudio.com>
> >> ---
> >
> > Applied to clk-next
> 
> This one fixes a regression caused by ba7f4f557eb6 ("clk: imx: correct
> AV PLL rate formula").
> 
> So it should go to clk-fixes instead with the stable tag:
> 
> Cc: <stable@vger.kernel.org> # 4.8.x
> 

The clk-fixes branch is for patches that fix problems in code
merged during the merge window as well as small one-liners and
things that are causing great pain for people on the latest
release. Given that this fixes a regression in v4.8 and we're
trying to stabilize v4.9 it looked like it could wait until
v4.10.

So is there something going on here where the pain is too much to
wait for the next merge window? If so the commit text should
mention something about what's causing that pain. Perhaps by
referencing the commit that merged outside of clk tree that
caused problems.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v3 1/2] clk: imx: fix integer overflow in AV PLL round rate

[Fabio Estevam]

On Fri, Oct 28, 2016 at 4:13 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:

> The clk-fixes branch is for patches that fix problems in code
> merged during the merge window as well as small one-liners and
> things that are causing great pain for people on the latest
> release. Given that this fixes a regression in v4.8 and we're
> trying to stabilize v4.9 it looked like it could wait until
> v4.10.

The regression affects 4.8 and 4.9.

> So is there something going on here where the pain is too much to
> wait for the next merge window? If so the commit text should

Yes, people reported HDMI issues because of this bug:
https://www.spinics.net/lists/arm-kernel/msg535304.html

> mention something about what's causing that pain. Perhaps by
> referencing the commit that merged outside of clk tree that
> caused problems.

This patch clearly states that the problem is caused by ba7f4f557eb6
("clk: imx: correct AV PLL rate formula").

Since this is a regression, I don't understand why we need to wait
until 4.10 to get it applied.

[PATCH v3 1/2] clk: imx: fix integer overflow in AV PLL round rate

[Fabio Estevam]

On Fri, Oct 28, 2016 at 4:13 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:

> The clk-fixes branch is for patches that fix problems in code
> merged during the merge window as well as small one-liners and
> things that are causing great pain for people on the latest
> release. Given that this fixes a regression in v4.8 and we're
> trying to stabilize v4.9 it looked like it could wait until
> v4.10.

The regression affects 4.8 and 4.9.

> So is there something going on here where the pain is too much to
> wait for the next merge window? If so the commit text should

Yes, people reported HDMI issues because of this bug:
https://www.spinics.net/lists/arm-kernel/msg535304.html

> mention something about what's causing that pain. Perhaps by
> referencing the commit that merged outside of clk tree that
> caused problems.

This patch clearly states that the problem is caused by ba7f4f557eb6
("clk: imx: correct AV PLL rate formula").

Since this is a regression, I don't understand why we need to wait
until 4.10 to get it applied.

Re: [PATCH v3 1/2] clk: imx: fix integer overflow in AV PLL round rate

[Stephen Boyd]

On 10/28, Fabio Estevam wrote:
> On Fri, Oct 28, 2016 at 4:13 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> 
> > The clk-fixes branch is for patches that fix problems in code
> > merged during the merge window as well as small one-liners and
> > things that are causing great pain for people on the latest
> > release. Given that this fixes a regression in v4.8 and we're
> > trying to stabilize v4.9 it looked like it could wait until
> > v4.10.
> 
> The regression affects 4.8 and 4.9.
> 
> > So is there something going on here where the pain is too much to
> > wait for the next merge window? If so the commit text should
> 
> Yes, people reported HDMI issues because of this bug:
> https://www.spinics.net/lists/arm-kernel/msg535304.html
> 
> > mention something about what's causing that pain. Perhaps by
> > referencing the commit that merged outside of clk tree that
> > caused problems.
> 
> This patch clearly states that the problem is caused by ba7f4f557eb6
> ("clk: imx: correct AV PLL rate formula").

That commit isn't outside of clk tree :(

> 
> Since this is a regression, I don't understand why we need to wait
> until 4.10 to get it applied.

Because we're stabilizing the 4.9-rc series and not the 4.8-rc
series and the assumption is people tested code no 4.8 before it
was released. But in cases where that doesn't happen and the bugs
cause problems with testing the latest rc series we just apply
the patch. It sounds like in this case that happened, so I'll
move this patch over to fixes.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v3 1/2] clk: imx: fix integer overflow in AV PLL round rate

[Stephen Boyd]

On 10/28, Fabio Estevam wrote:
> On Fri, Oct 28, 2016 at 4:13 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> 
> > The clk-fixes branch is for patches that fix problems in code
> > merged during the merge window as well as small one-liners and
> > things that are causing great pain for people on the latest
> > release. Given that this fixes a regression in v4.8 and we're
> > trying to stabilize v4.9 it looked like it could wait until
> > v4.10.
> 
> The regression affects 4.8 and 4.9.
> 
> > So is there something going on here where the pain is too much to
> > wait for the next merge window? If so the commit text should
> 
> Yes, people reported HDMI issues because of this bug:
> https://www.spinics.net/lists/arm-kernel/msg535304.html
> 
> > mention something about what's causing that pain. Perhaps by
> > referencing the commit that merged outside of clk tree that
> > caused problems.
> 
> This patch clearly states that the problem is caused by ba7f4f557eb6
> ("clk: imx: correct AV PLL rate formula").

That commit isn't outside of clk tree :(

> 
> Since this is a regression, I don't understand why we need to wait
> until 4.10 to get it applied.

Because we're stabilizing the 4.9-rc series and not the 4.8-rc
series and the assumption is people tested code no 4.8 before it
was released. But in cases where that doesn't happen and the bugs
cause problems with testing the latest rc series we just apply
the patch. It sounds like in this case that happened, so I'll
move this patch over to fixes.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project