[RFC Patch] pci: add power management for rtl8116af

[RFC Patch] pci: add power management for rtl8116af

[javen]

From: Javen Xu <javen_xu@realsil.com.cn>

RTL8116af is a multi function card. But due to the hardware design, only
function 0 and function 1(nic) are exposed to pci system. If the system
want to enter s0idle or cpu need to enter c10 when suspend, function 2
to 7 must be set to d3 and enable aspm. Function 5 and 6 are reserved,
so we skip them.

Signed-off-by: Javen Xu <javen_xu@realsil.com.cn>
---
Hi,
Just as the comments above, function 2 to 7 are hidden to pci system. So
we have to set d3 and aspm through our private register, which is CSI. I
have a discussion with netdev maintainer, and he thought this might be a
question to pci system.
I wonder whether this patch can be accepted here. Any feedback would be
highly appreciated!

Thanks,
BRs,
Javen
---
 drivers/pci/quirks.c | 119 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 119 insertions(+)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index caaed1a01dc0..e6538edcdff4 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -35,6 +35,7 @@
 #include <linux/sizes.h>
 #include <linux/suspend.h>
 #include <linux/switchtec.h>
+#include <linux/iopoll.h>
 #include "pci.h"
 
 static bool pcie_lbms_seen(struct pci_dev *dev, u16 lnksta)
@@ -6381,3 +6382,121 @@ static void pci_mask_replay_timer_timeout(struct pci_dev *pdev)
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_GLI, 0x9750, pci_mask_replay_timer_timeout);
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_GLI, 0x9755, pci_mask_replay_timer_timeout);
 #endif
+
+#define RTL_TX_CONFIG			0x40
+#define RTL_CSIDR			0x64
+#define RTL_CSIAR			0x68
+#define RTL_ERIDR			0x70
+#define RTL_ERIAR			0x74
+#define RTL_OCPDR			0xb0
+
+#define CSIAR_FLAG			0x80000000
+#define CSIAR_WRITE_CMD			0x80000000
+#define CSIAR_BYTE_ENABLE		0x0000f000
+#define CSIAR_ADDR_MASK			0x00000fff
+
+#define ERIAR_READ_CMD			0x80000000
+#define ERIAR_MASK_1111			0x0f000000
+#define ERIAR_EXGMAC			0
+#define ERIAR_FLAG			0x80000000
+
+static u32 quirk_rtl_csi_read(void __iomem *base, u8 multi_fun_sel_bit, int addr)
+{
+	u32 cmd = (addr & CSIAR_ADDR_MASK) | (multi_fun_sel_bit << 16) | CSIAR_BYTE_ENABLE;
+	u32 val;
+
+	writel(cmd, base + RTL_CSIAR);
+	if (readl_poll_timeout(base + RTL_CSIAR, val, val & CSIAR_FLAG, 10, 1000))
+		return ~0;
+	return readl(base + RTL_CSIDR);
+}
+
+static void quirk_rtl_csi_write(void __iomem *base, u8 multi_fun_sel_bit, int addr, int value)
+{
+	u32 cmd = CSIAR_WRITE_CMD | (addr & CSIAR_ADDR_MASK) |
+		  CSIAR_BYTE_ENABLE | (multi_fun_sel_bit << 16);
+	u32 val;
+
+	writel(value, base + RTL_CSIDR);
+	writel(cmd, base + RTL_CSIAR);
+	readl_poll_timeout(base + RTL_CSIAR, val, !(val & CSIAR_FLAG), 10, 1000);
+}
+
+static u16 quirk_r8168_mac_ocp_read(void __iomem *base, u32 reg)
+{
+	writel(reg << 15, base + RTL_OCPDR);
+	return (u16)readl(base + RTL_OCPDR);
+}
+
+static void quirk_rtl8168_clear_and_set_csi(void __iomem *base, u8 multi_fun_sel_bit,
+					    u32 addr, u32 clearmask, u32 setmask)
+{
+	u32 val = quirk_rtl_csi_read(base, multi_fun_sel_bit, addr);
+
+	if (val != ~0) {
+		val &= ~clearmask;
+		val |= setmask;
+		quirk_rtl_csi_write(base, multi_fun_sel_bit, addr, val);
+	}
+}
+
+static void quirk_rtl8168_other_fun_pci_setting(void __iomem *base, u32 addr,
+						u32 clearmask, u32 setmask)
+{
+	for (int i = 2; i < 8; i++) {
+		if (i == 5 || i == 6)
+			continue;
+		quirk_rtl8168_clear_and_set_csi(base, i, addr, clearmask, setmask);
+	}
+}
+
+static void quirk_rtl8168_set_aspm_clkreq(struct pci_dev *pdev)
+{
+	void __iomem *base;
+	u16 pci_command;
+	int mmio_bar;
+	u32 txconfig, xid;
+
+	pci_read_config_word(pdev, PCI_COMMAND, &pci_command);
+	if (!(pci_command & PCI_COMMAND_MEMORY))
+		return;
+
+	mmio_bar = pci_select_bars(pdev, IORESOURCE_MEM);
+	if (!mmio_bar)
+		return;
+
+	mmio_bar = ffs(mmio_bar) - 1;
+	base = pci_iomap(pdev, mmio_bar, 0);
+	if (!base)
+		return;
+
+	txconfig = readl(base + RTL_TX_CONFIG);
+	if (txconfig == ~0U) {
+		pci_iounmap(pdev, base);
+		return;
+	}
+
+	xid = (txconfig >> 20) & 0xfcf;
+	if ((xid & 0x7cf) != 0x54b && (xid & 0x7cf) != 0x54a) {
+		pci_iounmap(pdev, base);
+		return;
+	}
+
+	if ((quirk_r8168_mac_ocp_read(base, 0xdc00) & 0x0078) != 0x0030 ||
+	    (quirk_r8168_mac_ocp_read(base, 0xd006) & 0x00ff) != 0x0000) {
+		pci_iounmap(pdev, base);
+		return;
+	}
+
+	quirk_rtl8168_other_fun_pci_setting(base, 0x80,
+					    BIT(0) | BIT(1) | BIT(8),
+					    BIT(0) | BIT(1) | BIT(8));
+
+	quirk_rtl8168_other_fun_pci_setting(base, 0x44,
+					    0,
+					    BIT(0) | BIT(1));
+
+	pci_iounmap(pdev, base);
+}
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_REALTEK, 0x8168, quirk_rtl8168_set_aspm_clkreq);
+DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_REALTEK, 0x8168, quirk_rtl8168_set_aspm_clkreq);
-- 
2.43.0

Re: [RFC Patch] pci: add power management for rtl8116af

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Read-modify-write operations on 32-bit PCI configuration registers inadvertently clear Write-1-to-Clear (W1C) status bits.
- [High] The PCI fixup silently aborts without applying power management settings if memory decoding is not enabled.
- [High] Lockless accesses to shared indirect hardware registers race with concurrent quirk executions and the r8169 network driver.
- [Low] Introduction of unused #define constants for ERI registers.
--

commit c906289c8fdc6bf2fd98830ac376147e18168f4a
Author: Javen Xu <javen_xu@realsil.com.cn>

pci: add power management for rtl8116af

This commit adds power management for the RTL8116af multi-function card by
configuring functions 2 to 7 to enter D3 and enabling ASPM. It skips the
reserved functions 5 and 6.

> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index caaed1a01dc02..e6538edcdff40 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
[ ... ]
> @@ -6381,3 +6382,121 @@ static void pci_mask_replay_timer_timeout(struct pci_dev *pdev)
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_GLI, 0x9750, pci_mask_replay_timer_timeout);
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_GLI, 0x9755, pci_mask_replay_timer_timeout);
>  #endif
> +
> +#define RTL_TX_CONFIG			0x40
> +#define RTL_CSIDR			0x64
> +#define RTL_CSIAR			0x68
> +#define RTL_ERIDR			0x70
> +#define RTL_ERIAR			0x74
> +#define RTL_OCPDR			0xb0
> +
> +#define CSIAR_FLAG			0x80000000
> +#define CSIAR_WRITE_CMD			0x80000000
> +#define CSIAR_BYTE_ENABLE		0x0000f000
> +#define CSIAR_ADDR_MASK			0x00000fff
> +
> +#define ERIAR_READ_CMD			0x80000000
> +#define ERIAR_MASK_1111			0x0f000000
> +#define ERIAR_EXGMAC			0
> +#define ERIAR_FLAG			0x80000000

[Severity: Low]
Are these RTL_ERIDR, RTL_ERIAR, and ERIAR_* constants used anywhere in this
patch? It looks like they might be unused dead code.

> +static u32 quirk_rtl_csi_read(void __iomem *base, u8 multi_fun_sel_bit, int addr)
> +{
> +	u32 cmd = (addr & CSIAR_ADDR_MASK) | (multi_fun_sel_bit << 16) | CSIAR_BYTE_ENABLE;
> +	u32 val;
> +
> +	writel(cmd, base + RTL_CSIAR);
> +	if (readl_poll_timeout(base + RTL_CSIAR, val, val & CSIAR_FLAG, 10, 1000))
> +		return ~0;
> +	return readl(base + RTL_CSIDR);
> +}
> +
> +static void quirk_rtl_csi_write(void __iomem *base, u8 multi_fun_sel_bit, int addr, int value)
> +{
> +	u32 cmd = CSIAR_WRITE_CMD | (addr & CSIAR_ADDR_MASK) |
> +		  CSIAR_BYTE_ENABLE | (multi_fun_sel_bit << 16);
> +	u32 val;
> +
> +	writel(value, base + RTL_CSIDR);
> +	writel(cmd, base + RTL_CSIAR);
> +	readl_poll_timeout(base + RTL_CSIAR, val, !(val & CSIAR_FLAG), 10, 1000);
> +}

[Severity: High]
Can these lockless accesses to the shared RTL_CSIAR and RTL_CSIDR registers
race with other operations?

Because the RTL8116af is a multi-function device and this quirk runs for all
exposed functions (0 and 1), concurrent quirk executions during system or
runtime resume could corrupt the two-step indirect register accesses.

Additionally, the r8169 network driver independently uses these exact same
registers. Could a runtime resume of one function trigger the quirk and
interleave with the active network driver on the other function, leading to
hardware configuration corruption?

[ ... ]
> +static void quirk_rtl8168_clear_and_set_csi(void __iomem *base, u8 multi_fun_sel_bit,
> +					    u32 addr, u32 clearmask, u32 setmask)
> +{
> +	u32 val = quirk_rtl_csi_read(base, multi_fun_sel_bit, addr);
> +
> +	if (val != ~0) {
> +		val &= ~clearmask;
> +		val |= setmask;
> +		quirk_rtl_csi_write(base, multi_fun_sel_bit, addr, val);
> +	}
> +}

[Severity: High]
Does this read-modify-write operation inadvertently clear Write-1-to-Clear
status bits?

By reading the entire 32-bit register and writing it back, any
Write-1-to-Clear bits that are currently set to 1 will be written back as 1s,
which clears them in hardware.

For RTL8168-based devices, the registers at offsets 0x80 (PCIe Link Control
and Link Status) and 0x44 (PMCSR) contain Write-1-to-Clear bits such as Link
Bandwidth Management Status and PME_Status. This could destroy pending PME
wake events or PCIe link events.

