public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Stefano Babic <sbabic@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 5/7] mx35: Fix clock dividers
Date: Sat, 01 Sep 2012 11:15:35 +0200	[thread overview]
Message-ID: <5041D237.6070902@denx.de> (raw)
In-Reply-To: <1503202245.2411444.1344976407451.JavaMail.root@advansee.com>

On 14/08/2012 22:33, Beno?t Th?baudeau wrote:
> The clock dividers that were used do not match at all the reference manual. They
> were either completely broken, or came from an early silicon revision
> incompatible with the current one.
> 
> Signed-off-by: Beno?t Th?baudeau <benoit.thebaudeau@advansee.com>
> Cc: Stefano Babic <sbabic@denx.de>
> ---
>  .../arch/arm/cpu/arm1136/mx35/generic.c            |   48 ++++++++------------
>  .../arch/arm/include/asm/arch-mx35/crm_regs.h      |   42 ++++++-----------
>  2 files changed, 31 insertions(+), 59 deletions(-)
> 
> diff --git u-boot-4d3c95f.orig/arch/arm/cpu/arm1136/mx35/generic.c u-boot-4d3c95f/arch/arm/cpu/arm1136/mx35/generic.c
> index e369c86..4af052c 100644
> --- u-boot-4d3c95f.orig/arch/arm/cpu/arm1136/mx35/generic.c
> +++ u-boot-4d3c95f/arch/arm/cpu/arm1136/mx35/generic.c
> @@ -171,17 +171,14 @@ static u32 get_ipg_per_clk(void)
>  	u32 pdr4 = readl(&ccm->pdr4);
>  	u32 div;
>  	if (pdr0 & MXC_CCM_PDR0_PER_SEL) {
> -		div = (CCM_GET_DIVIDER(pdr4,
> -			MXC_CCM_PDR4_PER0_PRDF_MASK,
> -			MXC_CCM_PDR4_PER0_PODF_OFFSET) + 1) *
> -			(CCM_GET_DIVIDER(pdr4,

It seems also to me that the current code is wrong if
MXC_CCM_PDR0_PER_SEL is set. Maybe it was never set. As I see in figure
5-4, the ipg_per_clk depends only on pdr[21:16]. No idea where the
second multiplier comes.

> +		div = CCM_GET_DIVIDER(pdr4,
>  			MXC_CCM_PDR4_PER0_PODF_MASK,
> -			MXC_CCM_PDR4_PER0_PODF_OFFSET) + 1);
> +			MXC_CCM_PDR4_PER0_PODF_OFFSET) + 1;

The name remains quite confusing. In the manual is PER0_DIV, which is
the meaning of PODF here ?

Anyway, the masks you set are correct, I agree.

>  	} else {
>  		div = CCM_GET_DIVIDER(pdr0,
>  			MXC_CCM_PDR0_PER_PODF_MASK,
>  			MXC_CCM_PDR0_PER_PODF_OFFSET) + 1;
> -		freq /= get_ahb_div(pdr0);
> +		div *= get_ahb_div(pdr0);

This does not change the behavior.

>  	}
>  	return freq / div;
>  }
> @@ -199,19 +196,16 @@ u32 imx_get_uartclk(void)
>  		freq = decode_pll(readl(&ccm->ppctl),
>  			CONFIG_MX35_HCLK_FREQ);
>  	}
> -	freq /= ((CCM_GET_DIVIDER(pdr4,
> -			MXC_CCM_PDR4_UART_PRDF_MASK,
> -			MXC_CCM_PDR4_UART_PRDF_OFFSET) + 1) *
> -		(CCM_GET_DIVIDER(pdr4,
> +	freq /= CCM_GET_DIVIDER(pdr4,
>  			MXC_CCM_PDR4_UART_PODF_MASK,
> -			MXC_CCM_PDR4_UART_PODF_OFFSET) + 1));
> +			MXC_CCM_PDR4_UART_PODF_OFFSET) + 1;

This is also right. I am only asking myself why it works correctly now.


