From owner-freebsd-bugs@FreeBSD.ORG Sat Sep 1 10:00:33 2012 Return-Path: Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 441A0106566B for ; Sat, 1 Sep 2012 10:00:33 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id 92BF88FC19 for ; Sat, 1 Sep 2012 10:00:21 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.5/8.14.5) with ESMTP id q81A0L7d080456 for ; Sat, 1 Sep 2012 10:00:21 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.5/8.14.5/Submit) id q81A0LBl080455; Sat, 1 Sep 2012 10:00:21 GMT (envelope-from gnats) Resent-Date: Sat, 1 Sep 2012 10:00:21 GMT Resent-Message-Id: <201209011000.q81A0LBl080455@freefall.freebsd.org> Resent-From: FreeBSD-gnats-submit@FreeBSD.org (GNATS Filer) Resent-To: freebsd-bugs@FreeBSD.org Resent-Reply-To: FreeBSD-gnats-submit@FreeBSD.org, Dan Lukes Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 851C61065673 for ; Sat, 1 Sep 2012 09:57:11 +0000 (UTC) (envelope-from dan@kgw.obluda.cz) Received: from kgw.obluda.cz (kgw.obluda.cz [193.179.199.50]) by mx1.freebsd.org (Postfix) with ESMTP id B36628FC0C for ; Sat, 1 Sep 2012 09:57:10 +0000 (UTC) Received: from kgw.obluda.cz (localhost [127.0.0.1]) by kgw.obluda.cz (8.14.5/8.14.5) with ESMTP id q819ZVGx084356 for ; Sat, 1 Sep 2012 11:35:31 +0200 (CEST) (envelope-from dan@kgw.obluda.cz) Received: (from root@localhost) by kgw.obluda.cz (8.14.5/8.14.5/Submit) id q819ZVie084355; Sat, 1 Sep 2012 11:35:31 +0200 (CEST) (envelope-from dan) Message-Id: <201209010935.q819ZVie084355@kgw.obluda.cz> Date: Sat, 1 Sep 2012 11:35:31 +0200 (CEST) From: Dan Lukes To: FreeBSD-gnats-submit@FreeBSD.org X-Send-Pr-Version: 3.113 Cc: Subject: kern/171228: [ patch ] if_re - eeprom write issues X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Dan Lukes List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 01 Sep 2012 10:00:33 -0000 >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: