[U-Boot] [PATCH] arm: exynos: add missing readl
Wolfgang Denk
wd at denx.de
Sat Jan 11 17:51:59 CET 2014
Dear Inha Song,
In message <20140111180220.644269f8 at songinha-Samsung-DeskTop-System> you wrote:
> The readl function was missing in exynos/clock.c
>
> Signed-off-by: Inha Song <ideal.song at 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
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
I am a computer. I am dumber than any human and smarter than any ad-
ministrator.
More information about the U-Boot
mailing list