>Number: 171228
>Category: kern
>Synopsis: [ patch ] if_re - eeprom write issues
>Confidential: no
>Severity: non-critical
>Priority: medium
>Responsible: freebsd-bugs
>State: open
>Quarter:
>Keywords:
>Date-Required:
>Class: sw-bug
>Submitter-Id: current-users
>Arrival-Date: Sat Sep 01 10:00:20 UTC 2012
>Closed-Date:
>Last-Modified:
>Originator: Dan Lukes
>Release: FreeBSD 9.1-RC1 i386
>Organization:
Obludarium
>Environment:
Originally found on:
System: FreeBSD 9.1-RC1 i386
sys/dev/re/if_re.c,v 1.198.2.14 2012/07/02 19:56:31
verified to apply to HEAD & if_re.c 1.224 as well
>Description:
Realtek network card driver, if_re.c:
1. re_clrwol()
CFG5 register writen after config register write done claimed. Yes, some
documentation show that it's not necesarry to be in such mode to write CFG5,
but it's unclear it apply to all chip versions.
See SVN rev 232145 comment also.
It's good to be consistent in all parts of code. All other writes to CFG5
register in module are done in "EEPROM write enable" mode but this one in
re_clrwol() (see re_setwol and re_attach routines)
2. unecesarry writes into EEPROM (e.g. write of data althougth unchanged)
There are several places, where config register is read, examined, sometime)
modified, then written back into EEPROM.
Unfortunately, they are written back unconditionally, whenever they has been
modified or not.
As EEPROM have limited number of write cycles, such behavior may short
lifetime of network card
>How-To-Repeat:
see description
>Fix:
[1] - move CFG5 write before "EEPROM write mode" close. It's better to be on
safe side and it doesn't harm anything even it's not necesarry for particular
chip
[2] - store original value of configuration register read, write back only if
changed
--- patch-re begins here ---
--- sys/dev/re/if_re.c.orig 2012-07-02 21:56:31.000000000 +0200
+++ sys/dev/re/if_re.c 2012-09-01 10:01:33.000000000 +0200
@@ -1194,7 +1194,7 @@
u_int16_t devid, re_did = 0;
int error = 0, i, phy, rid;
int msic, msixc, reg;
- uint8_t cfg;
+ uint8_t ocfg, cfg;
sc = device_get_softc(dev);
sc->rl_dev = dev;
@@ -1294,9 +1294,10 @@
sc->rl_flags |= RL_FLAG_MSI;
/* Explicitly set MSI enable bit. */
CSR_WRITE_1(sc, RL_EECMD, RL_EE_MODE);
- cfg = CSR_READ_1(sc, RL_CFG2);
+ ocfg = cfg = CSR_READ_1(sc, RL_CFG2);
cfg |= RL_CFG2_MSI;
- CSR_WRITE_1(sc, RL_CFG2, cfg);
+ if (cfg != ocfg)
+ CSR_WRITE_1(sc, RL_CFG2, cfg);
CSR_WRITE_1(sc, RL_EECMD, RL_EEMODE_OFF);
} else
pci_release_msi(dev);
@@ -1500,12 +1501,14 @@
/* Enable PME. */
CSR_WRITE_1(sc, RL_EECMD, RL_EE_MODE);
- cfg = CSR_READ_1(sc, sc->rl_cfg1);
+ ocfg = cfg = CSR_READ_1(sc, sc->rl_cfg1);
cfg |= RL_CFG1_PME;
- CSR_WRITE_1(sc, sc->rl_cfg1, cfg);
- cfg = CSR_READ_1(sc, sc->rl_cfg5);
+ if (ocfg != cfg)
+ CSR_WRITE_1(sc, sc->rl_cfg1, cfg);
+ ocfg = cfg = CSR_READ_1(sc, sc->rl_cfg5);
cfg &= RL_CFG5_PME_STS;
- CSR_WRITE_1(sc, sc->rl_cfg5, cfg);
+ if (ocfg != cfg)
+ CSR_WRITE_1(sc, sc->rl_cfg5, cfg);
CSR_WRITE_1(sc, RL_EECMD, RL_EEMODE_OFF);
if ((sc->rl_flags & RL_FLAG_PAR) != 0) {
@@ -3780,7 +3783,7 @@
struct ifnet *ifp;
int pmc;
uint16_t pmstat;
- uint8_t v;
+ uint8_t ov, v;
RL_LOCK_ASSERT(sc);
@@ -3805,19 +3808,21 @@
CSR_WRITE_1(sc, RL_EECMD, RL_EE_MODE);
/* Enable PME. */
- v = CSR_READ_1(sc, sc->rl_cfg1);
+ ov = v = CSR_READ_1(sc, sc->rl_cfg1);
v &= ~RL_CFG1_PME;
if ((ifp->if_capenable & IFCAP_WOL) != 0)
v |= RL_CFG1_PME;
- CSR_WRITE_1(sc, sc->rl_cfg1, v);
+ if (ov != v)
+ CSR_WRITE_1(sc, sc->rl_cfg1, v);
- v = CSR_READ_1(sc, sc->rl_cfg3);
+ ov = v = CSR_READ_1(sc, sc->rl_cfg3);
v &= ~(RL_CFG3_WOL_LINK | RL_CFG3_WOL_MAGIC);
if ((ifp->if_capenable & IFCAP_WOL_MAGIC) != 0)
v |= RL_CFG3_WOL_MAGIC;
- CSR_WRITE_1(sc, sc->rl_cfg3, v);
+ if (ov != v)
+ CSR_WRITE_1(sc, sc->rl_cfg3, v);
- v = CSR_READ_1(sc, sc->rl_cfg5);
+ ov = v = CSR_READ_1(sc, sc->rl_cfg5);
v &= ~(RL_CFG5_WOL_BCAST | RL_CFG5_WOL_MCAST | RL_CFG5_WOL_UCAST |
RL_CFG5_WOL_LANWAKE);
if ((ifp->if_capenable & IFCAP_WOL_UCAST) != 0)
@@ -3826,7 +3831,8 @@
v |= RL_CFG5_WOL_MCAST | RL_CFG5_WOL_BCAST;
if ((ifp->if_capenable & IFCAP_WOL) != 0)
v |= RL_CFG5_WOL_LANWAKE;
- CSR_WRITE_1(sc, sc->rl_cfg5, v);
+ if (ov != v)
+ CSR_WRITE_1(sc, sc->rl_cfg5, v);
/* Config register write done. */
CSR_WRITE_1(sc, RL_EECMD, RL_EEMODE_OFF);
@@ -3852,7 +3858,7 @@
re_clrwol(struct rl_softc *sc)
{
int pmc;
- uint8_t v;
+ uint8_t ov, v;
RL_LOCK_ASSERT(sc);
@@ -3862,17 +3868,19 @@
/* Enable config register write. */
CSR_WRITE_1(sc, RL_EECMD, RL_EE_MODE);
- v = CSR_READ_1(sc, sc->rl_cfg3);
+ ov = v = CSR_READ_1(sc, sc->rl_cfg3);
v &= ~(RL_CFG3_WOL_LINK | RL_CFG3_WOL_MAGIC);
- CSR_WRITE_1(sc, sc->rl_cfg3, v);
+ if (ov != v)
+ CSR_WRITE_1(sc, sc->rl_cfg3, v);
- /* Config register write done. */
- CSR_WRITE_1(sc, RL_EECMD, RL_EEMODE_OFF);
-
- v = CSR_READ_1(sc, sc->rl_cfg5);
+ ov = v = CSR_READ_1(sc, sc->rl_cfg5);
v &= ~(RL_CFG5_WOL_BCAST | RL_CFG5_WOL_MCAST | RL_CFG5_WOL_UCAST);
v &= ~RL_CFG5_WOL_LANWAKE;
- CSR_WRITE_1(sc, sc->rl_cfg5, v);
+ if (ov != v)
+ CSR_WRITE_1(sc, sc->rl_cfg5, v);
+
+ /* Config register write done. */
+ CSR_WRITE_1(sc, RL_EECMD, RL_EEMODE_OFF);
}
static void
--- patch-re ends here ---
>Release-Note:
>Audit-Trail:
>Unformatted:
_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-bugs
To unsubscribe, send any mail to "[email protected]"
