Skip to contentSkip to conversation

odd RTL8139 quirk.

PATCH In review Dave Jones 5 participants 30 messages Apr 29, 2008 – Jul 16, 2008

Subsystems (2)
Networking Network drivers
lore
Share & tools
lore

Dave Jones

author
#0 lore raw

I've just been playing with a model 2 OQO, which has an RTL8139. It gets detected just fine, though it doesn't actually work..

eth0: RealTek RTL8139 at 0xf8830000, 00:00:00:00:00:00, IRQ 18
eth0: Identified 8139 chip type 'RTL-8139'

The null MAC address being one clue. Another oddity is that ethtool reports that there's no link detected, even though there is. (Enough for it to PXE boot a kernel from at least :)

Futzing with the debug= modparam didn't yield anything extra at all.

Any clues?

Dave

Signature · 1 line
http://www.codemonkey.org.uk
#1 lore raw
Quoted 6 lines · Dave Jones

I've just been playing with a model 2 OQO, which has an RTL8139. It gets detected just fine, though it doesn't actually work..

eth0: RealTek RTL8139 at 0xf8830000, 00:00:00:00:00:00, IRQ 18
eth0: Identified 8139 chip type 'RTL-8139'

Quoted 7 lines

The null MAC address being one clue. Another oddity is that ethtool reports that there's no link detected, even though there is. (Enough for it to PXE boot a kernel from at least :)

Futzing with the debug= modparam didn't yield anything extra at all.

Any clues?

Sounds like a broken EEPROM. Does supplying a MAC via ifconfig prior to 'ifconfig ... up' help?

Jeff

author
#2 lore raw
Quoted 11 lines · Jeff Garzik
Quoted 10 lines · Dave Jones

I've just been playing with a model 2 OQO, which has an RTL8139. It gets detected just fine, though it doesn't actually work..

eth0: RealTek RTL8139 at 0xf8830000, 00:00:00:00:00:00, IRQ 18
eth0: Identified 8139 chip type 'RTL-8139'

The null MAC address being one clue. Another oddity is that ethtool reports that there's no link detected, even though there is. (Enough for it to PXE boot a kernel from at least :)

Quoted 5 lines
Quoted 3 lines

Futzing with the debug= modparam didn't yield anything extra at all.

Any clues?

Sounds like a broken EEPROM.

Not that it counts for much, but the NIC works fine in the vista install that it came with.

Quoted 1 line

Does supplying a MAC via ifconfig prior to 'ifconfig ... up' help?

That fails spectacularly with 'eth0: PCI Bus error 0290' after I do the 'up' Someone suggested that the PHY isn't being detected.
I don't get any of the MII messages from rtl8139_init_one() Which is odd (to me at least) as ethtool reports MII as a supported port, (and even has it as the 'current' port).

Dave

Signature · 1 line
http://www.codemonkey.org.uk
author
#3 lore raw
In reply to Jeff Garzik
Quoted 11 lines · Jeff Garzik
Quoted 10 lines · Dave Jones

I've just been playing with a model 2 OQO, which has an RTL8139. It gets detected just fine, though it doesn't actually work..

eth0: RealTek RTL8139 at 0xf8830000, 00:00:00:00:00:00, IRQ 18
eth0: Identified 8139 chip type 'RTL-8139'

The null MAC address being one clue. Another oddity is that ethtool reports that there's no link detected, even though there is. (Enough for it to PXE boot a kernel from at least :)

Quoted 6 lines
Quoted 3 lines

Futzing with the debug= modparam didn't yield anything extra at all.

Any clues?

Sounds like a broken EEPROM. Does supplying a MAC via ifconfig prior to 'ifconfig ... up' help?

Ah. This sounds enlightening: http://www.oqotalk.com/index.php/topic,1511.0.html
Seems a shame to have to choose PIO vs MMIO for a distro kernel though. Would there be any objection to turning that into a modparam ?
(If we wanted to get really fancy, we could even quirk around it automatically
when we detect broken hardware).

Thoughts?

Dave

Signature · 1 line
http://www.codemonkey.org.uk
#4 lore raw
Quoted 16 lines · Dave Jones
Quoted 15 lines · Jeff Garzik
Quoted 13 lines · Dave Jones

I've just been playing with a model 2 OQO, which has an RTL8139. It gets detected just fine, though it doesn't actually work..

eth0: RealTek RTL8139 at 0xf8830000, 00:00:00:00:00:00, IRQ 18
eth0: Identified 8139 chip type 'RTL-8139'

The null MAC address being one clue. Another oddity is that ethtool reports that there's no link detected, even though there is. (Enough for it to PXE boot a kernel from at least :)

Futzing with the debug= modparam didn't yield anything extra at all.

Any clues?

Quoted 8 lines
Quoted 2 lines

Sounds like a broken EEPROM. Does supplying a MAC via ifconfig prior to 'ifconfig ... up' help?

Ah. This sounds enlightening: http://www.oqotalk.com/index.php/topic,1511.0.html
Seems a shame to have to choose PIO vs MMIO for a distro kernel though. Would there be any objection to turning that into a modparam ?
(If we wanted to get really fancy, we could even quirk around it automatically
when we detect broken hardware).

Something like this? :)

http://www.linux.sgi.com/archives/netdev/2004-11/msg00226.html

It did not go upstream but it needed some init-time bug fixing, IIRC. Maybe akpm remembers more why my patch sucked... :)

Jeff

author
#5 lore raw
Quoted 26 lines · Jeff Garzik
Quoted 16 lines · Dave Jones
Quoted 15 lines · Jeff Garzik
Quoted 13 lines · Dave Jones

I've just been playing with a model 2 OQO, which has an RTL8139. It gets detected just fine, though it doesn't actually work..

eth0: RealTek RTL8139 at 0xf8830000, 00:00:00:00:00:00, IRQ 18
eth0: Identified 8139 chip type 'RTL-8139'

The null MAC address being one clue. Another oddity is that ethtool reports that there's no link detected, even though there is. (Enough for it to PXE boot a kernel from at least :)

Futzing with the debug= modparam didn't yield anything extra at all.

Any clues?

Quoted 8 lines
Quoted 2 lines

Sounds like a broken EEPROM. Does supplying a MAC via ifconfig prior to 'ifconfig ... up' help?

Ah. This sounds enlightening: http://www.oqotalk.com/index.php/topic,1511.0.html
Seems a shame to have to choose PIO vs MMIO for a distro kernel though. Would there be any objection to turning that into a modparam ?
(If we wanted to get really fancy, we could even quirk around it automatically
when we detect broken hardware).

Quoted 5 lines

Something like this? :)

http://www.linux.sgi.com/archives/netdev/2004-11/msg00226.html

From a quick eyeball, that would seem that it still requires a choice

to be made whether or not to enable CONFIG_8139TOO_PIO What I had in mind was to make that whole USE_IO_OPS ifdef (and related bits) if (modparm==.. instead of #ifdefs.

Dave

Signature · 1 line
http://www.codemonkey.org.uk
#6 lore raw
Quoted 4 lines · Dave Jones
Quoted 1 line

From a quick eyeball, that would seem that it still requires a choice

to be made whether or not to enable CONFIG_8139TOO_PIO What I had in mind was to make that whole USE_IO_OPS ifdef (and related bits) if (modparm==.. instead of #ifdefs.

The patch is the requisite groundwork to add such a module param... iomap support is required to enable runtime rather than compile-time MMIO/PIO switching.

e100 does this with the 'use_io' module param.

Jeff

author
#7 lore raw
Quoted 7 lines · Jeff Garzik
Quoted 4 lines · Dave Jones
Quoted 1 line

From a quick eyeball, that would seem that it still requires a choice

to be made whether or not to enable CONFIG_8139TOO_PIO What I had in mind was to make that whole USE_IO_OPS ifdef (and related bits) if (modparm==.. instead of #ifdefs.

Quoted 5 lines

The patch is the requisite groundwork to add such a module param... iomap support is required to enable runtime rather than compile-time MMIO/PIO switching.

e100 does this with the 'use_io' module param.

Ah, in which case, I endorse this proposal heartily :)

Dave

Signature · 1 line
http://www.codemonkey.org.uk
#8 lore raw
Quoted 8 lines · Dave Jones
Quoted 7 lines · Jeff Garzik
Quoted 4 lines · Dave Jones
Quoted 1 line

From a quick eyeball, that would seem that it still requires a choice

to be made whether or not to enable CONFIG_8139TOO_PIO What I had in mind was to make that whole USE_IO_OPS ifdef (and related bits) if (modparm==.. instead of #ifdefs.

Quoted 7 lines
Quoted 5 lines

The patch is the requisite groundwork to add such a module param... iomap support is required to enable runtime rather than compile-time MMIO/PIO switching.

e100 does this with the 'use_io' module param.

Ah, in which case, I endorse this proposal heartily :)

Is that endorsement perchance accompanied by the time to resurrect the patch? :)

Also, specifically with your hardware, one would hope we could figure out some automatic way for the driver to detect the hardware and configure it accordingly.

Jeff

#9 lore raw
In reply to Jeff Garzik
Quoted 26 lines · On Tue, 29 Apr 2008 18:04:06 -0400 Jeff Garzik <[email protected]> wrote:
Quoted 16 lines · Dave Jones
Quoted 15 lines · Jeff Garzik
Quoted 13 lines · Dave Jones

I've just been playing with a model 2 OQO, which has an RTL8139. It gets detected just fine, though it doesn't actually work..

eth0: RealTek RTL8139 at 0xf8830000, 00:00:00:00:00:00, IRQ 18
eth0: Identified 8139 chip type 'RTL-8139'

The null MAC address being one clue. Another oddity is that ethtool reports that there's no link detected, even though there is. (Enough for it to PXE boot a kernel from at least :)

Futzing with the debug= modparam didn't yield anything extra at all.

Any clues?

Quoted 8 lines
Quoted 2 lines

Sounds like a broken EEPROM. Does supplying a MAC via ifconfig prior to 'ifconfig ... up' help?

Ah. This sounds enlightening: http://www.oqotalk.com/index.php/topic,1511.0.html
Seems a shame to have to choose PIO vs MMIO for a distro kernel though. Would there be any objection to turning that into a modparam ?
(If we wanted to get really fancy, we could even quirk around it automatically
when we detect broken hardware).

Quoted 6 lines

Something like this? :)

http://www.linux.sgi.com/archives/netdev/2004-11/msg00226.html

It did not go upstream but it needed some init-time bug fixing, IIRC. Maybe akpm remembers more why my patch sucked... :)

Apart from its From: address you mean? ;)

I can find no record, sorry.

#10 lore raw
Quoted 30 lines · Andrew Morton
Quoted 27 lines · On Tue, 29 Apr 2008 18:04:06 -0400 Jeff Garzik <[email protected]> wrote:
Quoted 16 lines · Dave Jones
Quoted 15 lines · Jeff Garzik
Quoted 13 lines · Dave Jones

I've just been playing with a model 2 OQO, which has an RTL8139. It gets detected just fine, though it doesn't actually work..

eth0: RealTek RTL8139 at 0xf8830000, 00:00:00:00:00:00, IRQ 18
eth0: Identified 8139 chip type 'RTL-8139'

The null MAC address being one clue. Another oddity is that ethtool reports that there's no link detected, even though there is. (Enough for it to PXE boot a kernel from at least :)

Futzing with the debug= modparam didn't yield anything extra at all.

Any clues?

Quoted 8 lines
Quoted 2 lines

Sounds like a broken EEPROM. Does supplying a MAC via ifconfig prior to 'ifconfig ... up' help?

Ah. This sounds enlightening: http://www.oqotalk.com/index.php/topic,1511.0.html
Seems a shame to have to choose PIO vs MMIO for a distro kernel though. Would there be any objection to turning that into a modparam ?
(If we wanted to get really fancy, we could even quirk around it automatically
when we detect broken hardware).

Something like this? :)

Quoted 7 lines
Quoted 3 lines

http://www.linux.sgi.com/archives/netdev/2004-11/msg00226.html

It did not go upstream but it needed some init-time bug fixing, IIRC. Maybe akpm remembers more why my patch sucked... :)

Apart from its From: address you mean? ;)

I can find no record, sorry.

Yeah, it was in -mm for a while (via netdev-2.6.git#ALL most likely), and a -mm tester reported that it consistently oops for him, or something along those lines.

You continually (and rightly!) pestered me about it, and I withdrew the patch from -mm since I didn't have time to futz with it.

Jeff

#11 lore raw
Quoted 38 lines · On Wed, 30 Apr 2008 07:13:48 -0400 Jeff Garzik <[email protected]> wrote:
Quoted 37 lines · Andrew Morton
Quoted 25 lines · On Tue, 29 Apr 2008 18:04:06 -0400 Jeff Garzik <[email protected]> wrote:
Quoted 16 lines · Dave Jones
Quoted 15 lines · Jeff Garzik
Quoted 13 lines · Dave Jones

I've just been playing with a model 2 OQO, which has an RTL8139. It gets detected just fine, though it doesn't actually work..

eth0: RealTek RTL8139 at 0xf8830000, 00:00:00:00:00:00, IRQ 18
eth0: Identified 8139 chip type 'RTL-8139'

The null MAC address being one clue. Another oddity is that ethtool reports that there's no link detected, even though there is. (Enough for it to PXE boot a kernel from at least :)

Futzing with the debug= modparam didn't yield anything extra at all.

Any clues?

Quoted 8 lines
Quoted 2 lines

Sounds like a broken EEPROM. Does supplying a MAC via ifconfig prior to 'ifconfig ... up' help?

Ah. This sounds enlightening: http://www.oqotalk.com/index.php/topic,1511.0.html
Seems a shame to have to choose PIO vs MMIO for a distro kernel though. Would there be any objection to turning that into a modparam ?
(If we wanted to get really fancy, we could even quirk around it automatically
when we detect broken hardware).

Quoted 6 lines

Something like this? :)

http://www.linux.sgi.com/archives/netdev/2004-11/msg00226.html

