Skip to content

Commit

Permalink
Merge branch 'PCI-let-pci_disable_link_state-propagate-errors'
Browse files Browse the repository at this point in the history
Heiner Kallweit says:

====================
PCI: let pci_disable_link_state propagate errors

Drivers like r8169 rely on pci_disable_link_state() having disabled
certain ASPM link states. If OS can't control ASPM then
pci_disable_link_state() turns into a no-op w/o informing the caller.
The driver therefore may falsely assume the respective ASPM link
states are disabled. Let pci_disable_link_state() propagate errors
to the caller, enabling the caller to react accordingly.

I'd propose to let this series go through the netdev tree if the PCI
core extension is acked by the PCI people.
====================

Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
David S. Miller committed Jun 22, 2019
2 parents dca73a6 + 62b1b3b commit e0effb5
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 14 deletions.
8 changes: 6 additions & 2 deletions drivers/net/ethernet/realtek/r8169_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -652,6 +652,7 @@ struct rtl8169_private {

unsigned irq_enabled:1;
unsigned supports_gmii:1;
unsigned aspm_manageable:1;
dma_addr_t counters_phys_addr;
struct rtl8169_counters *counters;
struct rtl8169_tc_offsets tc_offset;
Expand Down Expand Up @@ -4286,7 +4287,8 @@ static void rtl_pcie_state_l2l3_disable(struct rtl8169_private *tp)

static void rtl_hw_aspm_clkreq_enable(struct rtl8169_private *tp, bool enable)
{
if (enable) {
/* Don't enable ASPM in the chip if OS can't control ASPM */
if (enable && tp->aspm_manageable) {
RTL_W8(tp, Config5, RTL_R8(tp, Config5) | ASPM_en);
RTL_W8(tp, Config2, RTL_R8(tp, Config2) | ClkReqEn);
} else {
Expand Down Expand Up @@ -6678,7 +6680,9 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
/* Disable ASPM completely as that cause random device stop working
* problems as well as full system hangs for some PCIe devices users.
*/
pci_disable_link_state(pdev, PCIE_LINK_STATE_L0S | PCIE_LINK_STATE_L1);
rc = pci_disable_link_state(pdev, PCIE_LINK_STATE_L0S |
PCIE_LINK_STATE_L1);
tp->aspm_manageable = !rc;

/* enable device (incl. PCI PM wakeup and hotplug setup) */
rc = pcim_enable_device(pdev);
Expand Down
20 changes: 11 additions & 9 deletions drivers/pci/pcie/aspm.c
Original file line number Diff line number Diff line change
Expand Up @@ -1062,18 +1062,18 @@ void pcie_aspm_powersave_config_link(struct pci_dev *pdev)
up_read(&pci_bus_sem);
}

static void __pci_disable_link_state(struct pci_dev *pdev, int state, bool sem)
static int __pci_disable_link_state(struct pci_dev *pdev, int state, bool sem)
{
struct pci_dev *parent = pdev->bus->self;
struct pcie_link_state *link;

if (!pci_is_pcie(pdev))
return;
return 0;

if (pdev->has_secondary_link)
parent = pdev;
if (!parent || !parent->link_state)
return;
return -EINVAL;

/*
* A driver requested that ASPM be disabled on this device, but
Expand All @@ -1085,7 +1085,7 @@ static void __pci_disable_link_state(struct pci_dev *pdev, int state, bool sem)
*/
if (aspm_disabled) {
pci_warn(pdev, "can't disable ASPM; OS doesn't have ASPM control\n");
return;
return -EPERM;
}

if (sem)
Expand All @@ -1105,26 +1105,28 @@ static void __pci_disable_link_state(struct pci_dev *pdev, int state, bool sem)
mutex_unlock(&aspm_lock);
if (sem)
up_read(&pci_bus_sem);

return 0;
}

void pci_disable_link_state_locked(struct pci_dev *pdev, int state)
int pci_disable_link_state_locked(struct pci_dev *pdev, int state)
{
__pci_disable_link_state(pdev, state, false);
return __pci_disable_link_state(pdev, state, false);
}
EXPORT_SYMBOL(pci_disable_link_state_locked);

/**
* pci_disable_link_state - Disable device's link state, so the link will
* never enter specific states. Note that if the BIOS didn't grant ASPM
* control to the OS, this does nothing because we can't touch the LNKCTL
* register.
* register. Returns 0 or a negative errno.
*
* @pdev: PCI device
* @state: ASPM link state to disable
*/
void pci_disable_link_state(struct pci_dev *pdev, int state)
int pci_disable_link_state(struct pci_dev *pdev, int state)
{
__pci_disable_link_state(pdev, state, true);
return __pci_disable_link_state(pdev, state, true);
}
EXPORT_SYMBOL(pci_disable_link_state);

Expand Down
7 changes: 4 additions & 3 deletions include/linux/pci-aspm.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,12 @@
#define PCIE_LINK_STATE_CLKPM 4

#ifdef CONFIG_PCIEASPM
void pci_disable_link_state(struct pci_dev *pdev, int state);
void pci_disable_link_state_locked(struct pci_dev *pdev, int state);
int pci_disable_link_state(struct pci_dev *pdev, int state);
int pci_disable_link_state_locked(struct pci_dev *pdev, int state);
void pcie_no_aspm(void);
#else
static inline void pci_disable_link_state(struct pci_dev *pdev, int state) { }
static inline int pci_disable_link_state(struct pci_dev *pdev, int state)
{ return 0; }
static inline void pcie_no_aspm(void) { }
#endif

Expand Down

0 comments on commit e0effb5

Please sign in to comment.