| 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
| ||
|
Message-ID: <CAJZ5v0iLNZT4jU1QV=HTbAv76s6Ay3tx93_K_QoM0XbvSgZF1w@mail.gmail.com>
Date: Tue, 28 May 2024 21:52:38 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Armin Wolf <W_Armin@....de>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>, mlj@...elec.com, rafael.j.wysocki@...el.com,
lenb@...nel.org, jdelvare@...e.com, andy.shevchenko@...il.com,
linux@...ck-us.net, linux@...ssschuh.net, ilpo.jarvinen@...ux.intel.com,
linux-acpi@...r.kernel.org, linux-hwmon@...r.kernel.org,
linux-kernel@...r.kernel.org, platform-driver-x86@...r.kernel.org
Subject: Re: [PATCH v8] ACPI: fan: Add hwmon support
On Tue, May 28, 2024 at 12:31 AM Armin Wolf <W_Armin@....de> wrote:
>
> Am 27.05.24 um 19:29 schrieb Rafael J. Wysocki:
>
> > On Fri, May 10, 2024 at 10:13 PM Armin Wolf <W_Armin@....de> wrote:
> >> Currently, the driver does only support a custom sysfs
> >> interface to allow userspace to read the fan speed.
> >> Add support for the standard hwmon interface so users
> >> can read the fan speed with standard tools like "sensors".
> >>
> >> Reviewed-by: Andy Shevchenko <andy@...nel.org>
> >> Signed-off-by: Armin Wolf <W_Armin@....de>
> >> ---
> >> Tested witha custom ACPI SSDT, available here:
> >> https://github.com/Wer-Wolf/acpi-fan-ssdt
> >>
> >> Changes since v7:
> >> - add Reviewed-by tag
> >> - spelling fixes
> >> - add missing types.h include
> >>
> >> Changes since v6:
> >> - add "hwmon" to the names of functions and variables
> >> related to hwmon
> >> - replace -ENODATA with -EIO/-ENODEV
> >>
> >> Changes since v5:
> >> - fix coding style issues
> >> - replace double break with return
> >> - add missing includes
> >>
> >> Changes since v4:
> >> - fix spelling issues
> >> - check power values for overflow condition too
> >>
> >> Changes since v3:
> >> - drop fault attrs
> >> - rework initialization
> >>
> >> Changes since v2:
> >> - add support for fanX_target and power attrs
> >>
> >> Changes since v1:
> >> - fix undefined reference error
> >> - fix fan speed validation
> >> - coding style fixes
> >> - clarify that the changes are compile-tested only
> >> - add hwmon maintainers to cc list
> >> ---
> >> drivers/acpi/Makefile | 1 +
> >> drivers/acpi/fan.h | 9 +++
> >> drivers/acpi/fan_core.c | 4 +
> >> drivers/acpi/fan_hwmon.c | 170 +++++++++++++++++++++++++++++++++++++++
> >> 4 files changed, 184 insertions(+)
> >> create mode 100644 drivers/acpi/fan_hwmon.c
> >>
> >> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> >> index 39ea5cfa8326..61ca4afe83dc 100644
> >> --- a/drivers/acpi/Makefile
> >> +++ b/drivers/acpi/Makefile
> >> @@ -77,6 +77,7 @@ obj-$(CONFIG_ACPI_TINY_POWER_BUTTON) += tiny-power-button.o
> >> obj-$(CONFIG_ACPI_FAN) += fan.o
> >> fan-objs := fan_core.o
> >> fan-objs += fan_attr.o
> >> +fan-$(CONFIG_HWMON) += fan_hwmon.o
> >>
> >> obj-$(CONFIG_ACPI_VIDEO) += video.o
> >> obj-$(CONFIG_ACPI_TAD) += acpi_tad.o
> >> diff --git a/drivers/acpi/fan.h b/drivers/acpi/fan.h
> >> index f89d19c922dc..db25a3898af7 100644
> >> --- a/drivers/acpi/fan.h
> >> +++ b/drivers/acpi/fan.h
> >> @@ -10,6 +10,8 @@
> >> #ifndef _ACPI_FAN_H_
> >> #define _ACPI_FAN_H_
> >>
> >> +#include <linux/kconfig.h>
> >> +
> >> #define ACPI_FAN_DEVICE_IDS \
> >> {"INT3404", }, /* Fan */ \
> >> {"INTC1044", }, /* Fan for Tiger Lake generation */ \
> >> @@ -57,4 +59,11 @@ struct acpi_fan {
> >> int acpi_fan_get_fst(struct acpi_device *device, struct acpi_fan_fst *fst);
> >> int acpi_fan_create_attributes(struct acpi_device *device);
> >> void acpi_fan_delete_attributes(struct acpi_device *device);
> >> +
> >> +#if IS_REACHABLE(CONFIG_HWMON)
> >> +int devm_acpi_fan_create_hwmon(struct acpi_device *device);
> >> +#else
> >> +static inline int devm_acpi_fan_create_hwmon(struct acpi_device *device) { return 0; };
> >> +#endif
> >> +
> >> #endif
> >> diff --git a/drivers/acpi/fan_core.c b/drivers/acpi/fan_core.c
> >> index ff72e4ef8738..7cea4495f19b 100644
> >> --- a/drivers/acpi/fan_core.c
> >> +++ b/drivers/acpi/fan_core.c
> >> @@ -336,6 +336,10 @@ static int acpi_fan_probe(struct platform_device *pdev)
> >> if (result)
> >> return result;
> >>
> >> + result = devm_acpi_fan_create_hwmon(device);
> >> + if (result)
> >> + return result;
> >> +
> >> result = acpi_fan_create_attributes(device);
> >> if (result)
> >> return result;
> >> diff --git a/drivers/acpi/fan_hwmon.c b/drivers/acpi/fan_hwmon.c
> >> new file mode 100644
> >> index 000000000000..bd0d31a398fa
> >> --- /dev/null
> >> +++ b/drivers/acpi/fan_hwmon.c
> >> @@ -0,0 +1,170 @@
> >> +// SPDX-License-Identifier: GPL-2.0-or-later
> >> +/*
> >> + * hwmon interface for the ACPI Fan driver.
> >> + *
> >> + * Copyright (C) 2024 Armin Wolf <W_Armin@....de>
> >> + */
> >> +
> >> +#include <linux/acpi.h>
> >> +#include <linux/device.h>
> >> +#include <linux/err.h>
> >> +#include <linux/hwmon.h>
> >> +#include <linux/limits.h>
> >> +#include <linux/types.h>
> >> +#include <linux/units.h>
> >> +
> >> +#include "fan.h"
> >> +
> >> +/* Returned when the ACPI fan does not support speed reporting */
> >> +#define FAN_SPEED_UNAVAILABLE U32_MAX
> >> +#define FAN_POWER_UNAVAILABLE U32_MAX
> >> +
> >> +static struct acpi_fan_fps *acpi_fan_get_current_fps(struct acpi_fan *fan, u64 control)
> >> +{
> >> + unsigned int i;
> >> +
> >> + for (i = 0; i < fan->fps_count; i++) {
> >> + if (fan->fps[i].control == control)
> >> + return &fan->fps[i];
> >> + }
> >> +
> >> + return NULL;
> >> +}
> >> +
> >> +static umode_t acpi_fan_hwmon_is_visible(const void *drvdata, enum hwmon_sensor_types type,
> >> + u32 attr, int channel)
> >> +{
> >> + const struct acpi_fan *fan = drvdata;
> >> + unsigned int i;
> > AFAICS, the code below can be rewritten as follows:
> >
> > if (fan->fif.fine_grain_ctrl)
> > return 0;
>
> Hi,
>
> this would break hwmon_fan_input.
Ah, I overlooked the first branch. Fair enough.
Applied as 6.11 material, thanks!
Powered by blists - more mailing lists