[patch] rtl8102e: commit 7bf6bf480 zeroized MAC address

[patch] rtl8102e: commit 7bf6bf480 zeroized MAC address

[c4p7n]

[-- 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 --]

Re: [patch] rtl8102e: commit 7bf6bf480 zeroized MAC address

[Ivan Vecera]

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

Re: [patch] rtl8102e: commit 7bf6bf480 zeroized MAC address

[c4p7n]

>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 ----

Re: [patch] rtl8102e: commit 7bf6bf480 zeroized MAC address

[Francois Romieu]

[-- 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

Re: [patch] rtl8102e: commit 7bf6bf480 zeroized MAC address

[Martin Capitanio]

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) ) ?
> 

Re: [patch] rtl8102e: commit 7bf6bf480 zeroized MAC address

[Ivan Vecera]

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