x86: hpet: Don't default CONFIG_HPET_TIMER to be y for X86_64

PATCH NAK Feng Tang 3 participants 15 messages Mar 28, 2014 – Apr 18, 2014

Subsystems (3)

x86 PCI Hpet: x86

lore

Share & tools

lore

Feng Tang

author

2014-03-28 02:56 UTC #0 lore raw

PATCH v2 1/1 x86: hpet: Don't default CONFIG_HPET_TIMER to be y for X86_64

On many new phone/tablet platforms like Baytrail/Merrifield etc, the HPET are either defeatured or has some problem to be used as a reliable timer. As these platforms also have X86_64, we should not make HPET_TIMER default y for all X86_64.

Signed-off-by Feng Tang <[email protected]>

---
 arch/x86/Kconfig |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

arch/x86/Kconfig +3 -4

index 0af5250..7f2fb4b 100644
@@ -695,8 +695,7 @@ config X86_CYCLONE_TIMER
 source "arch/x86/Kconfig.cpu"
 
 config HPET_TIMER
-	def_bool X86_64
-	prompt "HPET Timer Support" if X86_32
+	prompt "HPET Timer Support"
 	---help---
 	  Use the IA-PC HPET (High Precision Event Timer) to manage
 	  time in preference to the PIT and RTC, if a HPET is

@@ -709,9 +708,9 @@ config HPET_TIMER
 
 	  You can safely choose Y here.  However, HPET will only be
 	  activated if the platform and the BIOS support this feature.
-	  Otherwise the 8254 will be used for timing services.
 
-	  Choose N to continue using the legacy 8254 timer.
+	  Choose N to use the legacy 8254 timer or the latest
+	  lapic_timer+tsc solution.
 
 config HPET_EMULATE_RTC
 	def_bool y

Signature · 1 line

1.7.0.4

Ingo Molnar

NAK

2014-03-28 07:17 UTC #1 lore raw

Quoted 4 lines · Feng Tang

On many new phone/tablet platforms like Baytrail/Merrifield etc, the HPET are either defeatured or has some problem to be used as a reliable timer. As these platforms also have X86_64, we should not make HPET_TIMER default y for all X86_64.

NAK!

If the HPET is unreliable on a specific platform then any of the following solutions would address the problem (in order of preference):

- the hardware should not expose it. Why waste silicon on something that does not work?

- or the firmware should not expose it. Why expose something that does not work?

- or the kernel should have a quirk to reliably disable it. Why should we crash or misbehave if a driver is built into the kernel?

tweaking a default is _NOT_ a solution for an unreliable hpet.

Thanks,

Ingo

Feng Tang

author

2014-03-28 07:38 UTC #2 lore raw

Hi Ingo,

Thanks for reviewing this

Quoted 14 lines · Ingo Molnar

Quoted 4 lines · Feng Tang
On many new phone/tablet platforms like Baytrail/Merrifield etc, the HPET are either defeatured or has some problem to be used as a reliable timer. As these platforms also have X86_64, we should not make HPET_TIMER default y for all X86_64.
NAK!
If the HPET is unreliable on a specific platform then any of the following solutions would address the problem (in order of preference):

Quoted 5 lines

- the hardware should not expose it. Why waste silicon on something that does not work?
- or the firmware should not expose it. Why expose something that does not work?

Agreed, I've raised problem to BIOS vendor, but the response is very slow, and those hardware/BIOS may already hit the market as a product.

Quoted 4 lines

- or the kernel should have a quirk to reliably disable it. Why should we crash or misbehave if a driver is built into the kernel?

I thought about this before, HPET doesn't have PCI ID like stuff, only thing I can think of to identify them may be the CPU family/ID. Runtime check the reliability of HPET may be difficult, as we don't know if the 8254 or the TSC are the golden timer to check HPET.

Thanks,
Feng

Clemens Ladisch

2014-03-28 08:08 UTC #3 lore raw

Quoted 7 lines · Feng Tang

Quoted 4 lines · Ingo Molnar
* Feng Tang <[email protected]> wrote:
- or the kernel should have a quirk to reliably disable it. Why should we crash or misbehave if a driver is built into the kernel?
I thought about this before, HPET doesn't have PCI ID like stuff,

HPET does have the PCI vendor ID in the first register.

Quoted 1 line

