Re: [PATCH v7 5/6] PCI: dwc: ep: Support BAR subrange inbound mapping via Address Match Mode iATU
From: Koichiro Den <hidden>
Date: 2026-01-14 17:29:45
Also in:
imx, linux-arm-msm, linux-omap, linux-pci, linux-renesas-soc, linux-rockchip, linux-tegra, lkml
On Wed, Jan 14, 2026 at 11:39:03AM +0100, Niklas Cassel wrote:
quoted hunk ↗ jump to hunk
On Wed, Jan 14, 2026 at 12:54:37PM +0900, Koichiro Den wrote:quoted
I realized that I missed one case in v7. I think dw_pcie_ep_clear_ib_maps() should also be called from dw_pcie_ep_ib_atu_bar() to tear down any existing inbound mappings for the same BAR before re-programming it in BAR Match Mode. This matters when updating inbound mappings for a BAR without resetting the BAR in between. There are four possible transition patterns, and pattern #4 below was overlooked: 1. BAR Match Mode -> BAR Match Mode As the current implementation does, the mapping is simply updated (with the same atu index) 2. BAR Match Mode -> Address Match Mode This patch series already ensures the old BAR Match mapping is torn down before reprogramming. 3. Address Match Mode -> Address Match Mode Likewise, existing Address Match mappings are cleared first. 4. Address Match Mode -> BAR Match Mode This case was not handled. The change below adds the missing teardown so that stale Address Match mappings do not remain active. --- a/drivers/pci/controller/dwc/pcie-designware-ep.c +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c @@ -148,9 +148,12 @@ static int dw_pcie_ep_ib_atu_bar(struct dw_pcie_ep *ep, u8 func_no, int type, u32 free_win; struct dw_pcie *pci = to_dw_pcie_from_ep(ep); - if (!ep->bar_to_atu[bar]) + if (!ep->bar_to_atu[bar]) { + /* Tear down existing mappings before (re)programming. */ + dw_pcie_ep_clear_ib_maps(ep, bar); + free_win = find_first_zero_bit(ep->ib_window_map, pci->num_ib_windows); - else + } else free_win = ep->bar_to_atu[bar] - 1;If one of the branches has braces, both branches should have braces: https://www.kernel.org/doc/html/latest/process/coding-style.html#placing-braces-and-spacesquoted
Unless there are objections, I'll include this fix in v8.Isn't it easier/cleaner if we call dw_pcie_ep_clear_ib_maps() in dw_pcie_ep_set_bar(), rather than calling it in both dw_pcie_ep_ib_atu_addr() and dw_pcie_ep_ib_atu_bar() ? dw_pcie_ep_set_bar() knows the condition if we are dynamically reprogramming a BAR or not, and all the four cases are when dynamically reprogramming a BAR. I.e. instead of adding additional code to dw_pcie_ep_ib_atu_bar(), we do something like:diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c index b2ea2c2c986f..63ae5471fe13 100644 --- a/drivers/pci/controller/dwc/pcie-designware-ep.c +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c@@ -318,9 +318,6 @@ static int dw_pcie_ep_ib_atu_addr(struct dw_pcie_ep *ep, u8 func_no, int type, return -EINVAL; } - /* Tear down any existing mappings before (re)programming. */ - dw_pcie_ep_clear_ib_maps(ep, bar); - for (i = 0; i < epf_bar->num_submap; i++) { off = submap[i].offset; size = submap[i].size;@@ -571,6 +568,9 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no, ep->epf_bar[bar]->flags != flags) return -EINVAL; + if (ep->epf_bar[bar]->num_submap || epf_bar->num_submap) + dw_pcie_ep_clear_ib_maps(ep, bar); + /* * When dynamically changing a BAR, skip writing the BAR reg, as * that would clear the BAR's PCI address assigned by the host.
For pattern #2 and #3 (ie. either mode -> Address Match Mode), the v7 code withholds the dw_pcie_ep_clear_ib_maps() call unless the submap validation passes. The above patch differs slightly in that sense, but I agree it looks much simpler. I don't think the difference matters much, since pci_epc_set_bar() with an invalid submap should already indicate that something has gone wrong (most likely a bug in the API call site). So I think I'll go with your suggestion. Thanks! Koichiro