| Message ID | 20120320093709.GA5058@electric-eye.fr.zoreil.com |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
On 03/20/2012 02:37 AM, Francois Romieu wrote: > (Larry removed) > > Justin Mattock<justinmattock@gmail.com> : > [...] >> seems I see this with the latest linux-next: > > Thanks for testing. > > [...] >> [21740.318685] r8169 0000:06:00.0: eth0: link up >> [21752.292679] r8169 0000:06:00.0: eth0: link up >> [21764.268569] r8169 0000:06:00.0: eth0: link up >> [21776.254393] r8169 0000:06:00.0: eth0: link up >> [21788.235797] r8169 0000:06:00.0: eth0: link up >> [21800.196524] r8169 0000:06:00.0: eth0: link up >> [21812.172497] r8169 0000:06:00.0: eth0: link up > > This is completely broken. I could understand a few up/down link changes > until things settles but the driver should not claim periodically that > the link is up when there is no cable, at least not with a supported chipset. > > Can you apply the debug helper below and report a complete dmesg from > boot with the same test (please remove l-k, netdev is good enough) ? I removed linux-kernel from the Cc's and applied your patch.. system built fine, and suspend wakes up fine without the ethernet trying to connect like it was doing. will run this patch for a few Thanks for this! dmesg is here: http://fpaste.org/jHSX/ (I have another patch added in, but it should not affect anything). > > diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c > index 61e6ab4..880264a 100644 > --- a/drivers/net/ethernet/realtek/r8169.c > +++ b/drivers/net/ethernet/realtek/r8169.c > @@ -61,7 +61,8 @@ > #endif /* RTL8169_DEBUG */ > > #define R8169_MSG_DEFAULT \ > - (NETIF_MSG_DRV | NETIF_MSG_PROBE | NETIF_MSG_IFUP | NETIF_MSG_IFDOWN) > + (NETIF_MSG_DRV | NETIF_MSG_PROBE | NETIF_MSG_IFUP | NETIF_MSG_IFDOWN | \ > + NETIF_MSG_LINK) > > #define TX_BUFFS_AVAIL(tp) \ > (tp->dirty_tx + NUM_TX_DESC - tp->cur_tx - 1) > @@ -731,7 +732,7 @@ struct rtl8169_private { > void (*phy_reset_enable)(struct rtl8169_private *tp); > void (*hw_start)(struct net_device *); > unsigned int (*phy_reset_pending)(struct rtl8169_private *tp); > - unsigned int (*link_ok)(void __iomem *); > + unsigned int (*link_ok)(struct rtl8169_private *); > int (*do_ioctl)(struct rtl8169_private *tp, struct mii_ioctl_data *data, int cmd); > > struct { > @@ -1260,14 +1261,28 @@ static unsigned int rtl8169_xmii_reset_pending(struct rtl8169_private *tp) > return rtl_readphy(tp, MII_BMCR)& BMCR_RESET; > } > > -static unsigned int rtl8169_tbi_link_ok(void __iomem *ioaddr) > +static unsigned int rtl8169_tbi_link_ok(struct rtl8169_private *tp) > { > + void __iomem *ioaddr = tp->mmio_addr; > + > return RTL_R32(TBICSR)& TBILinkOk; > } > > -static unsigned int rtl8169_xmii_link_ok(void __iomem *ioaddr) > +static unsigned int rtl8169_xmii_link_ok(struct rtl8169_private *tp) > { > - return RTL_R8(PHYstatus)& LinkStatus; > + void __iomem *ioaddr = tp->mmio_addr; > + struct net_device *dev = tp->dev; > + u8 status; > + > + status = RTL_R8(PHYstatus)& LinkStatus; > + netif_info(tp, link, dev, > + "bmcr: %04x bmsr: %04x gbcr: %04x gbsr: %04x\n", > + rtl_readphy(tp, MII_BMCR), > + rtl_readphy(tp, MII_BMSR), > + rtl_readphy(tp, MII_CTRL1000), > + rtl_readphy(tp, MII_STAT1000)); > + > + return status; > } > > static void rtl8169_tbi_reset_enable(struct rtl8169_private *tp) > @@ -1335,7 +1350,7 @@ static void __rtl8169_check_link_status(struct net_device *dev, > struct rtl8169_private *tp, > void __iomem *ioaddr, bool pm) > { > - if (tp->link_ok(ioaddr)) { > + if (tp->link_ok(tp)) { > rtl_link_chg_patch(tp); > /* This is to cancel a scheduled suspend if there's one. */ > if (pm) > @@ -3309,7 +3324,6 @@ static void rtl_hw_phy_config(struct net_device *dev) > static void rtl_phy_work(struct rtl8169_private *tp) > { > struct timer_list *timer =&tp->timer; > - void __iomem *ioaddr = tp->mmio_addr; > unsigned long timeout = RTL8169_PHY_TIMEOUT; > > assert(tp->mac_version> RTL_GIGA_MAC_VER_01); > @@ -3323,7 +3337,7 @@ static void rtl_phy_work(struct rtl8169_private *tp) > goto out_mod_timer; > } > > - if (tp->link_ok(ioaddr)) > + if (tp->link_ok(tp)) > return; > > netif_warn(tp, link, tp->dev, "PHY reset until link up\n"); -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 03/20/2012 07:16 AM, Justin P. Mattock wrote: > On 03/20/2012 02:37 AM, Francois Romieu wrote: >> (Larry removed) >> >> Justin Mattock<justinmattock@gmail.com> : >> [...] >>> seems I see this with the latest linux-next: >> >> Thanks for testing. >> >> [...] >>> [21740.318685] r8169 0000:06:00.0: eth0: link up >>> [21752.292679] r8169 0000:06:00.0: eth0: link up >>> [21764.268569] r8169 0000:06:00.0: eth0: link up >>> [21776.254393] r8169 0000:06:00.0: eth0: link up >>> [21788.235797] r8169 0000:06:00.0: eth0: link up >>> [21800.196524] r8169 0000:06:00.0: eth0: link up >>> [21812.172497] r8169 0000:06:00.0: eth0: link up >> >> This is completely broken. I could understand a few up/down link changes >> until things settles but the driver should not claim periodically that >> the link is up when there is no cable, at least not with a supported >> chipset. >> >> Can you apply the debug helper below and report a complete dmesg from >> boot with the same test (please remove l-k, netdev is good enough) ? > > I removed linux-kernel from the Cc's and applied your patch.. system > built fine, and suspend wakes up fine without the ethernet trying to > connect like it was doing. will run this patch for a few Thanks for this! > > dmesg is here: http://fpaste.org/jHSX/ > (I have another patch added in, but it should not affect anything). > > well i dont know what happened.. I suspend throughout the day without any issues, but then on one wakeup everything went to crap ethernet started to try and connect without any wires in.. only thing I can think of is I was dongling with my phone(but the phone was disconnected before suspend),which uses a different module.. dmesg here: http://fpaste.org/mcNC/ Justin P. Mattock -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Justin P. Mattock <justinmattock@gmail.com> : [...] > well i dont know what happened.. I suspend throughout the day > without any issues, but then on one wakeup everything went to crap > ethernet started to try and connect without any wires in.. only > thing I can think of is I was dongling with my phone(but the phone > was disconnected before suspend),which uses a different module.. The MII registers read as 0xff. I'll have to figure why the suspend / resume cycle failed, or why it seemed to work. :o/
On 03/20/2012 04:27 PM, Francois Romieu wrote: > Justin P. Mattock<justinmattock@gmail.com> : > [...] >> well i dont know what happened.. I suspend throughout the day >> without any issues, but then on one wakeup everything went to crap >> ethernet started to try and connect without any wires in.. only >> thing I can think of is I was dongling with my phone(but the phone >> was disconnected before suspend),which uses a different module.. > > The MII registers read as 0xff. I'll have to figure why the > suspend / resume cycle failed, or why it seemed to work. :o/ > not sure why this did this, I rebooted and have suspended numerous times without an issue with ethernet, even retraced my steps to try and re-created but nothing. seems solid to me beside that hickup. I am trying to gather any info for you, if you have any ideas on what I can try let me know! Thanks for the patch and info with this. Justin P. Mattock -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c index 61e6ab4..880264a 100644 --- a/drivers/net/ethernet/realtek/r8169.c +++ b/drivers/net/ethernet/realtek/r8169.c @@ -61,7 +61,8 @@ #endif /* RTL8169_DEBUG */ #define R8169_MSG_DEFAULT \ - (NETIF_MSG_DRV | NETIF_MSG_PROBE | NETIF_MSG_IFUP | NETIF_MSG_IFDOWN) + (NETIF_MSG_DRV | NETIF_MSG_PROBE | NETIF_MSG_IFUP | NETIF_MSG_IFDOWN | \ + NETIF_MSG_LINK) #define TX_BUFFS_AVAIL(tp) \ (tp->dirty_tx + NUM_TX_DESC - tp->cur_tx - 1) @@ -731,7 +732,7 @@ struct rtl8169_private { void (*phy_reset_enable)(struct rtl8169_private *tp); void (*hw_start)(struct net_device *); unsigned int (*phy_reset_pending)(struct rtl8169_private *tp); - unsigned int (*link_ok)(void __iomem *); + unsigned int (*link_ok)(struct rtl8169_private *); int (*do_ioctl)(struct rtl8169_private *tp, struct mii_ioctl_data *data, int cmd); struct { @@ -1260,14 +1261,28 @@ static unsigned int rtl8169_xmii_reset_pending(struct rtl8169_private *tp) return rtl_readphy(tp, MII_BMCR) & BMCR_RESET; } -static unsigned int rtl8169_tbi_link_ok(void __iomem *ioaddr) +static unsigned int rtl8169_tbi_link_ok(struct rtl8169_private *tp) { + void __iomem *ioaddr = tp->mmio_addr; + return RTL_R32(TBICSR) & TBILinkOk; } -static unsigned int rtl8169_xmii_link_ok(void __iomem *ioaddr) +static unsigned int rtl8169_xmii_link_ok(struct rtl8169_private *tp) { - return RTL_R8(PHYstatus) & LinkStatus; + void __iomem *ioaddr = tp->mmio_addr; + struct net_device *dev = tp->dev; + u8 status; + + status = RTL_R8(PHYstatus) & LinkStatus; + netif_info(tp, link, dev, + "bmcr: %04x bmsr: %04x gbcr: %04x gbsr: %04x\n", + rtl_readphy(tp, MII_BMCR), + rtl_readphy(tp, MII_BMSR), + rtl_readphy(tp, MII_CTRL1000), + rtl_readphy(tp, MII_STAT1000)); + + return status; } static void rtl8169_tbi_reset_enable(struct rtl8169_private *tp) @@ -1335,7 +1350,7 @@ static void __rtl8169_check_link_status(struct net_device *dev, struct rtl8169_private *tp, void __iomem *ioaddr, bool pm) { - if (tp->link_ok(ioaddr)) { + if (tp->link_ok(tp)) { rtl_link_chg_patch(tp); /* This is to cancel a scheduled suspend if there's one. */ if (pm) @@ -3309,7 +3324,6 @@ static void rtl_hw_phy_config(struct net_device *dev) static void rtl_phy_work(struct rtl8169_private *tp) { struct timer_list *timer = &tp->timer; - void __iomem *ioaddr = tp->mmio_addr; unsigned long timeout = RTL8169_PHY_TIMEOUT; assert(tp->mac_version > RTL_GIGA_MAC_VER_01); @@ -3323,7 +3337,7 @@ static void rtl_phy_work(struct rtl8169_private *tp) goto out_mod_timer; } - if (tp->link_ok(ioaddr)) + if (tp->link_ok(tp)) return; netif_warn(tp, link, tp->dev, "PHY reset until link up\n");