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

[Niklas Cassel <cassel@kernel.org>]

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++) {