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 <lekensteyn@gmail.com>
---
 drivers/net/ethernet/realtek/r8169.c | 1 -
 1 file changed, 1 deletion(-)

Peter Wu <lekensteyn@gmail.com> :
> 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) ?

[...]
> 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
--
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 Tuesday 13 August 2013 23:28:43 Francois Romieu wrote:
> > 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
--
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

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:
> > > 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));

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

--
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