[PATCH v6 0/3] PCI: Precision Time Measurement support

[PATCH v6 0/3] PCI: Precision Time Measurement support

[Bjorn Helgaas]

This is a slightly different proposal for the PTM support Jonathan
proposed here:

  http://lkml.kernel.org/r/1462956446-27361-2-git-send-email-jonathan.yong@intel.com

I split this into three pieces mostly for ease in reviewing.  They
could all be squashed:

  - Enable PTM in root ports and switches automatically at boot
  - Enable PTM in endpoints when requested by driver
  - Add clock granularity information

I have some open questions about how PTM works on Root Complex
Integrated Endpoints and whether we should enable it automatically
even without a driver request.  And I probably left out some details
of the clock granularity computation, so treat this as more of an RFC
than anything.

Bjorn

---

Bjorn Helgaas (2):
      PCI: Add pci_enable_ptm() for drivers to enable PTM on endpoints
      PCI: Add PTM clock granularity information

Jonathan Yong (1):
      PCI: Add Precision Time Measurement (PTM) support


 drivers/pci/pci.h             |    6 ++
 drivers/pci/pcie/Kconfig      |   12 ++++
 drivers/pci/pcie/Makefile     |    1 
 drivers/pci/pcie/ptm.c        |  140 +++++++++++++++++++++++++++++++++++++++++
 drivers/pci/probe.c           |    3 +
 include/linux/pci.h           |   13 ++++
 include/uapi/linux/pci_regs.h |   12 +++-
 7 files changed, 186 insertions(+), 1 deletion(-)
 create mode 100644 drivers/pci/pcie/ptm.c

[PATCH v6 1/3] PCI: Add Precision Time Measurement (PTM) support

[Bjorn Helgaas]

From: Jonathan Yong <jonathan.yong@intel.com>

Add Precision Time Measurement (PTM) support (see PCIe r3.1, sec 6.22).

Enable PTM on PTM Root devices and switch ports.  This does not enable PTM
on endpoints.

There currently are no PTM-capable devices on the market, but it is
expected to be supported by the Intel Apollo Lake platform.

[bhelgaas: complete rework]
Signed-off-by: Jonathan Yong <jonathan.yong@intel.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/pci.h             |    6 ++++
 drivers/pci/pcie/Kconfig      |   12 +++++++
 drivers/pci/pcie/Makefile     |    1 +
 drivers/pci/pcie/ptm.c        |   70 +++++++++++++++++++++++++++++++++++++++++
 drivers/pci/probe.c           |    3 ++
 include/linux/pci.h           |    5 +++
 include/uapi/linux/pci_regs.h |   10 +++++-
 7 files changed, 106 insertions(+), 1 deletion(-)
 create mode 100644 drivers/pci/pcie/ptm.c

diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index a814bbb..faed7b7 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -321,6 +321,12 @@ static inline resource_size_t pci_resource_alignment(struct pci_dev *dev,
 
 void pci_enable_acs(struct pci_dev *dev);
 
+#ifdef CONFIG_PCIE_PTM
+void pci_ptm_init(struct pci_dev *dev);
+#else
+static inline void pci_ptm_init(struct pci_dev *dev) { }
+#endif
+
 struct pci_dev_reset_methods {
 	u16 vendor;
 	u16 device;
diff --git a/drivers/pci/pcie/Kconfig b/drivers/pci/pcie/Kconfig
index 22ca641..52c0c0f5 100644
--- a/drivers/pci/pcie/Kconfig
+++ b/drivers/pci/pcie/Kconfig
@@ -95,3 +95,15 @@ config PCIE_DPC
 
 	  To compile this driver as a module, choose M here: the module
 	  will be called pcie-dpc.
+
+config PCIE_PTM
+	bool "PCIe Precision Time Measurement support"
+	default y
+	depends on PCIEPORTBUS
+	help
+	  This enables PCI Express Precision Time Measurement (PTM)
+	  support.  If you have devices that support PTM, it will be
+	  enabled automatically.
+
+	  This is only useful if you have devices that support PTM, but it
+	  is safe to enable even if you don't.
diff --git a/drivers/pci/pcie/Makefile b/drivers/pci/pcie/Makefile
index b24525b..36e35ea 100644
--- a/drivers/pci/pcie/Makefile
+++ b/drivers/pci/pcie/Makefile
@@ -16,3 +16,4 @@ obj-$(CONFIG_PCIEAER)		+= aer/
 obj-$(CONFIG_PCIE_PME) += pme.o
 
 obj-$(CONFIG_PCIE_DPC) += pcie-dpc.o
+obj-$(CONFIG_PCIE_PTM) += ptm.o
diff --git a/drivers/pci/pcie/ptm.c b/drivers/pci/pcie/ptm.c
new file mode 100644
index 0000000..5c8479b
--- /dev/null
+++ b/drivers/pci/pcie/ptm.c
@@ -0,0 +1,70 @@
+/*
+ * PCI Express Precision Time Measurement
+ * Copyright (c) 2016, Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/pci.h>
+#include "../pci.h"
+
+static void pci_ptm_info(struct pci_dev *dev)
+{
+	dev_info(&dev->dev, "PTM enabled%s\n", dev->ptm_root ? " (root)" : "");
+}
+
+void pci_ptm_init(struct pci_dev *dev)
+{
+	int pos;
+	struct pci_dev *ups;
+	u32 cap, ctrl;
+
+	if (!pci_is_pcie(dev))
+		return;
+
+	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_PTM);
+	if (!pos)
+		return;
+
+	/*
+	 * Enable PTM only on upstream devices (root ports, switch ports,
+	 * etc.) on the assumption that it causes no link traffic until an
+	 * endpoint enables it.
+	 */
+	if ((pci_pcie_type(dev) == PCI_EXP_TYPE_ENDPOINT ||
+	     pci_pcie_type(dev) == PCI_EXP_TYPE_RC_END))
+		return;
+
+	pci_read_config_dword(dev, pos + PCI_PTM_CAP, &cap);
+
+	/*
+	 * There's no point in enabling PTM unless it's enabled in the
+	 * upstream device or this device can be a PTM Root itself.  Per
+	 * the spec recommendation (PCIe r3.1, sec 7.32.3), select the
+	 * furthest upstream Time Source as the PTM Root.
+	 */
+	ups = pci_upstream_bridge(dev);
+	if (ups && ups->ptm_enabled) {
+		ctrl = PCI_PTM_CTRL_ENABLE;
+	} else {
+		if (cap & PCI_PTM_CAP_ROOT) {
+			ctrl = PCI_PTM_CTRL_ENABLE | PCI_PTM_CTRL_ROOT;
+			dev->ptm_root = 1;
+		} else
+			return;
+	}
+
+	pci_write_config_dword(dev, pos + PCI_PTM_CTRL, ctrl);
+	dev->ptm_enabled = 1;
+
+	pci_ptm_info(dev);
+}
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 8e3ef72..4b5c099 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1658,6 +1658,9 @@ static void pci_init_capabilities(struct pci_dev *dev)
 	pci_enable_acs(dev);
 
 	pci_cleanup_aer_error_status_regs(dev);
+
+	/* Precision Time Measurement */
+	pci_ptm_init(dev);
 }
 
 /*
diff --git a/include/linux/pci.h b/include/linux/pci.h
index b67e4df..09e6c18 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -361,6 +361,11 @@ struct pci_dev {
 	int rom_attr_enabled;		/* has display of the rom attribute been enabled? */
 	struct bin_attribute *res_attr[DEVICE_COUNT_RESOURCE]; /* sysfs file for resources */
 	struct bin_attribute *res_attr_wc[DEVICE_COUNT_RESOURCE]; /* sysfs file for WC mapping of resources */
+
+#ifdef CONFIG_PCIE_PTM
+	unsigned int	ptm_root:1;
+	unsigned int	ptm_enabled:1;
+#endif
 #ifdef CONFIG_PCI_MSI
 	const struct attribute_group **msi_irq_groups;
 #endif
diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index 4040951..926fff4 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -671,7 +671,8 @@
 #define PCI_EXT_CAP_ID_PMUX	0x1A	/* Protocol Multiplexing */
 #define PCI_EXT_CAP_ID_PASID	0x1B	/* Process Address Space ID */
 #define PCI_EXT_CAP_ID_DPC	0x1D	/* Downstream Port Containment */
