netdev - RE: [PATCH net-next] r8169: add rtl_disable_exit

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <0124f075142a458d91e5b41ce3b0ed5a@realtek.com>
Date:   Fri, 28 Jan 2022 08:09:03 +0000
From:   Hau <hau@...ltek.com>
To:     Heiner Kallweit <hkallweit1@...il.com>,
        Jakub Kicinski <kuba@...nel.org>,
        David Miller <davem@...emloft.net>,
        nic_swsd <nic_swsd@...ltek.com>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "grundler@...omium.org" <grundler@...omium.org>
Subject: RE: [PATCH net-next] r8169: add rtl_disable_exit_l1()



> -----Original Message-----
> From: Heiner Kallweit [mailto:hkallweit1@...il.com]
> Sent: Friday, January 28, 2022 6:15 AM
> To: Jakub Kicinski <kuba@...nel.org>; David Miller <davem@...emloft.net>;
> nic_swsd <nic_swsd@...ltek.com>
> Cc: netdev@...r.kernel.org; Hau <hau@...ltek.com>
> Subject: [PATCH net-next] r8169: add rtl_disable_exit_l1()
> 
> Add rtl_disable_exit_l1() for ensuring that the chip doesn't inadvertently exit
> ASPM L1 when being in a low-power mode.
> The new function is called from rtl_prepare_power_down() which has to be
> moved in the code to avoid a forward declaration.
> 
> According to Realtek OCP register 0xc0ac shadows ERI register 0xd4 on
> RTL8168 versions from RTL8168g. This allows to simplify the code a little.
> 
> Suggested-by: Chun-Hao Lin <hau@...ltek.com>
> Signed-off-by: Heiner Kallweit <hkallweit1@...il.com>
> ---
>  drivers/net/ethernet/realtek/r8169_main.c | 65 ++++++++++++++---------
>  1 file changed, 39 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/net/ethernet/realtek/r8169_main.c
> b/drivers/net/ethernet/realtek/r8169_main.c
> index 3c3d1506b..104ebc0fb 100644
> --- a/drivers/net/ethernet/realtek/r8169_main.c
> +++ b/drivers/net/ethernet/realtek/r8169_main.c
> @@ -2231,28 +2231,6 @@ static int rtl_set_mac_address(struct net_device
> *dev, void *p)
>  	return 0;
>  }
> 
> -static void rtl_wol_enable_rx(struct rtl8169_private *tp) -{
> -	if (tp->mac_version >= RTL_GIGA_MAC_VER_25)
> -		RTL_W32(tp, RxConfig, RTL_R32(tp, RxConfig) |
> -			AcceptBroadcast | AcceptMulticast | AcceptMyPhys);
> -}
> -
> -static void rtl_prepare_power_down(struct rtl8169_private *tp) -{
> -	if (tp->dash_type != RTL_DASH_NONE)
> -		return;
> -
> -	if (tp->mac_version == RTL_GIGA_MAC_VER_32 ||
> -	    tp->mac_version == RTL_GIGA_MAC_VER_33)
> -		rtl_ephy_write(tp, 0x19, 0xff64);
> -
> -	if (device_may_wakeup(tp_to_dev(tp))) {
> -		phy_speed_down(tp->phydev, false);
> -		rtl_wol_enable_rx(tp);
> -	}
> -}
> -
>  static void rtl_init_rxcfg(struct rtl8169_private *tp)  {
>  	switch (tp->mac_version) {
> @@ -2667,10 +2645,7 @@ static void rtl_enable_exit_l1(struct
> rtl8169_private *tp)
>  	case RTL_GIGA_MAC_VER_37 ... RTL_GIGA_MAC_VER_38:
>  		rtl_eri_set_bits(tp, 0xd4, 0x0c00);
>  		break;
> -	case RTL_GIGA_MAC_VER_40 ... RTL_GIGA_MAC_VER_53:
> -		rtl_eri_set_bits(tp, 0xd4, 0x1f80);
> -		break;
> -	case RTL_GIGA_MAC_VER_60 ... RTL_GIGA_MAC_VER_63:
> +	case RTL_GIGA_MAC_VER_40 ... RTL_GIGA_MAC_VER_63:
>  		r8168_mac_ocp_modify(tp, 0xc0ac, 0, 0x1f80);
>  		break;
>  	default:
> @@ -2678,6 +2653,20 @@ static void rtl_enable_exit_l1(struct
> rtl8169_private *tp)
>  	}
>  }
> 
> +static void rtl_disable_exit_l1(struct rtl8169_private *tp) {
> +	switch (tp->mac_version) {
> +	case RTL_GIGA_MAC_VER_34 ... RTL_GIGA_MAC_VER_38:
> +		rtl_eri_clear_bits(tp, 0xd4, 0x1f00);
> +		break;
> +	case RTL_GIGA_MAC_VER_40 ... RTL_GIGA_MAC_VER_63:
> +		r8168_mac_ocp_modify(tp, 0xc0ac, 0x1f80, 0);
> +		break;
> +	default:
> +		break;
> +	}
> +}
> +
>  static void rtl_hw_aspm_clkreq_enable(struct rtl8169_private *tp, bool
> enable)  {
>  	/* Don't enable ASPM in the chip if OS can't control ASPM */ @@ -
> 4689,6 +4678,30 @@ static int r8169_phy_connect(struct rtl8169_private *tp)
>  	return 0;
>  }
> 
> +static void rtl_wol_enable_rx(struct rtl8169_private *tp) {
> +	if (tp->mac_version >= RTL_GIGA_MAC_VER_25)
> +		RTL_W32(tp, RxConfig, RTL_R32(tp, RxConfig) |
> +			AcceptBroadcast | AcceptMulticast | AcceptMyPhys); }
> +
> +static void rtl_prepare_power_down(struct rtl8169_private *tp) {
> +	if (tp->dash_type != RTL_DASH_NONE)
> +		return;
> +
> +	if (tp->mac_version == RTL_GIGA_MAC_VER_32 ||
> +	    tp->mac_version == RTL_GIGA_MAC_VER_33)
> +		rtl_ephy_write(tp, 0x19, 0xff64);
> +
> +	if (device_may_wakeup(tp_to_dev(tp))) {
> +		phy_speed_down(tp->phydev, false);
> +		rtl_wol_enable_rx(tp);
> +	} else {
> +		rtl_disable_exit_l1(tp);
> +	}
> +}
> +
Hi Heiner,

rtl_disable_exit_l1(tp) can be called before device enter D3 state. I think you don’t need to check wol status.
You may update the code link following. 

static void rtl_prepare_power_down(struct rtl8169_private *tp) {
	rtl_disable_exit_l1(tp);

	if (tp->dash_type != RTL_DASH_NONE)
		return;

	if (tp->mac_version == RTL_GIGA_MAC_VER_32 ||
	    tp->mac_version == RTL_GIGA_MAC_VER_33)
		rtl_ephy_write(tp, 0x19, 0xff64);

	if (device_may_wakeup(tp_to_dev(tp))) {
		phy_speed_down(tp->phydev, false);
		rtl_wol_enable_rx(tp);
	} 
}

Thanks.
>  static void rtl8169_down(struct rtl8169_private *tp)  {
>  	/* Clear all task flags */
> --
> 2.35.0
> 
> ------Please consider the environment before printing this e-mail.