[PATCH 1/2] dt-bindings: power: Add the BQ27561 fuel gauge bindings

[PATCH 1/2] dt-bindings: power: Add the BQ27561 fuel gauge bindings

[Dan Murphy]

Introduce the Texas Instrument BQ27561 and BQ27750 fuel gauge device
tree bindings.

Cc: Rob Herring <robh@kernel.org>
Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
 .../bindings/power/supply/bq27561.yaml        | 46 +++++++++++++++++++
 1 file changed, 46 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/supply/bq27561.yaml

diff --git a/Documentation/devicetree/bindings/power/supply/bq27561.yaml b/Documentation/devicetree/bindings/power/supply/bq27561.yaml
new file mode 100644
index 000000000000..abc9acdbc704
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/supply/bq27561.yaml
@@ -0,0 +1,46 @@
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (C) 2020 Texas Instruments Incorporated
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/power/supply/bq27561.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: TI BQ27561 and BQ27750 Fuel Gauges
+
+maintainers:
+  - Dan Murphy <dmurphy@ti.com>
+
+description: |
+  The bq27z561 device provides a feature-rich gas gauging solution for
+  single-cell battery pack applications.
+
+  Specifications about the charger can be found at:
+    https://www.ti.com/lit/gpn/bq27z561
+    https://www.ti.com/lit/gpn/bq27750
+
+properties:
+  compatible:
+    enum:
+      - ti,bq27561
+      - ti,bq27750
+
+  reg:
+    maxItems: 1
+    description: |
+      I2C address of the device which is 0x55
+
+required:
+  - compatible
+  - reg
+additionalProperties: false
+
+examples:
+  - |
+    i2c0 {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      fuel_gauge@55 {
+        compatible = "ti,bq27561";
+        reg = <0x55>;
+      };
+    };
-- 
2.25.1

[PATCH 2/2] power: supply: Introduce the BQ27561 Fuel gauge driver

[Dan Murphy]

Introduce the BQ27561 Fuel gauge driver from Texas Instruments.  The
driver also supports the BQ27750 as it has the same register map and
bit mask.

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
 drivers/power/supply/Kconfig      |   9 +
 drivers/power/supply/Makefile     |   1 +
 drivers/power/supply/bq27561_fg.c | 304 ++++++++++++++++++++++++++++++
 3 files changed, 314 insertions(+)
 create mode 100644 drivers/power/supply/bq27561_fg.c

diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index f3424fdce341..febf6bf8c4ea 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -597,6 +597,15 @@ config CHARGER_BQ25890
 	help
 	  Say Y to enable support for the TI BQ25890 battery charger.
 
+config FUEL_GAUGE_BQ27561
+	tristate "Texas Instruments bq27561 fuel gauge driver"
+	depends on I2C
+	select REGMAP_I2C
+	depends on GPIOLIB || COMPILE_TEST
+	help
+	 Say Y here to enable support for fuel gauge with TI bq27561
+	 fuel gauge chips.
+
 config CHARGER_SMB347
 	tristate "Summit Microelectronics SMB347 Battery Charger"
 	depends on I2C
diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
index 6c7da920ea83..4e402292b276 100644
--- a/drivers/power/supply/Makefile
+++ b/drivers/power/supply/Makefile
@@ -81,6 +81,7 @@ obj-$(CONFIG_CHARGER_BQ24190)	+= bq24190_charger.o
 obj-$(CONFIG_CHARGER_BQ24257)	+= bq24257_charger.o
 obj-$(CONFIG_CHARGER_BQ24735)	+= bq24735-charger.o
 obj-$(CONFIG_CHARGER_BQ25890)	+= bq25890_charger.o
+obj-$(CONFIG_FUEL_GAUGE_BQ27561)	+= bq27561_fg.o
 obj-$(CONFIG_CHARGER_SMB347)	+= smb347-charger.o
 obj-$(CONFIG_CHARGER_TPS65090)	+= tps65090-charger.o
 obj-$(CONFIG_CHARGER_TPS65217)	+= tps65217_charger.o
