[PATCH v2 0/2] Add driver support for ESWIN EIC7700 PVT controller

[PATCH v2 0/2] Add driver support for ESWIN EIC7700 PVT controller

[hehuan1]

From: Huan He <hehuan1@eswincomputing.com>

Add support for the ESWIN EIC7700 PVT (Process, Voltage, Temperature)
sensor

Features:
The driver supports monitoring of process, voltage and temperature
parameters through the hardware monitoring subsystem. It provides an
access to the sampled Temperature and Voltage.

Test:
Tested this patch on the SiFive HiFive Premier P550 (which uses the ESWIN
EIC7700 SoC).

Updates:

  Changes in v2:
  - Update eswin,eic7700-pvt.yaml
    - Reference the hwmon-common.yaml file
    - Remove the clock-names and reset-names properties
    - Move additionalProperties: false after the required block
    - Remove one example node to avoid redundancy
  - Update eic7700-pvt.c and eic7700-pvt.h
    - Remove unused sensor macros (PVT_SENSOR_FIRST, PVT_SENSOR_LAST,
      PVT_SENSORS_NUM)
    - Drop the unnecessary hwmon-sysfs.h header
    - Replace dynamic sensor info allocation with a static array and unify
      sensor labels
    - Remove unused hwmon_temp_type attribute
    - Eliminate redundant validation checks
    - Remove mutex and related locking, relying on hwmon core
      serialization
    - Replace per-sensor caches and completions with a single data cache
      and completion object
    - Remove pvt->sensor tracking. ISR no longer depends on the currently
      selected sensor
    - Move devm_add_action() registration after init_completion() for
      safer cleanup, and update cleanup function (pvt_clear_data)
    - Replace devm_reset_control_get_optional_exclusive() with
      devm_reset_control_get_exclusive_deasserted()
    - Replace eic7700_pvt_remove() with eic7700_pvt_disable_pm_runtime()
      and move it after PM runtime enable to avoid resource leaks on probe
      failure and remove clock disable and reset assert from
      eic7700_pvt_disable_pm_runtime() as it is already handled by devm_*
      framework
    - Remove redundant clock presence check in runtime_resume

  - Link to v1: https://lore.kernel.org/all/20260109090718.442-1-hehuan1@eswincomputing.com/

Huan He (2):
  dt-bindings: hwmon: Add Eswin EIC7700 PVT sensor
  hwmon: Add Eswin EIC7700 PVT sensor driver

 .../bindings/hwmon/eswin,eic7700-pvt.yaml     |  70 ++
 drivers/hwmon/Kconfig                         |  12 +
 drivers/hwmon/Makefile                        |   1 +
 drivers/hwmon/eic7700-pvt.c                   | 612 ++++++++++++++++++
 drivers/hwmon/eic7700-pvt.h                   | 106 +++
 5 files changed, 801 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/eswin,eic7700-pvt.yaml
 create mode 100644 drivers/hwmon/eic7700-pvt.c
 create mode 100644 drivers/hwmon/eic7700-pvt.h

-- 
2.25.1

[PATCH v2 1/2] dt-bindings: hwmon: Add Eswin EIC7700 PVT sensor

[hehuan1]

From: Huan He <hehuan1@eswincomputing.com>

Add device tree binding documentation for ESWIN EIC7700 Process, Voltage
and Temperature sensor.

The EIC7700 SoC integrates two PVT instances for monitoring SoC and DDR
power domains respectively.

Signed-off-by: Yulin Lu <luyulin@eswincomputing.com>
Signed-off-by: Huan He <hehuan1@eswincomputing.com>
---
 .../bindings/hwmon/eswin,eic7700-pvt.yaml     | 70 +++++++++++++++++++
 1 file changed, 70 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/eswin,eic7700-pvt.yaml

diff --git a/Documentation/devicetree/bindings/hwmon/eswin,eic7700-pvt.yaml b/Documentation/devicetree/bindings/hwmon/eswin,eic7700-pvt.yaml
new file mode 100644
index 000000000000..f4ba228924fe
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/eswin,eic7700-pvt.yaml
@@ -0,0 +1,70 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/hwmon/eswin,eic7700-pvt.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ESWIN EIC7700 PVT Sensor
+
+maintainers:
+  - Yulin Lu <luyulin@eswincomputing.com>
+  - Huan He <hehuan1@eswincomputing.com>
+
+description:
+  ESWIN EIC7700 SoC integrates embedded process, voltage and temperature
+  sensors to monitor the internal SoC environment. The system includes two
+  PVT sensor instances. The PVT0 monitors the main SoC power domain. The
+  PVT1 sensor monitors the DDR core power domain.
+
+allOf:
+  - $ref: /schemas/hwmon/hwmon-common.yaml#
+
+properties:
+  compatible:
+    const: eswin,eic7700-pvt
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  label:
+    description:
+      Human readable identifier used to distinguish between different PVT
+      instances. Typically "pvt0" for SoC PVT sensor and "pvt1" for DDR
+      core PVT sensor.
+
+  resets:
+    maxItems: 1
+
+  '#thermal-sensor-cells':
+    description: Thermal sensor cells if used for thermal sensoring.
+    const: 0
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - interrupts
+  - label
+  - resets
+
+additionalProperties: false
+
+examples:
+  - |
+    pvt@50b00000 {
+      compatible = "eswin,eic7700-pvt";
+      reg = <0x50b00000 0x10000>;
+      clocks = <&clocks 244>;
+      interrupts = <349>;
+      interrupt-parent = <&plic>;
+      label = "pvt0";
+      resets = <&reset 111>;
+      #thermal-sensor-cells = <0>;
+    };
+...
-- 
2.25.1

[PATCH v2 2/2] hwmon: Add Eswin EIC7700 PVT sensor driver

[hehuan1]

From: Huan He <hehuan1@eswincomputing.com>

Add support for ESWIN EIC7700 Process, Voltage and Temperature sensor. The
driver supports temperature and voltage monitoring with polynomial
conversion, and provides sysfs interface for sensor data access.

The PVT IP contains one temperature sensor and four voltage sensors for
process variation monitoring.

Signed-off-by: Yulin Lu <luyulin@eswincomputing.com>
Signed-off-by: Huan He <hehuan1@eswincomputing.com>
---
 drivers/hwmon/Kconfig       |  12 +
 drivers/hwmon/Makefile      |   1 +
 drivers/hwmon/eic7700-pvt.c | 612 ++++++++++++++++++++++++++++++++++++
 drivers/hwmon/eic7700-pvt.h | 106 +++++++
 4 files changed, 731 insertions(+)
 create mode 100644 drivers/hwmon/eic7700-pvt.c
 create mode 100644 drivers/hwmon/eic7700-pvt.h

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 157678b821fc..de04ef41bcd9 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -2043,6 +2043,18 @@ config SENSORS_DME1737
 	  This driver can also be built as a module. If so, the module
 	  will be called dme1737.
 
+config SENSORS_EIC7700_PVT
+	tristate "Eswin EIC7700 Process, Voltage, Temperature sensor driver"
+	depends on ARCH_ESWIN || COMPILE_TEST
+	depends on HWMON
+	select POLYNOMIAL
+	help
+	  If you say yes here you get support for Eswin EIC7700 PVT sensor
+	  embedded into the SoC.
+
+	  This driver can also be built as a module. If so, the module will be
+	  called eic7700-pvt.
+
 config SENSORS_EMC1403
 	tristate "SMSC EMC1403/23 thermal sensor"
 	depends on I2C
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index eade8e3b1bde..1b2845bd5a54 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -72,6 +72,7 @@ obj-$(CONFIG_SENSORS_DME1737)	+= dme1737.o
 obj-$(CONFIG_SENSORS_DRIVETEMP)	+= drivetemp.o
 obj-$(CONFIG_SENSORS_DS620)	+= ds620.o
 obj-$(CONFIG_SENSORS_DS1621)	+= ds1621.o
+obj-$(CONFIG_SENSORS_EIC7700_PVT) += eic7700-pvt.o
 obj-$(CONFIG_SENSORS_EMC1403)	+= emc1403.o
 obj-$(CONFIG_SENSORS_EMC2103)	+= emc2103.o
 obj-$(CONFIG_SENSORS_EMC2305)	+= emc2305.o
diff --git a/drivers/hwmon/eic7700-pvt.c b/drivers/hwmon/eic7700-pvt.c
new file mode 100644
index 000000000000..faee3fe39161
--- /dev/null
+++ b/drivers/hwmon/eic7700-pvt.c
@@ -0,0 +1,612 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ESWIN EIC7700 Process, Voltage, Temperature sensor driver
+ *
+ * Copyright 2026, Beijing ESWIN Computing Technology Co., Ltd.
+ *
+ * Authors:
+ *   Yulin Lu <luyulin@eswincomputing.com>
+ *   Huan He <hehuan1@eswincomputing.com>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/polynomial.h>
+#include <linux/reset.h>
+#include "eic7700-pvt.h"
+
+static const struct pvt_sensor_info pvt_info[] = {
+	PVT_SENSOR_INFO(0, "Temperature", hwmon_temp, TEMP),
+	PVT_SENSOR_INFO(0, "Voltage", hwmon_in, VOLT),
+	PVT_SENSOR_INFO(1, "Low-Vt", hwmon_in, LVT),
+	PVT_SENSOR_INFO(2, "UltraLow-Vt", hwmon_in, ULVT),
+	PVT_SENSOR_INFO(3, "Standard-Vt", hwmon_in, SVT),
+};
+
+/*
+ * The original translation formulae of the temperature (in degrees of Celsius)
+ * to PVT data and vice-versa are following:
+ * N = 6.0818e-8*(T^4) +1.2873e-5*(T^3) + 7.2244e-3*(T^2) + 3.6484*(T^1) +
+ *     1.6198e2,
+ * T = -1.8439e-11*(N^4) + 8.0705e-8*(N^3) + -1.8501e-4*(N^2) +
+ *     3.2843e-1*(N^1) - 4.8690e1,
+ * where T = [-40, 125]C and N = [27, 771].
+ * They must be accordingly altered to be suitable for the integer arithmetics.
+ * The technique is called 'factor redistribution', which just makes sure the
+ * multiplications and divisions are made so to have a result of the operations
+ * within the integer numbers limit. In addition we need to translate the
+ * formulae to accept millidegrees of Celsius. Here what they look like after
+ * the alterations:
+ * N = (60818e-20*(T^4) + 12873e-14*(T^3) + 72244e-9*(T^2) + 36484e-3*T +
+ *     16198e2) / 1e4,
+ * T = -18439e-12*(N^4) + 80705e-9*(N^3) - 185010e-6*(N^2) + 328430e-3*N -
+ *     48690,
+ * where T = [-40000, 125000] mC and N = [27, 771].
+ */
+static const struct polynomial poly_N_to_temp = {
+	.total_divider = 1,
+	.terms = {
+		{4, -18439, 1000, 1},
+		{3, 80705, 1000, 1},
+		{2, -185010, 1000, 1},
+		{1, 328430, 1000, 1},
+		{0, -48690, 1, 1}
+	}
+};
+
+/*
+ * Similar alterations are performed for the voltage conversion equations.
+ * The original formulae are:
+ * N = 1.3905e3*V - 5.7685e2,
+ * V = (N + 5.7685e2) / 1.3905e3,
+ * where V = [0.72, 0.88] V and N = [424, 646].
+ * After the optimization they looks as follows:
+ * N = (13905e-3*V - 5768.5) / 10,
+ * V = (N * 10^5 / 13905 + 57685 * 10^3 / 13905) / 10.
+ * where V = [720, 880] mV and N = [424, 646].
+ */
+static const struct polynomial poly_N_to_volt = {
+	.total_divider = 10,
+	.terms = {
+		{1, 100000, 13905, 1},
+		{0, 57685000, 1, 13905}
+	}
+};
+
+static inline u32 eic7700_pvt_update(void __iomem *reg, u32 mask, u32 data)
+{
+	u32 old;
+
+	old = readl_relaxed(reg);
+	writel((old & ~mask) | (data & mask), reg);
+
+	return old & mask;
+}
+
+static inline void eic7700_pvt_set_mode(struct pvt_hwmon *pvt, u32 mode)
+{
+	u32 old;
+
+	mode = FIELD_PREP(PVT_MODE_MASK, mode);
+
+	old = eic7700_pvt_update(pvt->regs + PVT_ENA, PVT_ENA_EN, 0);
+	eic7700_pvt_update(pvt->regs + PVT_MODE, PVT_MODE_MASK, mode);
+	eic7700_pvt_update(pvt->regs + PVT_ENA, PVT_ENA_EN, old);
+}
+
+static inline void eic7700_pvt_set_trim(struct pvt_hwmon *pvt, u32 val)
+{
+	u32 old;
+
+	old = eic7700_pvt_update(pvt->regs + PVT_ENA, PVT_ENA_EN, 0);
+	writel(val, pvt->regs + PVT_TRIM);
+	eic7700_pvt_update(pvt->regs + PVT_ENA, PVT_ENA_EN, old);
+}
+
+static irqreturn_t eic7700_pvt_hard_isr(int irq, void *data)
+{
+	struct pvt_hwmon *pvt = data;
+	u32 val;
+
+	eic7700_pvt_update(pvt->regs + PVT_INT, PVT_INT_CLR, PVT_INT_CLR);
+	/*
+	 * Read the data, update the cache and notify a waiter of this event.
+	 */
+	val = readl(pvt->regs + PVT_DATA);
+	WRITE_ONCE(pvt->data_cache, FIELD_GET(PVT_DATA_OUT, val));
+	complete(&pvt->conversion);
+
+	return IRQ_HANDLED;
+}
+
+static int eic7700_pvt_read_data(struct pvt_hwmon *pvt,
+				 enum pvt_sensor_type type, long *val)
+{
+	u32 data;
+	int ret;
+
+	/*
+	 * Wait for PVT conversion to complete and update the data cache. The
+	 * data read procedure is following: set the requested PVT sensor mode,
+	 * enable conversion, wait until conversion is finished, then disable
+	 * conversion and IRQ, and read the cached data.
+	 */
+	reinit_completion(&pvt->conversion);
+
+	eic7700_pvt_set_mode(pvt, pvt_info[type].mode);
+	eic7700_pvt_update(pvt->regs + PVT_ENA, PVT_ENA_EN, PVT_ENA_EN);
+
+	ret = wait_for_completion_interruptible(&pvt->conversion);
+
+	eic7700_pvt_update(pvt->regs + PVT_ENA, PVT_ENA_EN, 0);
+	eic7700_pvt_update(pvt->regs + PVT_INT, PVT_INT_CLR, PVT_INT_CLR);
+
+	data = READ_ONCE(pvt->data_cache);
+
+	if (ret && (ret != -ERESTARTSYS))
+		return ret;
+
+	if (type == PVT_TEMP)
+		*val = polynomial_calc(&poly_N_to_temp, data);
+	else if (type == PVT_VOLT)
+		*val = polynomial_calc(&poly_N_to_volt, data);
+	else
+		*val = data;
+
+	return 0;
+}
+
+static const struct hwmon_channel_info *pvt_channel_info[] = {
+	HWMON_CHANNEL_INFO(chip, HWMON_C_REGISTER_TZ),
+	HWMON_CHANNEL_INFO(temp,
+			   HWMON_T_INPUT | HWMON_T_LABEL | HWMON_T_OFFSET),
+	HWMON_CHANNEL_INFO(in,
+			   HWMON_I_INPUT | HWMON_I_LABEL,
+			   HWMON_I_INPUT | HWMON_I_LABEL,
+			   HWMON_I_INPUT | HWMON_I_LABEL,
+			   HWMON_I_INPUT | HWMON_I_LABEL),
+	NULL
+};
+
+static umode_t eic7700_pvt_hwmon_is_visible(const void *data,
+					    enum hwmon_sensor_types type,
+					    u32 attr, int ch)
+{
+	switch (type) {
+	case hwmon_temp:
+		switch (attr) {
+		case hwmon_temp_input:
+		case hwmon_temp_label:
+			return 0444;
+		case hwmon_temp_offset:
+			return 0644;
+		}
+		break;
+	case hwmon_in:
+		switch (attr) {
+		case hwmon_in_input:
+		case hwmon_in_label:
+			return 0444;
+		}
+		break;
+	default:
+		break;
+	}
+
+	return 0;
+}
+
+static int eic7700_pvt_read_trim(struct pvt_hwmon *pvt, long *val)
+{
+	u32 data;
+
+	data = readl(pvt->regs + PVT_TRIM);
+	*val = data;
+
+	return 0;
+}
+
+static int eic7700_pvt_write_trim(struct pvt_hwmon *pvt, long val)
+{
+	/*
+	 * Update PVT trim register safely while the controller is temporarily
+	 * disabled.
+	 */
+	eic7700_pvt_set_trim(pvt, val);
+
+	return 0;
+}
+
+static int eic7700_pvt_hwmon_read(struct device *dev,
+				  enum hwmon_sensor_types type, u32 attr,
+				  int ch, long *val)
+{
+	struct pvt_hwmon *pvt = dev_get_drvdata(dev);
+	int ret;
+
+	ret = pm_runtime_get_sync(pvt->dev);
+	if (ret < 0) {
+		dev_err(pvt->dev, "Failed to resume PVT device: %d\n", ret);
+		return ret;
+	}
+
+	switch (type) {
+	case hwmon_temp:
+		switch (attr) {
+		case hwmon_temp_input:
+			ret = eic7700_pvt_read_data(pvt, ch, val);
+			break;
+		case hwmon_temp_offset:
+			ret = eic7700_pvt_read_trim(pvt, val);
+			break;
+		default:
+			ret = -EOPNOTSUPP;
+		}
+		break;
+	case hwmon_in:
+		if (attr == hwmon_in_input)
+			ret = eic7700_pvt_read_data(pvt, PVT_VOLT + ch, val);
+		else
+			ret = -EOPNOTSUPP;
+		break;
+	default:
+		ret = -EOPNOTSUPP;
+	}
+
+	pm_runtime_mark_last_busy(pvt->dev);
+	pm_runtime_put_autosuspend(pvt->dev);
+	return ret;
+}
+
+static int eic7700_pvt_hwmon_read_string(struct device *dev,
+					 enum hwmon_sensor_types type, u32 attr,
+					 int ch, const char **str)
+{
+	switch (type) {
+	case hwmon_temp:
+		if (attr == hwmon_temp_label) {
+			*str = pvt_info[ch].label;
+			return 0;
+		}
+		break;
+	case hwmon_in:
+		if (attr == hwmon_in_label) {
+			*str = pvt_info[PVT_VOLT + ch].label;
+			return 0;
+		}
+		break;
+	default:
+		break;
+	}
+
+	return -EOPNOTSUPP;
+}
+
+static int eic7700_pvt_hwmon_write(struct device *dev,
+				   enum hwmon_sensor_types type, u32 attr,
+				   int ch, long val)
+{
+	struct pvt_hwmon *pvt = dev_get_drvdata(dev);
+	int ret;
+
+	ret = pm_runtime_get_sync(pvt->dev);
+	if (ret < 0) {
+		dev_err(pvt->dev, "Failed to resume PVT device: %d\n", ret);
+		return ret;
+	}
+
+	if (type == hwmon_temp && attr == hwmon_temp_offset)
+		ret = eic7700_pvt_write_trim(pvt, val);
+	else
+		ret = -EOPNOTSUPP;
+
+	pm_runtime_mark_last_busy(pvt->dev);
+	pm_runtime_put_autosuspend(pvt->dev);
+	return ret;
+}
+
+static const struct hwmon_ops pvt_hwmon_ops = {
+	.is_visible = eic7700_pvt_hwmon_is_visible,
+	.read = eic7700_pvt_hwmon_read,
+	.read_string = eic7700_pvt_hwmon_read_string,
+	.write = eic7700_pvt_hwmon_write
+};
+
+static const struct hwmon_chip_info pvt_hwmon_info = {
+	.ops = &pvt_hwmon_ops,
+	.info = pvt_channel_info
+};
+
+static void pvt_clear_data(void *data)
+{
+	struct pvt_hwmon *pvt = data;
+
+	complete_all(&pvt->conversion);
+}
+
+static struct pvt_hwmon *eic7700_pvt_create_data(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct pvt_hwmon *pvt;
+	int ret;
+
+	pvt = devm_kzalloc(dev, sizeof(*pvt), GFP_KERNEL);
+	if (!pvt)
+		return ERR_PTR(-ENOMEM);
+
+	pvt->dev = dev;
+	init_completion(&pvt->conversion);
+
+	ret = devm_add_action(dev, pvt_clear_data, pvt);
+	if (ret) {
+		dev_err(dev, "Can't add PVT data clear action\n");
+		return ERR_PTR(ret);
+	}
+
+	return pvt;
+}
+
+static int eic7700_pvt_check_pwr(struct pvt_hwmon *pvt)
+{
+	unsigned long tout;
+	int ret = 0;
+
+	/*
+	 * Test out the sensor conversion functionality. If it is not done on
+	 * time then the domain must have been unpowered and we won't be able
+	 * to use the device later in this driver.
+	 */
+	eic7700_pvt_update(pvt->regs + PVT_INT, PVT_INT_CLR, PVT_INT_CLR);
+	eic7700_pvt_update(pvt->regs + PVT_ENA, PVT_ENA_EN, PVT_ENA_EN);
+	readl(pvt->regs + PVT_DATA);
+
+	tout = PVT_TOUT_MIN / NSEC_PER_USEC;
+	usleep_range(tout, 2 * tout);
+
+	readl(pvt->regs + PVT_DATA);
+	if (!(readl(pvt->regs + PVT_INT) & PVT_INT_STAT)) {
+		ret = -ENODEV;
+		dev_err(pvt->dev,
+			"Sensor is powered down - no interrupt generated\n");
+	}
+
+	eic7700_pvt_update(pvt->regs + PVT_ENA, PVT_ENA_EN, 0);
+	eic7700_pvt_update(pvt->regs + PVT_INT, PVT_INT_CLR, PVT_INT_CLR);
+
+	return ret;
+}
+
+static int eic7700_pvt_init_iface(struct pvt_hwmon *pvt)
+{
+	unsigned long rate;
+
+	rate = clk_get_rate(pvt->clk);
+	if (!rate) {
+		dev_err(pvt->dev, "Invalid reference clock rate\n");
+		return -ENODEV;
+	}
+	/*
+	 * Make sure controller are disabled so not to accidentally have ISR
+	 * executed before the driver data is fully initialized. Clear the IRQ
+	 * status as well.
+	 */
+	eic7700_pvt_update(pvt->regs + PVT_INT, PVT_INT_CLR, PVT_INT_CLR);
+	eic7700_pvt_update(pvt->regs + PVT_ENA, PVT_ENA_EN, 0);
+	readl(pvt->regs + PVT_INT);
+	readl(pvt->regs + PVT_DATA);
+
+	/* Setup default sensor mode and temperature trim. */
+	eic7700_pvt_set_mode(pvt, pvt_info[PVT_TEMP].mode);
+
+	eic7700_pvt_set_trim(pvt, PVT_TRIM_DEF);
+
+	return 0;
+}
+
+static int eic7700_pvt_request_irq(struct pvt_hwmon *pvt)
+{
+	struct platform_device *pdev = to_platform_device(pvt->dev);
+	int ret;
+
+	pvt->irq = platform_get_irq(pdev, 0);
+	if (pvt->irq < 0)
+		return pvt->irq;
+
+	ret = devm_request_threaded_irq(pvt->dev, pvt->irq,
+					eic7700_pvt_hard_isr, NULL,
+					IRQF_SHARED | IRQF_TRIGGER_HIGH, "pvt",
+					pvt);
+	if (ret) {
+		dev_err(pvt->dev, "Couldn't request PVT IRQ\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static int eic7700_pvt_create_hwmon(struct pvt_hwmon *pvt)
+{
+	struct device *dev = pvt->dev;
+	struct device_node *np = dev->of_node;
+	const char *node_label;
+	int type;
+	const char *names[2] = {"soc_pvt", "ddr_pvt"};
+
+	if (of_property_read_string(np, "label", &node_label)) {
+		dev_err(dev, "Missing 'label' property in DTS node\n");
+		return -EINVAL;
+	}
+
+	if (strcmp(node_label, "pvt0") == 0) {
+		type = 0;
+	} else if (strcmp(node_label, "pvt1") == 0) {
+		type = 1;
+	} else {
+		dev_err(pvt->dev, "Unsupported label: %s\n", node_label);
+		return -EINVAL;
+	}
+
+	pvt->hwmon = devm_hwmon_device_register_with_info(pvt->dev, names[type],
+							  pvt, &pvt_hwmon_info,
+							  NULL);
+	if (IS_ERR(pvt->hwmon)) {
+		dev_err(pvt->dev, "Couldn't create hwmon device\n");
+		return PTR_ERR(pvt->hwmon);
+	}
+
+	return 0;
+}
+
+static void eic7700_pvt_disable_pm_runtime(void *data)
+{
+	struct pvt_hwmon *pvt = data;
+
+	pm_runtime_dont_use_autosuspend(pvt->dev);
+	pm_runtime_disable(pvt->dev);
+}
+
+static int eic7700_pvt_probe(struct platform_device *pdev)
+{
+	struct pvt_hwmon *pvt;
+	int ret;
+
+	pvt = eic7700_pvt_create_data(pdev);
+	if (IS_ERR(pvt))
+		return PTR_ERR(pvt);
+
+	platform_set_drvdata(pdev, pvt);
+
+	pvt->regs = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(pvt->regs))
+		return PTR_ERR(pvt->regs);
+
+	pvt->clk = devm_clk_get_enabled(&pdev->dev, NULL);
+	if (IS_ERR(pvt->clk))
+		return dev_err_probe(&pdev->dev, PTR_ERR(pvt->clk),
+				     "Couldn't get clock\n");
+
+	pvt->rst = devm_reset_control_get_exclusive_deasserted(&pdev->dev,
+							       NULL);
+	if (IS_ERR(pvt->rst))
+		return dev_err_probe(pvt->dev, PTR_ERR(pvt->rst),
+				     "Couldn't get reset control\n");
+
+	ret = eic7700_pvt_check_pwr(pvt);
+	if (ret)
+		return ret;
+
+	ret = eic7700_pvt_init_iface(pvt);
+	if (ret)
+		return ret;
+
+	ret = eic7700_pvt_request_irq(pvt);
+	if (ret)
+		return ret;
+
+	pm_runtime_enable(&pdev->dev);
+	pm_runtime_set_autosuspend_delay(&pdev->dev, 3000);
+	pm_runtime_use_autosuspend(&pdev->dev);
+	pm_runtime_get_noresume(&pdev->dev);
+
+	ret = devm_add_action_or_reset(pvt->dev, eic7700_pvt_disable_pm_runtime,
+				       pvt);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret,
+				     "Can't register PM cleanup\n");
+
+	ret = eic7700_pvt_create_hwmon(pvt);
+	if (ret)
+		return ret;
+
+	pm_runtime_put_autosuspend(&pdev->dev);
+
+	return 0;
+}
+
+static int __maybe_unused eic7700_pvt_runtime_resume(struct device *dev)
+{
+	struct pvt_hwmon *pvt = dev_get_drvdata(dev);
+	int ret;
+
+	ret = clk_prepare_enable(pvt->clk);
+	if (ret) {
+		dev_err(dev, "Failed to enable clock: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int __maybe_unused eic7700_pvt_runtime_suspend(struct device *dev)
+{
+	struct pvt_hwmon *pvt = dev_get_drvdata(dev);
+
+	clk_disable_unprepare(pvt->clk);
+
+	return 0;
+}
+
+static int __maybe_unused eic7700_pvt_suspend(struct device *dev)
+{
+	int ret = 0;
+
+	if (!pm_runtime_status_suspended(dev)) {
+		ret = eic7700_pvt_runtime_suspend(dev);
+		if (ret) {
+			dev_err(dev, "Failed to suspend: %d\n", ret);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static int __maybe_unused eic7700_pvt_resume(struct device *dev)
+{
+	int ret = 0;
+
+	if (!pm_runtime_status_suspended(dev)) {
+		ret = eic7700_pvt_runtime_resume(dev);
+		if (ret) {
+			dev_err(dev, "Failed to resume: %d\n", ret);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static const struct dev_pm_ops eic7700_pvt_pm_ops = {
+	SYSTEM_SLEEP_PM_OPS(eic7700_pvt_suspend, eic7700_pvt_resume)
+	RUNTIME_PM_OPS(eic7700_pvt_runtime_suspend, eic7700_pvt_runtime_resume,
+		       NULL)
+};
+
+static const struct of_device_id pvt_of_match[] = {
+	{ .compatible = "eswin,eic7700-pvt"},
+	{ }
+};
+MODULE_DEVICE_TABLE(of, pvt_of_match);
+
+static struct platform_driver pvt_driver = {
+	.probe = eic7700_pvt_probe,
+	.driver = {
+		.name = "eic7700-pvt",
+		.of_match_table = pvt_of_match,
+		.pm = pm_sleep_ptr(&eic7700_pvt_pm_ops),
+	},
+};
+module_platform_driver(pvt_driver);
+
+MODULE_AUTHOR("Yulin Lu <luyulin@eswincomputing.com>");
+MODULE_AUTHOR("Huan He <hehuan1@eswincomputing.com>");
+MODULE_DESCRIPTION("Eswin eic7700 PVT driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/hwmon/eic7700-pvt.h b/drivers/hwmon/eic7700-pvt.h
new file mode 100644
index 000000000000..29f38fe48eb2
--- /dev/null
+++ b/drivers/hwmon/eic7700-pvt.h
@@ -0,0 +1,106 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * ESWIN EIC7700 Process, Voltage, Temperature sensor driver
+ *
+ * Copyright 2026, Beijing ESWIN Computing Technology Co., Ltd.
+ */
+#ifndef __HWMON_EIC7700_PVT_H__
+#define __HWMON_EIC7700_PVT_H__
+
+#include <linux/completion.h>
+#include <linux/hwmon.h>
+#include <linux/kernel.h>
+#include <linux/time.h>
+
+/* ESWIN EIC7700 PVT registers and their bitfields */
+#define PVT_TRIM		0x04
+#define PVT_MODE		0x08
+#define PVT_MODE_MASK		GENMASK(2, 0)
+#define PVT_CTRL_MODE_TEMP	0x0
+#define PVT_CTRL_MODE_VOLT	0x4
+#define PVT_CTRL_MODE_LVT	0x1
+#define PVT_CTRL_MODE_ULVT	0x2
+#define PVT_CTRL_MODE_SVT	0x3
+#define PVT_ENA			0x0c
+#define PVT_ENA_EN		BIT(0)
+#define PVT_INT			0x10
+#define PVT_INT_STAT		BIT(0)
+#define PVT_INT_CLR		BIT(1)
+#define PVT_DATA		0x14
+#define PVT_DATA_OUT		GENMASK(9, 0)
+
+/*
+ * PVT sensors-related limits and default values
+ * @PVT_TEMP_CHS: Number of temperature hwmon channels.
+ * @PVT_VOLT_CHS: Number of voltage hwmon channels.
+ * @PVT_TRIM_DEF: Default temperature sensor trim value (set a proper value
+ *		  when one is determined for ESWIN EIC7700 SoC).
+ * @PVT_TOUT_MIN: Minimal timeout between samples in nanoseconds.
+ */
+#define PVT_TEMP_CHS		1
+#define PVT_VOLT_CHS		4
+#define PVT_TRIM_DEF		0
+#define PVT_TOUT_MIN		(NSEC_PER_SEC / 3000)
+
+/*
+ * enum pvt_sensor_type - ESWIN EIC7700 PVT sensor types (correspond to each PVT
+ *			  sampling mode)
+ * @PVT_TEMP: PVT Temperature sensor.
+ * @PVT_VOLT: PVT Voltage sensor.
+ * @PVT_LVT: PVT Low-Voltage threshold sensor.
+ * @PVT_ULVT: PVT UltraLow-Voltage threshold sensor.
+ * @PVT_SVT: PVT Standard-Voltage threshold sensor.
+ */
+enum pvt_sensor_type {
+	PVT_TEMP = 0,
+	PVT_VOLT,
+	PVT_LVT,
+	PVT_ULVT,
+	PVT_SVT
+};
+
+/*
+ * struct pvt_sensor_info - ESWIN EIC7700 PVT sensor informational structure
+ * @channel: Sensor channel ID.
+ * @label: hwmon sensor label.
+ * @mode: PVT mode corresponding to the channel.
+ * @type: Sensor type.
+ */
+struct pvt_sensor_info {
+	int channel;
+	const char *label;
+	u32 mode;
+	enum hwmon_sensor_types type;
+};
+
+#define PVT_SENSOR_INFO(_ch, _label, _type, _mode)	\
+	{						\
+		.channel = _ch,				\
+		.label = _label,			\
+		.mode = PVT_CTRL_MODE_ ##_mode,		\
+		.type = _type,				\
+	}
+
+/*
+ * struct pvt_hwmon - Eswin EIC7700 PVT private data
+ * @dev: device structure of the PVT platform device.
+ * @hwmon: hwmon device structure.
+ * @regs: pointer to the Eswin EIC7700 PVT registers region.
+ * @irq: PVT events IRQ number.
+ * @clk: PVT core clock (1.2MHz).
+ * @rst: pointer to the reset descriptor.
+ * @data_cache: data cache in raw format.
+ * @conversion: data conversion completion.
+ */
+struct pvt_hwmon {
+	struct device *dev;
+	struct device *hwmon;
+	void __iomem *regs;
+	int irq;
+	struct clk *clk;
+	struct reset_control *rst;
+	u32 data_cache;
+	struct completion conversion;
+};
+
+#endif /* __HWMON_EIC7700_PVT_H__ */
-- 
2.25.1

Re: [PATCH v2 1/2] dt-bindings: hwmon: Add Eswin EIC7700 PVT sensor

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 3228 bytes --]

On Wed, Jan 28, 2026 at 06:16:36PM +0800, hehuan1@eswincomputing.com wrote:
> From: Huan He <hehuan1@eswincomputing.com>
> 
> Add device tree binding documentation for ESWIN EIC7700 Process, Voltage
> and Temperature sensor.
> 
> The EIC7700 SoC integrates two PVT instances for monitoring SoC and DDR
> power domains respectively.
> 
> Signed-off-by: Yulin Lu <luyulin@eswincomputing.com>
> Signed-off-by: Huan He <hehuan1@eswincomputing.com>
> ---
>  .../bindings/hwmon/eswin,eic7700-pvt.yaml     | 70 +++++++++++++++++++
>  1 file changed, 70 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/hwmon/eswin,eic7700-pvt.yaml
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/eswin,eic7700-pvt.yaml b/Documentation/devicetree/bindings/hwmon/eswin,eic7700-pvt.yaml
> new file mode 100644
> index 000000000000..f4ba228924fe
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/eswin,eic7700-pvt.yaml
> @@ -0,0 +1,70 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/hwmon/eswin,eic7700-pvt.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: ESWIN EIC7700 PVT Sensor
> +
> +maintainers:
> +  - Yulin Lu <luyulin@eswincomputing.com>
> +  - Huan He <hehuan1@eswincomputing.com>
> +
> +description:
> +  ESWIN EIC7700 SoC integrates embedded process, voltage and temperature
> +  sensors to monitor the internal SoC environment. The system includes two
> +  PVT sensor instances. The PVT0 monitors the main SoC power domain. The
> +  PVT1 sensor monitors the DDR core power domain.
> +
> +allOf:
> +  - $ref: /schemas/hwmon/hwmon-common.yaml#

FYI, including this is kinda pointless because you have the label
property defined below and your "additionalProperties: false" blocks
shunt-resistor-micro-ohms from being used.

> +
> +properties:
> +  compatible:
> +    const: eswin,eic7700-pvt
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  label:
> +    description:
> +      Human readable identifier used to distinguish between different PVT
> +      instances. Typically "pvt0" for SoC PVT sensor and "pvt1" for DDR
> +      core PVT sensor.
> +
> +  resets:
> +    maxItems: 1
> +
> +  '#thermal-sensor-cells':

> +    description: Thermal sensor cells if used for thermal sensoring.

You can drop this description if there's a resend, common properties
used in the obvious way don't need any more info.

Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable

Cheers,
Conor.

> +    const: 0
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - interrupts
> +  - label
> +  - resets
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    pvt@50b00000 {
> +      compatible = "eswin,eic7700-pvt";
> +      reg = <0x50b00000 0x10000>;
> +      clocks = <&clocks 244>;
> +      interrupts = <349>;
> +      interrupt-parent = <&plic>;
> +      label = "pvt0";
> +      resets = <&reset 111>;
> +      #thermal-sensor-cells = <0>;
> +    };
> +...
> -- 
> 2.25.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: Re: [PATCH v2 1/2] dt-bindings: hwmon: Add Eswin EIC7700 PVT sensor

[Huan He]

> > 
> > Add device tree binding documentation for ESWIN EIC7700 Process, Voltage
> > and Temperature sensor.
> > 
> > The EIC7700 SoC integrates two PVT instances for monitoring SoC and DDR
> > power domains respectively.
> > 
> > Signed-off-by: Yulin Lu <luyulin@eswincomputing.com>
> > Signed-off-by: Huan He <hehuan1@eswincomputing.com>
> > ---
> >  .../bindings/hwmon/eswin,eic7700-pvt.yaml     | 70 +++++++++++++++++++
> >  1 file changed, 70 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/hwmon/eswin,eic7700-pvt.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/hwmon/eswin,eic7700-pvt.yaml b/Documentation/devicetree/bindings/hwmon/eswin,eic7700-pvt.yaml
> > new file mode 100644
> > index 000000000000..f4ba228924fe
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/hwmon/eswin,eic7700-pvt.yaml
> > @@ -0,0 +1,70 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/hwmon/eswin,eic7700-pvt.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: ESWIN EIC7700 PVT Sensor
> > +
> > +maintainers:
> > +  - Yulin Lu <luyulin@eswincomputing.com>
> > +  - Huan He <hehuan1@eswincomputing.com>
> > +
> > +description:
> > +  ESWIN EIC7700 SoC integrates embedded process, voltage and temperature
> > +  sensors to monitor the internal SoC environment. The system includes two
> > +  PVT sensor instances. The PVT0 monitors the main SoC power domain. The
> > +  PVT1 sensor monitors the DDR core power domain.
> > +
> > +allOf:
> > +  - $ref: /schemas/hwmon/hwmon-common.yaml#
> 
> FYI, including this is kinda pointless because you have the label
> property defined below and your "additionalProperties: false" blocks
> shunt-resistor-micro-ohms from being used.

I plan to keep the $ref: /schemas/hwmon/hwmon-common.yaml in use, change
the original additionalProperties: false to unevaluatedProperties: false,
and remove the label definition from our schema since it is already
provided by hwmon-common.yaml. Could you please confirm if this
modification conforms to the community standards?

> 
> > +
> > +properties:
> > +  compatible:
> > +    const: eswin,eic7700-pvt
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  label:
> > +    description:
> > +      Human readable identifier used to distinguish between different PVT
> > +      instances. Typically "pvt0" for SoC PVT sensor and "pvt1" for DDR
> > +      core PVT sensor.
> > +
> > +  resets:
> > +    maxItems: 1
> > +
> > +  '#thermal-sensor-cells':
> 
> > +    description: Thermal sensor cells if used for thermal sensoring.
> 
> You can drop this description if there's a resend, common properties
> used in the obvious way don't need any more info.

We will remove this in the next patch.

Thank you very much for taking the time to review the patch and for your
valuable feedback.

Best regards,
Huan He

> 
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> pw-bot: not-applicable
> 
> Cheers,
> Conor.
> 
> > +    const: 0
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - clocks
> > +  - interrupts
> > +  - label
> > +  - resets
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    pvt@50b00000 {
> > +      compatible = "eswin,eic7700-pvt";
> > +      reg = <0x50b00000 0x10000>;
> > +      clocks = <&clocks 244>;
> > +      interrupts = <349>;
> > +      interrupt-parent = <&plic>;
> > +      label = "pvt0";
> > +      resets = <&reset 111>;
> > +      #thermal-sensor-cells = <0>;
> > +    };
> > +...
> > -- 
> > 2.25.1
> > 

Re: Re: [PATCH v2 1/2] dt-bindings: hwmon: Add Eswin EIC7700 PVT sensor

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2431 bytes --]

On Thu, Jan 29, 2026 at 11:06:17AM +0800, Huan He wrote:
> > > 
> > > Add device tree binding documentation for ESWIN EIC7700 Process, Voltage
> > > and Temperature sensor.
> > > 
> > > The EIC7700 SoC integrates two PVT instances for monitoring SoC and DDR
> > > power domains respectively.
> > > 
> > > Signed-off-by: Yulin Lu <luyulin@eswincomputing.com>
> > > Signed-off-by: Huan He <hehuan1@eswincomputing.com>
> > > ---
> > >  .../bindings/hwmon/eswin,eic7700-pvt.yaml     | 70 +++++++++++++++++++
> > >  1 file changed, 70 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/hwmon/eswin,eic7700-pvt.yaml
> > > 
> > > diff --git a/Documentation/devicetree/bindings/hwmon/eswin,eic7700-pvt.yaml b/Documentation/devicetree/bindings/hwmon/eswin,eic7700-pvt.yaml
> > > new file mode 100644
> > > index 000000000000..f4ba228924fe
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/hwmon/eswin,eic7700-pvt.yaml
> > > @@ -0,0 +1,70 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/hwmon/eswin,eic7700-pvt.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: ESWIN EIC7700 PVT Sensor
> > > +
> > > +maintainers:
> > > +  - Yulin Lu <luyulin@eswincomputing.com>
> > > +  - Huan He <hehuan1@eswincomputing.com>
> > > +
> > > +description:
> > > +  ESWIN EIC7700 SoC integrates embedded process, voltage and temperature
> > > +  sensors to monitor the internal SoC environment. The system includes two
> > > +  PVT sensor instances. The PVT0 monitors the main SoC power domain. The
> > > +  PVT1 sensor monitors the DDR core power domain.
> > > +
> > > +allOf:
> > > +  - $ref: /schemas/hwmon/hwmon-common.yaml#
> > 
> > FYI, including this is kinda pointless because you have the label
> > property defined below and your "additionalProperties: false" blocks
> > shunt-resistor-micro-ohms from being used.
> 
> I plan to keep the $ref: /schemas/hwmon/hwmon-common.yaml in use, change
> the original additionalProperties: false to unevaluatedProperties: false,
> and remove the label definition from our schema since it is already
> provided by hwmon-common.yaml. Could you please confirm if this
> modification conforms to the community standards?

That's fine. Does the shunt resistor property apply on your platform?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: Re: Re: [PATCH v2 1/2] dt-bindings: hwmon: Add Eswin EIC7700 PVT sensor

[Huan He]

> > > > 
> > > > Add device tree binding documentation for ESWIN EIC7700 Process, Voltage
> > > > and Temperature sensor.
> > > > 
> > > > The EIC7700 SoC integrates two PVT instances for monitoring SoC and DDR
> > > > power domains respectively.
> > > > 
> > > > Signed-off-by: Yulin Lu <luyulin@eswincomputing.com>
> > > > Signed-off-by: Huan He <hehuan1@eswincomputing.com>
> > > > ---
> > > >  .../bindings/hwmon/eswin,eic7700-pvt.yaml     | 70 +++++++++++++++++++
> > > >  1 file changed, 70 insertions(+)
> > > >  create mode 100644 Documentation/devicetree/bindings/hwmon/eswin,eic7700-pvt.yaml
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/hwmon/eswin,eic7700-pvt.yaml b/Documentation/devicetree/bindings/hwmon/eswin,eic7700-pvt.yaml
> > > > new file mode 100644
> > > > index 000000000000..f4ba228924fe
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/hwmon/eswin,eic7700-pvt.yaml
> > > > @@ -0,0 +1,70 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/hwmon/eswin,eic7700-pvt.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: ESWIN EIC7700 PVT Sensor
> > > > +
> > > > +maintainers:
> > > > +  - Yulin Lu <luyulin@eswincomputing.com>
> > > > +  - Huan He <hehuan1@eswincomputing.com>
> > > > +
> > > > +description:
> > > > +  ESWIN EIC7700 SoC integrates embedded process, voltage and temperature
> > > > +  sensors to monitor the internal SoC environment. The system includes two
> > > > +  PVT sensor instances. The PVT0 monitors the main SoC power domain. The
> > > > +  PVT1 sensor monitors the DDR core power domain.
> > > > +
> > > > +allOf:
> > > > +  - $ref: /schemas/hwmon/hwmon-common.yaml#
> > > 
> > > FYI, including this is kinda pointless because you have the label
> > > property defined below and your "additionalProperties: false" blocks
> > > shunt-resistor-micro-ohms from being used.
> > 
> > I plan to keep the $ref: /schemas/hwmon/hwmon-common.yaml in use, change
> > the original additionalProperties: false to unevaluatedProperties: false,
> > and remove the label definition from our schema since it is already
> > provided by hwmon-common.yaml. Could you please confirm if this
> > modification conforms to the community standards?
> 
> That's fine. Does the shunt resistor property apply on your platform?

The shunt-resistor-micro-ohms property does not apply to our platform. We
only use the label property from hwmon-common.yaml.

Best regards,
Huan He

Re: Re: Re: [PATCH v2 1/2] dt-bindings: hwmon: Add Eswin EIC7700 PVT sensor

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 3066 bytes --]

On Fri, Jan 30, 2026 at 10:00:15AM +0800, Huan He wrote:
> > > > > 
> > > > > Add device tree binding documentation for ESWIN EIC7700 Process, Voltage
> > > > > and Temperature sensor.
> > > > > 
> > > > > The EIC7700 SoC integrates two PVT instances for monitoring SoC and DDR
> > > > > power domains respectively.
> > > > > 
> > > > > Signed-off-by: Yulin Lu <luyulin@eswincomputing.com>
> > > > > Signed-off-by: Huan He <hehuan1@eswincomputing.com>
> > > > > ---
> > > > >  .../bindings/hwmon/eswin,eic7700-pvt.yaml     | 70 +++++++++++++++++++
> > > > >  1 file changed, 70 insertions(+)
> > > > >  create mode 100644 Documentation/devicetree/bindings/hwmon/eswin,eic7700-pvt.yaml
> > > > > 
> > > > > diff --git a/Documentation/devicetree/bindings/hwmon/eswin,eic7700-pvt.yaml b/Documentation/devicetree/bindings/hwmon/eswin,eic7700-pvt.yaml
> > > > > new file mode 100644
> > > > > index 000000000000..f4ba228924fe
> > > > > --- /dev/null
> > > > > +++ b/Documentation/devicetree/bindings/hwmon/eswin,eic7700-pvt.yaml
> > > > > @@ -0,0 +1,70 @@
> > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > > +%YAML 1.2
> > > > > +---
> > > > > +$id: http://devicetree.org/schemas/hwmon/eswin,eic7700-pvt.yaml#
> > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > +
> > > > > +title: ESWIN EIC7700 PVT Sensor
> > > > > +
> > > > > +maintainers:
> > > > > +  - Yulin Lu <luyulin@eswincomputing.com>
> > > > > +  - Huan He <hehuan1@eswincomputing.com>
> > > > > +
> > > > > +description:
> > > > > +  ESWIN EIC7700 SoC integrates embedded process, voltage and temperature
> > > > > +  sensors to monitor the internal SoC environment. The system includes two
> > > > > +  PVT sensor instances. The PVT0 monitors the main SoC power domain. The
> > > > > +  PVT1 sensor monitors the DDR core power domain.
> > > > > +
> > > > > +allOf:
> > > > > +  - $ref: /schemas/hwmon/hwmon-common.yaml#
> > > > 
> > > > FYI, including this is kinda pointless because you have the label
> > > > property defined below and your "additionalProperties: false" blocks
> > > > shunt-resistor-micro-ohms from being used.
> > > 
> > > I plan to keep the $ref: /schemas/hwmon/hwmon-common.yaml in use, change
> > > the original additionalProperties: false to unevaluatedProperties: false,
> > > and remove the label definition from our schema since it is already
> > > provided by hwmon-common.yaml. Could you please confirm if this
> > > modification conforms to the community standards?
> > 
> > That's fine. Does the shunt resistor property apply on your platform?
> 
> The shunt-resistor-micro-ohms property does not apply to our platform. We
> only use the label property from hwmon-common.yaml.

You could just remove
  label:
    description:
      Human readable identifier used to distinguish between different PVT
      instances. Typically "pvt0" for SoC PVT sensor and "pvt1" for DDR
      core PVT sensor.

and do
  label: true

or leave it as-is.

I don't mind.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 0/2] Add driver support for ESWIN EIC7700 PVT controller

[Huan He]

> 
> Add support for the ESWIN EIC7700 PVT (Process, Voltage, Temperature)
> sensor
> 
> Features:
> The driver supports monitoring of process, voltage and temperature
> parameters through the hardware monitoring subsystem. It provides an
> access to the sampled Temperature and Voltage.
> 
> Test:
> Tested this patch on the SiFive HiFive Premier P550 (which uses the ESWIN
> EIC7700 SoC).
> 
> Updates:
> 
>   Changes in v2:
>   - Update eswin,eic7700-pvt.yaml
>     - Reference the hwmon-common.yaml file
>     - Remove the clock-names and reset-names properties
>     - Move additionalProperties: false after the required block
>     - Remove one example node to avoid redundancy
>   - Update eic7700-pvt.c and eic7700-pvt.h
>     - Remove unused sensor macros (PVT_SENSOR_FIRST, PVT_SENSOR_LAST,
>       PVT_SENSORS_NUM)
>     - Drop the unnecessary hwmon-sysfs.h header
>     - Replace dynamic sensor info allocation with a static array and unify
>       sensor labels
>     - Remove unused hwmon_temp_type attribute
>     - Eliminate redundant validation checks
>     - Remove mutex and related locking, relying on hwmon core
>       serialization
>     - Replace per-sensor caches and completions with a single data cache
>       and completion object
>     - Remove pvt->sensor tracking. ISR no longer depends on the currently
>       selected sensor
>     - Move devm_add_action() registration after init_completion() for
>       safer cleanup, and update cleanup function (pvt_clear_data)
>     - Replace devm_reset_control_get_optional_exclusive() with
>       devm_reset_control_get_exclusive_deasserted()
>     - Replace eic7700_pvt_remove() with eic7700_pvt_disable_pm_runtime()
>       and move it after PM runtime enable to avoid resource leaks on probe
>       failure and remove clock disable and reset assert from
>       eic7700_pvt_disable_pm_runtime() as it is already handled by devm_*
>       framework
>     - Remove redundant clock presence check in runtime_resume
> 

Hi Krzysztof, Guenter, all,

The v1 patch received reviews and comments from Krzysztof and Guenter.
The v2 patch incorporates changes based on those comments and has received
feedback from Conor.

I'm not sure if there has been time to review the updated v2 yet. Should I
wait for further feedback, or is any action needed from my side?

Best regards,
Huan He

>   - Link to v1: https://lore.kernel.org/all/20260109090718.442-1-hehuan1@eswincomputing.com/
>

Re: [PATCH v2 0/2] Add driver support for ESWIN EIC7700 PVT controller

[Krzysztof Kozlowski]

On 11/02/2026 10:55, Huan He wrote:
>>     - Remove redundant clock presence check in runtime_resume
>>
> 
> Hi Krzysztof, Guenter, all,
> 
> The v1 patch received reviews and comments from Krzysztof and Guenter.
> The v2 patch incorporates changes based on those comments and has received
> feedback from Conor.
> 
> I'm not sure if there has been time to review the updated v2 yet. Should I
> wait for further feedback, or is any action needed from my side?


Huh? You received review on v2 thus such ping suggests that you don't
consider that given review as an important one. That would be very sad
impression to make...

Best regards,
Krzysztof

Re: Re: [PATCH v2 0/2] Add driver support for ESWIN EIC7700 PVT controller

[Huan He]

> > Hi Krzysztof, Guenter, all,
> > 
> > The v1 patch received reviews and comments from Krzysztof and Guenter.
> > The v2 patch incorporates changes based on those comments and has received
> > feedback from Conor.
> > 
> > I'm not sure if there has been time to review the updated v2 yet. Should I
> > wait for further feedback, or is any action needed from my side?
> 
> 
> Huh? You received review on v2 thus such ping suggests that you don't
> consider that given review as an important one. That would be very sad
> impression to make...
> 

Hi Krzysztof, Guenter, Conor,

Sorry for the confusion - I did not mean to disregard the review.

In v1, I received comments on both the YAML binding and the driver code
from you and Guenter, and updated both parts in v2 accordingly.

For v2, Conor provided further feedback on the binding part, which I will
address in v3.

My question was mainly about the driver changes in v2. I have not received
further comments on that part since posting v2, so I just wanted to check
whether there are any remaining concerns regarding the driver code.

Best regards,
Huan He

Re: Re: [PATCH v2 0/2] Add driver support for ESWIN EIC7700 PVT controller

[Guenter Roeck]

On Thu, Feb 12, 2026 at 12:24:29PM +0800, Huan He wrote:
> > > Hi Krzysztof, Guenter, all,
> > > 
> > > The v1 patch received reviews and comments from Krzysztof and Guenter.
> > > The v2 patch incorporates changes based on those comments and has received
> > > feedback from Conor.
> > > 
> > > I'm not sure if there has been time to review the updated v2 yet. Should I
> > > wait for further feedback, or is any action needed from my side?
> > 
> > 
> > Huh? You received review on v2 thus such ping suggests that you don't
> > consider that given review as an important one. That would be very sad
> > impression to make...
> > 
> 
> Hi Krzysztof, Guenter, Conor,
> 
> Sorry for the confusion - I did not mean to disregard the review.
> 
> In v1, I received comments on both the YAML binding and the driver code
> from you and Guenter, and updated both parts in v2 accordingly.
> 
> For v2, Conor provided further feedback on the binding part, which I will
> address in v3.
> 
> My question was mainly about the driver changes in v2. I have not received
> further comments on that part since posting v2, so I just wanted to check
> whether there are any remaining concerns regarding the driver code.
> 

See below.

Guenter

---
# Task
Date: 2026-02-11 21:54:07
Model: gemini-3-pro-preview
Prompts SHA: HEAD
Commits to review:
- 2b148b14e234 ("dt-bindings: hwmon: Add Eswin EIC7700 PVT sensor")
- 6f4d5698f334 ("hwmon: Add Eswin EIC7700 PVT sensor driver")


# Commit 2b148b14e234 ("dt-bindings: hwmon: Add Eswin EIC7700 PVT sensor")
Ref: 2b148b14e2344fc9403c02cc27ff0bec77d1568c

# Analysis of commit 2b148b14e2344fc9403c02cc27ff0bec77d1568c

## Context
This commit adds a new DeviceTree binding documentation for the Eswin EIC7700 PVT sensor. The binding is located at `Documentation/devicetree/bindings/hwmon/eswin,eic7700-pvt.yaml`.

## Changes
- Adds `Documentation/devicetree/bindings/hwmon/eswin,eic7700-pvt.yaml`

## Analysis
The new binding file was checked against standard DeviceTree binding conventions and the `hwmon` subsystem requirements.

### Correctness Checks
- **Schema Validation**: The YAML syntax is valid. `$id` and `$schema` are correct.
- **Properties**:
    - `compatible`: `eswin,eic7700-pvt` uses a documented vendor prefix (`eswin`).
    - `reg`, `clocks`, `resets`, `interrupts`: All defined with `maxItems: 1` or standard constraints.
    - `label`: Defined as a required property to distinguish between two instances (SoC vs DDR). This aligns with `hwmon-common.yaml` and is justified by the hardware description.
    - `#thermal-sensor-cells`: Defined as `const: 0`, appropriate for a sensor provider.
- **Example**: The example node `pvt@50b00000` is syntactically correct and matches the schema. The node name `pvt` is consistent with other similar bindings (e.g., `baikal,bt1-pvt.yaml`).
- **Maintainers**: Listed maintainers match the Signed-off-by lines in the commit.

### Regressions
No regressions were found. The binding file is self-contained and follows established patterns.

## False Positives Eliminated
- **Required `label` property**: Initially flagged as potentially non-standard for a required property, but verification with `hwmon-common.yaml` and the specific need to distinguish identical hardware blocks for different domains (SoC vs DDR) confirmed it is valid usage.
- **Node name `pvt`**: Verified against `baikal,bt1-pvt.yaml` to confirm `pvt` is an accepted generic node name for PVT sensors.

FINAL REGRESSIONS FOUND: 0

# Commit 6f4d5698f334 ("hwmon: Add Eswin EIC7700 PVT sensor driver")
1.  eic7700-pvt.c:487: ERROR: Unbalanced clock refcount with Runtime PM
    > +	pvt->clk = devm_clk_get_enabled(&pdev->dev, NULL);

    Using `devm_clk_get_enabled()` enables the clock and registers a devm action
    to disable it on removal. However, the driver also uses Runtime PM to manage
    the same clock:

    > +static int __maybe_unused eic7700_pvt_runtime_suspend(struct device *dev)
    > +{
    > +	struct pvt_hwmon *pvt = dev_get_drvdata(dev);
    > +
    > +	clk_disable_unprepare(pvt->clk);

    If the device is runtime suspended when `remove()` is called:
    1. `probe()`: `clk_prepare_enable()` (Ref: 1)
    2. `runtime_suspend()`: `clk_disable_unprepare()` (Ref: 0)
    3. `remove()` (via devm): `clk_disable_unprepare()` (Ref: -1)

    This leads to a refcount underflow and warning.

    Fix: Use `devm_clk_get()` and manually call `clk_prepare_enable()` in probe.
    Ensure that the manual enable is balanced correctly, or rely on Runtime PM
    (and `pm_runtime_get_sync` in probe) to handle the clock, ensuring
    `pm_runtime_put` balances it. Since `check_pwr` needs the clock before
    Runtime PM is enabled, you should enable it manually and then possibly
    disable it before enabling Runtime PM, or keep it enabled and let Runtime PM
    take over (but ensure `remove` doesn't double disable).

    A common pattern:
    ```c
    pvt->clk = devm_clk_get(dev, NULL);
    ...
    clk_prepare_enable(pvt->clk);
    ...
    pm_runtime_enable(dev);
    pm_runtime_get_noresume(dev);
    ...
    pm_runtime_put(dev);
    ```
    And ensure `remove` (or devm action) disables it ONLY if not suspended?
    Actually, simpler is:
    Don't use `devm_clk_get_enabled` if you use `runtime_suspend` to disable it.
    Use `devm_clk_get`.
    In probe: `clk_prepare_enable`.
    Register a `devm_add_action` that calls `clk_disable_unprepare` *only if* the
    driver is not using Runtime PM to control it? No, that's messy.

    Better fix:
    Use `devm_clk_get`.
    In probe: `clk_prepare_enable`.
    In remove (devm action?): `clk_disable_unprepare` (but this still has the issue).

    Correct Fix:
    Do not use `clk_disable_unprepare` in `runtime_suspend` if you used
    `devm_clk_get_enabled`.
    OR
    Don't use `devm_clk_get_enabled`. Use `devm_clk_get`.
    Manage the clock entirely via Runtime PM.
    In probe:
    `clk_prepare_enable(pvt->clk);` (Temporary for check_pwr)
    `check_pwr...`
    `clk_disable_unprepare(pvt->clk);`
    `pm_runtime_enable(dev);`
    ...

2.  eic7700-pvt.c:154: ERROR: Swallowing signal interruption
    > +	if (ret && (ret != -ERESTARTSYS))
    > +		return ret;

    If `wait_for_completion_interruptible()` returns `-ERESTARTSYS` (which is
    non-zero), the condition `(ret != -ERESTARTSYS)` is false, so the if-statement
    is skipped. The function then proceeds to return 0 (success) with potentially
    uninitialized/stale data.

    Fix:
    ```c
    if (ret)
        return ret;
    ```

3.  eic7700-pvt.c:368: WARN: Spurious interrupts on shared IRQ line
    > +	eic7700_pvt_update(pvt->regs + PVT_ENA, PVT_ENA_EN, PVT_ENA_EN);

    `check_pwr` enables the device (and thus potential interrupts) before
    `request_irq` is called. If the IRQ line is shared and the device asserts
    an interrupt immediately, the interrupt will be unhandled (spurious) because
    no handler is registered yet.

    Fix: Request the IRQ before enabling the device, or ensure interrupts are masked
    at the controller level (if possible) before enabling the block. Since `check_pwr`
    relies on polling and ISR clears the status, moving `request_irq` is tricky.
    Verify if `PVT_ENA` has a separate interrupt enable bit or if `PVT_INT` has a mask.
    If not, this is a hardware/driver design risk.

4.  eic7700-pvt.c:147: NOTE: Infinite wait possibility
    > +	ret = wait_for_completion_interruptible(&pvt->conversion);

    If the hardware fails to generate an interrupt (e.g., hangs), this will wait
    indefinitely (unless a signal is sent). It is generally safer to use
    `wait_for_completion_timeout` in hardware drivers.

# Summary

| Commit                                                            | Regressions |
| :---------------------------------------------------------------- | :---------- |
| 2b148b14e234 ("dt-bindings: hwmon: Add Eswin EIC7700 PVT sensor") | 0           |
| 6f4d5698f334 ("hwmon: Add Eswin EIC7700 PVT sensor driver")       | 4           |

Re: Re: Re: [PATCH v2 0/2] Add driver support for ESWIN EIC7700 PVT controller

[Huan He]

Hi Guenter,

Thank you very much for taking the time to review this patch and for
providing such detailed feedback.

> 
> # Commit 6f4d5698f334 ("hwmon: Add Eswin EIC7700 PVT sensor driver")
> 1.  eic7700-pvt.c:487: ERROR: Unbalanced clock refcount with Runtime PM
>     > +	pvt->clk = devm_clk_get_enabled(&pdev->dev, NULL);
> 
>     Using `devm_clk_get_enabled()` enables the clock and registers a devm action
>     to disable it on removal. However, the driver also uses Runtime PM to manage
>     the same clock:
> 
>     > +static int __maybe_unused eic7700_pvt_runtime_suspend(struct device *dev)
>     > +{
>     > +	struct pvt_hwmon *pvt = dev_get_drvdata(dev);
>     > +
>     > +	clk_disable_unprepare(pvt->clk);
> 
>     If the device is runtime suspended when `remove()` is called:
>     1. `probe()`: `clk_prepare_enable()` (Ref: 1)
>     2. `runtime_suspend()`: `clk_disable_unprepare()` (Ref: 0)
>     3. `remove()` (via devm): `clk_disable_unprepare()` (Ref: -1)
> 
>     This leads to a refcount underflow and warning.
> 
>     Fix: Use `devm_clk_get()` and manually call `clk_prepare_enable()` in probe.
>     Ensure that the manual enable is balanced correctly, or rely on Runtime PM
>     (and `pm_runtime_get_sync` in probe) to handle the clock, ensuring
>     `pm_runtime_put` balances it. Since `check_pwr` needs the clock before
>     Runtime PM is enabled, you should enable it manually and then possibly
>     disable it before enabling Runtime PM, or keep it enabled and let Runtime PM
>     take over (but ensure `remove` doesn't double disable).
> 
>     A common pattern:
>     ```c
>     pvt->clk = devm_clk_get(dev, NULL);
>     ...
>     clk_prepare_enable(pvt->clk);
>     ...
>     pm_runtime_enable(dev);
>     pm_runtime_get_noresume(dev);
>     ...
>     pm_runtime_put(dev);
>     ```
>     And ensure `remove` (or devm action) disables it ONLY if not suspended?
>     Actually, simpler is:
>     Don't use `devm_clk_get_enabled` if you use `runtime_suspend` to disable it.
>     Use `devm_clk_get`.
>     In probe: `clk_prepare_enable`.
>     Register a `devm_add_action` that calls `clk_disable_unprepare` *only if* the
>     driver is not using Runtime PM to control it? No, that's messy.
> 
>     Better fix:
>     Use `devm_clk_get`.
>     In probe: `clk_prepare_enable`.
>     In remove (devm action?): `clk_disable_unprepare` (but this still has the issue).
> 
>     Correct Fix:
>     Do not use `clk_disable_unprepare` in `runtime_suspend` if you used
>     `devm_clk_get_enabled`.
>     OR
>     Don't use `devm_clk_get_enabled`. Use `devm_clk_get`.
>     Manage the clock entirely via Runtime PM.
>     In probe:
>     `clk_prepare_enable(pvt->clk);` (Temporary for check_pwr)
>     `check_pwr...`
>     `clk_disable_unprepare(pvt->clk);`
>     `pm_runtime_enable(dev);`
>     ...

We will reproduce and further analyze the clock refcount imbalance
scenario.

> 
> 2.  eic7700-pvt.c:154: ERROR: Swallowing signal interruption
>     > +	if (ret && (ret != -ERESTARTSYS))
>     > +		return ret;
> 
>     If `wait_for_completion_interruptible()` returns `-ERESTARTSYS` (which is
>     non-zero), the condition `(ret != -ERESTARTSYS)` is false, so the if-statement
>     is skipped. The function then proceeds to return 0 (success) with potentially
>     uninitialized/stale data.
> 
>     Fix:
>     ```c
>     if (ret)
>         return ret;
>     ```

In v3, we will return all non-zero error codes directly, without
special-casing -ERESTARTSYS.

> 
> 3.  eic7700-pvt.c:368: WARN: Spurious interrupts on shared IRQ line
>     > +	eic7700_pvt_update(pvt->regs + PVT_ENA, PVT_ENA_EN, PVT_ENA_EN);
> 
>     `check_pwr` enables the device (and thus potential interrupts) before
>     `request_irq` is called. If the IRQ line is shared and the device asserts
>     an interrupt immediately, the interrupt will be unhandled (spurious) because
>     no handler is registered yet.
> 
>     Fix: Request the IRQ before enabling the device, or ensure interrupts are masked
>     at the controller level (if possible) before enabling the block. Since `check_pwr`
>     relies on polling and ISR clears the status, moving `request_irq` is tricky.
>     Verify if `PVT_ENA` has a separate interrupt enable bit or if `PVT_INT` has a mask.
>     If not, this is a hardware/driver design risk.
> 

Confirmed with the hardware team, the PVT_ENA register has no independent
interrupt enable, and PVT_INT does not support masking.
Enabling the device before request_irq may generate interrupts, but the
driver disables the PVT module (PVT_ENA_EN = 0) and clears interrupts by
writing PVT_INT_CLR. In practice, no issues have been observed.

> 4.  eic7700-pvt.c:147: NOTE: Infinite wait possibility
>     > +	ret = wait_for_completion_interruptible(&pvt->conversion);
> 
>     If the hardware fails to generate an interrupt (e.g., hangs), this will wait
>     indefinitely (unless a signal is sent). It is generally safer to use
>     `wait_for_completion_timeout` in hardware drivers.
> 

In v3, we will replace wait_for_completion_interruptible() with
wait_for_completion_timeout().

Best regards,
Huan He

Re: Re: Re: Re: [PATCH v2 0/2] Add driver support for ESWIN EIC7700 PVT controller

[Huan He]

Hi Guenter,

Thank you very much for your detailed review and valuable feedback. I
apologize for the delayed response.

> > 
> > # Commit 6f4d5698f334 ("hwmon: Add Eswin EIC7700 PVT sensor driver")
> > 1.  eic7700-pvt.c:487: ERROR: Unbalanced clock refcount with Runtime PM
> >     > +	pvt->clk = devm_clk_get_enabled(&pdev->dev, NULL);
> > 
> >     Using `devm_clk_get_enabled()` enables the clock and registers a devm action
> >     to disable it on removal. However, the driver also uses Runtime PM to manage
> >     the same clock:
> > 
> >     > +static int __maybe_unused eic7700_pvt_runtime_suspend(struct device *dev)
> >     > +{
> >     > +	struct pvt_hwmon *pvt = dev_get_drvdata(dev);
> >     > +
> >     > +	clk_disable_unprepare(pvt->clk);
> > 
> >     If the device is runtime suspended when `remove()` is called:
> >     1. `probe()`: `clk_prepare_enable()` (Ref: 1)
> >     2. `runtime_suspend()`: `clk_disable_unprepare()` (Ref: 0)
> >     3. `remove()` (via devm): `clk_disable_unprepare()` (Ref: -1)
> > 
> >     This leads to a refcount underflow and warning.
> > 
> >     Fix: Use `devm_clk_get()` and manually call `clk_prepare_enable()` in probe.
> >     Ensure that the manual enable is balanced correctly, or rely on Runtime PM
> >     (and `pm_runtime_get_sync` in probe) to handle the clock, ensuring
> >     `pm_runtime_put` balances it. Since `check_pwr` needs the clock before
> >     Runtime PM is enabled, you should enable it manually and then possibly
> >     disable it before enabling Runtime PM, or keep it enabled and let Runtime PM
> >     take over (but ensure `remove` doesn't double disable).
> > 
> >     A common pattern:
> >     ```c
> >     pvt->clk = devm_clk_get(dev, NULL);
> >     ...
> >     clk_prepare_enable(pvt->clk);
> >     ...
> >     pm_runtime_enable(dev);
> >     pm_runtime_get_noresume(dev);
> >     ...
> >     pm_runtime_put(dev);
> >     ```
> >     And ensure `remove` (or devm action) disables it ONLY if not suspended?
> >     Actually, simpler is:
> >     Don't use `devm_clk_get_enabled` if you use `runtime_suspend` to disable it.
> >     Use `devm_clk_get`.
> >     In probe: `clk_prepare_enable`.
> >     Register a `devm_add_action` that calls `clk_disable_unprepare` *only if* the
> >     driver is not using Runtime PM to control it? No, that's messy.
> > 
> >     Better fix:
> >     Use `devm_clk_get`.
> >     In probe: `clk_prepare_enable`.
> >     In remove (devm action?): `clk_disable_unprepare` (but this still has the issue).
> > 
> >     Correct Fix:
> >     Do not use `clk_disable_unprepare` in `runtime_suspend` if you used
> >     `devm_clk_get_enabled`.
> >     OR
> >     Don't use `devm_clk_get_enabled`. Use `devm_clk_get`.
> >     Manage the clock entirely via Runtime PM.
> >     In probe:
> >     `clk_prepare_enable(pvt->clk);` (Temporary for check_pwr)
> >     `check_pwr...`
> >     `clk_disable_unprepare(pvt->clk);`
> >     `pm_runtime_enable(dev);`
> >     ...
> 
> We will reproduce and further analyze the clock refcount imbalance
> scenario.

Regarding the Runtime PM issue potentially causing clock refcount
imbalance, we have investigated it and will address this in the v3 patch.

> 
> > 
> > 3.  eic7700-pvt.c:368: WARN: Spurious interrupts on shared IRQ line
> >     > +	eic7700_pvt_update(pvt->regs + PVT_ENA, PVT_ENA_EN, PVT_ENA_EN);
> > 
> >     `check_pwr` enables the device (and thus potential interrupts) before
> >     `request_irq` is called. If the IRQ line is shared and the device asserts
> >     an interrupt immediately, the interrupt will be unhandled (spurious) because
> >     no handler is registered yet.
> > 
> >     Fix: Request the IRQ before enabling the device, or ensure interrupts are masked
> >     at the controller level (if possible) before enabling the block. Since `check_pwr`
> >     relies on polling and ISR clears the status, moving `request_irq` is tricky.
> >     Verify if `PVT_ENA` has a separate interrupt enable bit or if `PVT_INT` has a mask.
> >     If not, this is a hardware/driver design risk.
> > 
> 
> Confirmed with the hardware team, the PVT_ENA register has no independent
> interrupt enable, and PVT_INT does not support masking.
> Enabling the device before request_irq may generate interrupts, but the
> driver disables the PVT module (PVT_ENA_EN = 0) and clears interrupts by
> writing PVT_INT_CLR. In practice, no issues have been observed.
> 

For the spurious interrupt concern: after confirming with the hardware
team, the PVT_ENA register has no independent interrupt enable, and
PVT_INT does not support masking. In the current implementation, enabling
the device during check_pwr may generate an interrupt, but the driver
subsequently disables the PVT module (PVT_ENA_EN = 0) and clears the
interrupt status by writing PVT_INT_CLR. In practice, no issues have been
observed.

Could you please confirm whether it is acceptable to keep the current
implementation under these conditions?
 
Best regards,
Huan He

Re: Re: Re: Re: [PATCH v2 0/2] Add driver support for ESWIN EIC7700 PVT controller

[Guenter Roeck]

On Thu, Mar 05, 2026 at 07:12:37PM +0800, Huan He wrote:
> Hi Guenter,
> 
> Thank you very much for your detailed review and valuable feedback. I
> apologize for the delayed response.
> 
> > > 
> > > # Commit 6f4d5698f334 ("hwmon: Add Eswin EIC7700 PVT sensor driver")
> > > 1.  eic7700-pvt.c:487: ERROR: Unbalanced clock refcount with Runtime PM
> > >     > +	pvt->clk = devm_clk_get_enabled(&pdev->dev, NULL);
> > > 
> > >     Using `devm_clk_get_enabled()` enables the clock and registers a devm action
> > >     to disable it on removal. However, the driver also uses Runtime PM to manage
> > >     the same clock:
> > > 
> > >     > +static int __maybe_unused eic7700_pvt_runtime_suspend(struct device *dev)
> > >     > +{
> > >     > +	struct pvt_hwmon *pvt = dev_get_drvdata(dev);
> > >     > +
> > >     > +	clk_disable_unprepare(pvt->clk);
> > > 
> > >     If the device is runtime suspended when `remove()` is called:
> > >     1. `probe()`: `clk_prepare_enable()` (Ref: 1)
> > >     2. `runtime_suspend()`: `clk_disable_unprepare()` (Ref: 0)
> > >     3. `remove()` (via devm): `clk_disable_unprepare()` (Ref: -1)
> > > 
> > >     This leads to a refcount underflow and warning.
> > > 
> > >     Fix: Use `devm_clk_get()` and manually call `clk_prepare_enable()` in probe.
> > >     Ensure that the manual enable is balanced correctly, or rely on Runtime PM
> > >     (and `pm_runtime_get_sync` in probe) to handle the clock, ensuring
> > >     `pm_runtime_put` balances it. Since `check_pwr` needs the clock before
> > >     Runtime PM is enabled, you should enable it manually and then possibly
> > >     disable it before enabling Runtime PM, or keep it enabled and let Runtime PM
> > >     take over (but ensure `remove` doesn't double disable).
> > > 
> > >     A common pattern:
> > >     ```c
> > >     pvt->clk = devm_clk_get(dev, NULL);
> > >     ...
> > >     clk_prepare_enable(pvt->clk);
> > >     ...
> > >     pm_runtime_enable(dev);
> > >     pm_runtime_get_noresume(dev);
> > >     ...
> > >     pm_runtime_put(dev);
> > >     ```
> > >     And ensure `remove` (or devm action) disables it ONLY if not suspended?
> > >     Actually, simpler is:
> > >     Don't use `devm_clk_get_enabled` if you use `runtime_suspend` to disable it.
> > >     Use `devm_clk_get`.
> > >     In probe: `clk_prepare_enable`.
> > >     Register a `devm_add_action` that calls `clk_disable_unprepare` *only if* the
> > >     driver is not using Runtime PM to control it? No, that's messy.
> > > 
> > >     Better fix:
> > >     Use `devm_clk_get`.
> > >     In probe: `clk_prepare_enable`.
> > >     In remove (devm action?): `clk_disable_unprepare` (but this still has the issue).
> > > 
> > >     Correct Fix:
> > >     Do not use `clk_disable_unprepare` in `runtime_suspend` if you used
> > >     `devm_clk_get_enabled`.
> > >     OR
> > >     Don't use `devm_clk_get_enabled`. Use `devm_clk_get`.
> > >     Manage the clock entirely via Runtime PM.
> > >     In probe:
> > >     `clk_prepare_enable(pvt->clk);` (Temporary for check_pwr)
> > >     `check_pwr...`
> > >     `clk_disable_unprepare(pvt->clk);`
> > >     `pm_runtime_enable(dev);`
> > >     ...
> > 
> > We will reproduce and further analyze the clock refcount imbalance
> > scenario.
> 
> Regarding the Runtime PM issue potentially causing clock refcount
> imbalance, we have investigated it and will address this in the v3 patch.
> 
> > 
> > > 
> > > 3.  eic7700-pvt.c:368: WARN: Spurious interrupts on shared IRQ line
> > >     > +	eic7700_pvt_update(pvt->regs + PVT_ENA, PVT_ENA_EN, PVT_ENA_EN);
> > > 
> > >     `check_pwr` enables the device (and thus potential interrupts) before
> > >     `request_irq` is called. If the IRQ line is shared and the device asserts
> > >     an interrupt immediately, the interrupt will be unhandled (spurious) because
> > >     no handler is registered yet.
> > > 
> > >     Fix: Request the IRQ before enabling the device, or ensure interrupts are masked
> > >     at the controller level (if possible) before enabling the block. Since `check_pwr`
> > >     relies on polling and ISR clears the status, moving `request_irq` is tricky.
> > >     Verify if `PVT_ENA` has a separate interrupt enable bit or if `PVT_INT` has a mask.
> > >     If not, this is a hardware/driver design risk.
> > > 
> > 
> > Confirmed with the hardware team, the PVT_ENA register has no independent
> > interrupt enable, and PVT_INT does not support masking.
> > Enabling the device before request_irq may generate interrupts, but the
> > driver disables the PVT module (PVT_ENA_EN = 0) and clears interrupts by
> > writing PVT_INT_CLR. In practice, no issues have been observed.
> > 
> 
> For the spurious interrupt concern: after confirming with the hardware
> team, the PVT_ENA register has no independent interrupt enable, and
> PVT_INT does not support masking. In the current implementation, enabling
> the device during check_pwr may generate an interrupt, but the driver
> subsequently disables the PVT module (PVT_ENA_EN = 0) and clears the
> interrupt status by writing PVT_INT_CLR. In practice, no issues have been
> observed.
> 
> Could you please confirm whether it is acceptable to keep the current
> implementation under these conditions?

Yes, but please explain in the code.

Thanks,
Guenter