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