diff --git a/drivers/power/supply/bq27561_fg.c b/drivers/power/supply/bq27561_fg.c
new file mode 100644
index 000000000000..b9f7cdc00d2e
--- /dev/null
+++ b/drivers/power/supply/bq27561_fg.c
@@ -0,0 +1,304 @@
+// SPDX-License-Identifier: GPL-2.0
+// BQ27561 Fuel Gauge Driver
+// Copyright (C) 2020 Texas Instruments Incorporated - http://www.ti.com/
+// Author: Dan Murphy <dmurphy@ti.com>
+
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/power_supply.h>
+#include <linux/types.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/moduleparam.h>
+#include <linux/slab.h>
+#include <linux/of.h>
+
+#define BQ27561_REG_CNTL	0x00
+#define BQ27561_REG_AR		0x02
+#define BQ27561_REG_TEMP	0x06
+#define BQ27561_REG_VOLT	0x08
+#define BQ27561_REG_REMAIN_CAP	0x10
+#define BQ27561_REG_BATTSTAT	0x0a
+#define BQ27561_REG_CURRENT	0x0c
+#define BQ27561_REG_FCC		0x12
+#define BQ27561_REG_AVG_CURRENT	0x14
+#define BQ27561_REG_TTE		0x16
+#define BQ27561_REG_TTF		0x18
+#define BQ27561_REG_CYCLECNT	0x2a
+#define BQ27561_REG_RSOC	0x2c
+
+#define BQ27561_VAL_MULTIPLIER	1000
+#define BQ27561_TEMP_K_TO_C	2732
+#define BQ27561_TIME_MIN_TO_S	60
+
+#define BQ27561_BATT_DEPLETED	BIT(4)
+#define BQ27561_BATT_CHARGED	BIT(5)
+#define BQ27561_BATT_DISCHARGING	BIT(6)
+
+enum {
+	BQ27561,
+	BQ27750,
+};
+
+struct bq27561_device {
+	struct device *dev;
+	struct i2c_client *client;
+	struct power_supply *battery;
+};
+
+static enum power_supply_property bq27561_properties[] = {
+	POWER_SUPPLY_PROP_STATUS,
+	POWER_SUPPLY_PROP_CAPACITY,
+	POWER_SUPPLY_PROP_TEMP,
+	POWER_SUPPLY_PROP_VOLTAGE_NOW,
+	POWER_SUPPLY_PROP_CURRENT_NOW,
+	POWER_SUPPLY_PROP_CHARGE_NOW,
+	POWER_SUPPLY_PROP_CHARGE_FULL,
+	POWER_SUPPLY_PROP_TIME_TO_EMPTY_NOW,
+};
+
+static int bq27561_get_temp(struct bq27561_device *bq)
+{
+	unsigned int temp;
+
+	temp = i2c_smbus_read_word_data(bq->client, BQ27561_REG_TEMP);
+	if (temp < 0)
+		return temp;
+
+	return temp - BQ27561_TEMP_K_TO_C;
+}
+
+static int bq27561_get_voltage(struct bq27561_device *bq)
+{
+	unsigned int batt_volt;
+
+	batt_volt = i2c_smbus_read_word_data(bq->client, BQ27561_REG_VOLT);
+	if (batt_volt < 0)
+		return batt_volt;
+
+	return batt_volt * BQ27561_VAL_MULTIPLIER;
+}
+
+static int bq27561_get_current(struct bq27561_device *bq)
+{
+	unsigned int batt_curr;
+
+	batt_curr = i2c_smbus_read_word_data(bq->client, BQ27561_REG_CURRENT);
+	if (batt_curr < 0)
+		return batt_curr;
+
+	return batt_curr * BQ27561_VAL_MULTIPLIER;
+}
+
+static int bq27561_get_charge_capacity(struct bq27561_device *bq)
+{
+	unsigned int charge_cap;
+
+	charge_cap = i2c_smbus_read_word_data(bq->client, BQ27561_REG_FCC);
+	if (charge_cap < 0)
+		return charge_cap;
+
+	return charge_cap * BQ27561_VAL_MULTIPLIER;
+}
+
+static int bq27561_get_batt_status(struct bq27561_device *bq)
+{
+	unsigned int batt_stat;
+	unsigned int curr;
+	int ret;
+
+	batt_stat = i2c_smbus_read_word_data(bq->client, BQ27561_REG_BATTSTAT);
+	if (batt_stat < 0)
+		return batt_stat;
+
+	curr = i2c_smbus_read_word_data(bq->client, BQ27561_REG_CURRENT);
+	if (curr < 0)
+		return curr;
+
+	if (batt_stat & BQ27561_BATT_CHARGED)
+		ret = POWER_SUPPLY_STATUS_FULL;
+	else if (batt_stat & BQ27561_BATT_DISCHARGING || curr == 0)
+		ret = POWER_SUPPLY_STATUS_DISCHARGING;
+	else if (curr > 0)
+		ret = POWER_SUPPLY_STATUS_CHARGING;
+	else
+		ret = POWER_SUPPLY_STATUS_UNKNOWN;
+
+	return ret;
+}
+
+static int bq27561_get_empty_time(struct bq27561_device *bq)
+{
+	unsigned int empty_time;
+
+	empty_time = i2c_smbus_read_word_data(bq->client, BQ27561_REG_TTE);
+	if (empty_time < 0)
+		return empty_time;
+
+	return empty_time * BQ27561_TIME_MIN_TO_S;
+}
+
+static int bq27561_get_property(struct power_supply *psy,
+				enum power_supply_property psp,
+				union power_supply_propval *val)
+{
+	struct bq27561_device *bq = power_supply_get_drvdata(psy);
+	int ret = 0;
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+		ret = bq27561_get_voltage(bq);
+		if (ret < 0)
+			return ret;
+
+		val->intval = ret;
+		break;
+	case POWER_SUPPLY_PROP_CURRENT_NOW:
+		ret = bq27561_get_current(bq);
+		if (ret < 0)
+			return ret;
+
+		val->intval = ret;
+		break;
+	case POWER_SUPPLY_PROP_STATUS:
+		ret = bq27561_get_batt_status(bq);
+		if (ret < 0)
+			return ret;
+
+		val->intval = ret;
+		break;
+	case POWER_SUPPLY_PROP_TEMP:
+		val->intval = bq27561_get_temp(bq);
+		break;
+	case POWER_SUPPLY_PROP_CAPACITY:
+		ret = i2c_smbus_read_word_data(bq->client, BQ27561_REG_RSOC);
+		if (ret < 0)
+			return ret;
+
+		val->intval = ret;
+		break;
+	case POWER_SUPPLY_PROP_CHARGE_FULL:
+		ret = bq27561_get_charge_capacity(bq);
+		if (ret < 0)
+			return ret;
+
+		val->intval = ret;
+		break;
+	case POWER_SUPPLY_PROP_TIME_TO_EMPTY_NOW:
+		ret = bq27561_get_empty_time(bq);
+		if (ret < 0)
+			return ret;
+
+		val->intval = ret;
+		break;
+	case POWER_SUPPLY_PROP_CHARGE_NOW:
+		ret = i2c_smbus_read_word_data(bq->client, BQ27561_REG_AR);
+		if (ret < 0)
+			return ret;
+
+		val->intval = ret;
+		break;
+	default:
+		return -EINVAL;
+	};
+
+	return 0;
+}
+
+static int bq27561_set_property(struct power_supply *psy,
+				enum power_supply_property psp,
+				const union power_supply_propval *val)
+{
+	struct bq27561_device *bq = power_supply_get_drvdata(psy);
+	int ret = 0;
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_CHARGE_NOW:
+		ret = i2c_smbus_write_word_data(bq->client, BQ27561_REG_AR,
+						val->intval);
+		break;
+	default:
+		return -EINVAL;
+	};
+
+	return ret;
+}
+
+static int bq27561_prop_is_writeable(struct power_supply *psy,
+				     enum power_supply_property prop)
+{
+	switch (prop) {
+	case POWER_SUPPLY_PROP_CHARGE_NOW:
+		return 1;
+	default:
+		return 0;
+	};
+}
+
+static struct power_supply_desc bq27561_power_supply_desc = {
+	.type = POWER_SUPPLY_TYPE_BATTERY,
+	.properties = bq27561_properties,
+	.num_properties = ARRAY_SIZE(bq27561_properties),
+	.get_property = bq27561_get_property,
+	.set_property = bq27561_set_property,
+	.property_is_writeable = bq27561_prop_is_writeable,
+};
+
+static int bq27561_probe(struct i2c_client *client,
+				 const struct i2c_device_id *id)
+{
+	struct bq27561_device *bq;
+	struct power_supply_config psy_cfg;
+
+	bq = devm_kzalloc(&client->dev, sizeof(*bq), GFP_KERNEL);
+	if (!bq)
+		return -ENOMEM;
+
+	bq->dev = &client->dev;
+	bq->client = client;
+	i2c_set_clientdata(client, bq);
+
+	psy_cfg.drv_data = bq;
+	bq27561_power_supply_desc.name = id->name;
+	bq->battery = devm_power_supply_register(bq->dev,
+						 &bq27561_power_supply_desc,
+						 &psy_cfg);
+
+	if (IS_ERR(bq->battery))
+		return -EINVAL;
+
+	return 0;
+}
+
+static const struct of_device_id bq27561_match_table[] = {
+	{.compatible = "ti,bq27561",},
+	{.compatible = "ti,bq27750",},
+	{},
+};
+MODULE_DEVICE_TABLE(of, bq_fg_match_table);
+
+static const struct i2c_device_id bq27561_id[] = {
+	{"bq27561", BQ27561},
+	{"bq27750", BQ27750},
+	{},
+};
+MODULE_DEVICE_TABLE(i2c, bq27561_id);
+
+static struct i2c_driver bq27561_battery_driver = {
+	.driver = {
+		   .name = "bq27561-fuel-gauge",
+		   .of_match_table = bq27561_match_table,
+		   .owner = THIS_MODULE,
+		   },
+	.probe = bq27561_probe,
+	.id_table = bq27561_id,
+};
+module_i2c_driver(bq27561_battery_driver);
+
+MODULE_AUTHOR("Texas Instruments");
+MODULE_DESCRIPTION("bq27561 battery monitor driver");
+MODULE_LICENSE("GPL v2");
-- 
2.25.1

