Re: [PATCH 3/4] PCI: dwc: Add sysfs support for PTM

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Feb 18, 2025 at 08:06:42PM +0530, Manivannan Sadhasivam via B4 Relay wrote:
> From: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx>
> 
> Precision Time Management (PTM) mechanism defined in PCIe spec r6.0,
> sec 6.22 allows precise coordination of timing information across multiple
> components in a PCIe hierarchy with independent local time clocks.
> 
> While the PTM support itself is indicated by the presence of PTM Extended
> Capability structure, Synopsys Designware IPs expose the PTM context
> (timing information) through Vendor Specific Extended Capability (VSEC)
> registers.
> 
> Hence, add the sysfs support to expose the PTM context information to
> userspace from both PCIe RC and EP controllers. Below PTM context are
> exposed through sysfs:
> 
> PCIe RC
> =======
> 
> 1. PTM Local clock
> 2. PTM T2 timestamp
> 3. PTM T3 timestamp
> 4. PTM Context valid
> 
> PCIe EP
> =======
> 
> 1. PTM Local clock
> 2. PTM T1 timestamp
> 3. PTM T4 timestamp
> 4. PTM Master clock
> 5. PTM Context update
> 
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx>
> ---
>  Documentation/ABI/testing/sysfs-platform-dwc-pcie  |  70 ++++++
>  MAINTAINERS                                        |   1 +
>  drivers/pci/controller/dwc/Makefile                |   2 +-
>  drivers/pci/controller/dwc/pcie-designware-ep.c    |   3 +
>  drivers/pci/controller/dwc/pcie-designware-host.c  |   4 +
>  drivers/pci/controller/dwc/pcie-designware-sysfs.c | 278 +++++++++++++++++++++
>  drivers/pci/controller/dwc/pcie-designware.c       |   6 +
>  drivers/pci/controller/dwc/pcie-designware.h       |  22 ++
>  include/linux/pcie-dwc.h                           |   8 +
>  9 files changed, 393 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-platform-dwc-pcie b/Documentation/ABI/testing/sysfs-platform-dwc-pcie
> new file mode 100644
> index 000000000000..6b429108cd09
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-platform-dwc-pcie

Should be a class or just a ptm group in the PCIe controller device? How
generic are those attributes?

> @@ -0,0 +1,70 @@
> +What:		/sys/devices/platform/*/dwc/ptm/ptm_local_clock
> +Date:		February 2025
> +Contact:	Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx>
> +Description:
> +		(RO) PTM local clock in nanoseconds. Applicable for both Root
> +		Complex and Endpoint mode.
> +
> +What:		/sys/devices/platform/*/dwc/ptm/ptm_master_clock
> +Date:		February 2025
> +Contact:	Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx>
> +Description:
> +		(RO) PTM master clock in nanoseconds. Applicable only for
> +		Endpoint mode.
> +
> +What:		/sys/devices/platform/*/dwc/ptm/ptm_t1
> +Date:		February 2025
> +Contact:	Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx>
> +Description:
> +		(RO) PTM T1 timestamp in nanoseconds. Applicable only for
> +		Endpoint mode.
> +
> +What:		/sys/devices/platform/*/dwc/ptm/ptm_t2
> +Date:		February 2025
> +Contact:	Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx>
> +Description:
> +		(RO) PTM T2 timestamp in nanoseconds. Applicable only for
> +		Root Complex mode.
> +
> +What:		/sys/devices/platform/*/dwc/ptm/ptm_t3
> +Date:		February 2025
> +Contact:	Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx>
> +Description:
> +		(RO) PTM T3 timestamp in nanoseconds. Applicable only for
> +		Root Complex mode.
> +
> +What:		/sys/devices/platform/*/dwc/ptm/ptm_t4
> +Date:		February 2025
> +Contact:	Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx>
> +Description:
> +		(RO) PTM T4 timestamp in nanoseconds. Applicable only for
> +		Endpoint mode.
> +
> +What:		/sys/devices/platform/*/dwc/ptm/ptm_context_update
> +Date:		February 2025
> +Contact:	Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx>
> +Description:
> +		(RW) Control the PTM context update mode. Applicable only for
> +		Endpoint mode.
> +
> +		Following values are supported:
> +
> +		* auto = PTM context auto update trigger for every 10ms
> +
> +		* manual = PTM context manual update. Writing 'manual' to this
> +			   file triggers PTM context update (default)
> +
> +What:		/sys/devices/platform/*/dwc/ptm/ptm_context_valid
> +Date:		February 2025
> +Contact:	Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx>
> +Description:
> +		(RW) Control the PTM context validity (local clock timing).
> +		Applicable only for Root Complex mode. PTM context is
> +		invalidated by hardware if the Root Complex enters low power
> +		mode or changes link frequency.
> +
> +		Following values are supported:
> +
> +		* 0 = PTM context invalid (default)
> +
> +		* 1 = PTM context valid
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b4d09d52a750..1c3e21cfbc6e 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -18120,6 +18120,7 @@ M:	Jingoo Han <jingoohan1@xxxxxxxxx>
>  M:	Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx>
>  L:	linux-pci@xxxxxxxxxxxxxxx
>  S:	Maintained
> +F:	Documentation/ABI/testing/sysfs-platform-dwc-pcie
>  F:	Documentation/devicetree/bindings/pci/snps,dw-pcie-ep.yaml
>  F:	Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
>  F:	drivers/pci/controller/dwc/*designware*

[...]

> +
> +static struct attribute *ptm_attrs[] = {
> +	&dev_attr_ptm_context_update.attr,
> +	&dev_attr_ptm_context_valid.attr,
> +	&dev_attr_ptm_local_clock.attr,
> +	&dev_attr_ptm_master_clock.attr,
> +	&dev_attr_ptm_t1.attr,
> +	&dev_attr_ptm_t2.attr,
> +	&dev_attr_ptm_t3.attr,
> +	&dev_attr_ptm_t4.attr,
> +	NULL
> +};
> +
> +static umode_t ptm_attr_visible(struct kobject *kobj, struct attribute *attr,
> +				int n)
> +{
> +	struct device *dev = container_of(kobj, struct device, kobj);
> +	struct dw_pcie *pci = dev_get_drvdata(dev);
> +
> +	/* RC only needs local, t2 and t3 clocks and context_valid */
> +	if ((attr == &dev_attr_ptm_t1.attr && pci->mode == DW_PCIE_RC_TYPE) ||
> +	    (attr == &dev_attr_ptm_t4.attr && pci->mode == DW_PCIE_RC_TYPE) ||
> +	    (attr == &dev_attr_ptm_master_clock.attr && pci->mode == DW_PCIE_RC_TYPE) ||
> +	    (attr == &dev_attr_ptm_context_update.attr && pci->mode == DW_PCIE_RC_TYPE))
> +		return 0;

