On Wed, Mar 27, 2024 at 02:43:37PM +0530, Manivannan Sadhasivam wrote:
> "core_init_notifier" flag is set by the glue drivers requiring refclk from
> the host to complete the DWC core initialization. Also, those drivers will
> send a notification to the EPF drivers once the initialization is fully
> completed using the pci_epc_init_notify() API. Only then, the EPF drivers
> will start functioning.
>
> For the rest of the drivers generating refclk locally, EPF drivers will
> start functioning post binding with them. EPF drivers rely on the
> 'core_init_notifier' flag to differentiate between the drivers.
> Unfortunately, this creates two different flows for the EPF drivers.
>
> So to avoid that, let's get rid of the "core_init_notifier" flag and follow
> a single initialization flow for the EPF drivers. This is done by calling
> the dw_pcie_ep_init_notify() from all glue drivers after the completion of
> dw_pcie_ep_init_registers() API. This will allow all the glue drivers to
> send the notification to the EPF drivers once the initialization is fully
> completed.
Thanks for doing this! I think this is a significantly nicer
solution than core_init_notifier was.
One question: both qcom and tegra194 call dw_pcie_ep_init_registers()
from an interrupt handler, but they register that handler in a
different order with respect to dw_pcie_ep_init().
I don't know what actually starts the process that leads to the
interrupt, but if it's dw_pcie_ep_init(), then one of these (qcom, I
think) must be racy:
qcom_pcie_ep_probe
dw_pcie_ep_init <- A
qcom_pcie_ep_enable_irq_resources
devm_request_threaded_irq(qcom_pcie_ep_perst_irq_thread) <- B
qcom_pcie_ep_perst_irq_thread
qcom_pcie_perst_deassert
dw_pcie_ep_init_registers
tegra_pcie_dw_probe
tegra_pcie_config_ep
devm_request_threaded_irq(tegra_pcie_ep_pex_rst_irq) <- B
dw_pcie_ep_init <- A
tegra_pcie_ep_pex_rst_irq
pex_ep_event_pex_rst_deassert
dw_pcie_ep_init_registers
Whatever the right answer is, I think qcom and tegra194 should both
order dw_pcie_ep_init() and the devm_request_threaded_irq() the same
way.
Bjorn
