All of lore.kernel.org
 help / color / mirror / Atom feed
From: Feng Tang <feng.tang@intel.com>
To: Ingo Molnar <mingo@kernel.org>
Cc: Clemens Ladisch <clemens@ladisch.de>,
	tglx@linutronix.de, mingo@elte.hu, hpa@zytor.com,
	linux-kernel@vger.kernel.org,
	John Stultz <john.stultz@linaro.org>,
	Andy Lutomirski <luto@amacapital.net>
Subject: Re: [PATCH v2] x86: hpet: Don't default CONFIG_HPET_TIMER to be y for X86_64
Date: Fri, 18 Apr 2014 15:18:23 +0800	[thread overview]
Message-ID: <20140418071823.GA4533@feng-snb> (raw)
In-Reply-To: <20140416065148.GB1193@gmail.com>

Hi All,

On Wed, Apr 16, 2014 at 08:51:48AM +0200, Ingo Molnar wrote:
> 
> * Feng Tang <feng.tang@intel.com> wrote:
 > > > > > > > >
> > > > > > > > > I thought about this before, HPET doesn't have PCI ID like stuff,
> > > > > > > > 
> > > > > > > > HPET does have the PCI vendor ID in the first register.
> > > > > > > > 
> > > > > > > > > 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);
> > > > > > >   [...]
> > > > > > 
> > > > > > 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?
> > > > 
> > > > 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.
> > 
> > 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


  reply	other threads:[~2014-04-18  7:19 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-03-28  2:55 [PATCH v2] x86: hpet: Don't default CONFIG_HPET_TIMER to be y for X86_64 Feng Tang
2014-03-28  7:17 ` Ingo Molnar
2014-03-28  7:37   ` Feng Tang
2014-03-28  8:08     ` Clemens Ladisch
2014-03-28  8:11       ` Ingo Molnar
2014-03-28  8:20         ` Feng Tang
2014-04-15  7:44         ` Feng Tang
2014-04-15  9:00           ` Ingo Molnar
2014-04-15  9:55             ` Feng Tang
2014-04-15 10:42               ` Ingo Molnar
2014-04-15 14:12                 ` Feng Tang
2014-04-16  6:51                   ` Ingo Molnar
2014-04-18  7:18                     ` Feng Tang [this message]
2014-04-18  7:49                     ` [RFC PATCH] x86, hpet: Add quirk to disable HPET for baytrail platform Feng Tang
2014-04-18  8:15                       ` Ingo Molnar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140418071823.GA4533@feng-snb \
    --to=feng.tang@intel.com \
    --cc=clemens@ladisch.de \
    --cc=hpa@zytor.com \
    --cc=john.stultz@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@amacapital.net \
    --cc=mingo@elte.hu \
    --cc=mingo@kernel.org \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.