* [PATCH v5 0/2] Add driver support for ESWIN EIC7700 PVT controller
@ 2026-05-15 9:19 hehuan1
2026-05-15 9:20 ` [PATCH v5 1/2] dt-bindings: hwmon: Add Eswin EIC7700 PVT sensor hehuan1
2026-05-15 9:21 ` [PATCH v5 2/2] hwmon: Add Eswin EIC7700 PVT sensor driver hehuan1
0 siblings, 2 replies; 5+ messages in thread
From: hehuan1 @ 2026-05-15 9:19 UTC (permalink / raw)
To: linux, robh, krzk+dt, conor+dt, p.zabel, linux-hwmon, devicetree,
linux-kernel
Cc: ningyu, linmin, pinkesh.vaghela, luyulin, Huan He
From: Huan He <hehuan1@eswincomputing.com>
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 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/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 | 63 +++
drivers/hwmon/Kconfig | 12 +
drivers/hwmon/Makefile | 1 +
drivers/hwmon/eic7700-pvt.c | 506 ++++++++++++++++++
drivers/hwmon/eic7700-pvt.h | 99 ++++
5 files changed, 681 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
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH v5 1/2] dt-bindings: hwmon: Add Eswin EIC7700 PVT sensor 2026-05-15 9:19 [PATCH v5 0/2] Add driver support for ESWIN EIC7700 PVT controller hehuan1 @ 2026-05-15 9:20 ` hehuan1 2026-05-15 9:21 ` [PATCH v5 2/2] hwmon: Add Eswin EIC7700 PVT sensor driver hehuan1 1 sibling, 0 replies; 5+ messages in thread From: hehuan1 @ 2026-05-15 9:20 UTC (permalink / raw) To: linux, robh, krzk+dt, conor+dt, p.zabel, linux-hwmon, devicetree, linux-kernel Cc: ningyu, linmin, pinkesh.vaghela, luyulin, Huan He From: Huan He <hehuan1@eswincomputing.com> 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 <luyulin@eswincomputing.com> Signed-off-by: Huan He <hehuan1@eswincomputing.com> --- .../bindings/hwmon/eswin,eic7700-pvt.yaml | 63 +++++++++++++++++++ 1 file changed, 63 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..27cc90e52d4b --- /dev/null +++ b/Documentation/devicetree/bindings/hwmon/eswin,eic7700-pvt.yaml @@ -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 <luyulin@eswincomputing.com> + - Huan He <hehuan1@eswincomputing.com> + +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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v5 2/2] hwmon: Add Eswin EIC7700 PVT sensor driver 2026-05-15 9:19 [PATCH v5 0/2] Add driver support for ESWIN EIC7700 PVT controller hehuan1 2026-05-15 9:20 ` [PATCH v5 1/2] dt-bindings: hwmon: Add Eswin EIC7700 PVT sensor hehuan1 @ 2026-05-15 9:21 ` hehuan1 2026-05-15 10:03 ` sashiko-bot 1 sibling, 1 reply; 5+ messages in thread From: hehuan1 @ 2026-05-15 9:21 UTC (permalink / raw) To: linux, robh, krzk+dt, conor+dt, p.zabel, linux-hwmon, devicetree, linux-kernel Cc: ningyu, linmin, pinkesh.vaghela, luyulin, Huan He From: Huan He <hehuan1@eswincomputing.com> 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 sensors. 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 | 506 ++++++++++++++++++++++++++++++++++++ drivers/hwmon/eic7700-pvt.h | 99 +++++++ 4 files changed, 618 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 14e4cea48acc..6cb60355aa0c 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -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 diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile index 982ee2c6f9de..d647669074e1 100644 --- a/drivers/hwmon/Makefile +++ b/drivers/hwmon/Makefile @@ -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 diff --git a/drivers/hwmon/eic7700-pvt.c b/drivers/hwmon/eic7700-pvt.c new file mode 100644 index 000000000000..cdcfa031cdbd --- /dev/null +++ b/drivers/hwmon/eic7700-pvt.c @@ -0,0 +1,506 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * ESWIN EIC7700 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), +}; + +/* + * 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; + int active; + u32 val; + + if (IS_ENABLED(CONFIG_PM)) { + active = pm_runtime_get_if_active(pvt->dev); + if (active <= 0) + return IRQ_NONE; + } + + 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); + + if (IS_ENABLED(CONFIG_PM)) + pm_runtime_put_noidle(pvt->dev); + + return IRQ_HANDLED; +} + +static int eic7700_pvt_read_data(struct pvt_hwmon *pvt, + enum pvt_sensor_type type, long *val) +{ + unsigned long timeout; + 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); + + /* + * Wait with timeout since in case if the sensor is suddenly powered + * down the request won't be completed and the caller will hang up on + * this procedure until the power is back up again. Multiply the + * timeout by the factor of two to prevent a false timeout. + */ + timeout = 2 * usecs_to_jiffies(ktime_to_us(pvt->timeout)); + ret = wait_for_completion_timeout(&pvt->conversion, timeout); + + eic7700_pvt_update(pvt->regs + PVT_ENA, PVT_ENA_EN, 0); + eic7700_pvt_update(pvt->regs + PVT_INT, PVT_INT_CLR, PVT_INT_CLR); + + if (!ret) + synchronize_irq(pvt->irq); + + data = READ_ONCE(pvt->data_cache); + + if (!ret) + return -ETIMEDOUT; + + if (type == PVT_TEMP) + *val = polynomial_calc(&poly_N_to_temp, data); + else + *val = polynomial_calc(&poly_N_to_volt, 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_CHANNEL_INFO(in, 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; + } + 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_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); + pm_runtime_put_noidle(pvt->dev); + return ret; + } + + switch (type) { + case hwmon_temp: + switch (attr) { + case hwmon_temp_input: + ret = eic7700_pvt_read_data(pvt, ch, 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 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 +}; + +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_init_iface(struct pvt_hwmon *pvt) +{ + /* + * 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); + + /* + * Max conversion latency (~333 µs) derived from PVT spec: + * maximum sampling rate = 3000 samples/sec. + */ + pvt->timeout = ns_to_ktime(PVT_TOUT_MIN); + + 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_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) +{ + pvt->hwmon = devm_hwmon_device_register_with_info(pvt->dev, "pvt", + 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_force_suspend(pvt->dev); + + if (!IS_ENABLED(CONFIG_PM)) + clk_disable_unprepare(pvt->clk); +} + +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(&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 = clk_prepare_enable(pvt->clk); + if (ret) + return dev_err_probe(pvt->dev, ret, "Failed to enable clock\n"); + + ret = eic7700_pvt_init_iface(pvt); + if (ret) { + clk_disable_unprepare(pvt->clk); + return ret; + } + + if (IS_ENABLED(CONFIG_PM)) + clk_disable_unprepare(pvt->clk); + + 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) { + pm_runtime_put_noidle(&pdev->dev); + return dev_err_probe(&pdev->dev, ret, + "Can't register PM cleanup\n"); + } + + ret = eic7700_pvt_create_hwmon(pvt); + if (ret) + goto err_put_pm_runtime; + + pm_runtime_put_autosuspend(&pdev->dev); + + return 0; + +err_put_pm_runtime: + pm_runtime_put_noidle(&pdev->dev); + return ret; +} + +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 const struct dev_pm_ops eic7700_pvt_pm_ops = { + SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, pm_runtime_force_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_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..6640328de1cd --- /dev/null +++ b/drivers/hwmon/eic7700-pvt.h @@ -0,0 +1,99 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * ESWIN EIC7700 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_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 1 +#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. + */ +enum pvt_sensor_type { + PVT_TEMP = 0, + PVT_VOLT +}; + +/* + * 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. + * @timeout: conversion timeout. + */ +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; + ktime_t timeout; +}; + +#endif /* __HWMON_EIC7700_PVT_H__ */ -- 2.25.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v5 2/2] hwmon: Add Eswin EIC7700 PVT sensor driver 2026-05-15 9:21 ` [PATCH v5 2/2] hwmon: Add Eswin EIC7700 PVT sensor driver hehuan1 @ 2026-05-15 10:03 ` sashiko-bot 2026-05-15 10:24 ` Guenter Roeck 0 siblings, 1 reply; 5+ messages in thread From: sashiko-bot @ 2026-05-15 10:03 UTC (permalink / raw) To: hehuan1; +Cc: linux-hwmon, robh, krzk+dt, conor+dt, devicetree 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 <hehuan1@eswincomputing.com> 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. > diff --git a/drivers/hwmon/eic7700-pvt.c b/drivers/hwmon/eic7700-pvt.c > index 0000000000000..cdcfa031cdbdb > --- /dev/null > +++ b/drivers/hwmon/eic7700-pvt.c [ ... ] > +static irqreturn_t eic7700_pvt_hard_isr(int irq, void *data) > +{ [ ... ] > + 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()? [ ... ] > +static int eic7700_pvt_read_data(struct pvt_hwmon *pvt, > + enum pvt_sensor_type type, long *val) > +{ > + unsigned long timeout; [ ... ] > + 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. [ ... ] > +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? [ ... ] > +static struct pvt_hwmon *eic7700_pvt_create_data(struct platform_device *pdev) > +{ [ ... ] > + 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. [ ... ] > +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? > + > +static int eic7700_pvt_probe(struct platform_device *pdev) > +{ [ ... ] > + 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. > + > + 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? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260515091942.449-1-hehuan1@eswincomputing.com?part=2 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v5 2/2] hwmon: Add Eswin EIC7700 PVT sensor driver 2026-05-15 10:03 ` sashiko-bot @ 2026-05-15 10:24 ` Guenter Roeck 0 siblings, 0 replies; 5+ messages in thread From: Guenter Roeck @ 2026-05-15 10:24 UTC (permalink / raw) To: sashiko-reviews, hehuan1; +Cc: linux-hwmon, robh, krzk+dt, conor+dt, devicetree On 5/15/26 03:03, sashiko-bot@kernel.org wrote: > 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 <hehuan1@eswincomputing.com> > > 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. > >> diff --git a/drivers/hwmon/eic7700-pvt.c b/drivers/hwmon/eic7700-pvt.c >> index 0000000000000..cdcfa031cdbdb >> --- /dev/null >> +++ b/drivers/hwmon/eic7700-pvt.c > [ ... ] >> +static irqreturn_t eic7700_pvt_hard_isr(int irq, void *data) >> +{ > [ ... ] >> + 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()? > > [ ... ] >> +static int eic7700_pvt_read_data(struct pvt_hwmon *pvt, >> + enum pvt_sensor_type type, long *val) >> +{ >> + unsigned long timeout; > [ ... ] >> + 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. > > [ ... ] >> +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? > 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 > [ ... ] >> +static struct pvt_hwmon *eic7700_pvt_create_data(struct platform_device *pdev) >> +{ > [ ... ] >> + 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. > > [ ... ] >> +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? > >> + >> +static int eic7700_pvt_probe(struct platform_device *pdev) >> +{ > [ ... ] >> + 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. > >> + >> + 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? > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-05-15 10:24 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-05-15 9:19 [PATCH v5 0/2] Add driver support for ESWIN EIC7700 PVT controller hehuan1 2026-05-15 9:20 ` [PATCH v5 1/2] dt-bindings: hwmon: Add Eswin EIC7700 PVT sensor hehuan1 2026-05-15 9:21 ` [PATCH v5 2/2] hwmon: Add Eswin EIC7700 PVT sensor driver hehuan1 2026-05-15 10:03 ` sashiko-bot 2026-05-15 10:24 ` Guenter Roeck
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox