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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Clearing the interrupt status before disabling sensor conversions in init_iface() creates a race condition that can lead to an unhandled interrupt storm.
- [Medium] Missing pm_runtime_disable() in the device cleanup path causes an unbalanced PM runtime state.
--

commit 5842ad32271f7d967e9f1e10c13474a5b57da8ad
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
> new file mode 100644
> --- /dev/null
> +++ b/drivers/hwmon/eic7700-pvt.c
[ ... ]
> +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);

[Severity: High]
Could this clear-then-disable sequence cause an unhandled interrupt storm?

If a bootloader left the sensor active, a conversion might complete in the
small window between clearing PVT_INT_CLR and disabling PVT_ENA_EN. This
would assert the IRQ line.

When the IRQ is later requested in probe(), the ISR eic7700_pvt_hard_isr()
will fire immediately. However, since the device is suspended,
pm_runtime_get_if_active() returns <= 0, causing the ISR to return IRQ_NONE
without clearing the interrupt.

Because the interrupt is level-triggered (IRQF_TRIGGER_HIGH), would this
result in a continuous interrupt storm that ultimately forces the kernel
to permanently disable the IRQ line?

Should the conversion engine be disabled before the interrupt is cleared?

[ ... ]
> +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);
> +}

[Severity: Medium]
Does this device cleanup path need a call to pm_runtime_disable()?

In eic7700_pvt_probe(), pm_runtime_enable() is called, which alters the
PM core's disable_depth count. Since pm_runtime_disable() is missing here,
if the driver is unbound and later rebound, the next pm_runtime_enable()
will encounter an unbalanced depth.

Could this trigger an "Unbalanced pm_runtime_enable!" warning and cause
inconsistent PM state tracking?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260604080430.126-1-hehuan1@eswincomputing.com?part=2