The kernel provides the macro MODULE_DEVICE_TABLE() which can help
driver authors to generate the appropriate MODULE_ALIAS() output. The
WMI device type is currently not supported by MODULE_DEVICE_TABLE().
While using MODULE_DEVICE_TABLE() does increase the complexity as well
as spreading out the implementation across the kernel, it does come with
some benefits too;
* It makes different drivers look more similar; if you can specify the
array of device_ids any device type specific input to MODULE_ALIAS()
will automatically be generated for you.
* It helps each driver avoid keeping multiple versions of the same
information in sync. That is, both the array of device_ids and the
potential multitude of MODULE_ALIAS()'s.
* Other things eg. [2]
This patchset adds WMI support to MODULE_DEVICE_TABLE().
[PATCH 1/3]: prepare struct wmi_device_id
[PATCH 2/3]: add support
[PATCH 3+/3]: update existing drivers to use MODULE_DEVICE_TABLE()
Changes in v4:
* formatting(declare/assignment, line-breaks) according to comments
* split "PATCH 3/3" into one patch per driver according to comments
* depend upon patch [3] (macro ALIAS_SIZE)
Changes in v3:
* use UUID_STRING_LEN instead of self-defined
* change loop condition in wmi_dev_match() according to comments
* change the usage of snprintf return code in do_wmi_entry()
Changes in v2:
* add one Suggested-by and one Reviewed-by tag
* depend upon patch [1]
* reword commit message for [PATCH 2/3] and [PATCH 3/3] to document the
reasoning behind the changes
[1]: https://lkml.kernel.org/r/[email protected]
[2]: https://lkml.kernel.org/r/20190126210634.GB13882@wrath
[3]: https://lore.kernel.org/lkml/[email protected]/
Mattias Jacobsson (8):
platform/x86: wmi: move struct wmi_device_id to mod_devicetable.h
platform/x86: wmi: add WMI support to MODULE_DEVICE_TABLE()
platform/x86: dell-smbios-wmi: use MODULE_DEVICE_TABLE() instead of
MODULE_ALIAS()
platform/x86: dell-wmi-descriptor: use MODULE_DEVICE_TABLE() instead
of MODULE_ALIAS()
platform/x86: dell-wmi: use MODULE_DEVICE_TABLE() instead of
MODULE_ALIAS()
platform/x86: huawei-wmi: use MODULE_DEVICE_TABLE() instead of
MODULE_ALIAS()
platform/x86: intel-wmi-thunderbolt: use MODULE_DEVICE_TABLE() instead
of MODULE_ALIAS()
platform/x86: wmi-bmof: use MODULE_DEVICE_TABLE() instead of
MODULE_ALIAS()
drivers/platform/x86/dell-smbios-wmi.c | 2 +-
drivers/platform/x86/dell-wmi-descriptor.c | 2 +-
drivers/platform/x86/dell-wmi.c | 4 ++--
drivers/platform/x86/huawei-wmi.c | 3 +--
drivers/platform/x86/intel-wmi-thunderbolt.c | 2 +-
drivers/platform/x86/wmi-bmof.c | 2 +-
drivers/platform/x86/wmi.c | 2 +-
include/linux/mod_devicetable.h | 12 ++++++++++
include/linux/wmi.h | 5 +----
scripts/mod/devicetable-offsets.c | 3 +++
scripts/mod/file2alias.c | 23 ++++++++++++++++++++
11 files changed, 47 insertions(+), 13 deletions(-)
--
2.20.1
In preparation for adding WMI support to MODULE_DEVICE_TABLE() move the
definition of struct wmi_device_id to mod_devicetable.h and inline
guid_string in the struct.
Changing guid_string to an inline char array changes the loop conditions
when looping over an array of struct wmi_device_id. Therefore update
wmi_dev_match()'s loop to check for an empty guid_string instead of a
NULL pointer.
Signed-off-by: Mattias Jacobsson <[email protected]>
---
drivers/platform/x86/wmi.c | 2 +-
include/linux/mod_devicetable.h | 12 ++++++++++++
include/linux/wmi.h | 5 +----
3 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
index b0f3d8ecd898..7b26b6ccf1a0 100644
--- a/drivers/platform/x86/wmi.c
+++ b/drivers/platform/x86/wmi.c
@@ -771,7 +771,7 @@ static int wmi_dev_match(struct device *dev, struct device_driver *driver)
if (id == NULL)
return 0;
- while (id->guid_string) {
+ while (*id->guid_string) {
uuid_le driver_guid;
if (WARN_ON(uuid_le_to_bin(id->guid_string, &driver_guid)))
diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index f9bd2f34b99f..e44b90fa0aef 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -779,4 +779,16 @@ struct typec_device_id {
kernel_ulong_t driver_data;
};
+/* WMI */
+
+#define WMI_MODULE_PREFIX "wmi:"
+
+/**
+ * struct wmi_device_id - WMI device identifier
+ * @guid_string: 36 char string of the form fa50ff2b-f2e8-45de-83fa-65417f2f49ba
+ */
+struct wmi_device_id {
+ const char guid_string[UUID_STRING_LEN+1];
+};
+
#endif /* LINUX_MOD_DEVICETABLE_H */
diff --git a/include/linux/wmi.h b/include/linux/wmi.h
index 4757cb5077e5..592f81afecbb 100644
--- a/include/linux/wmi.h
+++ b/include/linux/wmi.h
@@ -18,6 +18,7 @@
#include <linux/device.h>
#include <linux/acpi.h>
+#include <linux/mod_devicetable.h>
#include <uapi/linux/wmi.h>
struct wmi_device {
@@ -39,10 +40,6 @@ extern union acpi_object *wmidev_block_query(struct wmi_device *wdev,
extern int set_required_buffer_size(struct wmi_device *wdev, u64 length);
-struct wmi_device_id {
- const char *guid_string;
-};
-
struct wmi_driver {
struct device_driver driver;
const struct wmi_device_id *id_table;
--
2.20.1
The kernel provides the macro MODULE_DEVICE_TABLE() where driver authors
can specify their device type and their array of device_ids and thereby
trigger the generation of the appropriate MODULE_ALIAS() output. This is
opposed to having to specify one MODULE_ALIAS() for each device. The WMI
device type is currently not supported.
While using MODULE_DEVICE_TABLE() does increase the complexity as well
as spreading out the implementation across the kernel, it does come with
some benefits too;
* It makes different drivers look more similar; if you can specify the
array of device_ids any device type specific input to MODULE_ALIAS()
will automatically be generated for you.
* It helps each driver avoid keeping multiple versions of the same
information in sync. That is, both the array of device_ids and the
potential multitude of MODULE_ALIAS()'s.
Add WMI support to MODULE_DEVICE_TABLE() by adding info about struct
wmi_device_id in devicetable-offsets.c and add a WMI entry point in
file2alias.c.
The type argument for MODULE_DEVICE_TABLE(type, name) is wmi.
Suggested-by: Pali Rohár <[email protected]>
Signed-off-by: Mattias Jacobsson <[email protected]>
---
What do you think about writing it this way now, pretty much the same as
before, but now using the macro ALIAS_SIZE instead of a hardcoded value?
Also, as I mentioned before, I've submitted the ALIAS_SIZE patch and
have recieved a question on which tree the patch should be go through.
See the original question on [1]. I don't mind either way, do you have a
preference on this?
[1]: https://lore.kernel.org/lkml/CAK7LNAQZ7PDZ4+uZu-FT4V8yw9oUGz6e++9xSshWJvALj3gN0Q@mail.gmail.com
--
scripts/mod/devicetable-offsets.c | 3 +++
scripts/mod/file2alias.c | 23 +++++++++++++++++++++++
2 files changed, 26 insertions(+)
diff --git a/scripts/mod/devicetable-offsets.c b/scripts/mod/devicetable-offsets.c
index 293004499b4d..99276a422e77 100644
--- a/scripts/mod/devicetable-offsets.c
+++ b/scripts/mod/devicetable-offsets.c
@@ -225,5 +225,8 @@ int main(void)
DEVID_FIELD(typec_device_id, svid);
DEVID_FIELD(typec_device_id, mode);
+ DEVID(wmi_device_id);
+ DEVID_FIELD(wmi_device_id, guid_string);
+
return 0;
}
diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
index afe22af20d7d..6dedc31a4925 100644
--- a/scripts/mod/file2alias.c
+++ b/scripts/mod/file2alias.c
@@ -37,6 +37,7 @@ typedef unsigned char __u8;
typedef struct {
__u8 b[16];
} uuid_le;
+#define UUID_STRING_LEN 36
/* Big exception to the "don't include kernel headers into userspace, which
* even potentially has different endianness and word sizes, since
@@ -1290,6 +1291,27 @@ static int do_typec_entry(const char *filename, void *symval, char *alias)
return 1;
}
+/* Looks like: wmi:guid */
+static int do_wmi_entry(const char *filename, void *symval, char *alias)
+{
+ int len;
+ DEF_FIELD_ADDR(symval, wmi_device_id, guid_string);
+
+ if (strlen(*guid_string) != UUID_STRING_LEN) {
+ warn("Invalid WMI device id 'wmi:%s' in '%s'\n",
+ *guid_string, filename);
+ return 0;
+ }
+
+ len = snprintf(alias, ALIAS_SIZE, WMI_MODULE_PREFIX "%s", *guid_string);
+ if (len < 0 || len >= ALIAS_SIZE) {
+ warn("Could not generate all MODULE_ALIAS's in '%s'\n",
+ filename);
+ return 0;
+ }
+ return 1;
+}
+
/* Does namelen bytes of name exactly match the symbol? */
static bool sym_is(const char *name, unsigned namelen, const char *symbol)
{
@@ -1360,6 +1382,7 @@ static const struct devtable devtable[] = {
{"fslmc", SIZE_fsl_mc_device_id, do_fsl_mc_entry},
{"tbsvc", SIZE_tb_service_id, do_tbsvc_entry},
{"typec", SIZE_typec_device_id, do_typec_entry},
+ {"wmi", SIZE_wmi_device_id, do_wmi_entry},
};
/* Create MODULE_ALIAS() statements.
--
2.20.1
WMI drivers can if they have specified an array of struct wmi_device_id
use the MODULE_DEVICE_TABLE() macro to automatically generate the
appropriate MODULE_ALIAS() output. Thus avoiding to keep both the array
of struct wmi_device_id and the MODULE_ALIAS() declaration(s) in sync.
Change driver to use MODULE_DEVICE_TABLE() instead of MODULE_ALIAS().
Signed-off-by: Mattias Jacobsson <[email protected]>
---
drivers/platform/x86/huawei-wmi.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/platform/x86/huawei-wmi.c b/drivers/platform/x86/huawei-wmi.c
index 59872f87b741..52fcac5b393a 100644
--- a/drivers/platform/x86/huawei-wmi.c
+++ b/drivers/platform/x86/huawei-wmi.c
@@ -201,8 +201,7 @@ static struct wmi_driver huawei_wmi_driver = {
module_wmi_driver(huawei_wmi_driver);
-MODULE_ALIAS("wmi:"WMI0_EVENT_GUID);
-MODULE_ALIAS("wmi:"AMW0_EVENT_GUID);
+MODULE_DEVICE_TABLE(wmi, huawei_wmi_id_table);
MODULE_AUTHOR("Ayman Bagabas <[email protected]>");
MODULE_DESCRIPTION("Huawei WMI hotkeys");
MODULE_LICENSE("GPL v2");
--
2.20.1
WMI drivers can if they have specified an array of struct wmi_device_id
use the MODULE_DEVICE_TABLE() macro to automatically generate the
appropriate MODULE_ALIAS() output. Thus avoiding to keep both the array
of struct wmi_device_id and the MODULE_ALIAS() declaration(s) in sync.
Change driver to use MODULE_DEVICE_TABLE() instead of MODULE_ALIAS().
Signed-off-by: Mattias Jacobsson <[email protected]>
---
drivers/platform/x86/wmi-bmof.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/platform/x86/wmi-bmof.c b/drivers/platform/x86/wmi-bmof.c
index c4530ba715e8..8751a13134be 100644
--- a/drivers/platform/x86/wmi-bmof.c
+++ b/drivers/platform/x86/wmi-bmof.c
@@ -119,7 +119,7 @@ static struct wmi_driver wmi_bmof_driver = {
module_wmi_driver(wmi_bmof_driver);
-MODULE_ALIAS("wmi:" WMI_BMOF_GUID);
+MODULE_DEVICE_TABLE(wmi, wmi_bmof_id_table);
MODULE_AUTHOR("Andrew Lutomirski <[email protected]>");
MODULE_DESCRIPTION("WMI embedded Binary MOF driver");
MODULE_LICENSE("GPL");
--
2.20.1
WMI drivers can if they have specified an array of struct wmi_device_id
use the MODULE_DEVICE_TABLE() macro to automatically generate the
appropriate MODULE_ALIAS() output. Thus avoiding to keep both the array
of struct wmi_device_id and the MODULE_ALIAS() declaration(s) in sync.
Change driver to use MODULE_DEVICE_TABLE() instead of MODULE_ALIAS().
Signed-off-by: Mattias Jacobsson <[email protected]>
---
drivers/platform/x86/intel-wmi-thunderbolt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/platform/x86/intel-wmi-thunderbolt.c b/drivers/platform/x86/intel-wmi-thunderbolt.c
index 9ded8e2af312..4dfa61434a76 100644
--- a/drivers/platform/x86/intel-wmi-thunderbolt.c
+++ b/drivers/platform/x86/intel-wmi-thunderbolt.c
@@ -88,7 +88,7 @@ static struct wmi_driver intel_wmi_thunderbolt_driver = {
module_wmi_driver(intel_wmi_thunderbolt_driver);
-MODULE_ALIAS("wmi:" INTEL_WMI_THUNDERBOLT_GUID);
+MODULE_DEVICE_TABLE(wmi, intel_wmi_thunderbolt_id_table);
MODULE_AUTHOR("Mario Limonciello <[email protected]>");
MODULE_DESCRIPTION("Intel WMI Thunderbolt force power driver");
MODULE_LICENSE("GPL v2");
--
2.20.1
WMI drivers can if they have specified an array of struct wmi_device_id
use the MODULE_DEVICE_TABLE() macro to automatically generate the
appropriate MODULE_ALIAS() output. Thus avoiding to keep both the array
of struct wmi_device_id and the MODULE_ALIAS() declaration(s) in sync.
Change driver to use MODULE_DEVICE_TABLE() instead of MODULE_ALIAS().
Signed-off-by: Mattias Jacobsson <[email protected]>
Reviewed-by: Mario Limonciello <[email protected]>
---
drivers/platform/x86/dell-wmi.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c
index 16c7f3d9a335..0602aba62b3f 100644
--- a/drivers/platform/x86/dell-wmi.c
+++ b/drivers/platform/x86/dell-wmi.c
@@ -50,8 +50,6 @@ MODULE_LICENSE("GPL");
static bool wmi_requires_smbios_request;
-MODULE_ALIAS("wmi:"DELL_EVENT_GUID);
-
struct dell_wmi_priv {
struct input_dev *input_dev;
u32 interface_version;
@@ -738,3 +736,5 @@ static void __exit dell_wmi_exit(void)
wmi_driver_unregister(&dell_wmi_driver);
}
module_exit(dell_wmi_exit);
+
+MODULE_DEVICE_TABLE(wmi, dell_wmi_id_table);
--
2.20.1
WMI drivers can if they have specified an array of struct wmi_device_id
use the MODULE_DEVICE_TABLE() macro to automatically generate the
appropriate MODULE_ALIAS() output. Thus avoiding to keep both the array
of struct wmi_device_id and the MODULE_ALIAS() declaration(s) in sync.
Change driver to use MODULE_DEVICE_TABLE() instead of MODULE_ALIAS().
Signed-off-by: Mattias Jacobsson <[email protected]>
Reviewed-by: Mario Limonciello <[email protected]>
---
drivers/platform/x86/dell-wmi-descriptor.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/platform/x86/dell-wmi-descriptor.c b/drivers/platform/x86/dell-wmi-descriptor.c
index 072821aa47fc..14ab250b7d5a 100644
--- a/drivers/platform/x86/dell-wmi-descriptor.c
+++ b/drivers/platform/x86/dell-wmi-descriptor.c
@@ -207,7 +207,7 @@ static struct wmi_driver dell_wmi_descriptor_driver = {
module_wmi_driver(dell_wmi_descriptor_driver);
-MODULE_ALIAS("wmi:" DELL_WMI_DESCRIPTOR_GUID);
+MODULE_DEVICE_TABLE(wmi, dell_wmi_descriptor_id_table);
MODULE_AUTHOR("Mario Limonciello <[email protected]>");
MODULE_DESCRIPTION("Dell WMI descriptor driver");
MODULE_LICENSE("GPL");
--
2.20.1
WMI drivers can if they have specified an array of struct wmi_device_id
use the MODULE_DEVICE_TABLE() macro to automatically generate the
appropriate MODULE_ALIAS() output. Thus avoiding to keep both the array
of struct wmi_device_id and the MODULE_ALIAS() declaration(s) in sync.
Change driver to use MODULE_DEVICE_TABLE() instead of MODULE_ALIAS().
Signed-off-by: Mattias Jacobsson <[email protected]>
Reviewed-by: Mario Limonciello <[email protected]>
---
drivers/platform/x86/dell-smbios-wmi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/platform/x86/dell-smbios-wmi.c b/drivers/platform/x86/dell-smbios-wmi.c
index cf2229ece9ff..c3ed3c8c17b9 100644
--- a/drivers/platform/x86/dell-smbios-wmi.c
+++ b/drivers/platform/x86/dell-smbios-wmi.c
@@ -277,4 +277,4 @@ void exit_dell_smbios_wmi(void)
wmi_driver_unregister(&dell_smbios_wmi_driver);
}
-MODULE_ALIAS("wmi:" DELL_WMI_SMBIOS_GUID);
+MODULE_DEVICE_TABLE(wmi, dell_smbios_wmi_id_table);
--
2.20.1
On Tue, Feb 19, 2019 at 08:59:48PM +0100, Mattias Jacobsson wrote:
> The kernel provides the macro MODULE_DEVICE_TABLE() which can help
> driver authors to generate the appropriate MODULE_ALIAS() output. The
> WMI device type is currently not supported by MODULE_DEVICE_TABLE().
Thanks Mattias. I have this queued up for testing, and with an ack from
Masahiro for 2/8, I'll pull this in. Thanks for the patches.
--
Darren Hart
VMware Open Source Technology Center