[dpdk-dev] [PATCH v10 00/25] Introducing rte_driver/rte_device generalization

[Shreyansh Jain]

Hi David,

> -----Original Message-----
> From: David Marchand [mailto:david.marchand at 6wind.com]
> Sent: Friday, September 16, 2016 7:52 PM
> To: Shreyansh Jain <shreyansh.jain at nxp.com>; Thomas Monjalon
> <thomas.monjalon at 6wind.com>; Jan Viktorin <viktorin at rehivetech.com>
> Subject: Re: [PATCH v10 00/25] Introducing rte_driver/rte_device
> generalization
> 
> Commenting in the cover letter because these are details and it is
> easier to track down what is missing before push for Thomas.

OK.
Should I still wait for more patch level comments before releasing v11?

> 
> 
> >   driver: init/uninit common wrappers for PCI drivers
> 
> In this patch subject, init/uninit -> prove/remove.

Ok. I will do this.

> 
> 
> >   eal: define container macro
> 
> This patch is fine, but not used in the patchset. I would postpone it
> until we actually need it.

I introduced this based on the merging of Jan's series and subsequent use in SoC framework series. But, yes, it is not being used in this series.
I will remove it.

> 
> 
> >   eal: extract vdev infra
> 
> This patch commit log tells that the vdev register macro does not call
> DRIVER_EXPORT_NAME while it actually does so.

Yes, I updated the patch based on your suggestion but didn't notice that I had put a counter notice about this in commit log in v9. I will remove this.

> 
> 
> >   eal: remove unused PMD types
> 
> This patch commit log has some outdated note about pmdinfo.

Well, this patch didn't update the documentation and I was not sure if the only change for PMD Info tool is in the mk file as per your patch. 
I will change the suggested patch and add documentation related note in the commit log.
> 
> We still have a reference about PMD_REGISTER_DRIVER in the
> documentation that could be replaced with a note on
> DRIVER_REGISTER_PCI()

Are you suggesting in the code as a comment? If so, I don't think that is good idea.

> But I would do this in "drivers: convert all phy drivers as PCI drivers".

If I have to update, I will use the above patch.

> 
> 
> >   eal/pci: replace PCI devinit/devuninit with probe/remove
> 
> This patch only replaces the init/uninit methods with prove/remove for pci.
> I would generalise this to vdev as well.

That is because I didn't like using 'probe/remove' for VDEV. The semantic of probing is not really correct for a virtual device. We 'add' and initialize the virtual device actually.
If you and other still feel that making it analogous with PCI probe/remove, I will update it.

> 
> 
> --
> David Marchand

Thanks a lot for your time and comments.

-
Shreyansh