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

[Hans Zhang <18255117159@163.com>]

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