[PATCH] r8169: remember WOL preferences on driver load

Do not clear Broadcast/Multicast/Unicast Wake Flag or LanWake in
Config5. This is necessary to preserve WOL state when the driver is
loaded. The r8168 vendor driver used to write to both Config1 and
Config5, but in recent versions, this is commented out. Here we keep
writing PMEnable to Config1 because there may be older chips where
PMEnable is not sticky.

Signed-off-by: Peter Wu <redacted>
---
 drivers/net/ethernet/realtek/r8169.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 28af01c..7bdf322 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c

@@ -7093,7 +7093,6 @@ rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	RTL_W8(Cfg9346, Cfg9346_Unlock);
 	RTL_W8(Config1, RTL_R8(Config1) | PMEnable);
-	RTL_W8(Config5, RTL_R8(Config5) & PMEStatus);
 	if ((RTL_R8(Config3) & (LinkUp | MagicPacket)) != 0)
 		tp->features |= RTL_FEATURE_WOL;
 	if ((RTL_R8(Config5) & (UWF | BWF | MWF)) != 0)

-- 
1.8.3.4

Peter Wu [off-list ref] :

Do not clear Broadcast/Multicast/Unicast Wake Flag or LanWake in
Config5. This is necessary to preserve WOL state when the driver is
loaded.

Ok, it makes sense.

The r8168 vendor driver used to write to both Config1 and Config5, but
in recent versions, this is commented out. Here we keep writing PMEnable
to Config1 because there may be older chips where PMEnable is not sticky.

<sneak regression paranoia>
Please include the bits you want to keep in the "&" mask.
</sneak regression paranoia>

Hayes, any advice (especially regarding Config5) ?

[...]

On Tuesday 13 August 2013 23:28:43 Francois Romieu wrote:

quoted

The r8168 vendor driver used to write to both Config1 and Config5, but
in recent versions, this is commented out. Here we keep writing PMEnable
to Config1 because there may be older chips where PMEnable is not sticky.

<sneak regression paranoia>
Please include the bits you want to keep in the "&" mask.
</sneak regression paranoia>

I previously tried:

    RTL_W8(Config5, RTL_R8(Config5) & (BWF | MWF | UWF | LanWake | PMEStatus));

but then I realized that the Realtek r8168 vendor driver does not touch this 
register on load. The bits not included in the above mask are 2, 3 and 7. On 
most datasheets for recent hardware, these are marked reserved.

For RTL8100C(L) (datasheet 1.3 from 2007), RTL8139D(L) (datasheet 1.3 from 
2005) and RTL8101L (1.6 from 2008)
- bit 2 is Link Down Power Saving mode (1=disable, 0=enable; loaded from 
EEPROM on RTL8139D(L))
- bit 3 is FIFOAddrPtr (internal use for testing FIFO SRAM, default 0)
- bit 7 is reserved

I'll let Hayes judge which approach is more appropriate.

Regards,
Peter

Peter Wu [mailto:lekensteyn@gmail.com] 

Sent: Wednesday, August 14, 2013 8:57 PM
To: Francois Romieu
Cc: netdev@vger.kernel.org; nic_swsd; Hayeswang
Subject: Re: [PATCH] r8169: remember WOL preferences on driver load

On Tuesday 13 August 2013 23:28:43 Francois Romieu wrote:

quoted

quoted

The r8168 vendor driver used to write to both Config1 and 

Config5, but

quoted

quoted

in recent versions, this is commented out. Here we keep 

writing PMEnable

quoted

quoted

to Config1 because there may be older chips where 

PMEnable is not sticky.

quoted

<sneak regression paranoia>
Please include the bits you want to keep in the "&" mask.
</sneak regression paranoia>

I previously tried:

    RTL_W8(Config5, RTL_R8(Config5) & (BWF | MWF | UWF | 
LanWake | PMEStatus));

I think this way is better and safe.

but then I realized that the Realtek r8168 vendor driver does 
not touch this 
register on load. The bits not included in the above mask are 
2, 3 and 7. On 
most datasheets for recent hardware, these are marked reserved.

Some settings would be in rtl8168_hw_start() for different chips.
We sure only correct bit would be enabled.

The simple way is to clear bit 2, 3 and 7 for all chips. Or set desired
bit which you want.

 
Best Regards,
Hayes