[dpdk-dev] bus: fix driver registration
Checks
Commit Message
The bus name was stored with embedded double quotes.
Indeed the bus name is given with a string in a macro,
which is not used elsewhere.
These macros are useless because the buses are drivers,
so they must not have any API for the application writer.
The registration can be done with a hardcoded value without quotes.
There is another (small) benefit of not using macros for driver names:
it is to have a meaningful constructor function name.
For instance, it was businitfn_PCI_BUS_NAME instead of businitfn_pci.
The bus registration macro is also changed to use
the new RTE_INIT_PRIO macro, similar to RTE_INIT used for other drivers.
The priority is the highest (101) in order to be sure that the bus driver
is registered before its device drivers.
Fixes: 0fd1a0eaae19 ("pci: add bus driver")
Fixes: fea892e35f21 ("bus/vdev: use standard bus registration")
Fixes: 7e7df6d0a41d ("bus/fslmc: introduce fsl-mc bus driver")
Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
This patch is a proposal to replace the patch
"bus: fix bus name registration" in the series "rte_bus parse API".
---
drivers/bus/fslmc/fslmc_bus.c | 2 +-
drivers/bus/fslmc/rte_fslmc.h | 3 ---
lib/librte_eal/common/eal_common_pci.c | 2 +-
lib/librte_eal/common/eal_common_vdev.c | 2 +-
lib/librte_eal/common/include/rte_bus.h | 3 ++-
lib/librte_eal/common/include/rte_eal.h | 3 +++
lib/librte_eal/common/include/rte_pci.h | 3 ---
lib/librte_eal/common/include/rte_vdev.h | 2 --
8 files changed, 8 insertions(+), 12 deletions(-)
Comments
On Wednesday 05 July 2017 03:13 AM, Thomas Monjalon wrote:
> The bus name was stored with embedded double quotes.
> Indeed the bus name is given with a string in a macro,
> which is not used elsewhere.
> These macros are useless because the buses are drivers,
> so they must not have any API for the application writer.
> The registration can be done with a hardcoded value without quotes.
>
> There is another (small) benefit of not using macros for driver names:
> it is to have a meaningful constructor function name.
> For instance, it was businitfn_PCI_BUS_NAME instead of businitfn_pci.
>
> The bus registration macro is also changed to use
> the new RTE_INIT_PRIO macro, similar to RTE_INIT used for other drivers.
> The priority is the highest (101) in order to be sure that the bus driver
> is registered before its device drivers.
>
> Fixes: 0fd1a0eaae19 ("pci: add bus driver")
> Fixes: fea892e35f21 ("bus/vdev: use standard bus registration")
> Fixes: 7e7df6d0a41d ("bus/fslmc: introduce fsl-mc bus driver")
>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
> This patch is a proposal to replace the patch
> "bus: fix bus name registration" in the series "rte_bus parse API".
> ---
> drivers/bus/fslmc/fslmc_bus.c | 2 +-
> drivers/bus/fslmc/rte_fslmc.h | 3 ---
> lib/librte_eal/common/eal_common_pci.c | 2 +-
> lib/librte_eal/common/eal_common_vdev.c | 2 +-
> lib/librte_eal/common/include/rte_bus.h | 3 ++-
> lib/librte_eal/common/include/rte_eal.h | 3 +++
> lib/librte_eal/common/include/rte_pci.h | 3 ---
> lib/librte_eal/common/include/rte_vdev.h | 2 --
> 8 files changed, 8 insertions(+), 12 deletions(-)
>
For DPAA2 as well as generic change:
Acked-by: Shreyansh Jain <shreyansh.jain@nxp.com>
On Wednesday 05 July 2017 11:17 AM, Shreyansh Jain wrote:
> On Wednesday 05 July 2017 03:13 AM, Thomas Monjalon wrote:
>> The bus name was stored with embedded double quotes.
>> Indeed the bus name is given with a string in a macro,
>> which is not used elsewhere.
>> These macros are useless because the buses are drivers,
>> so they must not have any API for the application writer.
>> The registration can be done with a hardcoded value without quotes.
>>
>> There is another (small) benefit of not using macros for driver names:
>> it is to have a meaningful constructor function name.
>> For instance, it was businitfn_PCI_BUS_NAME instead of businitfn_pci.
>>
>> The bus registration macro is also changed to use
>> the new RTE_INIT_PRIO macro, similar to RTE_INIT used for other drivers.
>> The priority is the highest (101) in order to be sure that the bus driver
>> is registered before its device drivers.
>>
>> Fixes: 0fd1a0eaae19 ("pci: add bus driver")
>> Fixes: fea892e35f21 ("bus/vdev: use standard bus registration")
>> Fixes: 7e7df6d0a41d ("bus/fslmc: introduce fsl-mc bus driver")
>>
>> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
>> ---
>> This patch is a proposal to replace the patch
>> "bus: fix bus name registration" in the series "rte_bus parse API".
>> ---
>> drivers/bus/fslmc/fslmc_bus.c | 2 +-
>> drivers/bus/fslmc/rte_fslmc.h | 3 ---
>> lib/librte_eal/common/eal_common_pci.c | 2 +-
>> lib/librte_eal/common/eal_common_vdev.c | 2 +-
>> lib/librte_eal/common/include/rte_bus.h | 3 ++-
>> lib/librte_eal/common/include/rte_eal.h | 3 +++
>> lib/librte_eal/common/include/rte_pci.h | 3 ---
>> lib/librte_eal/common/include/rte_vdev.h | 2 --
>> 8 files changed, 8 insertions(+), 12 deletions(-)
>>
> For DPAA2 as well as generic change:
>
> Acked-by: Shreyansh Jain <shreyansh.jain@nxp.com>
>
I just noticed that Bruce has already Acked the patch which is super-seeded by this. I didn't mean to conflict here.
I don't have a reservation to either of these - but, I like this patch as it does away with priority as well as need for drivers to define an internal name strings. Thus, my ACK to this.
@@ -161,4 +161,4 @@ struct rte_fslmc_bus rte_fslmc_bus = {
.driver_list = TAILQ_HEAD_INITIALIZER(rte_fslmc_bus.driver_list),
};
-RTE_REGISTER_BUS(FSLMC_BUS_NAME, rte_fslmc_bus.bus);
+RTE_REGISTER_BUS(fslmc, rte_fslmc_bus.bus);
@@ -56,9 +56,6 @@ extern "C" {
#include <rte_dev.h>
#include <rte_bus.h>
-/** Name of FSLMC Bus */
-#define FSLMC_BUS_NAME "FSLMC"
-
struct rte_dpaa2_driver;
/* DPAA2 Device and Driver lists for FSLMC bus */
@@ -549,4 +549,4 @@ struct rte_pci_bus rte_pci_bus = {
.driver_list = TAILQ_HEAD_INITIALIZER(rte_pci_bus.driver_list),
};
-RTE_REGISTER_BUS(PCI_BUS_NAME, rte_pci_bus.bus);
+RTE_REGISTER_BUS(pci, rte_pci_bus.bus);
@@ -374,4 +374,4 @@ static struct rte_bus rte_vdev_bus = {
.unplug = vdev_unplug,
};
-RTE_REGISTER_BUS(VIRTUAL_BUS_NAME, rte_vdev_bus);
+RTE_REGISTER_BUS(vdev, rte_vdev_bus);
@@ -256,7 +256,8 @@ struct rte_bus *rte_bus_find_by_name(const char *busname);
* The constructor has higher priority than PMD constructors.
*/
#define RTE_REGISTER_BUS(nm, bus) \
-static void __attribute__((constructor(101), used)) businitfn_ ##nm(void) \
+RTE_INIT_PRIO(businitfn_ ##nm, 101); \
+static void businitfn_ ##nm(void) \
{\
(bus).name = RTE_STR(nm);\
rte_bus_register(&bus); \
@@ -286,6 +286,9 @@ static inline int rte_gettid(void)
#define RTE_INIT(func) \
static void __attribute__((constructor, used)) func(void)
+#define RTE_INIT_PRIO(func, prio) \
+static void __attribute__((constructor(prio), used)) func(void)
+
#ifdef __cplusplus
}
#endif
@@ -77,9 +77,6 @@ const char *pci_get_sysfs_path(void);
/** Maximum number of PCI resources. */
#define PCI_MAX_RESOURCE 6
-/** Name of PCI Bus */
-#define PCI_BUS_NAME "PCI"
-
/* Forward declarations */
struct rte_pci_device;
struct rte_pci_driver;
@@ -41,8 +41,6 @@ extern "C" {
#include <rte_dev.h>
#include <rte_devargs.h>
-#define VIRTUAL_BUS_NAME "virtual"
-
struct rte_vdev_device {
TAILQ_ENTRY(rte_vdev_device) next; /**< Next attached vdev */
struct rte_device device; /**< Inherit core device */