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

[Gerhard Sittig]

On Mon, Jan 27, 2014 at 09:14 +0900, Inha Song wrote:
 
 Use setbits/clrbits macro instead of readl/writel function
 
 Signed-off-by: Inha Song ideal.s...@samsung.com

You make this look like a mechanical edit sweep, and keep
ignoring that it fixes a real bug where clock setup was operating
on random data, and don't pass this information to those who work
with your submission (maintainers and release engineers).

Can you please be bothered to accept and incorporate the feedback
that you get, and don't waste the time of those who spend this
time and effort?  Thank you!


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: off...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

[Inha Song]

Use setbits/clrbits macro instead of readl/writel function

Signed-off-by: Inha Song ideal.s...@samsung.com
---
Changes for v2:
- Coding Style cleanup
- add signed-off-by

Changes for v3:
- Modified to use mask value for clear bit
- Use clrsetbits instead of clrbits in exynos5_set_lcd_clk(void) function

 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..5b7e12f 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, 0xf, 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, 0xf, 0x1);
 }
 
 void