On 01/16/2014 05:20 PM, Inha Song wrote:
>
> Hi,
>
> On Thu, 16 Jan 2014 16:50:37 +0900
> Minkyu Kang <[email protected]> wrote:
>
>> On 15/01/14 14:27, Inha Song wrote:
>>> Use setbits/clrbits macro instead of readl/writel function
>>>
>>> Signed-off-by: Inha Song <[email protected]>
>>> Signed-off-by: Minkyu Kang <[email protected]>
>>> Tested-by: Przemyslaw Marczak <[email protected]>
>>> ---
>>> 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,
>
_______________________________________________
U-Boot mailing list
[email protected]
http://lists.denx.de/mailman/listinfo/u-boot
