21/06/2017 13:39, Gaëtan Rivet: > On Wed, Jun 21, 2017 at 12:00:24PM +0200, Thomas Monjalon wrote: > > 21/06/2017 11:40, Gaëtan Rivet: > > > On Wed, Jun 21, 2017 at 09:57:18AM +0200, Thomas Monjalon wrote: > > > > Another (probably better) solution is to keep basic definitions > > > > and helpers in EAL: > > > > - rte_pci.h keeps only some PCI definitions and helpers > > > > like rte_pci_addr and eal_parse_pci_BDF() in EAL > > > > - bus management is done in the PCI driver > > > > > > > > For pmdinfogen, we just need struct rte_pci_id. > > > > Other tools or applications will probably need this kind of basic > > > > struct and functions available in EAL. > > > > > > I mostly agree, this proposal should be kept to a minimum at first for > > > this release and carefully expanded afterward. > > > > > > If that's ok, I will propose a new version of this patchset with a new > > > librte_pci, that might fix both pmdinfogen and librte_kni. > > > > Why creating a new librte_pci instead of just keeping it in EAL? > > While I agree that it makes sense to have PCI helpers shared among > several subsystems, I do not see a reason for the EAL to rely on it. > > The EAL is the bedrock of the whole system. Having those helpers within > would mean that one expects them to be used to build this bedrock. It > would be misleading. > > In the context of a framework, aimed at being used by others, an okay > architecture is one that works. A good architecture is one that > intrinsically convey meaning and explains its goal to developers relying > on it. I think that having this PCI lib within EAL just because nothing > prevents us from doing so is mistaken, in this regard. > > Conversely, the argument about being conservative in the changes, > especially to an essential part such as the EAL, is obsolete for this > release and this subsystem, as deep changes are necessary anyway, > and the design should be right from the get go to allow further stability. > > Finally, the PCI lib and bus is also an example for other developers. I > do not think that all other hardware buses should be allowed in adding > their own specific helpers to the EAL. In this regard, there is no > reason to have an exception made just for the PCI lib.
You get a point :)