Re: [PATCH 2/2] power: supply: Introduce the BQ27561 Fuel gauge driver

[Sebastian Reichel]

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

Hi,

On Fri, Apr 17, 2020 at 12:22:27PM -0500, Dan Murphy wrote:
> Introduce the BQ27561 Fuel gauge driver from Texas Instruments. The
> driver also supports the BQ27750 as it has the same register map and
> bit mask.

Why a separate driver instead of adding support to bq27xxx? The register
map seems to be similar enough.

> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
>  drivers/power/supply/Kconfig      |   9 +
>  drivers/power/supply/Makefile     |   1 +
>  drivers/power/supply/bq27561_fg.c | 304 ++++++++++++++++++++++++++++++
>  3 files changed, 314 insertions(+)
>  create mode 100644 drivers/power/supply/bq27561_fg.c
> 
> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
> index f3424fdce341..febf6bf8c4ea 100644
> --- a/drivers/power/supply/Kconfig
> +++ b/drivers/power/supply/Kconfig
> @@ -597,6 +597,15 @@ config CHARGER_BQ25890
>  	help
>  	  Say Y to enable support for the TI BQ25890 battery charger.
>  
> +config FUEL_GAUGE_BQ27561
> +	tristate "Texas Instruments bq27561 fuel gauge driver"
> +	depends on I2C
> +	select REGMAP_I2C

good :)