only thing I can think of to identify them may be the CPU family/ID.

The HPET is implemented by some actual chip, and that chip also has lots of PCI devices. (In the case of a SoC, the CPU ID would work, too).

Regards,
Clemens

Ingo Molnar

2014-03-28 08:11 UTC #4 lore raw

Quoted 7 lines · Clemens Ladisch

Quoted 6 lines · Feng Tang
Quoted 4 lines · Ingo Molnar
* Feng Tang <[email protected]> wrote:
- or the kernel should have a quirk to reliably disable it. Why should we crash or misbehave if a driver is built into the kernel?

Quoted 8 lines

Quoted 1 line
I thought about this before, HPET doesn't have PCI ID like stuff,
HPET does have the PCI vendor ID in the first register.
Quoted 1 line
only thing I can think of to identify them may be the CPU family/ID.
The HPET is implemented by some actual chip, and that chip also has lots of PCI devices. (In the case of a SoC, the CPU ID would work, too).

Correct. See arch/x86/kernel/hpet.c, which has a large number of HPET quirks keyed off chipset PCI IDs:

DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ESB2_0,

                           ich_force_enable_hpet);
  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH6_0,
                           ich_force_enable_hpet);
  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH6_1,
                           ich_force_enable_hpet);

[...]

Thanks,

Ingo

Feng Tang

author

2014-03-28 08:21 UTC #5 lore raw

Quoted 22 lines · Ingo Molnar

Quoted 7 lines · Clemens Ladisch
Quoted 6 lines · Feng Tang
Quoted 4 lines · Ingo Molnar
* Feng Tang <[email protected]> wrote:
- or the kernel should have a quirk to reliably disable it. Why should we crash or misbehave if a driver is built into the kernel?
Quoted 8 lines
Quoted 1 line
I thought about this before, HPET doesn't have PCI ID like stuff,
HPET does have the PCI vendor ID in the first register.
Quoted 1 line
only thing I can think of to identify them may be the CPU family/ID.
The HPET is implemented by some actual chip, and that chip also has lots of PCI devices. (In the case of a SoC, the CPU ID would work, too).
Correct. See arch/x86/kernel/hpet.c, which has a large number of HPET quirks keyed off chipset PCI IDs:

Quoted 7 lines

DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ESB2_0,

                           ich_force_enable_hpet);
  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH6_0,
                           ich_force_enable_hpet);
  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH6_1,
                           ich_force_enable_hpet);

[...]

Got it, thanks Ingo and Clemens for the pointer. Will try to figure it out.

- Feng

Feng Tang

author

2014-04-15 07:45 UTC #6 lore raw

In reply to Ingo Molnar

Hi Ingo,

Quoted 22 lines · Ingo Molnar

Quoted 7 lines · Clemens Ladisch
Quoted 6 lines · Feng Tang
Quoted 4 lines · Ingo Molnar
* Feng Tang <[email protected]> wrote:
- or the kernel should have a quirk to reliably disable it. Why should we crash or misbehave if a driver is built into the kernel?
Quoted 8 lines
Quoted 1 line
I thought about this before, HPET doesn't have PCI ID like stuff,
HPET does have the PCI vendor ID in the first register.
Quoted 1 line
only thing I can think of to identify them may be the CPU family/ID.
The HPET is implemented by some actual chip, and that chip also has lots of PCI devices. (In the case of a SoC, the CPU ID would work, too).
Correct. See arch/x86/kernel/hpet.c, which has a large number of HPET quirks keyed off chipset PCI IDs:

Quoted 7 lines

DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ESB2_0,

                           ich_force_enable_hpet);
  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH6_0,
                           ich_force_enable_hpet);
  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH6_1,
                           ich_force_enable_hpet);

[...]

I just gave it another thought, that the HPET on our platform currently do have some problem to be used as clocksource/clockevent, but it may get fixed in future version (by Silicon or BIOS).

If I add quirk to block it now, I may revert this code in future when it get fixed, same problem applis for the future generation of platform.

So can we do a small change like below, so that we are able to disable the HPET_TIMER in our own x86_64 .config, while all exising distributions are not affected as it is still default "y" .

---

arch/x86/Kconfig +1 -1

index f67e839..a1027a5 100644
@@ -711,7 +711,7 @@ source "arch/x86/Kconfig.cpu"
 
 config HPET_TIMER
 	def_bool X86_64
