* [RFC PATCH v1 12/13] IB/hfi1: Use pci_disable/enable_link_state()
[not found] <20230602114751.19671-1-ilpo.jarvinen@linux.intel.com>
@ 2023-06-02 11:47 ` Ilpo Järvinen
2023-06-05 16:27 ` Dean Luick
0 siblings, 1 reply; 2+ messages in thread
From: Ilpo Järvinen @ 2023-06-02 11:47 UTC (permalink / raw)
To: linux-pci, Bjorn Helgaas, Lorenzo Pieralisi, Rob Herring,
Krzysztof Wilczyński, Lukas Wunner, Rafael J . Wysocki,
Heiner Kallweit, Emmanuel Grumbach, Dennis Dalessandro,
Jason Gunthorpe, Leon Romanovsky, linux-rdma, linux-kernel
Cc: Ilpo Järvinen
IB/hfi1 driver adjusts ASPM state itself which leaves ASPM service
driver in PCI core unaware of the link state changes the driver
implemented.
Call pci_disable_link_state() and pci_enable_link_state() instead of
adjusting ASPMC field in LNKCTL directly in the driver and let PCI core
handle the ASPM state management. Remove the functions that handled the
ASPM changes that are now unnecessary.
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
drivers/infiniband/hw/hfi1/aspm.c | 38 +++----------------------------
drivers/infiniband/hw/hfi1/pcie.c | 2 +-
2 files changed, 4 insertions(+), 36 deletions(-)
diff --git a/drivers/infiniband/hw/hfi1/aspm.c b/drivers/infiniband/hw/hfi1/aspm.c
index a3c53be4072c..8e3fc1d4c9c6 100644
--- a/drivers/infiniband/hw/hfi1/aspm.c
+++ b/drivers/infiniband/hw/hfi1/aspm.c
@@ -54,45 +54,13 @@ static void aspm_hw_set_l1_ent_latency(struct hfi1_devdata *dd)
pci_write_config_dword(dd->pcidev, PCIE_CFG_REG_PL3, reg32);
}
-static void aspm_hw_enable_l1(struct hfi1_devdata *dd)
-{
- struct pci_dev *parent = dd->pcidev->bus->self;
-
- /*
- * If the driver does not have access to the upstream component,
- * it cannot support ASPM L1 at all.
- */
- if (!parent)
- return;
-
- /* Enable ASPM L1 first in upstream component and then downstream */
- pcie_capability_clear_and_set_word(parent, PCI_EXP_LNKCTL,
- PCI_EXP_LNKCTL_ASPMC,
- PCI_EXP_LNKCTL_ASPM_L1);
- pcie_capability_clear_and_set_word(dd->pcidev, PCI_EXP_LNKCTL,
- PCI_EXP_LNKCTL_ASPMC,
- PCI_EXP_LNKCTL_ASPM_L1);
-}
-
-void aspm_hw_disable_l1(struct hfi1_devdata *dd)
-{
- struct pci_dev *parent = dd->pcidev->bus->self;
-
- /* Disable ASPM L1 first in downstream component and then upstream */
- pcie_capability_clear_and_set_word(dd->pcidev, PCI_EXP_LNKCTL,
- PCI_EXP_LNKCTL_ASPMC, 0x0);
- if (parent)
- pcie_capability_clear_and_set_word(parent, PCI_EXP_LNKCTL,
- PCI_EXP_LNKCTL_ASPMC, 0x0);
-}
-
static void aspm_enable(struct hfi1_devdata *dd)
{
if (dd->aspm_enabled || aspm_mode == ASPM_MODE_DISABLED ||
!dd->aspm_supported)
return;
- aspm_hw_enable_l1(dd);
+ pci_enable_link_state(dd->pcidev, PCI_EXP_LNKCTL_ASPM_L1);
dd->aspm_enabled = true;
}
@@ -101,7 +69,7 @@ static void aspm_disable(struct hfi1_devdata *dd)
if (!dd->aspm_enabled || aspm_mode == ASPM_MODE_ENABLED)
return;
- aspm_hw_disable_l1(dd);
+ pci_disable_link_state(dd->pcidev, PCIE_LINK_STATE_L0S | PCIE_LINK_STATE_L1);
dd->aspm_enabled = false;
}
@@ -254,7 +222,7 @@ void aspm_init(struct hfi1_devdata *dd)
/* Start with ASPM disabled */
aspm_hw_set_l1_ent_latency(dd);
dd->aspm_enabled = false;
- aspm_hw_disable_l1(dd);
+ pci_disable_link_state(dd->pcidev, PCIE_LINK_STATE_L0S | PCIE_LINK_STATE_L1);
/* Now turn on ASPM if configured */
aspm_enable_all(dd);
diff --git a/drivers/infiniband/hw/hfi1/pcie.c b/drivers/infiniband/hw/hfi1/pcie.c
index 08732e1ac966..767f6cb770b6 100644
--- a/drivers/infiniband/hw/hfi1/pcie.c
+++ b/drivers/infiniband/hw/hfi1/pcie.c
@@ -1182,7 +1182,7 @@ int do_pcie_gen3_transition(struct hfi1_devdata *dd)
* will be enabled if required later
*/
dd_dev_info(dd, "%s: clearing ASPM\n", __func__);
- aspm_hw_disable_l1(dd);
+ pci_disable_link_state(dd->pcidev, PCIE_LINK_STATE_L0S | PCIE_LINK_STATE_L1);
/*
* step 5f: clear DirectSpeedChange
--
2.30.2
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [RFC PATCH v1 12/13] IB/hfi1: Use pci_disable/enable_link_state()
2023-06-02 11:47 ` [RFC PATCH v1 12/13] IB/hfi1: Use pci_disable/enable_link_state() Ilpo Järvinen
@ 2023-06-05 16:27 ` Dean Luick
0 siblings, 0 replies; 2+ messages in thread
From: Dean Luick @ 2023-06-05 16:27 UTC (permalink / raw)
To: Ilpo Järvinen, linux-pci, Bjorn Helgaas, Lorenzo Pieralisi,
Rob Herring, Krzysztof Wilczyński, Lukas Wunner,
Rafael J . Wysocki, Heiner Kallweit, Emmanuel Grumbach,
Dennis Dalessandro, Jason Gunthorpe, Leon Romanovsky, linux-rdma,
linux-kernel
On 6/2/2023 6:47 AM, Ilpo Järvinen wrote:
> IB/hfi1 driver adjusts ASPM state itself which leaves ASPM service
> driver in PCI core unaware of the link state changes the driver
> implemented.
>
> Call pci_disable_link_state() and pci_enable_link_state() instead of
> adjusting ASPMC field in LNKCTL directly in the driver and let PCI core
> handle the ASPM state management. Remove the functions that handled the
> ASPM changes that are now unnecessary.
>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> ---
> drivers/infiniband/hw/hfi1/aspm.c | 38 +++----------------------------
> drivers/infiniband/hw/hfi1/pcie.c | 2 +-
> 2 files changed, 4 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/infiniband/hw/hfi1/aspm.c b/drivers/infiniband/hw/hfi1/aspm.c
> index a3c53be4072c..8e3fc1d4c9c6 100644
> --- a/drivers/infiniband/hw/hfi1/aspm.c
> +++ b/drivers/infiniband/hw/hfi1/aspm.c
> @@ -54,45 +54,13 @@ static void aspm_hw_set_l1_ent_latency(struct hfi1_devdata *dd)
> pci_write_config_dword(dd->pcidev, PCIE_CFG_REG_PL3, reg32);
> }
>
> -static void aspm_hw_enable_l1(struct hfi1_devdata *dd)
> -{
> - struct pci_dev *parent = dd->pcidev->bus->self;
> -
> - /*
> - * If the driver does not have access to the upstream component,
> - * it cannot support ASPM L1 at all.
> - */
> - if (!parent)
> - return;
> -
> - /* Enable ASPM L1 first in upstream component and then downstream */
> - pcie_capability_clear_and_set_word(parent, PCI_EXP_LNKCTL,
> - PCI_EXP_LNKCTL_ASPMC,
> - PCI_EXP_LNKCTL_ASPM_L1);
> - pcie_capability_clear_and_set_word(dd->pcidev, PCI_EXP_LNKCTL,
> - PCI_EXP_LNKCTL_ASPMC,
> - PCI_EXP_LNKCTL_ASPM_L1);
> -}
> -
> -void aspm_hw_disable_l1(struct hfi1_devdata *dd)
> -{
> - struct pci_dev *parent = dd->pcidev->bus->self;
> -
> - /* Disable ASPM L1 first in downstream component and then upstream */
> - pcie_capability_clear_and_set_word(dd->pcidev, PCI_EXP_LNKCTL,
> - PCI_EXP_LNKCTL_ASPMC, 0x0);
> - if (parent)
> - pcie_capability_clear_and_set_word(parent, PCI_EXP_LNKCTL,
> - PCI_EXP_LNKCTL_ASPMC, 0x0);
> -}
> -
> static void aspm_enable(struct hfi1_devdata *dd)
> {
> if (dd->aspm_enabled || aspm_mode == ASPM_MODE_DISABLED ||
> !dd->aspm_supported)
> return;
>
> - aspm_hw_enable_l1(dd);
> + pci_enable_link_state(dd->pcidev, PCI_EXP_LNKCTL_ASPM_L1);
> dd->aspm_enabled = true;
> }
>
> @@ -101,7 +69,7 @@ static void aspm_disable(struct hfi1_devdata *dd)
> if (!dd->aspm_enabled || aspm_mode == ASPM_MODE_ENABLED)
> return;
>
> - aspm_hw_disable_l1(dd);
> + pci_disable_link_state(dd->pcidev, PCIE_LINK_STATE_L0S | PCIE_LINK_STATE_L1);
> dd->aspm_enabled = false;
> }
>
> @@ -254,7 +222,7 @@ void aspm_init(struct hfi1_devdata *dd)
> /* Start with ASPM disabled */
> aspm_hw_set_l1_ent_latency(dd);
> dd->aspm_enabled = false;
> - aspm_hw_disable_l1(dd);
> + pci_disable_link_state(dd->pcidev, PCIE_LINK_STATE_L0S | PCIE_LINK_STATE_L1);
>
> /* Now turn on ASPM if configured */
> aspm_enable_all(dd);
> diff --git a/drivers/infiniband/hw/hfi1/pcie.c b/drivers/infiniband/hw/hfi1/pcie.c
> index 08732e1ac966..767f6cb770b6 100644
> --- a/drivers/infiniband/hw/hfi1/pcie.c
> +++ b/drivers/infiniband/hw/hfi1/pcie.c
> @@ -1182,7 +1182,7 @@ int do_pcie_gen3_transition(struct hfi1_devdata *dd)
> * will be enabled if required later
> */
> dd_dev_info(dd, "%s: clearing ASPM\n", __func__);
> - aspm_hw_disable_l1(dd);
> + pci_disable_link_state(dd->pcidev, PCIE_LINK_STATE_L0S | PCIE_LINK_STATE_L1);
>
> /*
> * step 5f: clear DirectSpeedChange
Looks good.
Reviewed-by: Dean Luick <dean.luick@cornelisnetworks.com>
-Dean
External recipient
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2023-06-05 16:28 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20230602114751.19671-1-ilpo.jarvinen@linux.intel.com>
2023-06-02 11:47 ` [RFC PATCH v1 12/13] IB/hfi1: Use pci_disable/enable_link_state() Ilpo Järvinen
2023-06-05 16:27 ` Dean Luick
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox