Re: [patch 2/4] forcedeth: fix MAC address detection on network card (regression in 2.6.23)

Re: [patch 2/4] forcedeth: fix MAC address detection on network card (regression in 2.6.23)

[Jeff Garzik]

akpm@linux-foundation.org wrote:
> From: Michael Pyne <michael.pyne@kdemail.net>
> 
> Partially revert a change to mac address detection introduced to the forcedeth
> driver.  The change was intended to correct mac address detection for newer
> nVidia chipsets where the mac address was stored in reverse order.  One of
> those chipsets appears to still have the mac address in reverse order (or at
> least, it does on my system).
> 
> The change that broke mac address detection for my card was commit
> ef756b3e56c68a4d76d9d7b9a73fa8f4f739180f "forcedeth: mac address correct"
> 
> My network card is an nVidia built-in Ethernet card, output from lspci as
> follows (with text and numeric ids):
> $ lspci | grep Ethernet
> 00:07.0 Bridge: nVidia Corporation MCP61 Ethernet (rev a2)
> $ lspci -n | grep 07.0
> 00:07.0 0680: 10de:03ef (rev a2)
> 
> The vendor id is, of course, nVidia.  The device id corresponds to the
> NVIDIA_NVENET_19 entry.
> 
> The included patch fixes the MAC address detection on my system.
> Interestingly, the MAC address appears to be in the range reserved for my
> motherboard manufacturer (Gigabyte) and not nVidia.
> 
> Signed-off-by: Michael J. Pyne <michael.pyne@kdemail.net>
> Cc: Jeff Garzik <jeff@garzik.org>
> Cc: Ayaz Abdulla <aabdulla@nvidia.com>
> Cc: <stable@kernel.org>
> 
> On Wed, 21 Nov 2007 15:34:52 -0800
> "Ayaz Abdulla" <AAbdulla@nvidia.com> wrote:
> 
>> The solution is to get the OEM to update their BIOS (instead of
>> integrating this patch) since the MCP61 specs indicate that the MAC
>> Address should be in correct order from BIOS.
>>
>> By changing the feature DEV_HAS_CORRECT_MACADDR to all MCP61 boards, it
>> could cause it to break on other OEM systems who have implemented it
>> correctly.
>>
> 
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

NAK - this fixes one set of users, and breaks a working set of users.

Need to add DMI check for the specific motherboard (dmi_check_system), 
and flip flag according to success/failure of that check.

	Jeff

Re: [patch 2/4] forcedeth: fix MAC address detection on network card (regression in 2.6.23)

[H. Peter Anvin]

Jeff Garzik wrote:
>>
>> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> 
> NAK - this fixes one set of users, and breaks a working set of users.
> 
> Need to add DMI check for the specific motherboard (dmi_check_system), 
> and flip flag according to success/failure of that check.
> 

Either that, or detect the Gigabyte prefix used as a suffix, since they 
seem to be the source of this particular f*ckup.

	-hpa

Re: [patch 2/4] forcedeth: fix MAC address detection on network card (regression in 2.6.23)

[Jeff Garzik]

H. Peter Anvin wrote:
> Jeff Garzik wrote:
>>>
>>> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
>>
>> NAK - this fixes one set of users, and breaks a working set of users.
>>
>> Need to add DMI check for the specific motherboard (dmi_check_system), 
>> and flip flag according to success/failure of that check.
>>
> 
> Either that, or detect the Gigabyte prefix used as a suffix, since they 
> seem to be the source of this particular f*ckup.

Indeed, that would work too...  though we would need to put out a call 
for Gigabyte testers during 2.6.25-rc.

It is an entirely reasonable scenario for NVIDIA to deploy a fix to 
Gigabyte, which would then return us to the same scenario we have today: 
  some work and some don't.

That's my main reason for leaning more conservative here.

	Jeff

Re: [patch 2/4] forcedeth: fix MAC address detection on network card (regression in 2.6.23)

[H. Peter Anvin]

