All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v2] arm: exynos: change to use clrbits macro instead of readl/writel function
@ 2014-01-15  5:27 Inha Song
  2014-01-15  5:40 ` Jaehoon Chung
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Inha Song @ 2014-01-15  5:27 UTC (permalink / raw)
  To: u-boot

Use setbits/clrbits macro instead of readl/writel function

Signed-off-by: Inha Song <ideal.song@samsung.com>
Signed-off-by: Minkyu Kang <mk7.kang@samsung.com>
Tested-by: Przemyslaw Marczak <p.marczak@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(-)

diff --git a/arch/arm/cpu/armv7/exynos/clock.c b/arch/arm/cpu/armv7/exynos/clock.c
index 5bde9d1..6c589c9 100644
--- a/arch/arm/cpu/armv7/exynos/clock.c
+++ b/arch/arm/cpu/armv7/exynos/clock.c
@@ -870,7 +870,6 @@ static void exynos4_set_mmc_clk(int dev_index, unsigned int div)
 	struct exynos4_clock *clk =
 		(struct exynos4_clock *)samsung_get_base_clock();
 	unsigned int addr;
-	unsigned int val;
 
 	/*
 	 * CLK_DIV_FSYS1
@@ -890,10 +889,8 @@ static void exynos4_set_mmc_clk(int dev_index, unsigned int div)
 		dev_index -= 2;
 	}
 
-	val = readl(addr);
-	val &= ~(0xff << ((dev_index << 4) + 8));
-	val |= (div & 0xff) << ((dev_index << 4) + 8);
-	writel(val, addr);
+	clrsetbits_le32(addr, 0xff << ((dev_index << 4) + 8),
+			(div & 0xff) << ((dev_index << 4) + 8));
 }
 
 /* exynos4x12: set the mmc clock */
@@ -902,7 +899,6 @@ static void exynos4x12_set_mmc_clk(int dev_index, unsigned int div)
 	struct exynos4x12_clock *clk =
 		(struct exynos4x12_clock *)samsung_get_base_clock();
 	unsigned int addr;
-	unsigned int val;
 
 	/*
 	 * CLK_DIV_FSYS1
@@ -917,10 +913,8 @@ static void exynos4x12_set_mmc_clk(int dev_index, unsigned int div)
 		dev_index -= 2;
 	}
 
-	val = readl(addr);
-	val &= ~(0xff << ((dev_index << 4) + 8));
-	val |= (div & 0xff) << ((dev_index << 4) + 8);
-	writel(val, addr);
+	clrsetbits_le32(addr, 0xff << ((dev_index << 4) + 8),
+			(div & 0xff) << ((dev_index << 4) + 8));
 }
 
 /* exynos5: set the mmc clock */
@@ -929,7 +923,6 @@ static void exynos5_set_mmc_clk(int dev_index, unsigned int div)
 	struct exynos5_clock *clk =
 		(struct exynos5_clock *)samsung_get_base_clock();
 	unsigned int addr;
-	unsigned int val;
 
 	/*
 	 * CLK_DIV_FSYS1
@@ -944,10 +937,8 @@ static void exynos5_set_mmc_clk(int dev_index, unsigned int div)
 		dev_index -= 2;
 	}
 
-	val = readl(addr);
-	val &= ~(0xff << ((dev_index << 4) + 8));
-	val |= (div & 0xff) << ((dev_index << 4) + 8);
-	writel(val, addr);
+	clrsetbits_le32(addr, 0xff << ((dev_index << 4) + 8),
+			(div & 0xff) << ((dev_index << 4) + 8));
 }
 
 /* exynos5: set the mmc clock */
@@ -956,7 +947,7 @@ static void exynos5420_set_mmc_clk(int dev_index, unsigned int div)
 	struct exynos5420_clock *clk =
 		(struct exynos5420_clock *)samsung_get_base_clock();
 	unsigned int addr;
-	unsigned int val, shift;
+	unsigned int shift;
 
 	/*
 	 * CLK_DIV_FSYS1
@@ -967,10 +958,7 @@ static void exynos5420_set_mmc_clk(int dev_index, unsigned int div)
 	addr = (unsigned int)&clk->div_fsys1;
 	shift = dev_index * 10;
 
-	val = readl(addr);
-	val &= ~(0x3ff << shift);
-	val |= (div & 0x3ff) << shift;
-	writel(val, addr);
+	clrsetbits_le32(addr, 0x3ff << shift, (div & 0x3ff) << shift);
 }
 
 /* get_lcd_clk: return lcd clock frequency */
@@ -1061,7 +1049,6 @@ void exynos4_set_lcd_clk(void)
 {
 	struct exynos4_clock *clk =
 	    (struct exynos4_clock *)samsung_get_base_clock();
-	unsigned int cfg = 0;
 
 	/*
 	 * CLK_GATE_BLOCK
@@ -1073,9 +1060,7 @@ void exynos4_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
@@ -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);
 
 	/*
 	 * 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);
 }
 
 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);
 
 	/*
 	 * 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);
 
 	/*
 	 * 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);
 }
 
 /*
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [U-Boot] [PATCH v2] arm: exynos: change to use clrbits macro instead of readl/writel function
  2014-01-15  5:27 [U-Boot] [PATCH v2] arm: exynos: change to use clrbits macro instead of readl/writel function Inha Song
@ 2014-01-15  5:40 ` Jaehoon Chung
  2014-01-15 16:33 ` Gerhard Sittig
  2014-01-16  7:50 ` Minkyu Kang
  2 siblings, 0 replies; 8+ messages in thread
