All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/3] Cadence PCIe PHY latency for PTM
@ 2023-04-27  5:50 Dominic Rath
  2023-04-27  5:50 ` [PATCH v2 1/3] dt-bindings: phy: cadence-torrent: Add latency properties Dominic Rath
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Dominic Rath @ 2023-04-27  5:50 UTC (permalink / raw)
  To: robh+dt, krzysztof.kozlowski+dt, tjoseph
  Cc: bhelgaas, lpieralisi, nm, vigneshr, devicetree, linux-kernel,
	linux-pci, christian.gmeiner, bahle, Dominic Rath

Hello Everyone,

this series adds PHY latency properties to the Cadence PCIe
driver to improve PTM accuracy, and configures the necessary
values for TI's AM64x processors.

These latencies are implementation specific and need to be
configured in the PCIe IP core's registers to allow the
PCIe controller to exactly determine the RX/TX timestamps for
PCIe PTM messages.

TI doesn't document these values in the datasheet or reference
manual as of now, but provided the necessary data via TI's E2E
forums (see PATCH 3/3).

Changes from v1 to v2:
   - move latency property to PHY instead of PCIe controller
   - drop vendor prefix from property name
   - rephrase commit message regarding optional properties
   - emit an info message instead of a warning in case
     an optional property is missing

Best Regards,

Dominic

Alexander Bahle (3):
  dt-bindings: phy: cadence-torrent: Add latency properties
  PCI: cadence: Use DT bindings to set PHY latencies
  arm64: dts: ti: k3-am64: Add PCIe PHY latency DT binding

 .../bindings/phy/phy-cadence-torrent.yaml     | 20 ++++
 arch/arm64/boot/dts/ti/k3-am642-evm.dts       |  2 +
 .../pci/controller/cadence/pcie-cadence-ep.c  |  2 +
 .../controller/cadence/pcie-cadence-host.c    |  1 +
 drivers/pci/controller/cadence/pcie-cadence.c | 92 +++++++++++++++++++
 drivers/pci/controller/cadence/pcie-cadence.h | 23 +++++
 6 files changed, 140 insertions(+)


base-commit: 0cfd8703e7da687924371e9bc77a025bdeba9637
-- 
2.36.0


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 1/3] dt-bindings: phy: cadence-torrent: Add latency properties
  2023-04-27  5:50 [PATCH v2 0/3] Cadence PCIe PHY latency for PTM Dominic Rath
@ 2023-04-27  5:50 ` Dominic Rath
  2023-04-27 18:30   ` Bjorn Helgaas
  2023-05-30  8:19   ` Christian Gmeiner
  2023-04-27  5:50 ` [PATCH v2 2/3] PCI: cadence: Use DT bindings to set PHY latencies Dominic Rath
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 14+ messages in thread
From: Dominic Rath @ 2023-04-27  5:50 UTC (permalink / raw)
  To: robh+dt, krzysztof.kozlowski+dt, tjoseph
  Cc: bhelgaas, lpieralisi, nm, vigneshr, devicetree, linux-kernel,
	linux-pci, christian.gmeiner, bahle, Dominic Rath

From: Alexander Bahle <bahle@ibv-augsburg.de>

Add "tx-phy-latency-ps" and "rx-phy-latency-ps" DT bindings for
setting the PCIe PHY latencies.
The properties expect a list of uint32 PHY latencies in picoseconds for
every supported speed starting at PCIe Gen1, e.g.:

  tx-phy-latency-ps = <100000 200000>; /* Gen1: 100ns, Gen2: 200ns */
  rx-phy-latency-ps = <150000 250000>; /* Gen1: 150ns, Gen2: 250ns */

Signed-off-by: Alexander Bahle <bahle@ibv-augsburg.de>
Signed-off-by: Dominic Rath <rath@ibv-augsburg.de>
---
 .../bindings/phy/phy-cadence-torrent.yaml     | 20 +++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/phy-cadence-torrent.yaml b/Documentation/devicetree/bindings/phy/phy-cadence-torrent.yaml
index 2ad1faadda2a..93228a304395 100644
--- a/Documentation/devicetree/bindings/phy/phy-cadence-torrent.yaml
+++ b/Documentation/devicetree/bindings/phy/phy-cadence-torrent.yaml
@@ -126,6 +126,24 @@ patternProperties:
         enum: [2160, 2430, 2700, 3240, 4320, 5400, 8100]
         default: 8100
 
+      tx-phy-latency-ps:
+        description:
+          The PHY latencies for the TX direction applied to PCIe PTM timestamps. Most
+          PCIe PHYs have asynchronous latencies for their RX and TX paths. To obtain
+          accurate PTM timestamps, the PCIe PTM specification requires that the time
+          at which the first serial bit is present on the serial lines be taken.
+          Should contain picosecond latency values for each supported speed,
+          starting with Gen1 latency.
+
+      rx-phy-latency-ps:
+        description:
+          The PHY latencies for the RX direction applied to the PTM timestamps. Most
+          PCIe PHYs have asynchronous latencies for their RX and TX paths. To obtain
+          accurate PTM timestamps, the PCIe PTM specification requires that the time
+          at which the first serial bit is present on the serial lines be taken.
+          Should contain picosecond latency values for each supported speed,
+          starting with Gen1 latency.
+
     required:
       - reg
       - resets
@@ -203,6 +221,8 @@ examples:
                 cdns,phy-type = <PHY_TYPE_PCIE>;
                 cdns,num-lanes = <2>;
                 cdns,ssc-mode = <CDNS_SERDES_NO_SSC>;
+                tx-phy-latency-ps = <138800 69400>;
+                rx-phy-latency-ps = <185200 92600>;
             };
 
             phy@2 {
-- 
2.36.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 2/3] PCI: cadence: Use DT bindings to set PHY latencies
  2023-04-27  5:50 [PATCH v2 0/3] Cadence PCIe PHY latency for PTM Dominic Rath
  2023-04-27  5:50 ` [PATCH v2 1/3] dt-bindings: phy: cadence-torrent: Add latency properties Dominic Rath
@ 2023-04-27  5:50 ` Dominic Rath
  2023-05-04 11:33   ` Verma, Achal
  2023-04-27  5:50 ` [PATCH v2 3/3] arm64: dts: ti: k3-am64: Add PCIe PHY latency DT binding Dominic Rath
  2023-05-30  8:21 ` [PATCH v2 0/3] Cadence PCIe PHY latency for PTM Christian Gmeiner
  3 siblings, 1 reply; 14+ messages in thread
From: Dominic Rath @ 2023-04-27  5:50 UTC (permalink / raw)
  To: robh+dt, krzysztof.kozlowski+dt, tjoseph
  Cc: bhelgaas, lpieralisi, nm, vigneshr, devicetree, linux-kernel,
	linux-pci, christian.gmeiner, bahle, Dominic Rath

From: Alexander Bahle <bahle@ibv-augsburg.de>

Use optional "tx-phy-latency-ps" and "rx-phy-latency-ps"
DeviceTree bindings to set the CDNS_PCIE_LM_PTM_LAT_PARAM(_IDX)
register(s) during PCIe host and endpoint setup.

The properties are lists of uint32 PHY latencies in picoseconds for
every supported speed starting at PCIe Gen1, e.g.:

  tx-phy-latency-ps = <100000 200000>; /* Gen1: 100ns, Gen2: 200ns */
  rx-phy-latency-ps = <150000 250000>; /* Gen1: 150ns, Gen2: 250ns */

There should be a value for every supported speed, otherwise a info
message is emitted to let users know that the PTM timestamps from this
PCIe device may not be precise enough for some applications.