> +	depends on GPIOLIB || COMPILE_TEST
> +	help
> +	 Say Y here to enable support for fuel gauge with TI bq27561
> +	 fuel gauge chips.
> +
>  config CHARGER_SMB347
>  	tristate "Summit Microelectronics SMB347 Battery Charger"
>  	depends on I2C
> diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
> index 6c7da920ea83..4e402292b276 100644
> --- a/drivers/power/supply/Makefile
> +++ b/drivers/power/supply/Makefile
> @@ -81,6 +81,7 @@ obj-$(CONFIG_CHARGER_BQ24190)	+= bq24190_charger.o
>  obj-$(CONFIG_CHARGER_BQ24257)	+= bq24257_charger.o
>  obj-$(CONFIG_CHARGER_BQ24735)	+= bq24735-charger.o
>  obj-$(CONFIG_CHARGER_BQ25890)	+= bq25890_charger.o
> +obj-$(CONFIG_FUEL_GAUGE_BQ27561)	+= bq27561_fg.o
>  obj-$(CONFIG_CHARGER_SMB347)	+= smb347-charger.o
>  obj-$(CONFIG_CHARGER_TPS65090)	+= tps65090-charger.o
>  obj-$(CONFIG_CHARGER_TPS65217)	+= tps65217_charger.o
> diff --git a/drivers/power/supply/bq27561_fg.c b/drivers/power/supply/bq27561_fg.c
> new file mode 100644
> index 000000000000..b9f7cdc00d2e
> --- /dev/null
> +++ b/drivers/power/supply/bq27561_fg.c
> @@ -0,0 +1,304 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// BQ27561 Fuel Gauge Driver
> +// Copyright (C) 2020 Texas Instruments Incorporated - http://www.ti.com/
> +// Author: Dan Murphy <dmurphy@ti.com>
> +
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/power_supply.h>
> +#include <linux/types.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/moduleparam.h>
> +#include <linux/slab.h>
> +#include <linux/of.h>
> +
> +#define BQ27561_REG_CNTL	0x00
> +#define BQ27561_REG_AR		0x02
> +#define BQ27561_REG_TEMP	0x06
> +#define BQ27561_REG_VOLT	0x08
> +#define BQ27561_REG_REMAIN_CAP	0x10
> +#define BQ27561_REG_BATTSTAT	0x0a
> +#define BQ27561_REG_CURRENT	0x0c
> +#define BQ27561_REG_FCC		0x12
> +#define BQ27561_REG_AVG_CURRENT	0x14
> +#define BQ27561_REG_TTE		0x16
> +#define BQ27561_REG_TTF		0x18
> +#define BQ27561_REG_CYCLECNT	0x2a
> +#define BQ27561_REG_RSOC	0x2c
> +
> +#define BQ27561_VAL_MULTIPLIER	1000
> +#define BQ27561_TEMP_K_TO_C	2732
> +#define BQ27561_TIME_MIN_TO_S	60
> +
> +#define BQ27561_BATT_DEPLETED	BIT(4)
> +#define BQ27561_BATT_CHARGED	BIT(5)
> +#define BQ27561_BATT_DISCHARGING	BIT(6)
> +
> +enum {
> +	BQ27561,
> +	BQ27750,
> +};
> +
> +struct bq27561_device {
> +	struct device *dev;
> +	struct i2c_client *client;
> +	struct power_supply *battery;
> +};
> +
> +static enum power_supply_property bq27561_properties[] = {
> +	POWER_SUPPLY_PROP_STATUS,
> +	POWER_SUPPLY_PROP_CAPACITY,
> +	POWER_SUPPLY_PROP_TEMP,
> +	POWER_SUPPLY_PROP_VOLTAGE_NOW,
> +	POWER_SUPPLY_PROP_CURRENT_NOW,
> +	POWER_SUPPLY_PROP_CHARGE_NOW,
> +	POWER_SUPPLY_PROP_CHARGE_FULL,
> +	POWER_SUPPLY_PROP_TIME_TO_EMPTY_NOW,
> +};
> +
> +static int bq27561_get_temp(struct bq27561_device *bq)
> +{
> +	unsigned int temp;
> +
> +	temp = i2c_smbus_read_word_data(bq->client, BQ27561_REG_TEMP);

please use regmap instead of direct i2c read/write commands!

> +	if (temp < 0)
> +		return temp;

so e.g. -EIO will become -0.5°C?

> +	return temp - BQ27561_TEMP_K_TO_C;
> +}
> +
> +static int bq27561_get_voltage(struct bq27561_device *bq)
> +{
> +	unsigned int batt_volt;
> +
> +	batt_volt = i2c_smbus_read_word_data(bq->client, BQ27561_REG_VOLT);
> +	if (batt_volt < 0)
> +		return batt_volt;
> +
> +	return batt_volt * BQ27561_VAL_MULTIPLIER;
> +}
> +
> +static int bq27561_get_current(struct bq27561_device *bq)
> +{
> +	unsigned int batt_curr;
> +
> +	batt_curr = i2c_smbus_read_word_data(bq->client, BQ27561_REG_CURRENT);
> +	if (batt_curr < 0)
> +		return batt_curr;
> +
> +	return batt_curr * BQ27561_VAL_MULTIPLIER;
> +}
> +
> +static int bq27561_get_charge_capacity(struct bq27561_device *bq)
> +{
> +	unsigned int charge_cap;
> +
> +	charge_cap = i2c_smbus_read_word_data(bq->client, BQ27561_REG_FCC);
> +	if (charge_cap < 0)
> +		return charge_cap;
> +
> +	return charge_cap * BQ27561_VAL_MULTIPLIER;
> +}
> +
> +static int bq27561_get_batt_status(struct bq27561_device *bq)
> +{
> +	unsigned int batt_stat;
> +	unsigned int curr;
> +	int ret;
> +
> +	batt_stat = i2c_smbus_read_word_data(bq->client, BQ27561_REG_BATTSTAT);
> +	if (batt_stat < 0)
> +		return batt_stat;
> +
> +	curr = i2c_smbus_read_word_data(bq->client, BQ27561_REG_CURRENT);
> +	if (curr < 0)
> +		return curr;
> +
> +	if (batt_stat & BQ27561_BATT_CHARGED)
> +		ret = POWER_SUPPLY_STATUS_FULL;
> +	else if (batt_stat & BQ27561_BATT_DISCHARGING || curr == 0)
> +		ret = POWER_SUPPLY_STATUS_DISCHARGING;
> +	else if (curr > 0)
> +		ret = POWER_SUPPLY_STATUS_CHARGING;
> +	else
> +		ret = POWER_SUPPLY_STATUS_UNKNOWN;
> +
> +	return ret;
> +}
> +
> +static int bq27561_get_empty_time(struct bq27561_device *bq)
> +{
> +	unsigned int empty_time;
> +
> +	empty_time = i2c_smbus_read_word_data(bq->client, BQ27561_REG_TTE);
> +	if (empty_time < 0)
> +		return empty_time;
> +
> +	return empty_time * BQ27561_TIME_MIN_TO_S;
> +}
> +
> +static int bq27561_get_property(struct power_supply *psy,
> +				enum power_supply_property psp,
> +				union power_supply_propval *val)
> +{
> +	struct bq27561_device *bq = power_supply_get_drvdata(psy);
> +	int ret = 0;
> +
> +	switch (psp) {
> +	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
> +		ret = bq27561_get_voltage(bq);
> +		if (ret < 0)
> +			return ret;
> +
> +		val->intval = ret;
> +		break;
> +	case POWER_SUPPLY_PROP_CURRENT_NOW:
> +		ret = bq27561_get_current(bq);
> +		if (ret < 0)
> +			return ret;

current can be negative (charging vs discharging).

> +
> +		val->intval = ret;
> +		break;
> +	case POWER_SUPPLY_PROP_STATUS:
> +		ret = bq27561_get_batt_status(bq);
> +		if (ret < 0)
> +			return ret;
> +
> +		val->intval = ret;
> +		break;
> +	case POWER_SUPPLY_PROP_TEMP:
> +		val->intval = bq27561_get_temp(bq);
> +		break;

no error handling?

> +	case POWER_SUPPLY_PROP_CAPACITY:
> +		ret = i2c_smbus_read_word_data(bq->client, BQ27561_REG_RSOC);
> +		if (ret < 0)
> +			return ret;
> +
> +		val->intval = ret;
> +		break;
> +	case POWER_SUPPLY_PROP_CHARGE_FULL:
> +		ret = bq27561_get_charge_capacity(bq);
> +		if (ret < 0)
> +			return ret;
> +
> +		val->intval = ret;
> +		break;
> +	case POWER_SUPPLY_PROP_TIME_TO_EMPTY_NOW:
> +		ret = bq27561_get_empty_time(bq);
> +		if (ret < 0)
> +			return ret;
> +
> +		val->intval = ret;
> +		break;
> +	case POWER_SUPPLY_PROP_CHARGE_NOW:
> +		ret = i2c_smbus_read_word_data(bq->client, BQ27561_REG_AR);
> +		if (ret < 0)
> +			return ret;
> +
> +		val->intval = ret;
> +		break;
> +	default:
> +		return -EINVAL;
> +	};
> +
> +	return 0;
> +}
> +
> +static int bq27561_set_property(struct power_supply *psy,
> +				enum power_supply_property psp,
> +				const union power_supply_propval *val)
> +{
> +	struct bq27561_device *bq = power_supply_get_drvdata(psy);
> +	int ret = 0;
> +
> +	switch (psp) {
> +	case POWER_SUPPLY_PROP_CHARGE_NOW:
> +		ret = i2c_smbus_write_word_data(bq->client, BQ27561_REG_AR,
> +						val->intval);
> +		break;
> +	default:
> +		return -EINVAL;
> +	};
> +
> +	return ret;
> +}
> +
> +static int bq27561_prop_is_writeable(struct power_supply *psy,
> +				     enum power_supply_property prop)
> +{
> +	switch (prop) {
> +	case POWER_SUPPLY_PROP_CHARGE_NOW:
> +		return 1;
> +	default:
> +		return 0;
> +	};
> +}
> +
> +static struct power_supply_desc bq27561_power_supply_desc = {
> +	.type = POWER_SUPPLY_TYPE_BATTERY,
> +	.properties = bq27561_properties,
> +	.num_properties = ARRAY_SIZE(bq27561_properties),
> +	.get_property = bq27561_get_property,
> +	.set_property = bq27561_set_property,
> +	.property_is_writeable = bq27561_prop_is_writeable,
> +};
> +
> +static int bq27561_probe(struct i2c_client *client,
> +				 const struct i2c_device_id *id)
> +{
> +	struct bq27561_device *bq;
> +	struct power_supply_config psy_cfg;
> +
> +	bq = devm_kzalloc(&client->dev, sizeof(*bq), GFP_KERNEL);
> +	if (!bq)
> +		return -ENOMEM;
> +
> +	bq->dev = &client->dev;
> +	bq->client = client;
> +	i2c_set_clientdata(client, bq);
> +
> +	psy_cfg.drv_data = bq;

You should setup psy_cfg.of_node, so that it can be referenced
in DT.

> +	bq27561_power_supply_desc.name = id->name;
> +	bq->battery = devm_power_supply_register(bq->dev,
> +						 &bq27561_power_supply_desc,
> +						 &psy_cfg);
> +
> +	if (IS_ERR(bq->battery))
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id bq27561_match_table[] = {
> +	{.compatible = "ti,bq27561",},
> +	{.compatible = "ti,bq27750",},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, bq_fg_match_table);
> +
> +static const struct i2c_device_id bq27561_id[] = {
> +	{"bq27561", BQ27561},
> +	{"bq27750", BQ27750},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(i2c, bq27561_id);
> +
> +static struct i2c_driver bq27561_battery_driver = {
> +	.driver = {
> +		   .name = "bq27561-fuel-gauge",
> +		   .of_match_table = bq27561_match_table,
> +		   .owner = THIS_MODULE,

i2c_register_driver() sets it owner automatically, so
drop this.

> +		   },
> +	.probe = bq27561_probe,
> +	.id_table = bq27561_id,
> +};
> +module_i2c_driver(bq27561_battery_driver);
> +
> +MODULE_AUTHOR("Texas Instruments");
> +MODULE_DESCRIPTION("bq27561 battery monitor driver");
> +MODULE_LICENSE("GPL v2");

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/2] dt-bindings: power: Add the BQ27561 fuel gauge bindings

[Sebastian Reichel]

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

Hi,

On Fri, Apr 17, 2020 at 12:22:26PM -0500, Dan Murphy wrote:
> Introduce the Texas Instrument BQ27561 and BQ27750 fuel gauge device
> tree bindings.
> 
> Cc: Rob Herring <robh@kernel.org>
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---

I think its best to just add them to bq27xxx.txt. Conversion to YAML
would still be nice of course :)

