2012-08-08 12:17:28

by Axel Lin

[permalink] [raw]
Subject: [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(+)

diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
index bac4c87..c10012f 100644
--- a/include/linux/regulator/driver.h
+++ b/include/linux/regulator/driver.h
@@ -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.
--
1.7.9.5



2012-08-08 12:18:44

by Axel Lin

[permalink] [raw]
Subject: [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(-)

diff --git a/drivers/regulator/wm8350-regulator.c b/drivers/regulator/wm8350-regulator.c
index 7f0fa22..1e4f69e 100644
--- a/drivers/regulator/wm8350-regulator.c
+++ b/drivers/regulator/wm8350-regulator.c
@@ -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;
--
1.7.9.5


2012-08-08 12:19:41

by Axel Lin

[permalink] [raw]
Subject: [PATCH 3/7] regulator: wm831x-isink: set_current_limit should select the maximum current in specific range

Signed-off-by: Axel Lin <[email protected]>
---
drivers/regulator/wm831x-isink.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/regulator/wm831x-isink.c b/drivers/regulator/wm831x-isink.c
index 0d207c2..5dba339 100644
--- a/drivers/regulator/wm831x-isink.c
+++ b/drivers/regulator/wm831x-isink.c
@@ -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);
}
}

--
1.7.9.5


2012-08-08 12:20:35

by Axel Lin

[permalink] [raw]
Subject: [PATCH 4/7] regulator: wm831x-dcdc: set_current_limit should select the maximum current in specific range

Signed-off-by: Axel Lin <[email protected]>
---
drivers/regulator/wm831x-dcdc.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/regulator/wm831x-dcdc.c b/drivers/regulator/wm831x-dcdc.c
index 7413885..90cbcc6 100644
--- a/drivers/regulator/wm831x-dcdc.c
+++ b/drivers/regulator/wm831x-dcdc.c
@@ -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)
--
1.7.9.5


2012-08-08 12:21:37

by Axel Lin

[permalink] [raw]
Subject: [PATCH 5/7] regulator: tps6524x: set_current_limit should select the maximum current in specific range

Signed-off-by: Axel Lin <[email protected]>
---
drivers/regulator/tps6524x-regulator.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/regulator/tps6524x-regulator.c b/drivers/regulator/tps6524x-regulator.c
index 947ece9..058d2f2 100644
--- a/drivers/regulator/tps6524x-regulator.c
+++ b/drivers/regulator/tps6524x-regulator.c
@@ -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)
--
1.7.9.5


2012-08-08 12:22:42

by Axel Lin

[permalink] [raw]
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(-)

diff --git a/drivers/regulator/lp872x.c b/drivers/regulator/lp872x.c
index 212c38e..6199d0f 100644
--- a/drivers/regulator/lp872x.c
+++ b/drivers/regulator/lp872x.c
@@ -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)
--
1.7.9.5


2012-08-08 12:24:01

by Axel Lin

[permalink] [raw]
Subject: [PATCH 7/7] regulator: da9052: set_current_limit should select the maximum current in specific range

Signed-off-by: Axel Lin <[email protected]>
---
drivers/regulator/da9052-regulator.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/da9052-regulator.c b/drivers/regulator/da9052-regulator.c
index 903299c..27355b1 100644
--- a/drivers/regulator/da9052-regulator.c
+++ b/drivers/regulator/da9052-regulator.c
@@ -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;
}
--
1.7.9.5


2012-08-09 07:10:14

by Kim, Milo

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

> -----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(-)
>
> diff --git a/drivers/regulator/lp872x.c b/drivers/regulator/lp872x.c
> index 212c38e..6199d0f 100644
> --- a/drivers/regulator/lp872x.c
> +++ b/drivers/regulator/lp872x.c
> @@ -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)
> --
> 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?

2012-08-09 10:47:51

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 1/7] regulator: Update comment for set_current_limit callback of struct regulator_ops

On Wed, Aug 08, 2012 at 08:17:18PM +0800, Axel Lin wrote:
> 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.