numa.c is the only user of the {in,out}*_quad functions. And it has only a few call
sites. Change them to open code the magic NUMAQ port access.
Only compile tested
Signed-off-by: Andi Kleen <[email protected]>
Index: linux/arch/x86/pci/numa.c
===================================================================
--- linux.orig/arch/x86/pci/numa.c
+++ linux/arch/x86/pci/numa.c
@@ -5,36 +5,62 @@
#include <linux/pci.h>
#include <linux/init.h>
#include <linux/nodemask.h>
+#include <mach_apic.h>
#include "pci.h"
+#define XQUAD_PORTIO_BASE 0xfe400000
+#define XQUAD_PORTIO_QUAD 0x40000 /* 256k per quad. */
+
#define BUS2QUAD(global) (mp_bus_id_to_node[global])
#define BUS2LOCAL(global) (mp_bus_id_to_local[global])
#define QUADLOCAL2BUS(quad,local) (quad_local_to_mp_bus_id[quad][local])
+extern void *xquad_portio; /* Where the IO area was mapped */
+#define XQUAD_PORT_ADDR(port, quad) (xquad_portio + (XQUAD_PORTIO_QUAD*quad) + port)
+
#define PCI_CONF1_MQ_ADDRESS(bus, devfn, reg) \
(0x80000000 | (BUS2LOCAL(bus) << 16) | (devfn << 8) | (reg & ~3))
+static void write_cf8(unsigned bus, unsigned devfn, unsigned reg)
+{
+ unsigned val = PCI_CONF1_MQ_ADDRESS(bus, devfn, reg);
+ if (xquad_portio)
+ writel(val, XQUAD_PORT_ADDR(0xcf8, BUS2QUAD(bus)));
+ else
+ outl(val, 0xCF8);
+}
+
static int pci_conf1_mq_read(unsigned int seg, unsigned int bus,
unsigned int devfn, int reg, int len, u32 *value)
{
unsigned long flags;
+ void *adr __iomem = XQUAD_PORT_ADDR(0xcfc, BUS2QUAD(bus));
if (!value || (bus >= MAX_MP_BUSSES) || (devfn > 255) || (reg > 255))
return -EINVAL;
spin_lock_irqsave(&pci_config_lock, flags);
- outl_quad(PCI_CONF1_MQ_ADDRESS(bus, devfn, reg), 0xCF8, BUS2QUAD(bus));
+ write_cf8(bus, devfn, reg);
switch (len) {
case 1:
- *value = inb_quad(0xCFC + (reg & 3), BUS2QUAD(bus));
+ if (xquad_portio)
+ *value = readb(adr + (reg & 3));
+ else
+ *value = inb(0xCFC + (reg & 3));
break;
case 2:
- *value = inw_quad(0xCFC + (reg & 2), BUS2QUAD(bus));
+ if (xquad_portio)
+ *value = readw(adr + (reg & 2));
+ else
+ *value = inw(0xCFC + (reg & 2));
break;
case 4:
- *value = inl_quad(0xCFC, BUS2QUAD(bus));
+ if (xquad_portio)
+ *value = readl(adr);
+ else
+ *value = inl(0xCFC);
break;
}
@@ -47,23 +73,33 @@ static int pci_conf1_mq_write(unsigned i
unsigned int devfn, int reg, int len, u32 value)
{
unsigned long flags;
+ void *adr __iomem = XQUAD_PORT_ADDR(0xcfc, BUS2QUAD(bus));
if ((bus >= MAX_MP_BUSSES) || (devfn > 255) || (reg > 255))
return -EINVAL;
spin_lock_irqsave(&pci_config_lock, flags);
- outl_quad(PCI_CONF1_MQ_ADDRESS(bus, devfn, reg), 0xCF8, BUS2QUAD(bus));
+ write_cf8(bus, devfn, reg);
switch (len) {
case 1:
- outb_quad((u8)value, 0xCFC + (reg & 3), BUS2QUAD(bus));
+ if (xquad_portio)
+ writeb(value, adr + (reg & 3));
+ else
+ outb((u8)value, 0xCFC + (reg & 3));
break;
case 2:
- outw_quad((u16)value, 0xCFC + (reg & 2), BUS2QUAD(bus));
+ if (xquad_portio)
+ writew(value, adr + (reg & 2));
+ else
+ outw((u16)value, 0xCFC + (reg & 2));
break;
case 4:
- outl_quad((u32)value, 0xCFC, BUS2QUAD(bus));
+ if (xquad_portio)
+ writel(value, adr + reg);
+ else
+ outl((u32)value, 0xCFC);
break;
}
Index: linux/include/asm-x86/mach-numaq/mach_apic.h
===================================================================
--- linux.orig/include/asm-x86/mach-numaq/mach_apic.h
+++ linux/include/asm-x86/mach-numaq/mach_apic.h
@@ -109,6 +109,8 @@ static inline int mpc_apic_id(struct mpc
return logical_apicid;
}
+extern void *xquad_portio;
+
static inline void setup_portio_remap(void)
{
int num_quads = num_online_nodes();
Now that the only user does it on its own remove the NUMAQ support macros in io_32.h
The next step would be to convert the preprocessor mess to actually readable
standard inlines.
Signed-off-by: Andi Kleen <[email protected]>
Index: linux/include/asm-x86/io_32.h
===================================================================
--- linux.orig/include/asm-x86/io_32.h
+++ linux/include/asm-x86/io_32.h
@@ -275,29 +275,6 @@ static inline void slow_down_io(void) {
#endif
-#ifdef CONFIG_X86_NUMAQ
-extern void *xquad_portio; /* Where the IO area was mapped */
-#define XQUAD_PORT_ADDR(port, quad) (xquad_portio + (XQUAD_PORTIO_QUAD*quad) + port)
-#define __BUILDIO(bwl,bw,type) \
-static inline void out##bwl##_quad(unsigned type value, int port, int quad) { \
- if (xquad_portio) \
- write##bwl(value, XQUAD_PORT_ADDR(port, quad)); \
- else \
- out##bwl##_local(value, port); \
-} \
-static inline void out##bwl(unsigned type value, int port) { \
- out##bwl##_quad(value, port, 0); \
-} \
-static inline unsigned type in##bwl##_quad(int port, int quad) { \
- if (xquad_portio) \
- return read##bwl(XQUAD_PORT_ADDR(port, quad)); \
- else \
- return in##bwl##_local(port); \
-} \
-static inline unsigned type in##bwl(int port) { \
- return in##bwl##_quad(port, 0); \
-}
-#else
#define __BUILDIO(bwl,bw,type) \
static inline void out##bwl(unsigned type value, int port) { \
out##bwl##_local(value, port); \
@@ -305,8 +282,6 @@ static inline void out##bwl(unsigned typ
static inline unsigned type in##bwl(int port) { \
return in##bwl##_local(port); \
}
-#endif
-
#define BUILDIO(bwl,bw,type) \
static inline void out##bwl##_local(unsigned type value, int port) { \
* Andi Kleen <[email protected]> wrote:
> numa.c is the only user of the {in,out}*_quad functions. And it has
> only a few call sites. Change them to open code the magic NUMAQ port
> access.
thanks, applied.
Ingo
* Andi Kleen <[email protected]> wrote:
> Now that the only user does it on its own remove the NUMAQ support
> macros in io_32.h
thanks, applied.
> The next step would be to convert the preprocessor mess to actually
> readable standard inlines.
agreed.
Ingo
Hi Andi,
On (01/02/08 18:08), Andi Kleen didst pronounce:
>
> numa.c is the only user of the {in,out}*_quad functions. And it has only a few call
> sites. Change them to open code the magic NUMAQ port access.
>
> Only compile tested
>
This failed a boot-test on one of the quad machines. Before the patches I see
qla1280: QLA1040 found on PCI bus 0, dev 11
scsi(0:0): Resetting SCSI BUS
scsi0 : QLogic QLA1040 PCI to SCSI Host Adapter
Firmware version: 7.65.06, Driver version 3.26
scsi 0:0:0:0: Direct-Access IBM DGHS18X 0360 PQ: 0 ANSI: 3
scsi(0:0:0:0): Sync: period 10, offset 12, Wide
scsi 0:0:1:0: Direct-Access IBM DGHS18X 0360 PQ: 0 ANSI: 3
scsi(0:0:1:0): Sync: period 10, offset 12, Wide
scsi 0:0:2:0: Direct-Access IBM DGHS18X 0360 PQ: 0 ANSI: 3
scsi(0:0:2:0): Sync: period 10, offset 12, Wide
scsi 0:0:3:0: Direct-Access IBM OEM DCHS09X 5454 PQ: 0 ANSI: 2
scsi(0:0:3:0): Sync: period 10, offset 12, Wide
scsi 0:0:4:0: Direct-Access IBM OEM DCHS09X 5454 PQ: 0 ANSI: 2
scsi(0:0:4:0): Sync: period 10, offset 12, Wide
qla1280: QLA1040 found on PCI bus 2, dev 11
scsi(1:0): Resetting SCSI BUS
scsi1 : QLogic QLA1040 PCI to SCSI Host Adapter
Firmware version: 7.65.06, Driver version 3.26
qla1280: QLA1040 found on PCI bus 4, dev 11
scsi(2:0): Resetting SCSI BUS
scsi2 : QLogic QLA1040 PCI to SCSI Host Adapter
Firmware version: 7.65.06, Driver version 3.26
qla1280: QLA1040 found on PCI bus 6, dev 11
scsi(3:0): Resetting SCSI BUS
scsi3 : QLogic QLA1040 PCI to SCSI Host Adapter
Firmware version: 7.65.06, Driver version 3.26
After both patches are applied, it looks like
qla1280: QLA1040 found on PCI bus 0, dev 11
qla1280: Unable to map I/O memory
qla1280: QLA1040 found on PCI bus 2, dev 11
qla1280: Unable to map I/O memory
qla1280: QLA1040 found on PCI bus 4, dev 11
qla1280: Unable to map I/O memory
qla1280: QLA1040 found on PCI bus 6, dev 11
qla1280: Unable to map I/O memory
and of course the root device is not found. Comments on patch and fix is below.
> Signed-off-by: Andi Kleen <[email protected]>
>
> Index: linux/arch/x86/pci/numa.c
> ===================================================================
> --- linux.orig/arch/x86/pci/numa.c
> +++ linux/arch/x86/pci/numa.c
> @@ -5,36 +5,62 @@
> #include <linux/pci.h>
> #include <linux/init.h>
> #include <linux/nodemask.h>
> +#include <mach_apic.h>
> #include "pci.h"
>
> +#define XQUAD_PORTIO_BASE 0xfe400000
There is still a definition in include/asm-x86/io_32.h . Is this
intentional?
> +#define XQUAD_PORTIO_QUAD 0x40000 /* 256k per quad. */
> +
Similarly in io_32.h
> #define BUS2QUAD(global) (mp_bus_id_to_node[global])
> #define BUS2LOCAL(global) (mp_bus_id_to_local[global])
> #define QUADLOCAL2BUS(quad,local) (quad_local_to_mp_bus_id[quad][local])
>
> +extern void *xquad_portio; /* Where the IO area was mapped */
Does the extern need to be here when you've added it (minus the comment)
to mach_apic.h later in the patch?
> +#define XQUAD_PORT_ADDR(port, quad) (xquad_portio + (XQUAD_PORTIO_QUAD*quad) + port)
> +
> #define PCI_CONF1_MQ_ADDRESS(bus, devfn, reg) \
> (0x80000000 | (BUS2LOCAL(bus) << 16) | (devfn << 8) | (reg & ~3))
>
> +static void write_cf8(unsigned bus, unsigned devfn, unsigned reg)
> +{
> + unsigned val = PCI_CONF1_MQ_ADDRESS(bus, devfn, reg);
> + if (xquad_portio)
> + writel(val, XQUAD_PORT_ADDR(0xcf8, BUS2QUAD(bus)));
> + else
> + outl(val, 0xCF8);
> +}
Other than a case change for 0xcf8, this looks equivilant. xquad_portio
should be set and I see in the dmesg;
Before: xquad_portio vaddr 0xf8800000, len 00100000
After: xquad_portio vaddr 0xf8800000, len 00100000
> +
> static int pci_conf1_mq_read(unsigned int seg, unsigned int bus,
> unsigned int devfn, int reg, int len, u32 *value)
> {
> unsigned long flags;
> + void *adr __iomem = XQUAD_PORT_ADDR(0xcfc, BUS2QUAD(bus));
>
It shouldn't matter but for clarity should this value only be calculated
when xquad_portio is set?
> if (!value || (bus >= MAX_MP_BUSSES) || (devfn > 255) || (reg > 255))
> return -EINVAL;
>
> spin_lock_irqsave(&pci_config_lock, flags);
>
> - outl_quad(PCI_CONF1_MQ_ADDRESS(bus, devfn, reg), 0xCF8, BUS2QUAD(bus));
> + write_cf8(bus, devfn, reg);
Before
writel(PCI_CONF1_MQ_ADDRESS(bus, devfn, reg), XQUAD_PORT_ADDR(0xCF8, BUS2QUAD(bus)))
After
writel(PCI_CONF1_MQ_ADDRESS(bus, devfn, reg), XQUAD_PORT_ADDR(0xcf8, BUS2QUAD(bus)))
No problem there.
>
> switch (len) {
> case 1:
> - *value = inb_quad(0xCFC + (reg & 3), BUS2QUAD(bus));
> + if (xquad_portio)
> + *value = readb(adr + (reg & 3));
> + else
> + *value = inb(0xCFC + (reg & 3));
> break;
Before
readb(XQUAD_PORT_ADDR(0xCFC + (reg & 3), BUS2QUAD(bus)))
After
readb(XQUAD_PORT_ADDR(0xcfc, BUS2QUAD(bus)) + (reg & 3))
Not exactly equivilant but does it matter?
#define XQUAD_PORT_ADDR(port, quad) (xquad_portio + (XQUAD_PORTIO_QUAD*quad) + port)
Before
readb(xquad_portio + (XQUAD_PORTIO_QUAD*BUS2QUAD(bus)) + 0xCFC + (reg & 3))
After
readb(xquad_portio + (XQUAD_PORTIO_QUAD*BUS2QUAD(bus)) + 0xcfc + (reg & 3))
Apparently no actual difference.
> case 2:
> - *value = inw_quad(0xCFC + (reg & 2), BUS2QUAD(bus));
> + if (xquad_portio)
> + *value = readw(adr + (reg & 2));
> + else
> + *value = inw(0xCFC + (reg & 2));
> break;
> case 4:
> - *value = inl_quad(0xCFC, BUS2QUAD(bus));
> + if (xquad_portio)
> + *value = readl(adr);
> + else
> + *value = inl(0xCFC);
> break;
> }
>
> @@ -47,23 +73,33 @@ static int pci_conf1_mq_write(unsigned i
> unsigned int devfn, int reg, int len, u32 value)
> {
> unsigned long flags;
> + void *adr __iomem = XQUAD_PORT_ADDR(0xcfc, BUS2QUAD(bus));
>
> if ((bus >= MAX_MP_BUSSES) || (devfn > 255) || (reg > 255))
> return -EINVAL;
>
> spin_lock_irqsave(&pci_config_lock, flags);
>
> - outl_quad(PCI_CONF1_MQ_ADDRESS(bus, devfn, reg), 0xCF8, BUS2QUAD(bus));
> + write_cf8(bus, devfn, reg);
>
Same as above so should be ok.
> switch (len) {
> case 1:
> - outb_quad((u8)value, 0xCFC + (reg & 3), BUS2QUAD(bus));
> + if (xquad_portio)
> + writeb(value, adr + (reg & 3));
> + else
> + outb((u8)value, 0xCFC + (reg & 3));
> break;
Before
writeb((u8)value, XQUAD_PORT_ADDR(0xCFC + (reg & 3), BUS2QUAD(bus)))
After
writeb(value, XQUAD_PORT_ADDR(0xcfc, BUS2QUAD(bus)) + (reg & 3))
The only difference is the explicit cast and the signature of writeb()
should have forced that cast anyway
> case 2:
> - outw_quad((u16)value, 0xCFC + (reg & 2), BUS2QUAD(bus));
> + if (xquad_portio)
> + writew(value, adr + (reg & 2));
> + else
> + outw((u16)value, 0xCFC + (reg & 2));
> break;
> case 4:
> - outl_quad((u32)value, 0xCFC, BUS2QUAD(bus));
> + if (xquad_portio)
> + writel(value, adr + reg);
> + else
> + outl((u32)value, 0xCFC);
> break;
(adr + reg) is not equivilant to 0xCFC here. This is the source of the
boot breakage. Fix is below
> }
>
> Index: linux/include/asm-x86/mach-numaq/mach_apic.h
> ===================================================================
> --- linux.orig/include/asm-x86/mach-numaq/mach_apic.h
> +++ linux/include/asm-x86/mach-numaq/mach_apic.h
> @@ -109,6 +109,8 @@ static inline int mpc_apic_id(struct mpc
> return logical_apicid;
> }
>
> +extern void *xquad_portio;
> +
> static inline void setup_portio_remap(void)
> {
> int num_quads = num_online_nodes();
>
Patch to fix boot problem as follows
====================================
Fix for "x86: move NUMAQ io handling into arch/x86/pci/numa.c"
The intention of commit c7e844f0415252c7e1a2153a97e7a0c511d61ada was to
replace a number of outl_quad() calls with a writel() equivilant. One of
the changes was not equivilant. This patch corrects it.
Signed-off-by: Mel Gorman <[email protected]>
---
arch/x86/pci/numa.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/pci/numa.c b/arch/x86/pci/numa.c
index 55270c2..ecbcd51 100644
--- a/arch/x86/pci/numa.c
+++ b/arch/x86/pci/numa.c
@@ -97,7 +97,7 @@ static int pci_conf1_mq_write(unsigned int seg, unsigned int bus,
break;
case 4:
if (xquad_portio)
- writel(value, adr + reg);
+ writel(value, adr);
else
outl((u32)value, 0xCFC);
break;
--
Mel Gorman
Part-time Phd Student Linux Technology Center
University of Limerick IBM Dublin Software Lab
> > +#define XQUAD_PORTIO_BASE 0xfe400000
>
> There is still a definition in include/asm-x86/io_32.h . Is this
> intentional?
Probably not.
>
> > +extern void *xquad_portio; /* Where the IO area was mapped */
>
> Does the extern need to be here when you've added it (minus the comment)
> to mach_apic.h later in the patch?
The extern was intended to be removed, but somehow that change didn't make
it into the final patch. Doesn't matter much either way though.
> Fix for "x86: move NUMAQ io handling into arch/x86/pci/numa.c"
>
> The intention of commit c7e844f0415252c7e1a2153a97e7a0c511d61ada was to
> replace a number of outl_quad() calls with a writel() equivilant. One of
> the changes was not equivilant. This patch corrects it.
Yes looks good; thanks.
-Andi