[PATCH v2 2/3] PCI: cadence: Use DT bindings to set PHY latencies

[PATCH v2 2/3] PCI: cadence: Use DT bindings to set PHY latencies

[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

Re: [PATCH v2 2/3] PCI: cadence: Use DT bindings to set PHY latencies

[Verma, Achal]

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>

Re: [PATCH v2 2/3] PCI: cadence: Use DT bindings to set PHY latencies

[Christian Gmeiner]

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

[PATCH v2 3/3] arm64: dts: ti: k3-am64: Add PCIe PHY latency DT binding

[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

Re: [PATCH v2 3/3] arm64: dts: ti: k3-am64: Add PCIe PHY latency DT binding

[Christian Gmeiner]

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

Re: [PATCH v2 1/3] dt-bindings: phy: cadence-torrent: Add latency properties

[Bjorn Helgaas]

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

Re: [PATCH v2 1/3] dt-bindings: phy: cadence-torrent: Add latency properties

[Christian Gmeiner]

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

Re: [PATCH v2 1/3] dt-bindings: phy: cadence-torrent: Add latency properties

[Bjorn Helgaas]

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

Re: [PATCH v2 1/3] dt-bindings: phy: cadence-torrent: Add latency properties

[Krzysztof Kozlowski]

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

Re: [PATCH v2 1/3] dt-bindings: phy: cadence-torrent: Add latency properties

[Bjorn Helgaas]

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

Re: [PATCH v2 1/3] dt-bindings: phy: cadence-torrent: Add latency properties

[Christian Gmeiner]

>
> 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

Re: [PATCH v2 0/3] Cadence PCIe PHY latency for PTM

[Christian Gmeiner]

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