[PATCH] regulator: extend the fixed voltage regulator to accept voltage

[PATCH] regulator: extend the fixed voltage regulator to accept voltage

[Guennadi Liakhovetski]

Trivially extend the regulator_register_fixed() helper function to be even
more useful by letting it accept a voltage parameter. The original function
is provided as a wrapper macro.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/regulator/fixed-helper.c |   21 ++++++++++++++++-----
 include/linux/regulator/fixed.h  |   10 ++++++----
 2 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/drivers/regulator/fixed-helper.c b/drivers/regulator/fixed-helper.c
index cacd33c..d979ff0 100644
--- a/drivers/regulator/fixed-helper.c
+++ b/drivers/regulator/fixed-helper.c
@@ -1,12 +1,17 @@
 #include <linux/slab.h>
+#include <linux/string.h>
 #include <linux/platform_device.h>
 #include <linux/regulator/machine.h>
 #include <linux/regulator/fixed.h>
 
+#define FIXED_BASE "fixed-dummy"
+#define FIXED_NAME FIXED_BASE "-00"
+
 struct fixed_regulator_data {
 	struct fixed_voltage_config cfg;
 	struct regulator_init_data init_data;
 	struct platform_device pdev;
+	char name[sizeof(FIXED_NAME)];
 };
 
 static void regulator_fixed_release(struct device *dev)
@@ -17,13 +22,14 @@ static void regulator_fixed_release(struct device *dev)
 }
 
 /**
- * regulator_register_fixed - register a no-op fixed regulator
+ * regulator_register_fixed_volt - register a no-op fixed regulator
  * @id: platform device id
  * @supplies: consumers for this regulator
  * @num_supplies: number of consumers
+ * @uv: voltage in microvolts
  */
