* [PATCH v3] platform/x86: add Gigabyte WMI temperature driver [not found] <117cadef-c1cb-d66a-15f8-ce50d596be4b@redhat.com> @ 2021-04-10 14:40 ` Thomas Weißschuh 2021-04-10 14:57 ` Hans de Goede 2021-04-10 15:15 ` Guenter Roeck 0 siblings, 2 replies; 10+ messages in thread From: Thomas Weißschuh @ 2021-04-10 14:40 UTC (permalink / raw) To: platform-driver-x86, Mark Gross, Hans de Goede, linux-kernel, Barnabás Pőcze, Jean Delvare, Guenter Roeck, linux-hwmon Cc: Thomas Weißschuh, Matthew Garrett Changes since v1: * Incorporate feedback from Barnabás Pőcze * Use a WMI driver instead of a platform driver * Let the kernel manage the driver lifecycle * Fix errno/ACPI error confusion * Fix resource cleanup * Document reason for integer casting Changes since v2: * Style cleanups * Test for usability during probing * DMI-based whitelist * CC hwmon maintainers -- >8 -- Tested with a X570 I Aorus Pro Wifi. The mainboard contains an ITE IT8688E chip for management. This chips is also handled by drivers/hwmon/i87.c but as it is also used by the firmware itself it needs an ACPI driver. Unfortunately not all sensor registers are handled by the firmware and even less are exposed via WMI. Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> --- MAINTAINERS | 6 + drivers/platform/x86/Kconfig | 11 ++ drivers/platform/x86/Makefile | 1 + drivers/platform/x86/gigabyte-wmi.c | 194 ++++++++++++++++++++++++++++ 4 files changed, 212 insertions(+) create mode 100644 drivers/platform/x86/gigabyte-wmi.c diff --git a/MAINTAINERS b/MAINTAINERS index d92f85ca831d..9c10cfc00fe8 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -7543,6 +7543,12 @@ F: Documentation/filesystems/gfs2* F: fs/gfs2/ F: include/uapi/linux/gfs2_ondisk.h +GIGABYTE WMI DRIVER +M: Thomas Weißschuh <linux@weissschuh.net> +L: platform-driver-x86@vger.kernel.org +S: Maintained +F: drivers/platform/x86/gigabyte-wmi.c + GNSS SUBSYSTEM M: Johan Hovold <johan@kernel.org> S: Maintained diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig index ad4e630e73e2..96622a2106f7 100644 --- a/drivers/platform/x86/Kconfig +++ b/drivers/platform/x86/Kconfig @@ -123,6 +123,17 @@ config XIAOMI_WMI To compile this driver as a module, choose M here: the module will be called xiaomi-wmi. +config GIGABYTE_WMI + tristate "Gigabyte WMI temperature driver" + depends on ACPI_WMI + depends on HWMON + help + Say Y here if you want to support WMI-based temperature reporting on + Gigabyte mainboards. + + To compile this driver as a module, choose M here: the module will + be called gigabyte-wmi. + config ACERHDF tristate "Acer Aspire One temperature and fan driver" depends on ACPI && THERMAL diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile index 60d554073749..1621ebfd04fd 100644 --- a/drivers/platform/x86/Makefile +++ b/drivers/platform/x86/Makefile @@ -15,6 +15,7 @@ obj-$(CONFIG_INTEL_WMI_THUNDERBOLT) += intel-wmi-thunderbolt.o obj-$(CONFIG_MXM_WMI) += mxm-wmi.o obj-$(CONFIG_PEAQ_WMI) += peaq-wmi.o obj-$(CONFIG_XIAOMI_WMI) += xiaomi-wmi.o +obj-$(CONFIG_GIGABYTE_WMI) += gigabyte-wmi.o # Acer obj-$(CONFIG_ACERHDF) += acerhdf.o diff --git a/drivers/platform/x86/gigabyte-wmi.c b/drivers/platform/x86/gigabyte-wmi.c new file mode 100644 index 000000000000..fb4e6d4c1823 --- /dev/null +++ b/drivers/platform/x86/gigabyte-wmi.c @@ -0,0 +1,194 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (C) 2021 Thomas Weißschuh <thomas@weissschuh.net> + */ +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + +#include <linux/acpi.h> +#include <linux/dmi.h> +#include <linux/hwmon.h> +#include <linux/module.h> +#include <linux/wmi.h> + +#define GIGABYTE_WMI_GUID "DEADBEEF-2001-0000-00A0-C90629100000" +#define NUM_TEMPERATURE_SENSORS 6 + +static bool force_load; +module_param(force_load, bool, 0); +MODULE_PARM_DESC(force_load, "Force loading on non-whitelisted platform"); + +enum gigabyte_wmi_commandtype { + GIGABYTE_WMI_BUILD_DATE_QUERY = 0x1, + GIGABYTE_WMI_MAINBOARD_TYPE_QUERY = 0x2, + GIGABYTE_WMI_FIRMWARE_VERSION_QUERY = 0x4, + GIGABYTE_WMI_MAINBOARD_NAME_QUERY = 0x5, + GIGABYTE_WMI_TEMPERATURE_QUERY = 0x125, +}; + +struct gigabyte_wmi_args { + u32 arg1; +}; + +static int gigabyte_wmi_perform_query(struct wmi_device *wdev, + enum gigabyte_wmi_commandtype command, + struct gigabyte_wmi_args *args, struct acpi_buffer *out) +{ + const struct acpi_buffer in = { + .length = sizeof(*args), + .pointer = args, + }; + + acpi_status ret = wmidev_evaluate_method(wdev, 0x0, command, &in, out); + + if ACPI_FAILURE(ret) + return -EIO; + + return 0; +} + +static int gigabyte_wmi_query_integer(struct wmi_device *wdev, + enum gigabyte_wmi_commandtype command, + struct gigabyte_wmi_args *args, u64 *res) +{ + union acpi_object *obj; + struct acpi_buffer result = { ACPI_ALLOCATE_BUFFER, NULL }; + int ret; + + ret = gigabyte_wmi_perform_query(wdev, command, args, &result); + if (ret) + return ret; + obj = result.pointer; + if (obj && obj->type == ACPI_TYPE_INTEGER) + *res = obj->integer.value; + else + ret = -EIO; + kfree(result.pointer); + return ret; +} + +static int gigabyte_wmi_temperature(struct wmi_device *wdev, u8 sensor, long *res) +{ + struct gigabyte_wmi_args args = { + .arg1 = sensor, + }; + u64 temp; + acpi_status ret; + + ret = gigabyte_wmi_query_integer(wdev, GIGABYTE_WMI_TEMPERATURE_QUERY, &args, &temp); + if (ret == 0) { + if (temp == 0) + return -ENODEV; + *res = (s8)temp * 1000; // value is a signed 8-bit integer + } + return ret; +} + +static int gigabyte_wmi_hwmon_read(struct device *dev, enum hwmon_sensor_types type, + u32 attr, int channel, long *val) +{ + struct wmi_device *wdev = dev_get_drvdata(dev); + + return gigabyte_wmi_temperature(wdev, channel, val); +} + +static umode_t gigabyte_wmi_hwmon_is_visible(const void *data, enum hwmon_sensor_types type, + u32 attr, int channel) +{ + return 0444; +} + +static const struct hwmon_channel_info *gigabyte_wmi_hwmon_info[] = { + HWMON_CHANNEL_INFO(temp, + HWMON_T_INPUT, + HWMON_T_INPUT, + HWMON_T_INPUT, + HWMON_T_INPUT, + HWMON_T_INPUT, + HWMON_T_INPUT), + NULL +}; + +static const struct hwmon_ops gigabyte_wmi_hwmon_ops = { + .read = gigabyte_wmi_hwmon_read, + .is_visible = gigabyte_wmi_hwmon_is_visible, +}; + +static const struct hwmon_chip_info gigabyte_wmi_hwmon_chip_info = { + .ops = &gigabyte_wmi_hwmon_ops, + .info = gigabyte_wmi_hwmon_info, +}; + +static int gigabyte_wmi_validate_sensor_presence(struct wmi_device *wdev) +{ + int working_sensors = 0, i; + long temp; + + for (i = 0; i < NUM_TEMPERATURE_SENSORS; i++) { + if (!gigabyte_wmi_temperature(wdev, i, &temp)) + working_sensors++; + } + return working_sensors ? 0 : -ENODEV; +} + +static const struct dmi_system_id gigabyte_wmi_known_working_platforms[] = { + { .matches = { + DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "Gigabyte Technology Co., Ltd."), + DMI_EXACT_MATCH(DMI_BOARD_NAME, "B550 GAMING X V2"), + }}, + { .matches = { + DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "Gigabyte Technology Co., Ltd."), + DMI_EXACT_MATCH(DMI_BOARD_NAME, "B550M DS3H"), + }}, + { .matches = { + DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "Gigabyte Technology Co., Ltd."), + DMI_EXACT_MATCH(DMI_BOARD_NAME, "Z390 I AORUS PRO WIFI-CF"), + }}, + { .matches = { + DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "Gigabyte Technology Co., Ltd."), + DMI_EXACT_MATCH(DMI_BOARD_NAME, "X570 I AORUS PRO WIFI"), + }}, + { } +}; + +static int gigabyte_wmi_probe(struct wmi_device *wdev, const void *context) +{ + struct device *hwmon_dev; + int ret; + + if (!dmi_check_system(gigabyte_wmi_known_working_platforms)) { + if (force_load) + dev_warn(&wdev->dev, "Forcing loading on non-whitelisted platform"); + else + return -ENODEV; + } + + ret = gigabyte_wmi_validate_sensor_presence(wdev); + if (ret) { + dev_info(&wdev->dev, "No temperature sensors usable"); + return ret; + } + + hwmon_dev = devm_hwmon_device_register_with_info(&wdev->dev, "gigabyte_wmi", wdev, + &gigabyte_wmi_hwmon_chip_info, NULL); + + return PTR_ERR_OR_ZERO(hwmon_dev); +} + +static const struct wmi_device_id gigabyte_wmi_id_table[] = { + { GIGABYTE_WMI_GUID, NULL }, + { } +}; + +static struct wmi_driver gigabyte_wmi_driver = { + .driver = { + .name = "gigabyte-wmi", + }, + .id_table = gigabyte_wmi_id_table, + .probe = gigabyte_wmi_probe, +}; +module_wmi_driver(gigabyte_wmi_driver); + +MODULE_DEVICE_TABLE(wmi, gigabyte_wmi_id_table); +MODULE_AUTHOR("Thomas Weißschuh <thomas@weissschuh.net>"); +MODULE_DESCRIPTION("Gigabyte WMI temperature Driver"); +MODULE_LICENSE("GPL"); base-commit: 144c79ef33536b4ecb4951e07dbc1f2b7fa99d32 -- 2.31.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3] platform/x86: add Gigabyte WMI temperature driver 2021-04-10 14:40 ` [PATCH v3] platform/x86: add Gigabyte WMI temperature driver Thomas Weißschuh @ 2021-04-10 14:57 ` Hans de Goede 2021-04-10 15:21 ` Guenter Roeck 2021-04-10 15:15 ` Guenter Roeck 1 sibling, 1 reply; 10+ messages in thread From: Hans de Goede @ 2021-04-10 14:57 UTC (permalink / raw) To: Thomas Weißschuh, platform-driver-x86, Mark Gross, linux-kernel, Barnabás Pőcze, Jean Delvare, Guenter Roeck, linux-hwmon Cc: Matthew Garrett Hi, On 4/10/21 4:40 PM, Thomas Weißschuh wrote: > Changes since v1: > * Incorporate feedback from Barnabás Pőcze > * Use a WMI driver instead of a platform driver > * Let the kernel manage the driver lifecycle > * Fix errno/ACPI error confusion > * Fix resource cleanup > * Document reason for integer casting > > Changes since v2: > * Style cleanups > * Test for usability during probing > * DMI-based whitelist > * CC hwmon maintainers > > -- >8 -- > > Tested with a X570 I Aorus Pro Wifi. > The mainboard contains an ITE IT8688E chip for management. > This chips is also handled by drivers/hwmon/i87.c but as it is also used > by the firmware itself it needs an ACPI driver. > > Unfortunately not all sensor registers are handled by the firmware and even > less are exposed via WMI. > > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> This looks good, one small nitpick: I know this is a touchy subject for some, but we are trying to move away from the whitelist/blacklist naming where possible and we don't want to introduce any new cases, see: https://github.com/torvalds/linux/blob/master/Documentation/process/coding-style.rst#4-naming The driver currently uses this twice: "Force loading on non-whitelisted platform" "Forcing loading on non-whitelisted platform" Interestingly enough you already avoided naming the dmi_system_id table a whitelist (good). I would like to see "non-whitelisted" replaced with "unknown" so that we end up with: "Force loading on unknown platform" "Forcing loading on unknown platform" And while at it, I think for the second sentence this would be better English (I'm not a native speaker myself): "Forcing load on unknown platform" If you are ok with these changes I can fix this up while merging, no need to send a v4. Although if you prefer to send a v4 that is fine too. Either way let me know. Regards, Hans p.s. For v4 or for a next patch, the way to add the changelog so that it does not get picked up / automatically gets snipped by git am is to put it below the Signed-off-by at the end of the commit message like this: Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> --- Changes since v1: * Incorporate feedback from Barnabás Pőcze * Use a WMI driver instead of a platform driver * Let the kernel manage the driver lifecycle * Fix errno/ACPI error confusion * Fix resource cleanup * Document reason for integer casting Changes since v2: * Style cleanups * Test for usability during probing * DMI-based whitelist * CC hwmon maintainers > --- > MAINTAINERS | 6 + > drivers/platform/x86/Kconfig | 11 ++ > drivers/platform/x86/Makefile | 1 + > drivers/platform/x86/gigabyte-wmi.c | 194 ++++++++++++++++++++++++++++ > 4 files changed, 212 insertions(+) > create mode 100644 drivers/platform/x86/gigabyte-wmi.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index d92f85ca831d..9c10cfc00fe8 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -7543,6 +7543,12 @@ F: Documentation/filesystems/gfs2* > F: fs/gfs2/ > F: include/uapi/linux/gfs2_ondisk.h > > +GIGABYTE WMI DRIVER > +M: Thomas Weißschuh <linux@weissschuh.net> > +L: platform-driver-x86@vger.kernel.org > +S: Maintained > +F: drivers/platform/x86/gigabyte-wmi.c > + > GNSS SUBSYSTEM > M: Johan Hovold <johan@kernel.org> > S: Maintained > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig > index ad4e630e73e2..96622a2106f7 100644 > --- a/drivers/platform/x86/Kconfig > +++ b/drivers/platform/x86/Kconfig > @@ -123,6 +123,17 @@ config XIAOMI_WMI > To compile this driver as a module, choose M here: the module will > be called xiaomi-wmi. > > +config GIGABYTE_WMI > + tristate "Gigabyte WMI temperature driver" > + depends on ACPI_WMI > + depends on HWMON > + help > + Say Y here if you want to support WMI-based temperature reporting on > + Gigabyte mainboards. > + > + To compile this driver as a module, choose M here: the module will > + be called gigabyte-wmi. > + > config ACERHDF > tristate "Acer Aspire One temperature and fan driver" > depends on ACPI && THERMAL > diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile > index 60d554073749..1621ebfd04fd 100644 > --- a/drivers/platform/x86/Makefile > +++ b/drivers/platform/x86/Makefile > @@ -15,6 +15,7 @@ obj-$(CONFIG_INTEL_WMI_THUNDERBOLT) += intel-wmi-thunderbolt.o > obj-$(CONFIG_MXM_WMI) += mxm-wmi.o > obj-$(CONFIG_PEAQ_WMI) += peaq-wmi.o > obj-$(CONFIG_XIAOMI_WMI) += xiaomi-wmi.o > +obj-$(CONFIG_GIGABYTE_WMI) += gigabyte-wmi.o > > # Acer > obj-$(CONFIG_ACERHDF) += acerhdf.o > diff --git a/drivers/platform/x86/gigabyte-wmi.c b/drivers/platform/x86/gigabyte-wmi.c > new file mode 100644 > index 000000000000..fb4e6d4c1823 > --- /dev/null > +++ b/drivers/platform/x86/gigabyte-wmi.c > @@ -0,0 +1,194 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Copyright (C) 2021 Thomas Weißschuh <thomas@weissschuh.net> > + */ > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > + > +#include <linux/acpi.h> > +#include <linux/dmi.h> > +#include <linux/hwmon.h> > +#include <linux/module.h> > +#include <linux/wmi.h> > + > +#define GIGABYTE_WMI_GUID "DEADBEEF-2001-0000-00A0-C90629100000" > +#define NUM_TEMPERATURE_SENSORS 6 > + > +static bool force_load; > +module_param(force_load, bool, 0); > +MODULE_PARM_DESC(force_load, "Force loading on non-whitelisted platform"); > + > +enum gigabyte_wmi_commandtype { > + GIGABYTE_WMI_BUILD_DATE_QUERY = 0x1, > + GIGABYTE_WMI_MAINBOARD_TYPE_QUERY = 0x2, > + GIGABYTE_WMI_FIRMWARE_VERSION_QUERY = 0x4, > + GIGABYTE_WMI_MAINBOARD_NAME_QUERY = 0x5, > + GIGABYTE_WMI_TEMPERATURE_QUERY = 0x125, > +}; > + > +struct gigabyte_wmi_args { > + u32 arg1; > +}; > + > +static int gigabyte_wmi_perform_query(struct wmi_device *wdev, > + enum gigabyte_wmi_commandtype command, > + struct gigabyte_wmi_args *args, struct acpi_buffer *out) > +{ > + const struct acpi_buffer in = { > + .length = sizeof(*args), > + .pointer = args, > + }; > + > + acpi_status ret = wmidev_evaluate_method(wdev, 0x0, command, &in, out); > + > + if ACPI_FAILURE(ret) > + return -EIO; > + > + return 0; > +} > + > +static int gigabyte_wmi_query_integer(struct wmi_device *wdev, > + enum gigabyte_wmi_commandtype command, > + struct gigabyte_wmi_args *args, u64 *res) > +{ > + union acpi_object *obj; > + struct acpi_buffer result = { ACPI_ALLOCATE_BUFFER, NULL }; > + int ret; > + > + ret = gigabyte_wmi_perform_query(wdev, command, args, &result); > + if (ret) > + return ret; > + obj = result.pointer; > + if (obj && obj->type == ACPI_TYPE_INTEGER) > + *res = obj->integer.value; > + else > + ret = -EIO; > + kfree(result.pointer); > + return ret; > +} > + > +static int gigabyte_wmi_temperature(struct wmi_device *wdev, u8 sensor, long *res) > +{ > + struct gigabyte_wmi_args args = { > + .arg1 = sensor, > + }; > + u64 temp; > + acpi_status ret; > + > + ret = gigabyte_wmi_query_integer(wdev, GIGABYTE_WMI_TEMPERATURE_QUERY, &args, &temp); > + if (ret == 0) { > + if (temp == 0) > + return -ENODEV; > + *res = (s8)temp * 1000; // value is a signed 8-bit integer > + } > + return ret; > +} > + > +static int gigabyte_wmi_hwmon_read(struct device *dev, enum hwmon_sensor_types type, > + u32 attr, int channel, long *val) > +{ > + struct wmi_device *wdev = dev_get_drvdata(dev); > + > + return gigabyte_wmi_temperature(wdev, channel, val); > +} > + > +static umode_t gigabyte_wmi_hwmon_is_visible(const void *data, enum hwmon_sensor_types type, > + u32 attr, int channel) > +{ > + return 0444; > +} > + > +static const struct hwmon_channel_info *gigabyte_wmi_hwmon_info[] = { > + HWMON_CHANNEL_INFO(temp, > + HWMON_T_INPUT, > + HWMON_T_INPUT, > + HWMON_T_INPUT, > + HWMON_T_INPUT, > + HWMON_T_INPUT, > + HWMON_T_INPUT), > + NULL > +}; > + > +static const struct hwmon_ops gigabyte_wmi_hwmon_ops = { > + .read = gigabyte_wmi_hwmon_read, > + .is_visible = gigabyte_wmi_hwmon_is_visible, > +}; > + > +static const struct hwmon_chip_info gigabyte_wmi_hwmon_chip_info = { > + .ops = &gigabyte_wmi_hwmon_ops, > + .info = gigabyte_wmi_hwmon_info, > +}; > + > +static int gigabyte_wmi_validate_sensor_presence(struct wmi_device *wdev) > +{ > + int working_sensors = 0, i; > + long temp; > + > + for (i = 0; i < NUM_TEMPERATURE_SENSORS; i++) { > + if (!gigabyte_wmi_temperature(wdev, i, &temp)) > + working_sensors++; > + } > + return working_sensors ? 0 : -ENODEV; > +} > + > +static const struct dmi_system_id gigabyte_wmi_known_working_platforms[] = { > + { .matches = { > + DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "Gigabyte Technology Co., Ltd."), > + DMI_EXACT_MATCH(DMI_BOARD_NAME, "B550 GAMING X V2"), > + }}, > + { .matches = { > + DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "Gigabyte Technology Co., Ltd."), > + DMI_EXACT_MATCH(DMI_BOARD_NAME, "B550M DS3H"), > + }}, > + { .matches = { > + DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "Gigabyte Technology Co., Ltd."), > + DMI_EXACT_MATCH(DMI_BOARD_NAME, "Z390 I AORUS PRO WIFI-CF"), > + }}, > + { .matches = { > + DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "Gigabyte Technology Co., Ltd."), > + DMI_EXACT_MATCH(DMI_BOARD_NAME, "X570 I AORUS PRO WIFI"), > + }}, > + { } > +}; > + > +static int gigabyte_wmi_probe(struct wmi_device *wdev, const void *context) > +{ > + struct device *hwmon_dev; > + int ret; > + > + if (!dmi_check_system(gigabyte_wmi_known_working_platforms)) { > + if (force_load) > + dev_warn(&wdev->dev, "Forcing loading on non-whitelisted platform"); > + else > + return -ENODEV; > + } > + > + ret = gigabyte_wmi_validate_sensor_presence(wdev); > + if (ret) { > + dev_info(&wdev->dev, "No temperature sensors usable"); > + return ret; > + } > + > + hwmon_dev = devm_hwmon_device_register_with_info(&wdev->dev, "gigabyte_wmi", wdev, > + &gigabyte_wmi_hwmon_chip_info, NULL); > + > + return PTR_ERR_OR_ZERO(hwmon_dev); > +} > + > +static const struct wmi_device_id gigabyte_wmi_id_table[] = { > + { GIGABYTE_WMI_GUID, NULL }, > + { } > +}; > + > +static struct wmi_driver gigabyte_wmi_driver = { > + .driver = { > + .name = "gigabyte-wmi", > + }, > + .id_table = gigabyte_wmi_id_table, > + .probe = gigabyte_wmi_probe, > +}; > +module_wmi_driver(gigabyte_wmi_driver); > + > +MODULE_DEVICE_TABLE(wmi, gigabyte_wmi_id_table); > +MODULE_AUTHOR("Thomas Weißschuh <thomas@weissschuh.net>"); > +MODULE_DESCRIPTION("Gigabyte WMI temperature Driver"); > +MODULE_LICENSE("GPL"); > > base-commit: 144c79ef33536b4ecb4951e07dbc1f2b7fa99d32 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3] platform/x86: add Gigabyte WMI temperature driver 2021-04-10 14:57 ` Hans de Goede @ 2021-04-10 15:21 ` Guenter Roeck 0 siblings, 0 replies; 10+ messages in thread From: Guenter Roeck @ 2021-04-10 15:21 UTC (permalink / raw) To: Hans de Goede, Thomas Weißschuh, platform-driver-x86, Mark Gross, linux-kernel, Barnabás Pőcze, Jean Delvare, linux-hwmon Cc: Matthew Garrett On 4/10/21 7:57 AM, Hans de Goede wrote: > Hi, > > On 4/10/21 4:40 PM, Thomas Weißschuh wrote: >> Changes since v1: >> * Incorporate feedback from Barnabás Pőcze >> * Use a WMI driver instead of a platform driver >> * Let the kernel manage the driver lifecycle >> * Fix errno/ACPI error confusion >> * Fix resource cleanup >> * Document reason for integer casting >> >> Changes since v2: >> * Style cleanups >> * Test for usability during probing >> * DMI-based whitelist >> * CC hwmon maintainers >> >> -- >8 -- >> >> Tested with a X570 I Aorus Pro Wifi. >> The mainboard contains an ITE IT8688E chip for management. >> This chips is also handled by drivers/hwmon/i87.c but as it is also used >> by the firmware itself it needs an ACPI driver. >> >> Unfortunately not all sensor registers are handled by the firmware and even >> less are exposed via WMI. >> >> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> > > This looks good, one small nitpick: > > I know this is a touchy subject for some, but we are trying to move away > from the whitelist/blacklist naming where possible and we don't want to > introduce any new cases, see: > > https://github.com/torvalds/linux/blob/master/Documentation/process/coding-style.rst#4-naming > > The driver currently uses this twice: > "Force loading on non-whitelisted platform" > "Forcing loading on non-whitelisted platform" > > Interestingly enough you already avoided naming the dmi_system_id table > a whitelist (good). > > I would like to see "non-whitelisted" replaced with "unknown" so that we end up with: > > "Force loading on unknown platform" > "Forcing loading on unknown platform" > > And while at it, I think for the second sentence this would be better English > (I'm not a native speaker myself): > > "Forcing load on unknown platform" > Not native either, but I think it is either "Forcing load" or "Force loading". > If you are ok with these changes I can fix this up while merging, no need > to send a v4. Although if you prefer to send a v4 that is fine too. > Please consider adding an existence check into the is_visible function. Sysfs attributes for non-existing sensors should not be created. Thanks, Guenter > Either way let me know. > > Regards, > > Hans > > p.s. > > For v4 or for a next patch, the way to add the changelog so that it does > not get picked up / automatically gets snipped by git am is to put it > below the Signed-off-by at the end of the commit message like this: > > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> > --- > Changes since v1: > * Incorporate feedback from Barnabás Pőcze > * Use a WMI driver instead of a platform driver > * Let the kernel manage the driver lifecycle > * Fix errno/ACPI error confusion > * Fix resource cleanup > * Document reason for integer casting > > Changes since v2: > * Style cleanups > * Test for usability during probing > * DMI-based whitelist > * CC hwmon maintainers > > > > > > > >> --- >> MAINTAINERS | 6 + >> drivers/platform/x86/Kconfig | 11 ++ >> drivers/platform/x86/Makefile | 1 + >> drivers/platform/x86/gigabyte-wmi.c | 194 ++++++++++++++++++++++++++++ >> 4 files changed, 212 insertions(+) >> create mode 100644 drivers/platform/x86/gigabyte-wmi.c >> >> diff --git a/MAINTAINERS b/MAINTAINERS >> index d92f85ca831d..9c10cfc00fe8 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -7543,6 +7543,12 @@ F: Documentation/filesystems/gfs2* >> F: fs/gfs2/ >> F: include/uapi/linux/gfs2_ondisk.h >> >> +GIGABYTE WMI DRIVER >> +M: Thomas Weißschuh <linux@weissschuh.net> >> +L: platform-driver-x86@vger.kernel.org >> +S: Maintained >> +F: drivers/platform/x86/gigabyte-wmi.c >> + >> GNSS SUBSYSTEM >> M: Johan Hovold <johan@kernel.org> >> S: Maintained >> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig >> index ad4e630e73e2..96622a2106f7 100644 >> --- a/drivers/platform/x86/Kconfig >> +++ b/drivers/platform/x86/Kconfig >> @@ -123,6 +123,17 @@ config XIAOMI_WMI >> To compile this driver as a module, choose M here: the module will >> be called xiaomi-wmi. >> >> +config GIGABYTE_WMI >> + tristate "Gigabyte WMI temperature driver" >> + depends on ACPI_WMI >> + depends on HWMON >> + help >> + Say Y here if you want to support WMI-based temperature reporting on >> + Gigabyte mainboards. >> + >> + To compile this driver as a module, choose M here: the module will >> + be called gigabyte-wmi. >> + >> config ACERHDF >> tristate "Acer Aspire One temperature and fan driver" >> depends on ACPI && THERMAL >> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile >> index 60d554073749..1621ebfd04fd 100644 >> --- a/drivers/platform/x86/Makefile >> +++ b/drivers/platform/x86/Makefile >> @@ -15,6 +15,7 @@ obj-$(CONFIG_INTEL_WMI_THUNDERBOLT) += intel-wmi-thunderbolt.o >> obj-$(CONFIG_MXM_WMI) += mxm-wmi.o >> obj-$(CONFIG_PEAQ_WMI) += peaq-wmi.o >> obj-$(CONFIG_XIAOMI_WMI) += xiaomi-wmi.o >> +obj-$(CONFIG_GIGABYTE_WMI) += gigabyte-wmi.o >> >> # Acer >> obj-$(CONFIG_ACERHDF) += acerhdf.o >> diff --git a/drivers/platform/x86/gigabyte-wmi.c b/drivers/platform/x86/gigabyte-wmi.c >> new file mode 100644 >> index 000000000000..fb4e6d4c1823 >> --- /dev/null >> +++ b/drivers/platform/x86/gigabyte-wmi.c >> @@ -0,0 +1,194 @@ >> +// SPDX-License-Identifier: GPL-2.0-or-later >> +/* >> + * Copyright (C) 2021 Thomas Weißschuh <thomas@weissschuh.net> >> + */ >> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt >> + >> +#include <linux/acpi.h> >> +#include <linux/dmi.h> >> +#include <linux/hwmon.h> >> +#include <linux/module.h> >> +#include <linux/wmi.h> >> + >> +#define GIGABYTE_WMI_GUID "DEADBEEF-2001-0000-00A0-C90629100000" >> +#define NUM_TEMPERATURE_SENSORS 6 >> + >> +static bool force_load; >> +module_param(force_load, bool, 0); >> +MODULE_PARM_DESC(force_load, "Force loading on non-whitelisted platform"); >> + >> +enum gigabyte_wmi_commandtype { >> + GIGABYTE_WMI_BUILD_DATE_QUERY = 0x1, >> + GIGABYTE_WMI_MAINBOARD_TYPE_QUERY = 0x2, >> + GIGABYTE_WMI_FIRMWARE_VERSION_QUERY = 0x4, >> + GIGABYTE_WMI_MAINBOARD_NAME_QUERY = 0x5, >> + GIGABYTE_WMI_TEMPERATURE_QUERY = 0x125, >> +}; >> + >> +struct gigabyte_wmi_args { >> + u32 arg1; >> +}; >> + >> +static int gigabyte_wmi_perform_query(struct wmi_device *wdev, >> + enum gigabyte_wmi_commandtype command, >> + struct gigabyte_wmi_args *args, struct acpi_buffer *out) >> +{ >> + const struct acpi_buffer in = { >> + .length = sizeof(*args), >> + .pointer = args, >> + }; >> + >> + acpi_status ret = wmidev_evaluate_method(wdev, 0x0, command, &in, out); >> + >> + if ACPI_FAILURE(ret) >> + return -EIO; >> + >> + return 0; >> +} >> + >> +static int gigabyte_wmi_query_integer(struct wmi_device *wdev, >> + enum gigabyte_wmi_commandtype command, >> + struct gigabyte_wmi_args *args, u64 *res) >> +{ >> + union acpi_object *obj; >> + struct acpi_buffer result = { ACPI_ALLOCATE_BUFFER, NULL }; >> + int ret; >> + >> + ret = gigabyte_wmi_perform_query(wdev, command, args, &result); >> + if (ret) >> + return ret; >> + obj = result.pointer; >> + if (obj && obj->type == ACPI_TYPE_INTEGER) >> + *res = obj->integer.value; >> + else >> + ret = -EIO; >> + kfree(result.pointer); >> + return ret; >> +} >> + >> +static int gigabyte_wmi_temperature(struct wmi_device *wdev, u8 sensor, long *res) >> +{ >> + struct gigabyte_wmi_args args = { >> + .arg1 = sensor, >> + }; >> + u64 temp; >> + acpi_status ret; >> + >> + ret = gigabyte_wmi_query_integer(wdev, GIGABYTE_WMI_TEMPERATURE_QUERY, &args, &temp); >> + if (ret == 0) { >> + if (temp == 0) >> + return -ENODEV; >> + *res = (s8)temp * 1000; // value is a signed 8-bit integer >> + } >> + return ret; >> +} >> + >> +static int gigabyte_wmi_hwmon_read(struct device *dev, enum hwmon_sensor_types type, >> + u32 attr, int channel, long *val) >> +{ >> + struct wmi_device *wdev = dev_get_drvdata(dev); >> + >> + return gigabyte_wmi_temperature(wdev, channel, val); >> +} >> + >> +static umode_t gigabyte_wmi_hwmon_is_visible(const void *data, enum hwmon_sensor_types type, >> + u32 attr, int channel) >> +{ >> + return 0444; >> +} >> + >> +static const struct hwmon_channel_info *gigabyte_wmi_hwmon_info[] = { >> + HWMON_CHANNEL_INFO(temp, >> + HWMON_T_INPUT, >> + HWMON_T_INPUT, >> + HWMON_T_INPUT, >> + HWMON_T_INPUT, >> + HWMON_T_INPUT, >> + HWMON_T_INPUT), >> + NULL >> +}; >> + >> +static const struct hwmon_ops gigabyte_wmi_hwmon_ops = { >> + .read = gigabyte_wmi_hwmon_read, >> + .is_visible = gigabyte_wmi_hwmon_is_visible, >> +}; >> + >> +static const struct hwmon_chip_info gigabyte_wmi_hwmon_chip_info = { >> + .ops = &gigabyte_wmi_hwmon_ops, >> + .info = gigabyte_wmi_hwmon_info, >> +}; >> + >> +static int gigabyte_wmi_validate_sensor_presence(struct wmi_device *wdev) >> +{ >> + int working_sensors = 0, i; >> + long temp; >> + >> + for (i = 0; i < NUM_TEMPERATURE_SENSORS; i++) { >> + if (!gigabyte_wmi_temperature(wdev, i, &temp)) >> + working_sensors++; >> + } >> + return working_sensors ? 0 : -ENODEV; >> +} >> + >> +static const struct dmi_system_id gigabyte_wmi_known_working_platforms[] = { >> + { .matches = { >> + DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "Gigabyte Technology Co., Ltd."), >> + DMI_EXACT_MATCH(DMI_BOARD_NAME, "B550 GAMING X V2"), >> + }}, >> + { .matches = { >> + DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "Gigabyte Technology Co., Ltd."), >> + DMI_EXACT_MATCH(DMI_BOARD_NAME, "B550M DS3H"), >> + }}, >> + { .matches = { >> + DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "Gigabyte Technology Co., Ltd."), >> + DMI_EXACT_MATCH(DMI_BOARD_NAME, "Z390 I AORUS PRO WIFI-CF"), >> + }}, >> + { .matches = { >> + DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "Gigabyte Technology Co., Ltd."), >> + DMI_EXACT_MATCH(DMI_BOARD_NAME, "X570 I AORUS PRO WIFI"), >> + }}, >> + { } >> +}; >> + >> +static int gigabyte_wmi_probe(struct wmi_device *wdev, const void *context) >> +{ >> + struct device *hwmon_dev; >> + int ret; >> + >> + if (!dmi_check_system(gigabyte_wmi_known_working_platforms)) { >> + if (force_load) >> + dev_warn(&wdev->dev, "Forcing loading on non-whitelisted platform"); >> + else >> + return -ENODEV; >> + } >> + >> + ret = gigabyte_wmi_validate_sensor_presence(wdev); >> + if (ret) { >> + dev_info(&wdev->dev, "No temperature sensors usable"); >> + return ret; >> + } >> + >> + hwmon_dev = devm_hwmon_device_register_with_info(&wdev->dev, "gigabyte_wmi", wdev, >> + &gigabyte_wmi_hwmon_chip_info, NULL); >> + >> + return PTR_ERR_OR_ZERO(hwmon_dev); >> +} >> + >> +static const struct wmi_device_id gigabyte_wmi_id_table[] = { >> + { GIGABYTE_WMI_GUID, NULL }, >> + { } >> +}; >> + >> +static struct wmi_driver gigabyte_wmi_driver = { >> + .driver = { >> + .name = "gigabyte-wmi", >> + }, >> + .id_table = gigabyte_wmi_id_table, >> + .probe = gigabyte_wmi_probe, >> +}; >> +module_wmi_driver(gigabyte_wmi_driver); >> + >> +MODULE_DEVICE_TABLE(wmi, gigabyte_wmi_id_table); >> +MODULE_AUTHOR("Thomas Weißschuh <thomas@weissschuh.net>"); >> +MODULE_DESCRIPTION("Gigabyte WMI temperature Driver"); >> +MODULE_LICENSE("GPL"); >> >> base-commit: 144c79ef33536b4ecb4951e07dbc1f2b7fa99d32 >> > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3] platform/x86: add Gigabyte WMI temperature driver 2021-04-10 14:40 ` [PATCH v3] platform/x86: add Gigabyte WMI temperature driver Thomas Weißschuh 2021-04-10 14:57 ` Hans de Goede @ 2021-04-10 15:15 ` Guenter Roeck 2021-04-10 15:23 ` Hans de Goede 1 sibling, 1 reply; 10+ messages in thread From: Guenter Roeck @ 2021-04-10 15:15 UTC (permalink / raw) To: Thomas Weißschuh, platform-driver-x86, Mark Gross, Hans de Goede, linux-kernel, Barnabás Pőcze, Jean Delvare, linux-hwmon Cc: Matthew Garrett On 4/10/21 7:40 AM, Thomas Weißschuh wrote: > Changes since v1: > * Incorporate feedback from Barnabás Pőcze > * Use a WMI driver instead of a platform driver > * Let the kernel manage the driver lifecycle > * Fix errno/ACPI error confusion > * Fix resource cleanup > * Document reason for integer casting > > Changes since v2: > * Style cleanups > * Test for usability during probing > * DMI-based whitelist > * CC hwmon maintainers > > -- >8 -- > > Tested with a X570 I Aorus Pro Wifi. > The mainboard contains an ITE IT8688E chip for management. > This chips is also handled by drivers/hwmon/i87.c but as it is also used > by the firmware itself it needs an ACPI driver. > > Unfortunately not all sensor registers are handled by the firmware and even > less are exposed via WMI. > > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> > --- > MAINTAINERS | 6 + > drivers/platform/x86/Kconfig | 11 ++ > drivers/platform/x86/Makefile | 1 + > drivers/platform/x86/gigabyte-wmi.c | 194 ++++++++++++++++++++++++++++ > 4 files changed, 212 insertions(+) > create mode 100644 drivers/platform/x86/gigabyte-wmi.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index d92f85ca831d..9c10cfc00fe8 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -7543,6 +7543,12 @@ F: Documentation/filesystems/gfs2* > F: fs/gfs2/ > F: include/uapi/linux/gfs2_ondisk.h > > +GIGABYTE WMI DRIVER > +M: Thomas Weißschuh <linux@weissschuh.net> > +L: platform-driver-x86@vger.kernel.org > +S: Maintained > +F: drivers/platform/x86/gigabyte-wmi.c > + > GNSS SUBSYSTEM > M: Johan Hovold <johan@kernel.org> > S: Maintained > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig > index ad4e630e73e2..96622a2106f7 100644 > --- a/drivers/platform/x86/Kconfig > +++ b/drivers/platform/x86/Kconfig > @@ -123,6 +123,17 @@ config XIAOMI_WMI > To compile this driver as a module, choose M here: the module will > be called xiaomi-wmi. > > +config GIGABYTE_WMI > + tristate "Gigabyte WMI temperature driver" > + depends on ACPI_WMI > + depends on HWMON > + help > + Say Y here if you want to support WMI-based temperature reporting on > + Gigabyte mainboards. > + > + To compile this driver as a module, choose M here: the module will > + be called gigabyte-wmi. > + > config ACERHDF > tristate "Acer Aspire One temperature and fan driver" > depends on ACPI && THERMAL > diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile > index 60d554073749..1621ebfd04fd 100644 > --- a/drivers/platform/x86/Makefile > +++ b/drivers/platform/x86/Makefile > @@ -15,6 +15,7 @@ obj-$(CONFIG_INTEL_WMI_THUNDERBOLT) += intel-wmi-thunderbolt.o > obj-$(CONFIG_MXM_WMI) += mxm-wmi.o > obj-$(CONFIG_PEAQ_WMI) += peaq-wmi.o > obj-$(CONFIG_XIAOMI_WMI) += xiaomi-wmi.o > +obj-$(CONFIG_GIGABYTE_WMI) += gigabyte-wmi.o > > # Acer > obj-$(CONFIG_ACERHDF) += acerhdf.o > diff --git a/drivers/platform/x86/gigabyte-wmi.c b/drivers/platform/x86/gigabyte-wmi.c > new file mode 100644 > index 000000000000..fb4e6d4c1823 > --- /dev/null > +++ b/drivers/platform/x86/gigabyte-wmi.c > @@ -0,0 +1,194 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Copyright (C) 2021 Thomas Weißschuh <thomas@weissschuh.net> > + */ > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > + > +#include <linux/acpi.h> > +#include <linux/dmi.h> > +#include <linux/hwmon.h> > +#include <linux/module.h> > +#include <linux/wmi.h> > + > +#define GIGABYTE_WMI_GUID "DEADBEEF-2001-0000-00A0-C90629100000" > +#define NUM_TEMPERATURE_SENSORS 6 Style: #define<space>name<tab>value but of course that is Hans' call. > + > +static bool force_load; > +module_param(force_load, bool, 0); > +MODULE_PARM_DESC(force_load, "Force loading on non-whitelisted platform"); > + > +enum gigabyte_wmi_commandtype { > + GIGABYTE_WMI_BUILD_DATE_QUERY = 0x1, > + GIGABYTE_WMI_MAINBOARD_TYPE_QUERY = 0x2, > + GIGABYTE_WMI_FIRMWARE_VERSION_QUERY = 0x4, > + GIGABYTE_WMI_MAINBOARD_NAME_QUERY = 0x5, > + GIGABYTE_WMI_TEMPERATURE_QUERY = 0x125, > +}; > + > +struct gigabyte_wmi_args { > + u32 arg1; > +}; > + > +static int gigabyte_wmi_perform_query(struct wmi_device *wdev, > + enum gigabyte_wmi_commandtype command, > + struct gigabyte_wmi_args *args, struct acpi_buffer *out) > +{ > + const struct acpi_buffer in = { > + .length = sizeof(*args), > + .pointer = args, > + }; > + > + acpi_status ret = wmidev_evaluate_method(wdev, 0x0, command, &in, out); > + > + if ACPI_FAILURE(ret) > + return -EIO; > + > + return 0; > +} > + > +static int gigabyte_wmi_query_integer(struct wmi_device *wdev, > + enum gigabyte_wmi_commandtype command, > + struct gigabyte_wmi_args *args, u64 *res) > +{ > + union acpi_object *obj; > + struct acpi_buffer result = { ACPI_ALLOCATE_BUFFER, NULL }; > + int ret; > + > + ret = gigabyte_wmi_perform_query(wdev, command, args, &result); > + if (ret) > + return ret; > + obj = result.pointer; > + if (obj && obj->type == ACPI_TYPE_INTEGER) > + *res = obj->integer.value; > + else > + ret = -EIO; > + kfree(result.pointer); > + return ret; > +} > + > +static int gigabyte_wmi_temperature(struct wmi_device *wdev, u8 sensor, long *res) > +{ > + struct gigabyte_wmi_args args = { > + .arg1 = sensor, > + }; > + u64 temp; > + acpi_status ret; > + > + ret = gigabyte_wmi_query_integer(wdev, GIGABYTE_WMI_TEMPERATURE_QUERY, &args, &temp); > + if (ret == 0) { > + if (temp == 0) > + return -ENODEV; That should be checked in gigabyte_wmi_hwmon_is_visible(); that is what that function is for. > + *res = (s8)temp * 1000; // value is a signed 8-bit integer > + } > + return ret; > +} > + > +static int gigabyte_wmi_hwmon_read(struct device *dev, enum hwmon_sensor_types type, > + u32 attr, int channel, long *val) > +{ > + struct wmi_device *wdev = dev_get_drvdata(dev); > + > + return gigabyte_wmi_temperature(wdev, channel, val); > +} > + > +static umode_t gigabyte_wmi_hwmon_is_visible(const void *data, enum hwmon_sensor_types type, > + u32 attr, int channel) > +{ > + return 0444; > +} > + > +static const struct hwmon_channel_info *gigabyte_wmi_hwmon_info[] = { > + HWMON_CHANNEL_INFO(temp, > + HWMON_T_INPUT, > + HWMON_T_INPUT, > + HWMON_T_INPUT, > + HWMON_T_INPUT, > + HWMON_T_INPUT, > + HWMON_T_INPUT), > + NULL > +}; > + > +static const struct hwmon_ops gigabyte_wmi_hwmon_ops = { > + .read = gigabyte_wmi_hwmon_read, > + .is_visible = gigabyte_wmi_hwmon_is_visible, > +}; > + > +static const struct hwmon_chip_info gigabyte_wmi_hwmon_chip_info = { > + .ops = &gigabyte_wmi_hwmon_ops, > + .info = gigabyte_wmi_hwmon_info, > +}; > + > +static int gigabyte_wmi_validate_sensor_presence(struct wmi_device *wdev) > +{ > + int working_sensors = 0, i; > + long temp; > + > + for (i = 0; i < NUM_TEMPERATURE_SENSORS; i++) { > + if (!gigabyte_wmi_temperature(wdev, i, &temp)) > + working_sensors++; > + } > + return working_sensors ? 0 : -ENODEV; > +} > + > +static const struct dmi_system_id gigabyte_wmi_known_working_platforms[] = { > + { .matches = { > + DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "Gigabyte Technology Co., Ltd."), > + DMI_EXACT_MATCH(DMI_BOARD_NAME, "B550 GAMING X V2"), > + }}, > + { .matches = { > + DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "Gigabyte Technology Co., Ltd."), > + DMI_EXACT_MATCH(DMI_BOARD_NAME, "B550M DS3H"), > + }}, > + { .matches = { > + DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "Gigabyte Technology Co., Ltd."), > + DMI_EXACT_MATCH(DMI_BOARD_NAME, "Z390 I AORUS PRO WIFI-CF"), > + }}, > + { .matches = { > + DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "Gigabyte Technology Co., Ltd."), > + DMI_EXACT_MATCH(DMI_BOARD_NAME, "X570 I AORUS PRO WIFI"), > + }}, > + { } > +}; > + > +static int gigabyte_wmi_probe(struct wmi_device *wdev, const void *context) > +{ > + struct device *hwmon_dev; > + int ret; > + > + if (!dmi_check_system(gigabyte_wmi_known_working_platforms)) { > + if (force_load) > + dev_warn(&wdev->dev, "Forcing loading on non-whitelisted platform"); > + else > + return -ENODEV; Style: if (!force_load) return -ENODEV; dev_warn(&wdev->dev, "Forcing loading on non-whitelisted platform"); > + } > + > + ret = gigabyte_wmi_validate_sensor_presence(wdev); > + if (ret) { > + dev_info(&wdev->dev, "No temperature sensors usable"); Normally one does not display a message if a probe function returns -ENODEV, unless it is an error, to avoid polluting the kernel log. > + return ret; > + } > + > + hwmon_dev = devm_hwmon_device_register_with_info(&wdev->dev, "gigabyte_wmi", wdev, > + &gigabyte_wmi_hwmon_chip_info, NULL); > + > + return PTR_ERR_OR_ZERO(hwmon_dev); > +} > + > +static const struct wmi_device_id gigabyte_wmi_id_table[] = { > + { GIGABYTE_WMI_GUID, NULL }, > + { } > +}; > + > +static struct wmi_driver gigabyte_wmi_driver = { > + .driver = { > + .name = "gigabyte-wmi", > + }, > + .id_table = gigabyte_wmi_id_table, > + .probe = gigabyte_wmi_probe, > +}; > +module_wmi_driver(gigabyte_wmi_driver); > + > +MODULE_DEVICE_TABLE(wmi, gigabyte_wmi_id_table); > +MODULE_AUTHOR("Thomas Weißschuh <thomas@weissschuh.net>"); > +MODULE_DESCRIPTION("Gigabyte WMI temperature Driver"); > +MODULE_LICENSE("GPL"); > > base-commit: 144c79ef33536b4ecb4951e07dbc1f2b7fa99d32 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3] platform/x86: add Gigabyte WMI temperature driver 2021-04-10 15:15 ` Guenter Roeck @ 2021-04-10 15:23 ` Hans de Goede 2021-04-10 18:18 ` [PATCH v4] " Thomas Weißschuh 0 siblings, 1 reply; 10+ messages in thread From: Hans de Goede @ 2021-04-10 15:23 UTC (permalink / raw) To: Guenter Roeck, Thomas Weißschuh, platform-driver-x86, Mark Gross, linux-kernel, Barnabás Pőcze, Jean Delvare, linux-hwmon Cc: Matthew Garrett Hi, On 4/10/21 5:15 PM, Guenter Roeck wrote: > On 4/10/21 7:40 AM, Thomas Weißschuh wrote: >> Changes since v1: >> * Incorporate feedback from Barnabás Pőcze >> * Use a WMI driver instead of a platform driver >> * Let the kernel manage the driver lifecycle >> * Fix errno/ACPI error confusion >> * Fix resource cleanup >> * Document reason for integer casting >> >> Changes since v2: >> * Style cleanups >> * Test for usability during probing >> * DMI-based whitelist >> * CC hwmon maintainers >> >> -- >8 -- >> >> Tested with a X570 I Aorus Pro Wifi. >> The mainboard contains an ITE IT8688E chip for management. >> This chips is also handled by drivers/hwmon/i87.c but as it is also used >> by the firmware itself it needs an ACPI driver. >> >> Unfortunately not all sensor registers are handled by the firmware and even >> less are exposed via WMI. >> >> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> >> --- >> MAINTAINERS | 6 + >> drivers/platform/x86/Kconfig | 11 ++ >> drivers/platform/x86/Makefile | 1 + >> drivers/platform/x86/gigabyte-wmi.c | 194 ++++++++++++++++++++++++++++ >> 4 files changed, 212 insertions(+) >> create mode 100644 drivers/platform/x86/gigabyte-wmi.c >> >> diff --git a/MAINTAINERS b/MAINTAINERS >> index d92f85ca831d..9c10cfc00fe8 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -7543,6 +7543,12 @@ F: Documentation/filesystems/gfs2* >> F: fs/gfs2/ >> F: include/uapi/linux/gfs2_ondisk.h >> >> +GIGABYTE WMI DRIVER >> +M: Thomas Weißschuh <linux@weissschuh.net> >> +L: platform-driver-x86@vger.kernel.org >> +S: Maintained >> +F: drivers/platform/x86/gigabyte-wmi.c >> + >> GNSS SUBSYSTEM >> M: Johan Hovold <johan@kernel.org> >> S: Maintained >> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig >> index ad4e630e73e2..96622a2106f7 100644 >> --- a/drivers/platform/x86/Kconfig >> +++ b/drivers/platform/x86/Kconfig >> @@ -123,6 +123,17 @@ config XIAOMI_WMI >> To compile this driver as a module, choose M here: the module will >> be called xiaomi-wmi. >> >> +config GIGABYTE_WMI >> + tristate "Gigabyte WMI temperature driver" >> + depends on ACPI_WMI >> + depends on HWMON >> + help >> + Say Y here if you want to support WMI-based temperature reporting on >> + Gigabyte mainboards. >> + >> + To compile this driver as a module, choose M here: the module will >> + be called gigabyte-wmi. >> + >> config ACERHDF >> tristate "Acer Aspire One temperature and fan driver" >> depends on ACPI && THERMAL >> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile >> index 60d554073749..1621ebfd04fd 100644 >> --- a/drivers/platform/x86/Makefile >> +++ b/drivers/platform/x86/Makefile >> @@ -15,6 +15,7 @@ obj-$(CONFIG_INTEL_WMI_THUNDERBOLT) += intel-wmi-thunderbolt.o >> obj-$(CONFIG_MXM_WMI) += mxm-wmi.o >> obj-$(CONFIG_PEAQ_WMI) += peaq-wmi.o >> obj-$(CONFIG_XIAOMI_WMI) += xiaomi-wmi.o >> +obj-$(CONFIG_GIGABYTE_WMI) += gigabyte-wmi.o >> >> # Acer >> obj-$(CONFIG_ACERHDF) += acerhdf.o >> diff --git a/drivers/platform/x86/gigabyte-wmi.c b/drivers/platform/x86/gigabyte-wmi.c >> new file mode 100644 >> index 000000000000..fb4e6d4c1823 >> --- /dev/null >> +++ b/drivers/platform/x86/gigabyte-wmi.c >> @@ -0,0 +1,194 @@ >> +// SPDX-License-Identifier: GPL-2.0-or-later >> +/* >> + * Copyright (C) 2021 Thomas Weißschuh <thomas@weissschuh.net> >> + */ >> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt >> + >> +#include <linux/acpi.h> >> +#include <linux/dmi.h> >> +#include <linux/hwmon.h> >> +#include <linux/module.h> >> +#include <linux/wmi.h> >> + >> +#define GIGABYTE_WMI_GUID "DEADBEEF-2001-0000-00A0-C90629100000" >> +#define NUM_TEMPERATURE_SENSORS 6 > > Style: #define<space>name<tab>value > > but of course that is Hans' call. I agree that aligning the 2 define values with tabs would be better. > >> + >> +static bool force_load; >> +module_param(force_load, bool, 0); >> +MODULE_PARM_DESC(force_load, "Force loading on non-whitelisted platform"); >> + >> +enum gigabyte_wmi_commandtype { >> + GIGABYTE_WMI_BUILD_DATE_QUERY = 0x1, >> + GIGABYTE_WMI_MAINBOARD_TYPE_QUERY = 0x2, >> + GIGABYTE_WMI_FIRMWARE_VERSION_QUERY = 0x4, >> + GIGABYTE_WMI_MAINBOARD_NAME_QUERY = 0x5, >> + GIGABYTE_WMI_TEMPERATURE_QUERY = 0x125, >> +}; >> + >> +struct gigabyte_wmi_args { >> + u32 arg1; >> +}; >> + >> +static int gigabyte_wmi_perform_query(struct wmi_device *wdev, >> + enum gigabyte_wmi_commandtype command, >> + struct gigabyte_wmi_args *args, struct acpi_buffer *out) >> +{ >> + const struct acpi_buffer in = { >> + .length = sizeof(*args), >> + .pointer = args, >> + }; >> + >> + acpi_status ret = wmidev_evaluate_method(wdev, 0x0, command, &in, out); >> + >> + if ACPI_FAILURE(ret) >> + return -EIO; >> + >> + return 0; >> +} >> + >> +static int gigabyte_wmi_query_integer(struct wmi_device *wdev, >> + enum gigabyte_wmi_commandtype command, >> + struct gigabyte_wmi_args *args, u64 *res) >> +{ >> + union acpi_object *obj; >> + struct acpi_buffer result = { ACPI_ALLOCATE_BUFFER, NULL }; >> + int ret; >> + >> + ret = gigabyte_wmi_perform_query(wdev, command, args, &result); >> + if (ret) >> + return ret; >> + obj = result.pointer; >> + if (obj && obj->type == ACPI_TYPE_INTEGER) >> + *res = obj->integer.value; >> + else >> + ret = -EIO; >> + kfree(result.pointer); >> + return ret; >> +} >> + >> +static int gigabyte_wmi_temperature(struct wmi_device *wdev, u8 sensor, long *res) >> +{ >> + struct gigabyte_wmi_args args = { >> + .arg1 = sensor, >> + }; >> + u64 temp; >> + acpi_status ret; >> + >> + ret = gigabyte_wmi_query_integer(wdev, GIGABYTE_WMI_TEMPERATURE_QUERY, &args, &temp); >> + if (ret == 0) { >> + if (temp == 0) >> + return -ENODEV; > > That should be checked in gigabyte_wmi_hwmon_is_visible(); that is what that > function is for. Good point, actually the way I think this should be done is cache the result of the initial probe done in gigabyte_wmi_validate_sensor_presence() and use that in is_visible to return either 0 (not visible) or 0444, this way you can also hide sensors when there is a whole in the range of sensors somehow. So I guess we do need a v4 of this patch after all. > >> + *res = (s8)temp * 1000; // value is a signed 8-bit integer >> + } >> + return ret; >> +} >> + >> +static int gigabyte_wmi_hwmon_read(struct device *dev, enum hwmon_sensor_types type, >> + u32 attr, int channel, long *val) >> +{ >> + struct wmi_device *wdev = dev_get_drvdata(dev); >> + >> + return gigabyte_wmi_temperature(wdev, channel, val); >> +} >> + >> +static umode_t gigabyte_wmi_hwmon_is_visible(const void *data, enum hwmon_sensor_types type, >> + u32 attr, int channel) >> +{ >> + return 0444; >> +} >> + >> +static const struct hwmon_channel_info *gigabyte_wmi_hwmon_info[] = { >> + HWMON_CHANNEL_INFO(temp, >> + HWMON_T_INPUT, >> + HWMON_T_INPUT, >> + HWMON_T_INPUT, >> + HWMON_T_INPUT, >> + HWMON_T_INPUT, >> + HWMON_T_INPUT), >> + NULL >> +}; >> + >> +static const struct hwmon_ops gigabyte_wmi_hwmon_ops = { >> + .read = gigabyte_wmi_hwmon_read, >> + .is_visible = gigabyte_wmi_hwmon_is_visible, >> +}; >> + >> +static const struct hwmon_chip_info gigabyte_wmi_hwmon_chip_info = { >> + .ops = &gigabyte_wmi_hwmon_ops, >> + .info = gigabyte_wmi_hwmon_info, >> +}; >> + >> +static int gigabyte_wmi_validate_sensor_presence(struct wmi_device *wdev) >> +{ >> + int working_sensors = 0, i; >> + long temp; >> + >> + for (i = 0; i < NUM_TEMPERATURE_SENSORS; i++) { >> + if (!gigabyte_wmi_temperature(wdev, i, &temp)) >> + working_sensors++; >> + } >> + return working_sensors ? 0 : -ENODEV; >> +} >> + >> +static const struct dmi_system_id gigabyte_wmi_known_working_platforms[] = { >> + { .matches = { >> + DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "Gigabyte Technology Co., Ltd."), >> + DMI_EXACT_MATCH(DMI_BOARD_NAME, "B550 GAMING X V2"), >> + }}, >> + { .matches = { >> + DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "Gigabyte Technology Co., Ltd."), >> + DMI_EXACT_MATCH(DMI_BOARD_NAME, "B550M DS3H"), >> + }}, >> + { .matches = { >> + DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "Gigabyte Technology Co., Ltd."), >> + DMI_EXACT_MATCH(DMI_BOARD_NAME, "Z390 I AORUS PRO WIFI-CF"), >> + }}, >> + { .matches = { >> + DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "Gigabyte Technology Co., Ltd."), >> + DMI_EXACT_MATCH(DMI_BOARD_NAME, "X570 I AORUS PRO WIFI"), >> + }}, >> + { } >> +}; >> + >> +static int gigabyte_wmi_probe(struct wmi_device *wdev, const void *context) >> +{ >> + struct device *hwmon_dev; >> + int ret; >> + >> + if (!dmi_check_system(gigabyte_wmi_known_working_platforms)) { >> + if (force_load) >> + dev_warn(&wdev->dev, "Forcing loading on non-whitelisted platform"); >> + else >> + return -ENODEV; > > Style: > if (!force_load) > return -ENODEV; > dev_warn(&wdev->dev, "Forcing loading on non-whitelisted platform"); Ack, that would be better. > >> + } >> + >> + ret = gigabyte_wmi_validate_sensor_presence(wdev); >> + if (ret) { >> + dev_info(&wdev->dev, "No temperature sensors usable"); > > Normally one does not display a message if a probe function returns -ENODEV, > unless it is an error, to avoid polluting the kernel log. This will normally only be shown when the force_load module parameter is used, at which point I think it makes sense to explain why the driver is still not loading. Regards, Hans > >> + return ret; >> + } >> + >> + hwmon_dev = devm_hwmon_device_register_with_info(&wdev->dev, "gigabyte_wmi", wdev, >> + &gigabyte_wmi_hwmon_chip_info, NULL); >> + >> + return PTR_ERR_OR_ZERO(hwmon_dev); >> +} >> + >> +static const struct wmi_device_id gigabyte_wmi_id_table[] = { >> + { GIGABYTE_WMI_GUID, NULL }, >> + { } >> +}; >> + >> +static struct wmi_driver gigabyte_wmi_driver = { >> + .driver = { >> + .name = "gigabyte-wmi", >> + }, >> + .id_table = gigabyte_wmi_id_table, >> + .probe = gigabyte_wmi_probe, >> +}; >> +module_wmi_driver(gigabyte_wmi_driver); >> + >> +MODULE_DEVICE_TABLE(wmi, gigabyte_wmi_id_table); >> +MODULE_AUTHOR("Thomas Weißschuh <thomas@weissschuh.net>"); >> +MODULE_DESCRIPTION("Gigabyte WMI temperature Driver"); >> +MODULE_LICENSE("GPL"); >> >> base-commit: 144c79ef33536b4ecb4951e07dbc1f2b7fa99d32 >> > ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v4] platform/x86: add Gigabyte WMI temperature driver 2021-04-10 15:23 ` Hans de Goede @ 2021-04-10 18:18 ` Thomas Weißschuh 2021-04-11 0:58 ` Guenter Roeck ` (2 more replies) 0 siblings, 3 replies; 10+ messages in thread From: Thomas Weißschuh @ 2021-04-10 18:18 UTC (permalink / raw) To: platform-driver-x86, Mark Gross, Hans de Goede, linux-kernel, Barnabás Pőcze, Jean Delvare, Guenter Roeck, linux-hwmon Cc: Thomas Weißschuh, Matthew Garrett Changes since v3: * Completely hide unusable sensors * Expose force_load parameter read-only via sysfs * Naming * Style cleanups -- >8 -- Tested with * X570 I Aorus Pro Wifi (rev 1.0) * B550M DS3H * B550 Gaming X V2 (rev.1.x) * Z390 I AORUS PRO WIFI (rev. 1.0) The mainboard contains an ITE IT8688E chip for management. This chips is also handled by drivers/hwmon/i87.c but as it is also used by the firmware itself it needs an ACPI driver. Unfortunately not all sensor registers are handled by the firmware and even less are exposed via WMI. Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> --- MAINTAINERS | 6 + drivers/platform/x86/Kconfig | 11 ++ drivers/platform/x86/Makefile | 1 + drivers/platform/x86/gigabyte-wmi.c | 195 ++++++++++++++++++++++++++++ 4 files changed, 213 insertions(+) create mode 100644 drivers/platform/x86/gigabyte-wmi.c diff --git a/MAINTAINERS b/MAINTAINERS index d92f85ca831d..9c10cfc00fe8 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -7543,6 +7543,12 @@ F: Documentation/filesystems/gfs2* F: fs/gfs2/ F: include/uapi/linux/gfs2_ondisk.h +GIGABYTE WMI DRIVER +M: Thomas Weißschuh <linux@weissschuh.net> +L: platform-driver-x86@vger.kernel.org +S: Maintained +F: drivers/platform/x86/gigabyte-wmi.c + GNSS SUBSYSTEM M: Johan Hovold <johan@kernel.org> S: Maintained diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig index ad4e630e73e2..96622a2106f7 100644 --- a/drivers/platform/x86/Kconfig +++ b/drivers/platform/x86/Kconfig @@ -123,6 +123,17 @@ config XIAOMI_WMI To compile this driver as a module, choose M here: the module will be called xiaomi-wmi. +config GIGABYTE_WMI + tristate "Gigabyte WMI temperature driver" + depends on ACPI_WMI + depends on HWMON + help + Say Y here if you want to support WMI-based temperature reporting on + Gigabyte mainboards. + + To compile this driver as a module, choose M here: the module will + be called gigabyte-wmi. + config ACERHDF tristate "Acer Aspire One temperature and fan driver" depends on ACPI && THERMAL diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile index 60d554073749..1621ebfd04fd 100644 --- a/drivers/platform/x86/Makefile +++ b/drivers/platform/x86/Makefile @@ -15,6 +15,7 @@ obj-$(CONFIG_INTEL_WMI_THUNDERBOLT) += intel-wmi-thunderbolt.o obj-$(CONFIG_MXM_WMI) += mxm-wmi.o obj-$(CONFIG_PEAQ_WMI) += peaq-wmi.o obj-$(CONFIG_XIAOMI_WMI) += xiaomi-wmi.o +obj-$(CONFIG_GIGABYTE_WMI) += gigabyte-wmi.o # Acer obj-$(CONFIG_ACERHDF) += acerhdf.o diff --git a/drivers/platform/x86/gigabyte-wmi.c b/drivers/platform/x86/gigabyte-wmi.c new file mode 100644 index 000000000000..c17e51fcf000 --- /dev/null +++ b/drivers/platform/x86/gigabyte-wmi.c @@ -0,0 +1,195 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (C) 2021 Thomas Weißschuh <thomas@weissschuh.net> + */ +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + +#include <linux/acpi.h> +#include <linux/dmi.h> +#include <linux/hwmon.h> +#include <linux/module.h> +#include <linux/wmi.h> + +#define GIGABYTE_WMI_GUID "DEADBEEF-2001-0000-00A0-C90629100000" +#define NUM_TEMPERATURE_SENSORS 6 + +static bool force_load; +module_param(force_load, bool, 0444); +MODULE_PARM_DESC(force_load, "Force loading on unknown platform"); + +static u8 usable_sensors_mask; + +enum gigabyte_wmi_commandtype { + GIGABYTE_WMI_BUILD_DATE_QUERY = 0x1, + GIGABYTE_WMI_MAINBOARD_TYPE_QUERY = 0x2, + GIGABYTE_WMI_FIRMWARE_VERSION_QUERY = 0x4, + GIGABYTE_WMI_MAINBOARD_NAME_QUERY = 0x5, + GIGABYTE_WMI_TEMPERATURE_QUERY = 0x125, +}; + +struct gigabyte_wmi_args { + u32 arg1; +}; + +static int gigabyte_wmi_perform_query(struct wmi_device *wdev, + enum gigabyte_wmi_commandtype command, + struct gigabyte_wmi_args *args, struct acpi_buffer *out) +{ + const struct acpi_buffer in = { + .length = sizeof(*args), + .pointer = args, + }; + + acpi_status ret = wmidev_evaluate_method(wdev, 0x0, command, &in, out); + + if ACPI_FAILURE(ret) + return -EIO; + + return 0; +} + +static int gigabyte_wmi_query_integer(struct wmi_device *wdev, + enum gigabyte_wmi_commandtype command, + struct gigabyte_wmi_args *args, u64 *res) +{ + union acpi_object *obj; + struct acpi_buffer result = { ACPI_ALLOCATE_BUFFER, NULL }; + int ret; + + ret = gigabyte_wmi_perform_query(wdev, command, args, &result); + if (ret) + return ret; + obj = result.pointer; + if (obj && obj->type == ACPI_TYPE_INTEGER) + *res = obj->integer.value; + else + ret = -EIO; + kfree(result.pointer); + return ret; +} + +static int gigabyte_wmi_temperature(struct wmi_device *wdev, u8 sensor, long *res) +{ + struct gigabyte_wmi_args args = { + .arg1 = sensor, + }; + u64 temp; + acpi_status ret; + + ret = gigabyte_wmi_query_integer(wdev, GIGABYTE_WMI_TEMPERATURE_QUERY, &args, &temp); + if (ret == 0) { + if (temp == 0) + return -ENODEV; + *res = (s8)temp * 1000; // value is a signed 8-bit integer + } + return ret; +} + +static int gigabyte_wmi_hwmon_read(struct device *dev, enum hwmon_sensor_types type, + u32 attr, int channel, long *val) +{ + struct wmi_device *wdev = dev_get_drvdata(dev); + + return gigabyte_wmi_temperature(wdev, channel, val); +} + +static umode_t gigabyte_wmi_hwmon_is_visible(const void *data, enum hwmon_sensor_types type, + u32 attr, int channel) +{ + return usable_sensors_mask & BIT(channel) ? 0444 : 0; +} + +static const struct hwmon_channel_info *gigabyte_wmi_hwmon_info[] = { + HWMON_CHANNEL_INFO(temp, + HWMON_T_INPUT, + HWMON_T_INPUT, + HWMON_T_INPUT, + HWMON_T_INPUT, + HWMON_T_INPUT, + HWMON_T_INPUT), + NULL +}; + +static const struct hwmon_ops gigabyte_wmi_hwmon_ops = { + .read = gigabyte_wmi_hwmon_read, + .is_visible = gigabyte_wmi_hwmon_is_visible, +}; + +static const struct hwmon_chip_info gigabyte_wmi_hwmon_chip_info = { + .ops = &gigabyte_wmi_hwmon_ops, + .info = gigabyte_wmi_hwmon_info, +}; + +static u8 gigabyte_wmi_detect_sensor_usability(struct wmi_device *wdev) +{ + int i; + long temp; + u8 r = 0; + + for (i = 0; i < NUM_TEMPERATURE_SENSORS; i++) { + if (!gigabyte_wmi_temperature(wdev, i, &temp)) + r |= BIT(i); + } + return r; +} + +static const struct dmi_system_id gigabyte_wmi_known_working_platforms[] = { + { .matches = { + DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "Gigabyte Technology Co., Ltd."), + DMI_EXACT_MATCH(DMI_BOARD_NAME, "B550 GAMING X V2"), + }}, + { .matches = { + DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "Gigabyte Technology Co., Ltd."), + DMI_EXACT_MATCH(DMI_BOARD_NAME, "B550M DS3H"), + }}, + { .matches = { + DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "Gigabyte Technology Co., Ltd."), + DMI_EXACT_MATCH(DMI_BOARD_NAME, "Z390 I AORUS PRO WIFI-CF"), + }}, + { .matches = { + DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "Gigabyte Technology Co., Ltd."), + DMI_EXACT_MATCH(DMI_BOARD_NAME, "X570 I AORUS PRO WIFI"), + }}, + { } +}; + +static int gigabyte_wmi_probe(struct wmi_device *wdev, const void *context) +{ + struct device *hwmon_dev; + + if (!dmi_check_system(gigabyte_wmi_known_working_platforms)) { + if (!force_load) + return -ENODEV; + dev_warn(&wdev->dev, "Forcing load on unknown platform"); + } + + usable_sensors_mask = gigabyte_wmi_detect_sensor_usability(wdev); + if (!usable_sensors_mask) { + dev_info(&wdev->dev, "No temperature sensors usable"); + return -ENODEV; + } + + hwmon_dev = devm_hwmon_device_register_with_info(&wdev->dev, "gigabyte_wmi", wdev, + &gigabyte_wmi_hwmon_chip_info, NULL); + + return PTR_ERR_OR_ZERO(hwmon_dev); +} + +static const struct wmi_device_id gigabyte_wmi_id_table[] = { + { GIGABYTE_WMI_GUID, NULL }, + { } +}; + +static struct wmi_driver gigabyte_wmi_driver = { + .driver = { + .name = "gigabyte-wmi", + }, + .id_table = gigabyte_wmi_id_table, + .probe = gigabyte_wmi_probe, +}; +module_wmi_driver(gigabyte_wmi_driver); + +MODULE_DEVICE_TABLE(wmi, gigabyte_wmi_id_table); +MODULE_AUTHOR("Thomas Weißschuh <thomas@weissschuh.net>"); +MODULE_DESCRIPTION("Gigabyte WMI temperature Driver"); +MODULE_LICENSE("GPL"); base-commit: 144c79ef33536b4ecb4951e07dbc1f2b7fa99d32 -- 2.31.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v4] platform/x86: add Gigabyte WMI temperature driver 2021-04-10 18:18 ` [PATCH v4] " Thomas Weißschuh @ 2021-04-11 0:58 ` Guenter Roeck 2021-04-11 14:05 ` Barnabás Pőcze 2021-04-12 12:35 ` [PATCH v5] " Thomas Weißschuh 2 siblings, 0 replies; 10+ messages in thread From: Guenter Roeck @ 2021-04-11 0:58 UTC (permalink / raw) To: Thomas Weißschuh, platform-driver-x86, Mark Gross, Hans de Goede, linux-kernel, Barnabás Pőcze, Jean Delvare, linux-hwmon Cc: Matthew Garrett On 4/10/21 11:18 AM, Thomas Weißschuh wrote: > Changes since v3: > * Completely hide unusable sensors > * Expose force_load parameter read-only via sysfs > * Naming > * Style cleanups > > -- >8 -- > > Tested with > * X570 I Aorus Pro Wifi (rev 1.0) > * B550M DS3H > * B550 Gaming X V2 (rev.1.x) > * Z390 I AORUS PRO WIFI (rev. 1.0) > > The mainboard contains an ITE IT8688E chip for management. > This chips is also handled by drivers/hwmon/i87.c but as it is also used > by the firmware itself it needs an ACPI driver. > > Unfortunately not all sensor registers are handled by the firmware and even > less are exposed via WMI. > > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> Good enough, though you may want to improve your description. Reviewed-by: Guenter Roeck <linux@roeck-us.net> FWIW, on B450 AORUS M: gigabyte-wmi DEADBEEF-2001-0000-00A0-C90629100000: Forcing load on unknown platform gigabyte-wmi DEADBEEF-2001-0000-00A0-C90629100000: No temperature sensors usable Wonder who came up with that GUID. > --- Change log and everything that is not supposed to show up in the git history is supposed to go here. Guenter > MAINTAINERS | 6 + > drivers/platform/x86/Kconfig | 11 ++ > drivers/platform/x86/Makefile | 1 + > drivers/platform/x86/gigabyte-wmi.c | 195 ++++++++++++++++++++++++++++ > 4 files changed, 213 insertions(+) > create mode 100644 drivers/platform/x86/gigabyte-wmi.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index d92f85ca831d..9c10cfc00fe8 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -7543,6 +7543,12 @@ F: Documentation/filesystems/gfs2* > F: fs/gfs2/ > F: include/uapi/linux/gfs2_ondisk.h > > +GIGABYTE WMI DRIVER > +M: Thomas Weißschuh <linux@weissschuh.net> > +L: platform-driver-x86@vger.kernel.org > +S: Maintained > +F: drivers/platform/x86/gigabyte-wmi.c > + > GNSS SUBSYSTEM > M: Johan Hovold <johan@kernel.org> > S: Maintained > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig > index ad4e630e73e2..96622a2106f7 100644 > --- a/drivers/platform/x86/Kconfig > +++ b/drivers/platform/x86/Kconfig > @@ -123,6 +123,17 @@ config XIAOMI_WMI > To compile this driver as a module, choose M here: the module will > be called xiaomi-wmi. > > +config GIGABYTE_WMI > + tristate "Gigabyte WMI temperature driver" > + depends on ACPI_WMI > + depends on HWMON > + help > + Say Y here if you want to support WMI-based temperature reporting on > + Gigabyte mainboards. > + > + To compile this driver as a module, choose M here: the module will > + be called gigabyte-wmi. > + > config ACERHDF > tristate "Acer Aspire One temperature and fan driver" > depends on ACPI && THERMAL > diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile > index 60d554073749..1621ebfd04fd 100644 > --- a/drivers/platform/x86/Makefile > +++ b/drivers/platform/x86/Makefile > @@ -15,6 +15,7 @@ obj-$(CONFIG_INTEL_WMI_THUNDERBOLT) += intel-wmi-thunderbolt.o > obj-$(CONFIG_MXM_WMI) += mxm-wmi.o > obj-$(CONFIG_PEAQ_WMI) += peaq-wmi.o > obj-$(CONFIG_XIAOMI_WMI) += xiaomi-wmi.o > +obj-$(CONFIG_GIGABYTE_WMI) += gigabyte-wmi.o > > # Acer > obj-$(CONFIG_ACERHDF) += acerhdf.o > diff --git a/drivers/platform/x86/gigabyte-wmi.c b/drivers/platform/x86/gigabyte-wmi.c > new file mode 100644 > index 000000000000..c17e51fcf000 > --- /dev/null > +++ b/drivers/platform/x86/gigabyte-wmi.c > @@ -0,0 +1,195 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Copyright (C) 2021 Thomas Weißschuh <thomas@weissschuh.net> > + */ > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > + > +#include <linux/acpi.h> > +#include <linux/dmi.h> > +#include <linux/hwmon.h> > +#include <linux/module.h> > +#include <linux/wmi.h> > + > +#define GIGABYTE_WMI_GUID "DEADBEEF-2001-0000-00A0-C90629100000" > +#define NUM_TEMPERATURE_SENSORS 6 > + > +static bool force_load; > +module_param(force_load, bool, 0444); > +MODULE_PARM_DESC(force_load, "Force loading on unknown platform"); > + > +static u8 usable_sensors_mask; > + > +enum gigabyte_wmi_commandtype { > + GIGABYTE_WMI_BUILD_DATE_QUERY = 0x1, > + GIGABYTE_WMI_MAINBOARD_TYPE_QUERY = 0x2, > + GIGABYTE_WMI_FIRMWARE_VERSION_QUERY = 0x4, > + GIGABYTE_WMI_MAINBOARD_NAME_QUERY = 0x5, > + GIGABYTE_WMI_TEMPERATURE_QUERY = 0x125, > +}; > + > +struct gigabyte_wmi_args { > + u32 arg1; > +}; > + > +static int gigabyte_wmi_perform_query(struct wmi_device *wdev, > + enum gigabyte_wmi_commandtype command, > + struct gigabyte_wmi_args *args, struct acpi_buffer *out) > +{ > + const struct acpi_buffer in = { > + .length = sizeof(*args), > + .pointer = args, > + }; > + > + acpi_status ret = wmidev_evaluate_method(wdev, 0x0, command, &in, out); > + > + if ACPI_FAILURE(ret) > + return -EIO; > + > + return 0; > +} > + > +static int gigabyte_wmi_query_integer(struct wmi_device *wdev, > + enum gigabyte_wmi_commandtype command, > + struct gigabyte_wmi_args *args, u64 *res) > +{ > + union acpi_object *obj; > + struct acpi_buffer result = { ACPI_ALLOCATE_BUFFER, NULL }; > + int ret; > + > + ret = gigabyte_wmi_perform_query(wdev, command, args, &result); > + if (ret) > + return ret; > + obj = result.pointer; > + if (obj && obj->type == ACPI_TYPE_INTEGER) > + *res = obj->integer.value; > + else > + ret = -EIO; > + kfree(result.pointer); > + return ret; > +} > + > +static int gigabyte_wmi_temperature(struct wmi_device *wdev, u8 sensor, long *res) > +{ > + struct gigabyte_wmi_args args = { > + .arg1 = sensor, > + }; > + u64 temp; > + acpi_status ret; > + > + ret = gigabyte_wmi_query_integer(wdev, GIGABYTE_WMI_TEMPERATURE_QUERY, &args, &temp); > + if (ret == 0) { > + if (temp == 0) > + return -ENODEV; > + *res = (s8)temp * 1000; // value is a signed 8-bit integer > + } > + return ret; > +} > + > +static int gigabyte_wmi_hwmon_read(struct device *dev, enum hwmon_sensor_types type, > + u32 attr, int channel, long *val) > +{ > + struct wmi_device *wdev = dev_get_drvdata(dev); > + > + return gigabyte_wmi_temperature(wdev, channel, val); > +} > + > +static umode_t gigabyte_wmi_hwmon_is_visible(const void *data, enum hwmon_sensor_types type, > + u32 attr, int channel) > +{ > + return usable_sensors_mask & BIT(channel) ? 0444 : 0; > +} > + > +static const struct hwmon_channel_info *gigabyte_wmi_hwmon_info[] = { > + HWMON_CHANNEL_INFO(temp, > + HWMON_T_INPUT, > + HWMON_T_INPUT, > + HWMON_T_INPUT, > + HWMON_T_INPUT, > + HWMON_T_INPUT, > + HWMON_T_INPUT), > + NULL > +}; > + > +static const struct hwmon_ops gigabyte_wmi_hwmon_ops = { > + .read = gigabyte_wmi_hwmon_read, > + .is_visible = gigabyte_wmi_hwmon_is_visible, > +}; > + > +static const struct hwmon_chip_info gigabyte_wmi_hwmon_chip_info = { > + .ops = &gigabyte_wmi_hwmon_ops, > + .info = gigabyte_wmi_hwmon_info, > +}; > + > +static u8 gigabyte_wmi_detect_sensor_usability(struct wmi_device *wdev) > +{ > + int i; > + long temp; > + u8 r = 0; > + > + for (i = 0; i < NUM_TEMPERATURE_SENSORS; i++) { > + if (!gigabyte_wmi_temperature(wdev, i, &temp)) > + r |= BIT(i); > + } > + return r; > +} > + > +static const struct dmi_system_id gigabyte_wmi_known_working_platforms[] = { > + { .matches = { > + DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "Gigabyte Technology Co., Ltd."), > + DMI_EXACT_MATCH(DMI_BOARD_NAME, "B550 GAMING X V2"), > + }}, > + { .matches = { > + DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "Gigabyte Technology Co., Ltd."), > + DMI_EXACT_MATCH(DMI_BOARD_NAME, "B550M DS3H"), > + }}, > + { .matches = { > + DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "Gigabyte Technology Co., Ltd."), > + DMI_EXACT_MATCH(DMI_BOARD_NAME, "Z390 I AORUS PRO WIFI-CF"), > + }}, > + { .matches = { > + DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "Gigabyte Technology Co., Ltd."), > + DMI_EXACT_MATCH(DMI_BOARD_NAME, "X570 I AORUS PRO WIFI"), > + }}, > + { } > +}; > + > +static int gigabyte_wmi_probe(struct wmi_device *wdev, const void *context) > +{ > + struct device *hwmon_dev; > + > + if (!dmi_check_system(gigabyte_wmi_known_working_platforms)) { > + if (!force_load) > + return -ENODEV; > + dev_warn(&wdev->dev, "Forcing load on unknown platform"); > + } > + > + usable_sensors_mask = gigabyte_wmi_detect_sensor_usability(wdev); > + if (!usable_sensors_mask) { > + dev_info(&wdev->dev, "No temperature sensors usable"); > + return -ENODEV; > + } > + > + hwmon_dev = devm_hwmon_device_register_with_info(&wdev->dev, "gigabyte_wmi", wdev, > + &gigabyte_wmi_hwmon_chip_info, NULL); > + > + return PTR_ERR_OR_ZERO(hwmon_dev); > +} > + > +static const struct wmi_device_id gigabyte_wmi_id_table[] = { > + { GIGABYTE_WMI_GUID, NULL }, > + { } > +}; > + > +static struct wmi_driver gigabyte_wmi_driver = { > + .driver = { > + .name = "gigabyte-wmi", > + }, > + .id_table = gigabyte_wmi_id_table, > + .probe = gigabyte_wmi_probe, > +}; > +module_wmi_driver(gigabyte_wmi_driver); > + > +MODULE_DEVICE_TABLE(wmi, gigabyte_wmi_id_table); > +MODULE_AUTHOR("Thomas Weißschuh <thomas@weissschuh.net>"); > +MODULE_DESCRIPTION("Gigabyte WMI temperature Driver"); > +MODULE_LICENSE("GPL"); > > base-commit: 144c79ef33536b4ecb4951e07dbc1f2b7fa99d32 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4] platform/x86: add Gigabyte WMI temperature driver 2021-04-10 18:18 ` [PATCH v4] " Thomas Weißschuh 2021-04-11 0:58 ` Guenter Roeck @ 2021-04-11 14:05 ` Barnabás Pőcze 2021-04-12 12:35 ` [PATCH v5] " Thomas Weißschuh 2 siblings, 0 replies; 10+ messages in thread From: Barnabás Pőcze @ 2021-04-11 14:05 UTC (permalink / raw) To: Thomas Weißschuh Cc: platform-driver-x86, Mark Gross, Hans de Goede, linux-kernel, Jean Delvare, Guenter Roeck, linux-hwmon, Matthew Garrett Hi 2021. április 10., szombat 20:18 keltezéssel, Thomas Weißschuh írta: > [...] > diff --git a/drivers/platform/x86/gigabyte-wmi.c b/drivers/platform/x86/gigabyte-wmi.c > new file mode 100644 > index 000000000000..c17e51fcf000 > --- /dev/null > +++ b/drivers/platform/x86/gigabyte-wmi.c > @@ -0,0 +1,195 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Copyright (C) 2021 Thomas Weißschuh <thomas@weissschuh.net> > + */ > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > + > +#include <linux/acpi.h> > +#include <linux/dmi.h> > +#include <linux/hwmon.h> > +#include <linux/module.h> > +#include <linux/wmi.h> > + > +#define GIGABYTE_WMI_GUID "DEADBEEF-2001-0000-00A0-C90629100000" > +#define NUM_TEMPERATURE_SENSORS 6 > + > +static bool force_load; > +module_param(force_load, bool, 0444); > +MODULE_PARM_DESC(force_load, "Force loading on unknown platform"); > + > +static u8 usable_sensors_mask; > + > +enum gigabyte_wmi_commandtype { > + GIGABYTE_WMI_BUILD_DATE_QUERY = 0x1, > + GIGABYTE_WMI_MAINBOARD_TYPE_QUERY = 0x2, > + GIGABYTE_WMI_FIRMWARE_VERSION_QUERY = 0x4, > + GIGABYTE_WMI_MAINBOARD_NAME_QUERY = 0x5, > + GIGABYTE_WMI_TEMPERATURE_QUERY = 0x125, > +}; > + > +struct gigabyte_wmi_args { > + u32 arg1; > +}; > + > +static int gigabyte_wmi_perform_query(struct wmi_device *wdev, > + enum gigabyte_wmi_commandtype command, > + struct gigabyte_wmi_args *args, struct acpi_buffer *out) > +{ > + const struct acpi_buffer in = { > + .length = sizeof(*args), > + .pointer = args, > + }; > + > + acpi_status ret = wmidev_evaluate_method(wdev, 0x0, command, &in, out); > + > + if ACPI_FAILURE(ret) Please use `if (...)`. > + return -EIO; > + > + return 0; > +} > [...] > +static struct wmi_driver gigabyte_wmi_driver = { > + .driver = { > + .name = "gigabyte-wmi", > + }, > + .id_table = gigabyte_wmi_id_table, > + .probe = gigabyte_wmi_probe, > +}; > +module_wmi_driver(gigabyte_wmi_driver); > + > +MODULE_DEVICE_TABLE(wmi, gigabyte_wmi_id_table); > +MODULE_AUTHOR("Thomas Weißschuh <thomas@weissschuh.net>"); > +MODULE_DESCRIPTION("Gigabyte WMI temperature Driver"); ^ It's a minor thing, but I think a lowercase 'd' would be better. > +MODULE_LICENSE("GPL"); > > base-commit: 144c79ef33536b4ecb4951e07dbc1f2b7fa99d32 > -- > 2.31.1 Regards, Barnabás Pőcze ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v5] platform/x86: add Gigabyte WMI temperature driver 2021-04-10 18:18 ` [PATCH v4] " Thomas Weißschuh 2021-04-11 0:58 ` Guenter Roeck 2021-04-11 14:05 ` Barnabás Pőcze @ 2021-04-12 12:35 ` Thomas Weißschuh 2021-04-13 8:14 ` Hans de Goede 2 siblings, 1 reply; 10+ messages in thread From: Thomas Weißschuh @ 2021-04-12 12:35 UTC (permalink / raw) To: platform-driver-x86, Mark Gross, Hans de Goede, linux-kernel, Barnabás Pőcze, Jean Delvare, Guenter Roeck, linux-hwmon Cc: Thomas Weißschuh, Matthew Garrett Tested with * X570 I Aorus Pro Wifi (rev 1.0) * B550M DS3H * B550 Gaming X V2 (rev.1.x) * Z390 I AORUS PRO WIFI (rev. 1.0) Those mainboards contain an ITE chips for management and monitoring. They could also be handled by drivers/hwmon/i87.c. But the SuperIO range used by i87 is already claimed and used by the firmware. The following warning is printed at boot: kernel: ACPI Warning: SystemIO range 0x0000000000000A45-0x0000000000000A46 conflicts with OpRegion 0x0000000000000A45-0x0000000000000A46 (\GSA1.SIO1) (20200528/utaddress-204) kernel: ACPI: This conflict may cause random problems and system instability kernel: ACPI: If an ACPI driver is available for this device, you should use it instead of the native driver This driver implements such an ACPI driver. Unfortunately not all sensor registers are handled by the firmware and even less are exposed via WMI. Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> Reviewed-by: Guenter Roeck <linux@roeck-us.net> --- Changes since v4: * Style * Wording * Alignment of email addresses --- MAINTAINERS | 6 + drivers/platform/x86/Kconfig | 11 ++ drivers/platform/x86/Makefile | 1 + drivers/platform/x86/gigabyte-wmi.c | 195 ++++++++++++++++++++++++++++ 4 files changed, 213 insertions(+) create mode 100644 drivers/platform/x86/gigabyte-wmi.c diff --git a/MAINTAINERS b/MAINTAINERS index d92f85ca831d..7fb5e2ba489b 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -7543,6 +7543,12 @@ F: Documentation/filesystems/gfs2* F: fs/gfs2/ F: include/uapi/linux/gfs2_ondisk.h +GIGABYTE WMI DRIVER +M: Thomas Weißschuh <thomas@weissschuh.net> +L: platform-driver-x86@vger.kernel.org +S: Maintained +F: drivers/platform/x86/gigabyte-wmi.c + GNSS SUBSYSTEM M: Johan Hovold <johan@kernel.org> S: Maintained diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig index ad4e630e73e2..96622a2106f7 100644 --- a/drivers/platform/x86/Kconfig +++ b/drivers/platform/x86/Kconfig @@ -123,6 +123,17 @@ config XIAOMI_WMI To compile this driver as a module, choose M here: the module will be called xiaomi-wmi. +config GIGABYTE_WMI + tristate "Gigabyte WMI temperature driver" + depends on ACPI_WMI + depends on HWMON + help + Say Y here if you want to support WMI-based temperature reporting on + Gigabyte mainboards. + + To compile this driver as a module, choose M here: the module will + be called gigabyte-wmi. + config ACERHDF tristate "Acer Aspire One temperature and fan driver" depends on ACPI && THERMAL diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile index 60d554073749..1621ebfd04fd 100644 --- a/drivers/platform/x86/Makefile +++ b/drivers/platform/x86/Makefile @@ -15,6 +15,7 @@ obj-$(CONFIG_INTEL_WMI_THUNDERBOLT) += intel-wmi-thunderbolt.o obj-$(CONFIG_MXM_WMI) += mxm-wmi.o obj-$(CONFIG_PEAQ_WMI) += peaq-wmi.o obj-$(CONFIG_XIAOMI_WMI) += xiaomi-wmi.o +obj-$(CONFIG_GIGABYTE_WMI) += gigabyte-wmi.o # Acer obj-$(CONFIG_ACERHDF) += acerhdf.o diff --git a/drivers/platform/x86/gigabyte-wmi.c b/drivers/platform/x86/gigabyte-wmi.c new file mode 100644 index 000000000000..bb1b0b205fa7 --- /dev/null +++ b/drivers/platform/x86/gigabyte-wmi.c @@ -0,0 +1,195 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (C) 2021 Thomas Weißschuh <thomas@weissschuh.net> + */ +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + +#include <linux/acpi.h> +#include <linux/dmi.h> +#include <linux/hwmon.h> +#include <linux/module.h> +#include <linux/wmi.h> + +#define GIGABYTE_WMI_GUID "DEADBEEF-2001-0000-00A0-C90629100000" +#define NUM_TEMPERATURE_SENSORS 6 + +static bool force_load; +module_param(force_load, bool, 0444); +MODULE_PARM_DESC(force_load, "Force loading on unknown platform"); + +static u8 usable_sensors_mask; + +enum gigabyte_wmi_commandtype { + GIGABYTE_WMI_BUILD_DATE_QUERY = 0x1, + GIGABYTE_WMI_MAINBOARD_TYPE_QUERY = 0x2, + GIGABYTE_WMI_FIRMWARE_VERSION_QUERY = 0x4, + GIGABYTE_WMI_MAINBOARD_NAME_QUERY = 0x5, + GIGABYTE_WMI_TEMPERATURE_QUERY = 0x125, +}; + +struct gigabyte_wmi_args { + u32 arg1; +}; + +static int gigabyte_wmi_perform_query(struct wmi_device *wdev, + enum gigabyte_wmi_commandtype command, + struct gigabyte_wmi_args *args, struct acpi_buffer *out) +{ + const struct acpi_buffer in = { + .length = sizeof(*args), + .pointer = args, + }; + + acpi_status ret = wmidev_evaluate_method(wdev, 0x0, command, &in, out); + + if (ACPI_FAILURE(ret)) + return -EIO; + + return 0; +} + +static int gigabyte_wmi_query_integer(struct wmi_device *wdev, + enum gigabyte_wmi_commandtype command, + struct gigabyte_wmi_args *args, u64 *res) +{ + union acpi_object *obj; + struct acpi_buffer result = { ACPI_ALLOCATE_BUFFER, NULL }; + int ret; + + ret = gigabyte_wmi_perform_query(wdev, command, args, &result); + if (ret) + return ret; + obj = result.pointer; + if (obj && obj->type == ACPI_TYPE_INTEGER) + *res = obj->integer.value; + else + ret = -EIO; + kfree(result.pointer); + return ret; +} + +static int gigabyte_wmi_temperature(struct wmi_device *wdev, u8 sensor, long *res) +{ + struct gigabyte_wmi_args args = { + .arg1 = sensor, + }; + u64 temp; + acpi_status ret; + + ret = gigabyte_wmi_query_integer(wdev, GIGABYTE_WMI_TEMPERATURE_QUERY, &args, &temp); + if (ret == 0) { + if (temp == 0) + return -ENODEV; + *res = (s8)temp * 1000; // value is a signed 8-bit integer + } + return ret; +} + +static int gigabyte_wmi_hwmon_read(struct device *dev, enum hwmon_sensor_types type, + u32 attr, int channel, long *val) +{ + struct wmi_device *wdev = dev_get_drvdata(dev); + + return gigabyte_wmi_temperature(wdev, channel, val); +} + +static umode_t gigabyte_wmi_hwmon_is_visible(const void *data, enum hwmon_sensor_types type, + u32 attr, int channel) +{ + return usable_sensors_mask & BIT(channel) ? 0444 : 0; +} + +static const struct hwmon_channel_info *gigabyte_wmi_hwmon_info[] = { + HWMON_CHANNEL_INFO(temp, + HWMON_T_INPUT, + HWMON_T_INPUT, + HWMON_T_INPUT, + HWMON_T_INPUT, + HWMON_T_INPUT, + HWMON_T_INPUT), + NULL +}; + +static const struct hwmon_ops gigabyte_wmi_hwmon_ops = { + .read = gigabyte_wmi_hwmon_read, + .is_visible = gigabyte_wmi_hwmon_is_visible, +}; + +static const struct hwmon_chip_info gigabyte_wmi_hwmon_chip_info = { + .ops = &gigabyte_wmi_hwmon_ops, + .info = gigabyte_wmi_hwmon_info, +}; + +static u8 gigabyte_wmi_detect_sensor_usability(struct wmi_device *wdev) +{ + int i; + long temp; + u8 r = 0; + + for (i = 0; i < NUM_TEMPERATURE_SENSORS; i++) { + if (!gigabyte_wmi_temperature(wdev, i, &temp)) + r |= BIT(i); + } + return r; +} + +static const struct dmi_system_id gigabyte_wmi_known_working_platforms[] = { + { .matches = { + DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "Gigabyte Technology Co., Ltd."), + DMI_EXACT_MATCH(DMI_BOARD_NAME, "B550 GAMING X V2"), + }}, + { .matches = { + DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "Gigabyte Technology Co., Ltd."), + DMI_EXACT_MATCH(DMI_BOARD_NAME, "B550M DS3H"), + }}, + { .matches = { + DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "Gigabyte Technology Co., Ltd."), + DMI_EXACT_MATCH(DMI_BOARD_NAME, "Z390 I AORUS PRO WIFI-CF"), + }}, + { .matches = { + DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "Gigabyte Technology Co., Ltd."), + DMI_EXACT_MATCH(DMI_BOARD_NAME, "X570 I AORUS PRO WIFI"), + }}, + { } +}; + +static int gigabyte_wmi_probe(struct wmi_device *wdev, const void *context) +{ + struct device *hwmon_dev; + + if (!dmi_check_system(gigabyte_wmi_known_working_platforms)) { + if (!force_load) + return -ENODEV; + dev_warn(&wdev->dev, "Forcing load on unknown platform"); + } + + usable_sensors_mask = gigabyte_wmi_detect_sensor_usability(wdev); + if (!usable_sensors_mask) { + dev_info(&wdev->dev, "No temperature sensors usable"); + return -ENODEV; + } + + hwmon_dev = devm_hwmon_device_register_with_info(&wdev->dev, "gigabyte_wmi", wdev, + &gigabyte_wmi_hwmon_chip_info, NULL); + + return PTR_ERR_OR_ZERO(hwmon_dev); +} + +static const struct wmi_device_id gigabyte_wmi_id_table[] = { + { GIGABYTE_WMI_GUID, NULL }, + { } +}; + +static struct wmi_driver gigabyte_wmi_driver = { + .driver = { + .name = "gigabyte-wmi", + }, + .id_table = gigabyte_wmi_id_table, + .probe = gigabyte_wmi_probe, +}; +module_wmi_driver(gigabyte_wmi_driver); + +MODULE_DEVICE_TABLE(wmi, gigabyte_wmi_id_table); +MODULE_AUTHOR("Thomas Weißschuh <thomas@weissschuh.net>"); +MODULE_DESCRIPTION("Gigabyte WMI temperature driver"); +MODULE_LICENSE("GPL"); base-commit: 144c79ef33536b4ecb4951e07dbc1f2b7fa99d32 -- 2.31.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v5] platform/x86: add Gigabyte WMI temperature driver 2021-04-12 12:35 ` [PATCH v5] " Thomas Weißschuh @ 2021-04-13 8:14 ` Hans de Goede 0 siblings, 0 replies; 10+ messages in thread From: Hans de Goede @ 2021-04-13 8:14 UTC (permalink / raw) To: Thomas Weißschuh, platform-driver-x86, Mark Gross, linux-kernel, Barnabás Pőcze, Jean Delvare, Guenter Roeck, linux-hwmon Cc: Matthew Garrett Hi, On 4/12/21 2:35 PM, Thomas Weißschuh wrote: > Tested with > * X570 I Aorus Pro Wifi (rev 1.0) > * B550M DS3H > * B550 Gaming X V2 (rev.1.x) > * Z390 I AORUS PRO WIFI (rev. 1.0) > > Those mainboards contain an ITE chips for management and > monitoring. > > They could also be handled by drivers/hwmon/i87.c. > But the SuperIO range used by i87 is already claimed and used by the > firmware. > > The following warning is printed at boot: > > kernel: ACPI Warning: SystemIO range 0x0000000000000A45-0x0000000000000A46 conflicts with OpRegion 0x0000000000000A45-0x0000000000000A46 (\GSA1.SIO1) (20200528/utaddress-204) > kernel: ACPI: This conflict may cause random problems and system instability > kernel: ACPI: If an ACPI driver is available for this device, you should use it instead of the native driver > > This driver implements such an ACPI driver. Thank you for your patch, I've applied this patch to my review-hans branch: https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans Note it will show up in my review-hans branch once I've pushed my local branch there, which might take a while. Once I've run some tests on this branch the patches there will be added to the platform-drivers-x86/for-next branch and eventually will be included in the pdx86 pull-request to Linus for the next merge-window. Regards, Hans > > Unfortunately not all sensor registers are handled by the firmware and even > less are exposed via WMI. > > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> > Reviewed-by: Guenter Roeck <linux@roeck-us.net> > > --- > > Changes since v4: > * Style > * Wording > * Alignment of email addresses > --- > MAINTAINERS | 6 + > drivers/platform/x86/Kconfig | 11 ++ > drivers/platform/x86/Makefile | 1 + > drivers/platform/x86/gigabyte-wmi.c | 195 ++++++++++++++++++++++++++++ > 4 files changed, 213 insertions(+) > create mode 100644 drivers/platform/x86/gigabyte-wmi.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index d92f85ca831d..7fb5e2ba489b 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -7543,6 +7543,12 @@ F: Documentation/filesystems/gfs2* > F: fs/gfs2/ > F: include/uapi/linux/gfs2_ondisk.h > > +GIGABYTE WMI DRIVER > +M: Thomas Weißschuh <thomas@weissschuh.net> > +L: platform-driver-x86@vger.kernel.org > +S: Maintained > +F: drivers/platform/x86/gigabyte-wmi.c > + > GNSS SUBSYSTEM > M: Johan Hovold <johan@kernel.org> > S: Maintained > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig > index ad4e630e73e2..96622a2106f7 100644 > --- a/drivers/platform/x86/Kconfig > +++ b/drivers/platform/x86/Kconfig > @@ -123,6 +123,17 @@ config XIAOMI_WMI > To compile this driver as a module, choose M here: the module will > be called xiaomi-wmi. > > +config GIGABYTE_WMI > + tristate "Gigabyte WMI temperature driver" > + depends on ACPI_WMI > + depends on HWMON > + help > + Say Y here if you want to support WMI-based temperature reporting on > + Gigabyte mainboards. > + > + To compile this driver as a module, choose M here: the module will > + be called gigabyte-wmi. > + > config ACERHDF > tristate "Acer Aspire One temperature and fan driver" > depends on ACPI && THERMAL > diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile > index 60d554073749..1621ebfd04fd 100644 > --- a/drivers/platform/x86/Makefile > +++ b/drivers/platform/x86/Makefile > @@ -15,6 +15,7 @@ obj-$(CONFIG_INTEL_WMI_THUNDERBOLT) += intel-wmi-thunderbolt.o > obj-$(CONFIG_MXM_WMI) += mxm-wmi.o > obj-$(CONFIG_PEAQ_WMI) += peaq-wmi.o > obj-$(CONFIG_XIAOMI_WMI) += xiaomi-wmi.o > +obj-$(CONFIG_GIGABYTE_WMI) += gigabyte-wmi.o > > # Acer > obj-$(CONFIG_ACERHDF) += acerhdf.o > diff --git a/drivers/platform/x86/gigabyte-wmi.c b/drivers/platform/x86/gigabyte-wmi.c > new file mode 100644 > index 000000000000..bb1b0b205fa7 > --- /dev/null > +++ b/drivers/platform/x86/gigabyte-wmi.c > @@ -0,0 +1,195 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Copyright (C) 2021 Thomas Weißschuh <thomas@weissschuh.net> > + */ > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > + > +#include <linux/acpi.h> > +#include <linux/dmi.h> > +#include <linux/hwmon.h> > +#include <linux/module.h> > +#include <linux/wmi.h> > + > +#define GIGABYTE_WMI_GUID "DEADBEEF-2001-0000-00A0-C90629100000" > +#define NUM_TEMPERATURE_SENSORS 6 > + > +static bool force_load; > +module_param(force_load, bool, 0444); > +MODULE_PARM_DESC(force_load, "Force loading on unknown platform"); > + > +static u8 usable_sensors_mask; > + > +enum gigabyte_wmi_commandtype { > + GIGABYTE_WMI_BUILD_DATE_QUERY = 0x1, > + GIGABYTE_WMI_MAINBOARD_TYPE_QUERY = 0x2, > + GIGABYTE_WMI_FIRMWARE_VERSION_QUERY = 0x4, > + GIGABYTE_WMI_MAINBOARD_NAME_QUERY = 0x5, > + GIGABYTE_WMI_TEMPERATURE_QUERY = 0x125, > +}; > + > +struct gigabyte_wmi_args { > + u32 arg1; > +}; > + > +static int gigabyte_wmi_perform_query(struct wmi_device *wdev, > + enum gigabyte_wmi_commandtype command, > + struct gigabyte_wmi_args *args, struct acpi_buffer *out) > +{ > + const struct acpi_buffer in = { > + .length = sizeof(*args), > + .pointer = args, > + }; > + > + acpi_status ret = wmidev_evaluate_method(wdev, 0x0, command, &in, out); > + > + if (ACPI_FAILURE(ret)) > + return -EIO; > + > + return 0; > +} > + > +static int gigabyte_wmi_query_integer(struct wmi_device *wdev, > + enum gigabyte_wmi_commandtype command, > + struct gigabyte_wmi_args *args, u64 *res) > +{ > + union acpi_object *obj; > + struct acpi_buffer result = { ACPI_ALLOCATE_BUFFER, NULL }; > + int ret; > + > + ret = gigabyte_wmi_perform_query(wdev, command, args, &result); > + if (ret) > + return ret; > + obj = result.pointer; > + if (obj && obj->type == ACPI_TYPE_INTEGER) > + *res = obj->integer.value; > + else > + ret = -EIO; > + kfree(result.pointer); > + return ret; > +} > + > +static int gigabyte_wmi_temperature(struct wmi_device *wdev, u8 sensor, long *res) > +{ > + struct gigabyte_wmi_args args = { > + .arg1 = sensor, > + }; > + u64 temp; > + acpi_status ret; > + > + ret = gigabyte_wmi_query_integer(wdev, GIGABYTE_WMI_TEMPERATURE_QUERY, &args, &temp); > + if (ret == 0) { > + if (temp == 0) > + return -ENODEV; > + *res = (s8)temp * 1000; // value is a signed 8-bit integer > + } > + return ret; > +} > + > +static int gigabyte_wmi_hwmon_read(struct device *dev, enum hwmon_sensor_types type, > + u32 attr, int channel, long *val) > +{ > + struct wmi_device *wdev = dev_get_drvdata(dev); > + > + return gigabyte_wmi_temperature(wdev, channel, val); > +} > + > +static umode_t gigabyte_wmi_hwmon_is_visible(const void *data, enum hwmon_sensor_types type, > + u32 attr, int channel) > +{ > + return usable_sensors_mask & BIT(channel) ? 0444 : 0; > +} > + > +static const struct hwmon_channel_info *gigabyte_wmi_hwmon_info[] = { > + HWMON_CHANNEL_INFO(temp, > + HWMON_T_INPUT, > + HWMON_T_INPUT, > + HWMON_T_INPUT, > + HWMON_T_INPUT, > + HWMON_T_INPUT, > + HWMON_T_INPUT), > + NULL > +}; > + > +static const struct hwmon_ops gigabyte_wmi_hwmon_ops = { > + .read = gigabyte_wmi_hwmon_read, > + .is_visible = gigabyte_wmi_hwmon_is_visible, > +}; > + > +static const struct hwmon_chip_info gigabyte_wmi_hwmon_chip_info = { > + .ops = &gigabyte_wmi_hwmon_ops, > + .info = gigabyte_wmi_hwmon_info, > +}; > + > +static u8 gigabyte_wmi_detect_sensor_usability(struct wmi_device *wdev) > +{ > + int i; > + long temp; > + u8 r = 0; > + > + for (i = 0; i < NUM_TEMPERATURE_SENSORS; i++) { > + if (!gigabyte_wmi_temperature(wdev, i, &temp)) > + r |= BIT(i); > + } > + return r; > +} > + > +static const struct dmi_system_id gigabyte_wmi_known_working_platforms[] = { > + { .matches = { > + DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "Gigabyte Technology Co., Ltd."), > + DMI_EXACT_MATCH(DMI_BOARD_NAME, "B550 GAMING X V2"), > + }}, > + { .matches = { > + DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "Gigabyte Technology Co., Ltd."), > + DMI_EXACT_MATCH(DMI_BOARD_NAME, "B550M DS3H"), > + }}, > + { .matches = { > + DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "Gigabyte Technology Co., Ltd."), > + DMI_EXACT_MATCH(DMI_BOARD_NAME, "Z390 I AORUS PRO WIFI-CF"), > + }}, > + { .matches = { > + DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "Gigabyte Technology Co., Ltd."), > + DMI_EXACT_MATCH(DMI_BOARD_NAME, "X570 I AORUS PRO WIFI"), > + }}, > + { } > +}; > + > +static int gigabyte_wmi_probe(struct wmi_device *wdev, const void *context) > +{ > + struct device *hwmon_dev; > + > + if (!dmi_check_system(gigabyte_wmi_known_working_platforms)) { > + if (!force_load) > + return -ENODEV; > + dev_warn(&wdev->dev, "Forcing load on unknown platform"); > + } > + > + usable_sensors_mask = gigabyte_wmi_detect_sensor_usability(wdev); > + if (!usable_sensors_mask) { > + dev_info(&wdev->dev, "No temperature sensors usable"); > + return -ENODEV; > + } > + > + hwmon_dev = devm_hwmon_device_register_with_info(&wdev->dev, "gigabyte_wmi", wdev, > + &gigabyte_wmi_hwmon_chip_info, NULL); > + > + return PTR_ERR_OR_ZERO(hwmon_dev); > +} > + > +static const struct wmi_device_id gigabyte_wmi_id_table[] = { > + { GIGABYTE_WMI_GUID, NULL }, > + { } > +}; > + > +static struct wmi_driver gigabyte_wmi_driver = { > + .driver = { > + .name = "gigabyte-wmi", > + }, > + .id_table = gigabyte_wmi_id_table, > + .probe = gigabyte_wmi_probe, > +}; > +module_wmi_driver(gigabyte_wmi_driver); > + > +MODULE_DEVICE_TABLE(wmi, gigabyte_wmi_id_table); > +MODULE_AUTHOR("Thomas Weißschuh <thomas@weissschuh.net>"); > +MODULE_DESCRIPTION("Gigabyte WMI temperature driver"); > +MODULE_LICENSE("GPL"); > > base-commit: 144c79ef33536b4ecb4951e07dbc1f2b7fa99d32 > ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-04-13 8:14 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <117cadef-c1cb-d66a-15f8-ce50d596be4b@redhat.com>
2021-04-10 14:40 ` [PATCH v3] platform/x86: add Gigabyte WMI temperature driver Thomas Weißschuh
2021-04-10 14:57 ` Hans de Goede
2021-04-10 15:21 ` Guenter Roeck
2021-04-10 15:15 ` Guenter Roeck
2021-04-10 15:23 ` Hans de Goede
2021-04-10 18:18 ` [PATCH v4] " Thomas Weißschuh
2021-04-11 0:58 ` Guenter Roeck
2021-04-11 14:05 ` Barnabás Pőcze
2021-04-12 12:35 ` [PATCH v5] " Thomas Weißschuh
2021-04-13 8:14 ` Hans de Goede
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).