[U-Boot] [PATCH v2] arm: exynos: change to use clrbits macro instead of readl/writel function

[Inha Song]

Hi,

On Thu, 16 Jan 2014 16:50:37 +0900
Minkyu Kang <mk7.kang at samsung.com> wrote:

> On 15/01/14 14:27, Inha Song wrote:
> > Use setbits/clrbits macro instead of readl/writel function
> > 
> > Signed-off-by: Inha Song <ideal.song at samsung.com>
> > Signed-off-by: Minkyu Kang <mk7.kang at samsung.com>
> > Tested-by: Przemyslaw Marczak <p.marczak at samsung.com>
> > ---
> > Changes for v2:
> > - Coding Style cleanup
> > - add signed-off-by
> > 
> >  arch/arm/cpu/armv7/exynos/clock.c |   82 +++++++++----------------------------
> >  1 file changed, 20 insertions(+), 62 deletions(-)
>   
> >  	/*
> >  	 * CLK_SRC_LCD0
> > @@ -1085,10 +1070,7 @@ void exynos4_set_lcd_clk(void)
> >  	 * MIPI0_SEL		[12:15]
> >  	 * set lcd0 src clock 0x6: SCLK_MPLL
> >  	 */
> > -	cfg = readl(&clk->src_lcd0);
> > -	cfg &= ~(0xf);
> > -	cfg |= 0x6;
> > -	writel(cfg, &clk->src_lcd0);
> > +	clrsetbits_le32(&clk->src_lcd0, 0x9, 0x6);
> 
> 0x9? It seems to be 0xf.

I have set the only bit that must be cleared.

