[Patch V5] drivers: power: Add support for bq24735 charger

[Patch V5] drivers: power: Add support for bq24735 charger

[Rhyland Klein]

From: Darbha Sriharsha <dsriharsha-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

Adds support for the bq24735 charger chipset. The bq24735 is a
high-efficiency, synchronous battery charger.

It allows control of the charging current, input current, and the charger
voltage DAC's through SMBus.

Signed-off-by: Darbha Sriharsha <dsriharsha-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Rhyland Klein <rklein-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
v5:
 - Clarified ti,charge-[current/voltage] and input-current properties in
   documentation

v4:
 - Added GPIO # to dev_err when gpio_request fails, for verbosity.

v3:
*Note: I decided to maintain non-DT support, but tried to do it in the most
clean manner using the suggestions from Thierry Reding.

 - Simplified devicetree bindings for charger current/voltage and input current
 - added optional interrupts property for setting client->irq
 - renamed ti,ac-detect to ti,ac-detect-gpios
 - modeled Kconfig entry after TI BQ24190 entry
 - cleaned up whitespace/sorting review comments
 - moved register/bit definitions to .c file
 - renamed gpio_active_low to status_gpio_active_low
 - added status_gpio_valid to handle gpio = 0 as valid
 - simplified the struct bq24735_charger -> struct bq24735 and removed
   dev and irq fields
 - made bq24735[write/read]_word inline functions with error checking at
   caller sites
 - fixed support for enable/disable and added calls into isr as appropriate
 - moved charger present logic into separate function
 - moved devm_request_threaded_irq after power_supply_register so we can't
   get into the isr with an unregistered power_supply
 - removed ifdeffery around CONFIG_OF, using IS_ENABLED() instead

v2:
 - gpio_request -> devm_gpio_request

 .../bindings/power_supply/ti,bq24735.txt           |   31 ++
 drivers/power/Kconfig                              |    6 +
 drivers/power/Makefile                             |    1 +
 drivers/power/bq24735-charger.c                    |  419 ++++++++++++++++++++
 include/linux/power/bq24735-charger.h              |   39 ++
 5 files changed, 496 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power_supply/ti,bq24735.txt
 create mode 100644 drivers/power/bq24735-charger.c
 create mode 100644 include/linux/power/bq24735-charger.h

diff --git a/Documentation/devicetree/bindings/power_supply/ti,bq24735.txt b/Documentation/devicetree/bindings/power_supply/ti,bq24735.txt
new file mode 100644
index 0000000..e61d5eb
--- /dev/null
+++ b/Documentation/devicetree/bindings/power_supply/ti,bq24735.txt
@@ -0,0 +1,31 @@
+TI BQ24735 Charge Controller
+~~~~~~~~~~
+
+Required properties :
+ - compatible : "ti,bq24735"
+
+Optional properties :
+ - interrupts : Specify the interrupt to be used to trigger when the AC
+   adapter is either plugged in or removed.
+ - ti,ac-detect-gpios : This GPIO is optionally used to read the AC adapter
+   presence.
+ - ti,charge-current : Used to control and set the charging current. This value
+   must be between 128mA and 8.128A with a 64mA step resolution. The POR value
+   is 0x0000h. This number is in mA (e.g. 8192), see spec for more information
+   about the ChargeCurrent (0x14h) register.
+ - ti,charge-voltage : Used to control and set the charging voltage. This value
+   must be between 1.024V and 19.2V with a 16mV step resolution. The POR value
+   is 0x0000h. This number is in mV (e.g. 19200), see spec for more information
+   about the ChargeVoltage (0x15h) register.
+ - ti,input-current : Used to control and set the charger input current. This
+   value must be between 128mA and 8.064A with a 128mA step resolution. The
+   POR value is 0x1000h. This number is in mA (e.g. 8064), see the spec for
+   more information about the InputCurrent (0x3fh) register.
+
+Example:
+
+	bq24735@9 {
+		compatible = "ti,bq24735";
+		reg = <0x9>;
+		ti,ac-detect-gpios = <&gpio 72 0x1>;
+	}
diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index e8970a6..655e8bf 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -340,6 +340,12 @@ config CHARGER_BQ24190
 	help
 	  Say Y to enable support for the TI BQ24190 battery charger.
 
+config CHARGER_BQ24735
+	tristate "TI BQ24735 battery charger support"
+	depends on I2C && GPIOLIB
+	help
+	  Say Y to enable support for the TI BQ24735 battery charger.
+
 config CHARGER_SMB347
 	tristate "Summit Microelectronics SMB347 Battery Charger"
 	depends on I2C
diff --git a/drivers/power/Makefile b/drivers/power/Makefile
index 4ae4533..e65487b 100644
--- a/drivers/power/Makefile
+++ b/drivers/power/Makefile
@@ -51,6 +51,7 @@ obj-$(CONFIG_CHARGER_MAX8997)	+= max8997_charger.o
 obj-$(CONFIG_CHARGER_MAX8998)	+= max8998_charger.o
 obj-$(CONFIG_CHARGER_BQ2415X)	+= bq2415x_charger.o
 obj-$(CONFIG_CHARGER_BQ24190)	+= bq24190_charger.o
+obj-$(CONFIG_CHARGER_BQ24735)	+= bq24735-charger.o
 obj-$(CONFIG_POWER_AVS)		+= avs/
 obj-$(CONFIG_CHARGER_SMB347)	+= smb347-charger.o
 obj-$(CONFIG_CHARGER_TPS65090)	+= tps65090-charger.o
diff --git a/drivers/power/bq24735-charger.c b/drivers/power/bq24735-charger.c
new file mode 100644
index 0000000..d022b82
--- /dev/null
+++ b/drivers/power/bq24735-charger.c
@@ -0,0 +1,419 @@
+/*
+ * Battery charger driver for TI BQ24735
+ *
+ * Copyright (c) 2013, NVIDIA CORPORATION.  All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation;
+ *
+ * This program is distributed in the hope that 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, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+
+#include <linux/err.h>
+#include <linux/gpio.h>
+#include <linux/i2c.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
+#include <linux/power_supply.h>
+#include <linux/slab.h>
+
+#include <linux/power/bq24735-charger.h>
+
+#define BQ24735_CHG_OPT			0x12
+#define BQ24735_CHG_OPT_CHARGE_DISABLE	(1 << 0)
+#define BQ24735_CHG_OPT_AC_PRESENT	(1 << 4)
+#define BQ24735_CHARGE_CURRENT		0x14
+#define BQ24735_CHARGE_CURRENT_MASK	0x1fc0
+#define BQ24735_CHARGE_VOLTAGE		0x15
+#define BQ24735_CHARGE_VOLTAGE_MASK	0x7ff0
+#define BQ24735_INPUT_CURRENT		0x3f
+#define BQ24735_INPUT_CURRENT_MASK	0x1f80
+#define BQ24735_MANUFACTURER_ID		0xfe
+#define BQ24735_DEVICE_ID		0xff
+
+struct bq24735 {
+	struct power_supply	charger;
+	struct i2c_client	*client;
+	struct bq24735_platform	*pdata;
+};
+
+static inline struct bq24735 *to_bq24735(struct power_supply *psy)
+{
+	return container_of(psy, struct bq24735, charger);
+}
+
+static enum power_supply_property bq24735_charger_properties[] = {
+	POWER_SUPPLY_PROP_ONLINE,
+};
+
+static inline int bq24735_write_word(struct i2c_client *client, u8 reg,
+				     u16 value)
+{
+	return i2c_smbus_write_word_data(client, reg, le16_to_cpu(value));
+}
+
+static inline int bq24735_read_word(struct i2c_client *client, u8 reg)
+{
+	s32 ret = i2c_smbus_read_word_data(client, reg);
+
+	return ret < 0 ? ret : le16_to_cpu(ret);
+}
+
+static int bq24735_update_word(struct i2c_client *client, u8 reg,
+			       u16 mask, u16 value)
+{
+	unsigned int tmp;
+	int ret;
+
+	ret = bq24735_read_word(client, reg);
+	if (ret < 0)
+		return ret;
+
+	tmp = ret & ~mask;
+	tmp |= value & mask;
+
+	return bq24735_write_word(client, reg, tmp);
+}
+
+static inline int bq24735_enable_charging(struct bq24735 *charger)
+{
+	return bq24735_update_word(charger->client, BQ24735_CHG_OPT,
+				   BQ24735_CHG_OPT_CHARGE_DISABLE,
+				   ~BQ24735_CHG_OPT_CHARGE_DISABLE);
+}
+
+static inline int bq24735_disable_charging(struct bq24735 *charger)
+{
+	return bq24735_update_word(charger->client, BQ24735_CHG_OPT,
+				   BQ24735_CHG_OPT_CHARGE_DISABLE,
+				   BQ24735_CHG_OPT_CHARGE_DISABLE);
+}
+
+static int bq24735_config_charger(struct bq24735 *charger)
+{
+	struct bq24735_platform *pdata = charger->pdata;
+	int ret;
+	u16 value;
+
+	if (pdata->charge_current) {
+		value = pdata->charge_current & BQ24735_CHARGE_CURRENT_MASK;
+
+		ret = bq24735_write_word(charger->client,
+					 BQ24735_CHARGE_CURRENT, value);
+		if (ret < 0) {
+			dev_err(&charger->client->dev,
+				"Failed to write charger current : %d\n",
+				ret);
+			return ret;
+		}
+	}
+
+	if (pdata->charge_voltage) {
+		value = pdata->charge_voltage & BQ24735_CHARGE_VOLTAGE_MASK;
+
+		ret = bq24735_write_word(charger->client,
+					 BQ24735_CHARGE_VOLTAGE, value);
+		if (ret < 0) {
+			dev_err(&charger->client->dev,
+				"Failed to write charger voltage : %d\n",
+				ret);
+			return ret;
+		}
+	}
+
+	if (pdata->input_current) {
+		value = pdata->input_current & BQ24735_INPUT_CURRENT_MASK;
+
+		ret = bq24735_write_word(charger->client,
+					 BQ24735_INPUT_CURRENT, value);
+		if (ret < 0) {
+			dev_err(&charger->client->dev,
+				"Failed to write input current : %d\n",
+				ret);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static bool bq24735_charger_is_present(struct bq24735 *charger)
+{
+	struct bq24735_platform *pdata = charger->pdata;
+	int ret;
+
+	if (pdata->status_gpio_valid) {
+		ret = gpio_get_value_cansleep(pdata->status_gpio);
+		return ret ^= pdata->status_gpio_active_low == 0;
+	} else {
+		int ac = 0;
+
+		ac = bq24735_read_word(charger->client, BQ24735_CHG_OPT);
+		if (ac < 0) {
+			dev_err(&charger->client->dev,
+				"Failed to read charger options : %d\n",
+				ac);
+			return false;
+		}
+		return (ac & BQ24735_CHG_OPT_AC_PRESENT) ? true : false;
+	}
+
+	return false;
+}
+
+static irqreturn_t bq24735_charger_isr(int irq, void *devid)
+{
+	struct power_supply *psy = devid;
+	struct bq24735 *charger = to_bq24735(psy);
+
+	if (bq24735_charger_is_present(charger))
+		bq24735_enable_charging(charger);
+	else
+		bq24735_disable_charging(charger);
+
+	power_supply_changed(psy);
+
+	return IRQ_HANDLED;
+}
+
+static int bq24735_charger_get_property(struct power_supply *psy,
+					enum power_supply_property psp,
+					union power_supply_propval *val)
+{
+	struct bq24735 *charger;
+
+	charger = container_of(psy, struct bq24735, charger);
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_ONLINE:
+		val->intval = bq24735_charger_is_present(charger) ? 1 : 0;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static struct bq24735_platform *bq24735_parse_dt_data(struct i2c_client *client)
+{
+	struct bq24735_platform *pdata;
+	struct device_node *np = client->dev.of_node;
+	u32 val;
+	int ret;
+	enum of_gpio_flags flags;
+
+	pdata = devm_kzalloc(&client->dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata) {
+		dev_err(&client->dev,
+			"Memory alloc for bq24735 pdata failed\n");
+		return NULL;
+	}
+
+	pdata->status_gpio = of_get_named_gpio_flags(np, "ti,ac-detect-gpios",
+						     0, &flags);
+
+	if (flags & OF_GPIO_ACTIVE_LOW)
+		pdata->status_gpio_active_low = 1;
+
+	ret = of_property_read_u32(np, "ti,charge-current", &val);
+	if (!ret)
+		pdata->charge_current = val;
+
+	ret = of_property_read_u32(np, "ti,charge-voltage", &val);
+	if (!ret)
+		pdata->charge_voltage = val;
+
+	ret = of_property_read_u32(np, "ti,input-current", &val);
+	if (!ret)
+		pdata->input_current = val;
+
+	return pdata;
+}
+
+static int bq24735_charger_probe(struct i2c_client *client,
+				 const struct i2c_device_id *id)
+{
+	int ret;
+	struct bq24735 *charger;
+	struct power_supply *supply;
+	char *name;
+
+	charger = devm_kzalloc(&client->dev, sizeof(*charger), GFP_KERNEL);
+	if (!charger)
+		return -ENOMEM;
+
+	charger->pdata = client->dev.platform_data;
+
+	if (IS_ENABLED(CONFIG_OF) && !charger->pdata && client->dev.of_node)
+		charger->pdata = bq24735_parse_dt_data(client);
+
+	if (!charger->pdata) {
+		dev_err(&client->dev, "no platform data provided\n");
+		return -EINVAL;
+	}
+
+	name = (char *)charger->pdata->name;
+	if (!name) {
+		name = kasprintf(GFP_KERNEL, "bq24735@%s",
+				 dev_name(&client->dev));
+		if (!name) {
+			dev_err(&client->dev, "Failed to alloc device name\n");
+			return -ENOMEM;
+		}
+	}
+
+	charger->client = client;
+
+	supply = &charger->charger;
+
+	supply->name = name;
+	supply->type = POWER_SUPPLY_TYPE_MAINS;
+	supply->properties = bq24735_charger_properties;
+	supply->num_properties = ARRAY_SIZE(bq24735_charger_properties);
+	supply->get_property = bq24735_charger_get_property;
+	supply->supplied_to = charger->pdata->supplied_to;
+	supply->num_supplicants = charger->pdata->num_supplicants;
+	supply->of_node = client->dev.of_node;
+
+	i2c_set_clientdata(client, charger);
+
+	ret = bq24735_read_word(client, BQ24735_MANUFACTURER_ID);
+	if (ret < 0) {
+		dev_err(&client->dev, "Failed to read manufacturer id : %d\n",
+			ret);
+		goto err_free_name;
+	} else if (ret != 0x0040) {
+		dev_err(&client->dev,
+			"manufacturer id mismatch. 0x0040 != 0x%04x\n", ret);
+		ret = -ENODEV;
+		goto err_free_name;
+	}
+
+	ret = bq24735_read_word(client, BQ24735_DEVICE_ID);
+	if (ret < 0) {
+		dev_err(&client->dev, "Failed to read device id : %d\n", ret);
+		goto err_free_name;
+	} else if (ret != 0x000B) {
+		dev_err(&client->dev,
+			"device id mismatch. 0x000b != 0x%04x\n", ret);
+		ret = -ENODEV;
+		goto err_free_name;
+	}
+
+	if (gpio_is_valid(charger->pdata->status_gpio)) {
+		ret = devm_gpio_request(&client->dev,
+					charger->pdata->status_gpio,
+					name);
+		if (ret) {
+			dev_err(&client->dev,
+				"Failed GPIO request for GPIO %d: %d\n",
+				charger->pdata->status_gpio, ret);
+		}
+
+		charger->pdata->status_gpio_valid = !ret;
+	}
+
+	ret = bq24735_config_charger(charger);
+	if (ret < 0) {
+		dev_err(&client->dev, "failed in configuring charger");
+		goto err_free_name;
+	}
+
+	/* check for AC adapter presence */
+	if (bq24735_charger_is_present(charger)) {
+		ret = bq24735_enable_charging(charger);
+		if (ret < 0) {
+			dev_err(&client->dev, "Failed to enable charging\n");
+			goto err_free_name;
+		}
+	}
+
+	ret = power_supply_register(&client->dev, supply);
+	if (ret < 0) {
+		dev_err(&client->dev, "Failed to register power supply: %d\n",
+			ret);
+		goto err_free_name;
+	}
+
+	if (client->irq) {
+		ret = devm_request_threaded_irq(&client->dev, client->irq,
+						NULL, bq24735_charger_isr,
+						IRQF_TRIGGER_RISING |
+						IRQF_TRIGGER_FALLING |
+						IRQF_ONESHOT,
+						supply->name, supply);
+		if (ret) {
+			dev_err(&client->dev,
+				"Unable to register IRQ %d err %d\n",
+				client->irq, ret);
+			goto err_unregister_supply;
+		}
+	}
+
+	return 0;
+err_unregister_supply:
+	power_supply_unregister(supply);
+err_free_name:
+	if (name != charger->pdata->name)
+		kfree(name);
+
+	return ret;
+}
+
+static int bq24735_charger_remove(struct i2c_client *client)
+{
+	struct bq24735 *charger = i2c_get_clientdata(client);
+
+	if (charger->client->irq)
+		devm_free_irq(&charger->client->dev, charger->client->irq,
+			      &charger->charger);
+
+	power_supply_unregister(&charger->charger);
+
+	if (charger->charger.name != charger->pdata->name)
+		kfree(charger->charger.name);
+
+	return 0;
+}
+
+static const struct i2c_device_id bq24735_charger_id[] = {
+	{ "bq24735-charger", 0 },
+	{}
+};
+MODULE_DEVICE_TABLE(i2c, bq24735_charger_id);
+
+static const struct of_device_id bq24735_match_ids[] = {
+	{ .compatible = "ti,bq24735", },
+	{ /* end */ }
+};
+MODULE_DEVICE_TABLE(of, bq24735_match_ids);
+
+static struct i2c_driver bq24735_charger_driver = {
+	.driver = {
+		.name = "bq24735-charger",
+		.owner = THIS_MODULE,
+		.of_match_table = bq24735_match_ids,
+	},
+	.probe = bq24735_charger_probe,
+	.remove = bq24735_charger_remove,
+	.id_table = bq24735_charger_id,
+};
+
+module_i2c_driver(bq24735_charger_driver);
+
+MODULE_DESCRIPTION("bq24735 battery charging driver");
+MODULE_AUTHOR("Darbha Sriharsha <dsriharsha-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/power/bq24735-charger.h b/include/linux/power/bq24735-charger.h
new file mode 100644
index 0000000..f536164
--- /dev/null
+++ b/include/linux/power/bq24735-charger.h
@@ -0,0 +1,39 @@
+/*
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that 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, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#ifndef __CHARGER_BQ24735_H_
+#define __CHARGER_BQ24735_H_
+
+#include <linux/types.h>
+#include <linux/power_supply.h>
+
+struct bq24735_platform {
+	uint32_t charge_current;
+	uint32_t charge_voltage;
+	uint32_t input_current;
+
+	const char *name;
+
+	int status_gpio;
+	int status_gpio_active_low;
+	bool status_gpio_valid;
+
+	char **supplied_to;
+	size_t num_supplicants;
+};
+
+#endif /* __CHARGER_BQ24735_H_ */
-- 
1.7.9.5

--
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 V5] drivers: power: Add support for bq24735 charger