Signed-off-by: Alexander Bahle <bahle@ibv-augsburg.de>
Signed-off-by: Dominic Rath <rath@ibv-augsburg.de>
---
 .../pci/controller/cadence/pcie-cadence-ep.c  |  2 +
 .../controller/cadence/pcie-cadence-host.c    |  1 +
 drivers/pci/controller/cadence/pcie-cadence.c | 92 +++++++++++++++++++
 drivers/pci/controller/cadence/pcie-cadence.h | 23 +++++
 4 files changed, 118 insertions(+)

diff --git a/drivers/pci/controller/cadence/pcie-cadence-ep.c b/drivers/pci/controller/cadence/pcie-cadence-ep.c
index b8b655d4047e..6e39126922d1 100644
--- a/drivers/pci/controller/cadence/pcie-cadence-ep.c
+++ b/drivers/pci/controller/cadence/pcie-cadence-ep.c
@@ -664,6 +664,8 @@ int cdns_pcie_ep_setup(struct cdns_pcie_ep *ep)
 	}
 	pcie->mem_res = res;
 
+	cdns_pcie_init_ptm_phy_latency(dev, pcie);
+
 	ep->max_regions = CDNS_PCIE_MAX_OB;
 	of_property_read_u32(np, "cdns,max-outbound-regions", &ep->max_regions);
 
diff --git a/drivers/pci/controller/cadence/pcie-cadence-host.c b/drivers/pci/controller/cadence/pcie-cadence-host.c
index 940c7dd701d6..8933002f828e 100644
--- a/drivers/pci/controller/cadence/pcie-cadence-host.c
+++ b/drivers/pci/controller/cadence/pcie-cadence-host.c
@@ -510,6 +510,7 @@ int cdns_pcie_host_setup(struct cdns_pcie_rc *rc)
 		cdns_pcie_detect_quiet_min_delay_set(&rc->pcie);
 
 	cdns_pcie_host_enable_ptm_response(pcie);
