Integrates voltage and temperature monitoring into the driver via the hwmon
subsystem, enabling standardized reporting via tools like lm-sensors.

Signed-off-by: Yen-Chi Huang <jesse.huang@portwell.com.tw>
---

This version removes board-specific details such as labels and voltage
scaling factors. The driver now follows a generic approach, delegating
customization to userspace (e.g. via sensors.conf in lm-sensors), which
is the standard and more flexible practice.

v3->v4:
  - Remove .driver_data and related per-board hwmon data structures.
  - Remove unused pwec_hwmon_is_visible() and pwec_hwmon_init().
  - Clarify driver header comments.

V2->V3:
  - Replace hardcoded `1000` with `MILLIDEGREE_PER_DEGREE` and double check
  - Fix comma placement and spacing coding style issues
  - Simplify pwec_hwmon_is_visible() with ternary operator

V1->V2:
  - Removed `msb_reg` from `struct pwec_hwmon_data`
  - Updated `pwec_read16_stable()` to assume MSB follows LSB
  - Moved `hwmon_channel_info` to per-board data and assigned it to `.info` at runtime
  - Replaced the `pwec_board_data[]` array with a standalone struct
  - Replaced literal `1000` with `MILLIDEGREE_PER_DEGREE`
  - Removed unused include and sorted header includes

Previous versions (for reference):
  v3 patch: https://lore.kernel.org/platform-driver-x86/d6429164-46dc-4c0d-8d6f-4650e0b92f22@portwell.com.tw/

---
 drivers/platform/x86/portwell-ec.c | 84 ++++++++++++++++++++++++++++--
 1 file changed, 79 insertions(+), 5 deletions(-)

diff --git a/drivers/platform/x86/portwell-ec.c b/drivers/platform/x86/portwell-ec.c
index 322f296e9315..0e748bfe4a23 100644
--- a/drivers/platform/x86/portwell-ec.c
+++ b/drivers/platform/x86/portwell-ec.c
@@ -5,15 +5,13 @@
  * Tested on:
  *  - Portwell NANO-6064
  *
- * This driver provides support for GPIO and Watchdog Timer
- * functionalities of the Portwell boards with ITE embedded controller (EC).
+ * This driver supports Portwell boards with an ITE embedded controller (EC).
  * The EC is accessed through I/O ports and provides:
+ *  - Temperature and voltage readings (hwmon)
  *  - 8 GPIO pins for control and monitoring
  *  - Hardware watchdog with 1-15300 second timeout range
  *
- * It integrates with the Linux GPIO and Watchdog subsystems, allowing
- * userspace interaction with EC GPIO pins and watchdog control,
- * ensuring system stability and configurability.
+ * It integrates with the Linux hwmon, GPIO and Watchdog subsystems.
  *
  * (C) Copyright 2025 Portwell, Inc.
  * Author: Yen-Chi Huang (jesse.huang@portwell.com.tw)
@@ -25,6 +23,7 @@
 #include <linux/bitfield.h>
 #include <linux/dmi.h>
 #include <linux/gpio/driver.h>
+#include <linux/hwmon.h>
 #include <linux/init.h>
 #include <linux/io.h>
 #include <linux/ioport.h>
@@ -32,6 +31,7 @@
 #include <linux/platform_device.h>
 #include <linux/sizes.h>
 #include <linux/string.h>
+#include <linux/units.h>
 #include <linux/watchdog.h>
 
 #define PORTWELL_EC_IOSPACE              0xe300
@@ -52,6 +52,9 @@
 #define PORTWELL_EC_FW_VENDOR_LENGTH     3
 #define PORTWELL_EC_FW_VENDOR_NAME       "PWG"
 
+#define PORTWELL_EC_ADC_VREF             3000
+#define PORTWELL_EC_ADC_MAX              1023
+
 static bool force;
 module_param(force, bool, 0444);
 MODULE_PARM_DESC(force, "Force loading EC driver without checking DMI boardname");
@@ -79,6 +82,20 @@ static u8 pwec_read(u8 address)
 	return inb(PORTWELL_EC_IOSPACE + address);
 }
 
+/* Ensure consistent 16-bit read across potential MSB rollover. */
+static u16 pwec_read16_stable(u8 lsb_reg)
+{
+	u8 lsb, msb, old_msb;
+
+	do {
+		old_msb = pwec_read(lsb_reg + 1);
+		lsb = pwec_read(lsb_reg);
+		msb = pwec_read(lsb_reg + 1);
+	} while (msb != old_msb);
+
+	return (msb << 8) | lsb;
+}
+
 /* GPIO functions */
 
 static int pwec_gpio_get(struct gpio_chip *chip, unsigned int offset)
@@ -204,6 +221,54 @@ static struct watchdog_device ec_wdt_dev = {
 	.max_timeout = PORTWELL_WDT_EC_MAX_COUNT_SECOND,
 };
 
