rgephy(4): Always use RL_GMEDIASTAT for reading link/media status for re(4) PHY

2013-12-28 Thread Brad Smith 
This moves rgephy(4) back to using RL_GMEDIASTAT to read the link/media
status for re(4) attached Realtek PHY as was done before rev 1.25. rev
1.25 was to add support for external rgephy(4) attached to other MAC
such as nfe(4), but the PHY Specific Status register doesn't seem to
work properly with some integrated PHY with re(4).

Tested with..
rgephy0 at re0 phy 7: RTL8169S/8110S PHY, rev. 2
rgephy0 at re0 phy 7: RTL8169S/8110S PHY, rev. 4
rgephy0 at re0 phy 7: RTL8169S/8110S PHY, rev. 5

and some newer PHY where this was problematic but existing 8169 PHY
rev board combos are affected too.

From FreeBSD and matches what the Linux driver does.


Index: rgephy.c
===
RCS file: /home/cvs/src/sys/dev/mii/rgephy.c,v
retrieving revision 1.30
diff -u -p -r1.30 rgephy.c
--- rgephy.c28 May 2013 09:46:06 -  1.30
+++ rgephy.c19 Dec 2013 11:13:54 -
@@ -145,6 +145,9 @@ rgephy_service(struct mii_softc *sc, str
 {
struct ifmedia_entry *ife = mii-mii_media.ifm_cur;
int anar, reg, speed, gig = 0;
+   char *devname;
+
+   devname = sc-mii_dev.dv_parent-dv_cfdata-cf_driver-cd_name;
 
switch (cmd) {
case MII_POLLSTAT:
@@ -249,7 +252,7 @@ setit:
 * need to restart the autonegotiation process.  Read
 * the BMSR twice in case it's latched.
 */
-   if (sc-mii_rev  2) {
+   if (strcmp(devname, re) == 0) {
reg = PHY_READ(sc, RL_GMEDIASTAT);
if (reg  RL_GMEDIASTAT_LINK) {
sc-mii_ticks = 0;
@@ -298,13 +301,15 @@ rgephy_status(struct mii_softc *sc)
 {
struct mii_data *mii = sc-mii_pdata;
int bmsr, bmcr, gtsr;
+   char *devname;
+
+   devname = sc-mii_dev.dv_parent-dv_cfdata-cf_driver-cd_name;
 
mii-mii_media_status = IFM_AVALID;
mii-mii_media_active = IFM_ETHER;
 
-   if (sc-mii_rev  2) {
+   if (strcmp(devname, re) == 0) {
bmsr = PHY_READ(sc, RL_GMEDIASTAT);
-
if (bmsr  RL_GMEDIASTAT_LINK)
mii-mii_media_status |= IFM_ACTIVE;
} else {
@@ -328,7 +333,7 @@ rgephy_status(struct mii_softc *sc)
}
}
 
-   if (sc-mii_rev  2) {
+   if (strcmp(devname, re) == 0) {
bmsr = PHY_READ(sc, RL_GMEDIASTAT);
if (bmsr  RL_GMEDIASTAT_1000MBPS)
mii-mii_media_active |= IFM_1000_T;

-- 
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.

Re: rgephy(4): Always use RL_GMEDIASTAT for reading link/media status for re(4) PHY

2013-12-28 Thread Mark Kettenis 
 Date: Sat, 28 Dec 2013 15:42:12 -0500
 From: Brad Smith b...@comstyle.com
 
 This moves rgephy(4) back to using RL_GMEDIASTAT to read the link/media
 status for re(4) attached Realtek PHY as was done before rev 1.25. rev
 1.25 was to add support for external rgephy(4) attached to other MAC
 such as nfe(4), but the PHY Specific Status register doesn't seem to
 work properly with some integrated PHY with re(4).
 
 Tested with..
 rgephy0 at re0 phy 7: RTL8169S/8110S PHY, rev. 2
 rgephy0 at re0 phy 7: RTL8169S/8110S PHY, rev. 4
 rgephy0 at re0 phy 7: RTL8169S/8110S PHY, rev. 5
 
 and some newer PHY where this was problematic but existing 8169 PHY
 rev board combos are affected too.
 
 From FreeBSD and matches what the Linux driver does.

I hate mii(4) drivers that look at the name of their parent!

 Index: rgephy.c
 ===
 RCS file: /home/cvs/src/sys/dev/mii/rgephy.c,v
 retrieving revision 1.30
 diff -u -p -r1.30 rgephy.c
 --- rgephy.c  28 May 2013 09:46:06 -  1.30
 +++ rgephy.c  19 Dec 2013 11:13:54 -
 @@ -145,6 +145,9 @@ rgephy_service(struct mii_softc *sc, str
  {
   struct ifmedia_entry *ife = mii-mii_media.ifm_cur;
   int anar, reg, speed, gig = 0;
 + char *devname;
 +
 + devname = sc-mii_dev.dv_parent-dv_cfdata-cf_driver-cd_name;
  
   switch (cmd) {
   case MII_POLLSTAT:
 @@ -249,7 +252,7 @@ setit:
* need to restart the autonegotiation process.  Read
* the BMSR twice in case it's latched.
*/
 - if (sc-mii_rev  2) {
 + if (strcmp(devname, re) == 0) {
   reg = PHY_READ(sc, RL_GMEDIASTAT);
   if (reg  RL_GMEDIASTAT_LINK) {
   sc-mii_ticks = 0;
 @@ -298,13 +301,15 @@ rgephy_status(struct mii_softc *sc)
  {
   struct mii_data *mii = sc-mii_pdata;
   int bmsr, bmcr, gtsr;
 + char *devname;
 +
 + devname = sc-mii_dev.dv_parent-dv_cfdata-cf_driver-cd_name;
  
   mii-mii_media_status = IFM_AVALID;
   mii-mii_media_active = IFM_ETHER;
  
 - if (sc-mii_rev  2) {
 + if (strcmp(devname, re) == 0) {
   bmsr = PHY_READ(sc, RL_GMEDIASTAT);
 -
   if (bmsr  RL_GMEDIASTAT_LINK)
   mii-mii_media_status |= IFM_ACTIVE;
   } else {
 @@ -328,7 +333,7 @@ rgephy_status(struct mii_softc *sc)
   }
   }
  
 - if (sc-mii_rev  2) {
 + if (strcmp(devname, re) == 0) {
   bmsr = PHY_READ(sc, RL_GMEDIASTAT);
   if (bmsr  RL_GMEDIASTAT_1000MBPS)
   mii-mii_media_active |= IFM_1000_T;
 
 -- 
 This message has been scanned for viruses and
 dangerous content by MailScanner, and is
 believed to be clean.

Re: rgephy(4): Always use RL_GMEDIASTAT for reading link/media status for re(4) PHY

2013-12-28 Thread Theo de Raadt 
  This moves rgephy(4) back to using RL_GMEDIASTAT to read the link/media
  status for re(4) attached Realtek PHY as was done before rev 1.25. rev
  1.25 was to add support for external rgephy(4) attached to other MAC
  such as nfe(4), but the PHY Specific Status register doesn't seem to
  work properly with some integrated PHY with re(4).
  
  Tested with..
  rgephy0 at re0 phy 7: RTL8169S/8110S PHY, rev. 2
  rgephy0 at re0 phy 7: RTL8169S/8110S PHY, rev. 4
  rgephy0 at re0 phy 7: RTL8169S/8110S PHY, rev. 5
  
  and some newer PHY where this was problematic but existing 8169 PHY
  rev board combos are affected too.
  
  From FreeBSD and matches what the Linux driver does.
 
 I hate mii(4) drivers that look at the name of their parent!

Unfortunately it is a harsh reality that some of these PHY's are
sometimes integrated unified-fashion into a MAC chip, without evident
register changes to identify them, and then they have a subtly
different behaviour.

It happens in Broadcom products too.