| Message ID | 20121205223452.GA24164@electric-eye.fr.zoreil.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On Wed, Dec 05, 2012 at 11:34:52PM +0100, Francois Romieu wrote: > GigaMAC registers have been reported left unitialized in several > situations: > - after cold boot from power-off state > - after S3 resume > > Tweaking rtl_hw_phy_config takes care of both. Hi Francois. Are you sure we will lost GigaMAC registers's content after NIC into PCI_D3hot state? Thanks -- 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
Wang YanQing: [...] > Are you sure we will lose GigaMAC registers's content > after NIC into PCI_D3hot state ? I have not made such a claim. Only some bioses f*ck things up on resume, see: http://marc.info/?l=linux-netdev&m=132195832624117&w=2 My test computer and its 8168evl (courtesy of Realtek) was not able to make a difference w/o the patch: it always works.
On Wed, Dec 05, 2012 at 11:34:52PM +0100, Francois Romieu wrote: > GigaMAC registers have been reported left unitialized in several > situations: > - after cold boot from power-off state > - after S3 resume > > Tweaking rtl_hw_phy_config takes care of both. > > Signed-off-by: Francois Romieu <romieu@fr.zoreil.com> > Cc: Hayes Wang <hayeswang@realtek.com> > --- > drivers/net/ethernet/realtek/r8169.c | 42 ++++++++++++++++++++---------------- > 1 file changed, 24 insertions(+), 18 deletions(-) > > YanQing and Chun-Yi, can you add your Signed-off-by to this patch ? > It contains bits of everybody's work but it does not match any. :o) > > I apparently play in the safe bios league since I did not notice any > difference before or after the patch. > > Beware, this patch seems to apply to net-next but doing so moves > rtl_rar_exgmac_set from rtl8168e_2_hw_phy_config to rtl8168f_hw_phy_config. > > Hayes, your comments are welcome if any. > > diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c > index 927aa33..b353003 100644 > --- a/drivers/net/ethernet/realtek/r8169.c > +++ b/drivers/net/ethernet/realtek/r8169.c > @@ -3096,6 +3096,23 @@ static void rtl8168e_1_hw_phy_config(struct rtl8169_private *tp) > rtl_writephy(tp, 0x0d, 0x0000); > } > > +static void rtl_rar_exgmac_set(struct rtl8169_private *tp, u8 *addr) > +{ > + const u16 w[] = { > + addr[0] | (addr[1] << 8), > + addr[2] | (addr[3] << 8), > + addr[4] | (addr[5] << 8) > + }; > + const struct exgmac_reg e[] = { > + { .addr = 0xe0, ERIAR_MASK_1111, .val = w[0] | (w[1] << 16) }, > + { .addr = 0xe4, ERIAR_MASK_1111, .val = w[2] }, > + { .addr = 0xf0, ERIAR_MASK_1111, .val = w[0] << 16 }, > + { .addr = 0xf4, ERIAR_MASK_1111, .val = w[1] | (w[2] << 16) }, > + }; > + > + rtl_write_exgmac_batch(tp, e, ARRAY_SIZE(e)); > +} > + > static void rtl8168e_2_hw_phy_config(struct rtl8169_private *tp) > { > static const struct phy_reg phy_reg_init[] = { > @@ -3178,6 +3195,9 @@ static void rtl8168e_2_hw_phy_config(struct rtl8169_private *tp) > rtl_w1w0_phy(tp, 0x19, 0x0000, 0x0001); > rtl_w1w0_phy(tp, 0x10, 0x0000, 0x0400); > rtl_writephy(tp, 0x1f, 0x0000); > + > + /* Broken BIOS workaround: feed GigaMAC registers with MAC address. */ > + rtl_rar_exgmac_set(tp, tp->dev->dev_addr); > } > > static void rtl8168f_hw_phy_config(struct rtl8169_private *tp) > @@ -3708,33 +3728,19 @@ static void rtl8169_init_phy(struct net_device *dev, struct rtl8169_private *tp) > static void rtl_rar_set(struct rtl8169_private *tp, u8 *addr) > { > void __iomem *ioaddr = tp->mmio_addr; > - u32 high; > - u32 low; > - > - low = addr[0] | (addr[1] << 8) | (addr[2] << 16) | (addr[3] << 24); > - high = addr[4] | (addr[5] << 8); > > rtl_lock_work(tp); > > RTL_W8(Cfg9346, Cfg9346_Unlock); > > - RTL_W32(MAC4, high); > + RTL_W32(MAC4, addr[4] | addr[5] << 8); > RTL_R32(MAC4); > > - RTL_W32(MAC0, low); > + RTL_W32(MAC0, addr[0] | addr[1] << 8 | addr[2] << 16 | addr[3] << 24); > RTL_R32(MAC0); > > - if (tp->mac_version == RTL_GIGA_MAC_VER_34) { > - const struct exgmac_reg e[] = { > - { .addr = 0xe0, ERIAR_MASK_1111, .val = low }, > - { .addr = 0xe4, ERIAR_MASK_1111, .val = high }, > - { .addr = 0xf0, ERIAR_MASK_1111, .val = low << 16 }, > - { .addr = 0xf4, ERIAR_MASK_1111, .val = high << 16 | > - low >> 16 }, > - }; > - > - rtl_write_exgmac_batch(tp, e, ARRAY_SIZE(e)); > - } > + if (tp->mac_version == RTL_GIGA_MAC_VER_34) > + rtl_rar_exgmac_set(tp, addr); > > RTL_W8(Cfg9346, Cfg9346_Lock); > > -- > 1.7.11.7 Signed-off-by: Wang YanQing <udknight@gmail.com> -- 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
From: Francois Romieu <romieu@fr.zoreil.com> Date: Wed, 5 Dec 2012 23:34:52 +0100 > GigaMAC registers have been reported left unitialized in several > situations: > - after cold boot from power-off state > - after S3 resume > > Tweaking rtl_hw_phy_config takes care of both. > > Signed-off-by: Francois Romieu <romieu@fr.zoreil.com> > Cc: Hayes Wang <hayeswang@realtek.com> > --- > drivers/net/ethernet/realtek/r8169.c | 42 ++++++++++++++++++++---------------- > 1 file changed, 24 insertions(+), 18 deletions(-) > > YanQing and Chun-Yi, can you add your Signed-off-by to this patch ? > It contains bits of everybody's work but it does not match any. :o) > > I apparently play in the safe bios league since I did not notice any > difference before or after the patch. > > Beware, this patch seems to apply to net-next but doing so moves > rtl_rar_exgmac_set from rtl8168e_2_hw_phy_config to rtl8168f_hw_phy_config. > > Hayes, your comments are welcome if any. Francois could you please respin this against net-next to avoid the unintended consequence of applying the change to the wrong function? If this change turns out to be more critical than it appears, and impact more people than it appears, we can queue it up for -stable later. Thanks. -- 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 927aa33..b353003 100644 --- a/drivers/net/ethernet/realtek/r8169.c +++ b/drivers/net/ethernet/realtek/r8169.c @@ -3096,6 +3096,23 @@ static void rtl8168e_1_hw_phy_config(struct rtl8169_private *tp) rtl_writephy(tp, 0x0d, 0x0000); } +static void rtl_rar_exgmac_set(struct rtl8169_private *tp, u8 *addr) +{ + const u16 w[] = { + addr[0] | (addr[1] << 8), + addr[2] | (addr[3] << 8), + addr[4] | (addr[5] << 8) + }; + const struct exgmac_reg e[] = { + { .addr = 0xe0, ERIAR_MASK_1111, .val = w[0] | (w[1] << 16) }, + { .addr = 0xe4, ERIAR_MASK_1111, .val = w[2] }, + { .addr = 0xf0, ERIAR_MASK_1111, .val = w[0] << 16 }, + { .addr = 0xf4, ERIAR_MASK_1111, .val = w[1] | (w[2] << 16) }, + }; + + rtl_write_exgmac_batch(tp, e, ARRAY_SIZE(e)); +} + static void rtl8168e_2_hw_phy_config(struct rtl8169_private *tp) { static const struct phy_reg phy_reg_init[] = { @@ -3178,6 +3195,9 @@ static void rtl8168e_2_hw_phy_config(struct rtl8169_private *tp) rtl_w1w0_phy(tp, 0x19, 0x0000, 0x0001); rtl_w1w0_phy(tp, 0x10, 0x0000, 0x0400); rtl_writephy(tp, 0x1f, 0x0000); + + /* Broken BIOS workaround: feed GigaMAC registers with MAC address. */ + rtl_rar_exgmac_set(tp, tp->dev->dev_addr); } static void rtl8168f_hw_phy_config(struct rtl8169_private *tp) @@ -3708,33 +3728,19 @@ static void rtl8169_init_phy(struct net_device *dev, struct rtl8169_private *tp) static void rtl_rar_set(struct rtl8169_private *tp, u8 *addr) { void __iomem *ioaddr = tp->mmio_addr; - u32 high; - u32 low; - - low = addr[0] | (addr[1] << 8) | (addr[2] << 16) | (addr[3] << 24); - high = addr[4] | (addr[5] << 8); rtl_lock_work(tp); RTL_W8(Cfg9346, Cfg9346_Unlock); - RTL_W32(MAC4, high); + RTL_W32(MAC4, addr[4] | addr[5] << 8); RTL_R32(MAC4); - RTL_W32(MAC0, low); + RTL_W32(MAC0, addr[0] | addr[1] << 8 | addr[2] << 16 | addr[3] << 24); RTL_R32(MAC0); - if (tp->mac_version == RTL_GIGA_MAC_VER_34) { - const struct exgmac_reg e[] = { - { .addr = 0xe0, ERIAR_MASK_1111, .val = low }, - { .addr = 0xe4, ERIAR_MASK_1111, .val = high }, - { .addr = 0xf0, ERIAR_MASK_1111, .val = low << 16 }, - { .addr = 0xf4, ERIAR_MASK_1111, .val = high << 16 | - low >> 16 }, - }; - - rtl_write_exgmac_batch(tp, e, ARRAY_SIZE(e)); - } + if (tp->mac_version == RTL_GIGA_MAC_VER_34) + rtl_rar_exgmac_set(tp, addr); RTL_W8(Cfg9346, Cfg9346_Lock);
GigaMAC registers have been reported left unitialized in several situations: - after cold boot from power-off state - after S3 resume Tweaking rtl_hw_phy_config takes care of both. Signed-off-by: Francois Romieu <romieu@fr.zoreil.com> Cc: Hayes Wang <hayeswang@realtek.com> --- drivers/net/ethernet/realtek/r8169.c | 42 ++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 18 deletions(-) YanQing and Chun-Yi, can you add your Signed-off-by to this patch ? It contains bits of everybody's work but it does not match any. :o) I apparently play in the safe bios league since I did not notice any difference before or after the patch. Beware, this patch seems to apply to net-next but doing so moves rtl_rar_exgmac_set from rtl8168e_2_hw_phy_config to rtl8168f_hw_phy_config. Hayes, your comments are welcome if any.