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

[Armin Wolf <W_Armin@gmx.de>]

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;