Message-ID: <karx7k6lkem22dwyon2veqwvhko6e5jgzsk6bfhuymyzbdohqs@cqmsmclsgqnj>
Date: Thu, 15 Jan 2026 02:29:41 +0900
From: Koichiro Den <den@...inux.co.jp>
To: Niklas Cassel <cassel@...nel.org>
Cc: jingoohan1@...il.com, mani@...nel.org, lpieralisi@...nel.org, 
	kwilczynski@...nel.org, robh@...nel.org, bhelgaas@...gle.com, vigneshr@...com, 
	s-vadapalli@...com, hongxing.zhu@....com, l.stach@...gutronix.de, 
	shawnguo@...nel.org, s.hauer@...gutronix.de, kernel@...gutronix.de, 
	festevam@...il.com, minghuan.Lian@....com, mingkai.hu@....com, roy.zang@....com, 
	jesper.nilsson@...s.com, heiko@...ech.de, srikanth.thokala@...el.com, 
	marek.vasut+renesas@...il.com, yoshihiro.shimoda.uh@...esas.com, geert+renesas@...der.be, 
	magnus.damm@...il.com, christian.bruel@...s.st.com, mcoquelin.stm32@...il.com, 
	alexandre.torgue@...s.st.com, thierry.reding@...il.com, jonathanh@...dia.com, 
	hayashi.kunihiko@...ionext.com, mhiramat@...nel.org, kishon@...nel.org, jirislaby@...nel.org, 
	rongqianfeng@...o.com, 18255117159@....com, shawn.lin@...k-chips.com, 
	nicolas.frattaroli@...labora.com, linux.amoon@...il.com, vidyas@...dia.com, Frank.Li@....com, 
	linux-omap@...r.kernel.org, linux-pci@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, 
	linux-kernel@...r.kernel.org, imx@...ts.linux.dev, linuxppc-dev@...ts.ozlabs.org, 
	linux-arm-kernel@...s.com, linux-rockchip@...ts.infradead.org, 
	linux-arm-msm@...r.kernel.org, linux-renesas-soc@...r.kernel.org, 
	linux-stm32@...md-mailman.stormreply.com, linux-tegra@...r.kernel.org
Subject: Re: [PATCH v7 5/6] PCI: dwc: ep: Support BAR subrange inbound
 mapping via Address Match Mode iATU

On Wed, Jan 14, 2026 at 11:39:03AM +0100, Niklas Cassel wrote:
> On Wed, Jan 14, 2026 at 12:54:37PM +0900, Koichiro Den wrote:
> > 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-spaces
> 
> 
> > 
> > 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

> 
> 

Powered by blists - more mailing lists