[dpdk-dev] [PATCH v6 5/8] eal: introduce bus scan and probe in EAL

[Ferruh Yigit]

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.jain at 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);
> +

<...>