Hi Alexey,
On 31 January 2017 at 07:35, Alexey Brodkin Alexey.Brodkin@synopsys.com wrote:
Hi Simon,
On Tue, 2017-01-31 at 14:44 +0000, Vlad Zakharov wrote:
Hi Simon,
On Fri, 2017-01-20 at 20:51 -0700, Simon Glass wrote:
switch (priv->timer_id) {case 0:/* Disable timer if CPU is halted */write_aux_reg(ARC_AUX_TIMER0_CTRL, NH_MODE);/* Set max value for counter/timer */write_aux_reg(ARC_AUX_TIMER0_LIMIT, 0xffffffff);/* Set initial count value and restart counter/timer */write_aux_reg(ARC_AUX_TIMER0_CNT, 0);break;case 1:/* Disable timer if CPU is halted */write_aux_reg(ARC_AUX_TIMER1_CTRL, NH_MODE);/* Set max value for counter/timer */write_aux_reg(ARC_AUX_TIMER1_LIMIT, 0xffffffff);/* Set initial count value and restart counter/timer */write_aux_reg(ARC_AUX_TIMER1_CNT, 0);You are writing the same values in each case. Can you set a value to either ARC_AUX_TIMER0 or ARC_AUX_TIMER1 and then just have the code once?
Yes, here we have some code duplication. But in fact we have a reason for this. ARC timers are controlled by auxiliary registers that are not mapped anywhere. They have their fixed numbers and are being accessed using special ARC asm commands. Of course I can write something like this: ---------------------------------->8------------------------------------ timer_base = priv->timer_id ? ARC_AUX_TIMER1_BASE : ARC_AUX_TIMER0_BASE;
write_aux_reg(timer_base + ARC_TIMER_CTRL, NH_MODE); write_aux_reg(timer_base + ARC_TIMER_LIMIT, 0xffffffff); write_aux_reg(timer_base + ARC_TIMER_CNT, 0); ---------------------------------->8------------------------------------ But unfortunately it will not reflect the real life. We don't have any ARC timer base addresses, only fixed register numbers.
Just a subtle clarification.
In ARC world we have addition address space for controlling built-in features of the core. These are so-called AUX[iliary] registers. We access them via special commands like LR/SR (load/store AUX reg) and each AUX reg has its own pre-defined index.
In other words ARC_AUX_TIMER0_CTRL and ARC_AUX_TIMER1_CTRL are 2 very special values/indexes (in terms they are always the same within all ARC cores with the same ISA). Compared to common MMIO peripherals which might be mapped to different memory location these are constant. And IMHO for ARC user it is much more natural to see a particular AUX reg number and then just get all the info about it from ARC core documentation. Otherwise if we start using fake bases it may just mislead a reader.
OK that's fine. I don't want to interfere in how ARC does things. It just stood out as odd :-)
Regards, Simon