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

["Shradha Todi" <shradha.t@samsung.com>]

> -----Original Message-----
> From: Krzysztof Wilczyński <kw@linux.com>
> Sent: 06 March 2025 14:33
> To: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>; Bjorn Helgaas <helgaas@kernel.org>; Fan Ni
> <nifan.cxl@gmail.com>; Shradha Todi <shradha.t@samsung.com>; linux-kernel@vger.kernel.org; linux-pci@vger.kernel.org; linux-
> arm-kernel@lists.infradead.org; linux-perf-users@vger.kernel.org; lpieralisi@kernel.org; robh@kernel.org; bhelgaas@google.com;
> jingoohan1@gmail.com; Jonathan.Cameron@huawei.com; a.manzanares@samsung.com; pankaj.dubey@samsung.com;
> cassel@kernel.org; 18255117159@163.com; xueshuai@linux.alibaba.com; renyu.zj@linux.alibaba.com; will@kernel.org;
> mark.rutland@arm.com; Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>; Linux-Renesas <linux-renesas-
> soc@vger.kernel.org>
> 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