Skip to contentSkip to conversation

regulator: Update comment for set_current_limit callback of struct regulator_ops

PATCH In review Axel Lin 3 participants 9 messages Aug 8, 2012 – Aug 9, 2012

Subsystems (3)
Voltage and current regulator framework Wolfson microelectronics drivers Dialog semiconductor drivers
lore
Share & tools
lore

Axel Lin

author
#0 lore raw

PATCH 1/7 regulator: Update comment for set_current_limit callback of struct regulator_ops

The regulators should be tending to the maximum in the available range and consumers should specify the widest range possible.

Signed-off-by Axel Lin <[email protected]>
---
 include/linux/regulator/driver.h |    1 +
 1 file changed, 1 insertion(+)
include/linux/regulator/driver.h +1 -0
index bac4c87..c10012f 100644
@@ -58,6 +58,7 @@ enum regulator_status {
  *	regulator_desc.n_voltages.  Voltages may be reported in any order.
  *
  * @set_current_limit: Configure a limit for a current-limited regulator.
+ *                     The driver should select the current closest to max_uA.
  * @get_current_limit: Get the configured limit for a current-limited regulator.
  *
  * @set_mode: Set the configured operating mode for the regulator.
Signature · 1 line
1.7.9.5
author
#1 lore raw

PATCH 2/7 regulator: wm8350: set_current_limit should select the maximum current in specific range

Signed-off-by Axel Lin <[email protected]>
---
 drivers/regulator/wm8350-regulator.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/regulator/wm8350-regulator.c +1 -1
index 7f0fa22..1e4f69e 100644
@@ -99,7 +99,7 @@ static int get_isink_val(int min_uA, int max_uA, u16 *setting)
 {
 	int i;
 
-	for (i = 0; i < ARRAY_SIZE(isink_cur); i++) {
+	for (i = ARRAY_SIZE(isink_cur) - 1; i >= 0; i--) {
 		if (min_uA <= isink_cur[i] && max_uA >= isink_cur[i]) {
 			*setting = i;
 			return 0;
Signature · 1 line
1.7.9.5
author
#2 lore raw

PATCH 3/7 regulator: wm831x-isink: set_current_limit should select the maximum current in specific range

In reply to Axel Lin
Signed-off-by Axel Lin <[email protected]>
---
 drivers/regulator/wm831x-isink.c |    9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)
drivers/regulator/wm831x-isink.c +4 -5
index 0d207c2..5dba339 100644
@@ -97,14 +97,13 @@ static int wm831x_isink_set_current(struct regulator_dev *rdev,
 {
 	struct wm831x_isink *isink = rdev_get_drvdata(rdev);
 	struct wm831x *wm831x = isink->wm831x;
-	int ret, i;
+	int i;
 
-	for (i = 0; i < ARRAY_SIZE(wm831x_isinkv_values); i++) {
+	for (i = ARRAY_SIZE(wm831x_isinkv_values) - 1; i >= 0; i--) {
 		int val = wm831x_isinkv_values[i];
 		if (min_uA <= val && val <= max_uA) {
-			ret = wm831x_set_bits(wm831x, isink->reg,
-					      WM831X_CS1_ISEL_MASK, i);
-			return ret;
+			return wm831x_set_bits(wm831x, isink->reg,
+					       WM831X_CS1_ISEL_MASK, i);
 		}
 	}
Signature · 1 line
1.7.9.5
author
#3 lore raw

PATCH 4/7 regulator: wm831x-dcdc: set_current_limit should select the maximum current in specific range

In reply to Axel Lin
Signed-off-by Axel Lin <[email protected]>
---
 drivers/regulator/wm831x-dcdc.c |   11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)
drivers/regulator/wm831x-dcdc.c +5 -6
index 7413885..90cbcc6 100644
@@ -339,16 +339,15 @@ static int wm831x_buckv_set_current_limit(struct regulator_dev *rdev,
 	u16 reg = dcdc->base + WM831X_DCDC_CONTROL_2;
 	int i;
 
-	for (i = 0; i < ARRAY_SIZE(wm831x_dcdc_ilim); i++) {
+	for (i = ARRAY_SIZE(wm831x_dcdc_ilim) - 1; i >= 0; i--) {
 		if ((min_uA <= wm831x_dcdc_ilim[i]) &&
 		    (wm831x_dcdc_ilim[i] <= max_uA))
-			break;
+			return wm831x_set_bits(wm831x, reg,
+					       WM831X_DC1_HC_THR_MASK,
+						i << WM831X_DC1_HC_THR_SHIFT);
 	}
-	if (i == ARRAY_SIZE(wm831x_dcdc_ilim))
-		return -EINVAL;
 
-	return wm831x_set_bits(wm831x, reg, WM831X_DC1_HC_THR_MASK,
-			       i << WM831X_DC1_HC_THR_SHIFT);
+	return -EINVAL;
 }
 
 static int wm831x_buckv_get_current_limit(struct regulator_dev *rdev)
Signature · 1 line
1.7.9.5
author
#4 lore raw

PATCH 5/7 regulator: tps6524x: set_current_limit should select the maximum current in specific range

In reply to Axel Lin
Signed-off-by Axel Lin <[email protected]>
---
 drivers/regulator/tps6524x-regulator.c |   10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)
drivers/regulator/tps6524x-regulator.c +4 -6
index 947ece9..058d2f2 100644
@@ -502,15 +502,13 @@ static int set_current_limit(struct regulator_dev *rdev, int min_uA,
 	if (info->n_ilimsels == 1)
 		return -EINVAL;
 
-	for (i = 0; i < info->n_ilimsels; i++)
+	for (i = info->n_ilimsels - 1; i >= 0; i--) {
 		if (min_uA <= info->ilimsels[i] &&
 		    max_uA >= info->ilimsels[i])
-			break;
-
-	if (i >= info->n_ilimsels)
-		return -EINVAL;
+			return write_field(hw, &info->ilimsel, i);
+	}
 
-	return write_field(hw, &info->ilimsel, i);
+	return -EINVAL;
 }
 
 static int get_current_limit(struct regulator_dev *rdev)
Signature · 1 line
1.7.9.5
author
#5 lore raw

PATCH 6/7 regulator: lp872x: set_current_limit should select the maximum current in specific range

In reply to Axel Lin
Signed-off-by Axel Lin <[email protected]>
---
 drivers/regulator/lp872x.c |   18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)
drivers/regulator/lp872x.c +8 -10
index 212c38e..6199d0f 100644
@@ -374,8 +374,8 @@ static int lp8725_buck_set_current_limit(struct regulator_dev *rdev,
 {
 	struct lp872x *lp = rdev_get_drvdata(rdev);
 	enum lp872x_regulator_id buck = rdev_get_id(rdev);
-	int i, max = ARRAY_SIZE(lp8725_buck_uA);
-	u8 addr, val;
+	int i;
+	u8 addr;
 
 	switch (buck) {
 	case LP8725_ID_BUCK1:

@@ -388,17 +388,15 @@ static int lp8725_buck_set_current_limit(struct regulator_dev *rdev,
 		return -EINVAL;
 	}
 
-	for (i = 0 ; i < max ; i++)
+	for (i = ARRAY_SIZE(lp8725_buck_uA) - 1 ; i >= 0; i--) {
 		if (lp8725_buck_uA[i] >= min_uA &&
 			lp8725_buck_uA[i] <= max_uA)
-			break;
-
-	if (i == max)
-		return -EINVAL;
-
-	val = i << LP8725_BUCK_CL_S;
+			return lp872x_update_bits(lp, addr,
+						  LP8725_BUCK_CL_M,
+						  i << LP8725_BUCK_CL_S);
+	}
 
-	return lp872x_update_bits(lp, addr, LP8725_BUCK_CL_M, val);
+	return -EINVAL;
 }
 
 static int lp8725_buck_get_current_limit(struct regulator_dev *rdev)
Signature · 1 line
1.7.9.5
#6 lore raw

RE: [PATCH 6/7] regulator: lp872x: set_current_limit should select the maximum current in specific range

Quoted 53 lines

-----Original Message-----
From: Axel Lin [mailto:[email protected]]
Sent: Wednesday, August 08, 2012 9:23 PM
To: Mark Brown
Cc: Kim, Milo; Girdwood, Liam; [email protected]
Subject: [PATCH 6/7] regulator: lp872x: set_current_limit should select the maximum current in specific range

Signed-off-by Axel Lin <[email protected]>
---
 drivers/regulator/lp872x.c |   18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)
drivers/regulator/lp872x.c +2 -2
index 212c38e..6199d0f 100644
@@ -374,8 +374,8 @@ static int lp8725_buck_set_current_limit(struct
 regulator_dev *rdev,
 {
 	struct lp872x *lp = rdev_get_drvdata(rdev);
 	enum lp872x_regulator_id buck = rdev_get_id(rdev);
-	int i, max = ARRAY_SIZE(lp8725_buck_uA);
-	u8 addr, val;
+	int i;
+	u8 addr;
 
 	switch (buck) {
drivers/regulator/lp872x.c +6 -8
 	case LP8725_ID_BUCK1:

@@ -388,17 +388,15 @@ static int lp8725_buck_set_current_limit(struct
 regulator_dev *rdev,
 		return -EINVAL;
 	}
 
-	for (i = 0 ; i < max ; i++)
+	for (i = ARRAY_SIZE(lp8725_buck_uA) - 1 ; i >= 0; i--) {
 		if (lp8725_buck_uA[i] >= min_uA &&
 			lp8725_buck_uA[i] <= max_uA)
-			break;
-
-	if (i == max)
-		return -EINVAL;
-
-	val = i << LP8725_BUCK_CL_S;
+			return lp872x_update_bits(lp, addr,
+						  LP8725_BUCK_CL_M,
+						  i << LP8725_BUCK_CL_S);
+	}
 
-	return lp872x_update_bits(lp, addr, LP8725_BUCK_CL_M, val);
+	return -EINVAL;
 }
Quoted 5 lines

static int lp8725_buck_get_current_limit(struct regulator_dev *rdev)
--
1.7.9.5

It looks a semantic patch rather than operation issue. Could you let me know why we need this patch in more details ? Thank you, all the time !

Best Regards,
Milo ���{.n�+�������+%�����ݶ��w��{.n�+����{��G�����{ay�ʇڙ�,j��f���h��������z_�(�階�ݢj"���m������G������?����&���~��iO��z��v�^�m���� ������?�I�

author
#7 lore raw

PATCH 7/7 regulator: da9052: set_current_limit should select the maximum current in specific range

In reply to Axel Lin
Signed-off-by Axel Lin <[email protected]>
---
 drivers/regulator/da9052-regulator.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/regulator/da9052-regulator.c +2 -2
index 903299c..27355b1 100644
@@ -133,8 +133,8 @@ static int da9052_dcdc_set_current_limit(struct regulator_dev *rdev, int min_uA,
 	    max_uA < da9052_current_limits[row][DA9052_MIN_UA])
 		return -EINVAL;
 
-	for (i = 0; i < DA9052_CURRENT_RANGE; i++) {
-		if (min_uA <= da9052_current_limits[row][i]) {
+	for (i = DA9052_CURRENT_RANGE - 1; i >= 0; i--) {
+		if (da9052_current_limits[row][i] <= max_uA) {
 			reg_val = i;
 			break;
 		}
Signature · 1 line
1.7.9.5
#8 lore raw
In reply to Axel Lin
Quoted 2 lines · Axel Lin

The regulators should be tending to the maximum in the available range and consumers should specify the widest range possible.

Applied all except 2 and 3. Those are for actual current regulators which are a bit odd here - I'm not really sure they should be in the regulator API at all but their function is very different to the current limits that are normally being specified.