[PATCH V2] regulator: disable supply regulator if it is enabled for boot-on

[PATCH V2] regulator: disable supply regulator if it is enabled for boot-on

[Laxman Dewangan]

If supply regulator is enabled because of boot-on (not always-on)
then disable regulator need to be call if regulator have some
user or full constraint has been enabled.
This will make sure that reference count of supply regulator
is in sync with child regulator's state.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
Reported-by: Rabin Vincent <rabin.vincent@gmail.com>
---
Changes from V1:
Rabin reported that the nested locking trace is getting generated.
This is because the regulator lock mutex and disable himself and then
call for supply disable which again lock the regulator lock.
Probably because both lock is taken from regulator structure in nested
manner, the trace is getting generated.
Rewritten patch to avoid nested locking.

 drivers/regulator/core.c |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 06186ba..e54537f 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -3606,6 +3606,7 @@ static int __init regulator_init_complete(void)
 	 * default behaviour in the future.
 	 */
 	list_for_each_entry(rdev, &regulator_list, list) {
+		bool supply_disable = false;
 		ops = rdev->desc->ops;
 		c = rdev->constraints;
 
@@ -3614,8 +3615,11 @@ static int __init regulator_init_complete(void)
 
 		mutex_lock(&rdev->mutex);
 
-		if (rdev->use_count)
+		if (rdev->use_count) {
+			if (rdev->supply && c->boot_on)
+				supply_disable = true;
 			goto unlock;
+		}
 
 		/* If we can't read the status assume it's on. */
 		if (ops->is_enabled)
@@ -3634,6 +3638,8 @@ static int __init regulator_init_complete(void)
 			if (ret != 0) {
 				rdev_err(rdev, "couldn't disable: %d\n", ret);
 			}
+			if (rdev->supply)
+				supply_disable = true;
 		} else {
 			/* The intention is that in future we will
 			 * assume that full constraints are provided
@@ -3645,6 +3651,8 @@ static int __init regulator_init_complete(void)
 
 unlock:
 		mutex_unlock(&rdev->mutex);
+		if (supply_disable)
+			regulator_disable(rdev->supply);
 	}
 
 	mutex_unlock(&regulator_list_mutex);
-- 
1.7.1.1

Re: [PATCH V2] regulator: disable supply regulator if it is enabled for boot-on

[Rabin Vincent]

2012/8/29 Laxman Dewangan <ldewangan@nvidia.com>:
> @@ -3614,8 +3615,11 @@ static int __init regulator_init_complete(void)
>
>                 mutex_lock(&rdev->mutex);
>
> -               if (rdev->use_count)
> +               if (rdev->use_count) {
> +                       if (rdev->supply && c->boot_on)
> +                               supply_disable = true;
>                         goto unlock;
> +               }
>
>                 /* If we can't read the status assume it's on. */
>                 if (ops->is_enabled)
> @@ -3634,6 +3638,8 @@ static int __init regulator_init_complete(void)
>                         if (ret != 0) {
>                                 rdev_err(rdev, "couldn't disable: %d\n", ret);
>                         }
> +                       if (rdev->supply)
> +                               supply_disable = true;
>                 } else {
>                         /* The intention is that in future we will
>                          * assume that full constraints are provided

This does not handle the case where a regulator is not set boot_on but
is considered on (for example, because of the lack of an is_enabled
callback), and is later actually enabled by a consumer before
regulator_init_complete().  In this case, the supply's use count will
still be one more than it should be, because the "&& c->boot_on"
condition above will fail.

To fix this, you should probably note which regulators' supplies you
enable in regulator_register() and use that information in the above two
checks here in regulator_init_complete().

Re: [PATCH V2] regulator: disable supply regulator if it is enabled for boot-on

[Mark Brown]

On Thu, Aug 30, 2012 at 08:24:15PM +0200, Rabin Vincent wrote:

> This does not handle the case where a regulator is not set boot_on but
> is considered on (for example, because of the lack of an is_enabled
> callback), and is later actually enabled by a consumer before
> regulator_init_complete().  In this case, the supply's use count will
> still be one more than it should be, because the "&& c->boot_on"
> condition above will fail.

> To fix this, you should probably note which regulators' supplies you
> enable in regulator_register() and use that information in the above two
> checks here in regulator_init_complete().

Yeah, I didn't read this properly yet but this was my main concern -
this should all just fall out naturally through the normal disable
process.  Doing special case bodges feels really fragile and error
prone.