+	cdns_pcie_init_ptm_phy_latency(dev, pcie);
 
 	ret = cdns_pcie_start_link(pcie);
 	if (ret) {
diff --git a/drivers/pci/controller/cadence/pcie-cadence.c b/drivers/pci/controller/cadence/pcie-cadence.c
index 13c4032ca379..1a282ed9b888 100644
--- a/drivers/pci/controller/cadence/pcie-cadence.c
+++ b/drivers/pci/controller/cadence/pcie-cadence.c
@@ -5,8 +5,100 @@
 
 #include <linux/kernel.h>
 
+#include "../../pci.h"
 #include "pcie-cadence.h"
 
+void cdns_pcie_set_ptm_phy_latency_param(struct cdns_pcie *pcie, bool rx,
+					 u32 speed_index, u32 latency)
+{
+	u32 val;
+
+	/* Set the speed index */
+	val = cdns_pcie_readl(pcie, CDNS_PCIE_LM_PTM_LAT_PARAM_IDX);
+	val = ((val & ~CDNS_PCIE_LM_PTM_LAT_PARAM_IDX_PTMLATIN_MASK) |
+		CDNS_PCIE_LM_PTM_LAT_PARAM_IDX_PTMLATIN(speed_index));
+	cdns_pcie_writel(pcie, CDNS_PCIE_LM_PTM_LAT_PARAM_IDX, val);
+
+	val = cdns_pcie_readl(pcie, CDNS_PCIE_LM_PTM_LAT_PARAM);
+	if (rx)	{
+		/* Set the RX direction latency */
+		val = ((val & ~CDNS_PCIE_LM_PTM_LAT_PARAM_PTMRXLAT_MASK) |
+			CDNS_PCIE_LM_PTM_LAT_PARAM_PTMRXLAT(latency));
+	} else {
+		/* Set TX direction latency */
+		val = ((val & ~CDNS_PCIE_LM_PTM_LAT_PARAM_PTMTXLAT_MASK) |
+			CDNS_PCIE_LM_PTM_LAT_PARAM_PTMTXLAT(latency));
+	}
+	cdns_pcie_writel(pcie, CDNS_PCIE_LM_PTM_LAT_PARAM, val);
+}
+
+static int cdns_pcie_set_ptm_phy_latency(struct device *dev, struct cdns_pcie *pcie,
+					 bool rx, const char *key)
+{
+	struct device_node *np;
+	int max_link_speed;
+	int param_count;
+	u32 latency;
+	int ret;
+	int i;
+
+	/* Do nothing if there is no phy */
+	if (pcie->phy_count < 1)
+		return 0;
+
+	np = dev->of_node;
+	max_link_speed = of_pci_get_max_link_speed(np);
+	if (max_link_speed < 1)
+		return -EINVAL;
+
+	/* Only check and use params of the first phy */
+	np = pcie->phy[0]->dev.of_node;
+	param_count = of_property_count_u32_elems(np, key);
+	if (param_count < 0 || param_count < max_link_speed) {
+		dev_info(dev,
+			 "PTM: no %s set for one or more speeds: %d\n",
+			 key, param_count);
+	}
+
+	/* Don't set param for unsupported speed */
+	if (param_count > max_link_speed)
+		param_count = max_link_speed;
+
+	for (i = 0; i < param_count; i++) {
+		ret = of_property_read_u32_index(np, key, i, &latency);
+		if (ret != 0) {
+			dev_err(dev, "failed to read PTM latency for speed %d from %s\n",
+				i, key);
+			return ret;
+		}
+
+		/* convert ps to ns */
+		latency /= 1000;
+
+		cdns_pcie_set_ptm_phy_latency_param(pcie, rx,
+						    i, latency);
+
+		dev_dbg(dev, "PTM: %s phy latency Gen.%d: %uns\n",
+			rx ? "rx" : "tx", i+1, latency);
+	}
+
+	return 0;
+}
+
+int cdns_pcie_init_ptm_phy_latency(struct device *dev, struct cdns_pcie *pcie)
+{
+	int ret;
+
+	ret = cdns_pcie_set_ptm_phy_latency(dev, pcie, false,
+					    "tx-phy-latency-ps");
+	if (ret)
+		return ret;
+
+	ret = cdns_pcie_set_ptm_phy_latency(dev, pcie, true,
+					    "rx-phy-latency-ps");
+	return ret;
+}
+
 void cdns_pcie_detect_quiet_min_delay_set(struct cdns_pcie *pcie)
 {
 	u32 delay = 0x3;
diff --git a/drivers/pci/controller/cadence/pcie-cadence.h b/drivers/pci/controller/cadence/pcie-cadence.h
index 190786e47df9..483b957a8212 100644
--- a/drivers/pci/controller/cadence/pcie-cadence.h
+++ b/drivers/pci/controller/cadence/pcie-cadence.h
@@ -120,6 +120,26 @@
 #define CDNS_PCIE_LM_PTM_CTRL 	(CDNS_PCIE_LM_BASE + 0x0da8)
 #define CDNS_PCIE_LM_TPM_CTRL_PTMRSEN 	BIT(17)
 
+/* PTM Latency Parameters Index Register */
+#define CDNS_PCIE_LM_PTM_LAT_PARAM_IDX \
+				(CDNS_PCIE_LM_BASE + 0x0db0)
+#define  CDNS_PCIE_LM_PTM_LAT_PARAM_IDX_PTMLATIN_MASK \
+					GENMASK(3, 0)
+#define  CDNS_PCIE_LM_PTM_LAT_PARAM_IDX_PTMLATIN(a) \
+	(((a) << 0) & CDNS_PCIE_LM_PTM_LAT_PARAM_IDX_PTMLATIN_MASK)
+
+/* PTM Latency Parameters Register */
+#define CDNS_PCIE_LM_PTM_LAT_PARAM \
+				(CDNS_PCIE_LM_BASE + 0x0db4)
+#define  CDNS_PCIE_LM_PTM_LAT_PARAM_PTMTXLAT_MASK \
+					GENMASK(9, 0)
+#define  CDNS_PCIE_LM_PTM_LAT_PARAM_PTMTXLAT(a) \
+	(((a) << 0) & CDNS_PCIE_LM_PTM_LAT_PARAM_PTMTXLAT_MASK)
+#define  CDNS_PCIE_LM_PTM_LAT_PARAM_PTMRXLAT_MASK \
+					GENMASK(19, 10)
+#define  CDNS_PCIE_LM_PTM_LAT_PARAM_PTMRXLAT(b) \
+	(((b) << 10) & CDNS_PCIE_LM_PTM_LAT_PARAM_PTMRXLAT_MASK)
+
 /*
  * Endpoint Function Registers (PCI configuration space for endpoint functions)
  */
@@ -541,6 +561,9 @@ static inline int cdns_pcie_ep_setup(struct cdns_pcie_ep *ep)
 #endif
 
 void cdns_pcie_detect_quiet_min_delay_set(struct cdns_pcie *pcie);
+void cdns_pcie_set_ptm_phy_latency_param(struct cdns_pcie *pcie, bool rx,
+					 u32 speed_index, u32 latency);
+int cdns_pcie_init_ptm_phy_latency(struct device *dev, struct cdns_pcie *pcie);
 
 void cdns_pcie_set_outbound_region(struct cdns_pcie *pcie, u8 busnr, u8 fn,
 				   u32 r, bool is_io,
-- 
2.36.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 3/3] arm64: dts: ti: k3-am64: Add PCIe PHY latency DT binding
  2023-04-27  5:50 [PATCH v2 0/3] Cadence PCIe PHY latency for PTM Dominic Rath
  2023-04-27  5:50 ` [PATCH v2 1/3] dt-bindings: phy: cadence-torrent: Add latency properties Dominic Rath
  2023-04-27  5:50 ` [PATCH v2 2/3] PCI: cadence: Use DT bindings to set PHY latencies Dominic Rath
@ 2023-04-27  5:50 ` Dominic Rath
  2023-05-09 15:27   ` Christian Gmeiner
  2023-05-30  8:21 ` [PATCH v2 0/3] Cadence PCIe PHY latency for PTM Christian Gmeiner
  3 siblings, 1 reply; 14+ messages in thread
From: Dominic Rath @ 2023-04-27  5:50 UTC (permalink / raw)
  To: robh+dt, krzysztof.kozlowski+dt, tjoseph
  Cc: bhelgaas, lpieralisi, nm, vigneshr, devicetree, linux-kernel,
	linux-pci, christian.gmeiner, bahle, Dominic Rath

From: Alexander Bahle <bahle@ibv-augsburg.de>

Add DT bindings for the PCIe PHY latencies. Applies to PCIe in host and
endpoint mode. Setting these improves the PTM timestamp accuracy.

The values are taken from the Link below.

Signed-off-by: Alexander Bahle <bahle@ibv-augsburg.de>
Signed-off-by: Dominic Rath <rath@ibv-augsburg.de>
Link: https://e2e.ti.com/support/processors-group/processors/f/processors-forum/998749/am6442-details-regarding-ptm-implementation
---
 arch/arm64/boot/dts/ti/k3-am642-evm.dts | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/ti/k3-am642-evm.dts b/arch/arm64/boot/dts/ti/k3-am642-evm.dts
index 39feea78a084..f448c98f1aa1 100644
--- a/arch/arm64/boot/dts/ti/k3-am642-evm.dts
+++ b/arch/arm64/boot/dts/ti/k3-am642-evm.dts
@@ -552,6 +552,8 @@ serdes0_pcie_link: phy@0 {
 		#phy-cells = <0>;
 		cdns,phy-type = <PHY_TYPE_PCIE>;
 		resets = <&serdes_wiz0 1>;
+		tx-phy-latency-ps = <138800 69400>;
+		rx-phy-latency-ps = <185200 92600>;
 	};
 };
 
-- 
2.36.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/3] dt-bindings: phy: cadence-torrent: Add latency properties
  2023-04-27  5:50 ` [PATCH v2 1/3] dt-bindings: phy: cadence-torrent: Add latency properties Dominic Rath
@ 2023-04-27 18:30   ` Bjorn Helgaas
  2023-05-09 15:31     ` Christian Gmeiner
  2023-05-30  8:19   ` Christian Gmeiner
  1 sibling, 1 reply; 14+ messages in thread
From: Bjorn Helgaas @ 2023-04-27 18:30 UTC (permalink / raw)
  To: Dominic Rath
  Cc: robh+dt, krzysztof.kozlowski+dt, tjoseph, bhelgaas, lpieralisi,
	nm, vigneshr, devicetree, linux-kernel, linux-pci,
	christian.gmeiner, bahle

On Thu, Apr 27, 2023 at 07:50:30AM +0200, Dominic Rath wrote:
> From: Alexander Bahle <bahle@ibv-augsburg.de>
> 
> Add "tx-phy-latency-ps" and "rx-phy-latency-ps" DT bindings for
> setting the PCIe PHY latencies.
> The properties expect a list of uint32 PHY latencies in picoseconds for
> every supported speed starting at PCIe Gen1, e.g.:
> 
>   tx-phy-latency-ps = <100000 200000>; /* Gen1: 100ns, Gen2: 200ns */
>   rx-phy-latency-ps = <150000 250000>; /* Gen1: 150ns, Gen2: 250ns */

Are these things that could/should be described in a more generic
place?  They don't look necessarily Cadence-specific.

Bjorn

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 2/3] PCI: cadence: Use DT bindings to set PHY latencies
  2023-04-27  5:50 ` [PATCH v2 2/3] PCI: cadence: Use DT bindings to set PHY latencies Dominic Rath
@ 2023-05-04 11:33   ` Verma, Achal
  2023-05-30  8:20     ` Christian Gmeiner
  0 siblings, 1 reply; 14+ messages in thread
From: Verma, Achal @ 2023-05-04 11:33 UTC (permalink / raw)
  To: Dominic Rath, robh+dt, krzysztof.kozlowski+dt, tjoseph
  Cc: bhelgaas, lpieralisi, nm, vigneshr, devicetree, linux-kernel,
	linux-pci, christian.gmeiner, bahle



On 4/27/2023 11:20 AM, Dominic Rath wrote:
> From: Alexander Bahle <bahle@ibv-augsburg.de>
> 
> Use optional "tx-phy-latency-ps" and "rx-phy-latency-ps"
> DeviceTree bindings to set the CDNS_PCIE_LM_PTM_LAT_PARAM(_IDX)
> register(s) during PCIe host and endpoint setup.
> 
> The properties are lists of uint32 PHY latencies in picoseconds for
> every supported speed starting at PCIe Gen1, e.g.:
> 
>    tx-phy-latency-ps = <100000 200000>; /* Gen1: 100ns, Gen2: 200ns */
>    rx-phy-latency-ps = <150000 250000>; /* Gen1: 150ns, Gen2: 250ns */
> 
> There should be a value for every supported speed, otherwise a info
> message is emitted to let users know that the PTM timestamps from this
> PCIe device may not be precise enough for some applications.
> 
> Signed-off-by: Alexander Bahle <bahle@ibv-augsburg.de>
> Signed-off-by: Dominic Rath <rath@ibv-augsburg.de>
> ---
>   .../pci/controller/cadence/pcie-cadence-ep.c  |  2 +
>   .../controller/cadence/pcie-cadence-host.c    |  1 +
>   drivers/pci/controller/cadence/pcie-cadence.c | 92 +++++++++++++++++++
>   drivers/pci/controller/cadence/pcie-cadence.h | 23 +++++
>   4 files changed, 118 insertions(+)
> 
> diff --git a/drivers/pci/controller/cadence/pcie-cadence-ep.c b/drivers/pci/controller/cadence/pcie-cadence-ep.c
> index b8b655d4047e..6e39126922d1 100644
> --- a/drivers/pci/controller/cadence/pcie-cadence-ep.c
> +++ b/drivers/pci/controller/cadence/pcie-cadence-ep.c
> @@ -664,6 +664,8 @@ int cdns_pcie_ep_setup(struct cdns_pcie_ep *ep)
>   	}
>   	pcie->mem_res = res;
>   
> +	cdns_pcie_init_ptm_phy_latency(dev, pcie);
> +
>   	ep->max_regions = CDNS_PCIE_MAX_OB;
>   	of_property_read_u32(np, "cdns,max-outbound-regions", &ep->max_regions);
>   
> diff --git a/drivers/pci/controller/cadence/pcie-cadence-host.c b/drivers/pci/controller/cadence/pcie-cadence-host.c
> index 940c7dd701d6..8933002f828e 100644
> --- a/drivers/pci/controller/cadence/pcie-cadence-host.c
> +++ b/drivers/pci/controller/cadence/pcie-cadence-host.c
> @@ -510,6 +510,7 @@ int cdns_pcie_host_setup(struct cdns_pcie_rc *rc)
>   		cdns_pcie_detect_quiet_min_delay_set(&rc->pcie);
>   
>   	cdns_pcie_host_enable_ptm_response(pcie)
Hi Dominic,
Shouldn't cdns_pcie_init_ptm_phy_latency() called before 
cdns_pcie_host_enable_ptm_response(), enabling host to reply PTM 
requests. However host could receive PTM requests later only after 
cdns_pcie_start_link().
> +	cdns_pcie_init_ptm_phy_latency(dev, pcie);
>   
>   	ret = cdns_pcie_start_link(pcie);
>   	if (ret) {
> diff --git a/drivers/pci/controller/cadence/pcie-cadence.c b/drivers/pci/controller/cadence/pcie-cadence.c
> index 13c4032ca379..1a282ed9b888 100644
> --- a/drivers/pci/controller/cadence/pcie-cadence.c
> +++ b/drivers/pci/controller/cadence/pcie-cadence.c
> @@ -5,8 +5,100 @@
>   
>   #include <linux/kernel.h>
>   
> +#include "../../pci.h"
>   #include "pcie-cadence.h"
>   
> +void cdns_pcie_set_ptm_phy_latency_param(struct cdns_pcie *pcie, bool rx,
> +					 u32 speed_index, u32 latency)
> +{
> +	u32 val;
> +
> +	/* Set the speed index */
> +	val = cdns_pcie_readl(pcie, CDNS_PCIE_LM_PTM_LAT_PARAM_IDX);
> +	val = ((val & ~CDNS_PCIE_LM_PTM_LAT_PARAM_IDX_PTMLATIN_MASK) |
> +		CDNS_PCIE_LM_PTM_LAT_PARAM_IDX_PTMLATIN(speed_index));
> +	cdns_pcie_writel(pcie, CDNS_PCIE_LM_PTM_LAT_PARAM_IDX, val);
> +
> +	val = cdns_pcie_readl(pcie, CDNS_PCIE_LM_PTM_LAT_PARAM);
> +	if (rx)	{
> +		/* Set the RX direction latency */
> +		val = ((val & ~CDNS_PCIE_LM_PTM_LAT_PARAM_PTMRXLAT_MASK) |
> +			CDNS_PCIE_LM_PTM_LAT_PARAM_PTMRXLAT(latency));
> +	} else {
> +		/* Set TX direction latency */
> +		val = ((val & ~CDNS_PCIE_LM_PTM_LAT_PARAM_PTMTXLAT_MASK) |
> +			CDNS_PCIE_LM_PTM_LAT_PARAM_PTMTXLAT(latency));
> +	}
> +	cdns_pcie_writel(pcie, CDNS_PCIE_LM_PTM_LAT_PARAM, val);
> +}
> +
> +static int cdns_pcie_set_ptm_phy_latency(struct device *dev, struct cdns_pcie *pcie,
> +					 bool rx, const char *key)
> +{
> +	struct device_node *np;
> +	int max_link_speed;
> +	int param_count;
> +	u32 latency;
> +	int ret;
> +	int i;
> +
> +	/* Do nothing if there is no phy */
> +	if (pcie->phy_count < 1)
> +		return 0;
> +
> +	np = dev->of_node;
> +	max_link_speed = of_pci_get_max_link_speed(np);
> +	if (max_link_speed < 1)
> +		return -EINVAL;
> +
> +	/* Only check and use params of the first phy */
> +	np = pcie->phy[0]->dev.of_node;
> +	param_count = of_property_count_u32_elems(np, key);
> +	if (param_count < 0 || param_count < max_link_speed) {
> +		dev_info(dev,
> +			 "PTM: no %s set for one or more speeds: %d\n",
> +			 key, param_count);
> +	}
> +
> +	/* Don't set param for unsupported speed */
> +	if (param_count > max_link_speed)
> +		param_count = max_link_speed;
> +
> +	for (i = 0; i < param_count; i++) {
> +		ret = of_property_read_u32_index(np, key, i, &latency);
> +		if (ret != 0) {
> +			dev_err(dev, "failed to read PTM latency for speed %d from %s\n",
> +				i, key);
> +			return ret;
> +		}
> +
> +		/* convert ps to ns */
> +		latency /= 1000;
> +
> +		cdns_pcie_set_ptm_phy_latency_param(pcie, rx,
> +						    i, latency);
> +
> +		dev_dbg(dev, "PTM: %s phy latency Gen.%d: %uns\n",
> +			rx ? "rx" : "tx", i+1, latency);
> +	}
> +
> +	return 0;
> +}
> +
> +int cdns_pcie_init_ptm_phy_latency(struct device *dev, struct cdns_pcie *pcie)
> +{
> +	int ret;
> +
> +	ret = cdns_pcie_set_ptm_phy_latency(dev, pcie, false,
> +					    "tx-phy-latency-ps");
> +	if (ret)
> +		return ret;
> +
> +	ret = cdns_pcie_set_ptm_phy_latency(dev, pcie, true,
> +					    "rx-phy-latency-ps");
> +	return ret;
> +}
> +
>   void cdns_pcie_detect_quiet_min_delay_set(struct cdns_pcie *pcie)
>   {
>   	u32 delay = 0x3;
> diff --git a/drivers/pci/controller/cadence/pcie-cadence.h b/drivers/pci/controller/cadence/pcie-cadence.h
> index 190786e47df9..483b957a8212 100644
> --- a/drivers/pci/controller/cadence/pcie-cadence.h
> +++ b/drivers/pci/controller/cadence/pcie-cadence.h
> @@ -120,6 +120,26 @@
>   #define CDNS_PCIE_LM_PTM_CTRL 	(CDNS_PCIE_LM_BASE + 0x0da8)
>   #define CDNS_PCIE_LM_TPM_CTRL_PTMRSEN 	BIT(17)
>   
> +/* PTM Latency Parameters Index Register */
> +#define CDNS_PCIE_LM_PTM_LAT_PARAM_IDX \
> +				(CDNS_PCIE_LM_BASE + 0x0db0)
> +#define  CDNS_PCIE_LM_PTM_LAT_PARAM_IDX_PTMLATIN_MASK \
> +					GENMASK(3, 0)
> +#define  CDNS_PCIE_LM_PTM_LAT_PARAM_IDX_PTMLATIN(a) \
> +	(((a) << 0) & CDNS_PCIE_LM_PTM_LAT_PARAM_IDX_PTMLATIN_MASK)
> +
> +/* PTM Latency Parameters Register */
> +#define CDNS_PCIE_LM_PTM_LAT_PARAM \
> +				(CDNS_PCIE_LM_BASE + 0x0db4)
> +#define  CDNS_PCIE_LM_PTM_LAT_PARAM_PTMTXLAT_MASK \
> +					GENMASK(9, 0)
> +#define  CDNS_PCIE_LM_PTM_LAT_PARAM_PTMTXLAT(a) \
> +	(((a) << 0) & CDNS_PCIE_LM_PTM_LAT_PARAM_PTMTXLAT_MASK)
> +#define  CDNS_PCIE_LM_PTM_LAT_PARAM_PTMRXLAT_MASK \
> +					GENMASK(19, 10)
> +#define  CDNS_PCIE_LM_PTM_LAT_PARAM_PTMRXLAT(b) \
> +	(((b) << 10) & CDNS_PCIE_LM_PTM_LAT_PARAM_PTMRXLAT_MASK)
> +
>   /*
>    * Endpoint Function Registers (PCI configuration space for endpoint functions)
>    */
> @@ -541,6 +561,9 @@ static inline int cdns_pcie_ep_setup(struct cdns_pcie_ep *ep)
>   #endif
>   
>   void cdns_pcie_detect_quiet_min_delay_set(struct cdns_pcie *pcie);
> +void cdns_pcie_set_ptm_phy_latency_param(struct cdns_pcie *pcie, bool rx,
> +					 u32 speed_index, u32 latency);
> +int cdns_pcie_init_ptm_phy_latency(struct device *dev, struct cdns_pcie *pcie);
>   
>   void cdns_pcie_set_outbound_region(struct cdns_pcie *pcie, u8 busnr, u8 fn,
>   				   u32 r, bool is_io,
Acked-by: Achal Verma <a-verma1@ti.com>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 3/3] arm64: dts: ti: k3-am64: Add PCIe PHY latency DT binding
  2023-04-27  5:50 ` [PATCH v2 3/3] arm64: dts: ti: k3-am64: Add PCIe PHY latency DT binding Dominic Rath
@ 2023-05-09 15:27   ` Christian Gmeiner
  0 siblings, 0 replies; 14+ messages in thread
From: Christian Gmeiner @ 2023-05-09 15:27 UTC (permalink / raw)
  To: Dominic Rath
  Cc: robh+dt, krzysztof.kozlowski+dt, tjoseph, bhelgaas, lpieralisi,
	nm, vigneshr, devicetree, linux-kernel, linux-pci, bahle

Hi

>
> From: Alexander Bahle <bahle@ibv-augsburg.de>
>
> Add DT bindings for the PCIe PHY latencies. Applies to PCIe in host and
> endpoint mode. Setting these improves the PTM timestamp accuracy.
>
> The values are taken from the Link below.
>
> Signed-off-by: Alexander Bahle <bahle@ibv-augsburg.de>
> Signed-off-by: Dominic Rath <rath@ibv-augsburg.de>
> Link: https://e2e.ti.com/support/processors-group/processors/f/processors-forum/998749/am6442-details-regarding-ptm-implementation
> ---

Would it not make sense to add these dts properties to
k3-am64-main.dtsi? At least this is what the commit message says.

>  arch/arm64/boot/dts/ti/k3-am642-evm.dts | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/ti/k3-am642-evm.dts b/arch/arm64/boot/dts/ti/k3-am642-evm.dts
> index 39feea78a084..f448c98f1aa1 100644
> --- a/arch/arm64/boot/dts/ti/k3-am642-evm.dts
> +++ b/arch/arm64/boot/dts/ti/k3-am642-evm.dts
> @@ -552,6 +552,8 @@ serdes0_pcie_link: phy@0 {
>                 #phy-cells = <0>;
>                 cdns,phy-type = <PHY_TYPE_PCIE>;
>                 resets = <&serdes_wiz0 1>;
> +               tx-phy-latency-ps = <138800 69400>;
> +               rx-phy-latency-ps = <185200 92600>;
>         };
>  };
>
> --
> 2.36.0
>

-- 
greets
--
Christian Gmeiner, MSc

https://christian-gmeiner.info/privacypolicy

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/3] dt-bindings: phy: cadence-torrent: Add latency properties
  2023-04-27 18:30   ` Bjorn Helgaas
@ 2023-05-09 15:31     ` Christian Gmeiner
  2023-05-09 21:57       ` Bjorn Helgaas
  0 siblings, 1 reply; 14+ messages in thread
From: Christian Gmeiner @ 2023-05-09 15:31 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Dominic Rath, robh+dt, krzysztof.kozlowski+dt, tjoseph, bhelgaas,
	lpieralisi, nm, vigneshr, devicetree, linux-kernel, linux-pci,
	bahle

Hi Bjorn,

>
> On Thu, Apr 27, 2023 at 07:50:30AM +0200, Dominic Rath wrote:
> > From: Alexander Bahle <bahle@ibv-augsburg.de>
> >
> > Add "tx-phy-latency-ps" and "rx-phy-latency-ps" DT bindings for
> > setting the PCIe PHY latencies.
> > The properties expect a list of uint32 PHY latencies in picoseconds for
> > every supported speed starting at PCIe Gen1, e.g.:
> >
> >   tx-phy-latency-ps = <100000 200000>; /* Gen1: 100ns, Gen2: 200ns */
> >   rx-phy-latency-ps = <150000 250000>; /* Gen1: 150ns, Gen2: 250ns */
>
> Are these things that could/should be described in a more generic
> place?  They don't look necessarily Cadence-specific.
>

As there is currently no generic binding, would you like to see a new
yaml binding
added (Documentation/devicetree/bindings/phy/phy.yaml) that contains just the
two phy properties?

-- 
greets
--
Christian Gmeiner, MSc

https://christian-gmeiner.info/privacypolicy

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/3] dt-bindings: phy: cadence-torrent: Add latency properties
  2023-05-09 15:31     ` Christian Gmeiner
@ 2023-05-09 21:57       ` Bjorn Helgaas
  2023-05-10  7:08         ` Krzysztof Kozlowski
  0 siblings, 1 reply; 14+ messages in thread
From: Bjorn Helgaas @ 2023-05-09 21:57 UTC (permalink / raw)
  To: Christian Gmeiner, Rob Herring
  Cc: Dominic Rath, krzysztof.kozlowski+dt, tjoseph, bhelgaas,
	lpieralisi, nm, vigneshr, devicetree, linux-kernel, linux-pci,
	bahle

On Tue, May 09, 2023 at 05:31:19PM +0200, Christian Gmeiner wrote:
> > On Thu, Apr 27, 2023 at 07:50:30AM +0200, Dominic Rath wrote:
> > > From: Alexander Bahle <bahle@ibv-augsburg.de>
> > >
> > > Add "tx-phy-latency-ps" and "rx-phy-latency-ps" DT bindings for
> > > setting the PCIe PHY latencies.
> > > The properties expect a list of uint32 PHY latencies in picoseconds for
> > > every supported speed starting at PCIe Gen1, e.g.:
> > >
> > >   tx-phy-latency-ps = <100000 200000>; /* Gen1: 100ns, Gen2: 200ns */
> > >   rx-phy-latency-ps = <150000 250000>; /* Gen1: 150ns, Gen2: 250ns */
> >
> > Are these things that could/should be described in a more generic
> > place?  They don't look necessarily Cadence-specific.
> 
> As there is currently no generic binding, would you like to see a new
> yaml binding
> added (Documentation/devicetree/bindings/phy/phy.yaml) that contains just the
> two phy properties?

The whole thing is more a question for Rob.

Bjorn

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/3] dt-bindings: phy: cadence-torrent: Add latency properties
  2023-05-09 21:57       ` Bjorn Helgaas
@ 2023-05-10  7:08         ` Krzysztof Kozlowski
  2023-05-10 21:12           ` Bjorn Helgaas
  0 siblings, 1 reply; 14+ messages in thread
From: Krzysztof Kozlowski @ 2023-05-10  7:08 UTC (permalink / raw)
  To: Bjorn Helgaas, Christian Gmeiner, Rob Herring
  Cc: Dominic Rath, krzysztof.kozlowski+dt, tjoseph, bhelgaas,
	lpieralisi, nm, vigneshr, devicetree, linux-kernel, linux-pci,
	bahle

On 09/05/2023 23:57, Bjorn Helgaas wrote:
> On Tue, May 09, 2023 at 05:31:19PM +0200, Christian Gmeiner wrote:
>>> On Thu, Apr 27, 2023 at 07:50:30AM +0200, Dominic Rath wrote:
>>>> From: Alexander Bahle <bahle@ibv-augsburg.de>
>>>>
>>>> Add "tx-phy-latency-ps" and "rx-phy-latency-ps" DT bindings for
>>>> setting the PCIe PHY latencies.
>>>> The properties expect a list of uint32 PHY latencies in picoseconds for
>>>> every supported speed starting at PCIe Gen1, e.g.:
>>>>
>>>>   tx-phy-latency-ps = <100000 200000>; /* Gen1: 100ns, Gen2: 200ns */
>>>>   rx-phy-latency-ps = <150000 250000>; /* Gen1: 150ns, Gen2: 250ns */
>>>
>>> Are these things that could/should be described in a more generic
>>> place?  They don't look necessarily Cadence-specific.
>>
>> As there is currently no generic binding, would you like to see a new
>> yaml binding
>> added (Documentation/devicetree/bindings/phy/phy.yaml) that contains just the
>> two phy properties?
> 
> The whole thing is more a question for Rob.

For which you might wait a bit currently.

If the question is only about location of the properties - device schema
or something generic - then for now you can keep it here. Moving to
generic schema is always easy later.

Better to have proper names for properties.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/3] dt-bindings: phy: cadence-torrent: Add latency properties
  2023-05-10  7:08         ` Krzysztof Kozlowski
