Message ID | 20250123071326.1810751-1-18255117159@163.com (mailing list archive) |
---|---|
State | New |
Delegated to: | Krzysztof WilczyĆski |
Headers | show |
Series | PCI: dwc: Add the sysfs property to provide the LTSSM status of the PCIe link | expand |
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 >
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
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
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 >
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 >
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
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
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
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(+) base-commit: 7004a2e46d1693848370809aa3d9c340a209edbb