-	prompt "HPET Timer Support" if X86_32
+	prompt "HPET Timer Support"
 	---help---
 	  Use the IA-PC HPET (High Precision Event Timer) to manage
 	  time in preference to the PIT and RTC, if a HPET is

Thanks,
Feng

Ingo Molnar

2014-04-15 09:00 UTC #7 lore raw

Quoted 33 lines · Feng Tang

Hi Ingo,
Quoted 22 lines · Ingo Molnar
Quoted 7 lines · Clemens Ladisch
Quoted 6 lines · Feng Tang
Quoted 4 lines · Ingo Molnar
* Feng Tang <[email protected]> wrote:
- or the kernel should have a quirk to reliably disable it. Why should we crash or misbehave if a driver is built into the kernel?
Quoted 8 lines
Quoted 1 line
I thought about this before, HPET doesn't have PCI ID like stuff,
HPET does have the PCI vendor ID in the first register.
Quoted 1 line
only thing I can think of to identify them may be the CPU family/ID.
The HPET is implemented by some actual chip, and that chip also has lots of PCI devices. (In the case of a SoC, the CPU ID would work, too).
Correct. See arch/x86/kernel/hpet.c, which has a large number of HPET quirks keyed off chipset PCI IDs:
Quoted 7 lines
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ESB2_0,
                           ich_force_enable_hpet);
  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH6_0,
                           ich_force_enable_hpet);
  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH6_1,
                           ich_force_enable_hpet);
[...]

Quoted 7 lines

I just gave it another thought, that the HPET on our platform currently do have some problem to be used as clocksource/clockevent, but it may get fixed in future version (by Silicon or BIOS).
If I add quirk to block it now, I may revert this code in future when it get fixed, same problem applis for the future generation of platform.

If the hardware or BIOS gets fixed then that will be visible in the revision level of the hardware, right?

Such kind of revision level, once it is known, can then be used to turn the quirk on/off precisely.

Thanks,

Ingo

Feng Tang

author

2014-04-15 09:56 UTC #8 lore raw

Quoted 40 lines · Ingo Molnar

Quoted 37 lines · Feng Tang
Hi Ingo,
Quoted 22 lines · Ingo Molnar
Quoted 7 lines · Clemens Ladisch
Quoted 6 lines · Feng Tang
Quoted 4 lines · Ingo Molnar
* Feng Tang <[email protected]> wrote:
- or the kernel should have a quirk to reliably disable it. Why should we crash or misbehave if a driver is built into the kernel?
Quoted 8 lines
Quoted 1 line
I thought about this before, HPET doesn't have PCI ID like stuff,
HPET does have the PCI vendor ID in the first register.
Quoted 1 line
only thing I can think of to identify them may be the CPU family/ID.
The HPET is implemented by some actual chip, and that chip also has lots of PCI devices. (In the case of a SoC, the CPU ID would work, too).
Correct. See arch/x86/kernel/hpet.c, which has a large number of HPET quirks keyed off chipset PCI IDs:
Quoted 7 lines
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ESB2_0,
                           ich_force_enable_hpet);
  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH6_0,
                           ich_force_enable_hpet);
  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH6_1,
                           ich_force_enable_hpet);
[...]
I just gave it another thought, that the HPET on our platform currently do have some problem to be used as clocksource/clockevent, but it may get fixed in future version (by Silicon or BIOS).

Quoted 6 lines

Quoted 3 lines
If I add quirk to block it now, I may revert this code in future when it get fixed, same problem applis for the future generation of platform.
If the hardware or BIOS gets fixed then that will be visible in the revision level of the hardware, right?

AFAIK, if it is fixed in a new silicon version, we should be able to detect it by the "stepping", but the PCI DEV ID is not likely to change.

If it is fixed by BIOS or PUNIT FW, then we may go through the DMI info to check the BIOS version, One problem is there is some Baytrail tablet platforms that doesn't provide DMI info as its FW is not UEFI/Legacy BIOS compatible version.

Thanks,
Feng

Quoted 7 lines

Such kind of revision level, once it is known, can then be used to turn the quirk on/off precisely.
Thanks,
Ingo

Ingo Molnar

2014-04-15 10:42 UTC #9 lore raw

