diff mbox

[dpdk-dev,08/13] eal: enable probe and remove from bus infrastructure

Message ID 1480846288-2517-9-git-send-email-shreyansh.jain@nxp.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers

Checks

Context Check Description
checkpatch/checkpatch success coding style OK

Commit Message

Shreyansh Jain Dec. 4, 2016, 10:11 a.m. UTC 
  The model is:
 rte_eal_init
 `--> calls rte_eal_bus_probe()
      This iterates over all the drivers and devices and matches them. For
      matched bus specific device-driver:
      `--> bus->probe()
           This would be responsible for generic work, equivalent to
           rte_eal_pci_probe - specific to the bus. Handles over control
           to,
           `--> rte_driver->probe()
                which works equivalent to rte_eth_dev_pci_probe for alloc-
                -ating a ethernet device. This would hand over the control
                to,
                `--> rte_xxx_driver->probe()
                     Calls driver specific initialization of the eth_dev
                     Similar to what eth_dev_init of eth_driver does.
                     In further changes, eth_driver would be removed.

Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
---
 lib/librte_eal/common/eal_common_bus.c | 50 +++++++++++++++++++++++++++++++++-
 1 file changed, 49 insertions(+), 1 deletion(-)

Comments

Shreyansh Jain Dec. 6, 2016, 10:45 a.m. UTC | #1 
On Sunday 04 December 2016 03:41 PM, Shreyansh Jain wrote:
> The model is:
>  rte_eal_init
>  `--> calls rte_eal_bus_probe()
>       This iterates over all the drivers and devices and matches them. For
>       matched bus specific device-driver:
>       `--> bus->probe()
>            This would be responsible for generic work, equivalent to
>            rte_eal_pci_probe - specific to the bus. Handles over control
>            to,
>            `--> rte_driver->probe()
>                 which works equivalent to rte_eth_dev_pci_probe for alloc-
>                 -ating a ethernet device. This would hand over the control
>                 to,
>                 `--> rte_xxx_driver->probe()
>                      Calls driver specific initialization of the eth_dev
>                      Similar to what eth_dev_init of eth_driver does.
>                      In further changes, eth_driver would be removed.
>
> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
> ---
>  lib/librte_eal/common/eal_common_bus.c | 50 +++++++++++++++++++++++++++++++++-
>  1 file changed, 49 insertions(+), 1 deletion(-)
>
> diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
> index 67b808b..469abac 100644
> --- a/lib/librte_eal/common/eal_common_bus.c
> +++ b/lib/librte_eal/common/eal_common_bus.c
> @@ -195,11 +195,59 @@ rte_eal_bus_scan(void)
>  	return 0;
>  }
>
> +static int
> +perform_probe(struct rte_bus *bus __rte_unused, struct rte_driver *driver,
> +	      struct rte_device *device)
> +{
> +	int ret;
> +
> +	if (!driver->probe) {
> +		RTE_LOG(ERR, EAL, "Driver (%s) doesn't support probe.\n",
> +			driver->name);
> +		/* This is not an error - just a badly implemented PMD */
> +		return 0;
> +	}
> +
> +	ret = driver->probe(driver, device);

Just noticed an issue here: It is unlike what I have stated in the patch 
headline. bus->probe() is not getting called.


Just to open a debate:
eal=> rte_bus->probe() => rte_driver->probe() => rte_pci_driver->probe()
is debatable. 1) bus never probes and 2) it just looks very odd.

But, if bus->probe() doesn't happen, which layer is responsible for 
things like devargs which currently rte_eal_pci_probe() does in EAL.
If rte_driver->probe() does the devargs layer, eth_dev allocation might 
be shifted to rte_pci_driver->probe() - and subsequently calling 
rte_eth_dev->eth_dev_init().

I do need to get my head around this. Suggestions would be really 
appreciated.

> +	if (ret < 0)
> +		/* One of the probes failed */
> +		RTE_LOG(ERR, EAL, "Probe failed for (%s).\n", driver->name);
> +
> +	/* In either case, ret <0 (error), ret > 0 (not supported) and ret = 0
> +	 * success, return ret
> +	 */
> +	return ret;
> +}
> +
>  /* Match driver<->device and call driver->probe() */
>  int
>  rte_eal_bus_probe(void)
>  {
> -	/* Until driver->probe is available, this is dummy implementation */
> +	int ret;
> +	struct rte_bus *bus;
> +	struct rte_device *device;
> +	struct rte_driver *driver;
> +
> +	/* For each bus registered with EAL */
> +	TAILQ_FOREACH(bus, &rte_bus_list, next) {
> +		TAILQ_FOREACH(device, &bus->device_list, next) {
> +			TAILQ_FOREACH(driver, &bus->driver_list, next) {
> +				ret = bus->match(driver, device);
> +				if (!ret) {
> +					ret = perform_probe(bus, driver,
> +							    device);
> +					if (ret < 0)
> +						return ret;
> +
> +					/* ret == 0 is success; ret >0 implies
> +					 * driver doesn't support the device.
> +					 * in either case, continue
> +					 */
> +				}
> +			}
> +		}
> +	}
> +
>  	return 0;
>  }
>
>
diff mbox

Patch

diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
index 67b808b..469abac 100644
--- a/lib/librte_eal/common/eal_common_bus.c
+++ b/lib/librte_eal/common/eal_common_bus.c
@@ -195,11 +195,59 @@  rte_eal_bus_scan(void)
 	return 0;
 }
 
+static int
+perform_probe(struct rte_bus *bus __rte_unused, struct rte_driver *driver,
+	      struct rte_device *device)
+{
+	int ret;
+
+	if (!driver->probe) {
+		RTE_LOG(ERR, EAL, "Driver (%s) doesn't support probe.\n",
+			driver->name);
+		/* This is not an error - just a badly implemented PMD */
+		return 0;
+	}
+
+	ret = driver->probe(driver, device);
+	if (ret < 0)
+		/* One of the probes failed */
+		RTE_LOG(ERR, EAL, "Probe failed for (%s).\n", driver->name);
+
+	/* In either case, ret <0 (error), ret > 0 (not supported) and ret = 0
+	 * success, return ret
+	 */
+	return ret;
+}
+
 /* Match driver<->device and call driver->probe() */
 int
 rte_eal_bus_probe(void)
 {
-	/* Until driver->probe is available, this is dummy implementation */
+	int ret;
+	struct rte_bus *bus;
+	struct rte_device *device;
+	struct rte_driver *driver;
+
+	/* For each bus registered with EAL */
+	TAILQ_FOREACH(bus, &rte_bus_list, next) {
+		TAILQ_FOREACH(device, &bus->device_list, next) {
+			TAILQ_FOREACH(driver, &bus->driver_list, next) {
+				ret = bus->match(driver, device);
+				if (!ret) {
+					ret = perform_probe(bus, driver,
+							    device);
+					if (ret < 0)
+						return ret;
+
+					/* ret == 0 is success; ret >0 implies
+					 * driver doesn't support the device.
+					 * in either case, continue
+					 */
+				}
+			}
+		}
+	}
+
 	return 0;
 }