@ 2023-05-10 21:12           ` Bjorn Helgaas
  0 siblings, 0 replies; 14+ messages in thread
From: Bjorn Helgaas @ 2023-05-10 21:12 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Christian Gmeiner, Rob Herring, Dominic Rath,
	krzysztof.kozlowski+dt, tjoseph, bhelgaas, lpieralisi, nm,
	vigneshr, devicetree, linux-kernel, linux-pci, bahle

On Wed, May 10, 2023 at 09:08:39AM +0200, Krzysztof Kozlowski wrote:
> On 09/05/2023 23:57, Bjorn Helgaas wrote:
> > On Tue, May 09, 2023 at 05:31:19PM +0200, Christian Gmeiner wrote:
> >>> On Thu, Apr 27, 2023 at 07:50:30AM +0200, Dominic Rath wrote:
> >>>> From: Alexander Bahle <bahle@ibv-augsburg.de>
> >>>>
> >>>> Add "tx-phy-latency-ps" and "rx-phy-latency-ps" DT bindings for
> >>>> setting the PCIe PHY latencies.
> >>>> The properties expect a list of uint32 PHY latencies in picoseconds for
> >>>> every supported speed starting at PCIe Gen1, e.g.:
> >>>>
> >>>>   tx-phy-latency-ps = <100000 200000>; /* Gen1: 100ns, Gen2: 200ns */
> >>>>   rx-phy-latency-ps = <150000 250000>; /* Gen1: 150ns, Gen2: 250ns */
> >>>
> >>> Are these things that could/should be described in a more generic
> >>> place?  They don't look necessarily Cadence-specific.
> >>
> >> As there is currently no generic binding, would you like to see a new
> >> yaml binding
> >> added (Documentation/devicetree/bindings/phy/phy.yaml) that contains just the
> >> two phy properties?
> > 
> > The whole thing is more a question for Rob.
> 
> For which you might wait a bit currently.
> 
> If the question is only about location of the properties - device schema
> or something generic - then for now you can keep it here. Moving to
> generic schema is always easy later.
> 
> Better to have proper names for properties.

Good point.  The current names seem fine to me since the names
themselves aren't Cadence-specific.

Bjorn

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/3] dt-bindings: phy: cadence-torrent: Add latency properties
  2023-04-27  5:50 ` [PATCH v2 1/3] dt-bindings: phy: cadence-torrent: Add latency properties Dominic Rath
  2023-04-27 18:30   ` Bjorn Helgaas
@ 2023-05-30  8:19   ` Christian Gmeiner
  1 sibling, 0 replies; 14+ messages in thread
From: Christian Gmeiner @ 2023-05-30  8:19 UTC (permalink / raw)
  To: Dominic Rath
  Cc: robh+dt, krzysztof.kozlowski+dt, tjoseph, bhelgaas, lpieralisi,
	nm, vigneshr, devicetree, linux-kernel, linux-pci, bahle

>
> From: Alexander Bahle <bahle@ibv-augsburg.de>
>
> Add "tx-phy-latency-ps" and "rx-phy-latency-ps" DT bindings for
> setting the PCIe PHY latencies.
> The properties expect a list of uint32 PHY latencies in picoseconds for
> every supported speed starting at PCIe Gen1, e.g.:
>
>   tx-phy-latency-ps = <100000 200000>; /* Gen1: 100ns, Gen2: 200ns */
>   rx-phy-latency-ps = <150000 250000>; /* Gen1: 150ns, Gen2: 250ns */
>
> Signed-off-by: Alexander Bahle <bahle@ibv-augsburg.de>
> Signed-off-by: Dominic Rath <rath@ibv-augsburg.de>

Reviewed-by: Christian Gmeiner <christian.gmeiner@gmail.com>

> ---
>  .../bindings/phy/phy-cadence-torrent.yaml     | 20 +++++++++++++++++++
>  1 file changed, 20 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/phy/phy-cadence-torrent.yaml b/Documentation/devicetree/bindings/phy/phy-cadence-torrent.yaml
> index 2ad1faadda2a..93228a304395 100644
> --- a/Documentation/devicetree/bindings/phy/phy-cadence-torrent.yaml
> +++ b/Documentation/devicetree/bindings/phy/phy-cadence-torrent.yaml
> @@ -126,6 +126,24 @@ patternProperties:
>          enum: [2160, 2430, 2700, 3240, 4320, 5400, 8100]
>          default: 8100
>
> +      tx-phy-latency-ps:
> +        description:
> +          The PHY latencies for the TX direction applied to PCIe PTM timestamps. Most
> +          PCIe PHYs have asynchronous latencies for their RX and TX paths. To obtain
> +          accurate PTM timestamps, the PCIe PTM specification requires that the time
> +          at which the first serial bit is present on the serial lines be taken.
> +          Should contain picosecond latency values for each supported speed,
> +          starting with Gen1 latency.
> +
> +      rx-phy-latency-ps:
> +        description:
> +          The PHY latencies for the RX direction applied to the PTM timestamps. Most
> +          PCIe PHYs have asynchronous latencies for their RX and TX paths. To obtain
> +          accurate PTM timestamps, the PCIe PTM specification requires that the time
> +          at which the first serial bit is present on the serial lines be taken.
> +          Should contain picosecond latency values for each supported speed,
> +          starting with Gen1 latency.
> +
>      required:
>        - reg
>        - resets
> @@ -203,6 +221,8 @@ examples:
>                  cdns,phy-type = <PHY_TYPE_PCIE>;
>                  cdns,num-lanes = <2>;
>                  cdns,ssc-mode = <CDNS_SERDES_NO_SSC>;
> +                tx-phy-latency-ps = <138800 69400>;
> +                rx-phy-latency-ps = <185200 92600>;
>              };
>
>              phy@2 {
> --
> 2.36.0
>


-- 
greets
--
Christian Gmeiner, MSc

https://christian-gmeiner.info/privacypolicy

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 2/3] PCI: cadence: Use DT bindings to set PHY latencies
  2023-05-04 11:33   ` Verma, Achal
@ 2023-05-30  8:20     ` Christian Gmeiner
  0 siblings, 0 replies; 14+ messages in thread