-#define PCI_EXT_CAP_ID_MAX	PCI_EXT_CAP_ID_DPC
+#define PCI_EXT_CAP_ID_PTM	0x1F	/* Precision Time Measurement */
+#define PCI_EXT_CAP_ID_MAX	PCI_EXT_CAP_ID_PTM
 
 #define PCI_EXT_CAP_DSN_SIZEOF	12
 #define PCI_EXT_CAP_MCAST_ENDPOINT_SIZEOF 40
@@ -964,4 +965,11 @@
 
 #define PCI_EXP_DPC_SOURCE_ID		10	/* DPC Source Identifier */
 
+/* Precision Time Measurement */
+#define PCI_PTM_CAP			0x04	    /* PTM Capability */
+#define  PCI_PTM_CAP_ROOT		0x00000004  /* Root capable */
+#define PCI_PTM_CTRL			0x08	    /* PTM Control */
+#define  PCI_PTM_CTRL_ENABLE		0x00000001  /* PTM enable */
+#define  PCI_PTM_CTRL_ROOT		0x00000002  /* Root select */
+
 #endif /* LINUX_PCI_REGS_H */

[PATCH v6 2/3] PCI: Add pci_enable_ptm() for drivers to enable PTM on endpoints

[Bjorn Helgaas]

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/pcie/ptm.c        |   45 +++++++++++++++++++++++++++++++++++++++++
 include/linux/pci.h           |    7 ++++++
 include/uapi/linux/pci_regs.h |    1 +
 3 files changed, 53 insertions(+)

diff --git a/drivers/pci/pcie/ptm.c b/drivers/pci/pcie/ptm.c
index 5c8479b..9cfa64a 100644
--- a/drivers/pci/pcie/ptm.c
+++ b/drivers/pci/pcie/ptm.c
@@ -68,3 +68,48 @@ void pci_ptm_init(struct pci_dev *dev)
 
 	pci_ptm_info(dev);
 }
+
+int pci_enable_ptm(struct pci_dev *dev, u8 *granularity)
+{
+	int pos;
+	struct pci_dev *ups;
+	u32 cap, ctrl;
+
+	if (!pci_is_pcie(dev))
+		return -EINVAL;
+
+	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_PTM);
+	if (!pos)
+		return -EINVAL;
+
+	/*
+	 * For a PCIe Endpoint, PTM is only useful if the endpoint can
+	 * issue PTM requests to upstream devices that have PTM enabled and
+	 * can respond.
+	 *
+	 * For Root Complex Integrated Endpoints, there is no upstream
+	 * device, so there must be some implementation-specific way to
+	 * associate the endpoint with a time source.
+	 */
+	if (pci_pcie_type(dev) == PCI_EXP_TYPE_ENDPOINT) {
+		/* PTM is only useful if an upstream device has it enabled */
+		ups = pci_upstream_bridge(dev);
+		if (!ups || !ups->ptm_enabled)
+			return -EINVAL;
+	} else if (pci_pcie_type(dev) != PCI_EXP_TYPE_RC_END)
+		return -EINVAL;
+
+	pci_read_config_dword(dev, pos + PCI_PTM_CAP, &cap);
+	if (!(cap & PCI_PTM_CAP_REQ))
+		return -EINVAL;
+
+	ctrl = PCI_PTM_CTRL_ENABLE;
+	pci_write_config_dword(dev, pos + PCI_PTM_CTRL, ctrl);
+	dev->ptm_enabled = 1;
+
+	pci_ptm_info(dev);
+
+	if (granularity)
+		*granularity = 0;
+	return 0;
+}
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 09e6c18..593b2c1 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1365,6 +1365,13 @@ static inline void pci_disable_ats(struct pci_dev *d) { }
 static inline int pci_ats_queue_depth(struct pci_dev *d) { return -ENODEV; }
 #endif
 
+#ifdef CONFIG_PCIE_PTM
+int pci_enable_ptm(struct pci_dev *dev, u8 *granularity);
+#else
+static inline int pci_enable_ptm(struct pci_dev *dev, u8 *granularity)
+{ return -EINVAL; }
+#endif
+
 void pci_cfg_access_lock(struct pci_dev *dev);
 bool pci_cfg_access_trylock(struct pci_dev *dev);
 void pci_cfg_access_unlock(struct pci_dev *dev);
diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index 926fff4..72bbe14 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -967,6 +967,7 @@
 
 /* Precision Time Measurement */
 #define PCI_PTM_CAP			0x04	    /* PTM Capability */
+#define  PCI_PTM_CAP_REQ		0x00000001  /* Requester capable */
 #define  PCI_PTM_CAP_ROOT		0x00000004  /* Root capable */
 #define PCI_PTM_CTRL			0x08	    /* PTM Control */
 #define  PCI_PTM_CTRL_ENABLE		0x00000001  /* PTM enable */

[PATCH v6 3/3] PCI: Add PTM clock granularity information

[Bjorn Helgaas]

I don't know how to figure out clock granularity for Root Complex
Integrated Endpoints.  The spec (PCIe r3.1, sec 7.32.3) says:

  system software must set [Effective Granularity] to the value reported in
  the Local Clock Granularity field by the associated PTM Time Source

but I don't know how to identify the associated PTM Time Source.  An
integrated endpoint has no upstream bridge.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/pcie/ptm.c        |   29 +++++++++++++++++++++++++++--
 include/linux/pci.h           |    1 +
 include/uapi/linux/pci_regs.h |    1 +
 3 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pcie/ptm.c b/drivers/pci/pcie/ptm.c
index 9cfa64a..a6ec4ae 100644
--- a/drivers/pci/pcie/ptm.c
+++ b/drivers/pci/pcie/ptm.c
@@ -19,7 +19,22 @@
 
 static void pci_ptm_info(struct pci_dev *dev)
 {
-	dev_info(&dev->dev, "PTM enabled%s\n", dev->ptm_root ? " (root)" : "");
+	char clock_desc[8];
+
+	switch (dev->ptm_granularity) {
+	case 0:
+		snprintf(clock_desc, sizeof(clock_desc), "unknown");
+		break;
+	case 255:
+		snprintf(clock_desc, sizeof(clock_desc), ">254ns");
+		break;
+	default:
+		snprintf(clock_desc, sizeof(clock_desc), "%udns",
+			 dev->ptm_granularity);
+		break;
+	}
+	dev_info(&dev->dev, "PTM enabled%s, %s granularity\n",
+		 dev->ptm_root ? " (root)" : "", clock_desc);
 }
 
 void pci_ptm_init(struct pci_dev *dev)
@@ -27,6 +42,7 @@ void pci_ptm_init(struct pci_dev *dev)
 	int pos;
 	struct pci_dev *ups;
 	u32 cap, ctrl;
+	u8 local_clock;
 
 	if (!pci_is_pcie(dev))
 		return;
@@ -45,6 +61,7 @@ void pci_ptm_init(struct pci_dev *dev)
 		return;
 
 	pci_read_config_dword(dev, pos + PCI_PTM_CAP, &cap);
