[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aXyMWIHy_gMnIxOX@ryzen>
Date: Fri, 30 Jan 2026 11:47:52 +0100
From: Niklas Cassel <cassel@...nel.org>
To: Aksh Garg <a-garg7@...com>
Cc: Koichiro Den <den@...inux.co.jp>, linux-pci@...r.kernel.org,
jingoohan1@...il.com, mani@...nel.org, lpieralisi@...nel.org,
kwilczynski@...nel.org, robh@...nel.org, bhelgaas@...gle.com,
Zhiqiang.Hou@....com, gustavo.pimentel@...opsys.com,
linux-kernel@...r.kernel.org, s-vadapalli@...com,
danishanwar@...com
Subject: Re: [PATCH v4 2/3] PCI: dwc: ep: Add per-PF BAR and inbound ATU
mapping support
On Fri, Jan 30, 2026 at 04:09:26PM +0530, Aksh Garg wrote:
> On 30/01/26 15:23, Niklas Cassel wrote:
> > On Fri, Jan 30, 2026 at 09:42:43AM +0530, Aksh Garg wrote:
> > > On 29/01/26 19:44, Niklas Cassel wrote:
> > > > 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.
> > > >
> > >
> > > I omitted this NULL check here because all the current call sites of
> > > dw_pcie_ep_clear_ib_maps() already perform this validation. I felt
> > > adding it here would add redundancy in the code.
> >
> > Ok, but with that logic, shouldn't we also remove the NULL checks from
> > dw_pcie_ep_ib_atu_bar() and dw_pcie_ep_ib_atu_addr(), because they are
> > only called from dw_pcie_ep_set_bar(), which already has the ep_func
> > NULL check?
> >
>
> Yes, that's correct. Alternatively, we can add the NULL check in
> dw_pcie_ep_clear_ib_maps() as well, making all the functions using
> ep_func self-contained and defensive, removing the dependency on whether
> the callers perform NULL checks. This makes the code more future proof,
> as new callers won't need to be aware of the NULL pointer possibility.
Sounds like a good idea to me.
Neither of these functions is in the hot path, so the performance of having
one less NULL check is not super critical.
Kind regards,
Niklas
Powered by blists - more mailing lists