[Stephen Warren]

On 10/11/2013 03:15 PM, Rhyland Klein wrote:
> From: Darbha Sriharsha <dsriharsha-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> 
> Adds support for the bq24735 charger chipset. The bq24735 is a
> high-efficiency, synchronous battery charger.
> 
> It allows control of the charging current, input current, and the charger
> voltage DAC's through SMBus.

Please CC the DT bindings maintainers on patches that add DT bindings.

> diff --git a/Documentation/devicetree/bindings/power_supply/ti,bq24735.txt b/Documentation/devicetree/bindings/power_supply/ti,bq24735.txt

> +Optional properties :

> + - ti,ac-detect-gpios : This GPIO is optionally used to read the AC adapter
> +   presence.

Is that actually a property of the BQ24735 chip itself (i.e. is it an
output signal from the chip), or part of the board/system?

Aside from that, the binding looks reasonable to me (although I'm no
longer a DT bindings maintainer).
--
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 V5] drivers: power: Add support for bq24735 charger

[Rhyland Klein]

On 10/14/2013 2:21 PM, Stephen Warren wrote:
> On 10/11/2013 03:15 PM, Rhyland Klein wrote:
>> From: Darbha Sriharsha <dsriharsha@nvidia.com>
>>
>> Adds support for the bq24735 charger chipset. The bq24735 is a
>> high-efficiency, synchronous battery charger.
>>
>> It allows control of the charging current, input current, and the charger
>> voltage DAC's through SMBus.
> 
> Please CC the DT bindings maintainers on patches that add DT bindings.

