All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] bq24022: Add support for bq2407x family
@ 2011-08-28 13:07 Heiko Stübner
  2011-08-28 13:08 ` [PATCH 1/3] bq24022: Evaluate returns of gpio_direction_output-calls Heiko Stübner
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Heiko Stübner @ 2011-08-28 13:07 UTC (permalink / raw)
  To: Mark Brown, Liam Girdwood; +Cc: linux-pm, Philipp Zabel

The bq2407x family is quite similar to the bq24022 charger.
As suggested by MyungJoo Ham it should be possible to implement
support for it inside the bq24022 driver instead of adding a new one.

The first two patches fix small things Mark Brown identified in my
submission of a separate bq2407x driver that also exist in the bq24022
while the third implements the bq2407x specifics.

Heiko Stuebner (3):
  bq24022: Evaluate returns of gpio_direction_output-calls
  bq24022: Keep track of gpio states
  bq24022: Add support for the bq2407x family

 drivers/regulator/Kconfig         |    7 +-
 drivers/regulator/bq24022.c       |  138 ++++++++++++++++++++++++++++++-------
 include/linux/regulator/bq24022.h |    7 ++
 3 files changed, 125 insertions(+), 27 deletions(-)

-- 
1.7.2.3

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/3] bq24022: Evaluate returns of gpio_direction_output-calls
  2011-08-28 13:07 [PATCH 0/3] bq24022: Add support for bq2407x family Heiko Stübner
@ 2011-08-28 13:08 ` Heiko Stübner
  2011-08-29  9:16   ` Mark Brown
  2011-08-28 13:09 ` [PATCH 2/3] bq24022: Keep track of gpio states Heiko Stübner
  2011-08-28 13:10 ` [PATCH 3/3] bq24022: Add support for the bq2407x family Heiko Stübner
  2 siblings, 1 reply; 6+ messages in thread
From: Heiko Stübner @ 2011-08-28 13:08 UTC (permalink / raw)
  To: Mark Brown, Liam Girdwood; +Cc: linux-pm, Philipp Zabel

It wasn't done before.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 drivers/regulator/bq24022.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/drivers/regulator/bq24022.c b/drivers/regulator/bq24022.c
index e24d1b7..973af08 100644
--- a/drivers/regulator/bq24022.c
+++ b/drivers/regulator/bq24022.c
@@ -104,7 +104,17 @@ static int __init bq24022_probe(struct platform_device *pdev)
 		goto err_iset2;
 	}
 	ret = gpio_direction_output(pdata->gpio_iset2, 0);
+	if (ret) {
+		dev_dbg(&pdev->dev, "couldn't set ISET2 GPIO: %d\n",
+			pdata->gpio_iset2);
+		goto err_reg;
+	}
 	ret = gpio_direction_output(pdata->gpio_nce, 1);
+	if (ret) {
+		dev_dbg(&pdev->dev, "couldn't set nCE GPIO: %d\n",
+			pdata->gpio_nce);
+		goto err_reg;
+	}
 
 	bq24022 = regulator_register(&bq24022_desc, &pdev->dev,
 				     pdata->init_data, pdata);
-- 
1.7.2.3

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/3] bq24022: Keep track of gpio states
  2011-08-28 13:07 [PATCH 0/3] bq24022: Add support for bq2407x family Heiko Stübner
  2011-08-28 13:08 ` [PATCH 1/3] bq24022: Evaluate returns of gpio_direction_output-calls Heiko Stübner
@ 2011-08-28 13:09 ` Heiko Stübner
  2011-08-29  9:18   ` Mark Brown
  2011-08-28 13:10 ` [PATCH 3/3] bq24022: Add support for the bq2407x family Heiko Stübner
  2 siblings, 1 reply; 6+ messages in thread
From: Heiko Stübner @ 2011-08-28 13:09 UTC (permalink / raw)
  To: Mark Brown, Liam Girdwood; +Cc: linux-pm, Philipp Zabel

gpio_get_value is not definied for output-gpios and can therefore not be
used for the is_enabled and get_current_limit methods of the bq24022.

This patch introduces a private struct to keep track of the values set.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 drivers/regulator/bq24022.c |   67 +++++++++++++++++++++++++++++-------------
 1 files changed, 46 insertions(+), 21 deletions(-)

diff --git a/drivers/regulator/bq24022.c b/drivers/regulator/bq24022.c
index 973af08..b5a6755 100644
--- a/drivers/regulator/bq24022.c
+++ b/drivers/regulator/bq24022.c
@@ -15,56 +15,69 @@
 #include <linux/platform_device.h>
 #include <linux/err.h>
 #include <linux/module.h>
+#include <linux/slab.h>
 #include <linux/gpio.h>
 #include <linux/regulator/bq24022.h>
 #include <linux/regulator/driver.h>
 
+struct bq24022 {
+	struct regulator_dev	*rdev;
+
+	int gpio_nce;
+	int gpio_iset2;
+
+	int state_nce;
+	int state_iset2;
+};
 
 static int bq24022_set_current_limit(struct regulator_dev *rdev,
 					int min_uA, int max_uA)
 {
-	struct bq24022_mach_info *pdata = rdev_get_drvdata(rdev);
+	struct bq24022 *bq = rdev_get_drvdata(rdev);
 
 	dev_dbg(rdev_get_dev(rdev), "setting current limit to %s mA\n",
 		max_uA >= 500000 ? "500" : "100");
 
 	/* REVISIT: maybe return error if min_uA != 0 ? */
-	gpio_set_value(pdata->gpio_iset2, max_uA >= 500000);
+	bq->state_iset2 = (max_uA >= 500000);
+	gpio_set_value(bq->gpio_iset2, bq->state_iset2);
 	return 0;
 }
 
 static int bq24022_get_current_limit(struct regulator_dev *rdev)
 {
-	struct bq24022_mach_info *pdata = rdev_get_drvdata(rdev);
+	struct bq24022 *bq = rdev_get_drvdata(rdev);
 
-	return gpio_get_value(pdata->gpio_iset2) ? 500000 : 100000;
+	return bq->state_iset2 ? 500000 : 100000;
 }
 
 static int bq24022_enable(struct regulator_dev *rdev)
 {
-	struct bq24022_mach_info *pdata = rdev_get_drvdata(rdev);
+	struct bq24022 *bq = rdev_get_drvdata(rdev);
 
 	dev_dbg(rdev_get_dev(rdev), "enabling charger\n");
 
-	gpio_set_value(pdata->gpio_nce, 0);
+	gpio_set_value(bq->gpio_nce, 0);
+	bq->state_nce = 0;
 	return 0;
 }
 
 static int bq24022_disable(struct regulator_dev *rdev)
 {
-	struct bq24022_mach_info *pdata = rdev_get_drvdata(rdev);
+	struct bq24022 *bq = rdev_get_drvdata(rdev);
 
 	dev_dbg(rdev_get_dev(rdev), "disabling charger\n");
 
-	gpio_set_value(pdata->gpio_nce, 1);
+	gpio_set_value(bq->gpio_nce, 1);
+	bq->state_nce = 1;
 	return 0;
 }
 
 static int bq24022_is_enabled(struct regulator_dev *rdev)
 {
-	struct bq24022_mach_info *pdata = rdev_get_drvdata(rdev);
+	struct bq24022 *bq = rdev_get_drvdata(rdev);
 
-	return !gpio_get_value(pdata->gpio_nce);
+	return !bq->state_nce;
 }
 
 static struct regulator_ops bq24022_ops = {
@@ -85,12 +98,18 @@ static struct regulator_desc bq24022_desc = {
 static int __init bq24022_probe(struct platform_device *pdev)
 {
 	struct bq24022_mach_info *pdata = pdev->dev.platform_data;
-	struct regulator_dev *bq24022;
+	struct bq24022 *bq;
 	int ret;
 
 	if (!pdata || !pdata->gpio_nce || !pdata->gpio_iset2)
 		return -EINVAL;
 
+	bq = kzalloc(sizeof(struct bq24022), GFP_KERNEL);
+	if (!bq) {
+		dev_err(&pdev->dev, "cannot allocate memory\n");
+		return -ENOMEM;
+	}
+
 	ret = gpio_request(pdata->gpio_nce, "ncharge_en");
 	if (ret) {
 		dev_dbg(&pdev->dev, "couldn't request nCE GPIO: %d\n",
@@ -103,27 +122,32 @@ static int __init bq24022_probe(struct platform_device *pdev)
 			pdata->gpio_iset2);
 		goto err_iset2;
 	}
+
+	/* set initial current to 100mA and disable regulator */
 	ret = gpio_direction_output(pdata->gpio_iset2, 0);
 	if (ret) {
 		dev_dbg(&pdev->dev, "couldn't set ISET2 GPIO: %d\n",
 			pdata->gpio_iset2);
 		goto err_reg;
 	}
+	bq->gpio_iset2  = pdata->gpio_iset2;
 	ret = gpio_direction_output(pdata->gpio_nce, 1);
 	if (ret) {
 		dev_dbg(&pdev->dev, "couldn't set nCE GPIO: %d\n",
 			pdata->gpio_nce);
 		goto err_reg;
 	}
+	bq->gpio_nce  = pdata->gpio_nce;
+	bq->state_nce = 1;
 
-	bq24022 = regulator_register(&bq24022_desc, &pdev->dev,
-				     pdata->init_data, pdata);
-	if (IS_ERR(bq24022)) {
+	bq->rdev = regulator_register(&bq24022_desc, &pdev->dev,
+				     pdata->init_data, bq);
+	if (IS_ERR(bq->rdev)) {
 		dev_dbg(&pdev->dev, "couldn't register regulator\n");
-		ret = PTR_ERR(bq24022);
+		ret = PTR_ERR(bq->rdev);
 		goto err_reg;
 	}
-	platform_set_drvdata(pdev, bq24022);
+	platform_set_drvdata(pdev, bq);
 	dev_dbg(&pdev->dev, "registered regulator\n");
 
 	return 0;
@@ -137,12 +161,13 @@ err_ce:
 
 static int __devexit bq24022_remove(struct platform_device *pdev)
 {
-	struct bq24022_mach_info *pdata = pdev->dev.platform_data;
-	struct regulator_dev *bq24022 = platform_get_drvdata(pdev);
+	struct bq24022 *bq = platform_get_drvdata(pdev);
 
-	regulator_unregister(bq24022);
-	gpio_free(pdata->gpio_iset2);
-	gpio_free(pdata->gpio_nce);
+	regulator_unregister(bq->rdev);
+	gpio_free(bq->gpio_iset2);
+	gpio_free(bq->gpio_nce);
+
+	kfree(bq);
 
 	return 0;
 }
-- 
1.7.2.3

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 3/3] bq24022: Add support for the bq2407x family
  2011-08-28 13:07 [PATCH 0/3] bq24022: Add support for bq2407x family Heiko Stübner
  2011-08-28 13:08 ` [PATCH 1/3] bq24022: Evaluate returns of gpio_direction_output-calls Heiko Stübner
  2011-08-28 13:09 ` [PATCH 2/3] bq24022: Keep track of gpio states Heiko Stübner
@ 2011-08-28 13:10 ` Heiko Stübner
  2 siblings, 0 replies; 6+ messages in thread
