On Mon, May 13, 2024 at 05:49:00PM +0200, Manivannan Sadhasivam wrote: > Tegra194 driver supports both the host and endpoint mode, but there are no > checks to validate whether the corresponding mode is enabled in kernel > config or not. So if the driver tries to function without enabling the > required mode (CONFIG_PCIE_TEGRA194_HOST/CONFIG_PCIE_TEGRA194_EP), then it > will result in driver malfunction. > > So let's add the checks in probe() before doing the mode specific config > and fail probe() if the corresponding mode is not enabled. > > But this also requires adding one redundant check in > pex_ep_event_pex_rst_assert() for pci_epc_deinit_notify(). Because the > function is called outside of probe() and the compiler fails to spot the > dependency in probe() and still complains about the undefined reference to > pci_epc_deinit_notify(). > > Reported-by: kernel test robot <lkp@xxxxxxxxx> > Closes: https://lore.kernel.org/oe-kbuild-all/202405130815.BwBrIepL-lkp@xxxxxxxxx > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx> > --- > drivers/pci/controller/dwc/pcie-tegra194.c | 21 ++++++++++++++++++++- > 1 file changed, 20 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c > index d2223821e122..e02a9bca70ef 100644 > --- a/drivers/pci/controller/dwc/pcie-tegra194.c > +++ b/drivers/pci/controller/dwc/pcie-tegra194.c > @@ -1715,7 +1715,16 @@ static void pex_ep_event_pex_rst_assert(struct tegra_pcie_dw *pcie) > if (ret) > dev_err(pcie->dev, "Failed to go Detect state: %d\n", ret); > > - pci_epc_deinit_notify(pcie->pci.ep.epc); > + /* > + * We do not really need the below guard as the driver won't probe > + * successfully if it tries to probe in EP mode and > + * CONFIG_PCIE_TEGRA194_EP is not enabled. But since this function is > + * being called outside of probe(), compiler fails to spot the > + * dependency in probe() and hence this redundant check. > + */ > + if (IS_ENABLED(CONFIG_PCIE_TEGRA194_EP)) > + pci_epc_deinit_notify(pcie->pci.ep.epc); > + This big comment is a bit ugly. It would be nice if it could be avoided. (pci-epc.h does not provide any dummy implementations for any of the functions, so I suggest that we leave it like that.) However, if we look at dw_pcie_ep_init_notify(), it is called from pex_ep_event_pex_rst_deassert(), and we do not get a build warning for that. The reason is that dw_pcie_ep_init_notify() has a dummy implementation in pcie-designware.h. May I suggest that we add a dw_pcie_ep_deinit_notify() wrapper around pci_epc_deinit_notify(), while also providing a dummy implementation in pcie-designware.h ? That way, the code in pcie-tegra194.c (and pcie-qcom-ep.c) would be more symmetrical, calling dw_pcie_ep_init_notify() for init notification, and dw_pcie_ep_deinit_notify() for deinit notification. (Instead of dw_pcie_ep_init_notify() + pci_epc_deinit_notify()) Kind regards, Niklas
