[PATCH] regulator: Use regulator_lock_supply() for get_voltage() too

[PATCH] regulator: Use regulator_lock_supply() for get_voltage() too

[Mark Brown]

Since we need to read voltages of parents as part of setting supply
voltages we need to be able to do get_voltage() internally without
taking locks so reorganize the locking to take locks on the full tree on
entry rather than as we recurse when called externally.

Reported-by: John Stultz <john.stultz@linaro.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/regulator/core.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index ff9cc3a6895f..97749c3330a4 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -3110,7 +3110,7 @@ static int _regulator_get_voltage(struct regulator_dev *rdev)
 	} else if (rdev->desc->fixed_uV && (rdev->desc->n_voltages == 1)) {
 		ret = rdev->desc->fixed_uV;
 	} else if (rdev->supply) {
-		ret = regulator_get_voltage(rdev->supply);
+		ret = _regulator_get_voltage(rdev->supply);
 	} else {
 		return -EINVAL;
 	}
@@ -3133,11 +3133,11 @@ int regulator_get_voltage(struct regulator *regulator)
 {
 	int ret;
 
-	mutex_lock(&regulator->rdev->mutex);
+	regulator_lock_supply(regulator->rdev);
 
 	ret = _regulator_get_voltage(regulator->rdev);
 
-	mutex_unlock(&regulator->rdev->mutex);
+	regulator_unlock_supply(&regulator->rdev->mutex);
 
 	return ret;
 }
-- 
2.6.2

Re: [PATCH] regulator: Use regulator_lock_supply() for get_voltage() too

[kbuild test robot]

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

Hi Mark,

[auto build test WARNING on regulator/for-next -- if it's inappropriate base, please suggest rules for selecting the more suitable base]

url:    https://github.com/0day-ci/linux/commits/Mark-Brown/regulator-Use-regulator_lock_supply-for-get_voltage-too/20151103-144509
config: x86_64-randconfig-x013-11022153 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   drivers/regulator/core.c: In function '_regulator_get_voltage':
>> drivers/regulator/core.c:3113:32: warning: passing argument 1 of '_regulator_get_voltage' from incompatible pointer type [-Wincompatible-pointer-types]
      ret = _regulator_get_voltage(rdev->supply);
                                   ^
   drivers/regulator/core.c:3097:12: note: expected 'struct regulator_dev *' but argument is of type 'struct regulator *'
    static int _regulator_get_voltage(struct regulator_dev *rdev)
               ^
   drivers/regulator/core.c: In function 'regulator_get_voltage':
>> drivers/regulator/core.c:3140:26: warning: passing argument 1 of 'regulator_unlock_supply' from incompatible pointer type [-Wincompatible-pointer-types]
     regulator_unlock_supply(&regulator->rdev->mutex);
                             ^
   drivers/regulator/core.c:159:13: note: expected 'struct regulator_dev *' but argument is of type 'struct mutex *'
    static void regulator_unlock_supply(struct regulator_dev *rdev)
                ^

vim +/_regulator_get_voltage +3113 drivers/regulator/core.c

  3107			ret = rdev->desc->ops->get_voltage(rdev);
  3108		} else if (rdev->desc->ops->list_voltage) {
  3109			ret = rdev->desc->ops->list_voltage(rdev, 0);
  3110		} else if (rdev->desc->fixed_uV && (rdev->desc->n_voltages == 1)) {
  3111			ret = rdev->desc->fixed_uV;
  3112		} else if (rdev->supply) {
> 3113			ret = _regulator_get_voltage(rdev->supply);
  3114		} else {
  3115			return -EINVAL;
  3116		}
  3117	
  3118		if (ret < 0)
  3119			return ret;
  3120		return ret - rdev->constraints->uV_offset;
  3121	}
  3122	
  3123	/**
  3124	 * regulator_get_voltage - get regulator output voltage
  3125	 * @regulator: regulator source
  3126	 *
  3127	 * This returns the current regulator voltage in uV.
  3128	 *
  3129	 * NOTE: If the regulator is disabled it will return the voltage value. This
  3130	 * function should not be used to determine regulator state.
  3131	 */
  3132	int regulator_get_voltage(struct regulator *regulator)
  3133	{
  3134		int ret;
  3135	
  3136		regulator_lock_supply(regulator->rdev);
  3137	
  3138		ret = _regulator_get_voltage(regulator->rdev);
  3139	
> 3140		regulator_unlock_supply(&regulator->rdev->mutex);
  3141	
  3142		return ret;
  3143	}

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 26049 bytes --]

Re: [PATCH] regulator: Use regulator_lock_supply() for get_voltage() too

[John Stultz]

On Tue, Nov 3, 2015 at 1:23 AM, Mark Brown <broonie@kernel.org> wrote:
> Since we need to read voltages of parents as part of setting supply
> voltages we need to be able to do get_voltage() internally without
> taking locks so reorganize the locking to take locks on the full tree on
> entry rather than as we recurse when called externally.
>
> Reported-by: John Stultz <john.stultz@linaro.org>
> Signed-off-by: Mark Brown <broonie@kernel.org>

This resolves the issue for me, thanks!
Tested-by: John Stultz <john.stultz@linaro.org>
-john

[PATCH] regulator: Use regulator_lock_supply() for get_voltage() too

[Mark Brown]

Since we need to read voltages of parents as part of setting supply
voltages we need to be able to do get_voltage() internally without
taking locks so reorganize the locking to take locks on the full tree on
entry rather than as we recurse when called externally.

Reported-by: John Stultz <john.stultz@linaro.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/regulator/core.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index ff9cc3a6895f..73b7683355cd 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -3110,7 +3110,7 @@ static int _regulator_get_voltage(struct regulator_dev *rdev)
 	} else if (rdev->desc->fixed_uV && (rdev->desc->n_voltages == 1)) {
 		ret = rdev->desc->fixed_uV;
 	} else if (rdev->supply) {
-		ret = regulator_get_voltage(rdev->supply);
+		ret = _regulator_get_voltage(rdev->supply->rdev);
 	} else {
 		return -EINVAL;
 	}
@@ -3133,11 +3133,11 @@ int regulator_get_voltage(struct regulator *regulator)
 {
 	int ret;
 
-	mutex_lock(&regulator->rdev->mutex);
+	regulator_lock_supply(regulator->rdev);
 
 	ret = _regulator_get_voltage(regulator->rdev);
 
-	mutex_unlock(&regulator->rdev->mutex);
+	regulator_unlock_supply(regulator->rdev);
 
 	return ret;
 }
-- 
2.6.2