From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jaehoon Chung Date: Thu, 16 Jan 2014 17:27:47 +0900 Subject: [U-Boot] [PATCH v2] arm: exynos: change to use clrbits macro instead of readl/writel function In-Reply-To: <20140116172046.4a40b71d@songinha-Samsung-DeskTop-System> References: <20140115142759.66afd1ce@songinha-Samsung-DeskTop-System> <52D78F4D.5020208@samsung.com> <20140116172046.4a40b71d@songinha-Samsung-DeskTop-System> Message-ID: <52D79803.1040900@samsung.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 01/16/2014 05:20 PM, Inha Song wrote: > > Hi, > > On Thu, 16 Jan 2014 16:50:37 +0900 > Minkyu Kang wrote: > >> On 15/01/14 14:27, Inha Song wrote: >>> Use setbits/clrbits macro instead of readl/writel function >>> >>> Signed-off-by: Inha Song >>> Signed-off-by: Minkyu Kang >>> Tested-by: Przemyslaw Marczak >>> --- >>> 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, >