Oops, sorry, will do in the future.

> 
>> diff --git a/Documentation/devicetree/bindings/power_supply/ti,bq24735.txt b/Documentation/devicetree/bindings/power_supply/ti,bq24735.txt
> 
>> +Optional properties :
> 
>> + - ti,ac-detect-gpios : This GPIO is optionally used to read the AC adapter
>> +   presence.
> 
> Is that actually a property of the BQ24735 chip itself (i.e. is it an
> output signal from the chip), or part of the board/system?

The gpio itself is actually what the IRQ line is tied to from the
bq24735 -> tegra (in this case). In other words this is a Host GPIO that
is configured as an input and connected to the bq24735.

-rhyland

> 
> Aside from that, the binding looks reasonable to me (although I'm no
> longer a DT bindings maintainer).
> 


-- 
nvpublic

Re: [Patch V5] drivers: power: Add support for bq24735 charger

[Stephen Warren]

On 10/14/2013 12:26 PM, Rhyland Klein wrote:
> On 10/14/2013 2:21 PM, Stephen Warren wrote:
>> On 10/11/2013 03:15 PM, Rhyland Klein wrote:
>>> From: Darbha Sriharsha <dsriharsha-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>>>
>>> Adds support for the bq24735 charger chipset. The bq24735 is a
>>> high-efficiency, synchronous battery charger.
>>>
>>> It allows control of the charging current, input current, and the charger
>>> voltage DAC's through SMBus.
>>
>> Please CC the DT bindings maintainers on patches that add DT bindings.
> 
> Oops, sorry, will do in the future.
> 
>>
>>> diff --git a/Documentation/devicetree/bindings/power_supply/ti,bq24735.txt b/Documentation/devicetree/bindings/power_supply/ti,bq24735.txt
>>
>>> +Optional properties :
>>
>>> + - ti,ac-detect-gpios : This GPIO is optionally used to read the AC adapter
>>> +   presence.
>>
>> Is that actually a property of the BQ24735 chip itself (i.e. is it an
>> output signal from the chip), or part of the board/system?
> 
> The gpio itself is actually what the IRQ line is tied to from the
> bq24735 -> tegra (in this case). In other words this is a Host GPIO that
> is configured as an input and connected to the bq24735.

