Re: [RFC] TSC patch v2

[Colin D Bennett <colin@gibibit.com>]

[-- Attachment #1: Type: text/plain, Size: 2240 bytes --]

On Fri, 04 Jul 2008 22:49:59 +0300
Vesa Jääskeläinen <chaac@nic.fi> wrote:

> Hi Colin,
> 
> Colin D Bennett wrote:
> > Here is an updated version of the TSC patch.
> > 
> > I addressed the issues in your comments with the exception of
> > supporting x86 CPUs that don't have a TSC (386 and 486).
> > 
> > I eliminated the grub_time_init() function and the call to it in
> > grub_main() in favor of having grub_machine_init() simply call
> > grub_tsc_calibrate().  The i386-pc and i386-efi platforms use the
> > TSC.
> 
> Please still use grub_time_init(). Detect TSC, if it is there
> calibrate and adapt for it. If it is not there, use RTC.

Should grub_time_init() be a CPU or machine independent function?
Should it use conditional compilation to be usable on different CPUs?
Or should grub_time_init() be specific to i386 kernels at this point?
(Since we don't have anything to initialize for non-TSC-supporting
platforms.)

If grub_time_init() is specific to the i386 CPU, then grub_time_init()
should check whether the CPU has TSC support, and if so,
grub_tsc_calibrate() should be called, and calls to grub_get_time_ms()
should be directed to the TSC implementation.  If no TSC support is
available, then no calibration is done, and calls to grub_get_time_ms()
should be directed to the generic get_time_ms() implementation that
uses grub_get_rtc() to do its job.

I want to make sure that I clearly understand what you want, since I
have made major re-designs of the TSC support in GRUB in an attempt to
do what you want.  Just in the past couple of days I have spent about 8
hours getting the TSC patch where it is now. (I know that seems
like a lot since it's not that much code, but I am just beginning to get
familiar with the GRUB kernel organization, so simple changes have been
taking me a long time.)

> grub_time_init() is just more clearer term than tsc. At least for
> people that are not familiar with it. in grub_time_init() you can
> have anything then. And people that are not interested on time stuff
> do not need to bother their heads about it. Also modifying logic
> afterwards do not change other parts and it will be less error prone.

OK.

Regards,
Colin

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 197 bytes --]