+	local_clock = (cap & PCI_PTM_GRANULARITY_MASK) >> 8;
 
 	/*
 	 * There's no point in enabling PTM unless it's enabled in the
@@ -55,14 +72,20 @@ void pci_ptm_init(struct pci_dev *dev)
 	ups = pci_upstream_bridge(dev);
 	if (ups && ups->ptm_enabled) {
 		ctrl = PCI_PTM_CTRL_ENABLE;
+		if (ups->ptm_granularity == 0)
+			dev->ptm_granularity = 0;
+		else if (ups->ptm_granularity > local_clock)
+			dev->ptm_granularity = ups->ptm_granularity;
 	} else {
 		if (cap & PCI_PTM_CAP_ROOT) {
 			ctrl = PCI_PTM_CTRL_ENABLE | PCI_PTM_CTRL_ROOT;
 			dev->ptm_root = 1;
+			dev->ptm_granularity = local_clock;
 		} else
 			return;
 	}
 
+	ctrl |= dev->ptm_granularity << 8;
 	pci_write_config_dword(dev, pos + PCI_PTM_CTRL, ctrl);
 	dev->ptm_enabled = 1;
 
@@ -103,6 +126,8 @@ int pci_enable_ptm(struct pci_dev *dev, u8 *granularity)
 	if (!(cap & PCI_PTM_CAP_REQ))
 		return -EINVAL;
 
+	dev->ptm_granularity = ups->ptm_granularity;
+
 	ctrl = PCI_PTM_CTRL_ENABLE;
 	pci_write_config_dword(dev, pos + PCI_PTM_CTRL, ctrl);
 	dev->ptm_enabled = 1;
@@ -110,6 +135,6 @@ int pci_enable_ptm(struct pci_dev *dev, u8 *granularity)
 	pci_ptm_info(dev);
 
 	if (granularity)
-		*granularity = 0;
+		*granularity = dev->ptm_granularity;
 	return 0;
 }
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 593b2c1..73b70d3 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -365,6 +365,7 @@ struct pci_dev {
 #ifdef CONFIG_PCIE_PTM
 	unsigned int	ptm_root:1;
 	unsigned int	ptm_enabled:1;
+	u8		ptm_granularity;
 #endif
 #ifdef CONFIG_PCI_MSI
 	const struct attribute_group **msi_irq_groups;
diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index 72bbe14..d812172 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -969,6 +969,7 @@
 #define PCI_PTM_CAP			0x04	    /* PTM Capability */
 #define  PCI_PTM_CAP_REQ		0x00000001  /* Requester capable */
 #define  PCI_PTM_CAP_ROOT		0x00000004  /* Root capable */
+#define  PCI_PTM_GRANULARITY_MASK	0x0000FF00  /* Clock granularity */
 #define PCI_PTM_CTRL			0x08	    /* PTM Control */
 #define  PCI_PTM_CTRL_ENABLE		0x00000001  /* PTM enable */
 #define  PCI_PTM_CTRL_ROOT		0x00000002  /* Root select */

Re: [PATCH v6 3/3] PCI: Add PTM clock granularity information

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2152 bytes --]

Hi,

[auto build test WARNING on pci/next]
[also build test WARNING on v4.7-rc3 next-20160609]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Bjorn-Helgaas/PCI-Precision-Time-Measurement-support/20160614-031902
base:   https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: i386-defconfig (attached as .config)
compiler: gcc-6 (Debian 6.1.1-1) 6.1.1 20160430
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   drivers/pci/pcie/ptm.c: In function 'pci_enable_ptm':
>> drivers/pci/pcie/ptm.c:129:28: warning: 'ups' may be used uninitialized in this function [-Wmaybe-uninitialized]
     dev->ptm_granularity = ups->ptm_granularity;
                            ~~~^~~~~~~~~~~~~~~~~

vim +/ups +129 drivers/pci/pcie/ptm.c

   113		 * For Root Complex Integrated Endpoints, there is no upstream
   114		 * device, so there must be some implementation-specific way to
   115		 * associate the endpoint with a time source.
   116		 */
   117		if (pci_pcie_type(dev) == PCI_EXP_TYPE_ENDPOINT) {
   118			/* PTM is only useful if an upstream device has it enabled */
   119			ups = pci_upstream_bridge(dev);
   120			if (!ups || !ups->ptm_enabled)
   121				return -EINVAL;
   122		} else if (pci_pcie_type(dev) != PCI_EXP_TYPE_RC_END)
   123			return -EINVAL;
   124	
   125		pci_read_config_dword(dev, pos + PCI_PTM_CAP, &cap);
   126		if (!(cap & PCI_PTM_CAP_REQ))
   127			return -EINVAL;
   128	
 > 129		dev->ptm_granularity = ups->ptm_granularity;
   130	
   131		ctrl = PCI_PTM_CTRL_ENABLE;
   132		pci_write_config_dword(dev, pos + PCI_PTM_CTRL, ctrl);
   133		dev->ptm_enabled = 1;
   134	
   135		pci_ptm_info(dev);
   136	
   137		if (granularity)

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 24885 bytes --]

Re: [PATCH v6 0/3] PCI: Precision Time Measurement support

[Bjorn Helgaas]

