From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id 531DE98 for ; Mon, 16 Jan 2017 20:58:05 +0100 (CET) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga105.fm.intel.com with ESMTP; 16 Jan 2017 11:58:04 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,240,1477983600"; d="scan'208";a="923182108" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.237.220.38]) ([10.237.220.38]) by orsmga003.jf.intel.com with ESMTP; 16 Jan 2017 11:58:03 -0800 To: Shreyansh Jain , david.marchand@6wind.com References: <1484581107-2025-1-git-send-email-shreyansh.jain@nxp.com> <1484581107-2025-6-git-send-email-shreyansh.jain@nxp.com> Cc: dev@dpdk.org, thomas.monjalon@6wind.com From: Ferruh Yigit Message-ID: <4721b3d8-7374-76e8-d859-61c86a88a314@intel.com> Date: Mon, 16 Jan 2017 19:58:02 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.6.0 MIME-Version: 1.0 In-Reply-To: <1484581107-2025-6-git-send-email-shreyansh.jain@nxp.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH v6 5/8] eal: introduce bus scan and probe in EAL X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 16 Jan 2017 19:58:05 -0000 On 1/16/2017 3:38 PM, Shreyansh Jain wrote: > Each bus implementation defines their own callbacks for scanning bus > and probing devices available on the bus. Enable EAL to call bus specific > scan and probe functions during DPDK initialization. > > Existing PCI scan/probe continues to exist. It will removed in subsequent > patches. > > Signed-off-by: Shreyansh Jain <...> > +/* Scan all the buses for registering devices */ > +int > +rte_bus_scan(void) I hesitate to make this kind of (not really functional) comments in this stage of the release, but please feel free to ignore them as your wish. Previous patch is (4/8) for adding bus scan support, so why not this function (also .map and .h file part of it) added in prev patch? And if there is a specific patch for scan, probe can be another one? > +{ > + int ret; > + struct rte_bus *bus = NULL; > + > + TAILQ_FOREACH(bus, &rte_bus_list, next) { > + ret = bus->scan(); > + if (ret) { > + RTE_LOG(ERR, EAL, "Scan for (%s) bus failed.\n", > + bus->name); > + /* Error in scanning any bus stops the EAL init. */ > + return ret; > + } > + } > + > + return 0; > +} <...> > diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/common/eal_common_pci.c > index 0d799be..35da451 100644 <...> > + > +/* Add a PCI device to PCI Bus */ > +void > +rte_eal_pci_add_device(struct rte_pci_bus *pci_bus, > + struct rte_pci_device *pci_dev) I think more generic approach from previous version was better (rte_eal_bus_add_device()), but I guess they sacrificed for less modification. > +{ > + TAILQ_INSERT_TAIL(&pci_bus->device_list, pci_dev, next); > + /* Update Bus references */ > + pci_dev->device.bus = &pci_bus->bus; > +} > + <...> > > +/** > + * Structure describing the PCI bus > + */ > +struct rte_pci_bus { > + struct rte_bus bus; /**< Inherit the generic class */ > + struct rte_pci_device_list device_list; /**< List of PCI devices */ > + struct rte_pci_driver_list driver_list; /**< List of PCI drivers */ Why bus device/driver lists moved from rte_bus? Each bus will need this? Is it to change as less code as possible? <...> > + > +/** > + * Insert a PCI device in the PCI Bus at a particular location in the device > + * list. It also updates the PCI Bus reference of the new devices to be > + * inserted. Minor issue in document compilation: - warning: argument 'pci_dev' of command @param is not found - parameter 'new_pci_dev' not documented Similar warnings exists for rte_eal_pci_remove_device() too. Also following in rte_bus_scan(void) and rte_bus_probe(void) - warning: argument 'void' of command @param is not found > + * > + * @param exist_pci_dev > + * Existing PCI device in PCI Bus > + * @param pci_dev > + * PCI device to be added before exist_pci_dev > + * @return void > + */ > +void rte_eal_pci_insert_device(struct rte_pci_device *exist_pci_dev, > + struct rte_pci_device *new_pci_dev); > + <...>