[RFC] PCI: PTM Driver

[RFC] PCI: PTM Driver

[Yong, Jonathan]

Hello LKML,

This is a preliminary implementation of the PTM[1] support driver, the code
is obviously hacked together and in need of refactoring. This driver has
only been tested against a virtual PCI bus.

The drivers job is to get to every PTM capable device, set some PCI config
space bits, then go back to sleep [2].

PTM capable PCIe devices will get a new sysfs entry to allow PTM to be
enabled if automatic PTM activation is disabled, or disabled if so desired.

Comments? Should I explain the PTM registers in more details?
Please CC me, thanks.

[1] Precision Time Measurement: A protocol for synchronizing PCIe endpoint
clocks against the host clock as specified in the PCI Express Base
Specification 3.1. It is identified by the 0x001f extended capability ID.

PTM capable devices are split into 3 roles, master, responder and requester.
Summary as follows:

A master holds the master clock that will be used for all devices under its
domain (not to be confused with PCI domains). There may be multiple masters
in a PTM hierarchy, in which case, the highest master closest to the root
complex will be selected for the PTM domain. A master is also always
responder capable. Clock precision is signified by a Local Clock
Granularity field, in nano-seconds.

A responder responds to any PTM synchronization requests from a downstream
device. A responder is typically a switch device. It may also hold a local
clock signified by a non-zero Local Clock Granularity field. A value of 0
signifies that the device simply propagates timing information from
upstream devices.

A requester is typically an endpoint that will request synchronization
updates from an upstream PTM capable time source. The driver will update
the Effective Clock Granularity field based on the same field from the
PTM domain master. The field should be programed with a value of 0 if any
intervening responder has a Local Clock Granularity field value of 0.

[2] The software drivers never see the PTM packets, the PCI Express Base
Specificaton 3.1 reads:
	PTM capable components can make their PTM context available for
	inspection by software, enabling software to translate timing
	information between local times and PTM Master Time.

This isn't very informative.

Yong, Jonathan (1):
  PCI: PTM preliminary implementation

 drivers/pci/pci-sysfs.c     |   7 +
 drivers/pci/pci.h           |  21 +++
 drivers/pci/pcie/Kconfig    |   8 +
 drivers/pci/pcie/Makefile   |   2 +-
 drivers/pci/pcie/pcie_ptm.c | 353 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/pci/probe.c         |   3 +
 6 files changed, 393 insertions(+), 1 deletion(-)
 create mode 100644 drivers/pci/pcie/pcie_ptm.c

-- 
2.4.10

[PATCH] PCI: PTM preliminary implementation

[Yong, Jonathan]

Simplified Precision Time Measurement driver, activates PTM feature
if a PCIe PTM requester (as per PCI Express 3.1 Base Specification
section 7.32)is found, but not before checking if the rest of the
PCI hierarchy can support it.

The driver does not take part in facilitating PTM conversations,
neither does it provide any useful services, it is only responsible
for setting up the required configuration space bits.

As of writing, there aren't any PTM capable devices on the market
yet, but it is supported by the Intel Apollo Lake platform.

Signed-off-by: Yong, Jonathan <jonathan.yong@intel.com>
---
 drivers/pci/pci-sysfs.c     |   7 +
 drivers/pci/pci.h           |  21 +++
 drivers/pci/pcie/Kconfig    |   8 +
 drivers/pci/pcie/Makefile   |   2 +-
 drivers/pci/pcie/pcie_ptm.c | 353 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/pci/probe.c         |   3 +
 6 files changed, 393 insertions(+), 1 deletion(-)
 create mode 100644 drivers/pci/pcie/pcie_ptm.c

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 95d9e7b..c634fd11 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -1335,6 +1335,9 @@ static int pci_create_capabilities_sysfs(struct pci_dev *dev)
 	/* Active State Power Management */
 	pcie_aspm_create_sysfs_dev_files(dev);
 
+	/* PTM */
+	pci_create_ptm_sysfs(dev);
+
 	if (!pci_probe_reset_function(dev)) {
 		retval = device_create_file(&dev->dev, &reset_attr);
 		if (retval)
@@ -1433,6 +1436,10 @@ static void pci_remove_capabilities_sysfs(struct pci_dev *dev)
 	}
 
 	pcie_aspm_remove_sysfs_dev_files(dev);
+
+	/* PTM */
+	pci_release_ptm_sysfs(dev);
+
 	if (dev->reset_fn) {
 		device_remove_file(&dev->dev, &reset_attr);
 		dev->reset_fn = 0;
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 9a1660f..fb90420 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -320,6 +320,27 @@ static inline resource_size_t pci_resource_alignment(struct pci_dev *dev,
 
 void pci_enable_acs(struct pci_dev *dev);
 
+#ifdef CONFIG_PCIEPORTBUS
+int pci_enable_ptm(struct pci_dev *dev);
+void pci_create_ptm_sysfs(struct pci_dev *dev);
+void pci_release_ptm_sysfs(struct pci_dev *dev);
+void pci_disable_ptm(struct pci_dev *dev);
+#else
+static inline int pci_enable_ptm(struct pci_dev *dev)
+{
+	return -ENXIO;
+}
+static inline void pci_create_ptm_sysfs(struct pci_dev *dev)
+{
+}
+static inline void pci_release_ptm_sysfs(struct pci_dev *dev)
+{
+}
+static inline void pci_disable_ptm(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 e294713..f65ff4d 100644
--- a/drivers/pci/pcie/Kconfig
+++ b/drivers/pci/pcie/Kconfig
@@ -80,3 +80,11 @@ endchoice
 config PCIE_PME
 	def_bool y
 	depends on PCIEPORTBUS && PM
+
+config PCIE_PTM
+	bool "Turn on Precision Time Management by default"
+	depends on PCIEPORTBUS
+	help
+	  Say Y here to enable PTM feature on PCI Express devices that
+	  support them as they are found during device enumeration. Otherwise
+	  the feature can be enabled manually through sysfs entries.
diff --git a/drivers/pci/pcie/Makefile b/drivers/pci/pcie/Makefile
index 00c62df..d18b4c7 100644
--- a/drivers/pci/pcie/Makefile
+++ b/drivers/pci/pcie/Makefile
@@ -5,7 +5,7 @@
 # Build PCI Express ASPM if needed
 obj-$(CONFIG_PCIEASPM)		+= aspm.o
 
-pcieportdrv-y			:= portdrv_core.o portdrv_pci.o portdrv_bus.o
+pcieportdrv-y			:= portdrv_core.o portdrv_pci.o portdrv_bus.o pcie_ptm.o
 pcieportdrv-$(CONFIG_ACPI)	+= portdrv_acpi.o
 
 obj-$(CONFIG_PCIEPORTBUS)	+= pcieportdrv.o
diff --git a/drivers/pci/pcie/pcie_ptm.c b/drivers/pci/pcie/pcie_ptm.c
new file mode 100644
index 0000000..a128c79
--- /dev/null
+++ b/drivers/pci/pcie/pcie_ptm.c
@@ -0,0 +1,353 @@
+/*
+ * 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"
+
+#define PCI_PTM_REQ		0x0001  /* Requester capable */
+#define  PCI_PTM_RSP		0x0002  /* Responder capable */
+#define  PCI_PTM_ROOT		0x0004  /* Root capable */
+#define  PCI_PTM_GRANULITY	0xFF00  /* Local clock granulity */
+#define PCI_PTM_ENABLE		0x0001  /* PTM enable */
+#define  PCI_PTM_ROOT_SEL	0x0002  /* Root select */
+
+#define PCI_PTM_HEADER_REG_OFFSET	0x00
+#define PCI_PTM_CAPABILITY_REG_OFFSET	0x04
+#define PCI_PTM_CONTROL_REG_OFFSET	0x08
+
+#define PCI_EXT_CAP_ID_PTM		0x001f
+
+#ifdef CONFIG_PCIE_PTM
+static bool disable_ptm;
+#else
+static bool disable_ptm = 1;
+#endif
+
+module_param_named(disable_ptm, disable_ptm, bool, S_IRUGO | S_IWUSR);
+MODULE_PARM_DESC(disable_ptm,
+	"Don't automatically enable even if supported.");
+
+static inline u8 get_granularity(u32 in)
+{
+	return (in & PCI_PTM_GRANULITY) >> 8;
+}
+
+static ssize_t ptm_status_show(struct device *dev,
+	struct device_attribute *attr, char *buf)
+{
+	struct pci_dev *pdev = to_pci_dev(dev);
+	u16 word;
+	int pos;
+
+	pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PTM);
+	if (!pos)
+		return -ENXIO;
+
+	pci_read_config_word(pdev, pos + PCI_PTM_CONTROL_REG_OFFSET, &word);
+	return sprintf(buf, "%u\n", word & PCI_PTM_ENABLE);
+}
+
+static ssize_t ptm_status_store(struct device *dev,
+	struct device_attribute *attr, const char *buf, size_t count)
+{
+	struct pci_dev *pdev = to_pci_dev(dev);
+	unsigned long val;
+	ssize_t ret;
+	int pos;
+
+	pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PTM);
+	if (!pos)
+		return -ENXIO;
+
+	ret = kstrtoul(buf, 0, &val);
+	if (ret)
+		return ret;
+	if (val)
+		return pci_enable_ptm(pdev);
+	pci_disable_ptm(pdev);
+	return 0;
+}
+
+static DEVICE_ATTR_RW(ptm_status);
+
+void pci_release_ptm_sysfs(struct pci_dev *dev)
+{
+	if (!pci_find_ext_capability(dev, PCI_EXT_CAP_ID_PTM))
+		return;
+	device_remove_file(&dev->dev, &dev_attr_ptm_status);
+}
+
+void pci_create_ptm_sysfs(struct pci_dev *dev)
+{
+	if (!pci_find_ext_capability(dev, PCI_EXT_CAP_ID_PTM))
+		return;
+	device_create_file(&dev->dev, &dev_attr_ptm_status);
+}
+
+/**
+ * pci_enable_ptm - Try to activate PTM functionality on device.
+ * @dev: PCI Express device with PTM requester role to enable.
+ *
+ * The function will crawl through the PCI Hierarchy to determine if it is
+ * possible to enable the Precision Time Measurement requester role on @dev,
+ * and if so, activate it by setting the granularity field.
+ *
+ * NOTE: Each requester must be associated with a PTM root (not to be confused
+ * with a root port or root complex). There can be multiple PTM roots in a
+ * a system forming multiple domains. All intervening bridges/switches in a
+ * domain must support PTM responder roles to relay PTM dialogues.
+ */
+int pci_enable_ptm(struct pci_dev *dev)
+{
+	struct pci_dev *curr, **steps;
+	size_t i = 0, root = 0;
+	int pos, pos2;
+	u8 granularity = 0;
+	u16 word;
+	int ret = 0;
+
+	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_PTM);
+	if (!pos) {
+		dev_dbg(&dev->dev, "Not PTM capable, skipping.\n");
+		return -ENXIO;
+	}
+
+	if (disable_ptm)
+		return 0;
+
+	/* Skip if not requester role. */
+	pci_read_config_word(dev, pos + PCI_PTM_CAPABILITY_REG_OFFSET, &word);
+	if (!(word & PCI_PTM_REQ)) {
+		dev_dbg(&dev->dev, "Not a PTM requester, skipping for now.\n");
+		return 0;
+	}
+
+	/* Just copy and enable PTM granularity for integrated endpoints. */
+	if (pci_pcie_type(dev) == PCI_EXP_TYPE_RC_END) {
+		dev_dbg(&dev->dev,
+			"Root integrated endpoint, attempting to copy root granularity.\n");
+		curr = pci_upstream_bridge(dev);
+		if (!curr)
+			return 0;
+
+		pos2 = pci_find_ext_capability(curr, PCI_EXT_CAP_ID_PTM);
+		if (!pos2)
+			return 0;
+
+		/* Get granularity field. */
+		pci_read_config_word(curr,
+			pos2 + PCI_PTM_CAPABILITY_REG_OFFSET,
+			&word);
+		word &= PCI_PTM_GRANULITY;
+
+		/* Copy it over. */
+		word |= PCI_PTM_ENABLE;
+		pci_write_config_word(dev, pos + PCI_PTM_CONTROL_REG_OFFSET,
+			word);
+
+		/* Enable PTM on root complex if not already so. */
+		pci_read_config_word(curr, pos2 + PCI_PTM_CONTROL_REG_OFFSET,
+			&word);
+		if (!(word & PCI_PTM_ENABLE)) {
+			pci_read_config_word(curr,
+				pos2 + PCI_PTM_CONTROL_REG_OFFSET, &word);
+			word |= PCI_PTM_ENABLE | PCI_PTM_ROOT_SEL;
+			pci_write_config_word(curr,
+				pos2 + PCI_PTM_CONTROL_REG_OFFSET, word);
+		}
+	} else {
+		/* For holding all the bridge/switches in between. */
+		steps = kzalloc(sizeof(*steps) * dev->bus->number + 1,
+			GFP_KERNEL);
+		if (!steps)
+			return -ENOMEM;
+
+		/* Gather all the upstream devices. */
+		curr = pci_upstream_bridge(dev);
+		if (curr) {
+			dev_dbg(&dev->dev,
+				"Upstream is %d:%d:%x.%d\n",
+				pci_domain_nr(curr->bus),
+				curr->bus->number,
+				PCI_SLOT(curr->devfn),
+				PCI_FUNC(curr->devfn)
+				);
+		} else {
+			dev_dbg(&dev->dev, "No upstream??\n");
+			ret = -ENXIO;
+			goto abort;
+		}
+
+		do {
+			/* sanity check */
+			if (i > dev->bus->number)
+				break;
+			steps[i++] = curr;
+		} while ((curr = pci_upstream_bridge(curr)));
+
+		/* Check upstream chains capability. */
+		dev_dbg(&dev->dev,
+				   "Checking hierarchy capabilities\n");
+		for (i = 0; i < dev->bus->number + 1; i++) {
+			curr = steps[i];
+			if (!curr)
+				break;
+
+			pos2 = pci_find_ext_capability(curr,
+				PCI_EXT_CAP_ID_PTM);
+
+			if (!pos2) {
+				dev_dbg(&curr->dev,
+					"PTM Hierarchy %zx: not PTM aware\n",
+					i);
+				break;
+			}
+
+			/* End if upstream cannot respond. */
+			pci_read_config_word(curr,
+				pos2 + PCI_PTM_CAPABILITY_REG_OFFSET, &word);
+			if (!(word & PCI_PTM_RSP)) {
+				dev_dbg(&curr->dev,
+				"PTM Hierarchy: skipping non-responder\n");
+				break;
+			}
+
+			/* Is root capable? */
+			if (word & PCI_PTM_ROOT) {
+				root = i;
+				granularity = get_granularity(word);
+			}
+		}
+
+		if (!steps[root]) {
+			dev_dbg(&dev->dev, "Cannot find root, aborting\n");
+			ret = -ENXIO;
+			goto abort;
+		}
+
+		dev_dbg(&dev->dev,
+			"Found PTM root at %d:%d:%x.%d granularity %u\n",
+			pci_domain_nr(steps[root]->bus),
+			steps[root]->bus->number,
+			PCI_SLOT(steps[root]->devfn),
+			PCI_FUNC(steps[root]->devfn),
+			granularity);
+
+		/* Program granularity field. */
+		for (i = root;;) {
+			curr = steps[i];
+			pos2 = pci_find_ext_capability(curr,
+				PCI_EXT_CAP_ID_PTM);
+			pci_read_config_word(curr,
+				pos2 + PCI_PTM_CONTROL_REG_OFFSET, &word);
+
+			/* If not yet PTM enabled. */
+			if (!(word & PCI_PTM_ENABLE)) {
+				pci_read_config_word(curr,
+					pos2 + PCI_PTM_CAPABILITY_REG_OFFSET,
+					&word);
+				/* If requester capable, program granularity. */
+				if (word & PCI_PTM_REQ) {
+					dev_dbg(&curr->dev,
+						"Programming granularity %u\n",
+						granularity);
+					pci_write_config_word(curr,
+						pos2 +
+						PCI_PTM_CONTROL_REG_OFFSET,
+						((u16)granularity) << 8);
+				}
+				if ((word & PCI_PTM_ROOT) &&
+					granularity != 0 &&
+					((granularity < get_granularity(word))
+					|| get_granularity(word) == 0)) {
+					dev_dbg(&curr->dev,
+					"Updating granularity %u to %u\n",
+						granularity,
+						get_granularity(word));
+					granularity = get_granularity(word);
+				}
+			}
+			if (!i)
+				break;
+			i--;
+		}
+
+		/* Program current device granularity and enable it. */
+		pci_read_config_word(dev, pos + PCI_PTM_CONTROL_REG_OFFSET,
+			&word);
+		word = (word & ~PCI_PTM_GRANULITY) | ((u16)granularity) << 8
+			| PCI_PTM_ENABLE;
+		pci_write_config_word(dev, pos + PCI_PTM_CONTROL_REG_OFFSET,
+			word);
+		dev_dbg(&dev->dev,
+				"Using granularity %u, %x\n", granularity,
+				word);
+
+		/* Enable PTM root. */
+		pos2 = pci_find_ext_capability(steps[root],
+			PCI_EXT_CAP_ID_PTM);
+		pci_read_config_word(steps[root],
+			pos2 + PCI_PTM_CONTROL_REG_OFFSET, &word);
+		word |= PCI_PTM_ROOT_SEL | PCI_PTM_ENABLE;
+		pci_write_config_word(steps[root],
+			pos2 + PCI_PTM_CONTROL_REG_OFFSET, word);
+
+		/* PTM enable from the bottom up. */
+		for (i = 0; i <= root; i++) {
+			pos2 = pci_find_ext_capability(steps[i],
+				PCI_EXT_CAP_ID_PTM);
+			pci_read_config_word(steps[i],
+				pos2 + PCI_PTM_CONTROL_REG_OFFSET, &word);
+			word |= PCI_PTM_ENABLE;
+			pci_write_config_word(steps[i],
+				pos2 + PCI_PTM_CONTROL_REG_OFFSET,
+				word);
+		}
+abort:
+		kfree(steps);
+	}
+	return ret;
+}
+
+static int do_disable_ptm(struct pci_dev *dev, void *v)
+{
+	int pos;
+	u16 word;
+
+	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_PTM);
+	if (!pos)
+		return 0;
+
+	pci_read_config_word(dev, pos + PCI_PTM_CONTROL_REG_OFFSET, &word);
+	word &= ~PCI_PTM_ENABLE;
+	pci_write_config_word(dev, pos + PCI_PTM_CONTROL_REG_OFFSET, word);
+	return 0;
+}
+
+/**
+ * pci_disable_ptm - Turn off PTM functionality on device.
+ * @dev: PCI Express device with PTM function to disable.
+ *
+ * Disables PTM functionality by clearing the PTM enable bit, if device is a
+ * switch/bridge it will also disable PTM function on other devices behind it.
+ */
+void pci_disable_ptm(struct pci_dev *dev)
+{
+	if (pci_is_bridge(dev))
+		pci_walk_bus(dev->bus, &do_disable_ptm, NULL);
+	else
+		do_disable_ptm(dev, NULL);
+}
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 6d7ab9b..8dc8bbc 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1623,6 +1623,9 @@ static void pci_init_capabilities(struct pci_dev *dev)
 	pci_enable_acs(dev);
 
 	pci_cleanup_aer_error_status_regs(dev);
