linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] Allow regulator nodes to hold their own init data
@ 2018-11-29 10:28 Charles Keepax
  2018-11-29 10:28 ` [PATCH 1/2] regulator: Factor out location of init data OF node Charles Keepax
  2018-11-29 10:28 ` [PATCH 2/2] regulator: Allow regulator nodes to contain their own init data Charles Keepax
  0 siblings, 2 replies; 3+ messages in thread
From: Charles Keepax @ 2018-11-29 10:28 UTC (permalink / raw)
  To: broonie; +Cc: lgirdwood, linux-kernel, patches

Unfortunately due to a rather large testing oversight on my
part the recently merged Lochnagar regulator binding does not
actually work. The binding looks like this:

lochnagar {
        compatible = "cirrus,lochnagar1";

        ...

        lochnagar-micvdd: MICVDD {
                compatible = "cirrus,lochnagar2-micvdd";

                SYSVDD-supply = <&wallvdd>;

                regulator-min-microvolt = <3300000>;
                regulator-max-microvolt = <3300000>;
        };
        lochnagar-vddcore: VDDCORE {
                compatible = "cirrus,lochnagar2-vddcore";

                SYSVDD-supply = <&wallvdd>;

                regulator-min-microvolt = <1200000>;
                regulator-max-microvolt = <1200000>;
        };
};

The trouble is that each regulator node individually binds
in a driver and contains the init data. The regulator core
appears to require the init data to be a sub-node of the node
that bound in the regulator driver. As the above binding seems
reasonable I opted to try and update the core to support the
current binding, although as the rest of the Lochnagar driver
isn't merged yet we could still update the binding if that comes
out of the review as a preferred option. Apologies for missing
such a glaring issue in my testing.

Thanks,
Charles

Charles Keepax (2):
  regulator: Factor out location of init data OF node
  regulator: Allow regulator nodes to contain their own init data

 drivers/regulator/of_regulator.c | 72 ++++++++++++++++++++++++----------------
 1 file changed, 43 insertions(+), 29 deletions(-)

-- 
2.11.0


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

* [PATCH 1/2] regulator: Factor out location of init data OF node
  2018-11-29 10:28 [PATCH 0/2] Allow regulator nodes to hold their own init data Charles Keepax
@ 2018-11-29 10:28 ` Charles Keepax
  2018-11-29 10:28 ` [PATCH 2/2] regulator: Allow regulator nodes to contain their own init data Charles Keepax
  1 sibling, 0 replies; 3+ messages in thread
From: Charles Keepax @ 2018-11-29 10:28 UTC (permalink / raw)
  To: broonie; +Cc: lgirdwood, linux-kernel, patches

To support future additions factor out the location of the OF node
containing the init data for the regulator from the code that parses the
init data.

Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
 drivers/regulator/of_regulator.c | 64 +++++++++++++++++++++++-----------------
 1 file changed, 37 insertions(+), 27 deletions(-)

diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c
index c711a0a2bc4b..4bb8928bdb3f 100644
--- a/drivers/regulator/of_regulator.c
+++ b/drivers/regulator/of_regulator.c
@@ -371,13 +371,10 @@ int of_regulator_match(struct device *dev, struct device_node *node,
 }
 EXPORT_SYMBOL_GPL(of_regulator_match);
 
