Hi Guenter,

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

quoted

# 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:

    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.

quoted

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