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.

-Alexey
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to