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

Hello,

[...]

quoted

quoted

quoted

quoted

quoted

+   ret = dwc_pcie_rasdes_debugfs_init(pci, dir);
+   if (ret)
+           dev_dbg(dev, "RASDES debugfs init failed\n");

What will happen if ret != 0? still return 0?

And that is exactly what happens on Gray Hawk Single with R-Car
V4M: dw_pcie_find_rasdes_capability() returns NULL, causing
dwc_pcie_rasdes_debugfs_init() to return -ENODEV.

Thank you for testing and for catching this issue.  Much appreciated.

quoted

quoted

Given that callers of dwc_pcie_debugfs_init() check for errors,

Debugfs issues should never be propagated upstream!

Makes complete sense.  Sorry for breaking things here!

quoted

quoted

this probably should correctly bubble up any failure coming from
dwc_pcie_rasdes_debugfs_init().

I made updates to the code directly on the current branch, have a look:

So while applying, you changed this like:

            ret = dwc_pcie_rasdes_debugfs_init(pci, dir);
    -       if (ret)
    -               dev_dbg(dev, "RASDES debugfs init failed\n");
    +       if (ret) {
    +               dev_err(dev, "failed to initialize RAS DES debugfs\n");
    +               return ret;
    +       }

            return 0;

Hence this is now a fatal error, causing the probe to fail.

I removed the changed, and also move the log level to be a warning, per:

  https://web.git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/commit/?h=controller/dwc&id=c6759a967e69aba16aef0d92f43e527b112e98a5

Would this be acceptable here?

Mani, would this be acceptable to you, too?  Given that you posted the
following recently:

  https://lore.kernel.org/linux-pci/20250303200055.GA1881771@rocinante/T/#mab9cbd5834390d259afea056eee9a73d8c3b435f (local)

That said, perhaps moving the log level to a debug would be better served here.

Even though debugfs_init() failure is not supposed to fail the probe(),
dwc_pcie_rasdes_debugfs_init() has a devm_kzalloc() and propagating that
failure would be canolically correct IMO.

So I would still opt to have my version + your previous one.

Sounds good!

I combined both changes (squashed your fix for the RAS DES capability
detection) together directly on the branch.

Thank you!

	Krzysztof