On Thu, Mar 07, 2024 at 10:09:06PM +0100, Niklas Cassel wrote:

On Mon, Mar 04, 2024 at 02:52:19PM +0530, Manivannan Sadhasivam wrote:

quoted

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

Only difference here is that, the drivers requiring refclk from host will
send the notification once refclk is received, while others will send it
during probe time itself.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Manivannan Sadhasivam <redacted>
---

You have removed the .core_init_notifier from EPC drivers,
but the callback in EPF drivers is still called .core_init.

Yes, this was a confusing name even before this patch, but
after this patch, it is probably even worse :)

The callback should be named from the perspective of EPF drivers IMO.
.core_init sounds like a EPF driver should initialize the core.
(But that is of course done by the EPC driver.)

The .link_up() callback name is better, the EPF driver is informed
that the link is up.

Perhaps we could rename .core_init to .core_up ?

It tells the EPF drivers that the core is now up.
(And the EPF driver can configure the BARs.)

I don't disagree :) I thought about it but then decided to not extend the scope
of this series further. So saved that for next series.

But yeah, it is good to clean it up here itself.

Considering that you are not changing the name of the callback,
and that it was already confusing before this patch:
Reviewed-by: Niklas Cassel <cassel@kernel.org>

Thanks!

- Mani

-- 
மணிவண்ணன் சதாசிவம்