[parisc-linux] proposed changes to dino.c

[parisc-linux] proposed changes to dino.c

[James Bottomley]

This updates dino to configure unassigned resources (primarily so I can
get it to recognise my yenta cardbus bridge).

The side effect is that it will reorder all the resources in the dino
window.

James

===== drivers/parisc/dino.c 1.9 vs edited =====
--- 1.9/drivers/parisc/dino.c	Fri Oct 10 05:11:03 2003
+++ edited/drivers/parisc/dino.c	Tue Dec  9 22:33:18 2003
@@ -165,6 +165,13 @@
 
 #define DINO_CFG_TOK(bus,dfn,pos) ((u32) ((bus)<<16 | (dfn)<<8 | (pos)))
 
+/*
+ * keep the current highest bus count to assist in allocating busses.  This
+ * tries to keep a global bus count total so that when we discover an 
+ * entirely new bus, it can be given a unique bus number.
+ */
+static int dino_current_bus = 0;
+
 static int dino_cfg_read(struct pci_bus *bus, unsigned int devfn, int where,
 		int size, u32 *val)
 {
@@ -559,8 +566,14 @@
 			__FUNCTION__, bus, bus->secondary, bus->dev->platform_data);
 
 	/* Firmware doesn't set up card-mode dino, so we have to */
-	if (is_card_dino(&dino_dev->hba.dev->id))
+	if (is_card_dino(&dino_dev->hba.dev->id)) {
 		dino_card_setup(bus, dino_dev->hba.base_addr);
+	} else {
+		bus->resource[0] = &(dino_dev->hba.io_space);
+		bus->resource[1] = &(dino_dev->hba.lmmio_space); 
+		pci_bus_assign_resources(bus);
+		pci_enable_bridges(bus);
+	}
 
 	/* If this is a PCI-PCI Bridge, read the window registers etc */
 	if (bus->self)
@@ -595,9 +608,29 @@
 			}
 #endif
 		}
+		if(dev->irq == 255) {
+#if 0
+			/* This code tries to assign an unassigned interrupt
+			 * leave it disabled unless you know what you're doing
+			 * since the pin<->interrupt line mapping varies
+			 * by bus and machine */
+
+			u32 irq_pin;
+			
+			dino_cfg_read(dev->bus, dev->devfn, PCI_INTERRUPT_PIN, 1, &irq_pin);
+			dev->irq = (irq_pin + PCI_SLOT(dev->devfn)) % 4 ;
+			dino_cfg_write(dev->bus, dev->devfn, PCI_INTERRUPT_LINE, 1, dev->irq);
+			dev->irq += dino_dev->dino_region->data.irqbase
+			printk(KERN_WARNING "Device %s has undefined IRQ, setting to %d\n", dev->slot_name, irq_pin);
+#else
+			dev->irq = 65535;
+			printk(KERN_WARNING "Device %s has unassigned IRQ\n", dev->slot_name);	
+#endif
+		} else {
 
-		/* Adjust INT_LINE for that busses region */
-		dev->irq = dino_dev->dino_region->data.irqbase + dev->irq;
+			/* Adjust INT_LINE for that busses region */
+			dev->irq += dino_dev->dino_region->data.irqbase;
+		}
 	}
 }
 
@@ -826,6 +859,7 @@
 	const int name_len = 32;
 	char *name;
 	int is_cujo = 0;
+	struct pci_bus *bus;
 
 	name = kmalloc(name_len, GFP_KERNEL);
 	if(name)
@@ -911,9 +945,19 @@
 	** It's not used to avoid chicken/egg problems
 	** with configuration accessor functions.
 	*/
-	dino_dev->hba.hba_bus = 
-		pci_scan_bus_parented(&dev->dev, dino_dev->hba.hba_num,
-				      &dino_cfg_ops, NULL);
+	bus = pci_scan_bus_parented(&dev->dev, dino_current_bus,
+				    &dino_cfg_ops, NULL);
+	if(bus) {
+		/* This code *depends* on scanning being single threaded
+		 * if it isn't, this global bus number count will fail
+		 */
+		dino_current_bus = bus->subordinate + 1;
+	} else {
+		printk(KERN_ERR "ERROR: failed to scan PCI bus on %s (probably duplicate bus number %d)\n", dev->dev.bus_id, dino_current_bus);
+		/* increment the bus number in case of duplicates */
+		dino_current_bus++;
+	}
+	dino_dev->hba.hba_bus = bus;
 	return 0;
 }
 