Jeff Garzik wrote:
> 
> Indeed, that would work too...  though we would need to put out a call 
> for Gigabyte testers during 2.6.25-rc.
> 
> It is an entirely reasonable scenario for NVIDIA to deploy a fix to 
> Gigabyte, which would then return us to the same scenario we have today: 
>  some work and some don't.
> 
> That's my main reason for leaning more conservative here.
> 

If they deploy a fix, then the Gigabyte prefix will show up as a prefix, 
rather than a suffix - no problem.  That's actually the reason to deploy 
that solution rather than relying on DMI.

	-hpa

Re: [patch 2/4] forcedeth: fix MAC address detection on network card (regression in 2.6.23)

[Andrew Morton]

On Tue, 05 Feb 2008 13:20:59 -0500 Jeff Garzik <jeff@garzik.org> wrote:

> > Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> 
> NAK - this fixes one set of users, and breaks a working set of users.
> 
> Need to add DMI check for the specific motherboard (dmi_check_system), 
> and flip flag according to success/failure of that check.

OK :)  I added the above to the changelog for next time.

You guys can hide, but this patch isn't going away!

Re: [patch 2/4] forcedeth: fix MAC address detection on network card (regression in 2.6.23)

[Ayaz Abdulla]

Andrew Morton wrote:
> On Tue, 05 Feb 2008 13:20:59 -0500 Jeff Garzik <jeff@garzik.org> wrote:
> 
> 
>>>Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
>>
>>NAK - this fixes one set of users, and breaks a working set of users.
>>
>>Need to add DMI check for the specific motherboard (dmi_check_system), 
>>and flip flag according to success/failure of that check.
> 
> 
> OK :)  I added the above to the changelog for next time.
> 
> You guys can hide, but this patch isn't going away!

I believe Michael determined that a newer BIOS fixes this issue.

-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

Re: [patch 2/4] forcedeth: fix MAC address detection on network card (regression in 2.6.23)

[Jeff Garzik]

Ayaz Abdulla wrote:
> Andrew Morton wrote:
>> On Tue, 05 Feb 2008 13:20:59 -0500 Jeff Garzik <jeff@garzik.org> wrote:
>>
>>
>>>> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
>>>
>>> NAK - this fixes one set of users, and breaks a working set of users.
>>>
>>> Need to add DMI check for the specific motherboard 
>>> (dmi_check_system), and flip flag according to success/failure of 
>>> that check.
>>
>>
>> OK :)  I added the above to the changelog for next time.
>>
>> You guys can hide, but this patch isn't going away!
> 
> I believe Michael determined that a newer BIOS fixes this issue.


That's a solution that makes vendors happy... but we still have to deal 
with it in Linux.  There are plenty of the old broken BIOS still out in 
the field...

	Jeff

Re: [patch 2/4] forcedeth: fix MAC address detection on network card (regression in 2.6.23)

[Ayaz Abdulla]

Jeff Garzik wrote:
> Ayaz Abdulla wrote:
> 
>> Andrew Morton wrote:
>>
>>> On Tue, 05 Feb 2008 13:20:59 -0500 Jeff Garzik <jeff@garzik.org> wrote:
>>>
>>>
>>>>> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
>>>>
>>>>
>>>> NAK - this fixes one set of users, and breaks a working set of users.
>>>>
>>>> Need to add DMI check for the specific motherboard 
>>>> (dmi_check_system), and flip flag according to success/failure of 
>>>> that check.
>>>
>>>
>>>
>>> OK :)  I added the above to the changelog for next time.
>>>
>>> You guys can hide, but this patch isn't going away!
>>
>>
>> I believe Michael determined that a newer BIOS fixes this issue.
> 
> 
> 
> That's a solution that makes vendors happy... but we still have to deal 
> with it in Linux.  There are plenty of the old broken BIOS still out in 
> the field...
> 
>     Jeff
> 
> 

Michael, can you provide which BIOS version had this issue and which 
version fixed the issue?



-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

Re: [patch 2/4] forcedeth: fix MAC address detection on network card (regression in 2.6.23)

[Michael Pyne]

