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