[PATCH v1 03/10] platform/x86: msi-wmi-platform: Add quirk system

Antheas Kapenekakis posted 10 patches 7 months, 1 week ago
[PATCH v1 03/10] platform/x86: msi-wmi-platform: Add quirk system
Posted by Antheas Kapenekakis 7 months, 1 week ago
MSI uses the WMI interface as a passthrough for writes to the EC
and uses a board name match and a quirk table from userspace on
Windows. Therefore, there is no auto-detection functionality and
we have to fallback to a quirk table.

Introduce it here, prior to starting to add features to it.

Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
---
 drivers/platform/x86/msi-wmi-platform.c | 45 +++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/drivers/platform/x86/msi-wmi-platform.c b/drivers/platform/x86/msi-wmi-platform.c
index f0d1b8e1a2fec..408d42ab19e20 100644
--- a/drivers/platform/x86/msi-wmi-platform.c
+++ b/drivers/platform/x86/msi-wmi-platform.c
@@ -14,6 +14,7 @@
 #include <linux/debugfs.h>
 #include <linux/device.h>
 #include <linux/device/driver.h>
+#include <linux/dmi.h>
 #include <linux/errno.h>
 #include <linux/hwmon.h>
 #include <linux/kernel.h>
@@ -79,8 +80,12 @@ enum msi_wmi_platform_method {
 	MSI_PLATFORM_GET_WMI		= 0x1d,
 };
 
+struct msi_wmi_platform_quirk {
+};
+
 struct msi_wmi_platform_data {
 	struct wmi_device *wdev;
+	struct msi_wmi_platform_quirk *quirks;
 	struct mutex wmi_lock;	/* Necessary when calling WMI methods */
 };
 
@@ -124,6 +129,39 @@ static const char * const msi_wmi_platform_debugfs_names[] = {
 	"get_wmi"
 };
 