>  	return freq;
>  }
>  
>  unsigned int mxc_get_main_clock(enum mxc_main_clock clk)
>  {
>  	u32 nfc_pdf, hsp_podf;
> -	u32 pll, ret_val = 0, usb_prdf, usb_podf;
> +	u32 pll, ret_val = 0, usb_podf;
>  	struct ccm_regs *ccm =
>  		(struct ccm_regs *)IMX_CCM_BASE;
>  
> @@ -255,8 +249,7 @@ unsigned int mxc_get_main_clock(enum mxc_main_clock clk)
>  		ret_val = pll / (nfc_pdf + 1);
>  		break;
>  	case USB_CLK:
> -		usb_prdf = (reg4 >> 25) & 0x7;
> -		usb_podf = (reg4 >> 22) & 0x7;
> +		usb_podf = (reg4 >> 22) & 0x3F;

Agree again. The code seems generated from another manual. Maybe the
USB_DIV field was split into two fields. With this in mind, there is no
apparent error in current code, but it cannot be derived from the manual.

>  		break;
>  	default:
>  		printf("Unknown clock: %d\n", clk);
> @@ -287,11 +280,10 @@ unsigned int mxc_get_peri_clock(enum mxc_peri_clock clk)
>  	case UART2_BAUD:
>  	case UART3_BAUD:
>  		clk_sel = mpdr3 & (1 << 14);
> -		pre_pdf = (mpdr4 >> 13) & 0x7;
> -		pdf = (mpdr4 >> 10) & 0x7;
> +		pdf = (mpdr4 >> 10) & 0x3F;

Right !


The main issue about this patch is that it does not fix one problem, but
a lot of. Really it should be split into several patches. Anyway, I will
make some tests on the mx35 boards I have - if I will not get problems,
I will push it, hoping that someone else can make some further tests.

Best regards,
Stefano

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

  reply	other threads:[~2012-09-01  9:15 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-14 20:32 [U-Boot] [PATCH 1/7] mx35: Move clock enums to clock.h Benoît Thébaudeau
2012-08-14 20:32 ` [U-Boot] [PATCH 2/7] mx35: Remove declaration of non-existing function Benoît Thébaudeau
2012-08-20 11:56   ` Stefano Babic
2012-08-14 20:32 ` [U-Boot] [PATCH 3/7] mx35: Fix decode_pll Benoît Thébaudeau
2012-09-01  8:15   ` Stefano Babic
2012-09-02 13:01     ` stefano babic
2012-09-06  9:03   ` Stefano Babic
2012-08-14 20:33 ` [U-Boot] [PATCH 4/7] mx35: Add definitions for clock gate values Benoît Thébaudeau
2012-09-06  9:02   ` Stefano Babic
2012-08-14 20:33 ` [U-Boot] [PATCH 5/7] mx35: Fix clock dividers Benoît Thébaudeau
2012-09-01  9:15   ` Stefano Babic [this message]
2012-09-03 15:31     ` Benoît Thébaudeau
2012-09-03 15:55       ` Stefano Babic
2012-09-06  9:01         ` Stefano Babic
2012-09-06  9:03   ` Stefano Babic
2012-08-14 20:33 ` [U-Boot] [PATCH 6/7] mx35: Fix eSDHC clocks Benoît Thébaudeau
2012-08-20 12:07   ` Stefano Babic
2012-08-20 12:27     ` Benoît Thébaudeau
2012-08-20 12:34       ` Benoît Thébaudeau
2012-08-20 12:43         ` Stefano Babic
2012-08-20 12:55           ` Benoît Thébaudeau
2012-08-20 12:55             ` Stefano Babic
2012-08-20 13:12               ` Benoît Thébaudeau
2012-09-27 20:26   ` [U-Boot] [PATCH v2] " Benoît Thébaudeau
2012-10-10 11:27     ` Stefano Babic
2012-08-14 20:33 ` [U-Boot] [PATCH 7/7] mx35: Add cpu_mmc_init() Benoît Thébaudeau
2012-08-17 20:43   ` [U-Boot] [PATCH v2 " Benoît Thébaudeau
2012-08-17 21:18     ` Stefano Babic
2012-08-20  8:01     ` Stefano Babic
2012-08-20 11:50 ` [U-Boot] [PATCH 1/7] mx35: Move clock enums to clock.h Stefano Babic
2012-08-23  9:08 ` Stefano Babic

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=5041D237.6070902@denx.de \
    --to=sbabic@denx.de \
    --cc=u-boot@lists.denx.de \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox