Just an update on things fixed/updated in v7 against these comments:

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.

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?

v7 Contains a split patch for introducing probe handler and introducing
scan and probe in EAL.


+{
+       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

Pushed v7 to ML. 'make doc' is not reporting any warn/error now.
Thanks for pointing it out.


+ *
+ * @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);
+

<...>



Reply via email to