2022-09-09 20:45:36

by Bjorn Helgaas

[permalink] [raw]
Subject: [PATCH v4 2/9] PCI/PTM: Add pci_upstream_ptm() helper

From: Bjorn Helgaas <[email protected]>

PTM requires an unbroken path of PTM-supporting devices between the PTM
Root and the ultimate PTM Requester, but if a Switch supports PTM, only the
Upstream Port can have a PTM Capability; the Downstream Ports do not.

Previously we copied the PTM configuration from the Switch Upstream Port to
the Downstream Ports so dev->ptm_enabled for any device implied that all
the upstream devices support PTM.

Instead of making it look like Downstream Ports have their own PTM config,
add pci_upstream_ptm(), which returns the upstream device that has a PTM
Capability (either a Root Port or a Switch Upstream Port).

Signed-off-by: Bjorn Helgaas <[email protected]>
---
drivers/pci/pcie/ptm.c | 39 +++++++++++++++++++++++++--------------
1 file changed, 25 insertions(+), 14 deletions(-)

diff --git a/drivers/pci/pcie/ptm.c b/drivers/pci/pcie/ptm.c
index 85382c135885..0df6cdfe38b4 100644
--- a/drivers/pci/pcie/ptm.c
+++ b/drivers/pci/pcie/ptm.c
@@ -76,6 +76,29 @@ void pci_restore_ptm_state(struct pci_dev *dev)
pci_write_config_word(dev, ptm + PCI_PTM_CTRL, *cap);
}

+/*
+ * If the next upstream device supports PTM, return it; otherwise return
+ * NULL. PTM Messages are local, so both link partners must support it.
+ */
+static struct pci_dev *pci_upstream_ptm(struct pci_dev *dev)
+{
+ struct pci_dev *ups = pci_upstream_bridge(dev);
+
+ /*
+ * Switch Downstream Ports are not permitted to have a PTM
+ * capability; their PTM behavior is controlled by the Upstream
+ * Port (PCIe r5.0, sec 7.9.16), so if the upstream bridge is a
+ * Switch Downstream Port, look up one more level.
+ */
+ if (ups && pci_pcie_type(ups) == PCI_EXP_TYPE_DOWNSTREAM)
+ ups = pci_upstream_bridge(ups);
+
+ if (ups && ups->ptm_cap)
+ return ups;
+
+ return NULL;
+}
+
void pci_ptm_init(struct pci_dev *dev)
{
u16 ptm;
@@ -95,19 +118,6 @@ void pci_ptm_init(struct pci_dev *dev)
pci_pcie_type(dev) == PCI_EXP_TYPE_RC_END))
return;

- /*
- * Switch Downstream Ports are not permitted to have a PTM
- * capability; their PTM behavior is controlled by the Upstream
- * Port (PCIe r5.0, sec 7.9.16).
- */
- ups = pci_upstream_bridge(dev);
- if (pci_pcie_type(dev) == PCI_EXP_TYPE_DOWNSTREAM &&
- ups && ups->ptm_enabled) {
- dev->ptm_granularity = ups->ptm_granularity;
- dev->ptm_enabled = 1;
- return;
- }
-
ptm = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_PTM);
if (!ptm)
return;
@@ -124,6 +134,7 @@ void pci_ptm_init(struct pci_dev *dev)
* the spec recommendation (PCIe r3.1, sec 7.32.3), select the
* furthest upstream Time Source as the PTM Root.
*/
+ ups = pci_upstream_ptm(dev);
if (ups && ups->ptm_enabled) {
ctrl = PCI_PTM_CTRL_ENABLE;
if (ups->ptm_granularity == 0)
@@ -173,7 +184,7 @@ int pci_enable_ptm(struct pci_dev *dev, u8 *granularity)
* associate the endpoint with a time source.
*/
if (pci_pcie_type(dev) == PCI_EXP_TYPE_ENDPOINT) {
- ups = pci_upstream_bridge(dev);
+ ups = pci_upstream_ptm(dev);
if (!ups || !ups->ptm_enabled)
return -EINVAL;

--
2.25.1


Subject: Re: [PATCH v4 2/9] PCI/PTM: Add pci_upstream_ptm() helper



On 9/9/22 1:24 PM, Bjorn Helgaas wrote:
> From: Bjorn Helgaas <[email protected]>
>
> PTM requires an unbroken path of PTM-supporting devices between the PTM
> Root and the ultimate PTM Requester, but if a Switch supports PTM, only the
> Upstream Port can have a PTM Capability; the Downstream Ports do not.
>
> Previously we copied the PTM configuration from the Switch Upstream Port to
> the Downstream Ports so dev->ptm_enabled for any device implied that all
> the upstream devices support PTM.
>
> Instead of making it look like Downstream Ports have their own PTM config,
> add pci_upstream_ptm(), which returns the upstream device that has a PTM
> Capability (either a Root Port or a Switch Upstream Port).
>
> Signed-off-by: Bjorn Helgaas <[email protected]>
> ---

Looks good to me.

Reviewed-by: Kuppuswamy Sathyanarayanan <[email protected]>

> drivers/pci/pcie/ptm.c | 39 +++++++++++++++++++++++++--------------
> 1 file changed, 25 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/pci/pcie/ptm.c b/drivers/pci/pcie/ptm.c
> index 85382c135885..0df6cdfe38b4 100644
> --- a/drivers/pci/pcie/ptm.c
> +++ b/drivers/pci/pcie/ptm.c
> @@ -76,6 +76,29 @@ void pci_restore_ptm_state(struct pci_dev *dev)
> pci_write_config_word(dev, ptm + PCI_PTM_CTRL, *cap);
> }
>
> +/*
> + * If the next upstream device supports PTM, return it; otherwise return
> + * NULL. PTM Messages are local, so both link partners must support it.
> + */
> +static struct pci_dev *pci_upstream_ptm(struct pci_dev *dev)
> +{
> + struct pci_dev *ups = pci_upstream_bridge(dev);
> +
> + /*
> + * Switch Downstream Ports are not permitted to have a PTM
> + * capability; their PTM behavior is controlled by the Upstream
> + * Port (PCIe r5.0, sec 7.9.16), so if the upstream bridge is a
> + * Switch Downstream Port, look up one more level.
> + */
> + if (ups && pci_pcie_type(ups) == PCI_EXP_TYPE_DOWNSTREAM)
> + ups = pci_upstream_bridge(ups);
> +
> + if (ups && ups->ptm_cap)
> + return ups;
> +
> + return NULL;
> +}
> +
> void pci_ptm_init(struct pci_dev *dev)
> {
> u16 ptm;
> @@ -95,19 +118,6 @@ void pci_ptm_init(struct pci_dev *dev)
> pci_pcie_type(dev) == PCI_EXP_TYPE_RC_END))
> return;
>
> - /*
> - * Switch Downstream Ports are not permitted to have a PTM
> - * capability; their PTM behavior is controlled by the Upstream
> - * Port (PCIe r5.0, sec 7.9.16).
> - */
> - ups = pci_upstream_bridge(dev);
> - if (pci_pcie_type(dev) == PCI_EXP_TYPE_DOWNSTREAM &&
> - ups && ups->ptm_enabled) {
> - dev->ptm_granularity = ups->ptm_granularity;
> - dev->ptm_enabled = 1;
> - return;
> - }
> -
> ptm = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_PTM);
> if (!ptm)
> return;
> @@ -124,6 +134,7 @@ void pci_ptm_init(struct pci_dev *dev)
> * the spec recommendation (PCIe r3.1, sec 7.32.3), select the
> * furthest upstream Time Source as the PTM Root.
> */
> + ups = pci_upstream_ptm(dev);
> if (ups && ups->ptm_enabled) {
> ctrl = PCI_PTM_CTRL_ENABLE;
> if (ups->ptm_granularity == 0)
> @@ -173,7 +184,7 @@ int pci_enable_ptm(struct pci_dev *dev, u8 *granularity)
> * associate the endpoint with a time source.
> */
> if (pci_pcie_type(dev) == PCI_EXP_TYPE_ENDPOINT) {
> - ups = pci_upstream_bridge(dev);
> + ups = pci_upstream_ptm(dev);
> if (!ups || !ups->ptm_enabled)
> return -EINVAL;
>

--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer