Messages in this thread | | | From | "Thokala, Srikanth" <> | Subject | RE: [PATCH v2 09/34] misc: xlink-pcie: lh: Add PCIe EPF driver for Local Host | Date | Sun, 24 Jan 2021 18:18:06 +0000 |
| |
Hi Greg,
> -----Original Message----- > From: Greg KH <gregkh@linuxfoundation.org> > Sent: Sunday, January 24, 2021 5:27 PM > To: Thokala, Srikanth <srikanth.thokala@intel.com> > Cc: mgross@linux.intel.com; markgross@kernel.org; arnd@arndb.de; > bp@suse.de; damien.lemoal@wdc.com; dragan.cvetic@xilinx.com; > corbet@lwn.net; leonard.crestez@nxp.com; palmerdabbelt@google.com; > paul.walmsley@sifive.com; peng.fan@nxp.com; robh+dt@kernel.org; > shawnguo@kernel.org; jassisinghbrar@gmail.com; linux- > kernel@vger.kernel.org; Derek Kiernan <derek.kiernan@xilinx.com> > Subject: Re: [PATCH v2 09/34] misc: xlink-pcie: lh: Add PCIe EPF driver > for Local Host > > On Sun, Jan 24, 2021 at 11:48:29AM +0000, Thokala, Srikanth wrote: > > > > +{ > > > > + struct pci_epf_bar *epf_bar; > > > > + bool bar_fixed_64bit; > > > > + int ret, i; > > > > + > > > > + for (i = BAR_0; i <= BAR_5; i++) { > > > > + epf_bar = &epf->bar[i]; > > > > + bar_fixed_64bit = !!(epc_features->bar_fixed_64bit & (1 > << > > > i)); > > > > + if (bar_fixed_64bit) > > > > + epf_bar->flags |= PCI_BASE_ADDRESS_MEM_TYPE_64; > > > > + if (epc_features->bar_fixed_size[i]) > > > > + epf_bar->size = epc_features->bar_fixed_size[i]; > > > > + > > > > + if (i == BAR_2) { > > > > + ret = intel_xpcie_check_bar(epf, epf_bar, BAR_2, > > > > + BAR2_MIN_SIZE, > > > > + epc_features->reserved_bar); > > > > + if (ret) > > > > + return ret; > > > > + } > > > > + > > > > + if (i == BAR_4) { > > > > + ret = intel_xpcie_check_bar(epf, epf_bar, BAR_4, > > > > + BAR4_MIN_SIZE, > > > > + epc_features->reserved_bar); > > > > + if (ret) > > > > + return ret; > > > > + } > > > > > > Why do you need to check all of this? Where is the data coming from > > > that could be incorrect? > > > > PCI BAR attributes, as inputs, are coming from the PCIe controller > driver > > through PCIe End Point Framework. These checks are required to compare > the > > configuration this driver is expecting to the configuration coming from > > the PCIe controller driver. > > So why do you not trust that information coming from the caller? > Shouldn't it always be correct as it already is validated by that > in-kernel caller? Don't check for things you don't have to check for > because you control the code that calls this stuff.
Sure, I agree to your point. I will fix it in my next version.
Thanks! Srikanth
> > thanks, > > greg k-h
| |