Add driver support for ESWIN EIC7700 PVT controller

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.

Signed-off-by Yulin Lu <[email protected]>

Signed-off-by Huan He <[email protected]>

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

Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
- [Critical] The timeout calculation for sensor conversion evaluates to 0 jiffies, causing all reads to instantly fail.
- [High] Missing pm_runtime_disable() in the devres cleanup path leads to an unbalanced disable_depth.
- [High] Clock leakage on the IRQ request error path when CONFIG_PM is disabled.
- [High] Race condition between ISR and main thread leaks PM runtime active state.
- [High] Incorrect initialization order causes an IRQ storm during probe if an interrupt fires early.
- [High] Lack of internal locking in eic7700_pvt_hwmon_read allows concurrent hardware state corruption.
- [Medium] Useless and incorrectly ordered devres action for unblocking sysfs readers.
--

commit fe78922d5101b3a07ca5ea0fbd0fd5e9cba0dcc9
Author: Huan He <[email protected]>

hwmon: Add Eswin EIC7700 PVT sensor driver

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.

Quoted 4 lines

drivers/hwmon/eic7700-pvt.c new file +0 -0

index 0000000000000..cdcfa031cdbdb

[ ... ]

Quoted 2 lines

drivers/hwmon/eic7700-pvt.c new file +2 -0

+static irqreturn_t eic7700_pvt_hard_isr(int irq, void *data)
+{

[ ... ]

drivers/hwmon/eic7700-pvt.c line 135

Quoted 9 lines

drivers/hwmon/eic7700-pvt.c new file +9 -0

+	val = readl(pvt->regs + PVT_DATA);
+	WRITE_ONCE(pvt->data_cache, FIELD_GET(PVT_DATA_OUT, val));
+	complete(&pvt->conversion);
+
+	if (IS_ENABLED(CONFIG_PM))
+		pm_runtime_put_noidle(pvt->dev);
+
+	return IRQ_HANDLED;
+}

If the main thread in eic7700_pvt_hwmon_read() drops the usage count from 2 to 1 before the ISR drops it from 1 to 0, will both threads fail to trigger the idle callback?

This could leave the device indefinitely in the active state. Should the ISR use pm_runtime_put_autosuspend() or pm_runtime_put() instead of pm_runtime_put_noidle()?

[ ... ]

Quoted 4 lines

drivers/hwmon/eic7700-pvt.c new file +4 -0

+static int eic7700_pvt_read_data(struct pvt_hwmon *pvt,
+				 enum pvt_sensor_type type, long *val)
+{
+	unsigned long timeout;

[ ... ]

drivers/hwmon/eic7700-pvt.c line 162

Quoted 2 lines

drivers/hwmon/eic7700-pvt.c new file +2 -0

+	timeout = 2 * usecs_to_jiffies(ktime_to_us(pvt->timeout));
+	ret = wait_for_completion_timeout(&pvt->conversion, timeout);

Because pvt->timeout is 333 microseconds, will usecs_to_jiffies(333) evaluate to 0 on standard kernel configurations like HZ=1000?

If so, this acts as a non-blocking check and wait_for_completion_timeout() could instantly return 0, causing all reads to permanently fail with -ETIMEDOUT.

[ ... ]

drivers/hwmon/eic7700-pvt.c line 220

Quoted 5 lines

drivers/hwmon/eic7700-pvt.c new file +5 -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);

The driver registers the temperature sensor with the thermal subsystem via the HWMON_C_REGISTER_TZ flag.

If thermal zone background polling directly invokes this read callback while a user concurrently reads a hwmon sysfs attribute, is there a race condition? Does this need an internal mutex to prevent concurrent hardware state corruption?

[ ... ]

Quoted 2 lines

drivers/hwmon/eic7700-pvt.c new file +2 -0