+/* HWMON functions */
+
+static const u8 pwec_hwmon_temp_regs[] = { 0x0, 0x2, 0x4 };
+static const u8 pwec_hwmon_in_regs[] = { 0x20, 0x22, 0x24, 0x30, 0x32 };
+
+static int pwec_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
+			   u32 attr, int channel, long *val)
+{
+	u8 tmp8;
+	u16 tmp16;
+
+	switch (type) {
+	case hwmon_temp:
+		tmp8 = pwec_read(pwec_hwmon_temp_regs[channel]);
+		*val = tmp8 * MILLIDEGREE_PER_DEGREE;
+		return 0;
+	case hwmon_in:
+		tmp16 = pwec_read16_stable(pwec_hwmon_in_regs[channel]);
+		*val = (tmp16 * PORTWELL_EC_ADC_VREF) / PORTWELL_EC_ADC_MAX;
+		return 0;
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static const struct hwmon_channel_info *pwec_hwmon_info[] = {
+	HWMON_CHANNEL_INFO(temp,
+		HWMON_T_INPUT,
+		HWMON_T_INPUT,
+		HWMON_T_INPUT),
+	HWMON_CHANNEL_INFO(in,
+		HWMON_I_INPUT,
+		HWMON_I_INPUT,
+		HWMON_I_INPUT,
+		HWMON_I_INPUT,
+		HWMON_I_INPUT),
+	NULL
+};
+
+static const struct hwmon_ops pwec_hwmon_ops = {
+	.read = pwec_hwmon_read,
+};
+
+static struct hwmon_chip_info pwec_hwmon_chip_info = {
+	.ops = &pwec_hwmon_ops,
+	.info = pwec_hwmon_info,
+};
+
 static int pwec_firmware_vendor_check(void)
 {
 	u8 buf[PORTWELL_EC_FW_VENDOR_LENGTH + 1];
@@ -218,6 +283,7 @@ static int pwec_firmware_vendor_check(void)
 
 static int pwec_probe(struct platform_device *pdev)
 {
+	struct device *hwmon_dev;
 	int ret;
 
 	if (!devm_request_region(&pdev->dev, PORTWELL_EC_IOSPACE,
@@ -236,6 +302,14 @@ static int pwec_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	if (IS_REACHABLE(CONFIG_HWMON)) {
+		hwmon_dev = devm_hwmon_device_register_with_info(&pdev->dev,
+				"portwell-ec", NULL, &pwec_hwmon_chip_info, NULL);
+		ret = PTR_ERR_OR_ZERO(hwmon_dev);
+		if (ret)
+			return ret;
+	}
+
 	ec_wdt_dev.parent = &pdev->dev;
 	ret = devm_watchdog_register_device(&pdev->dev, &ec_wdt_dev);
 	if (ret < 0) {
-- 
2.34.1

On Thu, Sep 04, 2025 at 05:29:23PM +0800, Yen-Chi Huang wrote:
> Integrates voltage and temperature monitoring into the driver via the hwmon
> subsystem, enabling standardized reporting via tools like lm-sensors.
> 
> Signed-off-by: Yen-Chi Huang <jesse.huang@portwell.com.tw>
> ---
> 
> This version removes board-specific details such as labels and voltage
> scaling factors. The driver now follows a generic approach, delegating
> customization to userspace (e.g. via sensors.conf in lm-sensors), which
> is the standard and more flexible practice.

FWIW, this reasoning is wrong. sensors.conf is intended to be used for
situations where board specific scaling factors are _not_ known.

Guenter

On 9/4/25 02:29, Yen-Chi Huang wrote:
> Integrates voltage and temperature monitoring into the driver via the hwmon
> subsystem, enabling standardized reporting via tools like lm-sensors.
> 
> Signed-off-by: Yen-Chi Huang <jesse.huang@portwell.com.tw>
> ---
> 
...

> +
> +static const struct hwmon_ops pwec_hwmon_ops = {
> +	.read = pwec_hwmon_read,
> +};

And this works ? That would be quite surprising.

Guenter

Hi Guenter,

Apologies for the error.

In v4 I mistakenly removed '.is_visible' while simplifying and then
incorrectly tested against an older tree.

A corrected '.is_visible' will be restored in v5.

Thanks for the review.

Best regards,
Yen-Chi Huang

On 9/4/25 22:35, Yen-Chi Huang wrote:
> Hi Guenter,
> 
> Apologies for the error.
> 
> In v4 I mistakenly removed '.is_visible' while simplifying and then
> incorrectly tested against an older tree.
> 
> A corrected '.is_visible' will be restored in v5.
> 

I don't know what the function does. If it sets a constant value,
you could use .visible instead.

Guenter