-struct platform_device *regulator_register_fixed(int id,
-		struct regulator_consumer_supply *supplies, int num_supplies)
+struct platform_device *regulator_register_fixed_volt(int id,
+	struct regulator_consumer_supply *supplies, int num_supplies, int uv)
 {
 	struct fixed_regulator_data *data;
 
@@ -31,8 +37,13 @@ struct platform_device *regulator_register_fixed(int id,
 	if (!data)
 		return NULL;
 
-	data->cfg.supply_name = "fixed-dummy";
-	data->cfg.microvolts = 0;
+	if (id < 0)
+		strcpy(data->name, FIXED_BASE);
+	else
+		snprintf(data->name, sizeof(FIXED_NAME), FIXED_BASE "-%d", id);
+
+	data->cfg.supply_name = data->name;
+	data->cfg.microvolts = uv;
 	data->cfg.gpio = -EINVAL;
 	data->cfg.enabled_at_boot = 1;
 	data->cfg.init_data = &data->init_data;
diff --git a/include/linux/regulator/fixed.h b/include/linux/regulator/fixed.h
index f83f744..1735f0a 100644
--- a/include/linux/regulator/fixed.h
+++ b/include/linux/regulator/fixed.h
@@ -58,14 +58,16 @@ struct fixed_voltage_config {
 struct regulator_consumer_supply;
 
 #if IS_ENABLED(CONFIG_REGULATOR)
-struct platform_device *regulator_register_fixed(int id,
-		struct regulator_consumer_supply *supplies, int num_supplies);
+struct platform_device *regulator_register_fixed_volt(int id,
+	struct regulator_consumer_supply *supplies, int num_supplies, int uv);
 #else
-static inline struct platform_device *regulator_register_fixed(int id,
-		struct regulator_consumer_supply *supplies, int num_supplies)
+static inline struct platform_device *regulator_register_fixed_volt(int id,
+	struct regulator_consumer_supply *supplies, int num_supplies, int uv)
 {
 	return NULL;
 }
 #endif
 
+#define regulator_register_fixed(id, s, ns) regulator_register_fixed_volt(id, s, ns, 0)
+
 #endif
-- 
1.7.2.5

Re: [PATCH] regulator: extend the fixed voltage regulator to accept voltage

[Mark Brown]

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

On Mon, Jun 18, 2012 at 10:19:42AM +0200, Guennadi Liakhovetski wrote:

> +#define FIXED_BASE "fixed-dummy"
> +#define FIXED_NAME FIXED_BASE "-00"
> +
>  struct fixed_regulator_data {
>  	struct fixed_voltage_config cfg;
>  	struct regulator_init_data init_data;
>  	struct platform_device pdev;
> +	char name[sizeof(FIXED_NAME)];

So, this looks pretty ugly and doesn't seem terribly directly related to
supplying a voltage parameter.  Please split it out into a separate
change.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] regulator: extend the fixed voltage regulator to accept voltage

[Guennadi Liakhovetski]

On Mon, 18 Jun 2012, Mark Brown wrote:

> On Mon, Jun 18, 2012 at 10:19:42AM +0200, Guennadi Liakhovetski wrote:
> 
> > +#define FIXED_BASE "fixed-dummy"
> > +#define FIXED_NAME FIXED_BASE "-00"
> > +
> >  struct fixed_regulator_data {
> >  	struct fixed_voltage_config cfg;
> >  	struct regulator_init_data init_data;
> >  	struct platform_device pdev;
> > +	char name[sizeof(FIXED_NAME)];
> 
> So, this looks pretty ugly and doesn't seem terribly directly related to
> supplying a voltage parameter.  Please split it out into a separate
> change.

Well, I never insist on being the most advanced aesthetics connaisseur, 
but I think these two changes are related. The point is, that if we want 
to support different voltages, boards will have several of these 
regulators, therefore they'll need different names. We could splt this, 
but just the first part - changing the name - would look kinda pointless 
without the second one, don't you think?

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH] regulator: extend the fixed voltage regulator to accept voltage

[Mark Brown]

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

On Mon, Jun 18, 2012 at 11:52:13AM +0200, Guennadi Liakhovetski wrote:

> Well, I never insist on being the most advanced aesthetics connaisseur, 
> but I think these two changes are related. The point is, that if we want 
> to support different voltages, boards will have several of these 
> regulators, therefore they'll need different names. We could splt this, 
> but just the first part - changing the name - would look kinda pointless 
> without the second one, don't you think?

The major point there is I shouldn't be reading the change and going
"hang on, this is talking about names not voltages but the changelog
only mentioned voltages, what's that about then?".  The code looked more
complex than I'd expect too.  I suspect we should be using kstrdup()...

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] regulator: extend the fixed voltage regulator to accept voltage

[Guennadi Liakhovetski]

On Mon, 18 Jun 2012, Mark Brown wrote:

> On Mon, Jun 18, 2012 at 11:52:13AM +0200, Guennadi Liakhovetski wrote:
> 
> > Well, I never insist on being the most advanced aesthetics connaisseur, 
> > but I think these two changes are related. The point is, that if we want 
> > to support different voltages, boards will have several of these 
> > regulators, therefore they'll need different names. We could splt this, 
> > but just the first part - changing the name - would look kinda pointless 
> > without the second one, don't you think?
> 
> The major point there is I shouldn't be reading the change and going
> "hang on, this is talking about names not voltages but the changelog
> only mentioned voltages, what's that about then?".  The code looked more
> complex than I'd expect too.

Ok, I can add an explanation, why the name changes are necessary.

> I suspect we should be using kstrdup()...

I wouldn't. It would add one more kmalloc(), which is avoided with my 
approach, then it would make a memcpy(), which we also don't need, because 
we have to print the id into the string.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH] regulator: extend the fixed voltage regulator to accept voltage

[Mark Brown]

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

On Mon, Jun 18, 2012 at 12:13:58PM +0200, Guennadi Liakhovetski wrote:
> On Mon, 18 Jun 2012, Mark Brown wrote:
> 
> > On Mon, Jun 18, 2012 at 11:52:13AM +0200, Guennadi Liakhovetski wrote:

> Ok, I can add an explanation, why the name changes are necessary.

Please do split it into a separate patch.

> > I suspect we should be using kstrdup()...

> I wouldn't. It would add one more kmalloc(), which is avoided with my 
> approach, then it would make a memcpy(), which we also don't need, because 
> we have to print the id into the string.

This is because you've decided to go and select a particular set of
names for the supplies rather than letting people configure them which
again isn't 100% obvious...

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 1/2 v2] regulator: support multiple dummy fixed regulators