Re: [parisc-linux] proposed changes to dino.c

[Grant Grundler]

On Tue, Dec 09, 2003 at 11:37:16PM -0500, James Bottomley wrote:
> This updates dino to configure unassigned resources (primarily so I can
> get it to recognise my yenta cardbus bridge).

James and I talked about this offline. Issue was card-mode dino
and the PCI yenta bridge were colliding on the bus numbering.
The card-mode Dino bus numbering was assuming no PCI-PCI bridges
for *all* the host bridges, not just the card-mode bridges.
I expect a combination of 4-port tulip and card-mode dinoe would
have exposed this problem too.

> The side effect is that it will reorder all the resources in the dino
> window.

I think this will be ok as long firmware doesn't try to remember
and PCI resources. STI is what I don't know about and worry a bit.

Otherwise the diff looks good to me. I trust it works for you
and hopefully a few other people can try it too. If not, we
can always fix it more later...

thanks,
grant

Re: [parisc-linux] proposed changes to dino.c

[James Bottomley]

On Tue, 2003-12-09 at 23:54, Grant Grundler wrote:
> James and I talked about this offline. Issue was card-mode dino
> and the PCI yenta bridge were colliding on the bus numbering.
> The card-mode Dino bus numbering was assuming no PCI-PCI bridges
> for *all* the host bridges, not just the card-mode bridges.
> I expect a combination of 4-port tulip and card-mode dinoe would
> have exposed this problem too.

Actually, I have a 4 and 2 port tulip card, and can verify that it does
(as expected) obscure the cardmode dino.  Obviously, any PCI card with
an internal bridge would cause the same problem.


> Otherwise the diff looks good to me. I trust it works for you
> and hopefully a few other people can try it too. If not, we
> can always fix it more later...

well, the bad news is that with the four port and the patches, the
machine hangs loading the tulip module...looks like some resource got
assigned to the wrong window.

James

Re: [parisc-linux] proposed changes to dino.c

[James Bottomley]

On Tue, 2003-12-09 at 23:54, Grant Grundler wrote:
> James and I talked about this offline. Issue was card-mode dino
> and the PCI yenta bridge were colliding on the bus numbering.
> The card-mode Dino bus numbering was assuming no PCI-PCI bridges
> for *all* the host bridges, not just the card-mode bridges.
> I expect a combination of 4-port tulip and card-mode dinoe would
> have exposed this problem too.

OK, the attached actually correctly configures and works with both the
cardbus bridge and the four port.  In order to get the four port
working, I've actually added bridge configuration code now, so the
cardbus dino with bridges should also actually work (although I didn't
remove the check on them).

Let me know how this works.

James

===== drivers/parisc/dino.c 1.9 vs edited =====
--- 1.9/drivers/parisc/dino.c	Fri Oct 10 05:11:03 2003
+++ edited/drivers/parisc/dino.c	Wed Dec 10 15:50:42 2003
@@ -165,6 +165,13 @@
 
 #define DINO_CFG_TOK(bus,dfn,pos) ((u32) ((bus)<<16 | (dfn)<<8 | (pos)))
 
+/*
+ * keep the current highest bus count to assist in allocating busses.  This
+ * tries to keep a global bus count total so that when we discover an 
+ * entirely new bus, it can be given a unique bus number.
+ */
+static int dino_current_bus = 0;
+
 static int dino_cfg_read(struct pci_bus *bus, unsigned int devfn, int where,
 		int size, u32 *val)
 {
@@ -504,8 +511,6 @@
 	DBG("DINO GSC WRITE i=%d, start=%lx, dino addr = %lx\n",
 	    i, res->start, base_addr + DINO_IO_ADDR_EN);
 	gsc_writel(1 << i, base_addr + DINO_IO_ADDR_EN);
-
-	pci_bus_assign_resources(bus);
 }
 
 static void __init
