Re: [PATCH] r8169: enable ASPM on Dell platforms

On Fri, Sep 12, 2025 at 05:30:52PM +0200, Heiner Kallweit wrote:

On 9/12/2025 9:29 AM, Chia-Lin Kao (AceLan) wrote:

quoted

Enable PCIe ASPM for RTL8169 NICs on Dell platforms that have been
verified to work reliably with this power management feature. The
r8169 driver traditionally disables ASPM to prevent random link
failures and system hangs on problematic hardware.

Dell has validated these product families to work correctly with
RTL NIC ASPM and commits to addressing any ASPM-related issues
with RTL hardware in collaboration with Realtek.

This change enables ASPM for the following Dell product families:
- Alienware
- Dell Laptops/Pro Laptops/Pro Max Laptops
- Dell Desktops/Pro Desktops/Pro Max Desktops
- Dell Pro Rugged Laptops

I'd like to avoid DMI-based whitelists in kernel code. If more system
vendors do it the same way, then this becomes hard to maintain.

I totally understand your point; I also don’t like constantly adding DMI
info to the list. But this list isn’t for a single product name, it’s a
product family that covers a series of products, and it probably won’t
change anytime soon.

There is already a mechanism for vendors to flag that they successfully
tested ASPM. See c217ab7a3961 ("r8169: enable ASPM L1.2 if system vendor
flags it as safe").

Right, but writing the flag is not applicable for Dell manufacturing
processes.

Last but not least ASPM can be (re-)enabled from userspace, using sysfs.

That doesn't sound like a good solution to push the list to userspace.

Dell has already been working with Canonical for more than a decade to
ship their products with r8169 ASPM enabled. Dell has also had lengthy
discussions with Realtek to have this feature enabled by default in the
r8169 driver. I think this is a good opportunity for Dell to work with
Realtek to spot bugs and refine the r8169 driver.

BTW, I found the dmi.h header file is missing in the patch, so I
submitted a v2 patch.
https://lore.kernel.org/lkml/20250915013555.365230-1-acelan.kao@canonical.com/T/#u (local)

quoted

Signed-off-by: Chia-Lin Kao (AceLan) <acelan.kao@canonical.com>
---
 drivers/net/ethernet/realtek/r8169_main.c | 29 +++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index 9c601f271c02..63e83cf071de 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c

@@ -5366,6 +5366,32 @@ static void rtl_init_mac_address(struct rtl8169_private *tp)
 	rtl_rar_set(tp, mac_addr);
 }
 
+bool rtl_aspm_new_dell_platforms(void)
+{
+	const char *family = dmi_get_system_info(DMI_PRODUCT_FAMILY);
+	static const char * const dell_product_families[] = {
+		"Alienware",
+		"Dell Laptops",
+		"Dell Pro Laptops",
+		"Dell Pro Max Laptops",
+		"Dell Desktops",
+		"Dell Pro Desktops",
+		"Dell Pro Max Desktops",
+		"Dell Pro Rugged Laptops"
+	};
+	int i;
+
+	if (!family)
+		return false;
+
+	for (i = 0; i < ARRAY_SIZE(dell_product_families); i++) {
+		if (str_has_prefix(family, dell_product_families[i]))
+			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)
 {

@@ -5373,6 +5399,9 @@ static bool rtl_aspm_is_safe(struct rtl8169_private *tp)
 	    r8168_mac_ocp_read(tp, 0xc0b2) & 0xf)
 		return true;
 
+	if (rtl_aspm_new_dell_platforms())
+		return true;
+
 	return false;
 }