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

Shreyansh Jain shreyansh.jain at nxp.com
Tue Jan 17 06:03:14 CET 2017


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.

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



More information about the dev mailing list