In any case the binding is incomplete, since batteries should contain
a power-supplies node referencing their charger via a phandle, see
power_suppliy.txt (which still needs to be converted to YAML).

-- Sebastian

>  .../bindings/power/supply/bq27561.yaml        | 46 +++++++++++++++++++
>  1 file changed, 46 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/power/supply/bq27561.yaml
> 
> diff --git a/Documentation/devicetree/bindings/power/supply/bq27561.yaml b/Documentation/devicetree/bindings/power/supply/bq27561.yaml
> new file mode 100644
> index 000000000000..abc9acdbc704
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/supply/bq27561.yaml
> @@ -0,0 +1,46 @@
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (C) 2020 Texas Instruments Incorporated
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/power/supply/bq27561.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: TI BQ27561 and BQ27750 Fuel Gauges
> +
> +maintainers:
> +  - Dan Murphy <dmurphy@ti.com>
> +
> +description: |
> +  The bq27z561 device provides a feature-rich gas gauging solution for
> +  single-cell battery pack applications.
> +
> +  Specifications about the charger can be found at:
> +    https://www.ti.com/lit/gpn/bq27z561
> +    https://www.ti.com/lit/gpn/bq27750
> +
> +properties:
> +  compatible:
> +    enum:
> +      - ti,bq27561
> +      - ti,bq27750
> +
> +  reg:
> +    maxItems: 1
> +    description: |
> +      I2C address of the device which is 0x55
> +
> +required:
> +  - compatible
> +  - reg
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c0 {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +      fuel_gauge@55 {
> +        compatible = "ti,bq27561";
> +        reg = <0x55>;
> +      };
> +    };
> -- 
> 2.25.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/2] power: supply: Introduce the BQ27561 Fuel gauge driver

