Skip to contentSkip to conversation

yenta resource allocation fix

PATCH In review Andreas Bombe 2 participants 4 messages Aug 28, 2001 – Aug 29, 2001

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

PATCH 1/1 yenta resource allocation fix

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

drivers/pcmcia/yenta.c Sun Aug 26 23:26:17 2001 renamed from drivers/pcmcia/yenta.c Fri Jul 27 02:21:28 2001 +8 -2
@@ -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

Signature · 1 line
Andreas E. Bombe <[email protected]>    DSA key 0x04880A44
#1 lore raw
Quoted 3 lines · Andreas Bombe

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

Quoted 4 lines

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?

Quoted 9 lines

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

#2 lore raw
In reply to Andreas Bombe
Quoted 4 lines · Andreas Bombe

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

------

drivers/pcmcia/yenta.c renamed from linux/drivers/pcmcia/yenta.c +8 -2
diff -u --recursive --new-file pre2/linux/drivers/pcmcia/yenta.c linux/drivers/pcmcia/yenta.c
@@ -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;
mm/vmscan.c renamed from linux/mm/vmscan.c +6 -3
diff -u --recursive --new-file pre2/linux/mm/vmscan.c linux/mm/vmscan.c
@@ -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);
#3 lore raw
In reply to Andreas Bombe
Quoted 9 lines · Linus Torvalds
Quoted 4 lines · Andreas Bombe

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.

Quoted 8 lines

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.

Quoted 1 line

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.

Signature · 1 line
Andreas E. Bombe <[email protected]>    DSA key 0x04880A44