+
+	/* Enable PTM Capabilities */
+	pci_enable_ptm(dev);
 }
 
 /*
-- 
2.4.10

Re: [PATCH] PCI: PTM preliminary implementation

[Bjorn Helgaas]

Hi Jonathan,

Sorry I didn't see this earlier.  I don't follow linux-kernel, so I
didn't see it until it appeared on linux-pci.  I don't have time for a
full review right now since the merge window is about to open, but
I'll give you some first impressions to get started.

On Fri, Mar 11, 2016 at 07:26:24AM +0000, Yong, Jonathan wrote:
> Simplified Precision Time Measurement driver, activates PTM feature
> if a PCIe PTM requester (as per PCI Express 3.1 Base Specification
> section 7.32)is found, but not before checking if the rest of the
> PCI hierarchy can support it.
> 
> The driver does not take part in facilitating PTM conversations,
> neither does it provide any useful services, it is only responsible
> for setting up the required configuration space bits.
> 
> As of writing, there aren't any PTM capable devices on the market
> yet, but it is supported by the Intel Apollo Lake platform.
> 
> Signed-off-by: Yong, Jonathan <jonathan.yong@intel.com>
> ---
>  drivers/pci/pci-sysfs.c     |   7 +
>  drivers/pci/pci.h           |  21 +++
>  drivers/pci/pcie/Kconfig    |   8 +
>  drivers/pci/pcie/Makefile   |   2 +-
>  drivers/pci/pcie/pcie_ptm.c | 353 ++++++++++++++++++++++++++++++++++++++++++++
>  drivers/pci/probe.c         |   3 +
>  6 files changed, 393 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/pci/pcie/pcie_ptm.c
> 
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index 95d9e7b..c634fd11 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -1335,6 +1335,9 @@ static int pci_create_capabilities_sysfs(struct pci_dev *dev)
>  	/* Active State Power Management */
>  	pcie_aspm_create_sysfs_dev_files(dev);
>  
> +	/* PTM */

Don't add comments like this that merely repeat the code.

> +	pci_create_ptm_sysfs(dev);
> +
>  	if (!pci_probe_reset_function(dev)) {
>  		retval = device_create_file(&dev->dev, &reset_attr);
>  		if (retval)
> @@ -1433,6 +1436,10 @@ static void pci_remove_capabilities_sysfs(struct pci_dev *dev)
>  	}
>  
>  	pcie_aspm_remove_sysfs_dev_files(dev);
> +
> +	/* PTM */
> +	pci_release_ptm_sysfs(dev);
> +
>  	if (dev->reset_fn) {
>  		device_remove_file(&dev->dev, &reset_attr);
>  		dev->reset_fn = 0;
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 9a1660f..fb90420 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -320,6 +320,27 @@ static inline resource_size_t pci_resource_alignment(struct pci_dev *dev,
>  
>  void pci_enable_acs(struct pci_dev *dev);
>  
> +#ifdef CONFIG_PCIEPORTBUS
> +int pci_enable_ptm(struct pci_dev *dev);
> +void pci_create_ptm_sysfs(struct pci_dev *dev);
> +void pci_release_ptm_sysfs(struct pci_dev *dev);
> +void pci_disable_ptm(struct pci_dev *dev);
> +#else
> +static inline int pci_enable_ptm(struct pci_dev *dev)
> +{
> +	return -ENXIO;
> +}
> +static inline void pci_create_ptm_sysfs(struct pci_dev *dev)
> +{
> +}
> +static inline void pci_release_ptm_sysfs(struct pci_dev *dev)
> +{
> +}
> +static inline void pci_disable_ptm(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 e294713..f65ff4d 100644
> --- a/drivers/pci/pcie/Kconfig
> +++ b/drivers/pci/pcie/Kconfig
> @@ -80,3 +80,11 @@ endchoice
>  config PCIE_PME
>  	def_bool y
>  	depends on PCIEPORTBUS && PM
> +
> +config PCIE_PTM
> +	bool "Turn on Precision Time Management by default"
> +	depends on PCIEPORTBUS
> +	help
> +	  Say Y here to enable PTM feature on PCI Express devices that
> +	  support them as they are found during device enumeration. Otherwise
> +	  the feature can be enabled manually through sysfs entries.
> diff --git a/drivers/pci/pcie/Makefile b/drivers/pci/pcie/Makefile
> index 00c62df..d18b4c7 100644
> --- a/drivers/pci/pcie/Makefile
> +++ b/drivers/pci/pcie/Makefile
> @@ -5,7 +5,7 @@
>  # Build PCI Express ASPM if needed
>  obj-$(CONFIG_PCIEASPM)		+= aspm.o
>  
> -pcieportdrv-y			:= portdrv_core.o portdrv_pci.o portdrv_bus.o
> +pcieportdrv-y			:= portdrv_core.o portdrv_pci.o portdrv_bus.o pcie_ptm.o
>  pcieportdrv-$(CONFIG_ACPI)	+= portdrv_acpi.o
>  
>  obj-$(CONFIG_PCIEPORTBUS)	+= pcieportdrv.o
> diff --git a/drivers/pci/pcie/pcie_ptm.c b/drivers/pci/pcie/pcie_ptm.c
> new file mode 100644
> index 0000000..a128c79
> --- /dev/null
> +++ b/drivers/pci/pcie/pcie_ptm.c
> @@ -0,0 +1,353 @@
> +/*
> + * 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"
> +
> +#define PCI_PTM_REQ		0x0001  /* Requester capable */
> +#define  PCI_PTM_RSP		0x0002  /* Responder capable */
> +#define  PCI_PTM_ROOT		0x0004  /* Root capable */
> +#define  PCI_PTM_GRANULITY	0xFF00  /* Local clock granulity */

s/granulity/granularity/

> +#define PCI_PTM_ENABLE		0x0001  /* PTM enable */
> +#define  PCI_PTM_ROOT_SEL	0x0002  /* Root select */
> +
> +#define PCI_PTM_HEADER_REG_OFFSET	0x00
> +#define PCI_PTM_CAPABILITY_REG_OFFSET	0x04
> +#define PCI_PTM_CONTROL_REG_OFFSET	0x08
> +
> +#define PCI_EXT_CAP_ID_PTM		0x001f

These should go in include/uapi/linux/pci_regs.h along with the other
PCI capability definitions.  Obviously they should match the style of
the existing code there.

> +#ifdef CONFIG_PCIE_PTM
> +static bool disable_ptm;
> +#else
> +static bool disable_ptm = 1;
> +#endif
> +
> +module_param_named(disable_ptm, disable_ptm, bool, S_IRUGO | S_IWUSR);
> +MODULE_PARM_DESC(disable_ptm,
> +	"Don't automatically enable even if supported.");
> +
> +static inline u8 get_granularity(u32 in)
> +{
> +	return (in & PCI_PTM_GRANULITY) >> 8;
> +}
> +
> +static ssize_t ptm_status_show(struct device *dev,
> +	struct device_attribute *attr, char *buf)
> +{
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +	u16 word;
> +	int pos;
> +
> +	pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PTM);
> +	if (!pos)
> +		return -ENXIO;
> +
> +	pci_read_config_word(pdev, pos + PCI_PTM_CONTROL_REG_OFFSET, &word);
> +	return sprintf(buf, "%u\n", word & PCI_PTM_ENABLE);
> +}
> +
> +static ssize_t ptm_status_store(struct device *dev,
> +	struct device_attribute *attr, const char *buf, size_t count)
> +{
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +	unsigned long val;
> +	ssize_t ret;
> +	int pos;
> +
> +	pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PTM);
> +	if (!pos)
> +		return -ENXIO;
> +
> +	ret = kstrtoul(buf, 0, &val);
> +	if (ret)
> +		return ret;
> +	if (val)
> +		return pci_enable_ptm(pdev);
> +	pci_disable_ptm(pdev);
> +	return 0;
> +}
> +
> +static DEVICE_ATTR_RW(ptm_status);
> +
> +void pci_release_ptm_sysfs(struct pci_dev *dev)
> +{
> +	if (!pci_find_ext_capability(dev, PCI_EXT_CAP_ID_PTM))
> +		return;
> +	device_remove_file(&dev->dev, &dev_attr_ptm_status);
> +}
> +
> +void pci_create_ptm_sysfs(struct pci_dev *dev)
> +{
> +	if (!pci_find_ext_capability(dev, PCI_EXT_CAP_ID_PTM))
> +		return;
> +	device_create_file(&dev->dev, &dev_attr_ptm_status);
> +}
> +
> +/**
> + * pci_enable_ptm - Try to activate PTM functionality on device.
> + * @dev: PCI Express device with PTM requester role to enable.
> + *
> + * The function will crawl through the PCI Hierarchy to determine if it is
> + * possible to enable the Precision Time Measurement requester role on @dev,
> + * and if so, activate it by setting the granularity field.
> + *
> + * NOTE: Each requester must be associated with a PTM root (not to be confused
> + * with a root port or root complex). There can be multiple PTM roots in a
> + * a system forming multiple domains. All intervening bridges/switches in a
> + * domain must support PTM responder roles to relay PTM dialogues.
> + */
> +int pci_enable_ptm(struct pci_dev *dev)
> +{

This function is way too long to review as it is.  Can you split it up
somehow?

> +	struct pci_dev *curr, **steps;
> +	size_t i = 0, root = 0;
> +	int pos, pos2;
> +	u8 granularity = 0;
> +	u16 word;
> +	int ret = 0;
> +
> +	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_PTM);
> +	if (!pos) {
> +		dev_dbg(&dev->dev, "Not PTM capable, skipping.\n");
> +		return -ENXIO;
> +	}
> +
> +	if (disable_ptm)
> +		return 0;
> +
> +	/* Skip if not requester role. */
> +	pci_read_config_word(dev, pos + PCI_PTM_CAPABILITY_REG_OFFSET, &word);
> +	if (!(word & PCI_PTM_REQ)) {
> +		dev_dbg(&dev->dev, "Not a PTM requester, skipping for now.\n");
> +		return 0;
> +	}
> +
> +	/* Just copy and enable PTM granularity for integrated endpoints. */
> +	if (pci_pcie_type(dev) == PCI_EXP_TYPE_RC_END) {
> +		dev_dbg(&dev->dev,
> +			"Root integrated endpoint, attempting to copy root granularity.\n");
> +		curr = pci_upstream_bridge(dev);
> +		if (!curr)
> +			return 0;
> +
> +		pos2 = pci_find_ext_capability(curr, PCI_EXT_CAP_ID_PTM);
> +		if (!pos2)
> +			return 0;
> +
> +		/* Get granularity field. */
> +		pci_read_config_word(curr,
> +			pos2 + PCI_PTM_CAPABILITY_REG_OFFSET,
> +			&word);
> +		word &= PCI_PTM_GRANULITY;
> +
> +		/* Copy it over. */
> +		word |= PCI_PTM_ENABLE;
> +		pci_write_config_word(dev, pos + PCI_PTM_CONTROL_REG_OFFSET,
> +			word);
> +
> +		/* Enable PTM on root complex if not already so. */
> +		pci_read_config_word(curr, pos2 + PCI_PTM_CONTROL_REG_OFFSET,
> +			&word);
> +		if (!(word & PCI_PTM_ENABLE)) {
> +			pci_read_config_word(curr,
> +				pos2 + PCI_PTM_CONTROL_REG_OFFSET, &word);
> +			word |= PCI_PTM_ENABLE | PCI_PTM_ROOT_SEL;
> +			pci_write_config_word(curr,
> +				pos2 + PCI_PTM_CONTROL_REG_OFFSET, word);
> +		}
> +	} else {
> +		/* For holding all the bridge/switches in between. */
> +		steps = kzalloc(sizeof(*steps) * dev->bus->number + 1,
> +			GFP_KERNEL);
> +		if (!steps)
> +			return -ENOMEM;
> +
> +		/* Gather all the upstream devices. */
> +		curr = pci_upstream_bridge(dev);
> +		if (curr) {
> +			dev_dbg(&dev->dev,
> +				"Upstream is %d:%d:%x.%d\n",
> +				pci_domain_nr(curr->bus),
> +				curr->bus->number,
> +				PCI_SLOT(curr->devfn),
> +				PCI_FUNC(curr->devfn)
> +				);
> +		} else {
> +			dev_dbg(&dev->dev, "No upstream??\n");
> +			ret = -ENXIO;
> +			goto abort;
> +		}
> +
> +		do {
> +			/* sanity check */
> +			if (i > dev->bus->number)
> +				break;
> +			steps[i++] = curr;
> +		} while ((curr = pci_upstream_bridge(curr)));
> +
> +		/* Check upstream chains capability. */
> +		dev_dbg(&dev->dev,
> +				   "Checking hierarchy capabilities\n");
> +		for (i = 0; i < dev->bus->number + 1; i++) {
> +			curr = steps[i];
> +			if (!curr)
> +				break;
> +
> +			pos2 = pci_find_ext_capability(curr,
> +				PCI_EXT_CAP_ID_PTM);
> +
> +			if (!pos2) {
> +				dev_dbg(&curr->dev,
> +					"PTM Hierarchy %zx: not PTM aware\n",
> +					i);
> +				break;
> +			}
> +
> +			/* End if upstream cannot respond. */
> +			pci_read_config_word(curr,
> +				pos2 + PCI_PTM_CAPABILITY_REG_OFFSET, &word);
> +			if (!(word & PCI_PTM_RSP)) {
> +				dev_dbg(&curr->dev,
> +				"PTM Hierarchy: skipping non-responder\n");
> +				break;
> +			}
> +
> +			/* Is root capable? */
> +			if (word & PCI_PTM_ROOT) {
> +				root = i;
> +				granularity = get_granularity(word);
> +			}
> +		}
> +
> +		if (!steps[root]) {
> +			dev_dbg(&dev->dev, "Cannot find root, aborting\n");
> +			ret = -ENXIO;
> +			goto abort;
> +		}
> +
> +		dev_dbg(&dev->dev,
> +			"Found PTM root at %d:%d:%x.%d granularity %u\n",
> +			pci_domain_nr(steps[root]->bus),
> +			steps[root]->bus->number,
> +			PCI_SLOT(steps[root]->devfn),
> +			PCI_FUNC(steps[root]->devfn),
> +			granularity);
> +
> +		/* Program granularity field. */
> +		for (i = root;;) {
> +			curr = steps[i];
> +			pos2 = pci_find_ext_capability(curr,
> +				PCI_EXT_CAP_ID_PTM);
> +			pci_read_config_word(curr,
> +				pos2 + PCI_PTM_CONTROL_REG_OFFSET, &word);
> +
> +			/* If not yet PTM enabled. */
> +			if (!(word & PCI_PTM_ENABLE)) {
> +				pci_read_config_word(curr,
> +					pos2 + PCI_PTM_CAPABILITY_REG_OFFSET,
> +					&word);
> +				/* If requester capable, program granularity. */
> +				if (word & PCI_PTM_REQ) {
> +					dev_dbg(&curr->dev,
> +						"Programming granularity %u\n",
> +						granularity);
> +					pci_write_config_word(curr,
> +						pos2 +
> +						PCI_PTM_CONTROL_REG_OFFSET,
> +						((u16)granularity) << 8);
> +				}
> +				if ((word & PCI_PTM_ROOT) &&
> +					granularity != 0 &&
> +					((granularity < get_granularity(word))
> +					|| get_granularity(word) == 0)) {
> +					dev_dbg(&curr->dev,
> +					"Updating granularity %u to %u\n",
> +						granularity,
> +						get_granularity(word));
> +					granularity = get_granularity(word);
> +				}
> +			}
> +			if (!i)
> +				break;
> +			i--;
> +		}
> +
> +		/* Program current device granularity and enable it. */
> +		pci_read_config_word(dev, pos + PCI_PTM_CONTROL_REG_OFFSET,
> +			&word);
> +		word = (word & ~PCI_PTM_GRANULITY) | ((u16)granularity) << 8
> +			| PCI_PTM_ENABLE;
> +		pci_write_config_word(dev, pos + PCI_PTM_CONTROL_REG_OFFSET,
> +			word);
> +		dev_dbg(&dev->dev,
> +				"Using granularity %u, %x\n", granularity,
> +				word);
> +
> +		/* Enable PTM root. */
> +		pos2 = pci_find_ext_capability(steps[root],
> +			PCI_EXT_CAP_ID_PTM);
> +		pci_read_config_word(steps[root],
> +			pos2 + PCI_PTM_CONTROL_REG_OFFSET, &word);
> +		word |= PCI_PTM_ROOT_SEL | PCI_PTM_ENABLE;
> +		pci_write_config_word(steps[root],
> +			pos2 + PCI_PTM_CONTROL_REG_OFFSET, word);
> +
> +		/* PTM enable from the bottom up. */
> +		for (i = 0; i <= root; i++) {
> +			pos2 = pci_find_ext_capability(steps[i],
> +				PCI_EXT_CAP_ID_PTM);
> +			pci_read_config_word(steps[i],
> +				pos2 + PCI_PTM_CONTROL_REG_OFFSET, &word);
> +			word |= PCI_PTM_ENABLE;
> +			pci_write_config_word(steps[i],
> +				pos2 + PCI_PTM_CONTROL_REG_OFFSET,
> +				word);
> +		}

I haven't read the PTM spec yet so I don't know how it works.  But in
general, I don't like having to tweak a setting all the way up the
hierarchy based on a leaf device.  That makes it hard to handle
hotplug correctly, because obviously there may be many leaf devices
that share part of all of the upstream path.

> +abort:
> +		kfree(steps);
> +	}
> +	return ret;
> +}
> +
> +static int do_disable_ptm(struct pci_dev *dev, void *v)
> +{
> +	int pos;
> +	u16 word;
> +
> +	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_PTM);
> +	if (!pos)
> +		return 0;
> +
> +	pci_read_config_word(dev, pos + PCI_PTM_CONTROL_REG_OFFSET, &word);
> +	word &= ~PCI_PTM_ENABLE;
> +	pci_write_config_word(dev, pos + PCI_PTM_CONTROL_REG_OFFSET, word);
> +	return 0;
> +}
> +
> +/**
> + * pci_disable_ptm - Turn off PTM functionality on device.
> + * @dev: PCI Express device with PTM function to disable.
> + *
> + * Disables PTM functionality by clearing the PTM enable bit, if device is a
> + * switch/bridge it will also disable PTM function on other devices behind it.
> + */
> +void pci_disable_ptm(struct pci_dev *dev)
> +{
> +	if (pci_is_bridge(dev))
> +		pci_walk_bus(dev->bus, &do_disable_ptm, NULL);
> +	else
> +		do_disable_ptm(dev, NULL);
> +}
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 6d7ab9b..8dc8bbc 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1623,6 +1623,9 @@ static void pci_init_capabilities(struct pci_dev *dev)
>  	pci_enable_acs(dev);
>  
>  	pci_cleanup_aer_error_status_regs(dev);
> +
> +	/* Enable PTM Capabilities */

Another superfluous comment.  No need to repeat what the code already
says.

