Add driver support for ESWIN EIC7700 PVT controller

From: Huan He <[email protected]>

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

Features:
The driver supports monitoring of 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 v6:
- Fix the !CONFIG_PM probe error path by disabling the clock if IRQ request fails before the PM cleanup action is registered
- Replace pm_runtime_put_noidle() with pm_runtime_put() in the IRQ handler to avoid a runtime PM reference-count race with the read path
- Remove the unused pvt_clear_data() devres action and its associated devm_add_action() registration

Changes in v5:
- Update eswin,eic7700-pvt.yaml
- Drop the label enum constraint and remove label from the required list
- Add '#thermal-sensor-cells' to the required list
- Rename the example node to the generic sensor@... form
- Update the binding description to describe one temperature sensor and one voltage sensor
- Update eic7700-pvt.c
- Register the hwmon device with the fixed name "pvt"
- Remove label-based instance identification from the driver
- Fix CONFIG_PM=n support by keeping the clock enabled when runtime PM is unavailable
- Add pm_runtime_force_suspend() in the cleanup path to avoid leaving the device active during unbind
- Switch system sleep callbacks to pm_runtime_force_suspend() and pm_runtime_force_resume()
- Guard ISR register accesses with pm_runtime_get_if_active()
- Add synchronize_irq() on the timeout path to avoid stale completion races
- Remove temp_offset support because the raw trim register does not match the hwmon ABI
- Align the commit message with the implementation (one temperature sensor, one voltage sensor)

Changes in v4:
- Update eswin,eic7700-pvt.yaml
- Delete reviewed-by tag of Conor Dooley, because the label enum constraint is introduced
- Update eic7700-pvt.c and eic7700-pvt.h
- Remove the unused LVT/ULVT/SVT process-monitoring channels
- Remove the probe-time power check since the PVT block is always powered on EIC7700 and the extra verification is unnecessary
- Stop requesting the interrupt as shared and use the dedicated PVT IRQ only
- Reorder probe initialization so the interface is initialized before the clock is disabled, avoiding register accesses with the clock gated
- Fix runtime PM reference handling on error paths by balancing pm_runtime_get_noresume() with pm_runtime_put_noidle()
- Add pm_runtime_put_noidle() handling for failed pm_runtime_get_sync() calls in hwmon read/write paths
- Switch the PM callback registration from pm_sleep_ptr() to pm_ptr()

Changes in v3:
- Update eswin,eic7700-pvt.yaml
- Remove redundant label property description and use 'label: true' to reference the definition in hwmon-common.yaml
- Replace 'additionalProperties: false' with 'unevaluatedProperties: false'
- Remove the description for '#thermal-sensor-cells'
- Update eic7700-pvt.c and eic7700-pvt.h
- Fix clock reference count imbalance with Runtime PM:
Replace devm_clk_get_enabled() with devm_clk_get() and manually manage clock enable/disable to avoid double-disable in remove() when Runtime PM is active. Clock is now enabled only during probe for eic7700_pvt_check_pwr(), then disabled before enabling Runtime PM, which takes full control of the clock thereafter
- Add detailed comment explaining the spurious interrupt risk in eic7700_pvt_check_pwr()
- Replace wait_for_completion_interruptible() with wait_for_completion_timeout() to prevent infinite wait

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/[email protected]/

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

 .../bindings/hwmon/eswin,eic7700-pvt.yaml     |  63 +++
 drivers/hwmon/Kconfig                         |  12 +
 drivers/hwmon/Makefile                        |   1 +
 drivers/hwmon/eic7700-pvt.c                   | 495 ++++++++++++++++++
 drivers/hwmon/eic7700-pvt.h                   |  99 ++++
 5 files changed, 670 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

From: Huan He <[email protected]>

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

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

---
 .../bindings/hwmon/eswin,eic7700-pvt.yaml     | 63 +++++++++++++++++++
 1 file changed, 63 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/eswin,eic7700-pvt.yaml

new file mode 100644
index 000000000000..27cc90e52d4b
@@ -0,0 +1,63 @@
+# 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 <[email protected]>
+  - Huan He <[email protected]>
+
+description:
+  ESWIN EIC7700 SoC integrates embedded 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
+
+  resets:
+    maxItems: 1
+
+  '#thermal-sensor-cells':
+    const: 0
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - interrupts
+  - resets
+  - '#thermal-sensor-cells'
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    sensor@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

From: Huan He <[email protected]>

Add support for ESWIN EIC7700 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 one voltage sensor.

---
 drivers/hwmon/Kconfig       |  12 +
 drivers/hwmon/Makefile      |   1 +
 drivers/hwmon/eic7700-pvt.c | 495 ++++++++++++++++++++++++++++++++++++
 drivers/hwmon/eic7700-pvt.h |  99 ++++++++
 4 files changed, 607 insertions(+)
 create mode 100644 drivers/hwmon/eic7700-pvt.c
 create mode 100644 drivers/hwmon/eic7700-pvt.h

index 14e4cea48acc..6cb60355aa0c 100644
@@ -2033,6 +2033,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 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

index 982ee2c6f9de..d647669074e1 100644
@@ -71,6 +71,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

2.25.1

Hi Sashiko,

Thank you very much for your detailed review. We appreciate the feedback.

I will fix this by disabling the conversion engine first, and only then clearing the pending interrupt status.

The initialization sequence will be changed along these lines:

    eic7700_pvt_update(pvt->regs + PVT_ENA, PVT_ENA_EN, 0); eic7700_pvt_update(pvt->regs + PVT_INT, PVT_INT_CLR, PVT_INT_CLR); readl(pvt->regs + PVT_INT);

I don't think this is an actual issue.

I re-checked the runtime PM pairing here.

The probe path enables runtime PM with pm_runtime_enable(). The device cleanup path calls pm_runtime_force_suspend(), and pm_runtime_force_suspend() already disables runtime PM internally:

int pm_runtime_force_suspend(struct device *dev)
{
    ...
    pm_runtime_disable(dev);
    ...
}

This cleanup path intentionally uses pm_runtime_force_suspend() rather than a standalone pm_runtime_disable().

A standalone pm_runtime_disable() only disables runtime PM. It does not force an RPM_ACTIVE device through the driver's runtime_suspend() callback, so the clock could remain enabled if the driver is unbound during the autosuspend delay window.

pm_runtime_force_suspend() handles that case by disabling runtime PM internally and, if the device is still active, invoking the driver's runtime_suspend() callback so the clock is actually turned off.

Therefore, adding another explicit pm_runtime_disable() after pm_runtime_force_suspend() would result in an extra disable_depth increment and could leave the runtime PM state unbalanced.

Please let me know if you would prefer a different runtime PM cleanup approach here.

Best regards,
Huan He

[...]

[...]

Hi Philipp,

Thank you very much for your detailed review. We appreciate the feedback.

The reset control is only needed during probe to deassert the reset line, and it is not used afterwards.

I will change it to a local variable in eic7700_pvt_probe() and remove the rst field from struct pvt_hwmon.

The code will be changed along these lines:

    struct reset_control *rst;

    rst = devm_reset_control_get_exclusive_deasserted(&pdev->dev, NULL); if (IS_ERR(rst))
        return dev_err_probe(pvt->dev, PTR_ERR(rst), "Couldn't get reset control\n");

Best regards,
Huan He