All of lore.kernel.org
 help / color / mirror / Atom feed
From: Niklas Cassel <cassel@kernel.org>
To: Koichiro Den <den@valinux.co.jp>
Cc: jingoohan1@gmail.com, mani@kernel.org, lpieralisi@kernel.org,
	kwilczynski@kernel.org, robh@kernel.org, bhelgaas@google.com,
	Frank.Li@nxp.com, linux-pci@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 2/2] PCI: dwc: ep: Support BAR subrange inbound mapping via address-match iATU
Date: Wed, 7 Jan 2026 15:27:28 +0100	[thread overview]
Message-ID: <aV5tUE02ipda-R76@ryzen> (raw)
In-Reply-To: <20260107041358.1986701-3-den@valinux.co.jp>

Hello Koichiro,


I like this design way more, where you have a one-shot (all-or-nothing)
submap programming to avoid leaving half-programmed BAR state.


On Wed, Jan 07, 2026 at 01:13:58PM +0900, Koichiro Den wrote:
> +/* Address Match Mode IB 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)
> +{
> +	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;
> +
> +	/* Work on a sorted copy */
> +	struct pci_epf_bar_submap *smap __free(kfree) = kcalloc(
> +				epf_bar->num_submap, sizeof(*smap), GFP_KERNEL);
> +	if (!smap)
> +		return -ENOMEM;
> +
> +	memcpy(smap, submap, epf_bar->num_submap * sizeof(*smap));
> +	sort(smap, epf_bar->num_submap, sizeof(*smap),
> +	     dw_pcie_ep_submap_offset_cmp, NULL);

My only comment is that:

Why not simply let dw_pcie_ep_validate_submap() return an error if the
caller of dw_pcie_ep_set_bar() did not provide a submap with offsets in
ascending order (i.e. sorted).

Performing an unconditional sort of the submap here looks a bit out of
place, IMO.


> +
> +	ret = dw_pcie_ep_validate_submap(ep, smap, epf_bar->num_submap, epf_bar->size);
> +	if (ret)
> +		return ret;


Kind regards,
Niklas

  reply	other threads:[~2026-01-07 14:27 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-07  4:13 [PATCH v2 0/2] PCI: endpoint: BAR subrange mapping support Koichiro Den
2026-01-07  4:13 ` [PATCH v2 1/2] PCI: endpoint: Add " Koichiro Den
2026-01-07  4:13 ` [PATCH v2 2/2] PCI: dwc: ep: Support BAR subrange inbound mapping via address-match iATU Koichiro Den
2026-01-07 14:27   ` Niklas Cassel [this message]
2026-01-08  1:42     ` Koichiro Den

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aV5tUE02ipda-R76@ryzen \
    --to=cassel@kernel.org \
    --cc=Frank.Li@nxp.com \
    --cc=bhelgaas@google.com \
    --cc=den@valinux.co.jp \
    --cc=jingoohan1@gmail.com \
    --cc=kwilczynski@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=lpieralisi@kernel.org \
    --cc=mani@kernel.org \
    --cc=robh@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.