[-- Attachment #1: Type: text/plain, Size: 1163 bytes --]

On Monday 04 February 2008, Ayaz Abdulla wrote:
> Jeff Garzik wrote:
> > Ayaz Abdulla wrote:
> >> I believe Michael determined that a newer BIOS fixes this issue.
> >
> > That's a solution that makes vendors happy... but we still have to deal
> > with it in Linux.  There are plenty of the old broken BIOS still out in
> > the field...
> >
> >     Jeff
>
> Michael, can you provide which BIOS version had this issue and which
> version fixed the issue?

Ayaz,

One of my earlier messages to the list was from BIOS revision F3 from what I 
can tell (which matches pretty well with what I remember having).  I am 
currently on F8.

I may go back to F3 if I can get booting from USB to work just to verify 
because I could have sworn it was still broken after going to F8.  But since 
unpatched Linux 2.6.23.12 apparently works fine and I'm not sure when exactly 
that happened (I use Ketchup to maintain the sources and somewhere it 
unpatched my forcedeth.c :) I want to double-check that a simple BIOS upgrade 
will solve it.

But I also don't have a lot of time before I go underway for a few months. :-/

Regards,
 - Michael Pyne

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [patch 2/4] forcedeth: fix MAC address detection on network card (regression in 2.6.23)

[Jeff Garzik]

[-- Attachment #1: Type: text/plain, Size: 619 bytes --]

Andrew Morton wrote:
> On Tue, 05 Feb 2008 13:20:59 -0500 Jeff Garzik <jeff@garzik.org> wrote:
> 
>>> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
>> NAK - this fixes one set of users, and breaks a working set of users.
>>
>> Need to add DMI check for the specific motherboard (dmi_check_system), 
>> and flip flag according to success/failure of that check.
> 
> OK :)  I added the above to the changelog for next time.
> 
> You guys can hide, but this patch isn't going away!

In the original thread, I even posted a 
fill-in-the-blanks-with-the-motherboard-name patch.  I'll attach it again...

	Jeff




[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 1459 bytes --]

diff --git a/drivers/net/forcedeth.c b/drivers/net/forcedeth.c
index 3634223..eab85fa 100644
--- a/drivers/net/forcedeth.c
+++ b/drivers/net/forcedeth.c
@@ -147,6 +147,7 @@
 #include <linux/init.h>
 #include <linux/if_vlan.h>
 #include <linux/dma-mapping.h>
+#include <linux/dmi.h>
 
 #include <asm/irq.h>
 #include <asm/io.h>
@@ -4992,6 +4993,26 @@ static int nv_close(struct net_device *dev)
 	return 0;
 }
 
+static int nv_reversed_macaddr(void)
+{
+	static const struct dmi_system_id sysids[] = {
+		{
+			.ident = "",
+			.matches = {
+				DMI_MATCH(DMI_SYS_VENDOR, ""),
+				DMI_MATCH(DMI_PRODUCT_NAME, ""),
+			},
+		},
+
+		{ }	/* terminate list */
+	};
+
+	if (dmi_check_system(sysids))
+		return 1;
+	
+	return 0;
+}
+
 static int __devinit nv_probe(struct pci_dev *pci_dev, const struct pci_device_id *id)
 {
 	struct net_device *dev;
@@ -5187,8 +5208,9 @@ static int __devinit nv_probe(struct pci_dev *pci_dev, const struct pci_device_i
 
 	/* check the workaround bit for correct mac address order */
 	txreg = readl(base + NvRegTransmitPoll);
-	if ((txreg & NVREG_TRANSMITPOLL_MAC_ADDR_REV) ||
-	    (id->driver_data & DEV_HAS_CORRECT_MACADDR)) {
+	if (((txreg & NVREG_TRANSMITPOLL_MAC_ADDR_REV) ||
+	    (id->driver_data & DEV_HAS_CORRECT_MACADDR))) &&
+	    (!nv_reversed_macaddr())) {
 		/* mac address is already in correct order */
 		dev->dev_addr[0] = (np->orig_mac[0] >>  0) & 0xff;
 		dev->dev_addr[1] = (np->orig_mac[0] >>  8) & 0xff;