Hi all,
This patch contains the support parisc-linux needs in PCI generic.
My patch is not as clean as I'd like - but it should work.
Please send changes/feedback directly to me.
Code in parisc-linux CVS (based on 2.4.0) does boot on my OB800
(133Mhz Pentium), C3000, and A500 with PCI-PCI bridge support
working. I'm quite certain PCI-PCI bridge configuration (ie BIOS
didn't configure the bridge) support was broken. I'm not able to
test on alpha though...alpha may want to see #ifdef __hppa__
around some of the code I've changed.
I think the plan is to update the arch/parisc support in the near
future so parisc builds actually work from linus' tree.
grant
Grant Grundler
parisc-linux {PCI|IOMMU|SMP} hacker
+1.408.447.7253
Index: drivers/pci/Makefile
===================================================================
RCS file: /home/cvs/parisc/linux/drivers/pci/Makefile,v
retrieving revision 1.1.1.4
retrieving revision 1.6
diff -u -p -r1.1.1.4 -r1.6
--- Makefile 2001/01/09 16:57:56 1.1.1.4
+++ Makefile 2001/02/02 15:35:25 1.6
@@ -21,6 +21,7 @@ obj-$(CONFIG_PROC_FS) += proc.o
#
obj-$(CONFIG_ALPHA) += setup-bus.o setup-irq.o
obj-$(CONFIG_ARM) += setup-bus.o setup-irq.o
+obj-$(CONFIG_PARISC64) += setup-bus.o
ifndef CONFIG_X86
obj-y += syscall.o
Index: drivers/pci/pci.c
===================================================================
RCS file: /home/cvs/parisc/linux/drivers/pci/pci.c,v
retrieving revision 1.1.1.6
diff -u -p -r1.1.1.6 pci.c
--- pci.c 2001/01/09 16:57:56 1.1.1.6
+++ pci.c 2001/03/02 18:44:59
@@ -615,6 +615,7 @@ static void pci_read_bases(struct pci_de
}
}
+
void __init pci_read_bridge_bases(struct pci_bus *child)
{
struct pci_dev *dev = child->self;
@@ -628,7 +629,7 @@ void __init pci_read_bridge_bases(struct
if (!dev) /* It's a host bus, nothing to read */
return;
- for(i=0; i<3; i++)
+ for(i=0; i<4; i++)
child->resource[i] = &dev->resource[PCI_BRIDGE_RESOURCES+i];
res = child->resource[0];
@@ -644,12 +645,16 @@ void __init pci_read_bridge_bases(struct
res->end = limit + 0xfff;
res->name = child->name;
} else {
+
/*
- * Ugh. We don't know enough about this bridge. Just assume
- * that it's entirely transparent.
+ * Either this is not a PCI-PCI bridge or it's not
+ * configured yet. Since this code only supports PCI-PCI
+ * bridge, we better not be called for any other type.
+ * Don't muck the resources since it will confuse the
+ * platform specific code which does that.
*/
- printk("Unknown bridge resource %d: assuming transparent\n", 0);
- child->resource[0] = child->parent->resource[0];
+ printk("PCI : ignoring %s PCI-PCI bridge (I/O BASE not configured)\n", child->self->slot_name);
+ return;
}
res = child->resource[1];
@@ -664,8 +669,8 @@ void __init pci_read_bridge_bases(struct
res->name = child->name;
} else {
/* See comment above. Same thing */
- printk("Unknown bridge resource %d: assuming transparent\n", 1);
- child->resource[1] = child->parent->resource[1];
+ printk("PCI : ignoring %s PCI-PCI bridge (MMIO base not configured)\n", child->self->slot_name);
+ return;
}
res = child->resource[2];
@@ -690,11 +695,10 @@ void __init pci_read_bridge_bases(struct
res->end = limit + 0xfffff;
res->name = child->name;
} else {
- /* See comments above */
- printk("Unknown bridge resource %d: assuming transparent\n", 2);
- child->resource[2] = child->parent->resource[2];
+ /* Base > limit means the prefetchable mem is disabled.*/
}
}
+
static struct pci_bus * __init pci_alloc_bus(void)
{
Index: drivers/pci/setup-bus.c
===================================================================
RCS file: /home/cvs/parisc/linux/drivers/pci/setup-bus.c,v
retrieving revision 1.1.1.2
retrieving revision 1.5
diff -u -p -r1.1.1.2 -r1.5
--- setup-bus.c 2001/01/09 16:57:56 1.1.1.2
+++ setup-bus.c 2001/02/22 01:11:47 1.5
@@ -23,7 +23,7 @@
#include <linux/slab.h>
-#define DEBUG_CONFIG 1
+#define DEBUG_CONFIG 0
#if DEBUG_CONFIG
# define DBGC(args) printk args
#else
@@ -32,6 +32,7 @@
#define ROUND_UP(x, a) (((x) + (a) - 1) & ~((a) - 1))
+
static int __init
pbus_assign_resources_sorted(struct pci_bus *bus,
struct pbus_set_ranges_data *ranges)
@@ -46,7 +47,6 @@ pbus_assign_resources_sorted(struct pci_
for (ln=bus->devices.next; ln != &bus->devices; ln=ln->next) {
struct pci_dev *dev = pci_dev_b(ln);
u16 class = dev->class >> 8;
- u16 cmd;
/* First, disable the device to avoid side
effects of possibly overlapping I/O and
@@ -57,12 +57,23 @@ pbus_assign_resources_sorted(struct pci_
if (class == PCI_CLASS_DISPLAY_VGA
|| class == PCI_CLASS_NOT_DEFINED_VGA)
found_vga = 1;
+#ifndef __hppa__
+/*
+** If I/O or MEM ranges are overlapping, that's a BIOS bug.
+** Fix it in quirks?
+**
+** Disabling *all* devices is bad. Console, root, etc get
+** disabled this way.
+** -ggg
+*/
else if (class >> 8 != PCI_BASE_CLASS_BRIDGE) {
+ u16 cmd;
pci_read_config_word(dev, PCI_COMMAND, &cmd);
cmd &= ~(PCI_COMMAND_IO | PCI_COMMAND_MEMORY
| PCI_COMMAND_MASTER);
pci_write_config_word(dev, PCI_COMMAND, cmd);
}
+#endif
/* Reserve some resources for CardBus.
Are these values reasonable? */
@@ -137,8 +148,10 @@ pci_setup_bridge(struct pci_bus *bus)
pcibios_fixup_pbus_ranges(bus, &ranges);
DBGC(("PCI: Bus %d, bridge: %s\n", bus->number, bridge->name));
- DBGC((" IO window: %04lx-%04lx\n", ranges.io_start, ranges.io_end));
- DBGC((" MEM window: %08lx-%08lx\n", ranges.mem_start, ranges.mem_end));
+ DBGC((" IO window : %04lx-%04lx\n", ranges.io_start, ranges.io_end));
+ DBGC((" MEM window: %08lx-%08lx\n", ranges.mem_start,ranges.mem_end));
+ DBGC((" Pref MEM : %lx-%lx\n", bus->resource[2]->start,
+ bus->resource[2]->end));
/* Set up the top and bottom of the PCI I/O segment for this bus. */
pci_read_config_dword(bridge, PCI_IO_BASE, &l);
@@ -161,17 +174,57 @@ pci_setup_bridge(struct pci_bus *bus)
pci_write_config_dword(bridge, PCI_MEMORY_BASE, l);
/* Set up PREF base/limit. */
- l = (bus->resource[2]->start >> 16) & 0xfff0;
- l |= bus->resource[2]->end & 0xfff00000;
+ if (bus->resource[2]->start == bus->resource[2]->end) {
+ /*
+ ** 5.3.2 Prefetchable Memory Base and Limit Address Registers
+ ** (From DEC 21154 Data sheet, page 67)
+ ** "To turn off the prefetchable memory address range,
+ ** write the prefetchable memory base address register
+ ** with a value greater than that of the prefetchable
+ ** memory limit address register...."
+ **
+ ** We can't otherwise disable Prefetchable mem window
+ ** since the PCI_COMMAND_MEMORY bit is shared with
+ ** non-prefetchable MEM window register.
+ */
+ l = 0x0000ffff;
+ } else {
+ l = (bus->resource[2]->start >> 16) & 0xfff0;
+ l |= bus->resource[2]->end & 0xfff00000;
+ }
pci_write_config_dword(bridge, PCI_PREF_MEMORY_BASE, l);
+#ifdef __hppa__
+/* XXX FIXME
+** PCI_BRIDGE_CONTROL and PCI_COMMAND programming need to be revisited
+** to support FBB. Make all this crud "configurable" by the arch specific
+** (ie "PCI BIOS") support and the ifdef __hppa__ crap can go away then.
+*/
+ /*
+ ** ISA stuff confuses PDC PAT configuration.
+ ** VGA will probably crash the system at the moment.
+ ** (Fortunately, only _A_ dares install VGA in a parisc box :^)
+ ** - ggg
+ */
+ l = PCI_BRIDGE_CTL_PARITY | PCI_BRIDGE_CTL_SERR;
+#else
/* Check if we have VGA behind the bridge.
Enable ISA in either case. */
l = (bus->resource[0]->flags & IORESOURCE_BUS_HAS_VGA) ? 0x0c : 0x04;
+#endif
pci_write_config_word(bridge, PCI_BRIDGE_CONTROL, l);
+
+
+ pci_write_config_dword(bridge, PCI_COMMAND, 0xffff0007
+#ifdef __hppa__
+ /* servers definitely want SERR/PERR enabled. */
+ | PCI_COMMAND_SERR | PCI_COMMAND_PARITY
+#endif
+ );
}
-static void __init
+
+void __init
pbus_assign_resources(struct pci_bus *bus, struct pbus_set_ranges_data *ranges)
{
struct list_head *ln;
@@ -183,24 +236,16 @@ pbus_assign_resources(struct pci_bus *bu
ranges->found_vga = 1;
/* Propogate presence of the VGA to upstream bridges */
for (b = bus; b->parent; b = b->parent) {
-#if 0
- /* ? Do we actually need to enable PF memory? */
- b->resource[2]->start = 0;
-#endif
b->resource[0]->flags |= IORESOURCE_BUS_HAS_VGA;
}
}
+
for (ln=bus->children.next; ln != &bus->children; ln=ln->next) {
struct pci_bus *b = pci_bus_b(ln);
- b->resource[0]->start = ranges->io_start = ranges->io_end;
- b->resource[1]->start = ranges->mem_start = ranges->mem_end;
-
+ ranges->io_start = ranges->io_end;
+ ranges->mem_start = ranges->mem_end;
pbus_assign_resources(b, ranges);
-
- b->resource[0]->end = ranges->io_end - 1;
- b->resource[1]->end = ranges->mem_end - 1;
-
pci_setup_bridge(b);
}
}
Index: drivers/pci/setup-res.c
===================================================================
RCS file: /home/cvs/parisc/linux/drivers/pci/setup-res.c,v
retrieving revision 1.1.1.3
retrieving revision 1.7
diff -u -p -r1.1.1.3 -r1.7
--- setup-res.c 2001/01/09 16:57:56 1.1.1.3
+++ setup-res.c 2001/02/22 01:11:47 1.7
@@ -14,6 +14,8 @@
/*
* Nov 2000, Ivan Kokshaysky <[email protected]>
* Resource sorting
+ * Feb 2001, Grant Grundler <[email protected]>
+ * Fix PCI-PCI bridge support and add __hppa__ support
*/
#include <linux/init.h>
@@ -25,7 +27,7 @@
#include <linux/slab.h>
-#define DEBUG_CONFIG 1
+#define DEBUG_CONFIG 0
#if DEBUG_CONFIG
# define DBGC(args) printk args
#else
@@ -115,7 +117,7 @@ pci_assign_resource(struct pci_dev *dev,
* window (it will just not perform as well).
*/
if (!(res->flags & IORESOURCE_PREFETCH) || pci_assign_bus_resource(bus, dev, res, size, min, 0, i) < 0) {
- printk(KERN_ERR "PCI: Failed to allocate resource %d for %s\n", i, dev->name);
+ printk(KERN_ERR "PCI: Failed to allocate resource %d for %s\n", i, dev->slot_name);
return -EBUSY;
}
}
@@ -138,11 +140,13 @@ pdev_sort_resources(struct pci_dev *dev,
struct resource_list *list, *tmp;
unsigned long r_size;
+#ifndef __hppa__
/* PCI-PCI bridges may have I/O ports or
memory on the primary bus */
if (dev->class >> 8 == PCI_CLASS_BRIDGE_PCI &&
i >= PCI_BRIDGE_RESOURCES)
continue;
+#endif
r = &dev->resource[i];
r_size = r->end - r->start;
Grant Grundler wrote:
> Index: drivers/pci/pci.c
> ===================================================================
> RCS file: /home/cvs/parisc/linux/drivers/pci/pci.c,v
> retrieving revision 1.1.1.6
> diff -u -p -r1.1.1.6 pci.c
> --- pci.c 2001/01/09 16:57:56 1.1.1.6
> +++ pci.c 2001/03/02 18:44:59
> @@ -644,12 +645,16 @@ void __init pci_read_bridge_bases(struct
> } else {
> +
> /*
> - * Ugh. We don't know enough about this bridge. Just assume
> - * that it's entirely transparent.
> + * Either this is not a PCI-PCI bridge or it's not
> + * configured yet. Since this code only supports PCI-PCI
> + * bridge, we better not be called for any other type.
> + * Don't muck the resources since it will confuse the
> + * platform specific code which does that.
> */
> - printk("Unknown bridge resource %d: assuming transparent\n", 0);
> - child->resource[0] = child->parent->resource[0];
> + printk("PCI : ignoring %s PCI-PCI bridge (I/O BASE not configured)\n", child->self->slot_name);
> + return;
> }
>
> res = child->resource[1];
> @@ -664,8 +669,8 @@ void __init pci_read_bridge_bases(struct
> res->name = child->name;
> } else {
> /* See comment above. Same thing */
> - printk("Unknown bridge resource %d: assuming transparent\n", 1);
> - child->resource[1] = child->parent->resource[1];
> + printk("PCI : ignoring %s PCI-PCI bridge (MMIO base not configured)\n", child->self->slot_name);
> + return;
> }
>
> res = child->resource[2];
> @@ -690,11 +695,10 @@ void __init pci_read_bridge_bases(struct
> res->end = limit + 0xfffff;
> res->name = child->name;
> } else {
> - /* See comments above */
> - printk("Unknown bridge resource %d: assuming transparent\n", 2);
> - child->resource[2] = child->parent->resource[2];
> + /* Base > limit means the prefetchable mem is disabled.*/
> }
IIRC these "assuming transparent" lines were put in to -fix- PCI-PCI
bridges on at least some x86 boxes... I didn't really understand the
bridge code well enough at the time to comment one way or the other on
its correctness, but it definitely fixed some problems.
Jeff
--
Jeff Garzik | "You see, in this world there's two kinds of
Building 1024 | people, my friend: Those with loaded guns
MandrakeSoft | and those who dig. You dig." --Blondie
Jeff Garzik wrote:
> IIRC these "assuming transparent" lines were put in to -fix- PCI-PCI
> bridges on at least some x86 boxes... I didn't really understand the
> bridge code well enough at the time to comment one way or the other on
> its correctness, but it definitely fixed some problems.
Jeff,
If someone could clarify, I'd be happy to rework/resubmit the patch.
My gut feeling is it was to support something other than a PCI-PCI bridge.
pci_read_bridge_bases() assumes the device is a PCI-PCI Bridge (layout
and interpretation of the window registers). Either the code needs to
be more explicit about the type of bridge being handled or the caller
(arch specific code) should.
Only x86 and parisc PCI support call this code in my 2.4.0 tree.
Maybe the right answer is the "assuming transperent" support in
pci_read_bridge_bases() move to arch/x86.
I'm pretty sure Alpha and parisc/PAT_PDC systems don't use this
code since the registers programmed in pci_setup_bridge().
This makes me think none of the other arches attempt to
support PCI-PCI bridges. Is that correct?
thanks,
grant
Grant Grundler
parisc-linux {PCI|IOMMU|SMP} hacker
+1.408.447.7253
--- rum-lspci-before.txt Fri Mar 2 17:42:23 2001
+++ rum-lspci-after.txt Fri Mar 2 17:07:26 2001
@@ -88,7 +88,7 @@
Status: Cap+ 66Mhz+ UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
Latency: 64 (1250ns min, 63750ns max), cache line size 08
Interrupt: pin A routed to IRQ 16
- Region 0: Memory at d0000000 (32-bit, non-prefetchable) [size=128M]
+ Region 0: Memory at <ignored> (32-bit, non-prefetchable) [size=128M]
Expansion ROM at e5ff0000 [disabled] [size=64K]
Capabilities: [dc] Power Management version 1
Flags: PMEClk- DSI+ D1+ D2+ AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot-,D3cold-)
Jeff Garzik wrote:
> The patch worked 100% on my laptop, but failed to allocate a PCI memory
> region on my desktop machine. Two attachments... "diff -u" output for
> dmesg before and after your patch, and "diff -u" output for lspci before
> and after your patch.
Jeff,
Thanks for trying. I'll rework and resubmit later.
Can you send me the complete lspci output of your desktop?
(either with or without the patch)
I'd like to pull whatever docs I can find on the offending bridge.
I'll also look at moving "transparent Bridge" support to x86
pci_fixup_bus() code (and see if I can find a machine locally
which has this same "feature").
thanks,
grant
Grant Grundler
parisc-linux {PCI|IOMMU|SMP} hacker
+1.408.447.7253
On Fri, Mar 02, 2001 at 11:32:35AM -0800, Grant Grundler wrote:
> Code in parisc-linux CVS (based on 2.4.0) does boot on my OB800
> (133Mhz Pentium), C3000, and A500 with PCI-PCI bridge support
> working. I'm quite certain PCI-PCI bridge configuration (ie BIOS
> didn't configure the bridge) support was broken.
I believe it isn't. ;-) It works on various alphas including
configurations with chained PCI-PCI bridges.
Some comments on the patch:
> +** If I/O or MEM ranges are overlapping, that's a BIOS bug.
No. As we reallocate everything, it is quite possible that we'll
have temporary overlaps during setup with resources allocated
by BIOS. I'm not sure if it is harmful though.
> +#ifdef __hppa__
> +/* XXX FIXME
> +** PCI_BRIDGE_CONTROL and PCI_COMMAND programming need to be revisited
> +** to support FBB. Make all this crud "configurable" by the arch specific
> +** (ie "PCI BIOS") support and the ifdef __hppa__ crap can go away then.
> +*/
Agreed. Something like pcibios_set_bridge_control().
> for (ln=bus->children.next; ln != &bus->children; ln=ln->next) {
> struct pci_bus *b = pci_bus_b(ln);
>
> - b->resource[0]->start = ranges->io_start = ranges->io_end;
> - b->resource[1]->start = ranges->mem_start = ranges->mem_end;
> -
> + ranges->io_start = ranges->io_end;
> + ranges->mem_start = ranges->mem_end;
> pbus_assign_resources(b, ranges);
> -
> - b->resource[0]->end = ranges->io_end - 1;
> - b->resource[1]->end = ranges->mem_end - 1;
> -
> pci_setup_bridge(b);
> }
This change totally breaks PCI allocation logic.
Probably you assign PCI-PCI bridge windows in arch specific
code - why?
The only thing you need is to set up the root bus resources
properly and generic code will do the rest.
> +#ifndef __hppa__
> /* PCI-PCI bridges may have I/O ports or
> memory on the primary bus */
> if (dev->class >> 8 == PCI_CLASS_BRIDGE_PCI &&
> i >= PCI_BRIDGE_RESOURCES)
> continue;
> +#endif
Same here.
Ivan.
Ivan Kokshaysky wrote:
> On Fri, Mar 02, 2001 at 11:32:35AM -0800, Grant Grundler wrote:
> > Code in parisc-linux CVS (based on 2.4.0) does boot on my OB800
> > (133Mhz Pentium), C3000, and A500 with PCI-PCI bridge support
> > working. I'm quite certain PCI-PCI bridge configuration (ie BIOS
> > didn't configure the bridge) support was broken.
>
> I believe it isn't. ;-) It works on various alphas including
> configurations with chained PCI-PCI bridges.
Ok. I overlooked the changes in arch/alpha/kernel/pci.c:pcibios_fixup_bus().
(As you know, things changed alot between -test10 and -test12)
> Some comments on the patch:
>
> > +** If I/O or MEM ranges are overlapping, that's a BIOS bug.
>
> No. As we reallocate everything, it is quite possible that we'll
> have temporary overlaps during setup with resources allocated
> by BIOS. I'm not sure if it is harmful though.
The other part of the comment I added was:
+** Disabling *all* devices is bad. Console, root, etc get
+** disabled this way.
I can't debug with *all* devices disabled.
Normally, the whole point of resource mgt is to (a) avoid overlaps
and (b) reflect the state of the HW. I thought the arch specific code
was responsible for setting the HW state and resource mgt state congruent.
If the arch/alpha code wants everything reallocated anyway, why not have
the arch code disable the HW during the bus walk in pcibios_fixup_bus()
before calling pci_assign_unassigned_resources()?
(I'm looking at 2.4.0 linux/arch/alpha/kernel/pci.c:common_init_pci() )
FYI, under PDC PAT (eg A500), unused devices are left in the "power
on" state (which AFAIK, implies disabled).
> > +#ifdef __hppa__
> > +/* XXX FIXME
> > +** PCI_BRIDGE_CONTROL and PCI_COMMAND programming need to be revisited
> > +** to support FBB. Make all this crud "configurable" by the arch specific
> > +** (ie "PCI BIOS") support and the ifdef __hppa__ crap can go away then.
> > +*/
>
> Agreed. Something like pcibios_set_bridge_control().
possibly...I have another problem I wanted to solve - FBB support.
I think generic Fast Back-Back support wants a new field in struct pci_bus
(u32 bridge_ctl) to save and manage the FBB state (and SERR/PERR).
Arch support would need a way to initialize bridge_ctl *before*
pci_do_scan_bus() is called to indicate FBB is or is not supported
by the parent PCI bus controller (either HBA or PCI-PCI Bridge).
Originally I was thinking of seperating the "root" bus allocation
from pci_scan_bus(). But calling pcibios_set_bridge_control()
before the bus walk would work too if it passes struct pci_bus *
as the parameter. And that could allow arch specific control over
SERR/PERR bits as well.
In pcibios_fixup_bus(), the arch code could check FBB state to see
if it should be enabled on that HBA or not. Ideally, generic code
would fully handle FBB for PCI-PCI secondary busses. Perhaps the FBB
test could be in pci_setup_bridge() but I'm not sure if that would work
for all arches (ie not sure off hand which uses pci_setup_bridge()).
[ deleted code changes in
drivers/pci/setup-bus.c:pbus_assign_resources()
driver/pci/setup-res.c:pdev_sort_resources()
]
> This change totally breaks PCI allocation logic.
> Probably you assign PCI-PCI bridge windows in arch specific code - why?
I think my change in pdev_sort_resources() permitted it to occur in
generic code. parisc HBA code only calls request_resources for resources
assigned by firmware to the HBA.
> The only thing you need is to set up the root bus resources
> properly and generic code will do the rest.
hmmm...Code in alpha's pcibios_fixup_bus() modifies PCI-PCI Bridge
resources. It wouldn't if your statement were literally true.
I reversed the two changes in my tree to see what breaks on A500:
| lba version TR4.0 (0x5) found at 0xfffffffffed3c000
| lba_fixup_bus(0x0000000018b4b780) bus 48 sysdata 0x0000000018b4a800
| lba_fixup_bus() LBA I/O Port [30000/3ffff]/100
| lba_fixup_bus() LBA LMMIO [fffffffffb000000/fffffffffb7fffff]/200
| lba_fixup_bus(0x0000000018b4b880) bus 49 sysdata 0x0000000018b4a800
| lba_fixup_bus(0x0000000018b4b980) bus 50 sysdata 0x0000000018b4a800
| PCI: Failed to allocate resource 0 for 31:04.0
| PCI: Failed to allocate resource 0 for 31:04.1
[ I have a 4-port 100BT card and a 2-port 100BT/896 SCSI "combo" card
installed in bus 48 - both have PCI-PCI bridges.
No resources are available for any devices under either PPB.
]
...
| tulip: eth1: I/O region (0xfffffffffffd0000@0x31000) unavailable, aborting
...
| sym53c896-6: rev 0x5 on pci bus 49 device 4 function 0 irq 320
| sym53c896-6: ID 7, Fast-40, Parity Checking
| sym53c896-6: on-chip RAM at 0xfffffffffb100000
| CACHE TEST FAILED: reg dstat-sstat2 readback ffffffff.
| CACHE INCORRECTLY CONFIGURED.
...
Should I try to follow alpha's pcibios_fixup_bus() and add the code following
(linux 2.4.0, arch/alpha/kernel/pci.c line 256)
/* This is a bridge. Do not care how it's initialized,
just link its resources to the bus ones */
to the parisc pcibios_fixup_bus() ?
Or do you want to change how alpha works to follow what you said?
I would prefer this but it doesn't matter ATM; just needs to work.
thanks!
grant
Grant Grundler
parisc-linux {PCI|IOMMU|SMP} hacker
+1.408.447.7253
On Mon, Mar 05, 2001 at 02:16:05PM -0800, Grant Grundler wrote:
> > I believe it isn't. ;-) It works on various alphas including
> > configurations with chained PCI-PCI bridges.
>
> Ok. I overlooked the changes in arch/alpha/kernel/pci.c:pcibios_fixup_bus().
> (As you know, things changed alot between -test10 and -test12)
Yes, all that stuff appeared in -test12, IIRC.
Now ARM port uses it too, BTW.
> > > +** If I/O or MEM ranges are overlapping, that's a BIOS bug.
> >
> > No. As we reallocate everything, it is quite possible that we'll
> > have temporary overlaps during setup with resources allocated
> > by BIOS. I'm not sure if it is harmful though.
>
> The other part of the comment I added was:
> +** Disabling *all* devices is bad. Console, root, etc get
> +** disabled this way.
>
> I can't debug with *all* devices disabled.
What is why I leave VGAs and all sorts of bridges enabled.
If you have some other type of console sitting on the PCI
bus you need this enabled as well, of course.
> Normally, the whole point of resource mgt is to (a) avoid overlaps
> and (b) reflect the state of the HW. I thought the arch specific code
> was responsible for setting the HW state and resource mgt state congruent.
> If the arch/alpha code wants everything reallocated anyway, why not have
> the arch code disable the HW during the bus walk in pcibios_fixup_bus()
> before calling pci_assign_unassigned_resources()?
It's possible.
> > > +** PCI_BRIDGE_CONTROL and PCI_COMMAND programming need to be revisited
> > > +** to support FBB. Make all this crud "configurable" by the arch specific
> > > +** (ie "PCI BIOS") support and the ifdef __hppa__ crap can go away then.
> > > +*/
> >
> > Agreed. Something like pcibios_set_bridge_control().
>
> possibly...I have another problem I wanted to solve - FBB support.
>
> I think generic Fast Back-Back support wants a new field in struct pci_bus
> (u32 bridge_ctl) to save and manage the FBB state (and SERR/PERR).
> Arch support would need a way to initialize bridge_ctl *before*
> pci_do_scan_bus() is called to indicate FBB is or is not supported
> by the parent PCI bus controller (either HBA or PCI-PCI Bridge).
Or let pci_do_scan_bus() check the FBB capability of all
devices on the given bus and then arch code will decide whether
enable FBB or not on the bus controller.
Anyway, new member in struct pci_bus makes a sense.
> I think my change in pdev_sort_resources() permitted it to occur in
> generic code. parisc HBA code only calls request_resources for resources
> assigned by firmware to the HBA.
I'm afraid I don't understand how that could affect hppa. PCI-to-PCI
bridge specification allows PCI-PCI bridge to have some device-specific
IO, MEM or ROM resources. If any of these are present, we must allocate
them properly.
> > The only thing you need is to set up the root bus resources
> > properly and generic code will do the rest.
>
> hmmm...Code in alpha's pcibios_fixup_bus() modifies PCI-PCI Bridge
> resources. It wouldn't if your statement were literally true.
Oh, well. That code actually initializes limits of the bridge
windows with the values of the root bus to make a room for
resource allocation. I can't recall now why it was placed in the
arch code. Maybe one of the reasons was that I wasn't sure whether
someone want to use prefetchable memory...
> Should I try to follow alpha's pcibios_fixup_bus() and add the code following
> (linux 2.4.0, arch/alpha/kernel/pci.c line 256)
>
> /* This is a bridge. Do not care how it's initialized,
> just link its resources to the bus ones */
>
> to the parisc pcibios_fixup_bus() ?
>
> Or do you want to change how alpha works to follow what you said?
You mean something like this (moving code from arch/alpha to generic)?
--- 2.4.2/drivers/pci/setup-bus.c Tue Dec 12 00:46:26 2000
+++ linux/drivers/pci/setup-bus.c Tue Mar 6 19:46:30 2001
@@ -192,9 +192,23 @@ pbus_assign_resources(struct pci_bus *bu
}
for (ln=bus->children.next; ln != &bus->children; ln=ln->next) {
struct pci_bus *b = pci_bus_b(ln);
+ struct pci_dev *bridge = b->self;
+ int i;
+ for(i=0; i<3; i++) {
+ b->resource[i] =
+ &bridge->resource[PCI_BRIDGE_RESOURCES+i];
+ b->resource[i]->name = bus->name;
+ }
+ b->resource[0]->flags |= pci_bridge_check_io(dev);
+ b->resource[1]->flags |= IORESOURCE_MEM;
b->resource[0]->start = ranges->io_start = ranges->io_end;
b->resource[1]->start = ranges->mem_start = ranges->mem_end;
+ b->resource[0]->end = bus->resource[0]->end;
+ b->resource[1]->end = bus->resource[1]->end;
+ /* Turn off downstream PF memory address range */
+ bus->resource[2]->start = 1024*1024;
+ bus->resource[2]->end = 0;
pbus_assign_resources(b, ranges);
> I would prefer this but it doesn't matter ATM; just needs to work.
Certainly all this should be cleaned up in 2.5; I'm not sure for 2.4.
Ivan.
Ivan Kokshaysky wrote:
> Yes, all that stuff appeared in -test12, IIRC.
> Now ARM port uses it too, BTW.
ok. I'll keep that in mind.
> > I can't debug with *all* devices disabled.
>
> What is why I leave VGAs and all sorts of bridges enabled.
> If you have some other type of console sitting on the PCI
> bus you need this enabled as well, of course.
My A500 console is a "regular" PCI serial device.
[ I use quotes because linux sees the device as a 16550.
But I'm told it's fully emulated in firmware on a special card called
the "GSP" (Guardian Service Processor). ]
> > If the arch/alpha code wants everything reallocated anyway, why not have
> > the arch code disable the HW during the bus walk in pcibios_fixup_bus()
> > before calling pci_assign_unassigned_resources()?
>
> It's possible.
Ok. I can't implement/test this since I don't have an Alpha box.
I'm not going to submit patches to l-k for code I can't test.
If someone has an old alpha box I can use to test PCI developement
(if a jensen is suitable, fine), I could easily trade for a 712 or 715
workstation (128MB mem, 2GB SCSI).
[ re FBB proposal ]
> Or let pci_do_scan_bus() check the FBB capability of all
> devices on the given bus and then arch code will decide whether
> enable FBB or not on the bus controller.
Yes - that's the part I ommitted to keep my proposal shorter. :^)
So it's not "Or" - I intended "And let..."
Initializing the FBB bit in bridge_ctl before pci_do_scan_bus() gets
called would allow pci_do_scan_bus() to clear FBB bit if any device
on that bus does not support FBB. Arch support can then program
the HBA's FBB capability to match in pcibios_fixup_bus().
The advantage here is:
o arch's not looking at or setting FBB would continue to work as before.
o pci_do_scan_bus() can treat secondary busses (ie below PCI-PCI Bridge)
the same since it doesn't need to worry about who uses the FBB state.
(pci_setup_bridge() could deal with it for PCI-PCI bridges.)
> Anyway, new member in struct pci_bus makes a sense.
Ok. I was hoping to work on an FBB patch this week but I first
need to get parisc-linux support with linus' generic PCI tree.
I'd like to have parisc linux in sync with some version of
AC's or Linus' tree before starting that.
[ re patch to pdev_sort_resources() ]
> I'm afraid I don't understand how that could affect hppa. PCI-to-PCI
> bridge specification allows PCI-PCI bridge to have some device-specific
> IO, MEM or ROM resources. If any of these are present, we must allocate
> them properly.
Agreed. Removing the following "if/continue" statement doesn't
affect device-specific (aka "built-in") IO/MEM/ROM resources.
| if (dev->class >> 8 == PCI_CLASS_BRIDGE_PCI &&
| i >= PCI_BRIDGE_RESOURCES)
| continue;
This code causes the outer loop to not link the &dev->resource[i] into
the "sorted list" for the *secondary* bus.
(ie PCI_BRIDGE_RESOURCES <= i < PCI_NUM_RESOURCES)
By including the secondary bus "window" resources in the sorted list,
the resources for the PCI-PCI Bridge window registers are allocated
too. Thus the change in pbus_assign_resources() to avoid clobbering
the contents already placed in the sorted list.
I worked this out before but right now am not 100% certain some
details haven't escaped me. But I still think I'm on the right track.
I know it works on an A500. I've reviewed the resulting resource tree
and am certain it was correct for the configuration I tested.
[ re arch/alpha code ]
> Oh, well. That code actually initializes limits of the bridge
> windows with the values of the root bus to make a room for
> resource allocation. I can't recall now why it was placed in the
> arch code. Maybe one of the reasons was that I wasn't sure whether
> someone want to use prefetchable memory...
Ok. If it can be removed, I'd be happier since it means I wouldn't
need similar code in arch/parisc.
> You mean something like this (moving code from arch/alpha to generic)?
>
> --- 2.4.2/drivers/pci/setup-bus.c Tue Dec 12 00:46:26 2000
> +++ linux/drivers/pci/setup-bus.c Tue Mar 6 19:46:30 2001
> @@ -192,9 +192,23 @@ pbus_assign_resources(struct pci_bus *bu
> }
> for (ln=bus->children.next; ln != &bus->children; ln=ln->next) {
> struct pci_bus *b = pci_bus_b(ln);
> + struct pci_dev *bridge = b->self;
> + int i;
>
> + for(i=0; i<3; i++) {
> + b->resource[i] =
> + &bridge->resource[PCI_BRIDGE_RESOURCES+i];
> + b->resource[i]->name = bus->name;
> + }
> + b->resource[0]->flags |= pci_bridge_check_io(dev);
> + b->resource[1]->flags |= IORESOURCE_MEM;
> b->resource[0]->start = ranges->io_start = ranges->io_end;
> b->resource[1]->start = ranges->mem_start = ranges->mem_end;
> + b->resource[0]->end = bus->resource[0]->end;
> + b->resource[1]->end = bus->resource[1]->end;
> + /* Turn off downstream PF memory address range */
> + bus->resource[2]->start = 1024*1024;
> + bus->resource[2]->end = 0;
>
> pbus_assign_resources(b, ranges);
>
Yes, sort of. If my patch to pdev_sort_resources() makes sense now,
I'm not sure this is needed either.
My first reaction was initialization of b->resource pointer would have
to happen earlier in order to match arch code in pcibios_fixup_bus().
The idea being generic PCI code *in general* do the same things for
primary bus resources as secondary bus resources (ie window registers).
> > I would prefer this but it doesn't matter ATM; just needs to work.
>
> Certainly all this should be cleaned up in 2.5; I'm not sure for 2.4.
I don't think existing PCI code is very "dirty". Understanding
the interactions between generic and arch PCI code is non-trivial.
But it's not that bad. Understanding how various arches use the
code is the hard part.
parisc support is mostly in place in 2.4.0. It would be quite
frustrating to not have it fully working in a 2.4.x release because
of 10 or 20 lines of code change. FBB support will cause more change
than what I've proposed for in the patch parisc support.
thanks again,
grant
ps. Ivan - this has been a good exchange since it's forcing me to revisit
code I haven't looked at in a few monthes with a fresh perspective.
This (and my previous) reply took me about 4 hours to write.
I have to keep looking at code. :^)
>
> Ivan.
Grant Grundler
parisc-linux {PCI|IOMMU|SMP} hacker
+1.408.447.7253
On Tue, Mar 06, 2001 at 12:57:00PM -0800, Grant Grundler wrote:
> My A500 console is a "regular" PCI serial device.
>
> [ I use quotes because linux sees the device as a 16550.
> But I'm told it's fully emulated in firmware on a special card called
> the "GSP" (Guardian Service Processor). ]
Ok. Maybe this change would make a sense in general case:
- else if (class >> 8 != PCI_BASE_CLASS_BRIDGE) {
+ else if (class >> 8 != PCI_BASE_CLASS_BRIDGE ||
+ class >> 8 != PCI_BASE_CLASS_COMMUNICATION) {
pci_read_config_word(dev, PCI_COMMAND, &cmd);
...
> [ re FBB proposal ]
...
Basically I agree. I think you will need to discuss it with Martin, as it
will touch all platforms.
> [ re patch to pdev_sort_resources() ]
>
> > I'm afraid I don't understand how that could affect hppa. PCI-to-PCI
> > bridge specification allows PCI-PCI bridge to have some device-specific
> > IO, MEM or ROM resources. If any of these are present, we must allocate
> > them properly.
>
> Agreed. Removing the following "if/continue" statement doesn't
> affect device-specific (aka "built-in") IO/MEM/ROM resources.
>
>
> | if (dev->class >> 8 == PCI_CLASS_BRIDGE_PCI &&
> | i >= PCI_BRIDGE_RESOURCES)
> | continue;
>
> This code causes the outer loop to not link the &dev->resource[i] into
> the "sorted list" for the *secondary* bus.
> (ie PCI_BRIDGE_RESOURCES <= i < PCI_NUM_RESOURCES)
>
> By including the secondary bus "window" resources in the sorted list,
> the resources for the PCI-PCI Bridge window registers are allocated
> too. Thus the change in pbus_assign_resources() to avoid clobbering
> the contents already placed in the sorted list.
>
> I worked this out before but right now am not 100% certain some
> details haven't escaped me. But I still think I'm on the right track.
> I know it works on an A500. I've reviewed the resulting resource tree
> and am certain it was correct for the configuration I tested.
Well, it seems that I finally see what is wrong with your code, and
why it worked in your case. You assume that "window" resources of
the bridge are already known when we call pbus_assign_resources_sorted().
This is incorrect. Probably you rely on pci_read_bridge_bases() doing
something meaningful (I looked at the parisc pci code in current 2.4.x,
don't know about your CVS tree). Yes, at least some of the DEC bridges
after power-up/reset have 0s in base/limit registers. This means
that you have ranges 0000-0fff (4K) for IO and 00000000-000fffff (1M)
for MEM. Obviously it's enough to hold all resources on the
cards you've tested, but it won't work in common case. There is
a lot of reasons why; just a couple of them:
- according to PPB specification, base/limits registers of the bridge
after reset are *undefined*, so you'll probably have troubles
with non-DEC bridges.
- there is a number of alpha systems with a built-in PCI-PCI bridge
and real PCI slots behind it. Obviously 4K/1M isn't enough for
these systems, and it was the main reason of rewriting that code.
etc etc etc.
Basically, you won't know bridge "window" size for a given bus until
you'll have allocated *all* devices on *all* its child busses.
Besides, including bridge resources in the "sort lists" is meaningless,
since these resources have fixed alignment - 4K for IO and 1M for MEM,
unlike "regular" ones, which alignment == size.
> [ re arch/alpha code ]
...
>
> Ok. If it can be removed, I'd be happier since it means I wouldn't
> need similar code in arch/parisc.
Unfortunately I haven't anything with a bridge handy at the moment
to test that patch. Besides, we'll have here a sort of holidays till
Sunday. So maybe next week...
> I don't think existing PCI code is very "dirty".
I hope so. :-)
However, some problems need to be worked out:
1. generic vs. arch code - we've already discussed some of these
2. Prefetchable Memory - do we need to deal with it? Though looking
at modern x86 systems I tend to keep it disabled :-)
3. pdev_enable_device() - it's a bit ugly, confuses people and
possibly is not needed at all.
> ps. Ivan - this has been a good exchange since it's forcing me to revisit
> code I haven't looked at in a few monthes with a fresh perspective.
> This (and my previous) reply took me about 4 hours to write.
> I have to keep looking at code. :^)
Same with me - I've started to forget all that stuff...
thank you,
Ivan.
Ivan Kokshaysky wrote:
...
> Well, it seems that I finally see what is wrong with your code, and
> why it worked in your case. You assume that "window" resources of
> the bridge are already known when we call pbus_assign_resources_sorted().
> This is incorrect.
Oh...do we know the "sizes" of all child resources from the bus walk?
I'll check that and see if it can be safely changed.
> Probably you rely on pci_read_bridge_bases() doing
> something meaningful (I looked at the parisc pci code in current 2.4.x,
> don't know about your CVS tree).
Nope - don't call that for A500 (machines with PDC PAT)...that might in
fact be another problem later related to some PDC (aka BIOS) changes.
> Yes, at least some of the DEC bridges
> after power-up/reset have 0s in base/limit registers. This means
> that you have ranges 0000-0fff (4K) for IO and 00000000-000fffff (1M)
> for MEM. Obviously it's enough to hold all resources on the
> cards you've tested, but it won't work in common case. There is
> a lot of reasons why; just a couple of them:
> - according to PPB specification, base/limits registers of the bridge
> after reset are *undefined*, so you'll probably have troubles
> with non-DEC bridges.
> - there is a number of alpha systems with a built-in PCI-PCI bridge
> and real PCI slots behind it. Obviously 4K/1M isn't enough for
> these systems, and it was the main reason of rewriting that code.
> etc etc etc.
Yup - I think you are right on all counts here.
I'll rework the parisc code tonight/tomorrow and see if I can get rid
of the contentious generic PCI changes. I should be able to.
> Basically, you won't know bridge "window" size for a given bus until
> you'll have allocated *all* devices on *all* its child busses.
Linux doesn't. It's possible to deal with window register size in
the initial bus walk (where BAR sizes are determined).
> Besides, including bridge resources in the "sort lists" is meaningless,
> since these resources have fixed alignment - 4K for IO and 1M for MEM,
> unlike "regular" ones, which alignment == size.
The alignment would have to be handled correctly and I thought
pcibios_align_resource() did that. I see now the arch/parisc one doesn't
and others probably don't either. Let me think about this more...
> Unfortunately I haven't anything with a bridge handy at the moment
> to test that patch. Besides, we'll have here a sort of holidays till
> Sunday. So maybe next week...
np. thanks.
>
> > I don't think existing PCI code is very "dirty".
>
> I hope so. :-)
:^)
> However, some problems need to be worked out:
> 1. generic vs. arch code - we've already discussed some of these
> 2. Prefetchable Memory - do we need to deal with it? Though looking
> at modern x86 systems I tend to keep it disabled :-)
Ditto for parisc.
> 3. pdev_enable_device() - it's a bit ugly, confuses people and
> possibly is not needed at all.
Agreed.
thanks,
grant
Grant Grundler
parisc-linux {PCI|IOMMU|SMP} hacker
+1.408.447.7253