Re: [PATCH v2 4/7] i2c: of-prober: Defer regulator_disable() on successful probe in simple helper

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The prober unconditionally sleeps for 100ms during the cleanup path even when no matching I2C component is found, unnecessarily delaying system boot.
--

commit 772a8f5b00b1078ef6740a0746df36699760f2f2
Author: Chen-Yu Tsai <wenst@chromium.org>

i2c: of-prober: Defer regulator_disable() on successful probe in simple helper

This patch defers regulator disabling in the I2C OF component prober by adding
a 100ms delay. The delay allows asynchronously probing drivers to catch and
increase the regulator enable count, avoiding hardware re-initialization.

> diff --git a/drivers/i2c/i2c-core-of-prober.c b/drivers/i2c/i2c-core-of-prober.c
> index f274e260353cd..17495125467ed 100644
> --- a/drivers/i2c/i2c-core-of-prober.c
> +++ b/drivers/i2c/i2c-core-of-prober.c
> @@ -243,11 +243,23 @@ static int i2c_of_probe_simple_enable_regulator(struct device *dev, struct i2c_o
>  	return 0;
>  }
>  
> -static void i2c_of_probe_simple_disable_regulator(struct device *dev, struct i2c_of_probe_simple_ctx *ctx)
> +static void i2c_of_probe_simple_disable_regulator(struct device *dev,
> +						  struct i2c_of_probe_simple_ctx *ctx,
> +						  bool defer_disable)
>  {
>  	if (!ctx->supply)
>  		return;
>  
> +	/*
> +	 * Wait a bit of time for async drivers to probe and increase the
> +	 * regulator enable count. This allows the drivers to check and
> +	 * skip waiting for re-initialization.
> +	 */
> +	if (defer_disable) {
> +		dev_dbg(dev, "Deferring regulator disable\n");
> +		msleep(100);
> +	}
> +

[ ... ]

> @@ -409,7 +421,7 @@ void i2c_of_probe_simple_cleanup(struct device *dev, void *data)
>  	i2c_of_probe_simple_disable_gpio(dev, ctx);
>  	i2c_of_probe_simple_put_gpiod(ctx);
>  
> -	i2c_of_probe_simple_disable_regulator(dev, ctx);
> +	i2c_of_probe_simple_disable_regulator(dev, ctx, true);

[Severity: Medium]
Does this unconditionally sleep for 100ms even when no matching I2C component
is found?

If i2c_of_probe_component() iterates over potential components but finds none,
it calls ops->cleanup(dev, ctx), which invokes i2c_of_probe_simple_cleanup().

Because defer_disable is hardcoded to true here,
i2c_of_probe_simple_disable_regulator() will execute the 100ms sleep.

Could this result in an unnecessary 100ms boot delay on systems missing the
probed component, since no async driver will ever probe in this failure path?

>  	i2c_of_probe_simple_put_supply(ctx);
>  }

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