From: Heiko Stübner @ 2011-08-28 13:10 UTC (permalink / raw)
  To: Mark Brown, Liam Girdwood; +Cc: linux-pm, Philipp Zabel

The bq2407x family of charger ICs simply supports another machine
specific charging current above 500mA. To use this setting a
second gpio pin is used while the rest of the pins stay the same.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 drivers/regulator/Kconfig         |    7 ++--
 drivers/regulator/bq24022.c       |   65 ++++++++++++++++++++++++++++++++++---
 include/linux/regulator/bq24022.h |    7 ++++
 3 files changed, 71 insertions(+), 8 deletions(-)

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index c7fd2c0..546a7a6 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -67,10 +67,11 @@ config REGULATOR_USERSPACE_CONSUMER
 config REGULATOR_BQ24022
 	tristate "TI bq24022 Dual Input 1-Cell Li-Ion Charger IC"
 	help
-	  This driver controls a TI bq24022 Charger attached via
-	  GPIOs. The provided current regulator can enable/disable
+	  This driver controls a TI bq24022 or bq2407x Charger attached
+	  via GPIOs. The provided current regulator can enable/disable
 	  charging select between 100 mA and 500 mA charging current
-	  limit.
+	  limit. The bq2407x family also supports a machine specific
+	  current limit above 500 mA.
 
 config REGULATOR_MAX1586
 	tristate "Maxim 1586/1587 voltage regulator"
