DWC PCIe endpoint iATU vs BAR MASK ordering

[Niklas Cassel <cassel@kernel.org>]

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@kernel.org/