Quoted 48 lines · Feng Tang

Quoted 40 lines · Ingo Molnar
Quoted 37 lines · Feng Tang
Hi Ingo,
Quoted 22 lines · Ingo Molnar
Quoted 7 lines · Clemens Ladisch
Quoted 6 lines · Feng Tang
Quoted 4 lines · Ingo Molnar
* Feng Tang <[email protected]> wrote:
- or the kernel should have a quirk to reliably disable it. Why should we crash or misbehave if a driver is built into the kernel?
Quoted 8 lines
Quoted 1 line
I thought about this before, HPET doesn't have PCI ID like stuff,
HPET does have the PCI vendor ID in the first register.
Quoted 1 line
only thing I can think of to identify them may be the CPU family/ID.
The HPET is implemented by some actual chip, and that chip also has lots of PCI devices. (In the case of a SoC, the CPU ID would work, too).
Correct. See arch/x86/kernel/hpet.c, which has a large number of HPET quirks keyed off chipset PCI IDs:
Quoted 7 lines
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ESB2_0,
                           ich_force_enable_hpet);
  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH6_0,
                           ich_force_enable_hpet);
  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH6_1,
                           ich_force_enable_hpet);
[...]
I just gave it another thought, that the HPET on our platform currently do have some problem to be used as clocksource/clockevent, but it may get fixed in future version (by Silicon or BIOS).
Quoted 6 lines
Quoted 3 lines
If I add quirk to block it now, I may revert this code in future when it get fixed, same problem applis for the future generation of platform.
If the hardware or BIOS gets fixed then that will be visible in the revision level of the hardware, right?

Quoted 8 lines

AFAIK, if it is fixed in a new silicon version, we should be able to detect it by the "stepping", but the PCI DEV ID is not likely to change.
If it is fixed by BIOS or PUNIT FW, then we may go through the DMI info to check the BIOS version, One problem is there is some Baytrail tablet platforms that doesn't provide DMI info as its FW is not UEFI/Legacy BIOS compatible version.

So just turn it off for the current hardware, via PCI ID. When you get a fix, you can turn it back on again by refining the quirk check, for that specific hardware.

Thanks,

Ingo

Feng Tang

author

2014-04-15 14:13 UTC #10 lore raw

Quoted 55 lines · Ingo Molnar

Quoted 52 lines · Feng Tang
Quoted 40 lines · Ingo Molnar
Quoted 37 lines · Feng Tang
Hi Ingo,
Quoted 22 lines · Ingo Molnar
Quoted 7 lines · Clemens Ladisch
Quoted 6 lines · Feng Tang
Quoted 4 lines · Ingo Molnar
* Feng Tang <[email protected]> wrote:
- or the kernel should have a quirk to reliably disable it. Why should we crash or misbehave if a driver is built into the kernel?
Quoted 8 lines
Quoted 1 line
I thought about this before, HPET doesn't have PCI ID like stuff,
HPET does have the PCI vendor ID in the first register.
Quoted 1 line
only thing I can think of to identify them may be the CPU family/ID.
The HPET is implemented by some actual chip, and that chip also has lots of PCI devices. (In the case of a SoC, the CPU ID would work, too).
Correct. See arch/x86/kernel/hpet.c, which has a large number of HPET quirks keyed off chipset PCI IDs:
Quoted 7 lines
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ESB2_0,
                           ich_force_enable_hpet);
  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH6_0,
                           ich_force_enable_hpet);
  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH6_1,
                           ich_force_enable_hpet);
[...]
I just gave it another thought, that the HPET on our platform currently do have some problem to be used as clocksource/clockevent, but it may get fixed in future version (by Silicon or BIOS).
Quoted 6 lines
Quoted 3 lines
If I add quirk to block it now, I may revert this code in future when it get fixed, same problem applis for the future generation of platform.
If the hardware or BIOS gets fixed then that will be visible in the revision level of the hardware, right?
AFAIK, if it is fixed in a new silicon version, we should be able to detect it by the "stepping", but the PCI DEV ID is not likely to change.

Quoted 8 lines

Quoted 4 lines
If it is fixed by BIOS or PUNIT FW, then we may go through the DMI info to check the BIOS version, One problem is there is some Baytrail tablet platforms that doesn't provide DMI info as its FW is not UEFI/Legacy BIOS compatible version.
So just turn it off for the current hardware, via PCI ID. When you get a fix, you can turn it back on again by refining the quirk check, for that specific hardware.

