Re: [PATCH v4 2/2] PCI: cadence: Add debugfs property to provide LTSSM status of the PCIe link

[Hans Zhang <18255117159@163.com>]

On 5/13/26 19:30, Aksh Garg wrote:
> 
> 
> On 13/05/26 14:25, Hans Zhang wrote:
>>
>>
>> On 5/13/26 16:34, Aksh Garg wrote:
>>
>>>>>
>>>>
>>>> The above LTSSM states are internal LTSSM encoding states and may 
>>>> not be available for software to use.
>>>> The LTSSM states in the document pointed by Aksh (TI Soc) are the 
>>>> states  available in all cadence controllers.
>>>
>>> Is this true for HPA IPs as well? The test performed by Hans:
>>> root@orangepi6plus:~# cat /sys/kernel/debug/cdns_pcie_a0*/ltss*
>>> L0_STATE (0x29)
>>> L0_STATE (0x29)
>>> L0_STATE (0x29)
>>>
>>> This implies that the L0_STATE LTSSM state is mapped to 0x29 (41) there,
>>> which according to your response is internal LTSSM encoding, and hence
>>> the register read should have resulted in 0x10 instead of 0x29.
>>>
>>
>> Hi Aksh,
>>
>>
>> For HPA, my view is similar to that of DWC - it requires a common 
>> internal LTSSM state. For each of its own Root Port drivers, a 
>> callback can be used to implement the reading of LTSSM. This part can 
>> be referred to as the implementation of the function in DWC.
>>
>>
>> static inline enum dw_pcie_ltssm dw_pcie_get_ltssm(struct dw_pcie *pci)
>> {
>>      u32 val;
>>
>>      if (pci->ops && pci->ops->get_ltssm)
>>          return pci->ops->get_ltssm(pci);
>>
>>      val = dw_pcie_readl_dbi(pci, PCIE_PORT_DEBUG0);
>>
>>      return (enum dw_pcie_ltssm)FIELD_GET(PORT_LOGIC_LTSSM_STATE_MASK, 
>> val);
>> }
>>
>>
>> static int ltssm_status_show(struct seq_file *s, void *v)
>> {
>>      struct dw_pcie *pci = s->private;
>>      enum dw_pcie_ltssm val;
>>
>>      val = dw_pcie_get_ltssm(pci);
>>      seq_printf(s, "%s (0x%02x)\n", dw_pcie_ltssm_status_string(val), 
>> val);
>>
>>      return 0;
>> }
>>
>>
>>
>> For example, it can be modified as follows. Of course, the function 
>> name will start with "cdns".
>>
>> For LGA IP, currently we will allow each Root Port driver to implement 
>> the corresponding ops::get_ltssm() by itself.
>>
>> static int ltssm_status_show(struct seq_file *s, void *v)
>> {
>>      struct dw_pcie *pci = s->private;
>>      enum dw_pcie_ltssm val;
>>
>>      if (pci->ops && pci->ops->get_ltssm)
>>          val = pci->ops->get_ltssm(pci);
>>      else
>>          val = dw_pcie_get_ltssm(pci);
>>      seq_printf(s, "%s (0x%02x)\n", dw_pcie_ltssm_status_string(val), 
>> val);
>>
>>      return 0;
>> }
>>
> 
> The process above tells how to read the register and get the LTSSM
> state values. However, my concern is whether we require different LTSSM
> state encoding in your debugfs patch, one for LGA, and other for HPA.
> This is because the L0_state seems to have different values in the LTSSM
> fields of different IPs. On LGA, the L0_state seems to have value as
> 0x10 in the register (as can be seen in the J7200 TRM). On HPA, the
> L0_state seems to have value of 0x29 in the register (as can be seen in
> your test logs in the cover letter). Hence, if we want to print the
> LTSSM state string in the debugfs, then for LGA IPs, 0x10 value should
> print L0_STATE, and for HPA IPs, 0x41 value should print L0_state.
> 

Hi Aksh,

Yes, different codes will be used. The reason for this was because of 
the result I obtained from consulting Manikandan. This time, it was a 
problem identified by Sashiko's review. So, we need to ask you and 
Manikandan to confirm this LTSSM code again.


Best regards,
Hans