[PATCH] yenta resource allocation fix

[PATCH] yenta resource allocation fix

[Andreas Bombe]

Now that Linus is back, and since he wrote most of yenta.c (according to
copyright notice), I'll just reiterate this (sent to the list and Alan
Cox).  I removed a seemingly bogus bit mask and replaced it with a
consistent IO range mask.

I have no idea why the 0xfff was in place.  Or, on second thought, this
might be to allocate memory space behind official end as slack?  This
would defy the end > start check then, anyway.  Linus?


---- start repeat ----

This fixes the problem of the yenta_socket driver not allocating IO port
windows if they weren't already.  This caused no ports to be available
to a inserted CardBus card which in turn made the tulip driver complain
since it can't run without IO ports.

I'm no expert on the PCI/CardBus bridge stuff, but let's see:  The OR
operation on the range end register with 0xfff seems pretty bogus to me.
Also the "if (start && ..." was always true since start included the 32
bit IO flag.  What my patch does is to mask out the IO flags on both
start and end if the resource is indeed an IO resource, which seems
correct to me.

Now everything works for me.  If I had known the fix was so easy I would
never have bothered with pcmcia-cs.  Well, there is a problem that it
only picks up every second card insertion, so I have to
insert/remove/insert for my card to be recognized, but then it works.


diff -ruN linux-2.4.orig/drivers/pcmcia/yenta.c
linux-2.4/drivers/pcmcia/yenta.c   
--- linux-2.4.orig/drivers/pcmcia/yenta.c       Fri Jul 27 02:21:28 2001
+++ linux-2.4/drivers/pcmcia/yenta.c    Sun Aug 26 23:26:17 2001
@@ -701,8 +701,14 @@
        struct resource *root, *res;
        u32 start, end;
        u32 align, size, min, max;
+       u32 mask;
        unsigned offset;

+       if (type & IORESOURCE_IO)
+               mask = PCI_CB_IO_RANGE_MASK;
+       else
+               mask = ~0U;
+
        offset = 0x1c + 8*nr;
        bus = socket->dev->subordinate;
        res = socket->dev->resource + PCI_BRIDGE_RESOURCES + nr;
@@ -715,8 +721,8 @@
        if (!root)
                return;

