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

Shreyansh Jain shreyansh.jain at nxp.com
Tue Jan 17 11:13:06 CET 2017


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.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?

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



More information about the dev mailing list