[ ... ]
> +static void quirk_rtl8168_set_aspm_clkreq(struct pci_dev *pdev)
> +{
> +	void __iomem *base;
> +	u16 pci_command;
> +	int mmio_bar;
> +	u32 txconfig, xid;
> +
> +	pci_read_config_word(pdev, PCI_COMMAND, &pci_command);
> +	if (!(pci_command & PCI_COMMAND_MEMORY))
> +		return;

[Severity: High]
Will this silently abort without applying power management settings if memory
decoding is not enabled?

Since this quirk is registered using DECLARE_PCI_FIXUP_FINAL, it executes
during device enumeration, before the device driver is bound and calls
pci_enable_device().

If system firmware or BIOS leaves PCIe functions with memory decoding
disabled, the fixup will return here, meaning functions 2-7 are never put
into D3hot and ASPM is not configured.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260521033827.502-1-javen_xu@realsil.com.cn?part=1

Re: [RFC Patch] pci: add power management for rtl8116af

[Bjorn Helgaas]

On Thu, May 21, 2026 at 11:38:27AM +0800, javen wrote:
> From: Javen Xu <javen_xu@realsil.com.cn>
> 
> RTL8116af is a multi function card. But due to the hardware design, only
> function 0 and function 1(nic) are exposed to pci system. If the system
> want to enter s0idle or cpu need to enter c10 when suspend, function 2
> to 7 must be set to d3 and enable aspm. Function 5 and 6 are reserved,
> so we skip them.

If the other functions aren't visible, does Linux use them at all?
Can you just put them in D3hot and leave them there indefinitely?

Other than quirk_rtl8168_set_aspm_clkreq() function name,
I can't tell from the code what this does with ASPM.  I'm a little
dubious because (a) ASPM is supposed to be managed by the PCI core,
not by individual drivers and (b) I can't tell whether this conflicts
or races with anything in pci/pcie/aspm.c.

Smells like a device that doesn't make much effort to conform to the
PCI specs.

> Signed-off-by: Javen Xu <javen_xu@realsil.com.cn>
> ---
> Hi,
> Just as the comments above, function 2 to 7 are hidden to pci system. So
> we have to set d3 and aspm through our private register, which is CSI. I
> have a discussion with netdev maintainer, and he thought this might be a
> question to pci system.

Pointer to that discussion?

> I wonder whether this patch can be accepted here. Any feedback would be
> highly appreciated!

Pay attention to previous history and match style and capitalization
in subject line, commit log, etc.  Match text width in the patch
itself.

> ---
>  drivers/pci/quirks.c | 119 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 119 insertions(+)
> 
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index caaed1a01dc0..e6538edcdff4 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -35,6 +35,7 @@
>  #include <linux/sizes.h>
>  #include <linux/suspend.h>
>  #include <linux/switchtec.h>
> +#include <linux/iopoll.h>
>  #include "pci.h"
>  
>  static bool pcie_lbms_seen(struct pci_dev *dev, u16 lnksta)
> @@ -6381,3 +6382,121 @@ static void pci_mask_replay_timer_timeout(struct pci_dev *pdev)
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_GLI, 0x9750, pci_mask_replay_timer_timeout);
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_GLI, 0x9755, pci_mask_replay_timer_timeout);
>  #endif
> +
> +#define RTL_TX_CONFIG			0x40
> +#define RTL_CSIDR			0x64
> +#define RTL_CSIAR			0x68
> +#define RTL_ERIDR			0x70
> +#define RTL_ERIAR			0x74
> +#define RTL_OCPDR			0xb0
> +
> +#define CSIAR_FLAG			0x80000000
> +#define CSIAR_WRITE_CMD			0x80000000
> +#define CSIAR_BYTE_ENABLE		0x0000f000
> +#define CSIAR_ADDR_MASK			0x00000fff
> +
> +#define ERIAR_READ_CMD			0x80000000
> +#define ERIAR_MASK_1111			0x0f000000
> +#define ERIAR_EXGMAC			0
> +#define ERIAR_FLAG			0x80000000
> +
> +static u32 quirk_rtl_csi_read(void __iomem *base, u8 multi_fun_sel_bit, int addr)
> +{
> +	u32 cmd = (addr & CSIAR_ADDR_MASK) | (multi_fun_sel_bit << 16) | CSIAR_BYTE_ENABLE;

Possible opportunity for FIELD_PREP().

> +	u32 val;
> +
> +	writel(cmd, base + RTL_CSIAR);
> +	if (readl_poll_timeout(base + RTL_CSIAR, val, val & CSIAR_FLAG, 10, 1000))
> +		return ~0;
> +	return readl(base + RTL_CSIDR);
> +}
> +
> +static void quirk_rtl_csi_write(void __iomem *base, u8 multi_fun_sel_bit, int addr, int value)
> +{
> +	u32 cmd = CSIAR_WRITE_CMD | (addr & CSIAR_ADDR_MASK) |
> +		  CSIAR_BYTE_ENABLE | (multi_fun_sel_bit << 16);

Ditto.

> +	u32 val;
> +
> +	writel(value, base + RTL_CSIDR);
> +	writel(cmd, base + RTL_CSIAR);
> +	readl_poll_timeout(base + RTL_CSIAR, val, !(val & CSIAR_FLAG), 10, 1000);
> +}
> +
> +static u16 quirk_r8168_mac_ocp_read(void __iomem *base, u32 reg)
> +{
> +	writel(reg << 15, base + RTL_OCPDR);
> +	return (u16)readl(base + RTL_OCPDR);
> +}
> +
> +static void quirk_rtl8168_clear_and_set_csi(void __iomem *base, u8 multi_fun_sel_bit,
> +					    u32 addr, u32 clearmask, u32 setmask)
> +{
> +	u32 val = quirk_rtl_csi_read(base, multi_fun_sel_bit, addr);
> +
> +	if (val != ~0) {
> +		val &= ~clearmask;
> +		val |= setmask;
> +		quirk_rtl_csi_write(base, multi_fun_sel_bit, addr, val);
> +	}
> +}
> +
> +static void quirk_rtl8168_other_fun_pci_setting(void __iomem *base, u32 addr,
> +						u32 clearmask, u32 setmask)
> +{
> +	for (int i = 2; i < 8; i++) {
> +		if (i == 5 || i == 6)
> +			continue;
> +		quirk_rtl8168_clear_and_set_csi(base, i, addr, clearmask, setmask);
> +	}
> +}
> +
> +static void quirk_rtl8168_set_aspm_clkreq(struct pci_dev *pdev)
> +{
> +	void __iomem *base;
> +	u16 pci_command;
> +	int mmio_bar;
> +	u32 txconfig, xid;
> +
> +	pci_read_config_word(pdev, PCI_COMMAND, &pci_command);
> +	if (!(pci_command & PCI_COMMAND_MEMORY))
> +		return;
> +
> +	mmio_bar = pci_select_bars(pdev, IORESOURCE_MEM);
> +	if (!mmio_bar)
> +		return;
> +
> +	mmio_bar = ffs(mmio_bar) - 1;
> +	base = pci_iomap(pdev, mmio_bar, 0);
> +	if (!base)
> +		return;
> +
> +	txconfig = readl(base + RTL_TX_CONFIG);
> +	if (txconfig == ~0U) {
> +		pci_iounmap(pdev, base);
> +		return;
> +	}
> +
> +	xid = (txconfig >> 20) & 0xfcf;
> +	if ((xid & 0x7cf) != 0x54b && (xid & 0x7cf) != 0x54a) {
> +		pci_iounmap(pdev, base);
> +		return;
> +	}
> +
> +	if ((quirk_r8168_mac_ocp_read(base, 0xdc00) & 0x0078) != 0x0030 ||
> +	    (quirk_r8168_mac_ocp_read(base, 0xd006) & 0x00ff) != 0x0000) {
> +		pci_iounmap(pdev, base);
> +		return;
> +	}
> +
> +	quirk_rtl8168_other_fun_pci_setting(base, 0x80,
> +					    BIT(0) | BIT(1) | BIT(8),
> +					    BIT(0) | BIT(1) | BIT(8));
> +
> +	quirk_rtl8168_other_fun_pci_setting(base, 0x44,
> +					    0,
> +					    BIT(0) | BIT(1));
> +
> +	pci_iounmap(pdev, base);
> +}
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_REALTEK, 0x8168, quirk_rtl8168_set_aspm_clkreq);
> +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_REALTEK, 0x8168, quirk_rtl8168_set_aspm_clkreq);
> -- 
> 2.43.0
> 