From: Jaehoon Chung @ 2014-01-15  5:40 UTC (permalink / raw)
  To: u-boot

Acked-by: Jaehoon Chung <jh80.chung@samsung.com>

Best Regards,
Jaehoon Chung

On 01/15/2014 02:27 PM, Inha Song wrote:
> Use setbits/clrbits macro instead of readl/writel function
> 
> Signed-off-by: Inha Song <ideal.song@samsung.com>
> Signed-off-by: Minkyu Kang <mk7.kang@samsung.com>
> Tested-by: Przemyslaw Marczak <p.marczak@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(-)
> 
> diff --git a/arch/arm/cpu/armv7/exynos/clock.c b/arch/arm/cpu/armv7/exynos/clock.c
> index 5bde9d1..6c589c9 100644
> --- a/arch/arm/cpu/armv7/exynos/clock.c
> +++ b/arch/arm/cpu/armv7/exynos/clock.c
> @@ -870,7 +870,6 @@ static void exynos4_set_mmc_clk(int dev_index, unsigned int div)
>  	struct exynos4_clock *clk =
>  		(struct exynos4_clock *)samsung_get_base_clock();
>  	unsigned int addr;
> -	unsigned int val;
>  
>  	/*
>  	 * CLK_DIV_FSYS1
> @@ -890,10 +889,8 @@ static void exynos4_set_mmc_clk(int dev_index, unsigned int div)
>  		dev_index -= 2;
>  	}
>  
> -	val = readl(addr);
> -	val &= ~(0xff << ((dev_index << 4) + 8));
> -	val |= (div & 0xff) << ((dev_index << 4) + 8);
> -	writel(val, addr);
> +	clrsetbits_le32(addr, 0xff << ((dev_index << 4) + 8),
> +			(div & 0xff) << ((dev_index << 4) + 8));
>  }
>  
>  /* exynos4x12: set the mmc clock */
> @@ -902,7 +899,6 @@ static void exynos4x12_set_mmc_clk(int dev_index, unsigned int div)
>  	struct exynos4x12_clock *clk =
>  		(struct exynos4x12_clock *)samsung_get_base_clock();
>  	unsigned int addr;
> -	unsigned int val;
>  
>  	/*
>  	 * CLK_DIV_FSYS1
> @@ -917,10 +913,8 @@ static void exynos4x12_set_mmc_clk(int dev_index, unsigned int div)
>  		dev_index -= 2;
>  	}
>  
> -	val = readl(addr);
> -	val &= ~(0xff << ((dev_index << 4) + 8));
> -	val |= (div & 0xff) << ((dev_index << 4) + 8);
> -	writel(val, addr);
> +	clrsetbits_le32(addr, 0xff << ((dev_index << 4) + 8),
> +			(div & 0xff) << ((dev_index << 4) + 8));
>  }
>  
>  /* exynos5: set the mmc clock */
> @@ -929,7 +923,6 @@ static void exynos5_set_mmc_clk(int dev_index, unsigned int div)
>  	struct exynos5_clock *clk =
>  		(struct exynos5_clock *)samsung_get_base_clock();
>  	unsigned int addr;
> -	unsigned int val;
>  
>  	/*
>  	 * CLK_DIV_FSYS1
> @@ -944,10 +937,8 @@ static void exynos5_set_mmc_clk(int dev_index, unsigned int div)
>  		dev_index -= 2;
>  	}
>  
> -	val = readl(addr);
> -	val &= ~(0xff << ((dev_index << 4) + 8));
> -	val |= (div & 0xff) << ((dev_index << 4) + 8);
> -	writel(val, addr);
> +	clrsetbits_le32(addr, 0xff << ((dev_index << 4) + 8),
> +			(div & 0xff) << ((dev_index << 4) + 8));
>  }
>  
>  /* exynos5: set the mmc clock */
> @@ -956,7 +947,7 @@ static void exynos5420_set_mmc_clk(int dev_index, unsigned int div)
>  	struct exynos5420_clock *clk =
>  		(struct exynos5420_clock *)samsung_get_base_clock();
>  	unsigned int addr;
> -	unsigned int val, shift;
> +	unsigned int shift;
>  
>  	/*
>  	 * CLK_DIV_FSYS1
> @@ -967,10 +958,7 @@ static void exynos5420_set_mmc_clk(int dev_index, unsigned int div)
>  	addr = (unsigned int)&clk->div_fsys1;
>  	shift = dev_index * 10;
>  
> -	val = readl(addr);
> -	val &= ~(0x3ff << shift);
> -	val |= (div & 0x3ff) << shift;
> -	writel(val, addr);
> +	clrsetbits_le32(addr, 0x3ff << shift, (div & 0x3ff) << shift);
>  }
>  
>  /* get_lcd_clk: return lcd clock frequency */
> @@ -1061,7 +1049,6 @@ void exynos4_set_lcd_clk(void)
>  {
>  	struct exynos4_clock *clk =
>  	    (struct exynos4_clock *)samsung_get_base_clock();
> -	unsigned int cfg = 0;
>  
>  	/*
>  	 * CLK_GATE_BLOCK
> @@ -1073,9 +1060,7 @@ void exynos4_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
> @@ -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);
>  
>  	/*
>  	 * 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);
>  }
>  
>  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);
>  
>  	/*
>  	 * 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);
>  
>  	/*
>  	 * 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);
>  }
>  
>  /*
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [U-Boot] [PATCH v2] arm: exynos: change to use clrbits macro instead of readl/writel function
  2014-01-15  5:27 [U-Boot] [PATCH v2] arm: exynos: change to use clrbits macro instead of readl/writel function Inha Song
  2014-01-15  5:40 ` Jaehoon Chung
@ 2014-01-15 16:33 ` Gerhard Sittig
  2014-01-16  7:50 ` Minkyu Kang
  2 siblings, 0 replies; 8+ messages in thread
From: Gerhard Sittig @ 2014-01-15 16:33 UTC (permalink / raw)
  To: u-boot

On Wed, Jan 15, 2014 at 14:27 +0900, Inha Song wrote:
> 
> Use setbits/clrbits macro instead of readl/writel function

Just saw v2 after replying to v1 (which is only from yesterday).
Please consider the concerns raised there, too.


virtually yours
Gerhard Sittig
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [U-Boot] [PATCH v2] arm: exynos: change to use clrbits macro instead of readl/writel function
  2014-01-15  5:27 [U-Boot] [PATCH v2] arm: exynos: change to use clrbits macro instead of readl/writel function Inha Song
  2014-01-15  5:40 ` Jaehoon Chung
  2014-01-15 16:33 ` Gerhard Sittig
@ 2014-01-16  7:50 ` Minkyu Kang
  2014-01-16  8:20   ` Inha Song
  2 siblings, 1 reply; 8+ messages in thread
From: Minkyu Kang @ 2014-01-16  7:50 UTC (permalink / raw)
  To: u-boot

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@samsung.com>
> Signed-off-by: Minkyu Kang <mk7.kang@samsung.com>
> Tested-by: Przemyslaw Marczak <p.marczak@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.

>  
>  	/*
>  	 * 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.

>  }
>  
>  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.

>  
>  	/*
>  	 * 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.

>  
>  	/*
>  	 * 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.

>  }
>  
>  /*
> 

Thanks,
Minkyu Kang.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [U-Boot] [PATCH v2] arm: exynos: change to use clrbits macro instead of readl/writel function
  2014-01-16  7:50 ` Minkyu Kang
@ 2014-01-16  8:20   ` Inha Song
  2014-01-16  8:27     ` Jaehoon Chung
  2014-01-16  8:34     ` Minkyu Kang
  0 siblings, 2 replies; 8+ messages in thread
From: Inha Song @ 2014-01-16  8:20 UTC (permalink / raw)
  To: u-boot


Hi,

On Thu, 16 Jan 2014 16:50:37 +0900
Minkyu Kang <mk7.kang@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@samsung.com>
> > Signed-off-by: Minkyu Kang <mk7.kang@samsung.com>
> > Tested-by: Przemyslaw Marczak <p.marczak@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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [U-Boot] [PATCH v2] arm: exynos: change to use clrbits macro instead of readl/writel function
  2014-01-16  8:20   ` Inha Song
@ 2014-01-16  8:27     ` Jaehoon Chung
  2014-01-16  8:34     ` Minkyu Kang
  1 sibling, 0 replies; 8+ messages in thread
From: Jaehoon Chung @ 2014-01-16  8:27 UTC (permalink / raw)
  To: u-boot

On 01/16/2014 05:20 PM, Inha Song wrote:
> 
> Hi,
> 
> On Thu, 16 Jan 2014 16:50:37 +0900
> Minkyu Kang <mk7.kang@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@samsung.com>
>>> Signed-off-by: Minkyu Kang <mk7.kang@samsung.com>
>>> Tested-by: Przemyslaw Marczak <p.marczak@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)
Don't care bits? then use the 0xf. 
I think that it's more readable to use 0xf.

Best Regards,
Jaehoon Chung

> 
> 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,
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [U-Boot] [PATCH v2] arm: exynos: change to use clrbits macro instead of readl/writel function
  2014-01-16  8:20   ` Inha Song
  2014-01-16  8:27     ` Jaehoon Chung
@ 2014-01-16  8:34     ` Minkyu Kang
  2014-01-16  9:04       ` Jaehoon Chung
  1 sibling, 1 reply; 8+ messages in thread
From: Minkyu Kang @ 2014-01-16  8:34 UTC (permalink / raw)
  To: u-boot

On 16/01/14 17:20, Inha Song wrote:
> 
> Hi,
> 
> On Thu, 16 Jan 2014 16:50:37 +0900
> Minkyu Kang <mk7.kang@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@samsung.com>
>>> Signed-off-by: Minkyu Kang <mk7.kang@samsung.com>
>>> Tested-by: Przemyslaw Marczak <p.marczak@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?
> 

No..
0xF is mask of FIMD0_SEL that is 4 bits.
The mask value never be changed.

Thanks,
Minkyu Kang.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [U-Boot] [PATCH v2] arm: exynos: change to use clrbits macro instead of readl/writel function
  2014-01-16  8:34     ` Minkyu Kang
@ 2014-01-16  9:04       ` Jaehoon Chung
  0 siblings, 0 replies; 8+ messages in thread
From: Jaehoon Chung @ 2014-01-16  9:04 UTC (permalink / raw)
  To: u-boot

On 01/16/2014 05:34 PM, Minkyu Kang wrote:
> On 16/01/14 17:20, Inha Song wrote:
>>
>> Hi,
>>
>> On Thu, 16 Jan 2014 16:50:37 +0900
>> Minkyu Kang <mk7.kang@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@samsung.com>
>>>> Signed-off-by: Minkyu Kang <mk7.kang@samsung.com>
>>>> Tested-by: Przemyslaw Marczak <p.marczak@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?
>>
> 
> No..
> 0xF is mask of FIMD0_SEL that is 4 bits.
> The mask value never be changed.
Right, It's not "don't care bit".
If we want to change the source clock, just change the last argument.
It's not fixed 0x6 as source clock.

Best Regards,
Jaehoon Chung

> 
> Thanks,
> Minkyu Kang.
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2014-01-16  9:04 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-15  5:27 [U-Boot] [PATCH v2] arm: exynos: change to use clrbits macro instead of readl/writel function Inha Song
2014-01-15  5:40 ` Jaehoon Chung
2014-01-15 16:33 ` Gerhard Sittig
2014-01-16  7:50 ` Minkyu Kang
2014-01-16  8:20   ` Inha Song
2014-01-16  8:27     ` Jaehoon Chung
2014-01-16  8:34     ` Minkyu Kang
2014-01-16  9:04       ` Jaehoon Chung

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.