[PATCH] PCI: dwc: Add the sysfs property to provide the LTSSM status of the PCIe link

Hans Zhang posted 1 patch 11 months ago
.../pci/controller/dwc/pcie-designware-host.c | 86 +++++++++++++++++++
drivers/pci/controller/dwc/pcie-designware.h  | 32 +++++++
drivers/pci/pci-sysfs.c                       |  3 +
drivers/pci/pci.h                             |  4 +
4 files changed, 125 insertions(+)
[PATCH] PCI: dwc: Add the sysfs property to provide the LTSSM status of the PCIe link
Posted by Hans Zhang 11 months ago
Add the sysfs property to provide a view of the current link's LTSSM
status from the root port device.

  /sys/bus/pci/devices/<dev>/ltssm_status

Signed-off-by: Hans Zhang <18255117159@163.com>
---
 .../pci/controller/dwc/pcie-designware-host.c | 86 +++++++++++++++++++
 drivers/pci/controller/dwc/pcie-designware.h  | 32 +++++++
 drivers/pci/pci-sysfs.c                       |  3 +
 drivers/pci/pci.h                             |  4 +
 4 files changed, 125 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index d2291c3ceb8b..8ec0e35cca8f 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -418,6 +418,92 @@ static void dw_pcie_host_request_msg_tlp_res(struct dw_pcie_rp *pp)
 	}
 }
 
+static char *dw_ltssm_sts_string(enum dw_pcie_ltssm ltssm)
+{
+	char *str;
+
+	switch (ltssm) {
+#define DW_PCIE_LTSSM_NAME(n) case n: str = #n; break
+	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_DETECT_QUIET);
+	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_DETECT_ACT);
+	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_POLL_ACTIVE);
+	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_POLL_COMPLIANCE);
+	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_POLL_CONFIG);
+	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_PRE_DETECT_QUIET);
+	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_DETECT_WAIT);
+	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_CFG_LINKWD_START);
+	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_CFG_LINKWD_ACEPT);
+	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_CFG_LANENUM_WAI);
+	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_CFG_LANENUM_ACEPT);
+	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_CFG_COMPLETE);
+	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_CFG_IDLE);
+	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_RCVRY_LOCK);
+	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_RCVRY_SPEED);
+	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_RCVRY_RCVRCFG);
+	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_RCVRY_IDLE);
+	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_L0);
+	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_L0S);
+	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_L123_SEND_EIDLE);
+	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_L1_IDLE);
+	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_L2_IDLE);
+	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_L2_WAKE);
+	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_DISABLED_ENTRY);
+	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_DISABLED_IDLE);
+	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_DISABLED);
+	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_LPBK_ENTRY);
+	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_LPBK_ACTIVE);
+	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_LPBK_EXIT);
+	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_LPBK_EXIT_TIMEOUT);
+	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_HOT_RESET_ENTRY);
+	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_HOT_RESET);
+	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_RCVRY_EQ0);
+	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_RCVRY_EQ1);
+	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_RCVRY_EQ2);
+	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_RCVRY_EQ3);
+	default:
+		str = "DW_PCIE_LTSSM_UNKNOWN";
+		break;
+	}
+
+	return str;
+}
+
+static ssize_t ltssm_status_show(struct device *dev,
+				 struct device_attribute *attr, char *buf)
+{
+	struct pci_dev *pdev = to_pci_dev(dev);
+	struct pci_host_bridge *bridge = pci_find_host_bridge(pdev->bus);
+	struct dw_pcie_rp *pp = bridge->sysdata;
+	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+
+	return sysfs_emit(buf, "%s\n",
+			  dw_ltssm_sts_string(dw_pcie_get_ltssm(pci)));
+}
+static DEVICE_ATTR_RO(ltssm_status);
+
+static struct attribute *ltssm_status_attrs[] __ro_after_init = {
+	&dev_attr_ltssm_status.attr, NULL
+};
+
+static umode_t ltssm_status_attrs_are_visible(struct kobject *kobj,
+					      struct attribute *a, int n)
+{
+	struct device *dev = kobj_to_dev(kobj);
+	struct pci_dev *pdev = to_pci_dev(dev);
+
+	if ((a == &dev_attr_ltssm_status.attr) &&
+	    ((pci_pcie_type(pdev) != PCI_EXP_TYPE_ROOT_PORT) &&
+	     (pci_pcie_type(pdev) != PCI_EXP_TYPE_RC_EC)))
+		return 0;
+
+	return a->mode;
+}
+
+const struct attribute_group dw_ltssm_status_attr_group = {
+	.attrs = ltssm_status_attrs,
+	.is_visible = ltssm_status_attrs_are_visible,
+};
+
 int dw_pcie_host_init(struct dw_pcie_rp *pp)
 {
 	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 347ab74ac35a..fb7e3c14e523 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -330,8 +330,40 @@ enum dw_pcie_ltssm {
 	/* Need to align with PCIE_PORT_DEBUG0 bits 0:5 */
 	DW_PCIE_LTSSM_DETECT_QUIET = 0x0,
 	DW_PCIE_LTSSM_DETECT_ACT = 0x1,
+	DW_PCIE_LTSSM_POLL_ACTIVE = 0x2,
+	DW_PCIE_LTSSM_POLL_COMPLIANCE = 0x3,
+	DW_PCIE_LTSSM_POLL_CONFIG = 0x4,
+	DW_PCIE_LTSSM_PRE_DETECT_QUIET = 0x5,
+	DW_PCIE_LTSSM_DETECT_WAIT = 0x6,
+	DW_PCIE_LTSSM_CFG_LINKWD_START = 0x7,
+	DW_PCIE_LTSSM_CFG_LINKWD_ACEPT = 0x8,
+	DW_PCIE_LTSSM_CFG_LANENUM_WAI = 0x9,
+	DW_PCIE_LTSSM_CFG_LANENUM_ACEPT = 0xa,
+	DW_PCIE_LTSSM_CFG_COMPLETE = 0xb,
+	DW_PCIE_LTSSM_CFG_IDLE = 0xc,
+	DW_PCIE_LTSSM_RCVRY_LOCK = 0xd,
+	DW_PCIE_LTSSM_RCVRY_SPEED = 0xe,
+	DW_PCIE_LTSSM_RCVRY_RCVRCFG = 0xf,
+	DW_PCIE_LTSSM_RCVRY_IDLE = 0x10,
 	DW_PCIE_LTSSM_L0 = 0x11,
+	DW_PCIE_LTSSM_L0S = 0x12,
+	DW_PCIE_LTSSM_L123_SEND_EIDLE = 0x13,
+	DW_PCIE_LTSSM_L1_IDLE = 0x14,
 	DW_PCIE_LTSSM_L2_IDLE = 0x15,
+	DW_PCIE_LTSSM_L2_WAKE = 0x16,
+	DW_PCIE_LTSSM_DISABLED_ENTRY = 0x17,
+	DW_PCIE_LTSSM_DISABLED_IDLE = 0x18,
+	DW_PCIE_LTSSM_DISABLED = 0x19,
+	DW_PCIE_LTSSM_LPBK_ENTRY = 0x1a,
+	DW_PCIE_LTSSM_LPBK_ACTIVE = 0x1b,
+	DW_PCIE_LTSSM_LPBK_EXIT = 0x1c,
+	DW_PCIE_LTSSM_LPBK_EXIT_TIMEOUT = 0x1d,
+	DW_PCIE_LTSSM_HOT_RESET_ENTRY = 0x1e,
+	DW_PCIE_LTSSM_HOT_RESET = 0x1f,
+	DW_PCIE_LTSSM_RCVRY_EQ0 = 0x20,
+	DW_PCIE_LTSSM_RCVRY_EQ1 = 0x21,
+	DW_PCIE_LTSSM_RCVRY_EQ2 = 0x22,
+	DW_PCIE_LTSSM_RCVRY_EQ3 = 0x23,
 
 	DW_PCIE_LTSSM_UNKNOWN = 0xFFFFFFFF,
 };
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 7679d75d71e5..c23d188323f4 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -1696,6 +1696,9 @@ const struct attribute_group *pci_dev_attr_groups[] = {
 #endif
 #ifdef CONFIG_PCIEASPM
 	&aspm_ctrl_attr_group,
+#endif
+#ifdef CONFIG_PCIE_DW_HOST
+	&dw_ltssm_status_attr_group,
 #endif
 	NULL,
 };
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 2e40fc63ba31..3775841b5714 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -960,6 +960,10 @@ static inline pci_power_t acpi_pci_choose_state(struct pci_dev *pdev)
 extern const struct attribute_group aspm_ctrl_attr_group;
 #endif
 
+#ifdef CONFIG_PCIE_DW_HOST
+extern const struct attribute_group dw_ltssm_status_attr_group;
+#endif
+
 extern const struct attribute_group pci_dev_reset_method_attr_group;
 
 #ifdef CONFIG_X86_INTEL_MID

base-commit: 7004a2e46d1693848370809aa3d9c340a209edbb
-- 
2.25.1
Re: [PATCH] PCI: dwc: Add the sysfs property to provide the LTSSM status of the PCIe link
Posted by Manivannan Sadhasivam 10 months, 4 weeks ago
On Thu, Jan 23, 2025 at 03:13:26PM +0800, Hans Zhang wrote:
> Add the sysfs property to provide a view of the current link's LTSSM
> status from the root port device.
> 
>   /sys/bus/pci/devices/<dev>/ltssm_status
> 

The LTSSM values coming out of DWC is related to host bridgei, isn't it? If so,
you cannot expose them under generic PCI sysfs. You should expose these as
debugfs attributes as done in DWC glue drivers like pcie-qcom.

- Mani

> Signed-off-by: Hans Zhang <18255117159@163.com>
> ---
>  .../pci/controller/dwc/pcie-designware-host.c | 86 +++++++++++++++++++
>  drivers/pci/controller/dwc/pcie-designware.h  | 32 +++++++
>  drivers/pci/pci-sysfs.c                       |  3 +
>  drivers/pci/pci.h                             |  4 +
>  4 files changed, 125 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index d2291c3ceb8b..8ec0e35cca8f 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -418,6 +418,92 @@ static void dw_pcie_host_request_msg_tlp_res(struct dw_pcie_rp *pp)
>  	}
>  }
>  
> +static char *dw_ltssm_sts_string(enum dw_pcie_ltssm ltssm)
> +{
> +	char *str;
> +
> +	switch (ltssm) {
> +#define DW_PCIE_LTSSM_NAME(n) case n: str = #n; break
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_DETECT_QUIET);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_DETECT_ACT);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_POLL_ACTIVE);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_POLL_COMPLIANCE);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_POLL_CONFIG);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_PRE_DETECT_QUIET);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_DETECT_WAIT);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_CFG_LINKWD_START);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_CFG_LINKWD_ACEPT);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_CFG_LANENUM_WAI);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_CFG_LANENUM_ACEPT);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_CFG_COMPLETE);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_CFG_IDLE);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_RCVRY_LOCK);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_RCVRY_SPEED);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_RCVRY_RCVRCFG);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_RCVRY_IDLE);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_L0);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_L0S);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_L123_SEND_EIDLE);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_L1_IDLE);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_L2_IDLE);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_L2_WAKE);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_DISABLED_ENTRY);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_DISABLED_IDLE);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_DISABLED);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_LPBK_ENTRY);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_LPBK_ACTIVE);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_LPBK_EXIT);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_LPBK_EXIT_TIMEOUT);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_HOT_RESET_ENTRY);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_HOT_RESET);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_RCVRY_EQ0);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_RCVRY_EQ1);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_RCVRY_EQ2);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_RCVRY_EQ3);
> +	default:
> +		str = "DW_PCIE_LTSSM_UNKNOWN";
> +		break;
> +	}
> +
> +	return str;
> +}
> +
> +static ssize_t ltssm_status_show(struct device *dev,
> +				 struct device_attribute *attr, char *buf)
> +{
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +	struct pci_host_bridge *bridge = pci_find_host_bridge(pdev->bus);
> +	struct dw_pcie_rp *pp = bridge->sysdata;
> +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> +
> +	return sysfs_emit(buf, "%s\n",
> +			  dw_ltssm_sts_string(dw_pcie_get_ltssm(pci)));
> +}
> +static DEVICE_ATTR_RO(ltssm_status);
> +
> +static struct attribute *ltssm_status_attrs[] __ro_after_init = {
> +	&dev_attr_ltssm_status.attr, NULL
> +};
> +
> +static umode_t ltssm_status_attrs_are_visible(struct kobject *kobj,
> +					      struct attribute *a, int n)
> +{
> +	struct device *dev = kobj_to_dev(kobj);
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +
> +	if ((a == &dev_attr_ltssm_status.attr) &&
> +	    ((pci_pcie_type(pdev) != PCI_EXP_TYPE_ROOT_PORT) &&
> +	     (pci_pcie_type(pdev) != PCI_EXP_TYPE_RC_EC)))
> +		return 0;
> +
> +	return a->mode;
> +}
> +
> +const struct attribute_group dw_ltssm_status_attr_group = {
> +	.attrs = ltssm_status_attrs,
> +	.is_visible = ltssm_status_attrs_are_visible,
> +};
> +
>  int dw_pcie_host_init(struct dw_pcie_rp *pp)
>  {
>  	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 347ab74ac35a..fb7e3c14e523 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -330,8 +330,40 @@ enum dw_pcie_ltssm {
>  	/* Need to align with PCIE_PORT_DEBUG0 bits 0:5 */
>  	DW_PCIE_LTSSM_DETECT_QUIET = 0x0,
>  	DW_PCIE_LTSSM_DETECT_ACT = 0x1,
> +	DW_PCIE_LTSSM_POLL_ACTIVE = 0x2,
> +	DW_PCIE_LTSSM_POLL_COMPLIANCE = 0x3,
> +	DW_PCIE_LTSSM_POLL_CONFIG = 0x4,
> +	DW_PCIE_LTSSM_PRE_DETECT_QUIET = 0x5,
> +	DW_PCIE_LTSSM_DETECT_WAIT = 0x6,
> +	DW_PCIE_LTSSM_CFG_LINKWD_START = 0x7,
> +	DW_PCIE_LTSSM_CFG_LINKWD_ACEPT = 0x8,
> +	DW_PCIE_LTSSM_CFG_LANENUM_WAI = 0x9,
> +	DW_PCIE_LTSSM_CFG_LANENUM_ACEPT = 0xa,
> +	DW_PCIE_LTSSM_CFG_COMPLETE = 0xb,
> +	DW_PCIE_LTSSM_CFG_IDLE = 0xc,
> +	DW_PCIE_LTSSM_RCVRY_LOCK = 0xd,
> +	DW_PCIE_LTSSM_RCVRY_SPEED = 0xe,
> +	DW_PCIE_LTSSM_RCVRY_RCVRCFG = 0xf,
> +	DW_PCIE_LTSSM_RCVRY_IDLE = 0x10,
>  	DW_PCIE_LTSSM_L0 = 0x11,
> +	DW_PCIE_LTSSM_L0S = 0x12,
> +	DW_PCIE_LTSSM_L123_SEND_EIDLE = 0x13,
> +	DW_PCIE_LTSSM_L1_IDLE = 0x14,
>  	DW_PCIE_LTSSM_L2_IDLE = 0x15,
> +	DW_PCIE_LTSSM_L2_WAKE = 0x16,
> +	DW_PCIE_LTSSM_DISABLED_ENTRY = 0x17,
> +	DW_PCIE_LTSSM_DISABLED_IDLE = 0x18,
> +	DW_PCIE_LTSSM_DISABLED = 0x19,
> +	DW_PCIE_LTSSM_LPBK_ENTRY = 0x1a,
> +	DW_PCIE_LTSSM_LPBK_ACTIVE = 0x1b,
> +	DW_PCIE_LTSSM_LPBK_EXIT = 0x1c,
> +	DW_PCIE_LTSSM_LPBK_EXIT_TIMEOUT = 0x1d,
> +	DW_PCIE_LTSSM_HOT_RESET_ENTRY = 0x1e,
> +	DW_PCIE_LTSSM_HOT_RESET = 0x1f,
> +	DW_PCIE_LTSSM_RCVRY_EQ0 = 0x20,
> +	DW_PCIE_LTSSM_RCVRY_EQ1 = 0x21,
> +	DW_PCIE_LTSSM_RCVRY_EQ2 = 0x22,
> +	DW_PCIE_LTSSM_RCVRY_EQ3 = 0x23,
>  
>  	DW_PCIE_LTSSM_UNKNOWN = 0xFFFFFFFF,
>  };
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index 7679d75d71e5..c23d188323f4 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -1696,6 +1696,9 @@ const struct attribute_group *pci_dev_attr_groups[] = {
>  #endif
>  #ifdef CONFIG_PCIEASPM
>  	&aspm_ctrl_attr_group,
> +#endif
> +#ifdef CONFIG_PCIE_DW_HOST
> +	&dw_ltssm_status_attr_group,
>  #endif
>  	NULL,
>  };
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 2e40fc63ba31..3775841b5714 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -960,6 +960,10 @@ static inline pci_power_t acpi_pci_choose_state(struct pci_dev *pdev)
>  extern const struct attribute_group aspm_ctrl_attr_group;
>  #endif
>  
> +#ifdef CONFIG_PCIE_DW_HOST
> +extern const struct attribute_group dw_ltssm_status_attr_group;
> +#endif
> +
>  extern const struct attribute_group pci_dev_reset_method_attr_group;
>  
>  #ifdef CONFIG_X86_INTEL_MID
> 
> base-commit: 7004a2e46d1693848370809aa3d9c340a209edbb
> -- 
> 2.25.1
> 

-- 
மணிவண்ணன் சதாசிவம்
Re: [PATCH] PCI: dwc: Add the sysfs property to provide the LTSSM status of the PCIe link
Posted by Bjorn Helgaas 11 months ago
On Thu, Jan 23, 2025 at 03:13:26PM +0800, Hans Zhang wrote:
> Add the sysfs property to provide a view of the current link's LTSSM
> status from the root port device.
> 
>   /sys/bus/pci/devices/<dev>/ltssm_status

Would need a rationale, i.e., what benefit this provides.  Obviously
this is currently only implemented for DWC-based controllers and
probably not ever available for ACPI or other generic host bridges.

Also documentation somewhere in
Documentation/ABI/testing/sysfs-bus-pci*.

LTSSM is applicable to all Downstream Ports, including both Root Ports
and Switch Downstream Ports, but in general I doubt this information
is available for Switches in any generic way.

> +++ b/drivers/pci/pci-sysfs.c
> @@ -1696,6 +1696,9 @@ const struct attribute_group *pci_dev_attr_groups[] = {
>  #endif
>  #ifdef CONFIG_PCIEASPM
>  	&aspm_ctrl_attr_group,
> +#endif
> +#ifdef CONFIG_PCIE_DW_HOST
> +	&dw_ltssm_status_attr_group,
>  #endif

I'm not convinced of the value of potentially dozens of device- or
vendor-specific additions like this.

Bjorn
Re: [PATCH] PCI: dwc: Add the sysfs property to provide the LTSSM status of the PCIe link
Posted by Hans Zhang 10 months, 3 weeks ago

On 2025/1/24 00:49, Bjorn Helgaas wrote:
> On Thu, Jan 23, 2025 at 03:13:26PM +0800, Hans Zhang wrote:
>> Add the sysfs property to provide a view of the current link's LTSSM
>> status from the root port device.
>>
>>    /sys/bus/pci/devices/<dev>/ltssm_status
> 
> Would need a rationale, i.e., what benefit this provides.  Obviously
> this is currently only implemented for DWC-based controllers and
> probably not ever available for ACPI or other generic host bridges.
> 
> Also documentation somewhere in
> Documentation/ABI/testing/sysfs-bus-pci*.
> 
> LTSSM is applicable to all Downstream Ports, including both Root Ports
> and Switch Downstream Ports, but in general I doubt this information
> is available for Switches in any generic way.
> 
>> +++ b/drivers/pci/pci-sysfs.c
>> @@ -1696,6 +1696,9 @@ const struct attribute_group *pci_dev_attr_groups[] = {
>>   #endif
>>   #ifdef CONFIG_PCIEASPM
>>   	&aspm_ctrl_attr_group,
>> +#endif
>> +#ifdef CONFIG_PCIE_DW_HOST
>> +	&dw_ltssm_status_attr_group,
>>   #endif
> 
> I'm not convinced of the value of potentially dozens of device- or
> vendor-specific additions like this.


At present, someone has submitted the dwc common debugfs driver.

dwc common driver: add debugfs
https://lore.kernel.org/linux-pci/20250121111421.35437-3-shradha.t@samsung.com/


I will make a patch version based on this patch.

Best regards
Hans
Re: [PATCH] PCI: dwc: Add the sysfs property to provide the LTSSM status of the PCIe link
Posted by Frank Li 11 months ago
On Thu, Jan 23, 2025 at 03:13:26PM +0800, Hans Zhang wrote:
> Add the sysfs property to provide a view of the current link's LTSSM
> status from the root port device.
>
>   /sys/bus/pci/devices/<dev>/ltssm_status
>

Thank, I like this.

> Signed-off-by: Hans Zhang <18255117159@163.com>
> ---
>  .../pci/controller/dwc/pcie-designware-host.c | 86 +++++++++++++++++++
>  drivers/pci/controller/dwc/pcie-designware.h  | 32 +++++++
>  drivers/pci/pci-sysfs.c                       |  3 +
>  drivers/pci/pci.h                             |  4 +
>  4 files changed, 125 insertions(+)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index d2291c3ceb8b..8ec0e35cca8f 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -418,6 +418,92 @@ static void dw_pcie_host_request_msg_tlp_res(struct dw_pcie_rp *pp)
>  	}
>  }
>
> +static char *dw_ltssm_sts_string(enum dw_pcie_ltssm ltssm)
> +{
> +	char *str;
> +
> +	switch (ltssm) {
> +#define DW_PCIE_LTSSM_NAME(n) case n: str = #n; break
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_DETECT_QUIET);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_DETECT_ACT);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_POLL_ACTIVE);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_POLL_COMPLIANCE);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_POLL_CONFIG);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_PRE_DETECT_QUIET);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_DETECT_WAIT);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_CFG_LINKWD_START);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_CFG_LINKWD_ACEPT);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_CFG_LANENUM_WAI);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_CFG_LANENUM_ACEPT);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_CFG_COMPLETE);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_CFG_IDLE);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_RCVRY_LOCK);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_RCVRY_SPEED);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_RCVRY_RCVRCFG);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_RCVRY_IDLE);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_L0);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_L0S);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_L123_SEND_EIDLE);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_L1_IDLE);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_L2_IDLE);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_L2_WAKE);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_DISABLED_ENTRY);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_DISABLED_IDLE);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_DISABLED);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_LPBK_ENTRY);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_LPBK_ACTIVE);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_LPBK_EXIT);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_LPBK_EXIT_TIMEOUT);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_HOT_RESET_ENTRY);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_HOT_RESET);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_RCVRY_EQ0);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_RCVRY_EQ1);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_RCVRY_EQ2);
> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_RCVRY_EQ3);
> +	default:
> +		str = "DW_PCIE_LTSSM_UNKNOWN";
> +		break;