It did not go upstream but it needed some init-time bug fixing, IIRC. Maybe akpm remembers more why my patch sucked... :)

Apart from its From: address you mean? ;)

Quoted 8 lines
Quoted 1 line

I can find no record, sorry.

Yeah, it was in -mm for a while (via netdev-2.6.git#ALL most likely), and a -mm tester reported that it consistently oops for him, or something along those lines.

You continually (and rightly!) pestered me about it, and I withdrew the patch from -mm since I didn't have time to futz with it.

Is there any possibility for the driver to discover broken MMIO at runtime? Could it look for some portion like mac address or HW_REVID being all zeros or all ones?

author
#12 lore raw
In reply to Jeff Garzik
Quoted 38 lines · Jeff Garzik
Quoted 37 lines · Andrew Morton
Quoted 25 lines · On Tue, 29 Apr 2008 18:04:06 -0400 Jeff Garzik <[email protected]> wrote:
Quoted 16 lines · Dave Jones
Quoted 15 lines · Jeff Garzik
Quoted 13 lines · Dave Jones

I've just been playing with a model 2 OQO, which has an RTL8139. It gets detected just fine, though it doesn't actually work..

eth0: RealTek RTL8139 at 0xf8830000, 00:00:00:00:00:00, IRQ 18
eth0: Identified 8139 chip type 'RTL-8139'

The null MAC address being one clue. Another oddity is that ethtool reports that there's no link detected, even though there is. (Enough for it to PXE boot a kernel from at least :)

Futzing with the debug= modparam didn't yield anything extra at all.

Any clues?

Quoted 8 lines
Quoted 2 lines

Sounds like a broken EEPROM. Does supplying a MAC via ifconfig prior to 'ifconfig ... up' help?

Ah. This sounds enlightening: http://www.oqotalk.com/index.php/topic,1511.0.html
Seems a shame to have to choose PIO vs MMIO for a distro kernel though. Would there be any objection to turning that into a modparam ?
(If we wanted to get really fancy, we could even quirk around it automatically
when we detect broken hardware).

Quoted 6 lines

Something like this? :)

http://www.linux.sgi.com/archives/netdev/2004-11/msg00226.html

It did not go upstream but it needed some init-time bug fixing, IIRC. Maybe akpm remembers more why my patch sucked... :)

Apart from its From: address you mean? ;)

Quoted 8 lines
Quoted 1 line

I can find no record, sorry.

Yeah, it was in -mm for a while (via netdev-2.6.git#ALL most likely), and a -mm tester reported that it consistently oops for him, or something along those lines.

You continually (and rightly!) pestered me about it, and I withdrew the patch from -mm since I didn't have time to futz with it.

So I got back to looking at this problem yesterday. Turns out the patch you reference above was merged back in 2005 :-) See commit 22f714b64b55012fa4e0d77132fa82719180f994

On top of this, it turns out to be really easy to make this a runtime modparam. I now have the OQO2 up and running with working ethernet. Patches to follow.

Dave

Signature · 1 line
http://www.codemonkey.org.uk
author
#13 lore raw

PATCH 1/2 8139too: Make PIO/MMIO a modparam

In reply to Jeff Garzik

Make PIO/MMIO a runtime thing via a module parameter. This is needed to support devices that only work with PIO without penalising devices that work fine with MMIO in distro kernels.

Signed-off-by Dave Jones <[email protected]>
drivers/net/8139too.c +73 -67
index 53bd903..09ace39 100644
@@ -120,11 +120,6 @@
                                  NETIF_MSG_LINK)
 
 
-/* enable PIO instead of MMIO, if CONFIG_8139TOO_PIO is selected */
-#ifdef CONFIG_8139TOO_PIO
-#define USE_IO_OPS 1
-#endif
-
 /* define to 1, 2 or 3 to enable copious debugging info */
 #define RTL8139_DEBUG 0
 

@@ -156,6 +151,9 @@
 static int media[MAX_UNITS] = {-1, -1, -1, -1, -1, -1, -1, -1};
 static int full_duplex[MAX_UNITS] = {-1, -1, -1, -1, -1, -1, -1, -1};
 
+/* Whether to use MMIO or PIO. Default to MMIO. */
+static int use_pio;
+
 /* Maximum number of multicast addresses to filter (vs. Rx-all-multicast).
    The RTL chips use a 64 element hash table based on the Ethernet CRC.  */
 static int multicast_filter_limit = 32;

@@ -615,6 +613,7 @@ MODULE_DESCRIPTION ("RealTek RTL-8139 Fast Ethernet driver");
 MODULE_LICENSE("GPL");
 MODULE_VERSION(DRV_VERSION);
 
+module_param(use_pio, int, 0);
 module_param(multicast_filter_limit, int, 0);
 module_param_array(media, int, NULL, 0);
 module_param_array(full_duplex, int, NULL, 0);

@@ -710,14 +709,13 @@ static void __rtl8139_cleanup_dev (struct net_device *dev)
 	assert (tp->pci_dev != NULL);
 	pdev = tp->pci_dev;
 
-#ifdef USE_IO_OPS
-	if (tp->mmio_addr)
-		ioport_unmap (tp->mmio_addr);
-#else
-	if (tp->mmio_addr)
-		pci_iounmap (pdev, tp->mmio_addr);
-#endif /* USE_IO_OPS */
-
+	if (use_pio) {
+		if (tp->mmio_addr)
+			ioport_unmap (tp->mmio_addr);
+	} else {
+		if (tp->mmio_addr)
+			pci_iounmap (pdev, tp->mmio_addr);
+	}
 	/* it's ok to call this even if we have no regions to free */
 	pci_release_regions (pdev);
 