-       start = config_readl(socket, offset);
-       end = config_readl(socket, offset+4) | 0xfff;
+       start = config_readl(socket, offset) & mask;
+       end = config_readl(socket, offset+4) & mask;
        if (start && end > start) {
                res->start = start;
                res->end = end;



---- end repeat ----


Some additional data for the missed card insertions:  I put printk()s in
yenta_interrupt() and yenta_bh() printing events (yenta_bh() just out of
curiosity, no events get lost/delayed between interrupt and bh, all fine
there).

This showed that the first insertion after module load doesn't even
arrive as an interrupt.  After that, with only pcmcia_core and
yenta_socket loaded, every card removal and insertion show up as event
0x80 (to be more precise cb_event 0x6, csc 0x0).

With ds loaded first insertion does not arrive also.  The second
insertion gets picked up and card is initialized.  After removal the
next insertion is again missed (no interrupt)!  cb_event is 0x6 for
received insertions, 0x7 for removal after successful insertion and 0x6
for removal after missed insertion.



For reference, hardware is an IBM Thinkpad (i1200 or more specifically
1161-267) with 550MHz Coppermine-Celeron and the following relevant
cardbus bridge:

00:03.0 CardBus bridge: O2 Micro, Inc. OZ6812 Cardbus Controller (rev 05)
        Subsystem: IBM: Unknown device 01a3
        Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping+ SERR- FastB2B-
        Status: Cap+ 66Mhz- UDF- FastB2B- ParErr- DEVSEL=slow >TAbort- <TAbort- <MAbort- >SERR- <PERR-
        Latency: 168
        Interrupt: pin A routed to IRQ 10
        Region 0: Memory at 10000000 (32-bit, non-prefetchable) [size=4K]
        Bus: primary=00, secondary=01, subordinate=04, sec-latency=176
        Memory window 0: 10400000-107ff000 (prefetchable)
        Memory window 1: 10800000-10bff000
        I/O window 0: 00001000-000010ff
        I/O window 1: 00001400-000014ff
        BridgeCtl: Parity- SERR- ISA- VGA- MAbort- >Reset- 16bInt- PostWrite+
        16-bit legacy interface ports at 0001

Some log messages for this:

PCI: Found IRQ 11 for device 00:03.0
PCI: Sharing IRQ 11 with 00:00.1
PCI: Sharing IRQ 11 with 00:00.2
IRQ routing conflict for 00:03.0, have irq 10, want irq 11
Yenta IRQ list 00b8, PCI irq10
Socket status: 30000827

-- 
Andreas E. Bombe <andreas.bombe@munich.netsurf.de>    DSA key 0x04880A44

Re: [PATCH] yenta resource allocation fix

[Linus Torvalds]

On Wed, 29 Aug 2001, Andreas Bombe wrote:
>
> I'm no expert on the PCI/CardBus bridge stuff, but let's see:  The OR
> operation on the range end register with 0xfff seems pretty bogus to me.

No. The windows are supposed to be in "pages", so what the code does is to
say
 - we read the start page and the end page
 - the end is up-to-and-including the final page, so if both start and end
   were "page 1" (0x1000), that actually means that the area is from
   0x1000 to 0x1fff

> Also the "if (start && ..." was always true since start included the 32
> bit IO flag.  What my patch does is to mask out the IO flags on both
> start and end if the resource is indeed an IO resource, which seems
> correct to me.

Now, the masking off of the low bits is very possibly the right thing. I
don't have the documentation on what the low bits contain in front of me,
I'll try to find it. But I suspect that _that_ part of the patch may be
right.

Does it work for you if you do a minimal one-liner patch

-	start = config_readl(socket, offset);
+	start = config_readl(socket, offset) & ~0xfff;
 	end = config_readl(socket, offset+4) | 0xfff;
 	if (start && end > start) {
 		res->start = start;
 		res->end = end;

instead?

> Some additional data for the missed card insertions:  I put printk()s in
> yenta_interrupt() and yenta_bh() printing events (yenta_bh() just out of
> curiosity, no events get lost/delayed between interrupt and bh, all fine
> there).
>
> This showed that the first insertion after module load doesn't even
> arrive as an interrupt.  After that, with only pcmcia_core and
> yenta_socket loaded, every card removal and insertion show up as event
> 0x80 (to be more precise cb_event 0x6, csc 0x0).

This sounds like a separate problem, and might be due to not ACK'ing the
changes (ie writing the status register with all ones) always. But we _do_
do the

	cb_writel(socket, CB_SOCKET_EVENT, -1);
	cb_writel(socket, CB_SOCKET_MASK, CB_CDMASK);

at init time - I wonder if they should be done in the reverse order..

		Linus

Re: [PATCH] yenta resource allocation fix

[Linus Torvalds]

On Wed, 29 Aug 2001, Andreas Bombe wrote:
>
> I have no idea why the 0xfff was in place.  Or, on second thought, this
> might be to allocate memory space behind official end as slack?  This
> would defy the end > start check then, anyway.  Linus?

I've looked more at the issue.

0xfff is definitely right for memory windows and is generally right for
PCI-PCI bridges too - they cannot have IO or memory windows that are
anything but 4kB aligned.

But it turns out that the Yenta specification actually expanded on the
PCI-PCI bridge window specs for IO space - a Yenta bridge is supposed to
be able to handle IO windows at 4-byte granularity, not the 4kB a regular
PCI bridge does.

Does this alternate patch work for you?

		Linus

------
diff -u --recursive --new-file pre2/linux/drivers/pcmcia/yenta.c linux/drivers/pcmcia/yenta.c
--- pre2/linux/drivers/pcmcia/yenta.c	Wed Aug 29 06:20:01 2001
+++ linux/drivers/pcmcia/yenta.c	Wed Aug 29 06:13:40 2001
@@ -702,6 +702,12 @@
 	u32 start, end;
 	u32 align, size, min, max;
 	unsigned offset;
+	unsigned mask;
+
+	/* The granularity of the memory limit is 4kB, on IO it's 4 bytes */
+	mask = ~0xfff;
+	if (type & IORESOURCE_IO)
+		mask = ~3;

 	offset = 0x1c + 8*nr;
 	bus = socket->dev->subordinate;
@@ -715,8 +721,8 @@
 	if (!root)
 		return;

-	start = config_readl(socket, offset);
-	end = config_readl(socket, offset+4) | 0xfff;
+	start = config_readl(socket, offset) & mask;
+	end = config_readl(socket, offset+4) | ~mask;
 	if (start && end > start) {
 		res->start = start;
 		res->end = end;
diff -u --recursive --new-file pre2/linux/mm/vmscan.c linux/mm/vmscan.c
--- pre2/linux/mm/vmscan.c	Wed Aug 15 02:37:07 2001
+++ linux/mm/vmscan.c	Wed Aug 29 06:02:46 2001
@@ -818,10 +818,12 @@
 #define GENERAL_SHORTAGE 4
 static int do_try_to_free_pages(unsigned int gfp_mask, int user)
 {
-	/* Always walk at least the active queue when called */
-	int shortage = INACTIVE_SHORTAGE;
+	int shortage = 0;
 	int maxtry;

+	/* Always walk at least the active queue when called */
+	refill_inactive_scan(DEF_PRIORITY);
+
 	maxtry = 1 << DEF_PRIORITY;
 	do {
 		/*
@@ -872,7 +874,8 @@
 			break;
 	} while (shortage);

-	return !shortage;
+	/* Return success if we're not "totally short" */
+	return shortage != FREE_SHORTAGE | INACTIVE_SHORTAGE | GENERAL_SHORTAGE;
 }

 DECLARE_WAIT_QUEUE_HEAD(kswapd_wait);

Re: [PATCH] yenta resource allocation fix

[Andreas Bombe]

On Wed, Aug 29, 2001 at 06:48:26AM -0700, Linus Torvalds wrote:
> 
> On Wed, 29 Aug 2001, Andreas Bombe wrote:
> >
> > I have no idea why the 0xfff was in place.  Or, on second thought, this
> > might be to allocate memory space behind official end as slack?  This
> > would defy the end > start check then, anyway.  Linus?
> 
> I've looked more at the issue.
> 
> 0xfff is definitely right for memory windows and is generally right for
> PCI-PCI bridges too - they cannot have IO or memory windows that are
> anything but 4kB aligned.
> 
> But it turns out that the Yenta specification actually expanded on the
> PCI-PCI bridge window specs for IO space - a Yenta bridge is supposed to
> be able to handle IO windows at 4-byte granularity, not the 4kB a regular
> PCI bridge does.

Even then the old code would have been incorrect.  Further down the
yenta_allocate_res() function, allocate_resource() is called with
align = 1024 and size = 256 for IO port windows.  It also promptly got
0x1000-0x10ff and 0x1400-0x14ff allocated.

> Does this alternate patch work for you?

Ignoring the unrelated vmscan.c patch, yes, it works as it should,
thanks (I never hit the memory window case anyway, since that is
allocated fine before yenta.c gets to it).

About the other thing with missed card insertion events there is nothing
new.  I tried a few things but nothing helped.  There is the suspicious
thing that CB_SOCKET_STATE has CB_CBCARD always set, whether there is a
card or not, but I don't know enough of the code to see where it
matters.

-- 
Andreas E. Bombe <andreas.bombe@munich.netsurf.de>    DSA key 0x04880A44

[PATCH] yenta resource allocation fix

[Andreas Bombe]

This fixes the problem of the yenta_socket driver not allocating IO port
windows if they weren't already.  This caused no ports to be available
to a inserted CardBus card which in turn made the tulip driver complain
since it can't run without IO ports.

I'm no expert on the PCI/CardBus bridge stuff, but let's see:  The OR
operation on the range end register with 0xfff seems pretty bogus to me.
Also the "if (start && ..." was always true since start included the 32
bit IO flag.  What my patch does is to mask out the IO flags on both
start and end if the resource is indeed an IO resource, which seems
correct to me.

Now everything works for me.  If I had known the fix was so easy I would
never have bothered with pcmcia-cs.  Well, there is a problem that it
only picks up every second card insertion, so I have to
insert/remove/insert for my card to be recognized, but then it works.


diff -ruN linux-2.4.orig/drivers/pcmcia/yenta.c linux-2.4/drivers/pcmcia/yenta.c
--- linux-2.4.orig/drivers/pcmcia/yenta.c	Fri Jul 27 02:21:28 2001
+++ linux-2.4/drivers/pcmcia/yenta.c	Sun Aug 26 23:26:17 2001
@@ -701,8 +701,14 @@
 	struct resource *root, *res;
 	u32 start, end;
 	u32 align, size, min, max;
+	u32 mask;
 	unsigned offset;
 
+	if (type & IORESOURCE_IO)
+		mask = PCI_CB_IO_RANGE_MASK;
+	else
+		mask = ~0U;
+
 	offset = 0x1c + 8*nr;
 	bus = socket->dev->subordinate;
 	res = socket->dev->resource + PCI_BRIDGE_RESOURCES + nr;
@@ -715,8 +721,8 @@
 	if (!root)
 		return;
 
-	start = config_readl(socket, offset);
-	end = config_readl(socket, offset+4) | 0xfff;
+	start = config_readl(socket, offset) & mask;
+	end = config_readl(socket, offset+4) & mask;
 	if (start && end > start) {
 		res->start = start;
 		res->end = end;

-- 
Andreas E. Bombe <andreas.bombe@munich.netsurf.de>    DSA key 0x04880A44