RE: [PATCH v7 3/5] Add debugfs based silicon debug support in DWC

[Shradha Todi]

> -----Original Message-----
> From: Krzysztof Wilczyński <kw@xxxxxxxxx>
> Sent: 06 March 2025 14:33
> To: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> Cc: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx>; Bjorn Helgaas <helgaas@xxxxxxxxxx>; Fan Ni
> <nifan.cxl@xxxxxxxxx>; Shradha Todi <shradha.t@xxxxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx; linux-pci@xxxxxxxxxxxxxxx; linux-
> arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-perf-users@xxxxxxxxxxxxxxx; lpieralisi@xxxxxxxxxx; robh@xxxxxxxxxx; bhelgaas@xxxxxxxxxx;
> jingoohan1@xxxxxxxxx; Jonathan.Cameron@xxxxxxxxxx; a.manzanares@xxxxxxxxxxx; pankaj.dubey@xxxxxxxxxxx;
> cassel@xxxxxxxxxx; 18255117159@xxxxxxx; xueshuai@xxxxxxxxxxxxxxxxx; renyu.zj@xxxxxxxxxxxxxxxxx; will@xxxxxxxxxx;
> mark.rutland@xxxxxxx; Yoshihiro Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx>; Linux-Renesas <linux-renesas-
> soc@xxxxxxxxxxxxxxx>
> Subject: Re: [PATCH v7 3/5] Add debugfs based silicon debug support in DWC
> 
> Hello,
> 
> [...]
> > Another issue is that the caller does not handle failures correctly,
> > given (A) the irqdomain WARNING I got, and (B) the half-registered PCI
> > bus, oopsing on "lspci"...
> 
> This is something we will look into.  A more robust DesignWare core is something we would definitely want to have.
> 

The issue here was that " pci_host_probe(bridge)" was being called right before doing the debugfs init.
So upon failure, the cleanup path should have included:
        pci_stop_root_bus(pp->bridge->bus);
        pci_remove_root_bus(pp->bridge->bus);

I missed that, which was probably causing the half-registered PCI bus. Since we are going with no error
propagation now, there is no issue anymore. Sorry for missing that.

> Sorry about the issues with this...
> 
> [...]
> > > -int dwc_pcie_debugfs_init(struct dw_pcie *pci)
> > > +void dwc_pcie_debugfs_init(struct dw_pcie *pci)
> > >  {
> > >         char dirname[DWC_DEBUGFS_BUF_MAX];
> > >         struct device *dev = pci->dev; @@ -174,17 +174,15 @@ int
> > > dwc_pcie_debugfs_init(struct dw_pcie *pci)
> > >         snprintf(dirname, DWC_DEBUGFS_BUF_MAX, "dwc_pcie_%s", dev_name(dev));
> > >         dir = debugfs_create_dir(dirname, NULL);
> > >         debugfs = devm_kzalloc(dev, sizeof(*debugfs), GFP_KERNEL);
> > > -       if (!debugfs)
> > > -               return -ENOMEM;
> > > +       if (!debugfs) {
> > > +               dev_err(dev, "failed to allocate memory for
> > > + debugfs\n");
> >
> > There is no need to print an error message when a memory allocation
> > fails, as the memory allocation core already takes care of that.
> > So please drop the dev_err() call.
> 
> Done.  Thank you!
> 
> 	Krzysztof