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

[Bin Meng]

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
not make sense.

Regards,
Bin