[PATCH 0/3] Add support for tps65217 charger

[PATCH 0/3] Add support for tps65217 charger

[Enric Balletbo i Serra]

Hi all,

The following series add initial support for tps65217 battery charger. This
series is a first attempt and will have mistake so all comments and
suggestions are welcomed.

Best regards,

Enric Balletbo i Serra (3):
  devicetree: Add TPS65217 charger binding.
  power_supply: Add support for tps65217-charger.
  mfd: Add battery charger as subdevice to the tps65217.

 .../bindings/power_supply/tps65217_charger.txt     |  12 +
 drivers/mfd/tps65217.c                             |   4 +
 drivers/power/Kconfig                              |   7 +
 drivers/power/Makefile                             |   1 +
 drivers/power/tps65217_charger.c                   | 269 +++++++++++++++++++++
 5 files changed, 293 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power_supply/tps65217_charger.txt
 create mode 100644 drivers/power/tps65217_charger.c

-- 
2.1.0

[PATCH 1/3] devicetree: Add TPS65217 charger binding.

[Enric Balletbo i Serra]

The TPS65217 charger is a subnode of the TPS65217 MFD.

Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
 .../devicetree/bindings/power_supply/tps65217_charger.txt    | 12 ++++++++++++
 1 file changed, 12 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power_supply/tps65217_charger.txt

diff --git a/Documentation/devicetree/bindings/power_supply/tps65217_charger.txt b/Documentation/devicetree/bindings/power_supply/tps65217_charger.txt
new file mode 100644
index 0000000..98d131a
--- /dev/null
+++ b/Documentation/devicetree/bindings/power_supply/tps65217_charger.txt
@@ -0,0 +1,12 @@
+TPS65217 Charger
+
+Required Properties:
+-compatible: "ti,tps65217-charger"
+
+This node is a subnode of the tps65217 PMIC.
+
+Example:
+
+	tps65217-charger {
+		compatible = "ti,tps65090-charger";
+	};
-- 
2.1.0

[PATCH 2/3] power_supply: Add support for tps65217-charger.

[Enric Balletbo i Serra]

This patch adds support for the tps65217 charger driver. This driver is
responsible for controlling the charger aspect of the tps65217 mfd.
Currently, this mainly consists of turning on and off the charger, but
some other features of the charger can be supported through this driver.

Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
 drivers/power/Kconfig            |   7 +
 drivers/power/Makefile           |   1 +
 drivers/power/tps65217_charger.c | 269 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 277 insertions(+)
 create mode 100644 drivers/power/tps65217_charger.c

diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index f8758d6..57fdc80 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -434,6 +434,13 @@ config CHARGER_TPS65090
 	 Say Y here to enable support for battery charging with TPS65090
 	 PMIC chips.
 
+config CHARGER_TPS65217
+	tristate "TPS65217 battery charger driver"
+	depends on MFD_TPS65217
+	help
+	 Say Y here to enable support for battery charging with TPS65217
+	 PMIC chips.
+
 config BATTERY_GAUGE_LTC2941
 	tristate "LTC2941/LTC2943 Battery Gauge Driver"
 	depends on I2C
diff --git a/drivers/power/Makefile b/drivers/power/Makefile
index 5752ce8..c1409b3 100644
--- a/drivers/power/Makefile
+++ b/drivers/power/Makefile
@@ -65,6 +65,7 @@ obj-$(CONFIG_CHARGER_BQ25890)	+= bq25890_charger.o
 obj-$(CONFIG_POWER_AVS)		+= avs/
 obj-$(CONFIG_CHARGER_SMB347)	+= smb347-charger.o
 obj-$(CONFIG_CHARGER_TPS65090)	+= tps65090-charger.o
+obj-$(CONFIG_CHARGER_TPS65217)	+= tps65217_charger.o
 obj-$(CONFIG_POWER_RESET)	+= reset/
 obj-$(CONFIG_AXP288_FUEL_GAUGE) += axp288_fuel_gauge.o
 obj-$(CONFIG_AXP288_CHARGER)	+= axp288_charger.o
