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

[Hans Zhang]

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