On Thu, 5 Mar 2026 at 18:06, Bruce Richardson <[email protected]> wrote: > > 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 <[email protected]> > > --- > > Looks generally good. Couple of minor naming suggestions inline below. > > Acked-by: Bruce Richardson <[email protected]> > > > > 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.]
Good catch. Err well, on doxygen / prototype misalignment.. documentation generation should fail if it is not aligned. Strange I did not see this. Ah.. it is a driver header, so we probably don't generate any doc for this. -- David Marchand
