From mboxrd@z Thu Jan 1 00:00:00 1970 From: Shreyansh Jain Subject: Re: [PATCH v3 04/12] eal/bus: add scan, match and insert support Date: Fri, 16 Dec 2016 18:55:21 +0530 Message-ID: <8c576d0f-cac5-2a0e-8e33-fccbc6c90787@nxp.com> References: <1481636232-2300-1-git-send-email-shreyansh.jain@nxp.com> <1481893853-31790-1-git-send-email-shreyansh.jain@nxp.com> <1481893853-31790-5-git-send-email-shreyansh.jain@nxp.com> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Cc: , , , To: Return-path: Received: from NAM01-BY2-obe.outbound.protection.outlook.com (mail-by2nam01on0076.outbound.protection.outlook.com [104.47.34.76]) by dpdk.org (Postfix) with ESMTP id 60FBC3990 for ; Fri, 16 Dec 2016 14:22:41 +0100 (CET) In-Reply-To: <1481893853-31790-5-git-send-email-shreyansh.jain@nxp.com> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Friday 16 December 2016 06:40 PM, Shreyansh Jain wrote: > When a PMD is registred, it will associate itself with a bus. > > A bus is responsible for 'scan' of all the devices attached to it. > All the scanned devices are attached to bus specific device_list. > During the probe operation, 'match' of the drivers and devices would > be done. > > Also, rather than adding a device to tail, a new device might be added to > the list (pivoted on bus) at a predefined position, for example, adding it > in order of addressing. Support for this is added as '*bus_insert'. > > This patch also adds necessary test framework to test the scan and > match callbacks. > > Signed-off-by: Shreyansh Jain > --- > app/test/test_bus.c | 265 ++++++++++++++++++++++++++++++++ > lib/librte_eal/common/eal_common_bus.c | 15 ++ > lib/librte_eal/common/include/rte_bus.h | 64 ++++++++ > 3 files changed, 344 insertions(+) > > diff --git a/app/test/test_bus.c b/app/test/test_bus.c > index 760d40a..ed95479 100644 > --- a/app/test/test_bus.c > +++ b/app/test/test_bus.c > @@ -80,12 +80,32 @@ struct dummy_bus { > struct rte_bus_list orig_bus_list = > TAILQ_HEAD_INITIALIZER(orig_bus_list); > > +/* Forward declarations for callbacks from bus */ > + > +/* Bus A > + * Scan would register devA1 and devA2 to bus > + */ > +int scan_fn_for_busA(struct rte_bus *bus); > + > +/* Bus B > + * Scan would register devB1 and devB2 to bus > + */ > +int scan_fn_for_busB(struct rte_bus *bus); > + > +/* generic implementations wrapped around by above declarations */ > +static int generic_scan_fn(struct rte_bus *bus); > +static int generic_match_fn(struct rte_driver *drv, struct rte_device *dev); > + > struct rte_bus busA = { > .name = "busA", /* "busA" */ > + .scan = scan_fn_for_busA, > + .match = generic_match_fn, > }; > > struct rte_bus busB = { > .name = "busB", /* "busB */ > + .scan = scan_fn_for_busB, > + .match = generic_match_fn, > }; > > struct rte_driver driverA = { > @@ -184,6 +204,92 @@ dump_device_tree(void) > printf("------>8-------\n"); > } > > +/* @internal > + * Move over the dummy_buses and find the entry matching the bus object > + * passed as argument. > + * For each device in that dummy_buses list, register. > + * > + * @param bus > + * bus to scan againt test entry > + * @return > + * 0 for successful scan, even if no devices are found > + * !0 for any error in scanning (like, invalid bus) > + */ > +static int > +generic_scan_fn(struct rte_bus *bus) > +{ > + int i = 0; > + struct rte_device *dev = NULL; > + struct dummy_bus *db = NULL; > + > + if (!bus) > + return -1; > + > + /* Extract the device tree node using the bus passed */ > + for (i = 0; dummy_buses[i].name; i++) { > + if (!strcmp(dummy_buses[i].name, bus->name)) { > + db = &dummy_buses[i]; > + break; > + } > + } > + > + if (!db) > + return -1; > + > + /* For all the devices in the device tree (dummy_buses), add device */ > + for (i = 0; db->devices[i]; i++) { > + dev = &(db->devices[i]->dev); > + rte_eal_bus_add_device(bus, dev); > + } > + > + return 0; > +} > + > +/* @internal > + * Obtain bus from driver object. Match the address of rte_device object > + * with all the devices associated with that bus. > + * > + * Being a test function, all this does is validate that device object > + * provided is available on the same bus to which driver is registered. > + * > + * @param drv > + * driver to which matching is to be performed > + * @param dev > + * device object to match with driver > + * @return > + * 0 for successful match > + * !0 for failed match > + */ > +static int > +generic_match_fn(struct rte_driver *drv, struct rte_device *dev) > +{ > + struct rte_bus *bus; > + struct rte_device *dev_p = NULL; > + > + /* Match is based entirely on address of 'dev' and 'dev_p' extracted > + * from bus->device_list. > + */ > + > + /* a driver is registered with the bus *before* the scan. */ > + bus = drv->bus; > + TAILQ_FOREACH(dev_p, &bus->device_list, next) { > + if (dev == dev_p) > + return 0; > + } > + > + return 1; > +} > + > +int > +scan_fn_for_busA(struct rte_bus *bus) { > + return generic_scan_fn(bus); > +} > + > +int > +scan_fn_for_busB(struct rte_bus *bus) { > + return generic_scan_fn(bus); > +} > + > static int > test_bus_setup(void) > { > @@ -391,6 +497,155 @@ test_driver_unregistration_on_bus(void) > > } > > +static int > +test_device_unregistration_on_bus(void) > +{ > + int i; > + struct rte_bus *bus = NULL; > + struct rte_device *dev; > + > + for (i = 0; dummy_buses[i].name; i++) { > + bus = rte_eal_get_bus(dummy_buses[i].name); > + if (!bus) { > + printf("Unable to find bus (%s)\n", > + dummy_buses[i].name); > + return -1; > + } > + > + /* For bus 'bus', unregister all devices */ > + TAILQ_FOREACH(dev, &bus->device_list, next) { > + rte_eal_bus_remove_device(dev); > + } > + } > + > + for (i = 0; dummy_buses[i].name; i++) { > + bus = rte_eal_get_bus(dummy_buses[i].name); > + > + if (!TAILQ_EMPTY(&bus->device_list)) { > + printf("Unable to remove all devices on bus (%s)\n", > + bus->name); > + return -1; > + } > + } > + > + /* All devices from all buses have been removed */ > + printf("All devices on all buses unregistered.\n"); > + dump_device_tree(); > + > + return 0; > +} > + > +/* @internal > + * For each bus registered, call the scan function to identify devices > + * on the bus. > + * > + * @param void > + * @return > + * 0 for successful scan > + * !0 for unsuccessful scan > + * > + */ > +static int > +test_bus_scan(void) > +{ > + int ret; > + struct rte_bus *bus; > + > + TAILQ_FOREACH(bus, &rte_bus_list, next) { > + /* Call the scan function for each bus */ > + ret = bus->scan(bus); > + if (ret) { > + printf("Scan of buses failed.\n"); > + return -1; > + } > + } > + > + printf("Scan of all buses completed.\n"); > + dump_device_tree(); > + > + return 0; > +} > + > +/* @internal > + * Function to perform 'probe' and link devices and drivers on a bus. > + * This would work over all the buses registered, and all devices and drivers > + * registered with it - call match on each pair. > + * Aim is to test the match_fn for each bus. > + * > + * @param void > + * @return > + * 0 for successful probe > + * !0 for failure in probe > + * > + */ > +static int > +test_probe_on_bus(void) > +{ > + int ret = 0; > + int i, j; > + struct rte_bus *bus = NULL; > + struct rte_device *dev = NULL; > + struct rte_driver *drv = NULL; > + > + /* In case of this test: > + * 1. for each bus in rte_bus_list > + * 2. for each device in bus->device_list > + * 3. for each driver in bus->driver_list > + * 4. call match > + * 5. link driver and device > + * 6. Verify the linkage. > + */ This comment style is indeed in a warning from checkpatch. My bad - somehow I missed fixing this before sending out. v4, along with any other major comments/reviews. - Shreyansh