[PATCH] regulator: Allow consumer supplies to be set up with dev_name()

[PATCH] regulator: Allow consumer supplies to be set up with dev_name()

[Mark Brown]

Follow the approach suggested by Russell King and implemented by him in
the clkdev API and allow consumer device supply mapings to be set up
using the dev_name() for the consumer instead of the struct device.
In order to avoid making existing machines instabuggy and creating merge
issues the use of struct device is still supported for the time being.

This resolves problems working with buses such as I2C which make the
struct device available late providing that the final device name is
known, which is the case for most embedded systems with fixed setups.

Consumers must still use the struct device when calling regulator_get().

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
For the avoidance of doubt this is most definately 2.6.32 material, not
2.6.31.

 drivers/regulator/core.c          |   62 ++++++++++++++++++++++++++++--------
 include/linux/regulator/machine.h |    7 +++-
 2 files changed, 54 insertions(+), 15 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 98c3a74..eed62ad 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -37,7 +37,7 @@ static int has_full_constraints;
  */
 struct regulator_map {
 	struct list_head list;
-	struct device *dev;
+	const char *dev_name;   /* The dev_name() for the consumer */
 	const char *supply;
 	struct regulator_dev *regulator;
 };
@@ -857,23 +857,33 @@ out:
  * set_consumer_device_supply: Bind a regulator to a symbolic supply
  * @rdev:         regulator source
  * @consumer_dev: device the supply applies to
+ * @consumer_dev_name: dev_name() string for device supply applies to
  * @supply:       symbolic name for supply
  *
  * Allows platform initialisation code to map physical regulator
  * sources to symbolic names for supplies for use by devices.  Devices
  * should use these symbolic names to request regulators, avoiding the
  * need to provide board-specific regulator names as platform data.
+ *
+ * Only one of consumer_dev and consumer_dev_name may be specified.
  */
 static int set_consumer_device_supply(struct regulator_dev *rdev,
-	struct device *consumer_dev, const char *supply)
+	struct device *consumer_dev, const char *consumer_dev_name,
+	const char *supply)
 {
 	struct regulator_map *node;
 
+	if (consumer_dev && consumer_dev_name)
+		return -EINVAL;
+
+	if (!consumer_dev_name && consumer_dev)
+		consumer_dev_name = dev_name(consumer_dev);
+
 	if (supply == NULL)
 		return -EINVAL;
 
 	list_for_each_entry(node, &regulator_map_list, list) {
-		if (consumer_dev != node->dev)
+		if (consumer_dev_name != node->dev_name)
 			continue;
 		if (strcmp(node->supply, supply) != 0)
 			continue;
@@ -891,25 +901,38 @@ static int set_consumer_device_supply(struct regulator_dev *rdev,
 		return -ENOMEM;
 
 	node->regulator = rdev;
-	node->dev = consumer_dev;
+	node->dev_name = kstrdup(consumer_dev_name, GFP_KERNEL);
 	node->supply = supply;
 
+	if (node->dev_name == NULL) {
+		kfree(node);
+		return -ENOMEM;
+	}
+
 	list_add(&node->list, &regulator_map_list);
 	return 0;
 }
 
 static void unset_consumer_device_supply(struct regulator_dev *rdev,
-	struct device *consumer_dev)
+	const char *consumer_dev_name, struct device *consumer_dev)
 {
 	struct regulator_map *node, *n;
 
+	if (consumer_dev && !consumer_dev_name)
+		consumer_dev_name = dev_name(consumer_dev);
+
 	list_for_each_entry_safe(node, n, &regulator_map_list, list) {
-		if (rdev == node->regulator &&
-			consumer_dev == node->dev) {
-			list_del(&node->list);
-			kfree(node);
-			return;
-		}
+		if (rdev != node->regulator)
+			continue;
+
+		if (consumer_dev_name && node->dev_name &&
+		    strcmp(consumer_dev_name, node->dev_name))
+			continue;
+
+		list_del(&node->list);
+		kfree(node->dev_name);
+		kfree(node);
+		return;
 	}
 }
 
@@ -920,6 +943,7 @@ static void unset_regulator_supplies(struct regulator_dev *rdev)
 	list_for_each_entry_safe(node, n, &regulator_map_list, list) {
 		if (rdev == node->regulator) {
 			list_del(&node->list);
+			kfree(node->dev_name);
 			kfree(node);
 			return;
 		}
@@ -1019,17 +1043,25 @@ struct regulator *regulator_get(struct device *dev, const char *id)
 	struct regulator_dev *rdev;
 	struct regulator_map *map;
 	struct regulator *regulator = ERR_PTR(-ENODEV);
+	const char *devname = NULL;
 
 	if (id == NULL) {
 		printk(KERN_ERR "regulator: get() with no identifier\n");
 		return regulator;
 	}
 
+	if (dev)
+		devname = dev_name(dev);
+
 	mutex_lock(&regulator_list_mutex);
 
 	list_for_each_entry(map, &regulator_map_list, list) {
-		if (dev == map->dev &&
-		    strcmp(map->supply, id) == 0) {
+		/* If the mapping has a device set up it must match */
+		if (map->dev_name &&
+		    (!devname || strcmp(map->dev_name, devname)))
+			continue;
+
+		if (strcmp(map->supply, id) == 0) {
 			rdev = map->regulator;
 			goto found;
 		}
@@ -2067,11 +2099,13 @@ struct regulator_dev *regulator_register(struct regulator_desc *regulator_desc,
 	for (i = 0; i < init_data->num_consumer_supplies; i++) {
 		ret = set_consumer_device_supply(rdev,
 			init_data->consumer_supplies[i].dev,
+			init_data->consumer_supplies[i].dev_name,
 			init_data->consumer_supplies[i].supply);
 		if (ret < 0) {
 			for (--i; i >= 0; i--)
 				unset_consumer_device_supply(rdev,
-					init_data->consumer_supplies[i].dev);
+				    init_data->consumer_supplies[i].dev_name,
+				    init_data->consumer_supplies[i].dev);
 			goto scrub;
 		}
 	}
diff --git a/include/linux/regulator/machine.h b/include/linux/regulator/machine.h
index bac64fa..9328090 100644
--- a/include/linux/regulator/machine.h
+++ b/include/linux/regulator/machine.h
@@ -126,13 +126,18 @@ struct regulation_constraints {
 /**
  * struct regulator_consumer_supply - supply -> device mapping
  *
- * This maps a supply name to a device.
+ * This maps a supply name to a device.  Only one of dev or dev_name
+ * can be specified.  Use of dev_name allows support for buses which
+ * make struct device available late such as I2C and is the preferred
+ * form.
  *
  * @dev: Device structure for the consumer.
+ * @dev_name: Result of dev_name() for the consumer.
  * @supply: Name for the supply.
  */
 struct regulator_consumer_supply {
 	struct device *dev;	/* consumer */
+	const char *dev_name;   /* dev_name() for consumer */
 	const char *supply;	/* consumer supply - e.g. "vcc" */
 };
 
-- 
1.6.3.1

Re: [PATCH] regulator: Allow consumer supplies to be set up with dev_name()

[Liam Girdwood]

On Wed, 2009-06-17 at 17:56 +0100, Mark Brown wrote:
> Follow the approach suggested by Russell King and implemented by him in
> the clkdev API and allow consumer device supply mapings to be set up
> using the dev_name() for the consumer instead of the struct device.
> In order to avoid making existing machines instabuggy and creating merge
> issues the use of struct device is still supported for the time being.
> 

We should probably mark this as deprecated in the docs.

> This resolves problems working with buses such as I2C which make the
> struct device available late providing that the final device name is
> known, which is the case for most embedded systems with fixed setups.
> 
> Consumers must still use the struct device when calling regulator_get().
> 
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> ---
> For the avoidance of doubt this is most definately 2.6.32 material, not
> 2.6.31.
> 
>  drivers/regulator/core.c          |   62 ++++++++++++++++++++++++++++--------
>  include/linux/regulator/machine.h |    7 +++-
>  2 files changed, 54 insertions(+), 15 deletions(-)
> 

Applied.

Thanks

Liam

Re: [PATCH] regulator: Allow consumer supplies to be set up with dev_name()

[Mark Brown]

On Thu, Jun 18, 2009 at 11:54:03AM +0100, Liam Girdwood wrote:
> On Wed, 2009-06-17 at 17:56 +0100, Mark Brown wrote:

> > In order to avoid making existing machines instabuggy and creating merge
> > issues the use of struct device is still supported for the time being.

> We should probably mark this as deprecated in the docs.

Yeah, probably.  I was swithering a bit since while obviously it doesn't
work so well for things like I2C when you do have the struct device
there it's pretty convenient to use it, but having both ways to specify
it makes things more confusing than they need to be.

Re: [PATCH] regulator: Allow consumer supplies to be set up with dev_name()

[Jonathan Cameron]

Mark Brown wrote:
> Follow the approach suggested by Russell King and implemented by him in
> the clkdev API and allow consumer device supply mapings to be set up
> using the dev_name() for the consumer instead of the struct device.
> In order to avoid making existing machines instabuggy and creating merge
> issues the use of struct device is still supported for the time being.
>
> This resolves problems working with buses such as I2C which make the
> struct device available late providing that the final device name is
> known, which is the case for most embedded systems with fixed setups.
>
> Consumers must still use the struct device when calling regulator_get().
>
>   
Hi,

This patch is an excellent solution to the problem.

I just wonder if we need to suggest subsystem maintainers make sure that
their
device names are suitably unique and identifiable?  For example, i2c device
tend to have names like 0-0032.  Perhaps we need something to identify
that they
are indeed i2c devices?

For that matter, is there anything other than blind luck preventing two
regulator
consumers having the same dev_name?

---
Jonathan Cameron

Re: [PATCH] regulator: Allow consumer supplies to be set up with dev_name()

[Mark Brown]

On Thu, Jun 25, 2009 at 01:11:46PM +0000, Jonathan Cameron wrote:

> I just wonder if we need to suggest subsystem maintainers make sure that
> their
> device names are suitably unique and identifiable?  For example, i2c device
> tend to have names like 0-0032.  Perhaps we need something to identify
> that they
> are indeed i2c devices?

Either that or if it becomes a problem we add the ability to match on
bus type as well - since bus type already exists it'd probably be better
to go down that road rather than cause userspace-visible changes.  I've
not looked in sufficient detail to confirm that this would cover
everything but I believe it should.

> For that matter, is there anything other than blind luck preventing two
> regulator
> consumers having the same dev_name?

Due to sysfs requirements I think we should be safe with the addition of
bus type matching but like I say I may be missing something.