On 2/7/25 01:13, Jason Gunthorpe wrote:
From: Saeed Mahameed <saeedm@xxxxxxxxxx>
In part this is a general feedback for the subsystem too.
+FWCTL MLX5 DRIVER
I don't like this design.
That way each and every real driver would need to make another one to
just use fwctl.
Why not just require the real driver to call fwctl_register(opsstruct),
with the required .validate, .do_cmd, etc commands backed there?
There will be much less scaffolding.
Or the intention is to have this little driver replaced by OOT one,
but keep the real (say networking) driver as-is from intree?
+++ b/drivers/fwctl/mlx5/main.c
@@ -0,0 +1,340 @@
+// SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0
+/*
+ * Copyright (c) 2024, NVIDIA CORPORATION & AFFILIATES
-2025
+ */
+#include <linux/fwctl.h>
+#include <linux/auxiliary_bus.h>
+#include <linux/mlx5/device.h>
+#include <linux/mlx5/driver.h>
this breaks abstraction (at least your headers are in nice place, but
this is rather uncommon, typical solution is to have them backed inside
the driver directory) - the two drivers will be tightly coupled
+module_auxiliary_driver(mlx5ctl_driver);
+
+MODULE_IMPORT_NS("FWCTL");
+MODULE_DESCRIPTION("mlx5 ConnectX fwctl driver");
+MODULE_AUTHOR("Saeed Mahameed <saeedm@xxxxxxxxxx>");
+MODULE_LICENSE("Dual BSD/GPL");
diff --git a/include/uapi/fwctl/fwctl.h b/include/uapi/fwctl/fwctl.h
index 7a21f2f011917a..0790b8291ee1bd 100644
--- a/include/uapi/fwctl/fwctl.h
+++ b/include/uapi/fwctl/fwctl.h
@@ -42,6 +42,7 @@ enum {
enum fwctl_device_type {
FWCTL_DEVICE_TYPE_ERROR = 0,
+ FWCTL_DEVICE_TYPE_MLX5 = 1,
is that for fwctl info to be able to properly report what device user
has asked ioctl on? Would be great to embed 32byte long cstring of
DRIVER_NAME, to don't need each and every device to come to you and
ask for inclusion, that would also resolve problem of conflicting IDs
(my-driver-id prior-to and after upstreaming)
