Re: [PATCH v3 4/9] HID: i2c-hid-of: skip post-power-on delay if powered on sufficiently long

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Skipping the post-power-on delay when regulators are already enabled reduces the reset assertion pulse width to near zero, causing device initialization failures.
--

--- Patch [4]: [PATCH v3 4/9] HID: i2c-hid-of: skip post-power-on delay if powered on sufficiently long ---
commit 77e6d660661b46c12652ca11459dae1f91a79af7
Author: Chen-Yu Tsai <wenst@chromium.org>

HID: i2c-hid-of: skip post-power-on delay if powered on sufficiently long

This commit switches the post-power-on delay to use the new
regulator_bulk_enable_and_wait() API. This avoids unnecessary boot time
delays on devices where the power rail is always-on or left enabled by
the bootloader.

> diff --git a/drivers/hid/i2c-hid/i2c-hid-of.c b/drivers/hid/i2c-hid/i2c-hid-of.c
> index 59393d71ddb9d..fdaad451e7102 100644
> --- a/drivers/hid/i2c-hid/i2c-hid-of.c
> +++ b/drivers/hid/i2c-hid/i2c-hid-of.c

[ ... ]

> @@ -48,16 +49,14 @@ static int i2c_hid_of_power_up(struct i2chid_ops *ops)
>  	struct device *dev = &ihid_of->client->dev;
>  	int ret;
>  
> -	ret = regulator_bulk_enable(ARRAY_SIZE(ihid_of->supplies),
> -				    ihid_of->supplies);
> +	ret = regulator_bulk_enable_and_wait(ARRAY_SIZE(ihid_of->supplies),
> +					     ihid_of->supplies,
> +					     ihid_of->post_power_delay_ms * USEC_PER_MSEC);
>  	if (ret) {
>  		dev_warn(dev, "Failed to enable supplies: %d\n", ret);
>  		return ret;
>  	}
>  
> -	if (ihid_of->post_power_delay_ms)
> -		msleep(ihid_of->post_power_delay_ms);
> -
>  	gpiod_set_value_cansleep(ihid_of->reset_gpio, 0);

[Severity: High]
Does skipping this delay cause a regression where the reset assertion pulse 
becomes too short?

If the regulators are already enabled (e.g., from an always-on rail or
bootloader), regulator_bulk_enable_and_wait() will return immediately
without waiting. 

Because the reset GPIO is asserted during probe just before calling
i2c_hid_of_power_up(), skipping the post-power-on delay here causes the
reset line to be de-asserted almost instantly. This reduces the reset
assertion pulse width to near zero, which might violate the minimum reset
assertion time required by the hardware.

>  	if (ihid_of->post_reset_delay_ms)
>  		msleep(ihid_of->post_reset_delay_ms);

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