@@ -791,32 +789,32 @@ static int __devinit rtl8139_init_board (struct pci_dev *pdev,
 	DPRINTK("PIO region size == 0x%02X\n", pio_len);
 	DPRINTK("MMIO region size == 0x%02lX\n", mmio_len);
 
-#ifdef USE_IO_OPS
-	/* make sure PCI base addr 0 is PIO */
-	if (!(pio_flags & IORESOURCE_IO)) {
-		dev_err(&pdev->dev, "region #0 not a PIO resource, aborting\n");
-		rc = -ENODEV;
-		goto err_out;
-	}
-	/* check for weird/broken PCI region reporting */
-	if (pio_len < RTL_MIN_IO_SIZE) {
-		dev_err(&pdev->dev, "Invalid PCI I/O region size(s), aborting\n");
-		rc = -ENODEV;
-		goto err_out;
-	}
-#else
-	/* make sure PCI base addr 1 is MMIO */
-	if (!(mmio_flags & IORESOURCE_MEM)) {
-		dev_err(&pdev->dev, "region #1 not an MMIO resource, aborting\n");
-		rc = -ENODEV;
-		goto err_out;
-	}
-	if (mmio_len < RTL_MIN_IO_SIZE) {
-		dev_err(&pdev->dev, "Invalid PCI mem region size(s), aborting\n");
-		rc = -ENODEV;
-		goto err_out;
+	if (use_pio) {
+		/* make sure PCI base addr 0 is PIO */
+		if (!(pio_flags & IORESOURCE_IO)) {
+			dev_err(&pdev->dev, "region #0 not a PIO resource, aborting\n");
+			rc = -ENODEV;
+			goto err_out;
+		}
+		/* check for weird/broken PCI region reporting */
+		if (pio_len < RTL_MIN_IO_SIZE) {
+			dev_err(&pdev->dev, "Invalid PCI I/O region size(s), aborting\n");
+			rc = -ENODEV;
+			goto err_out;
+		}
+	} else {
+		/* make sure PCI base addr 1 is MMIO */
+		if (!(mmio_flags & IORESOURCE_MEM)) {
+			dev_err(&pdev->dev, "region #1 not an MMIO resource, aborting\n");
+			rc = -ENODEV;
+			goto err_out;
+		}
+		if (mmio_len < RTL_MIN_IO_SIZE) {
+			dev_err(&pdev->dev, "Invalid PCI mem region size(s), aborting\n");
+			rc = -ENODEV;
+			goto err_out;
+		}
 	}
-#endif
 
 	rc = pci_request_regions (pdev, DRV_NAME);
 	if (rc)

@@ -826,28 +824,28 @@ static int __devinit rtl8139_init_board (struct pci_dev *pdev,
 	/* enable PCI bus-mastering */
 	pci_set_master (pdev);
 
-#ifdef USE_IO_OPS
-	ioaddr = ioport_map(pio_start, pio_len);
-	if (!ioaddr) {
-		dev_err(&pdev->dev, "cannot map PIO, aborting\n");
-		rc = -EIO;
-		goto err_out;
-	}
-	dev->base_addr = pio_start;
-	tp->mmio_addr = ioaddr;
-	tp->regs_len = pio_len;
-#else
-	/* ioremap MMIO region */
-	ioaddr = pci_iomap(pdev, 1, 0);
-	if (ioaddr == NULL) {
-		dev_err(&pdev->dev, "cannot remap MMIO, aborting\n");
-		rc = -EIO;
-		goto err_out;
+	if (use_pio) {
+		ioaddr = ioport_map(pio_start, pio_len);
+		if (!ioaddr) {
+			dev_err(&pdev->dev, "cannot map PIO, aborting\n");
+			rc = -EIO;
+			goto err_out;
+		}
+		dev->base_addr = pio_start;
+		tp->mmio_addr = ioaddr;
+		tp->regs_len = pio_len;
+	} else {
+		/* ioremap MMIO region */
+		ioaddr = pci_iomap(pdev, 1, 0);
+		if (ioaddr == NULL) {
+			dev_err(&pdev->dev, "cannot remap MMIO, aborting\n");
+			rc = -EIO;
+			goto err_out;
+		}
+		dev->base_addr = (long) ioaddr;
+		tp->mmio_addr = ioaddr;
+		tp->regs_len = mmio_len;
 	}
-	dev->base_addr = (long) ioaddr;
-	tp->mmio_addr = ioaddr;
-	tp->regs_len = mmio_len;
-#endif /* USE_IO_OPS */
 
 	/* Bring old chips out of low-power mode. */
 	RTL_W8 (HltClk, 'R');

@@ -2383,20 +2381,24 @@ static void rtl8139_set_msglevel(struct net_device *dev, u32 datum)
 	np->msg_enable = datum;
 }
 
-/* TODO: we are too slack to do reg dumping for pio, for now */
-#ifdef CONFIG_8139TOO_PIO
-#define rtl8139_get_regs_len	NULL
-#define rtl8139_get_regs	NULL
-#else
 static int rtl8139_get_regs_len(struct net_device *dev)
 {
-	struct rtl8139_private *np = netdev_priv(dev);
+	struct rtl8139_private *np;
+	/* TODO: we are too slack to do reg dumping for pio, for now */
+	if (use_pio)
+		return 0;
+	np = netdev_priv(dev);
 	return np->regs_len;
 }
 
 static void rtl8139_get_regs(struct net_device *dev, struct ethtool_regs *regs, void *regbuf)
 {
-	struct rtl8139_private *np = netdev_priv(dev);
+	struct rtl8139_private *np;
+
+	/* TODO: we are too slack to do reg dumping for pio, for now */
+	if (use_pio)
+		return;
+	np = netdev_priv(dev);
 
 	regs->version = RTL_REGS_VER;
 

@@ -2404,7 +2406,6 @@ static void rtl8139_get_regs(struct net_device *dev, struct ethtool_regs *regs,
 	memcpy_fromio(regbuf, np->mmio_addr, regs->len);
 	spin_unlock_irq(&np->lock);
 }
-#endif /* CONFIG_8139TOO_MMIO */
 
 static int rtl8139_get_sset_count(struct net_device *dev, int sset)
 {

@@ -2610,6 +2611,11 @@ static int __init rtl8139_init_module (void)
 	printk (KERN_INFO RTL8139_DRIVER_NAME "\n");
 #endif
 
+	/* enable PIO instead of MMIO, if CONFIG_8139TOO_PIO is selected */
+#ifdef CONFIG_8139TOO_PIO
+	use_pio = 1;
+#endif
+
 	return pci_register_driver(&rtl8139_pci_driver);
 }
Signature · 1 line
http://www.codemonkey.org.uk
author
#28 lore raw

PATCH 2/2 8139too: Make the OQO2 automatically use PIO mode.

In reply to Jeff Garzik

The OQO model 2 has an RTL8139 from Atheros that doesn't like MMIO. Force it to always use polled IO.

Signed-off-by Dave Jones <[email protected]>
drivers/net/8139too.c renamed from drivers/net/8139too.c~ +8 -0
@@ -951,6 +951,14 @@ static int __devinit rtl8139_init_one (s
 			   "Use the \"8139cp\" driver for improved performance and stability.\n");
 	}
 
+	if (pdev->vendor == PCI_VENDOR_ID_REALTEK &&
+	    pdev->device == PCI_DEVICE_ID_REALTEK_8139 &&
+	    pdev->subsystem_vendor == PCI_VENDOR_ID_ATHEROS &&
+	    pdev->subsystem_device == PCI_DEVICE_ID_REALTEK_8139) {
+		printk(KERN_INFO "8139too: OQO Model 2 detected. Forcing PIO\n");
+		use_pio = 1;
+	}
+
 	i = rtl8139_init_board (pdev, &dev);
 	if (i < 0)
 		return i;
Signature · 1 line
http://www.codemonkey.org.uk
#14 lore raw

Re: [PATCH 1/2] 8139too: Make PIO/MMIO a modparam

In reply to Dave Jones
Quoted 6 lines · Dave Jones

Make PIO/MMIO a runtime thing via a module parameter. This is needed to support devices that only work with PIO without penalising devices that work fine with MMIO in distro kernels.

Signed-off-by Dave Jones <[email protected]>

two comments:

1) should use pci_iomap() for both PIO and MMIO

2) modparam should be "use_io" mirroring existing drivers

#29 lore raw

Re: [PATCH 2/2] 8139too: Make the OQO2 automatically use PIO mode.

In reply to Dave Jones
Quoted 9 lines · Dave Jones

The OQO model 2 has an RTL8139 from Atheros that doesn't like MMIO. Force it to always use polled IO.

Signed-off-by Dave Jones <[email protected]>
drivers/net/8139too.c renamed from drivers/net/8139too.c~ +0 -0
@@ -951,6 +951,14 @@ static int __devinit rtl8139_init_one (s
 			   "Use the \"8139cp\" driver for improved performance and stability.\n");

drivers/net/8139too.c static int __devinit rtl8139_init_one (s · line 961

Quoted 10 lines
drivers/net/8139too.c renamed from drivers/net/8139too.c~ +8 -0
 	}
 
+	if (pdev->vendor == PCI_VENDOR_ID_REALTEK &&
+	    pdev->device == PCI_DEVICE_ID_REALTEK_8139 &&
+	    pdev->subsystem_vendor == PCI_VENDOR_ID_ATHEROS &&
+	    pdev->subsystem_device == PCI_DEVICE_ID_REALTEK_8139) {
+		printk(KERN_INFO "8139too: OQO Model 2 detected. Forcing PIO\n");
+		use_pio = 1;
+	}
+

these days we almost never hand-craft PCI ID checks like this... Add this to the pci_device_id table, and create a new RTL8139_PIO entry for driver_data that has your desired end result

That way, it is trivial to expand the list simply by updating the PCI ID table

Table-driven approaches are really superior for things like this

author
#15 lore raw

Re: [PATCH 1/2] 8139too: Make PIO/MMIO a modparam

In reply to Jeff Garzik
Quoted 6 lines · Jeff Garzik
Quoted 5 lines · Dave Jones

Make PIO/MMIO a runtime thing via a module parameter. This is needed to support devices that only work with PIO without penalising devices that work fine with MMIO in distro kernels.

Quoted 7 lines
Quoted 1 line
Signed-off-by Dave Jones <[email protected]>

two comments:

1) should use pci_iomap() for both PIO and MMIO

2) modparam should be "use_io" mirroring existing drivers

Ok, I'll take a stab at those later today (and also your comment from the other mail).

One other thing: I wasn't sure about my changes to rtl8139_get_regs and rtl8139_get_regs_len. Is what I did there safe? Obviously actually implementing support for PIO reg dumping would be better, but I think that's beyond the scope of what I'm trying to do for now.

Dave

Signature · 1 line
http://www.codemonkey.org.uk
#16 lore raw

Re: [PATCH 1/2] 8139too: Make PIO/MMIO a modparam

Quoted 15 lines · Dave Jones
Quoted 6 lines · Jeff Garzik
Quoted 5 lines · Dave Jones

Make PIO/MMIO a runtime thing via a module parameter. This is needed to support devices that only work with PIO without penalising devices that work fine with MMIO in distro kernels.

Quoted 7 lines
Quoted 1 line
Signed-off-by Dave Jones <[email protected]>

two comments:

1) should use pci_iomap() for both PIO and MMIO

2) modparam should be "use_io" mirroring existing drivers

Quoted 8 lines

Ok, I'll take a stab at those later today (and also your comment from the other mail).

One other thing: I wasn't sure about my changes to rtl8139_get_regs and rtl8139_get_regs_len. Is what I did there safe? Obviously actually implementing support for PIO reg dumping would be better, but I think that's beyond the scope of what I'm trying to do for now.

Yeah, that should be reasonable

Jeff

author
#17 lore raw

Re: [PATCH 1/2] 8139too: Make PIO/MMIO a modparam

Quoted 19 lines · Jeff Garzik
Quoted 18 lines · Dave Jones
Quoted 6 lines · Jeff Garzik
Quoted 5 lines · Dave Jones

Make PIO/MMIO a runtime thing via a module parameter. This is needed to support devices that only work with PIO without penalising devices that work fine with MMIO in distro kernels.

Quoted 7 lines
Quoted 1 line
Signed-off-by Dave Jones <[email protected]>

two comments:

1) should use pci_iomap() for both PIO and MMIO

2) modparam should be "use_io" mirroring existing drivers

Ok, I'll take a stab at those later today (and also your comment from the other mail).

Quoted 7 lines
Quoted 5 lines

One other thing: I wasn't sure about my changes to rtl8139_get_regs and rtl8139_get_regs_len. Is what I did there safe? Obviously actually implementing support for PIO reg dumping would be better, but I think that's beyond the scope of what I'm trying to do for now.

Yeah, that should be reasonable

Finally got back to this. How does this look?

Dave

Make PIO/MMIO a runtime thing via a module parameter. This is needed to support devices that only work with PIO without penalising devices that work fine with MMIO in distro kernels.

Signed-off-by Dave Jones <[email protected]>
drivers/net/8139too.c +64 -64
index 53bd903..b620317 100644
@@ -98,7 +98,6 @@
 #include <linux/compiler.h>
 #include <linux/pci.h>
 #include <linux/init.h>
-#include <linux/ioport.h>
 #include <linux/netdevice.h>
 #include <linux/etherdevice.h>
 #include <linux/rtnetlink.h>

@@ -120,11 +119,6 @@
                                  NETIF_MSG_LINK)
 
 
-/* enable PIO instead of MMIO, if CONFIG_8139TOO_PIO is selected */
-#ifdef CONFIG_8139TOO_PIO
-#define USE_IO_OPS 1
-#endif
-
 /* define to 1, 2 or 3 to enable copious debugging info */
 #define RTL8139_DEBUG 0
 

@@ -156,6 +150,9 @@
 static int media[MAX_UNITS] = {-1, -1, -1, -1, -1, -1, -1, -1};
 static int full_duplex[MAX_UNITS] = {-1, -1, -1, -1, -1, -1, -1, -1};
 
+/* Whether to use MMIO or PIO. Default to MMIO. */
+static int use_io;
+
 /* Maximum number of multicast addresses to filter (vs. Rx-all-multicast).
    The RTL chips use a 64 element hash table based on the Ethernet CRC.  */
 static int multicast_filter_limit = 32;

@@ -615,6 +612,7 @@ MODULE_DESCRIPTION ("RealTek RTL-8139 Fast Ethernet driver");
 MODULE_LICENSE("GPL");
 MODULE_VERSION(DRV_VERSION);
 
+module_param(use_io, int, 0);
 module_param(multicast_filter_limit, int, 0);
 module_param_array(media, int, NULL, 0);
 module_param_array(full_duplex, int, NULL, 0);

@@ -710,13 +708,8 @@ static void __rtl8139_cleanup_dev (struct net_device *dev)
 	assert (tp->pci_dev != NULL);
 	pdev = tp->pci_dev;
 
-#ifdef USE_IO_OPS
-	if (tp->mmio_addr)
-		ioport_unmap (tp->mmio_addr);
-#else
 	if (tp->mmio_addr)
 		pci_iounmap (pdev, tp->mmio_addr);
-#endif /* USE_IO_OPS */
 
 	/* it's ok to call this even if we have no regions to free */
 	pci_release_regions (pdev);

