sunlance probing problem

PATCH In review Krzysztof Helt 3 participants 6 messages Aug 11, 2006 – Aug 18, 2006 Network drivers

lore

Share & tools

lore

Krzysztof Helt

author

2006-08-11 22:03 UTC #0 lore raw

PATCH 1/1 sunlance probing problem

From: Krzysztof Helt ([email protected])

The patch removes the "le" device from matching list of probed devices.

Signed-off-by Krzysztof Helt ([email protected])

---
The sunlance driver allows probing of three devices: le, ledma and lebuffer. The "le" device is a child entry for either ledma or lebuffer devices. Thus, each Ethernet adapter is probed twice and registered as two Ethernet adapters.

I tested it on SS20 and 2.6.18-rc4 kernel.

----------------------------------------------------
Czy 10 minut rozmowy = ponad 11 zł oszczędności ?
Sprawdź: http://klik.wp.pl/?adr=http%3A%2F%2Ftelefon.wp.pl%2Foszczednosci.html&sid=842

drivers/net/sunlance.c +1 -6

diff -ur linux-2.6.17/drivers/net/sunlance.c linux-new/drivers/net/sunlance.c
@@ -1569,9 +1569,7 @@
 	struct device_node *dp = dev->node;
 	int err;
 
-	if (!strcmp(dp->name, "le")) {
-		err = sparc_lance_probe_one(sdev, NULL, NULL);
-	} else if (!strcmp(dp->name, "ledma")) {
+	if (!strcmp(dp->name, "ledma")) {
 		struct sbus_dma *ledma = find_ledma(sdev);
 
 		err = sparc_lance_probe_one(sdev->child, ledma, NULL);

@@ -1602,9 +1600,6 @@
 
 static struct of_device_id sunlance_sbus_match[] = {
 	{
-		.name = "le",
-	},
-	{
 		.name = "ledma",
 	},
 	{

Eric Brower

2006-08-11 23:11 UTC #1 lore raw

The "le" is not always a child of lebuffer or ledma. Attached is output from an SS2, where le hangs directly off sbus. Granted, this is an older "le" implementation.

This data was obtained from the old vger prtconf/examples repo. I'm not sure if this is still hosted somewhere, but I found it to be a very good reference.

Thanks,
E

Quoted 22 lines · Krzysztof Helt

From: Krzysztof Helt ([email protected])
The patch removes the "le" device from matching list of probed devices.
Signed-off-by Krzysztof Helt ([email protected])
---
The sunlance driver allows probing of three devices: le, ledma and lebuffer. The "le" device is a child entry for either ledma or lebuffer devices. Thus, each Ethernet adapter is probed twice and registered as two Ethernet adapters.
I tested it on SS20 and 2.6.18-rc4 kernel.
----------------------------------------------------
Czy 10 minut rozmowy = ponad 11 zł oszczędności ?
Sprawdź: http://klik.wp.pl/?adr=http%3A%2F%2Ftelefon.wp.pl%2Foszczednosci.html&sid=842

Signature · 1 line

David Miller

maintainer

2006-08-11 23:27 UTC #2 lore raw

In reply to Krzysztof Helt

From: "Eric Brower" <[email protected]>
Date: Fri, 11 Aug 2006 16:11:49 -0700

Quoted 3 lines

This data was obtained from the old vger prtconf/examples repo. I'm not sure if this is still hosted somewhere, but I found it to be a very good reference.

It's present in the sources of the sparc-utils package that most sparc dists ship.

Krzysztof Helt

author

2006-08-12 13:20 UTC #3 lore raw

In reply to Krzysztof Helt

Dnia 12-08-2006 o godz. 1:11 Eric Brower napisał(a):

Quoted 2 lines

The "le" is not always a child of lebuffer or ledma. Attached is output from an SS2, where le hangs directly off sbus. Granted,

this

Quoted 2 lines

is an older "le" implementation.

Thank you for the hint. This is a corrected patch which checks if the "le" is child of ledma or lebuffer.

From: Krzysztof Helt ([email protected])

The patch skips probing of the "le" device if it is a child of "ledma" or "lebuffer".

Signed-off-by Krzysztof Helt ([email protected])

---
I tested it on SS20 and 2.6.18-rc4 kernel.

----------------------------------------------------
Czy 10 minut rozmowy = ponad 11 zł oszczędności ?
Sprawdź: http://klik.wp.pl/?adr=http%3A%2F%2Ftelefon.wp.pl%2Foszczednosci.html&sid=842

drivers/net/sunlance.c +11 -1

diff -upr linux-2.6.17/drivers/net/sunlance.c linux-new/drivers/net/sunlance.c
@@ -1570,7 +1570,17 @@ static int __devinit sunlance_sbus_probe
 	int err;
 
 	if (!strcmp(dp->name, "le")) {
-		err = sparc_lance_probe_one(sdev, NULL, NULL);
+		if (sdev->parent) {
+			struct of_device *parent = &sdev->parent->ofdev;
+
+			if (!strcmp(parent->node->name, "ledma") ||
+			    !strcmp(parent->node->name, "lebuffer")) {
+				/* this is a child of already probed device */
+				err = 0;
+			} else
+				err = sparc_lance_probe_one(sdev->child, NULL, NULL);
+		} else
+			err = sparc_lance_probe_one(sdev->child, NULL, NULL);
 	} else if (!strcmp(dp->name, "ledma")) {
 		struct sbus_dma *ledma = find_ledma(sdev);

David Miller

maintainer

2006-08-18 01:10 UTC #4 lore raw

In reply to Krzysztof Helt

From: "Krzysztof Helt" <[email protected]>
Date: Sat, 12 Aug 2006 15:20:55 +0200

Quoted 4 lines

The patch skips probing of the "le" device if it is a child of "ledma" or "lebuffer".
Signed-off-by Krzysztof Helt ([email protected])

If you return "0" as the error, the caller in the device probing layer grabs a reference to the device which will be a leak of the undelying of_device object.

A better way to fix this is probably to only match "le" and have the probe code check the device parent for "ledma" or "lebuffer". You should be able to confine all of your changes to the probe match table and sunlance_sbus_probe().

Could you implement it like that and give it a test?

Thanks.

Krzysztof Helt

author

2006-08-18 17:32 UTC #5 lore raw

In reply to Krzysztof Helt

Dnia 18-08-2006 o godz. 3:10 David Miller napisał(a):

Quoted 1 line

If you return "0" as the error, the caller in the device

probing layer

Quoted 1 line

grabs a reference to the device which will be a leak of the

undelying

Quoted 11 lines

of_device object.
A better way to fix this is probably to only match "le" and have the probe code check the device parent for "ledma" or "lebuffer". You should be able to confine all of your changes to the probe match table and sunlance_sbus_probe().
Could you implement it like that and give it a test?
Thanks.

This is a tested patch according to your sugestions (as far as I understood them).

Regards,
Krzysztof
PS. Forget the previous patch. It is incorrect if "le" is standalone.

----------------------------------------------------
Najnowsze odkrycie światowej muzyki- smacznej i snobistycznej. Wschodząca gwiazda, wybitnie utalentowana Sophie Solomon wraz z zespołem w Warszawie!!
http://klik.wp.pl/?adr=http%3A%2F%2Fadv.reklama.wp.pl%2Fas%2Fsophie_solomon_w_fabryce_trzciny.html&sid=846

drivers/net/sunlance.c +11 ~~-16~~

@@ -1566,20 +1566,21 @@ static int __exit sunlance_sun4_remove(v
 static int __devinit sunlance_sbus_probe(struct of_device *dev, const struct of_device_id *match)
 {
 	struct sbus_dev *sdev = to_sbus_device(&dev->dev);
-	struct device_node *dp = dev->node;
 	int err;
 
-	if (!strcmp(dp->name, "le")) {
-		err = sparc_lance_probe_one(sdev, NULL, NULL);
-	} else if (!strcmp(dp->name, "ledma")) {
-		struct sbus_dma *ledma = find_ledma(sdev);
+	if (sdev->parent) {
+		struct of_device *parent = &sdev->parent->ofdev;
 
-		err = sparc_lance_probe_one(sdev->child, ledma, NULL);
-	} else {
-		BUG_ON(strcmp(dp->name, "lebuffer"));
+		if (!strcmp(parent->node->name, "ledma")) {
+			struct sbus_dma *ledma = find_ledma(to_sbus_device(&parent->dev));
 
-		err = sparc_lance_probe_one(sdev->child, NULL, sdev);
-	}
+			err = sparc_lance_probe_one(sdev, ledma, NULL);
+		} else if (!strcmp(parent->node->name, "lebuffer")) {
+			err = sparc_lance_probe_one(sdev, NULL, to_sbus_device(&parent->dev));
+		} else
+			err = sparc_lance_probe_one(sdev, NULL, NULL);
+	} else
+		err = sparc_lance_probe_one(sdev, NULL, NULL);
 
 	return err;
 }

@@ -1604,12 +1605,6 @@ static struct of_device_id sunlance_sbus
 	{
 		.name = "le",
 	},
-	{
-		.name = "ledma",
-	},
-	{
-		.name = "lebuffer",
-	},
 	{},
 };