Skip to contentSkip to conversation

Small fixes for 2.6.15-rc5

PATCH In review Greg Kroah-Hartman 5 participants 11 messages Dec 12, 2005 – Dec 13, 2005

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

cover Small fixes for 2.6.15-rc5

Here are some small fixes for your current 2.6.15-rc5 git tree. They have all been in the past -mm releases, and fix problems. The PCI patches were also sent by Andi to you, so you might receive them through his emails too.

Here's the diffstat of the combined patches.

 arch/i386/pci/direct.c           |    8 +-
 arch/i386/pci/mmconfig.c         |  141 ++++++++++++++++++++++++++++++++-------
 arch/i386/pci/pci.h              |   14 +++
 arch/x86_64/pci/mmconfig.c       |  133 ++++++++++++++++++++++++++++++------
 drivers/i2c/busses/i2c-mv64xxx.c |   25 +++---
 drivers/usb/host/uhci-hcd.c      |    6 +
 6 files changed, 261 insertions(+), 66 deletions(-)

thanks,

greg k-h

#1 lore raw

PATCH 1/4 i2c: Fix i2c-mv64xxx compilation error

From: "Mark A. Greer" <mgreer at mvista.com>

The busses/i2c-mv64xxx.c driver doesn't currently compile because of an incorrect argument to dev_err(). This patch fixes that.

Signed-off-by Mark A. Greer <mgreer at mvista.com>
Signed-off-by Jean Delvare <khali at linux-fr.org>
Signed-off-by Greg Kroah-Hartman <gregkh at suse.de>
---
 drivers/i2c/busses/i2c-mv64xxx.c |   11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)
drivers/i2c/busses/i2c-mv64xxx.c +6 -5
@@ -529,14 +529,15 @@ mv64xxx_i2c_probe(struct platform_device
 	i2c_set_adapdata(&drv_data->adapter, drv_data);
 
 	if (request_irq(drv_data->irq, mv64xxx_i2c_intr, 0,
-		MV64XXX_I2C_CTLR_NAME, drv_data)) {
-
-		dev_err(dev, "mv64xxx: Can't register intr handler "
-			"irq: %d\n", drv_data->irq);
+			MV64XXX_I2C_CTLR_NAME, drv_data)) {
+		dev_err(&drv_data->adapter.dev,
+			"mv64xxx: Can't register intr handler irq: %d\n",
+			drv_data->irq);
 		rc = -EINVAL;
 		goto exit_unmap_regs;
 	} else if ((rc = i2c_add_adapter(&drv_data->adapter)) != 0) {
-		dev_err(dev, "mv64xxx: Can't add i2c adapter, rc: %d\n", -rc);
+		dev_err(&drv_data->adapter.dev,
+			"mv64xxx: Can't add i2c adapter, rc: %d\n", -rc);
 		goto exit_free_irq;
 	}
 

--

#2 lore raw

PATCH 2/4 i386/x86-64: Implement fallback for PCI mmconfig to type1

In reply to Greg Kroah-Hartman

From: Andi Kleen <[email protected]>

When there is no entry for a bus in MCFG fall back to type1. This is especially important on K8 systems where always some devices can't be accessed using mmconfig (in particular the builtin northbridge doesn't support it for its own devices)

To be complete needs also Jeff's _SEG fixes, but that is something for after 2.6.15

---
 arch/i386/pci/direct.c     |    4 ++--
 arch/i386/pci/mmconfig.c   |   24 ++++++++++++++++--------
 arch/i386/pci/pci.h        |    7 +++++++
 arch/x86_64/pci/mmconfig.c |   29 ++++++++++++++++++++---------
 4 files changed, 45 insertions(+), 19 deletions(-)
arch/i386/pci/direct.c +2 -2
@@ -13,7 +13,7 @@
 #define PCI_CONF1_ADDRESS(bus, devfn, reg) \
 	(0x80000000 | (bus << 16) | (devfn << 8) | (reg & ~3))
 
