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.thebaud...@advansee.com>
> Cc: Stefano Babic <sba...@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: sba...@denx.de
=====================================================================
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
