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.
If you insist I will use the latter code snippet. But from our point of view the code is being duplicated in this patch is very small but helps to understand what is really happening with ARC timers much better.
Thanks.