From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marek Vasut <marex@denx.de>
Date: Tue, 14 Aug 2012 21:11:10 +0200
Subject: [U-Boot] [PATCH 03/10] mx5 clocks: Add and use CCSR definitions
In-Reply-To: <1369929236.2410129.1344971699769.JavaMail.root@advansee.com>
References: <1369929236.2410129.1344971699769.JavaMail.root@advansee.com>
Message-ID: <201208142111.10802.marex@denx.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Dear Beno?t Th?baudeau,

> Dear Marek Vasut,
> 
> > Dear Beno?t Th?baudeau,
> > 
> > > This fixes config_pll_clk(), which used 0x20 instead of 0x200 for
> > > PLL4_CLOCK.
> > > 
> > > Signed-off-by: Beno?t Th?baudeau <benoit.thebaudeau@advansee.com>
> > > Cc: Stefano Babic <sbabic@denx.de>
> > > ---
> > > 
> > >  .../arch/arm/cpu/armv7/mx5/clock.c                 |   34
> > > 
> > > +++++++++++++------- .../arch/arm/include/asm/arch-mx5/crm_regs.h
> > > 
> > >  17 ++++++++++ 2 files changed, 40 insertions(+), 11 deletions(-)
> > > 
> > > diff --git u-boot-4d3c95f.orig/arch/arm/cpu/armv7/mx5/clock.c
> > > u-boot-4d3c95f/arch/arm/cpu/armv7/mx5/clock.c index
> > > 9b083c0..10843a4
> > > 100644
> > > --- u-boot-4d3c95f.orig/arch/arm/cpu/armv7/mx5/clock.c
> > > +++ u-boot-4d3c95f/arch/arm/cpu/armv7/mx5/clock.c
> > > @@ -36,7 +36,9 @@ enum pll_clocks {
> > > 
> > >  	PLL1_CLOCK = 0,
> > >  	PLL2_CLOCK,
> > >  	PLL3_CLOCK,
> > > 
> > > +#ifdef CONFIG_MX53
> > > 
> > >  	PLL4_CLOCK,
> > > 
> > > +#endif
> > > 
> > >  	PLL_CLOCKS,
> > >  
> > >  };
> > > 
> > > @@ -379,10 +381,10 @@ static u32 get_lp_apm(void)
> > > 
> > >  	u32 ret_val = 0;
> > >  	u32 ccsr = __raw_readl(&mxc_ccm->ccsr);
> > > 
> > > -	if (((ccsr >> 9) & 1) == 0)
> > > -		ret_val = CONFIG_SYS_MX5_HCLK;
> > > +	if (ccsr & MXC_CCM_CCSR_LP_APM)
> > > +		ret_val = 32768 * 1024;
> > > 
> > >  	else
> > > 
> > > -		ret_val = ((32768 * 1024));
> > > +		ret_val = CONFIG_SYS_MX5_HCLK;
> > > 
> > >  	return ret_val;
> > >  
> > >  }
> > > 
> > > @@ -660,40 +662,50 @@ static int config_pll_clk(enum pll_clocks
> > > index,
> > > struct pll_param *pll_param) switch (index) {
> > > 
> > >  	case PLL1_CLOCK:
> > >  		/* Switch ARM to PLL2 clock */
> > > 
> > > -		__raw_writel(ccsr | 0x4, &mxc_ccm->ccsr);
> > > +		__raw_writel(ccsr | MXC_CCM_CCSR_PLL1_SW_CLK_SEL,
> > > +				&mxc_ccm->ccsr);
> > > 
> > >  		CHANGE_PLL_SETTINGS(pll, pll_param->pd,
> > >  		
> > >  					pll_param->mfi, pll_param->mfn,
> > >  					pll_param->mfd);
> > >  		
> > >  		/* Switch back */
> > > 
> > > -		__raw_writel(ccsr & ~0x4, &mxc_ccm->ccsr);
> > > +		__raw_writel(ccsr & ~MXC_CCM_CCSR_PLL1_SW_CLK_SEL,
> > > +				&mxc_ccm->ccsr);
> > 
> > Why use __raw_writel() instead of writel() ? Besides ... again,
> > clrsetbits_le32() is a better bet.
> 
> Sure, but it's again for consistency with the existing code.

Existing code looks crappy, any reason to not fix it ;-)
[...]