From: Christian Gmeiner @ 2023-05-30  8:20 UTC (permalink / raw)
  To: Verma, Achal
  Cc: Dominic Rath, robh+dt, krzysztof.kozlowski+dt, tjoseph, bhelgaas,
	lpieralisi, nm, vigneshr, devicetree, linux-kernel, linux-pci,
	bahle

Am Do., 4. Mai 2023 um 13:33 Uhr schrieb Verma, Achal <a-verma1@ti.com>:
>
>
>
> On 4/27/2023 11:20 AM, Dominic Rath wrote:
> > From: Alexander Bahle <bahle@ibv-augsburg.de>
> >
> > Use optional "tx-phy-latency-ps" and "rx-phy-latency-ps"
> > DeviceTree bindings to set the CDNS_PCIE_LM_PTM_LAT_PARAM(_IDX)
> > register(s) during PCIe host and endpoint setup.
> >
> > The properties are lists of uint32 PHY latencies in picoseconds for
> > every supported speed starting at PCIe Gen1, e.g.:
> >
> >    tx-phy-latency-ps = <100000 200000>; /* Gen1: 100ns, Gen2: 200ns */
> >    rx-phy-latency-ps = <150000 250000>; /* Gen1: 150ns, Gen2: 250ns */
> >
> > There should be a value for every supported speed, otherwise a info
> > message is emitted to let users know that the PTM timestamps from this
> > PCIe device may not be precise enough for some applications.
> >
> > Signed-off-by: Alexander Bahle <bahle@ibv-augsburg.de>
> > Signed-off-by: Dominic Rath <rath@ibv-augsburg.de>
> > ---
> >   .../pci/controller/cadence/pcie-cadence-ep.c  |  2 +
> >   .../controller/cadence/pcie-cadence-host.c    |  1 +
> >   drivers/pci/controller/cadence/pcie-cadence.c | 92 +++++++++++++++++++
> >   drivers/pci/controller/cadence/pcie-cadence.h | 23 +++++
> >   4 files changed, 118 insertions(+)
> >
> > diff --git a/drivers/pci/controller/cadence/pcie-cadence-ep.c b/drivers/pci/controller/cadence/pcie-cadence-ep.c
> > index b8b655d4047e..6e39126922d1 100644
> > --- a/drivers/pci/controller/cadence/pcie-cadence-ep.c
> > +++ b/drivers/pci/controller/cadence/pcie-cadence-ep.c
> > @@ -664,6 +664,8 @@ int cdns_pcie_ep_setup(struct cdns_pcie_ep *ep)
> >       }
> >       pcie->mem_res = res;
> >
> > +     cdns_pcie_init_ptm_phy_latency(dev, pcie);
> > +
> >       ep->max_regions = CDNS_PCIE_MAX_OB;
> >       of_property_read_u32(np, "cdns,max-outbound-regions", &ep->max_regions);
> >
> > diff --git a/drivers/pci/controller/cadence/pcie-cadence-host.c b/drivers/pci/controller/cadence/pcie-cadence-host.c
> > index 940c7dd701d6..8933002f828e 100644
> > --- a/drivers/pci/controller/cadence/pcie-cadence-host.c
> > +++ b/drivers/pci/controller/cadence/pcie-cadence-host.c
> > @@ -510,6 +510,7 @@ int cdns_pcie_host_setup(struct cdns_pcie_rc *rc)
> >               cdns_pcie_detect_quiet_min_delay_set(&rc->pcie);
> >
> >       cdns_pcie_host_enable_ptm_response(pcie)
> Hi Dominic,
> Shouldn't cdns_pcie_init_ptm_phy_latency() called before
> cdns_pcie_host_enable_ptm_response(), enabling host to reply PTM
> requests. However host could receive PTM requests later only after
> cdns_pcie_start_link().