On Mon, Jun 13, 2016 at 02:05:26PM -0500, Bjorn Helgaas wrote:
> This is a slightly different proposal for the PTM support Jonathan
> proposed here:
> 
>   http://lkml.kernel.org/r/1462956446-27361-2-git-send-email-jonathan.yong@intel.com
> 
> I split this into three pieces mostly for ease in reviewing.  They
> could all be squashed:
> 
>   - Enable PTM in root ports and switches automatically at boot
>   - Enable PTM in endpoints when requested by driver
>   - Add clock granularity information
> 
> I have some open questions about how PTM works on Root Complex
> Integrated Endpoints and whether we should enable it automatically
> even without a driver request.  And I probably left out some details
> of the clock granularity computation, so treat this as more of an RFC
> than anything.
> 
> Bjorn
> 
> ---
> 
> Bjorn Helgaas (2):
>       PCI: Add pci_enable_ptm() for drivers to enable PTM on endpoints
>       PCI: Add PTM clock granularity information
> 
> Jonathan Yong (1):
>       PCI: Add Precision Time Measurement (PTM) support

Jonathan, any comments?

Re: [PATCH v6 0/3] PCI: Precision Time Measurement support

[Yong, Jonathan]

On 07/20/2016 05:19, Bjorn Helgaas wrote:
> On Mon, Jun 13, 2016 at 02:05:26PM -0500, Bjorn Helgaas wrote:
>> This is a slightly different proposal for the PTM support Jonathan
>> proposed here:
>>
>>   http://lkml.kernel.org/r/1462956446-27361-2-git-send-email-jonathan.yong@intel.com
>>
>> I split this into three pieces mostly for ease in reviewing.  They
>> could all be squashed:
>>
>>   - Enable PTM in root ports and switches automatically at boot
>>   - Enable PTM in endpoints when requested by driver
>>   - Add clock granularity information
>>
>> I have some open questions about how PTM works on Root Complex
>> Integrated Endpoints and whether we should enable it automatically
>> even without a driver request.  And I probably left out some details
>> of the clock granularity computation, so treat this as more of an RFC
>> than anything.
>>
> 
> Jonathan, any comments?
> 

I don't have any new information on how to configure integrated endpoints.

This line:
ctrl = PCI_PTM_CTRL_ENABLE | PCI_PTM_CTRL_ROOT;

should also set the responder capable bit (7.32.2):
	If PTM Root Capable is Set, this bit must be Set to 1b.

Re: [PATCH v6 1/3] PCI: Add Precision Time Measurement (PTM) support

[Yong, Jonathan]

On 06/14/2016 03:05, Bjorn Helgaas wrote:
> From: Jonathan Yong <jonathan.yong@intel.com>
> 
> Add Precision Time Measurement (PTM) support (see PCIe r3.1, sec 6.22).
> 
> Enable PTM on PTM Root devices and switch ports.  This does not enable PTM
> on endpoints.
> 
> There currently are no PTM-capable devices on the market, but it is
> expected to be supported by the Intel Apollo Lake platform.
> 
> [bhelgaas: complete rework]
> Signed-off-by: Jonathan Yong <jonathan.yong@intel.com>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

Hi,

Any updates on the PTM changes?

Re: [PATCH v6 0/3] PCI: Precision Time Measurement support

[Bjorn Helgaas]

On Wed, Jul 20, 2016 at 07:49:21AM +0800, Yong, Jonathan wrote:
> On 07/20/2016 05:19, Bjorn Helgaas wrote:
> > On Mon, Jun 13, 2016 at 02:05:26PM -0500, Bjorn Helgaas wrote:
> >> This is a slightly different proposal for the PTM support Jonathan
> >> proposed here:
> >>
> >>   http://lkml.kernel.org/r/1462956446-27361-2-git-send-email-jonathan.yong@intel.com
> >>
> >> I split this into three pieces mostly for ease in reviewing.  They
> >> could all be squashed:
> >>
> >>   - Enable PTM in root ports and switches automatically at boot
> >>   - Enable PTM in endpoints when requested by driver
> >>   - Add clock granularity information
> >>
> >> I have some open questions about how PTM works on Root Complex
> >> Integrated Endpoints and whether we should enable it automatically
> >> even without a driver request.  And I probably left out some details
> >> of the clock granularity computation, so treat this as more of an RFC
> >> than anything.
> >>
> > 
> > Jonathan, any comments?
> > 
> 
> I don't have any new information on how to configure integrated endpoints.
> 
> This line:
> ctrl = PCI_PTM_CTRL_ENABLE | PCI_PTM_CTRL_ROOT;
> 
> should also set the responder capable bit (7.32.2):
> 	If PTM Root Capable is Set, this bit must be Set to 1b.

