Re: [PATCH 2/2] [RFC] platform/x86: toshiba_acpi: Add fan RPM reading (hwmon interface)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 9/1/22 07:49, 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@xxxxxxxxx>
---
  drivers/platform/x86/Kconfig        |  1 +
  drivers/platform/x86/toshiba_acpi.c | 49 +++++++++++++++++++++++++++++
  2 files changed, 50 insertions(+)

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index f2f98e942cf2..9a98974ab8bf 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -799,6 +799,7 @@ config ACPI_TOSHIBA
  	depends on ACPI_VIDEO || ACPI_VIDEO = n
  	depends on RFKILL || RFKILL = n
  	depends on IIO
+	select HWMON

This is wrong. I know other drivers in this directory do it, but it is
still wrong. It should be something like

	depends on HWMON || HWMON=n

and the code should deal with the conditionals.

  	select INPUT_SPARSEKMAP
  	help
  	  This driver adds support for access to certain system settings
diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
index 02e3522f4eeb..2b71dac34cf0 100644
--- a/drivers/platform/x86/toshiba_acpi.c
+++ b/drivers/platform/x86/toshiba_acpi.c
@@ -39,6 +39,7 @@
  #include <linux/i8042.h>
  #include <linux/acpi.h>
  #include <linux/dmi.h>
+#include <linux/hwmon.h>
  #include <linux/uaccess.h>
  #include <linux/miscdevice.h>
  #include <linux/rfkill.h>
@@ -171,6 +172,7 @@ struct toshiba_acpi_dev {
  	struct miscdevice miscdev;
  	struct rfkill *wwan_rfk;
  	struct iio_dev *indio_dev;
+	struct device *hwmon_device;
int force_fan;
  	int last_key_event;
@@ -2941,6 +2943,38 @@ static int toshiba_acpi_setup_backlight(struct toshiba_acpi_dev *dev)
  	return 0;
  }
+
+/* HWMON support for fan */
+static ssize_t fan_fan1_input_show(struct device *dev,
+				   struct device_attribute *attr,
+				   char *buf)
+{
+	u32 value;
+	int ret;
+
+	ret = get_fan_rpm(toshiba_acpi, &value);
+	if (ret)
+		return ret;
+
+	return sysfs_emit(buf, "%u\n", value);
+}
+
+static DEVICE_ATTR(fan1_input, S_IRUGO, fan_fan1_input_show, NULL);
+
+static struct attribute *fan_attributes[] = {
+	&dev_attr_fan1_input.attr,
+	NULL
+};
+
+static const struct attribute_group fan_attr_group = {
+	.attrs = fan_attributes,
+};
+
+static const struct attribute_group *toshiba_acpi_hwmon_groups[] = {
+	&fan_attr_group,
+	NULL,
+};
+
  static void print_supported_features(struct toshiba_acpi_dev *dev)
  {
  	pr_info("Supported laptop features:");
@@ -2995,6 +3029,9 @@ static int toshiba_acpi_remove(struct acpi_device *acpi_dev)
remove_toshiba_proc_entries(dev); + if (dev->hwmon_device)
+		hwmon_device_unregister(dev->hwmon_device);
+
  	if (dev->accelerometer_supported && dev->indio_dev) {
  		iio_device_unregister(dev->indio_dev);
  		iio_device_free(dev->indio_dev);
@@ -3187,6 +3224,18 @@ static int toshiba_acpi_add(struct acpi_device *acpi_dev)
  	ret = get_fan_rpm(dev, &dummy);
  	dev->fan_rpm_supported = !ret;
+ if (dev->fan_rpm_supported) {
+		dev->hwmon_device = hwmon_device_register_with_groups(

New drivers should register using [devm_]hwmon_device_register_with_info().

+			&dev->acpi_dev->dev, "toshiba_acpi_sensors", NULL,
+			toshiba_acpi_hwmon_groups);
+		if (IS_ERR(dev->hwmon_device)) {
+			ret = PTR_ERR(dev->hwmon_device);
+			dev->hwmon_device = NULL;
+			pr_err("unable to register hwmon device\n");
+			goto error;

The driver works just fine without hwmon, and should not fail to probe
if hwmon registration fails. It did not fail before this patch was applied
either, and hwmon is not essential functionality for this driver.

Guenter

+		}
+	}
+
  	toshiba_wwan_available(dev);
  	if (dev->wwan_supported)
  		toshiba_acpi_setup_wwan_rfkill(dev);




[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux