sungem driver patch testing..

sungem driver patch testing..

[Linus Torvalds]

I've been grepping around for things that do their own PCI ROM mapping and 
do it badly, and one thing that matches that description is the sungem 
ethernet driver on PC's.

If anybody has such a beast, can you please try this patch and report 
whether it works for you? 

		Linus

---
diff --git a/drivers/net/sungem.c b/drivers/net/sungem.c
--- a/drivers/net/sungem.c
+++ b/drivers/net/sungem.c
@@ -2817,7 +2817,7 @@ static int gem_ioctl(struct net_device *
 
 #if (!defined(__sparc__) && !defined(CONFIG_PPC_PMAC))
 /* Fetch MAC address from vital product data of PCI ROM. */
-static void find_eth_addr_in_vpd(void __iomem *rom_base, int len, unsigned char *dev_addr)
+static int find_eth_addr_in_vpd(void __iomem *rom_base, int len, unsigned char *dev_addr)
 {
 	int this_offset;
 
@@ -2838,35 +2838,27 @@ static void find_eth_addr_in_vpd(void __
 
 		for (i = 0; i < 6; i++)
 			dev_addr[i] = readb(p + i);
-		break;
+		return 1;
 	}
+	return 0;
 }
 
 static void get_gem_mac_nonobp(struct pci_dev *pdev, unsigned char *dev_addr)
 {
-	u32 rom_reg_orig;
-	void __iomem *p;
-
-	if (pdev->resource[PCI_ROM_RESOURCE].parent == NULL) {
-		if (pci_assign_resource(pdev, PCI_ROM_RESOURCE) < 0)
-			goto use_random;
-	}
-
-	pci_read_config_dword(pdev, pdev->rom_base_reg, &rom_reg_orig);
-	pci_write_config_dword(pdev, pdev->rom_base_reg,
-			       rom_reg_orig | PCI_ROM_ADDRESS_ENABLE);
+	size_t size;
+	void __iomem *p = pci_map_rom(pdev, &size);
 
-	p = ioremap(pci_resource_start(pdev, PCI_ROM_RESOURCE), (64 * 1024));
-	if (p != NULL && readb(p) == 0x55 && readb(p + 1) == 0xaa)
-		find_eth_addr_in_vpd(p, (64 * 1024), dev_addr);
+	if (p) {
+			int found;
 
-	if (p != NULL)
-		iounmap(p);
-
-	pci_write_config_dword(pdev, pdev->rom_base_reg, rom_reg_orig);
-	return;
+		found = readb(p) == 0x55 &&
+			readb(p + 1) == 0xaa &&
+			find_eth_addr_in_vpd(p, (64 * 1024), dev_addr);
+		pci_unmap_rom(pdev, p);
+		if (found)
+			return;
+	}
 
-use_random:
 	/* Sun MAC prefix then 3 random bytes. */
 	dev_addr[0] = 0x08;
 	dev_addr[1] = 0x00;

Re: sungem driver patch testing..

[Willy Tarreau]

Hi Linus,

On Sat, Sep 10, 2005 at 08:11:22PM -0700, Linus Torvalds wrote:
> 
> I've been grepping around for things that do their own PCI ROM mapping and 
> do it badly, and one thing that matches that description is the sungem 
> ethernet driver on PC's.
> 
> If anybody has such a beast, can you please try this patch and report 
> whether it works for you? 

I've ported it to sunhme (which uses the same PCI ROM mapping code).
Without the patch, I get NULL MAC addresses for all 4 ports (it's a SUN
QFE). With the patch, I get the correct addresses (the ones printed on
the label on the card).

I attach the patch for sunhme, and Cc: Davem so that he updates his tree.

Thanks,
Willy


--- linux-2.6.13.1.orig/drivers/net/sunhme.c	Wed Aug 24 22:51:25 2005
+++ linux-2.6.13.1/drivers/net/sunhme.c	Sun Sep 11 09:06:26 2005
@@ -2954,7 +2954,7 @@ static int is_quattro_p(struct pci_dev *
 }
 
 /* Fetch MAC address from vital product data of PCI ROM. */
-static void find_eth_addr_in_vpd(void __iomem *rom_base, int len, int index, unsigned char *dev_addr)
+static int find_eth_addr_in_vpd(void __iomem *rom_base, int len, int index, unsigned char *dev_addr)
 {
 	int this_offset;
 
@@ -2977,42 +2977,33 @@ static void find_eth_addr_in_vpd(void __
 
 			for (i = 0; i < 6; i++)
 				dev_addr[i] = readb(p + i);
-			break;
+			return 1;
 		}
 		index--;
 	}
+	return 0;
 }
 
 static void get_hme_mac_nonsparc(struct pci_dev *pdev, unsigned char *dev_addr)
 {
-	u32 rom_reg_orig;
-	void __iomem *p;
-	int index;
-
-	index = 0;
-	if (is_quattro_p(pdev))
-		index = PCI_SLOT(pdev->devfn);
-
-	if (pdev->resource[PCI_ROM_RESOURCE].parent == NULL) {
-		if (pci_assign_resource(pdev, PCI_ROM_RESOURCE) < 0)
-			goto use_random;
-	}
-
-	pci_read_config_dword(pdev, pdev->rom_base_reg, &rom_reg_orig);
-	pci_write_config_dword(pdev, pdev->rom_base_reg,
-			       rom_reg_orig | PCI_ROM_ADDRESS_ENABLE);
-
-	p = ioremap(pci_resource_start(pdev, PCI_ROM_RESOURCE), (64 * 1024));
-	if (p != NULL && readb(p) == 0x55 && readb(p + 1) == 0xaa)
-		find_eth_addr_in_vpd(p, (64 * 1024), index, dev_addr);
+	size_t size;
+	void __iomem *p = pci_map_rom(pdev, &size);
 
-	if (p != NULL)
-		iounmap(p);
-
-	pci_write_config_dword(pdev, pdev->rom_base_reg, rom_reg_orig);
-	return;
+	if (p) {
+		int index = 0;
+		int found;
+
+		if (is_quattro_p(pdev))
+			index = PCI_SLOT(pdev->devfn);
+
+		found = readb(p) == 0x55 &&
+			readb(p + 1) == 0xaa &&
+			find_eth_addr_in_vpd(p, (64 * 1024), index, dev_addr);
+		pci_unmap_rom(pdev, p);
+		if (found)
+			return;
+	}
 
-use_random:
 	/* Sun MAC prefix then 3 random bytes. */
 	dev_addr[0] = 0x08;
 	dev_addr[1] = 0x00;

Re: sungem driver patch testing..

[David S. Miller]

From: Willy Tarreau <willy@w.ods.org>
Date: Sun, 11 Sep 2005 09:04:07 +0200

> I've ported it to sunhme (which uses the same PCI ROM mapping code).
> Without the patch, I get NULL MAC addresses for all 4 ports (it's a SUN
> QFE). With the patch, I get the correct addresses (the ones printed on
> the label on the card).
> 
> I attach the patch for sunhme, and Cc: Davem so that he updates his tree.

Thanks, I'll put this into my tree along with Linus's SunGEM
stuff and push upstream after I play with it a little bit
myself :-)

Thanks again.

Re: sungem driver patch testing..

[Christoph Hellwig]

On Sat, Sep 10, 2005 at 08:11:22PM -0700, Linus Torvalds wrote:
> 
> I've been grepping around for things that do their own PCI ROM mapping and 
> do it badly, and one thing that matches that description is the sungem 
> ethernet driver on PC's.
> 
> If anybody has such a beast, can you please try this patch and report 
> whether it works for you? 
> 
> 		Linus
> 
> ---
> diff --git a/drivers/net/sungem.c b/drivers/net/sungem.c
> --- a/drivers/net/sungem.c
> +++ b/drivers/net/sungem.c
> @@ -2817,7 +2817,7 @@ static int gem_ioctl(struct net_device *
>  
>  #if (!defined(__sparc__) && !defined(CONFIG_PPC_PMAC))
>  /* Fetch MAC address from vital product data of PCI ROM. */
> -static void find_eth_addr_in_vpd(void __iomem *rom_base, int len, unsigned char *dev_addr)
> +static int find_eth_addr_in_vpd(void __iomem *rom_base, int len, unsigned char *dev_addr)

While we're at it the cpp conditioal looks bogus.  We definitly needs this
when plugging a SUN card into a mac.  I'd suggest compiling this
unconditionally and fall back to it when whatever firmware method to get
the mac address fails.

Re: sungem driver patch testing..

[Linus Torvalds]

On Sun, 11 Sep 2005, Christoph Hellwig wrote:
> 
> While we're at it the cpp conditioal looks bogus.  We definitly needs this
> when plugging a SUN card into a mac.  I'd suggest compiling this
> unconditionally and fall back to it when whatever firmware method to get
> the mac address fails.

Here's a patch (on top of the previous PCI ROM mapping fix) that does
that. It seems to work for me, but I can't really test it, and it's mostly
just cleanup, so I'm not going to apply it.

David can decide to apply it or not as he sees fit (I applied the ROM
mapping fix, since the same fix was already reported to fix a real problem
on the happy-meal ethernet).

David: my G5 doesn't trigger the code, since it finds the MAC address in 
the OF tables. However, if I force that to fail (and add a few printk's), 
I get:

	OF prom reports MAC 00:0d:93:5a:8c:2c
	sungem: no MAC info in OF - falling back to PCI ROM code
	PCI ROM reports MAC 08:00:20:b6:cf:93

so the code seems to work (but because it's an Apple ROM, it doesn't
actually find the magic sequence, so it will have selected a random
address - the three last bytes will change every boot - but the thing does
work..).

Maybe the PCI rom mapping code should report when it just makes up a
random address?

		Linus
----
sungem: fall back on PCI ROM code on PMAC

This falls back to finding the MAC address from the PCI expansion ROM if 
the card information cannot be found in the Mac OF tables.

Signed-off-by: Linus Torvalds <torvalds@osdl.org>
---
diff --git a/drivers/net/sungem.c b/drivers/net/sungem.c
--- a/drivers/net/sungem.c
+++ b/drivers/net/sungem.c
@@ -2815,7 +2815,33 @@ static int gem_ioctl(struct net_device *
 	return rc;
 }
 
-#if (!defined(__sparc__) && !defined(CONFIG_PPC_PMAC))
+/*
+ * On Sparc, we get the MAC address from the Sun prom property tables
+ */
+#ifdef __sparc__
+
+static int __devinit gem_get_device_address(struct gem *gp)
+{
+	struct net_device *dev = gp->dev;
+	struct pci_dev *pdev = gp->pdev;
+	struct pcidev_cookie *pcp = pdev->sysdata;
+	int node = -1;
+
+	if (pcp != NULL) {
+		node = pcp->prom_node;
+		if (prom_getproplen(node, "local-mac-address") == 6)
+			prom_getproperty(node, "local-mac-address",
+					 dev->dev_addr, 6);
+		else
+			node = -1;
+	}
+	if (node == -1)
+		memcpy(dev->dev_addr, idprom->id_ethaddr, 6);
+	return 0;
+}
+
+#else /* Not __sparc__ */
+
 /* Fetch MAC address from vital product data of PCI ROM. */
 static int find_eth_addr_in_vpd(void __iomem *rom_base, int len, unsigned char *dev_addr)
 {
@@ -2866,45 +2892,30 @@ static void get_gem_mac_nonobp(struct pc
 	get_random_bytes(dev_addr + 3, 3);
 	return;
 }
-#endif /* not Sparc and not PPC */
 
+/*
+ * On PPC PMAC, we try the OF first, then fall back to searching
+ * the PCI ROM.
+ */
 static int __devinit gem_get_device_address(struct gem *gp)
 {
-#if defined(__sparc__) || defined(CONFIG_PPC_PMAC)
+#if defined(CONFIG_PPC_PMAC)
 	struct net_device *dev = gp->dev;
-#endif
-
-#if defined(__sparc__)
-	struct pci_dev *pdev = gp->pdev;
-	struct pcidev_cookie *pcp = pdev->sysdata;
-	int node = -1;
-
-	if (pcp != NULL) {
-		node = pcp->prom_node;
-		if (prom_getproplen(node, "local-mac-address") == 6)
-			prom_getproperty(node, "local-mac-address",
-					 dev->dev_addr, 6);
-		else
-			node = -1;
-	}
-	if (node == -1)
-		memcpy(dev->dev_addr, idprom->id_ethaddr, 6);
-#elif defined(CONFIG_PPC_PMAC)
 	unsigned char *addr;
 
 	addr = get_property(gp->of_node, "local-mac-address", NULL);
-	if (addr == NULL) {
-		printk("\n");
-		printk(KERN_ERR "%s: can't get mac-address\n", dev->name);
-		return -1;
+	if (addr) {
+		memcpy(dev->dev_addr, addr, 6);
+		return 0;
 	}
-	memcpy(dev->dev_addr, addr, 6);
-#else
-	get_gem_mac_nonobp(gp->pdev, gp->dev->dev_addr);
+	printk("sungem: no MAC info in OF - falling back to PCI ROM code\n");
 #endif
+	get_gem_mac_nonobp(gp->pdev, gp->dev->dev_addr);
 	return 0;
 }
 
+#endif /* Not sparc */
+
 static void __devexit gem_remove_one(struct pci_dev *pdev)
 {
 	struct net_device *dev = pci_get_drvdata(pdev);

Re: sungem driver patch testing..

[David S. Miller]

From: Linus Torvalds <torvalds@osdl.org>
Date: Sun, 11 Sep 2005 10:07:14 -0700 (PDT)

> On Sun, 11 Sep 2005, Christoph Hellwig wrote:
> > 
> > While we're at it the cpp conditioal looks bogus.  We definitly needs this
> > when plugging a SUN card into a mac.  I'd suggest compiling this
> > unconditionally and fall back to it when whatever firmware method to get
> > the mac address fails.
> 
> Here's a patch (on top of the previous PCI ROM mapping fix) that does
> that.

The Apple firmware actually is the same kind of FORTH firmware the
SUN cards use too.  Apple bought the FORTH firmware technology from
Sun so they could use it in their machines.

Whether it actually _works_ I do not know, but in theory it is very
possible that it does.

Just something to keep in mind. :-)

> Maybe the PCI rom mapping code should report when it just makes up a
> random address?

I agree that it should.

Re: sungem driver patch testing..

[Linus Torvalds]

On Sun, 11 Sep 2005, David S. Miller wrote:
> 
> The Apple firmware actually is the same kind of FORTH firmware the
> SUN cards use too.  Apple bought the FORTH firmware technology from
> Sun so they could use it in their machines.

I think what Christoph was thinking of is when you insert a _regular_ PCI 
card, ie no apple firmware stuff. Now, what OF will do about such a thing, 
I don't know, but I assume it's possible.

		Linus

Re: sungem driver patch testing..

[David S. Miller]

From: Linus Torvalds <torvalds@osdl.org>
Date: Sun, 11 Sep 2005 11:52:59 -0700 (PDT)

> On Sun, 11 Sep 2005, David S. Miller wrote:
> > 
> > The Apple firmware actually is the same kind of FORTH firmware the
> > SUN cards use too.  Apple bought the FORTH firmware technology from
> > Sun so they could use it in their machines.
> 
> I think what Christoph was thinking of is when you insert a _regular_ PCI 
> card, ie no apple firmware stuff. Now, what OF will do about such a thing, 
> I don't know, but I assume it's possible.

Even the PCI SunGEM cards have the FORTH firmware stuff in their PCI
ROMs.  The FORTH firmware on the Sun or MAC machine, at boot time,
looks for the FORTH signature in the PCI ROMs of all the devices in
the machine, and executes whatever matches it finds.  The SunGEM PCI
card variants have this firmware, just like the motherboard variants
do.

I don't think a SunGEM chipset card or motherboard implementation ever
existed without the necessary FORTH firmware in the PCI ROM.  And,
certainly, no SunGEM was ever made with accompanying x86 style
firmware in the PCI ROM.

Hmmm... actually, I think I have a dual-function PCI card somewhere
that has a SunGEM and a Qlogic scsi controller.  If I can dig it up
I'll stick it into one of my x86 boxes and play with your patch :-)

Re: sungem driver patch testing..

[Pascal Schmidt]

On Sun, 11 Sep 2005 19:10:06 +0200, you wrote:

> Here's a patch (on top of the previous PCI ROM mapping fix) that does
> that. It seems to work for me, but I can't really test it, and it's mostly
> just cleanup, so I'm not going to apply it.

I can confirm that it doesn't break the Sun GEM in my iBook G4,
but then that probably also doesn't trigger the changed code.

-- 
Ciao,
Pascal