[dpdk-dev] [PATCH v7 5/6] bus: add helper to find a bus from a device name
Shreyansh Jain
shreyansh.jain at nxp.com
Wed Jul 5 15:35:40 CEST 2017
Hello Gaetan,
Some comments inline:
On Wednesday 05 July 2017 05:25 AM, Gaetan Rivet wrote:
> Find which bus should be able to parse this device name into an internal
> device representation.
>
> Signed-off-by: Gaetan Rivet <gaetan.rivet at 6wind.com>
> ---
> lib/librte_eal/common/eal_common_bus.c | 21 +++++++++++++++++++++
> lib/librte_eal/common/eal_private.h | 12 ++++++++++++
> 2 files changed, 33 insertions(+)
>
> diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
> index 87b0c6e..34fcfa1 100644
> --- a/lib/librte_eal/common/eal_common_bus.c
> +++ b/lib/librte_eal/common/eal_common_bus.c
> @@ -204,3 +204,24 @@ rte_bus_find_by_name(const char *busname)
> {
> return rte_bus_find(NULL, cmp_bus_name, (const void *)busname);
> }
> +
> +static int
> +bus_can_parse(const struct rte_bus *bus, const void *_name)
> +{
> + const char *name = _name;
> +
> + return !(bus->parse && bus->parse(name, NULL) == 0);
> +}
> +
> +struct rte_bus *
> +rte_bus_find_by_device_name(const char *str)
> +{
> + char name[32];
It is possible to use a constant here? Basically, I am not sure why '32'
has been chosen - or maybe, it might remind a reader in future the
reason for this value.
Just to clarify: is there any documented limit on bus name? Until this
point, the name (and length) of bus was responsibility of bus driver
implementation. eal_common_bus.c doesn't codify any limit - so, this may
have to be advertised, even if just within the code.
> + char *c;
> +
> + snprintf(name, sizeof(name), "%s", str);
> + c = strchr(name, ',');
I saw a lot of discussion on the naming assumptions/presumptions.
Though, I am not sure I have an understood conclusion from that.
So, this might be incorrect/ill-informed:
Is it assumed that ',' is not present in device name?
Do you think it would be better if this is documented that ',' in a
device name is reserved (as per devargs)? (or, is this already known?)
For example, in case of DPAA2 devices, I didn't consider this as a case
while generating names while scanning the bus (though, I didn't have a
',' in the name). But, if a well known fact, bus drivers can normalize
their device names before creating device names.
> + if (c != NULL)
> + c[0] = '\0';
> + return rte_bus_find(NULL, bus_can_parse, name);
> +}
> diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h
> index 6cacce0..0836339 100644
> --- a/lib/librte_eal/common/eal_private.h
> +++ b/lib/librte_eal/common/eal_private.h
> @@ -338,4 +338,16 @@ int rte_eal_hugepage_attach(void);
> */
> bool rte_eal_using_phys_addrs(void);
>
> +/**
> + * Find a bus capable of identifying a device.
> + *
> + * @param str
> + * A device identifier (PCI address, virtual PMD name, ...).
> + *
> + * @return
> + * A valid bus handle if found.
> + * NULL if no bus is able to parse this device.
> + */
> +struct rte_bus *rte_bus_find_by_device_name(const char *str);
> +
> #endif /* _EAL_PRIVATE_H_ */
>
(Apologies for commenting so late in series - feel free to ignore if
this disrupts your cycle or sounds out-of-context.)
-
Shreyansh
More information about the dev
mailing list