[dpdk-dev] [PATCH 2/3] bus/pci: expose sysfs parsing API

[Wang, Xiao W]

Hi Rivet,

> -----Original Message-----
> From: Gaëtan Rivet [mailto:gaetan.rivet at 6wind.com]
> Sent: Friday, March 16, 2018 1:19 AM
> To: Wang, Xiao W <xiao.w.wang at intel.com>
> Cc: Burakov, Anatoly <anatoly.burakov at intel.com>; dev at dpdk.org; Wang,
> Zhihong <zhihong.wang at intel.com>; maxime.coquelin at redhat.com;
> yliu at fridaylinux.org; Liang, Cunming <cunming.liang at intel.com>; Xu, Rosen
> <rosen.xu at intel.com>; Chen, Junjie J <junjie.j.chen at intel.com>; Daly, Dan
> <dan.daly at intel.com>
> Subject: Re: [dpdk-dev] [PATCH 2/3] bus/pci: expose sysfs parsing API
> 
> On Thu, Mar 15, 2018 at 04:49:41PM +0000, Wang, Xiao W wrote:
> > Hi Rivet,
> >
> > > -----Original Message-----
> > > From: Gaëtan Rivet [mailto:gaetan.rivet at 6wind.com]
> > > Sent: Wednesday, March 14, 2018 9:31 PM
> > > To: Burakov, Anatoly <anatoly.burakov at intel.com>
> > > Cc: Wang, Xiao W <xiao.w.wang at intel.com>; dev at dpdk.org; Wang,
> Zhihong
> > > <zhihong.wang at intel.com>; maxime.coquelin at redhat.com;
> > > yliu at fridaylinux.org; Liang, Cunming <cunming.liang at intel.com>; Xu,
> Rosen
> > > <rosen.xu at intel.com>; Chen, Junjie J <junjie.j.chen at intel.com>; Daly, Dan
> > > <dan.daly at intel.com>
> > > Subject: Re: [dpdk-dev] [PATCH 2/3] bus/pci: expose sysfs parsing API
> > >
> > > Hi,
> > >
> > > On Wed, Mar 14, 2018 at 11:19:31AM +0000, Burakov, Anatoly wrote:
> > > > On 09-Mar-18 11:08 PM, Xiao Wang wrote:
> > > > > Some existing sysfs parsing functions are helpful for the later vDPA
> > > > > driver, this patch make them global and expose them to shared lib.
> > > > >
> > > > > Signed-off-by: Xiao Wang <xiao.w.wang at intel.com>
> > > > > ---
> > > > >   drivers/bus/pci/linux/pci.c             | 9 ++++-----
> > > > >   drivers/bus/pci/linux/pci_init.h        | 8 ++++++++
> > > > >   drivers/bus/pci/rte_bus_pci_version.map | 8 ++++++++
> > > > >   3 files changed, 20 insertions(+), 5 deletions(-)
> > > > >
> > > > > diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
> > > > > index abde64119..81e5e5650 100644
> > > > > --- a/drivers/bus/pci/linux/pci.c
> > > > > +++ b/drivers/bus/pci/linux/pci.c
> > > > > @@ -32,7 +32,7 @@
> > > > >   extern struct rte_pci_bus rte_pci_bus;
> > > > > -static int
> > > > > +int
> > > > >   pci_get_kernel_driver_by_path(const char *filename, char *dri_name)
> > > >
> > > > Here and in other places - shouldn't this too be prefixed with rte_?
> > > >
> > >
> > > A public PCI function should be prefixed by rte_pci_ yes.
> >
> > OK, will add this prefix.
> >
> > >
> > > Additionally, if this function was to be exposed, then there should be a
> > > BSD implementation as well (shared map file).
> > >
> > > I don't know how BSD works, I'm not sure parsing the filesystem is the
> > > way to get a PCI driver name. If so, maybe the function should be called
> > > another, generic, way, that would work for both linux and BSD (and
> > > ideally, having a real BSD implementation).
> >
> > BSD is not parsing the filesystem, it uses PCIOCGETCONF ioctl to retrieve
> > PCI device information.
> > This function is quite linux, especially for the API name. I'm afraid we can
> > only return err on BSD for this API.
> 
> How about renaming the function to something like
> rte_pci_device_kdriver_name();
> 
> and allowing for a sensible BSD implementation to happen if someone
> needs it?

Yes, it looks more generic, and allows a BSD implementation to happen.
I will rename it as below in next version.
rte_pci_device_kdriver_name(const struct rte_pci_addr *addr, char *dri_name)

BRs,
Xiao

> 
> --
> Gaëtan Rivet
> 6WIND