Hello DWC PCIe endpoint developers, As I wrote in patch [1] (please review): The DWC Databook description for the LWR_TARGET_RW and LWR_TARGET_HW fields in the IATU_LWR_TARGET_ADDR_OFF_INBOUND_i registers state that: "Field size depends on log2(BAR_MASK+1) in BAR match mode." I.e. only the upper bits are writable, and the number of writable bits is dependent on the configured BAR_MASK. While patch [1] is a nice fail-safe that we definitely want to have... I can see that the DWC PCIe EP driver is currently broken (and has been since the beginning). We are programming the iATU _before_ configuring the BAR: https://github.com/torvalds/linux/blob/v6.12/drivers/pci/controller/dwc/pcie-designware-ep.c#L232-L247 The problem is of course that the iATU registers depend on the BAR mask (the number of read-only bits depends on the currently set BAR mask), so the iATU registers should be done _after_ configuring the BAR. Doing it in this was makes a lot of sense. First you configure the BAR, then you configure the address translation for that BAR. (Since the iATU in BAR match mode obviously depends on how the BAR is configured.) Now, I would like to send a patch to change this order, so that we actually write things in the only order that makes sense, my problem is this line: https://github.com/torvalds/linux/blob/v6.12/drivers/pci/controller/dwc/pcie-designware-ep.c#L236-L237 This code makes no sense to me whatsoever. If I look at the commit that introduced this early return: 4284c88fff0e ("PCI: designware-ep: Allow pci_epc_set_bar() update inbound map address") It is not signed off by any of the regular PCI maintainers, neither does it have any Acked-by by any of the regular PCI maintainers, so I kind of wonder why this patch is there is the first place... (Taking something via a different tree is fine, but that would still require an Acked-by by one of the maintainers for the tree which owns that file.) While I am tempted to simply send a revert for this commit, so that we can write the registers in the correct order (iATU after BAR mask), I still do not want to regress the NTB EPF driver (even if the commit appears to have bypassed the regular PCI maintainers). Frank, since you are the author of: 4284c88fff0e ("PCI: designware-ep: Allow pci_epc_set_bar() update inbound map address") Please advice on what to do here. The only thing that makes sense to me is that dw_pcie_ep_set_bar() always writes the BAR MASK. If dw_pcie_ep_set_bar() does NOT write the BAR MASK, we depend on whatever BAR MASK was there before dw_pcie_ep_set_bar(), which no matter how I look at it, seems horribly wrong. Kind regards, Niklas [1] https://lore.kernel.org/linux-pci/20241116163558.2606874-10-cassel@xxxxxxxxxx/