I prefer
const char * str[] =
{
	"detect_quitet",
	"detect_act",
	...
}

	return str[ltssm];

Or
	#define DW_PCIE_LTSSM_NAME(n) case n: return #n;
	...
	default:
		return "DW_PCIE_LTSSM_UNKNOWN";
> +	}
> +
> +	return str;
> +}
> +
> +static ssize_t ltssm_status_show(struct device *dev,
> +				 struct device_attribute *attr, char *buf)
> +{
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +	struct pci_host_bridge *bridge = pci_find_host_bridge(pdev->bus);
> +	struct dw_pcie_rp *pp = bridge->sysdata;
> +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> +
> +	return sysfs_emit(buf, "%s\n",
> +			  dw_ltssm_sts_string(dw_pcie_get_ltssm(pci)));

Suggest dump raw value also

val = dw_pcie_get_ltssm(pci);
return sysfs_emit(buf, "%s (0x%02x)\n",
		  dw_ltssm_sts_string(val), val);


Frank

> +}
> +static DEVICE_ATTR_RO(ltssm_status);
> +
> +static struct attribute *ltssm_status_attrs[] __ro_after_init = {
> +	&dev_attr_ltssm_status.attr, NULL
> +};
> +
> +static umode_t ltssm_status_attrs_are_visible(struct kobject *kobj,
> +					      struct attribute *a, int n)
> +{
> +	struct device *dev = kobj_to_dev(kobj);
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +
> +	if ((a == &dev_attr_ltssm_status.attr) &&
> +	    ((pci_pcie_type(pdev) != PCI_EXP_TYPE_ROOT_PORT) &&
> +	     (pci_pcie_type(pdev) != PCI_EXP_TYPE_RC_EC)))
> +		return 0;
> +
> +	return a->mode;
> +}
> +
> +const struct attribute_group dw_ltssm_status_attr_group = {
> +	.attrs = ltssm_status_attrs,
> +	.is_visible = ltssm_status_attrs_are_visible,
> +};
> +
>  int dw_pcie_host_init(struct dw_pcie_rp *pp)
>  {
>  	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 347ab74ac35a..fb7e3c14e523 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -330,8 +330,40 @@ enum dw_pcie_ltssm {
>  	/* Need to align with PCIE_PORT_DEBUG0 bits 0:5 */
>  	DW_PCIE_LTSSM_DETECT_QUIET = 0x0,
>  	DW_PCIE_LTSSM_DETECT_ACT = 0x1,
> +	DW_PCIE_LTSSM_POLL_ACTIVE = 0x2,
> +	DW_PCIE_LTSSM_POLL_COMPLIANCE = 0x3,
> +	DW_PCIE_LTSSM_POLL_CONFIG = 0x4,
> +	DW_PCIE_LTSSM_PRE_DETECT_QUIET = 0x5,
> +	DW_PCIE_LTSSM_DETECT_WAIT = 0x6,
> +	DW_PCIE_LTSSM_CFG_LINKWD_START = 0x7,
> +	DW_PCIE_LTSSM_CFG_LINKWD_ACEPT = 0x8,
> +	DW_PCIE_LTSSM_CFG_LANENUM_WAI = 0x9,
> +	DW_PCIE_LTSSM_CFG_LANENUM_ACEPT = 0xa,
> +	DW_PCIE_LTSSM_CFG_COMPLETE = 0xb,
> +	DW_PCIE_LTSSM_CFG_IDLE = 0xc,
> +	DW_PCIE_LTSSM_RCVRY_LOCK = 0xd,
> +	DW_PCIE_LTSSM_RCVRY_SPEED = 0xe,
> +	DW_PCIE_LTSSM_RCVRY_RCVRCFG = 0xf,
> +	DW_PCIE_LTSSM_RCVRY_IDLE = 0x10,
>  	DW_PCIE_LTSSM_L0 = 0x11,
> +	DW_PCIE_LTSSM_L0S = 0x12,
> +	DW_PCIE_LTSSM_L123_SEND_EIDLE = 0x13,
> +	DW_PCIE_LTSSM_L1_IDLE = 0x14,
>  	DW_PCIE_LTSSM_L2_IDLE = 0x15,
> +	DW_PCIE_LTSSM_L2_WAKE = 0x16,
> +	DW_PCIE_LTSSM_DISABLED_ENTRY = 0x17,
> +	DW_PCIE_LTSSM_DISABLED_IDLE = 0x18,
> +	DW_PCIE_LTSSM_DISABLED = 0x19,
> +	DW_PCIE_LTSSM_LPBK_ENTRY = 0x1a,
> +	DW_PCIE_LTSSM_LPBK_ACTIVE = 0x1b,
> +	DW_PCIE_LTSSM_LPBK_EXIT = 0x1c,
> +	DW_PCIE_LTSSM_LPBK_EXIT_TIMEOUT = 0x1d,
> +	DW_PCIE_LTSSM_HOT_RESET_ENTRY = 0x1e,
> +	DW_PCIE_LTSSM_HOT_RESET = 0x1f,
> +	DW_PCIE_LTSSM_RCVRY_EQ0 = 0x20,
> +	DW_PCIE_LTSSM_RCVRY_EQ1 = 0x21,
> +	DW_PCIE_LTSSM_RCVRY_EQ2 = 0x22,
> +	DW_PCIE_LTSSM_RCVRY_EQ3 = 0x23,
>
>  	DW_PCIE_LTSSM_UNKNOWN = 0xFFFFFFFF,
>  };
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index 7679d75d71e5..c23d188323f4 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -1696,6 +1696,9 @@ const struct attribute_group *pci_dev_attr_groups[] = {
>  #endif
>  #ifdef CONFIG_PCIEASPM
>  	&aspm_ctrl_attr_group,
> +#endif
> +#ifdef CONFIG_PCIE_DW_HOST
> +	&dw_ltssm_status_attr_group,
>  #endif
>  	NULL,
>  };
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 2e40fc63ba31..3775841b5714 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -960,6 +960,10 @@ static inline pci_power_t acpi_pci_choose_state(struct pci_dev *pdev)
>  extern const struct attribute_group aspm_ctrl_attr_group;
>  #endif
>
> +#ifdef CONFIG_PCIE_DW_HOST
> +extern const struct attribute_group dw_ltssm_status_attr_group;
> +#endif
> +
>  extern const struct attribute_group pci_dev_reset_method_attr_group;
>
>  #ifdef CONFIG_X86_INTEL_MID
>
> base-commit: 7004a2e46d1693848370809aa3d9c340a209edbb
> --
> 2.25.1
>
Re: [PATCH] PCI: dwc: Add the sysfs property to provide the LTSSM status of the PCIe link
Posted by Hans Zhang 10 months, 4 weeks ago

On 2025/1/24 00:16, Frank Li wrote:
>> +static char *dw_ltssm_sts_string(enum dw_pcie_ltssm ltssm)
>> +{
>> +	char *str;
>> +
>> +	switch (ltssm) {
>> +#define DW_PCIE_LTSSM_NAME(n) case n: str = #n; break
>> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_DETECT_QUIET);
>> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_DETECT_ACT);
>> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_POLL_ACTIVE);
>> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_POLL_COMPLIANCE);
>> +	...
>> +	default:
>> +		str = "DW_PCIE_LTSSM_UNKNOWN";
>> +		break;
> 
> I prefer
> const char * str[] =
> {
> 	"detect_quitet",
> 	"detect_act",
> 	...
> }
> 
> 	return str[ltssm];
> 
> Or
> 	#define DW_PCIE_LTSSM_NAME(n) case n: return #n;
> 	...
> 	default:
> 		return "DW_PCIE_LTSSM_UNKNOWN";
Hi Frank,

