On DT platforms, if a regulator init_data is provided in config, it is
silently ignored in favor of the DT parsing done by the framework.

If the regulator provider passed init_data it must be because it is useful
somehow. If the driver expects the framework to initialize this data on its
own, it should leave init_data clear.

Adjust the regulator registration accordingly.

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
Note that it is probably not problem at the moment since no one complained
about ignored data.
---
 drivers/regulator/core.c | 51 +++++++++++++++++++++++++-----------------------
 1 file changed, 27 insertions(+), 24 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 1179766811f5..fcafebcebf48 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -5681,32 +5681,35 @@ regulator_register(struct device *dev,
 		goto clean;
 	}
 
-	init_data = regulator_of_get_init_data(dev, regulator_desc, config,
-					       &rdev->dev.of_node);
-
-	/*
-	 * Sometimes not all resources are probed already so we need to take
-	 * that into account. This happens most the time if the ena_gpiod comes
-	 * from a gpio extender or something else.
-	 */
-	if (PTR_ERR(init_data) == -EPROBE_DEFER) {
-		ret = -EPROBE_DEFER;
-		goto clean;
-	}
-
-	/*
-	 * We need to keep track of any GPIO descriptor coming from the
-	 * device tree until we have handled it over to the core. If the
-	 * config that was passed in to this function DOES NOT contain
-	 * a descriptor, and the config after this call DOES contain
-	 * a descriptor, we definitely got one from parsing the device
-	 * tree.
-	 */
-	if (!cfg->ena_gpiod && config->ena_gpiod)
-		dangling_of_gpiod = true;
-	if (!init_data) {
+	if (config->init_data) {
 		init_data = config->init_data;
 		rdev->dev.of_node = of_node_get(config->of_node);
+
+	} else {
+		init_data = regulator_of_get_init_data(dev, regulator_desc,
+						       config,
+						       &rdev->dev.of_node);
+
+		/*
+		 * Sometimes not all resources are probed already so we need to
+		 * take that into account. This happens most the time if the
+		 * ena_gpiod comes from a gpio extender or something else.
+		 */
+		if (PTR_ERR(init_data) == -EPROBE_DEFER) {
+			ret = -EPROBE_DEFER;
+			goto clean;
+		}
+
+		/*
+		 * We need to keep track of any GPIO descriptor coming from the
+		 * device tree until we have handled it over to the core. If the
+		 * config that was passed in to this function DOES NOT contain a
+		 * descriptor, and the config after this call DOES contain a
+		 * descriptor, we definitely got one from parsing the device
+		 * tree.
+		 */
+		if (!cfg->ena_gpiod && config->ena_gpiod)
+			dangling_of_gpiod = true;
 	}
 
 	ww_mutex_init(&rdev->mutex, &regulator_ww_class);

---
base-commit: cd87a98b53518e44cf3c1a7c1c07c869ce33bf83
change-id: 20240920-regulator-ignored-data-78e7a855643e

Best regards,
-- 
Jerome

On Fri, Sep 20, 2024 at 07:21:12PM +0200, Jerome Brunet wrote:

> Note that it is probably not problem at the moment since no one complained
> about ignored data.

I'm fine with tweaking the functionality here but since I don't think
we're using this at all we should probably warn about systems with both
forms of constraints specified since they probably need some
consideration needed and people might not be doing this intentionally.

That probably means checking if regulator_of_get_init_node() can find
something and warning if that's the case.

On Mon 23 Sep 2024 at 16:19, Mark Brown <broonie@kernel.org> wrote:

> On Fri, Sep 20, 2024 at 07:21:12PM +0200, Jerome Brunet wrote:
>
>> Note that it is probably not problem at the moment since no one complained
>> about ignored data.
>
> I'm fine with tweaking the functionality here but since I don't think
> we're using this at all we should probably warn about systems with both
> forms of constraints specified since they probably need some
> consideration needed and people might not be doing this intentionally.
>
> That probably means checking if regulator_of_get_init_node() can find
> something and warning if that's the case.

We could warn if both init_data and desc->of_match are set ?

Setting desc->of_match is an indication regulator is expected to search
DT, is'nt it ? Having both set be the indicaction of the conflict.

Maybe the warn is enough then. Do you prefer if I keep the change of v1
or drop it ?

-- 
Jerome

On Mon, Sep 23, 2024 at 06:50:33PM +0200, Jerome Brunet wrote:
> On Mon 23 Sep 2024 at 16:19, Mark Brown <broonie@kernel.org> wrote:

> > That probably means checking if regulator_of_get_init_node() can find
> > something and warning if that's the case.

> We could warn if both init_data and desc->of_match are set ?

> Setting desc->of_match is an indication regulator is expected to search
> DT, is'nt it ? Having both set be the indicaction of the conflict.

> Maybe the warn is enough then. Do you prefer if I keep the change of v1
> or drop it ?

Yes, that'd detect problems.  There's some older drivers that don't use
of_match but it'd cover most things quite cheaply.