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?
> +{
> + 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);
> +
<...>
