All of lore.kernel.org
 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 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.