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

[Hans Zhang]

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