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

[Hans Zhang]

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