diff mbox

[U-Boot,PATCHv1] ARM: Add Altera SOCFPGA Cyclone5

Message ID 20120830181203.GB29900@elf.ucw.cz
State Superseded
Delegated to: Marek Vasut
Headers show

Commit Message

Pavel Machek Aug. 30, 2012, 6:12 p.m. UTC 
Hi!

> > > > +	writel(TIMER_LOAD_VAL, &timer_base->load_val);
> > > > +	writel(TIMER_LOAD_VAL, &timer_base->curr_val);
> > > > +	writel((readl((&timer_base->ctrl)) | 0x3),
> > > 
> > > I think you should stick to programming in C here, not ((((LISP)))), so
> > > try cutting down on the ((braces)) :-)
> > 
> > Some braces have been harmed by creation of this patch.
> 
> At least one more pair must be killed on the above statement :)

I removed two pairs but failed to post the patch ;-). Be sure you'll
never see them again.

> > > btw. what's this 0x3 magic constant ?
> > 
> > Dinh has to help here :-(.
> 
> Also, use setbits_le32() there instead anyway.

Uff, setbits_le32 looks seriously strange. Should I do this?

Thanks for review,
										Pavel

Comments

Marek Vasut Aug. 30, 2012, 6:16 p.m. UTC | #1 
Dear Pavel Machek,

> Hi!
> 
> > > > > +	writel(TIMER_LOAD_VAL, &timer_base->load_val);
> > > > > +	writel(TIMER_LOAD_VAL, &timer_base->curr_val);
> > > > > +	writel((readl((&timer_base->ctrl)) | 0x3),
> > > > 
> > > > I think you should stick to programming in C here, not ((((LISP)))),
> > > > so try cutting down on the ((braces)) :-)
> > > 
> > > Some braces have been harmed by creation of this patch.
> > 
> > At least one more pair must be killed on the above statement :)
> 
> I removed two pairs but failed to post the patch ;-). Be sure you'll
> never see them again.
> 
> > > > btw. what's this 0x3 magic constant ?
> > > 
> > > Dinh has to help here :-(.
> > 
> > Also, use setbits_le32() there instead anyway.
> 
> Uff, setbits_le32 looks seriously strange. Should I do this?

Yes please, that's the suggested way:

Instead of:
ret = readl()
ret |= 1 << BIT;
writel(ret, )

You do:
setbits_le32()

> 
> Thanks for review,
> 										
Pavel
> 
> diff --git a/arch/arm/cpu/armv7/socfpga/timer.c
> b/arch/arm/cpu/armv7/socfpga/timer.c index 79fa081..1ceb6e9 100644
> --- a/arch/arm/cpu/armv7/socfpga/timer.c
> +++ b/arch/arm/cpu/armv7/socfpga/timer.c
> @@ -30,7 +30,7 @@ int timer_init(void)
>  {
>  	writel(TIMER_LOAD_VAL, &timer_base->load_val);
>  	writel(TIMER_LOAD_VAL, &timer_base->curr_val);
> -	writel(readl(&timer_base->ctrl) | 0x3, &timer_base->ctrl);
> +	setbits_le32(&timer_base->ctrl, 0x3);
>  	return 0;
>  }

Best regards,
Marek Vasut
diff mbox

Patch

diff --git a/arch/arm/cpu/armv7/socfpga/timer.c b/arch/arm/cpu/armv7/socfpga/timer.c
index 79fa081..1ceb6e9 100644
--- a/arch/arm/cpu/armv7/socfpga/timer.c
+++ b/arch/arm/cpu/armv7/socfpga/timer.c
@@ -30,7 +30,7 @@  int timer_init(void)
 {
 	writel(TIMER_LOAD_VAL, &timer_base->load_val);
 	writel(TIMER_LOAD_VAL, &timer_base->curr_val);
-	writel(readl(&timer_base->ctrl) | 0x3, &timer_base->ctrl);
+	setbits_le32(&timer_base->ctrl, 0x3);
 	return 0;
 }