Ok, so this is the ACOK output pin? In that case, it's fine. It might be
worth mentioning that explicitly though.
--
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 V5] drivers: power: Add support for bq24735 charger

[Anton Vorontsov]

On Mon, Oct 14, 2013 at 12:40:26PM -0600, Stephen Warren wrote:
> >>> diff --git a/Documentation/devicetree/bindings/power_supply/ti,bq24735.txt b/Documentation/devicetree/bindings/power_supply/ti,bq24735.txt
> >>
> >>> +Optional properties :
> >>
> >>> + - ti,ac-detect-gpios : This GPIO is optionally used to read the AC adapter
> >>> +   presence.
> >>
> >> Is that actually a property of the BQ24735 chip itself (i.e. is it an
> >> output signal from the chip), or part of the board/system?
> > 
> > The gpio itself is actually what the IRQ line is tied to from the
> > bq24735 -> tegra (in this case). In other words this is a Host GPIO that
> > is configured as an input and connected to the bq24735.
> 
> Ok, so this is the ACOK output pin? In that case, it's fine. It might be
> worth mentioning that explicitly though.

I added the comment to the documentation and applied the patch with a
"Thanks-to: Stephen Warren <swarren@wwwdotorg.org>" tag. :)

Anton

Re: [Patch V5] drivers: power: Add support for bq24735 charger

