[PATCH] Revert commit e8aa4667baf74dfd85fbaab86861465acb811085

[PATCH] Revert commit e8aa4667baf74dfd85fbaab86861465acb811085

[Andreas Herrmann]

This reverts commit e8aa4667baf74dfd85fbaab86861465acb811085
 (x86: enable hpet=force for AMD SB400)

Since ATI/AMD decided not to support HPET on SB4xx it doesn't
make sense to enable this unsupported feature.
(I was not aware of this when submitting the quirk.)

If a system with SB4xx chipset provides an ACPI HPET table and does
not boot, "nohpet" should be used as kernel parameter.

Signed-off-by: Andreas Herrmann <andreas.herrmann3@amd.com>
---
 arch/x86/kernel/quirks.c |   34 ----------------------------------
 1 files changed, 0 insertions(+), 34 deletions(-)

Please apply for 2.6.27.


Thanks,

Andreas

diff --git a/arch/x86/kernel/quirks.c b/arch/x86/kernel/quirks.c
index d138588..9ed3a64 100644
--- a/arch/x86/kernel/quirks.c
+++ b/arch/x86/kernel/quirks.c
@@ -65,7 +65,6 @@ static enum {
 	ICH_FORCE_HPET_RESUME,
 	VT8237_FORCE_HPET_RESUME,
 	NVIDIA_FORCE_HPET_RESUME,
-	ATI_FORCE_HPET_RESUME,
 } force_hpet_resume_type;
 
 static void __iomem *rcba_base;
@@ -348,36 +347,6 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_8235,
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_8237,
 			 vt8237_force_enable_hpet);
 
-static void ati_force_hpet_resume(void)
-{
-	pci_write_config_dword(cached_dev, 0x14, 0xfed00000);
-	printk(KERN_DEBUG "Force enabled HPET at resume\n");
-}
-
-static void ati_force_enable_hpet(struct pci_dev *dev)
-{
-	u32 uninitialized_var(val);
-
-	if (hpet_address || force_hpet_address)
-		return;
-
-	if (!hpet_force_user) {
-		hpet_print_force_info();
-		return;
-	}
-
-	pci_write_config_dword(dev, 0x14, 0xfed00000);
-	pci_read_config_dword(dev, 0x14, &val);
-	force_hpet_address = val;
-	force_hpet_resume_type = ATI_FORCE_HPET_RESUME;
-	dev_printk(KERN_DEBUG, &dev->dev, "Force enabled HPET at 0x%lx\n",
-		   force_hpet_address);
-	cached_dev = dev;
-	return;
-}
-DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ATI, PCI_DEVICE_ID_ATI_IXP400_SMBUS,
-			 ati_force_enable_hpet);
-
 /*
  * Undocumented chipset feature taken from LinuxBIOS.
  */
@@ -450,9 +419,6 @@ void force_hpet_resume(void)
 	case NVIDIA_FORCE_HPET_RESUME:
 		nvidia_force_hpet_resume();
 		return;
-	case ATI_FORCE_HPET_RESUME:
-		ati_force_hpet_resume();
-		return;
 	default:
 		break;
 	}
-- 
1.6.0.1

Re: [PATCH] Revert commit e8aa4667baf74dfd85fbaab86861465acb811085

[Ingo Molnar]

* Andreas Herrmann <andreas.herrmann3@amd.com> wrote:

> This reverts commit e8aa4667baf74dfd85fbaab86861465acb811085
>  (x86: enable hpet=force for AMD SB400)
> 
> Since ATI/AMD decided not to support HPET on SB4xx it doesn't
> make sense to enable this unsupported feature.
> (I was not aware of this when submitting the quirk.)
> 
> If a system with SB4xx chipset provides an ACPI HPET table and does
> not boot, "nohpet" should be used as kernel parameter.
> 
> Signed-off-by: Andreas Herrmann <andreas.herrmann3@amd.com>

applied to tip/x86/urgent, thanks Andreas. I guess a system broke due to 
this commit?

	Ingo

Re: [PATCH] Revert commit e8aa4667baf74dfd85fbaab86861465acb811085

[Thomas Gleixner]

On Thu, 4 Sep 2008, Ingo Molnar wrote:

> 
> * Andreas Herrmann <andreas.herrmann3@amd.com> wrote:
> 
> > This reverts commit e8aa4667baf74dfd85fbaab86861465acb811085
> >  (x86: enable hpet=force for AMD SB400)
> > 
> > Since ATI/AMD decided not to support HPET on SB4xx it doesn't
> > make sense to enable this unsupported feature.
> > (I was not aware of this when submitting the quirk.)
> > 
> > If a system with SB4xx chipset provides an ACPI HPET table and does
> > not boot, "nohpet" should be used as kernel parameter.
> > 
> > Signed-off-by: Andreas Herrmann <andreas.herrmann3@amd.com>
> 
> applied to tip/x86/urgent, thanks Andreas. I guess a system broke due to 
> this commit?

Hmm, why do we remove something which needs to be force enabled by the
user anyway ?  

Is the HPET on these systems not working at all so the force enable
code is useless ?

Thanks,
	
	tglx

Re: [PATCH] Revert commit e8aa4667baf74dfd85fbaab86861465acb811085

[Ingo Molnar]

* Thomas Gleixner <tglx@linutronix.de> wrote:

> On Thu, 4 Sep 2008, Ingo Molnar wrote:
> 
> > 
> > * Andreas Herrmann <andreas.herrmann3@amd.com> wrote:
> > 
> > > This reverts commit e8aa4667baf74dfd85fbaab86861465acb811085
> > >  (x86: enable hpet=force for AMD SB400)
> > > 
> > > Since ATI/AMD decided not to support HPET on SB4xx it doesn't
> > > make sense to enable this unsupported feature.
> > > (I was not aware of this when submitting the quirk.)
> > > 
> > > If a system with SB4xx chipset provides an ACPI HPET table and does
> > > not boot, "nohpet" should be used as kernel parameter.
> > > 
> > > Signed-off-by: Andreas Herrmann <andreas.herrmann3@amd.com>
> > 
> > applied to tip/x86/urgent, thanks Andreas. I guess a system broke due to 
> > this commit?
> 
> Hmm, why do we remove something which needs to be force enabled by the 
> user anyway ?

good point, i thought the original commit caused unconditional 
force-enabling - but indeed it is only relevant if hpet=force is 
specified. (which should be rare and specific)

> Is the HPET on these systems not working at all so the force enable 
> code is useless ?

also, if a user does hpet=force and thing break he's got to keep all the 
pieces, right?

or is there any other side-effect of the commit that matters here?

	Ingo

Re: [PATCH] Revert commit e8aa4667baf74dfd85fbaab86861465acb811085

[Andreas Herrmann]

On Thu, Sep 04, 2008 at 06:14:38PM +0200, Thomas Gleixner wrote:
> On Thu, 4 Sep 2008, Ingo Molnar wrote:
> 
> > 
> > * Andreas Herrmann <andreas.herrmann3@amd.com> wrote:
> > 
> > > This reverts commit e8aa4667baf74dfd85fbaab86861465acb811085
> > >  (x86: enable hpet=force for AMD SB400)
> > > 
> > > Since ATI/AMD decided not to support HPET on SB4xx it doesn't
> > > make sense to enable this unsupported feature.
> > > (I was not aware of this when submitting the quirk.)
> > > 
> > > If a system with SB4xx chipset provides an ACPI HPET table and does
> > > not boot, "nohpet" should be used as kernel parameter.
> > > 
> > > Signed-off-by: Andreas Herrmann <andreas.herrmann3@amd.com>
> > 
> > applied to tip/x86/urgent, thanks Andreas. I guess a system broke due to 
> > this commit?
> 
> Hmm, why do we remove something which needs to be force enabled by the
> user anyway ?  

> Is the HPET on these systems not working at all so the force enable
> code is useless ?

The current quirk is incomplete. Some more chipset fiddling has to be
done to enable HPET interrupts. I have a patch that would do this.
And from my tests it seems to work faultlessly.

But the official statement is that HPET is not supported on SB4XX.

Thus there are 2 alternatives:
(1) Remove the current (incomplete) quirk.
(2) Extent the quirk.
    But whoever forces HPET would use it on his own risk.

I decided to do (1) because it's safest.
Other opinions?


Regards,

Andreas

Re: [PATCH] Revert commit e8aa4667baf74dfd85fbaab86861465acb811085

[Thomas Gleixner]

On Fri, 5 Sep 2008, Andreas Herrmann wrote:
> > Is the HPET on these systems not working at all so the force enable
> > code is useless ?
> 
> The current quirk is incomplete. Some more chipset fiddling has to be
> done to enable HPET interrupts. I have a patch that would do this.
> And from my tests it seems to work faultlessly.
> 
> But the official statement is that HPET is not supported on SB4XX.
> 
> Thus there are 2 alternatives:
> (1) Remove the current (incomplete) quirk.
> (2) Extent the quirk.
>     But whoever forces HPET would use it on his own risk.
> 
> I decided to do (1) because it's safest.
> Other opinions?

