Re: [PATCH] Revert commit e8aa4667baf74dfd85fbaab86861465acb811085

Previous thread: Re: [PATCH] ncurses based config by Bodo Eggert on Thursday, September 4, 2008 - 7:37 am. (1 message)

Next thread: [PATCH] nfsd/nfs4acl: Number of used used array elements needs to be zeroed. by Jiri Pirko on Thursday, September 4, 2008 - 7:55 am. (6 messages)
From: Andreas Herrmann
Date: Thursday, September 4, 2008 - 7:46 am

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 ...
From: Ingo Molnar
Date: Thursday, September 4, 2008 - 8:32 am

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

	Ingo
--

From: Thomas Gleixner
Date: Thursday, September 4, 2008 - 9:14 am

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
--

From: Ingo Molnar
Date: Thursday, September 4, 2008 - 9:17 am

good point, i thought the original commit caused unconditional 
force-enabling - but indeed it is only relevant if hpet=force is 

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
--

From: Andreas Herrmann
Date: Friday, September 5, 2008 - 5:42 am

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


--

From: Thomas Gleixner
Date: Friday, September 5, 2008 - 7:03 am

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
--

From: Andreas Herrmann
Date: Friday, September 5, 2008 - 9:33 am

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;
+
 ...
From: Ingo Molnar
Date: Friday, September 5, 2008 - 9:59 am

applied to tip/timers/hpet - thanks Andreas!

	Ingo
--

Previous thread: Re: [PATCH] ncurses based config by Bodo Eggert on Thursday, September 4, 2008 - 7:37 am. (1 message)

Next thread: [PATCH] nfsd/nfs4acl: Number of used used array elements needs to be zeroed. by Jiri Pirko on Thursday, September 4, 2008 - 7:55 am. (6 messages)