> +	pci_enable_ptm(dev);
>  }
>  
>  /*
> -- 
> 2.4.10
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] PCI: PTM preliminary implementation

[Yong, Jonathan]

On 03/11/2016 23:53, Bjorn Helgaas wrote:

> I haven't read the PTM spec yet so I don't know how it works.  But in
> general, I don't like having to tweak a setting all the way up the
> hierarchy based on a leaf device.  That makes it hard to handle
> hotplug correctly, because obviously there may be many leaf devices
> that share part of all of the upstream path.
>

This part in 6.22.3 in the PCIe 3.1 spec:

Software must not have the PTM Enable bit Set in the PTM Control 
register on a Function associated with an Upstream Port unless the 
associated Downstream Port on the Link already has the PTM Enable bit 
Set in its associated PTM Control register.

Seems to suggest starting from the leaf device, I'm open to suggestions 
on how to better do this.

Re: [PATCH] PCI: PTM preliminary implementation

[Bjorn Helgaas]

On Mon, Mar 14, 2016 at 03:44:51PM +0800, Yong, Jonathan wrote:
> On 03/11/2016 23:53, Bjorn Helgaas wrote:
> 
> >I haven't read the PTM spec yet so I don't know how it works.  But in
> >general, I don't like having to tweak a setting all the way up the
> >hierarchy based on a leaf device.  That makes it hard to handle
> >hotplug correctly, because obviously there may be many leaf devices
> >that share part of all of the upstream path.
> >
> 
> This part in 6.22.3 in the PCIe 3.1 spec:
> 
> Software must not have the PTM Enable bit Set in the PTM Control
> register on a Function associated with an Upstream Port unless the
> associated Downstream Port on the Link already has the PTM Enable
> bit Set in its associated PTM Control register.
> 
> Seems to suggest starting from the leaf device, I'm open to
> suggestions on how to better do this.

The nomenclature is confusing, but I think you're reading this
backwards.  An Upstream Port is on the downstream end of a Link.  The
"Upstream" definition in the PCIe spec "Terms and Acronyms" section
says:

  The Port on a Switch that is closest topologically to the Root Complex
  is the Upstream Port. The Port on a component that contains only
  Endpoint or Bridge Functions is an Upstream Port.

I think the spec is saying that PTM must be enabled in a bridge before
it is enabled in any device downstream from the bridge.

Bjorn

Re: [PATCH] PCI: PTM preliminary implementation

[Yong, Jonathan]

On 03/14/2016 23:42, Bjorn Helgaas wrote:
>
> The nomenclature is confusing, but I think you're reading this
> backwards.  An Upstream Port is on the downstream end of a Link.  The
> "Upstream" definition in the PCIe spec "Terms and Acronyms" section
> says:
>
>    The Port on a Switch that is closest topologically to the Root Complex
>    is the Upstream Port. The Port on a component that contains only
>    Endpoint or Bridge Functions is an Upstream Port.
>
> I think the spec is saying that PTM must be enabled in a bridge before
> it is enabled in any device downstream from the bridge.
>

Thanks for the explanation, looks like back to the drawing board. Do you 
recommend using pci_walk_bus on all potential PTM masters?

Re: [PATCH] PCI: PTM preliminary implementation

[Bjorn Helgaas]

On Tue, Mar 15, 2016 at 04:27:29PM +0800, Yong, Jonathan wrote:
> On 03/14/2016 23:42, Bjorn Helgaas wrote:
> >
> >The nomenclature is confusing, but I think you're reading this
> >backwards.  An Upstream Port is on the downstream end of a Link.  The
> >"Upstream" definition in the PCIe spec "Terms and Acronyms" section
> >says:
> >
> >   The Port on a Switch that is closest topologically to the Root Complex
> >   is the Upstream Port. The Port on a component that contains only
> >   Endpoint or Bridge Functions is an Upstream Port.
> >
> >I think the spec is saying that PTM must be enabled in a bridge before
> >it is enabled in any device downstream from the bridge.
> >
> 
> Thanks for the explanation, looks like back to the drawing board. Do
> you recommend using pci_walk_bus on all potential PTM masters?

No, I try to avoid using pci_walk_bus().  I would try something like this
in the pci_init_capabilities() path:

  pci_ptm_init(struct pci_dev *dev)
  {
    if (!pci_find_ext_capability(dev, PCI_EXT_CAP_ID_PTM))
      return;

    type = pci_pcie_type(dev);
    if (type == PCI_EXP_TYPE_ENDPOINT || type == PCI_EXP_TYPE_RC_END) {
      if (pci_upstream_bridge(dev)->ptm_enabled)
	enable_ptm(dev);
	return;
    }

    if (type == PCI_EXP_TYPE_DOWNSTREAM) {
      dev->ptm_enabled = pci_upstream_bridge(dev)->ptm_enabled;
      return;
    }

    enable_ptm(dev);
    dev_info(&dev->dev, "PTM enabled, root, granularity, etc...")
  }

I've only skimmed the PTM spec, so this is just a brief and incomplete
sketch.

Bjorn

[RFC v2] PCI: PTM Driver

[Yong, Jonathan]

Hello LKML,

This is a preliminary implementation of the PTM[1] support driver, the code
is obviously hacked together and in need of refactoring. This driver has
only been tested against a virtual PCI bus.

The drivers job is to get to every PTM capable device, set some PCI config
space bits, then go back to sleep [2].

PTM capable PCIe devices will get a new sysfs entry to allow PTM to be
enabled if automatic PTM activation is disabled, or disabled if so desired.

Comments? Should I explain the PTM registers in more details?
Please CC me, thanks.

[1] Precision Time Measurement: A protocol for synchronizing PCIe endpoint
clocks against the host clock as specified in the PCI Express Base
Specification 3.1. It is identified by the 0x001f extended capability ID.

PTM capable devices are split into 3 roles, master, responder and requester.
Summary as follows:

A master holds the master clock that will be used for all devices under its
domain (not to be confused with PCI domains). There may be multiple masters
in a PTM hierarchy, in which case, the highest master closest to the root
complex will be selected for the PTM domain. A master is also always
responder capable. Clock precision is signified by a Local Clock
Granularity field, in nano-seconds.

A responder responds to any PTM synchronization requests from a downstream
device. A responder is typically a switch device. It may also hold a local
clock signified by a non-zero Local Clock Granularity field. A value of 0
signifies that the device simply propagates timing information from
upstream devices.

A requester is typically an endpoint that will request synchronization
updates from an upstream PTM capable time source. The driver will update
the Effective Clock Granularity field based on the same field from the
PTM domain master. The field should be programmed with a value of 0 if any
intervening responder has a Local Clock Granularity field value of 0.

[2] The software drivers never see the PTM packets, the PCI Express Base
Specification 3.1 reads:
	PTM capable components can make their PTM context available for
	inspection by software, enabling software to translate timing
	information between local times and PTM Master Time.

This isn't very informative.

Changes since v1:
* Moved register constants to pci_regs.h
* Use pci_dev to hold PTM status
* PTM initialization now done top-down hierarchy wise.

Yong, Jonathan (1):
  PCI: PTM preliminary implementation

 drivers/pci/pci-sysfs.c       |   7 ++
 drivers/pci/pci.h             |  25 +++++
 drivers/pci/pcie/Kconfig      |   8 ++
 drivers/pci/pcie/Makefile     |   2 +-
 drivers/pci/pcie/pcie_ptm.c   | 212 ++++++++++++++++++++++++++++++++++++++++++
 drivers/pci/probe.c           |   3 +
 include/linux/pci.h           |   9 ++
 include/uapi/linux/pci_regs.h |  12 +++
 8 files changed, 277 insertions(+), 1 deletion(-)
 create mode 100644 drivers/pci/pcie/pcie_ptm.c

-- 
2.7.3

[PATCH] PCI: PTM preliminary implementation

[Yong, Jonathan]

Simplified Precision Time Measurement driver, activates PTM feature
if a PCIe PTM requester (as per PCI Express 3.1 Base Specification
section 7.32)is found, but not before checking if the rest of the
PCI hierarchy can support it.

The driver does not take part in facilitating PTM conversations,
neither does it provide any useful services, it is only responsible
for setting up the required configuration space bits.

As of writing, there aren't any PTM capable devices on the market
yet, but it is supported by the Intel Apollo Lake platform.

Signed-off-by: Yong, Jonathan <jonathan.yong@intel.com>
---
 drivers/pci/pci-sysfs.c       |   7 ++
 drivers/pci/pci.h             |  25 +++++
 drivers/pci/pcie/Kconfig      |   8 ++
 drivers/pci/pcie/Makefile     |   2 +-
 drivers/pci/pcie/pcie_ptm.c   | 212 ++++++++++++++++++++++++++++++++++++++++++
 drivers/pci/probe.c           |   3 +
 include/linux/pci.h           |   9 ++
 include/uapi/linux/pci_regs.h |  12 +++
 8 files changed, 277 insertions(+), 1 deletion(-)
 create mode 100644 drivers/pci/pcie/pcie_ptm.c

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index e982010..edda4ba 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -1342,6 +1342,9 @@ static int pci_create_capabilities_sysfs(struct pci_dev *dev)
 	/* Active State Power Management */
 	pcie_aspm_create_sysfs_dev_files(dev);
 
+	/* PTM */
+	pci_create_ptm_sysfs(dev);
+
 	if (!pci_probe_reset_function(dev)) {
 		retval = device_create_file(&dev->dev, &reset_attr);
 		if (retval)
@@ -1436,6 +1439,10 @@ static void pci_remove_capabilities_sysfs(struct pci_dev *dev)
 	}
 
 	pcie_aspm_remove_sysfs_dev_files(dev);
+
+	/* PTM */
+	pci_release_ptm_sysfs(dev);
+
 	if (dev->reset_fn) {
 		device_remove_file(&dev->dev, &reset_attr);
 		dev->reset_fn = 0;
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index d0fb934..c5ece1e 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -320,6 +320,31 @@ static inline resource_size_t pci_resource_alignment(struct pci_dev *dev,
 
 void pci_enable_acs(struct pci_dev *dev);
 
+#ifdef CONFIG_PCIEPORTBUS
+int pci_enable_ptm(struct pci_dev *dev);
+void pci_create_ptm_sysfs(struct pci_dev *dev);
+void pci_release_ptm_sysfs(struct pci_dev *dev);
+void pci_disable_ptm(struct pci_dev *dev);
+void pci_ptm_init(struct pci_dev *dev);
+#else
+static inline int pci_enable_ptm(struct pci_dev *dev)
+{
+	return -ENXIO;
+}
+static inline void pci_create_ptm_sysfs(struct pci_dev *dev)
+{
+}
+static inline void pci_release_ptm_sysfs(struct pci_dev *dev)
+{
+}
+static inline void pci_disable_ptm(struct pci_dev *dev)
+{
+}
+static inline 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 72db7f4..9348cc3 100644
--- a/drivers/pci/pcie/Kconfig
+++ b/drivers/pci/pcie/Kconfig
@@ -81,3 +81,11 @@ endchoice
 config PCIE_PME
 	def_bool y
 	depends on PCIEPORTBUS && PM
+
+config PCIE_PTM
+	bool "Turn on Precision Time Management by default"
+	depends on PCIEPORTBUS
+	help
+	  Say Y here to enable PTM feature on PCI Express devices that
+	  support them as they are found during device enumeration. Otherwise
+	  the feature can be enabled manually through sysfs entries.
diff --git a/drivers/pci/pcie/Makefile b/drivers/pci/pcie/Makefile
index 00c62df..d18b4c7 100644
--- a/drivers/pci/pcie/Makefile
+++ b/drivers/pci/pcie/Makefile
@@ -5,7 +5,7 @@
 # Build PCI Express ASPM if needed
 obj-$(CONFIG_PCIEASPM)		+= aspm.o
 
-pcieportdrv-y			:= portdrv_core.o portdrv_pci.o portdrv_bus.o
+pcieportdrv-y			:= portdrv_core.o portdrv_pci.o portdrv_bus.o pcie_ptm.o
 pcieportdrv-$(CONFIG_ACPI)	+= portdrv_acpi.o
 
 obj-$(CONFIG_PCIEPORTBUS)	+= pcieportdrv.o
diff --git a/drivers/pci/pcie/pcie_ptm.c b/drivers/pci/pcie/pcie_ptm.c
new file mode 100644
index 0000000..3476e65
--- /dev/null
+++ b/drivers/pci/pcie/pcie_ptm.c
@@ -0,0 +1,212 @@
+/*
+ * 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"
+
+int pci_enable_ptm(struct pci_dev *dev);
+void pci_release_ptm(struct pci_dev *dev);
+
+#ifdef CONFIG_PCIE_PTM
+static bool disable_ptm;
+#else
+static bool disable_ptm = 1;
+#endif
+
+module_param_named(disable_ptm, disable_ptm, bool, S_IRUGO | S_IWUSR);
+MODULE_PARM_DESC(disable_ptm, "Don't automatically enable PCIe PTM even if supported.");
+
+static int ptm_commit(struct pci_dev *dev)
+{
+	u32 dword;
+	int pos;
+
+	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_PTM);
+
+	/* Is this even possible? */
+	if (!pos)
+		return -ENXIO;
+
+	pci_read_config_dword(dev, pos + PCI_PTM_CONTROL_REG_OFFSET, &dword);
+	dword = dev->is_ptm_enabled ? dword | PCI_PTM_CTRL_ENABLE :
+		dword & ~PCI_PTM_CTRL_ENABLE;
+	dword = dev->is_ptm_root ? dword | PCI_PTM_CTRL_ROOT :
+		dword & ~PCI_PTM_CTRL_ROOT;
+
+	/* Only requester should have it set */
+	if (dev->is_ptm_requester)
+		dword = dword | (((u32)dev->ptm_effective_granularity) << 8);
+	return pci_write_config_dword(dev, pos + PCI_PTM_CONTROL_REG_OFFSET, dword);
+}
+
+/**
+ * pci_enable_ptm - Try to activate PTM functionality on device.
+ * @dev: PCI Express device with PTM requester role to enable.
+ *
+ * All PCIe Switches/Bridges in between need to be enabled for this to work.
+ *
+ * NOTE: Each requester must be associated with a PTM root (not to be confused
+ * with a root port or root complex). There can be multiple PTM roots in a
+ * a system forming multiple domains. All intervening bridges/switches in a
+ * domain must support PTM responder roles to relay PTM dialogues.
+ */
+int pci_enable_ptm(struct pci_dev *dev)
+{
+	int type;
+	struct pci_dev *upstream;
+
+	upstream = pci_upstream_bridge(dev);
+	type = pci_pcie_type(dev);
+
+	if (dev->is_ptm_root_capable)
+	{
+		/* If we are root capable but already part of a chain, don't set
+		 * the root select bit, only enable PTM */
+		if (!upstream || !upstream->is_ptm_enabled)
+			dev->is_ptm_root = 1;
+		dev->is_ptm_enabled = 1;
+	}
+
+	/* Is possible to be part of the PTM chain */
+	if (dev->is_ptm_responder && upstream && upstream->is_ptm_enabled)
+		dev->is_ptm_enabled = 1;
+
+	if (dev->is_ptm_requester && upstream && upstream->is_ptm_enabled) {
+		dev->is_ptm_enabled = 1;
+		dev->ptm_effective_granularity =
+			upstream->ptm_clock_granularity;
+	}
+	return ptm_commit(dev);
+}
+
+void pci_ptm_init(struct pci_dev *dev)
+{
+	u32 dword;
+	int pos;
+	u8 ver;
+
+	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_PTM);
+	if (!pos)
+		return;
+
+	/* Check capability version */
+	pci_read_config_dword(dev, pos, &dword);
+	ver = PCI_EXT_CAP_VER(dword);
+	if (ver != 0x1)
+	{
+		dev_warn(&dev->dev, "Expected PTM v1, got %u\n", ver);
+		return;
+	}
+
+	/* Fill in caps, masters are implied to be responders as well */
+	pci_read_config_dword(dev, pos + PCI_PTM_CAPABILITY_REG_OFFSET, &dword);
+	dev->is_ptm_capable = 1;
+	dev->is_ptm_root_capable   = (dword & PCI_PTM_CAP_ROOT) ? 1 : 0;
+	dev->is_ptm_responder      = (dword & PCI_PTM_CAP_RSP) ? 1 : 0;
+	dev->is_ptm_requester      = (dword & PCI_PTM_CAP_REQ) ? 1 : 0;
+	dev->ptm_clock_granularity = dev->is_ptm_responder ?
+		((dword & PCI_PTM_GRANULARITY_MASK) >> 8) : 0;
+	dev_info(&dev->dev, "Found PTM %s type device with %uns clock\n",
+		dev->is_ptm_root_capable ? "root" :
+		dev->is_ptm_responder ? "responder" :
+		dev->is_ptm_requester ? "requester" : "unknown",
+		dev->ptm_clock_granularity);
+
+	/* Get existing settings */
+	pci_read_config_dword(dev, pos + PCI_PTM_CONTROL_REG_OFFSET, &dword);
+	dev->is_ptm_enabled            = (dword & PCI_PTM_CTRL_ENABLE) ? 1 : 0;
+	dev->is_ptm_root               = (dword & PCI_PTM_CTRL_ROOT) ? 1 : 0;
+	dev->ptm_effective_granularity =
+		(dword & PCI_PTM_GRANULARITY_MASK) >> 8;
+
+	if(!disable_ptm)
+		pci_enable_ptm(dev);
+}
+
+static int do_disable_ptm(struct pci_dev *dev, void *v)
+{
+	if (dev->is_ptm_enabled)
+	{
+		dev->is_ptm_enabled            = 0;
+		dev->is_ptm_root               = 0;
+		dev->ptm_effective_granularity = 0;
+		ptm_commit(dev);
+	}
+	return 0;
+}
+
+/**
+ * pci_disable_ptm - Turn off PTM functionality on device.
+ * @dev: PCI Express device with PTM function to disable.
+ *
+ * Disables PTM functionality by clearing the PTM enable bit, if device is a
+ * switch/bridge it will also disable PTM function on other devices behind it.
+ */
+void pci_disable_ptm(struct pci_dev *dev)
+{
+	if (pci_is_bridge(dev))
+		pci_walk_bus(dev->bus, &do_disable_ptm, NULL);
+	else
+		do_disable_ptm(dev, NULL);
+}
+
+static ssize_t ptm_status_show(struct device *dev,
+	struct device_attribute *attr, char *buf)
+{
+	struct pci_dev *pdev = to_pci_dev(dev);
+	u16 word;
+	int pos;
+
+	pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PTM);
+	if (!pos)
+		return -ENXIO;
+
+	pci_read_config_word(pdev, pos + PCI_PTM_CONTROL_REG_OFFSET, &word);
+	return sprintf(buf, "%u\n", word & PCI_PTM_CTRL_ENABLE ? 1 : 0);
+}
+
+static ssize_t ptm_status_store(struct device *dev,
+	struct device_attribute *attr, const char *buf, size_t count)
+{
+	struct pci_dev *pdev = to_pci_dev(dev);
+	unsigned long val;
+	ssize_t ret;
+
+	ret = kstrtoul(buf, 0, &val);
+	if (ret)
+		return ret;
+	if (val)
+		return pci_enable_ptm(pdev);
+	pci_disable_ptm(pdev);
+	return 0;
+}
+
+static DEVICE_ATTR_RW(ptm_status);
+
+void pci_release_ptm_sysfs(struct pci_dev *dev)
+{
+	if (!pci_find_ext_capability(dev, PCI_EXT_CAP_ID_PTM))
+		return;
+	device_remove_file(&dev->dev, &dev_attr_ptm_status);
+}
+
+void pci_create_ptm_sysfs(struct pci_dev *dev)
+{
+	if (!pci_find_ext_capability(dev, PCI_EXT_CAP_ID_PTM))
+		return;
+	device_create_file(&dev->dev, &dev_attr_ptm_status);
+}
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 8004f67..9d5e96e6 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1657,6 +1657,9 @@ static void pci_init_capabilities(struct pci_dev *dev)
 	pci_enable_acs(dev);
 
 	pci_cleanup_aer_error_status_regs(dev);
+
+	/* Enable PTM Capabilities */
+	pci_ptm_init(dev);
 }
 
 /*
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 004b813..2facd44 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -363,6 +363,15 @@ 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 */
+
+	unsigned int 	is_ptm_capable:1;
+	unsigned int	is_ptm_root_capable:1;
+	unsigned int	is_ptm_responder:1;
+	unsigned int	is_ptm_requester:1;
+	unsigned int	is_ptm_enabled:1;
+	unsigned int	is_ptm_root:1;
+	u8		ptm_clock_granularity;
+	u8		ptm_effective_granularity;
 #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 1becea8..559b28f 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -671,6 +671,7 @@
 #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_MAX	PCI_EXT_CAP_ID_PASID
+#define PCI_EXT_CAP_ID_PTM	0x1f	/* Precision Time Measurement */
 
 #define PCI_EXT_CAP_DSN_SIZEOF	12
 #define PCI_EXT_CAP_MCAST_ENDPOINT_SIZEOF 40
@@ -946,4 +947,15 @@
 #define PCI_TPH_CAP_ST_SHIFT	16	/* st table shift */
 #define PCI_TPH_BASE_SIZEOF	12	/* size with no st table */
 
+/* Precision Time Measurement */
+#define PCI_PTM_CAP_REQ			0x0001  /* Requester capable */
+#define PCI_PTM_CAP_RSP			0x0002  /* Responder capable */
+#define PCI_PTM_CAP_ROOT		0x0004  /* Root capable */
+#define PCI_PTM_GRANULARITY_MASK	0xFF00  /* Local clock granularity */
+#define PCI_PTM_CTRL_ENABLE		0x0001  /* PTM enable */
+#define PCI_PTM_CTRL_ROOT		0x0002  /* Root select */
+#define PCI_PTM_HEADER_REG_OFFSET       0x00	/* PTM version and such */
+#define PCI_PTM_CAPABILITY_REG_OFFSET   0x04	/* Capabilities */
+#define PCI_PTM_CONTROL_REG_OFFSET      0x08	/* Control reg */
+
 #endif /* LINUX_PCI_REGS_H */