@@ -791,32 +784,32 @@ static int __devinit rtl8139_init_board (struct pci_dev *pdev,
 	DPRINTK("PIO region size == 0x%02X\n", pio_len);
 	DPRINTK("MMIO region size == 0x%02lX\n", mmio_len);
 
-#ifdef USE_IO_OPS
-	/* make sure PCI base addr 0 is PIO */
-	if (!(pio_flags & IORESOURCE_IO)) {
-		dev_err(&pdev->dev, "region #0 not a PIO resource, aborting\n");
-		rc = -ENODEV;
-		goto err_out;
-	}
-	/* check for weird/broken PCI region reporting */
-	if (pio_len < RTL_MIN_IO_SIZE) {
-		dev_err(&pdev->dev, "Invalid PCI I/O region size(s), aborting\n");
-		rc = -ENODEV;
-		goto err_out;
-	}
-#else
-	/* make sure PCI base addr 1 is MMIO */
-	if (!(mmio_flags & IORESOURCE_MEM)) {
-		dev_err(&pdev->dev, "region #1 not an MMIO resource, aborting\n");
-		rc = -ENODEV;
-		goto err_out;
-	}
-	if (mmio_len < RTL_MIN_IO_SIZE) {
-		dev_err(&pdev->dev, "Invalid PCI mem region size(s), aborting\n");
-		rc = -ENODEV;
-		goto err_out;
+	if (use_io) {
+		/* make sure PCI base addr 0 is PIO */
+		if (!(pio_flags & IORESOURCE_IO)) {
+			dev_err(&pdev->dev, "region #0 not a PIO resource, aborting\n");
+			rc = -ENODEV;
+			goto err_out;
+		}
+		/* check for weird/broken PCI region reporting */
+		if (pio_len < RTL_MIN_IO_SIZE) {
+			dev_err(&pdev->dev, "Invalid PCI I/O region size(s), aborting\n");
+			rc = -ENODEV;
+			goto err_out;
+		}
+	} else {
+		/* make sure PCI base addr 1 is MMIO */
+		if (!(mmio_flags & IORESOURCE_MEM)) {
+			dev_err(&pdev->dev, "region #1 not an MMIO resource, aborting\n");
+			rc = -ENODEV;
+			goto err_out;
+		}
+		if (mmio_len < RTL_MIN_IO_SIZE) {
+			dev_err(&pdev->dev, "Invalid PCI mem region size(s), aborting\n");
+			rc = -ENODEV;
+			goto err_out;
+		}
 	}
-#endif
 
 	rc = pci_request_regions (pdev, DRV_NAME);
 	if (rc)

@@ -826,28 +819,27 @@ static int __devinit rtl8139_init_board (struct pci_dev *pdev,
 	/* enable PCI bus-mastering */
 	pci_set_master (pdev);
 
-#ifdef USE_IO_OPS
-	ioaddr = ioport_map(pio_start, pio_len);
-	if (!ioaddr) {
-		dev_err(&pdev->dev, "cannot map PIO, aborting\n");
-		rc = -EIO;
-		goto err_out;
-	}
-	dev->base_addr = pio_start;
-	tp->mmio_addr = ioaddr;
-	tp->regs_len = pio_len;
-#else
-	/* ioremap MMIO region */
-	ioaddr = pci_iomap(pdev, 1, 0);
-	if (ioaddr == NULL) {
-		dev_err(&pdev->dev, "cannot remap MMIO, aborting\n");
-		rc = -EIO;
-		goto err_out;
+	if (use_io) {
+		ioaddr = pci_iomap(pdev, 0, 0);
+		if (!ioaddr) {
+			dev_err(&pdev->dev, "cannot map PIO, aborting\n");
+			rc = -EIO;
+			goto err_out;
+		}
+		dev->base_addr = pio_start;
+		tp->regs_len = pio_len;
+	} else {
+		/* ioremap MMIO region */
+		ioaddr = pci_iomap(pdev, 1, 0);
+		if (ioaddr == NULL) {
+			dev_err(&pdev->dev, "cannot remap MMIO, aborting\n");
+			rc = -EIO;
+			goto err_out;
+		}
+		dev->base_addr = (long) ioaddr;
+		tp->regs_len = mmio_len;
 	}
-	dev->base_addr = (long) ioaddr;
 	tp->mmio_addr = ioaddr;
-	tp->regs_len = mmio_len;
-#endif /* USE_IO_OPS */
 
 	/* Bring old chips out of low-power mode. */
 	RTL_W8 (HltClk, 'R');

@@ -2383,20 +2375,24 @@ static void rtl8139_set_msglevel(struct net_device *dev, u32 datum)
 	np->msg_enable = datum;
 }
 
-/* TODO: we are too slack to do reg dumping for pio, for now */
-#ifdef CONFIG_8139TOO_PIO
-#define rtl8139_get_regs_len	NULL
-#define rtl8139_get_regs	NULL
-#else
 static int rtl8139_get_regs_len(struct net_device *dev)
 {
-	struct rtl8139_private *np = netdev_priv(dev);
+	struct rtl8139_private *np;
+	/* TODO: we are too slack to do reg dumping for pio, for now */
+	if (use_io)
+		return 0;
+	np = netdev_priv(dev);
 	return np->regs_len;
 }
 
 static void rtl8139_get_regs(struct net_device *dev, struct ethtool_regs *regs, void *regbuf)
 {
-	struct rtl8139_private *np = netdev_priv(dev);
+	struct rtl8139_private *np;
+
+	/* TODO: we are too slack to do reg dumping for pio, for now */
+	if (use_io)
+		return;
+	np = netdev_priv(dev);
 
 	regs->version = RTL_REGS_VER;
 

@@ -2404,7 +2400,6 @@ static void rtl8139_get_regs(struct net_device *dev, struct ethtool_regs *regs,
 	memcpy_fromio(regbuf, np->mmio_addr, regs->len);
 	spin_unlock_irq(&np->lock);
 }
-#endif /* CONFIG_8139TOO_MMIO */
 
 static int rtl8139_get_sset_count(struct net_device *dev, int sset)
 {

@@ -2610,6 +2605,11 @@ static int __init rtl8139_init_module (void)
 	printk (KERN_INFO RTL8139_DRIVER_NAME "\n");
 #endif
 
+	/* enable PIO instead of MMIO, if CONFIG_8139TOO_PIO is selected */
+#ifdef CONFIG_8139TOO_PIO
+	use_io = 1;
+#endif
+
 	return pci_register_driver(&rtl8139_pci_driver);
 }
Signature · 1 line
http://www.codemonkey.org.uk
#18 lore raw

Re: [PATCH 1/2] 8139too: Make PIO/MMIO a modparam

Quoted 34 lines · Dave Jones
Quoted 19 lines · Jeff Garzik
Quoted 18 lines · Dave Jones
Quoted 6 lines · Jeff Garzik
Quoted 5 lines · Dave Jones

Make PIO/MMIO a runtime thing via a module parameter. This is needed to support devices that only work with PIO without penalising devices that work fine with MMIO in distro kernels.

Quoted 7 lines
Quoted 1 line
Signed-off-by Dave Jones <[email protected]>

two comments:

1) should use pci_iomap() for both PIO and MMIO

2) modparam should be "use_io" mirroring existing drivers

Ok, I'll take a stab at those later today (and also your comment from the other mail).

Quoted 7 lines
Quoted 5 lines

One other thing: I wasn't sure about my changes to rtl8139_get_regs and rtl8139_get_regs_len. Is what I did there safe? Obviously actually implementing support for PIO reg dumping would be better, but I think that's beyond the scope of what I'm trying to do for now.

Yeah, that should be reasonable

Finally got back to this. How does this look?

Dave

Quoted 6 lines

Make PIO/MMIO a runtime thing via a module parameter. This is needed to support devices that only work with PIO without penalising devices that work fine with MMIO in distro kernels.

Signed-off-by Dave Jones <[email protected]>

Looks great overall.

Minor nits:

* need module param text description

* [optional] if code not too ugly, change mod param description based on CONFIG_8139TOO_PIO to indicate the currently compiled default

* [optional] would prefer CONFIG_8139TOO_PIO be handled at compile time, by changing the initialized value

* [extra project] would be highly useful for MMIO to fall back to PIO, and vice versa, should any resource be unavailable. Sometimes, mainly with MMIO and broken/weird BIOSen, only the PIO PCI BARs will be filled in with useful info.

author
#19 lore raw

Re: [PATCH 1/2] 8139too: Make PIO/MMIO a modparam

Quoted 5 lines · Jeff Garzik
Quoted 5 lines

Make PIO/MMIO a runtime thing via a module parameter. This is needed to support devices that only work with PIO without penalising devices that work fine with MMIO in distro kernels.

Quoted 7 lines
Quoted 1 line
Signed-off-by Dave Jones <[email protected]>

Looks great overall.

Minor nits:

* need module param text description

oops. will fix.

Quoted 2 lines

* [optional] if code not too ugly, change mod param description based on CONFIG_8139TOO_PIO to indicate the currently compiled default

Not sure of a non-icky way to do this other than ifdefs. The best I could come up with is.

#ifdef CONFIG_8139TOO_PIO
MODULE_PARM_DESC(use_io, "PIO/MMIO switch. 0=MMIO 1=PIO default=PIO"); #else
MODULE_PARM_DESC(use_io, "PIO/MMIO switch. 0=MMIO 1=PIO default=MMIO"); #endif

palatable?

Quoted 2 lines

* [optional] would prefer CONFIG_8139TOO_PIO be handled at compile time, by changing the initialized value

This bit should be taken care of in rtl8139_init_module() at the bottom..

+ /* enable PIO instead of MMIO, if CONFIG_8139TOO_PIO is selected */
+#ifdef CONFIG_8139TOO_PIO
+ use_io = 1;
+#endif

Quoted 4 lines

* [extra project] would be highly useful for MMIO to fall back to PIO, and vice versa, should any resource be unavailable. Sometimes, mainly with MMIO and broken/weird BIOSen, only the PIO PCI BARs will be filled in with useful info.

Sounds do-able. I'll add it to my rainy-day project list.

Dave

Signature · 1 line
http://www.codemonkey.org.uk
#20 lore raw

Re: [PATCH 1/2] 8139too: Make PIO/MMIO a modparam

Quoted 20 lines · Dave Jones
Quoted 5 lines · Jeff Garzik
Quoted 5 lines

Make PIO/MMIO a runtime thing via a module parameter. This is needed to support devices that only work with PIO without penalising devices that work fine with MMIO in distro kernels.

Quoted 7 lines
Quoted 1 line
Signed-off-by Dave Jones <[email protected]>

Looks great overall.

Minor nits:

* need module param text description

oops. will fix.

Quoted 2 lines

* [optional] if code not too ugly, change mod param description based on CONFIG_8139TOO_PIO to indicate the currently compiled default

Quoted 8 lines

Not sure of a non-icky way to do this other than ifdefs. The best I could come up with is.

#ifdef CONFIG_8139TOO_PIO
MODULE_PARM_DESC(use_io, "PIO/MMIO switch. 0=MMIO 1=PIO default=PIO"); #else MODULE_PARM_DESC(use_io, "PIO/MMIO switch. 0=MMIO 1=PIO default=MMIO"); #endif

palatable?

best you can do AFAIK, so yes

drivers/net/8139too.c static int __init rtl8139_init_module (void) · line 2611

Quoted 9 lines
Quoted 2 lines

* [optional] would prefer CONFIG_8139TOO_PIO be handled at compile time, by changing the initialized value

This bit should be taken care of in rtl8139_init_module() at the bottom..

drivers/net/8139too.c +4 -0
+       /* enable PIO instead of MMIO, if CONFIG_8139TOO_PIO is selected */
+#ifdef CONFIG_8139TOO_PIO
+       use_io = 1;
+#endif

Right, I was saying I prefer simply to set the initialized value appropriately, and avoid the extra code dollup during module load.

Quoted 6 lines
Quoted 4 lines

* [extra project] would be highly useful for MMIO to fall back to PIO, and vice versa, should any resource be unavailable. Sometimes, mainly with MMIO and broken/weird BIOSen, only the PIO PCI BARs will be filled in with useful info.

Sounds do-able. I'll add it to my rainy-day project list.

Thanks :) A few users out in the wild will definitely thank you.

Jeff

author
#25 lore raw

Re: [PATCH 1/2] 8139too: Make PIO/MMIO a modparam

In reply to Jeff Garzik
Quoted 4 lines · Jeff Garzik

* [extra project] would be highly useful for MMIO to fall back to PIO, and vice versa, should any resource be unavailable. Sometimes, mainly with MMIO and broken/weird BIOSen, only the PIO PCI BARs will be filled in with useful info.

Hmm, this bit might actually be fairly trivial on top of my other patch..

drivers/net/8139too.c renamed from drivers/net/8139too.c~ +5 -3
@@ -784,6 +784,7 @@ static int __devinit rtl8139_init_board 
 	DPRINTK("PIO region size == 0x%02X\n", pio_len);
 	DPRINTK("MMIO region size == 0x%02lX\n", mmio_len);
 
+retry:
 	if (use_io) {
 		/* make sure PCI base addr 0 is PIO */
 		if (!(pio_flags & IORESOURCE_IO)) {

@@ -832,9 +833,10 @@ static int __devinit rtl8139_init_board 
 		/* ioremap MMIO region */
 		ioaddr = pci_iomap(pdev, 1, 0);
 		if (ioaddr == NULL) {
-			dev_err(&pdev->dev, "cannot remap MMIO, aborting\n");
-			rc = -EIO;
-			goto err_out;
+			dev_err(&pdev->dev, "cannot remap MMIO, trying PIO\n");
+			pci_release_regions(pdev);
+			use_ui = 1;
+			goto retry;
 		}
 		dev->base_addr = (long) ioaddr;
 		tp->regs_len = mmio_len;
Signature · 1 line
http://www.codemonkey.org.uk
#26 lore raw

Re: [PATCH 1/2] 8139too: Make PIO/MMIO a modparam

Quoted 21 lines · Dave Jones
Quoted 4 lines · Jeff Garzik

* [extra project] would be highly useful for MMIO to fall back to PIO, and vice versa, should any resource be unavailable. Sometimes, mainly with MMIO and broken/weird BIOSen, only the PIO PCI BARs will be filled in with useful info.

Hmm, this bit might actually be fairly trivial on top of my other patch..

drivers/net/8139too.c renamed from drivers/net/8139too.c~ +1 -0
@@ -784,6 +784,7 @@ static int __devinit rtl8139_init_board 
 	DPRINTK("PIO region size == 0x%02X\n", pio_len);
 	DPRINTK("MMIO region size == 0x%02lX\n", mmio_len);
 
+retry:
 	if (use_io) {
 		/* make sure PCI base addr 0 is PIO */
 		if (!(pio_flags & IORESOURCE_IO)) {

drivers/net/8139too.c static int __devinit rtl8139_init_board · line 839

Quoted 10 lines
drivers/net/8139too.c renamed from drivers/net/8139too.c~ +4 -3
@@ -832,9 +833,10 @@ static int __devinit rtl8139_init_board 
 		/* ioremap MMIO region */
 		ioaddr = pci_iomap(pdev, 1, 0);
 		if (ioaddr == NULL) {
-			dev_err(&pdev->dev, "cannot remap MMIO, aborting\n");
-			rc = -EIO;
-			goto err_out;
+			dev_err(&pdev->dev, "cannot remap MMIO, trying PIO\n");
+			pci_release_regions(pdev);
+			use_ui = 1;
+			goto retry;

use_ui? Are you sure this is not not an X.org patch? :)

Yes, seems like that would do the trick quite nicely...

If you would be kind enough to post this as a separate patch from your use_io patch (since IMO its a separate logical change), that would be helpful.

Jeff

author
#21 lore raw

Re: [PATCH 1/2] 8139too: Make PIO/MMIO a modparam

In reply to Jeff Garzik
Quoted 5 lines · Jeff Garzik
Quoted 5 lines
Quoted 2 lines

* [optional] would prefer CONFIG_8139TOO_PIO be handled at compile time, by changing the initialized value

This bit should be taken care of in rtl8139_init_module() at the bottom..

Quoted 7 lines

drivers/net/8139too.c static int __init rtl8139_init_module (void) · line 2611

Quoted 4 lines
drivers/net/8139too.c +4 -0
+       /* enable PIO instead of MMIO, if CONFIG_8139TOO_PIO is selected */
+#ifdef CONFIG_8139TOO_PIO
+       use_io = 1;
+#endif

Right, I was saying I prefer simply to set the initialized value appropriately, and avoid the extra code dollup during module load.

Oh, you mean something like..

static int use_io = CONFIG_8139TOO_PIO;

?

Dave

Signature · 1 line
http://www.codemonkey.org.uk
author
#27 lore raw

Re: [PATCH 1/2] 8139too: Make PIO/MMIO a modparam

In reply to Jeff Garzik
Quoted 5 lines · Jeff Garzik

drivers/net/8139too.c static int __devinit rtl8139_init_board · line 839

Quoted 3 lines
drivers/net/8139too.c renamed from drivers/net/8139too.c~ +3 -0
+			pci_release_regions(pdev);
+			use_ui = 1;
+			goto retry;

use_ui? Are you sure this is not not an X.org patch? :)

heh, who needs compilers ?

Quoted 5 lines

Yes, seems like that would do the trick quite nicely...

If you would be kind enough to post this as a separate patch from your use_io patch (since IMO its a separate logical change), that would be helpful.

Sure, I'll repost the series in a bit.

Dave

Signature · 1 line
http://www.codemonkey.org.uk
#22 lore raw

Re: [PATCH 1/2] 8139too: Make PIO/MMIO a modparam

In reply to Dave Jones
Quoted 15 lines · Dave Jones
Quoted 5 lines · Jeff Garzik
Quoted 5 lines
Quoted 2 lines

* [optional] would prefer CONFIG_8139TOO_PIO be handled at compile time, by changing the initialized value

This bit should be taken care of in rtl8139_init_module() at the bottom..

Quoted 7 lines

drivers/net/8139too.c static int __init rtl8139_init_module (void) · line 2611

Quoted 4 lines
drivers/net/8139too.c +4 -0
+       /* enable PIO instead of MMIO, if CONFIG_8139TOO_PIO is selected */
+#ifdef CONFIG_8139TOO_PIO
+       use_io = 1;
+#endif

Right, I was saying I prefer simply to set the initialized value appropriately, and avoid the extra code dollup during module load.

Quoted 5 lines

Oh, you mean something like..

static int use_io = CONFIG_8139TOO_PIO;

?

Sorry for not being specific. I would have guessed the uglier

	#ifdef CONFIG_8139TOO_PIO
		static int use_io = 1;
	#else
		static int use_io;
	#endif

since CONFIG_8139TOO_PIO doesn't convert to a 1 or 0 neatly in C AFAIK?

But, overall, yes. You get the general idea.

Jeff

author
#23 lore raw

Re: [PATCH 1/2] 8139too: Make PIO/MMIO a modparam

Quoted 9 lines · Jeff Garzik

Sorry for not being specific. I would have guessed the uglier

	#ifdef CONFIG_8139TOO_PIO
		static int use_io = 1;
	#else
		static int use_io;
	#endif

since CONFIG_8139TOO_PIO doesn't convert to a 1 or 0 neatly in C AFAIK?

Hm, yeah. that only works in the PIO=1 case. When not set, it doesn't end up in autoconf.h. I'll go with the fugly ifdefs.

Dave

Signature · 1 line
http://www.codemonkey.org.uk
#24 lore raw

Re: [PATCH 1/2] 8139too: Make PIO/MMIO a modparam

In reply to Jeff Garzik
Quoted 9 lines
Quoted 9 lines
Quoted 3 lines
Quoted 1 line
Quoted 1 line

"Jeff" == Jeff Garzik <[email protected]> writes:

* [optional] if code not too ugly, change mod param description

based on > CONFIG_8139TOO_PIO to indicate the currently compiled default

Not sure of a non-icky way to do this other than ifdefs. The best I could come up with is.

Quoted 7 lines
Quoted 7 lines

#ifdef CONFIG_8139TOO_PIO MODULE_PARM_DESC(use_io, "PIO/MMIO switch. 0=MMIO 1=PIO default=PIO"); #else
MODULE_PARM_DESC(use_io, "PIO/MMIO switch. 0=MMIO 1=PIO default=MMIO"); #endif

palatable?

Jeff> best you can do AFAIK, so yes

Something like:
#ifdef CONFIG_8139TOO_PIO
#define DEFAULTMODE "PIO"
#else
#define DEFAULTMODE "MMIO"
#endif

MODULE_PARM_DESC(use_io, "PIO/MMIO switch. 0=MMIO 1=PIO default=" DEFAULTMODE);

would be nicer.

Signature · 1 line
Bye, Peter Korsgaard