diff --git a/drivers/power/tps65217_charger.c b/drivers/power/tps65217_charger.c
new file mode 100644
index 0000000..0b6a30d
--- /dev/null
+++ b/drivers/power/tps65217_charger.c
@@ -0,0 +1,269 @@
+/*
+ * Battery charger driver for TI's tps65217
+ *
+ * Copyright (c) 2015, Collabora Ltd.
+
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+/*
+ * Battery charger driver for TI's tps65217
+ */
+#include <linux/kernel.h>
+#include <linux/kthread.h>
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/slab.h>
+#include <linux/err.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/power_supply.h>
+
+#include <linux/mfd/core.h>
+#include <linux/mfd/tps65217.h>
+
+#define POLL_INTERVAL		(HZ * 2)
+
+struct tps65217_charger {
+	struct tps65217 *tps;
+	struct device *dev;
+	struct power_supply *ac;
+
+	int	ac_online;
+	int	prev_ac_online;
+
+	struct task_struct	*poll_task;
+};
+
+static enum power_supply_property tps65217_ac_props[] = {
+	POWER_SUPPLY_PROP_ONLINE,
+};
+
+static int tps65217_config_charger(struct tps65217_charger *charger)
+{
+	int ret;
+
+	dev_dbg(charger->dev, "%s\n", __func__);
+
+	/*
+	 * tps65217 rev. G, p. 31 (see p. 32 for NTC schematic)
+	 *
+	 * The device can be configured to support a 100k NTC (B = 3960) by
+	 * setting the the NTC_TYPE bit in register CHGCONFIG1 to 1. However it
+	 * is not recommended to do so. In sleep mode, the charger continues
+	 * charging the battery, but all register values are reset to default
+	 * values. Therefore, the charger would get the wrong temperature
+	 * information. If 100k NTC setting is required, please contact the
+	 * factory.
+	 *
+	 * ATTENTION, conflicting information, from p. 46
+	 *
+	 * NTC TYPE (for battery temperature measurement)
+	 *   0 – 100k (curve 1, B = 3960)
+	 *   1 – 10k  (curve 2, B = 3480) (default on reset)
+	 *
+	 */
+	ret = tps65217_clear_bits(charger->tps, TPS65217_REG_CHGCONFIG1,
+			TPS65217_CHGCONFIG1_NTC_TYPE,
+			TPS65217_PROTECT_NONE);
+	if (ret) {
+		dev_err(charger->dev,
+			"failed to set 100k NTC setting: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int tps65217_enable_charging(struct tps65217_charger *charger)
+{
+	int ret;
+
+	/* charger already enabled */
+	if (charger->ac_online)
+		return 0;
+
+	dev_dbg(charger->dev, "%s: enable charging\n", __func__);
+	ret = tps65217_set_bits(charger->tps, TPS65217_REG_CHGCONFIG1,
+			TPS65217_CHGCONFIG1_CHG_EN,
+			TPS65217_CHGCONFIG1_CHG_EN,
+			TPS65217_PROTECT_NONE);
+	if (ret) {
+		dev_err(charger->dev,
+			"%s: Error in writing CHG_EN in reg 0x%x: %d\n",
+			__func__, TPS65217_REG_CHGCONFIG1, ret);
+		return ret;
+	}
+
+	charger->ac_online = 1;
+
+	return 0;
+}
+
+static int tps65217_ac_get_property(struct power_supply *psy,
+			enum power_supply_property psp,
+			union power_supply_propval *val)
+{
+	struct tps65217_charger *charger = power_supply_get_drvdata(psy);
+
+	if (psp == POWER_SUPPLY_PROP_ONLINE) {
+		val->intval = charger->ac_online;
+		charger->prev_ac_online = charger->ac_online;
+		return 0;
+	}
+	return -EINVAL;
+}
+
+static irqreturn_t tps65217_charger_irq(int irq, void *dev)
+{
+	int ret, val;
+	struct tps65217_charger *charger = dev;
+
+	ret = tps65217_reg_read(charger->tps, TPS65217_REG_STATUS, &val);
+	if (ret < 0) {
+		dev_err(charger->dev, "%s: Error in reading reg 0x%x\n",
+				__func__, TPS65217_REG_STATUS);
+		return IRQ_HANDLED;
+	}
+
+	dev_dbg(charger->dev, "%s: 0x%x\n", __func__, val);
+
+	/* check for AC status bit */
+	if (val & TPS65217_STATUS_ACPWR) {
+		ret = tps65217_enable_charging(charger);
+		if (ret) {
+			dev_err(charger->dev,
+				"failed to enable charger: %d\n", ret);
+			return IRQ_HANDLED;
+		}
+	} else
+		charger->ac_online = 0;
+
+	if (charger->prev_ac_online != charger->ac_online)
+		power_supply_changed(charger->ac);
+
+
+	ret = tps65217_reg_read(charger->tps, TPS65217_REG_CHGCONFIG0, &val);
+	if (ret < 0) {
+		dev_err(charger->dev, "%s: Error in reading reg 0x%x\n",
+				__func__, TPS65217_REG_CHGCONFIG0);
+		return IRQ_HANDLED;
+	}
+
+	if (val & TPS65217_CHGCONFIG0_ACTIVE)
+		dev_dbg(charger->dev, "%s: charger is charging\n", __func__);
+	else
+		dev_dbg(charger->dev,
+			"%s: charger is NOT charging\n", __func__);
+
+	return IRQ_HANDLED;
+}
+
+static int tps65217_charger_poll_task(void *data)
+{
+	set_freezable();
+
+	while (!kthread_should_stop()) {
+		schedule_timeout_interruptible(POLL_INTERVAL);
+		try_to_freeze();
+		tps65217_charger_irq(-1, data);
+	}
+	return 0;
+}
+
+static const struct power_supply_desc tps65217_charger_desc = {
+	.name			= "tps65217-ac",
+	.type			= POWER_SUPPLY_TYPE_MAINS,
+	.get_property		= tps65217_ac_get_property,
+	.properties		= tps65217_ac_props,
+	.num_properties		= ARRAY_SIZE(tps65217_ac_props),
+};
+
+static int tps65217_charger_probe(struct platform_device *pdev)
+{
+	struct tps65217 *tps = dev_get_drvdata(pdev->dev.parent);
+	struct tps65217_charger *charger;
+	int ret;
+
+	dev_dbg(&pdev->dev, "%s\n", __func__);
+
+	charger = devm_kzalloc(&pdev->dev, sizeof(*charger), GFP_KERNEL);
+	if (charger == NULL)
+		return -ENOMEM;
+
+	charger->tps = tps;
+	charger->dev = &pdev->dev;
+
+	charger->ac = power_supply_register(&pdev->dev,
+					&tps65217_charger_desc, NULL);
+	if (IS_ERR(charger->ac)) {
+		dev_err(&pdev->dev, "failed: power supply register\n");
+		return PTR_ERR(charger->ac);
+	}
+
+	ret = tps65217_config_charger(charger);
+	if (ret < 0) {
+		dev_err(charger->dev, "charger config failed, err %d\n", ret);
+		goto fail_unregister_supply;
+	}
+
+	charger->poll_task = kthread_run(tps65217_charger_poll_task,
+				      charger, "ktps65217charger");
+	if (IS_ERR(charger->poll_task)) {
+		ret = PTR_ERR(charger->poll_task);
+		dev_err(charger->dev, "Unable to run kthread err %d\n", ret);
+		goto fail_unregister_supply;
+	}
+
+	return 0;
+
+fail_unregister_supply:
+	power_supply_unregister(charger->ac);
+
+	return ret;
+}
+
+static int tps65217_charger_remove(struct platform_device *pdev)
+{
+	struct tps65217_charger *charger = platform_get_drvdata(pdev);
+
+	kthread_stop(charger->poll_task);
+
+	power_supply_unregister(charger->ac);
+
+	return 0;
+}
+
+static const struct of_device_id tps65217_charger_match_table[] = {
+	{ .compatible = "ti,tps65217-charger", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, tps65217_charger_match_table);
+
+static struct platform_driver tps65217_charger_driver = {
+	.probe	= tps65217_charger_probe,
+	.remove = tps65217_charger_remove,
+	.driver	= {
+		.name	= "tps65217-charger",
+		.of_match_table = of_match_ptr(tps65217_charger_match_table),
+	},
+
+};
+module_platform_driver(tps65217_charger_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Enric Balletbo Serra <enric.balletbo@collabora.com>");
+MODULE_DESCRIPTION("TPS65217 battery charger driver");
-- 
2.1.0

Re: [PATCH 2/3] power_supply: Add support for tps65217-charger.

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1283 bytes --]

Hi,

On Tue, Sep 08, 2015 at 10:09:38AM +0200, Enric Balletbo i Serra wrote:
> This patch adds support for the tps65217 charger driver. This driver is
> responsible for controlling the charger aspect of the tps65217 mfd.
> Currently, this mainly consists of turning on and off the charger, but
> some other features of the charger can be supported through this driver.

Driver looks mostly fine. I have two comments though:

> [...]
>
> +static int tps65217_ac_get_property(struct power_supply *psy,
> +			enum power_supply_property psp,
> +			union power_supply_propval *val)
> +{
> +	struct tps65217_charger *charger = power_supply_get_drvdata(psy);
> +
> +	if (psp == POWER_SUPPLY_PROP_ONLINE) {
> +		val->intval = charger->ac_online;
> +		charger->prev_ac_online = charger->ac_online;

I think prev_ac_online should be set at the beginning of
tps65217_charger_irq().

> [...]
>
> +static int tps65217_charger_probe(struct platform_device *pdev)
> +{
>
> [...]
>
> +	charger->ac = power_supply_register(&pdev->dev,
> +					&tps65217_charger_desc, NULL);
> +	if (IS_ERR(charger->ac)) {
> +		dev_err(&pdev->dev, "failed: power supply register\n");
> +		return PTR_ERR(charger->ac);
> +	}

You can use devm_power_supply_register(...) to simplify the driver
a bit more.

-- Sebastian

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH 3/3] mfd: Add battery charger as subdevice to the tps65217.

[Enric Balletbo i Serra]

Add tps65217 battery charger subdevice.

Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
 drivers/mfd/tps65217.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/mfd/tps65217.c b/drivers/mfd/tps65217.c
index 55add04..d32b5442 100644
--- a/drivers/mfd/tps65217.c
+++ b/drivers/mfd/tps65217.c
@@ -39,6 +39,10 @@ static const struct mfd_cell tps65217s[] = {
 		.name = "tps65217-bl",
 		.of_compatible = "ti,tps65217-bl",
 	},
+	{
+		.name = "tps65217-charger",
+		.of_compatible = "ti,tps65217-charger",
+	},
 };
 
 /**
-- 
2.1.0

Re: [PATCH 3/3] mfd: Add battery charger as subdevice to the tps65217.

[Lee Jones]

On Tue, 08 Sep 2015, Enric Balletbo i Serra wrote:

> Add tps65217 battery charger subdevice.
> 
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---
>  drivers/mfd/tps65217.c | 4 ++++
>  1 file changed, 4 insertions(+)

Applied, thanks.

> diff --git a/drivers/mfd/tps65217.c b/drivers/mfd/tps65217.c
> index 55add04..d32b5442 100644
> --- a/drivers/mfd/tps65217.c
> +++ b/drivers/mfd/tps65217.c
> @@ -39,6 +39,10 @@ static const struct mfd_cell tps65217s[] = {
>  		.name = "tps65217-bl",
>  		.of_compatible = "ti,tps65217-bl",
>  	},
> +	{
> +		.name = "tps65217-charger",
> +		.of_compatible = "ti,tps65217-charger",
> +	},
>  };
>  
>  /**

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 3/3] mfd: Add battery charger as subdevice to the tps65217.

[Enric Balletbo Serra]

2015-09-20 6:19 GMT+02:00 Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>:
> On Tue, 08 Sep 2015, Enric Balletbo i Serra wrote:
>
>> Add tps65217 battery charger subdevice.
>>
>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
>> ---
>>  drivers/mfd/tps65217.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>
> Applied, thanks.
>

Many thanks, any feedback about ?

[PATCH 1/3] devicetree: Add TPS65217 charger binding
[PATCH 2/3] power_supply: Add support for tps65217-charger.

Or should go through another tree ?



>> diff --git a/drivers/mfd/tps65217.c b/drivers/mfd/tps65217.c
>> index 55add04..d32b5442 100644
>> --- a/drivers/mfd/tps65217.c
>> +++ b/drivers/mfd/tps65217.c
>> @@ -39,6 +39,10 @@ static const struct mfd_cell tps65217s[] = {
>>               .name = "tps65217-bl",
>>               .of_compatible = "ti,tps65217-bl",
>>       },
>> +     {
>> +             .name = "tps65217-charger",
>> +             .of_compatible = "ti,tps65217-charger",
>> +     },
>>  };
>>
>>  /**
>
> --
> Lee Jones
> Linaro STMicroelectronics Landing Team Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] mfd: Add battery charger as subdevice to the tps65217.

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 835 bytes --]

Hi,

On Tue, Sep 22, 2015 at 09:43:50AM +0200, Enric Balletbo Serra wrote:
> 2015-09-20 6:19 GMT+02:00 Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>:
> > On Tue, 08 Sep 2015, Enric Balletbo i Serra wrote:
> >
> >> Add tps65217 battery charger subdevice.
> >>
> >> Signed-off-by: Enric Balletbo i Serra <enric.balletbo-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
> >> ---
> >>  drivers/mfd/tps65217.c | 4 ++++
> >>  1 file changed, 4 insertions(+)
> >
> > Applied, thanks.
> >
> 
> Many thanks, any feedback about ?
> 
> [PATCH 1/3] devicetree: Add TPS65217 charger binding
> [PATCH 2/3] power_supply: Add support for tps65217-charger.
> 
> Or should go through another tree ?

Those go through my tree. I'm currently catching up with my backlog.
I will have a look at these later.

-- Sebastian

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]