RE: [RFC Patch] pci: add power management for rtl8116af

[Javen]

>On Thu, May 21, 2026 at 11:38:27AM +0800, javen wrote:
>> From: Javen Xu <javen_xu@realsil.com.cn>
>>
>> RTL8116af is a multi function card. But due to the hardware design,
>> only function 0 and function 1(nic) are exposed to pci system. If the
>> system want to enter s0idle or cpu need to enter c10 when suspend,
>> function 2 to 7 must be set to d3 and enable aspm. Function 5 and 6
>> are reserved, so we skip them.
>
>If the other functions aren't visible, does Linux use them at all?
>Can you just put them in D3hot and leave them there indefinitely?

Linux does not use them at all. The purpose of this patch is to put them in D3hot and enable their ASPM indefinitely, so the system can enter c10 and s0idle.

>
>Other than quirk_rtl8168_set_aspm_clkreq() function name, I can't tell from
>the code what this does with ASPM.  I'm a little dubious because (a) ASPM is
>supposed to be managed by the PCI core, not by individual drivers and (b) I
>can't tell whether this conflicts or races with anything in pci/pcie/aspm.c.
>
>Smells like a device that doesn't make much effort to conform to the PCI specs.
>
In function quirk_rtl8168_set_aspm_clkreq(), writing to 0x80 (0x70 + 0x10) is actually writing to PCIe Link Control Register. Setting BIT(0)|BIT(1)|BIT(8) here enables ASPM L0s/L1, and enable Clock Power Management.