Yup, I prefer (2). It might help users. It still needs hpet=force on
the kernel command line. So it's a documented "you forced it" feature
with no guarantees.

I don't expect that the systems will explode, burn out or fall into
pieces when hpet is enabled. That would be a reason to go for (1) :)

Thanks,

	tglx

[PATCH] x86: hpet: modify IXP400 quirk to enable interrupts

[Andreas Herrmann]

The current quirk is incomplete. Some more chipset fiddling has to be
done to enable HPET interrupts. This patch aims to do this. From my
tests it seems to work faultlessly.

But the official statement is that HPET is not supported on SB4X0.

Users will still have to use hpet=force to enable it.

Use it at your own risk.

Signed-off-by: Andreas Herrmann <andreas.herrmann3@amd.com>
---
 arch/x86/kernel/quirks.c |   41 +++++++++++++++++++++++++++++++++++++++--
 1 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/quirks.c b/arch/x86/kernel/quirks.c
index d138588..f6a11b9 100644
--- a/arch/x86/kernel/quirks.c
+++ b/arch/x86/kernel/quirks.c
@@ -354,9 +354,27 @@ static void ati_force_hpet_resume(void)
 	printk(KERN_DEBUG "Force enabled HPET at resume\n");
 }
 
+static u32 ati_ixp4x0_rev(struct pci_dev *dev)
+{
+	u32 d;
+	u8  b;
+
+	pci_read_config_byte(dev, 0xac, &b);
+	b &= ~(1<<5);
+	pci_write_config_byte(dev, 0xac, b);
+	pci_read_config_dword(dev, 0x70, &d);
+	d |= 1<<8;
+	pci_write_config_dword(dev, 0x70, d);
+	pci_read_config_dword(dev, 0x8, &d);
+	d &= 0xff;
+	dev_printk(KERN_DEBUG, &dev->dev, "SB4X0 revision 0x%x\n", d);
+	return d;
+}
+
 static void ati_force_enable_hpet(struct pci_dev *dev)
 {
-	u32 uninitialized_var(val);
+	u32 d, val;
+	u8  b;
 
 	if (hpet_address || force_hpet_address)
 		return;
@@ -366,14 +384,33 @@ static void ati_force_enable_hpet(struct pci_dev *dev)
 		return;
 	}
 
+	d = ati_ixp4x0_rev(dev);
+	if (d  < 0x82)
+		return;
+
+	/* base address */
 	pci_write_config_dword(dev, 0x14, 0xfed00000);
 	pci_read_config_dword(dev, 0x14, &val);
+
+	/* enable interrupt */
+	outb(0x72, 0xcd6); b = inb(0xcd7);
+	b |= 0x1;
+	outb(0x72, 0xcd6); outb(b, 0xcd7);
+	outb(0x72, 0xcd6); b = inb(0xcd7);
+	if (!(b & 0x1))
+		return;
+	pci_read_config_dword(dev, 0x64, &d);
+	d |= (1<<10);
+	pci_write_config_dword(dev, 0x64, d);
+	pci_read_config_dword(dev, 0x64, &d);
+	if (!(d & (1<<10)))
+		return;
+
 	force_hpet_address = val;
 	force_hpet_resume_type = ATI_FORCE_HPET_RESUME;
 	dev_printk(KERN_DEBUG, &dev->dev, "Force enabled HPET at 0x%lx\n",
 		   force_hpet_address);
 	cached_dev = dev;
-	return;
 }
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ATI, PCI_DEVICE_ID_ATI_IXP400_SMBUS,
 			 ati_force_enable_hpet);
-- 
1.6.0.1

Re: [PATCH] x86: hpet: modify IXP400 quirk to enable interrupts

[Ingo Molnar]

* Andreas Herrmann <andreas.herrmann3@amd.com> wrote:

> The current quirk is incomplete. Some more chipset fiddling has to be
> done to enable HPET interrupts. This patch aims to do this. From my
> tests it seems to work faultlessly.
> 
> But the official statement is that HPET is not supported on SB4X0.
> 
> Users will still have to use hpet=force to enable it.
> 
> Use it at your own risk.
> 
> Signed-off-by: Andreas Herrmann <andreas.herrmann3@amd.com>

applied to tip/timers/hpet - thanks Andreas!

	Ingo