From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A9D18C4360F for ; Tue, 2 Apr 2019 20:17:08 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 60E5D2070D for ; Tue, 2 Apr 2019 20:17:08 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="PPVjYrjh" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726263AbfDBURH (ORCPT ); Tue, 2 Apr 2019 16:17:07 -0400 Received: from mail-pg1-f193.google.com ([209.85.215.193]:33263 "EHLO mail-pg1-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725822AbfDBURH (ORCPT ); Tue, 2 Apr 2019 16:17:07 -0400 Received: by mail-pg1-f193.google.com with SMTP id k19so433726pgh.0 for ; Tue, 02 Apr 2019 13:17:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:cc:references:from:openpgp:autocrypt:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=QowleVZ0ICnmNvz1bPKKb0xVJSpPK+EaR/eyVVhfirM=; b=PPVjYrjhBRGWDxeZuONTqb71TZL0L2H6t9ropX9AbiLhhsuXAvGEm/2Mbfcu5z/hUp +hL0L6X0/HyPXhBTB24zVl1bV3HUIf/wt4rMSDtCO0Dze9ik2nEz4howM9axcxeL/SNW RDepLZPLTEDZ/5GKZu9OPaM2ITh/+JmRP9yH+G2yd2CV7fH9d+Yi7dmIF/w3+eP2L745 LTd6fwLp6uOq6qb60iNMhp+DO17znfdYlj/i45A4uxqSk1zKzd/LYzn8rD+9mJXi8GzQ prr65bWmfaz9A/HHR5lLerwt4CQ76zBTp80IQEX3NDW9Tp43pb2XU+/hfKGCEZNmMB/C 7nxw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:openpgp:autocrypt :message-id:date:user-agent:mime-version:in-reply-to :content-language:content-transfer-encoding; bh=QowleVZ0ICnmNvz1bPKKb0xVJSpPK+EaR/eyVVhfirM=; b=U0oQoZp33Z1+NOD3tCFdPm8qV4UtgQyQgIDxLeb4Byxmm6c6720PTrbJE5NaSXA5IE fsd7+p5NgTl7bdQ/+8BaFHR62FHvPCcZf0xCsGz9y5bL+G86G39ZfLMvXr/5FVsVxPp3 pwhALEmhMuWYDHW/W3+SZs44nJrsVm2iRxp/5aDvq2V0O2Y7hOBDNtqwUlgvRczKng5x EqQcsHOWIxVjXHJt7joOZCftE0qBDxzfPus27T8Q15gFl3H51PaBsthlQWtKtX0vMxBt hSWxQ01a27URUtdKeqdF8G6FHTKQrhMVoYpH1MoIX26J5cneAWvR6f+S/VIgbB5xVAoo xmwA== X-Gm-Message-State: APjAAAU4+Jgo9W7NeZ4ri0bqOI44Ppcb4tCOFdcLyKmh5Jr7HE93NWMc QqlmEUgjBCxL5SduOIvKqThOMOnf X-Google-Smtp-Source: APXvYqxVSHvhVcihsgcC4cAPaZlVwsAms/RNTGw21599HWHnW28B5enQ4IExWFJyXdy70EnTnniJ+w== X-Received: by 2002:a62:4610:: with SMTP id t16mr72404127pfa.217.1554236225166; Tue, 02 Apr 2019 13:17:05 -0700 (PDT) Received: from [10.67.49.126] ([192.19.223.250]) by smtp.googlemail.com with ESMTPSA id q87sm22636684pfa.133.2019.04.02.13.17.03 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 02 Apr 2019 13:17:04 -0700 (PDT) Subject: Re: [PATCH net] r8169: switch off ASPM by default and add sysfs attribute to control ASPM To: Heiner Kallweit , David Miller , Realtek linux nic maintainers Cc: "netdev@vger.kernel.org" References: <275eb60f-8cfe-9ff6-97bc-39d9ef280d36@gmail.com> From: Florian Fainelli Openpgp: preference=signencrypt Autocrypt: addr=f.fainelli@gmail.com; prefer-encrypt=mutual; keydata= mQGiBEjPuBIRBACW9MxSJU9fvEOCTnRNqG/13rAGsj+vJqontvoDSNxRgmafP8d3nesnqPyR xGlkaOSDuu09rxuW+69Y2f1TzjFuGpBk4ysWOR85O2Nx8AJ6fYGCoeTbovrNlGT1M9obSFGQ X3IzRnWoqlfudjTO5TKoqkbOgpYqIo5n1QbEjCCwCwCg3DOH/4ug2AUUlcIT9/l3pGvoRJ0E AICDzi3l7pmC5IWn2n1mvP5247urtHFs/uusE827DDj3K8Upn2vYiOFMBhGsxAk6YKV6IP0d ZdWX6fqkJJlu9cSDvWtO1hXeHIfQIE/xcqvlRH783KrihLcsmnBqOiS6rJDO2x1eAgC8meAX SAgsrBhcgGl2Rl5gh/jkeA5ykwbxA/9u1eEuL70Qzt5APJmqVXR+kWvrqdBVPoUNy/tQ8mYc nzJJ63ng3tHhnwHXZOu8hL4nqwlYHRa9eeglXYhBqja4ZvIvCEqSmEukfivk+DlIgVoOAJbh qIWgvr3SIEuR6ayY3f5j0f2ejUMYlYYnKdiHXFlF9uXm1ELrb0YX4GMHz7QnRmxvcmlhbiBG YWluZWxsaSA8Zi5mYWluZWxsaUBnbWFpbC5jb20+iGYEExECACYCGyMGCwkIBwMCBBUCCAME FgIDAQIeAQIXgAUCVF/S8QUJHlwd3wAKCRBhV5kVtWN2DvCVAJ4u4/bPF4P3jxb4qEY8I2gS 6hG0gACffNWlqJ2T4wSSn+3o7CCZNd7SLSC5BA0ESM+4EhAQAL/o09boR9D3Vk1Tt7+gpYr3 WQ6hgYVON905q2ndEoA2J0dQxJNRw3snabHDDzQBAcqOvdi7YidfBVdKi0wxHhSuRBfuOppu pdXkb7zxuPQuSveCLqqZWRQ+Cc2QgF7SBqgznbe6Ngout5qXY5Dcagk9LqFNGhJQzUGHAsIs hap1f0B1PoUyUNeEInV98D8Xd/edM3mhO9nRpUXRK9Bvt4iEZUXGuVtZLT52nK6Wv2EZ1TiT OiqZlf1P+vxYLBx9eKmabPdm3yjalhY8yr1S1vL0gSA/C6W1o/TowdieF1rWN/MYHlkpyj9c Rpc281gAO0AP3V1G00YzBEdYyi0gaJbCEQnq8Vz1vDXFxHzyhgGz7umBsVKmYwZgA8DrrB0M oaP35wuGR3RJcaG30AnJpEDkBYHznI2apxdcuTPOHZyEilIRrBGzDwGtAhldzlBoBwE3Z3MY 31TOpACu1ZpNOMysZ6xiE35pWkwc0KYm4hJA5GFfmWSN6DniimW3pmdDIiw4Ifcx8b3mFrRO BbDIW13E51j9RjbO/nAaK9ndZ5LRO1B/8Fwat7bLzmsCiEXOJY7NNpIEpkoNoEUfCcZwmLrU +eOTPzaF6drw6ayewEi5yzPg3TAT6FV3oBsNg3xlwU0gPK3v6gYPX5w9+ovPZ1/qqNfOrbsE FRuiSVsZQ5s3AAMFD/9XjlnnVDh9GX/r/6hjmr4U9tEsM+VQXaVXqZuHKaSmojOLUCP/YVQo 7IiYaNssCS4FCPe4yrL4FJJfJAsbeyDykMN7wAnBcOkbZ9BPJPNCbqU6dowLOiy8AuTYQ48m vIyQ4Ijnb6GTrtxIUDQeOBNuQC/gyyx3nbL/lVlHbxr4tb6YkhkO6shjXhQh7nQb33FjGO4P WU11Nr9i/qoV8QCo12MQEo244RRA6VMud06y/E449rWZFSTwGqb0FS0seTcYNvxt8PB2izX+ HZA8SL54j479ubxhfuoTu5nXdtFYFj5Lj5x34LKPx7MpgAmj0H7SDhpFWF2FzcC1bjiW9mjW HaKaX23Awt97AqQZXegbfkJwX2Y53ufq8Np3e1542lh3/mpiGSilCsaTahEGrHK+lIusl6mz Joil+u3k01ofvJMK0ZdzGUZ/aPMZ16LofjFA+MNxWrZFrkYmiGdv+LG45zSlZyIvzSiG2lKy kuVag+IijCIom78P9jRtB1q1Q5lwZp2TLAJlz92DmFwBg1hyFzwDADjZ2nrDxKUiybXIgZp9 aU2d++ptEGCVJOfEW4qpWCCLPbOT7XBr+g/4H3qWbs3j/cDDq7LuVYIe+wchy/iXEJaQVeTC y5arMQorqTFWlEOgRA8OP47L9knl9i4xuR0euV6DChDrguup2aJVU4hPBBgRAgAPAhsMBQJU X9LxBQkeXB3fAAoJEGFXmRW1Y3YOj4UAn3nrFLPZekMeqX5aD/aq/dsbXSfyAKC45Go0YyxV HGuUuzv+GKZ6nsysJ7kCDQRXG8fwARAA6q/pqBi5PjHcOAUgk2/2LR5LjjesK50bCaD4JuNc YDhFR7Vs108diBtsho3w8WRd9viOqDrhLJTroVckkk74OY8r+3t1E0Dd4wHWHQZsAeUvOwDM PQMqTUBFuMi6ydzTZpFA2wBR9x6ofl8Ax+zaGBcFrRlQnhsuXLnM1uuvS39+pmzIjasZBP2H UPk5ifigXcpelKmj6iskP3c8QN6x6GjUSmYx+xUfs/GNVSU1XOZn61wgPDbgINJd/THGdqiO iJxCLuTMqlSsmh1+E1dSdfYkCb93R/0ZHvMKWlAx7MnaFgBfsG8FqNtZu3PCLfizyVYYjXbV WO1A23riZKqwrSJAATo5iTS65BuYxrFsFNPrf7TitM8E76BEBZk0OZBvZxMuOs6Z1qI8YKVK UrHVGFq3NbuPWCdRul9SX3VfOunr9Gv0GABnJ0ET+K7nspax0xqq7zgnM71QEaiaH17IFYGS sG34V7Wo3vyQzsk7qLf9Ajno0DhJ+VX43g8+AjxOMNVrGCt9RNXSBVpyv2AMTlWCdJ5KI6V4 KEzWM4HJm7QlNKE6RPoBxJVbSQLPd9St3h7mxLcne4l7NK9eNgNnneT7QZL8fL//s9K8Ns1W t60uQNYvbhKDG7+/yLcmJgjF74XkGvxCmTA1rW2bsUriM533nG9gAOUFQjURkwI8jvMAEQEA AYkCaAQYEQIACQUCVxvH8AIbAgIpCRBhV5kVtWN2DsFdIAQZAQIABgUCVxvH8AAKCRCH0Jac RAcHBIkHD/9nmfog7X2ZXMzL9ktT++7x+W/QBrSTCTmq8PK+69+INN1ZDOrY8uz6htfTLV9+ e2W6G8/7zIvODuHk7r+yQ585XbplgP0V5Xc8iBHdBgXbqnY5zBrcH+Q/oQ2STalEvaGHqNoD UGyLQ/fiKoLZTPMur57Fy1c9rTuKiSdMgnT0FPfWVDfpR2Ds0gpqWePlRuRGOoCln5GnREA/ 2MW2rWf+CO9kbIR+66j8b4RUJqIK3dWn9xbENh/aqxfonGTCZQ2zC4sLd25DQA4w1itPo+f5 V/SQxuhnlQkTOCdJ7b/mby/pNRz1lsLkjnXueLILj7gNjwTabZXYtL16z24qkDTI1x3g98R/ xunb3/fQwR8FY5/zRvXJq5us/nLvIvOmVwZFkwXc+AF+LSIajqQz9XbXeIP/BDjlBNXRZNdo dVuSU51ENcMcilPr2EUnqEAqeczsCGpnvRCLfVQeSZr2L9N4svNhhfPOEscYhhpHTh0VPyxI pPBNKq+byuYPMyk3nj814NKhImK0O4gTyCK9b+gZAVvQcYAXvSouCnTZeJRrNHJFTgTgu6E0 caxTGgc5zzQHeX67eMzrGomG3ZnIxmd1sAbgvJUDaD2GrYlulfwGWwWyTNbWRvMighVdPkSF 6XFgQaosWxkV0OELLy2N485YrTr2Uq64VKyxpncLh50e2RnyAJ9Za0Dx0yyp44iD1OvHtkEI M5kY0ACeNhCZJvZ5g4C2Lc9fcTHu8jxmEkI= Message-ID: <0640ba80-519f-ab76-a86d-e42833df46fb@gmail.com> Date: Tue, 2 Apr 2019 13:16:57 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 MIME-Version: 1.0 In-Reply-To: <275eb60f-8cfe-9ff6-97bc-39d9ef280d36@gmail.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On 4/2/19 12:55 PM, Heiner Kallweit wrote: > There are numerous reports about different problems caused by ASPM > incompatibilities between certain network chip versions and board > chipsets. On the other hand on (especially mobile) systems where ASPM > works properly it can significantly contribute to power-saving and > increased battery runtime. > One problem so far to make ASPM configurable was to find an acceptable > way of configuration (e.g. module parameters are discouraged for that > purpose). > > As a new attempt let's switch off ASPM per default and make it > configurable by a sysfs attribute. The attribute is documented in > new file Documentation/networking/device_drivers/realtek/r8169.txt. I am not sure this is where it should be solved, there is definitively a device specific aspect to properly supporting the enabling of ASPM L0s, L1s etc, but the actual sysfs knobs should belong to the pci_device itself, since this is something that likely other drivers would want to be able to expose. You would probably want to work with the PCI maintainers to come up with a standard solution that applies beyond just r8169 since presumably there must be a gazillion of devices with the same issues. > > Fixes: a99790bf5c7f ("r8169: Reinstate ASPM Support") > Signed-off-by: Heiner Kallweit > --- > .../device_drivers/realtek/r8169.txt | 19 +++++ > drivers/net/ethernet/realtek/r8169.c | 75 +++++++++++++++++-- > 2 files changed, 86 insertions(+), 8 deletions(-) > create mode 100644 Documentation/networking/device_drivers/realtek/r8169.txt > > diff --git a/Documentation/networking/device_drivers/realtek/r8169.txt b/Documentation/networking/device_drivers/realtek/r8169.txt > new file mode 100644 > index 000000000..669995d0c > --- /dev/null > +++ b/Documentation/networking/device_drivers/realtek/r8169.txt > @@ -0,0 +1,19 @@ > +Written by Heiner Kallweit > + > +Version 04/02/2019 > + > +Driver-specific sysfs attributes > +================================ > + > +rtl_aspm (bool) > +--------------- > + > +Certain combinations of network chip versions and board chipsets result in > +increased packet latency, PCIe errors, or significantly reduced network > +performance. Therefore ASPM is off by default. On the other hand ASPM can > +significantly contribute to power-saving and thus increased battery > +runtime on notebooks. Therefore this sysfs attribute allows to switch on > +ASPM on systems where ASPM works properly. The attribute accepts any form > +of bool value, e.g. 1/y/on. See also kerneldoc of kstrtobool(). > +Note that the attribute is accessible only if interface is up. > +Else network chip and PCIe link may be powered-down and not reachable. > diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c > index 19efa88f3..e2de94dc3 100644 > --- a/drivers/net/ethernet/realtek/r8169.c > +++ b/drivers/net/ethernet/realtek/r8169.c > @@ -678,6 +678,8 @@ struct rtl8169_private { > struct work_struct work; > } wk; > > + unsigned aspm_supported:1; > + unsigned aspm_enabled:1; > unsigned irq_enabled:1; > unsigned supports_gmii:1; > dma_addr_t counters_phys_addr; > @@ -5091,7 +5093,7 @@ static void rtl_hw_start_8168e_2(struct rtl8169_private *tp) > RTL_W32(tp, MISC, RTL_R32(tp, MISC) | PWM_EN); > RTL_W8(tp, Config5, RTL_R8(tp, Config5) & ~Spi_en); > > - rtl_hw_aspm_clkreq_enable(tp, true); > + tp->aspm_supported = 1; > } > > static void rtl_hw_start_8168f(struct rtl8169_private *tp) > @@ -5205,7 +5207,7 @@ static void rtl_hw_start_8168g_1(struct rtl8169_private *tp) > /* disable aspm and clock request before access ephy */ > rtl_hw_aspm_clkreq_enable(tp, false); > rtl_ephy_init(tp, e_info_8168g_1, ARRAY_SIZE(e_info_8168g_1)); > - rtl_hw_aspm_clkreq_enable(tp, true); > + tp->aspm_supported = 1; > } > > static void rtl_hw_start_8168g_2(struct rtl8169_private *tp) > @@ -5240,7 +5242,7 @@ static void rtl_hw_start_8411_2(struct rtl8169_private *tp) > /* disable aspm and clock request before access ephy */ > rtl_hw_aspm_clkreq_enable(tp, false); > rtl_ephy_init(tp, e_info_8411_2, ARRAY_SIZE(e_info_8411_2)); > - rtl_hw_aspm_clkreq_enable(tp, true); > + tp->aspm_supported = 1; > } > > static void rtl_hw_start_8168h_1(struct rtl8169_private *tp) > @@ -5337,7 +5339,7 @@ static void rtl_hw_start_8168h_1(struct rtl8169_private *tp) > r8168_mac_ocp_write(tp, 0xc094, 0x0000); > r8168_mac_ocp_write(tp, 0xc09e, 0x0000); > > - rtl_hw_aspm_clkreq_enable(tp, true); > + tp->aspm_supported = 1; > } > > static void rtl_hw_start_8168ep(struct rtl8169_private *tp) > @@ -5394,7 +5396,7 @@ static void rtl_hw_start_8168ep_1(struct rtl8169_private *tp) > > rtl_hw_start_8168ep(tp); > > - rtl_hw_aspm_clkreq_enable(tp, true); > + tp->aspm_supported = 1; > } > > static void rtl_hw_start_8168ep_2(struct rtl8169_private *tp) > @@ -5414,7 +5416,7 @@ static void rtl_hw_start_8168ep_2(struct rtl8169_private *tp) > RTL_W8(tp, DLLPR, RTL_R8(tp, DLLPR) & ~PFM_EN); > RTL_W8(tp, MISC_1, RTL_R8(tp, MISC_1) & ~PFM_D3COLD_EN); > > - rtl_hw_aspm_clkreq_enable(tp, true); > + tp->aspm_supported = 1; > } > > static void rtl_hw_start_8168ep_3(struct rtl8169_private *tp) > @@ -5449,7 +5451,7 @@ static void rtl_hw_start_8168ep_3(struct rtl8169_private *tp) > data |= 0x0080; > r8168_mac_ocp_write(tp, 0xe860, data); > > - rtl_hw_aspm_clkreq_enable(tp, true); > + tp->aspm_supported = 1; > } > > static void rtl_hw_start_8168(struct rtl8169_private *tp) > @@ -5696,7 +5698,7 @@ static void rtl_hw_start_8106(struct rtl8169_private *tp) > RTL_W8(tp, DLLPR, RTL_R8(tp, DLLPR) & ~PFM_EN); > > rtl_pcie_state_l2l3_disable(tp); > - rtl_hw_aspm_clkreq_enable(tp, true); > + tp->aspm_supported = 1; > } > > static void rtl_hw_start_8101(struct rtl8169_private *tp) > @@ -7097,6 +7099,49 @@ static const struct rtl_cfg_info { > } > }; > > +static ssize_t rtl_aspm_show(struct device *dev, struct device_attribute *attr, > + char *buf) > +{ > + struct net_device *ndev = dev_get_drvdata(dev); > + struct rtl8169_private *tp = netdev_priv(ndev); > + > + return snprintf(buf, PAGE_SIZE, "%d\n", tp->aspm_enabled); > +} > + > +static ssize_t rtl_aspm_store(struct device *dev, struct device_attribute *attr, > + const char *buf, size_t size) > +{ > + struct net_device *ndev = dev_get_drvdata(dev); > + struct rtl8169_private *tp = netdev_priv(ndev); > + bool aspm; > + > + if (!tp->aspm_supported) > + return -EOPNOTSUPP; > + > + if (strtobool(buf, &aspm) < 0) > + return -EINVAL; > + > + pm_runtime_get_noresume(dev); > + > + if (!pm_runtime_active(dev)) { > + size = -ENETDOWN; > + goto out; > + } > + > + rtl_lock_work(tp); > + rtl_unlock_config_regs(tp); > + rtl_hw_aspm_clkreq_enable(tp, aspm); > + tp->aspm_enabled = aspm ? 1 : 0; > + rtl_lock_config_regs(tp); > + rtl_unlock_work(tp); > +out: > + pm_runtime_put_noidle(dev); > + > + return size; > +} > + > +static DEVICE_ATTR_RW(rtl_aspm); > + > static int rtl_alloc_irq(struct rtl8169_private *tp) > { > unsigned int flags; > @@ -7325,6 +7370,11 @@ static int rtl_get_ether_clk(struct rtl8169_private *tp) > return rc; > } > > +static void rtl_remove_sysfs_aspm_file(void *data) > +{ > + device_remove_file(data, &dev_attr_rtl_aspm); > +} > + > static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) > { > const struct rtl_cfg_info *cfg = rtl_cfg_infos + ent->driver_data; > @@ -7498,6 +7548,15 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) > > pci_set_drvdata(pdev, dev); > > + rc = device_create_file(&pdev->dev, &dev_attr_rtl_aspm); > + if (rc) > + return rc; > + > + rc = devm_add_action_or_reset(&pdev->dev, rtl_remove_sysfs_aspm_file, > + &pdev->dev); > + if (rc) > + return rc; > + > rc = r8169_mdio_register(tp); > if (rc) > return rc; > -- Florian