got it, thanks!

- Feng

Ingo Molnar

2014-04-16 06:51 UTC #11 lore raw

Quoted 61 lines · Feng Tang

Quoted 60 lines · Ingo Molnar
Quoted 56 lines · Feng Tang
Quoted 40 lines · Ingo Molnar
Quoted 37 lines · Feng Tang
Hi Ingo,
Quoted 22 lines · Ingo Molnar
Quoted 7 lines · Clemens Ladisch
Quoted 6 lines · Feng Tang
Quoted 4 lines · Ingo Molnar
* Feng Tang <[email protected]> wrote:
- or the kernel should have a quirk to reliably disable it. Why should we crash or misbehave if a driver is built into the kernel?
Quoted 8 lines
Quoted 1 line
I thought about this before, HPET doesn't have PCI ID like stuff,
HPET does have the PCI vendor ID in the first register.
Quoted 1 line
only thing I can think of to identify them may be the CPU family/ID.
The HPET is implemented by some actual chip, and that chip also has lots of PCI devices. (In the case of a SoC, the CPU ID would work, too).
Correct. See arch/x86/kernel/hpet.c, which has a large number of HPET quirks keyed off chipset PCI IDs:
Quoted 7 lines
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ESB2_0,
                           ich_force_enable_hpet);
  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH6_0,
                           ich_force_enable_hpet);
  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH6_1,
                           ich_force_enable_hpet);
[...]
I just gave it another thought, that the HPET on our platform currently do have some problem to be used as clocksource/clockevent, but it may get fixed in future version (by Silicon or BIOS).
Quoted 6 lines
Quoted 3 lines
If I add quirk to block it now, I may revert this code in future when it get fixed, same problem applis for the future generation of platform.
If the hardware or BIOS gets fixed then that will be visible in the revision level of the hardware, right?
AFAIK, if it is fixed in a new silicon version, we should be able to detect it by the "stepping", but the PCI DEV ID is not likely to change.
If it is fixed by BIOS or PUNIT FW, then we may go through the DMI info to check the BIOS version, One problem is there is some Baytrail tablet platforms that doesn't provide DMI info as its FW is not UEFI/Legacy BIOS compatible version.

Quoted 5 lines

Quoted 3 lines
So just turn it off for the current hardware, via PCI ID. When you get a fix, you can turn it back on again by refining the quirk check, for that specific hardware.
got it, thanks!

Btw., note that such narrow quirk refinings are pretty uncontroversial and have a pretty fast path upstream, once you have the initial quirk in there.

Thanks,

Ingo

Feng Tang

author

2014-04-18 07:19 UTC #12 lore raw

Hi All,

Quoted 47 lines · Ingo Molnar

Quoted 45 lines · Feng Tang
Quoted 44 lines
Quoted 32 lines
Quoted 25 lines
Quoted 25 lines
Quoted 13 lines
Quoted 9 lines
Quoted 2 lines
I thought about this before, HPET doesn't have PCI ID like stuff,
HPET does have the PCI vendor ID in the first register.
Quoted 1 line
only thing I can think of to identify them may be the CPU family/ID.
The HPET is implemented by some actual chip, and that chip also has lots of PCI devices. (In the case of a SoC, the CPU ID would work, too).
Correct. See arch/x86/kernel/hpet.c, which has a large number of HPET quirks keyed off chipset PCI IDs:
Quoted 7 lines
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ESB2_0,
                           ich_force_enable_hpet);
  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH6_0,
                           ich_force_enable_hpet);
  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH6_1,
                           ich_force_enable_hpet);
[...]
I just gave it another thought, that the HPET on our platform currently do have some problem to be used as clocksource/clockevent, but it may get fixed in future version (by Silicon or BIOS).
Quoted 6 lines
Quoted 3 lines
If I add quirk to block it now, I may revert this code in future when it get fixed, same problem applis for the future generation of platform.
If the hardware or BIOS gets fixed then that will be visible in the revision level of the hardware, right?
Quoted 8 lines
AFAIK, if it is fixed in a new silicon version, we should be able to detect it by the "stepping", but the PCI DEV ID is not likely to change.
If it is fixed by BIOS or PUNIT FW, then we may go through the DMI info to check the BIOS version, One problem is there is some Baytrail tablet platforms that doesn't provide DMI info as its FW is not UEFI/Legacy BIOS compatible version.
So just turn it off for the current hardware, via PCI ID. When you get a fix, you can turn it back on again by refining the quirk check, for that specific hardware.