I completely agree with the principle that ASPM should be managed by the PCI(pcie/aspm.c). However, the reason why we have to do this via a quirk using Function 0 private CSI interface is that Functions 2 to 7 are hidden from OS. And pci will not detect it. lspci is below:
03:00.0 Unassigned class[ff00]: Realtek Semiconductor Co., Ltd. Device 816e (rev 31)
03:00.1 Ethernet controller: Realtek Semiconductor Co., Ltd. RTL8111/8168/8411 PCI Express Gigabit Ethernet Controller (rev 24)

Since PCI core never enumerates them, and no struct pci_dev is ever created for them. I think pcie/aspm.c is completely unaware of their existence and will not cause conflicts or races.
And my quirk only targets these unseen function via function 1 private csi interface, leaving function 0 and 1 untouched for pcie/aspm.c to manage natively.

This hardware might not strictly conform to standard PCI spec. If you have any better suggestions or if there is a more preferred way within PCI system to handle such unhidden multi-function deivces, I would be very grateful to hear your advice.

Thanks for your time and review.
BRs,
Javen

>> Signed-off-by: Javen Xu <javen_xu@realsil.com.cn>
>> ---
>> Hi,
>> Just as the comments above, function 2 to 7 are hidden to pci system.
>> So we have to set d3 and aspm through our private register, which is
>> CSI. I have a discussion with netdev maintainer, and he thought this
>> might be a question to pci system.
>
>Pointer to that discussion?

Discussion link: https://lore.kernel.org/netdev/33d351be-8a91-4940-950a-7f0866d0cae9@gmail.com/

