2009-07-21 15:00:32

by Mark Brown

Subject: [PATCH 1/6] regulator: Add regulator_get_exclusive() API

Some consumers require complete control of the regulator and can't
tolerate sharing it with other consumers, most commonly because they need
to have the regulator actually disabled so can't have other consumers
forcing it on. This new regulator_get_exclusive() API call allows these
consumers to explicitly request this, documenting the assumptions that
they are making.

In order to simplify coding of such consumers the use count for regulators
they request is forced to match the enabled state of the regulator when
it is requested. This is not possible for consumers which can share
regulators due to the need to keep track of the ownership of use counts.

A new API call is used rather than an additional argument to the existing
regulator_get() in order to avoid merge headaches with driver code in
other trees.

Signed-off-by: Mark Brown <[email protected]>
---
drivers/regulator/core.c | 88 ++++++++++++++++++++++++++++++------
include/linux/regulator/consumer.h | 2 +
include/linux/regulator/driver.h | 2 +
3 files changed, 78 insertions(+), 14 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index eed62ad..8445524 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1025,25 +1025,15 @@ overflow_err:
return NULL;
}

-/**
- * regulator_get - lookup and obtain a reference to a regulator.
- * @dev: device for regulator "consumer"
- * @id: Supply name or regulator ID.
- *
- * Returns a struct regulator corresponding to the regulator producer,
- * or IS_ERR() condition containing errno.
- *
- * Use of supply names configured via regulator_set_device_supply() is
- * strongly encouraged. It is recommended that the supply name used
- * should match the name used for the supply and/or the relevant
- * device pins in the datasheet.
- */
-struct regulator *regulator_get(struct device *dev, const char *id)
+/* Internal regulator request function */
+static struct regulator *_regulator_get(struct device *dev, const char *id,
+ int exclusive)
{
struct regulator_dev *rdev;
struct regulator_map *map;
struct regulator *regulator = ERR_PTR(-ENODEV);
const char *devname = NULL;
+ int ret;

if (id == NULL) {
printk(KERN_ERR "regulator: get() with no identifier\n");
@@ -1070,6 +1060,16 @@ struct regulator *regulator_get(struct device *dev, const char *id)
return regulator;

found:
+ if (rdev->exclusive) {
+ regulator = ERR_PTR(-EPERM);
+ goto out;
+ }
+
+ if (exclusive && rdev->open_count) {
+ regulator = ERR_PTR(-EBUSY);
+ goto out;
+ }
+
if (!try_module_get(rdev->owner))
goto out;

@@ -1079,13 +1079,70 @@ found:
module_put(rdev->owner);
}

+ rdev->open_count++;
+ if (exclusive) {
+ rdev->exclusive = 1;
+
+ ret = _regulator_is_enabled(rdev);
+ if (ret > 0)
+ rdev->use_count = 1;
+ else
+ rdev->use_count = 0;
+ }
+
out:
mutex_unlock(&regulator_list_mutex);
+
return regulator;
}
+
+/**
+ * regulator_get - lookup and obtain a reference to a regulator.
+ * @dev: device for regulator "consumer"
+ * @id: Supply name or regulator ID.
+ *
+ * Returns a struct regulator corresponding to the regulator producer,
+ * or IS_ERR() condition containing errno.
+ *
+ * Use of supply names configured via regulator_set_device_supply() is
+ * strongly encouraged. It is recommended that the supply name used
+ * should match the name used for the supply and/or the relevant
+ * device pins in the datasheet.
+ */
+struct regulator *regulator_get(struct device *dev, const char *id)
+{
+ return _regulator_get(dev, id, 0);
+}
EXPORT_SYMBOL_GPL(regulator_get);

/**
+ * regulator_get_exclusive - obtain exclusive access to a regulator.
+ * @dev: device for regulator "consumer"
+ * @id: Supply name or regulator ID.
+ *
+ * Returns a struct regulator corresponding to the regulator producer,
+ * or IS_ERR() condition containing errno. Other consumers will be
+ * unable to obtain this reference is held and the use count for the
+ * regulator will be initialised to reflect the current state of the
+ * regulator.
+ *
+ * This is intended for use by consumers which cannot tolerate shared
+ * use of the regulator such as those which need to force the
+ * regulator off for correct operation of the hardware they are
+ * controlling.
+ *
+ * Use of supply names configured via regulator_set_device_supply() is
+ * strongly encouraged. It is recommended that the supply name used
+ * should match the name used for the supply and/or the relevant
+ * device pins in the datasheet.
+ */
+struct regulator *regulator_get_exclusive(struct device *dev, const char *id)
+{
+ return _regulator_get(dev, id, 1);
+}
+EXPORT_SYMBOL_GPL(regulator_get_exclusive);
+
+/**
* regulator_put - "free" the regulator source
* @regulator: regulator source
*
@@ -1113,6 +1170,9 @@ void regulator_put(struct regulator *regulator)
list_del(&regulator->list);
kfree(regulator);

+ rdev->open_count--;
+ rdev->exclusive = 0;
+
module_put(rdev->owner);
mutex_unlock(&regulator_list_mutex);
}
diff --git a/include/linux/regulator/consumer.h b/include/linux/regulator/consumer.h
index 277f4b9..976b57b 100644
--- a/include/linux/regulator/consumer.h
+++ b/include/linux/regulator/consumer.h
@@ -125,6 +125,8 @@ struct regulator_bulk_data {
/* regulator get and put */
struct regulator *__must_check regulator_get(struct device *dev,
const char *id);
+struct regulator *__must_check regulator_get_exclusive(struct device *dev,
+ const char *id);
void regulator_put(struct regulator *regulator);

/* regulator output control and status */
diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
index 225f733..60efbd1 100644
--- a/include/linux/regulator/driver.h
+++ b/include/linux/regulator/driver.h
@@ -162,6 +162,8 @@ struct regulator_desc {
struct regulator_dev {
struct regulator_desc *desc;
int use_count;
+ int open_count;
+ int exclusive;

/* lists we belong to */
struct list_head list; /* list of all regulators */
--
1.6.3.3

2009-07-21 15:00:42

by Mark Brown

Subject: [PATCH 4/6] regulator: Fix support for deviceless supply mappings

The patch to add support for looking up consumers by device name
had the side effect of causing us to require a device which is
at best premature since at least cpufreq still operates outside
the device model. Remove that requirement.

Reported-by: Haojian Zhuang <[email protected]>
Signed-off-by: Mark Brown <[email protected]>
---
drivers/regulator/core.c | 18 +++++++++++++-----
1 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index e5ccc9f..cba10ec 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -872,6 +872,7 @@ static int set_consumer_device_supply(struct regulator_dev *rdev,
const char *supply)
{
struct regulator_map *node;
+ int has_dev;

if (consumer_dev && consumer_dev_name)
return -EINVAL;
@@ -882,6 +883,11 @@ static int set_consumer_device_supply(struct regulator_dev *rdev,
if (supply == NULL)
return -EINVAL;

+ if (consumer_dev_name != NULL)
+ has_dev = 1;
+ else
+ has_dev = 0;
+
list_for_each_entry(node, &regulator_map_list, list) {
if (consumer_dev_name != node->dev_name)
continue;
@@ -896,17 +902,19 @@ static int set_consumer_device_supply(struct regulator_dev *rdev,
return -EBUSY;
}

- node = kmalloc(sizeof(struct regulator_map), GFP_KERNEL);
+ node = kzalloc(sizeof(struct regulator_map), GFP_KERNEL);
if (node == NULL)
return -ENOMEM;

node->regulator = rdev;
- node->dev_name = kstrdup(consumer_dev_name, GFP_KERNEL);
node->supply = supply;

- if (node->dev_name == NULL) {
- kfree(node);
- return -ENOMEM;
+ if (has_dev) {
+ node->dev_name = kstrdup(consumer_dev_name, GFP_KERNEL);
+ if (node->dev_name == NULL) {
+ kfree(node);
+ return -ENOMEM;
+ }
}

list_add(&node->list, &regulator_map_list);
--
1.6.3.3

2009-07-21 15:00:44

by Mark Brown

Subject: [PATCH 6/6] regulator: Provide mode to status conversion function

This is useful for implementing get_status() in terms of get_mode().

Signed-off-by: Mark Brown <[email protected]>
---
drivers/regulator/core.c | 24 ++++++++++++++++++++++++
include/linux/regulator/driver.h | 2 ++
2 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index cba10ec..13d0f92 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1993,6 +1993,30 @@ int regulator_notifier_call_chain(struct regulator_dev *rdev,
}
EXPORT_SYMBOL_GPL(regulator_notifier_call_chain);

+/**
+ * regulator_mode_to_status - convert a regulator mode into a status
+ *
+ * @mode: Mode to convert
+ *
+ * Convert a regulator mode into a status.
+ */
+int regulator_mode_to_status(unsigned int mode)
+{
+ switch (mode) {
+ case REGULATOR_MODE_FAST:
+ return REGULATOR_STATUS_FAST;
+ case REGULATOR_MODE_NORMAL:
+ return REGULATOR_STATUS_NORMAL;
+ case REGULATOR_MODE_IDLE:
+ return REGULATOR_STATUS_IDLE;
+ case REGULATOR_STATUS_STANDBY:
+ return REGULATOR_STATUS_STANDBY;
+ default:
+ return 0;
+ }
+}
+EXPORT_SYMBOL_GPL(regulator_mode_to_status);
+
/*
* To avoid cluttering sysfs (and memory) with useless state, only
* create attributes that can be meaningfully displayed.
diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
index 60efbd1..73c9cd6 100644
--- a/include/linux/regulator/driver.h
+++ b/include/linux/regulator/driver.h
@@ -195,6 +195,8 @@ void *rdev_get_drvdata(struct regulator_dev *rdev);
struct device *rdev_get_dev(struct regulator_dev *rdev);
int rdev_get_id(struct regulator_dev *rdev);

+int regulator_mode_to_status(unsigned int);
+
void *regulator_get_init_drvdata(struct regulator_init_data *reg_init_data);

#endif
--
1.6.3.3

2009-07-21 15:00:48

by Mark Brown

Subject: [PATCH 2/6] regulator: Add regulator voltage range check API

Simplify checking of support for voltage ranges by providing an API which
wraps the existing count and list operations.

Signed-off-by: Mark Brown <[email protected]>
---
drivers/regulator/core.c | 29 +++++++++++++++++++++++++++++
include/linux/regulator/consumer.h | 2 ++
2 files changed, 31 insertions(+), 0 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 8445524..33a8c3c 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1442,6 +1442,35 @@ int regulator_list_voltage(struct regulator *regulator, unsigned selector)
EXPORT_SYMBOL_GPL(regulator_list_voltage);

/**
+ * regulator_is_supported_voltage - check if a voltage range can be supported
+ *
+ * @regulator: Regulator to check.
+ * @min_uV: Minimum required voltage in uV.
+ * @max_uV: Maximum required voltage in uV.
+ *
+ * Returns a boolean or a negative error code.
+ */
+int regulator_is_supported_voltage(struct regulator *regulator,
+ int min_uV, int max_uV)
+{
+ int i, voltages, ret;
+
+ ret = regulator_count_voltages(regulator);
+ if (ret < 0)
+ return ret;
+ voltages = ret;
+
+ for (i = 0; i < voltages; i++) {
+ ret = regulator_list_voltage(regulator, i);
+
+ if (ret >= min_uV && ret <= max_uV)
+ return 1;
+ }
+
+ return 0;
+}
+
+/**
* regulator_set_voltage - set regulator output voltage
* @regulator: regulator source
* @min_uV: Minimum required voltage in uV
diff --git a/include/linux/regulator/consumer.h b/include/linux/regulator/consumer.h
index 976b57b..490c5b3 100644
--- a/include/linux/regulator/consumer.h
+++ b/include/linux/regulator/consumer.h
@@ -146,6 +146,8 @@ void regulator_bulk_free(int num_consumers,

int regulator_count_voltages(struct regulator *regulator);
int regulator_list_voltage(struct regulator *regulator, unsigned selector);
+int regulator_is_supported_voltage(struct regulator *regulator,
+ int min_uV, int max_uV);
int regulator_set_voltage(struct regulator *regulator, int min_uV, int max_uV);
int regulator_get_voltage(struct regulator *regulator);
int regulator_set_current_limit(struct regulator *regulator,
--
1.6.3.3

2009-07-21 15:00:58

by Mark Brown

Subject: [PATCH 5/6] regulator: Improve virtual consumer probe error handling

Report errors to the user and try harder to clean up if we're not
able to probe.

Signed-off-by: Mark Brown <[email protected]>
---
drivers/regulator/virtual.c | 12 ++++++++----
1 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/regulator/virtual.c b/drivers/regulator/virtual.c
index 1441107..addc032 100644
--- a/drivers/regulator/virtual.c
+++ b/drivers/regulator/virtual.c
@@ -286,8 +286,7 @@ static int regulator_virtual_consumer_probe(struct platform_device *pdev)

drvdata = kzalloc(sizeof(struct virtual_consumer_data), GFP_KERNEL);
if (drvdata == NULL) {
- ret = -ENOMEM;
- goto err;
+ return -ENOMEM;
}

mutex_init(&drvdata->lock);
@@ -302,8 +301,11 @@ static int regulator_virtual_consumer_probe(struct platform_device *pdev)

for (i = 0; i < ARRAY_SIZE(attributes); i++) {
ret = device_create_file(&pdev->dev, attributes[i]);
- if (ret != 0)
- goto err;
+ if (ret != 0) {
+ dev_err(&pdev->dev, "Failed to create attr %d: %d\n",
+ i, ret);
+ goto err_regulator;
+ }
}

drvdata->mode = regulator_get_mode(drvdata->regulator);
@@ -312,6 +314,8 @@ static int regulator_virtual_consumer_probe(struct platform_device *pdev)

return 0;

+err_regulator:
+ regulator_put(drvdata->regulator);
err:
for (i = 0; i < ARRAY_SIZE(attributes); i++)
device_remove_file(&pdev->dev, attributes[i]);
--
1.6.3.3

2009-07-21 15:01:17

by Mark Brown

Subject: [PATCH 3/6] regulator: Warn when unregistering an in-use regulator

We're probably going to start oopsing fairly soon after this happens.

Signed-off-by: Mark Brown <[email protected]>
---
drivers/regulator/core.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 33a8c3c..e5ccc9f 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2229,6 +2229,7 @@ void regulator_unregister(struct regulator_dev *rdev)
return;

mutex_lock(&regulator_list_mutex);
+ WARN_ON(rdev->open_count)
unset_regulator_supplies(rdev);
list_del(&rdev->list);
if (rdev->supply)
--
1.6.3.3

2009-07-21 20:50:10

by Daniel Ribeiro

Subject: Re: [PATCH 1/6] regulator: Add regulator_get_exclusive() API

Em Ter, 2009-07-21 às 16:00 +0100, Mark Brown escreveu:
> Some consumers require complete control of the regulator and can't
> tolerate sharing it with other consumers, most commonly because they need
> to have the regulator actually disabled so can't have other consumers
> forcing it on. This new regulator_get_exclusive() API call allows these
> consumers to explicitly request this, documenting the assumptions that
> they are making.
>
> In order to simplify coding of such consumers the use count for regulators
> they request is forced to match the enabled state of the regulator when
> it is requested. This is not possible for consumers which can share
> regulators due to the need to keep track of the ownership of use counts.
>
> A new API call is used rather than an additional argument to the existing
> regulator_get() in order to avoid merge headaches with driver code in
> other trees.
>
> Signed-off-by: Mark Brown <[email protected]>

Thanks for finally fixing the mmc/regulator issue. ;)

Can you update pxamci.c and twl4030-mmc.c to make use of this new API?

--
Daniel Ribeiro

Attachments:

2009-07-22 02:03:55

by Eric Miao

Subject: Re: [PATCH 1/6] regulator: Add regulator_get_exclusive() API

Daniel Ribeiro wrote:
> Em Ter, 2009-07-21 às 16:00 +0100, Mark Brown escreveu:
>> Some consumers require complete control of the regulator and can't
>> tolerate sharing it with other consumers, most commonly because they need
>> to have the regulator actually disabled so can't have other consumers
>> forcing it on. This new regulator_get_exclusive() API call allows these
>> consumers to explicitly request this, documenting the assumptions that
>> they are making.
>>
>> In order to simplify coding of such consumers the use count for regulators
>> they request is forced to match the enabled state of the regulator when
>> it is requested. This is not possible for consumers which can share
>> regulators due to the need to keep track of the ownership of use counts.
>>
>> A new API call is used rather than an additional argument to the existing
>> regulator_get() in order to avoid merge headaches with driver code in
>> other trees.
>>
>> Signed-off-by: Mark Brown <[email protected]>
>
> Thanks for finally fixing the mmc/regulator issue. ;)
>
> Can you update pxamci.c and twl4030-mmc.c to make use of this new API?
>

I think that deserves another two patches for making use of this
new API.

2009-07-22 09:58:07

by Mark Brown

Subject: Re: [PATCH 1/6] regulator: Add regulator_get_exclusive() API

On Wed, Jul 22, 2009 at 09:58:09AM +0800, Eric Miao wrote:
> Daniel Ribeiro wrote:

> > Can you update pxamci.c and twl4030-mmc.c to make use of this new API?

> I think that deserves another two patches for making use of this
> new API.

Due to my workload, the cross tree merge issues and my lack of any test
hardware I wasn't in a hurry to do that myself - probably not until
after the merge window. It'd probably be as easy for people with
relevant systems to do the updates.

2009-07-22 20:14:59

by Liam Girdwood

Subject: Re: [PATCH 3/6] regulator: Warn when unregistering an in-use regulator

On Tue, 2009-07-21 at 16:00 +0100, Mark Brown wrote:
> We're probably going to start oopsing fairly soon after this happens.
>
> Signed-off-by: Mark Brown <[email protected]>
> ---
> drivers/regulator/core.c | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
> index 33a8c3c..e5ccc9f 100644
> --- a/drivers/regulator/core.c
> +++ b/drivers/regulator/core.c
> @@ -2229,6 +2229,7 @@ void regulator_unregister(struct regulator_dev *rdev)
> return;
>
> mutex_lock(&regulator_list_mutex);
> + WARN_ON(rdev->open_count)

Missing ; Fixed.

> unset_regulator_supplies(rdev);
> list_del(&rdev->list);
> if (rdev->supply)

Applied.

Thanks

Liam

2009-07-22 20:41:31

by Liam Girdwood

Subject: Re: [PATCH 6/6] regulator: Provide mode to status conversion function

On Tue, 2009-07-21 at 16:00 +0100, Mark Brown wrote:
> This is useful for implementing get_status() in terms of get_mode().
>
> Signed-off-by: Mark Brown <[email protected]>
> ---
> drivers/regulator/core.c | 24 ++++++++++++++++++++++++
> include/linux/regulator/driver.h | 2 ++
> 2 files changed, 26 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
> index cba10ec..13d0f92 100644
> --- a/drivers/regulator/core.c
> +++ b/drivers/regulator/core.c
> @@ -1993,6 +1993,30 @@ int regulator_notifier_call_chain(struct regulator_dev *rdev,
> }
> EXPORT_SYMBOL_GPL(regulator_notifier_call_chain);
>
> +/**
> + * regulator_mode_to_status - convert a regulator mode into a status
> + *
> + * @mode: Mode to convert
> + *
> + * Convert a regulator mode into a status.
> + */
> +int regulator_mode_to_status(unsigned int mode)
> +{
> + switch (mode) {
> + case REGULATOR_MODE_FAST:
> + return REGULATOR_STATUS_FAST;
> + case REGULATOR_MODE_NORMAL:
> + return REGULATOR_STATUS_NORMAL;
> + case REGULATOR_MODE_IDLE:
> + return REGULATOR_STATUS_IDLE;
> + case REGULATOR_STATUS_STANDBY:
> + return REGULATOR_STATUS_STANDBY;
> + default:
> + return 0;
> + }
> +}
> +EXPORT_SYMBOL_GPL(regulator_mode_to_status);
> +
> /*
> * To avoid cluttering sysfs (and memory) with useless state, only
> * create attributes that can be meaningfully displayed.
> diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
> index 60efbd1..73c9cd6 100644
> --- a/include/linux/regulator/driver.h
> +++ b/include/linux/regulator/driver.h
> @@ -195,6 +195,8 @@ void *rdev_get_drvdata(struct regulator_dev *rdev);
> struct device *rdev_get_dev(struct regulator_dev *rdev);
> int rdev_get_id(struct regulator_dev *rdev);
>
> +int regulator_mode_to_status(unsigned int);
> +
> void *regulator_get_init_drvdata(struct regulator_init_data *reg_init_data);
>
> #endif

I've applied all patches, but I'm wondering whether it would be better
to eventually merge MODE and STATUS. i.e. have REGULATOR_FAST instead of
REGULATOR_STATUS_FAST and REGULATOR_MODE_FAST.

Thanks

Liam

2009-07-22 20:47:32

by Liam Girdwood

Subject: Re: [PATCH 1/6] regulator: Add regulator_get_exclusive() API

On Wed, 2009-07-22 at 10:58 +0100, Mark Brown wrote:
> On Wed, Jul 22, 2009 at 09:58:09AM +0800, Eric Miao wrote:
> > Daniel Ribeiro wrote:
>
> > > Can you update pxamci.c and twl4030-mmc.c to make use of this new API?
>
> > I think that deserves another two patches for making use of this
> > new API.
>
> Due to my workload, the cross tree merge issues and my lack of any test
> hardware I wasn't in a hurry to do that myself - probably not until
> after the merge window. It'd probably be as easy for people with
> relevant systems to do the updates.

I agree, my workload wont allow me to test this on my pxa atm :-/

Eric, Daniel, any chance you could oblige with a tested patch for your
respective systems ?

Thanks

Liam

2009-07-23 08:39:34

by Mark Brown

Subject: Re: [PATCH 3/6] regulator: Warn when unregistering an in-use regulator

On Wed, Jul 22, 2009 at 09:14:54PM +0100, Liam Girdwood wrote:
> On Tue, 2009-07-21 at 16:00 +0100, Mark Brown wrote:

> > mutex_lock(&regulator_list_mutex);
> > + WARN_ON(rdev->open_count)

> Missing ; Fixed.

Thanks. The scary thing is that it compiles for me.

>
> > unset_regulator_supplies(rdev);
> > list_del(&rdev->list);
> > if (rdev->supply)
>
> Applied.
>
> Thanks
>
> Liam
>

2009-07-23 08:42:23

by Mark Brown

Subject: Re: [PATCH 6/6] regulator: Provide mode to status conversion function

On Wed, Jul 22, 2009 at 09:41:28PM +0100, Liam Girdwood wrote:
> On Tue, 2009-07-21 at 16:00 +0100, Mark Brown wrote:

> > +int regulator_mode_to_status(unsigned int);
> > +
> > void *regulator_get_init_drvdata(struct regulator_init_data *reg_init_data);

> I've applied all patches, but I'm wondering whether it would be better
> to eventually merge MODE and STATUS. i.e. have REGULATOR_FAST instead of
> REGULATOR_STATUS_FAST and REGULATOR_MODE_FAST.

I agree, at least lining up the values so you can just assign between
the two would be nice.