* [PATCH net] r8169: disable L23
@ 2014-07-08 8:26 Hayes Wang
2014-07-08 20:16 ` Francois Romieu
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Hayes Wang @ 2014-07-08 8:26 UTC (permalink / raw)
To: romieu; +Cc: netdev, linux-kernel, nic_swsd, Hayes Wang
For RTL8411, RTL8111G, RTL8402, RTL8105, and RTL8106, the nic couldn't
leave the power saving mode of L23 for certain situation. This causes
the device lost for the system. Disable the L23 mode to avoid it.
Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
drivers/net/ethernet/realtek/r8169.c | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index be425ad..38144ee 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -538,6 +538,7 @@ enum rtl_register_content {
MagicPacket = (1 << 5), /* Wake up when receives a Magic Packet */
LinkUp = (1 << 4), /* Wake up when the cable connection is re-established */
Jumbo_En0 = (1 << 2), /* 8168 only. Reserved in the 8168b */
+ Rdy_to_L23 = (1 << 1), /* L23 Enable */
Beacon_en = (1 << 0), /* 8168 only. Reserved in the 8168b */
/* Config4 register */
@@ -4897,6 +4898,21 @@ static void rtl_enable_clock_request(struct pci_dev *pdev)
PCI_EXP_LNKCTL_CLKREQ_EN);
}
+static void rtl_l23_enable(struct rtl8169_private *tp, bool enable)
+{
+ void __iomem *ioaddr = tp->mmio_addr;
+ u8 data;
+
+ data = RTL_R8(Config3);
+
+ if (enable)
+ data |= Rdy_to_L23;
+ else
+ data &= ~Rdy_to_L23;
+
+ RTL_W8(Config3, data);
+}
+
#define R8168_CPCMD_QUIRK_MASK (\
EnableBist | \
Mac_dbgo_oe | \
@@ -5246,6 +5262,7 @@ static void rtl_hw_start_8411(struct rtl8169_private *tp)
};
rtl_hw_start_8168f(tp);
+ rtl_l23_enable(tp, false);
rtl_ephy_init(tp, e_info_8168f_1, ARRAY_SIZE(e_info_8168f_1));
@@ -5284,6 +5301,8 @@ static void rtl_hw_start_8168g_1(struct rtl8169_private *tp)
rtl_w1w0_eri(tp, 0x2fc, ERIAR_MASK_0001, 0x01, 0x06, ERIAR_EXGMAC);
rtl_w1w0_eri(tp, 0x1b0, ERIAR_MASK_0011, 0x0000, 0x1000, ERIAR_EXGMAC);
+
+ rtl_l23_enable(tp, false);
}
static void rtl_hw_start_8168g_2(struct rtl8169_private *tp)
@@ -5536,6 +5555,8 @@ static void rtl_hw_start_8105e_1(struct rtl8169_private *tp)
RTL_W8(DLLPR, RTL_R8(DLLPR) | PFM_EN);
rtl_ephy_init(tp, e_info_8105e_1, ARRAY_SIZE(e_info_8105e_1));
+
+ rtl_l23_enable(tp, false);
}
static void rtl_hw_start_8105e_2(struct rtl8169_private *tp)
@@ -5571,6 +5592,8 @@ static void rtl_hw_start_8402(struct rtl8169_private *tp)
rtl_eri_write(tp, 0xc0, ERIAR_MASK_0011, 0x0000, ERIAR_EXGMAC);
rtl_eri_write(tp, 0xb8, ERIAR_MASK_0011, 0x0000, ERIAR_EXGMAC);
rtl_w1w0_eri(tp, 0x0d4, ERIAR_MASK_0011, 0x0e00, 0xff00, ERIAR_EXGMAC);
+
+ rtl_l23_enable(tp, false);
}
static void rtl_hw_start_8106(struct rtl8169_private *tp)
@@ -5583,6 +5606,8 @@ static void rtl_hw_start_8106(struct rtl8169_private *tp)
RTL_W32(MISC, (RTL_R32(MISC) | DISABLE_LAN_EN) & ~EARLY_TALLY_EN);
RTL_W8(MCU, RTL_R8(MCU) | EN_NDP | EN_OOB_RESET);
RTL_W8(DLLPR, RTL_R8(DLLPR) & ~PFM_EN);
+
+ rtl_l23_enable(tp, false);
}
static void rtl_hw_start_8101(struct net_device *dev)
--
1.9.3
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH net] r8169: disable L23
2014-07-08 8:26 [PATCH net] r8169: disable L23 Hayes Wang
@ 2014-07-08 20:16 ` Francois Romieu
2014-07-09 6:36 ` Hayes Wang
2014-07-09 2:59 ` David Miller
2014-07-09 6:52 ` [PATCH net v2] " Hayes Wang
2 siblings, 1 reply; 7+ messages in thread
From: Francois Romieu @ 2014-07-08 20:16 UTC (permalink / raw)
To: Hayes Wang; +Cc: netdev, linux-kernel, nic_swsd
Hayes Wang <hayeswang@realtek.com> :
> For RTL8411, RTL8111G, RTL8402, RTL8105, and RTL8106, the nic couldn't
> leave the power saving mode of L23 for certain situation. This causes
> the device lost for the system. Disable the L23 mode to avoid it.
Ok with the patch as it exactly addresses the required hw_start methods
to cover the aforementionned RTL8xyz range.
Can you elaborate what L23 is or should the commit message be reworded
as "Disable a (yet undocumented) power saving feature that randomly
causes RTL8411, RTL8111G, RTL8402, RTL8105, and RTL8106 devices to be
lost on some systems." ?
If nobody really understands what the bug is nor what the specific
circumstances that triggers it are, it would be nice to state it clearly.
If not there is plenty of room to improve the description before it
comes close to full disclosure.
--
Ueimor
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH net] r8169: disable L23
2014-07-08 20:16 ` Francois Romieu
@ 2014-07-09 6:36 ` Hayes Wang
0 siblings, 0 replies; 7+ messages in thread
From: Hayes Wang @ 2014-07-09 6:36 UTC (permalink / raw)
To: Francois Romieu; +Cc: netdev, linux-kernel, nic_swsd
Francois Romieu [mailto:romieu@fr.zoreil.com]
> Sent: Wednesday, July 09, 2014 4:17 AM
> To: Hayes Wang
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; nic_swsd
> Subject: Re: [PATCH net] r8169: disable L23
[...]
> Can you elaborate what L23 is or should the commit message be reworded
> as "Disable a (yet undocumented) power saving feature that randomly
> causes RTL8411, RTL8111G, RTL8402, RTL8105, and RTL8106 devices to be
> lost on some systems." ?
The L2/L3 are the link status which are defined in the specification of the PCI
Express Base Specification for the link state power manager. I don't know them
much, so I don't sure if I could explain them clearly.
Best Regards,
Hayes
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net] r8169: disable L23
2014-07-08 8:26 [PATCH net] r8169: disable L23 Hayes Wang
2014-07-08 20:16 ` Francois Romieu
@ 2014-07-09 2:59 ` David Miller
2014-07-09 6:52 ` [PATCH net v2] " Hayes Wang
2 siblings, 0 replies; 7+ messages in thread
From: David Miller @ 2014-07-09 2:59 UTC (permalink / raw)
To: hayeswang; +Cc: romieu, netdev, linux-kernel, nic_swsd
From: Hayes Wang <hayeswang@realtek.com>
Date: Tue, 8 Jul 2014 16:26:10 +0800
> For RTL8411, RTL8111G, RTL8402, RTL8105, and RTL8106, the nic couldn't
> leave the power saving mode of L23 for certain situation. This causes
> the device lost for the system. Disable the L23 mode to avoid it.
>
> Signed-off-by: Hayes Wang <hayeswang@realtek.com>
I agree with Francois, you need to provide a more detailed changelog
for this patch.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH net v2] r8169: disable L23
2014-07-08 8:26 [PATCH net] r8169: disable L23 Hayes Wang
2014-07-08 20:16 ` Francois Romieu
2014-07-09 2:59 ` David Miller
@ 2014-07-09 6:52 ` Hayes Wang
2014-07-09 22:09 ` Francois Romieu
2 siblings, 1 reply; 7+ messages in thread
From: Hayes Wang @ 2014-07-09 6:52 UTC (permalink / raw)
To: romieu; +Cc: netdev, linux-kernel, nic_swsd, Hayes Wang
For RTL8411, RTL8111G, RTL8402, RTL8105, and RTL8106, disable the feature
of entering the L2/L3 link state of the PCIe. When the nic starts the process
of entering the L2/L3 link state and the PCI reset occurs before the work
is finished, the work would be queued and continue after the next the PCI
reset occurs. This causes the device stays in L2/L3 link state, and the system
couldn't find the device.
Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
drivers/net/ethernet/realtek/r8169.c | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index be425ad..38144ee 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -538,6 +538,7 @@ enum rtl_register_content {
MagicPacket = (1 << 5), /* Wake up when receives a Magic Packet */
LinkUp = (1 << 4), /* Wake up when the cable connection is re-established */
Jumbo_En0 = (1 << 2), /* 8168 only. Reserved in the 8168b */
+ Rdy_to_L23 = (1 << 1), /* L23 Enable */
Beacon_en = (1 << 0), /* 8168 only. Reserved in the 8168b */
/* Config4 register */
@@ -4897,6 +4898,21 @@ static void rtl_enable_clock_request(struct pci_dev *pdev)
PCI_EXP_LNKCTL_CLKREQ_EN);
}
+static void rtl_l23_enable(struct rtl8169_private *tp, bool enable)
+{
+ void __iomem *ioaddr = tp->mmio_addr;
+ u8 data;
+
+ data = RTL_R8(Config3);
+
+ if (enable)
+ data |= Rdy_to_L23;
+ else
+ data &= ~Rdy_to_L23;
+
+ RTL_W8(Config3, data);
+}
+
#define R8168_CPCMD_QUIRK_MASK (\
EnableBist | \
Mac_dbgo_oe | \
@@ -5246,6 +5262,7 @@ static void rtl_hw_start_8411(struct rtl8169_private *tp)
};
rtl_hw_start_8168f(tp);
+ rtl_l23_enable(tp, false);
rtl_ephy_init(tp, e_info_8168f_1, ARRAY_SIZE(e_info_8168f_1));
@@ -5284,6 +5301,8 @@ static void rtl_hw_start_8168g_1(struct rtl8169_private *tp)
rtl_w1w0_eri(tp, 0x2fc, ERIAR_MASK_0001, 0x01, 0x06, ERIAR_EXGMAC);
rtl_w1w0_eri(tp, 0x1b0, ERIAR_MASK_0011, 0x0000, 0x1000, ERIAR_EXGMAC);
+
+ rtl_l23_enable(tp, false);
}
static void rtl_hw_start_8168g_2(struct rtl8169_private *tp)
@@ -5536,6 +5555,8 @@ static void rtl_hw_start_8105e_1(struct rtl8169_private *tp)
RTL_W8(DLLPR, RTL_R8(DLLPR) | PFM_EN);
rtl_ephy_init(tp, e_info_8105e_1, ARRAY_SIZE(e_info_8105e_1));
+
+ rtl_l23_enable(tp, false);
}
static void rtl_hw_start_8105e_2(struct rtl8169_private *tp)
@@ -5571,6 +5592,8 @@ static void rtl_hw_start_8402(struct rtl8169_private *tp)
rtl_eri_write(tp, 0xc0, ERIAR_MASK_0011, 0x0000, ERIAR_EXGMAC);
rtl_eri_write(tp, 0xb8, ERIAR_MASK_0011, 0x0000, ERIAR_EXGMAC);
rtl_w1w0_eri(tp, 0x0d4, ERIAR_MASK_0011, 0x0e00, 0xff00, ERIAR_EXGMAC);
+
+ rtl_l23_enable(tp, false);
}
static void rtl_hw_start_8106(struct rtl8169_private *tp)
@@ -5583,6 +5606,8 @@ static void rtl_hw_start_8106(struct rtl8169_private *tp)
RTL_W32(MISC, (RTL_R32(MISC) | DISABLE_LAN_EN) & ~EARLY_TALLY_EN);
RTL_W8(MCU, RTL_R8(MCU) | EN_NDP | EN_OOB_RESET);
RTL_W8(DLLPR, RTL_R8(DLLPR) & ~PFM_EN);
+
+ rtl_l23_enable(tp, false);
}
static void rtl_hw_start_8101(struct net_device *dev)
--
1.9.3
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH net v2] r8169: disable L23
2014-07-09 6:52 ` [PATCH net v2] " Hayes Wang
@ 2014-07-09 22:09 ` Francois Romieu
2014-07-09 23:42 ` David Miller
0 siblings, 1 reply; 7+ messages in thread
From: Francois Romieu @ 2014-07-09 22:09 UTC (permalink / raw)
To: Hayes Wang; +Cc: davem, netdev, linux-kernel, nic_swsd
Hayes Wang <hayeswang@realtek.com> :
> For RTL8411, RTL8111G, RTL8402, RTL8105, and RTL8106, disable the feature
> of entering the L2/L3 link state of the PCIe. When the nic starts the process
> of entering the L2/L3 link state and the PCI reset occurs before the work
> is finished, the work would be queued and continue after the next the PCI
> reset occurs. This causes the device stays in L2/L3 link state, and the system
> couldn't find the device.
>
> Signed-off-by: Hayes Wang <hayeswang@realtek.com>
Acked-by: Francois Romieu <romieu@fr.zoreil.com>
Thanks, the explanation of the race between software induced PCI reset
and transition to PCIe L2/L3 should be clear enough. PM scared me, yet
it got worse.
Davem, would you mind to s/rtl_l23_enable/rtl_pcie_state_l2l3_enable/ or
do you want a new patch for it ?
--
Ueimor
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net v2] r8169: disable L23
2014-07-09 22:09 ` Francois Romieu
@ 2014-07-09 23:42 ` David Miller
0 siblings, 0 replies; 7+ messages in thread
From: David Miller @ 2014-07-09 23:42 UTC (permalink / raw)
To: romieu; +Cc: hayeswang, netdev, linux-kernel, nic_swsd
From: Francois Romieu <romieu@fr.zoreil.com>
Date: Thu, 10 Jul 2014 00:09:54 +0200
> Hayes Wang <hayeswang@realtek.com> :
>> For RTL8411, RTL8111G, RTL8402, RTL8105, and RTL8106, disable the feature
>> of entering the L2/L3 link state of the PCIe. When the nic starts the process
>> of entering the L2/L3 link state and the PCI reset occurs before the work
>> is finished, the work would be queued and continue after the next the PCI
>> reset occurs. This causes the device stays in L2/L3 link state, and the system
>> couldn't find the device.
>>
>> Signed-off-by: Hayes Wang <hayeswang@realtek.com>
>
> Acked-by: Francois Romieu <romieu@fr.zoreil.com>
>
> Thanks, the explanation of the race between software induced PCI reset
> and transition to PCIe L2/L3 should be clear enough. PM scared me, yet
> it got worse.
>
> Davem, would you mind to s/rtl_l23_enable/rtl_pcie_state_l2l3_enable/ or
> do you want a new patch for it ?
Applied with the function name adjusted, thanks.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-07-09 23:42 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-08 8:26 [PATCH net] r8169: disable L23 Hayes Wang
2014-07-08 20:16 ` Francois Romieu
2014-07-09 6:36 ` Hayes Wang
2014-07-09 2:59 ` David Miller
2014-07-09 6:52 ` [PATCH net v2] " Hayes Wang
2014-07-09 22:09 ` Francois Romieu
2014-07-09 23:42 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.