Patchwork [v2] ARM: imx: pllv1: Fix PLL calculation for i.MX27

login
register
mail settings
Submitter Alexander Shiyan
Date July 29, 2013, 3:37 p.m.
Message ID <1375112243-1557-1-git-send-email-shc_work@mail.ru>
Download mbox | patch
Permalink /patch/262826/
State New
Headers show

Comments

Alexander Shiyan - July 29, 2013, 3:37 p.m.
MFN bit 9 on i.MX27 has a different meaning than in other SOCs. This
is a just sign bit. This patch makes different calculation for i.MX27.

Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
---
 arch/arm/mach-imx/clk-pllv1.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)
Lothar Waßmann - July 30, 2013, 6:37 a.m.
Hi,

Alexander Shiyan writes:
> MFN bit 9 on i.MX27 has a different meaning than in other SOCs. This
> is a just sign bit. This patch makes different calculation for i.MX27.
> 
> Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> ---
>  arch/arm/mach-imx/clk-pllv1.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/mach-imx/clk-pllv1.c b/arch/arm/mach-imx/clk-pllv1.c
> index c1eaee3..a289ccd 100644
> --- a/arch/arm/mach-imx/clk-pllv1.c
> +++ b/arch/arm/mach-imx/clk-pllv1.c
> @@ -25,6 +25,11 @@ struct clk_pllv1 {
>  
>  #define to_clk_pllv1(clk) (container_of(clk, struct clk_pllv1, clk))
>  
> +static inline bool allow_signed_mfn(void)
> +{
> +	return !cpu_is_mx1() && !cpu_is_mx21();
> +}
> +
>  static unsigned long clk_pllv1_recalc_rate(struct clk_hw *hw,
>  		unsigned long parent_rate)
>  {
> @@ -59,9 +64,14 @@ static unsigned long clk_pllv1_recalc_rate(struct clk_hw *hw,
>  	/*
>  	 * On all i.MXs except i.MX1 and i.MX21 mfn is a 10bit
>  	 * 2's complements number
> +	 * On i.MX27 the bit 9 is the sign bit.
>  	 */
> -	if (!cpu_is_mx1() && !cpu_is_mx21() && mfn >= 0x200)
> -		mfn_abs = 0x400 - mfn;
> +	if (allow_signed_mfn() && (mfn & 0x200)) {
> +		if (cpu_is_mx27())
> +			mfn_abs = mfn & 0x1ff;
> +		else
> +			mfn_abs = 0x400 - mfn;
> +	}
>  
Please don't hardcode magic numbers. Even more when using variants of
the same number like 0x200 and 0x1ff (0x200 - 1) in multiple places.
If someone copies the code and wants to adapt it, they will have to
change lots of different but related numbers in different places which
is horribly error prone.

#define MFN_BITS 10
#define MFN_SIGN (1 << (MFN_BITS - 1))
#define MFN_MASK (MFN_SIGN - 1)
With these defines you can derive all the numbers you need in a
consistent way and need to change only ONE place to adapt the code.


Lothar Waßmann

Patch

diff --git a/arch/arm/mach-imx/clk-pllv1.c b/arch/arm/mach-imx/clk-pllv1.c
index c1eaee3..a289ccd 100644
--- a/arch/arm/mach-imx/clk-pllv1.c
+++ b/arch/arm/mach-imx/clk-pllv1.c
@@ -25,6 +25,11 @@  struct clk_pllv1 {
 
 #define to_clk_pllv1(clk) (container_of(clk, struct clk_pllv1, clk))
 
+static inline bool allow_signed_mfn(void)
+{
+	return !cpu_is_mx1() && !cpu_is_mx21();
+}
+
 static unsigned long clk_pllv1_recalc_rate(struct clk_hw *hw,
 		unsigned long parent_rate)
 {
@@ -59,9 +64,14 @@  static unsigned long clk_pllv1_recalc_rate(struct clk_hw *hw,
 	/*
 	 * On all i.MXs except i.MX1 and i.MX21 mfn is a 10bit
 	 * 2's complements number
+	 * On i.MX27 the bit 9 is the sign bit.
 	 */
-	if (!cpu_is_mx1() && !cpu_is_mx21() && mfn >= 0x200)
-		mfn_abs = 0x400 - mfn;
+	if (allow_signed_mfn() && (mfn & 0x200)) {
+		if (cpu_is_mx27())
+			mfn_abs = mfn & 0x1ff;
+		else
+			mfn_abs = 0x400 - mfn;
+	}
 
 	rate = parent_rate * 2;
 	rate /= pd + 1;
@@ -70,7 +80,7 @@  static unsigned long clk_pllv1_recalc_rate(struct clk_hw *hw,
 
 	do_div(ll, mfd + 1);
 
-	if (!cpu_is_mx1() && !cpu_is_mx21() && mfn >= 0x200)
+	if (allow_signed_mfn() && (mfn & 0x200))
 		ll = -ll;
 
 	ll = (rate * mfi) + ll;