Hi, On Thu, 16 Jan 2014 16:50:37 +0900 Minkyu Kang <mk7.k...@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.s...@samsung.com> > > Signed-off-by: Minkyu Kang <mk7.k...@samsung.com> > > Tested-by: Przemyslaw Marczak <p.marc...@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot