Re: [PATCH] regulator: ab8500: Fix get_mode for shared mode regulators

From: Bengt JÃnsson
Date: Mon Apr 15 2013 - 04:03:34 EST


On 04/08/2013 02:31 PM, Axel Lin wrote:
The special handling code for getting shared mode status is wrong
because it needs to check info->shared_mode->lp_mode_req for
both regulators that shared the same mode register.

In set_mode(), current code ensures we won't set mode to REGULATOR_MODE_IDLE
if only one of the regulator requests to set idle.

In get_mode(), we can just remove the special handling code for shared mode.
Read the register value always returns correct status no matter the regulator
has shared mode register or not.
I am not convinced about this patch. The purpose of the original code is that the regulator framework should be unaware that the mode register is shared with another regulator. If we apply this patch, get_mode may return different results depending on the other regulators mode settings.

Let me take an example: The other shared regulator is already set to LP mode and the current regulator is requested to low power mode. Then the framework first checks current mode and compares to requested mode. If equal, it returns. With this patch applied, it will see that the regulator is already set to LP mode and return without calling set_mode in the driver. However, the state information in the driver will be wrong so when the other regulator is requested to HP mode and back to LP mode it will not actually set LP mode again to HW.

Regards,

Bengt

Signed-off-by: Axel Lin <axel.lin@xxxxxxxxxx>
---
drivers/regulator/ab8500.c | 8 --------
1 file changed, 8 deletions(-)

diff --git a/drivers/regulator/ab8500.c b/drivers/regulator/ab8500.c
index 9ebd131..acdffc5 100644
--- a/drivers/regulator/ab8500.c
+++ b/drivers/regulator/ab8500.c
@@ -456,14 +456,6 @@ static unsigned int ab8500_regulator_get_mode(struct regulator_dev *rdev)
return -EINVAL;
}
- /* Need special handling for shared mode */
- if (info->shared_mode) {
- if (info->shared_mode->lp_mode_req)
- return REGULATOR_MODE_IDLE;
- else
- return REGULATOR_MODE_NORMAL;
- }
-
if (info->mode_mask) {
/* Dedicated register for handling mode */
ret = abx500_get_register_interruptible(info->dev,

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/