* [patch] rtl8102e: commit 7bf6bf480 zeroized MAC address @ 2008-10-15 10:11 c4p7n 2008-10-15 12:31 ` Ivan Vecera 0 siblings, 1 reply; 6+ messages in thread From: c4p7n @ 2008-10-15 10:11 UTC (permalink / raw) To: netdev, Francois Romieu, Ivan Vecera; +Cc: kernel, netdev, c4p7n1 [-- Attachment #1: Type: text/plain, Size: 105 bytes --] Tested on top of linus tree and back ported 2.6.27. Please cc me, thanks Martin Capitanio [-- Attachment #2: r8169-01.patch --] [-- Type: message/rfc822, Size: 1193 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] rtl8102e: commit 7bf6bf480 zeroized MAC address 2008-10-15 10:11 [patch] rtl8102e: commit 7bf6bf480 zeroized MAC address c4p7n @ 2008-10-15 12:31 ` Ivan Vecera 0 siblings, 0 replies; 6+ messages in thread From: Ivan Vecera @ 2008-10-15 12:31 UTC (permalink / raw) To: netdev; +Cc: c4p7n, Francois Romieu, kernel c4p7n@capitanio.org wrote: > Tested on top of linus tree and back ported 2.6.27. > > Please cc me, thanks > Martin Capitanio > I think this one is more common. Ivan ===== Subject: [PATCH] [r8169] initialize MAC address found in EEPROM only if is it valid Signed-off-by: Ivan Vecera <ivecera@redhat.com> --- drivers/net/r8169.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c index c821da2..587a96c 100644 --- a/drivers/net/r8169.c +++ b/drivers/net/r8169.c @@ -1975,8 +1975,9 @@ static void rtl_init_mac_address(struct rtl8169_private *tp, dprintk("MAC address found in EEPROM: %s\n", print_mac(buf, mac)); - /* Write MAC address */ - rtl_rar_set(tp, mac); + /* Write MAC address if is it valid */ + if (is_valid_ether_addr(mac)) + rtl_rar_set(tp, mac); } static int __devinit -- 1.5.6.3 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [patch] rtl8102e: commit 7bf6bf480 zeroized MAC address @ 2008-10-15 13:57 c4p7n 2008-10-15 22:50 ` Francois Romieu 0 siblings, 1 reply; 6+ messages in thread From: c4p7n @ 2008-10-15 13:57 UTC (permalink / raw) To: Ivan Vecera, netdev; +Cc: Francois Romieu, kernel >From: Ivan Vecera<ivecera@redhat.com> > c4p7n@capitanio.org wrote: > > Tested on top of linus tree and back ported 2.6.27. > > > > Please cc me, thanks > > Martin Capitanio > > > I think this one is more common. What about to mix both? If the address is not stored here than it must be somewhere else -- or the read algorithm is broken ;-) May be we are reading just by luck zeros and not a random perfectly sane address. Personally I would prefer in this fine driver, just not doing what is not absolutely needed. It would be great to figure out if all of the rtl8102e, etc. doesn't have this location. You can hardly desolder those chips from notebooks and I am really happy if this _just works_ with as much realtek blessing as possible. Martin P.S. Apologize, I'm forced to use this * webmailer. > > Ivan > > ===== > Subject: [PATCH] [r8169] initialize MAC address found in EEPROM only if is > it valid > > Signed-off-by: Ivan Vecera <ivecera@redhat.com> > --- > drivers/net/r8169.c | 5 +++-- > 1 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c > index c821da2..587a96c 100644 > --- a/drivers/net/r8169.c > +++ b/drivers/net/r8169.c > @@ -1975,8 +1975,9 @@ static void rtl_init_mac_address(struct > rtl8169_private *tp, > > dprintk("MAC address found in EEPROM: %s\n", print_mac(buf, mac)); > > - /* Write MAC address */ > - rtl_rar_set(tp, mac); > + /* Write MAC address if is it valid */ > + if (is_valid_ether_addr(mac)) > + rtl_rar_set(tp, mac); > } > > static int __devinit > -- > 1.5.6.3 > --- original message end ---- ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] rtl8102e: commit 7bf6bf480 zeroized MAC address 2008-10-15 13:57 c4p7n @ 2008-10-15 22:50 ` Francois Romieu 2008-10-16 11:19 ` Martin Capitanio 2008-10-21 11:15 ` Ivan Vecera 0 siblings, 2 replies; 6+ messages in thread From: Francois Romieu @ 2008-10-15 22:50 UTC (permalink / raw) To: c4p7n; +Cc: Ivan Vecera, netdev, kernel [-- Attachment #1: Type: text/plain, Size: 585 bytes --] c4p7n@capitanio.org <c4p7n@capitanio.org> : > May be we are reading just by luck zeros and not a random perfectly > sane address. I would use the attached patches. The former one is mostly Ivan's one with some rework of the debug messages. The later checks the eeprom for its signature. With these patches the driver notices that it can not get an address for my 8168b but it works ok with my 8169 after both an hot and a cold boot. Interestingly enough, the VPD is not enabled on the 8169 during the first boot. Ivan, can you test them (and fix them afterwards :o) ) ? -- Ueimor [-- Attachment #2: a --] [-- Type: text/plain, Size: 1346 bytes --] diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c index c821da2..e80ca26 100644 --- a/drivers/net/r8169.c +++ b/drivers/net/r8169.c @@ -1947,11 +1947,11 @@ static void rtl_init_mac_address(struct rtl8169_private *tp, u8 cfg1; int vpd_cap; u8 mac[8]; - DECLARE_MAC_BUF(buf); cfg1 = RTL_R8(Config1); if (!(cfg1 & VPD)) { - dprintk("VPD access not enabled, enabling\n"); + if (netif_msg_probe(tp)) + dev_info(&pdev->dev, "VPD access disabled, enabling\n"); RTL_W8(Cfg9346, Cfg9346_Unlock); RTL_W8(Config1, cfg1 | VPD); RTL_W8(Cfg9346, Cfg9346_Lock); @@ -1969,14 +1969,22 @@ static void rtl_init_mac_address(struct rtl8169_private *tp, */ if (rtl_eeprom_read(pdev, vpd_cap, 0x000e, (__le32*)&mac[0]) < 0 || rtl_eeprom_read(pdev, vpd_cap, 0x0012, (__le32*)&mac[4]) < 0) { - dprintk("Reading MAC address from EEPROM failed\n"); + if (netif_msg_probe(tp)) { + dev_warn(&pdev->dev, + "reading MAC address from EEPROM failed\n"); + } return; } - dprintk("MAC address found in EEPROM: %s\n", print_mac(buf, mac)); + if (netif_msg_probe(tp)) { + DECLARE_MAC_BUF(buf); + + dev_info(&pdev->dev, "MAC address found in EEPROM: %s\n", + print_mac(buf, mac)); + } - /* Write MAC address */ - rtl_rar_set(tp, mac); + if (is_valid_ether_addr(mac)) + rtl_rar_set(tp, mac); } static int __devinit [-- Attachment #3: b --] [-- Type: text/plain, Size: 2056 bytes --] diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c index c821da2..f26ef24 100644 --- a/drivers/net/r8169.c +++ b/drivers/net/r8169.c @@ -1944,23 +1944,34 @@ static void rtl_init_mac_address(struct rtl8169_private *tp, void __iomem *ioaddr) { struct pci_dev *pdev = tp->pci_dev; - u8 cfg1; int vpd_cap; + __le32 sig; u8 mac[8]; - DECLARE_MAC_BUF(buf); + u8 cfg1; cfg1 = RTL_R8(Config1); - if (!(cfg1 & VPD)) { - dprintk("VPD access not enabled, enabling\n"); - RTL_W8(Cfg9346, Cfg9346_Unlock); - RTL_W8(Config1, cfg1 | VPD); - RTL_W8(Cfg9346, Cfg9346_Lock); + if (netif_msg_probe(tp)) { + dev_info(&pdev->dev, "VPD access %sadvertised.\n", + (cfg1 & VPD) ? "" : "not "); } + RTL_W8(Cfg9346, Cfg9346_Unlock); + RTL_W8(Config1, cfg1 | VPD); + RTL_W8(Cfg9346, Cfg9346_Lock); + vpd_cap = pci_find_capability(pdev, PCI_CAP_ID_VPD); if (!vpd_cap) return; + if (rtl_eeprom_read(pdev, vpd_cap, 0x0000, &sig) < 0) + return; + + if ((sig & 0xffff) != 0x8129) { + dev_info(&pdev->dev, "Missing EEPROM signature: %04x\n", + sig & 0xffff); + return; + } + /* MAC address is stored in EEPROM at offset 0x0e * Realtek says: "The VPD address does not have to be a DWORD-aligned * address as defined in the PCI 2.2 Specifications, but the VPD data @@ -1969,14 +1980,22 @@ static void rtl_init_mac_address(struct rtl8169_private *tp, */ if (rtl_eeprom_read(pdev, vpd_cap, 0x000e, (__le32*)&mac[0]) < 0 || rtl_eeprom_read(pdev, vpd_cap, 0x0012, (__le32*)&mac[4]) < 0) { - dprintk("Reading MAC address from EEPROM failed\n"); + if (netif_msg_probe(tp)) { + dev_warn(&pdev->dev, + "reading MAC address from EEPROM failed\n"); + } return; } - dprintk("MAC address found in EEPROM: %s\n", print_mac(buf, mac)); + if (netif_msg_probe(tp)) { + DECLARE_MAC_BUF(buf); + + dev_info(&pdev->dev, "MAC address found in EEPROM: %s\n", + print_mac(buf, mac)); + } - /* Write MAC address */ - rtl_rar_set(tp, mac); + if (is_valid_ether_addr(mac)) + rtl_rar_set(tp, mac); } static int __devinit ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [patch] rtl8102e: commit 7bf6bf480 zeroized MAC address 2008-10-15 22:50 ` Francois Romieu @ 2008-10-16 11:19 ` Martin Capitanio 2008-10-21 11:15 ` Ivan Vecera 1 sibling, 0 replies; 6+ messages in thread From: Martin Capitanio @ 2008-10-16 11:19 UTC (permalink / raw) To: Francois Romieu; +Cc: Ivan Vecera, netdev, kernel Am Donnerstag, den 16.10.2008, 00:50 +0200 schrieb Francois Romieu: > c4p7n@capitanio.org <c4p7n@capitanio.org> : > > May be we are reading just by luck zeros and not a random perfectly > > sane address. > > I would use the attached patches. The former one is mostly Ivan's one > with some rework of the debug messages. The later checks the eeprom > for its signature. [ 8087.008071] r8169 Gigabit Ethernet driver 2.3LK-NAPI loaded [ 8087.008108] r8169 0000:02:00.0: PCI INT A -> GSI 16 (level, low) -> IRQ 16 [ 8087.008120] r8169 0000:02:00.0: enabling Mem-Wr-Inval [ 8087.008139] r8169 0000:02:00.0: enabling bus mastering [ 8087.008149] r8169 0000:02:00.0: setting latency timer to 64 [ 8087.008284] r8169 0000:02:00.0: VPD access advertised. [ 8087.008620] r8169 0000:02:00.0: Missing EEPROM signature: 0000 [ 8087.009390] eth0: RTL8102e at 0xffffc20000042000,, XID 34a00000 IRQ 316 [ 8091.040800] r8169: eth0: link up Thanks, Martin > > With these patches the driver notices that it can not get an address > for my 8168b but it works ok with my 8169 after both an hot and a > cold boot. Interestingly enough, the VPD is not enabled on the 8169 > during the first boot. > > Ivan, can you test them (and fix them afterwards :o) ) ? > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] rtl8102e: commit 7bf6bf480 zeroized MAC address 2008-10-15 22:50 ` Francois Romieu 2008-10-16 11:19 ` Martin Capitanio @ 2008-10-21 11:15 ` Ivan Vecera 1 sibling, 0 replies; 6+ messages in thread From: Ivan Vecera @ 2008-10-21 11:15 UTC (permalink / raw) To: Francois Romieu; +Cc: c4p7n, netdev, kernel Francois Romieu wrote: > c4p7n@capitanio.org <c4p7n@capitanio.org> : >> May be we are reading just by luck zeros and not a random perfectly >> sane address. > > I would use the attached patches. The former one is mostly Ivan's one > with some rework of the debug messages. The later checks the eeprom > for its signature. > > With these patches the driver notices that it can not get an address > for my 8168b but it works ok with my 8169 after both an hot and a > cold boot. Interestingly enough, the VPD is not enabled on the 8169 > during the first boot. > > Ivan, can you test them (and fix them afterwards :o) ) ? No problem found. Both 8168c (GIGA_MAC_VER_20) and 8102el (GIGA_MAC_VER_09) successfully loads MAC address from EEPROM (from power-off state and also after reboot). Ivan ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2008-10-21 11:15 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-10-15 10:11 [patch] rtl8102e: commit 7bf6bf480 zeroized MAC address c4p7n 2008-10-15 12:31 ` Ivan Vecera -- strict thread matches above, loose matches on Subject: below -- 2008-10-15 13:57 c4p7n 2008-10-15 22:50 ` Francois Romieu 2008-10-16 11:19 ` Martin Capitanio 2008-10-21 11:15 ` Ivan Vecera
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).