The PTM Responder Capable bit (bit 1 in Table 7-145) is a HwInit bit
in the PTM Capability register, so it's read-only from the kernel's
perspective.

The line you mention ("ctrl = PCI_PTM_CTRL_ENABLE | PCI_PTM_CTRL_ROOT")
is turning on bits in the PTM Control register, not the Capability
register.

Bjorn

Re: [PATCH v6 1/3] PCI: Add Precision Time Measurement (PTM) support

[Bjorn Helgaas]

Hi Jonathan,

On Wed, Aug 10, 2016 at 10:58:22AM +0800, Yong, Jonathan wrote:
> On 06/14/2016 03:05, Bjorn Helgaas wrote:
> > From: Jonathan Yong <jonathan.yong@intel.com>
> > 
> > Add Precision Time Measurement (PTM) support (see PCIe r3.1, sec 6.22).
> > 
> > Enable PTM on PTM Root devices and switch ports.  This does not enable PTM
> > on endpoints.
> > 
> > There currently are no PTM-capable devices on the market, but it is
> > expected to be supported by the Intel Apollo Lake platform.
> > 
> > [bhelgaas: complete rework]
> > Signed-off-by: Jonathan Yong <jonathan.yong@intel.com>
> > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> 
> Hi,
> 
> Any updates on the PTM changes?

I guess I was hoping you could test these patches.  Do you have any
way to do that?

I rebased them and pushed them to pci/ptm.  If everything looks good,
I'll merge them for v4.9.

Bjorn

Re: [PATCH v6 0/3] PCI: Precision Time Measurement support

[Yong, Jonathan]

On 08/16/2016 02:51, Bjorn Helgaas wrote:
>>
>> This line:
>> ctrl = PCI_PTM_CTRL_ENABLE | PCI_PTM_CTRL_ROOT;
>>
>> should also set the responder capable bit (7.32.2):
>> 	If PTM Root Capable is Set, this bit must be Set to 1b.
> 
> The PTM Responder Capable bit (bit 1 in Table 7-145) is a HwInit bit
> in the PTM Capability register, so it's read-only from the kernel's
> perspective.
> 
> The line you mention ("ctrl = PCI_PTM_CTRL_ENABLE | PCI_PTM_CTRL_ROOT")
> is turning on bits in the PTM Control register, not the Capability
> register.
> 

My bad, there is no "responder enable" bit control, patch looks good.

Re: [PATCH v6 1/3] PCI: Add Precision Time Measurement (PTM) support

[Yong, Jonathan]

On 08/16/2016 02:59, Bjorn Helgaas wrote:
> 
> I guess I was hoping you could test these patches.  Do you have any
> way to do that?
> 

No real hardware with this feature yet, so testing is entirely on software.

> I rebased them and pushed them to pci/ptm.  If everything looks good,
> I'll merge them for v4.9.
> 
> Bjorn
> 

Looks like there is still this compile warning to fix:
http://patchwork.ozlabs.org/patch/634825/

Re: [PATCH v6 1/3] PCI: Add Precision Time Measurement (PTM) support

[Bjorn Helgaas]

On Tue, Aug 16, 2016 at 04:04:31PM +0800, Yong, Jonathan wrote:
> On 08/16/2016 02:59, Bjorn Helgaas wrote:
> > 
> > I guess I was hoping you could test these patches.  Do you have any
> > way to do that?
> > 
> 
> No real hardware with this feature yet, so testing is entirely on software.

OK, let me know the results of your software testing with these patches.

> Looks like there is still this compile warning to fix:
> http://patchwork.ozlabs.org/patch/634825/

That report was from June 16, and I think I fixed the warning already.
Or do you still see the warning?  I don't see it myself.

Bjorn

Re: [PATCH v6 1/3] PCI: Add Precision Time Measurement (PTM) support

[Yong, Jonathan]

On 08/16/2016 21:37, Bjorn Helgaas wrote:
>> Looks like there is still this compile warning to fix:
>> http://patchwork.ozlabs.org/patch/634825/
> 
> That report was from June 16, and I think I fixed the warning already.
> Or do you still see the warning?  I don't see it myself.
> 
> Bjorn
> 

If that is fixed, I don't anything else to add.

Re: [PATCH v6 1/3] PCI: Add Precision Time Measurement (PTM) support

[Bjorn Helgaas]

On Tue, Aug 16, 2016 at 08:37:15AM -0500, Bjorn Helgaas wrote:
> On Tue, Aug 16, 2016 at 04:04:31PM +0800, Yong, Jonathan wrote:
> > On 08/16/2016 02:59, Bjorn Helgaas wrote:
> > > 
> > > I guess I was hoping you could test these patches.  Do you have any
> > > way to do that?
> > > 
> > 
> > No real hardware with this feature yet, so testing is entirely on software.
> 
> OK, let me know the results of your software testing with these patches.

Do you have some software testing you can do on these patches?

Re: [PATCH v6 1/3] PCI: Add Precision Time Measurement (PTM) support

[Yong, Jonathan]

On 08/23/2016 01:01, Bjorn Helgaas wrote:
> On Tue, Aug 16, 2016 at 08:37:15AM -0500, Bjorn Helgaas wrote:
>> On Tue, Aug 16, 2016 at 04:04:31PM +0800, Yong, Jonathan wrote:
>>> On 08/16/2016 02:59, Bjorn Helgaas wrote:
>>>>
>>>> I guess I was hoping you could test these patches.  Do you have any
>>>> way to do that?
>>>>
>>>
>>> No real hardware with this feature yet, so testing is entirely on software.
>>
>> OK, let me know the results of your software testing with these patches.
> 
> Do you have some software testing you can do on these patches?
> 

Yes, they're entirely synthetic however and do not reflect real hardware.

At the moment, they're hooks to pci_scan_bus and providing a fake config
space for the driver to manipulate, inspected with lspci.

The PTM bits are set properly as far as I can tell.

Re: [PATCH v6 0/3] PCI: Precision Time Measurement support

[Bjorn Helgaas]

On Mon, Jun 13, 2016 at 02:05:26PM -0500, Bjorn Helgaas wrote:
> This is a slightly different proposal for the PTM support Jonathan
> proposed here:
> 
>   http://lkml.kernel.org/r/1462956446-27361-2-git-send-email-jonathan.yong@intel.com
> 
> I split this into three pieces mostly for ease in reviewing.  They
> could all be squashed:
> 
>   - Enable PTM in root ports and switches automatically at boot
>   - Enable PTM in endpoints when requested by driver
>   - Add clock granularity information
> 
> I have some open questions about how PTM works on Root Complex
> Integrated Endpoints and whether we should enable it automatically
> even without a driver request.  And I probably left out some details
> of the clock granularity computation, so treat this as more of an RFC
> than anything.
> 
> Bjorn
> 
> ---
> 
> Bjorn Helgaas (2):
>       PCI: Add pci_enable_ptm() for drivers to enable PTM on endpoints
>       PCI: Add PTM clock granularity information
> 
> Jonathan Yong (1):
>       PCI: Add Precision Time Measurement (PTM) support
> 
> 
>  drivers/pci/pci.h             |    6 ++
>  drivers/pci/pcie/Kconfig      |   12 ++++
>  drivers/pci/pcie/Makefile     |    1 
>  drivers/pci/pcie/ptm.c        |  140 +++++++++++++++++++++++++++++++++++++++++
>  drivers/pci/probe.c           |    3 +
>  include/linux/pci.h           |   13 ++++
>  include/uapi/linux/pci_regs.h |   12 +++-
>  7 files changed, 186 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/pci/pcie/ptm.c

These are on pci/ptm and queued for v4.9.

Re: [PATCH v6 0/3] PCI: Precision Time Measurement support

[Yong, Jonathan]

On 08/24/2016 05:42, Bjorn Helgaas wrote:
> 
> These are on pci/ptm and queued for v4.9.
> 

My thanks for making this possible!