LKML Archive mirror
 help / color / mirror / Atom feed
* [PATCH] regulator: do not ignore provided init_data
@ 2024-09-20 17:21 Jerome Brunet
  2024-09-23 14:19 ` Mark Brown
  0 siblings, 1 reply; 4+ messages in thread
From: Jerome Brunet @ 2024-09-20 17:21 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown; +Cc: linux-kernel, Jerome Brunet

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


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] regulator: do not ignore provided init_data
  2024-09-20 17:21 [PATCH] regulator: do not ignore provided init_data Jerome Brunet
@ 2024-09-23 14:19 ` Mark Brown
  2024-09-23 16:50   ` Jerome Brunet
  0 siblings, 1 reply; 4+ messages in thread
From: Mark Brown @ 2024-09-23 14:19 UTC (permalink / raw)
  To: Jerome Brunet; +Cc: Liam Girdwood, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 551 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] regulator: do not ignore provided init_data
  2024-09-23 14:19 ` Mark Brown
@ 2024-09-23 16:50   ` Jerome Brunet
  2024-09-24  8:13     ` Mark Brown
  0 siblings, 1 reply; 4+ messages in thread
From: Jerome Brunet @ 2024-09-23 16:50 UTC (permalink / raw)
  To: Mark Brown; +Cc: Liam Girdwood, linux-kernel

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] regulator: do not ignore provided init_data
  2024-09-23 16:50   ` Jerome Brunet
@ 2024-09-24  8:13     ` Mark Brown
  0 siblings, 0 replies; 4+ messages in thread
From: Mark Brown @ 2024-09-24  8:13 UTC (permalink / raw)
  To: Jerome Brunet; +Cc: Liam Girdwood, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 676 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2024-09-24  8:14 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-20 17:21 [PATCH] regulator: do not ignore provided init_data Jerome Brunet
2024-09-23 14:19 ` Mark Brown
2024-09-23 16:50   ` Jerome Brunet
2024-09-24  8:13     ` Mark Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).