Intel: How to share `tsc_freq.c` to select `UDELAY_TSC`

[Paul Menzel]

Dear coreboot folks,

after having looked at how to port Aaron’s commit

commit 7cb1ba9a61b244800eb65c08729f75d85a504de3
        Author: Aaron Durbin 
        Date:   Wed May 1 15:39:28 2013 -0500

haswell: use tsc for udelay()

Instead of using the local apic timer for udelay() use the
        tsc.
            That way SMM, romstage, and ramstage all use the same delay
            functionality.

Reviewed-on: http://review.coreboot.org/3169

to the i945-based T60, it looks like the following code

+#include <stdint.h>
        +#include <cpu/x86/msr.h>
        +#include <cpu/x86/tsc.h>
        +#include "cpu/intel/haswell/haswell.h"
        +
        +unsigned long tsc_freq_mhz(void)
        +{
        +       msr_t platform_info;
        +
        +       platform_info = rdmsr(MSR_PLATFORM_INFO);
        +       return HASWELL_BCLK * ((platform_info.lo >> 8) & 0xff);
        +}

is generic enough and can be used by the northbridges i945, i5000 and
Sandy Bridge as

#define MSR_PLATFORM_INFO               0xce

is true for the three of them.

So where should `tsc_freq.c` be put and how should the header be
included?

Thanks,

Paul

[Paul Menzel]

Dear coreboot folks,

Am Mittwoch, den 08.05.2013, 15:00 +0200 schrieb Paul Menzel:
> after having looked at how to port Aaron’s commit
>     commit 7cb1ba9a61b244800eb65c08729f75d85a504de3
>     Author: Aaron Durbin <[e-mail]>
>     Date:   Wed May 1 15:39:28 2013 -0500
>
>         haswell: use tsc for udelay()
>
>         Instead of using the local apic timer for udelay() use the
>     tsc.
>         That way SMM, romstage, and ramstage all use the same delay
>         functionality.
>
>         Reviewed-on: http://review.coreboot.org/3169
>
> to the i945-based T60, it looks like the following code
>     +#include <stdint.h>
>     +#include <cpu/x86/msr.h>
>     +#include <cpu/x86/tsc.h>
>     +#include "cpu/intel/haswell/haswell.h"
>     +
>     +unsigned long tsc_freq_mhz(void)
>     +{
>     +       msr_t platform_info;
>     +
>     +       platform_info = rdmsr(MSR_PLATFORM_INFO);
>     +       return HASWELL_BCLK * ((platform_info.lo >> 8) & 0xff);
>     +}
>
> is generic enough and can be used by the northbridges i945, i5000 and
> Sandy Bridge as
>     #define MSR_PLATFORM_INFO               0xce
>
> is true for the three of them.

that was incorrect. i945 and i5000 have the following code.

msr = rdmsr(0x198);
    divisor = (msr.hi >> 8) & 0x1f;
> So where should `tsc_freq.c` be put and how should the header be
> included?

If two separate files have to be used, what would be a good naming?
`tsc_freq_0x198.c` and `tsc_freq_0xce.c` is not very intuitive.

Thanks,

Paul

Intel: How to share `tsc_freq.c` to select `UDELAY_TSC`

[Aaron Durbin]

Paul,

I think what you'll find is that determining this is cpu specific. I'm
fairly sure it's not worth trying to generalize it.  The
implementations are already associated with the chipset/cpu code. Why
move them?

-Aaron

On Wed, May 8, 2013 at 8:07 AM, Paul Menzel
 wrote:
> Dear coreboot folks,
>
> Am Mittwoch, den 08.05.2013, 15:00 +0200 schrieb Paul Menzel:
> > after having looked at how to port Aaron’s commit
> >     commit 7cb1ba9a61b244800eb65c08729f75d85a504de3
> >     Author: Aaron Durbin <[e-mail]>
> >     Date:   Wed May 1 15:39:28 2013 -0500
> >
> >         haswell: use tsc for udelay()
> >
> >         Instead of using the local apic timer for udelay() use the
> >     tsc.
> >         That way SMM, romstage, and ramstage all use the same delay
> >         functionality.
> >
> >         Reviewed-on: http://review.coreboot.org/3169
> >
> > to the i945-based T60, it looks like the following code
> >     +#include <stdint.h>
> >     +#include <cpu/x86/msr.h>
> >     +#include <cpu/x86/tsc.h>
> >     +#include "cpu/intel/haswell/haswell.h"
> >     +
> >     +unsigned long tsc_freq_mhz(void)
> >     +{
> >     +       msr_t platform_info;
> >     +
> >     +       platform_info = rdmsr(MSR_PLATFORM_INFO);
> >     +       return HASWELL_BCLK * ((platform_info.lo >> 8) & 0xff);
> >     +}
> >
> > is generic enough and can be used by the northbridges i945, i5000 and
> > Sandy Bridge as
> >     #define MSR_PLATFORM_INFO               0xce
> >
> > is true for the three of them.
>
> that was incorrect. i945 and i5000 have the following code.
>     msr = rdmsr(0x198);
>     divisor = (msr.hi >> 8) & 0x1f;
>
> > So where should `tsc_freq.c` be put and how should the header be
> > included?
>
> If two separate files have to be used, what would be a good naming?
> `tsc_freq_0x198.c` and `tsc_freq_0xce.c` is not very intuitive.
>
> Thanks,
>
> Paul