| Message ID | 20140111180220.644269f8@songinha-Samsung-DeskTop-System |
|---|---|
| State | Changes Requested |
| Delegated to: | Minkyu Kang |
| Headers | show |
Dear Inha Song, In message <20140111180220.644269f8@songinha-Samsung-DeskTop-System> you wrote: > The readl function was missing in exynos/clock.c > > Signed-off-by: Inha Song <ideal.song@samsung.com> > --- > arch/arm/cpu/armv7/exynos/clock.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/arch/arm/cpu/armv7/exynos/clock.c b/arch/arm/cpu/armv7/exynos/clock.c > index 5bde9d1..08e7d50 100644 > --- a/arch/arm/cpu/armv7/exynos/clock.c > +++ b/arch/arm/cpu/armv7/exynos/clock.c > @@ -1114,6 +1114,7 @@ void exynos4_set_lcd_clk(void) > * MIPI0_PRE_RATIO [23:20] > * set fimd ratio > */ > + cfg = readl(&clk->div_lcd0); > cfg &= ~(0xf); > cfg |= 0x1; > writel(cfg, &clk->div_lcd0); Code like this should be rewritten to use better I/O accessors. Here these 4 statements could / should be rewritten as clrsetbits_le32(&clk->div_lcd0, 0xf, 0x1); Actually you should probably rather write: clrsetbits_le32(&clk->div_lcd0, 0xe, 0x1); because to me it seems silly to me to clear a bit that needs to be set? > + cfg = readl(&clk->div_disp1_0); > cfg &= ~(0xf); > cfg |= 0x0; > writel(cfg, &clk->div_disp1_0); Same here: clrbits_le32(&clk->div_disp1_0, 0xf); Note: it makes zero sense to set zero bits. Note 2: I wonder if these magic number (0xf, 0x1) could / should not rather be replaced by some readable preprocessor macro def? > * MIPI0_PRE_RATIO [23:20] > * set mipi ratio > */ > + cfg = readl(&clk->div_lcd0); > cfg &= ~(0xf << 16); > cfg |= (0x1 << 16); > writel(cfg, &clk->div_lcd0); And again: clrsetbits_le32(&clk->div_lcd0, 0xf << 16, 0x1 << 16); [again I wonder if the 0xf should not rathe rbe 0xe instead?] Best regards, Wolfgang Denk
diff --git a/arch/arm/cpu/armv7/exynos/clock.c b/arch/arm/cpu/armv7/exynos/clock.c index 5bde9d1..08e7d50 100644 --- a/arch/arm/cpu/armv7/exynos/clock.c +++ b/arch/arm/cpu/armv7/exynos/clock.c @@ -1114,6 +1114,7 @@ void exynos4_set_lcd_clk(void) * MIPI0_PRE_RATIO [23:20] * set fimd ratio */ + cfg = readl(&clk->div_lcd0); cfg &= ~(0xf); cfg |= 0x1; writel(cfg, &clk->div_lcd0); @@ -1176,6 +1177,7 @@ void exynos5_set_lcd_clk(void) * MIPI0_PRE_RATIO [23:20] * set fimd ratio */ + cfg = readl(&clk->div_disp1_0); cfg &= ~(0xf); cfg |= 0x0; writel(cfg, &clk->div_disp1_0); @@ -1236,6 +1238,7 @@ void exynos4_set_mipi_clk(void) * MIPI0_PRE_RATIO [23:20] * set mipi ratio */ + cfg = readl(&clk->div_lcd0); cfg &= ~(0xf << 16); cfg |= (0x1 << 16); writel(cfg, &clk->div_lcd0);
The readl function was missing in exynos/clock.c Signed-off-by: Inha Song <ideal.song@samsung.com> --- arch/arm/cpu/armv7/exynos/clock.c | 3 +++ 1 file changed, 3 insertions(+)