On Tue, Nov 19, 2024 at 10:44:38AM +0100, Niklas Cassel wrote: > 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.) > It was one such occurence where the PCI EP maintainer didn't respond promptly (not me) and the NTB maintainer merged the patch. I did complain about it: https://lore.kernel.org/linux-pci/20220818060230.GA12008@thinkpad - Mani -- மணிவண்ணன் சதாசிவம்
