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 :-)
Regards,
Simon
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
