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

Shradha Todi
2025-03-08T15:54:06Z

> -----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