In case, I want to set src_lcd0 register to 0x6(b0110).
Therefore, do not need to clear a bit of the second and third. (don't care bits)

clrsetbits_le32(addr, 0x9, 0x6) == clrsetbits_le32(addr, 0xf, 0x6)
( reg &= ~b1xx1, reg |= b0110 == reg &= ~b1111, reg |= b0110 )

Do you think any way is better?

> 
> >  
> >  	/*
> >  	 * CLK_GATE_IP_LCD0
> > @@ -1100,9 +1082,7 @@ void exynos4_set_lcd_clk(void)
> >  	 * CLK_PPMULCD0		[5]
> >  	 * Gating all clocks for FIMD0
> >  	 */
> > -	cfg = readl(&clk->gate_ip_lcd0);
> > -	cfg |= 1 << 0;
> > -	writel(cfg, &clk->gate_ip_lcd0);
> > +	setbits_le32(&clk->gate_ip_lcd0, 1 << 0);
> >  
> >  	/*
> >  	 * CLK_DIV_LCD0
> > @@ -1114,16 +1094,13 @@ void exynos4_set_lcd_clk(void)
> >  	 * MIPI0_PRE_RATIO	[23:20]
> >  	 * set fimd ratio
> >  	 */
> > -	cfg &= ~(0xf);
> > -	cfg |= 0x1;
> > -	writel(cfg, &clk->div_lcd0);
> > +	clrsetbits_le32(&clk->div_lcd0, 0xe, 0x1);
> 
> ditto.

Ditto,

> 
> >  }
> >  
> >  void exynos5_set_lcd_clk(void)
> >  {
> >  	struct exynos5_clock *clk =
> >  	    (struct exynos5_clock *)samsung_get_base_clock();
> > -	unsigned int cfg = 0;
> >  
> >  	/*
> >  	 * CLK_GATE_BLOCK
> > @@ -1135,9 +1112,7 @@ void exynos5_set_lcd_clk(void)
> >  	 * CLK_LCD1	[5]
> >  	 * CLK_GPS	[7]
> >  	 */
> > -	cfg = readl(&clk->gate_block);
> > -	cfg |= 1 << 4;
> > -	writel(cfg, &clk->gate_block);
> > +	setbits_le32(&clk->gate_block, 1 << 4);
> >  
> >  	/*
> >  	 * CLK_SRC_LCD0
> > @@ -1147,10 +1122,7 @@ void exynos5_set_lcd_clk(void)
> >  	 * MIPI0_SEL		[12:15]
> >  	 * set lcd0 src clock 0x6: SCLK_MPLL
> >  	 */
> > -	cfg = readl(&clk->src_disp1_0);
> > -	cfg &= ~(0xf);
> > -	cfg |= 0x6;
> > -	writel(cfg, &clk->src_disp1_0);
> > +	clrsetbits_le32(&clk->src_disp1_0, 0x9, 0x6);
> 
> ditto.

Ditto,

> 
> >  
> >  	/*
> >  	 * CLK_GATE_IP_LCD0
> > @@ -1162,9 +1134,7 @@ void exynos5_set_lcd_clk(void)
> >  	 * CLK_PPMULCD0		[5]
> >  	 * Gating all clocks for FIMD0
> >  	 */
> > -	cfg = readl(&clk->gate_ip_disp1);
> > -	cfg |= 1 << 0;
> > -	writel(cfg, &clk->gate_ip_disp1);
> > +	setbits_le32(&clk->gate_ip_disp1, 1 << 0);
> >  
> >  	/*
> >  	 * CLK_DIV_LCD0
> > @@ -1176,16 +1146,13 @@ void exynos5_set_lcd_clk(void)
> >  	 * MIPI0_PRE_RATIO	[23:20]
> >  	 * set fimd ratio
> >  	 */
> > -	cfg &= ~(0xf);
> > -	cfg |= 0x0;
> > -	writel(cfg, &clk->div_disp1_0);
> > +	clrbits_le32(&clk->div_disp1_0, 0xf);
> >  }
> >  
> >  void exynos4_set_mipi_clk(void)
> >  {
> >  	struct exynos4_clock *clk =
> >  	    (struct exynos4_clock *)samsung_get_base_clock();
> > -	unsigned int cfg = 0;
> >  
> >  	/*
> >  	 * CLK_SRC_LCD0
> > @@ -1195,10 +1162,7 @@ void exynos4_set_mipi_clk(void)
> >  	 * MIPI0_SEL		[12:15]
> >  	 * set mipi0 src clock 0x6: SCLK_MPLL
> >  	 */
> > -	cfg = readl(&clk->src_lcd0);
> > -	cfg &= ~(0xf << 12);
> > -	cfg |= (0x6 << 12);
> > -	writel(cfg, &clk->src_lcd0);
> > +	clrsetbits_le32(&clk->src_lcd0, 0x9 << 12, 0x6 << 12);
> 
> ditto.

Ditto,

> 
> >  
> >  	/*
> >  	 * CLK_SRC_MASK_LCD0
> > @@ -1208,9 +1172,7 @@ void exynos4_set_mipi_clk(void)
> >  	 * MIPI0_MASK		[12]
> >  	 * set src mask mipi0 0x1: Unmask
> >  	 */
> > -	cfg = readl(&clk->src_mask_lcd0);
> > -	cfg |= (0x1 << 12);
> > -	writel(cfg, &clk->src_mask_lcd0);
> > +	setbits_le32(&clk->src_mask_lcd0, 0x1 << 12);
> >  
> >  	/*
> >  	 * CLK_GATE_IP_LCD0
> > @@ -1222,9 +1184,7 @@ void exynos4_set_mipi_clk(void)
> >  	 * CLK_PPMULCD0		[5]
> >  	 * Gating all clocks for MIPI0
> >  	 */
> > -	cfg = readl(&clk->gate_ip_lcd0);
> > -	cfg |= 1 << 3;
> > -	writel(cfg, &clk->gate_ip_lcd0);
> > +	setbits_le32(&clk->gate_ip_lcd0, 1 << 3);
> >  
> >  	/*
> >  	 * CLK_DIV_LCD0
> > @@ -1236,9 +1196,7 @@ void exynos4_set_mipi_clk(void)
> >  	 * MIPI0_PRE_RATIO	[23:20]
> >  	 * set mipi ratio
> >  	 */
> > -	cfg &= ~(0xf << 16);
> > -	cfg |= (0x1 << 16);
> > -	writel(cfg, &clk->div_lcd0);
> > +	clrsetbits_le32(&clk->div_lcd0, 0xe << 16, 0x1 << 16);
> 
> ditto.

Ditto,

> 
> >  }
> >  
> >  /*
> > 
> 
> Thanks,
> Minkyu Kang.


Do you think any way is better?
e.g) clrsetbits_le32(addr, 0x9, 0x6) vs clrsetbits_le32(addr, 0xf, 0x6)



Thanks,

-- 
Best Regards,
Inha Song