On Mon, Nov 20, 2023 at 02:10:13PM +0530, Manivannan Sadhasivam wrote: > The drivers for platforms requiring reference clock from the PCIe host for > initializing their PCIe EP core, make use of the 'core_init_notifier' > feature exposed by the DWC common code. On these platforms, access to the > hw registers like DBI before completing the core initialization will result > in a whole system hang. But the current DWC EP driver tries to access DBI > registers during dw_pcie_ep_init() without waiting for core initialization > and it results in system hang on platforms making use of > 'core_init_notifier' such as Tegra194 and Qcom SM8450. > > To workaround this issue, users of the above mentioned platforms have to > maintain the dependency with the PCIe host by booting the PCIe EP after > host boot. But this won't provide a good user experience, since PCIe EP is > _one_ of the features of those platforms and it doesn't make sense to > delay the whole platform booting due to the PCIe dependency. > > So to fix this issue, let's move all the DBI access during > dw_pcie_ep_init() in the DWC EP driver to the dw_pcie_ep_init_complete() > API that gets called only after core initialization on these platforms. > This makes sure that the DBI register accesses are skipped during > dw_pcie_ep_init() and accessed later once the core initialization happens. > > For the rest of the platforms, DBI access happens as usual. > > Co-developed-by: Vidya Sagar <vidyas@xxxxxxxxxx> > Signed-off-by: Vidya Sagar <vidyas@xxxxxxxxxx> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx> > --- Hello Mani, I tried this patch on top of a work in progress EP driver, which, similar to pcie-qcom-ep.c has a perst gpio as input, and a .core_init_notifier. What I noticed is the following every time I reboot the RC, I get: [ 604.735115] debugfs: Directory 'a40000000.pcie_ep' with parent 'dmaengine' already present! [ 1000.713582] debugfs: Directory 'a40000000.pcie_ep' with parent 'dmaengine' already present! [ 1000.714355] debugfs: File 'mf' in directory '/' already present! [ 1000.714890] debugfs: File 'wr_ch_cnt' in directory '/' already present! [ 1000.715476] debugfs: File 'rd_ch_cnt' in directory '/' already present! [ 1000.716061] debugfs: Directory 'registers' with parent '/' already present! Also: # ls -al /sys/class/dma/dma*/device | grep pcie lrwxrwxrwx 1 root root 0 Jan 1 00:14 /sys/class/dma/dma3chan0/device -> ../../../a40000000.pcie_ep lrwxrwxrwx 1 root root 0 Jan 1 00:14 /sys/class/dma/dma3chan1/device -> ../../../a40000000.pcie_ep lrwxrwxrwx 1 root root 0 Jan 1 00:14 /sys/class/dma/dma3chan2/device -> ../../../a40000000.pcie_ep lrwxrwxrwx 1 root root 0 Jan 1 00:14 /sys/class/dma/dma3chan3/device -> ../../../a40000000.pcie_ep lrwxrwxrwx 1 root root 0 Jan 1 00:14 /sys/class/dma/dma4chan0/device -> ../../../a40000000.pcie_ep lrwxrwxrwx 1 root root 0 Jan 1 00:14 /sys/class/dma/dma4chan1/device -> ../../../a40000000.pcie_ep lrwxrwxrwx 1 root root 0 Jan 1 00:14 /sys/class/dma/dma4chan2/device -> ../../../a40000000.pcie_ep lrwxrwxrwx 1 root root 0 Jan 1 00:14 /sys/class/dma/dma4chan3/device -> ../../../a40000000.pcie_ep lrwxrwxrwx 1 root root 0 Jan 1 00:16 /sys/class/dma/dma5chan0/device -> ../../../a40000000.pcie_ep lrwxrwxrwx 1 root root 0 Jan 1 00:16 /sys/class/dma/dma5chan1/device -> ../../../a40000000.pcie_ep lrwxrwxrwx 1 root root 0 Jan 1 00:16 /sys/class/dma/dma5chan2/device -> ../../../a40000000.pcie_ep lrwxrwxrwx 1 root root 0 Jan 1 00:16 /sys/class/dma/dma5chan3/device -> ../../../a40000000.pcie_ep Adds another dmaX entry for each reboot. I'm quite sure that you will see the same with pcie-qcom-ep. I think that either the DWC drivers using core_init (only tegra and qcom) need to deinit the eDMA in their assert_perst() function, or this patch needs to deinit the eDMA before initializing it. A problem with the current code, if you do NOT have this patch, which I assume is also problem on pcie-qcom-ep, is that since assert_perst() function performs a core reset, all the eDMA setting written in the dbi by the eDMA driver will be cleared, so a PERST assert + deassert by the RC will wipe the eDMA settings. Hopefully, this will no longer be a problem after this patch has been merged. Kind regards, Niklas
