Re: [PATCH v4 2/3] PCI: dwc: ep: Add per-PF BAR and inbound ATU mapping support

From: Niklas Cassel

Date: Thu Jan 29 2026 - 09:17:01 EST


On Thu, Jan 29, 2026 at 02:47:52PM +0530, Aksh Garg wrote:
> -static void dw_pcie_ep_clear_ib_maps(struct dw_pcie_ep *ep, enum pci_barno bar)
> +static void dw_pcie_ep_clear_ib_maps(struct dw_pcie_ep *ep, u8 func_no, enum pci_barno bar)
> {
> + struct dw_pcie_ep_func *ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> struct device *dev = pci->dev;
> unsigned int i, num;
> @@ -152,18 +157,18 @@ static void dw_pcie_ep_clear_ib_maps(struct dw_pcie_ep *ep, enum pci_barno bar)
> u32 *indexes;

Hello Aksh,

Considering that all other functions that you have modified, you have added a:

if (!ep_func)
return;


I think you should do the same to this function.


>
> /* Tear down the BAR Match Mode mapping, if any. */
> - if (ep->bar_to_atu[bar]) {
> - atu_index = ep->bar_to_atu[bar] - 1;
> + if (ep_func->bar_to_atu[bar]) {
> + atu_index = ep_func->bar_to_atu[bar] - 1;
> dw_pcie_disable_atu(pci, PCIE_ATU_REGION_DIR_IB, atu_index);
> clear_bit(atu_index, ep->ib_window_map);
> - ep->bar_to_atu[bar] = 0;
> + ep_func->bar_to_atu[bar] = 0;

Not related to your patch,

Koichiro (he is on To:),

don't you think that it would be clearer if we had a:
return;

here...

I mean, a BAR can either have a BAR match mode mapping or a subrange mapping,
but not both... So continuing executing code beyond this point seems pointless,
possibly even confusing.


> }
>
> /* Tear down all Address Match Mode mappings, if any. */
> - indexes = ep->ib_atu_indexes[bar];
> - num = ep->num_ib_atu_indexes[bar];
> - ep->ib_atu_indexes[bar] = NULL;
> - ep->num_ib_atu_indexes[bar] = 0;
> + indexes = ep_func->ib_atu_indexes[bar];
> + num = ep_func->num_ib_atu_indexes[bar];
> + ep_func->ib_atu_indexes[bar] = NULL;
> + ep_func->num_ib_atu_indexes[bar] = 0;
> if (!indexes)
> return;

Sure, I see that this code will do a return here...

So there will be no harm done, but, having a simple return above
would make it extra clear to the reader that is is always one or
the other... you cannot have both.

If you agree, perhaps you could send a one liner patch?


> for (i = 0; i < num; i++) {