-static int pci_conf1_read(unsigned int seg, unsigned int bus,
+int pci_conf1_read(unsigned int seg, unsigned int bus,
 			  unsigned int devfn, int reg, int len, u32 *value)
 {
 	unsigned long flags;
@@ -42,7 +42,7 @@ static int pci_conf1_read(unsigned int s
 	return 0;
 }
 
-static int pci_conf1_write(unsigned int seg, unsigned int bus,
+int pci_conf1_write(unsigned int seg, unsigned int bus,
 			   unsigned int devfn, int reg, int len, u32 value)
 {
 	unsigned long flags;
arch/i386/pci/pci.h +7 -0
@@ -74,3 +74,10 @@ extern spinlock_t pci_config_lock;
 
 extern int (*pcibios_enable_irq)(struct pci_dev *dev);
 extern void (*pcibios_disable_irq)(struct pci_dev *dev);
+
+extern int pci_conf1_write(unsigned int seg, unsigned int bus,
+			   unsigned int devfn, int reg, int len, u32 value);
+extern int pci_conf1_read(unsigned int seg, unsigned int bus,
+			  unsigned int devfn, int reg, int len, u32 *value);
+
+
arch/i386/pci/mmconfig.c +16 -8
@@ -30,10 +30,8 @@ static u32 get_base_addr(unsigned int se
 	while (1) {
 		++cfg_num;
 		if (cfg_num >= pci_mmcfg_config_num) {
-			/* something bad is going on, no cfg table is found. */
-			/* so we fall back to the old way we used to do this */
-			/* and just rely on the first entry to be correct. */
-			return pci_mmcfg_config[0].base_address;
+			/* Not found - fallback to type 1 */
+			return 0;
 		}
 		cfg = &pci_mmcfg_config[cfg_num];
 		if (cfg->pci_segment_group_number != seg)
@@ -44,9 +42,9 @@ static u32 get_base_addr(unsigned int se
 	}
 }
 
-static inline void pci_exp_set_dev_base(unsigned int seg, int bus, int devfn)
+static inline void pci_exp_set_dev_base(unsigned int base, int bus, int devfn)
 {
-	u32 dev_base = get_base_addr(seg, bus) | (bus << 20) | (devfn << 12);
+	u32 dev_base = base | (bus << 20) | (devfn << 12);
 	if (dev_base != mmcfg_last_accessed_device) {
 		mmcfg_last_accessed_device = dev_base;
 		set_fixmap_nocache(FIX_PCIE_MCFG, dev_base);
@@ -57,13 +55,18 @@ static int pci_mmcfg_read(unsigned int s
 			  unsigned int devfn, int reg, int len, u32 *value)
 {
 	unsigned long flags;
+	u32 base;
 
 	if (!value || (bus > 255) || (devfn > 255) || (reg > 4095))
 		return -EINVAL;
 
+	base = get_base_addr(seg, bus);
+	if (!base)
+		return pci_conf1_read(seg,bus,devfn,reg,len,value);
+
 	spin_lock_irqsave(&pci_config_lock, flags);
 
-	pci_exp_set_dev_base(seg, bus, devfn);
+	pci_exp_set_dev_base(base, bus, devfn);
 
 	switch (len) {
 	case 1:
@@ -86,13 +89,18 @@ static int pci_mmcfg_write(unsigned int 
 			   unsigned int devfn, int reg, int len, u32 value)
 {
 	unsigned long flags;
+	u32 base;
 
 	if ((bus > 255) || (devfn > 255) || (reg > 4095)) 
 		return -EINVAL;
 
+	base = get_base_addr(seg, bus);
+	if (!base)
+		return pci_conf1_write(seg,bus,devfn,reg,len,value);
+
 	spin_lock_irqsave(&pci_config_lock, flags);
 
-	pci_exp_set_dev_base(seg, bus, devfn);
+	pci_exp_set_dev_base(base, bus, devfn);
 
 	switch (len) {
 	case 1:
arch/x86_64/pci/mmconfig.c +20 -9
@@ -19,7 +19,7 @@ struct mmcfg_virt {
 };
 static struct mmcfg_virt *pci_mmcfg_virt;
 
-static char *get_virt(unsigned int seg, int bus)
+static char *get_virt(unsigned int seg, unsigned bus)
 {
 	int cfg_num = -1;
 	struct acpi_table_mcfg_config *cfg;
@@ -27,10 +27,9 @@ static char *get_virt(unsigned int seg, 
 	while (1) {
 		++cfg_num;
 		if (cfg_num >= pci_mmcfg_config_num) {
-			/* something bad is going on, no cfg table is found. */
-			/* so we fall back to the old way we used to do this */
-			/* and just rely on the first entry to be correct. */
-			return pci_mmcfg_virt[0].virt;
+			/* Not found - fall back to type 1. This happens
+			   e.g. on the internal devices of a K8 northbridge. */
+			return NULL;
 		}
 		cfg = pci_mmcfg_virt[cfg_num].cfg;
 		if (cfg->pci_segment_group_number != seg)
@@ -43,18 +42,25 @@ static char *get_virt(unsigned int seg, 
 
 static inline char *pci_dev_base(unsigned int seg, unsigned int bus, unsigned int devfn)
 {
-
-	return get_virt(seg, bus) + ((bus << 20) | (devfn << 12));
+	char *addr = get_virt(seg, bus);
+	if (!addr)
+		return NULL;
+ 	return addr + ((bus << 20) | (devfn << 12));
 }
 
 static int pci_mmcfg_read(unsigned int seg, unsigned int bus,
 			  unsigned int devfn, int reg, int len, u32 *value)
 {
-	char *addr = pci_dev_base(seg, bus, devfn);
+	char *addr;
 
+	/* Why do we have this when nobody checks it. How about a BUG()!? -AK */
 	if (unlikely(!value || (bus > 255) || (devfn > 255) || (reg > 4095)))
 		return -EINVAL;
 
+	addr = pci_dev_base(seg, bus, devfn);
+	if (!addr)
+		return pci_conf1_read(seg,bus,devfn,reg,len,value);
+
 	switch (len) {
 	case 1:
 		*value = readb(addr + reg);
@@ -73,11 +79,16 @@ static int pci_mmcfg_read(unsigned int s
 static int pci_mmcfg_write(unsigned int seg, unsigned int bus,
 			   unsigned int devfn, int reg, int len, u32 value)
 {
-	char *addr = pci_dev_base(seg, bus, devfn);
+	char *addr;
 
+	/* Why do we have this when nobody checks it. How about a BUG()!? -AK */
 	if (unlikely((bus > 255) || (devfn > 255) || (reg > 4095)))
 		return -EINVAL;
 
+	addr = pci_dev_base(seg, bus, devfn);
+	if (!addr)
+		return pci_conf1_write(seg,bus,devfn,reg,len,value);
+
 	switch (len) {
 	case 1:
 		writeb(value, addr + reg);

--

#3 lore raw

Re: [patch 2/4] i386/x86-64: Implement fallback for PCI mmconfig to type1

Quoted 4 lines · Greg Kroah-Hartman

When there is no entry for a bus in MCFG fall back to type1. This is especially important on K8 systems where always some devices can't be accessed using mmconfig (in particular the builtin northbridge doesn't support it for its own devices)

[...]

arch/i386/pci/direct.c line 16

Quoted 2 lines
arch/i386/pci/direct.c +1 -1
-static int pci_conf1_read(unsigned int seg, unsigned int bus,
+int pci_conf1_read(unsigned int seg, unsigned int bus,

I don't like this at all. We already have a mechanism to use different accessors per-bus (bus->ops->read()); calling the type1 accessors from the mmconfig accessors just seems wrong.

arch/i386/pci/mmconfig.c static int pci_mmcfg_read(unsigned int s · line 65

Quoted 2 lines
arch/i386/pci/mmconfig.c +2 -0
+	if (!base)
+		return pci_conf1_read(seg,bus,devfn,reg,len,value);

Should be space after commas.

author
#4 lore raw

Re: [patch 2/4] i386/x86-64: Implement fallback for PCI mmconfig to type1

Quoted 6 lines · Matthew Wilcox
Quoted 4 lines · Greg Kroah-Hartman

When there is no entry for a bus in MCFG fall back to type1. This is especially important on K8 systems where always some devices can't be accessed using mmconfig (in particular the builtin northbridge doesn't support it for its own devices)

[...]

Quoted 8 lines

arch/i386/pci/direct.c line 16

Quoted 2 lines
arch/i386/pci/direct.c +1 -1
-static int pci_conf1_read(unsigned int seg, unsigned int bus,
+int pci_conf1_read(unsigned int seg, unsigned int bus,

I don't like this at all. We already have a mechanism to use different accessors per-bus (bus->ops->read()); calling the type1 accessors from the mmconfig accessors just seems wrong.

From what I can tell, it's too late in the callstack for us to change

the read ops for this device to be the other one. The problem is (and Andi can correct me if I'm wrong), some boxes basically have incomplete MCFG acpi tables (the tables do not describe all PCI busses that are present in the box). But we don't realize this until we are about to do the read function.

I remember I looked into trying to set this up at probe/init time, and it was almost impossible to do so, due to the structure of the code.

However, I might have missed something, and if you can point how to do this easier, please do.

As it is, this patch is needed to fix boxes that do not work at all with the current kernel.

thanks,

greg k-h

#5 lore raw

Re: [patch 2/4] i386/x86-64: Implement fallback for PCI mmconfig to type1

Sorry Greg, Linus,

I also submitted the two PCI patches on my own with the x86-64 patchkit, hopefully the duplication doesn't cause too much trouble.

Quoted 7 lines · Greg KH

From what I can tell, it's too late in the callstack for us to change the read ops for this device to be the other one. The problem is (and Andi can correct me if I'm wrong), some boxes basically have incomplete MCFG acpi tables (the tables do not describe all PCI busses that are present in the box). But we don't realize this until we are about to do the read function.

It can happen with perfectly legal MCFG tables. If a bus is not listed in MCFG then we must fallback to type1. This happens on AMD K8 systems because the busses in the builtin northbridge don't support mmconfig, only busses on external bridges do. In theory it could happen on other systems too (although external northbridges typically support mmconfig for everything if they do at all)

In addition we have some boxes with broken MCFG tables who don't get the tables right, this is what the next patch was trying to fix.

Quoted 2 lines

I remember I looked into trying to set this up at probe/init time, and it was almost impossible to do so, due to the structure of the code.

Yes, I also didn't see an easy way to do it, although it would be probably DTRT.

-Andi

#6 lore raw

PATCH 3/4 x86_64/i386: Correct for broken MCFG tables on K8 systems

In reply to Greg Kroah-Hartman

From: Andi Kleen <[email protected]>

They report all busses as MMCONFIG capable, but it never works for the internal devices in the CPU's builtin northbridge.

This causes problems - some devices are not visible and on x86-64 the IOMMU is not detected, causing boot failures on systems with >3GB of RAM. It also breaks AGP.

Workaround just probes all func 0 devices on bus 0 (the internal northbridge is currently always on bus 0) and if they are not accessible using MCFG they are put into a special fallback bitmap.

On systems where it isn't we assume the BIOS vendor supplied correct MCFG.

Requires the earlier patch for mmconfig type1 fallback

---
 arch/i386/pci/mmconfig.c   |   45 ++++++++++++++++++++++++++++++++++++++++++---
 arch/x86_64/pci/mmconfig.c |   36 ++++++++++++++++++++++++++++++++++--
 2 files changed, 76 insertions(+), 5 deletions(-)
arch/x86_64/pci/mmconfig.c +34 -2
@@ -8,10 +8,13 @@
 #include <linux/pci.h>
 #include <linux/init.h>
 #include <linux/acpi.h>
+#include <linux/bitmap.h>
 #include "pci.h"
 
 #define MMCONFIG_APER_SIZE (256*1024*1024)
 
+static DECLARE_BITMAP(fallback_slots, 32);
+
 /* Static virtual mapping of the MMCONFIG aperture */
 struct mmcfg_virt {
 	struct acpi_table_mcfg_config *cfg;
@@ -40,9 +43,12 @@ static char *get_virt(unsigned int seg, 
 	}
 }
 
-static inline char *pci_dev_base(unsigned int seg, unsigned int bus, unsigned int devfn)
+static char *pci_dev_base(unsigned int seg, unsigned int bus, unsigned int devfn)
 {
-	char *addr = get_virt(seg, bus);
+	char *addr;
+	if (seg == 0 && bus == 0 && test_bit(PCI_SLOT(devfn), &fallback_slots))
+		return NULL;
+	addr = get_virt(seg, bus);
 	if (!addr)
 		return NULL;
  	return addr + ((bus << 20) | (devfn << 12));
@@ -109,6 +115,30 @@ static struct pci_raw_ops pci_mmcfg = {
 	.write =	pci_mmcfg_write,
 };
 
+/* K8 systems have some devices (typically in the builtin northbridge)
+   that are only accessible using type1
+   Normally this can be expressed in the MCFG by not listing them
+   and assigning suitable _SEGs, but this isn't implemented in some BIOS.
+   Instead try to discover all devices on bus 0 that are unreachable using MM
+   and fallback for them.
+   We only do this for bus 0/seg 0 */
+static __init void unreachable_devices(void)
+{
+	int i;
+	for (i = 0; i < 32; i++) {
+		u32 val1;
+		char *addr;
+
+		pci_conf1_read(0, 0, PCI_DEVFN(i,0), 0, 4, &val1);
+		if (val1 == 0xffffffff)
+			continue;
+		addr = pci_dev_base(0, 0, PCI_DEVFN(i, 0));
+		if (addr == NULL || readl(addr) != val1) {
+			set_bit(i, &fallback_slots);
+		}
+	}
+}
+
 static int __init pci_mmcfg_init(void)
 {
 	int i;
@@ -139,6 +169,8 @@ static int __init pci_mmcfg_init(void)
 		printk(KERN_INFO "PCI: Using MMCONFIG at %x\n", pci_mmcfg_config[i].base_address);
 	}
 
+	unreachable_devices();
+
 	raw_pci_ops = &pci_mmcfg;
 	pci_probe = (pci_probe & ~PCI_PROBE_MASK) | PCI_PROBE_MMCONF;
 
arch/i386/pci/mmconfig.c +42 -3

82 lines not on this page · raw message

--

#7 lore raw

PATCH 4/4 UHCI: add missing memory barriers

In reply to Greg Kroah-Hartman

From: Alan Stern <[email protected]>

This patch (as617) adds a couple of memory barriers that Ben H. forgot in his recent suspend/resume fix.

---
 drivers/usb/host/uhci-hcd.c |    2 ++
 1 file changed, 2 insertions(+)
drivers/usb/host/uhci-hcd.c +2 -0
@@ -717,6 +717,7 @@ static int uhci_suspend(struct usb_hcd *
 	 * at the source, so we must turn off PIRQ.
 	 */
 	pci_write_config_word(to_pci_dev(uhci_dev(uhci)), USBLEGSUP, 0);
+	mb();
 	clear_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
 	uhci->hc_inaccessible = 1;
 	hcd->poll_rh = 0;
@@ -738,6 +739,7 @@ static int uhci_resume(struct usb_hcd *h
 	 * really don't want to keep a stale HCD_FLAG_HW_ACCESSIBLE=0
 	 */
 	set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
+	mb();
 
 	if (uhci->rh_state == UHCI_RH_RESET)	/* Dead */
 		return 0;

--

#8 lore raw

Re: [patch 4/4] UHCI: add missing memory barriers

Quoted 23 lines · Greg Kroah-Hartman

From: Alan Stern <[email protected]>

This patch (as617) adds a couple of memory barriers that Ben H. forgot in his recent suspend/resume fix.

---
 drivers/usb/host/uhci-hcd.c |    2 ++
 1 file changed, 2 insertions(+)
drivers/usb/host/uhci-hcd.c +1 -0
@@ -717,6 +717,7 @@ static int uhci_suspend(struct usb_hcd *
 	 * at the source, so we must turn off PIRQ.
 	 */
 	pci_write_config_word(to_pci_dev(uhci_dev(uhci)), USBLEGSUP, 0);
+	mb();
 	clear_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
 	uhci->hc_inaccessible = 1;
 	hcd->poll_rh = 0;

drivers/usb/host/uhci-hcd.c static int uhci_resume(struct usb_hcd *h · line 742

Quoted 4 lines
drivers/usb/host/uhci-hcd.c +1 -0
@@ -738,6 +739,7 @@ static int uhci_resume(struct usb_hcd *h
 	 * really don't want to keep a stale HCD_FLAG_HW_ACCESSIBLE=0
 	 */
 	set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
+	mb();

Are these just guesses, or what?

Why not smp_mb__before_clear_bit() or smp_mb__after_clear_bit() ?

Jeff

author
#9 lore raw

Re: [patch 4/4] UHCI: add missing memory barriers

Quoted 31 lines · Jeff Garzik
Quoted 23 lines · Greg Kroah-Hartman

From: Alan Stern <[email protected]>

This patch (as617) adds a couple of memory barriers that Ben H. forgot in his recent suspend/resume fix.

---
drivers/usb/host/uhci-hcd.c | 2 ++

1 file changed, 2 insertions(+)
drivers/usb/host/uhci-hcd.c +1 -0
@@ -717,6 +717,7 @@ static int uhci_suspend(struct usb_hcd *
 	 * at the source, so we must turn off PIRQ.
 	 */
 	pci_write_config_word(to_pci_dev(uhci_dev(uhci)), USBLEGSUP, 0);
+	mb();
 	clear_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
 	uhci->hc_inaccessible = 1;
 	hcd->poll_rh = 0;

drivers/usb/host/uhci-hcd.c static int uhci_resume(struct usb_hcd *h · line 742

Quoted 4 lines
drivers/usb/host/uhci-hcd.c +1 -0
@@ -738,6 +739,7 @@ static int uhci_resume(struct usb_hcd *h
 	 * really don't want to keep a stale HCD_FLAG_HW_ACCESSIBLE=0
 	 */
 	set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
+	mb();

Are these just guesses, or what?

Quoted 1 line

Why not smp_mb__before_clear_bit() or smp_mb__after_clear_bit() ?

I don't know, Alan?

maintainer
#10 lore raw

Re: [patch 4/4] UHCI: add missing memory barriers

In reply to Jeff Garzik
Quoted 22 lines · Jeff Garzik
Quoted 21 lines · Greg Kroah-Hartman

From: Alan Stern <[email protected]>

This patch (as617) adds a couple of memory barriers that Ben H. forgot in his recent suspend/resume fix.

---
 drivers/usb/host/uhci-hcd.c |    2 ++
 1 file changed, 2 insertions(+)
drivers/usb/host/uhci-hcd.c +1 -0
@@ -717,6 +717,7 @@ static int uhci_suspend(struct usb_hcd *
 	 * at the source, so we must turn off PIRQ.
 	 */
 	pci_write_config_word(to_pci_dev(uhci_dev(uhci)), USBLEGSUP, 0);
+	mb();
 	clear_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
 	uhci->hc_inaccessible = 1;
Quoted 8 lines

drivers/usb/host/uhci-hcd.c static int uhci_resume(struct usb_hcd *h · line 742

Quoted 6 lines
drivers/usb/host/uhci-hcd.c +1 -0
 	hcd->poll_rh = 0;
@@ -738,6 +739,7 @@ static int uhci_resume(struct usb_hcd *h
 	 * really don't want to keep a stale HCD_FLAG_HW_ACCESSIBLE=0
 	 */
 	set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
+	mb();

Are these just guesses, or what?

There's no need for sarcasm. No, they are not guesses. Ask Ben Herrenschmidt if you don't believe me.

Quoted 1 line

Why not smp_mb__before_clear_bit() or smp_mb__after_clear_bit() ?

Because the code needs to synchronize not with another CPU, but with a USB host controller. Those barriers are necessary even on a UP system.

By the way, what's the idea with this proliferation of little not-all-that-helpful routines, like smp_mb__before_clear_bit()? Are there architectures on which

	smp_mb__before_clear_bit(...);

is significantly superior to

	smp_mb();
	clear_bit(...);

? (It's certainly not easier to type.) Is this difference worth noting, considering how infrequently clear_bit() gets used?

Alan Stern