+static struct msi_wmi_platform_quirk quirk_default = {};
+static struct msi_wmi_platform_quirk quirk_gen1 = {
+};
+static struct msi_wmi_platform_quirk quirk_gen2 = {
+};
+
+static const struct dmi_system_id msi_quirks[] = {
+	{
+		.ident = "MSI Claw (gen 1)",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Micro-Star International Co., Ltd."),
+			DMI_MATCH(DMI_BOARD_NAME, "MS-1T41"),
+		},
+		.driver_data = &quirk_gen1,
+	},
+	{
+		.ident = "MSI Claw AI+ 7",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Micro-Star International Co., Ltd."),
+			DMI_MATCH(DMI_BOARD_NAME, "MS-1T42"),
+		},
+		.driver_data = &quirk_gen2,
+	},
+	{
+		.ident = "MSI Claw AI+ 8",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Micro-Star International Co., Ltd."),
+			DMI_MATCH(DMI_BOARD_NAME, "MS-1T52"),
+		},
+		.driver_data = &quirk_gen2,
+	},
+};
+
 static int msi_wmi_platform_parse_buffer(union acpi_object *obj, u8 *output, size_t length)
 {
 	if (obj->type != ACPI_TYPE_BUFFER)
@@ -413,6 +451,7 @@ static int msi_wmi_platform_init(struct msi_wmi_platform_data *data)
 static int msi_wmi_platform_probe(struct wmi_device *wdev, const void *context)
 {
 	struct msi_wmi_platform_data *data;
+	const struct dmi_system_id *dmi_id;
 	int ret;
 
 	data = devm_kzalloc(&wdev->dev, sizeof(*data), GFP_KERNEL);
@@ -422,6 +461,12 @@ static int msi_wmi_platform_probe(struct wmi_device *wdev, const void *context)
 	data->wdev = wdev;
 	dev_set_drvdata(&wdev->dev, data);
 
+	dmi_id = dmi_first_match(msi_quirks);
+	if (dmi_id)
+		data->quirks = dmi_id->driver_data;
+	else
+		data->quirks = &quirk_default;
+
 	ret = devm_mutex_init(&wdev->dev, &data->wmi_lock);
 	if (ret < 0)
 		return ret;
-- 
2.49.0
Re: [PATCH v1 03/10] platform/x86: msi-wmi-platform: Add quirk system
Posted by Armin Wolf 7 months, 1 week ago
Am 11.05.25 um 22:44 schrieb Antheas Kapenekakis:

> MSI uses the WMI interface as a passthrough for writes to the EC
> and uses a board name match and a quirk table from userspace on
> Windows. Therefore, there is no auto-detection functionality and
> we have to fallback to a quirk table.
>
> Introduce it here, prior to starting to add features to it.
>
> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> ---
>   drivers/platform/x86/msi-wmi-platform.c | 45 +++++++++++++++++++++++++
>   1 file changed, 45 insertions(+)
>
> diff --git a/drivers/platform/x86/msi-wmi-platform.c b/drivers/platform/x86/msi-wmi-platform.c
> index f0d1b8e1a2fec..408d42ab19e20 100644
> --- a/drivers/platform/x86/msi-wmi-platform.c
> +++ b/drivers/platform/x86/msi-wmi-platform.c
> @@ -14,6 +14,7 @@
>   #include <linux/debugfs.h>
>   #include <linux/device.h>
>   #include <linux/device/driver.h>
> +#include <linux/dmi.h>
>   #include <linux/errno.h>
>   #include <linux/hwmon.h>
>   #include <linux/kernel.h>
> @@ -79,8 +80,12 @@ enum msi_wmi_platform_method {
>   	MSI_PLATFORM_GET_WMI		= 0x1d,
>   };
>   
> +struct msi_wmi_platform_quirk {
> +};
> +
>   struct msi_wmi_platform_data {
>   	struct wmi_device *wdev;
> +	struct msi_wmi_platform_quirk *quirks;
>   	struct mutex wmi_lock;	/* Necessary when calling WMI methods */
>   };
>   
> @@ -124,6 +129,39 @@ static const char * const msi_wmi_platform_debugfs_names[] = {
>   	"get_wmi"
>   };
>   
> +static struct msi_wmi_platform_quirk quirk_default = {};
> +static struct msi_wmi_platform_quirk quirk_gen1 = {
> +};
> +static struct msi_wmi_platform_quirk quirk_gen2 = {
> +};
> +
> +static const struct dmi_system_id msi_quirks[] = {
> +	{
> +		.ident = "MSI Claw (gen 1)",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "Micro-Star International Co., Ltd."),
> +			DMI_MATCH(DMI_BOARD_NAME, "MS-1T41"),
> +		},
> +		.driver_data = &quirk_gen1,
> +	},
> +	{
> +		.ident = "MSI Claw AI+ 7",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "Micro-Star International Co., Ltd."),
> +			DMI_MATCH(DMI_BOARD_NAME, "MS-1T42"),
> +		},
> +		.driver_data = &quirk_gen2,
> +	},
> +	{
> +		.ident = "MSI Claw AI+ 8",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "Micro-Star International Co., Ltd."),
> +			DMI_MATCH(DMI_BOARD_NAME, "MS-1T52"),
> +		},
> +		.driver_data = &quirk_gen2,
> +	},
> +};

I would prefer if we rely on the model code reported by the embedded controller instead of
the SMBIOS board name.

The model code is contained inside the firmware string returned by the Get_EC() WMI method,
see https://github.com/BeardOverflow/msi-ec/blob/main/docs/device_support_guide.md (section "Additional Info") for details.
This way support for devices sharing a model code can be implemented more efficiently.

I suggest we extract this model code inside msi_wmi_platform_ec_init() and perform the quirk matching there.

Thanks,
Armin Wolf