-struct regulator_init_data *regulator_of_get_init_data(struct device *dev,
-					    const struct regulator_desc *desc,
-					    struct regulator_config *config,
-					    struct device_node **node)
+struct device_node *regulator_of_get_init_node(struct device *dev,
+					       const struct regulator_desc *desc)
 {
 	struct device_node *search, *child;
-	struct regulator_init_data *init_data = NULL;
 	const char *name;
 
 	if (!dev->of_node || !desc->of_match)
@@ -400,35 +397,48 @@ struct regulator_init_data *regulator_of_get_init_data(struct device *dev,
 		if (!name)
 			name = child->name;
 
-		if (strcmp(desc->of_match, name))
-			continue;
+		if (!strcmp(desc->of_match, name))
+			return of_node_get(child);
+	}
 
-		init_data = of_get_regulator_init_data(dev, child, desc);
-		if (!init_data) {
-			dev_err(dev,
-				"failed to parse DT for regulator %pOFn\n",
-				child);
-			break;
-		}
+	of_node_put(search);
 
-		if (desc->of_parse_cb) {
-			if (desc->of_parse_cb(child, desc, config)) {
-				dev_err(dev,
-					"driver callback failed to parse DT for regulator %pOFn\n",
-					child);
-				init_data = NULL;
-				break;
-			}
-		}
+	return NULL;
+}
 
-		of_node_get(child);
-		*node = child;
-		break;
+struct regulator_init_data *regulator_of_get_init_data(struct device *dev,
+					    const struct regulator_desc *desc,
+					    struct regulator_config *config,
+					    struct device_node **node)
+{
+	struct device_node *child;
+	struct regulator_init_data *init_data = NULL;
+
+	child = regulator_of_get_init_node(dev, desc);
+	if (!child)
+		return NULL;
+
+	init_data = of_get_regulator_init_data(dev, child, desc);
+	if (!init_data) {
+		dev_err(dev, "failed to parse DT for regulator %pOFn\n", child);
+		goto error;
 	}
 
-	of_node_put(search);
+	if (desc->of_parse_cb && desc->of_parse_cb(child, desc, config)) {
+		dev_err(dev,
+			"driver callback failed to parse DT for regulator %pOFn\n",
+			child);
+		goto error;
+	}
+
+	*node = child;
 
 	return init_data;
+
+error:
+	of_node_put(child);
+
+	return NULL;
 }
 
 static int of_node_match(struct device *dev, const void *data)
-- 
2.11.0


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

* [PATCH 2/2] regulator: Allow regulator nodes to contain their own init data
  2018-11-29 10:28 [PATCH 0/2] Allow regulator nodes to hold their own init data Charles Keepax
  2018-11-29 10:28 ` [PATCH 1/2] regulator: Factor out location of init data OF node Charles Keepax
@ 2018-11-29 10:28 ` Charles Keepax
  1 sibling, 0 replies; 3+ messages in thread
From: Charles Keepax @ 2018-11-29 10:28 UTC (permalink / raw)
  To: broonie; +Cc: lgirdwood, linux-kernel, patches

Currently it is expected that regulator init data will be defined as a
series of sub-nodes from the node that bound in the driver. Add support
for a node to both bind in a driver and contain init data for that
regulator.

Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
 drivers/regulator/of_regulator.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c
index 4bb8928bdb3f..ffa5fc3724e4 100644
--- a/drivers/regulator/of_regulator.c
+++ b/drivers/regulator/of_regulator.c
@@ -380,12 +380,16 @@ struct device_node *regulator_of_get_init_node(struct device *dev,
 	if (!dev->of_node || !desc->of_match)
 		return NULL;
 
-	if (desc->regulators_node)
+	if (desc->regulators_node) {
 		search = of_get_child_by_name(dev->of_node,
 					      desc->regulators_node);
-	else
+	} else {
 		search = of_node_get(dev->of_node);
 
+		if (!strcmp(desc->of_match, search->name))
+			return search;
+	}
+
 	if (!search) {
 		dev_dbg(dev, "Failed to find regulator container node '%s'\n",
 			desc->regulators_node);
-- 
2.11.0


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

end of thread, other threads:[~2018-11-29 10:28 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-29 10:28 [PATCH 0/2] Allow regulator nodes to hold their own init data Charles Keepax
2018-11-29 10:28 ` [PATCH 1/2] regulator: Factor out location of init data OF node Charles Keepax
2018-11-29 10:28 ` [PATCH 2/2] regulator: Allow regulator nodes to contain their own init data Charles Keepax

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 NNTP newsgroup(s).