[Dan Murphy]

Sebastian

On 4/29/20 5:05 PM, Sebastian Reichel wrote:
> Hi,
>
> On Fri, Apr 17, 2020 at 12:22:27PM -0500, Dan Murphy wrote:
>> Introduce the BQ27561 Fuel gauge driver from Texas Instruments. The
>> driver also supports the BQ27750 as it has the same register map and
>> bit mask.
> Why a separate driver instead of adding support to bq27xxx? The register
> map seems to be similar enough.

I will need to see if I can integrate this into the bq27xxx.  I have to 
look at these 2 devices as well as another device that we need to integrate

But preliminary analysis suggests that we can integrate these 2 devices 
plus the third one into the driver.

>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
>> ---
>>   drivers/power/supply/Kconfig      |   9 +
>>   drivers/power/supply/Makefile     |   1 +
>>   drivers/power/supply/bq27561_fg.c | 304 ++++++++++++++++++++++++++++++
>>   3 files changed, 314 insertions(+)
>>   create mode 100644 drivers/power/supply/bq27561_fg.c
>>
>> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
>> index f3424fdce341..febf6bf8c4ea 100644
>> --- a/drivers/power/supply/Kconfig
>> +++ b/drivers/power/supply/Kconfig
>> @@ -597,6 +597,15 @@ config CHARGER_BQ25890
>>   	help
>>   	  Say Y to enable support for the TI BQ25890 battery charger.
>>   
>> +config FUEL_GAUGE_BQ27561
>> +	tristate "Texas Instruments bq27561 fuel gauge driver"
>> +	depends on I2C
>> +	select REGMAP_I2C
> good :)
Ack
>
>> +	depends on GPIOLIB || COMPILE_TEST
>> +	help
>> +	 Say Y here to enable support for fuel gauge with TI bq27561
>> +	 fuel gauge chips.
>> +
>>   config CHARGER_SMB347
>>   	tristate "Summit Microelectronics SMB347 Battery Charger"
>>   	depends on I2C
>> diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
>> index 6c7da920ea83..4e402292b276 100644
>> --- a/drivers/power/supply/Makefile
>> +++ b/drivers/power/supply/Makefile
>> @@ -81,6 +81,7 @@ obj-$(CONFIG_CHARGER_BQ24190)	+= bq24190_charger.o
>>   obj-$(CONFIG_CHARGER_BQ24257)	+= bq24257_charger.o
>>   obj-$(CONFIG_CHARGER_BQ24735)	+= bq24735-charger.o
>>   obj-$(CONFIG_CHARGER_BQ25890)	+= bq25890_charger.o
>> +obj-$(CONFIG_FUEL_GAUGE_BQ27561)	+= bq27561_fg.o
>>   obj-$(CONFIG_CHARGER_SMB347)	+= smb347-charger.o
>>   obj-$(CONFIG_CHARGER_TPS65090)	+= tps65090-charger.o
>>   obj-$(CONFIG_CHARGER_TPS65217)	+= tps65217_charger.o
>> diff --git a/drivers/power/supply/bq27561_fg.c b/drivers/power/supply/bq27561_fg.c
>> new file mode 100644
>> index 000000000000..b9f7cdc00d2e
>> --- /dev/null
>> +++ b/drivers/power/supply/bq27561_fg.c
>> @@ -0,0 +1,304 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +// BQ27561 Fuel Gauge Driver
>> +// Copyright (C) 2020 Texas Instruments Incorporated - http://www.ti.com/
>> +// Author: Dan Murphy <dmurphy@ti.com>
>> +
>> +#include <linux/err.h>
>> +#include <linux/i2c.h>
>> +#include <linux/init.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/power_supply.h>
>> +#include <linux/types.h>
>> +#include <linux/delay.h>
>> +#include <linux/device.h>
>> +#include <linux/moduleparam.h>
>> +#include <linux/slab.h>
>> +#include <linux/of.h>
>> +
>> +#define BQ27561_REG_CNTL	0x00
>> +#define BQ27561_REG_AR		0x02
>> +#define BQ27561_REG_TEMP	0x06
>> +#define BQ27561_REG_VOLT	0x08
>> +#define BQ27561_REG_REMAIN_CAP	0x10
>> +#define BQ27561_REG_BATTSTAT	0x0a
>> +#define BQ27561_REG_CURRENT	0x0c
>> +#define BQ27561_REG_FCC		0x12
>> +#define BQ27561_REG_AVG_CURRENT	0x14
>> +#define BQ27561_REG_TTE		0x16
>> +#define BQ27561_REG_TTF		0x18
>> +#define BQ27561_REG_CYCLECNT	0x2a
>> +#define BQ27561_REG_RSOC	0x2c
>> +
>> +#define BQ27561_VAL_MULTIPLIER	1000
>> +#define BQ27561_TEMP_K_TO_C	2732
>> +#define BQ27561_TIME_MIN_TO_S	60
>> +
>> +#define BQ27561_BATT_DEPLETED	BIT(4)
>> +#define BQ27561_BATT_CHARGED	BIT(5)
>> +#define BQ27561_BATT_DISCHARGING	BIT(6)
>> +
>> +enum {
>> +	BQ27561,
>> +	BQ27750,
>> +};
>> +
>> +struct bq27561_device {
>> +	struct device *dev;
>> +	struct i2c_client *client;
>> +	struct power_supply *battery;
>> +};
>> +
>> +static enum power_supply_property bq27561_properties[] = {
>> +	POWER_SUPPLY_PROP_STATUS,
>> +	POWER_SUPPLY_PROP_CAPACITY,
>> +	POWER_SUPPLY_PROP_TEMP,
>> +	POWER_SUPPLY_PROP_VOLTAGE_NOW,
>> +	POWER_SUPPLY_PROP_CURRENT_NOW,
>> +	POWER_SUPPLY_PROP_CHARGE_NOW,
>> +	POWER_SUPPLY_PROP_CHARGE_FULL,
>> +	POWER_SUPPLY_PROP_TIME_TO_EMPTY_NOW,
>> +};
>> +
>> +static int bq27561_get_temp(struct bq27561_device *bq)
>> +{
>> +	unsigned int temp;
>> +
>> +	temp = i2c_smbus_read_word_data(bq->client, BQ27561_REG_TEMP);
> please use regmap instead of direct i2c read/write commands!
>
OK I did not look at regmap close enough to see if it supports smbus.  
But apparently it does.

I just need to make sure.


>> +	if (temp < 0)
>> +		return temp;
> so e.g. -EIO will become -0.5°C?
Ack.  It is hard to get a negative temp to be recorded by the thermister 
in testing.
>
>> +	return temp - BQ27561_TEMP_K_TO_C;
>> +}
>> +
>> +static int bq27561_get_voltage(struct bq27561_device *bq)
>> +{
>> +	unsigned int batt_volt;
>> +
>> +	batt_volt = i2c_smbus_read_word_data(bq->client, BQ27561_REG_VOLT);
>> +	if (batt_volt < 0)
>> +		return batt_volt;
>> +
>> +	return batt_volt * BQ27561_VAL_MULTIPLIER;
>> +}
>> +
>> +static int bq27561_get_current(struct bq27561_device *bq)
>> +{
>> +	unsigned int batt_curr;
>> +
>> +	batt_curr = i2c_smbus_read_word_data(bq->client, BQ27561_REG_CURRENT);
>> +	if (batt_curr < 0)
>> +		return batt_curr;
>> +
>> +	return batt_curr * BQ27561_VAL_MULTIPLIER;
>> +}
>> +
>> +static int bq27561_get_charge_capacity(struct bq27561_device *bq)
>> +{
>> +	unsigned int charge_cap;
>> +
>> +	charge_cap = i2c_smbus_read_word_data(bq->client, BQ27561_REG_FCC);
>> +	if (charge_cap < 0)
>> +		return charge_cap;
>> +
>> +	return charge_cap * BQ27561_VAL_MULTIPLIER;
>> +}
>> +
>> +static int bq27561_get_batt_status(struct bq27561_device *bq)
>> +{
>> +	unsigned int batt_stat;
>> +	unsigned int curr;
>> +	int ret;
>> +
>> +	batt_stat = i2c_smbus_read_word_data(bq->client, BQ27561_REG_BATTSTAT);
>> +	if (batt_stat < 0)
>> +		return batt_stat;
>> +
>> +	curr = i2c_smbus_read_word_data(bq->client, BQ27561_REG_CURRENT);
>> +	if (curr < 0)
>> +		return curr;
>> +
>> +	if (batt_stat & BQ27561_BATT_CHARGED)
>> +		ret = POWER_SUPPLY_STATUS_FULL;
>> +	else if (batt_stat & BQ27561_BATT_DISCHARGING || curr == 0)
>> +		ret = POWER_SUPPLY_STATUS_DISCHARGING;
>> +	else if (curr > 0)
>> +		ret = POWER_SUPPLY_STATUS_CHARGING;
>> +	else
>> +		ret = POWER_SUPPLY_STATUS_UNKNOWN;
>> +
>> +	return ret;
>> +}
>> +
>> +static int bq27561_get_empty_time(struct bq27561_device *bq)
>> +{
>> +	unsigned int empty_time;
>> +
>> +	empty_time = i2c_smbus_read_word_data(bq->client, BQ27561_REG_TTE);
>> +	if (empty_time < 0)
>> +		return empty_time;
>> +
>> +	return empty_time * BQ27561_TIME_MIN_TO_S;
>> +}
>> +
>> +static int bq27561_get_property(struct power_supply *psy,
>> +				enum power_supply_property psp,
>> +				union power_supply_propval *val)
>> +{
>> +	struct bq27561_device *bq = power_supply_get_drvdata(psy);
>> +	int ret = 0;
>> +
>> +	switch (psp) {
>> +	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
>> +		ret = bq27561_get_voltage(bq);
>> +		if (ret < 0)
>> +			return ret;
>> +
>> +		val->intval = ret;
>> +		break;
>> +	case POWER_SUPPLY_PROP_CURRENT_NOW:
>> +		ret = bq27561_get_current(bq);
>> +		if (ret < 0)
>> +			return ret;
> current can be negative (charging vs discharging).
Ack
>
>> +
>> +		val->intval = ret;
>> +		break;
>> +	case POWER_SUPPLY_PROP_STATUS:
>> +		ret = bq27561_get_batt_status(bq);
>> +		if (ret < 0)
>> +			return ret;
>> +
>> +		val->intval = ret;
>> +		break;
>> +	case POWER_SUPPLY_PROP_TEMP:
>> +		val->intval = bq27561_get_temp(bq);
>> +		break;
> no error handling?
Ack
>
>> +	case POWER_SUPPLY_PROP_CAPACITY:
>> +		ret = i2c_smbus_read_word_data(bq->client, BQ27561_REG_RSOC);
>> +		if (ret < 0)
>> +			return ret;
>> +
>> +		val->intval = ret;
>> +		break;
>> +	case POWER_SUPPLY_PROP_CHARGE_FULL:
>> +		ret = bq27561_get_charge_capacity(bq);
>> +		if (ret < 0)
>> +			return ret;
>> +
>> +		val->intval = ret;
>> +		break;
>> +	case POWER_SUPPLY_PROP_TIME_TO_EMPTY_NOW:
>> +		ret = bq27561_get_empty_time(bq);
>> +		if (ret < 0)
>> +			return ret;
>> +
>> +		val->intval = ret;
>> +		break;
>> +	case POWER_SUPPLY_PROP_CHARGE_NOW:
>> +		ret = i2c_smbus_read_word_data(bq->client, BQ27561_REG_AR);
>> +		if (ret < 0)
>> +			return ret;
>> +
>> +		val->intval = ret;
>> +		break;
>> +	default:
>> +		return -EINVAL;
>> +	};
>> +
>> +	return 0;
>> +}
>> +
>> +static int bq27561_set_property(struct power_supply *psy,
>> +				enum power_supply_property psp,
>> +				const union power_supply_propval *val)
>> +{
>> +	struct bq27561_device *bq = power_supply_get_drvdata(psy);
>> +	int ret = 0;
>> +
>> +	switch (psp) {
>> +	case POWER_SUPPLY_PROP_CHARGE_NOW:
>> +		ret = i2c_smbus_write_word_data(bq->client, BQ27561_REG_AR,
>> +						val->intval);
>> +		break;
>> +	default:
>> +		return -EINVAL;
>> +	};
>> +
>> +	return ret;
>> +}
>> +
>> +static int bq27561_prop_is_writeable(struct power_supply *psy,
>> +				     enum power_supply_property prop)
>> +{
>> +	switch (prop) {
>> +	case POWER_SUPPLY_PROP_CHARGE_NOW:
>> +		return 1;
>> +	default:
>> +		return 0;
>> +	};
>> +}
>> +
>> +static struct power_supply_desc bq27561_power_supply_desc = {
>> +	.type = POWER_SUPPLY_TYPE_BATTERY,
>> +	.properties = bq27561_properties,
>> +	.num_properties = ARRAY_SIZE(bq27561_properties),
>> +	.get_property = bq27561_get_property,
>> +	.set_property = bq27561_set_property,
>> +	.property_is_writeable = bq27561_prop_is_writeable,
>> +};
>> +
>> +static int bq27561_probe(struct i2c_client *client,
>> +				 const struct i2c_device_id *id)
>> +{
>> +	struct bq27561_device *bq;
>> +	struct power_supply_config psy_cfg;
>> +
>> +	bq = devm_kzalloc(&client->dev, sizeof(*bq), GFP_KERNEL);
>> +	if (!bq)
>> +		return -ENOMEM;
>> +
>> +	bq->dev = &client->dev;
>> +	bq->client = client;
>> +	i2c_set_clientdata(client, bq);
>> +
>> +	psy_cfg.drv_data = bq;
> You should setup psy_cfg.of_node, so that it can be referenced
> in DT.
>
Ack
>> +	bq27561_power_supply_desc.name = id->name;
>> +	bq->battery = devm_power_supply_register(bq->dev,
>> +						 &bq27561_power_supply_desc,
>> +						 &psy_cfg);
>> +
>> +	if (IS_ERR(bq->battery))
>> +		return -EINVAL;
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct of_device_id bq27561_match_table[] = {
>> +	{.compatible = "ti,bq27561",},
>> +	{.compatible = "ti,bq27750",},
>> +	{},
>> +};
>> +MODULE_DEVICE_TABLE(of, bq_fg_match_table);
>> +
>> +static const struct i2c_device_id bq27561_id[] = {
>> +	{"bq27561", BQ27561},
>> +	{"bq27750", BQ27750},
>> +	{},
>> +};
>> +MODULE_DEVICE_TABLE(i2c, bq27561_id);
>> +
>> +static struct i2c_driver bq27561_battery_driver = {
>> +	.driver = {
>> +		   .name = "bq27561-fuel-gauge",
>> +		   .of_match_table = bq27561_match_table,
>> +		   .owner = THIS_MODULE,
> i2c_register_driver() sets it owner automatically, so
> drop this.
>
>> +		   },
>> +	.probe = bq27561_probe,
>> +	.id_table = bq27561_id,
>> +};
>> +module_i2c_driver(bq27561_battery_driver);
>> +
>> +MODULE_AUTHOR("Texas Instruments");
>> +MODULE_DESCRIPTION("bq27561 battery monitor driver");
>> +MODULE_LICENSE("GPL v2");
> -- Sebastian

Re: [PATCH 1/2] dt-bindings: power: Add the BQ27561 fuel gauge bindings

[Dan Murphy]

Sebastien

On 4/29/20 5:31 PM, Sebastian Reichel wrote:
> Hi,
>
> On Fri, Apr 17, 2020 at 12:22:26PM -0500, Dan Murphy wrote:
>> Introduce the Texas Instrument BQ27561 and BQ27750 fuel gauge device
>> tree bindings.
>>
>> Cc: Rob Herring <robh@kernel.org>
>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
>> ---
> I think its best to just add them to bq27xxx.txt. Conversion to YAML
> would still be nice of course :)

Ack


> In any case the binding is incomplete, since batteries should contain
> a power-supplies node referencing their charger via a phandle, see
> power_suppliy.txt (which still needs to be converted to YAML).

Ack

Dan