Hi Pavel, On the constant 0x3, its to enable the timer plus configure the timer mode to user-defined count mode. We can covert them to constant as below:
#define TIMER_ENABLE (1<<0) #define TIMER_MODE_USERDEF (1<<1) writel((readl((&timer_base->ctrl)) | TIMER_ENABLE | TIMER_MODE_USERDEF ) Thanks and have a nice day! Chin Liang Direct Line: +604 - 636 8776 -----Original Message----- From: Pavel Machek [mailto:[email protected]] Sent: Friday, August 31, 2012 1:00 AM To: Marek Vasut Cc: Tom Rini; [email protected]; ZY - u-boot; [email protected]; Chin Liang See Subject: Re: [U-Boot] [PATCHv1] ARM: Add Altera SOCFPGA Cyclone5 Hi! > > I took oportunity to cleanup whitespace in > > arch/arm/cpu/armv7/omap-common/u-boot-spl.lds. Perhaps someone can > > merge that... > > Argh ... what about using git send-email for the patch submission please? I'm not supposed to submit patches here, these are just requests for review ;-). > NOTE: I really have a great deal of respect for Pavel, so i do have > trouble stepping on him properly during the patch review ;-) ;-). > > + ldr r0, =CONFIG_SYS_INIT_RAM_ADDR > > + ldr r1, =CONFIG_SYS_INIT_RAM_SIZE > > + add r0, r0, r1 > > Won't the preprocessor compute (CONFIG_SYS_INIT_RAM_ADDR + > CONFIG_SYS_INIT_RAM_SIZE) for you? Then you'd only have to do one LDR > and no ADD instruction, saving two ticks of CPU :-) :-). And opportunity for me to try my luck at arm assembly. Does this look correct? ldr r0, =(CONFIG_SYS_INIT_RAM_ADDR + CONFIG_SYS_INIT_RAM_SIZE) ldr r1, =IRQ_STACK_START_IN str r0, [r1] bx lr > > +static const struct socfpga_reset_manager *reset_manager_base = > > + (void *)SOCFPGA_RSTMGR_ADDRESS; > > + > > +/* > > + * Write the reset manager register to cause reset */ > > You might want to add __attribute__((noreturn)) to this function :-) Hmm... I guess that should go to the central header file? No need to sprinkle it all through the C files. (Done). > > + 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. > btw. what's this 0x3 magic constant ? Dinh has to help here :-(. > > +static inline void delay(unsigned long loops) { > > + __asm__ volatile ("1:\n" > > + "subs %0, %1, #1\n" > > + "bne 1b" : "=r" (loops) : "0" (loops)); } > > Am I flat blind or is this not used here? Killed, thanks. Lets see what the compiler thinks. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
<<attachment: winmail.dat>>
_______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
