Re: [PATCH v2 2/2] clocksource: Ingenic: Add SMP/SMT support for sysost driver.

[Daniel Lezcano]

Hi,

On 14/06/2021 18:42, Paul Cercueil wrote:
> Hi Zhou,
[ please trim ... ]

>>>  >> The DT documentation only mentions one memory resource. Here, you
>>>  >> map a second one, which is not used anywhere. I'm really confused
>>>  >> about what you're trying to do here.
>>>  >>
>>>  >
>>>  > X2000 and X2100 divide the OST into two parts. The global timer is
>>>  > the first part, which is still located at the address of 0x12000000,
>>>  > and the percpu timers are the second part, the starting address is
>>>  > 0x12100000, and each timer is offset by 0x100 (percpu timer0 is at
>>>  > 0x12100000, percpu timer1 is at 0x12100100, percpu timer2 is at
>>>  > 0x12100200, percpu timer3 is at 0x12100300). This one is used in
>>>  > line 593 of the code.
>>>
>>>  Then you need two different DT nodes, one at each start address.
>>>  Either use different drivers (since the register sets are different),
>>>  or *if* it can be implemented cleanly in ingenic-sysost.c, different
>>>  compatible strings - one for the global timer and one for the percpu
>>>  timers.
>>
>> Sorry, I did not describe it clearly. Although the global timer and
>> percpu timers are divided into two parts, they still belong to the same
>> hardware module. The base address of the entire module is 0x12000000,
>> but the control register of the global timer is not shifted. The percpu
>> timers are shifted by 0x100000 as a whole (the situation here is
>> similar to PDMA, which is also divided into two parts: the
>> channel-related registers are not shifted, while the system
>> control-related registers are shifted by 0x1000 as a whole). I think
>> maybe we can follow PDMA's approach and add corresponding instructions
>> in the DT documentation. This can avoid confusion caused by splitting
>> different parts of the same hardware module into two nodes, and at the
>> same time keep the code of the devicetree as simple and clear as
>> possible.
>>
>> What do you think about that?
> 
> Looking at the programming manual, these are not the same hardware
> module. They are on different addresses, are functionally independent,
> and are even described in different chapters of the PM; so I stick to
> what I said earlier.
> 
> Cheers,
> -Paul
> 
>> Thanks and best regards!
>>
>>>
>>>  -Paul

[ ... ]


-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog