On Tuesday 17 January 2017 01:28 AM, Ferruh Yigit wrote:
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 <shreyansh.j...@nxp.com>
<...>
+/* 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.
No issues - any review comment is welcome.
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?
Maybe I didn't get your point well, but this is my take:
4/8 is for introducing the scan callbacks into the Bus layer. There is
no work done for EAL yet in 4/8.
This patch, 5/8, adds functions (not callbacks) and modifications to EAL
for plugging the bus (with its scan/probe) into EAL.
And if there is a specific patch for scan, probe can be another one?
I agree with this.
+{
+ 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.
There was a lot of offline discussions after the reviews were posted
until v5. From the gist of it, I gathered that smaller 'specific'
changes are preferred as compared to complete generic approach without
an upfront usecase.
Also, this change became irrelevant once I moved out the device/driver
lists from rte_bus to rte_xxx_bus. This is inline with existing model of
(rte_pci_device->rte_device, rte_pci_driver->rte_driver,
rte_pci_bus->rte_bus) and having pci_device_list/pci_driver_list private
to PCI itself.
I guess what is going to happen is that in near future when buses start
moving to drivers/bus/*, this kind of generic layer would come in.
+{
+ 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?
Yes, to move towards a minimal change.
Also, in sync with existing 'pci_device_list/pci_driver_list'.
<...>
+
+/**
+ * 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
I will fix and post v7 very shortly.
+ *
+ * @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);
+
<...>
