* [PATCH] r8169: Add quirks to enable ASPM on Dell platforms
@ 2024-12-08 19:10 Guy Chronister
2024-12-08 21:01 ` Heiner Kallweit
2024-12-08 21:54 ` Heiner Kallweit
0 siblings, 2 replies; 3+ messages in thread
From: Guy Chronister @ 2024-12-08 19:10 UTC (permalink / raw)
To: hkallweit1
Cc: nic_swsd, andrew+netdev, davem, edumazet, kuba, pabeni, netdev,
linux-kernel, Guy Chronister, Koba Ko, Timo Aaltonen,
Andrea Righi
Some non-Dell platforms equipped with r8168h/r8111 have issues with ASPM. It's very hard to fix all known issues in a short time and r8168h/r8111 is not a brand new NIC chip, so introduce the quirk for Dell platforms. It's also easier to track the Dell platform and ask for Realtek's effort.
Make the original matching logic more explicit.
Signed-off-by: Koba Ko <koba.ko@canonical.com>
Signed-off-by: Timo Aaltonen <timo.aaltonen@canonical.com>
Signed-off-by: Andrea Righi <andrea.righi@canonical.com>
Signed-off-by: Guy Chronister <guyc.linux.patches@gmail.com>
---
drivers/net/ethernet/realtek/r8169_main.c | 83 ++++++++++++++++++++++-
1 file changed, 80 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index 6934bdee2a91..3c1cf704492f 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -15,6 +15,7 @@
#include <linux/etherdevice.h>
#include <linux/clk.h>
#include <linux/delay.h>
+#include <linux/dmi.h>
#include <linux/ethtool.h>
#include <linux/hwmon.h>
#include <linux/phy.h>
@@ -5322,13 +5323,89 @@ static void rtl_init_mac_address(struct rtl8169_private *tp)
rtl_rar_set(tp, mac_addr);
}
+static bool rtl_aspm_dell_workaround(struct rtl8169_private *tp)
+{
+ static const struct dmi_system_id sysids[] = {
+ {
+ .ident = "Dell",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "Vostro 16 5640"),
+ DMI_MATCH(DMI_PRODUCT_SKU, "0CA0"),
+ },
+ },
+ {
+ .ident = "Dell",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "Vostro 14 3440"),
+ DMI_MATCH(DMI_PRODUCT_SKU, "0CA5"),
+ },
+ },
+ {
+ .ident = "Dell",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "Vostro 14 3440"),
+ DMI_MATCH(DMI_PRODUCT_SKU, "0CA6"),
+ },
+ },
+ {
+ .ident = "Dell",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "Latitude 3450"),
+ DMI_MATCH(DMI_PRODUCT_SKU, "0C99"),
+ },
+ },
+ {
+ .ident = "Dell",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "Latitude 3450"),
+ DMI_MATCH(DMI_PRODUCT_SKU, "0C97"),
+ },
+ },
+ {
+ .ident = "Dell",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "Latitude 3550"),
+ DMI_MATCH(DMI_PRODUCT_SKU, "0C9A"),
+ },
+ },
+ {
+ .ident = "Dell",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "Latitude 3550"),
+ DMI_MATCH(DMI_PRODUCT_SKU, "0C98"),
+ },
+ },
+ {}
+ };
+
+ if (tp->mac_version == RTL_GIGA_MAC_VER_46 && dmi_check_system(sysids))
+ return true;
+
+ return false;
+}
+
/* register is set if system vendor successfully tested ASPM 1.2 */
static bool rtl_aspm_is_safe(struct rtl8169_private *tp)
{
- if (tp->mac_version >= RTL_GIGA_MAC_VER_61 &&
- r8168_mac_ocp_read(tp, 0xc0b2) & 0xf)
+ /* definition of 0xc0b2,
+ * 0: L1
+ * 1: ASPM L1.0
+ * 2: ASPM L0s
+ * 3: CLKEREQ
+ * 4-7: Reserved
+ */
+ if ((tp->mac_version >= RTL_GIGA_MAC_VER_61 &&
+ (r8168_mac_ocp_read(tp, 0xc0b2) & 0x0F)) ||
+ rtl_aspm_dell_workaround(tp)) {
return true;
-
+ }
return false;
}
--
2.45.2
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] r8169: Add quirks to enable ASPM on Dell platforms
2024-12-08 19:10 [PATCH] r8169: Add quirks to enable ASPM on Dell platforms Guy Chronister
@ 2024-12-08 21:01 ` Heiner Kallweit
2024-12-08 21:54 ` Heiner Kallweit
1 sibling, 0 replies; 3+ messages in thread
From: Heiner Kallweit @ 2024-12-08 21:01 UTC (permalink / raw)
To: Guy Chronister
Cc: nic_swsd, andrew+netdev, davem, edumazet, kuba, pabeni, netdev,
linux-kernel, Koba Ko, Timo Aaltonen, Andrea Righi
On 08.12.2024 20:10, Guy Chronister wrote:
> Some non-Dell platforms equipped with r8168h/r8111 have issues with ASPM. It's very hard to fix all known issues in a short time and r8168h/r8111 is not a brand new NIC chip, so introduce the quirk for Dell platforms. It's also easier to track the Dell platform and ask for Realtek's effort.
Which known issues do you refer to? If it's very hard to fix all, then fix some.
Did you verify that ASPM L1.2 doesn't result in missed rx packets even under heavy load?
Check chip hw stats. I have a test system with RTL8168h which works fine up to ASPM L1.1,
but misses rx packets with ASPM L1.2 under heavy load.
> Make the original matching logic more explicit.
>
> Signed-off-by: Koba Ko <koba.ko@canonical.com>
> Signed-off-by: Timo Aaltonen <timo.aaltonen@canonical.com>
> Signed-off-by: Andrea Righi <andrea.righi@canonical.com>
> Signed-off-by: Guy Chronister <guyc.linux.patches@gmail.com>
> ---
> drivers/net/ethernet/realtek/r8169_main.c | 83 ++++++++++++++++++++++-
> 1 file changed, 80 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
> index 6934bdee2a91..3c1cf704492f 100644
> --- a/drivers/net/ethernet/realtek/r8169_main.c
> +++ b/drivers/net/ethernet/realtek/r8169_main.c
> @@ -15,6 +15,7 @@
> #include <linux/etherdevice.h>
> #include <linux/clk.h>
> #include <linux/delay.h>
> +#include <linux/dmi.h>
> #include <linux/ethtool.h>
> #include <linux/hwmon.h>
> #include <linux/phy.h>
> @@ -5322,13 +5323,89 @@ static void rtl_init_mac_address(struct rtl8169_private *tp)
> rtl_rar_set(tp, mac_addr);
> }
>
> +static bool rtl_aspm_dell_workaround(struct rtl8169_private *tp)
> +{
> + static const struct dmi_system_id sysids[] = {
> + {
> + .ident = "Dell",
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> + DMI_MATCH(DMI_PRODUCT_NAME, "Vostro 16 5640"),
> + DMI_MATCH(DMI_PRODUCT_SKU, "0CA0"),
> + },
> + },
> + {
> + .ident = "Dell",
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> + DMI_MATCH(DMI_PRODUCT_NAME, "Vostro 14 3440"),
> + DMI_MATCH(DMI_PRODUCT_SKU, "0CA5"),
> + },
> + },
> + {
> + .ident = "Dell",
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> + DMI_MATCH(DMI_PRODUCT_NAME, "Vostro 14 3440"),
> + DMI_MATCH(DMI_PRODUCT_SKU, "0CA6"),
> + },
> + },
> + {
> + .ident = "Dell",
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> + DMI_MATCH(DMI_PRODUCT_NAME, "Latitude 3450"),
> + DMI_MATCH(DMI_PRODUCT_SKU, "0C99"),
> + },
> + },
> + {
> + .ident = "Dell",
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> + DMI_MATCH(DMI_PRODUCT_NAME, "Latitude 3450"),
> + DMI_MATCH(DMI_PRODUCT_SKU, "0C97"),
> + },
> + },
> + {
> + .ident = "Dell",
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> + DMI_MATCH(DMI_PRODUCT_NAME, "Latitude 3550"),
> + DMI_MATCH(DMI_PRODUCT_SKU, "0C9A"),
> + },
> + },
> + {
> + .ident = "Dell",
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> + DMI_MATCH(DMI_PRODUCT_NAME, "Latitude 3550"),
> + DMI_MATCH(DMI_PRODUCT_SKU, "0C98"),
> + },
> + },
> + {}
> + };
> +
> + if (tp->mac_version == RTL_GIGA_MAC_VER_46 && dmi_check_system(sysids))
> + return true;
> +
> + return false;
> +}
> +
> /* register is set if system vendor successfully tested ASPM 1.2 */
> static bool rtl_aspm_is_safe(struct rtl8169_private *tp)
> {
> - if (tp->mac_version >= RTL_GIGA_MAC_VER_61 &&
> - r8168_mac_ocp_read(tp, 0xc0b2) & 0xf)
> + /* definition of 0xc0b2,
> + * 0: L1
> + * 1: ASPM L1.0
> + * 2: ASPM L0s
> + * 3: CLKEREQ
> + * 4-7: Reserved
> + */
> + if ((tp->mac_version >= RTL_GIGA_MAC_VER_61 &&
> + (r8168_mac_ocp_read(tp, 0xc0b2) & 0x0F)) ||
> + rtl_aspm_dell_workaround(tp)) {
> return true;
> -
> + }
> return false;
> }
>
This approach doesn't scale. I don't want to end up with hundreds of such dmi checks.
I understand that the driver works as-is, you just want to enable ASPM per default
on these systems. You have two options to enable ASPM:
1. Check with Realtek whether this register 0xc0b2 (or an equivalent) can be used on
RTL8168h and these Dell systems too. If yes, may require a BIOS update to enable
the functionality.
2. (Re-)enable ASPM states from user space: Use attributes under /sys/class/net/<if>/device/link
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] r8169: Add quirks to enable ASPM on Dell platforms
2024-12-08 19:10 [PATCH] r8169: Add quirks to enable ASPM on Dell platforms Guy Chronister
2024-12-08 21:01 ` Heiner Kallweit
@ 2024-12-08 21:54 ` Heiner Kallweit
1 sibling, 0 replies; 3+ messages in thread
From: Heiner Kallweit @ 2024-12-08 21:54 UTC (permalink / raw)
To: Guy Chronister
Cc: nic_swsd, andrew+netdev, davem, edumazet, kuba, pabeni, netdev,
linux-kernel, Koba Ko, Timo Aaltonen, Andrea Righi
On 08.12.2024 20:10, Guy Chronister wrote:
> Some non-Dell platforms equipped with r8168h/r8111 have issues with ASPM. It's very hard to fix all known issues in a short time and r8168h/r8111 is not a brand new NIC chip, so introduce the quirk for Dell platforms. It's also easier to track the Dell platform and ask for Realtek's effort.
> Make the original matching logic more explicit.
>
> Signed-off-by: Koba Ko <koba.ko@canonical.com>
> Signed-off-by: Timo Aaltonen <timo.aaltonen@canonical.com>
> Signed-off-by: Andrea Righi <andrea.righi@canonical.com>
> Signed-off-by: Guy Chronister <guyc.linux.patches@gmail.com>
> ---
My response to Koba Ko and Andrea Righi bounced. So it seems they don't work
for canonical (any longer?).
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-12-08 21:54 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-08 19:10 [PATCH] r8169: Add quirks to enable ASPM on Dell platforms Guy Chronister
2024-12-08 21:01 ` Heiner Kallweit
2024-12-08 21:54 ` Heiner Kallweit
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.