* [PATCH 1/2] platform/x86: toshiba_acpi: Add fan RPM reading (internals)
2022-09-02 17:40 [PATCH v3 0/2] platform/x86: toshiba_acpi: HWMON Fan RPM support Arvid Norlander
@ 2022-09-02 17:40 ` Arvid Norlander
2022-09-02 17:40 ` [PATCH v3 2/2] platform/x86: toshiba_acpi: Add fan RPM reading (hwmon interface) Arvid Norlander
2022-09-09 16:01 ` [PATCH v3 0/2] platform/x86: toshiba_acpi: HWMON Fan RPM support Hans de Goede
2 siblings, 0 replies; 5+ messages in thread
From: Arvid Norlander @ 2022-09-02 17:40 UTC (permalink / raw)
To: platform-driver-x86; +Cc: Azael Avalos, linux-hwmon, Arvid Norlander
This add the internal feature detection and reading function for fan RPM.
The approach is based on tracing ACPI calls using AMLI (a tracer/debugger
built into ACPI.sys) while using the Windows cooling self-test software.
The call used is {HCI_GET, 0x45, 0, 1, 0, 0} which returns:
{0x0, 0x45, fan_rpm, probably_max_rpm, 0x0, 0x0}
What is probably the max RPM is not currently used.
Signed-off-by: Arvid Norlander <lkml@vorpal.se>
---
drivers/platform/x86/toshiba_acpi.c | 30 +++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
index 0fc9e8b8827b..02e3522f4eeb 100644
--- a/drivers/platform/x86/toshiba_acpi.c
+++ b/drivers/platform/x86/toshiba_acpi.c
@@ -106,6 +106,7 @@ MODULE_LICENSE("GPL");
#define HCI_VIDEO_OUT 0x001c
#define HCI_HOTKEY_EVENT 0x001e
#define HCI_LCD_BRIGHTNESS 0x002a
+#define HCI_FAN_RPM 0x0045
#define HCI_WIRELESS 0x0056
#define HCI_ACCELEROMETER 0x006d
#define HCI_COOLING_METHOD 0x007f
@@ -185,6 +186,7 @@ struct toshiba_acpi_dev {
unsigned int illumination_supported:1;
unsigned int video_supported:1;
unsigned int fan_supported:1;
+ unsigned int fan_rpm_supported:1;
unsigned int system_event_supported:1;
unsigned int ntfy_supported:1;
unsigned int info_supported:1;
@@ -1616,6 +1618,29 @@ static const struct proc_ops fan_proc_ops = {
.proc_write = fan_proc_write,
};
+/* Fan RPM */
+static int get_fan_rpm(struct toshiba_acpi_dev *dev, u32 *rpm)
+{
+ u32 in[TCI_WORDS] = { HCI_GET, HCI_FAN_RPM, 0, 1, 0, 0 };
+ u32 out[TCI_WORDS];
+ acpi_status status = tci_raw(dev, in, out);
+
+ if (ACPI_FAILURE(status)) {
+ pr_err("ACPI call to get Fan speed failed\n");
+ return -EIO;
+ }
+
+ if (out[0] == TOS_NOT_SUPPORTED)
+ return -ENODEV;
+
+ if (out[0] == TOS_SUCCESS) {
+ *rpm = out[2];
+ return 0;
+ }
+
+ return -EIO;
+}
+
static int keys_proc_show(struct seq_file *m, void *v)
{
struct toshiba_acpi_dev *dev = m->private;
@@ -2928,6 +2953,8 @@ static void print_supported_features(struct toshiba_acpi_dev *dev)
pr_cont(" video-out");
if (dev->fan_supported)
pr_cont(" fan");
+ if (dev->fan_rpm_supported)
+ pr_cont(" fan-rpm");
if (dev->tr_backlight_supported)
pr_cont(" transflective-backlight");
if (dev->illumination_supported)
@@ -3157,6 +3184,9 @@ static int toshiba_acpi_add(struct acpi_device *acpi_dev)
ret = get_fan_status(dev, &dummy);
dev->fan_supported = !ret;
+ ret = get_fan_rpm(dev, &dummy);
+ dev->fan_rpm_supported = !ret;
+
toshiba_wwan_available(dev);
if (dev->wwan_supported)
toshiba_acpi_setup_wwan_rfkill(dev);
--
2.37.3
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH v3 2/2] platform/x86: toshiba_acpi: Add fan RPM reading (hwmon interface)
2022-09-02 17:40 [PATCH v3 0/2] platform/x86: toshiba_acpi: HWMON Fan RPM support Arvid Norlander
2022-09-02 17:40 ` [PATCH 1/2] platform/x86: toshiba_acpi: Add fan RPM reading (internals) Arvid Norlander
@ 2022-09-02 17:40 ` Arvid Norlander
2022-09-02 18:24 ` Guenter Roeck
2022-09-09 16:01 ` [PATCH v3 0/2] platform/x86: toshiba_acpi: HWMON Fan RPM support Hans de Goede
2 siblings, 1 reply; 5+ messages in thread
From: Arvid Norlander @ 2022-09-02 17:40 UTC (permalink / raw)
To: platform-driver-x86; +Cc: Azael Avalos, linux-hwmon, Arvid Norlander
This expands on the previous commit, exporting the fan RPM via hwmon.
This will look something like the following when using the "sensors"
command from lm_sensors:
toshiba_acpi_sensors-acpi-0
Adapter: ACPI interface
fan1: 0 RPM
Signed-off-by: Arvid Norlander <lkml@vorpal.se>
---
drivers/platform/x86/Kconfig | 1 +
drivers/platform/x86/toshiba_acpi.c | 70 +++++++++++++++++++++++++++++
2 files changed, 71 insertions(+)
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index f2f98e942cf2..4d0d2676939a 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -797,6 +797,7 @@ config ACPI_TOSHIBA
depends on INPUT
depends on SERIO_I8042 || SERIO_I8042 = n
depends on ACPI_VIDEO || ACPI_VIDEO = n
+ depends on HWMON || HWMON = n
depends on RFKILL || RFKILL = n
depends on IIO
select INPUT_SPARSEKMAP
diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
index 02e3522f4eeb..0949b1bcab83 100644
--- a/drivers/platform/x86/toshiba_acpi.c
+++ b/drivers/platform/x86/toshiba_acpi.c
@@ -42,10 +42,12 @@
#include <linux/uaccess.h>
#include <linux/miscdevice.h>
#include <linux/rfkill.h>
+#include <linux/hwmon.h>
#include <linux/iio/iio.h>
#include <linux/toshiba.h>
#include <acpi/video.h>
+
MODULE_AUTHOR("John Belmonte");
MODULE_DESCRIPTION("Toshiba Laptop ACPI Extras Driver");
MODULE_LICENSE("GPL");
@@ -171,6 +173,9 @@ struct toshiba_acpi_dev {
struct miscdevice miscdev;
struct rfkill *wwan_rfk;
struct iio_dev *indio_dev;
+#if IS_ENABLED(CONFIG_HWMON)
+ struct device *hwmon_device;
+#endif
int force_fan;
int last_key_event;
@@ -2941,6 +2946,54 @@ static int toshiba_acpi_setup_backlight(struct toshiba_acpi_dev *dev)
return 0;
}
+/* HWMON support for fan */
+#if IS_ENABLED(CONFIG_HWMON)
+umode_t toshiba_acpi_hwmon_is_visible(const void *drvdata,
+ enum hwmon_sensor_types type,
+ u32 attr, int channel)
+{
+ return 0444;
+}
+
+int toshiba_acpi_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
+ u32 attr, int channel, long *val)
+{
+ /*
+ * There is only a single channel and single attribute (for the
+ * fan) at this point.
+ * This can be replaced with more advanced logic in the future,
+ * should the need arise.
+ */
+ if (type == hwmon_fan && channel == 0 && attr == hwmon_fan_input) {
+ u32 value;
+ int ret;
+
+ ret = get_fan_rpm(toshiba_acpi, &value);
+ if (ret)
+ return ret;
+
+ *val = value;
+ return 0;
+ }
+ return -EOPNOTSUPP;
+}
+
+static const struct hwmon_channel_info *toshiba_acpi_hwmon_info[] = {
+ HWMON_CHANNEL_INFO(fan, HWMON_F_INPUT),
+ NULL
+};
+
+static const struct hwmon_ops toshiba_acpi_hwmon_ops = {
+ .is_visible = toshiba_acpi_hwmon_is_visible,
+ .read = toshiba_acpi_hwmon_read,
+};
+
+static const struct hwmon_chip_info toshiba_acpi_hwmon_chip_info = {
+ .ops = &toshiba_acpi_hwmon_ops,
+ .info = toshiba_acpi_hwmon_info,
+};
+#endif
+
static void print_supported_features(struct toshiba_acpi_dev *dev)
{
pr_info("Supported laptop features:");
@@ -2995,6 +3048,11 @@ static int toshiba_acpi_remove(struct acpi_device *acpi_dev)
remove_toshiba_proc_entries(dev);
+#if IS_ENABLED(CONFIG_HWMON)
+ if (dev->hwmon_device)
+ hwmon_device_unregister(dev->hwmon_device);
+#endif
+
if (dev->accelerometer_supported && dev->indio_dev) {
iio_device_unregister(dev->indio_dev);
iio_device_free(dev->indio_dev);
@@ -3187,6 +3245,18 @@ static int toshiba_acpi_add(struct acpi_device *acpi_dev)
ret = get_fan_rpm(dev, &dummy);
dev->fan_rpm_supported = !ret;
+#if IS_ENABLED(CONFIG_HWMON)
+ if (dev->fan_rpm_supported) {
+ dev->hwmon_device = hwmon_device_register_with_info(
+ &dev->acpi_dev->dev, "toshiba_acpi_sensors", NULL,
+ &toshiba_acpi_hwmon_chip_info, NULL);
+ if (IS_ERR(dev->hwmon_device)) {
+ dev->hwmon_device = NULL;
+ pr_warn("unable to register hwmon device, skipping\n");
+ }
+ }
+#endif
+
toshiba_wwan_available(dev);
if (dev->wwan_supported)
toshiba_acpi_setup_wwan_rfkill(dev);
--
2.37.3
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v3 2/2] platform/x86: toshiba_acpi: Add fan RPM reading (hwmon interface)
2022-09-02 17:40 ` [PATCH v3 2/2] platform/x86: toshiba_acpi: Add fan RPM reading (hwmon interface) Arvid Norlander
@ 2022-09-02 18:24 ` Guenter Roeck
0 siblings, 0 replies; 5+ messages in thread
From: Guenter Roeck @ 2022-09-02 18:24 UTC (permalink / raw)
To: Arvid Norlander; +Cc: platform-driver-x86, Azael Avalos, linux-hwmon
On Fri, Sep 02, 2022 at 07:40:18PM +0200, Arvid Norlander wrote:
> This expands on the previous commit, exporting the fan RPM via hwmon.
>
> This will look something like the following when using the "sensors"
> command from lm_sensors:
>
> toshiba_acpi_sensors-acpi-0
> Adapter: ACPI interface
> fan1: 0 RPM
>
> Signed-off-by: Arvid Norlander <lkml@vorpal.se>
> ---
> drivers/platform/x86/Kconfig | 1 +
> drivers/platform/x86/toshiba_acpi.c | 70 +++++++++++++++++++++++++++++
> 2 files changed, 71 insertions(+)
>
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index f2f98e942cf2..4d0d2676939a 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -797,6 +797,7 @@ config ACPI_TOSHIBA
> depends on INPUT
> depends on SERIO_I8042 || SERIO_I8042 = n
> depends on ACPI_VIDEO || ACPI_VIDEO = n
> + depends on HWMON || HWMON = n
> depends on RFKILL || RFKILL = n
> depends on IIO
> select INPUT_SPARSEKMAP
> diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
> index 02e3522f4eeb..0949b1bcab83 100644
> --- a/drivers/platform/x86/toshiba_acpi.c
> +++ b/drivers/platform/x86/toshiba_acpi.c
> @@ -42,10 +42,12 @@
> #include <linux/uaccess.h>
> #include <linux/miscdevice.h>
> #include <linux/rfkill.h>
> +#include <linux/hwmon.h>
> #include <linux/iio/iio.h>
> #include <linux/toshiba.h>
> #include <acpi/video.h>
>
> +
Unnecessary double empty line
Otherwise
Acked-by: Guenter Roeck <linux@roeck-us.net>
Guenter
> MODULE_AUTHOR("John Belmonte");
> MODULE_DESCRIPTION("Toshiba Laptop ACPI Extras Driver");
> MODULE_LICENSE("GPL");
> @@ -171,6 +173,9 @@ struct toshiba_acpi_dev {
> struct miscdevice miscdev;
> struct rfkill *wwan_rfk;
> struct iio_dev *indio_dev;
> +#if IS_ENABLED(CONFIG_HWMON)
> + struct device *hwmon_device;
> +#endif
>
> int force_fan;
> int last_key_event;
> @@ -2941,6 +2946,54 @@ static int toshiba_acpi_setup_backlight(struct toshiba_acpi_dev *dev)
> return 0;
> }
>
> +/* HWMON support for fan */
> +#if IS_ENABLED(CONFIG_HWMON)
> +umode_t toshiba_acpi_hwmon_is_visible(const void *drvdata,
> + enum hwmon_sensor_types type,
> + u32 attr, int channel)
> +{
> + return 0444;
> +}
> +
> +int toshiba_acpi_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
> + u32 attr, int channel, long *val)
> +{
> + /*
> + * There is only a single channel and single attribute (for the
> + * fan) at this point.
> + * This can be replaced with more advanced logic in the future,
> + * should the need arise.
> + */
> + if (type == hwmon_fan && channel == 0 && attr == hwmon_fan_input) {
> + u32 value;
> + int ret;
> +
> + ret = get_fan_rpm(toshiba_acpi, &value);
> + if (ret)
> + return ret;
> +
> + *val = value;
> + return 0;
> + }
> + return -EOPNOTSUPP;
> +}
> +
> +static const struct hwmon_channel_info *toshiba_acpi_hwmon_info[] = {
> + HWMON_CHANNEL_INFO(fan, HWMON_F_INPUT),
> + NULL
> +};
> +
> +static const struct hwmon_ops toshiba_acpi_hwmon_ops = {
> + .is_visible = toshiba_acpi_hwmon_is_visible,
> + .read = toshiba_acpi_hwmon_read,
> +};
> +
> +static const struct hwmon_chip_info toshiba_acpi_hwmon_chip_info = {
> + .ops = &toshiba_acpi_hwmon_ops,
> + .info = toshiba_acpi_hwmon_info,
> +};
> +#endif
> +
> static void print_supported_features(struct toshiba_acpi_dev *dev)
> {
> pr_info("Supported laptop features:");
> @@ -2995,6 +3048,11 @@ static int toshiba_acpi_remove(struct acpi_device *acpi_dev)
>
> remove_toshiba_proc_entries(dev);
>
> +#if IS_ENABLED(CONFIG_HWMON)
> + if (dev->hwmon_device)
> + hwmon_device_unregister(dev->hwmon_device);
> +#endif
> +
> if (dev->accelerometer_supported && dev->indio_dev) {
> iio_device_unregister(dev->indio_dev);
> iio_device_free(dev->indio_dev);
> @@ -3187,6 +3245,18 @@ static int toshiba_acpi_add(struct acpi_device *acpi_dev)
> ret = get_fan_rpm(dev, &dummy);
> dev->fan_rpm_supported = !ret;
>
> +#if IS_ENABLED(CONFIG_HWMON)
> + if (dev->fan_rpm_supported) {
> + dev->hwmon_device = hwmon_device_register_with_info(
> + &dev->acpi_dev->dev, "toshiba_acpi_sensors", NULL,
> + &toshiba_acpi_hwmon_chip_info, NULL);
> + if (IS_ERR(dev->hwmon_device)) {
> + dev->hwmon_device = NULL;
> + pr_warn("unable to register hwmon device, skipping\n");
> + }
> + }
> +#endif
> +
> toshiba_wwan_available(dev);
> if (dev->wwan_supported)
> toshiba_acpi_setup_wwan_rfkill(dev);
> --
> 2.37.3
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3 0/2] platform/x86: toshiba_acpi: HWMON Fan RPM support
2022-09-02 17:40 [PATCH v3 0/2] platform/x86: toshiba_acpi: HWMON Fan RPM support Arvid Norlander
2022-09-02 17:40 ` [PATCH 1/2] platform/x86: toshiba_acpi: Add fan RPM reading (internals) Arvid Norlander
2022-09-02 17:40 ` [PATCH v3 2/2] platform/x86: toshiba_acpi: Add fan RPM reading (hwmon interface) Arvid Norlander
@ 2022-09-09 16:01 ` Hans de Goede
2 siblings, 0 replies; 5+ messages in thread
From: Hans de Goede @ 2022-09-09 16:01 UTC (permalink / raw)
To: Arvid Norlander, platform-driver-x86; +Cc: Azael Avalos, linux-hwmon
Hi,
On 9/2/22 19:40, Arvid Norlander wrote:
> Hi,
>
> Lets hope for third time being the charm!
>
> Changelog
> =========
> v2: Fixed feedback on usage of HWMON interfaces in patch 2.
> v3: Fixed #ifdef handling in patch 2.
>
> Fan
> ===
>
> Currently /sys/bus/acpi/devices/TOS6208:00/fan allows controlling the fan
> by writing 0 (off) or 1 (on at low speed). However when reading I have
> observed values up to 64 (fan at full speed during prime95 stress test).
>
> Removing the check for "zero or one" shows that on the Z830 at least 64
> levels do indeed seem possible. In fact higher values can be written.
>
> But anything above ~50 seems to max out the RPM.
>
> I don't know how to detect the supported range, so I have not created a
> patch for this. Advice is welcome.
>
>
> Fan RPM
> =======
>
> There is a way to read Fan RPM:
>
> #define HCI_FAN_RPM 0x45
>
> This one is weird. On windows I have observed the cooling self test program
> (which supposedly verifies that the cooling is working correctly) calling
> this a few different ways. Here is a summary of what I managed to figure
> out:
>
> HCI_SET, 0x45, 0, 1, 0, 0: This sets the fan to run at max speed, but it
> will not be visible when reading /sys/bus/acpi/devices/TOS6208:00/fan.
> I will refer to this operation as "set-max-fan" below.
>
> The only way I found to stop it running at max RPM is to use HCI_FAN
> (e.g. 0 > /sys/bus/acpi/devices/TOS6208:00/fan or call the ACPI method
> directly).
>
> However the get method is more interesting:
>
> HCI_GET, 0x45, 0, 1, 0, 0 returns: {0x0, 0x45, fan_rpm, 0x1db0, 0x0, 0x0}
>
> I believe fan_rpm is accurate, without any scaling factors needed:
> * It behaves properly (higher value when fan is louder/faster/higher
> pitched, 0 when fan is off).
> * It matches the value range reported by HwInfo64 on Windows (which seems
> to be able to read this, I have not looked into how it does that).
> * Unfortunately there is no tool by Toshiba that exposes the numerical
> value that I can find (that would have been ideal). Nor is it shown in
> BIOS. The Windows tools "Toshiba PC Health Monitor" reports everything in
> percentages. Yes even the temperatures!
> * It is definitely a loud and whiny fan, even by laptop standards, so the
> high reported RPM range of 3540-7600 RPM could make sense. Though it did
> seem a bit high.
> * Finally, to be sure, I borrowed a tachometer from work. Yes, the fan
> really spins that fast. Byt it is only 30 mm, so I guess that makes
> sense.
>
> HCI_GET 0x45, 0, 0, 0, 0 returns: {0x0, 0x45, fan_rpm, 0x0, 0x0, 0x0}
>
> The Windows software does *not* use this variant as far as I have observed.
> It appears to work the same except that it doesn't return 0x1db0 in index 3.
>
> I'm not sure, but I strongly suspect 0x1db0 could be the max RPM (7600).
> The most I have observed when using "set-max-fan" is 0x1da6 (7590 RPM),
> which is very close. Note that this is significantly more than I can get
> using just HCI_FAN, which seems to max out at 0x17ac (6060 RPM).
>
>
> Patches
> =======
>
> I'm not personally particularly interested in user space control of fan
> speed, plus the fact that there is a way to make the fan go faster than
> the *other* max speed makes me wonder about the safety of running the fan
> at that speed for prolonged periods of time. Thus, I have only added a
> read-only hwmon interface for reading the fan RPM.
>
> I elected to use the same call that the Windows code does, which fetches
> what I believe is the max RPM. I think it is safer to stay as close as
> possible to that code. However I don't currently make use of this value,
> suggestions for where to use it are welcome.
>
> Note! I assume that if the FAN RPM call do not result in an error, that
> it is in fact supported. This may not be true. I would welcome testing by
> anyone who owns a Toshiba laptop!
>
> Best regards,
> Arvid Norlander
>
> Arvid Norlander (2):
> platform/x86: toshiba_acpi: Add fan RPM reading (internals)
> platform/x86: toshiba_acpi: Add fan RPM reading (hwmon interface)
Thank you for your patch-series, I've applied the series 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
^ permalink raw reply [flat|nested] 5+ messages in thread