-- 
2.7.3

Re: [PATCH] PCI: PTM preliminary implementation

[kbuild test robot]

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

Hi Jonathan,

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

url:    https://github.com/0day-ci/linux/commits/Yong-Jonathan/PCI-PTM-preliminary-implementation/20160323-105046
base:   https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: x86_64-randconfig-x018-201612 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   In file included from drivers/pci/access.c:9:0:
>> drivers/pci/pci.h:343:15: warning: return type defaults to 'int' [-Wreturn-type]
    static inline pci_ptm_init(struct pci_dev *dev)
                  ^
--
   In file included from drivers/pci/probe.c:19:0:
>> drivers/pci/pci.h:343:15: warning: return type defaults to 'int' [-Wreturn-type]
    static inline pci_ptm_init(struct pci_dev *dev)
                  ^
   drivers/pci/pci.h: In function 'pci_ptm_init':
>> drivers/pci/pci.h:345:1: warning: control reaches end of non-void function [-Wreturn-type]
    }
    ^

vim +/int +343 drivers/pci/pci.h

   337	static inline void pci_release_ptm_sysfs(struct pci_dev *dev)
   338	{
   339	}
   340	static inline void pci_disable_ptm(struct pci_dev *dev)
   341	{
   342	}
 > 343	static inline pci_ptm_init(struct pci_dev *dev)
   344	{
 > 345	}
   346	#endif
   347	
   348	struct pci_dev_reset_methods {

---
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: 24513 bytes --]

Re: [PATCH] PCI: PTM preliminary implementation

[kbuild test robot]

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

Hi Jonathan,

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

url:    https://github.com/0day-ci/linux/commits/Yong-Jonathan/PCI-PTM-preliminary-implementation/20160323-105046
base:   https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: x86_64-randconfig-n0-03231028 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   In file included from drivers/xen/pci.c:29:0:
>> drivers/xen/../pci/pci.h:343:15: warning: return type defaults to 'int' [-Wreturn-type]
    static inline pci_ptm_init(struct pci_dev *dev)
                  ^

vim +/int +343 drivers/xen/../pci/pci.h

   327	void pci_disable_ptm(struct pci_dev *dev);
   328	void pci_ptm_init(struct pci_dev *dev);
   329	#else
   330	static inline int pci_enable_ptm(struct pci_dev *dev)
   331	{
   332		return -ENXIO;
   333	}
   334	static inline void pci_create_ptm_sysfs(struct pci_dev *dev)
   335	{
   336	}
   337	static inline void pci_release_ptm_sysfs(struct pci_dev *dev)
   338	{
   339	}
   340	static inline void pci_disable_ptm(struct pci_dev *dev)
   341	{
   342	}
 > 343	static inline pci_ptm_init(struct pci_dev *dev)
   344	{
   345	}
   346	#endif
   347	
   348	struct pci_dev_reset_methods {
   349		u16 vendor;
   350		u16 device;
   351		int (*reset)(struct pci_dev *dev, int probe);

---
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: 20552 bytes --]

[RFC v3] PCI: PTM Driver

[Yong, Jonathan]

Hello LKML,

This is a preliminary implementation of the PTM[1] support driver, the code
is obviously hacked together and in need of refactoring. This driver has
only been tested against a virtual PCI bus.

The drivers job is to get to every PTM capable device, set some PCI config
space bits, then go back to sleep [2].

PTM capable PCIe devices will get a new sysfs entry to allow PTM to be
enabled if automatic PTM activation is disabled, or disabled if so desired.

Comments? Should I explain the PTM registers in more details?
Please CC me, thanks.

[1] Precision Time Measurement: A protocol for synchronizing PCIe endpoint
clocks against the host clock as specified in the PCI Express Base
Specification 3.1. It is identified by the 0x001f extended capability ID.

PTM capable devices are split into 3 roles, master, responder and requester.
Summary as follows:

A master holds the master clock that will be used for all devices under its
domain (not to be confused with PCI domains). There may be multiple masters
in a PTM hierarchy, in which case, the highest master closest to the root
complex will be selected for the PTM domain. A master is also always
responder capable. Clock precision is signified by a Local Clock
Granularity field, in nano-seconds.

A responder responds to any PTM synchronization requests from a downstream
device. A responder is typically a switch device. It may also hold a local
clock signified by a non-zero Local Clock Granularity field. A value of 0
signifies that the device simply propagates timing information from
upstream devices.

A requester is typically an endpoint that will request synchronization
updates from an upstream PTM capable time source. The driver will update
the Effective Clock Granularity field based on the same field from the
PTM domain master. The field should be programmed with a value of 0 if any
intervening responder has a Local Clock Granularity field value of 0.

[2] The software drivers never see the PTM packets, the PCI Express Base
Specification 3.1 reads:
	PTM capable components can make their PTM context available for
	inspection by software, enabling software to translate timing
	information between local times and PTM Master Time.

This isn't very informative.

Changes since v1:
* Moved register constants to pci_regs.h
* Use pci_dev to hold PTM status
* PTM initialization now done top-down hierarchy wise.

Changes since v2:
* Added missing void return in the pci_ptm_init inline stub.

Yong, Jonathan (1):
  PCI: PTM preliminary implementation

 drivers/pci/pci-sysfs.c       |   7 ++
 drivers/pci/pci.h             |  25 +++++
 drivers/pci/pcie/Kconfig      |   8 ++
 drivers/pci/pcie/Makefile     |   2 +-
 drivers/pci/pcie/pcie_ptm.c   | 212 ++++++++++++++++++++++++++++++++++++++++++
 drivers/pci/probe.c           |   3 +
 include/linux/pci.h           |   9 ++
 include/uapi/linux/pci_regs.h |  12 +++
 8 files changed, 277 insertions(+), 1 deletion(-)
 create mode 100644 drivers/pci/pcie/pcie_ptm.c

-- 
2.7.3

[PATCH] PCI: PTM preliminary implementation

[Yong, Jonathan]

Simplified Precision Time Measurement driver, activates PTM feature
if a PCIe PTM requester (as per PCI Express 3.1 Base Specification
section 7.32)is found, but not before checking if the rest of the
PCI hierarchy can support it.

The driver does not take part in facilitating PTM conversations,
neither does it provide any useful services, it is only responsible
for setting up the required configuration space bits.

As of writing, there aren't any PTM capable devices on the market
yet, but it is supported by the Intel Apollo Lake platform.

Signed-off-by: Yong, Jonathan <jonathan.yong@intel.com>
---
 drivers/pci/pci-sysfs.c       |   7 ++
 drivers/pci/pci.h             |  25 +++++
 drivers/pci/pcie/Kconfig      |   8 ++
 drivers/pci/pcie/Makefile     |   2 +-
 drivers/pci/pcie/pcie_ptm.c   | 212 ++++++++++++++++++++++++++++++++++++++++++
 drivers/pci/probe.c           |   3 +
 include/linux/pci.h           |   9 ++
 include/uapi/linux/pci_regs.h |  12 +++
 8 files changed, 277 insertions(+), 1 deletion(-)
 create mode 100644 drivers/pci/pcie/pcie_ptm.c

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index e982010..edda4ba 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -1342,6 +1342,9 @@ static int pci_create_capabilities_sysfs(struct pci_dev *dev)
 	/* Active State Power Management */
 	pcie_aspm_create_sysfs_dev_files(dev);
 
+	/* PTM */
+	pci_create_ptm_sysfs(dev);
+
 	if (!pci_probe_reset_function(dev)) {
 		retval = device_create_file(&dev->dev, &reset_attr);
 		if (retval)
@@ -1436,6 +1439,10 @@ static void pci_remove_capabilities_sysfs(struct pci_dev *dev)
 	}
 
 	pcie_aspm_remove_sysfs_dev_files(dev);
+
+	/* PTM */
+	pci_release_ptm_sysfs(dev);
+
 	if (dev->reset_fn) {
 		device_remove_file(&dev->dev, &reset_attr);
 		dev->reset_fn = 0;
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index d0fb934..790024d 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -320,6 +320,31 @@ static inline resource_size_t pci_resource_alignment(struct pci_dev *dev,
 
 void pci_enable_acs(struct pci_dev *dev);
 
+#ifdef CONFIG_PCIEPORTBUS
+int pci_enable_ptm(struct pci_dev *dev);
+void pci_create_ptm_sysfs(struct pci_dev *dev);
+void pci_release_ptm_sysfs(struct pci_dev *dev);
+void pci_disable_ptm(struct pci_dev *dev);
+void pci_ptm_init(struct pci_dev *dev);
+#else
+static inline int pci_enable_ptm(struct pci_dev *dev)
+{
+	return -ENXIO;
+}
+static inline void pci_create_ptm_sysfs(struct pci_dev *dev)
+{
+}
+static inline void pci_release_ptm_sysfs(struct pci_dev *dev)
+{
+}
+static inline void pci_disable_ptm(struct pci_dev *dev)
+{
+}
+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 72db7f4..9348cc3 100644
--- a/drivers/pci/pcie/Kconfig
+++ b/drivers/pci/pcie/Kconfig
@@ -81,3 +81,11 @@ endchoice
 config PCIE_PME
 	def_bool y
 	depends on PCIEPORTBUS && PM
+
+config PCIE_PTM
+	bool "Turn on Precision Time Management by default"
+	depends on PCIEPORTBUS
+	help
+	  Say Y here to enable PTM feature on PCI Express devices that
+	  support them as they are found during device enumeration. Otherwise
+	  the feature can be enabled manually through sysfs entries.
diff --git a/drivers/pci/pcie/Makefile b/drivers/pci/pcie/Makefile
index 00c62df..d18b4c7 100644
--- a/drivers/pci/pcie/Makefile
+++ b/drivers/pci/pcie/Makefile
@@ -5,7 +5,7 @@
 # Build PCI Express ASPM if needed
 obj-$(CONFIG_PCIEASPM)		+= aspm.o
 
-pcieportdrv-y			:= portdrv_core.o portdrv_pci.o portdrv_bus.o
+pcieportdrv-y			:= portdrv_core.o portdrv_pci.o portdrv_bus.o pcie_ptm.o
 pcieportdrv-$(CONFIG_ACPI)	+= portdrv_acpi.o
 
 obj-$(CONFIG_PCIEPORTBUS)	+= pcieportdrv.o
diff --git a/drivers/pci/pcie/pcie_ptm.c b/drivers/pci/pcie/pcie_ptm.c
new file mode 100644
index 0000000..3476e65
--- /dev/null
+++ b/drivers/pci/pcie/pcie_ptm.c
@@ -0,0 +1,212 @@
+/*
+ * 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"
+
+int pci_enable_ptm(struct pci_dev *dev);
+void pci_release_ptm(struct pci_dev *dev);
+
+#ifdef CONFIG_PCIE_PTM
+static bool disable_ptm;
+#else
+static bool disable_ptm = 1;
+#endif
+
+module_param_named(disable_ptm, disable_ptm, bool, S_IRUGO | S_IWUSR);
+MODULE_PARM_DESC(disable_ptm, "Don't automatically enable PCIe PTM even if supported.");
+
+static int ptm_commit(struct pci_dev *dev)
+{
+	u32 dword;
+	int pos;
+
+	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_PTM);
+
+	/* Is this even possible? */
+	if (!pos)
+		return -ENXIO;
+
+	pci_read_config_dword(dev, pos + PCI_PTM_CONTROL_REG_OFFSET, &dword);
+	dword = dev->is_ptm_enabled ? dword | PCI_PTM_CTRL_ENABLE :
+		dword & ~PCI_PTM_CTRL_ENABLE;
+	dword = dev->is_ptm_root ? dword | PCI_PTM_CTRL_ROOT :
+		dword & ~PCI_PTM_CTRL_ROOT;
+
+	/* Only requester should have it set */
+	if (dev->is_ptm_requester)
+		dword = dword | (((u32)dev->ptm_effective_granularity) << 8);
+	return pci_write_config_dword(dev, pos + PCI_PTM_CONTROL_REG_OFFSET, dword);
+}
+
+/**
+ * pci_enable_ptm - Try to activate PTM functionality on device.
+ * @dev: PCI Express device with PTM requester role to enable.
+ *
+ * All PCIe Switches/Bridges in between need to be enabled for this to work.
+ *
+ * NOTE: Each requester must be associated with a PTM root (not to be confused
+ * with a root port or root complex). There can be multiple PTM roots in a
+ * a system forming multiple domains. All intervening bridges/switches in a
+ * domain must support PTM responder roles to relay PTM dialogues.
+ */
+int pci_enable_ptm(struct pci_dev *dev)
+{
+	int type;
+	struct pci_dev *upstream;
+
+	upstream = pci_upstream_bridge(dev);
+	type = pci_pcie_type(dev);
+
+	if (dev->is_ptm_root_capable)
+	{
+		/* If we are root capable but already part of a chain, don't set
+		 * the root select bit, only enable PTM */
+		if (!upstream || !upstream->is_ptm_enabled)
+			dev->is_ptm_root = 1;
+		dev->is_ptm_enabled = 1;
+	}
+
+	/* Is possible to be part of the PTM chain */
+	if (dev->is_ptm_responder && upstream && upstream->is_ptm_enabled)
+		dev->is_ptm_enabled = 1;
+
+	if (dev->is_ptm_requester && upstream && upstream->is_ptm_enabled) {
+		dev->is_ptm_enabled = 1;
+		dev->ptm_effective_granularity =
+			upstream->ptm_clock_granularity;
+	}
+	return ptm_commit(dev);
+}
+
+void pci_ptm_init(struct pci_dev *dev)
+{
+	u32 dword;
+	int pos;
+	u8 ver;
+
+	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_PTM);
+	if (!pos)
+		return;
+
+	/* Check capability version */
+	pci_read_config_dword(dev, pos, &dword);
+	ver = PCI_EXT_CAP_VER(dword);
+	if (ver != 0x1)
+	{
+		dev_warn(&dev->dev, "Expected PTM v1, got %u\n", ver);
+		return;
+	}
+
+	/* Fill in caps, masters are implied to be responders as well */
+	pci_read_config_dword(dev, pos + PCI_PTM_CAPABILITY_REG_OFFSET, &dword);
+	dev->is_ptm_capable = 1;
+	dev->is_ptm_root_capable   = (dword & PCI_PTM_CAP_ROOT) ? 1 : 0;
+	dev->is_ptm_responder      = (dword & PCI_PTM_CAP_RSP) ? 1 : 0;
+	dev->is_ptm_requester      = (dword & PCI_PTM_CAP_REQ) ? 1 : 0;
+	dev->ptm_clock_granularity = dev->is_ptm_responder ?
+		((dword & PCI_PTM_GRANULARITY_MASK) >> 8) : 0;
+	dev_info(&dev->dev, "Found PTM %s type device with %uns clock\n",
+		dev->is_ptm_root_capable ? "root" :
+		dev->is_ptm_responder ? "responder" :
+		dev->is_ptm_requester ? "requester" : "unknown",
+		dev->ptm_clock_granularity);
+
+	/* Get existing settings */
+	pci_read_config_dword(dev, pos + PCI_PTM_CONTROL_REG_OFFSET, &dword);
+	dev->is_ptm_enabled            = (dword & PCI_PTM_CTRL_ENABLE) ? 1 : 0;
+	dev->is_ptm_root               = (dword & PCI_PTM_CTRL_ROOT) ? 1 : 0;
+	dev->ptm_effective_granularity =
+		(dword & PCI_PTM_GRANULARITY_MASK) >> 8;
+
+	if(!disable_ptm)
+		pci_enable_ptm(dev);
+}
+
+static int do_disable_ptm(struct pci_dev *dev, void *v)
+{
+	if (dev->is_ptm_enabled)
+	{
+		dev->is_ptm_enabled            = 0;
+		dev->is_ptm_root               = 0;
+		dev->ptm_effective_granularity = 0;
+		ptm_commit(dev);
+	}
+	return 0;
+}
+
+/**
+ * pci_disable_ptm - Turn off PTM functionality on device.
+ * @dev: PCI Express device with PTM function to disable.
+ *
+ * Disables PTM functionality by clearing the PTM enable bit, if device is a
+ * switch/bridge it will also disable PTM function on other devices behind it.
+ */
+void pci_disable_ptm(struct pci_dev *dev)
+{
+	if (pci_is_bridge(dev))
+		pci_walk_bus(dev->bus, &do_disable_ptm, NULL);
+	else
+		do_disable_ptm(dev, NULL);
+}
+
+static ssize_t ptm_status_show(struct device *dev,
+	struct device_attribute *attr, char *buf)
+{
+	struct pci_dev *pdev = to_pci_dev(dev);
+	u16 word;
+	int pos;
+
+	pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PTM);
+	if (!pos)
+		return -ENXIO;
+
+	pci_read_config_word(pdev, pos + PCI_PTM_CONTROL_REG_OFFSET, &word);
+	return sprintf(buf, "%u\n", word & PCI_PTM_CTRL_ENABLE ? 1 : 0);
+}
+
+static ssize_t ptm_status_store(struct device *dev,
+	struct device_attribute *attr, const char *buf, size_t count)
+{
+	struct pci_dev *pdev = to_pci_dev(dev);
+	unsigned long val;
+	ssize_t ret;
+
+	ret = kstrtoul(buf, 0, &val);
+	if (ret)
+		return ret;
+	if (val)
+		return pci_enable_ptm(pdev);
+	pci_disable_ptm(pdev);
+	return 0;
+}
+
+static DEVICE_ATTR_RW(ptm_status);
+
+void pci_release_ptm_sysfs(struct pci_dev *dev)
+{
+	if (!pci_find_ext_capability(dev, PCI_EXT_CAP_ID_PTM))
+		return;
+	device_remove_file(&dev->dev, &dev_attr_ptm_status);
+}
+
+void pci_create_ptm_sysfs(struct pci_dev *dev)
+{
+	if (!pci_find_ext_capability(dev, PCI_EXT_CAP_ID_PTM))
+		return;
+	device_create_file(&dev->dev, &dev_attr_ptm_status);
+}
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 8004f67..9d5e96e6 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1657,6 +1657,9 @@ static void pci_init_capabilities(struct pci_dev *dev)
 	pci_enable_acs(dev);
 
 	pci_cleanup_aer_error_status_regs(dev);
+
+	/* Enable PTM Capabilities */
+	pci_ptm_init(dev);
 }
 
 /*
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 004b813..2facd44 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -363,6 +363,15 @@ 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 */
+
+	unsigned int 	is_ptm_capable:1;
+	unsigned int	is_ptm_root_capable:1;
+	unsigned int	is_ptm_responder:1;
+	unsigned int	is_ptm_requester:1;
+	unsigned int	is_ptm_enabled:1;
+	unsigned int	is_ptm_root:1;
+	u8		ptm_clock_granularity;
+	u8		ptm_effective_granularity;
 #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 1becea8..559b28f 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -671,6 +671,7 @@
 #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_MAX	PCI_EXT_CAP_ID_PASID
+#define PCI_EXT_CAP_ID_PTM	0x1f	/* Precision Time Measurement */
 
 #define PCI_EXT_CAP_DSN_SIZEOF	12
 #define PCI_EXT_CAP_MCAST_ENDPOINT_SIZEOF 40
@@ -946,4 +947,15 @@
 #define PCI_TPH_CAP_ST_SHIFT	16	/* st table shift */
 #define PCI_TPH_BASE_SIZEOF	12	/* size with no st table */
 
+/* Precision Time Measurement */
+#define PCI_PTM_CAP_REQ			0x0001  /* Requester capable */
+#define PCI_PTM_CAP_RSP			0x0002  /* Responder capable */
+#define PCI_PTM_CAP_ROOT		0x0004  /* Root capable */
+#define PCI_PTM_GRANULARITY_MASK	0xFF00  /* Local clock granularity */
+#define PCI_PTM_CTRL_ENABLE		0x0001  /* PTM enable */
+#define PCI_PTM_CTRL_ROOT		0x0002  /* Root select */
+#define PCI_PTM_HEADER_REG_OFFSET       0x00	/* PTM version and such */
+#define PCI_PTM_CAPABILITY_REG_OFFSET   0x04	/* Capabilities */
+#define PCI_PTM_CONTROL_REG_OFFSET      0x08	/* Control reg */
+
 #endif /* LINUX_PCI_REGS_H */
-- 
2.7.3

Re: [PATCH] PCI: PTM preliminary implementation

[Bjorn Helgaas]

Hi Jonathan,

Since you haven't been able to test this with hardware, and you say
it's hacked together and in need of refactoring, I'm just going to
give you some quick comments.  Most of my comments are pretty
superficial because they're obvious things that checkpatch or even a
glance at the surrounding code should have caught.

I assume you'll be able to test this with some early hardware before
you want this actually merged.

On Wed, Mar 23, 2016 at 04:04:08AM +0000, Yong, Jonathan wrote:
> Simplified Precision Time Measurement driver, activates PTM feature
> if a PCIe PTM requester (as per PCI Express 3.1 Base Specification
> section 7.32)is found, but not before checking if the rest of the
> PCI hierarchy can support it.
> 
> The driver does not take part in facilitating PTM conversations,
> neither does it provide any useful services, it is only responsible
> for setting up the required configuration space bits.
> 
> As of writing, there aren't any PTM capable devices on the market
> yet, but it is supported by the Intel Apollo Lake platform.
> 
> Signed-off-by: Yong, Jonathan <jonathan.yong@intel.com>
> ---
>  drivers/pci/pci-sysfs.c       |   7 ++
>  drivers/pci/pci.h             |  25 +++++
>  drivers/pci/pcie/Kconfig      |   8 ++
>  drivers/pci/pcie/Makefile     |   2 +-
>  drivers/pci/pcie/pcie_ptm.c   | 212 ++++++++++++++++++++++++++++++++++++++++++
>  drivers/pci/probe.c           |   3 +
>  include/linux/pci.h           |   9 ++
>  include/uapi/linux/pci_regs.h |  12 +++
>  8 files changed, 277 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/pci/pcie/pcie_ptm.c
> 
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index e982010..edda4ba 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -1342,6 +1342,9 @@ static int pci_create_capabilities_sysfs(struct pci_dev *dev)
>  	/* Active State Power Management */
>  	pcie_aspm_create_sysfs_dev_files(dev);
>  
> +	/* PTM */
> +	pci_create_ptm_sysfs(dev);

Follow naming convention by naming this "pcie_ptm_create_sysfs_dev_files()".

The "/* PTM */" comment is useless since the function name already
includes that.  Either remove it or make it more useful by spelling
out "Precision Time Measurement".

You need to cleanup the sysfs files when
pci_create_capabilities_sysfs() returns failure, just like we do for
pcie_aspm_create_sysfs_dev_files().

> +
>  	if (!pci_probe_reset_function(dev)) {
>  		retval = device_create_file(&dev->dev, &reset_attr);
>  		if (retval)
> @@ -1436,6 +1439,10 @@ static void pci_remove_capabilities_sysfs(struct pci_dev *dev)
>  	}
>  
>  	pcie_aspm_remove_sysfs_dev_files(dev);
> +
> +	/* PTM */
> +	pci_release_ptm_sysfs(dev);

Superfluous comment.

> +
>  	if (dev->reset_fn) {
>  		device_remove_file(&dev->dev, &reset_attr);
>  		dev->reset_fn = 0;
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index d0fb934..790024d 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -320,6 +320,31 @@ static inline resource_size_t pci_resource_alignment(struct pci_dev *dev,
>  
>  void pci_enable_acs(struct pci_dev *dev);
>  
> +#ifdef CONFIG_PCIEPORTBUS

Wrong config symbol in #ifdef.

> +int pci_enable_ptm(struct pci_dev *dev);
> +void pci_create_ptm_sysfs(struct pci_dev *dev);
> +void pci_release_ptm_sysfs(struct pci_dev *dev);
> +void pci_disable_ptm(struct pci_dev *dev);
> +void pci_ptm_init(struct pci_dev *dev);
> +#else
> +static inline int pci_enable_ptm(struct pci_dev *dev)
> +{
> +	return -ENXIO;
> +}
> +static inline void pci_create_ptm_sysfs(struct pci_dev *dev)
> +{
> +}
> +static inline void pci_release_ptm_sysfs(struct pci_dev *dev)
> +{
> +}
> +static inline void pci_disable_ptm(struct pci_dev *dev)
> +{
> +}
> +static inline void pci_ptm_init(struct pci_dev *dev)
> +{
> +}

Follow coding style (each of these stubs would fit on a single line as
is done elsewhere in this file).

> +#endif
> +
>  struct pci_dev_reset_methods {
>  	u16 vendor;
>  	u16 device;
> diff --git a/drivers/pci/pcie/Kconfig b/drivers/pci/pcie/Kconfig
> index 72db7f4..9348cc3 100644
> --- a/drivers/pci/pcie/Kconfig
> +++ b/drivers/pci/pcie/Kconfig
> @@ -81,3 +81,11 @@ endchoice
>  config PCIE_PME
>  	def_bool y
>  	depends on PCIEPORTBUS && PM
> +
> +config PCIE_PTM
> +	bool "Turn on Precision Time Management by default"
> +	depends on PCIEPORTBUS
> +	help
> +	  Say Y here to enable PTM feature on PCI Express devices that

Spell out PTM here.

> +	  support them as they are found during device enumeration. Otherwise
> +	  the feature can be enabled manually through sysfs entries.
> diff --git a/drivers/pci/pcie/Makefile b/drivers/pci/pcie/Makefile
> index 00c62df..d18b4c7 100644
> --- a/drivers/pci/pcie/Makefile
> +++ b/drivers/pci/pcie/Makefile
> @@ -5,7 +5,7 @@
>  # Build PCI Express ASPM if needed
>  obj-$(CONFIG_PCIEASPM)		+= aspm.o
>  
> -pcieportdrv-y			:= portdrv_core.o portdrv_pci.o portdrv_bus.o
> +pcieportdrv-y			:= portdrv_core.o portdrv_pci.o portdrv_bus.o pcie_ptm.o

Should use CONFIG_PCIE_PTM here as is done for CONFIG_PCIE_PME.

>  pcieportdrv-$(CONFIG_ACPI)	+= portdrv_acpi.o
>  
>  obj-$(CONFIG_PCIEPORTBUS)	+= pcieportdrv.o
> diff --git a/drivers/pci/pcie/pcie_ptm.c b/drivers/pci/pcie/pcie_ptm.c
> new file mode 100644
> index 0000000..3476e65
> --- /dev/null
> +++ b/drivers/pci/pcie/pcie_ptm.c
> @@ -0,0 +1,212 @@
> +/*
> + * 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.
> + *

Extra blank line in comment.

> + */
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/pci.h>
> +#include "../pci.h"
> +
> +int pci_enable_ptm(struct pci_dev *dev);

Useless forward declaration.

> +void pci_release_ptm(struct pci_dev *dev);

Also useless, because pci_release_ptm() doesn't exist at all.

> +
> +#ifdef CONFIG_PCIE_PTM
> +static bool disable_ptm;
> +#else
> +static bool disable_ptm = 1;
> +#endif
> +
> +module_param_named(disable_ptm, disable_ptm, bool, S_IRUGO | S_IWUSR);
> +MODULE_PARM_DESC(disable_ptm, "Don't automatically enable PCIe PTM even if supported.");
> +
> +static int ptm_commit(struct pci_dev *dev)
> +{
> +	u32 dword;
> +	int pos;
> +
> +	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_PTM);
> +
> +	/* Is this even possible? */
> +	if (!pos)
> +		return -ENXIO;
> +
> +	pci_read_config_dword(dev, pos + PCI_PTM_CONTROL_REG_OFFSET, &dword);
> +	dword = dev->is_ptm_enabled ? dword | PCI_PTM_CTRL_ENABLE :
> +		dword & ~PCI_PTM_CTRL_ENABLE;
> +	dword = dev->is_ptm_root ? dword | PCI_PTM_CTRL_ROOT :
> +		dword & ~PCI_PTM_CTRL_ROOT;
> +
> +	/* Only requester should have it set */
> +	if (dev->is_ptm_requester)
> +		dword = dword | (((u32)dev->ptm_effective_granularity) << 8);
> +	return pci_write_config_dword(dev, pos + PCI_PTM_CONTROL_REG_OFFSET, dword);
> +}
> +
> +/**
> + * pci_enable_ptm - Try to activate PTM functionality on device.
> + * @dev: PCI Express device with PTM requester role to enable.
> + *
> + * All PCIe Switches/Bridges in between need to be enabled for this to work.
> + *
> + * NOTE: Each requester must be associated with a PTM root (not to be confused
> + * with a root port or root complex). There can be multiple PTM roots in a
> + * a system forming multiple domains. All intervening bridges/switches in a
> + * domain must support PTM responder roles to relay PTM dialogues.
> + */
> +int pci_enable_ptm(struct pci_dev *dev)
> +{
> +	int type;
> +	struct pci_dev *upstream;
> +
> +	upstream = pci_upstream_bridge(dev);
> +	type = pci_pcie_type(dev);
> +
> +	if (dev->is_ptm_root_capable)
> +	{

Follow coding style (open brace goes on same line as "if").  Several
occurrences of this.

> +		/* If we are root capable but already part of a chain, don't set
> +		 * the root select bit, only enable PTM */

Follow comment style ("/*" and "*/ on line by themselves; see other
drivers/pci code.

> +		if (!upstream || !upstream->is_ptm_enabled)
> +			dev->is_ptm_root = 1;
> +		dev->is_ptm_enabled = 1;
> +	}
> +
> +	/* Is possible to be part of the PTM chain */
> +	if (dev->is_ptm_responder && upstream && upstream->is_ptm_enabled)
> +		dev->is_ptm_enabled = 1;
> +
> +	if (dev->is_ptm_requester && upstream && upstream->is_ptm_enabled) {
> +		dev->is_ptm_enabled = 1;
> +		dev->ptm_effective_granularity =
> +			upstream->ptm_clock_granularity;
> +	}
> +	return ptm_commit(dev);
> +}
> +
> +void pci_ptm_init(struct pci_dev *dev)
> +{
> +	u32 dword;
> +	int pos;
> +	u8 ver;
> +
> +	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_PTM);
> +	if (!pos)
> +		return;
> +
> +	/* Check capability version */
> +	pci_read_config_dword(dev, pos, &dword);
> +	ver = PCI_EXT_CAP_VER(dword);
> +	if (ver != 0x1)
> +	{
> +		dev_warn(&dev->dev, "Expected PTM v1, got %u\n", ver);
> +		return;
> +	}

Don't check version.  We assume future versions will be backwards
compatible.  If a future v2 adds features, we'll have to check the
version then before using those features.

> +
> +	/* Fill in caps, masters are implied to be responders as well */
> +	pci_read_config_dword(dev, pos + PCI_PTM_CAPABILITY_REG_OFFSET, &dword);
> +	dev->is_ptm_capable = 1;
> +	dev->is_ptm_root_capable   = (dword & PCI_PTM_CAP_ROOT) ? 1 : 0;
> +	dev->is_ptm_responder      = (dword & PCI_PTM_CAP_RSP) ? 1 : 0;
> +	dev->is_ptm_requester      = (dword & PCI_PTM_CAP_REQ) ? 1 : 0;
> +	dev->ptm_clock_granularity = dev->is_ptm_responder ?
> +		((dword & PCI_PTM_GRANULARITY_MASK) >> 8) : 0;
> +	dev_info(&dev->dev, "Found PTM %s type device with %uns clock\n",
> +		dev->is_ptm_root_capable ? "root" :
> +		dev->is_ptm_responder ? "responder" :
> +		dev->is_ptm_requester ? "requester" : "unknown",
> +		dev->ptm_clock_granularity);
> +
> +	/* Get existing settings */
> +	pci_read_config_dword(dev, pos + PCI_PTM_CONTROL_REG_OFFSET, &dword);
> +	dev->is_ptm_enabled            = (dword & PCI_PTM_CTRL_ENABLE) ? 1 : 0;
> +	dev->is_ptm_root               = (dword & PCI_PTM_CTRL_ROOT) ? 1 : 0;
> +	dev->ptm_effective_granularity =
> +		(dword & PCI_PTM_GRANULARITY_MASK) >> 8;
> +
> +	if(!disable_ptm)

Follow coding style (space required after "if").

> +		pci_enable_ptm(dev);
> +}
> +
> +static int do_disable_ptm(struct pci_dev *dev, void *v)
> +{
> +	if (dev->is_ptm_enabled)
> +	{
> +		dev->is_ptm_enabled            = 0;
> +		dev->is_ptm_root               = 0;
> +		dev->ptm_effective_granularity = 0;
> +		ptm_commit(dev);
> +	}
> +	return 0;
> +}
> +
> +/**
> + * pci_disable_ptm - Turn off PTM functionality on device.
> + * @dev: PCI Express device with PTM function to disable.
> + *
> + * Disables PTM functionality by clearing the PTM enable bit, if device is a
> + * switch/bridge it will also disable PTM function on other devices behind it.
> + */
> +void pci_disable_ptm(struct pci_dev *dev)
> +{
> +	if (pci_is_bridge(dev))
> +		pci_walk_bus(dev->bus, &do_disable_ptm, NULL);
> +	else
> +		do_disable_ptm(dev, NULL);
> +}
> +
> +static ssize_t ptm_status_show(struct device *dev,
> +	struct device_attribute *attr, char *buf)
> +{
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +	u16 word;
> +	int pos;
> +
> +	pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PTM);
> +	if (!pos)
> +		return -ENXIO;
> +
> +	pci_read_config_word(pdev, pos + PCI_PTM_CONTROL_REG_OFFSET, &word);
> +	return sprintf(buf, "%u\n", word & PCI_PTM_CTRL_ENABLE ? 1 : 0);
> +}
> +
> +static ssize_t ptm_status_store(struct device *dev,
> +	struct device_attribute *attr, const char *buf, size_t count)
> +{
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +	unsigned long val;
> +	ssize_t ret;
> +
> +	ret = kstrtoul(buf, 0, &val);
> +	if (ret)
> +		return ret;
> +	if (val)
> +		return pci_enable_ptm(pdev);
> +	pci_disable_ptm(pdev);

Looks wrong to conditionally enable PTM, then always disable it.

> +	return 0;
> +}
> +
> +static DEVICE_ATTR_RW(ptm_status);
> +
> +void pci_release_ptm_sysfs(struct pci_dev *dev)
> +{
> +	if (!pci_find_ext_capability(dev, PCI_EXT_CAP_ID_PTM))
> +		return;
> +	device_remove_file(&dev->dev, &dev_attr_ptm_status);
> +}
> +
> +void pci_create_ptm_sysfs(struct pci_dev *dev)
> +{
> +	if (!pci_find_ext_capability(dev, PCI_EXT_CAP_ID_PTM))
> +		return;
> +	device_create_file(&dev->dev, &dev_attr_ptm_status);
> +}
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 8004f67..9d5e96e6 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1657,6 +1657,9 @@ static void pci_init_capabilities(struct pci_dev *dev)
>  	pci_enable_acs(dev);
>  
>  	pci_cleanup_aer_error_status_regs(dev);
> +
> +	/* Enable PTM Capabilities */
> +	pci_ptm_init(dev);
>  }
>  
>  /*
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 004b813..2facd44 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -363,6 +363,15 @@ 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 */
> +
> +	unsigned int 	is_ptm_capable:1;
> +	unsigned int	is_ptm_root_capable:1;
> +	unsigned int	is_ptm_responder:1;
> +	unsigned int	is_ptm_requester:1;
> +	unsigned int	is_ptm_enabled:1;
> +	unsigned int	is_ptm_root:1;
> +	u8		ptm_clock_granularity;
> +	u8		ptm_effective_granularity;

Wrap with CONFIG_PCIE_PTM, like the example on the next line.

>  #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 1becea8..559b28f 100644
> --- a/include/uapi/linux/pci_regs.h
> +++ b/include/uapi/linux/pci_regs.h
> @@ -671,6 +671,7 @@
>  #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_MAX	PCI_EXT_CAP_ID_PASID
> +#define PCI_EXT_CAP_ID_PTM	0x1f	/* Precision Time Measurement */

Adjust PCI_EXT_CAP_ID_MAX.

>  #define PCI_EXT_CAP_DSN_SIZEOF	12
>  #define PCI_EXT_CAP_MCAST_ENDPOINT_SIZEOF 40
> @@ -946,4 +947,15 @@
>  #define PCI_TPH_CAP_ST_SHIFT	16	/* st table shift */
>  #define PCI_TPH_BASE_SIZEOF	12	/* size with no st table */
>  
> +/* Precision Time Measurement */
> +#define PCI_PTM_CAP_REQ			0x0001  /* Requester capable */
> +#define PCI_PTM_CAP_RSP			0x0002  /* Responder capable */
> +#define PCI_PTM_CAP_ROOT		0x0004  /* Root capable */
> +#define PCI_PTM_GRANULARITY_MASK	0xFF00  /* Local clock granularity */
> +#define PCI_PTM_CTRL_ENABLE		0x0001  /* PTM enable */
> +#define PCI_PTM_CTRL_ROOT		0x0002  /* Root select */
> +#define PCI_PTM_HEADER_REG_OFFSET       0x00	/* PTM version and such */
> +#define PCI_PTM_CAPABILITY_REG_OFFSET   0x04	/* Capabilities */
> +#define PCI_PTM_CONTROL_REG_OFFSET      0x08	/* Control reg */
> +
>  #endif /* LINUX_PCI_REGS_H */
> -- 
> 2.7.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] PCI: PTM preliminary implementation

[Bjorn Helgaas]

On Mon, Apr 11, 2016 at 11:23:31PM -0500, Bjorn Helgaas wrote:

> > --- a/include/uapi/linux/pci_regs.h
> > +++ b/include/uapi/linux/pci_regs.h
> > @@ -671,6 +671,7 @@
> >  #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_MAX	PCI_EXT_CAP_ID_PASID
> > +#define PCI_EXT_CAP_ID_PTM	0x1f	/* Precision Time Measurement */
> 
> Adjust PCI_EXT_CAP_ID_MAX.

And follow hex capitalization style of existing code.

[RFC v4] PCI: PTM Driver

[Yong, Jonathan]

Hello LKML,

This is a preliminary implementation of the PTM[1] support driver. This driver
has only been tested against a virtual PCI bus since there are no known
endpoints utilizing it yet.

The drivers job is to get to every PTM capable device, set some PCI config
space bits, then go back to sleep [2].

PTM capable PCIe devices will get a new sysfs entry to allow PTM to be
enabled/disabled individually (enable by default).

Comments? Should I explain the PTM registers in more details?
Please CC me, thanks.

[1] Precision Time Measurement: A protocol for synchronizing PCIe endpoint
clocks against the host clock as specified in the PCI Express Base
Specification 3.1. It is identified by the 0x001f extended capability ID.

PTM capable devices are split into 3 roles, master, responder and requester.
Summary as follows:

A master holds the master clock that will be used for all devices under its
domain (not to be confused with PCI domains). There may be multiple masters
in a PTM hierarchy, in which case, the highest master closest to the root
complex will be selected for the PTM domain. A master is also always
responder capable. Clock precision is signified by a Local Clock
Granularity field, in nano-seconds.

A responder responds to any PTM synchronization requests from a downstream
device. A responder is typically a switch device. It may also hold a local
clock signified by a non-zero Local Clock Granularity field. A value of 0
signifies that the device simply propagates timing information from
upstream devices.

A requester is typically an endpoint that will request synchronization
updates from an upstream PTM capable time source. The driver will update
the Effective Clock Granularity field based on the same field from the
PTM domain master. The field should be programmed with a value of 0 if any
intervening responder has a Local Clock Granularity field value of 0.

[2] The software drivers never see the PTM packets, the PCI Express Base
Specification 3.1 reads:
        PTM capable components can make their PTM context available for
        inspection by software, enabling software to translate timing
        information between local times and PTM Master Time.