[Anton Vorontsov]

On Fri, Oct 25, 2013 at 03:48:41PM -0700, Anton Vorontsov wrote:
> On Mon, Oct 14, 2013 at 12:40:26PM -0600, Stephen Warren wrote:
> > >>> diff --git a/Documentation/devicetree/bindings/power_supply/ti,bq24735.txt b/Documentation/devicetree/bindings/power_supply/ti,bq24735.txt
> > >>
> > >>> +Optional properties :
> > >>
> > >>> + - ti,ac-detect-gpios : This GPIO is optionally used to read the AC adapter
> > >>> +   presence.
> > >>
> > >> Is that actually a property of the BQ24735 chip itself (i.e. is it an
> > >> output signal from the chip), or part of the board/system?
> > > 
> > > The gpio itself is actually what the IRQ line is tied to from the
> > > bq24735 -> tegra (in this case). In other words this is a Host GPIO that
> > > is configured as an input and connected to the bq24735.
> > 
> > Ok, so this is the ACOK output pin? In that case, it's fine. It might be
> > worth mentioning that explicitly though.
> 
> I added the comment to the documentation and applied the patch with a
> "Thanks-to: Stephen Warren <swarren@wwwdotorg.org>" tag. :)

Oh, and as well as

Thanks-to: Thierry Reding <thierry.reding@gmail.com>

(People seem to review drivers, but do not give their Reviewed-by tags, so
the thanks tag is the best thing I can legally do in such a case.)

Thanks,

Anton