Quoted 5 lines

Quoted 1 line
got it, thanks!
Btw., note that such narrow quirk refinings are pretty uncontroversial and have a pretty fast path upstream, once you have the initial quirk in there.

During implementing, I found the arch/x86/kerne/quirks.c doesn't fit well as the DECLARE_PCI_FIXUP_HEADER/EARLY/FINAL() way of quirk happens inside the pci subsystem init phase, while the hpet_enable() will be called before that.

Then by searching the arch/x86/, the early-quirks.c can be used to add such a quirk as the early quirks are checked earlier than hpet_enable()

I'll post a RFC patch soon.

Thanks,
Feng

Feng Tang

author

2014-04-18 07:50 UTC #13 lore raw

RFC PATCH 1/1 x86, hpet: Add quirk to disable HPET for baytrail platform

In reply to Ingo Molnar

Hi All,

Please help to review this RFC patch, if you are ok with it. I'll separte them into 2 and repost.
1. make the boot_hpet_disable extern
2. add quirk for baytrail HPET.
thanks!

- Feng

Quoted 1 line

From 4b8c370649f1d2b320aa46a005b8b61eb59a539e Mon Sep 17 00:00:00 2001

From: Feng Tang <[email protected]>
Date: Fri, 18 Apr 2014 15:43:22 +0800
Subject: [PATCH] x86, hpet: Add quirk to disable HPET for baytrail platform

HPET on current baytrail platform has accuracy problem to be used as reliable clocksource/clockevent, so add a early quirk to disable it.

Signed-off-by Feng Tang <[email protected]>

---
 arch/x86/include/asm/hpet.h    |    1 +
 arch/x86/kernel/early-quirks.c |   14 ++++++++++++++
 arch/x86/kernel/hpet.c         |    2 +-
 3 files changed, 16 insertions(+), 1 deletions(-)

arch/x86/include/asm/hpet.h +1 -0

index b18df57..36f7125 100644
@@ -63,6 +63,7 @@
 /* hpet memory map physical address */
 extern unsigned long hpet_address;
 extern unsigned long force_hpet_address;
+extern int boot_hpet_disable;
 extern u8 hpet_blockid;
 extern int hpet_force_user;
 extern u8 hpet_msi_disable;

arch/x86/kernel/early-quirks.c +14 -0

index bc4a088..1e5f6bc 100644
@@ -17,6 +17,7 @@
 #include <asm/dma.h>
 #include <asm/io_apic.h>
 #include <asm/apic.h>
+#include <asm/hpet.h>
 #include <asm/iommu.h>
 #include <asm/gart.h>
 #include <asm/irq_remapping.h>

@@ -380,6 +381,13 @@ static void __init intel_graphics_stolen(int num, int slot, int func)
 	}
 }
 
+static void __init force_disable_hpet(int num, int slot, int func)
+{
+	boot_hpet_disable = 1;
+	pr_info("Will Disable HPET for this platform\n");
+}
+
+
 #define QFLAG_APPLY_ONCE 	0x1
 #define QFLAG_APPLIED		0x2
 #define QFLAG_DONE		(QFLAG_APPLY_ONCE|QFLAG_APPLIED)

@@ -417,6 +425,12 @@ static struct chipset early_qrk[] __initdata = {
 	  PCI_BASE_CLASS_BRIDGE, 0, intel_remapping_check },
 	{ PCI_VENDOR_ID_INTEL, PCI_ANY_ID, PCI_CLASS_DISPLAY_VGA, PCI_ANY_ID,
 	  QFLAG_APPLY_ONCE, intel_graphics_stolen },
+	/*
+	 * HPET on current version of Baytrail platform has accuracy
+	 * problem, disable it for now
+	 */
+	{ PCI_VENDOR_ID_INTEL, 0x0f00,
+		PCI_CLASS_BRIDGE_HOST, PCI_ANY_ID, 0, force_disable_hpet},
 	{}
 };