@@ -546,6 +551,9 @@
 	dino_cfg_write(dev->bus, dev->devfn, PCI_INTERRUPT_LINE, 1, dev->irq); 
 }
 
+/* The alignment contraints for PCI bridges under dino */
+#define DINO_BRIDGE_ALIGN 0x100000
+
 
 static void __init
 dino_fixup_bus(struct pci_bus *bus)
@@ -559,13 +567,48 @@
 			__FUNCTION__, bus, bus->secondary, bus->dev->platform_data);
 
 	/* Firmware doesn't set up card-mode dino, so we have to */
-	if (is_card_dino(&dino_dev->hba.dev->id))
+	if (is_card_dino(&dino_dev->hba.dev->id)) {
 		dino_card_setup(bus, dino_dev->hba.base_addr);
+	} else if(bus->parent == NULL) {
+		/* must have a dino above it, reparent the resources
+		 * into the dino window */
+		bus->resource[0] = &(dino_dev->hba.io_space);
+		bus->resource[1] = &(dino_dev->hba.lmmio_space); 
+	} else if(bus->self) {
+		int i;
 
-	/* If this is a PCI-PCI Bridge, read the window registers etc */
-	if (bus->self)
 		pci_read_bridge_bases(bus);
 
+
+		for(i = 0; i < PCI_NUM_RESOURCES; i++) {
+			if((bus->self->resource[i].flags & (IORESOURCE_IO | IORESOURCE_MEM)) == 0)
+				continue;
+			
+			if(bus->self->resource[i].flags & IORESOURCE_MEM) {
+				/* There's a quirk to alignment of
+				 * bridge memory resources: the start
+				 * is the alignment and start-end is
+				 * the size.  However, firmware will
+				 * have assigned start and end, so we
+				 * need to take this into account */
+				bus->self->resource[i].end = bus->self->resource[i].end - bus->self->resource[i].start + DINO_BRIDGE_ALIGN;
+				bus->self->resource[i].start = DINO_BRIDGE_ALIGN;
+				
+			}
+					
+			DBG("DEBUG %s assigning %d [0x%lx,0x%lx]\n",
+			    bus->self->dev.bus_id, i,
+			    bus->self->resource[i].start,
+			    bus->self->resource[i].end);
+			pci_assign_resource(bus->self, i);
+			DBG("DEBUG %s after assign %d [0x%lx,0x%lx]\n",
+			    bus->self->dev.bus_id, i,
+			    bus->self->resource[i].start,
+			    bus->self->resource[i].end);
+		}
+	}
+
+
 	list_for_each(ln, &bus->devices) {
 		int i;
 
@@ -595,9 +638,39 @@
 			}
 #endif
 		}
+		/* null out the ROM resource if there is one (we don't
+		 * care about an expansion rom on parisc, since it
+		 * usually contains (x86) bios code) */
+		dev->resource[PCI_ROM_RESOURCE].flags = 0;
+		dev->resource[PCI_ROM_RESOURCE].start = 0;
+		dev->resource[PCI_ROM_RESOURCE].end = 0;
+				
+		if(dev->irq == 255) {
+
+#ifdef DINO_FIX_UNASSIGNED_INTERRUPTS
+
+			/* This code tries to assign an unassigned
+			 * interrupt.  Leave it disabled unless you
+			 * *really* know what you're doing since the
+			 * pin<->interrupt line mapping varies by bus
+			 * and machine */
+
+			u32 irq_pin;
+			
+			dino_cfg_read(dev->bus, dev->devfn, PCI_INTERRUPT_PIN, 1, &irq_pin);
+			dev->irq = (irq_pin + PCI_SLOT(dev->devfn) - 1) % 4 ;
+			dino_cfg_write(dev->bus, dev->devfn, PCI_INTERRUPT_LINE, 1, dev->irq);
+			dev->irq += dino_dev->dino_region->data.irqbase
+			printk(KERN_WARNING "Device %s has undefined IRQ, setting to %d\n", dev->slot_name, irq_pin);
+#else
+			dev->irq = 65535;
+			printk(KERN_WARNING "Device %s has unassigned IRQ\n", dev->slot_name);	
+#endif
+		} else {
 
-		/* Adjust INT_LINE for that busses region */
-		dev->irq = dino_dev->dino_region->data.irqbase + dev->irq;
+			/* Adjust INT_LINE for that busses region */
+			dev->irq += dino_dev->dino_region->data.irqbase;
+		}
 	}
 }
 