The pci->mode checks definitely can be refactored to a top-level instead
of being repeated on each line.

> +
> +	/* EP only needs local, master, t1, and t4 clocks and context_update */
> +	if ((attr == &dev_attr_ptm_t2.attr && pci->mode == DW_PCIE_EP_TYPE) ||
> +	    (attr == &dev_attr_ptm_t3.attr && pci->mode == DW_PCIE_EP_TYPE) ||
> +	    (attr == &dev_attr_ptm_context_valid.attr && pci->mode == DW_PCIE_EP_TYPE))
> +		return 0;
> +
> +	return attr->mode;

I think it might be better to register two separate groups, one for RC,
one for EP and use presense of the corresponding capability in the
.is_visible callback to check if the PTM attributes should be visible at
all.

> +}
> +
> +static const struct attribute_group ptm_attr_group = {
> +	.name = "ptm",
> +	.attrs = ptm_attrs,
> +	.is_visible = ptm_attr_visible,
> +};
> +
> +static const struct attribute_group *dwc_pcie_attr_groups[] = {
> +	&ptm_attr_group,
> +	NULL,
> +};
> +
> +static void pcie_designware_sysfs_release(struct device *dev)
> +{
> +	kfree(dev);
> +}
> +
> +void pcie_designware_sysfs_init(struct dw_pcie *pci,
> +				    enum dw_pcie_device_mode mode)
> +{
> +	struct device *dev;
> +	int ret;
> +
> +	/* Check for capabilities before creating sysfs attrbutes */
> +	ret = dw_pcie_find_ptm_capability(pci);
> +	if (!ret) {
> +		dev_dbg(pci->dev, "PTM capability not present\n");
> +		return;
> +	}
> +
> +	pci->ptm_vsec_offset = ret;
> +	pci->mode = mode;
> +
> +	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
> +	if (!dev)
> +		return;
> +
> +	device_initialize(dev);
> +	dev->groups = dwc_pcie_attr_groups;
> +	dev->release = pcie_designware_sysfs_release;
> +	dev->parent = pci->dev;
> +	dev_set_drvdata(dev, pci);
> +
> +	ret = dev_set_name(dev, "dwc");
> +	if (ret)
> +		goto err_free;
> +
> +	ret = device_add(dev);
> +	if (ret)
> +		goto err_free;
> +
> +	pci->sysfs_dev = dev;

Why do you need to add a new device under the PCIe controller?

> +
> +	return;
> +
> +err_free:
> +	put_device(dev);
> +}
> +
> +void pcie_designware_sysfs_exit(struct dw_pcie *pci)
> +{
> +	if (pci->sysfs_dev)
> +		device_unregister(pci->sysfs_dev);
> +}
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index a7c0671c6715..30825ec0648e 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -323,6 +323,12 @@ static u16 dw_pcie_find_vsec_capability(struct dw_pcie *pci,
>  	return 0;
>  }
>  
> +u16 dw_pcie_find_ptm_capability(struct dw_pcie *pci)
> +{
> +	return dw_pcie_find_vsec_capability(pci, dwc_pcie_ptm_vsec_ids);
> +}
> +EXPORT_SYMBOL_GPL(dw_pcie_find_ptm_capability);

