Re: [PATCH 1/6] dt-bindings: timer: add Tegra210 timer

[Jon Hunter]

On 25/01/2019 03:23, Joseph Lo wrote:
> Hi Jon,
> 
> Thanks for reviewing.
> 
> On 1/24/19 6:30 PM, Jon Hunter wrote:
>>
>> On 07/01/2019 03:28, Joseph Lo wrote:
>>> The Tegra210 timer provides fourteen 29-bit timer counters and one
>>> 32-bit
>>> timestamp counter. The TMRs run at either a fixed 1 MHz clock rate
>>> derived
>>> from the oscillator clock (TMR0-TMR9) or directly at the oscillator
>>> clock
>>> (TMR10-TMR13). Each TMR can be programmed to generate one-shot periodic,
>>> or watchdog interrupts.
>>>
>>> Cc: Daniel Lezcano <daniel.lezcano@xxxxxxxxxx>
>>> Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
>>> Cc: linux-kernel@xxxxxxxxxxxxxxx
>>> Cc: devicetree@xxxxxxxxxxxxxxx
>>> Signed-off-by: Joseph Lo <josephl@xxxxxxxxxx>
>>> ---
>>>  .../bindings/timer/nvidia,tegra210-timer.txt | 25 +++++++++++++++++++
>>> Â 1 file changed, 25 insertions(+)
>>> Â create mode 100644
>>> Documentation/devicetree/bindings/timer/nvidia,tegra210-timer.txt
>>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/timer/nvidia,tegra210-timer.txt
>>> b/Documentation/devicetree/bindings/timer/nvidia,tegra210-timer.txt
>>> new file mode 100644
>>> index 000000000000..ba511220a669
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/timer/nvidia,tegra210-timer.txt
>>> @@ -0,0 +1,25 @@
>>> +NVIDIA Tegra210 timer
>>> +
>>> +The Tegra210 timer provides fourteen 29-bit timer counters and one
>>> 32-bit
>>> +timestamp counter. The TMRs run at either a fixed 1 MHz clock rate
>>> derived
>>> +from the oscillator clock (TMR0-TMR9) or directly at the oscillator
>>> clock
>>> +(TMR10-TMR13). Each TMR can be programmed to generate one-shot,
>>> periodic,
>>> +or watchdog interrupts.
>>> +
>>> +Required properties:
>>> +- compatible : "nvidia,tegra210-timer".
>>> +- reg : Specifies base physical address and size of the registers.
>>> +- interrupts : A list of 4 interrupts; one per each of TMR10 through
>>> TMR13.
>>
>> Why do we only add the interrupts for TMR10 - TMR13? What about the
>> others?
>>
> 
> The others (TMR0-TMR9) are occupied for other usages. TMR5 is occupied
> for the watchdog timer in the upstream kernel. And others (still in
> TMR0-TMR9) are occupied for different usages in our downstream kernel.

Where is TMR5 reserved for the watchdog? I don't see this?

> And notice that only TMR10-TMR13 are running at the oscillator clock
> (clk_m). With the Tegra210 timer driver, we introduce in this series,
> which only replace the clock event device function that was originally
> owned by the arch timer (armv8 timer) and it also running at the
> oscillator clock. The sched_timer still owns by the arch timer. So the
> timer resolution will be the same. That's why we choose TMR10-TMR13 as
> the timer for Tegra210.

That maybe fine, but DT should describe the hardware and so I don't see
why we would not list all the interrupts. We can still only use TMR10-13
in the driver.

Cheers
Jon

-- 
nvpublic