re(4) MAC address

[Frank Wille]

Hi,

I was testing a NH-230/231 NAS (running sandpoint) and wondered why
the PPCBoot firmware and the previously installed Linux used a different
MAC address than NetBSD does.

I found out that NetBSD's re(4) driver is reading the MAC from EEPROM
while PPCBoot and Linux are reading it from the chip's ID-registers
(RTK_IDRn).

What is correct? This is a Realtek 8169S:

# pcictl pci0 dump -d 15
PCI configuration registers:
Common header:
0x00: 0x816910ec 0x02b00107 0x02000010 0x00008008

Vendor Name: Realtek Semiconductor (0x10ec)
Device Name: 8169/8110 10/100/1000 Ethernet (0x8169)
[...]


Sorry for cross-posting, but I couldn't decide whether this belongs to
tech-kern or tech-net.

--
Frank Wille

[Izumi Tsutsui]

> I found out that NetBSD's re(4) driver is reading the MAC from EEPROM
> while PPCBoot and Linux are reading it from the chip's ID-registers
> (RTK_IDRn).
>
> What is correct? This is a Realtek 8169S:

Probably it's defined by hardware vendors, not chip.

The old RTL8139 (RTL8169 has compat mode) seems to read MAC address
from EEPROM and those values are stored into RTK_IDRn registers.
I guess some NAS vendors overwrite RTK_IDn registers by firmware
to avoid extra EEPROM configurations during production.

We can change values per hardware by adding device properties
(prop_dictionary(3)) calls (like sys/dev/pci/if_wm.c etc).

---
Izumi Tsutsui

[Frank Wille]

Izumi Tsutsui wrote:

On 02.12.12 00:40:44 you wrote:

>> I found out that NetBSD's re(4) driver is reading the MAC from EEPROM
>> while PPCBoot and Linux are reading it from the chip's ID-registers
>> (RTK_IDRn).
>>
>> What is correct? This is a Realtek 8169S:
>
> Probably it's defined by hardware vendors, not chip.
>
> The old RTL8139 (RTL8169 has compat mode) seems to read MAC address
> from EEPROM and those values are stored into RTK_IDRn registers.

Who writes it into the IDRn registers? The firmware? The driver? Or the chip
itself? When the chip does that automatically, then re(4) should depend on
RTK_IDRn and not on the EEPROM.

> I guess some NAS vendors overwrite RTK_IDn registers by firmware
> to avoid extra EEPROM configurations during production.

You may be right. I found a modification in the PPCBoot source, which reads
the environment variable "ethaddr" and copies it to RTK_IDRn.

But the EEPROM seems to have a valid contents (only the last three bytes
differ) and I wonder why it is not used.

> We can change values per hardware by adding device properties
> (prop_dictionary(3)) calls (like sys/dev/pci/if_wm.c etc).

Yes. I added a mac-address property to sk(4) myself, some time ago. But
re(4) doesn't support it yet.


--
Frank Wille

[Izumi Tsutsui]

Frank Wille wrote:

> > Probably it's defined by hardware vendors, not chip.
> >
> > The old RTL8139 (RTL8169 has compat mode) seems to read MAC address
> > from EEPROM and those values are stored into RTK_IDRn registers.
>
> Who writes it into the IDRn registers? The firmware? The driver? Or the chip
> itself? When the chip does that automatically, then re(4) should depend on
> RTK_IDRn and not on the EEPROM.

IIRC RTL8139 doc says the chip reads the values from EEPROM automatically.
We should follow what 8169 doc specifies, but I don't have 8169 docs.

> > I guess some NAS vendors overwrite RTK_IDn registers by firmware
> > to avoid extra EEPROM configurations during production.
>
> You may be right. I found a modification in the PPCBoot source, which reads
> the environment variable "ethaddr" and copies it to RTK_IDRn.
>
> But the EEPROM seems to have a valid contents (only the last three bytes
> differ) and I wonder why it is not used.