>
>> I wonder whether this patch can be accepted here. Any feedback would
>> be highly appreciated!
>
>Pay attention to previous history and match style and capitalization in subject
>line, commit log, etc.  Match text width in the patch itself.
>
>> ---
>>  drivers/pci/quirks.c | 119
>> +++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 119 insertions(+)
>>
>> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c index
>> caaed1a01dc0..e6538edcdff4 100644
>> --- a/drivers/pci/quirks.c
>> +++ b/drivers/pci/quirks.c
>> @@ -35,6 +35,7 @@
>>  #include <linux/sizes.h>
>>  #include <linux/suspend.h>
>>  #include <linux/switchtec.h>
>> +#include <linux/iopoll.h>
>>  #include "pci.h"
>>
>>  static bool pcie_lbms_seen(struct pci_dev *dev, u16 lnksta) @@
>> -6381,3 +6382,121 @@ static void pci_mask_replay_timer_timeout(struct
>> pci_dev *pdev)  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_GLI, 0x9750,
>> pci_mask_replay_timer_timeout);
>> DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_GLI, 0x9755,
>> pci_mask_replay_timer_timeout);  #endif
>> +
>> +#define RTL_TX_CONFIG                        0x40
>> +#define RTL_CSIDR                    0x64
>> +#define RTL_CSIAR                    0x68
>> +#define RTL_ERIDR                    0x70
>> +#define RTL_ERIAR                    0x74
>> +#define RTL_OCPDR                    0xb0
>> +
>> +#define CSIAR_FLAG                   0x80000000
>> +#define CSIAR_WRITE_CMD                      0x80000000
>> +#define CSIAR_BYTE_ENABLE            0x0000f000
>> +#define CSIAR_ADDR_MASK                      0x00000fff
>> +
>> +#define ERIAR_READ_CMD                       0x80000000
>> +#define ERIAR_MASK_1111                      0x0f000000
>> +#define ERIAR_EXGMAC                 0
>> +#define ERIAR_FLAG                   0x80000000
>> +
>> +static u32 quirk_rtl_csi_read(void __iomem *base, u8
>> +multi_fun_sel_bit, int addr) {
>> +     u32 cmd = (addr & CSIAR_ADDR_MASK) | (multi_fun_sel_bit << 16) |
>> +CSIAR_BYTE_ENABLE;
>
>Possible opportunity for FIELD_PREP().

Will do.
>
>> +     u32 val;
>> +
>> +     writel(cmd, base + RTL_CSIAR);
>> +     if (readl_poll_timeout(base + RTL_CSIAR, val, val & CSIAR_FLAG, 10,
>1000))
>> +             return ~0;
>> +     return readl(base + RTL_CSIDR);
>> +}
>> +
>> +static void quirk_rtl_csi_write(void __iomem *base, u8
>> +multi_fun_sel_bit, int addr, int value) {
>> +     u32 cmd = CSIAR_WRITE_CMD | (addr & CSIAR_ADDR_MASK) |
>> +               CSIAR_BYTE_ENABLE | (multi_fun_sel_bit << 16);
>
>Ditto.
>
>> +     u32 val;
>> +
>> +     writel(value, base + RTL_CSIDR);
>> +     writel(cmd, base + RTL_CSIAR);
>> +     readl_poll_timeout(base + RTL_CSIAR, val, !(val & CSIAR_FLAG),
>> +10, 1000); }
>> +
>> +static u16 quirk_r8168_mac_ocp_read(void __iomem *base, u32 reg) {
>> +     writel(reg << 15, base + RTL_OCPDR);
>> +     return (u16)readl(base + RTL_OCPDR); }
>> +
>> +static void quirk_rtl8168_clear_and_set_csi(void __iomem *base, u8
>multi_fun_sel_bit,
>> +                                         u32 addr, u32 clearmask, u32
>> +setmask) {
>> +     u32 val = quirk_rtl_csi_read(base, multi_fun_sel_bit, addr);
>> +
>> +     if (val != ~0) {
>> +             val &= ~clearmask;
>> +             val |= setmask;
>> +             quirk_rtl_csi_write(base, multi_fun_sel_bit, addr, val);
>> +     }
>> +}
>> +
>> +static void quirk_rtl8168_other_fun_pci_setting(void __iomem *base, u32
>addr,
>> +                                             u32 clearmask, u32
>> +setmask) {
>> +     for (int i = 2; i < 8; i++) {
>> +             if (i == 5 || i == 6)
>> +                     continue;
>> +             quirk_rtl8168_clear_and_set_csi(base, i, addr, clearmask, setmask);
>> +     }
>> +}
>> +
>> +static void quirk_rtl8168_set_aspm_clkreq(struct pci_dev *pdev) {
>> +     void __iomem *base;
>> +     u16 pci_command;
>> +     int mmio_bar;
>> +     u32 txconfig, xid;
>> +
>> +     pci_read_config_word(pdev, PCI_COMMAND, &pci_command);
>> +     if (!(pci_command & PCI_COMMAND_MEMORY))
>> +             return;
>> +
>> +     mmio_bar = pci_select_bars(pdev, IORESOURCE_MEM);
>> +     if (!mmio_bar)
>> +             return;
>> +
>> +     mmio_bar = ffs(mmio_bar) - 1;
>> +     base = pci_iomap(pdev, mmio_bar, 0);
>> +     if (!base)
>> +             return;
>> +
>> +     txconfig = readl(base + RTL_TX_CONFIG);
>> +     if (txconfig == ~0U) {
>> +             pci_iounmap(pdev, base);
>> +             return;
>> +     }
>> +
>> +     xid = (txconfig >> 20) & 0xfcf;
>> +     if ((xid & 0x7cf) != 0x54b && (xid & 0x7cf) != 0x54a) {
>> +             pci_iounmap(pdev, base);
>> +             return;
>> +     }
>> +
>> +     if ((quirk_r8168_mac_ocp_read(base, 0xdc00) & 0x0078) != 0x0030 ||
>> +         (quirk_r8168_mac_ocp_read(base, 0xd006) & 0x00ff) != 0x0000) {
>> +             pci_iounmap(pdev, base);
>> +             return;
>> +     }
>> +
>> +     quirk_rtl8168_other_fun_pci_setting(base, 0x80,
>> +                                         BIT(0) | BIT(1) | BIT(8),
>> +                                         BIT(0) | BIT(1) | BIT(8));
>> +
>> +     quirk_rtl8168_other_fun_pci_setting(base, 0x44,
>> +                                         0,
>> +                                         BIT(0) | BIT(1));
>> +
>> +     pci_iounmap(pdev, base);
>> +}
>> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_REALTEK, 0x8168,
>> +quirk_rtl8168_set_aspm_clkreq);
>> +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_REALTEK, 0x8168,
>> +quirk_rtl8168_set_aspm_clkreq);
>> --
>> 2.43.0
>>

Re: [RFC Patch] pci: add power management for rtl8116af

[Lukas Wunner]

On Thu, May 21, 2026 at 11:38:27AM +0800, javen wrote:
> From: Javen Xu <javen_xu@realsil.com.cn>
> 
> RTL8116af is a multi function card. But due to the hardware design, only
> function 0 and function 1(nic) are exposed to pci system. If the system
> want to enter s0idle or cpu need to enter c10 when suspend, function 2
> to 7 must be set to d3 and enable aspm. Function 5 and 6 are reserved,
> so we skip them.
[...]
> Just as the comments above, function 2 to 7 are hidden to pci system. So
> we have to set d3 and aspm through our private register, which is CSI. I
> have a discussion with netdev maintainer, and he thought this might be a
> question to pci system.
> I wonder whether this patch can be accepted here. Any feedback would be
> highly appreciated!

This is quite large for a quirk.  Have you considered writing a small
driver instead to configure power state for the invisible/unused functions?
Or integrating the functionality into an existing driver?

Usually a quirk only makes sense if the system becomes unusable
or if a device falls off the bus without the quirk.  It's not
suitable if the point is just to avoid unnecessarily high power
consumption.

Thanks,

Lukas