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

[Hans Zhang]

On 2025/2/12 19:39, Shradha Todi wrote:
> > @@ -463,6 +495,7 @@ struct dw_pcie {
> >   	struct reset_control_bulk_data	core_rsts[DW_PCIE_NUM_CORE_RSTS];
> >   	struct gpio_desc		*pe_rst;
> >   	bool			suspended;
> > +	struct dentry		*debugfs;
>
> This pointer to main directory dentry is already present as rasdes_dir in struct dwc_pcie_rasdes_info.
> So struct dentry *debugfs is duplicating it.
>
> We have a few options to solve this:
> 1. Remove struct dentry *rasdes_dir from dwc_pcie_rasdes_info and continue to have 2 pointers exposed
> in struct dw_pcie.
>
> struct dwc_pcie_rasdes_info {
>          u32 ras_cap_offset;
>          struct mutex reg_lock;
> };
> struct dw_pcie {
>         .
>         .
>         struct dentry           *debugfs;
>          void                    *rasdes_info;
>   };
>
> 2. Change rasdes_info to debugfs info:
>
> struct dwc_pcie_rasdes_info {
>          u32 ras_cap_offset;
>          struct mutex reg_lock;
> };
> struct dwc_pcie_debugfs_info {
>          struct dwc_pcie_rasdes_info *rinfo;
>          struct dentry           *debugfs;
> };
> struct dw_pcie {
>         .
>         .
>          void                    *debugfs_info;
>   };
>
> 3. Let ras related info get initialized to 0 even when rasdes cap is not present:
>
> struct dwc_pcie_debugfs_info {
>          u32 ras_cap_offset;
>          struct mutex reg_lock;
>          struct dentry *debugfs;
> };
> struct dw_pcie {
>         .
>         .
>          void                    *debugfs_info;
>   };
>
> I think option 2 would be the best, though it will need a bit of changes in my files. What do you suggest?

I couldn't agree more. Can you build the debugfs framework? I or other 
developers can add some other debugfs nodes to this framework, not only 
dwc_pcie_rasdes_debugfs_init, but also dwc_pcie_debugfs_init.

I will add my patch with your next version. Please CC email me.

Best regards
Hans