From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.tuxdriver.com (charlotte.tuxdriver.com [70.61.120.58]) by dpdk.org (Postfix) with ESMTP id 5B2176CA2 for ; Fri, 8 Jul 2016 15:57:28 +0200 (CEST) Received: from cpe-2606-a000-111b-40ed-7aac-c0ff-fec2-933b.dyn6.twc.com ([2606:a000:111b:40ed:7aac:c0ff:fec2:933b] helo=localhost) by smtp.tuxdriver.com with esmtpsa (TLSv1:AES256-SHA:256) (Exim 4.63) (envelope-from ) id 1bLWGv-0005Sh-0Q; Fri, 08 Jul 2016 09:56:57 -0400 Date: Fri, 8 Jul 2016 09:56:43 -0400 From: Neil Horman To: David Marchand Cc: Thomas Monjalon , "dev@dpdk.org" , Jan Viktorin Message-ID: <20160708135643.GC14917@hmsreliant.think-freely.org> References: <1467905790-10597-1-git-send-email-thomas.monjalon@6wind.com> <1467905790-10597-10-git-send-email-thomas.monjalon@6wind.com> <20160707161141.GH26064@hmsreliant.think-freely.org> <4984337.X4DjHj7U46@xps13> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.6.1 (2016-04-27) X-Spam-Score: -1.0 (-) X-Spam-Status: No Subject: Re: [dpdk-dev] [PATCH 09/11] eal: move PCI table macro X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 08 Jul 2016 13:57:28 -0000 On Fri, Jul 08, 2016 at 10:49:25AM +0200, David Marchand wrote: > Hello Thomas, Neil, > > (will be back in a couple of days, thanks Thomas for pointing this thread) > > > On Thu, Jul 7, 2016 at 6:25 PM, Thomas Monjalon > wrote: > > 2016-07-07 12:11, Neil Horman: > >> On Thu, Jul 07, 2016 at 05:36:28PM +0200, Thomas Monjalon wrote: > >> > Remove include of rte_pci.h in the generic header rte_dev.h > >> > and move the macro DRIVER_REGISTER_PCI_TABLE in rte_pci.h. > > [...] > >> > >> This seems strange to me, in that its odd for the driver information export > >> macros to be spread out in multiple locations. Specifically it enjoins the use > >> of the DRV_EXP_TAG macro, which helps centralize tag naming. Perhaps the happy > >> medium is to place all the export macros (includnig PMD_REGISTER_DRIVER) into > >> its own pmd_register.h header? > > > > I don't know. > > David, your opinion? > > - The suggestion I did offline to Thomas was to move pci stuff in pci headers. > We are trying to move from the "all pci" code in eal to accomodate for > other "buses" / architectures. I get that, but I'm not sure that applies here. The macro in question is specific to pci busses, and if there is additional bus information to export, it will have its own macro (e.g. DRIVER_REGISTER_USB_TABLE or some such). While I could see that being an argument for putting each macro in with its own bus type, I think thats the wrong organization here, in that people writing drivers will want to know what export macros are available and will expect to look in a single place for it. > Having a pci macro in a generic header like rte_dev.h is wrong to me. > Moving this to a new header like pmd_register.h sounds like a new > generic header with pci specific stuff in it. Well, yes, but I see that as no different than rte_ethdev.c or rte_pdump.c. both of those files will need to know about all the different types of busses you support and have to include those corresponding header files (i.e. they will have to include rte_pci.h, rte_usb.h, rte_i2c.h, etc). This is really no different in my mind. > So, I am not sure I follow you Neil. > > Can you elaborate ? > I suppose the best way to describe it is that while I understand and support the desire to separate and abstract bus information away from device function, I think theres a pragmatic descision here to prioritize functional domain over header inclusion. That is to say, I think when people are writing a driver, it will be helpful to have all the export macros in a single location so they know what information they can export, and that includes registration of various bus type identifiers. So a file like pmd_registration.h that includes rte_pci.h, rte_usb.h, rte_i2c.h, etc is more useful to a developer, than spreading these macros out to those various header files, for the sake of avoiding a potentially unneeded include. > > - Why do you want to centralise the tag naming ? > To avoid collisions ? Yes, and to centralize that information. Since the pmdinfogen tool needs to know what those tag names are as well, its useful to keep them in the same area to maintain co-ordination. Its also useful because it means we can use one macro to define tag naming convention, instead of having to re-implement or dead-reckon it in multiple files. > Well, adding those tags should not happen that often and I think we > can maintain this with careful reviews. I don't agree with that. This discussion is based on the fact that you expect that we will be adding additional bus types in the future correct? Well, given that we have a pci bus specific export macro, I would expect that to proliferate to every other bus type as well, and so we can expect to at least have a new tag added for every bus that is added, in addition to any other bus agnostic information people wish to export (just off hand, looking at the linux modinfo section, we might expect module author, module version, alias names, licensing infomration, and others to be potential export candidates). So, depending on how much this is adopted, I think we can potentially expect a great deal of additional tagging to be needed. Neil