[U-Boot] [PATCH 5/7] mx35: Fix clock dividers

[Stefano Babic]

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 at advansee.com>
> Cc: Stefano Babic <sbabic at 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
=====================================================================