This isn't very informative.

Changes since v1:
* Moved register constants to pci_regs.h
* Use pci_dev to hold PTM status
* PTM initialization now done top-down hierarchy wise.

Changes since v2:
* Added missing void return in the pci_ptm_init inline stub.

Changes since v3:
* Clearing CONFIG_PCIE_PTM now completely prevents the driver from being built.
* Renamed pci_*_ptm_sysfs to pcie_ptm_*_sysfs_dev_files for consistency.
* Removed useless prototypes.
* Driver no longer checks PTM capability version
* PCI_EXT_CAP_ID_MAX updated to include PTM (0x1F)

Yong, Jonathan (1):
  PCI: PTM preliminary implementation

 drivers/pci/pci-sysfs.c       |   7 ++
 drivers/pci/pci.h             |  20 +++++
 drivers/pci/pcie/Kconfig      |   9 ++
 drivers/pci/pcie/Makefile     |   3 +
 drivers/pci/pcie/pcie_ptm.c   | 194 ++++++++++++++++++++++++++++++++++++++++++
 drivers/pci/probe.c           |   3 +
 include/linux/pci.h           |  11 +++
 include/uapi/linux/pci_regs.h |  14 ++-
 8 files changed, 260 insertions(+), 1 deletion(-)
 create mode 100644 drivers/pci/pcie/pcie_ptm.c

-- 
2.7.3

[PATCH] PCI: PTM preliminary implementation

[Yong, Jonathan]

Simplified Precision Time Measurement driver, activates PTM feature
if a PCIe PTM requester (as per PCI Express 3.1 Base Specification
section 7.32)is found, but not before checking if the rest of the
PCI hierarchy can support it.

The driver does not take part in facilitating PTM conversations,
neither does it provide any useful services, it is only responsible
for setting up the required configuration space bits.

As of writing, there aren't any PTM capable devices on the market
yet, but it is supported by the Intel Apollo Lake platform.

Signed-off-by: Yong, Jonathan <jonathan.yong@intel.com>
---
 drivers/pci/pci-sysfs.c       |   7 ++
 drivers/pci/pci.h             |  20 +++++
 drivers/pci/pcie/Kconfig      |   9 ++
 drivers/pci/pcie/Makefile     |   3 +
 drivers/pci/pcie/pcie_ptm.c   | 194 ++++++++++++++++++++++++++++++++++++++++++
 drivers/pci/probe.c           |   3 +
 include/linux/pci.h           |  11 +++
 include/uapi/linux/pci_regs.h |  14 ++-
 8 files changed, 260 insertions(+), 1 deletion(-)
 create mode 100644 drivers/pci/pcie/pcie_ptm.c

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index e982010..11cf97b 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -1342,6 +1342,9 @@ static int pci_create_capabilities_sysfs(struct pci_dev *dev)
 	/* Active State Power Management */
 	pcie_aspm_create_sysfs_dev_files(dev);
 
+	/* Precision Time Measurement */
+	pcie_ptm_create_sysfs_dev_files(dev);
+
 	if (!pci_probe_reset_function(dev)) {
 		retval = device_create_file(&dev->dev, &reset_attr);
 		if (retval)
@@ -1351,6 +1354,7 @@ static int pci_create_capabilities_sysfs(struct pci_dev *dev)
 	return 0;
 
 error:
+	pcie_ptm_remove_sysfs_dev_files(dev);
 	pcie_aspm_remove_sysfs_dev_files(dev);
 	if (dev->vpd && dev->vpd->attr) {
 		sysfs_remove_bin_file(&dev->dev.kobj, dev->vpd->attr);
@@ -1436,6 +1440,9 @@ static void pci_remove_capabilities_sysfs(struct pci_dev *dev)
 	}
 
 	pcie_aspm_remove_sysfs_dev_files(dev);
+
+	pcie_ptm_remove_sysfs_dev_files(dev);
+
 	if (dev->reset_fn) {
 		device_remove_file(&dev->dev, &reset_attr);
 		dev->reset_fn = 0;
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index d0fb934..908445b 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -320,6 +320,26 @@ static inline resource_size_t pci_resource_alignment(struct pci_dev *dev,
 
 void pci_enable_acs(struct pci_dev *dev);
 
+#ifdef CONFIG_PCIE_PTM
+int pci_enable_ptm(struct pci_dev *dev);
+int pcie_ptm_create_sysfs_dev_files(struct pci_dev *dev);
+void pcie_ptm_remove_sysfs_dev_files(struct pci_dev *dev);
+void pci_disable_ptm(struct pci_dev *dev);
+void pci_ptm_init(struct pci_dev *dev);
+#else
+static inline int pci_enable_ptm(struct pci_dev *dev)
+{
+	return -ENXIO;
+}
+static inline int pcie_ptm_create_sysfs_dev_files(struct pci_dev *dev)
+{
+	return -ENXIO;
+}
+static inline void pcie_ptm_remove_sysfs_dev_files(struct pci_dev *dev) {}
+static inline void pci_disable_ptm(struct pci_dev *dev) {}
+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 72db7f4..2baddc5 100644
--- a/drivers/pci/pcie/Kconfig
+++ b/drivers/pci/pcie/Kconfig
@@ -81,3 +81,12 @@ endchoice
 config PCIE_PME
 	def_bool y
 	depends on PCIEPORTBUS && PM
+
+config PCIE_PTM
+	bool "Enable Precision Time Measurement support"
+	default y
+	depends on PCIEPORTBUS
+	help
+	  Say Y here if you have PCI Express devices that are capable of
+	  Precision Time Measurement (PTM). This also requires that your
+	  PCI Express controller or switch fabric is PTM capable.
diff --git a/drivers/pci/pcie/Makefile b/drivers/pci/pcie/Makefile
index 00c62df..726b972 100644
--- a/drivers/pci/pcie/Makefile
+++ b/drivers/pci/pcie/Makefile
@@ -14,3 +14,6 @@ obj-$(CONFIG_PCIEPORTBUS)	+= pcieportdrv.o
 obj-$(CONFIG_PCIEAER)		+= aer/
 
 obj-$(CONFIG_PCIE_PME) += pme.o
+
+# Precision Time Measurement support
+obj-$(CONFIG_PCIE_PTM) += pcie_ptm.o
diff --git a/drivers/pci/pcie/pcie_ptm.c b/drivers/pci/pcie/pcie_ptm.c
new file mode 100644
index 0000000..f4e4d61
--- /dev/null
+++ b/drivers/pci/pcie/pcie_ptm.c
@@ -0,0 +1,194 @@
+/*
+ * 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 bool disable_ptm;
+
+module_param_named(disable_ptm, disable_ptm, bool, S_IRUGO | S_IWUSR);
+MODULE_PARM_DESC(disable_ptm, "Don't automatically enable PCIe PTM even if supported.");
+
+static int ptm_commit(struct pci_dev *dev)
+{
+	u32 dword;
+	int pos;
+
+	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_PTM);
+
+	/* Is this even possible? */
+	if (!pos)
+		return -ENXIO;
+
+	pci_read_config_dword(dev, pos + PCI_PTM_CONTROL_REG_OFFSET, &dword);
+	dword = dev->is_ptm_enabled ? dword | PCI_PTM_CTRL_ENABLE :
+		dword & ~PCI_PTM_CTRL_ENABLE;
+	dword = dev->is_ptm_root ? dword | PCI_PTM_CTRL_ROOT :
+		dword & ~PCI_PTM_CTRL_ROOT;
+
+	/* Only requester should have it set */
+	if (dev->is_ptm_requester)
+		dword = dword | (((u32)dev->ptm_effective_granularity) << 8);
+	return pci_write_config_dword(dev, pos + PCI_PTM_CONTROL_REG_OFFSET,
+		dword);
+}
+
+/**
+ * pci_enable_ptm - Try to activate PTM functionality on device.
+ * @dev: PCI Express device with PTM requester role to enable.
+ *
+ * All PCIe Switches/Bridges in between need to be enabled for this to work.
+ *
+ * NOTE: Each requester must be associated with a PTM root (not to be confused
+ * with a root port or root complex). There can be multiple PTM roots in a
+ * a system forming multiple domains. All intervening bridges/switches in a
+ * domain must support PTM responder roles to relay PTM dialogues.
+ */
+int pci_enable_ptm(struct pci_dev *dev)
+{
+	int type;
+	struct pci_dev *upstream;
+
+	upstream = pci_upstream_bridge(dev);
+	type = pci_pcie_type(dev);
+
+	if (dev->is_ptm_root_capable) {
+		/* If we are root capable but already part of a chain, don't set
+		 * the root select bit, only enable PTM
+		 */
+		if (!upstream || !upstream->is_ptm_enabled)
+			dev->is_ptm_root = 1;
+		dev->is_ptm_enabled = 1;
+	}
+
+	/* Is possible to be part of the PTM chain */
+	if (dev->is_ptm_responder && upstream && upstream->is_ptm_enabled)
+		dev->is_ptm_enabled = 1;
+
+	if (dev->is_ptm_requester && upstream && upstream->is_ptm_enabled) {
+		dev->is_ptm_enabled = 1;
+		dev->ptm_effective_granularity =
+			upstream->ptm_clock_granularity;
+	}
+	return ptm_commit(dev);
+}
+
+void pci_ptm_init(struct pci_dev *dev)
+{
+	u32 dword;
+	int pos;
+
+	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_PTM);
+	if (!pos)
+		return;
+
+	/* Fill in caps, masters are implied to be responders as well */
+	pci_read_config_dword(dev, pos + PCI_PTM_CAPABILITY_REG_OFFSET, &dword);
+	dev->is_ptm_capable = 1;
+	dev->is_ptm_root_capable   = (dword & PCI_PTM_CAP_ROOT) ? 1 : 0;
+	dev->is_ptm_responder      = (dword & PCI_PTM_CAP_RSP) ? 1 : 0;
+	dev->is_ptm_requester      = (dword & PCI_PTM_CAP_REQ) ? 1 : 0;
+	dev->ptm_clock_granularity = dev->is_ptm_responder ?
+		((dword & PCI_PTM_GRANULARITY_MASK) >> 8) : 0;
+	dev_info(&dev->dev, "Found PTM %s type device with %uns clock\n",
+		dev->is_ptm_root_capable ? "root" :
+		dev->is_ptm_responder ? "responder" :
+		dev->is_ptm_requester ? "requester" : "unknown",
+		dev->ptm_clock_granularity);
+
+	/* Get existing settings */
+	pci_read_config_dword(dev, pos + PCI_PTM_CONTROL_REG_OFFSET, &dword);
+	dev->is_ptm_enabled            = (dword & PCI_PTM_CTRL_ENABLE) ? 1 : 0;
+	dev->is_ptm_root               = (dword & PCI_PTM_CTRL_ROOT) ? 1 : 0;
+	dev->ptm_effective_granularity =
+		(dword & PCI_PTM_GRANULARITY_MASK) >> 8;
+
+	if (!disable_ptm)
+		pci_enable_ptm(dev);
+}
+
+static int do_disable_ptm(struct pci_dev *dev, void *v)
+{
+	if (dev->is_ptm_enabled) {
+		dev->is_ptm_enabled            = 0;
+		dev->is_ptm_root               = 0;
+		dev->ptm_effective_granularity = 0;
+		ptm_commit(dev);
+	}
+	return 0;
+}
+
+/**
+ * pci_disable_ptm - Turn off PTM functionality on device.
+ * @dev: PCI Express device with PTM function to disable.
+ *
+ * Disables PTM functionality by clearing the PTM enable bit, if device is a
+ * switch/bridge it will also disable PTM function on other devices behind it.
+ */
+void pci_disable_ptm(struct pci_dev *dev)
+{
+	if (pci_is_bridge(dev))
+		pci_walk_bus(dev->bus, &do_disable_ptm, NULL);
+	else
+		do_disable_ptm(dev, NULL);
+}
+
+static ssize_t ptm_status_show(struct device *dev,
+	struct device_attribute *attr, char *buf)
+{
+	struct pci_dev *pdev = to_pci_dev(dev);
+	u16 word;
+	int pos;
+
+	pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PTM);
+	if (!pos)
+		return -ENXIO;
+
+	pci_read_config_word(pdev, pos + PCI_PTM_CONTROL_REG_OFFSET, &word);
+	return sprintf(buf, "%u\n", word & PCI_PTM_CTRL_ENABLE ? 1 : 0);
+}
+
+static ssize_t ptm_status_store(struct device *dev,
+	struct device_attribute *attr, const char *buf, size_t count)
+{
+	struct pci_dev *pdev = to_pci_dev(dev);
+	unsigned long val;
+	ssize_t ret;
+
+	ret = kstrtoul(buf, 0, &val);
+	if (ret)
+		return ret;
+	if (val)
+		return pci_enable_ptm(pdev);
+	pci_disable_ptm(pdev);
+	return 0;
+}
+
+static DEVICE_ATTR_RW(ptm_status);
+
+void pcie_ptm_remove_sysfs_dev_files(struct pci_dev *dev)
+{
+	if (!pci_find_ext_capability(dev, PCI_EXT_CAP_ID_PTM))
+		return;
+	device_remove_file(&dev->dev, &dev_attr_ptm_status);
+}
+
+int pcie_ptm_create_sysfs_dev_files(struct pci_dev *dev)
+{
+	if (!pci_find_ext_capability(dev, PCI_EXT_CAP_ID_PTM))
+		return -ENXIO;
+	return device_create_file(&dev->dev, &dev_attr_ptm_status);
+}
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 8004f67..9d5e96e6 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1657,6 +1657,9 @@ static void pci_init_capabilities(struct pci_dev *dev)
 	pci_enable_acs(dev);
 
 	pci_cleanup_aer_error_status_regs(dev);
+
+	/* Enable PTM Capabilities */
+	pci_ptm_init(dev);
 }
 
 /*
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 004b813..ba5dab4 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -363,6 +363,17 @@ 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	is_ptm_capable:1;
+	unsigned int	is_ptm_root_capable:1;
+	unsigned int	is_ptm_responder:1;
+	unsigned int	is_ptm_requester:1;
+	unsigned int	is_ptm_enabled:1;
+	unsigned int	is_ptm_root:1;
+	u8		ptm_clock_granularity;
+	u8		ptm_effective_granularity;
+#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 1becea8..9dd77be 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -670,7 +670,8 @@
 #define PCI_EXT_CAP_ID_SECPCI	0x19	/* Secondary PCIe Capability */
 #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_MAX	PCI_EXT_CAP_ID_PASID
+#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
@@ -946,4 +947,15 @@
 #define PCI_TPH_CAP_ST_SHIFT	16	/* st table shift */
 #define PCI_TPH_BASE_SIZEOF	12	/* size with no st table */
 
+/* Precision Time Measurement */
+#define PCI_PTM_CAP_REQ			0x0001  /* Requester capable */
+#define PCI_PTM_CAP_RSP			0x0002  /* Responder capable */
+#define PCI_PTM_CAP_ROOT		0x0004  /* Root capable */
+#define PCI_PTM_GRANULARITY_MASK	0xFF00  /* Local clock granularity */
+#define PCI_PTM_CTRL_ENABLE		0x0001  /* PTM enable */
+#define PCI_PTM_CTRL_ROOT		0x0002  /* Root select */
+#define PCI_PTM_HEADER_REG_OFFSET       0x00	/* PTM version and such */
+#define PCI_PTM_CAPABILITY_REG_OFFSET   0x04	/* Capabilities */
+#define PCI_PTM_CONTROL_REG_OFFSET      0x08	/* Control reg */
+
 #endif /* LINUX_PCI_REGS_H */
-- 
2.7.3

[RFC v4] PCI: PTM Driver

[Yong, Jonathan]

Hello LKML,

This is a preliminary implementation of the PTM[1] support driver. This driver
has only been tested against a virtual PCI bus since there are no known
endpoints utilizing it yet.

The drivers job is to get to every PTM capable device, set some PCI config
space bits, then go back to sleep [2].

PTM capable PCIe devices will get a new sysfs entry to allow PTM to be
enabled/disabled individually (enable by default).

Comments? Should I explain the PTM registers in more details?
Please CC me, thanks.

[1] Precision Time Measurement: A protocol for synchronizing PCIe endpoint
clocks against the host clock as specified in the PCI Express Base
Specification 3.1. It is identified by the 0x001f extended capability ID.

PTM capable devices are split into 3 roles, master, responder and requester.
Summary as follows:

A master holds the master clock that will be used for all devices under its
domain (not to be confused with PCI domains). There may be multiple masters
in a PTM hierarchy, in which case, the highest master closest to the root
complex will be selected for the PTM domain. A master is also always
responder capable. Clock precision is signified by a Local Clock
Granularity field, in nano-seconds.

A responder responds to any PTM synchronization requests from a downstream
device. A responder is typically a switch device. It may also hold a local
clock signified by a non-zero Local Clock Granularity field. A value of 0
signifies that the device simply propagates timing information from
upstream devices.

A requester is typically an endpoint that will request synchronization
updates from an upstream PTM capable time source. The driver will update
the Effective Clock Granularity field based on the same field from the
PTM domain master. The field should be programmed with a value of 0 if any
intervening responder has a Local Clock Granularity field value of 0.

[2] The software drivers never see the PTM packets, the PCI Express Base
Specification 3.1 reads:
        PTM capable components can make their PTM context available for
        inspection by software, enabling software to translate timing
        information between local times and PTM Master Time.

This isn't very informative.

Changes since v1:
* Moved register constants to pci_regs.h
* Use pci_dev to hold PTM status
* PTM initialization now done top-down hierarchy wise.

Changes since v2:
* Added missing void return in the pci_ptm_init inline stub.

Changes since v3:
* Clearing CONFIG_PCIE_PTM now completely prevents the driver from being built.
* Renamed pci_*_ptm_sysfs to pcie_ptm_*_sysfs_dev_files for consistency.
* Removed useless prototypes.
* Driver no longer checks PTM capability version
* PCI_EXT_CAP_ID_MAX updated to include PTM (0x1F)

Yong, Jonathan (1):
  PCI: PTM preliminary implementation

 drivers/pci/pci-sysfs.c       |   7 ++
 drivers/pci/pci.h             |  20 +++++
 drivers/pci/pcie/Kconfig      |   9 ++
 drivers/pci/pcie/Makefile     |   3 +
 drivers/pci/pcie/pcie_ptm.c   | 194 ++++++++++++++++++++++++++++++++++++++++++
 drivers/pci/probe.c           |   3 +
 include/linux/pci.h           |  11 +++
 include/uapi/linux/pci_regs.h |  14 ++-
 8 files changed, 260 insertions(+), 1 deletion(-)
 create mode 100644 drivers/pci/pcie/pcie_ptm.c

-- 
2.7.3

[PATCH] PCI: PTM preliminary implementation

[Yong, Jonathan]

Simplified Precision Time Measurement driver, activates PTM feature
if a PCIe PTM requester (as per PCI Express 3.1 Base Specification
section 7.32)is found, but not before checking if the rest of the
PCI hierarchy can support it.

The driver does not take part in facilitating PTM conversations,
neither does it provide any useful services, it is only responsible
for setting up the required configuration space bits.

As of writing, there aren't any PTM capable devices on the market
yet, but it is supported by the Intel Apollo Lake platform.

Signed-off-by: Yong, Jonathan <jonathan.yong@intel.com>
---
 drivers/pci/pci-sysfs.c       |   7 ++
 drivers/pci/pci.h             |  20 +++++
 drivers/pci/pcie/Kconfig      |   9 ++
 drivers/pci/pcie/Makefile     |   3 +
 drivers/pci/pcie/pcie_ptm.c   | 194 ++++++++++++++++++++++++++++++++++++++++++
 drivers/pci/probe.c           |   3 +
 include/linux/pci.h           |  11 +++
 include/uapi/linux/pci_regs.h |  14 ++-
 8 files changed, 260 insertions(+), 1 deletion(-)
 create mode 100644 drivers/pci/pcie/pcie_ptm.c

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index e982010..11cf97b 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -1342,6 +1342,9 @@ static int pci_create_capabilities_sysfs(struct pci_dev *dev)
 	/* Active State Power Management */
 	pcie_aspm_create_sysfs_dev_files(dev);
 