Good catch!


> > +     cdns_pcie_init_ptm_phy_latency(dev, pcie);
> >
> >       ret = cdns_pcie_start_link(pcie);
> >       if (ret) {
> > diff --git a/drivers/pci/controller/cadence/pcie-cadence.c b/drivers/pci/controller/cadence/pcie-cadence.c
> > index 13c4032ca379..1a282ed9b888 100644
> > --- a/drivers/pci/controller/cadence/pcie-cadence.c
> > +++ b/drivers/pci/controller/cadence/pcie-cadence.c
> > @@ -5,8 +5,100 @@
> >
> >   #include <linux/kernel.h>
> >
> > +#include "../../pci.h"
> >   #include "pcie-cadence.h"
> >
> > +void cdns_pcie_set_ptm_phy_latency_param(struct cdns_pcie *pcie, bool rx,
> > +                                      u32 speed_index, u32 latency)
> > +{
> > +     u32 val;
> > +
> > +     /* Set the speed index */
> > +     val = cdns_pcie_readl(pcie, CDNS_PCIE_LM_PTM_LAT_PARAM_IDX);
> > +     val = ((val & ~CDNS_PCIE_LM_PTM_LAT_PARAM_IDX_PTMLATIN_MASK) |
> > +             CDNS_PCIE_LM_PTM_LAT_PARAM_IDX_PTMLATIN(speed_index));
> > +     cdns_pcie_writel(pcie, CDNS_PCIE_LM_PTM_LAT_PARAM_IDX, val);
> > +
> > +     val = cdns_pcie_readl(pcie, CDNS_PCIE_LM_PTM_LAT_PARAM);
> > +     if (rx) {
> > +             /* Set the RX direction latency */
> > +             val = ((val & ~CDNS_PCIE_LM_PTM_LAT_PARAM_PTMRXLAT_MASK) |
> > +                     CDNS_PCIE_LM_PTM_LAT_PARAM_PTMRXLAT(latency));
> > +     } else {
> > +             /* Set TX direction latency */
> > +             val = ((val & ~CDNS_PCIE_LM_PTM_LAT_PARAM_PTMTXLAT_MASK) |
> > +                     CDNS_PCIE_LM_PTM_LAT_PARAM_PTMTXLAT(latency));
> > +     }
> > +     cdns_pcie_writel(pcie, CDNS_PCIE_LM_PTM_LAT_PARAM, val);
> > +}
> > +
> > +static int cdns_pcie_set_ptm_phy_latency(struct device *dev, struct cdns_pcie *pcie,
> > +                                      bool rx, const char *key)
> > +{
> > +     struct device_node *np;
> > +     int max_link_speed;
> > +     int param_count;
> > +     u32 latency;
> > +     int ret;
> > +     int i;
> > +
> > +     /* Do nothing if there is no phy */
> > +     if (pcie->phy_count < 1)
> > +             return 0;
> > +
> > +     np = dev->of_node;
> > +     max_link_speed = of_pci_get_max_link_speed(np);
> > +     if (max_link_speed < 1)
> > +             return -EINVAL;
> > +
> > +     /* Only check and use params of the first phy */
> > +     np = pcie->phy[0]->dev.of_node;
> > +     param_count = of_property_count_u32_elems(np, key);
> > +     if (param_count < 0 || param_count < max_link_speed) {
> > +             dev_info(dev,
> > +                      "PTM: no %s set for one or more speeds: %d\n",
> > +                      key, param_count);
> > +     }
> > +
> > +     /* Don't set param for unsupported speed */
> > +     if (param_count > max_link_speed)
> > +             param_count = max_link_speed;
> > +
> > +     for (i = 0; i < param_count; i++) {
> > +             ret = of_property_read_u32_index(np, key, i, &latency);
> > +             if (ret != 0) {
> > +                     dev_err(dev, "failed to read PTM latency for speed %d from %s\n",
> > +                             i, key);
> > +                     return ret;
> > +             }
> > +
> > +             /* convert ps to ns */
> > +             latency /= 1000;
> > +
> > +             cdns_pcie_set_ptm_phy_latency_param(pcie, rx,
> > +                                                 i, latency);
> > +
> > +             dev_dbg(dev, "PTM: %s phy latency Gen.%d: %uns\n",
> > +                     rx ? "rx" : "tx", i+1, latency);
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> > +int cdns_pcie_init_ptm_phy_latency(struct device *dev, struct cdns_pcie *pcie)
> > +{
> > +     int ret;
> > +
> > +     ret = cdns_pcie_set_ptm_phy_latency(dev, pcie, false,
> > +                                         "tx-phy-latency-ps");
> > +     if (ret)
> > +             return ret;
> > +
> > +     ret = cdns_pcie_set_ptm_phy_latency(dev, pcie, true,
> > +                                         "rx-phy-latency-ps");
> > +     return ret;
> > +}
> > +
> >   void cdns_pcie_detect_quiet_min_delay_set(struct cdns_pcie *pcie)
> >   {
> >       u32 delay = 0x3;
> > diff --git a/drivers/pci/controller/cadence/pcie-cadence.h b/drivers/pci/controller/cadence/pcie-cadence.h
> > index 190786e47df9..483b957a8212 100644
> > --- a/drivers/pci/controller/cadence/pcie-cadence.h
> > +++ b/drivers/pci/controller/cadence/pcie-cadence.h
> > @@ -120,6 +120,26 @@
> >   #define CDNS_PCIE_LM_PTM_CTRL       (CDNS_PCIE_LM_BASE + 0x0da8)
> >   #define CDNS_PCIE_LM_TPM_CTRL_PTMRSEN       BIT(17)
> >
> > +/* PTM Latency Parameters Index Register */
> > +#define CDNS_PCIE_LM_PTM_LAT_PARAM_IDX \
> > +                             (CDNS_PCIE_LM_BASE + 0x0db0)
> > +#define  CDNS_PCIE_LM_PTM_LAT_PARAM_IDX_PTMLATIN_MASK \
> > +                                     GENMASK(3, 0)
> > +#define  CDNS_PCIE_LM_PTM_LAT_PARAM_IDX_PTMLATIN(a) \
> > +     (((a) << 0) & CDNS_PCIE_LM_PTM_LAT_PARAM_IDX_PTMLATIN_MASK)
> > +
> > +/* PTM Latency Parameters Register */
> > +#define CDNS_PCIE_LM_PTM_LAT_PARAM \
> > +                             (CDNS_PCIE_LM_BASE + 0x0db4)
> > +#define  CDNS_PCIE_LM_PTM_LAT_PARAM_PTMTXLAT_MASK \
> > +                                     GENMASK(9, 0)
> > +#define  CDNS_PCIE_LM_PTM_LAT_PARAM_PTMTXLAT(a) \
> > +     (((a) << 0) & CDNS_PCIE_LM_PTM_LAT_PARAM_PTMTXLAT_MASK)
> > +#define  CDNS_PCIE_LM_PTM_LAT_PARAM_PTMRXLAT_MASK \
> > +                                     GENMASK(19, 10)
> > +#define  CDNS_PCIE_LM_PTM_LAT_PARAM_PTMRXLAT(b) \
> > +     (((b) << 10) & CDNS_PCIE_LM_PTM_LAT_PARAM_PTMRXLAT_MASK)
> > +
> >   /*
> >    * Endpoint Function Registers (PCI configuration space for endpoint functions)
> >    */
> > @@ -541,6 +561,9 @@ static inline int cdns_pcie_ep_setup(struct cdns_pcie_ep *ep)
> >   #endif
> >
> >   void cdns_pcie_detect_quiet_min_delay_set(struct cdns_pcie *pcie);
> > +void cdns_pcie_set_ptm_phy_latency_param(struct cdns_pcie *pcie, bool rx,
> > +                                      u32 speed_index, u32 latency);
> > +int cdns_pcie_init_ptm_phy_latency(struct device *dev, struct cdns_pcie *pcie);
> >
> >   void cdns_pcie_set_outbound_region(struct cdns_pcie *pcie, u8 busnr, u8 fn,
> >                                  u32 r, bool is_io,
> Acked-by: Achal Verma <a-verma1@ti.com>

With the suggested change:
Reviewed-by: Christian Gmeiner <christian.gmeiner@gmail.com>

-- 
greets
--
Christian Gmeiner, MSc

https://christian-gmeiner.info/privacypolicy

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 0/3] Cadence PCIe PHY latency for PTM
  2023-04-27  5:50 [PATCH v2 0/3] Cadence PCIe PHY latency for PTM Dominic Rath
                   ` (2 preceding siblings ...)
  2023-04-27  5:50 ` [PATCH v2 3/3] arm64: dts: ti: k3-am64: Add PCIe PHY latency DT binding Dominic Rath
@ 2023-05-30  8:21 ` Christian Gmeiner
  3 siblings, 0 replies; 14+ messages in thread
From: Christian Gmeiner @ 2023-05-30  8:21 UTC (permalink / raw)
  To: Dominic Rath
  Cc: robh+dt, krzysztof.kozlowski+dt, tjoseph, bhelgaas, lpieralisi,
	nm, vigneshr, devicetree, linux-kernel, linux-pci, bahle

Hi Dominic

Am Do., 27. Apr. 2023 um 07:50 Uhr schrieb Dominic Rath <rath@ibv-augsburg.de>:
>
> Hello Everyone,
>
> this series adds PHY latency properties to the Cadence PCIe
> driver to improve PTM accuracy, and configures the necessary
> values for TI's AM64x processors.
>
> These latencies are implementation specific and need to be
> configured in the PCIe IP core's registers to allow the
> PCIe controller to exactly determine the RX/TX timestamps for
> PCIe PTM messages.
>
> TI doesn't document these values in the datasheet or reference
> manual as of now, but provided the necessary data via TI's E2E
> forums (see PATCH 3/3).
>
> Changes from v1 to v2:
>    - move latency property to PHY instead of PCIe controller
>    - drop vendor prefix from property name
>    - rephrase commit message regarding optional properties
>    - emit an info message instead of a warning in case
>      an optional property is missing
>
> Best Regards,
>

Hope you send out a v3 soon.

-- 
greets
--
Christian Gmeiner, MSc

https://christian-gmeiner.info/privacypolicy

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2023-05-30  8:21 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-27  5:50 [PATCH v2 0/3] Cadence PCIe PHY latency for PTM Dominic Rath
2023-04-27  5:50 ` [PATCH v2 1/3] dt-bindings: phy: cadence-torrent: Add latency properties Dominic Rath
2023-04-27 18:30   ` Bjorn Helgaas
2023-05-09 15:31     ` Christian Gmeiner
2023-05-09 21:57       ` Bjorn Helgaas
2023-05-10  7:08         ` Krzysztof Kozlowski
2023-05-10 21:12           ` Bjorn Helgaas
2023-05-30  8:19   ` Christian Gmeiner
2023-04-27  5:50 ` [PATCH v2 2/3] PCI: cadence: Use DT bindings to set PHY latencies Dominic Rath
2023-05-04 11:33   ` Verma, Achal
2023-05-30  8:20     ` Christian Gmeiner
2023-04-27  5:50 ` [PATCH v2 3/3] arm64: dts: ti: k3-am64: Add PCIe PHY latency DT binding Dominic Rath
2023-05-09 15:27   ` Christian Gmeiner
2023-05-30  8:21 ` [PATCH v2 0/3] Cadence PCIe PHY latency for PTM Christian Gmeiner

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.