Re: [RFC v3 5/7] bus: factorize devargs lookup

[Bruce Richardson <bruce.richardson@intel.com>]

On Thu, Mar 05, 2026 at 05:45:47PM +0100, David Marchand wrote:
> Each bus reimplements some similar devargs lookup code.
> 
> The differences are in how some bus (PCI, VMBUS etc...) normalizes the
> device names. We can't use the .parse existing handler from outside the
> bus code itself, as the size of the bus specific device location address
> is unknown.
> Introduce a bus specific helper to compare two device names and
> hide this ugly detail.
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---

Looks generally good. Couple of minor naming suggestions inline below.

Acked-by: Bruce Richardson <bruce.richardson@intel.com>


>  drivers/bus/auxiliary/auxiliary_common.c | 16 ++-------
>  drivers/bus/cdx/cdx.c                    | 14 +-------
>  drivers/bus/dpaa/dpaa_bus.c              | 41 +++++++++++++-----------
>  drivers/bus/fslmc/fslmc_bus.c            | 34 ++++++++++----------
>  drivers/bus/pci/pci_common.c             | 38 +++++++++++-----------
>  drivers/bus/platform/platform.c          | 17 ++--------
>  drivers/bus/uacce/uacce.c                | 19 ++---------
>  drivers/bus/vmbus/linux/vmbus_bus.c      |  2 +-
>  drivers/bus/vmbus/private.h              |  3 --
>  drivers/bus/vmbus/vmbus_common.c         | 30 ++++++++---------
>  drivers/dma/idxd/idxd_bus.c              | 14 ++------
>  lib/eal/common/eal_common_bus.c          | 20 ++++++++++++
>  lib/eal/include/bus_driver.h             | 31 ++++++++++++++++++
>  13 files changed, 132 insertions(+), 147 deletions(-)
> 

<snip>

> diff --git a/lib/eal/common/eal_common_bus.c b/lib/eal/common/eal_common_bus.c
> index 0a2311a342..863c7418bb 100644
> --- a/lib/eal/common/eal_common_bus.c
> +++ b/lib/eal/common/eal_common_bus.c
> @@ -8,6 +8,7 @@
>  
>  #include <bus_driver.h>
>  #include <rte_debug.h>
> +#include <rte_devargs.h>
>  #include <rte_string_fns.h>
>  #include <rte_errno.h>
>  
> @@ -205,6 +206,25 @@ rte_bus_find_by_name(const char *busname)
>  	return rte_bus_find(NULL, cmp_bus_name, (const void *)busname);
>  }
>  
> +RTE_EXPORT_INTERNAL_SYMBOL(rte_bus_find_devargs)
> +struct rte_devargs *
> +rte_bus_find_devargs(const struct rte_bus *bus, const char *name)
> +{
> +	rte_bus_devname_compare_t cmp = bus->devname_compare;
> +	struct rte_devargs *devargs;
> +
> +	if (cmp == NULL)
> +		cmp = strcmp;
> +
> +	RTE_EAL_DEVARGS_FOREACH(rte_bus_name(bus), devargs) {
> +		if (cmp(name, devargs->name) != 0)
> +			continue;
> +		return devargs;
> +	}
> +
> +	return NULL;
> +}
> +
>  static int
>  bus_can_parse(const struct rte_bus *bus, const void *_name)
>  {
> diff --git a/lib/eal/include/bus_driver.h b/lib/eal/include/bus_driver.h
> index 60527b75b6..430906772d 100644
> --- a/lib/eal/include/bus_driver.h
> +++ b/lib/eal/include/bus_driver.h
> @@ -118,6 +118,21 @@ typedef int (*rte_bus_unplug_t)(struct rte_device *dev);
>   */
>  typedef int (*rte_bus_parse_t)(const char *name, void *addr);
>  
> +/**
> + * Bus specific device name comparison function.
> + * Bus can normalize the names of devices using an internal representation.
> + * This helper makes it possible to check whether two names refer to the same device.
> + *
> + * @param[in] name1
> + *	device information location address,
> + * @param[in] name2
> + *	device information location address,
> + *
> + * @return
> + *      true or false
> + */
> +typedef int (*rte_bus_devname_compare_t)(const char *name1, const char *name2);
> +

This is a good addition.

>  /**
>   * Parse bus part of the device arguments.
>   *
> @@ -258,6 +273,7 @@ struct rte_bus {
>  	rte_bus_plug_t plug;         /**< Probe single device for drivers */
>  	rte_bus_unplug_t unplug;     /**< Remove single device from driver */
>  	rte_bus_parse_t parse;       /**< Parse a device name */
> +	rte_bus_devname_compare_t devname_compare; /**< Compare two device names */
>  	rte_bus_devargs_parse_t devargs_parse; /**< Parse bus devargs */
>  	rte_dev_dma_map_t dma_map;   /**< DMA map for device in the bus */
>  	rte_dev_dma_unmap_t dma_unmap; /**< DMA unmap for device in the bus */
> @@ -281,6 +297,21 @@ struct rte_bus {
>  __rte_internal
>  void rte_bus_register(struct rte_bus *bus);
>  
> +/**
> + * Find the devargs associated to a device.
> + *
> + * @param bus
> + *   A pointer to a rte_bus structure describing the bus
> + *   to be unregistered.
> + * @param dev_name
> + *   A device name.
> + *
> + * @return
> + *   Pointer to the devargs, or NULL if none found.
> + */
> +__rte_internal
> +struct rte_devargs *rte_bus_find_devargs(const struct rte_bus *bus, const char *name);
> +

The doxygen says the parameter is dev_name, while the prototype only uses
name. I'd prefer this to be "dev_name". [I nearly one if the function
itself should have an extra "_" and be "find_dev_args", as it's finding
the args for a particular device, rather than finding the "devargs" for the
bus itself.]

>  /**
>   * Helper for Bus registration.
>   * The constructor has higher priority than PMD constructors.
> -- 
> 2.53.0
>