[Guennadi Liakhovetski]

Currently regulator_register_fixed() uses a constant name to register a
fixed dummy regulator. This is sufficient in principle, since there is no
reason to register multiple such regulators. The user can simply supply all
consumers in one array and use it to initialise such a regulator. However,
in some cases it can be convenient to register multiple such regulators.
This is also easy to achieve by appending the device ID to the name. It is
also consistent with the current behaviour of the code, that fixes a name
itself without giving the user a possibility to provide a custom one.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/regulator/fixed-helper.c |   12 +++++++++++-
 1 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/drivers/regulator/fixed-helper.c b/drivers/regulator/fixed-helper.c
index cacd33c..f66d9e8 100644
--- a/drivers/regulator/fixed-helper.c
+++ b/drivers/regulator/fixed-helper.c
@@ -1,12 +1,17 @@
 #include <linux/slab.h>
+#include <linux/string.h>
 #include <linux/platform_device.h>
 #include <linux/regulator/machine.h>
 #include <linux/regulator/fixed.h>
 
+#define FIXED_BASE "fixed-dummy"
+#define FIXED_NAME FIXED_BASE "-00"
+
 struct fixed_regulator_data {
 	struct fixed_voltage_config cfg;
 	struct regulator_init_data init_data;
 	struct platform_device pdev;
+	char name[sizeof(FIXED_NAME)];
 };
 
 static void regulator_fixed_release(struct device *dev)
@@ -31,7 +36,12 @@ struct platform_device *regulator_register_fixed(int id,
 	if (!data)
 		return NULL;
 
-	data->cfg.supply_name = "fixed-dummy";
+	if (id < 0)
+		strcpy(data->name, FIXED_BASE);
+	else
+		snprintf(data->name, sizeof(FIXED_NAME), FIXED_BASE "-%d", id);
+
+	data->cfg.supply_name = data->name;
 	data->cfg.microvolts = 0;
 	data->cfg.gpio = -EINVAL;
 	data->cfg.enabled_at_boot = 1;
-- 
1.7.2.5

[PATCH 2/2 v2] regulator: extend the fixed dummy voltage regulator to accept voltage

[Guennadi Liakhovetski]

Trivially extend the regulator_register_fixed() helper function to be even
more useful by letting it accept a voltage parameter. The original function
is provided as a wrapper macro.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/regulator/fixed-helper.c |    9 +++++----
 include/linux/regulator/fixed.h  |   10 ++++++----
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/regulator/fixed-helper.c b/drivers/regulator/fixed-helper.c
index f66d9e8..d979ff0 100644
--- a/drivers/regulator/fixed-helper.c
+++ b/drivers/regulator/fixed-helper.c
@@ -22,13 +22,14 @@ static void regulator_fixed_release(struct device *dev)
 }
 
 /**
- * regulator_register_fixed - register a no-op fixed regulator
+ * regulator_register_fixed_volt - register a no-op fixed regulator
  * @id: platform device id
  * @supplies: consumers for this regulator
  * @num_supplies: number of consumers
+ * @uv: voltage in microvolts
  */
-struct platform_device *regulator_register_fixed(int id,
-		struct regulator_consumer_supply *supplies, int num_supplies)
+struct platform_device *regulator_register_fixed_volt(int id,
+	struct regulator_consumer_supply *supplies, int num_supplies, int uv)
 {
 	struct fixed_regulator_data *data;
 
@@ -42,7 +43,7 @@ struct platform_device *regulator_register_fixed(int id,
 		snprintf(data->name, sizeof(FIXED_NAME), FIXED_BASE "-%d", id);
 
 	data->cfg.supply_name = data->name;
-	data->cfg.microvolts = 0;
+	data->cfg.microvolts = uv;
 	data->cfg.gpio = -EINVAL;
 	data->cfg.enabled_at_boot = 1;
 	data->cfg.init_data = &data->init_data;
diff --git a/include/linux/regulator/fixed.h b/include/linux/regulator/fixed.h
index f83f744..1735f0a 100644
--- a/include/linux/regulator/fixed.h
+++ b/include/linux/regulator/fixed.h
@@ -58,14 +58,16 @@ struct fixed_voltage_config {
 struct regulator_consumer_supply;
 
 #if IS_ENABLED(CONFIG_REGULATOR)
-struct platform_device *regulator_register_fixed(int id,
-		struct regulator_consumer_supply *supplies, int num_supplies);
+struct platform_device *regulator_register_fixed_volt(int id,
+	struct regulator_consumer_supply *supplies, int num_supplies, int uv);
 #else
-static inline struct platform_device *regulator_register_fixed(int id,
-		struct regulator_consumer_supply *supplies, int num_supplies)
+static inline struct platform_device *regulator_register_fixed_volt(int id,
+	struct regulator_consumer_supply *supplies, int num_supplies, int uv)
 {
 	return NULL;
 }
 #endif
 
+#define regulator_register_fixed(id, s, ns) regulator_register_fixed_volt(id, s, ns, 0)
+
 #endif
-- 
1.7.2.5

Re: [PATCH 1/2 v2] regulator: support multiple dummy fixed regulators

[Mark Brown]

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

On Mon, Jun 18, 2012 at 05:57:24PM +0200, Guennadi Liakhovetski wrote:
> Currently regulator_register_fixed() uses a constant name to register a
> fixed dummy regulator. This is sufficient in principle, since there is no
> reason to register multiple such regulators. The user can simply supply all
> consumers in one array and use it to initialise such a regulator. However,
> in some cases it can be convenient to register multiple such regulators.
> This is also easy to achieve by appending the device ID to the name. It is

None of this seems terribly obvious to me.  Once we're getting into
allowing the user to specify a voltage (and possibly other parameters)
for the regulator it's hopefully going to refer to an actual thing we
can point at on the board rather than a virtual thing we've got to
satisfy software so giving it a useful name seems more useful (like a
name correspoding to the relevant supply on the schematic).  

There's also the fact that there shouldn't be any need for unique
regulator names internally so if it really doesn't matter we should be
able to give everything the same name happily enough.

> also consistent with the current behaviour of the code, that fixes a name
> itself without giving the user a possibility to provide a custom one.

This is done on the basis that it's an entirely virtual regulator which
doesn't have any meaningful mapping into the hardware.  If it's got a
voltage then that's no longer true.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/2 v2] regulator: support multiple dummy fixed regulators

[Guennadi Liakhovetski]

On Mon, 18 Jun 2012, Mark Brown wrote:

> On Mon, Jun 18, 2012 at 05:57:24PM +0200, Guennadi Liakhovetski wrote:
> > Currently regulator_register_fixed() uses a constant name to register a
> > fixed dummy regulator. This is sufficient in principle, since there is no
> > reason to register multiple such regulators. The user can simply supply all
> > consumers in one array and use it to initialise such a regulator. However,
> > in some cases it can be convenient to register multiple such regulators.
> > This is also easy to achieve by appending the device ID to the name. It is
> 
> None of this seems terribly obvious to me.  Once we're getting into
> allowing the user to specify a voltage (and possibly other parameters)
> for the regulator it's hopefully going to refer to an actual thing we
> can point at on the board rather than a virtual thing we've got to
> satisfy software so giving it a useful name seems more useful (like a
> name correspoding to the relevant supply on the schematic).  

Sorry, don't understand. What do you mean by a "virtual" supply? There is 
a device, it is functional, doesn't this mean, that something is supplying 
power to it? And if power is supplied, then it also hopefully has a 
certain voltage :) Why I need to know it - to set an MMC OCR mask.

> There's also the fact that there shouldn't be any need for unique
> regulator names internally so if it really doesn't matter we should be
> able to give everything the same name happily enough.

What about entries under /proc/sys/debug/regulator/? Don't they have to be 
unique?

Thanks
Guennadi


> > also consistent with the current behaviour of the code, that fixes a name
> > itself without giving the user a possibility to provide a custom one.
> 
> This is done on the basis that it's an entirely virtual regulator which
> doesn't have any meaningful mapping into the hardware.  If it's got a
> voltage then that's no longer true.
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH 1/2 v2] regulator: support multiple dummy fixed regulators

[Mark Brown]

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

On Mon, Jun 18, 2012 at 06:30:30PM +0200, Guennadi Liakhovetski wrote:
> On Mon, 18 Jun 2012, Mark Brown wrote:

> > None of this seems terribly obvious to me.  Once we're getting into
> > allowing the user to specify a voltage (and possibly other parameters)
> > for the regulator it's hopefully going to refer to an actual thing we
> > can point at on the board rather than a virtual thing we've got to
> > satisfy software so giving it a useful name seems more useful (like a
> > name correspoding to the relevant supply on the schematic).  

> Sorry, don't understand. What do you mean by a "virtual" supply? There is 
> a device, it is functional, doesn't this mean, that something is supplying 
> power to it? And if power is supplied, then it also hopefully has a 
> certain voltage :) Why I need to know it - to set an MMC OCR mask.

It's for cases where people can't be bothered to specify the supply
properly but want to just put something in there to satisfy the sofware
without providing any useful information.  There will be one or more
physical supplies but the software is being non-committal about them.

> > There's also the fact that there shouldn't be any need for unique
> > regulator names internally so if it really doesn't matter we should be
> > able to give everything the same name happily enough.

> What about entries under /proc/sys/debug/regulator/? Don't they have to be 
> unique?

Meh, yes.  I did try to make them readable.  But then making up the
names in this fashion does rather defeat the point there...

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/2 v2] regulator: support multiple dummy fixed regulators

[Guennadi Liakhovetski]

On Mon, 18 Jun 2012, Mark Brown wrote:

> On Mon, Jun 18, 2012 at 06:30:30PM +0200, Guennadi Liakhovetski wrote:
> > On Mon, 18 Jun 2012, Mark Brown wrote:
> 
> > > None of this seems terribly obvious to me.  Once we're getting into
> > > allowing the user to specify a voltage (and possibly other parameters)
> > > for the regulator it's hopefully going to refer to an actual thing we
> > > can point at on the board rather than a virtual thing we've got to
> > > satisfy software so giving it a useful name seems more useful (like a
> > > name correspoding to the relevant supply on the schematic).  
> 
> > Sorry, don't understand. What do you mean by a "virtual" supply? There is 
> > a device, it is functional, doesn't this mean, that something is supplying 
> > power to it? And if power is supplied, then it also hopefully has a 
> > certain voltage :) Why I need to know it - to set an MMC OCR mask.
> 
> It's for cases where people can't be bothered to specify the supply
> properly but want to just put something in there to satisfy the sofware
> without providing any useful information.  There will be one or more
> physical supplies but the software is being non-committal about them.

Ok, but for that purpose I thought we already have the dummy regulator... 
So, what's the proper way to describe board common voltage rails? 
Obviously you don't want to disable and enable them. Of course, there are 
real physical regulators, that provide that voltage, but what does it 
change, whether we specify the real name of that device or just say - yes, 
we have 3.3V on this board? So, it IS just a fixed regulator with a fixed 
voltage with no way to control it. The only action you can perform with it 
is query the voltage.

> > > There's also the fact that there shouldn't be any need for unique
> > > regulator names internally so if it really doesn't matter we should be
> > > able to give everything the same name happily enough.
> 
> > What about entries under /proc/sys/debug/regulator/? Don't they have to be 
> > unique?
> 
> Meh, yes.  I did try to make them readable.  But then making up the
> names in this fashion does rather defeat the point there...

Ok, what shall we do with this? You want the user to supply a name for 
these fixed voltage always-on regulators? Or you're against these changes 
altogether? Strictly speaking, these patches are not required for boards 
to implement such regulators. It is "just" a helper function. But I think 
the task is pretty common - provide a power supply with a fixed queryable 
voltage. This can easily be implemented as a helper function in regulator 
core, or platforms will have to re-invent it themselves.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH 1/2 v2] regulator: support multiple dummy fixed regulators

[Mark Brown]

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

On Mon, Jun 18, 2012 at 07:13:06PM +0200, Guennadi Liakhovetski wrote:
> On Mon, 18 Jun 2012, Mark Brown wrote:

> > It's for cases where people can't be bothered to specify the supply
> > properly but want to just put something in there to satisfy the sofware
> > without providing any useful information.  There will be one or more
> > physical supplies but the software is being non-committal about them.

> Ok, but for that purpose I thought we already have the dummy regulator... 

The dummy regulator isn't supposed to be used in production whereas this
is - dummy will get substituted in for *any* missing supply as a crutch
to get things working while this is added where needed.  Having dummy on
can break things.

> So, what's the proper way to describe board common voltage rails? 

Fixed voltage regulators should be described with the fixed voltage
regulator driver.

> Obviously you don't want to disable and enable them. Of course, there are 
> real physical regulators, that provide that voltage, but what does it 
> change, whether we specify the real name of that device or just say - yes, 
> we have 3.3V on this board? So, it IS just a fixed regulator with a fixed 
> voltage with no way to control it. The only action you can perform with it 
> is query the voltage.

The name is mainly useful for allowing people to associate the code with
the schematic, by putting the name used in the schematic into the code
we make things much easier when people want to map between the two.

> > Meh, yes.  I did try to make them readable.  But then making up the
> > names in this fashion does rather defeat the point there...

> Ok, what shall we do with this? You want the user to supply a name for 
> these fixed voltage always-on regulators? Or you're against these changes 

Supply a name I think, the use case is totally sensible so it's
definitely totally reasonable to do something.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 1/2 v3] regulator: support multiple dummy fixed regulators

[Guennadi Liakhovetski]

Currently regulator_register_fixed() uses a constant name to register a
fixed dummy regulator. This is sufficient in principle, since there is no
reason to register multiple such regulators. The user can simply supply all
consumers in one array and use it to initialise such a regulator. However,
in some cases it can be convenient to register multiple such regulators.
This is also a prerequisite for the upcoming patch, that will add a voltage
parameter to this function. The original function is provided as a wrapper
macro.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---

v3: Let user provide a name. Thanks to Mark for comments.

 drivers/regulator/fixed-helper.c |   14 +++++++++++---
 include/linux/regulator/fixed.h  |    7 +++++--
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/regulator/fixed-helper.c b/drivers/regulator/fixed-helper.c
index cacd33c..3aa268d 100644
--- a/drivers/regulator/fixed-helper.c
+++ b/drivers/regulator/fixed-helper.c
@@ -1,4 +1,5 @@
 #include <linux/slab.h>
+#include <linux/string.h>
 #include <linux/platform_device.h>
 #include <linux/regulator/machine.h>
 #include <linux/regulator/fixed.h>
@@ -13,16 +14,18 @@ static void regulator_fixed_release(struct device *dev)
 {
 	struct fixed_regulator_data *data = container_of(dev,
 			struct fixed_regulator_data, pdev.dev);
+	kfree(data->cfg.supply_name);
 	kfree(data);
 }
 
 /**
- * regulator_register_fixed - register a no-op fixed regulator
+ * regulator_register_fixed_name - register a no-op fixed regulator
  * @id: platform device id
+ * @name: name to be used for the regulator
  * @supplies: consumers for this regulator
  * @num_supplies: number of consumers
  */
-struct platform_device *regulator_register_fixed(int id,
+struct platform_device *regulator_register_always_on(int id, const char *name,
 		struct regulator_consumer_supply *supplies, int num_supplies)
 {
 	struct fixed_regulator_data *data;
@@ -31,7 +34,12 @@ struct platform_device *regulator_register_fixed(int id,
 	if (!data)
 		return NULL;
 
-	data->cfg.supply_name = "fixed-dummy";
+	data->cfg.supply_name = kstrdup(name, GFP_KERNEL);
+	if (!data->cfg.supply_name) {
+		kfree(data);
+		return NULL;
+	}
+
 	data->cfg.microvolts = 0;
 	data->cfg.gpio = -EINVAL;
 	data->cfg.enabled_at_boot = 1;
diff --git a/include/linux/regulator/fixed.h b/include/linux/regulator/fixed.h
index f83f744..6b9325b 100644
--- a/include/linux/regulator/fixed.h
+++ b/include/linux/regulator/fixed.h
@@ -58,14 +58,17 @@ struct fixed_voltage_config {
 struct regulator_consumer_supply;
 
 #if IS_ENABLED(CONFIG_REGULATOR)
-struct platform_device *regulator_register_fixed(int id,
+struct platform_device *regulator_register_always_on(int id, const char *name,
 		struct regulator_consumer_supply *supplies, int num_supplies);
 #else
-static inline struct platform_device *regulator_register_fixed(int id,
+static inline struct platform_device *regulator_register_always_on(int id, const char *name,
 		struct regulator_consumer_supply *supplies, int num_supplies)
 {
 	return NULL;
 }
 #endif
 
+#define regulator_register_fixed(id, s, ns) regulator_register_always_on(id, \
+						"fixed-dummy", s, ns)
+
 #endif
-- 
1.7.2.5

[PATCH 2/2 v3] regulator: extend the fixed dummy voltage regulator to accept voltage

[Guennadi Liakhovetski]

Trivially extend the regulator_register_always_on() helper function to be
even more useful by adding a voltage parameter to it.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/regulator/fixed-helper.c |    5 +++--
 include/linux/regulator/fixed.h  |    6 +++---
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/regulator/fixed-helper.c b/drivers/regulator/fixed-helper.c
index 3aa268d..f9d0279 100644
--- a/drivers/regulator/fixed-helper.c
+++ b/drivers/regulator/fixed-helper.c
@@ -24,9 +24,10 @@ static void regulator_fixed_release(struct device *dev)
  * @name: name to be used for the regulator
  * @supplies: consumers for this regulator
  * @num_supplies: number of consumers
+ * @uv: voltage in microvolts
  */
 struct platform_device *regulator_register_always_on(int id, const char *name,
-		struct regulator_consumer_supply *supplies, int num_supplies)
+	struct regulator_consumer_supply *supplies, int num_supplies, int uv)
 {
 	struct fixed_regulator_data *data;
 
@@ -40,7 +41,7 @@ struct platform_device *regulator_register_always_on(int id, const char *name,
 		return NULL;
 	}
 
-	data->cfg.microvolts = 0;
+	data->cfg.microvolts = uv;
 	data->cfg.gpio = -EINVAL;
 	data->cfg.enabled_at_boot = 1;
 	data->cfg.init_data = &data->init_data;
diff --git a/include/linux/regulator/fixed.h b/include/linux/regulator/fixed.h
index 6b9325b..680f24e 100644
--- a/include/linux/regulator/fixed.h
+++ b/include/linux/regulator/fixed.h
@@ -59,16 +59,16 @@ struct regulator_consumer_supply;
 
 #if IS_ENABLED(CONFIG_REGULATOR)
 struct platform_device *regulator_register_always_on(int id, const char *name,
-		struct regulator_consumer_supply *supplies, int num_supplies);
+		struct regulator_consumer_supply *supplies, int num_supplies, int uv);
 #else
 static inline struct platform_device *regulator_register_always_on(int id, const char *name,
-		struct regulator_consumer_supply *supplies, int num_supplies)
+		struct regulator_consumer_supply *supplies, int num_supplies, int uv)
 {
 	return NULL;
 }
 #endif
 
 #define regulator_register_fixed(id, s, ns) regulator_register_always_on(id, \
-						"fixed-dummy", s, ns)
+						"fixed-dummy", s, ns, 0)
 
 #endif
-- 
1.7.2.5

Re: [PATCH 1/2 v3] regulator: support multiple dummy fixed regulators

[Mark Brown]

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

On Tue, Jun 19, 2012 at 05:43:56PM +0200, Guennadi Liakhovetski wrote:
> Currently regulator_register_fixed() uses a constant name to register a
> fixed dummy regulator. This is sufficient in principle, since there is no
> reason to register multiple such regulators. The user can simply supply all

Applied both, thanks.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]