Message-ID: <nqpwi6ewen4kf7jqgon4ljerceqjeaule25dzb6ytas3wslqhp@ddkr3jum6eac>
Date: Fri, 9 Jan 2026 16:29:14 +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 v5 3/3] PCI: dwc: ep: Support BAR subrange inbound
 mapping via Address Match Mode iATU

On Thu, Jan 08, 2026 at 09:55:27PM +0100, Niklas Cassel wrote:
> Hello Koichiro,
> 
> On Fri, Jan 09, 2026 at 02:24:03AM +0900, Koichiro Den wrote:
> 
> (snip)
> 
> > +/* Address Match Mode inbound iATU mapping */
> > +static int dw_pcie_ep_ib_atu_addr(struct dw_pcie_ep *ep, u8 func_no, int type,
> > +				  const struct pci_epf_bar *epf_bar)
> > +{
> > +	const struct pci_epf_bar_submap *submap = epf_bar->submap;
> > +	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > +	enum pci_barno bar = epf_bar->barno;
> > +	struct device *dev = pci->dev;
> > +	u64 pci_addr, parent_bus_addr;
> > +	struct dw_pcie_ib_map *new;
> > +	u64 size, off, base;
> > +	unsigned long flags;
> > +	int free_win, ret;
> > +	unsigned int i;
> > +
> > +	if (!epf_bar->num_submap || !submap || !epf_bar->size)
> > +		return -EINVAL;
> > +
> > +	ret = dw_pcie_ep_validate_submap(ep, submap, epf_bar->num_submap,
> > +					 epf_bar->size);
> > +	if (ret)
> > +		return ret;
> > +
> > +	base = dw_pcie_ep_read_bar_assigned(ep, func_no, bar, epf_bar->flags);
> > +	if (!base) {
> > +		dev_err(dev,
> > +			"BAR%u not assigned, cannot set up sub-range mappings\n",
> > +			bar);
> > +		return -EINVAL;
> > +	}
> 
> Sorry for giving additional review comments.

Thanks again for the detailed feedback.

> 
> But there is one thing that I might not be so obvious for someone just
> reading this source. How is this API supposed to be used in practice?
> 
> Most DWC-based controllers are not hotplug capable.
> 
> That means that we must boot the EP, create the EPF symlink in configfs,
> and start link training by writing to configfs, before starting the host.
> 
> dw_pcie_ep_ib_atu_addr() reads the PCI address that the host has assigned
> to the BAR, and returns an error if the host has not already assigned a
> PCI addres to the BAR.
> 
> Does that mean that the usage of this API will be something like:
> 
> 1) set_bar() ## using BAR match mode, since BAR match mode can write
>    the BAR mask to define a BAR size, so that the host can assign a
>    PCI address to the BAR.

BAR sizing is done by dw_pcie_ep_set_bar_{programmable,resizable}() before
iATU programming regardless of match mode. I keep BAR match mode here just
because Address match mode requires a valid base address. That's why I
added the `if (!base)` guard.

> 
> 2) start() ## start link
> 
> 3) link up
> 
> 4) wait for some special command, perhaps NTB_EPF_COMMAND
> CMD_CONFIGURE_DOORBELL or NTB_EPF_COMMAND CMD_CONFIGURE_MW
> 
> 5) set_bar() ## using Address match mode. Because address match mode
>    requires that the host has assigned a PCI address to the BAR, we
>    can only change the mapping for a BAR after the host has assigned
>    PCI addresses for all bars.
> 

The overall usage flow matches what I'm assuming.

> 
> 
> Perhaps you should add some text to:
> Documentation/PCI/endpoint/pci-endpoint.rst
> 
> Because right now the documentation for pci_epc_set_bar() says:
> 
>    The PCI endpoint function driver should use pci_epc_set_bar() to configure
>    the Base Address Register in order for the host to assign PCI addr space.
>    Register space of the function driver is usually configured
>    using this API.
> 
> So it is obviously meant to be called *before* the host assigns a PCI
> address for the BAR. Now with submap ranges, it appears that it has to
> be called *after* the host assigned a PCI address for the BAR.

I agree. As I understand it, the current text seems not to reflect mainline
since commit 4284c88fff0e ("PCI: designware-ep: Allow pci_epc_set_bar()
update inbound map address"), but anyway I should add explicit
documentation for this subrange mapping use case.

> 
> So I can only assume that you will call set_bar() twice.
> Once with BAR match mode, and then a second time with address map mode.
> 
> It might be obvious to you, but I think it makes sense to also have some
> kind of documentation for this feature.

Ok, I'll update Documentation/PCI/endpoint/pci-endpoint.rst accordingly.

Kind regards,
Koichiro

> 
> 
> Kind regards,
> Niklas

Powered by blists - more mailing lists