diff mbox series

[v4,2/6] regulator: Add regulator_of_get_optional() for pure DT regulator lookup

Message ID 20240808095931.2649657-3-wenst@chromium.org
State New
Headers show
Series platform/chrome: Introduce DT hardware prober | expand

Commit Message

Chen-Yu Tsai Aug. 8, 2024, 9:59 a.m. UTC 
The to-be-introduced I2C component prober needs to enable regulator
supplies (and toggle GPIO pins) for the various components it intends
to probe. To support this, a new "pure DT lookup" method for getting
regulator supplies is needed, since the device normally requesting
the supply won't get created until after the component is probed to
be available.

This adds a new regulator_of_get_optional() for this purpose. The
underlying code that support the existing regulator_get*() functions
are extended to support this specific case.

Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
Changes since v3:
- New patch
---
 drivers/regulator/core.c           | 81 ++++++++++++++++++++++--------
 drivers/regulator/devres.c         |  2 +-
 drivers/regulator/internal.h       |  2 +-
 include/linux/regulator/consumer.h |  8 +++
 4 files changed, 69 insertions(+), 24 deletions(-)

Comments

Andy Shevchenko Aug. 13, 2024, 11:22 a.m. UTC | #1 
On Thu, Aug 08, 2024 at 05:59:25PM +0800, Chen-Yu Tsai wrote:
> The to-be-introduced I2C component prober needs to enable regulator
> supplies (and toggle GPIO pins) for the various components it intends
> to probe. To support this, a new "pure DT lookup" method for getting
> regulator supplies is needed, since the device normally requesting
> the supply won't get created until after the component is probed to
> be available.
> 
> This adds a new regulator_of_get_optional() for this purpose. The
> underlying code that support the existing regulator_get*() functions
> are extended to support this specific case.

...

>  /**
>   * regulator_dev_lookup - lookup a regulator device.
>   * @dev: device for regulator "consumer".
> + * @node: device node for regulator supply lookup.
> + *        Falls back to dev->of_node if NULL.

Please, avoid using dereferences in the comments. Use plain language:
"Falls back to the OF node of the @dev, if NULL." or alike.

>   * @supply: Supply name or regulator ID.

>   */

...

>  static struct regulator_dev *regulator_dev_lookup(struct device *dev,
> +						  struct device_node *node,

This function has no of_ prefix in its name. If you want to make it for all,
please use fwnode instead. Otherwise I would expect a new one with of_ prefix.
(But I really prefer just agnostic, i.e. fwnode, approach!)

>  						  const char *supply)
>  {
> +	bool pure_dt_lookup = false;

Redundant assignment.

> +	pure_dt_lookup = (node && !dev);
>  
> +	/* Pure DT lookup should use given supply name directly */
> +	if (!pure_dt_lookup)
> +		regulator_supply_alias(&dev, &supply);
> +
> +	if (!node && dev && dev->of_node)

The dev->of_node is redundant and with the above...

> +		node = dev->of_node;

...this becomes as simple as

	if (!node && dev)

> +	/* Pure DT lookup stops here. */
> +	if (pure_dt_lookup)
> +		return ERR_PTR(-ENODEV);

Looking at this pure_dt_lookup and the above (somehow inverted) case I would
rather use (node && !dev) or (!node && dev) explicitly everywhere.

...

> +struct regulator *_regulator_get(struct device *dev, struct device_node *node,
> +				 const char *id, enum regulator_get_type get_type)

Again, no of_ prefix and function becomes OF-specific...
diff mbox series

Patch

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 7674b7f2df14..ba4542d76642 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -456,30 +456,29 @@  static struct device_node *of_get_child_regulator(struct device_node *parent,
 
 /**
  * of_get_regulator - get a regulator device node based on supply name
- * @dev: Device pointer for the consumer (of regulator) device
+ * @node: Device node pointer for supply property lookup
  * @supply: regulator supply name
  *
  * Extract the regulator device node corresponding to the supply name.
  * returns the device node corresponding to the regulator if found, else
  * returns NULL.
  */
-static struct device_node *of_get_regulator(struct device *dev, const char *supply)
+static struct device_node *of_get_regulator(struct device_node *node, const char *supply)
 {
 	struct device_node *regnode = NULL;
 	char prop_name[64]; /* 64 is max size of property name */
 
-	dev_dbg(dev, "Looking up %s-supply from device tree\n", supply);
+	pr_debug("%pOF: Looking up %s-supply from device tree\n", node, supply);
 
 	snprintf(prop_name, 64, "%s-supply", supply);
-	regnode = of_parse_phandle(dev->of_node, prop_name, 0);
+	regnode = of_parse_phandle(node, prop_name, 0);
 
 	if (!regnode) {
-		regnode = of_get_child_regulator(dev->of_node, prop_name);
+		regnode = of_get_child_regulator(node, prop_name);
 		if (regnode)
 			return regnode;
 
-		dev_dbg(dev, "Looking up %s property in node %pOF failed\n",
-				prop_name, dev->of_node);
+		pr_debug("%pOF: Looking up %s property failed\n", node, prop_name);
 		return NULL;
 	}
 	return regnode;
@@ -1996,8 +1995,14 @@  static struct regulator_dev *regulator_lookup_by_name(const char *name)
 /**
  * regulator_dev_lookup - lookup a regulator device.
  * @dev: device for regulator "consumer".
+ * @node: device node for regulator supply lookup.
+ *        Falls back to dev->of_node if NULL.
  * @supply: Supply name or regulator ID.
  *
+ * If dev is NULL and node is not NULL, a pure device tree lookup is assumed.
+ * That is, it will not use supply aliases and end after DT based lookup is
+ * done.
+ *
  * If successful, returns a struct regulator_dev that corresponds to the name
  * @supply and with the embedded struct device refcount incremented by one.
  * The refcount must be dropped by calling put_device().
@@ -2006,21 +2011,30 @@  static struct regulator_dev *regulator_lookup_by_name(const char *name)
  * in the future.
  */
 static struct regulator_dev *regulator_dev_lookup(struct device *dev,
+						  struct device_node *node,
 						  const char *supply)
 {
 	struct regulator_dev *r = NULL;
-	struct device_node *node;
+	struct device_node *regulator_node;
 	struct regulator_map *map;
 	const char *devname = NULL;
+	bool pure_dt_lookup = false;
+
+	pure_dt_lookup = (node && !dev);
 
-	regulator_supply_alias(&dev, &supply);
+	/* Pure DT lookup should use given supply name directly */
+	if (!pure_dt_lookup)
+		regulator_supply_alias(&dev, &supply);
+
+	if (!node && dev && dev->of_node)
+		node = dev->of_node;
 
 	/* first do a dt based lookup */
-	if (dev && dev->of_node) {
-		node = of_get_regulator(dev, supply);
-		if (node) {
-			r = of_find_regulator_by_node(node);
-			of_node_put(node);
+	if (node) {
+		regulator_node = of_get_regulator(node, supply);
+		if (regulator_node) {
+			r = of_find_regulator_by_node(regulator_node);
+			of_node_put(regulator_node);
 			if (r)
 				return r;
 
@@ -2032,6 +2046,10 @@  static struct regulator_dev *regulator_dev_lookup(struct device *dev,
 		}
 	}
 
+	/* Pure DT lookup stops here. */
+	if (pure_dt_lookup)
+		return ERR_PTR(-ENODEV);
+
 	/* if not found, try doing it non-dt way */
 	if (dev)
 		devname = dev_name(dev);
@@ -2076,7 +2094,7 @@  static int regulator_resolve_supply(struct regulator_dev *rdev)
 	if (rdev->supply)
 		return 0;
 
-	r = regulator_dev_lookup(dev, rdev->supply_name);
+	r = regulator_dev_lookup(dev, NULL, rdev->supply_name);
 	if (IS_ERR(r)) {
 		ret = PTR_ERR(r);
 
@@ -2169,8 +2187,8 @@  static int regulator_resolve_supply(struct regulator_dev *rdev)
 }
 
 /* Internal regulator request function */
-struct regulator *_regulator_get(struct device *dev, const char *id,
-				 enum regulator_get_type get_type)
+struct regulator *_regulator_get(struct device *dev, struct device_node *node,
+				 const char *id, enum regulator_get_type get_type)
 {
 	struct regulator_dev *rdev;
 	struct regulator *regulator;
@@ -2187,7 +2205,7 @@  struct regulator *_regulator_get(struct device *dev, const char *id,
 		return ERR_PTR(-EINVAL);
 	}
 
-	rdev = regulator_dev_lookup(dev, id);
+	rdev = regulator_dev_lookup(dev, node, id);
 	if (IS_ERR(rdev)) {
 		ret = PTR_ERR(rdev);
 
@@ -2318,7 +2336,7 @@  struct regulator *_regulator_get(struct device *dev, const char *id,
  */
 struct regulator *regulator_get(struct device *dev, const char *id)
 {
-	return _regulator_get(dev, id, NORMAL_GET);
+	return _regulator_get(dev, NULL, id, NORMAL_GET);
 }
 EXPORT_SYMBOL_GPL(regulator_get);
 
@@ -2345,7 +2363,7 @@  EXPORT_SYMBOL_GPL(regulator_get);
  */
 struct regulator *regulator_get_exclusive(struct device *dev, const char *id)
 {
-	return _regulator_get(dev, id, EXCLUSIVE_GET);
+	return _regulator_get(dev, NULL, id, EXCLUSIVE_GET);
 }
 EXPORT_SYMBOL_GPL(regulator_get_exclusive);
 
@@ -2371,10 +2389,29 @@  EXPORT_SYMBOL_GPL(regulator_get_exclusive);
  */
 struct regulator *regulator_get_optional(struct device *dev, const char *id)
 {
-	return _regulator_get(dev, id, OPTIONAL_GET);
+	return _regulator_get(dev, NULL, id, OPTIONAL_GET);
 }
 EXPORT_SYMBOL_GPL(regulator_get_optional);
 
+/**
+ * regulator_of_get_optional - get optional regulator via device tree lookup
+ * @node: device node for regulator "consumer"
+ * @id: Supply name
+ *
+ * Returns a struct regulator corresponding to the regulator producer,
+ * or IS_ERR() condition containing errno.
+ *
+ * This is intended for use by consumers that want to get a regulator
+ * supply directly from a device node, and can and want to deal with
+ * absence of such supplies. This will _not_ consider supply aliases.
+ * See regulator_dev_lookup().
+ */
+struct regulator *regulator_of_get_optional(struct device_node *node, const char *id)
+{
+	return _regulator_get(NULL, node, id, OPTIONAL_GET);
+}
+EXPORT_SYMBOL_GPL(regulator_of_get_optional);
+
 static void destroy_regulator(struct regulator *regulator)
 {
 	struct regulator_dev *rdev = regulator->rdev;
@@ -4928,7 +4965,7 @@  int _regulator_bulk_get(struct device *dev, int num_consumers,
 		consumers[i].consumer = NULL;
 
 	for (i = 0; i < num_consumers; i++) {
-		consumers[i].consumer = _regulator_get(dev,
+		consumers[i].consumer = _regulator_get(dev, NULL,
 						       consumers[i].supply, get_type);
 		if (IS_ERR(consumers[i].consumer)) {
 			ret = dev_err_probe(dev, PTR_ERR(consumers[i].consumer),
diff --git a/drivers/regulator/devres.c b/drivers/regulator/devres.c
index 7111c46e9de1..7d9ea8232959 100644
--- a/drivers/regulator/devres.c
+++ b/drivers/regulator/devres.c
@@ -28,7 +28,7 @@  static struct regulator *_devm_regulator_get(struct device *dev, const char *id,
 	if (!ptr)
 		return ERR_PTR(-ENOMEM);
 
-	regulator = _regulator_get(dev, id, get_type);
+	regulator = _regulator_get(dev, NULL, id, get_type);
 	if (!IS_ERR(regulator)) {
 		*ptr = regulator;
 		devres_add(dev, ptr);
diff --git a/drivers/regulator/internal.h b/drivers/regulator/internal.h
index 77a502141089..51eb552cba01 100644
--- a/drivers/regulator/internal.h
+++ b/drivers/regulator/internal.h
@@ -120,7 +120,7 @@  enum regulator_get_type {
 	MAX_GET_TYPE
 };
 
-struct regulator *_regulator_get(struct device *dev, const char *id,
+struct regulator *_regulator_get(struct device *dev, struct device_node *node, const char *id,
 				 enum regulator_get_type get_type);
 int _regulator_bulk_get(struct device *dev, int num_consumers,
 			struct regulator_bulk_data *consumers, enum regulator_get_type get_type);
diff --git a/include/linux/regulator/consumer.h b/include/linux/regulator/consumer.h
index d986ec13092e..76826d0d99f1 100644
--- a/include/linux/regulator/consumer.h
+++ b/include/linux/regulator/consumer.h
@@ -168,6 +168,8 @@  int devm_regulator_get_enable_read_voltage(struct device *dev, const char *id);
 void regulator_put(struct regulator *regulator);
 void devm_regulator_put(struct regulator *regulator);
 
+struct regulator *__must_check regulator_of_get_optional(struct device_node *node, const char *id);
+
 int regulator_register_supply_alias(struct device *dev, const char *id,
 				    struct device *alias_dev,
 				    const char *alias_id);
@@ -358,6 +360,12 @@  static inline void devm_regulator_put(struct regulator *regulator)
 {
 }
 
+static inline struct regulator *__must_check
+regulator_of_get_optional(struct device_node *node, const char *id)
+{
+	return ERR_PTR(-ENODEV);
+}
+
 static inline void devm_regulator_bulk_put(struct regulator_bulk_data *consumers)
 {
 }