On Thu, Nov 30, 2023 at 12:08:00PM +0530, Manivannan Sadhasivam wrote: > > Looking at this issue again, I think your statement may not be correct. In the > stop_link() callback, non-core_init_notifier platforms are just disabling the > LTSSM and enabling it again in start_link(). So all the rest of the > configurations (DBI etc...) should not be affected during EPF bind/unbind. Sorry for confusing you. When toggling PERST on a driver with a core_init_notifier, you will call dw_pcie_ep_init_complete() - which will initialize DBI settings, and then stop/start the link by deasserting/asserting LTSSM. (perst_assert()/perst_deassert() is functionally the equivalent to start_link()/ stop_link() on non-core_init_notifier platforms.) For drivers without a core_init_notifier, they currently don't listen to PERST input GPIO. Stopping and starting the link will not call dw_pcie_ep_init_complete(), so it will not (re-)initialize DBI settings. My problem is that a bunch of hardware registers gets reset when receiving a link-down reset or hot reset. It would be nice to write all DBI settings when starting the link. That way the link going down for a millisecond, and gettting immediately reestablished, will not be so bad. A stop + start link will rewrite the settings. (Just like toggling PERST rewrites the settings.) Currently, doing a bind + start link + stop link + unbind + bind + start link, will not reinitialize all DBI writes for a driver. (This is also true for a core_init_notifier driver, but there start/stop link is a no-op, other than enabling the GPIO irq handler.) What this will do during the second bind: -allocate BARs (DBI writes) -set iATUs (DBI writes) What it will not redo is: -what is done by dw_pcie_ep_init() - which is fine, because this is only supposed to allocate stuff or get stuff from DT, not actually touch HW registers. -what is done by dw_pcie_ep_init_complete() - I think this should be done since it does DBI writes. E.g. clearing PTM root Cap and fixing Resizable BAR Cap, calling dw_pcie_setup() which sets max link width etc. I guess the counter argument could be that... if you need to re-initialize DBI registers, you could probably do a: stop link + unbind EPF driver + unbind PCIe-EP driver + bind PCIe-EP driver + bind EPF driver + start link.. (But I don't need all that if I use a .core_init_notifier and just toggle PERST). Of course, toggling PERST and starting/stopping the link via sysfs is not exactly the same thing... For me personally, the platform I use do not require an external refclk to write DBI settings, but I would very much like the HW to be re-initialized either when stopping/starting the link in sysfs, or by toggling PERST, or both. I think that I will just add a .core_init_notifier to my WIP driver, even though I do not require an external refclk, and simply call dw_pcie_ep_init_complete() when receiving a PERST deassert, because I want _all_ settings (DBI writes) to be reinitialized. (If we receive a PERST reset request, I think it does make sense to obey and actually reset/re-write all settings.) A sysfs stop/start link would still not reinitialize everything... I think it would be good if the DWC EP driver actually did this too, but I'm fine if you consider it out of scope of your patch series. Kind regards, Niklas
