Thread (12 messages) flat view 12 messages, 2 authors, 6d ago

Re: [PATCH v6 4/8] i2c: of-prober: skip post-power-on delay if powered on sufficiently long

From: sashiko-bot@kernel.org
Date: 2026-09-01 08:02:09
Also in: linux-devicetree

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The i2c_of_probe_simple_get_supply function fails the probe if the regulator is not found in the device tree, contradicting its stated intent to treat missing regulators as optional.
--

commit c0491b6ceb5b608dbfb97b1a7a7116f271999495
Author: Chen-Yu Tsai [off-list ref]

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

This patch switches the I2C of-prober to use the new 
regulator_enable_and_wait() function to handle post-power-on delays. This
avoids unnecessary boot time penalties on devices where the power rail is
always-on or initialized by the bootloader.
quoted hunk ↗ jump to hunk
diff --git a/drivers/i2c/i2c-core-of-prober.c b/drivers/i2c/i2c-core-of-prober.c
index 6a82b03809d4f..f9f3c0ef93ff5 100644
--- a/drivers/i2c/i2c-core-of-prober.c
+++ b/drivers/i2c/i2c-core-of-prober.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -226,13 +227,11 @@ static int i2c_of_probe_simple_enable_regulator(struct device *dev, struct i2c_o
 
 	dev_dbg(dev, "Enabling regulator supply \"%s\"\n", ctx->opts->supply_name);
 
-	ret = regulator_enable(ctx->supply);
+	ret = regulator_enable_and_wait(ctx->supply,
+					ctx->opts->post_power_on_delay_ms * USEC_PER_MSEC);
 	if (ret)
 		return ret;
 
-	if (ctx->opts->post_power_on_delay_ms)
-		msleep(ctx->opts->post_power_on_delay_ms);
-
 	return 0;
 }
[Severity: High]
This is a pre-existing issue, but I noticed something related to the regulator
handling in i2c_of_probe_simple_get_supply().

Does this code incorrectly fail the probe when a regulator is missing?

In i2c_of_probe_simple_get_supply(), we call of_regulator_get_optional():

	supply = of_regulator_get_optional(dev, node, supply_name);
	if (IS_ERR(supply)) {
		return dev_err_probe(dev, PTR_ERR(supply),
				     "Failed to get regulator supply \"%s\" from %pOF\n",
				     supply_name, node);
	}

Since of_regulator_get_optional() returns -ENODEV when the supply is not found
in the device tree, does this IS_ERR() check cause a fatal initialization
failure for valid hardware configurations where the supply is always-on or
not modeled?

Could this contradict the stated intent of treating missing regulators as
optional?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901074930.764550-1-wenst@chromium.org?part=4
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help