[PATCH] regulator: pass voltage when calling notifier for change

[PATCH] regulator: pass voltage when calling notifier for change

[Philip Rakity]

The notified call back does pass the voltage that has
changed when calling the notifier callback.  This makes
it impossible to know what to do.  Pass the data
parameter (voltage) to the notifier.

A call to regulator_get_voltage cannot be made by the
receiver of the notifier since the mutex is held by
the regulator code and the call is blocked.  Deadlock
occurs.

This code was tested on a slightly earlier version of
linux where it was used to adjust the chip pad voltage
when doing SD vccq signaling for SDXC cards.

Signed-off-by: Philip Rakity <prakity@marvell.com>
---
 drivers/regulator/core.c |   15 ++++++++++-----
 1 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index e70dd38..f3d66d6 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1916,11 +1916,16 @@ static int _regulator_do_set_voltage(struct regulator_dev *rdev,
 		udelay(delay);
 	}
 
-	if (ret == 0)
-		_notifier_call_chain(rdev, REGULATOR_EVENT_VOLTAGE_CHANGE,
-				     NULL);
+	if (ret == 0) {
+		int voltage;
 
-	trace_regulator_set_voltage_complete(rdev_get_name(rdev), selector);
+		voltage = _regulator_get_voltage(rdev);
+		if (voltage >= 0)
+			_notifier_call_chain(rdev,
+					REGULATOR_EVENT_VOLTAGE_CHANGE,
+					(void *)voltage);
+	}
+	trace_regulator_set_voltage_complete(rdev_get_name(rdev), ret);
 
 	return ret;
 }
@@ -2405,7 +2410,7 @@ static void _notifier_call_chain(struct regulator_dev *rdev,
 				  unsigned long event, void *data)
 {
 	/* call rdev chain first */
-	blocking_notifier_call_chain(&rdev->notifier, event, NULL);
+	blocking_notifier_call_chain(&rdev->notifier, event, data);
 }
 
 /**
-- 
1.7.0.4

Re: [PATCH] regulator: pass voltage when calling notifier for change

[Mark Brown]

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

On Sat, Jun 09, 2012 at 12:39:15PM -0700, Philip Rakity wrote:

> -	trace_regulator_set_voltage_complete(rdev_get_name(rdev), selector);
> +		voltage = _regulator_get_voltage(rdev);
> +		if (voltage >= 0)
> +			_notifier_call_chain(rdev,
> +					REGULATOR_EVENT_VOLTAGE_CHANGE,
> +					(void *)voltage);
> +	}
> +	trace_regulator_set_voltage_complete(rdev_get_name(rdev), ret);

Hrm, I know we discussed this offline but now that I see the actual code
for the full change here I'm a bit worried about the performance impact
from calling get_voltage() again with devices that don't cache the
register map (it'll be negligable for those that do).  Looking a bit at
the context I think probably what we want to do here is directly call
list_voltage() for the selector we've just set so that the driver
doesn't end up going back to the hardware to read the register value.

Another option is to look at the notifier chain to see if there's any
actual users but that seems more complicated and less abstracted.

We definitely do want to pass the voltage out, now that I see this I
seem to remember that the reason we don't is exactly the performance
concern above but that all predates the use of selectors here which
means we should be able to avoid the I/O costs.

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