[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