Re: [PATCH rdma v8 1/7] RDMA/zrdma: Add build infrastructure and module skeleton

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] RDMA/zrdma: Kconfig depends on bool DINGHAI instead of tristate DINGHAI_PF
- [Low] RDMA/zrdma: open-coded zrdma_module_init() instead of module_auxiliary_driver()
--

commit 255288601ed5fbab9c26b7e4b182fd67e8a9395c
Author: Yanze Zhang <zhang.yanze@zte.com.cn>

RDMA/zrdma: Add build infrastructure and module skeleton

This commit adds the MAINTAINERS entry, Kconfig, Makefile, and a minimal
auxiliary driver module skeleton for the ZTE DingHai RDMA (zrdma) driver.

> diff --git a/drivers/infiniband/hw/zrdma/Kconfig b/drivers/infiniband/hw/zrdma/Kconfig
> new file mode 100644
> index 0000000000000..43cb55ae9c93b
> --- /dev/null
> +++ b/drivers/infiniband/hw/zrdma/Kconfig
> @@ -0,0 +1,11 @@
> +# SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB
> +config INFINIBAND_ZRDMA
> +	tristate "ZTE Ethernet Protocol Driver for RDMA"
> +	depends on DINGHAI
> +	select AUXILIARY_BUS

[Severity: Medium]
As reported by Zhu Yanjun on v3 [1], should INFINIBAND_ZRDMA depend on
DINGHAI_PF instead of DINGHAI?

In drivers/net/ethernet/zte/dinghai/Kconfig, DINGHAI is only a boolean vendor
menu guard, whereas the DingHai PF driver (dinghai10e.o) that creates the
dinghai10e.rdma_aux auxiliary device is controlled by the tristate symbol
DINGHAI_PF:

drivers/net/ethernet/zte/dinghai/Kconfig:
    config DINGHAI_PF
        tristate "ZTE DingHai PF (Physical Function) driver"

drivers/net/ethernet/zte/dinghai/Makefile:
    obj-$(CONFIG_DINGHAI_PF) += dinghai10e.o

Depending only on the bool DINGHAI symbol allows configuring
CONFIG_INFINIBAND_ZRDMA=y when CONFIG_DINGHAI_PF=n or CONFIG_DINGHAI_PF=m.

> diff --git a/drivers/infiniband/hw/zrdma/zrdma_main.c b/drivers/infiniband/hw/zrdma/zrdma_main.c
> new file mode 100644
> index 0000000000000..af1d37be63362
> --- /dev/null
> +++ b/drivers/infiniband/hw/zrdma/zrdma_main.c
> @@ -0,0 +1,57 @@

[ ... ]

> +static struct auxiliary_driver zxdh_auxiliary_drv = {
> +	.name = ZXDH_RDMA_DEV_NAME,
> +	.id_table = zxdh_auxiliary_id_table,
> +	.probe = zxdh_probe,
> +	.remove = zxdh_remove,
> +};
> +
> +static int __init zrdma_module_init(void)
> +{
> +	return auxiliary_driver_register(&zxdh_auxiliary_drv);
> +}
> +
> +static void __exit zrdma_module_exit(void)
> +{
> +	auxiliary_driver_unregister(&zxdh_auxiliary_drv);
> +}
> +
> +module_init(zrdma_module_init);
> +module_exit(zrdma_module_exit);

[Severity: Low]
This isn't a bug, but since zrdma_module_init() and zrdma_module_exit() only
register and unregister zxdh_auxiliary_drv, could this use
module_auxiliary_driver(zxdh_auxiliary_drv) instead of open-coding the module
init and exit functions?

[1] https://lore.kernel.org/r/21b7b324-9fd2-402d-8695-5dd061adb957@linux.dev

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261102082026.2645777-1-zhang.yanze@zte.com.cn?part=1