@@ -826,6 +899,7 @@
 	const int name_len = 32;
 	char *name;
 	int is_cujo = 0;
+	struct pci_bus *bus;
 
 	name = kmalloc(name_len, GFP_KERNEL);
 	if(name)
@@ -911,9 +985,20 @@
 	** It's not used to avoid chicken/egg problems
 	** with configuration accessor functions.
 	*/
-	dino_dev->hba.hba_bus = 
-		pci_scan_bus_parented(&dev->dev, dino_dev->hba.hba_num,
-				      &dino_cfg_ops, NULL);
+	bus = pci_scan_bus_parented(&dev->dev, dino_current_bus,
+				    &dino_cfg_ops, NULL);
+	if(bus) {
+		/* This code *depends* on scanning being single threaded
+		 * if it isn't, this global bus number count will fail
+		 */
+		dino_current_bus = bus->subordinate + 1;
+		pci_bus_assign_resources(bus);
+	} else {
+		printk(KERN_ERR "ERROR: failed to scan PCI bus on %s (probably duplicate bus number %d)\n", dev->dev.bus_id, dino_current_bus);
+		/* increment the bus number in case of duplicates */
+		dino_current_bus++;
+	}
+	dino_dev->hba.hba_bus = bus;
 	return 0;
 }
 

Re: [parisc-linux] proposed changes to dino.c

[Grant Grundler]

On Wed, Dec 10, 2003 at 04:54:24PM -0500, James Bottomley wrote:
> +/* The alignment contraints for PCI bridges under dino */
> +#define DINO_BRIDGE_ALIGN 0x100000

hrmm...I was expecting this to be defined in include/linux/pci.h
but it's not. "pbus_size_mem()" seems to take care of this
in the drivers/pci generic code.

> +		/* null out the ROM resource if there is one (we don't
> +		 * care about an expansion rom on parisc, since it
> +		 * usually contains (x86) bios code) */
> +		dev->resource[PCI_ROM_RESOURCE].flags = 0;
> +		dev->resource[PCI_ROM_RESOURCE].start = 0;
> +		dev->resource[PCI_ROM_RESOURCE].end = 0;

I just realized some drivers may need to access data (not code)
in the ROM. I'm not aware of any but I don't want to under
estimate how creative people can get. Just something to beware
when bringing up new cards.

The rest looked fine to me. I don't maintain any dino-based
machines any more and thus can't test it.

thanks,
grant

Re: [parisc-linux] proposed changes to dino.c

[Thomas Bogendoerfer]

On Fri, Dec 12, 2003 at 12:48:38AM -0700, Grant Grundler wrote:
> On Wed, Dec 10, 2003 at 04:54:24PM -0500, James Bottomley wrote:
> > +/* The alignment contraints for PCI bridges under dino */
> > +#define DINO_BRIDGE_ALIGN 0x100000
> 
> hrmm...I was expecting this to be defined in include/linux/pci.h
> but it's not. "pbus_size_mem()" seems to take care of this
> in the drivers/pci generic code.
> 
> > +		/* null out the ROM resource if there is one (we don't
> > +		 * care about an expansion rom on parisc, since it
> > +		 * usually contains (x86) bios code) */
> > +		dev->resource[PCI_ROM_RESOURCE].flags = 0;
> > +		dev->resource[PCI_ROM_RESOURCE].start = 0;
> > +		dev->resource[PCI_ROM_RESOURCE].end = 0;
> 
> I just realized some drivers may need to access data (not code)
> in the ROM. I'm not aware of any but I don't want to under
> estimate how creative people can get. Just something to beware
> when bringing up new cards.

doesn't this also break PCI Vis-EG and FX cards setup ? On these
cards the ROM contains PARISC firmware, which we need.

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessary a
good idea.                                 [ Alexander Viro on linux-kernel ]

Re: [parisc-linux] proposed changes to dino.c

