On Thu, Dec 14, 2023 at 10:19:03AM -0500, Frank Li wrote: > On Thu, Dec 14, 2023 at 02:31:04PM +0000, Niklas Cassel wrote: > > Hello Frank, > > > > On Tue, Feb 22, 2022 at 10:23:52AM -0600, Frank Li wrote: > > > ntb_mw_set_trans() will set memory map window after endpoint function > > > driver bind. The inbound map address need be updated dynamically when > > > using NTB by PCIe Root Port and PCIe Endpoint connection. > > > > > > Checking if iatu already assigned to the BAR, if yes, using assigned iatu > > > number to update inbound address map and skip set BAR's register. > > > > > > Signed-off-by: Frank Li <Frank.Li@xxxxxxx> > > > --- > > > > > > Change from V1: > > > - improve commit message > > > > > > drivers/pci/controller/dwc/pcie-designware-ep.c | 10 +++++++++- > > > 1 file changed, 9 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c > > > index 998b698f40858..cad5d9ea7cc6c 100644 > > > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c > > > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c > > > @@ -161,7 +161,11 @@ static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep, u8 func_no, > > > u32 free_win; > > > struct dw_pcie *pci = to_dw_pcie_from_ep(ep); > > > > > > - free_win = find_first_zero_bit(ep->ib_window_map, pci->num_ib_windows); > > > > find_first_zero_bit() can return 0, representing bit 0, > > which is a perfectly valid return value. > > > > > + if (!ep->bar_to_atu[bar]) > > > > so this check is not correct. > > > > Please sent out your fixed patch. If want to me fix it, please tell me > reproduce steps. Reproduce steps are simple: 1) Add debug messages to dw_pcie_ep_inbound_atu() to see the iATU index for each BAR. 2) Boot an EP platform with a core_init_notifier. 3) Boot the RC. 4) Reboot the RC, which will assert + deassert PERST, and will call pci_epc_init_notify(), which will call .core_init (pci_epf_test_core_init()) which will set the BARs. In step 3) BAR0 will use iATU0. In step 4) BAR0 will use iATU6 instead of iATU0. There is no reason for this, as it should really reuse the same iATU index as before, just like all the other BARs do. (This is because of find_first_zero_bit() misusage.) I could send out my patch, but from inspecting the code, it looks like: > > > + if (ep->epf_bar[bar]) > > > + return 0; from dw_pcie_ep_set_bar(), also needs to be dropped, so that the iATU settings will be re-written for platforms with core_init_notifiers. Right now, for a platform with a core_init_notifier, if you run pci_endpoint_test + reboot the RC (so that PERST is asserted + deasserted), and then run pci_endpoint_test again, then I'm quite sure that pci_endpoint_test will not pass the second time (because iATU settings were not rewritten after reset). It would be nice if Mani or Vidya could confirm this. I guess that you added this statement for some reason, so I assume that we can't just drop this line without breaking something else. I guess one option would be modify dw_pcie_ep_init_notify() to call dw_pcie_ep_clear_bar() on all non-NULL BARs stored in ep->epf_bar[], before calling pci_epc_init_notify(). That way the second .core_init (pci_epf_test_core_init()) call will use write the settings, because ep->epf_bar[] will be empty, so the "write the settings only the first time" approach will then also work for core_init_notifier platforms. Kind regards, Niklas