Probably all NASes has the same values in EEPROM?
(i.e. no re's EEPROM write operations during manufacture)

> > We can change values per hardware by adding device properties
> > (prop_dictionary(3)) calls (like sys/dev/pci/if_wm.c etc).
>
> Yes. I added a mac-address property to sk(4) myself, some time ago. But
> re(4) doesn't support it yet.

You can add it if necessary, to avoid unexpected changes on other NICs.

---
Izumi Tsutsui

[Frank Wille]

On Sun, 2 Dec 2012 07:41:56 +0900
Izumi Tsutsui <tsu...@ceres.dti.ne.jp> wrote:

> IIRC RTL8139 doc says the chip reads the values from EEPROM automatically.
> We should follow what 8169 doc specifies, but I don't have 8169 docs.

I checked the 8169 doc. In the EEPROM section there is the following
description for EEPROM address 0xe - 0x13:

"Ethernet ID: After auto-load command or hardware reset, the RTL8169
loads Ethernet ID to IDR0-IDR5 of the RTL8169's I/O registers."

As that happens automatically after reset, I would suggest that re(4)
should trust IDR0-IDR5 for the correct address, and not try to access
the EEPROM itself - which might be missing.

> Probably all NASes has the same values in EEPROM?
> (i.e. no re's EEPROM write operations during manufacture)

Your are right. The address read from EEPROM is the same on two different
machines. Probably because it uses default values when the EEPROM is
missing.

--
Frank Wille

[Izumi Tsutsui]

Frank Wille wrote:

(sorry for delay)

> > IIRC RTL8139 doc says the chip reads the values from EEPROM automatically.
> > We should follow what 8169 doc specifies, but I don't have 8169 docs.
>
> I checked the 8169 doc. In the EEPROM section there is the following
> description for EEPROM address 0xe - 0x13:
>
> "Ethernet ID: After auto-load command or hardware reset, the RTL8169
> loads Ethernet ID to IDR0-IDR5 of the RTL8169's I/O registers."
>
> As that happens automatically after reset, I would suggest that re(4)
> should trust IDR0-IDR5 for the correct address, and not try to access
> the EEPROM itself - which might be missing.

The attached patch make re(4) always use IDR register values
for its MAC address.

We no longer have to link rtl81x9.c for eeprom read functions
and I'm not sure if we should make the old behavoir optional
or remove completely.

But for now I think it's almost harmless so please commit
if it works on re(4) on your NAS boxes.

---
Index: conf/files
===================================================================
RCS file: /cvsroot/src/sys/conf/files,v
retrieving revision 1.1061
diff -u -p -r1.1061 files
--- conf/files 14 Nov 2012 02:03:25 -0000 1.1061
+++ conf/files 28 Dec 2012 14:24:20 -0000
@@ -943,7 +943,7 @@ file dev/ic/rtl80x9.c rtl80x9 needs-f
# Realtek 8129/8139 Ethernet controllers
#
device rtk: ether, ifnet, arp, mii
-file dev/ic/rtl81x9.c rtk | re
+file dev/ic/rtl81x9.c rtk

# Realtek 8169 Ethernet controllers
#
Index: dev/ic/rtl8169.c
===================================================================
RCS file: /cvsroot/src/sys/dev/ic/rtl8169.c,v
retrieving revision 1.136
diff -u -p -r1.136 rtl8169.c
--- dev/ic/rtl8169.c 22 Jul 2012 14:32:57 -0000 1.136
+++ dev/ic/rtl8169.c 28 Dec 2012 14:24:21 -0000
@@ -554,9 +554,8 @@ void
re_attach(struct rtk_softc *sc)
{
uint8_t eaddr[ETHER_ADDR_LEN];
- uint16_t val;
struct ifnet *ifp;
- int error = 0, i, addr_len;
+ int error = 0, i;

if ((sc->sc_quirk & RTKQ_8139CPLUS) == 0) {
uint32_t hwrev;
@@ -643,6 +642,12 @@ re_attach(struct rtk_softc *sc)
/* Reset the adapter. */
re_reset(sc);

+ /*
+ * RTL81x9 chips automatically read EEPROM to init MAC address, and
+ * some NAS override its MAC address per own configuration, so
+ * so no need to explicitely read EEPROM and set ID registers.
+ */
+#if 0
if ((sc->sc_quirk & RTKQ_NOEECMD) != 0) {
/*
* Get station address from ID registers.
@@ -650,6 +655,9 @@ re_attach(struct rtk_softc *sc)
for (i = 0; i < ETHER_ADDR_LEN; i++)
eaddr[i] = CSR_READ_1(sc, RTK_IDR0 + i);
} else {
+ uint16_t val;
+ int addr_len;
+
/*
* Get station address from the EEPROM.
*/
@@ -667,6 +675,13 @@ re_attach(struct rtk_softc *sc)
eaddr[(i * 2) + 1] = val >> 8;
}
}
+#else
+ /*
+ * Get station address from ID registers.
+ */
+ for (i = 0; i < ETHER_ADDR_LEN; i++)
+ eaddr[i] = CSR_READ_1(sc, RTK_IDR0 + i);
+#endif

/* Take PHY out of power down mode. */
if ((sc->sc_quirk & RTKQ_PHYWAKE_PM) != 0)
@@ -1725,11 +1740,13 @@ static int
re_init(struct ifnet *ifp)
{
struct rtk_softc *sc = ifp->if_softc;
- const uint8_t *enaddr;
uint32_t rxcfg = 0;
- uint32_t reg;
uint16_t cfg;
int error;
+#if 0
+ const uint8_t *enaddr;
+ uint32_t reg;
+#endif

if ((error = re_enable(sc)) != 0)
goto out;
@@ -1774,6 +1791,7 @@ re_init(struct ifnet *ifp)

DELAY(10000);

+#if 0
/*
* Init our MAC address. Even though the chipset
* documentation doesn't mention it, we need to enter "Config
@@ -1787,6 +1805,7 @@ re_init(struct ifnet *ifp)
reg = enaddr[4] | (enaddr[5] << 8);
CSR_WRITE_4(sc, RTK_IDR4, reg);
CSR_WRITE_1(sc, RTK_EECMD, RTK_EEMODE_OFF);
+#endif

/*
* For C+ mode, initialize the RX descriptors and mbufs.

---
Izumi Tsutsui