Re: [PATCH v10 00/25] Introducing rte_driver/rte_device generalization

Hi David,

-----Original Message-----
From: David Marchand [mailto:david.marchand@6wind.com]
Sent: Friday, September 16, 2016 7:52 PM
To: Shreyansh Jain <redacted>; Thomas Monjalon
[off-list ref]; Jan Viktorin [off-list ref]
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?

quoted

  driver: init/uninit common wrappers for PCI drivers

In this patch subject, init/uninit -> prove/remove.

Ok. I will do this.

quoted

  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.

quoted

  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.

quoted

  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.

quoted

  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