[U-Boot] [PATCH v5 2/2] timer: Add High Precision Event Timers (HPET) support

[Simon Glass]

Hi Bin,

On 15 April 2018 at 23:06, Bin Meng <bmeng.cn at gmail.com> wrote:
> Hi Simon,
>
> On Fri, Apr 13, 2018 at 12:42 AM, Simon Glass <sjg at chromium.org> wrote:
>> Hi,
>>
>> On 8 April 2018 at 19:22, Bin Meng <bmeng.cn at gmail.com> wrote:
>>> Hi Ivan,
>>>
>>> On Sat, Apr 7, 2018 at 3:18 AM, Ivan Gorinov <ivan.gorinov at intel.com> wrote:
>>>> Add HPET driver as an alternative timer for x86 (default is TSC).
>>>> HPET counter has constant frequency and does not need calibration.
>>>> This change also makes TSC timer driver optional on x86.
>>>> New HPET driver can also be selected as the early timer on x86.
>>>>
>>>> HPET can be selected as the tick timer in the Device Tree "chosen" node:
>>>>
>>>>     /include/ "hpet.dtsi"
>>>>
>>>> ...
>>>>
>>>>     chosen {
>>>>         tick-timer = "/hpet";
>>>>     };
>>>>
>>>> Signed-off-by: Ivan Gorinov <ivan.gorinov at intel.com>
>>>> ---
>>>>  arch/Kconfig               |   2 +-
>>>>  arch/x86/Kconfig           |  21 ++++++
>>>>  arch/x86/dts/hpet.dtsi     |   7 ++
>>>>  drivers/timer/Kconfig      |   9 +++
>>>>  drivers/timer/Makefile     |   1 +
>>>>  drivers/timer/hpet_timer.c | 179 +++++++++++++++++++++++++++++++++++++++++++++
>>>>  drivers/timer/tsc_timer.c  |   8 ++
>>>>  7 files changed, 226 insertions(+), 1 deletion(-)
>>>>  create mode 100644 arch/x86/dts/hpet.dtsi
>>>>  create mode 100644 drivers/timer/hpet_timer.c
>>>>
>>
>> [..]
>>>> diff --git a/drivers/timer/tsc_timer.c b/drivers/timer/tsc_timer.c
>>>> index 9296de6..bd0e75c 100644
>>>> --- a/drivers/timer/tsc_timer.c
>>>> +++ b/drivers/timer/tsc_timer.c
>>>> @@ -277,6 +277,8 @@ success:
>>>>         return delta / 1000;
>>>>  }
>>>>
>>>> +#ifdef CONFIG_X86_EARLY_TIMER_TSC
>>>
>>> Why do we surround the following APIs with CONFIG_X86_EARLY_TIMER_TSC?
>>> These APIs are generic U-Boot timer APIs. If we select
>>> CONFIG_X86_EARLY_TIMER_HPET, these APIs are not available and will
>>> cause build error.
>>>
>>> Simon, do you think we should fix such in the timer uclass driver?
>>
>> We should not have arch-specific code in the uclass, or in any generic driver.
>
> What I meant is these APIs like get_timer(). Can such APIs be moved to
> timer uclass driver? To avoid conflicts with other timer drivers, in
> this patch it was solved by using the config option
> CONFIG_X86_EARLY_TIMER_TSC to hide this in the TSC driver, which does

Well get_timer() just calls get_ticks() which uses DM, so what exactly
are you proposing here?

> not make sense.
>
> Regards,
> Bin

Regards,
Simon