[James Bottomley]

On Fri, 2003-12-12 at 02:48, Grant Grundler wrote:
> I just realized some drivers may need to access data (not code)
> in the ROM. I'm not aware of any but I don't want to under
> estimate how creative people can get. Just something to beware
> when bringing up new cards.

Hmm, well, we can back it out if people start hitting problems.

The reason for it is the Quad tulip card:  The bridge chip only requests
a 1MB PCI window, but each chip has two memory BARs: the iomem region
(128bytes, with page alignment) and the ROM (256Kb).  The bridge
allocation routines seem to work the BARs in reverse, so by the time the
ROM BARs are allocated, the memory window is full and none of the
devices can function.

I suppose I could look at a two phase approach: count all the resources
and sizes and then resize the bridge window appropriately...we actually
already need to do this for the GSC window on my C360...firmware
allocates too much space in the GSC/8 window and I run out of iomem
space for devices on the GSC/10.

James

Re: [parisc-linux] proposed changes to dino.c

[James Bottomley]

On Fri, 2003-12-12 at 06:30, Thomas Bogendoerfer wrote:
> doesn't this also break PCI Vis-EG and FX cards setup ? On these
> cards the ROM contains PARISC firmware, which we need.

Only if we physically need acess to the ROMs from the linux driver after
the firmware is used...this is rarely true, even on x86.  I'm afraid I
only have GSC Vis-EG cards, so I can't test this though.

James

Re: [parisc-linux] proposed changes to dino.c

[Thomas Bogendoerfer]

On Fri, Dec 12, 2003 at 10:00:29AM -0500, James Bottomley wrote:
> On Fri, 2003-12-12 at 06:30, Thomas Bogendoerfer wrote:
> > doesn't this also break PCI Vis-EG and FX cards setup ? On these
> > cards the ROM contains PARISC firmware, which we need.
> 
> Only if we physically need acess to the ROMs from the linux driver after
> the firmware is used...this is rarely true, even on x86.  I'm afraid I
> only have GSC Vis-EG cards, so I can't test this though.

sticon/stifb map the PCI ROM to copy out routines and fonts during
startup of the drivers.

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessary a
good idea.                                 [ Alexander Viro on linux-kernel ]

Re: [parisc-linux] proposed changes to dino.c

[Alan Cox]

On Gwe, 2003-12-12 at 14:58, James Bottomley wrote:
> The reason for it is the Quad tulip card:  The bridge chip only requests
> a 1MB PCI window, but each chip has two memory BARs: the iomem region
> (128bytes, with page alignment) and the ROM (256Kb).  The bridge
> allocation routines seem to work the BARs in reverse, so by the time the
> ROM BARs are allocated, the memory window is full and none of the
> devices can function.

You don't need the ROM mapped expect for reading stuff like MAC, and you
can do that by mapping each port over the same address range.

Re: [parisc-linux] proposed changes to dino.c

[Grant Grundler]

On Fri, Dec 12, 2003 at 07:28:18PM +0000, Alan Cox wrote:
> You don't need the ROM mapped expect for reading stuff like MAC, and you
> can do that by mapping each port over the same address range.

interesting trick.
That should work nicely for the 4-port tulip especially.

Can we allocate/map only the first tulip's ROM and point the remaining
tulip's ROM_RESOURCE at the first one?

I'm worried multiple PCI devices could attempt to respond to
the MMIO transaction and fight over the bus.

PCI Hotplug makes my suggestion at bad idea to generalize
for other arches.

thanks,
grant

Re: [parisc-linux] proposed changes to dino.c

[Grant Grundler]

On Fri, Dec 12, 2003 at 07:28:18PM +0000, Alan Cox wrote:
> You don't need the ROM mapped expect for reading stuff like MAC, and you
> can do that by mapping each port over the same address range.

doh...MAC is unique to each card...delete my previous mail.

But then the basic problem James is having is not with the virtual
address the ROM gets mapped to, but rather the PCI bus MMIO address
assigned to the ROM BAR.

grant

Re: [parisc-linux] proposed changes to dino.c

[Alan Cox]

