* [PATCH 0/2] Surface fan monitoring driver @ 2023-12-20 23:44 Ivor Wanders 2023-12-20 23:44 ` [PATCH 1/2] hwmon: add fan speed monitoring driver for Surface devices Ivor Wanders 0 siblings, 1 reply; 5+ messages in thread From: Ivor Wanders @ 2023-12-20 23:44 UTC (permalink / raw) To: Jean Delvare, Guenter Roeck, Jonathan Corbet, Maximilian Luz, Ivor Wanders, Hans de Goede, Mark Gross Cc: linux-hwmon, linux-doc, linux-kernel, platform-driver-x86 Currently, there is no way to obtain the fan's current speed on Microsoft Surface Series devices, this patch series adds a new module that provides read-only access to the fan's current speed through the hwmon system. This new module relies on the Surface System Aggregator Module which is the system responsible for communication with the EC on these devices. The first commit adds an entry into the SSAM registry for the fan's speed functionality (for the Surface Pro 9), the second commit adds the new module and documentation. Both patches can be applied independently of each other. Tested on a Microsoft Surface Pro 9. A full development log can be found on [1]. Fan control is always handled by the EC and cannot be influenced directly. It was identified during the development of this module that there are fan profiles between which can be switched. I'm currently developing improvements to the Surface platform profile module in [2] that will be submitted in the future. [1]: https://github.com/linux-surface/kernel/pull/144 [2]: https://github.com/linux-surface/kernel/pull/145 Ivor Wanders (2): hwmon: add fan speed monitoring driver for Surface devices platform/surface: aggregator_registry: add entry for fan speed Documentation/hwmon/index.rst | 1 + Documentation/hwmon/surface_fan.rst | 27 ++++ MAINTAINERS | 8 ++ drivers/hwmon/Kconfig | 13 ++ drivers/hwmon/Makefile | 1 + drivers/hwmon/surface_fan.c | 125 ++++++++++++++++++ .../surface/surface_aggregator_registry.c | 7 + 7 files changed, 182 insertions(+) create mode 100644 Documentation/hwmon/surface_fan.rst create mode 100644 drivers/hwmon/surface_fan.c -- 2.17.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] hwmon: add fan speed monitoring driver for Surface devices 2023-12-20 23:44 [PATCH 0/2] Surface fan monitoring driver Ivor Wanders @ 2023-12-20 23:44 ` Ivor Wanders 2023-12-20 23:55 ` Guenter Roeck 0 siblings, 1 reply; 5+ messages in thread From: Ivor Wanders @ 2023-12-20 23:44 UTC (permalink / raw) To: Jean Delvare, Guenter Roeck, Jonathan Corbet, Maximilian Luz, Ivor Wanders, Hans de Goede, Mark Gross Cc: linux-hwmon, linux-doc, linux-kernel Adds a driver that provides read only access to the fan speed for Microsoft Surface Pro devices. The fan speed is always regulated by the EC and cannot be influenced directly. Signed-off-by: Ivor Wanders <ivor@iwanders.net> Link: https://github.com/linux-surface/kernel/pull/144 --- Documentation/hwmon/index.rst | 1 + Documentation/hwmon/surface_fan.rst | 27 ++++++ MAINTAINERS | 8 ++ drivers/hwmon/Kconfig | 13 +++ drivers/hwmon/Makefile | 1 + drivers/hwmon/surface_fan.c | 125 ++++++++++++++++++++++++++++ 6 files changed, 175 insertions(+) create mode 100644 Documentation/hwmon/surface_fan.rst create mode 100644 drivers/hwmon/surface_fan.c diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst index 042e1cf95..4dfb3b9bd 100644 --- a/Documentation/hwmon/index.rst +++ b/Documentation/hwmon/index.rst @@ -202,6 +202,7 @@ Hardware Monitoring Kernel Drivers smsc47m1 sparx5-temp stpddc60 + surface_fan sy7636a-hwmon tc654 tc74 diff --git a/Documentation/hwmon/surface_fan.rst b/Documentation/hwmon/surface_fan.rst new file mode 100644 index 000000000..6e27a6653 --- /dev/null +++ b/Documentation/hwmon/surface_fan.rst @@ -0,0 +1,27 @@ +.. SPDX-License-Identifier: GPL-2.0-or-later + +Kernel driver surface_fan +========================= + +Supported Devices: + + * Microsoft Surface Pro 9 + +Author: Ivor Wanders <ivor@iwanders.net> + +Description +----------- + +This provides monitoring of the fan found in some Microsoft Surface Pro devices, +like the Surface Pro 9. The fan is always controlled by the onboard controller. + +Sysfs interface +--------------- + +======================= ======= ========================================= +Name Perm Description +======================= ======= ========================================= +``fan1_input`` RO Current fan speed in RPM. +``fan1_max`` RO Approximate maximum fan speed. +``fan1_min`` RO Minimum fan speed used by the controller. +======================= ======= ========================================= diff --git a/MAINTAINERS b/MAINTAINERS index 439cf523b..8e7870af3 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -14078,6 +14078,14 @@ F: Documentation/driver-api/surface_aggregator/clients/dtx.rst F: drivers/platform/surface/surface_dtx.c F: include/uapi/linux/surface_aggregator/dtx.h +MICROSOFT SURFACE SENSOR FAN DRIVER +M: Maximilian Luz <luzmaximilian@gmail.com> +M: Ivor Wanders <ivor@iwanders.net> +L: linux-hwmon@vger.kernel.org +S: Maintained +F: Documentation/hwmon/surface_fan.rst +F: drivers/hwmon/surface_fan.c + MICROSOFT SURFACE GPE LID SUPPORT DRIVER M: Maximilian Luz <luzmaximilian@gmail.com> L: platform-driver-x86@vger.kernel.org diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index 307477b8a..4b4d999af 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -1965,6 +1965,19 @@ config SENSORS_SMM665 This driver can also be built as a module. If so, the module will be called smm665. +config SENSORS_SURFACE_FAN + tristate "Surface Fan Driver" + depends on SURFACE_AGGREGATOR + help + Driver that provides monitoring of the fan on Surface Pro devices that + have a fan, like the Surface Pro 9. + + This makes the fan's current speed accessible through the hwmon + system. It does not provide control over the fan, the firmware is + responsible for that, this driver merely provides monitoring. + + Select M or Y here, if you want to be able to read the fan's speed. + config SENSORS_ADC128D818 tristate "Texas Instruments ADC128D818" depends on I2C diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile index 3f4b0fda0..5ae214c06 100644 --- a/drivers/hwmon/Makefile +++ b/drivers/hwmon/Makefile @@ -198,6 +198,7 @@ obj-$(CONFIG_SENSORS_SMSC47M1) += smsc47m1.o obj-$(CONFIG_SENSORS_SMSC47M192)+= smsc47m192.o obj-$(CONFIG_SENSORS_SPARX5) += sparx5-temp.o obj-$(CONFIG_SENSORS_STTS751) += stts751.o +obj-$(CONFIG_SENSORS_SURFACE_FAN)+= surface_fan.o obj-$(CONFIG_SENSORS_SY7636A) += sy7636a-hwmon.o obj-$(CONFIG_SENSORS_AMC6821) += amc6821.o obj-$(CONFIG_SENSORS_TC74) += tc74.o diff --git a/drivers/hwmon/surface_fan.c b/drivers/hwmon/surface_fan.c new file mode 100644 index 000000000..7129b25ed --- /dev/null +++ b/drivers/hwmon/surface_fan.c @@ -0,0 +1,125 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Surface Fan driver for Surface System Aggregator Module. It provides access + * to the fan's rpm through the hwmon system. + * + * Copyright (C) 2023 Ivor Wanders <ivor@iwanders.net> + */ + +#include <linux/hwmon.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/surface_aggregator/device.h> +#include <linux/types.h> + +// The minimum speed for the fan when turned on by the controller. The onboard +// controller uses this as minimum value before turning the fan on or off. +#define SURFACE_FAN_MIN_SPEED 3000 +// The maximum speed, determined by observation and rounding up to the nearest +// multiple of 500 to account for variation between individual fans. +#define SURFACE_FAN_MAX_SPEED 7500 + +// SSAM +SSAM_DEFINE_SYNC_REQUEST_CL_R(__ssam_fan_rpm_get, __le16, { + .target_category = SSAM_SSH_TC_FAN, + .command_id = 0x01, +}); + +// hwmon +umode_t surface_fan_hwmon_is_visible(const void *drvdata, + enum hwmon_sensor_types type, u32 attr, + int channel) +{ + if (type != hwmon_fan) + return 0; + + switch (attr) { + case hwmon_fan_input: + case hwmon_fan_label: + case hwmon_fan_min: + case hwmon_fan_max: + return 0444; + default: + return 0; + } +} + +static int surface_fan_hwmon_read(struct device *dev, + enum hwmon_sensor_types type, u32 attr, + int channel, long *val) +{ + struct ssam_device *sdev = dev_get_drvdata(dev); + __le16 value; + int res; + + if (type != hwmon_fan) + return -EOPNOTSUPP; + + switch (attr) { + case hwmon_fan_input: + res = __ssam_fan_rpm_get(sdev, &value); + if (res) + return -EIO; + *val = le16_to_cpu(value); + return 0; + case hwmon_fan_min: + *val = SURFACE_FAN_MIN_SPEED; + return 0; + case hwmon_fan_max: + *val = SURFACE_FAN_MAX_SPEED; + return 0; + default: + return -EOPNOTSUPP; + } +} + +static const struct hwmon_channel_info *const surface_fan_info[] = { + HWMON_CHANNEL_INFO(fan, HWMON_F_INPUT | HWMON_F_MAX | HWMON_F_MIN), + NULL +}; + +static const struct hwmon_ops surface_fan_hwmon_ops = { + .is_visible = surface_fan_hwmon_is_visible, + .read = surface_fan_hwmon_read, +}; + +static const struct hwmon_chip_info surface_fan_chip_info = { + .ops = &surface_fan_hwmon_ops, + .info = surface_fan_info, +}; + +static int surface_fan_probe(struct ssam_device *sdev) +{ + struct device *hdev; + + hdev = devm_hwmon_device_register_with_info(&sdev->dev, "fan", sdev, + &surface_fan_chip_info, + NULL); + if (IS_ERR(hdev)) + return PTR_ERR(hdev); + + ssam_device_set_drvdata(sdev, sdev); + + return 0; +} + +static const struct ssam_device_id ssam_fan_match[] = { + { SSAM_SDEV(FAN, SAM, 0x01, 0x01) }, + {}, +}; +MODULE_DEVICE_TABLE(ssam, ssam_fan_match); + +static struct ssam_device_driver surface_fan = { + .probe = surface_fan_probe, + .match_table = ssam_fan_match, + .driver = { + .name = "surface_fan", + .probe_type = PROBE_PREFER_ASYNCHRONOUS, + }, +}; +module_ssam_device_driver(surface_fan); + +MODULE_AUTHOR("Ivor Wanders <ivor@iwanders.net>"); +MODULE_DESCRIPTION("Fan Driver for Surface System Aggregator Module"); +MODULE_LICENSE("GPL"); -- 2.17.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] hwmon: add fan speed monitoring driver for Surface devices 2023-12-20 23:44 ` [PATCH 1/2] hwmon: add fan speed monitoring driver for Surface devices Ivor Wanders @ 2023-12-20 23:55 ` Guenter Roeck 2023-12-21 22:55 ` Ivor Wanders 0 siblings, 1 reply; 5+ messages in thread From: Guenter Roeck @ 2023-12-20 23:55 UTC (permalink / raw) To: Ivor Wanders Cc: Jean Delvare, Jonathan Corbet, Maximilian Luz, Hans de Goede, Mark Gross, linux-hwmon, linux-doc, linux-kernel On Wed, Dec 20, 2023 at 06:44:14PM -0500, Ivor Wanders wrote: > Adds a driver that provides read only access to the fan speed for Microsoft > Surface Pro devices. The fan speed is always regulated by the EC and cannot > be influenced directly. > > Signed-off-by: Ivor Wanders <ivor@iwanders.net> > Link: https://github.com/linux-surface/kernel/pull/144 > --- > Documentation/hwmon/index.rst | 1 + > Documentation/hwmon/surface_fan.rst | 27 ++++++ > MAINTAINERS | 8 ++ > drivers/hwmon/Kconfig | 13 +++ > drivers/hwmon/Makefile | 1 + > drivers/hwmon/surface_fan.c | 125 ++++++++++++++++++++++++++++ > 6 files changed, 175 insertions(+) > create mode 100644 Documentation/hwmon/surface_fan.rst > create mode 100644 drivers/hwmon/surface_fan.c > > diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst > index 042e1cf95..4dfb3b9bd 100644 > --- a/Documentation/hwmon/index.rst > +++ b/Documentation/hwmon/index.rst > @@ -202,6 +202,7 @@ Hardware Monitoring Kernel Drivers > smsc47m1 > sparx5-temp > stpddc60 > + surface_fan > sy7636a-hwmon > tc654 > tc74 > diff --git a/Documentation/hwmon/surface_fan.rst b/Documentation/hwmon/surface_fan.rst > new file mode 100644 > index 000000000..6e27a6653 > --- /dev/null > +++ b/Documentation/hwmon/surface_fan.rst > @@ -0,0 +1,27 @@ > +.. SPDX-License-Identifier: GPL-2.0-or-later > + > +Kernel driver surface_fan > +========================= > + > +Supported Devices: > + > + * Microsoft Surface Pro 9 > + > +Author: Ivor Wanders <ivor@iwanders.net> > + > +Description > +----------- > + > +This provides monitoring of the fan found in some Microsoft Surface Pro devices, > +like the Surface Pro 9. The fan is always controlled by the onboard controller. > + > +Sysfs interface > +--------------- > + > +======================= ======= ========================================= > +Name Perm Description > +======================= ======= ========================================= > +``fan1_input`` RO Current fan speed in RPM. > +``fan1_max`` RO Approximate maximum fan speed. > +``fan1_min`` RO Minimum fan speed used by the controller. > +======================= ======= ========================================= > diff --git a/MAINTAINERS b/MAINTAINERS > index 439cf523b..8e7870af3 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -14078,6 +14078,14 @@ F: Documentation/driver-api/surface_aggregator/clients/dtx.rst > F: drivers/platform/surface/surface_dtx.c > F: include/uapi/linux/surface_aggregator/dtx.h > > +MICROSOFT SURFACE SENSOR FAN DRIVER > +M: Maximilian Luz <luzmaximilian@gmail.com> > +M: Ivor Wanders <ivor@iwanders.net> > +L: linux-hwmon@vger.kernel.org > +S: Maintained > +F: Documentation/hwmon/surface_fan.rst > +F: drivers/hwmon/surface_fan.c > + > MICROSOFT SURFACE GPE LID SUPPORT DRIVER > M: Maximilian Luz <luzmaximilian@gmail.com> > L: platform-driver-x86@vger.kernel.org > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig > index 307477b8a..4b4d999af 100644 > --- a/drivers/hwmon/Kconfig > +++ b/drivers/hwmon/Kconfig > @@ -1965,6 +1965,19 @@ config SENSORS_SMM665 > This driver can also be built as a module. If so, the module will > be called smm665. > > +config SENSORS_SURFACE_FAN > + tristate "Surface Fan Driver" > + depends on SURFACE_AGGREGATOR > + help > + Driver that provides monitoring of the fan on Surface Pro devices that > + have a fan, like the Surface Pro 9. > + > + This makes the fan's current speed accessible through the hwmon > + system. It does not provide control over the fan, the firmware is > + responsible for that, this driver merely provides monitoring. > + > + Select M or Y here, if you want to be able to read the fan's speed. > + > config SENSORS_ADC128D818 > tristate "Texas Instruments ADC128D818" > depends on I2C > diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile > index 3f4b0fda0..5ae214c06 100644 > --- a/drivers/hwmon/Makefile > +++ b/drivers/hwmon/Makefile > @@ -198,6 +198,7 @@ obj-$(CONFIG_SENSORS_SMSC47M1) += smsc47m1.o > obj-$(CONFIG_SENSORS_SMSC47M192)+= smsc47m192.o > obj-$(CONFIG_SENSORS_SPARX5) += sparx5-temp.o > obj-$(CONFIG_SENSORS_STTS751) += stts751.o > +obj-$(CONFIG_SENSORS_SURFACE_FAN)+= surface_fan.o > obj-$(CONFIG_SENSORS_SY7636A) += sy7636a-hwmon.o > obj-$(CONFIG_SENSORS_AMC6821) += amc6821.o > obj-$(CONFIG_SENSORS_TC74) += tc74.o > diff --git a/drivers/hwmon/surface_fan.c b/drivers/hwmon/surface_fan.c > new file mode 100644 > index 000000000..7129b25ed > --- /dev/null > +++ b/drivers/hwmon/surface_fan.c > @@ -0,0 +1,125 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Surface Fan driver for Surface System Aggregator Module. It provides access > + * to the fan's rpm through the hwmon system. > + * > + * Copyright (C) 2023 Ivor Wanders <ivor@iwanders.net> > + */ > + > +#include <linux/hwmon.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/platform_device.h> > +#include <linux/surface_aggregator/device.h> > +#include <linux/types.h> > + > +// The minimum speed for the fan when turned on by the controller. The onboard > +// controller uses this as minimum value before turning the fan on or off. > +#define SURFACE_FAN_MIN_SPEED 3000 > +// The maximum speed, determined by observation and rounding up to the nearest > +// multiple of 500 to account for variation between individual fans. > +#define SURFACE_FAN_MAX_SPEED 7500 > + > +// SSAM > +SSAM_DEFINE_SYNC_REQUEST_CL_R(__ssam_fan_rpm_get, __le16, { > + .target_category = SSAM_SSH_TC_FAN, > + .command_id = 0x01, > +}); > + > +// hwmon > +umode_t surface_fan_hwmon_is_visible(const void *drvdata, > + enum hwmon_sensor_types type, u32 attr, > + int channel) > +{ > + if (type != hwmon_fan) > + return 0; > + > + switch (attr) { > + case hwmon_fan_input: > + case hwmon_fan_label: > + case hwmon_fan_min: > + case hwmon_fan_max: > + return 0444; > + default: > + return 0; > + } > +} > + > +static int surface_fan_hwmon_read(struct device *dev, > + enum hwmon_sensor_types type, u32 attr, > + int channel, long *val) > +{ > + struct ssam_device *sdev = dev_get_drvdata(dev); > + __le16 value; > + int res; > + > + if (type != hwmon_fan) > + return -EOPNOTSUPP; > + > + switch (attr) { > + case hwmon_fan_input: > + res = __ssam_fan_rpm_get(sdev, &value); > + if (res) > + return -EIO; > + *val = le16_to_cpu(value); > + return 0; > + case hwmon_fan_min: > + *val = SURFACE_FAN_MIN_SPEED; > + return 0; > + case hwmon_fan_max: > + *val = SURFACE_FAN_MAX_SPEED; > + return 0; No, sorry. Limit attributes are supposed to be used to program limits, not to report constant values to userspace (and please refrain from referring to other drivers doing the same. Two wrongs don't make it right). Guenter ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] hwmon: add fan speed monitoring driver for Surface devices 2023-12-20 23:55 ` Guenter Roeck @ 2023-12-21 22:55 ` Ivor Wanders 2023-12-22 16:15 ` Guenter Roeck 0 siblings, 1 reply; 5+ messages in thread From: Ivor Wanders @ 2023-12-21 22:55 UTC (permalink / raw) To: linux Cc: corbet, hdegoede, ivor, jdelvare, linux-doc, linux-hwmon, linux-kernel, luzmaximilian, markgross > No, sorry. Limit attributes are supposed to be used to program limits, > not to report constant values to userspace Thank you for this feedback, I've proposed improvements to the documentation in [1] that clarify this. I will incorporate this feedback and submit a second version. ~Ivor [1]: https://lore.kernel.org/linux-hwmon/20231221225149.11295-1-ivor@iwanders.net/T/ ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] hwmon: add fan speed monitoring driver for Surface devices 2023-12-21 22:55 ` Ivor Wanders @ 2023-12-22 16:15 ` Guenter Roeck 0 siblings, 0 replies; 5+ messages in thread From: Guenter Roeck @ 2023-12-22 16:15 UTC (permalink / raw) To: Ivor Wanders Cc: corbet, hdegoede, jdelvare, linux-doc, linux-hwmon, linux-kernel, luzmaximilian, markgross On Thu, Dec 21, 2023 at 05:55:21PM -0500, Ivor Wanders wrote: > > No, sorry. Limit attributes are supposed to be used to program limits, > > not to report constant values to userspace > > Thank you for this feedback, I've proposed improvements to the > documentation in [1] that clarify this. I will incorporate this feedback > and submit a second version. > This applies to all hwmon sysfs attributes and is already documented. Guenter ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-12-22 16:15 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-12-20 23:44 [PATCH 0/2] Surface fan monitoring driver Ivor Wanders 2023-12-20 23:44 ` [PATCH 1/2] hwmon: add fan speed monitoring driver for Surface devices Ivor Wanders 2023-12-20 23:55 ` Guenter Roeck 2023-12-21 22:55 ` Ivor Wanders 2023-12-22 16:15 ` Guenter Roeck
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox