On Thu, Jan 22, 2026 at 05:59:18PM +0100, Niklas Cassel wrote:
> On 22 January 2026 15:29:02 CET, Koichiro Den <[email protected]> wrote:
> >
> >> To make sure that dw_pcie_ep_ib_atu_addr() cannot be called without already
> >> having a BAR configured, to we perhaps want something like:
> >
> >Thanks for the review.
> >Isn't the existing guard in dw_pcie_ep_ib_atu_addr sufficient?
> >
> >        [...]
> >        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;
> >        }
> >
> 
> Well, for a driver that does not call dw_pcie_ep_reset_bar() in their .init() 
> to disable all BARs that are enabled in the controller by default, the host 
> side will assign an PCI address even if no EPF has called set_bar() on that 
> BAR.

Thanks for the explanation.

> 
> See e.g.
> https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/commit/drivers/pci/controller/dwc/pcie-tegra194.c?h=controller/dwc&id=42f9c66a6d0cc45758dab77233c5460e1cf003df
> 
> There might be other EPC drivers that don't disable all BARs in their 
> .init(), so I would say that simply checking if the BAR has an address is not 
> sufficient to guarantee that an EPF driver has called set_bar().
> 

Even if an EPC driver does not reset the BAR in their .init() and some
default translation is left exposed, wouldn't it be safe as long as
dw_pcie_ep_ib_atu_addr() succeeds in programming inbound mappings for the
entire BAR?

That said, such usage apparently contradicts the documented usage (1st
set_bar with no submap, then with submap) described in the docs and commit
messages in this series, and allowing it would make things unnecessarily
complicated. So I agree that adding such a safeguard is the right approach.

> 
> I think the safest option is my second suggestion because then we know that 
> we will only call
> dw_pcie_ep_ib_atu_addr()
> 
> When:
> 
> 1) If ep->epf_bar[bar] is set:
> https://github.com/torvalds/linux/blob/v6.19-rc6/drivers/pci/controller/dwc/pcie-designware-ep.c#L363
> 
> 
> 2) All the other requirements to dynamically update a BAR is also met:
> 
> https://github.com/torvalds/linux/blob/v6.19-rc6/drivers/pci/controller/dwc/pcie-designware-ep.c#L368-L370
> 

That makes sense, and it ensures that the behavior always accords with the
docs and commit messages in this series.

Thanks a lot for the careful review,
Koichiro

> 
> 
> Kind regards,
> Niklas
> 

Reply via email to