arch/x86/kernel/hpet.c +1 -1

index da85a8e..913e207 100644
@@ -88,7 +88,7 @@ static inline void hpet_clear_mapping(void)
 /*
  * HPET command line enable / disable
  */
-static int boot_hpet_disable;
+int boot_hpet_disable;
 int hpet_force_user;
 static int hpet_verbose;

Signature · 1 line

1.7.0.4

Ingo Molnar

2014-04-18 08:15 UTC #14 lore raw

Re: [RFC PATCH] x86, hpet: Add quirk to disable HPET for baytrail platform

Quoted 83 lines · Feng Tang

Hi All,

Please help to review this RFC patch, if you are ok with it. I'll separte them into 2 and repost.
1. make the boot_hpet_disable extern
2. add quirk for baytrail HPET.
thanks!

- Feng

From 4b8c370649f1d2b320aa46a005b8b61eb59a539e Mon Sep 17 00:00:00 2001
From: Feng Tang <[email protected]>
Date: Fri, 18 Apr 2014 15:43:22 +0800
Subject: [PATCH] x86, hpet: Add quirk to disable HPET for baytrail platform

HPET on current baytrail platform has accuracy problem to be used as reliable clocksource/clockevent, so add a early quirk to disable it.

Signed-off-by Feng Tang <[email protected]>

---
 arch/x86/include/asm/hpet.h    |    1 +
 arch/x86/kernel/early-quirks.c |   14 ++++++++++++++
 arch/x86/kernel/hpet.c         |    2 +-
 3 files changed, 16 insertions(+), 1 deletions(-)

arch/x86/include/asm/hpet.h +1 -0

index b18df57..36f7125 100644
@@ -63,6 +63,7 @@
 /* hpet memory map physical address */
 extern unsigned long hpet_address;
 extern unsigned long force_hpet_address;
+extern int boot_hpet_disable;
 extern u8 hpet_blockid;
 extern int hpet_force_user;
 extern u8 hpet_msi_disable;

arch/x86/kernel/early-quirks.c +14 -0

index bc4a088..1e5f6bc 100644
@@ -17,6 +17,7 @@
 #include <asm/dma.h>
 #include <asm/io_apic.h>
 #include <asm/apic.h>
+#include <asm/hpet.h>
 #include <asm/iommu.h>
 #include <asm/gart.h>
 #include <asm/irq_remapping.h>

@@ -380,6 +381,13 @@ static void __init intel_graphics_stolen(int num, int slot, int func)
 	}
 }
 
+static void __init force_disable_hpet(int num, int slot, int func)
+{
+	boot_hpet_disable = 1;
+	pr_info("Will Disable HPET for this platform\n");
+}
+
+
 #define QFLAG_APPLY_ONCE 	0x1
 #define QFLAG_APPLIED		0x2
 #define QFLAG_DONE		(QFLAG_APPLY_ONCE|QFLAG_APPLIED)

@@ -417,6 +425,12 @@ static struct chipset early_qrk[] __initdata = {
 	  PCI_BASE_CLASS_BRIDGE, 0, intel_remapping_check },
 	{ PCI_VENDOR_ID_INTEL, PCI_ANY_ID, PCI_CLASS_DISPLAY_VGA, PCI_ANY_ID,
 	  QFLAG_APPLY_ONCE, intel_graphics_stolen },
+	/*
+	 * HPET on current version of Baytrail platform has accuracy
+	 * problem, disable it for now
+	 */
+	{ PCI_VENDOR_ID_INTEL, 0x0f00,
+		PCI_CLASS_BRIDGE_HOST, PCI_ANY_ID, 0, force_disable_hpet},
 	{}
 };

arch/x86/kernel/hpet.c +0 -0

index da85a8e..913e207 100644

arch/x86/kernel/hpet.c static inline void hpet_clear_mapping(void) · line 93

Quoted 7 lines

arch/x86/kernel/hpet.c +1 -1

@@ -88,7 +88,7 @@ static inline void hpet_clear_mapping(void)
 /*
  * HPET command line enable / disable
  */
-static int boot_hpet_disable;
+int boot_hpet_disable;
 int hpet_force_user;
 static int hpet_verbose;

As long as all those PCI IDs have a buggy HPET, this looks good to me.

hpa, tglx, any preferences?

Thanks,

Ingo