+static struct pvt_hwmon *eic7700_pvt_create_data(struct platform_device *pdev)
+{

[ ... ]

drivers/hwmon/eic7700-pvt.c line 310

Quoted 1 line

drivers/hwmon/eic7700-pvt.c new file +1 -0

+	ret = devm_add_action(dev, pvt_clear_data, pvt);

Because devres actions execute in reverse order of registration (LIFO), will this action run after the hwmon device is unregistered?

Since hwmon unregistration blocks until all active sysfs read operations finish, the reader would have already completed before pvt_clear_data runs, which might make it ineffective for unblocking sysfs readers.

[ ... ]

drivers/hwmon/eic7700-pvt.c line 387

Quoted 10 lines

drivers/hwmon/eic7700-pvt.c new file +10 -0

+static void eic7700_pvt_disable_pm_runtime(void *data)
+{
+	struct pvt_hwmon *pvt = data;
+
+	pm_runtime_dont_use_autosuspend(pvt->dev);
+	pm_runtime_force_suspend(pvt->dev);
+
+	if (!IS_ENABLED(CONFIG_PM))
+		clk_disable_unprepare(pvt->clk);
+}

Should this custom devres cleanup action call pm_runtime_disable()?

If the module is unloaded and then re-probed, could the unbalanced disable_depth cause an "Unbalanced pm_runtime_enable!" warning and corrupt the power management state?

Quoted 3 lines

drivers/hwmon/eic7700-pvt.c new file +3 -0

+
+static int eic7700_pvt_probe(struct platform_device *pdev)
+{

[ ... ]

drivers/hwmon/eic7700-pvt.c line 430

Quoted 6 lines

drivers/hwmon/eic7700-pvt.c new file +6 -0

+	if (IS_ENABLED(CONFIG_PM))
+		clk_disable_unprepare(pvt->clk);
+
+	ret = eic7700_pvt_request_irq(pvt);
+	if (ret)
+		return ret;

If CONFIG_PM is disabled, the clock remains enabled. If eic7700_pvt_request_irq() fails here, does the function return without disabling the clock? This could permanently leak the clock reference because the devres cleanup action is not yet registered.

drivers/hwmon/eic7700-pvt.c line 432

Quoted 2 lines

drivers/hwmon/eic7700-pvt.c new file +2 -0

+
+	pm_runtime_enable(&pdev->dev);

Since the IRQ is unmasked before pm_runtime_enable() is called, what happens if the hardware asserts an interrupt in this window?

If eic7700_pvt_hard_isr() fires, pm_runtime_get_if_active() would return -EINVAL and the handler would return IRQ_NONE without clearing the interrupt. Since the interrupt is level-triggered (IRQF_TRIGGER_HIGH), could this cause an infinite IRQ storm that locks up the CPU?

Quoted 41 lines

Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
- [Critical] The timeout calculation for sensor conversion evaluates to 0 jiffies, causing all reads to instantly fail.
- [High] Missing pm_runtime_disable() in the devres cleanup path leads to an unbalanced disable_depth.
- [High] Clock leakage on the IRQ request error path when CONFIG_PM is disabled.
- [High] Race condition between ISR and main thread leaks PM runtime active state.
- [High] Incorrect initialization order causes an IRQ storm during probe if an interrupt fires early.
- [High] Lack of internal locking in eic7700_pvt_hwmon_read allows concurrent hardware state corruption.
- [Medium] Useless and incorrectly ordered devres action for unblocking sysfs readers.
--

commit fe78922d5101b3a07ca5ea0fbd0fd5e9cba0dcc9
Author: Huan He <[email protected]>

hwmon: Add Eswin EIC7700 PVT sensor driver

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.

Quoted 4 lines

drivers/hwmon/eic7700-pvt.c new file +0 -0

index 0000000000000..cdcfa031cdbdb

[ ... ]

Quoted 2 lines

drivers/hwmon/eic7700-pvt.c new file +2 -0

+static irqreturn_t eic7700_pvt_hard_isr(int irq, void *data)
+{

[ ... ]

drivers/hwmon/eic7700-pvt.c line 135

Quoted 9 lines

drivers/hwmon/eic7700-pvt.c new file +9 -0

+	val = readl(pvt->regs + PVT_DATA);
+	WRITE_ONCE(pvt->data_cache, FIELD_GET(PVT_DATA_OUT, val));
+	complete(&pvt->conversion);
+
+	if (IS_ENABLED(CONFIG_PM))
+		pm_runtime_put_noidle(pvt->dev);
+
+	return IRQ_HANDLED;
+}

If the main thread in eic7700_pvt_hwmon_read() drops the usage count from 2 to 1 before the ISR drops it from 1 to 0, will both threads fail to trigger the idle callback?

Quoted 3 lines

This could leave the device indefinitely in the active state. Should the ISR use pm_runtime_put_autosuspend() or pm_runtime_put() instead of pm_runtime_put_noidle()?

Quoted 13 lines

[ ... ]

Quoted 4 lines

drivers/hwmon/eic7700-pvt.c new file +4 -0

+static int eic7700_pvt_read_data(struct pvt_hwmon *pvt,
+				 enum pvt_sensor_type type, long *val)
+{
+	unsigned long timeout;

[ ... ]

drivers/hwmon/eic7700-pvt.c line 162

Quoted 2 lines

drivers/hwmon/eic7700-pvt.c new file +2 -0

+	timeout = 2 * usecs_to_jiffies(ktime_to_us(pvt->timeout));
+	ret = wait_for_completion_timeout(&pvt->conversion, timeout);

Because pvt->timeout is 333 microseconds, will usecs_to_jiffies(333) evaluate to 0 on standard kernel configurations like HZ=1000?

Quoted 3 lines

If so, this acts as a non-blocking check and wait_for_completion_timeout() could instantly return 0, causing all reads to permanently fail with -ETIMEDOUT.

Quoted 11 lines

[ ... ]

drivers/hwmon/eic7700-pvt.c line 220

Quoted 5 lines

drivers/hwmon/eic7700-pvt.c new file +5 -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);

The driver registers the temperature sensor with the thermal subsystem via the HWMON_C_REGISTER_TZ flag.

Quoted 5 lines

If thermal zone background polling directly invokes this read callback while a user concurrently reads a hwmon sysfs attribute, is there a race condition? Does this need an internal mutex to prevent concurrent hardware state corruption?

False positive. Polling is through the hwmon subsystem, which protects such accesses with the hwmon subsystem lock.

The other concerns seem real. Please address.

Thanks,
Guenter

Quoted 12 lines

[ ... ]

Quoted 2 lines

drivers/hwmon/eic7700-pvt.c new file +2 -0

+static struct pvt_hwmon *eic7700_pvt_create_data(struct platform_device *pdev)
+{

[ ... ]

drivers/hwmon/eic7700-pvt.c line 310

Quoted 1 line

drivers/hwmon/eic7700-pvt.c new file +1 -0

+	ret = devm_add_action(dev, pvt_clear_data, pvt);

Because devres actions execute in reverse order of registration (LIFO), will this action run after the hwmon device is unregistered?

Since hwmon unregistration blocks until all active sysfs read operations finish, the reader would have already completed before pvt_clear_data runs, which might make it ineffective for unblocking sysfs readers.

Quoted 15 lines

[ ... ]

drivers/hwmon/eic7700-pvt.c line 387

Quoted 10 lines

drivers/hwmon/eic7700-pvt.c new file +10 -0

+static void eic7700_pvt_disable_pm_runtime(void *data)
+{
+	struct pvt_hwmon *pvt = data;
+
+	pm_runtime_dont_use_autosuspend(pvt->dev);
+	pm_runtime_force_suspend(pvt->dev);
+
+	if (!IS_ENABLED(CONFIG_PM))
+		clk_disable_unprepare(pvt->clk);
+}

Should this custom devres cleanup action call pm_runtime_disable()?

Quoted 3 lines

If the module is unloaded and then re-probed, could the unbalanced disable_depth cause an "Unbalanced pm_runtime_enable!" warning and corrupt the power management state?

Quoted 5 lines

Quoted 4 lines

drivers/hwmon/eic7700-pvt.c new file +3 -0

+
+static int eic7700_pvt_probe(struct platform_device *pdev)
+{

[ ... ]

Quoted 11 lines

drivers/hwmon/eic7700-pvt.c line 430

Quoted 6 lines

drivers/hwmon/eic7700-pvt.c new file +6 -0

+	if (IS_ENABLED(CONFIG_PM))
+		clk_disable_unprepare(pvt->clk);
+
+	ret = eic7700_pvt_request_irq(pvt);
+	if (ret)
+		return ret;

If CONFIG_PM is disabled, the clock remains enabled. If eic7700_pvt_request_irq() fails here, does the function return without disabling the clock? This could permanently leak the clock reference because the devres cleanup action is not yet registered.

Quoted 11 lines

drivers/hwmon/eic7700-pvt.c line 432

Quoted 3 lines

drivers/hwmon/eic7700-pvt.c new file +2 -0

+
+	pm_runtime_enable(&pdev->dev);

Since the IRQ is unmasked before pm_runtime_enable() is called, what happens if the hardware asserts an interrupt in this window?

If eic7700_pvt_hard_isr() fires, pm_runtime_get_if_active() would return -EINVAL and the handler would return IRQ_NONE without clearing the interrupt. Since the interrupt is level-triggered (IRQF_TRIGGER_HIGH), could this cause an infinite IRQ storm that locks up the CPU?

Hi Sashiko, Guenter,

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

Quoted 42 lines · On 5/15/26 03:03, [email protected] wrote:

Quoted 41 lines

Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
- [Critical] The timeout calculation for sensor conversion evaluates to 0 jiffies, causing all reads to instantly fail.
- [High] Missing pm_runtime_disable() in the devres cleanup path leads to an unbalanced disable_depth.
- [High] Clock leakage on the IRQ request error path when CONFIG_PM is disabled.
- [High] Race condition between ISR and main thread leaks PM runtime active state.
- [High] Incorrect initialization order causes an IRQ storm during probe if an interrupt fires early.
- [High] Lack of internal locking in eic7700_pvt_hwmon_read allows concurrent hardware state corruption.
- [Medium] Useless and incorrectly ordered devres action for unblocking sysfs readers.
--

commit fe78922d5101b3a07ca5ea0fbd0fd5e9cba0dcc9
Author: Huan He <[email protected]>

hwmon: Add Eswin EIC7700 PVT sensor driver

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.

Quoted 4 lines

drivers/hwmon/eic7700-pvt.c new file +0 -0

index 0000000000000..cdcfa031cdbdb

[ ... ]

Quoted 2 lines

drivers/hwmon/eic7700-pvt.c new file +2 -0

+static irqreturn_t eic7700_pvt_hard_isr(int irq, void *data)
+{

[ ... ]

drivers/hwmon/eic7700-pvt.c line 135

Quoted 9 lines

drivers/hwmon/eic7700-pvt.c new file +9 -0

+	val = readl(pvt->regs + PVT_DATA);
+	WRITE_ONCE(pvt->data_cache, FIELD_GET(PVT_DATA_OUT, val));
+	complete(&pvt->conversion);
+
+	if (IS_ENABLED(CONFIG_PM))
+		pm_runtime_put_noidle(pvt->dev);
+
+	return IRQ_HANDLED;
+}

If the main thread in eic7700_pvt_hwmon_read() drops the usage count from 2 to 1 before the ISR drops it from 1 to 0, will both threads fail to trigger the idle callback?

Quoted 3 lines

Quoted 3 lines

This could leave the device indefinitely in the active state. Should the ISR use pm_runtime_put_autosuspend() or pm_runtime_put() instead of pm_runtime_put_noidle()?

I will fix this by replacing pm_runtime_put_noidle() in the ISR with pm_runtime_put(), so the final reference drop can still trigger runtime PM idle handling.

The ISR will be changed along these lines:

    complete(&pvt->conversion);

  if (IS_ENABLED(CONFIG_PM))
-       pm_runtime_put_noidle(pvt->dev);
+       pm_runtime_put(pvt->dev);

Quoted 13 lines

Quoted 13 lines

[ ... ]

Quoted 4 lines

drivers/hwmon/eic7700-pvt.c new file +4 -0

+static int eic7700_pvt_read_data(struct pvt_hwmon *pvt,
+				 enum pvt_sensor_type type, long *val)
+{
+	unsigned long timeout;

[ ... ]

drivers/hwmon/eic7700-pvt.c line 162

Quoted 2 lines

drivers/hwmon/eic7700-pvt.c new file +2 -0

+	timeout = 2 * usecs_to_jiffies(ktime_to_us(pvt->timeout));
+	ret = wait_for_completion_timeout(&pvt->conversion, timeout);

Because pvt->timeout is 333 microseconds, will usecs_to_jiffies(333) evaluate to 0 on standard kernel configurations like HZ=1000?

Quoted 3 lines

Quoted 3 lines

If so, this acts as a non-blocking check and wait_for_completion_timeout() could instantly return 0, causing all reads to permanently fail with -ETIMEDOUT.

This looks like a false positive.

usecs_to_jiffies() eventually goes through _usecs_to_jiffies(), which rounds up instead of truncating to zero:

    static inline unsigned long _usecs_to_jiffies(const unsigned int u) {
        return (u + (USEC_PER_SEC / HZ) - 1) / (USEC_PER_SEC / HZ); }

So with HZ=1000:

    usecs_to_jiffies(333) = (333 + 1000 - 1) / 1000 = 1

Therefore for pvt->timeout = 333 us, the timeout does not collapse to 0 jiffies. The current code evaluates to 2 jiffies, so wait_for_completion_timeout() remains blocking rather than becoming a non-blocking check.

Quoted 17 lines

Quoted 11 lines

[ ... ]

drivers/hwmon/eic7700-pvt.c line 220

Quoted 5 lines

drivers/hwmon/eic7700-pvt.c new file +5 -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);

The driver registers the temperature sensor with the thermal subsystem via the HWMON_C_REGISTER_TZ flag.

Quoted 5 lines

If thermal zone background polling directly invokes this read callback while a user concurrently reads a hwmon sysfs attribute, is there a race condition? Does this need an internal mutex to prevent concurrent hardware state corruption?

Quoted 21 lines

False positive. Polling is through the hwmon subsystem, which protects such accesses with the hwmon subsystem lock.

The other concerns seem real. Please address.

Thanks,
Guenter

Quoted 12 lines

[ ... ]

Quoted 2 lines

drivers/hwmon/eic7700-pvt.c new file +2 -0

+static struct pvt_hwmon *eic7700_pvt_create_data(struct platform_device *pdev)
+{

[ ... ]

drivers/hwmon/eic7700-pvt.c line 310

Quoted 1 line

drivers/hwmon/eic7700-pvt.c new file +1 -0

+	ret = devm_add_action(dev, pvt_clear_data, pvt);

Because devres actions execute in reverse order of registration (LIFO), will this action run after the hwmon device is unregistered?

Since hwmon unregistration blocks until all active sysfs read operations finish, the reader would have already completed before pvt_clear_data runs, which might make it ineffective for unblocking sysfs readers.

I will remove pvt_clear_data() and the associated devm_add_action() call.

Quoted 15 lines

Quoted 15 lines

[ ... ]

drivers/hwmon/eic7700-pvt.c line 387

Quoted 10 lines

drivers/hwmon/eic7700-pvt.c new file +10 -0

+static void eic7700_pvt_disable_pm_runtime(void *data)
+{
+	struct pvt_hwmon *pvt = data;
+
+	pm_runtime_dont_use_autosuspend(pvt->dev);
+	pm_runtime_force_suspend(pvt->dev);
+
+	if (!IS_ENABLED(CONFIG_PM))
+		clk_disable_unprepare(pvt->clk);
+}

Should this custom devres cleanup action call pm_runtime_disable()?

Quoted 3 lines

Quoted 3 lines

If the module is unloaded and then re-probed, could the unbalanced disable_depth cause an "Unbalanced pm_runtime_enable!" warning and corrupt the power management state?

This looks like a false positive.

The cleanup path already calls pm_runtime_force_suspend(), and pm_runtime_force_suspend() itself invokes pm_runtime_disable() in the PM core.
Adding another explicit pm_runtime_disable() after pm_runtime_force_suspend() would over-increment disable_depth and break the reprobe path.

Quoted 5 lines

Quoted 5 lines

Quoted 4 lines

drivers/hwmon/eic7700-pvt.c new file +3 -0

+
+static int eic7700_pvt_probe(struct platform_device *pdev)
+{

[ ... ]

Quoted 11 lines

Quoted 11 lines

drivers/hwmon/eic7700-pvt.c line 430

Quoted 6 lines

drivers/hwmon/eic7700-pvt.c new file +6 -0

+	if (IS_ENABLED(CONFIG_PM))
+		clk_disable_unprepare(pvt->clk);
+
+	ret = eic7700_pvt_request_irq(pvt);
+	if (ret)
+		return ret;

If CONFIG_PM is disabled, the clock remains enabled. If eic7700_pvt_request_irq() fails here, does the function return without disabling the clock? This could permanently leak the clock reference because the devres cleanup action is not yet registered.

This concern is valid.

When CONFIG_PM is disabled, the clock remains enabled after probe-time initialization. If eic7700_pvt_request_irq() fails before the devres PM cleanup action is registered, the clock can indeed be leaked.

I will fix this by explicitly disabling the clock on the IRQ request error path when CONFIG_PM is disabled.

The probe will be changed along these lines:

    ret = eic7700_pvt_request_irq(pvt);
-   if (ret)
+   if (ret) {
+       if (!IS_ENABLED(CONFIG_PM))
+           clk_disable_unprepare(pvt->clk);
        return ret;
+   }

Quoted 11 lines

Quoted 11 lines

drivers/hwmon/eic7700-pvt.c line 432

Quoted 3 lines

drivers/hwmon/eic7700-pvt.c new file +2 -0

+
+	pm_runtime_enable(&pdev->dev);

Since the IRQ is unmasked before pm_runtime_enable() is called, what happens if the hardware asserts an interrupt in this window?

If eic7700_pvt_hard_isr() fires, pm_runtime_get_if_active() would return -EINVAL and the handler would return IRQ_NONE without clearing the interrupt. Since the interrupt is level-triggered (IRQF_TRIGGER_HIGH), could this cause an infinite IRQ storm that locks up the CPU?

My understanding is that eic7700_pvt_init_iface() already clears the interrupt status and disables PVT_ENA_EN before the IRQ is requested, so no new interrupt is expected in the request_irq() -> pm_runtime_enable() window.

Given that, do you think an additional change is still needed here?