Re: [PATCH v2 1/7] Input: elan_i2c - Wait for initialization after enabling regulator supply
From: sashiko-bot@kernel.org
Date: 2026-07-03 12:06:01
Also in:
linux-devicetree
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] The return value of `regulator_is_enabled()` is implicitly cast to `bool`, causing negative error codes to evaluate as true and incorrectly bypassing the required power-on delay. - [High] Skipping the power-on delay for shared regulators is susceptible to unsafe power domain assumptions and TOCTOU races, leading to bypassed hardware initialization. -- commit 62ef124436c7ca613cd0f8d922094f42cdd2a0b8 Author: Chen-Yu Tsai [off-list ref] Input: elan_i2c - Wait for initialization after enabling regulator supply This commit adds proper delays after enabling the regulator supply for Elan trackpad controllers, which require time for hardware and firmware to initialize. It attempts to avoid boot time impact by skipping the delay if the regulator was already enabled.
quoted hunk ↗ jump to hunk
diff --git a/drivers/input/mouse/elan_i2c_core.c b/drivers/input/mouse/elan_i2c_core.c index fee1796da3d08..110c662cef96f 100644 --- a/drivers/input/mouse/elan_i2c_core.c +++ b/drivers/input/mouse/elan_i2c_core.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -1187,6 +1189,7 @@ static int elan_probe(struct i2c_client *client) struct device *dev = &client->dev; struct elan_tp_data *data; unsigned long irqflags; + bool supply_was_enabled; int error; if (IS_ENABLED(CONFIG_MOUSE_ELAN_I2C_I2C) &&@@ -1218,6 +1221,8 @@ static int elan_probe(struct i2c_client *client) if (IS_ERR(data->vcc)) return dev_err_probe(dev, PTR_ERR(data->vcc), "Failed to get 'vcc' regulator\n"); + supply_was_enabled = regulator_is_enabled(data->vcc);
[Severity: High] In elan_probe(), regulator_is_enabled() returns an int, which can include negative error codes like -EPROBE_DEFER or -EREMOTEIO. Will assigning this to the bool supply_was_enabled cause negative error codes to evaluate as true? If this happens, it looks like it would incorrectly bypass the power-on delay and lead to a permanent -ENXIO probe failure later rather than allowing a proper deferral.
quoted hunk ↗ jump to hunk
+ error = regulator_enable(data->vcc); if (error) { dev_err(dev, "Failed to enable regulator: %d\n", error);@@ -1231,6 +1236,9 @@ static int elan_probe(struct i2c_client *client) return error; } + if (!supply_was_enabled) + msleep(ETP_POWER_ON_DELAY);
[Severity: High] Is it safe to assume that a regulator that is already enabled has been powered on for a sufficient amount of time? If the trackpad shares a regulator with another device (like a touchscreen) that just finished its own probe milliseconds earlier, the regulator would already be enabled, but the trackpad hardware wouldn't be fully initialized. Could this cause elan_probe() to skip the required hardware initialization delay and fail the subsequent i2c_smbus_read_byte() check? [ ... ]
quoted hunk ↗ jump to hunk
@@ -1374,11 +1382,16 @@ static int elan_resume(struct device *dev) int error; if (!device_may_wakeup(dev)) { + bool supply_was_enabled = regulator_is_enabled(data->vcc);
[Severity: High] In elan_resume(), could the same boolean cast issue happen if regulator_is_enabled() encounters a transient failure during system resume, causing the resume delay to be skipped?
+
error = regulator_enable(data->vcc);
if (error) {
dev_err(dev, "error %d enabling regulator\n", error);
goto err;
}
+
+ if (!supply_was_enabled)
+ msleep(ETP_POWER_ON_DELAY);[Severity: High] Could the same shared-regulator timing race occur during the system resume path, skipping the initialization delay if another device on the same shared power domain resumed just before the trackpad?
} error = elan_set_power(data, true);
-- Sashiko AI review · https://sashiko.dev/#/patchset/20260703115601.1323491-1-wenst@chromium.org?part=1