diff --git a/drivers/regulator/bq24022.c b/drivers/regulator/bq24022.c
index b5a6755..ad6942a 100644
--- a/drivers/regulator/bq24022.c
+++ b/drivers/regulator/bq24022.c
@@ -19,15 +19,20 @@
 #include <linux/gpio.h>
 #include <linux/regulator/bq24022.h>
 #include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
 
 struct bq24022 {
 	struct regulator_dev	*rdev;
 
 	int gpio_nce;
 	int gpio_iset2;
+	int gpio_2407x_en2;
 
 	int state_nce;
 	int state_iset2;
+	int state_2407x_en2;
+
+	int max_uA;
 };
 
 static int bq24022_set_current_limit(struct regulator_dev *rdev,
@@ -35,12 +40,30 @@ static int bq24022_set_current_limit(struct regulator_dev *rdev,
 {
 	struct bq24022 *bq = rdev_get_drvdata(rdev);
 
-	dev_dbg(rdev_get_dev(rdev), "setting current limit to %s mA\n",
-		max_uA >= 500000 ? "500" : "100");
+	if (bq->max_uA && bq->max_uA > 500000
+			&& max_uA >= bq->max_uA && bq->gpio_2407x_en2) {
+		dev_dbg(rdev_get_dev(rdev),
+			"setting current limit to %d mA\n",
+			bq->max_uA / 1000);
+		gpio_set_value(bq->gpio_2407x_en2, 1);
+		bq->state_2407x_en2 = 1;
+		gpio_set_value(bq->gpio_iset2, 0);
+		bq->state_iset2 = 0;
+	} else if (max_uA >= 100000) {
+		dev_dbg(rdev_get_dev(rdev), "setting current limit to %s mA\n",
+			max_uA >= 500000 ? "500" : "100");
+		if (bq->gpio_2407x_en2) {
+			gpio_set_value(bq->gpio_2407x_en2, 0);
+			bq->state_2407x_en2 = 0;
+		}
+		bq->state_iset2 = (max_uA >= 500000);
+		gpio_set_value(bq->gpio_iset2, bq->state_iset2);
+	} else {
+		dev_err(rdev_get_dev(rdev), "cannot set current limit below 100 mA\n");
+		return -EINVAL;
+	}
 
 	/* REVISIT: maybe return error if min_uA != 0 ? */
-	bq->state_iset2 = (max_uA >= 500000);
-	gpio_set_value(bq->gpio_iset2, bq->state_iset2);
 	return 0;
 }
 
@@ -48,7 +71,10 @@ static int bq24022_get_current_limit(struct regulator_dev *rdev)
 {
 	struct bq24022 *bq = rdev_get_drvdata(rdev);
 
-	return bq->state_iset2 ? 500000 : 100000;
+	if (bq->state_2407x_en2)
+		return bq->state_iset2 ? 0 : bq->max_uA;
+	else
+		return bq->state_iset2 ? 500000 : 100000;
 }
 
 static int bq24022_enable(struct regulator_dev *rdev)
@@ -122,8 +148,25 @@ static int __init bq24022_probe(struct platform_device *pdev)
 			pdata->gpio_iset2);
 		goto err_iset2;
 	}
+	if (pdata->gpio_2407x_en2) {
+		ret = gpio_request(pdata->gpio_2407x_en2, "charge_mode2");
+		if (ret) {
+			dev_dbg(&pdev->dev, "couldn't request EN2 GPIO: %d\n",
+				pdata->gpio_2407x_en2);
+			goto err_en2;
+		}
+	}
 
 	/* set initial current to 100mA and disable regulator */
+	if (pdata->gpio_2407x_en2) {
+		ret = gpio_direction_output(pdata->gpio_2407x_en2, 0);
+		if (ret) {
+			dev_dbg(&pdev->dev, "couldn't set EN2 GPIO: %d\n",
+				pdata->gpio_2407x_en2);
+			goto err_reg;
+		}
+		bq->gpio_2407x_en2  = pdata->gpio_2407x_en2;
+	}
 	ret = gpio_direction_output(pdata->gpio_iset2, 0);
 	if (ret) {
 		dev_dbg(&pdev->dev, "couldn't set ISET2 GPIO: %d\n",
@@ -140,6 +183,13 @@ static int __init bq24022_probe(struct platform_device *pdev)
 	bq->gpio_nce  = pdata->gpio_nce;
 	bq->state_nce = 1;
 
+	/* get maximum current from regulator_init_data */
+	if (pdata->init_data) {
+		bq->max_uA = pdata->init_data->constraints.max_uA;
+		dev_dbg(&pdev->dev, "maximum current is %d mA\n",
+			bq->max_uA / 1000);
+	}
+
 	bq->rdev = regulator_register(&bq24022_desc, &pdev->dev,
 				     pdata->init_data, bq);
 	if (IS_ERR(bq->rdev)) {
@@ -152,6 +202,9 @@ static int __init bq24022_probe(struct platform_device *pdev)
 
 	return 0;
 err_reg:
+	if (pdata->gpio_2407x_en2)
+		gpio_free(pdata->gpio_2407x_en2);
+err_en2:
 	gpio_free(pdata->gpio_iset2);
 err_iset2:
 	gpio_free(pdata->gpio_nce);
@@ -164,6 +217,8 @@ static int __devexit bq24022_remove(struct platform_device *pdev)
 	struct bq24022 *bq = platform_get_drvdata(pdev);
 
 	regulator_unregister(bq->rdev);
+	if (bq->gpio_2407x_en2)
+		gpio_free(bq->gpio_2407x_en2);
 	gpio_free(bq->gpio_iset2);
 	gpio_free(bq->gpio_nce);
 
diff --git a/include/linux/regulator/bq24022.h b/include/linux/regulator/bq24022.h
index a6d0140..39a48e1 100644
--- a/include/linux/regulator/bq24022.h
+++ b/include/linux/regulator/bq24022.h
@@ -16,9 +16,16 @@ struct regulator_init_data;
  * bq24022_mach_info - platform data for bq24022
  * @gpio_nce: GPIO line connected to the nCE pin, used to enable / disable charging
  * @gpio_iset2: GPIO line connected to the ISET2 pin, used to limit charging current to 100 mA / 500 mA
+ * @gpio_2407x_en2: GPIO line connected to the en2 pin of the bq2407x family
+ * Modes of operation for bq2407x:
+ * EN2 = 0, ISET2 = 0: 100mA
+ * EN2 = 0, ISET2 = 1: 500mA
+ * EN2 = 1, ISET2 = 0: max_current
+ * EN2 = 1, ISET2 = 1: Standby (usb suspend)
  */
 struct bq24022_mach_info {
 	int gpio_nce;
 	int gpio_iset2;
+	int gpio_2407x_en2;
 	struct regulator_init_data *init_data;
 };
-- 
1.7.2.3

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/3] bq24022: Evaluate returns of gpio_direction_output-calls
  2011-08-28 13:08 ` [PATCH 1/3] bq24022: Evaluate returns of gpio_direction_output-calls Heiko Stübner
@ 2011-08-29  9:16   ` Mark Brown
  0 siblings, 0 replies; 6+ messages in thread
From: Mark Brown @ 2011-08-29  9:16 UTC (permalink / raw)
  To: Heiko Stübner; +Cc: linux-pm, Liam Girdwood, Philipp Zabel

On Sun, Aug 28, 2011 at 03:08:30PM +0200, Heiko Stübner wrote:

>  	ret = gpio_direction_output(pdata->gpio_iset2, 0);
> +	if (ret) {
> +		dev_dbg(&pdev->dev, "couldn't set ISET2 GPIO: %d\n",
> +			pdata->gpio_iset2);
> +		goto err_reg;
> +	}
>  	ret = gpio_direction_output(pdata->gpio_nce, 1);
> +	if (ret) {
> +		dev_dbg(&pdev->dev, "couldn't set nCE GPIO: %d\n",
> +			pdata->gpio_nce);
> +		goto err_reg;
> +	}

If these are fatal errors the log messages shouldn't be dev_dbg() but
should instead be something that will be displayed by default.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/3] bq24022: Keep track of gpio states
  2011-08-28 13:09 ` [PATCH 2/3] bq24022: Keep track of gpio states Heiko Stübner
@ 2011-08-29  9:18   ` Mark Brown
  0 siblings, 0 replies; 6+ messages in thread
From: Mark Brown @ 2011-08-29  9:18 UTC (permalink / raw)
  To: Heiko Stübner; +Cc: linux-pm, Liam Girdwood, Philipp Zabel

On Sun, Aug 28, 2011 at 03:09:23PM +0200, Heiko Stübner wrote:
> gpio_get_value is not definied for output-gpios and can therefore not be
> used for the is_enabled and get_current_limit methods of the bq24022.
> 
> This patch introduces a private struct to keep track of the values set.

Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

This also fixes the use of platform data after probe - platform data
should be able to be marked as initdata which may mean that the kernel
discards it after boot.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2011-08-29  9:18 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-28 13:07 [PATCH 0/3] bq24022: Add support for bq2407x family Heiko Stübner
2011-08-28 13:08 ` [PATCH 1/3] bq24022: Evaluate returns of gpio_direction_output-calls Heiko Stübner
2011-08-29  9:16   ` Mark Brown
2011-08-28 13:09 ` [PATCH 2/3] bq24022: Keep track of gpio states Heiko Stübner
2011-08-29  9:18   ` Mark Brown
2011-08-28 13:10 ` [PATCH 3/3] bq24022: Add support for the bq2407x family Heiko Stübner

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.