Re: [PATCH v2 5/7] platform/chrome: of_hw_prober: Add delay for hana trackpads

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The result of regulator_is_enabled() is assigned to or bitwise-ANDed with a boolean variable, causing negative error codes to incorrectly evaluate to true and skip required hardware initialization delays.
--

commit 1aa1abec8abc658e89bd1583fb36c5b23bda7cf8
Author: Chen-Yu Tsai <wenst@chromium.org>

platform/chrome: of_hw_prober: Add delay for hana trackpads

Up until now, the MT8173 elm/hana device tree has set the dedicated
regulator supplying the trackpad as always-on, simply because the Elan
driver was missing proper delays. As a result the delay for the
Synaptics trackpad was also omitted, as it was not strictly required
under such a model and delayed the availability of the trackpad to the
user.

> The Elan driver recently gained proper delays after power-up, with
> adaptive skipping of the delay if the regulator was originally
> on. The I2C HID driver and I2C OF component prober library gained
> similar adaptive delay skipping. The device tree will be fixed to have
> the regulator not be always on, and proper post-power-on delay time
> added to the I2C HID device.

[Severity: High]
Does the adaptive skipping logic mentioned here safely handle negative error
codes from regulator_is_enabled()?

Looking at the implementations of this adaptive skipping,
regulator_is_enabled() can return a negative error code (e.g., -EIO, -EBUSY)
on failure, but this result is being directly assigned to or bitwise-ANDed
with a boolean.

For example, in drivers/hid/i2c-hid/i2c-hid-of.c:i2c_hid_of_power_up():

    for (unsigned int i = 0; i < ARRAY_SIZE(ihid_of->supplies); i++)
        supply_was_enabled &= regulator_is_enabled(ihid_of->supplies[i].consumer);

Since supply_was_enabled is promoted to int for the bitwise AND, does this
outcome depend on the parity of the error code? It looks like odd errors
(like -EIO) will yield true (skipping the delay), while even errors (like
-EBUSY) will yield false.

Similarly, in drivers/i2c/i2c-core-of-prober.c:
i2c_of_probe_simple_enable_regulator():

    supply_was_on = regulator_is_enabled(ctx->supply);

And in drivers/input/mouse/elan_i2c_core.c:elan_probe() and elan_resume():

    supply_was_enabled = regulator_is_enabled(data->vcc);

Since any non-zero integer evaluates to true in C, assigning a negative error
code directly to a bool means all errors will incorrectly signal that the
regulator was already on.

Could this cause a regression where a transient I2C or hardware error when
querying the regulator state makes the driver incorrectly skip required
hardware initialization delays and fail the device probe?

> Also add the post-power-on delay to the ChromeOS OF component prober,
> so that if the regulator is off at the time of probing, the prober knows
> to wait for the hardware to initialize.
>
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260703115601.1323491-1-wenst@chromium.org?part=5