Hi Simon,
On Mon, 2017-02-06 at 07:33 -0800, Simon Glass wrote:
> Hi Alexey,
>
> On 31 January 2017 at 07:35, Alexey Brodkin <alexey.brod...@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 :-)
That was the whole point so people get it right :)
Maybe there's some sense in adding more comments to this section
to justify that solution.
-Alexey
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
