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
=====================================================================
next prev parent 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