I considered the two methods you mentioned before I submitted this patch.

The first, I think, will increase the memory overhead.

+static const char * const dw_pcie_ltssm_str[] = {
+	[DW_PCIE_LTSSM_DETECT_QUIET] = "DETECT_QUIET",
+	[DW_PCIE_LTSSM_DETECT_ACT] = "DETECT_ACT",
+	[DW_PCIE_LTSSM_POLL_ACTIVE] = "POLL_ACTIVE",
+	[DW_PCIE_LTSSM_POLL_COMPLIANCE] = "POLL_COMPLIANCE",
	...


The second, ./scripts/checkpatch.pl checks will have a warning

WARNING: Macros with flow control statements should be avoided
#121: FILE: drivers/pci/controller/dwc/pcie-designware.h:329:
+#define DW_PCIE_LTSSM_NAME(n) case n: return #n


>> +static ssize_t ltssm_status_show(struct device *dev,
>> +				 struct device_attribute *attr, char *buf)
>> +{
>> +	struct pci_dev *pdev = to_pci_dev(dev);
>> +	struct pci_host_bridge *bridge = pci_find_host_bridge(pdev->bus);
>> +	struct dw_pcie_rp *pp = bridge->sysdata;
>> +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>> +
>> +	return sysfs_emit(buf, "%s\n",
>> +			  dw_ltssm_sts_string(dw_pcie_get_ltssm(pci)));
> 
> Suggest dump raw value also
> 
> val = dw_pcie_get_ltssm(pci);
> return sysfs_emit(buf, "%s (0x%02x)\n",
> 		  dw_ltssm_sts_string(val), val);

Thanks, i think it's a good idea.

Best regards
Hans
Re: [PATCH] PCI: dwc: Add the sysfs property to provide the LTSSM status of the PCIe link
Posted by Frank Li 10 months, 4 weeks ago
On Fri, Jan 24, 2025 at 09:12:49PM +0800, Hans Zhang wrote:
>
>
> On 2025/1/24 00:16, Frank Li wrote:
> > > +static char *dw_ltssm_sts_string(enum dw_pcie_ltssm ltssm)
> > > +{
> > > +	char *str;
> > > +
> > > +	switch (ltssm) {
> > > +#define DW_PCIE_LTSSM_NAME(n) case n: str = #n; break
> > > +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_DETECT_QUIET);
> > > +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_DETECT_ACT);
> > > +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_POLL_ACTIVE);
> > > +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_POLL_COMPLIANCE);
> > > +	...
> > > +	default:
> > > +		str = "DW_PCIE_LTSSM_UNKNOWN";
> > > +		break;
> >
> > I prefer
> > const char * str[] =
> > {
> > 	"detect_quitet",
> > 	"detect_act",
> > 	...
> > }
> >
> > 	return str[ltssm];
> >
> > Or
> > 	#define DW_PCIE_LTSSM_NAME(n) case n: return #n;
> > 	...
> > 	default:
> > 		return "DW_PCIE_LTSSM_UNKNOWN";
> Hi Frank,
>
> I considered the two methods you mentioned before I submitted this patch.
>
> The first, I think, will increase the memory overhead.
>
> +static const char * const dw_pcie_ltssm_str[] = {
> +	[DW_PCIE_LTSSM_DETECT_QUIET] = "DETECT_QUIET",
> +	[DW_PCIE_LTSSM_DETECT_ACT] = "DETECT_ACT",
> +	[DW_PCIE_LTSSM_POLL_ACTIVE] = "POLL_ACTIVE",
> +	[DW_PCIE_LTSSM_POLL_COMPLIANCE] = "POLL_COMPLIANCE",
> 	...
>
>
> The second, ./scripts/checkpatch.pl checks will have a warning
>
> WARNING: Macros with flow control statements should be avoided
> #121: FILE: drivers/pci/controller/dwc/pcie-designware.h:329:
> +#define DW_PCIE_LTSSM_NAME(n) case n: return #n
>

Okay, it is not big deal
can you return
	str + strlen("DW_PCIE_LTSSM_");

Or trim useless "DW_PCIE_LTSSM_" information.

Frank

>
> > > +static ssize_t ltssm_status_show(struct device *dev,
> > > +				 struct device_attribute *attr, char *buf)
> > > +{
> > > +	struct pci_dev *pdev = to_pci_dev(dev);
> > > +	struct pci_host_bridge *bridge = pci_find_host_bridge(pdev->bus);
> > > +	struct dw_pcie_rp *pp = bridge->sysdata;
> > > +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> > > +
> > > +	return sysfs_emit(buf, "%s\n",
> > > +			  dw_ltssm_sts_string(dw_pcie_get_ltssm(pci)));
> >
> > Suggest dump raw value also
> >
> > val = dw_pcie_get_ltssm(pci);
> > return sysfs_emit(buf, "%s (0x%02x)\n",
> > 		  dw_ltssm_sts_string(val), val);
>
> Thanks, i think it's a good idea.
>
> Best regards
> Hans
>
Re: [PATCH] PCI: dwc: Add the sysfs property to provide the LTSSM status of the PCIe link
Posted by Hans Zhang 10 months, 3 weeks ago

On 2025/1/25 00:01, Frank Li wrote:
> On Fri, Jan 24, 2025 at 09:12:49PM +0800, Hans Zhang wrote:
>>
>>
>> On 2025/1/24 00:16, Frank Li wrote:
>>>> +static char *dw_ltssm_sts_string(enum dw_pcie_ltssm ltssm)
>>>> +{
>>>> +	char *str;
>>>> +
>>>> +	switch (ltssm) {
>>>> +#define DW_PCIE_LTSSM_NAME(n) case n: str = #n; break
>>>> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_DETECT_QUIET);
>>>> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_DETECT_ACT);
>>>> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_POLL_ACTIVE);
>>>> +	DW_PCIE_LTSSM_NAME(DW_PCIE_LTSSM_POLL_COMPLIANCE);
>>>> +	...
>>>> +	default:
>>>> +		str = "DW_PCIE_LTSSM_UNKNOWN";
>>>> +		break;
>>>
>>> I prefer
>>> const char * str[] =
>>> {
>>> 	"detect_quitet",
>>> 	"detect_act",
>>> 	...
>>> }
>>>
>>> 	return str[ltssm];
>>>
>>> Or
>>> 	#define DW_PCIE_LTSSM_NAME(n) case n: return #n;
>>> 	...
>>> 	default:
>>> 		return "DW_PCIE_LTSSM_UNKNOWN";
>> Hi Frank,
>>
>> I considered the two methods you mentioned before I submitted this patch.
>>
>> The first, I think, will increase the memory overhead.
>>
>> +static const char * const dw_pcie_ltssm_str[] = {
>> +	[DW_PCIE_LTSSM_DETECT_QUIET] = "DETECT_QUIET",
>> +	[DW_PCIE_LTSSM_DETECT_ACT] = "DETECT_ACT",
>> +	[DW_PCIE_LTSSM_POLL_ACTIVE] = "POLL_ACTIVE",
>> +	[DW_PCIE_LTSSM_POLL_COMPLIANCE] = "POLL_COMPLIANCE",
>> 	...
>>
>>
>> The second, ./scripts/checkpatch.pl checks will have a warning
>>
>> WARNING: Macros with flow control statements should be avoided
>> #121: FILE: drivers/pci/controller/dwc/pcie-designware.h:329:
>> +#define DW_PCIE_LTSSM_NAME(n) case n: return #n
>>
> 
> Okay, it is not big deal
> can you return
> 	str + strlen("DW_PCIE_LTSSM_");
> 
> Or trim useless "DW_PCIE_LTSSM_" information.

Ok, thanks Frank for the advice.

Best regards
Hans