linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] regulator: Use regulator_lock_supply() for get_voltage() too
@ 2015-11-03  9:23 Mark Brown
  2015-11-03 15:16 ` Applied "regulator: Use regulator_lock_supply() for get_voltage() too" to the regulator tree Mark Brown
  2015-11-03 23:50 ` [PATCH] regulator: Use regulator_lock_supply() for get_voltage() too John Stultz
  0 siblings, 2 replies; 5+ messages in thread
From: Mark Brown @ 2015-11-03  9:23 UTC (permalink / raw)
  To: Liam Girdwood, John Stultz; +Cc: linux-kernel, 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


^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Applied "regulator: Use regulator_lock_supply() for get_voltage() too" to the regulator tree
  2015-11-03  9:23 [PATCH] regulator: Use regulator_lock_supply() for get_voltage() too Mark Brown
@ 2015-11-03 15:16 ` Mark Brown
  2015-11-03 23:50 ` [PATCH] regulator: Use regulator_lock_supply() for get_voltage() too John Stultz
  1 sibling, 0 replies; 5+ messages in thread
From: Mark Brown @ 2015-11-03 15:16 UTC (permalink / raw)
  To: John Stultz, Mark Brown; +Cc: linux-kernel

The patch

   regulator: Use regulator_lock_supply() for get_voltage() too

has been applied to the regulator tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From e8debdfce55c8fcf2c9448b6adfd06ec75b7a844 Mon Sep 17 00:00:00 2001
From: Mark Brown <broonie@kernel.org>
Date: Tue, 3 Nov 2015 05:58:14 +0000
Subject: [PATCH] regulator: Use regulator_lock_supply() for get_voltage() too

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 771c6235cced..b4970eb85357 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -3055,7 +3055,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;
 	}
@@ -3078,11 +3078,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


^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH] regulator: Use regulator_lock_supply() for get_voltage() too
  2015-11-03  9:23 [PATCH] regulator: Use regulator_lock_supply() for get_voltage() too Mark Brown
  2015-11-03 15:16 ` Applied "regulator: Use regulator_lock_supply() for get_voltage() too" to the regulator tree Mark Brown
@ 2015-11-03 23:50 ` John Stultz
  1 sibling, 0 replies; 5+ messages in thread
From: John Stultz @ 2015-11-03 23:50 UTC (permalink / raw)
  To: Mark Brown; +Cc: Liam Girdwood, lkml

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

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH] regulator: Use regulator_lock_supply() for get_voltage() too
  2015-11-03  6:12 Mark Brown
@ 2015-11-03  7:04 ` kbuild test robot
  0 siblings, 0 replies; 5+ messages in thread
From: kbuild test robot @ 2015-11-03  7:04 UTC (permalink / raw)
  To: Mark Brown
  Cc: kbuild-all, Liam Girdwood, John Stultz, linux-kernel, Mark Brown

[-- 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 --]

^ permalink raw reply	[flat|nested] 5+ messages in thread
* [PATCH] regulator: Use regulator_lock_supply() for get_voltage() too
@ 2015-11-03  6:12 Mark Brown
  2015-11-03  7:04 ` kbuild test robot
  0 siblings, 1 reply; 5+ messages in thread
From: Mark Brown @ 2015-11-03  6:12 UTC (permalink / raw)
  To: Liam Girdwood, John Stultz; +Cc: linux-kernel, 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


^ permalink raw reply related	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-11-03 23:50 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-03  9:23 [PATCH] regulator: Use regulator_lock_supply() for get_voltage() too Mark Brown
2015-11-03 15:16 ` Applied "regulator: Use regulator_lock_supply() for get_voltage() too" to the regulator tree Mark Brown
2015-11-03 23:50 ` [PATCH] regulator: Use regulator_lock_supply() for get_voltage() too John Stultz
  -- strict thread matches above, loose matches on Subject: below --
2015-11-03  6:12 Mark Brown
2015-11-03  7:04 ` kbuild test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).