> +
>   static int msi_wmi_platform_parse_buffer(union acpi_object *obj, u8 *output, size_t length)
>   {
>   	if (obj->type != ACPI_TYPE_BUFFER)
> @@ -413,6 +451,7 @@ static int msi_wmi_platform_init(struct msi_wmi_platform_data *data)
>   static int msi_wmi_platform_probe(struct wmi_device *wdev, const void *context)
>   {
>   	struct msi_wmi_platform_data *data;
> +	const struct dmi_system_id *dmi_id;
>   	int ret;
>   
>   	data = devm_kzalloc(&wdev->dev, sizeof(*data), GFP_KERNEL);
> @@ -422,6 +461,12 @@ static int msi_wmi_platform_probe(struct wmi_device *wdev, const void *context)
>   	data->wdev = wdev;
>   	dev_set_drvdata(&wdev->dev, data);
>   
> +	dmi_id = dmi_first_match(msi_quirks);
> +	if (dmi_id)
> +		data->quirks = dmi_id->driver_data;
> +	else
> +		data->quirks = &quirk_default;
> +
>   	ret = devm_mutex_init(&wdev->dev, &data->wmi_lock);
>   	if (ret < 0)
>   		return ret;
Re: [PATCH v1 03/10] platform/x86: msi-wmi-platform: Add quirk system
Posted by Kurt Borja 7 months, 1 week ago
On Sun May 11, 2025 at 5:44 PM -03, Antheas Kapenekakis wrote:
> MSI uses the WMI interface as a passthrough for writes to the EC
> and uses a board name match and a quirk table from userspace on
> Windows. Therefore, there is no auto-detection functionality and
> we have to fallback to a quirk table.
>
> Introduce it here, prior to starting to add features to it.
>
> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> ---
>  drivers/platform/x86/msi-wmi-platform.c | 45 +++++++++++++++++++++++++
>  1 file changed, 45 insertions(+)
>
> diff --git a/drivers/platform/x86/msi-wmi-platform.c b/drivers/platform/x86/msi-wmi-platform.c
> index f0d1b8e1a2fec..408d42ab19e20 100644
> --- a/drivers/platform/x86/msi-wmi-platform.c
> +++ b/drivers/platform/x86/msi-wmi-platform.c
> @@ -14,6 +14,7 @@
>  #include <linux/debugfs.h>
>  #include <linux/device.h>
>  #include <linux/device/driver.h>
> +#include <linux/dmi.h>
>  #include <linux/errno.h>
>  #include <linux/hwmon.h>
>  #include <linux/kernel.h>
> @@ -79,8 +80,12 @@ enum msi_wmi_platform_method {
>  	MSI_PLATFORM_GET_WMI		= 0x1d,
>  };
>  
> +struct msi_wmi_platform_quirk {
> +};
> +
>  struct msi_wmi_platform_data {
>  	struct wmi_device *wdev;
> +	struct msi_wmi_platform_quirk *quirks;
>  	struct mutex wmi_lock;	/* Necessary when calling WMI methods */
>  };
>  
> @@ -124,6 +129,39 @@ static const char * const msi_wmi_platform_debugfs_names[] = {
>  	"get_wmi"
>  };
>  
> +static struct msi_wmi_platform_quirk quirk_default = {};

This is static, you can drop the `= {}`.

> +static struct msi_wmi_platform_quirk quirk_gen1 = {
> +};
> +static struct msi_wmi_platform_quirk quirk_gen2 = {
> +};
> +
> +static const struct dmi_system_id msi_quirks[] = {

This should be moved to an .init section, i.e. marked as __initconst.

To achieve this, you have to move DMI matching to module_init and
*quirks most be static.

This way this memory can be freed after init.

-- 
 ~ Kurt

> +	{
> +		.ident = "MSI Claw (gen 1)",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "Micro-Star International Co., Ltd."),
> +			DMI_MATCH(DMI_BOARD_NAME, "MS-1T41"),
> +		},
> +		.driver_data = &quirk_gen1,
> +	},
> +	{
> +		.ident = "MSI Claw AI+ 7",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "Micro-Star International Co., Ltd."),
> +			DMI_MATCH(DMI_BOARD_NAME, "MS-1T42"),
> +		},
> +		.driver_data = &quirk_gen2,
> +	},
> +	{
> +		.ident = "MSI Claw AI+ 8",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "Micro-Star International Co., Ltd."),
> +			DMI_MATCH(DMI_BOARD_NAME, "MS-1T52"),
> +		},
> +		.driver_data = &quirk_gen2,
> +	},
> +};
> +
>  static int msi_wmi_platform_parse_buffer(union acpi_object *obj, u8 *output, size_t length)
>  {
>  	if (obj->type != ACPI_TYPE_BUFFER)
> @@ -413,6 +451,7 @@ static int msi_wmi_platform_init(struct msi_wmi_platform_data *data)
>  static int msi_wmi_platform_probe(struct wmi_device *wdev, const void *context)
>  {
>  	struct msi_wmi_platform_data *data;
> +	const struct dmi_system_id *dmi_id;
>  	int ret;
>  
>  	data = devm_kzalloc(&wdev->dev, sizeof(*data), GFP_KERNEL);
> @@ -422,6 +461,12 @@ static int msi_wmi_platform_probe(struct wmi_device *wdev, const void *context)
>  	data->wdev = wdev;
>  	dev_set_drvdata(&wdev->dev, data);
>  
> +	dmi_id = dmi_first_match(msi_quirks);
> +	if (dmi_id)
> +		data->quirks = dmi_id->driver_data;
> +	else
> +		data->quirks = &quirk_default;
> +
>  	ret = devm_mutex_init(&wdev->dev, &data->wmi_lock);
>  	if (ret < 0)
>  		return ret;