[email protected] wrote:
> From: Michael Pyne <[email protected]>
>
> 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 <[email protected]>
> Cc: Jeff Garzik <[email protected]>
> Cc: Ayaz Abdulla <[email protected]>
> Cc: <[email protected]>
>
> On Wed, 21 Nov 2007 15:34:52 -0800
> "Ayaz Abdulla" <[email protected]> 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 <[email protected]>
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
Jeff Garzik wrote:
>>
>> Signed-off-by: Andrew Morton <[email protected]>
>
> 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
H. Peter Anvin wrote:
> Jeff Garzik wrote:
>>>
>>> Signed-off-by: Andrew Morton <[email protected]>
>>
>> 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
On Tue, 05 Feb 2008 13:20:59 -0500 Jeff Garzik <[email protected]> wrote:
> > Signed-off-by: Andrew Morton <[email protected]>
>
> 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!
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;
Ayaz Abdulla wrote:
> Andrew Morton wrote:
>> On Tue, 05 Feb 2008 13:20:59 -0500 Jeff Garzik <[email protected]> wrote:
>>
>>
>>>> Signed-off-by: Andrew Morton <[email protected]>
>>>
>>> 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
Andrew Morton wrote:
> On Tue, 05 Feb 2008 13:20:59 -0500 Jeff Garzik <[email protected]> wrote:
>
>
>>>Signed-off-by: Andrew Morton <[email protected]>
>>
>>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.
-----------------------------------------------------------------------------------
Jeff Garzik wrote:
> Ayaz Abdulla wrote:
>
>> Andrew Morton wrote:
>>
>>> On Tue, 05 Feb 2008 13:20:59 -0500 Jeff Garzik <[email protected]> wrote:
>>>
>>>
>>>>> Signed-off-by: Andrew Morton <[email protected]>
>>>>
>>>>
>>>> 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.
-----------------------------------------------------------------------------------
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
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