linux-kernel - Re: [PATCH v4 2/2] hwmon: add fan speed monitoring driver for Surface devices

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Tue, 30 Jan 2024 15:26:15 -0800
From: Guenter Roeck <linux@...ck-us.net>
To: Ivor Wanders <ivor@...nders.net>, Jean Delvare <jdelvare@...e.com>,
 Jonathan Corbet <corbet@....net>, Maximilian Luz <luzmaximilian@...il.com>,
 Hans de Goede <hdegoede@...hat.com>, Mark Gross <markgross@...nel.org>
Cc: linux-hwmon@...r.kernel.org, linux-doc@...r.kernel.org,
 linux-kernel@...r.kernel.org, platform-driver-x86@...r.kernel.org
Subject: Re: [PATCH v4 2/2] hwmon: add fan speed monitoring driver for Surface
 devices

On 1/30/24 15:06, 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@...nders.net>
> Link: https://github.com/linux-surface/kernel/pull/144
> Reviewed-by: Maximilian Luz <luzmaximilian@...il.com>
> Reviewed-by: Armin Wolf <W_Armin@....de>
> ---
> Changes in v4:
>    - Return 0 from surface_fan_hwmon_read instead of ret.
>    - Use PTR_ERR_OR_ZERO in probe instead of if statement.
> Changes in v3:
>    - Removed type and attr checks in read and is_visible.
>    - Removed assigning sdev to ssam_device drvdata.
>    - Propagate return from __ssam_fan_rpm_get.
>    - Renamed hwmon chip name from 'fan' to 'surface_fan'.
>    - Removed unnecessary platform_device header.
> Changes in v2:
>    - Removed all sysfs attributes except fan1_input. Simplified code
>      and updated documentation accordingly.
> ---
>   Documentation/hwmon/index.rst       |  1 +
>   Documentation/hwmon/surface_fan.rst | 25 ++++++++
>   MAINTAINERS                         |  8 +++
>   drivers/hwmon/Kconfig               | 13 +++++
>   drivers/hwmon/Makefile              |  1 +
>   drivers/hwmon/surface_fan.c         | 91 +++++++++++++++++++++++++++++
>   6 files changed, 139 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..07942574c
> --- /dev/null
> +++ b/Documentation/hwmon/surface_fan.rst
> @@ -0,0 +1,25 @@
> +.. SPDX-License-Identifier: GPL-2.0-or-later
> +
> +Kernel driver surface_fan
> +=========================
> +
> +Supported Devices:
> +
> +  * Microsoft Surface Pro 9
> +
> +Author: Ivor Wanders <ivor@...nders.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.
> +======================= ======= =========================================
> 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@...il.com>
> +M:	Ivor Wanders <ivor@...nders.net>
> +L:	linux-hwmon@...r.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@...il.com>
>   L:	platform-driver-x86@...r.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

I just noticed this. There is no SENSORS_SMM665 in the upstream kernel.
That driver was removed in v6.7. What is your baseline ? It doesn't even
seem to be based on a mainline kernel. When I try to apply the patch,
it fails completely.

Applying: hwmon: add fan speed monitoring driver for Surface devices
error: sha1 information is lacking or useless (MAINTAINERS).
error: could not build fake ancestor
Patch failed at 0001 hwmon: add fan speed monitoring driver for Surface devices

Please base your patches on the latest mainline kernel.

Thanks,
Guenter