On Sad, 2003-12-13 at 03:51, Grant Grundler wrote:
> On Fri, Dec 12, 2003 at 07:28:18PM +0000, Alan Cox wrote:
> > You don't need the ROM mapped expect for reading stuff like MAC, and you
> > can do that by mapping each port over the same address range.
> 
> interesting trick.
> That should work nicely for the 4-port tulip especially.
> 
> Can we allocate/map only the first tulip's ROM and point the remaining
> tulip's ROM_RESOURCE at the first one?
> 
> I'm worried multiple PCI devices could attempt to respond to
> the MMIO transaction and fight over the bus.

If the other ROMs are disabled then I don't see that happening. Its an
old trick btw - I claim no originality here. Don Becker had some driver
code that did this, and the graphics people in Linux and Windows pull
the trick of unmapping the frame buffer to use as the ROM map
temporarily

Re: [parisc-linux] proposed changes to dino.c

[James Bottomley]

On Sat, 2003-12-13 at 10:33, Alan Cox wrote:
> If the other ROMs are disabled then I don't see that happening. Its an
> old trick btw - I claim no originality here. Don Becker had some driver
> code that did this, and the graphics people in Linux and Windows pull
> the trick of unmapping the frame buffer to use as the ROM map
> temporarily

Hmm, yes...I completely blow away the ROM bar in the patch...what I
should do is just to remove it's IORESOURCE_MEM flag so it isn't
ordinarily allocated (but it's requested location and size are still
available).  Then any driver that wanted access could get it...we still
have 1020kB available for mapping up to three roms in the bridge window.

The alternative is just to resize the bridge window after we've counted
the resources...I'm less enthusiastic about this beacuse the dino PCI
window has a maximum size of 8MB for everything on the bus...plug a few
of these four ports into a dino PCI bus and we'll still run out of
memory.

James

Re: [parisc-linux] proposed changes to dino.c

[Matthew Wilcox]

On Sat, Dec 13, 2003 at 10:45:54AM -0500, James Bottomley wrote:
> Hmm, yes...I completely blow away the ROM bar in the patch...what I
> should do is just to remove it's IORESOURCE_MEM flag so it isn't
> ordinarily allocated (but it's requested location and size are still
> available).  Then any driver that wanted access could get it...we still
> have 1020kB available for mapping up to three roms in the bridge window.

I rather think the generic PCI code should be fixed to not enable the
ROM resource by default.  Many PCI devices do not cope well with this
*anyway*, we definitely shouldn't be doing it in anything except the
driver.

> The alternative is just to resize the bridge window after we've counted
> the resources...I'm less enthusiastic about this beacuse the dino PCI
> window has a maximum size of 8MB for everything on the bus...plug a few
> of these four ports into a dino PCI bus and we'll still run out of
> memory.

Well, we could allocate more than 8MB for a Dino's PCI window.  As I
recall (should check the spec ...) we can set any of the 30 bits in
the status register to forward any of those 8MB chunks to the PCI bus.
I think we could also get away with allocating less than 8MB to an
individual PCI bus by programming the CCIO registers to not forward all
that space to the parent GSC bus.  But none of this has been necessay so
far, and I hope we don't manage to concoct a situation where it might be.

-- 
"Next the statesmen will invent cheap lies, putting the blame upon 
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince 
himself that the war is just, and will thank God for the better sleep 
he enjoys after this process of grotesque self-deception." -- Mark Twain

Re: [parisc-linux] proposed changes to dino.c

[Grant Grundler]

On Sat, Dec 13, 2003 at 08:33:43PM +0000, Matthew Wilcox wrote:
> Well, we could allocate more than 8MB for a Dino's PCI window.  As I
> recall (should check the spec ...) we can set any of the 30 bits in
> the status register to forward any of those 8MB chunks to the PCI bus.

that's correct. Only the top most and bottom most bits are fixed
at zero (both 8MB ranges are not routeable).

> I think we could also get away with allocating less than 8MB to an
> individual PCI bus by programming the CCIO registers to not forward all
> that space to the parent GSC bus.  But none of this has been necessay so
> far, and I hope we don't manage to concoct a situation where it might be.

"Doctor, when I stand like this, my neck hurts."
"Don't do that."

:^)

grant