+	/* Precision Time Measurement */
+	pcie_ptm_create_sysfs_dev_files(dev);
+
 	if (!pci_probe_reset_function(dev)) {
 		retval = device_create_file(&dev->dev, &reset_attr);
 		if (retval)
@@ -1351,6 +1354,7 @@ static int pci_create_capabilities_sysfs(struct pci_dev *dev)
 	return 0;
 
 error:
+	pcie_ptm_remove_sysfs_dev_files(dev);
 	pcie_aspm_remove_sysfs_dev_files(dev);
 	if (dev->vpd && dev->vpd->attr) {
 		sysfs_remove_bin_file(&dev->dev.kobj, dev->vpd->attr);
@@ -1436,6 +1440,9 @@ static void pci_remove_capabilities_sysfs(struct pci_dev *dev)
 	}
 
 	pcie_aspm_remove_sysfs_dev_files(dev);
+
+	pcie_ptm_remove_sysfs_dev_files(dev);
+
 	if (dev->reset_fn) {
 		device_remove_file(&dev->dev, &reset_attr);
 		dev->reset_fn = 0;
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index d0fb934..908445b 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -320,6 +320,26 @@ static inline resource_size_t pci_resource_alignment(struct pci_dev *dev,
 
 void pci_enable_acs(struct pci_dev *dev);
 
+#ifdef CONFIG_PCIE_PTM
+int pci_enable_ptm(struct pci_dev *dev);
+int pcie_ptm_create_sysfs_dev_files(struct pci_dev *dev);
+void pcie_ptm_remove_sysfs_dev_files(struct pci_dev *dev);
+void pci_disable_ptm(struct pci_dev *dev);
+void pci_ptm_init(struct pci_dev *dev);
+#else
+static inline int pci_enable_ptm(struct pci_dev *dev)
+{
+	return -ENXIO;
+}
+static inline int pcie_ptm_create_sysfs_dev_files(struct pci_dev *dev)
+{
+	return -ENXIO;
+}
+static inline void pcie_ptm_remove_sysfs_dev_files(struct pci_dev *dev) {}
+static inline void pci_disable_ptm(struct pci_dev *dev) {}
+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 72db7f4..2baddc5 100644
--- a/drivers/pci/pcie/Kconfig
+++ b/drivers/pci/pcie/Kconfig
@@ -81,3 +81,12 @@ endchoice
 config PCIE_PME
 	def_bool y
 	depends on PCIEPORTBUS && PM
+
+config PCIE_PTM
+	bool "Enable Precision Time Measurement support"
+	default y
+	depends on PCIEPORTBUS
+	help
+	  Say Y here if you have PCI Express devices that are capable of
+	  Precision Time Measurement (PTM). This also requires that your
+	  PCI Express controller or switch fabric is PTM capable.
diff --git a/drivers/pci/pcie/Makefile b/drivers/pci/pcie/Makefile
index 00c62df..726b972 100644
--- a/drivers/pci/pcie/Makefile
+++ b/drivers/pci/pcie/Makefile
@@ -14,3 +14,6 @@ obj-$(CONFIG_PCIEPORTBUS)	+= pcieportdrv.o
 obj-$(CONFIG_PCIEAER)		+= aer/
 
 obj-$(CONFIG_PCIE_PME) += pme.o
+
+# Precision Time Measurement support
+obj-$(CONFIG_PCIE_PTM) += pcie_ptm.o
diff --git a/drivers/pci/pcie/pcie_ptm.c b/drivers/pci/pcie/pcie_ptm.c
new file mode 100644
index 0000000..f4e4d61
--- /dev/null
+++ b/drivers/pci/pcie/pcie_ptm.c
@@ -0,0 +1,194 @@
+/*
+ * 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 bool disable_ptm;
+
+module_param_named(disable_ptm, disable_ptm, bool, S_IRUGO | S_IWUSR);
+MODULE_PARM_DESC(disable_ptm, "Don't automatically enable PCIe PTM even if supported.");
+
+static int ptm_commit(struct pci_dev *dev)
+{
+	u32 dword;
+	int pos;
+
+	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_PTM);
+
+	/* Is this even possible? */
+	if (!pos)
+		return -ENXIO;
+
+	pci_read_config_dword(dev, pos + PCI_PTM_CONTROL_REG_OFFSET, &dword);
+	dword = dev->is_ptm_enabled ? dword | PCI_PTM_CTRL_ENABLE :
+		dword & ~PCI_PTM_CTRL_ENABLE;
+	dword = dev->is_ptm_root ? dword | PCI_PTM_CTRL_ROOT :
+		dword & ~PCI_PTM_CTRL_ROOT;
+
+	/* Only requester should have it set */
+	if (dev->is_ptm_requester)
+		dword = dword | (((u32)dev->ptm_effective_granularity) << 8);
+	return pci_write_config_dword(dev, pos + PCI_PTM_CONTROL_REG_OFFSET,
+		dword);
+}
+
+/**
+ * pci_enable_ptm - Try to activate PTM functionality on device.
+ * @dev: PCI Express device with PTM requester role to enable.
+ *
+ * All PCIe Switches/Bridges in between need to be enabled for this to work.
+ *
+ * NOTE: Each requester must be associated with a PTM root (not to be confused
+ * with a root port or root complex). There can be multiple PTM roots in a
+ * a system forming multiple domains. All intervening bridges/switches in a
+ * domain must support PTM responder roles to relay PTM dialogues.
+ */
+int pci_enable_ptm(struct pci_dev *dev)
+{
+	int type;
+	struct pci_dev *upstream;
+
+	upstream = pci_upstream_bridge(dev);
+	type = pci_pcie_type(dev);
+
+	if (dev->is_ptm_root_capable) {
+		/* If we are root capable but already part of a chain, don't set
+		 * the root select bit, only enable PTM
+		 */
+		if (!upstream || !upstream->is_ptm_enabled)
+			dev->is_ptm_root = 1;
+		dev->is_ptm_enabled = 1;
+	}
+
+	/* Is possible to be part of the PTM chain */
+	if (dev->is_ptm_responder && upstream && upstream->is_ptm_enabled)
+		dev->is_ptm_enabled = 1;
+
+	if (dev->is_ptm_requester && upstream && upstream->is_ptm_enabled) {
+		dev->is_ptm_enabled = 1;
+		dev->ptm_effective_granularity =
+			upstream->ptm_clock_granularity;
+	}
+	return ptm_commit(dev);
+}
+
+void pci_ptm_init(struct pci_dev *dev)
+{
+	u32 dword;
+	int pos;
+
+	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_PTM);
+	if (!pos)
+		return;
+
+	/* Fill in caps, masters are implied to be responders as well */
+	pci_read_config_dword(dev, pos + PCI_PTM_CAPABILITY_REG_OFFSET, &dword);
+	dev->is_ptm_capable = 1;
+	dev->is_ptm_root_capable   = (dword & PCI_PTM_CAP_ROOT) ? 1 : 0;
+	dev->is_ptm_responder      = (dword & PCI_PTM_CAP_RSP) ? 1 : 0;
+	dev->is_ptm_requester      = (dword & PCI_PTM_CAP_REQ) ? 1 : 0;
+	dev->ptm_clock_granularity = dev->is_ptm_responder ?
+		((dword & PCI_PTM_GRANULARITY_MASK) >> 8) : 0;
+	dev_info(&dev->dev, "Found PTM %s type device with %uns clock\n",
+		dev->is_ptm_root_capable ? "root" :
+		dev->is_ptm_responder ? "responder" :
+		dev->is_ptm_requester ? "requester" : "unknown",
+		dev->ptm_clock_granularity);
+
+	/* Get existing settings */
+	pci_read_config_dword(dev, pos + PCI_PTM_CONTROL_REG_OFFSET, &dword);
+	dev->is_ptm_enabled            = (dword & PCI_PTM_CTRL_ENABLE) ? 1 : 0;
+	dev->is_ptm_root               = (dword & PCI_PTM_CTRL_ROOT) ? 1 : 0;
+	dev->ptm_effective_granularity =
+		(dword & PCI_PTM_GRANULARITY_MASK) >> 8;
+
+	if (!disable_ptm)
+		pci_enable_ptm(dev);
+}
+
+static int do_disable_ptm(struct pci_dev *dev, void *v)
+{
+	if (dev->is_ptm_enabled) {
+		dev->is_ptm_enabled            = 0;
+		dev->is_ptm_root               = 0;
+		dev->ptm_effective_granularity = 0;
+		ptm_commit(dev);
+	}
+	return 0;
+}
+
+/**
+ * pci_disable_ptm - Turn off PTM functionality on device.
+ * @dev: PCI Express device with PTM function to disable.
+ *
+ * Disables PTM functionality by clearing the PTM enable bit, if device is a
+ * switch/bridge it will also disable PTM function on other devices behind it.
+ */
+void pci_disable_ptm(struct pci_dev *dev)
+{
+	if (pci_is_bridge(dev))
+		pci_walk_bus(dev->bus, &do_disable_ptm, NULL);
+	else
+		do_disable_ptm(dev, NULL);
+}
+
+static ssize_t ptm_status_show(struct device *dev,
+	struct device_attribute *attr, char *buf)
+{
+	struct pci_dev *pdev = to_pci_dev(dev);
+	u16 word;
+	int pos;
+
+	pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PTM);
+	if (!pos)
+		return -ENXIO;
+
+	pci_read_config_word(pdev, pos + PCI_PTM_CONTROL_REG_OFFSET, &word);
+	return sprintf(buf, "%u\n", word & PCI_PTM_CTRL_ENABLE ? 1 : 0);
+}
+
+static ssize_t ptm_status_store(struct device *dev,
+	struct device_attribute *attr, const char *buf, size_t count)
+{
+	struct pci_dev *pdev = to_pci_dev(dev);
+	unsigned long val;
+	ssize_t ret;
+
+	ret = kstrtoul(buf, 0, &val);
+	if (ret)
+		return ret;
+	if (val)
+		return pci_enable_ptm(pdev);
+	pci_disable_ptm(pdev);
+	return 0;
+}
+
+static DEVICE_ATTR_RW(ptm_status);
+
+void pcie_ptm_remove_sysfs_dev_files(struct pci_dev *dev)
+{
+	if (!pci_find_ext_capability(dev, PCI_EXT_CAP_ID_PTM))
+		return;
+	device_remove_file(&dev->dev, &dev_attr_ptm_status);
+}
+
+int pcie_ptm_create_sysfs_dev_files(struct pci_dev *dev)
+{
+	if (!pci_find_ext_capability(dev, PCI_EXT_CAP_ID_PTM))
+		return -ENXIO;
+	return device_create_file(&dev->dev, &dev_attr_ptm_status);
+}
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 8004f67..9d5e96e6 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1657,6 +1657,9 @@ static void pci_init_capabilities(struct pci_dev *dev)
 	pci_enable_acs(dev);
 
 	pci_cleanup_aer_error_status_regs(dev);
+
+	/* Enable PTM Capabilities */
+	pci_ptm_init(dev);
 }
 
 /*
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 004b813..ba5dab4 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -363,6 +363,17 @@ 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	is_ptm_capable:1;
+	unsigned int	is_ptm_root_capable:1;
+	unsigned int	is_ptm_responder:1;
+	unsigned int	is_ptm_requester:1;
+	unsigned int	is_ptm_enabled:1;
+	unsigned int	is_ptm_root:1;
+	u8		ptm_clock_granularity;
+	u8		ptm_effective_granularity;
+#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 1becea8..9dd77be 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -670,7 +670,8 @@
 #define PCI_EXT_CAP_ID_SECPCI	0x19	/* Secondary PCIe Capability */
 #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_MAX	PCI_EXT_CAP_ID_PASID
+#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
@@ -946,4 +947,15 @@
 #define PCI_TPH_CAP_ST_SHIFT	16	/* st table shift */
 #define PCI_TPH_BASE_SIZEOF	12	/* size with no st table */
 
+/* Precision Time Measurement */
+#define PCI_PTM_CAP_REQ			0x0001  /* Requester capable */
+#define PCI_PTM_CAP_RSP			0x0002  /* Responder capable */
+#define PCI_PTM_CAP_ROOT		0x0004  /* Root capable */
+#define PCI_PTM_GRANULARITY_MASK	0xFF00  /* Local clock granularity */
+#define PCI_PTM_CTRL_ENABLE		0x0001  /* PTM enable */
+#define PCI_PTM_CTRL_ROOT		0x0002  /* Root select */
+#define PCI_PTM_HEADER_REG_OFFSET       0x00	/* PTM version and such */
+#define PCI_PTM_CAPABILITY_REG_OFFSET   0x04	/* Capabilities */
+#define PCI_PTM_CONTROL_REG_OFFSET      0x08	/* Control reg */
+
 #endif /* LINUX_PCI_REGS_H */
-- 
2.7.3

Re: [PATCH] PCI: PTM preliminary implementation

[Bjorn Helgaas]

Hi Jonathan,

Thanks for polishing this up.  I think this is really coming together
nicely.

On Tue, Apr 19, 2016 at 06:29:18AM +0000, Yong, Jonathan wrote:
> Simplified Precision Time Measurement driver, activates PTM feature
> if a PCIe PTM requester (as per PCI Express 3.1 Base Specification
> section 7.32)is found, but not before checking if the rest of the
> PCI hierarchy can support it.
> 
> The driver does not take part in facilitating PTM conversations,
> neither does it provide any useful services, it is only responsible
> for setting up the required configuration space bits.
> 
> As of writing, there aren't any PTM capable devices on the market
> yet, but it is supported by the Intel Apollo Lake platform.
> 
> Signed-off-by: Yong, Jonathan <jonathan.yong@intel.com>
> ---
>  drivers/pci/pci-sysfs.c       |   7 ++
>  drivers/pci/pci.h             |  20 +++++
>  drivers/pci/pcie/Kconfig      |   9 ++
>  drivers/pci/pcie/Makefile     |   3 +
>  drivers/pci/pcie/pcie_ptm.c   | 194 ++++++++++++++++++++++++++++++++++++++++++
>  drivers/pci/probe.c           |   3 +
>  include/linux/pci.h           |  11 +++
>  include/uapi/linux/pci_regs.h |  14 ++-
>  8 files changed, 260 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/pci/pcie/pcie_ptm.c
> 
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index e982010..11cf97b 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -1342,6 +1342,9 @@ static int pci_create_capabilities_sysfs(struct pci_dev *dev)
>  	/* Active State Power Management */
>  	pcie_aspm_create_sysfs_dev_files(dev);
>  
> +	/* Precision Time Measurement */
> +	pcie_ptm_create_sysfs_dev_files(dev);
> +
>  	if (!pci_probe_reset_function(dev)) {
>  		retval = device_create_file(&dev->dev, &reset_attr);
>  		if (retval)
> @@ -1351,6 +1354,7 @@ static int pci_create_capabilities_sysfs(struct pci_dev *dev)
>  	return 0;
>  
>  error:
> +	pcie_ptm_remove_sysfs_dev_files(dev);
>  	pcie_aspm_remove_sysfs_dev_files(dev);
>  	if (dev->vpd && dev->vpd->attr) {
>  		sysfs_remove_bin_file(&dev->dev.kobj, dev->vpd->attr);
> @@ -1436,6 +1440,9 @@ static void pci_remove_capabilities_sysfs(struct pci_dev *dev)
>  	}
>  
>  	pcie_aspm_remove_sysfs_dev_files(dev);
> +
> +	pcie_ptm_remove_sysfs_dev_files(dev);
> +
>  	if (dev->reset_fn) {
>  		device_remove_file(&dev->dev, &reset_attr);
>  		dev->reset_fn = 0;
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index d0fb934..908445b 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -320,6 +320,26 @@ static inline resource_size_t pci_resource_alignment(struct pci_dev *dev,
>  
>  void pci_enable_acs(struct pci_dev *dev);
>  
> +#ifdef CONFIG_PCIE_PTM
> +int pci_enable_ptm(struct pci_dev *dev);
> +int pcie_ptm_create_sysfs_dev_files(struct pci_dev *dev);
> +void pcie_ptm_remove_sysfs_dev_files(struct pci_dev *dev);
> +void pci_disable_ptm(struct pci_dev *dev);
> +void pci_ptm_init(struct pci_dev *dev);
> +#else
> +static inline int pci_enable_ptm(struct pci_dev *dev)
> +{
> +	return -ENXIO;
> +}
> +static inline int pcie_ptm_create_sysfs_dev_files(struct pci_dev *dev)
> +{
> +	return -ENXIO;
> +}
> +static inline void pcie_ptm_remove_sysfs_dev_files(struct pci_dev *dev) {}
> +static inline void pci_disable_ptm(struct pci_dev *dev) {}
> +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 72db7f4..2baddc5 100644
> --- a/drivers/pci/pcie/Kconfig
> +++ b/drivers/pci/pcie/Kconfig
> @@ -81,3 +81,12 @@ endchoice
>  config PCIE_PME
>  	def_bool y
>  	depends on PCIEPORTBUS && PM
> +
> +config PCIE_PTM
> +	bool "Enable Precision Time Measurement support"
> +	default y
> +	depends on PCIEPORTBUS
> +	help
> +	  Say Y here if you have PCI Express devices that are capable of
> +	  Precision Time Measurement (PTM). This also requires that your
> +	  PCI Express controller or switch fabric is PTM capable.

I think this text might be slightly too discouraging.  Users might get
the idea that this config option cannot be enabled unless they have
PTM-capable hardware.  Maybe something like:

  Say Y here to enable support for PCI Express Precision Time
  Measurement (PTM).  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 00c62df..726b972 100644
> --- a/drivers/pci/pcie/Makefile
> +++ b/drivers/pci/pcie/Makefile
> @@ -14,3 +14,6 @@ obj-$(CONFIG_PCIEPORTBUS)	+= pcieportdrv.o
>  obj-$(CONFIG_PCIEAER)		+= aer/
>  
>  obj-$(CONFIG_PCIE_PME) += pme.o
> +
> +# Precision Time Measurement support
> +obj-$(CONFIG_PCIE_PTM) += pcie_ptm.o
> diff --git a/drivers/pci/pcie/pcie_ptm.c b/drivers/pci/pcie/pcie_ptm.c
> new file mode 100644
> index 0000000..f4e4d61
> --- /dev/null
> +++ b/drivers/pci/pcie/pcie_ptm.c
> @@ -0,0 +1,194 @@
> +/*
> + * 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 bool disable_ptm;
> +
> +module_param_named(disable_ptm, disable_ptm, bool, S_IRUGO | S_IWUSR);

This can be "module_param" since the variable and parameter names are
the same.

> +MODULE_PARM_DESC(disable_ptm, "Don't automatically enable PCIe PTM even if supported.");

Since this can only be built in statically (it can't be a module),
this parameter would have to be specified on the kernel command line
(or, I guess, done via sysfs, which would only affect devices added
via hotplug).

What exactly would that look like?  Maybe we could include an example
in the changelog, e.g., "pcie_ptm.disable_ptm" or whatever it is.  I
don't use these often enough to remember the details and it's always a
hassle for me to figure out the module name.

If it really is "pcie_ptm.disable_ptm", that looks a little redundant.
Probably no need to mention "ptm" twice.  But I suppose that would be
a problem in sysfs?  Where does it show up there?

We do have some similar kernel command-line arguments like "noaer",
"noari", "nomsi", "nommconf".  We could consider that strategy
instead, and I might even prefer it since this is a switch for the PCI
core, not something related to a driver or a loadable module.

> +static int ptm_commit(struct pci_dev *dev)
> +{
> +	u32 dword;
> +	int pos;
> +
> +	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_PTM);
> +
> +	/* Is this even possible? */
> +	if (!pos)
> +		return -ENXIO;

It makes me a little scared that it's not obvious whether this is
possible or not :)

I think it's not possible *except* that resetting the device throws a
monkey wrench in things.  When we reset a device, we retain all the
state, including the sysfs files and dev->is_ptm_* bits, but the reset
may have caused the device to load new firmware or otherwise change
its config space.  In that case, the new config space could have no
PTM capability.  Anyway, this scenario breaks lots of things, so I
guess you don't need to worry excessively here.

> +	pci_read_config_dword(dev, pos + PCI_PTM_CONTROL_REG_OFFSET, &dword);
> +	dword = dev->is_ptm_enabled ? dword | PCI_PTM_CTRL_ENABLE :
> +		dword & ~PCI_PTM_CTRL_ENABLE;
> +	dword = dev->is_ptm_root ? dword | PCI_PTM_CTRL_ROOT :
> +		dword & ~PCI_PTM_CTRL_ROOT;
> +
> +	/* Only requester should have it set */
> +	if (dev->is_ptm_requester)
> +		dword = dword | (((u32)dev->ptm_effective_granularity) << 8);
> +	return pci_write_config_dword(dev, pos + PCI_PTM_CONTROL_REG_OFFSET,
> +		dword);
> +}
> +
> +/**
> + * pci_enable_ptm - Try to activate PTM functionality on device.
> + * @dev: PCI Express device with PTM requester role to enable.
> + *
> + * All PCIe Switches/Bridges in between need to be enabled for this to work.
> + *
> + * NOTE: Each requester must be associated with a PTM root (not to be confused
> + * with a root port or root complex). There can be multiple PTM roots in a
> + * a system forming multiple domains. All intervening bridges/switches in a
> + * domain must support PTM responder roles to relay PTM dialogues.
> + */
> +int pci_enable_ptm(struct pci_dev *dev)
> +{
> +	int type;

Unused, remove.

> +	struct pci_dev *upstream;
> +
> +	upstream = pci_upstream_bridge(dev);
> +	type = pci_pcie_type(dev);
> +
> +	if (dev->is_ptm_root_capable) {
> +		/* If we are root capable but already part of a chain, don't set
> +		 * the root select bit, only enable PTM
> +		 */
> +		if (!upstream || !upstream->is_ptm_enabled)
> +			dev->is_ptm_root = 1;
> +		dev->is_ptm_enabled = 1;
> +	}
> +
> +	/* Is possible to be part of the PTM chain */
> +	if (dev->is_ptm_responder && upstream && upstream->is_ptm_enabled)
> +		dev->is_ptm_enabled = 1;

If we're trying to enable PTM on a device, and PTM is not enabled on
the upstream bridge, I think we should fail without touching the
register.  For example, if we're using sysfs to try to enable PTM on a
device below a non-PTM switch, the sysfs write should fail.
Currently, I think it will succeed.  PTM won't be enabled, but we will
write the config register and the sysfs write will return success.

> +
> +	if (dev->is_ptm_requester && upstream && upstream->is_ptm_enabled) {
> +		dev->is_ptm_enabled = 1;
> +		dev->ptm_effective_granularity =
> +			upstream->ptm_clock_granularity;

Per 7.32.3, software must program the Effective Granularity to the
"maximum Local Clock Granularity reported by the PTM Root and all
intervening PTM Time Sources."  So there should be some sort of a
max() somewhere here, shouldn't there?

The spec says this is only relevant for PTM Requesters, i.e.,
endpoints, and provides information about the expected PTM accuracy
but doesn't affect the PTM mechanism.  So I guess this is purely
informational?  But I suppose it would be good to have the right value
here for lspci.  (BTW, have you posted any lspci patches to dump the
PTM capability?)

> +	}
> +	return ptm_commit(dev);
> +}
> +
> +void pci_ptm_init(struct pci_dev *dev)
> +{
> +	u32 dword;
> +	int pos;
> +
> +	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_PTM);
> +	if (!pos)
> +		return;
> +
> +	/* Fill in caps, masters are implied to be responders as well */
> +	pci_read_config_dword(dev, pos + PCI_PTM_CAPABILITY_REG_OFFSET, &dword);
> +	dev->is_ptm_capable = 1;
> +	dev->is_ptm_root_capable   = (dword & PCI_PTM_CAP_ROOT) ? 1 : 0;
> +	dev->is_ptm_responder      = (dword & PCI_PTM_CAP_RSP) ? 1 : 0;
> +	dev->is_ptm_requester      = (dword & PCI_PTM_CAP_REQ) ? 1 : 0;
> +	dev->ptm_clock_granularity = dev->is_ptm_responder ?
> +		((dword & PCI_PTM_GRANULARITY_MASK) >> 8) : 0;
> +	dev_info(&dev->dev, "Found PTM %s type device with %uns clock\n",

s/Found //
s/type //
s/device //

> +		dev->is_ptm_root_capable ? "root" :
> +		dev->is_ptm_responder ? "responder" :
> +		dev->is_ptm_requester ? "requester" : "unknown",
> +		dev->ptm_clock_granularity);

I think this printk needs to be expanded a little bit to avoid
confusion.  For example, endpoints will all say:

  PTM requester with 0ns clock

which is not really what we want to know.  For endpoints, I think what
we *do* want to know is their Effective Granularity.

And switches might say:

  PTM responder with 0ns clock

which really means "no local clock".

I think what we'd like to have is information like:

  - PTM root has granularity X
  - endpoint has some larger granularity Y
  - endpoint is limited because an intermediate switch has granularity Y

I'm not sure we know enough at this point in the code.  We might need
to know whether Root Select is set to figure out which clock (Local or
Effective) to print.

> +
> +	/* Get existing settings */
> +	pci_read_config_dword(dev, pos + PCI_PTM_CONTROL_REG_OFFSET, &dword);
> +	dev->is_ptm_enabled            = (dword & PCI_PTM_CTRL_ENABLE) ? 1 : 0;
> +	dev->is_ptm_root               = (dword & PCI_PTM_CTRL_ROOT) ? 1 : 0;
> +	dev->ptm_effective_granularity =
> +		(dword & PCI_PTM_GRANULARITY_MASK) >> 8;
> +
> +	if (!disable_ptm)
> +		pci_enable_ptm(dev);
> +}
> +
> +static int do_disable_ptm(struct pci_dev *dev, void *v)
> +{
> +	if (dev->is_ptm_enabled) {
> +		dev->is_ptm_enabled            = 0;
> +		dev->is_ptm_root               = 0;
> +		dev->ptm_effective_granularity = 0;
> +		ptm_commit(dev);
> +	}
> +	return 0;
> +}
> +
> +/**
> + * pci_disable_ptm - Turn off PTM functionality on device.
> + * @dev: PCI Express device with PTM function to disable.
> + *
> + * Disables PTM functionality by clearing the PTM enable bit, if device is a
> + * switch/bridge it will also disable PTM function on other devices behind it.
> + */
> +void pci_disable_ptm(struct pci_dev *dev)

