On Wed, 16 Jul 2025, Manivannan Sadhasivam via B4 Relay wrote:
> From: Manivannan Sadhasivam <[email protected]>
>
> It is not recommended to enable/disable the ASPM states on the back of the
> PCI core directly using the LNKCTL register. It will break the PCI core's
> knowledge about the device ASPM states. So use the APIs exposed by the PCI
> core to enable/disable ASPM states.
>
> Tested-on: WCN7850 hw2.0 PCI
> WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3
>
> Reported-by: Qiang Yu <[email protected]>
> Signed-off-by: Manivannan Sadhasivam <[email protected]>
> ---
> drivers/net/wireless/ath/ath.h | 14 ++++++++++++++
> drivers/net/wireless/ath/ath12k/pci.c | 10 ++++------
> 2 files changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath.h b/drivers/net/wireless/ath/ath.h
> index
> 34654f710d8a1e63f65a47d4602e2035262a4d9e..ef685123b66bf4f41428fec67c1967f242a9ef27
> 100644
> --- a/drivers/net/wireless/ath/ath.h
> +++ b/drivers/net/wireless/ath/ath.h
> @@ -21,6 +21,8 @@
> #include <linux/skbuff.h>
> #include <linux/if_ether.h>
> #include <linux/spinlock.h>
> +#include <linux/pci.h>
> +#include <linux/pci_regs.h>
> #include <net/mac80211.h>
>
> /*
> @@ -336,4 +338,16 @@ static inline const char *ath_bus_type_to_string(enum
> ath_bus_type bustype)
> return ath_bus_type_strings[bustype];
> }
>
> +static inline int ath_pci_aspm_state(u16 lnkctl)
> +{
> + int state = 0;
> +
> + if (lnkctl & PCI_EXP_LNKCTL_ASPM_L0S)
> + state |= PCIE_LINK_STATE_L0S;
> + if (lnkctl & PCI_EXP_LNKCTL_ASPM_L1)
> + state |= PCIE_LINK_STATE_L1;
> +
> + return state;
> +}
> +
> #endif /* ATH_H */
> diff --git a/drivers/net/wireless/ath/ath12k/pci.c
> b/drivers/net/wireless/ath/ath12k/pci.c
> index
> 489d546390fcdab8f615cc9184006a958d9f140a..a5e11509e3ab8faad6638ff78ce6a8a5e9c3cbbd
> 100644
> --- a/drivers/net/wireless/ath/ath12k/pci.c
> +++ b/drivers/net/wireless/ath/ath12k/pci.c
> @@ -16,6 +16,8 @@
> #include "mhi.h"
> #include "debug.h"
>
> +#include "../ath.h"
> +
> #define ATH12K_PCI_BAR_NUM 0
> #define ATH12K_PCI_DMA_MASK 36
>
> @@ -928,8 +930,7 @@ static void ath12k_pci_aspm_disable(struct ath12k_pci
> *ab_pci)
> u16_get_bits(ab_pci->link_ctl, PCI_EXP_LNKCTL_ASPM_L1));
>
> /* disable L0s and L1 */
> - pcie_capability_clear_word(ab_pci->pdev, PCI_EXP_LNKCTL,
> - PCI_EXP_LNKCTL_ASPMC);
> + pci_disable_link_state(ab_pci->pdev, PCIE_LINK_STATE_L0S |
> PCIE_LINK_STATE_L1);
I'd remove to comment too as the code is self-explanatory after this
change.
>
> set_bit(ATH12K_PCI_ASPM_RESTORE, &ab_pci->flags);
> }
> @@ -958,10 +959,7 @@ static void ath12k_pci_aspm_restore(struct ath12k_pci
> *ab_pci)
> {
> if (ab_pci->ab->hw_params->supports_aspm &&
> test_and_clear_bit(ATH12K_PCI_ASPM_RESTORE, &ab_pci->flags))
> - pcie_capability_clear_and_set_word(ab_pci->pdev, PCI_EXP_LNKCTL,
> - PCI_EXP_LNKCTL_ASPMC,
> - ab_pci->link_ctl &
> - PCI_EXP_LNKCTL_ASPMC);
> + pci_enable_link_state(ab_pci->pdev,
> ath_pci_aspm_state(ab_pci->link_ctl));
> }
>
> static void ath12k_pci_cancel_workqueue(struct ath12k_base *ab)
As you now depend on ASPM driver being there, these should also add to
Kconfig:
depends on PCIEASPM
--
i.
