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

[Manikandan Karunakaran Pillai]

Sorry for all the confusion. Just checked with design and the HPA has different states from the LGA controller in cadence.
The HPA has the 127 states where 0x29 is the L0_state. So as you mentioned, the codes will have to be different between the 
HPA and LGA controllers.

>EXTERNAL MAIL
>
>
>
>
>On 5/14/26 09:39, Manikandan Karunakaran Pillai 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
>>
>> The LTSSM codes are the same across HPA and LGA. You have to refer to
>"External LTSSM state encoding" table in
>> the specification given by cadence.
>>
>
>Hi Manikandan,
>
>
>Are there 128 different states? If they are the same, for the LGA IP,
>which register should I read?
>
>Previously, Sashiko reported a problem with the review. Please click on
>the following link:
>
>https://urldefense.com/v3/__https://lore.kernel.org/linux-
>pci/20260508041956.C8F10C2BCB0@xxxxxxxxxxxxxxx/__;!!EHscmS1ygiU1lA!G
>U0U7Ef3eOobw7d7tpj9llgtGrgZdXwgWBd6BpVcZ-9F-KnuwY3gNqJor8iK-
>fPDWLaktvVvrk7QmTRGFSC9$
>
>
>"""
> > +#define CDNS_PCIE_LGA_LTSSM_STATUS_MASK	GENMASK(29, 24)
>
>Does this mask need to be extended to cover all possible states?
>
>This mask covers only 6 bits, allowing it to extract values up to 63.
>However, the cdns_pcie_ltssm enum defines states up to 127. If the
>hardware enters a state 64 or higher, FIELD_GET will silently truncate
>the value, making state 65 appear as state 1.
>
>"""
>
>
>Best regards,
>Hans
>