This API should go into the previous patch. Otherwise it will result in
unused function warnings.

> +
>  int dw_pcie_read(void __iomem *addr, int size, u32 *val)
>  {
>  	if (!IS_ALIGNED((uintptr_t)addr, size)) {
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 501d9ddfea16..7d3cbdce37c8 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -260,6 +260,21 @@
>  
>  #define PCIE_RAS_DES_EVENT_COUNTER_DATA		0xc
>  
> +/* PTM register definitions */
> +#define PTM_RES_REQ_CTRL		0x8
> +#define PTM_RES_CCONTEXT_VALID		BIT(0)
> +#define PTM_REQ_AUTO_UPDATE_ENABLED	BIT(0)
> +#define PTM_REQ_START_UPDATE		BIT(1)
> +
> +#define PTM_LOCAL_LSB			0x10
> +#define PTM_LOCAL_MSB			0x14
> +#define PTM_T1_T2_LSB			0x18
> +#define PTM_T1_T2_MSB			0x1c
> +#define PTM_T3_T4_LSB			0x28
> +#define PTM_T3_T4_MSB			0x2c
> +#define PTM_MASTER_LSB			0x38
> +#define PTM_MASTER_MSB			0x3c
> +
>  /*
>   * The default address offset between dbi_base and atu_base. Root controller
>   * drivers are not required to initialize atu_base if the offset matches this
> @@ -439,6 +454,7 @@ struct dw_pcie_ops {
>  
>  struct dw_pcie {
>  	struct device		*dev;
> +	struct device		*sysfs_dev;
>  	void __iomem		*dbi_base;
>  	resource_size_t		dbi_phys_addr;
>  	void __iomem		*dbi_base2;
> @@ -464,6 +480,8 @@ struct dw_pcie {
>  	struct reset_control_bulk_data	app_rsts[DW_PCIE_NUM_APP_RSTS];
>  	struct reset_control_bulk_data	core_rsts[DW_PCIE_NUM_CORE_RSTS];
>  	struct gpio_desc		*pe_rst;
> +	u16			ptm_vsec_offset;
> +	enum			dw_pcie_device_mode mode;
>  	bool			suspended;
>  };
>  
> @@ -478,6 +496,7 @@ void dw_pcie_version_detect(struct dw_pcie *pci);
>  
>  u8 dw_pcie_find_capability(struct dw_pcie *pci, u8 cap);
>  u16 dw_pcie_find_ext_capability(struct dw_pcie *pci, u8 cap);
> +u16 dw_pcie_find_ptm_capability(struct dw_pcie *pci);
>  
>  int dw_pcie_read(void __iomem *addr, int size, u32 *val);
>  int dw_pcie_write(void __iomem *addr, int size, u32 val);
> @@ -499,6 +518,9 @@ void dw_pcie_setup(struct dw_pcie *pci);
>  void dw_pcie_iatu_detect(struct dw_pcie *pci);
>  int dw_pcie_edma_detect(struct dw_pcie *pci);
>  void dw_pcie_edma_remove(struct dw_pcie *pci);
> +void pcie_designware_sysfs_init(struct dw_pcie *pci,
> +				enum dw_pcie_device_mode mode);
> +void pcie_designware_sysfs_exit(struct dw_pcie *pci);
>  
>  static inline void dw_pcie_writel_dbi(struct dw_pcie *pci, u32 reg, u32 val)
>  {
> diff --git a/include/linux/pcie-dwc.h b/include/linux/pcie-dwc.h
> index 261ae11d75a4..13835896290a 100644
> --- a/include/linux/pcie-dwc.h
> +++ b/include/linux/pcie-dwc.h
> @@ -31,4 +31,12 @@ static const struct dwc_pcie_vsec_id dwc_pcie_pmu_vsec_ids[] = {
>  	{} /* terminator */
>  };
>  
> +static const struct dwc_pcie_vsec_id dwc_pcie_ptm_vsec_ids[] = {
> +	{ .vendor_id = PCI_VENDOR_ID_QCOM, /* EP */
> +	  .vsec_id = 0x03, .vsec_rev = 0x1 },
> +	{ .vendor_id = PCI_VENDOR_ID_QCOM, /* RC */
> +	  .vsec_id = 0x04, .vsec_rev = 0x1 },
> +	{ }
> +};
> +
>  #endif /* LINUX_PCIE_DWC_H */
> 
> -- 
> 2.25.1
> 
> 

-- 
With best wishes
Dmitry




[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux