[PATCH 1/2] regulator: lp8788-buck: Remove val array in lp8788_init_dvs

[PATCH 1/2] regulator: lp8788-buck: Remove val array in lp8788_init_dvs

[Axel Lin]

All elements of val array are zero.
Simply set data argument to be 0 in lp8788_update_bits call and remove the val
array.

Signed-off-by: Axel Lin <axel.lin@ingics.com>
---
 drivers/regulator/lp8788-buck.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/regulator/lp8788-buck.c b/drivers/regulator/lp8788-buck.c
index 6cc02c3..d33b81a 100644
--- a/drivers/regulator/lp8788-buck.c
+++ b/drivers/regulator/lp8788-buck.c
@@ -478,7 +478,6 @@ static int lp8788_init_dvs(struct lp8788_buck *buck, enum lp8788_buck_id id)
 {
 	struct lp8788_platform_data *pdata = buck->lp->pdata;
 	u8 mask[] = { LP8788_BUCK1_DVS_SEL_M, LP8788_BUCK2_DVS_SEL_M };
-	u8 val[]  = { LP8788_BUCK1_DVS_PIN, LP8788_BUCK2_DVS_PIN };
 	u8 default_dvs_mode[] = { LP8788_BUCK1_DVS_I2C, LP8788_BUCK2_DVS_I2C };
 
 	/* no dvs for buck3, 4 */
@@ -496,8 +495,7 @@ static int lp8788_init_dvs(struct lp8788_buck *buck, enum lp8788_buck_id id)
 	if (lp8788_dvs_gpio_request(buck, id))
 		goto set_default_dvs_mode;
 
-	return lp8788_update_bits(buck->lp, LP8788_BUCK_DVS_SEL, mask[id],
-				val[id]);
+	return lp8788_update_bits(buck->lp, LP8788_BUCK_DVS_SEL, mask[id], 0);
 
 set_default_dvs_mode:
 	return lp8788_update_bits(buck->lp, LP8788_BUCK_DVS_SEL, mask[id],
-- 
1.7.9.5

[PATCH 2/2] regulator: lp8788-buck: Kill _gpio_request function

[Axel Lin]

Simply use devm_gpio_request_one() instead.

Signed-off-by: Axel Lin <axel.lin@ingics.com>
---
 drivers/regulator/lp8788-buck.c |   18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/drivers/regulator/lp8788-buck.c b/drivers/regulator/lp8788-buck.c
index d33b81a..369b034 100644
--- a/drivers/regulator/lp8788-buck.c
+++ b/drivers/regulator/lp8788-buck.c
@@ -429,18 +429,6 @@ static struct regulator_desc lp8788_buck_desc[] = {
 	},
 };
 
-static int _gpio_request(struct lp8788_buck *buck, int gpio, char *name)
-{
-	struct device *dev = buck->lp->dev;
-
-	if (!gpio_is_valid(gpio)) {
-		dev_err(dev, "invalid gpio: %d\n", gpio);
-		return -EINVAL;
-	}
-
-	return devm_gpio_request_one(dev, gpio, DVS_LOW, name);
-}
-
 static int lp8788_dvs_gpio_request(struct lp8788_buck *buck,
 				enum lp8788_buck_id id)
 {
@@ -452,7 +440,8 @@ static int lp8788_dvs_gpio_request(struct lp8788_buck *buck,
 	switch (id) {
 	case BUCK1:
 		gpio = pdata->buck1_dvs->gpio;
-		ret = _gpio_request(buck, gpio, b1_name);
+		ret = devm_gpio_request_one(buck->lp->dev, gpio, DVS_LOW,
+					    b1_name);
 		if (ret)
 			return ret;
 
@@ -461,7 +450,8 @@ static int lp8788_dvs_gpio_request(struct lp8788_buck *buck,
 	case BUCK2:
 		for (i = 0 ; i < LP8788_NUM_BUCK2_DVS ; i++) {
 			gpio = pdata->buck2_dvs->gpio[i];
-			ret = _gpio_request(buck, gpio, b2_name[i]);
+			ret = devm_gpio_request_one(buck->lp->dev, gpio,
+						    DVS_LOW, b2_name[i]);
 			if (ret)
 				return ret;
 		}
-- 
1.7.9.5

RE: [PATCH 2/2] regulator: lp8788-buck: Kill _gpio_request function

[Kim, Milo]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 313 bytes --]

> Simply use devm_gpio_request_one() instead.
> 
> Signed-off-by: Axel Lin <axel.lin@ingics.com>

Thanks!

Acked-by: Milo(Woogyom) Kim <milo.kim@ti.com>
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH 2/2] regulator: lp8788-buck: Kill _gpio_request function

[Mark Brown]

On Fri, Dec 07, 2012 at 05:25:13PM +0800, Axel Lin wrote:
> Simply use devm_gpio_request_one() instead.

Applied, thanks.

Milo, you don't have a MAINTAINERS entry for this chip - would you mind
sending a patch adding one as you are reviewing patches for the driver?
This will ensure that people send you patches for review.

RE: [PATCH 2/2] regulator: lp8788-buck: Kill _gpio_request function

[Kim, Milo]

> Applied, thanks.
> 
> Milo, you don't have a MAINTAINERS entry for this chip - would you mind
> sending a patch adding one as you are reviewing patches for the driver?
> This will ensure that people send you patches for review.

OK, I'll update MAINTAINERS file for lp872x and lp8788.
Thanks for your help.

Best Regards,
Milo

RE: [PATCH 1/2] regulator: lp8788-buck: Remove val array in lp8788_init_dvs

[Kim, Milo]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 525 bytes --]

> All elements of val array are zero.
> Simply set data argument to be 0 in lp8788_update_bits call and remove
> the val
> array.
> 
> Signed-off-by: Axel Lin <axel.lin@ingics.com>

Axel, sorry I disagree with this patch because DVS_PIN is meaningful.
In this case, explicit bit description is more important than code simplicity.
So I would keep it.

Thanks,
Milo
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

RE: [PATCH 1/2] regulator: lp8788-buck: Remove val array in lp8788_init_dvs

[Kim, Milo]

> Actually, what you want is to clear mask[id] bits.
> Setting 0 for the data argument here is clear.

I would say it's not clearing bit but the DVS control configuration.
If the bit is 0, external pin is used for the DVS.
If it is 1, then DVS is controlled by other register settings.
Therefore, it's a kind of default configuration rather than clearing bits.

> But if you prefer to keep it as is, it's ok.
Thanks for your understanding.

Best Regards,
Milo