This is currently only used in this file, so it should be static (and
removed from pci.h).

> +{
> +	if (pci_is_bridge(dev))
> +		pci_walk_bus(dev->bus, &do_disable_ptm, NULL);

This is asymmetric: the sysfs enable path only enables PTM for one
device (and it will fail unless the upstream switch has PTM enabled),
so a user has to walk the tree manually, but the disable will disable
a whole tree.  Since we default to enabling PTM, I think the sysfs
interface is mostly for debugging, and doing it on individual devices
is fine.

Besides, I don't think this actually works correctly.  If you use this
on a switch, we disable PTM on everything *below* the switch, but not
on the switch itself.

> +	else
> +		do_disable_ptm(dev, NULL);
> +}
> +
> +static ssize_t ptm_status_show(struct device *dev,
> +	struct device_attribute *attr, char *buf)
> +{
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +	u16 word;
> +	int pos;
> +
> +	pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PTM);
> +	if (!pos)
> +		return -ENXIO;
> +
> +	pci_read_config_word(pdev, pos + PCI_PTM_CONTROL_REG_OFFSET, &word);

This is a 32-bit register; read it as a dword, even though the upper
bits are currently reserved.

> +	return sprintf(buf, "%u\n", word & PCI_PTM_CTRL_ENABLE ? 1 : 0);
> +}
> +
> +static ssize_t ptm_status_store(struct device *dev,
> +	struct device_attribute *attr, const char *buf, size_t count)
> +{
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +	unsigned long val;
> +	ssize_t ret;
> +
> +	ret = kstrtoul(buf, 0, &val);
> +	if (ret)
> +		return ret;
> +	if (val)
> +		return pci_enable_ptm(pdev);
> +	pci_disable_ptm(pdev);
> +	return 0;
> +}
> +
> +static DEVICE_ATTR_RW(ptm_status);
> +
> +void pcie_ptm_remove_sysfs_dev_files(struct pci_dev *dev)
> +{
> +	if (!pci_find_ext_capability(dev, PCI_EXT_CAP_ID_PTM))
> +		return;
> +	device_remove_file(&dev->dev, &dev_attr_ptm_status);
> +}

This is called in the device remove path.  Obviously we want to
remove the sysfs file.  But we currently don't do anything to the PTM
capability itself.

What happens if the device has PTM enabled, we hotplug remove it
(using the sysfs interface so the device stays physically present and
powered up), manually disable PTM on the upstream switch, then hotplug
add the device back?  If the switch has PTM disabled but the device
has PTM enabled, that sounds like an illegal configuration.

Is this scenario possible?  Maybe the hotplug remove would power off
the device?  I guess the current pci_ptm_init() path actually would
disable PTM on the new device because it's disabled on the upstream
switch.

But since PTM is autonomous once enabled, I would assume the new
device (with PTM enabled) could send PTM messages upstream, and it
sounds (per sec 6.22.3) like those would be treated as Unsupported
Requests, which means we'd report errors.

I think we might want to disable PTM before we remove a device.

> +int pcie_ptm_create_sysfs_dev_files(struct pci_dev *dev)
> +{
> +	if (!pci_find_ext_capability(dev, PCI_EXT_CAP_ID_PTM))
> +		return -ENXIO;
> +	return device_create_file(&dev->dev, &dev_attr_ptm_status);
> +}
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 8004f67..9d5e96e6 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1657,6 +1657,9 @@ static void pci_init_capabilities(struct pci_dev *dev)
>  	pci_enable_acs(dev);
>  
>  	pci_cleanup_aer_error_status_regs(dev);
> +
> +	/* Enable PTM Capabilities */
> +	pci_ptm_init(dev);
>  }
>  
>  /*
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 004b813..ba5dab4 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
 @@ -363,6 +363,17 @@ 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	is_ptm_capable:1;
> +	unsigned int	is_ptm_root_capable:1;
> +	unsigned int	is_ptm_responder:1;
> +	unsigned int	is_ptm_requester:1;
> +	unsigned int	is_ptm_enabled:1;
> +	unsigned int	is_ptm_root:1;

s/is_// above.

> +	u8		ptm_clock_granularity;
> +	u8		ptm_effective_granularity;
> +#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 1becea8..9dd77be 100644
> --- a/include/uapi/linux/pci_regs.h
> +++ b/include/uapi/linux/pci_regs.h
> @@ -670,7 +670,8 @@
>  #define PCI_EXT_CAP_ID_SECPCI	0x19	/* Secondary PCIe Capability */
>  #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_MAX	PCI_EXT_CAP_ID_PASID
> +#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
> @@ -946,4 +947,15 @@
>  #define PCI_TPH_CAP_ST_SHIFT	16	/* st table shift */
>  #define PCI_TPH_BASE_SIZEOF	12	/* size with no st table */
>  
> +/* Precision Time Measurement */
> +#define PCI_PTM_CAP_REQ			0x0001  /* Requester capable */
> +#define PCI_PTM_CAP_RSP			0x0002  /* Responder capable */
> +#define PCI_PTM_CAP_ROOT		0x0004  /* Root capable */
> +#define PCI_PTM_GRANULARITY_MASK	0xFF00  /* Local clock granularity */
> +#define PCI_PTM_CTRL_ENABLE		0x0001  /* PTM enable */
> +#define PCI_PTM_CTRL_ROOT		0x0002  /* Root select */
> +#define PCI_PTM_HEADER_REG_OFFSET       0x00	/* PTM version and such */

Unused, remove.

> +#define PCI_PTM_CAPABILITY_REG_OFFSET   0x04	/* Capabilities */
> +#define PCI_PTM_CONTROL_REG_OFFSET      0x08	/* Control reg */

Follow existing style in this file for these #defines.  For example:

  - Offsets into the capability do not need "REG_OFFSET", e.g., use
    "#define PCI_PTM_CAP 0x04" and #define PCI_PTM_CTRL 0x08".

  - Definitions sorted by offset into capability, e.g.,
      #define PCI_PTM_CAP ...
      #define  PCI_PTM_CAP_REQ ...
      ...
      #define PCI_PTM_CTRL ...
      #define  PCI_PTM_CTRL_ENABLE ...

  - Definitions for bits inside a register indented an extra space,
    e.g., "#define  PCI_PTM_CAP_REQ ..."

  - The PTM Capability and Control registers are 32 bits wide, so use
    32-bit constants for the bits and fields in them, e.g.,
    "#define PCI_PTM_GRANULARITY_MASK 0x0000FF00"

Re: [PATCH] PCI: PTM preliminary implementation

[Bjorn Helgaas]

On Fri, Apr 29, 2016 at 11:20:26AM -0500, Bjorn Helgaas wrote:
> On Tue, Apr 19, 2016 at 06:29:18AM +0000, Yong, Jonathan wrote:
> > ...
> > +static ssize_t ptm_status_show(struct device *dev,
> > +	struct device_attribute *attr, char *buf)
> > +{
> > +	struct pci_dev *pdev = to_pci_dev(dev);
> > +	u16 word;
> > +	int pos;
> > +
> > +	pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PTM);
> > +	if (!pos)
> > +		return -ENXIO;
> > +
> > +	pci_read_config_word(pdev, pos + PCI_PTM_CONTROL_REG_OFFSET, &word);
> 
> This is a 32-bit register; read it as a dword, even though the upper
> bits are currently reserved.
> 
> > +	return sprintf(buf, "%u\n", word & PCI_PTM_CTRL_ENABLE ? 1 : 0);
> > +}
> > +
> > +static ssize_t ptm_status_store(struct device *dev,
> > +	struct device_attribute *attr, const char *buf, size_t count)
> > +{
> > +	struct pci_dev *pdev = to_pci_dev(dev);
> > +	unsigned long val;
> > +	ssize_t ret;
> > +
> > +	ret = kstrtoul(buf, 0, &val);
> > +	if (ret)
> > +		return ret;
> > +	if (val)
> > +		return pci_enable_ptm(pdev);
> > +	pci_disable_ptm(pdev);
> > +	return 0;
> > +}
> > +
> > +static DEVICE_ATTR_RW(ptm_status);

Thinking about this sysfs interface some more...  Do you think we
really need a knob in sysfs?  What's the use case for it?  I suppose
there's potentially a performance impact because we'll send more
messages across the link, but I'd be surprised if it were measurable.
If it's basically free, there shouldn't be a need to enable/disable it
individually, except maybe to work around hardware bugs.  A kernel
boot parameter might be enough for that.  Even a kernel boot parameter
should be regarded as a debug/testing tool, not a solution.  If we
find broken devices, we should add quirks for them so users don't need
to use a boot parameter.

Bjorn

Re: [PATCH] PCI: PTM preliminary implementation

[Yong, Jonathan]

On 04/30/2016 00:20, Bjorn Helgaas wrote:

>> +	return sprintf(buf, "%u\n", word & PCI_PTM_CTRL_ENABLE ? 1 : 0);
>> +}
>> +
>> +static ssize_t ptm_status_store(struct device *dev,
>> +	struct device_attribute *attr, const char *buf, size_t count)
>> +{
>> +	struct pci_dev *pdev = to_pci_dev(dev);
>> +	unsigned long val;
>> +	ssize_t ret;
>> +
>> +	ret = kstrtoul(buf, 0, &val);
>> +	if (ret)
>> +		return ret;
>> +	if (val)
>> +		return pci_enable_ptm(pdev);
>> +	pci_disable_ptm(pdev);
>> +	return 0;
>> +}
>> +
>> +static DEVICE_ATTR_RW(ptm_status);
>> +
>> +void pcie_ptm_remove_sysfs_dev_files(struct pci_dev *dev)
>> +{
>> +	if (!pci_find_ext_capability(dev, PCI_EXT_CAP_ID_PTM))
>> +		return;
>> +	device_remove_file(&dev->dev, &dev_attr_ptm_status);
>> +}
>
> This is called in the device remove path.  Obviously we want to
> remove the sysfs file.  But we currently don't do anything to the PTM
> capability itself.
>
> What happens if the device has PTM enabled, we hotplug remove it
> (using the sysfs interface so the device stays physically present and
> powered up), manually disable PTM on the upstream switch, then hotplug
> add the device back?  If the switch has PTM disabled but the device
> has PTM enabled, that sounds like an illegal configuration.
>
> Is this scenario possible?  Maybe the hotplug remove would power off
> the device?  I guess the current pci_ptm_init() path actually would
> disable PTM on the new device because it's disabled on the upstream
> switch.
>
> But since PTM is autonomous once enabled, I would assume the new
> device (with PTM enabled) could send PTM messages upstream, and it
> sounds (per sec 6.22.3) like those would be treated as Unsupported
> Requests, which means we'd report errors.
>
> I think we might want to disable PTM before we remove a device.
>

I'm not familiar with PCI hotplug, release_slot in acpiphp_core.c?

Re: [PATCH] PCI: PTM preliminary implementation

[Bjorn Helgaas]

On Tue, Apr 19, 2016 at 06:29:18AM +0000, Yong, Jonathan wrote:
> Simplified Precision Time Measurement driver, activates PTM feature
> if a PCIe PTM requester (as per PCI Express 3.1 Base Specification
> section 7.32)is found, but not before checking if the rest of the
> PCI hierarchy can support it.
> 
> The driver does not take part in facilitating PTM conversations,
> neither does it provide any useful services, it is only responsible
> for setting up the required configuration space bits.
> 
> As of writing, there aren't any PTM capable devices on the market
> yet, but it is supported by the Intel Apollo Lake platform.

Can you also add lspci support for the PTM capability?  That will help
debugging.

Bjorn

Re: [PATCH] PCI: PTM preliminary implementation

[Yong, Jonathan]

On 05/08/2016 10:38, Bjorn Helgaas wrote:
> On Tue, Apr 19, 2016 at 06:29:18AM +0000, Yong, Jonathan wrote:
>> Simplified Precision Time Measurement driver, activates PTM feature
>> if a PCIe PTM requester (as per PCI Express 3.1 Base Specification
>> section 7.32)is found, but not before checking if the rest of the
>> PCI hierarchy can support it.
>>
>> The driver does not take part in facilitating PTM conversations,
>> neither does it provide any useful services, it is only responsible
>> for setting up the required configuration space bits.
>>
>> As of writing, there aren't any PTM capable devices on the market
>> yet, but it is supported by the Intel Apollo Lake platform.
>
> Can you also add lspci support for the PTM capability?  That will help
> debugging.
>
> Bjorn
>

Do I also send it to this list? Or directly to Martin?

As for the kernel code side, still cleaning things up.

Re: [PATCH] PCI: PTM preliminary implementation

[Bjorn Helgaas]

On Mon, May 09, 2016 at 11:11:20AM +0800, Yong, Jonathan wrote:
> On 05/08/2016 10:38, Bjorn Helgaas wrote:
> >On Tue, Apr 19, 2016 at 06:29:18AM +0000, Yong, Jonathan wrote:
> >>Simplified Precision Time Measurement driver, activates PTM feature
> >>if a PCIe PTM requester (as per PCI Express 3.1 Base Specification
> >>section 7.32)is found, but not before checking if the rest of the
> >>PCI hierarchy can support it.
> >>
> >>The driver does not take part in facilitating PTM conversations,
> >>neither does it provide any useful services, it is only responsible
> >>for setting up the required configuration space bits.
> >>
> >>As of writing, there aren't any PTM capable devices on the market
> >>yet, but it is supported by the Intel Apollo Lake platform.
> >
> >Can you also add lspci support for the PTM capability?  That will help
> >debugging.
> >
> >Bjorn
> >
> 
> Do I also send it to this list? Or directly to Martin?

Send it to Martin and cc this list.