2020-01-16 17:55:22

by Dan Murphy

[permalink] [raw]
Subject: [PATCH v4 0/4] BQ25150/155 Driver introduction

Hello

I am introducing the TI BQ25150/155 Charge driver.
The driver also supports the JEITA spec for Thermal battery management. This
support for Hot, Warm and Cool are being added to the power supply class.

Datasheets for these devices can be found at:
http://www.ti.com/lit/ds/symlink/bq25150.pdf
http://www.ti.com/lit/ds/symlink/bq25155.pdf

Dan

Dan Murphy (4):
power: supply: core: Update sysfs-class-power ABI document
power_supply: Add additional health properties to the header
dt-bindings: power: Add the bq2515x family dt bindings
power: supply: bq2515x: Introduce the bq2515x family

Documentation/ABI/testing/sysfs-class-power | 3 +-
.../bindings/power/supply/bq2515x.yaml | 85 ++
drivers/power/supply/Kconfig | 8 +
drivers/power/supply/Makefile | 1 +
drivers/power/supply/bq2515x_charger.c | 739 ++++++++++++++++++
drivers/power/supply/power_supply_sysfs.c | 2 +-
include/linux/power_supply.h | 3 +
7 files changed, 839 insertions(+), 2 deletions(-)
create mode 100644 Documentation/devicetree/bindings/power/supply/bq2515x.yaml
create mode 100644 drivers/power/supply/bq2515x_charger.c

--
2.25.0


2020-01-16 17:56:14

by Dan Murphy

[permalink] [raw]
Subject: [PATCH v4 4/4] power: supply: bq2515x: Introduce the bq2515x family

Introduce the bq25150 and bq25155 supply chargers.

Signed-off-by: Dan Murphy <[email protected]>
---
drivers/power/supply/Kconfig | 8 +
drivers/power/supply/Makefile | 1 +
drivers/power/supply/bq2515x_charger.c | 739 +++++++++++++++++++++++++
3 files changed, 748 insertions(+)
create mode 100644 drivers/power/supply/bq2515x_charger.c

diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index 27164a1d3c7c..ee9f4b29e0b7 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -589,6 +589,14 @@ config CHARGER_BQ24735
help
Say Y to enable support for the TI BQ24735 battery charger.

+config CHARGER_BQ2515X
+ tristate "TI BQ2515X battery charger family"
+ depends on I2C
+ depends on GPIOLIB || COMPILE_TEST
+ select REGMAP_I2C
+ help
+ Say Y to enable support for the TI BQ2515X battery charger.
+
config CHARGER_BQ25890
tristate "TI BQ25890 battery charger driver"
depends on I2C
diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
index 6c7da920ea83..8fcc175a7e22 100644
--- a/drivers/power/supply/Makefile
+++ b/drivers/power/supply/Makefile
@@ -80,6 +80,7 @@ obj-$(CONFIG_CHARGER_BQ2415X) += bq2415x_charger.o
obj-$(CONFIG_CHARGER_BQ24190) += bq24190_charger.o
obj-$(CONFIG_CHARGER_BQ24257) += bq24257_charger.o
obj-$(CONFIG_CHARGER_BQ24735) += bq24735-charger.o
+obj-$(CONFIG_CHARGER_BQ2515X) += bq2515x_charger.o
obj-$(CONFIG_CHARGER_BQ25890) += bq25890_charger.o
obj-$(CONFIG_CHARGER_SMB347) += smb347-charger.o
obj-$(CONFIG_CHARGER_TPS65090) += tps65090-charger.o
diff --git a/drivers/power/supply/bq2515x_charger.c b/drivers/power/supply/bq2515x_charger.c
new file mode 100644
index 000000000000..dbf84bef4976
--- /dev/null
+++ b/drivers/power/supply/bq2515x_charger.c
@@ -0,0 +1,739 @@
+// SPDX-License-Identifier: GPL-2.0
+// BQ2515X Battery Charger Driver
+// Copyright (C) 2019 Texas Instruments Incorporated - http://www.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/of_gpio.h>
+#include <linux/power_supply.h>
+#include <linux/regmap.h>
+#include <linux/types.h>
+
+#define BQ2515X_MANUFACTURER "Texas Instruments"
+
+#define BQ2515X_STAT0 0x00
+#define BQ2515X_STAT1 0x01
+#define BQ2515X_STAT2 0x02
+#define BQ2515X_FLAG0 0x03
+#define BQ2515X_FLAG1 0x04
+#define BQ2515X_FLAG2 0x05
+#define BQ2515X_FLAG3 0x06
+#define BQ2515X_MASK0 0x07
+#define BQ2515X_MASK1 0x08
+#define BQ2515X_MASK2 0x09
+#define BQ2515X_MASK3 0x0a
+#define BQ2515X_VBAT_CTRL 0x12
+#define BQ2515X_ICHG_CTRL 0x13
+#define BQ2515X_PCHRGCTRL 0x14
+#define BQ2515X_TERMCTRL 0x15
+#define BQ2515X_BUVLO 0x16
+#define BQ2515X_CHARGERCTRL0 0x17
+#define BQ2515X_CHARGERCTRL1 0x18
+#define BQ2515X_ILIMCTRL 0x19
+#define BQ2515X_LDOCTRL 0x1d
+#define BQ2515X_MRCTRL 0x30
+#define BQ2515X_ICCTRL0 0x35
+#define BQ2515X_ICCTRL1 0x36
+#define BQ2515X_ICCTRL2 0x37
+#define BQ2515X_ADCCTRL0 0x40
+#define BQ2515X_ADCCTRL1 0x41
+#define BQ2515X_ADC_VBAT_M 0x42
+#define BQ2515X_ADC_VBAT_L 0x43
+#define BQ2515X_ADC_TS_M 0x44
+#define BQ2515X_ADC_TS_L 0x45
+#define BQ2515X_ADC_ICHG_M 0x46
+#define BQ2515X_ADC_ICHG_L 0x47
+#define BQ2515X_ADC_ADCIN_M 0x48
+#define BQ2515X_ADC_ADCIN_L 0x49
+#define BQ2515X_ADC_VIN_M 0x4a
+#define BQ2515X_ADC_VIN_L 0x4b
+#define BQ2515X_ADC_PMID_M 0x4c
+#define BQ2515X_ADC_PMID_L 0x4d
+#define BQ2515X_ADC_IIN_M 0x4e
+#define BQ2515X_ADC_IIN_L 0x4f
+#define BQ2515X_ADC_COMP1_M 0x52
+#define BQ2515X_ADC_COMP1_L 0X53
+#define BQ2515X_ADC_COMP2_M 0X54
+#define BQ2515X_ADC_COMP2_L 0x55
+#define BQ2515X_ADC_COMP3_M 0x56
+#define BQ2515X_ADC_COMP3_L 0x57
+#define BQ2515X_ADC_READ_EN 0x58
+#define BQ2515X_TS_FASTCHGCTRL 0x61
+#define BQ2515X_TS_COLD 0x62
+#define BQ2515X_TS_COOL 0x63
+#define BQ2515X_TS_WARM 0x64
+#define BQ2515X_TS_HOT 0x65
+#define BQ2515X_DEVICE_ID 0x6f
+
+#define BQ2515X_DIVISOR 65536
+#define BQ2515X_VBAT_BASE_VOLT 3600000
+#define BQ2515X_VBAT_REG_MAX 4600000
+#define BQ2515X_VBAT_REG_MIN 3600000
+#define BQ2515X_UV_FACTOR 10000
+
+#define BQ2515X_ILIM_150MA 0x2
+#define BQ2515X_ILIM_MASK 0x7
+#define BQ2515X_HEALTH_MASK 0xf
+#define BQ2515X_OVERVOLT_MASK 0x80
+
+#define BQ2515X_HOT_FLAG BIT(0)
+#define BQ2515X_WARM_FLAG BIT(1)
+#define BQ2515X_COOL_FLAG BIT(2)
+#define BQ2515X_COLD_FLAG BIT(3)
+#define BQ2515X_SAFETY_TIMER_EXP BIT(5)
+
+#define BQ2515X_VIN_GOOD BIT(0)
+#define BQ2515X_CHRG_DONE BIT(5)
+#define BQ2515X_CV_CHRG_MODE BIT(6)
+
+static const int bq2515x_ilim_lvl_values[] = {
+ 50000, 100000, 150000, 200000, 300000, 400000, 500000, 600000
+};
+
+/* initial field values, converted to register values */
+struct bq2515x_init_data {
+ int ichg; /* charge current */
+ int vreg; /* regulation voltage */
+};
+
+struct bq2515x_device {
+ struct power_supply *mains;
+ struct power_supply *battery;
+ struct i2c_client *client;
+ struct regmap *regmap;
+ struct device *dev;
+ struct mutex lock;
+
+ struct gpio_desc *reset_gpio;
+ struct gpio_desc *lp_gpio;
+ struct gpio_desc *pg_gpio;
+ struct gpio_desc *ce_gpio;
+
+ char model_name[I2C_NAME_SIZE];
+ int mains_online;
+
+ uint32_t voltage_min_design;
+ uint32_t voltage_max_design;
+ uint32_t charge_full_design;
+
+ struct bq2515x_init_data init_data;
+};
+
+static struct reg_default bq2515x_reg_defs[] = {
+ {BQ2515X_STAT0, 0xff},
+ {BQ2515X_STAT1, 0x0},
+ {BQ2515X_STAT2, 0x0},
+ {BQ2515X_FLAG0, 0x0},
+ {BQ2515X_FLAG1, 0x0},
+ {BQ2515X_FLAG2, 0x0},
+ {BQ2515X_FLAG3, 0x0},
+ {BQ2515X_MASK0, 0x0},
+ {BQ2515X_MASK1, 0x0},
+ {BQ2515X_MASK2, 0x0},
+ {BQ2515X_MASK3, 0x0},
+};
+
+static bool bq2515x_is_ps_online(struct bq2515x_device *bq2515x)
+{
+ return bq2515x->mains_online;
+}
+
+static int bq2515x_wake_up(struct bq2515x_device *bq2515x)
+{
+ int ret;
+ int val;
+
+ /* Read the STAT register if we can read it then the device is out
+ * of ship mode. If the register cannot be read then attempt to wake
+ * it up and enable the ADC.
+ */
+ ret = regmap_read(bq2515x->regmap, BQ2515X_STAT0, &val);
+ if (!ret)
+ return ret;
+
+ /* Need to toggle LP and MR here */
+ if (bq2515x->lp_gpio)
+ gpiod_direction_output(bq2515x->lp_gpio, 1);
+
+ if (bq2515x->reset_gpio) {
+ gpiod_direction_output(bq2515x->lp_gpio, 0);
+ mdelay(2000);
+ gpiod_direction_output(bq2515x->lp_gpio, 1);
+ }
+
+ return regmap_write(bq2515x->regmap, BQ2515X_ADC_READ_EN, BIT(3));
+}
+
+static int bq2515x_update_ps_status(struct bq2515x_device *bq2515x)
+{
+ bool dc = false;
+ unsigned int val;
+ int ret;
+
+ if (bq2515x->pg_gpio)
+ val = gpiod_get_value(bq2515x->pg_gpio);
+ else {
+ ret = regmap_read(bq2515x->regmap, BQ2515X_STAT0, &val);
+ if (ret < 0)
+ return ret;
+ }
+
+ dc = val & BQ2515X_VIN_GOOD;
+
+ ret = bq2515x->mains_online != dc;
+
+ bq2515x->mains_online = dc;
+
+ return ret;
+}
+
+static int get_const_charge_current(struct bq2515x_device *bq2515x)
+{
+ int ret;
+ int iin_msb;
+ int iin_lsb;
+ u16 ichg_measurement;
+ int ilim_val, ichg_multiplier;
+
+ if (!bq2515x_is_ps_online(bq2515x))
+ return -ENODATA;
+
+ ret = regmap_read(bq2515x->regmap, BQ2515X_ADC_IIN_M, &iin_msb);
+ if (ret < 0)
+ return ret;
+
+ ret = regmap_read(bq2515x->regmap, BQ2515X_ADC_IIN_L, &iin_lsb);
+ if (ret < 0)
+ return ret;
+
+ ichg_measurement = (iin_msb << 8) | iin_lsb;
+ ret = regmap_read(bq2515x->regmap, BQ2515X_ILIMCTRL, &ilim_val);
+ if (ret < 0)
+ return ret;
+
+ if (ilim_val >= BQ2515X_ILIM_150MA)
+ ichg_multiplier = 350;
+ else
+ ichg_multiplier = 750;
+
+ return (ichg_measurement * 100 / BQ2515X_DIVISOR) * ichg_multiplier;
+}
+
+static int get_const_charge_voltage(struct bq2515x_device *bq2515x)
+{
+ int ret;
+ int vin_msb;
+ int vin_lsb;
+ u16 vbat_measurement;
+
+ if (!bq2515x_is_ps_online(bq2515x))
+ bq2515x_wake_up(bq2515x);
+
+ ret = regmap_read(bq2515x->regmap, BQ2515X_ADC_VBAT_M, &vin_msb);
+ if (ret)
+ return ret;
+
+ ret = regmap_read(bq2515x->regmap, BQ2515X_ADC_VBAT_L, &vin_lsb);
+ if (ret)
+ return ret;
+
+ vbat_measurement = (vin_msb << 8) | vin_lsb;
+ return ((vbat_measurement * BQ2515X_UV_FACTOR) / BQ2515X_DIVISOR) * 6;
+}
+
+static int bq2515x_charging_status(struct bq2515x_device *bq2515x,
+ union power_supply_propval *val)
+{
+ unsigned int status;
+ int ret;
+
+ if (!bq2515x_is_ps_online(bq2515x))
+ return 0;
+
+ ret = regmap_read(bq2515x->regmap, BQ2515X_STAT0, &status);
+ if (ret) {
+ val->intval = POWER_SUPPLY_STATUS_UNKNOWN;
+ return ret;
+ }
+
+ if (status & BQ2515X_CV_CHRG_MODE && status & BQ2515X_VIN_GOOD)
+ val->intval = POWER_SUPPLY_STATUS_CHARGING;
+ else if (status & BQ2515X_CHRG_DONE)
+ val->intval = POWER_SUPPLY_STATUS_FULL;
+ else if (status & BQ2515X_VIN_GOOD)
+ val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING;
+ else
+ val->intval = POWER_SUPPLY_STATUS_UNKNOWN;
+
+ return ret;
+}
+
+static int bq2515x_get_batt_reg(struct bq2515x_device *bq2515x)
+{
+ int vbat_reg_code;
+ int ret;
+
+ ret = regmap_read(bq2515x->regmap, BQ2515X_VBAT_CTRL, &vbat_reg_code);
+ if (ret)
+ return ret;
+
+ return BQ2515X_VBAT_BASE_VOLT + vbat_reg_code * BQ2515X_UV_FACTOR;
+}
+
+static int bq2515x_set_batt_reg(struct bq2515x_device *bq2515x, int val)
+{
+ int vbat_reg_code;
+
+ if (val > BQ2515X_VBAT_REG_MAX || val < BQ2515X_VBAT_REG_MIN)
+ return -EINVAL;
+
+ vbat_reg_code = (val - BQ2515X_VBAT_BASE_VOLT) / BQ2515X_UV_FACTOR;
+
+ return regmap_write(bq2515x->regmap, BQ2515X_VBAT_CTRL, vbat_reg_code);
+}
+
+static int bq2515x_get_ilim_lvl(struct bq2515x_device *bq2515x)
+{
+ int ret;
+ int val;
+
+ ret = regmap_read(bq2515x->regmap, BQ2515X_ILIMCTRL, &val);
+ if (ret)
+ return ret;
+
+ return bq2515x_ilim_lvl_values[val & BQ2515X_ILIM_MASK];
+}
+
+static int bq2515x_set_ilim_lvl(struct bq2515x_device *bq2515x, int val)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(bq2515x_ilim_lvl_values); i++) {
+ if (val == bq2515x_ilim_lvl_values[i])
+ break;
+
+ if (val > bq2515x_ilim_lvl_values[i - 1] &&
+ val < bq2515x_ilim_lvl_values[i]) {
+ if (val - bq2515x_ilim_lvl_values[i - 1] <
+ bq2515x_ilim_lvl_values[i] - val) {
+ i = i - 1;
+ break;
+ }
+ }
+ }
+
+ return regmap_write(bq2515x->regmap, BQ2515X_ILIMCTRL, i);
+}
+
+static int bq2515x_power_supply_property_is_writeable(struct power_supply *psy,
+ enum power_supply_property prop)
+{
+ switch (prop) {
+ case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
+ case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE:
+ case POWER_SUPPLY_PROP_CHARGE_NOW:
+ return true;
+ default:
+ return false;
+ }
+}
+
+static int bq2515x_charger_get_health(struct bq2515x_device *bq2515x,
+ union power_supply_propval *val)
+{
+ int health;
+ int ret;
+ int v;
+
+ if (!bq2515x_is_ps_online(bq2515x))
+ bq2515x_wake_up(bq2515x);
+
+ ret = regmap_read(bq2515x->regmap, BQ2515X_FLAG1, &v);
+ if (ret)
+ return -EIO;
+
+ if (v & BQ2515X_HEALTH_MASK) {
+ switch (v & BQ2515X_HEALTH_MASK) {
+ case BQ2515X_HOT_FLAG:
+ health = POWER_SUPPLY_HEALTH_HOT;
+ break;
+ case BQ2515X_WARM_FLAG:
+ health = POWER_SUPPLY_HEALTH_WARM;
+ break;
+ case BQ2515X_COOL_FLAG:
+ health = POWER_SUPPLY_HEALTH_COOL;
+ break;
+ case BQ2515X_COLD_FLAG:
+ health = POWER_SUPPLY_HEALTH_COLD;
+ break;
+ default:
+ health = POWER_SUPPLY_HEALTH_UNKNOWN;
+ }
+ } else if (v & BQ2515X_OVERVOLT_MASK) {
+ health = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
+ } else {
+ health = POWER_SUPPLY_HEALTH_GOOD;
+ }
+
+ ret = regmap_read(bq2515x->regmap, BQ2515X_FLAG3, &v);
+ if (v & BQ2515X_SAFETY_TIMER_EXP)
+ health = POWER_SUPPLY_HEALTH_SAFETY_TIMER_EXPIRE;
+
+ val->intval = health;
+
+ return 0;
+}
+
+static int bq2515x_set_charge_enable(struct bq2515x_device *bq2515x, int val)
+{
+ if (bq2515x->ce_gpio)
+ gpiod_set_value(bq2515x->ce_gpio, val);
+
+ return 0;
+}
+
+static int bq2515x_get_charge_enable(struct bq2515x_device *bq2515x)
+{
+ int charge_value;
+
+ if (bq2515x->ce_gpio)
+ charge_value = gpiod_get_value(bq2515x->ce_gpio);
+ else
+ return -EINVAL;
+
+ return charge_value;
+}
+
+static int bq2515x_mains_set_property(struct power_supply *psy,
+ enum power_supply_property prop,
+ const union power_supply_propval *val)
+{
+ struct bq2515x_device *bq2515x = power_supply_get_drvdata(psy);
+ int ret;
+
+ switch (prop) {
+ case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
+ ret = bq2515x_set_ilim_lvl(bq2515x, val->intval);
+ break;
+ case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE:
+ ret = bq2515x_set_batt_reg(bq2515x, val->intval);
+ break;
+ case POWER_SUPPLY_PROP_CHARGE_NOW:
+ ret = bq2515x_set_charge_enable(bq2515x, val->intval);
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return ret;
+}
+
+static int bq2515x_mains_get_property(struct power_supply *psy,
+ enum power_supply_property prop,
+ union power_supply_propval *val)
+{
+ struct bq2515x_device *bq2515x = power_supply_get_drvdata(psy);
+ int ret;
+
+ switch (prop) {
+ case POWER_SUPPLY_PROP_ONLINE:
+ val->intval = bq2515x->mains_online;
+ break;
+ case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT:
+ ret = get_const_charge_current(bq2515x);
+ if (ret < 0)
+ return ret;
+
+ val->intval = ret;
+ break;
+ case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
+ ret = bq2515x_get_ilim_lvl(bq2515x);
+ if (ret < 0)
+ return ret;
+
+ val->intval = ret;
+ break;
+ case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE:
+ ret = bq2515x_get_batt_reg(bq2515x);
+ if (ret < 0)
+ return ret;
+
+ val->intval = ret;
+ break;
+ case POWER_SUPPLY_PROP_MODEL_NAME:
+ val->strval = bq2515x->model_name;
+ ret = 0;
+ break;
+ case POWER_SUPPLY_PROP_MANUFACTURER:
+ val->strval = BQ2515X_MANUFACTURER;
+ ret = 0;
+ break;
+ case POWER_SUPPLY_PROP_VOLTAGE_MAX:
+ val->intval = BQ2515X_VBAT_REG_MAX;
+ break;
+ case POWER_SUPPLY_PROP_VOLTAGE_MIN:
+ val->intval = BQ2515X_VBAT_REG_MIN;
+ break;
+ case POWER_SUPPLY_PROP_CHARGE_NOW:
+ ret = bq2515x_get_charge_enable(bq2515x);
+ if (ret < 0)
+ return ret;
+
+ val->intval = ret;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int bq2515x_battery_get_property(struct power_supply *psy,
+ enum power_supply_property prop,
+ union power_supply_propval *val)
+{
+ struct bq2515x_device *bq2515x = power_supply_get_drvdata(psy);
+ int ret;
+
+ ret = bq2515x_update_ps_status(bq2515x);
+ if (ret < 0)
+ return ret;
+
+ switch (prop) {
+ case POWER_SUPPLY_PROP_STATUS:
+ if (!bq2515x_is_ps_online(bq2515x)) {
+ val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
+ break;
+ }
+
+ ret = bq2515x_charging_status(bq2515x, val);
+ break;
+ case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+ ret = get_const_charge_voltage(bq2515x);
+ if (ret < 0)
+ return ret;
+
+ val->intval = ret;
+ break;
+ case POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN:
+ val->intval = bq2515x->voltage_min_design;
+ break;
+ case POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN:
+ val->intval = bq2515x->voltage_max_design;
+ break;
+ case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN:
+ val->intval = bq2515x->charge_full_design;
+ break;
+ case POWER_SUPPLY_PROP_HEALTH:
+ ret = bq2515x_charger_get_health(bq2515x, val);
+ if (ret)
+ val->intval = POWER_SUPPLY_HEALTH_UNKNOWN;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static enum power_supply_property bq2515x_battery_properties[] = {
+ POWER_SUPPLY_PROP_STATUS,
+ POWER_SUPPLY_PROP_HEALTH,
+ POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN,
+ POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN,
+ POWER_SUPPLY_PROP_VOLTAGE_NOW,
+ POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN,
+};
+
+static enum power_supply_property bq2515x_charger_properties[] = {
+ POWER_SUPPLY_PROP_ONLINE,
+ POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT,
+ POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE,
+ POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
+ POWER_SUPPLY_PROP_VOLTAGE_MAX,
+ POWER_SUPPLY_PROP_VOLTAGE_MIN,
+ POWER_SUPPLY_PROP_CHARGE_NOW,
+ POWER_SUPPLY_PROP_MODEL_NAME,
+ POWER_SUPPLY_PROP_MANUFACTURER,
+};
+
+static struct power_supply_desc bq2515x_mains_desc = {
+ .name = "bq2515x-mains",
+ .type = POWER_SUPPLY_TYPE_MAINS,
+ .get_property = bq2515x_mains_get_property,
+ .set_property = bq2515x_mains_set_property,
+ .properties = bq2515x_charger_properties,
+ .num_properties = ARRAY_SIZE(bq2515x_charger_properties),
+ .property_is_writeable = bq2515x_power_supply_property_is_writeable,
+
+};
+
+static struct power_supply_desc bq2515x_battery_desc = {
+ .name = "bq2515x-battery",
+ .type = POWER_SUPPLY_TYPE_BATTERY,
+ .get_property = bq2515x_battery_get_property,
+ .properties = bq2515x_battery_properties,
+ .num_properties = ARRAY_SIZE(bq2515x_battery_properties),
+};
+
+
+static int bq2515x_power_supply_register(struct bq2515x_device *bq2515x)
+{
+ struct power_supply_config psy_cfg = { .drv_data = bq2515x, };
+
+ bq2515x->mains = devm_power_supply_register(bq2515x->dev,
+ &bq2515x_mains_desc,
+ &psy_cfg);
+ if (IS_ERR(bq2515x->mains))
+ return -EINVAL;
+
+ bq2515x->battery = devm_power_supply_register(bq2515x->dev,
+ &bq2515x_battery_desc,
+ &psy_cfg);
+ if (IS_ERR(bq2515x->battery))
+ return -EINVAL;
+
+ return 0;
+}
+
+static int bq2515x_hw_init(struct bq2515x_device *bq2515x)
+{
+ int ret = 0;
+
+ if (bq2515x->init_data.ichg)
+ ret = bq2515x_set_ilim_lvl(bq2515x, bq2515x->init_data.ichg);
+
+ if (ret)
+ goto err_out;
+
+ if (bq2515x->init_data.vreg)
+ ret = bq2515x_set_batt_reg(bq2515x, bq2515x->init_data.vreg);
+
+err_out:
+ return ret;
+}
+
+static int bq2515x_read_properties(struct bq2515x_device *bq2515x)
+{
+ int ret;
+
+ ret = device_property_read_u32(bq2515x->dev,
+ "constant-charge-current-max-microamp",
+ &bq2515x->init_data.ichg);
+ if (ret)
+ bq2515x->init_data.ichg = bq2515x_ilim_lvl_values[1];
+
+ ret = device_property_read_u32(bq2515x->dev,
+ "constant-charge-voltage-max-microvolt",
+ &bq2515x->init_data.vreg);
+ if (ret)
+ bq2515x->init_data.vreg = 4200000;
+
+
+ bq2515x->pg_gpio = devm_gpiod_get_optional(bq2515x->dev,
+ "pg", GPIOD_IN);
+ if (IS_ERR(bq2515x->pg_gpio))
+ dev_info(bq2515x->dev, "PG GPIO not defined");
+
+ bq2515x->reset_gpio = devm_gpiod_get_optional(bq2515x->dev,
+ "reset", GPIOD_OUT_LOW);
+
+ if (PTR_ERR(bq2515x->reset_gpio) == -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+
+ bq2515x->lp_gpio = devm_gpiod_get_optional(bq2515x->dev, "low-power",
+ GPIOD_OUT_LOW);
+ if (PTR_ERR(bq2515x->lp_gpio) == -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+
+ bq2515x->ce_gpio = devm_gpiod_get_optional(bq2515x->dev,
+ "charge-enable",
+ GPIOD_OUT_HIGH);
+ if (PTR_ERR(bq2515x->ce_gpio) == -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+
+ return ret;
+}
+
+static const struct regmap_config bq2515x_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+
+ .max_register = BQ2515X_DEVICE_ID,
+ .reg_defaults = bq2515x_reg_defs,
+ .num_reg_defaults = ARRAY_SIZE(bq2515x_reg_defs),
+ .cache_type = REGCACHE_RBTREE,
+};
+
+static int bq2515x_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct device *dev = &client->dev;
+ struct bq2515x_device *bq;
+ int ret;
+
+ bq = devm_kzalloc(dev, sizeof(*bq), GFP_KERNEL);
+ if (!bq)
+ return -ENOMEM;
+
+ bq->client = client;
+ bq->dev = dev;
+
+ mutex_init(&bq->lock);
+
+ bq->regmap = devm_regmap_init_i2c(client, &bq2515x_regmap_config);
+ if (IS_ERR(bq->regmap)) {
+ dev_err(dev, "failed to allocate register map\n");
+ return PTR_ERR(bq->regmap);
+ }
+
+ strncpy(bq->model_name, id->name, I2C_NAME_SIZE);
+
+ i2c_set_clientdata(client, bq);
+
+ ret = bq2515x_read_properties(bq);
+ if (ret < 0) {
+ dev_err(dev, "Failed to register power supply %d\n", ret);
+ return ret;
+ }
+
+ ret = bq2515x_hw_init(bq);
+ if (ret < 0) {
+ dev_err(dev, "Cannot initialize the chip.\n");
+ return ret;
+ }
+
+ return bq2515x_power_supply_register(bq);
+}
+
+static const struct i2c_device_id bq2515x_i2c_ids[] = {
+ { "bq25150", 0 },
+ { "bq25155", 1 },
+ {},
+};
+MODULE_DEVICE_TABLE(i2c, bq2515x_i2c_ids);
+
+static const struct of_device_id bq2515x_of_match[] = {
+ { .compatible = "ti,bq25150", },
+ { .compatible = "ti,bq25155", },
+ { },
+};
+MODULE_DEVICE_TABLE(of, bq2515x_of_match);
+
+static struct i2c_driver bq2515x_driver = {
+ .driver = {
+ .name = "bq2515x-charger",
+ .of_match_table = bq2515x_of_match,
+ },
+ .probe = bq2515x_probe,
+ .id_table = bq2515x_i2c_ids,
+};
+module_i2c_driver(bq2515x_driver);
+
+MODULE_AUTHOR("Dan Murphy <[email protected]>");
+MODULE_DESCRIPTION("BQ2515X charger driver");
+MODULE_LICENSE("GPL v2");
--
2.25.0

2020-01-16 17:56:28

by Dan Murphy

[permalink] [raw]
Subject: [PATCH v4 1/4] power: supply: core: Update sysfs-class-power ABI document

Add the "Over Current" string to /sys/class/power_supply/<supply_name>/health
description.

Fixes: e3e83cc601e57 ("power: supply: core: Add POWER_SUPPLY_HEALTH_OVERCURRENT constant")
Signed-off-by: Dan Murphy <[email protected]>
---
Documentation/ABI/testing/sysfs-class-power | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/ABI/testing/sysfs-class-power b/Documentation/ABI/testing/sysfs-class-power
index 27edc06e2495..bf3b48f022dc 100644
--- a/Documentation/ABI/testing/sysfs-class-power
+++ b/Documentation/ABI/testing/sysfs-class-power
@@ -189,7 +189,8 @@ Description:
Access: Read
Valid values: "Unknown", "Good", "Overheat", "Dead",
"Over voltage", "Unspecified failure", "Cold",
- "Watchdog timer expire", "Safety timer expire"
+ "Watchdog timer expire", "Safety timer expire",
+ "Over current"

What: /sys/class/power_supply/<supply_name>/precharge_current
Date: June 2017
--
2.25.0

2020-01-16 22:11:47

by Dan Murphy

[permalink] [raw]
Subject: [PATCH v4 2/4] power_supply: Add additional health properties to the header

Add HEALTH_WARM, HEALTH_COOL and HEALTH_HOT to the health enum.

Signed-off-by: Dan Murphy <[email protected]>
---
Documentation/ABI/testing/sysfs-class-power | 2 +-
drivers/power/supply/power_supply_sysfs.c | 2 +-
include/linux/power_supply.h | 3 +++
3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-class-power b/Documentation/ABI/testing/sysfs-class-power
index bf3b48f022dc..9f3fd01a9373 100644
--- a/Documentation/ABI/testing/sysfs-class-power
+++ b/Documentation/ABI/testing/sysfs-class-power
@@ -190,7 +190,7 @@ Description:
Valid values: "Unknown", "Good", "Overheat", "Dead",
"Over voltage", "Unspecified failure", "Cold",
"Watchdog timer expire", "Safety timer expire",
- "Over current"
+ "Over current", "Warm", "Cool", "Hot"

What: /sys/class/power_supply/<supply_name>/precharge_current
Date: June 2017
diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
index f37ad4eae60b..d0d549611794 100644
--- a/drivers/power/supply/power_supply_sysfs.c
+++ b/drivers/power/supply/power_supply_sysfs.c
@@ -61,7 +61,7 @@ static const char * const power_supply_charge_type_text[] = {
static const char * const power_supply_health_text[] = {
"Unknown", "Good", "Overheat", "Dead", "Over voltage",
"Unspecified failure", "Cold", "Watchdog timer expire",
- "Safety timer expire", "Over current"
+ "Safety timer expire", "Over current", "Warm", "Cool", "Hot"
};

static const char * const power_supply_technology_text[] = {
diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index 28413f737e7d..bd0d3225f245 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -61,6 +61,9 @@ enum {
POWER_SUPPLY_HEALTH_WATCHDOG_TIMER_EXPIRE,
POWER_SUPPLY_HEALTH_SAFETY_TIMER_EXPIRE,
POWER_SUPPLY_HEALTH_OVERCURRENT,
+ POWER_SUPPLY_HEALTH_WARM,
+ POWER_SUPPLY_HEALTH_COOL,
+ POWER_SUPPLY_HEALTH_HOT,
};

enum {
--
2.25.0

2020-01-16 22:13:31

by Dan Murphy

[permalink] [raw]
Subject: [PATCH v4 3/4] dt-bindings: power: Add the bq2515x family dt bindings

Add the bindings for the bq25150 and bq25155 500mA
charging ICs from Texas Instruments.

Datasheet:
http://www.ti.com/lit/ds/symlink/bq25150.pdf
http://www.ti.com/lit/ds/symlink/bq25155.pdf

Signed-off-by: Dan Murphy <[email protected]>
---
.../bindings/power/supply/bq2515x.yaml | 85 +++++++++++++++++++
1 file changed, 85 insertions(+)
create mode 100644 Documentation/devicetree/bindings/power/supply/bq2515x.yaml

diff --git a/Documentation/devicetree/bindings/power/supply/bq2515x.yaml b/Documentation/devicetree/bindings/power/supply/bq2515x.yaml
new file mode 100644
index 000000000000..670952021e8d
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/supply/bq2515x.yaml
@@ -0,0 +1,85 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright (C) 2019 Texas Instruments Incorporated
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/power/supply/bq2515x.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: TI bq2515x 500-mA Linear charger family
+
+maintainers:
+ - Dan Murphy <[email protected]>
+
+description: |
+ The BQ2515x family is a highly integrated battery charge management IC that
+ integrates the most common functions for wearable devices, namely a charger,
+ an output voltage rail, ADC for battery and system monitoring, and
+ push-button controller.
+
+ Specifications about the charger can be found at:
+ http://www.ti.com/lit/ds/symlink/bq25150.pdf
+ http://www.ti.com/lit/ds/symlink/bq25155.pdf
+
+properties:
+ compatible:
+ enum:
+ - ti,bq25150
+ - ti,bq25155
+
+ reg:
+ maxItems: 1
+
+ pg-gpios:
+ description: |
+ GPIO used for connecting the bq2515x device PG (Power Good)
+ pin. This pin should be used if possible as this is the
+ recommended way to obtain the charger's input PG state.
+ If this pin is not specified a software-based approach for PG
+ detection is used.
+
+ reset-gpios:
+ description: |
+ GPIO used for hardware reset.
+
+ low-power-gpios:
+ description: |
+ GPIO used for low power mode of IC.
+
+ charge-enable-gpios:
+ description: |
+ GPIO used to turn on and off charging.
+
+ constant-charge-current-max-microamp:
+ description: |
+ Maximum charging current in micro Amps.
+ minimum: 50000
+ maximum: 600000
+
+ constant-charge-voltage-max-microvolt:
+ description: |
+ Maximum charging voltage in micro volts.
+ minimum: 3600000
+ maximum: 4600000
+
+required:
+ - compatible
+ - reg
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ i2c0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ charger@6b {
+ compatible = "ti,bq25150";
+ reg = <0x6b>;
+
+ pg-gpios = <&gpio1 28 GPIO_ACTIVE_HIGH>;
+ reset-gpios = <&gpio0 14 GPIO_ACTIVE_HIGH>;
+ low-power-gpios = <&gpio0 15 GPIO_ACTIVE_HIGH>;
+ charge-enable-gpios = <&gpio0 13 GPIO_ACTIVE_LOW>;
+ constant-charge-current-max-microamp = <300000>;
+ voltage-max-design-microvolt = <4200000>;
+ };
+ };
--
2.25.0

2020-01-17 03:54:31

by Sebastian Reichel

[permalink] [raw]
Subject: Re: [PATCH v4 2/4] power_supply: Add additional health properties to the header

Hi,

On Thu, Jan 16, 2020 at 11:50:37AM -0600, Dan Murphy wrote:
> Add HEALTH_WARM, HEALTH_COOL and HEALTH_HOT to the health enum.
>
> Signed-off-by: Dan Murphy <[email protected]>
> ---

Looks good. But I will not merge it without a user and have comments
for the driver.

-- Sebastian

> Documentation/ABI/testing/sysfs-class-power | 2 +-
> drivers/power/supply/power_supply_sysfs.c | 2 +-
> include/linux/power_supply.h | 3 +++
> 3 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-class-power b/Documentation/ABI/testing/sysfs-class-power
> index bf3b48f022dc..9f3fd01a9373 100644
> --- a/Documentation/ABI/testing/sysfs-class-power
> +++ b/Documentation/ABI/testing/sysfs-class-power
> @@ -190,7 +190,7 @@ Description:
> Valid values: "Unknown", "Good", "Overheat", "Dead",
> "Over voltage", "Unspecified failure", "Cold",
> "Watchdog timer expire", "Safety timer expire",
> - "Over current"
> + "Over current", "Warm", "Cool", "Hot"
>
> What: /sys/class/power_supply/<supply_name>/precharge_current
> Date: June 2017
> diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
> index f37ad4eae60b..d0d549611794 100644
> --- a/drivers/power/supply/power_supply_sysfs.c
> +++ b/drivers/power/supply/power_supply_sysfs.c
> @@ -61,7 +61,7 @@ static const char * const power_supply_charge_type_text[] = {
> static const char * const power_supply_health_text[] = {
> "Unknown", "Good", "Overheat", "Dead", "Over voltage",
> "Unspecified failure", "Cold", "Watchdog timer expire",
> - "Safety timer expire", "Over current"
> + "Safety timer expire", "Over current", "Warm", "Cool", "Hot"
> };
>
> static const char * const power_supply_technology_text[] = {
> diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
> index 28413f737e7d..bd0d3225f245 100644
> --- a/include/linux/power_supply.h
> +++ b/include/linux/power_supply.h
> @@ -61,6 +61,9 @@ enum {
> POWER_SUPPLY_HEALTH_WATCHDOG_TIMER_EXPIRE,
> POWER_SUPPLY_HEALTH_SAFETY_TIMER_EXPIRE,
> POWER_SUPPLY_HEALTH_OVERCURRENT,
> + POWER_SUPPLY_HEALTH_WARM,
> + POWER_SUPPLY_HEALTH_COOL,
> + POWER_SUPPLY_HEALTH_HOT,
> };
>
> enum {
> --
> 2.25.0
>


Attachments:
(No filename) (2.30 kB)
signature.asc (849.00 B)
Download all attachments

2020-01-17 03:55:11

by Sebastian Reichel

[permalink] [raw]
Subject: Re: [PATCH v4 3/4] dt-bindings: power: Add the bq2515x family dt bindings

Hi,

On Thu, Jan 16, 2020 at 11:50:38AM -0600, Dan Murphy wrote:
> Add the bindings for the bq25150 and bq25155 500mA
> charging ICs from Texas Instruments.
>
> Datasheet:
> http://www.ti.com/lit/ds/symlink/bq25150.pdf
> http://www.ti.com/lit/ds/symlink/bq25155.pdf
>
> Signed-off-by: Dan Murphy <[email protected]>
> ---
> .../bindings/power/supply/bq2515x.yaml | 85 +++++++++++++++++++
> 1 file changed, 85 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/power/supply/bq2515x.yaml
>
> diff --git a/Documentation/devicetree/bindings/power/supply/bq2515x.yaml b/Documentation/devicetree/bindings/power/supply/bq2515x.yaml
> new file mode 100644
> index 000000000000..670952021e8d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/supply/bq2515x.yaml
> @@ -0,0 +1,85 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright (C) 2019 Texas Instruments Incorporated
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/power/supply/bq2515x.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: TI bq2515x 500-mA Linear charger family
> +
> +maintainers:
> + - Dan Murphy <[email protected]>
> +
> +description: |
> + The BQ2515x family is a highly integrated battery charge management IC that
> + integrates the most common functions for wearable devices, namely a charger,
> + an output voltage rail, ADC for battery and system monitoring, and
> + push-button controller.
> +
> + Specifications about the charger can be found at:
> + http://www.ti.com/lit/ds/symlink/bq25150.pdf
> + http://www.ti.com/lit/ds/symlink/bq25155.pdf
> +
> +properties:
> + compatible:
> + enum:
> + - ti,bq25150
> + - ti,bq25155
> +
> + reg:
> + maxItems: 1
> +
> + pg-gpios:
> + description: |
> + GPIO used for connecting the bq2515x device PG (Power Good)
> + pin. This pin should be used if possible as this is the
> + recommended way to obtain the charger's input PG state.
> + If this pin is not specified a software-based approach for PG
> + detection is used.

Let's name this "ac-detect-gpios", which is easy to understand
an generic enough for all chargers.

> + reset-gpios:
> + description: |
> + GPIO used for hardware reset.
> +
> + low-power-gpios:
> + description: |
> + GPIO used for low power mode of IC.
> +
> + charge-enable-gpios:
> + description: |
> + GPIO used to turn on and off charging.
> +
> + constant-charge-current-max-microamp:
> + description: |
> + Maximum charging current in micro Amps.
> + minimum: 50000
> + maximum: 600000
> +
> + constant-charge-voltage-max-microvolt:
> + description: |
> + Maximum charging voltage in micro volts.
> + minimum: 3600000
> + maximum: 4600000

The above two are properties, that are coming from the battery.
Please use the simple-battery binding as described in this
file: Documentation/devicetree/bindings/power/supply/battery.txt

-- Sebastian

> +
> +required:
> + - compatible
> + - reg
> +
> +examples:
> + - |
> + #include <dt-bindings/gpio/gpio.h>
> + i2c0 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + charger@6b {
> + compatible = "ti,bq25150";
> + reg = <0x6b>;
> +
> + pg-gpios = <&gpio1 28 GPIO_ACTIVE_HIGH>;
> + reset-gpios = <&gpio0 14 GPIO_ACTIVE_HIGH>;
> + low-power-gpios = <&gpio0 15 GPIO_ACTIVE_HIGH>;
> + charge-enable-gpios = <&gpio0 13 GPIO_ACTIVE_LOW>;
> + constant-charge-current-max-microamp = <300000>;
> + voltage-max-design-microvolt = <4200000>;
> + };
> + };
> --
> 2.25.0
>


Attachments:
(No filename) (3.72 kB)
signature.asc (849.00 B)
Download all attachments

2020-01-17 04:48:23

by Sebastian Reichel

[permalink] [raw]
Subject: Re: [PATCH v4 1/4] power: supply: core: Update sysfs-class-power ABI document

Hi,

On Thu, Jan 16, 2020 at 11:50:36AM -0600, Dan Murphy wrote:
> Add the "Over Current" string to /sys/class/power_supply/<supply_name>/health
> description.
>
> Fixes: e3e83cc601e57 ("power: supply: core: Add POWER_SUPPLY_HEALTH_OVERCURRENT constant")
> Signed-off-by: Dan Murphy <[email protected]>
> ---

Thanks, queued.

-- Sebastian

> Documentation/ABI/testing/sysfs-class-power | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-class-power b/Documentation/ABI/testing/sysfs-class-power
> index 27edc06e2495..bf3b48f022dc 100644
> --- a/Documentation/ABI/testing/sysfs-class-power
> +++ b/Documentation/ABI/testing/sysfs-class-power
> @@ -189,7 +189,8 @@ Description:
> Access: Read
> Valid values: "Unknown", "Good", "Overheat", "Dead",
> "Over voltage", "Unspecified failure", "Cold",
> - "Watchdog timer expire", "Safety timer expire"
> + "Watchdog timer expire", "Safety timer expire",
> + "Over current"
>
> What: /sys/class/power_supply/<supply_name>/precharge_current
> Date: June 2017
> --
> 2.25.0
>


Attachments:
(No filename) (1.13 kB)
signature.asc (849.00 B)
Download all attachments

2020-01-17 05:08:43

by Sebastian Reichel

[permalink] [raw]
Subject: Re: [PATCH v4 4/4] power: supply: bq2515x: Introduce the bq2515x family

Hi Dan,

On Thu, Jan 16, 2020 at 11:50:39AM -0600, Dan Murphy wrote:
> Introduce the bq25150 and bq25155 supply chargers.
>
> Signed-off-by: Dan Murphy <[email protected]>
> ---
> drivers/power/supply/Kconfig | 8 +
> drivers/power/supply/Makefile | 1 +
> drivers/power/supply/bq2515x_charger.c | 739 +++++++++++++++++++++++++
> 3 files changed, 748 insertions(+)
> create mode 100644 drivers/power/supply/bq2515x_charger.c
>
> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
> index 27164a1d3c7c..ee9f4b29e0b7 100644
> --- a/drivers/power/supply/Kconfig
> +++ b/drivers/power/supply/Kconfig
> @@ -589,6 +589,14 @@ config CHARGER_BQ24735
> help
> Say Y to enable support for the TI BQ24735 battery charger.
>
> +config CHARGER_BQ2515X
> + tristate "TI BQ2515X battery charger family"
> + depends on I2C
> + depends on GPIOLIB || COMPILE_TEST
> + select REGMAP_I2C
> + help
> + Say Y to enable support for the TI BQ2515X battery charger.
> +
> config CHARGER_BQ25890
> tristate "TI BQ25890 battery charger driver"
> depends on I2C
> diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
> index 6c7da920ea83..8fcc175a7e22 100644
> --- a/drivers/power/supply/Makefile
> +++ b/drivers/power/supply/Makefile
> @@ -80,6 +80,7 @@ obj-$(CONFIG_CHARGER_BQ2415X) += bq2415x_charger.o
> obj-$(CONFIG_CHARGER_BQ24190) += bq24190_charger.o
> obj-$(CONFIG_CHARGER_BQ24257) += bq24257_charger.o
> obj-$(CONFIG_CHARGER_BQ24735) += bq24735-charger.o
> +obj-$(CONFIG_CHARGER_BQ2515X) += bq2515x_charger.o
> obj-$(CONFIG_CHARGER_BQ25890) += bq25890_charger.o
> obj-$(CONFIG_CHARGER_SMB347) += smb347-charger.o
> obj-$(CONFIG_CHARGER_TPS65090) += tps65090-charger.o
> diff --git a/drivers/power/supply/bq2515x_charger.c b/drivers/power/supply/bq2515x_charger.c
> new file mode 100644
> index 000000000000..dbf84bef4976
> --- /dev/null
> +++ b/drivers/power/supply/bq2515x_charger.c
> @@ -0,0 +1,739 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// BQ2515X Battery Charger Driver
> +// Copyright (C) 2019 Texas Instruments Incorporated - http://www.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/of_gpio.h>

Correct include is <linux/gpio/consumer.h>

> +#include <linux/power_supply.h>
> +#include <linux/regmap.h>
> +#include <linux/types.h>
> +
> +#define BQ2515X_MANUFACTURER "Texas Instruments"
> +
> +#define BQ2515X_STAT0 0x00
> +#define BQ2515X_STAT1 0x01
> +#define BQ2515X_STAT2 0x02
> +#define BQ2515X_FLAG0 0x03
> +#define BQ2515X_FLAG1 0x04
> +#define BQ2515X_FLAG2 0x05
> +#define BQ2515X_FLAG3 0x06
> +#define BQ2515X_MASK0 0x07
> +#define BQ2515X_MASK1 0x08
> +#define BQ2515X_MASK2 0x09
> +#define BQ2515X_MASK3 0x0a
> +#define BQ2515X_VBAT_CTRL 0x12
> +#define BQ2515X_ICHG_CTRL 0x13
> +#define BQ2515X_PCHRGCTRL 0x14
> +#define BQ2515X_TERMCTRL 0x15
> +#define BQ2515X_BUVLO 0x16
> +#define BQ2515X_CHARGERCTRL0 0x17
> +#define BQ2515X_CHARGERCTRL1 0x18
> +#define BQ2515X_ILIMCTRL 0x19
> +#define BQ2515X_LDOCTRL 0x1d
> +#define BQ2515X_MRCTRL 0x30
> +#define BQ2515X_ICCTRL0 0x35
> +#define BQ2515X_ICCTRL1 0x36
> +#define BQ2515X_ICCTRL2 0x37
> +#define BQ2515X_ADCCTRL0 0x40
> +#define BQ2515X_ADCCTRL1 0x41
> +#define BQ2515X_ADC_VBAT_M 0x42
> +#define BQ2515X_ADC_VBAT_L 0x43
> +#define BQ2515X_ADC_TS_M 0x44
> +#define BQ2515X_ADC_TS_L 0x45
> +#define BQ2515X_ADC_ICHG_M 0x46
> +#define BQ2515X_ADC_ICHG_L 0x47
> +#define BQ2515X_ADC_ADCIN_M 0x48
> +#define BQ2515X_ADC_ADCIN_L 0x49
> +#define BQ2515X_ADC_VIN_M 0x4a
> +#define BQ2515X_ADC_VIN_L 0x4b
> +#define BQ2515X_ADC_PMID_M 0x4c
> +#define BQ2515X_ADC_PMID_L 0x4d
> +#define BQ2515X_ADC_IIN_M 0x4e
> +#define BQ2515X_ADC_IIN_L 0x4f
> +#define BQ2515X_ADC_COMP1_M 0x52
> +#define BQ2515X_ADC_COMP1_L 0X53
> +#define BQ2515X_ADC_COMP2_M 0X54
> +#define BQ2515X_ADC_COMP2_L 0x55
> +#define BQ2515X_ADC_COMP3_M 0x56
> +#define BQ2515X_ADC_COMP3_L 0x57
> +#define BQ2515X_ADC_READ_EN 0x58
> +#define BQ2515X_TS_FASTCHGCTRL 0x61
> +#define BQ2515X_TS_COLD 0x62
> +#define BQ2515X_TS_COOL 0x63
> +#define BQ2515X_TS_WARM 0x64
> +#define BQ2515X_TS_HOT 0x65
> +#define BQ2515X_DEVICE_ID 0x6f
> +
> +#define BQ2515X_DIVISOR 65536
> +#define BQ2515X_VBAT_BASE_VOLT 3600000
> +#define BQ2515X_VBAT_REG_MAX 4600000
> +#define BQ2515X_VBAT_REG_MIN 3600000
> +#define BQ2515X_UV_FACTOR 10000
> +
> +#define BQ2515X_ILIM_150MA 0x2
> +#define BQ2515X_ILIM_MASK 0x7
> +#define BQ2515X_HEALTH_MASK 0xf
> +#define BQ2515X_OVERVOLT_MASK 0x80
> +
> +#define BQ2515X_HOT_FLAG BIT(0)
> +#define BQ2515X_WARM_FLAG BIT(1)
> +#define BQ2515X_COOL_FLAG BIT(2)
> +#define BQ2515X_COLD_FLAG BIT(3)
> +#define BQ2515X_SAFETY_TIMER_EXP BIT(5)
> +
> +#define BQ2515X_VIN_GOOD BIT(0)
> +#define BQ2515X_CHRG_DONE BIT(5)
> +#define BQ2515X_CV_CHRG_MODE BIT(6)
> +
> +static const int bq2515x_ilim_lvl_values[] = {
> + 50000, 100000, 150000, 200000, 300000, 400000, 500000, 600000
> +};
> +
> +/* initial field values, converted to register values */
> +struct bq2515x_init_data {
> + int ichg; /* charge current */
> + int vreg; /* regulation voltage */
> +};
> +
> +struct bq2515x_device {
> + struct power_supply *mains;
> + struct power_supply *battery;
> + struct i2c_client *client;
> + struct regmap *regmap;
> + struct device *dev;
> + struct mutex lock;
> +
> + struct gpio_desc *reset_gpio;
> + struct gpio_desc *lp_gpio;
> + struct gpio_desc *pg_gpio;
> + struct gpio_desc *ce_gpio;
> +
> + char model_name[I2C_NAME_SIZE];
> + int mains_online;

bool?

> + uint32_t voltage_min_design;
> + uint32_t voltage_max_design;
> + uint32_t charge_full_design;
> +
> + struct bq2515x_init_data init_data;
> +};
> +
> +static struct reg_default bq2515x_reg_defs[] = {
> + {BQ2515X_STAT0, 0xff},
> + {BQ2515X_STAT1, 0x0},
> + {BQ2515X_STAT2, 0x0},
> + {BQ2515X_FLAG0, 0x0},
> + {BQ2515X_FLAG1, 0x0},
> + {BQ2515X_FLAG2, 0x0},
> + {BQ2515X_FLAG3, 0x0},
> + {BQ2515X_MASK0, 0x0},
> + {BQ2515X_MASK1, 0x0},
> + {BQ2515X_MASK2, 0x0},
> + {BQ2515X_MASK3, 0x0},
> +};
> +
> +static bool bq2515x_is_ps_online(struct bq2515x_device *bq2515x)
> +{
> + return bq2515x->mains_online;
> +}
> +
> +static int bq2515x_wake_up(struct bq2515x_device *bq2515x)
> +{
> + int ret;
> + int val;
> +
> + /* Read the STAT register if we can read it then the device is out
> + * of ship mode. If the register cannot be read then attempt to wake
> + * it up and enable the ADC.
> + */
> + ret = regmap_read(bq2515x->regmap, BQ2515X_STAT0, &val);
> + if (!ret)
> + return ret;
> +
> + /* Need to toggle LP and MR here */
> + if (bq2515x->lp_gpio)
> + gpiod_direction_output(bq2515x->lp_gpio, 1);
> +
> + if (bq2515x->reset_gpio) {
> + gpiod_direction_output(bq2515x->lp_gpio, 0);
> + mdelay(2000);
> + gpiod_direction_output(bq2515x->lp_gpio, 1);
> + }

gpiod_set_value_cansleep()

> + return regmap_write(bq2515x->regmap, BQ2515X_ADC_READ_EN, BIT(3));
> +}
> +
> +static int bq2515x_update_ps_status(struct bq2515x_device *bq2515x)
> +{
> + bool dc = false;
> + unsigned int val;
> + int ret;
> +
> + if (bq2515x->pg_gpio)
> + val = gpiod_get_value(bq2515x->pg_gpio);

Should be ok to call gpiod_get_value_cansleep().

> + else {
> + ret = regmap_read(bq2515x->regmap, BQ2515X_STAT0, &val);
> + if (ret < 0)
> + return ret;
> + }
> +
> + dc = val & BQ2515X_VIN_GOOD;
> +
> + ret = bq2515x->mains_online != dc;
> +
> + bq2515x->mains_online = dc;
> +
> + return ret;
> +}
> +
> +static int get_const_charge_current(struct bq2515x_device *bq2515x)
> +{
> + int ret;
> + int iin_msb;
> + int iin_lsb;
> + u16 ichg_measurement;
> + int ilim_val, ichg_multiplier;
> +
> + if (!bq2515x_is_ps_online(bq2515x))
> + return -ENODATA;
> +
> + ret = regmap_read(bq2515x->regmap, BQ2515X_ADC_IIN_M, &iin_msb);
> + if (ret < 0)
> + return ret;
> +
> + ret = regmap_read(bq2515x->regmap, BQ2515X_ADC_IIN_L, &iin_lsb);
> + if (ret < 0)
> + return ret;
> +
> + ichg_measurement = (iin_msb << 8) | iin_lsb;
> + ret = regmap_read(bq2515x->regmap, BQ2515X_ILIMCTRL, &ilim_val);
> + if (ret < 0)
> + return ret;
> +
> + if (ilim_val >= BQ2515X_ILIM_150MA)
> + ichg_multiplier = 350;
> + else
> + ichg_multiplier = 750;
> +
> + return (ichg_measurement * 100 / BQ2515X_DIVISOR) * ichg_multiplier;
> +}
> +
> +static int get_const_charge_voltage(struct bq2515x_device *bq2515x)
> +{
> + int ret;
> + int vin_msb;
> + int vin_lsb;
> + u16 vbat_measurement;
> +
> + if (!bq2515x_is_ps_online(bq2515x))
> + bq2515x_wake_up(bq2515x);

Is it intentional, that get_const_charge_current returns -ENODATA
and this get_const_charge_voltage just wakes up the device without
AC being connected?

> + ret = regmap_read(bq2515x->regmap, BQ2515X_ADC_VBAT_M, &vin_msb);
> + if (ret)
> + return ret;
> +
> + ret = regmap_read(bq2515x->regmap, BQ2515X_ADC_VBAT_L, &vin_lsb);
> + if (ret)
> + return ret;
> +
> + vbat_measurement = (vin_msb << 8) | vin_lsb;
> + return ((vbat_measurement * BQ2515X_UV_FACTOR) / BQ2515X_DIVISOR) * 6;
> +}
> +
> +static int bq2515x_charging_status(struct bq2515x_device *bq2515x,
> + union power_supply_propval *val)
> +{
> + unsigned int status;
> + int ret;
> +
> + if (!bq2515x_is_ps_online(bq2515x))

val->intval = POWER_SUPPLY_STATUS_DISCHARGING;

> + return 0;
> +
> + ret = regmap_read(bq2515x->regmap, BQ2515X_STAT0, &status);
> + if (ret) {
> + val->intval = POWER_SUPPLY_STATUS_UNKNOWN;

You can drop val->intval assignment, since you return an error
anyways.

> + return ret;
> + }
> +
> + if (status & BQ2515X_CV_CHRG_MODE && status & BQ2515X_VIN_GOOD)
> + val->intval = POWER_SUPPLY_STATUS_CHARGING;
> + else if (status & BQ2515X_CHRG_DONE)
> + val->intval = POWER_SUPPLY_STATUS_FULL;
> + else if (status & BQ2515X_VIN_GOOD)
> + val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING;
> + else
> + val->intval = POWER_SUPPLY_STATUS_UNKNOWN;
> +
> + return ret;
> +}
> +
> +static int bq2515x_get_batt_reg(struct bq2515x_device *bq2515x)
> +{
> + int vbat_reg_code;
> + int ret;
> +
> + ret = regmap_read(bq2515x->regmap, BQ2515X_VBAT_CTRL, &vbat_reg_code);
> + if (ret)
> + return ret;
> +
> + return BQ2515X_VBAT_BASE_VOLT + vbat_reg_code * BQ2515X_UV_FACTOR;
> +}
> +
> +static int bq2515x_set_batt_reg(struct bq2515x_device *bq2515x, int val)
> +{
> + int vbat_reg_code;
> +
> + if (val > BQ2515X_VBAT_REG_MAX || val < BQ2515X_VBAT_REG_MIN)
> + return -EINVAL;
> +
> + vbat_reg_code = (val - BQ2515X_VBAT_BASE_VOLT) / BQ2515X_UV_FACTOR;
> +
> + return regmap_write(bq2515x->regmap, BQ2515X_VBAT_CTRL, vbat_reg_code);
> +}
> +
> +static int bq2515x_get_ilim_lvl(struct bq2515x_device *bq2515x)
> +{
> + int ret;
> + int val;
> +
> + ret = regmap_read(bq2515x->regmap, BQ2515X_ILIMCTRL, &val);
> + if (ret)
> + return ret;
> +
> + return bq2515x_ilim_lvl_values[val & BQ2515X_ILIM_MASK];
> +}
> +
> +static int bq2515x_set_ilim_lvl(struct bq2515x_device *bq2515x, int val)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(bq2515x_ilim_lvl_values); i++) {
> + if (val == bq2515x_ilim_lvl_values[i])
> + break;
> +
> + if (val > bq2515x_ilim_lvl_values[i - 1] &&
> + val < bq2515x_ilim_lvl_values[i]) {
> + if (val - bq2515x_ilim_lvl_values[i - 1] <
> + bq2515x_ilim_lvl_values[i] - val) {
> + i = i - 1;
> + break;
> + }
> + }
> + }
> +
> + return regmap_write(bq2515x->regmap, BQ2515X_ILIMCTRL, i);
> +}
> +
> +static int bq2515x_power_supply_property_is_writeable(struct power_supply *psy,
> + enum power_supply_property prop)
> +{
> + switch (prop) {
> + case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
> + case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE:
> + case POWER_SUPPLY_PROP_CHARGE_NOW:

You probably do not want to use this property, it is used by
batteries to expose the uAh currently available (i.e. 3000mAh /
7000mAh).

> + return true;
> + default:
> + return false;
> + }
> +}
> +
> +static int bq2515x_charger_get_health(struct bq2515x_device *bq2515x,
> + union power_supply_propval *val)
> +{
> + int health;
> + int ret;
> + int v;
> +
> + if (!bq2515x_is_ps_online(bq2515x))
> + bq2515x_wake_up(bq2515x);
> +
> + ret = regmap_read(bq2515x->regmap, BQ2515X_FLAG1, &v);
> + if (ret)
> + return -EIO;
> +
> + if (v & BQ2515X_HEALTH_MASK) {
> + switch (v & BQ2515X_HEALTH_MASK) {
> + case BQ2515X_HOT_FLAG:
> + health = POWER_SUPPLY_HEALTH_HOT;
> + break;
> + case BQ2515X_WARM_FLAG:
> + health = POWER_SUPPLY_HEALTH_WARM;
> + break;
> + case BQ2515X_COOL_FLAG:
> + health = POWER_SUPPLY_HEALTH_COOL;
> + break;
> + case BQ2515X_COLD_FLAG:
> + health = POWER_SUPPLY_HEALTH_COLD;
> + break;
> + default:
> + health = POWER_SUPPLY_HEALTH_UNKNOWN;
> + }
> + } else if (v & BQ2515X_OVERVOLT_MASK) {
> + health = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
> + } else {
> + health = POWER_SUPPLY_HEALTH_GOOD;
> + }
> +
> + ret = regmap_read(bq2515x->regmap, BQ2515X_FLAG3, &v);
> + if (v & BQ2515X_SAFETY_TIMER_EXP)
> + health = POWER_SUPPLY_HEALTH_SAFETY_TIMER_EXPIRE;
> +
> + val->intval = health;
> +
> + return 0;
> +}
> +
> +static int bq2515x_set_charge_enable(struct bq2515x_device *bq2515x, int val)
> +{
> + if (bq2515x->ce_gpio)
> + gpiod_set_value(bq2515x->ce_gpio, val);

gpiod_set_value_cansleep() ?

Also it should return without doing anything if the supplied gpio is
NULL, so you can drop the protection.

> +
> + return 0;
> +}
> +
> +static int bq2515x_get_charge_enable(struct bq2515x_device *bq2515x)
> +{
> + int charge_value;
> +
> + if (bq2515x->ce_gpio)
> + charge_value = gpiod_get_value(bq2515x->ce_gpio);
> + else
> + return -EINVAL;
> +
> + return charge_value;
> +}

Maybe just

{
if (!bq2515x->ce_gpio)
return -EINVAL;

return gpiod_get_value_cansleep(bq2515x->ce_gpio);
}

> +static int bq2515x_mains_set_property(struct power_supply *psy,
> + enum power_supply_property prop,
> + const union power_supply_propval *val)
> +{
> + struct bq2515x_device *bq2515x = power_supply_get_drvdata(psy);
> + int ret;
> +
> + switch (prop) {
> + case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
> + ret = bq2515x_set_ilim_lvl(bq2515x, val->intval);
> + break;
> + case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE:
> + ret = bq2515x_set_batt_reg(bq2515x, val->intval);
> + break;
> + case POWER_SUPPLY_PROP_CHARGE_NOW:
> + ret = bq2515x_set_charge_enable(bq2515x, val->intval);
> + break;

As I guessed the wrong property is used. Make the STATUS property
writable instead.

> + default:
> + return -EINVAL;
> + }
> +
> + return ret;
> +}
> +
> +static int bq2515x_mains_get_property(struct power_supply *psy,
> + enum power_supply_property prop,
> + union power_supply_propval *val)
> +{
> + struct bq2515x_device *bq2515x = power_supply_get_drvdata(psy);
> + int ret;

ret = 0;

> +
> + switch (prop) {
> + case POWER_SUPPLY_PROP_ONLINE:
> + val->intval = bq2515x->mains_online;
> + break;
> + case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT:
> + ret = get_const_charge_current(bq2515x);
> + if (ret < 0)
> + return ret;
> +
> + val->intval = ret;

if (!ret)
val->intval = ret;

> + break;
> + case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
> + ret = bq2515x_get_ilim_lvl(bq2515x);
> + if (ret < 0)
> + return ret;
> +
> + val->intval = ret;

(see above)

> + break;
> + case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE:
> + ret = bq2515x_get_batt_reg(bq2515x);
> + if (ret < 0)
> + return ret;
> +
> + val->intval = ret;

(see above)

> + break;
> + case POWER_SUPPLY_PROP_MODEL_NAME:
> + val->strval = bq2515x->model_name;
> + ret = 0;

ret = 0 can be removed (after my suggested change).

> + break;
> + case POWER_SUPPLY_PROP_MANUFACTURER:
> + val->strval = BQ2515X_MANUFACTURER;
> + ret = 0;

ret = 0 can be removed (after my suggested change).

> + break;
> + case POWER_SUPPLY_PROP_VOLTAGE_MAX:
> + val->intval = BQ2515X_VBAT_REG_MAX;
> + break;
> + case POWER_SUPPLY_PROP_VOLTAGE_MIN:
> + val->intval = BQ2515X_VBAT_REG_MIN;
> + break;
> + case POWER_SUPPLY_PROP_CHARGE_NOW:
> + ret = bq2515x_get_charge_enable(bq2515x);
> + if (ret < 0)
> + return ret;
> +
> + val->intval = ret;

(see above)

> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;

return ret;

> +}
> +
> +static int bq2515x_battery_get_property(struct power_supply *psy,
> + enum power_supply_property prop,
> + union power_supply_propval *val)
> +{
> + struct bq2515x_device *bq2515x = power_supply_get_drvdata(psy);
> + int ret;
> +
> + ret = bq2515x_update_ps_status(bq2515x);
> + if (ret < 0)
> + return ret;
> +
> + switch (prop) {
> + case POWER_SUPPLY_PROP_STATUS:
> + if (!bq2515x_is_ps_online(bq2515x)) {
> + val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
> + break;
> + }
> +
> + ret = bq2515x_charging_status(bq2515x, val);
> + break;
> + case POWER_SUPPLY_PROP_VOLTAGE_NOW:
> + ret = get_const_charge_voltage(bq2515x);
> + if (ret < 0)
> + return ret;
> +
> + val->intval = ret;
> + break;
> + case POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN:
> + val->intval = bq2515x->voltage_min_design;
> + break;
> + case POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN:
> + val->intval = bq2515x->voltage_max_design;
> + break;
> + case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN:
> + val->intval = bq2515x->charge_full_design;
> + break;
> + case POWER_SUPPLY_PROP_HEALTH:
> + ret = bq2515x_charger_get_health(bq2515x, val);
> + if (ret)
> + val->intval = POWER_SUPPLY_HEALTH_UNKNOWN;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static enum power_supply_property bq2515x_battery_properties[] = {
> + POWER_SUPPLY_PROP_STATUS,
> + POWER_SUPPLY_PROP_HEALTH,
> + POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN,
> + POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN,
> + POWER_SUPPLY_PROP_VOLTAGE_NOW,
> + POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN,
> +};
> +
> +static enum power_supply_property bq2515x_charger_properties[] = {
> + POWER_SUPPLY_PROP_ONLINE,
> + POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT,
> + POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE,
> + POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
> + POWER_SUPPLY_PROP_VOLTAGE_MAX,
> + POWER_SUPPLY_PROP_VOLTAGE_MIN,
> + POWER_SUPPLY_PROP_CHARGE_NOW,
> + POWER_SUPPLY_PROP_MODEL_NAME,
> + POWER_SUPPLY_PROP_MANUFACTURER,
> +};
> +
> +static struct power_supply_desc bq2515x_mains_desc = {
> + .name = "bq2515x-mains",
> + .type = POWER_SUPPLY_TYPE_MAINS,
> + .get_property = bq2515x_mains_get_property,
> + .set_property = bq2515x_mains_set_property,
> + .properties = bq2515x_charger_properties,
> + .num_properties = ARRAY_SIZE(bq2515x_charger_properties),
> + .property_is_writeable = bq2515x_power_supply_property_is_writeable,
> +
> +};
> +
> +static struct power_supply_desc bq2515x_battery_desc = {
> + .name = "bq2515x-battery",
> + .type = POWER_SUPPLY_TYPE_BATTERY,
> + .get_property = bq2515x_battery_get_property,
> + .properties = bq2515x_battery_properties,
> + .num_properties = ARRAY_SIZE(bq2515x_battery_properties),
> +};
> +
> +
> +static int bq2515x_power_supply_register(struct bq2515x_device *bq2515x)
> +{
> + struct power_supply_config psy_cfg = { .drv_data = bq2515x, };
> +
> + bq2515x->mains = devm_power_supply_register(bq2515x->dev,
> + &bq2515x_mains_desc,
> + &psy_cfg);
> + if (IS_ERR(bq2515x->mains))
> + return -EINVAL;
> +
> + bq2515x->battery = devm_power_supply_register(bq2515x->dev,
> + &bq2515x_battery_desc,
> + &psy_cfg);
> + if (IS_ERR(bq2515x->battery))
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +static int bq2515x_hw_init(struct bq2515x_device *bq2515x)
> +{
> + int ret = 0;
> +
> + if (bq2515x->init_data.ichg)
> + ret = bq2515x_set_ilim_lvl(bq2515x, bq2515x->init_data.ichg);
> +
> + if (ret)
> + goto err_out;
> +
> + if (bq2515x->init_data.vreg)
> + ret = bq2515x_set_batt_reg(bq2515x, bq2515x->init_data.vreg);
> +
> +err_out:
> + return ret;
> +}
> +
> +static int bq2515x_read_properties(struct bq2515x_device *bq2515x)
> +{
> + int ret;
> +
> + ret = device_property_read_u32(bq2515x->dev,
> + "constant-charge-current-max-microamp",
> + &bq2515x->init_data.ichg);
> + if (ret)
> + bq2515x->init_data.ichg = bq2515x_ilim_lvl_values[1];
> +
> + ret = device_property_read_u32(bq2515x->dev,
> + "constant-charge-voltage-max-microvolt",
> + &bq2515x->init_data.vreg);
> + if (ret)
> + bq2515x->init_data.vreg = 4200000;
> +
> +
> + bq2515x->pg_gpio = devm_gpiod_get_optional(bq2515x->dev,
> + "pg", GPIOD_IN);
> + if (IS_ERR(bq2515x->pg_gpio))
> + dev_info(bq2515x->dev, "PG GPIO not defined");

Let's name the GPIO "ac-detect" in DT, which is descriptive.

> + bq2515x->reset_gpio = devm_gpiod_get_optional(bq2515x->dev,
> + "reset", GPIOD_OUT_LOW);
> +
> + if (PTR_ERR(bq2515x->reset_gpio) == -EPROBE_DEFER)
> + return -EPROBE_DEFER;
> +
> + bq2515x->lp_gpio = devm_gpiod_get_optional(bq2515x->dev, "low-power",
> + GPIOD_OUT_LOW);
> + if (PTR_ERR(bq2515x->lp_gpio) == -EPROBE_DEFER)
> + return -EPROBE_DEFER;
> +
> + bq2515x->ce_gpio = devm_gpiod_get_optional(bq2515x->dev,
> + "charge-enable",
> + GPIOD_OUT_HIGH);
> + if (PTR_ERR(bq2515x->ce_gpio) == -EPROBE_DEFER)
> + return -EPROBE_DEFER;

You silently drop any error from devm_gpiod_get_optional(),
which does not seem like a good idea. I suggest to just
error out on any problem, since there will be only errors
when the GPIO is described in DT (or ACPI/...).

> + return ret;
> +}
> +
> +static const struct regmap_config bq2515x_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> +
> + .max_register = BQ2515X_DEVICE_ID,
> + .reg_defaults = bq2515x_reg_defs,
> + .num_reg_defaults = ARRAY_SIZE(bq2515x_reg_defs),
> + .cache_type = REGCACHE_RBTREE,
> +};

Thanks for using regmap :)

> +static int bq2515x_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct device *dev = &client->dev;
> + struct bq2515x_device *bq;
> + int ret;
> +
> + bq = devm_kzalloc(dev, sizeof(*bq), GFP_KERNEL);
> + if (!bq)
> + return -ENOMEM;
> +
> + bq->client = client;
> + bq->dev = dev;
> +
> + mutex_init(&bq->lock);
> +
> + bq->regmap = devm_regmap_init_i2c(client, &bq2515x_regmap_config);
> + if (IS_ERR(bq->regmap)) {
> + dev_err(dev, "failed to allocate register map\n");
> + return PTR_ERR(bq->regmap);
> + }
> +
> + strncpy(bq->model_name, id->name, I2C_NAME_SIZE);
> +
> + i2c_set_clientdata(client, bq);
> +
> + ret = bq2515x_read_properties(bq);
> + if (ret < 0) {
> + dev_err(dev, "Failed to register power supply %d\n", ret);

Wrong error message?

> + return ret;
> + }
> +
> + ret = bq2515x_hw_init(bq);
> + if (ret < 0) {
> + dev_err(dev, "Cannot initialize the chip.\n");
> + return ret;
> + }
> +
> + return bq2515x_power_supply_register(bq);
> +}
> +
> +static const struct i2c_device_id bq2515x_i2c_ids[] = {
> + { "bq25150", 0 },
> + { "bq25155", 1 },
> + {},
> +};
> +MODULE_DEVICE_TABLE(i2c, bq2515x_i2c_ids);
> +
> +static const struct of_device_id bq2515x_of_match[] = {
> + { .compatible = "ti,bq25150", },
> + { .compatible = "ti,bq25155", },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, bq2515x_of_match);
> +
> +static struct i2c_driver bq2515x_driver = {
> + .driver = {
> + .name = "bq2515x-charger",
> + .of_match_table = bq2515x_of_match,
> + },
> + .probe = bq2515x_probe,
> + .id_table = bq2515x_i2c_ids,
> +};
> +module_i2c_driver(bq2515x_driver);
> +
> +MODULE_AUTHOR("Dan Murphy <[email protected]>");
> +MODULE_DESCRIPTION("BQ2515X charger driver");
> +MODULE_LICENSE("GPL v2");

So apart from the things above, I have two general issues:

1. Exposing a battery device from the charger should be avoided.
Userspace assumes that every battery device is a separate
device. When the charger exposes a battery there are to devices
for the same physical device.

2. The properties "constant-charge-current-max-microamp" and
"constant-charge-voltage-max-microvolt" are battery properties,
that should be put into a battery node and consumed by the
charger. There is a nice helper function available to do this:

power_supply_get_battery_info()

The idea is to improve this in the future, so that a fuel
gauge consumes this and becomes a smart battery. Then the
charger asks the smart battery for any required battery
information. This is not yet ready, though. So for now the
charger should fetch the information from the simple-battery
node.

-- Sebastian


Attachments:
(No filename) (24.76 kB)
signature.asc (849.00 B)
Download all attachments

2020-03-03 21:49:08

by Guru Das Srinagesh

[permalink] [raw]
Subject: Re: [PATCH v4 2/4] power_supply: Add additional health properties to the header

On Thu, Jan 16, 2020 at 11:50:37AM -0600, Dan Murphy wrote:
> Add HEALTH_WARM, HEALTH_COOL and HEALTH_HOT to the health enum.
>
> Signed-off-by: Dan Murphy <[email protected]>
> ---
> Documentation/ABI/testing/sysfs-class-power | 2 +-
> drivers/power/supply/power_supply_sysfs.c | 2 +-
> include/linux/power_supply.h | 3 +++
> 3 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-class-power b/Documentation/ABI/testing/sysfs-class-power
> index bf3b48f022dc..9f3fd01a9373 100644
> --- a/Documentation/ABI/testing/sysfs-class-power
> +++ b/Documentation/ABI/testing/sysfs-class-power
> @@ -190,7 +190,7 @@ Description:
> Valid values: "Unknown", "Good", "Overheat", "Dead",
> "Over voltage", "Unspecified failure", "Cold",
> "Watchdog timer expire", "Safety timer expire",
> - "Over current"
> + "Over current", "Warm", "Cool", "Hot"
>
> What: /sys/class/power_supply/<supply_name>/precharge_current
> Date: June 2017
> diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
> index f37ad4eae60b..d0d549611794 100644
> --- a/drivers/power/supply/power_supply_sysfs.c
> +++ b/drivers/power/supply/power_supply_sysfs.c
> @@ -61,7 +61,7 @@ static const char * const power_supply_charge_type_text[] = {
> static const char * const power_supply_health_text[] = {
> "Unknown", "Good", "Overheat", "Dead", "Over voltage",
> "Unspecified failure", "Cold", "Watchdog timer expire",
> - "Safety timer expire", "Over current"
> + "Safety timer expire", "Over current", "Warm", "Cool", "Hot"
> };
>
> static const char * const power_supply_technology_text[] = {
> diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
> index 28413f737e7d..bd0d3225f245 100644
> --- a/include/linux/power_supply.h
> +++ b/include/linux/power_supply.h
> @@ -61,6 +61,9 @@ enum {
> POWER_SUPPLY_HEALTH_WATCHDOG_TIMER_EXPIRE,
> POWER_SUPPLY_HEALTH_SAFETY_TIMER_EXPIRE,
> POWER_SUPPLY_HEALTH_OVERCURRENT,
> + POWER_SUPPLY_HEALTH_WARM,
> + POWER_SUPPLY_HEALTH_COOL,
> + POWER_SUPPLY_HEALTH_HOT,
> };
>
> enum {

Tested-by: Guru Das Srinagesh <[email protected]>

Applied this patch and verified that the new enums and the display
strings match, using a downstream driver (currently under
development).

2020-03-06 23:56:37

by Sandeep Patil

[permalink] [raw]
Subject: Re: [PATCH v4 2/4] power_supply: Add additional health properties to the header

Hi Sebastian,

On Fri, Jan 17, 2020 at 02:06:58AM +0100, Sebastian Reichel wrote:
> Hi,
>
> On Thu, Jan 16, 2020 at 11:50:37AM -0600, Dan Murphy wrote:
> > Add HEALTH_WARM, HEALTH_COOL and HEALTH_HOT to the health enum.
> >
> > Signed-off-by: Dan Murphy <[email protected]>
> > ---
>
> Looks good. But I will not merge it without a user and have comments
> for the driver.

Android has been looking for these properties for a while now [1].
It was added[2] when we saw that the manufacturers were implementing these
properties in the driver. I didn't know the properties were absent upstream
until yesterday. Somebody pointed out in our ongoing effort to make sure
all core kernel changes that android depends on are present upstream.

I think those values are also propagated in application facing APIs in
Android (but I am not sure yet, let me know if that's something you want
to find out).

I wanted to chime in and present you a 'user' for this if that helps.

Cc: [email protected]

- ssp

1. https://android.googlesource.com/platform/system/core/+/refs/heads/master/healthd/BatteryMonitor.cpp#162
2. https://android-review.googlesource.com/c/platform/system/core/+/414481

>
> -- Sebastian
>
> > Documentation/ABI/testing/sysfs-class-power | 2 +-
> > drivers/power/supply/power_supply_sysfs.c | 2 +-
> > include/linux/power_supply.h | 3 +++
> > 3 files changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/ABI/testing/sysfs-class-power b/Documentation/ABI/testing/sysfs-class-power
> > index bf3b48f022dc..9f3fd01a9373 100644
> > --- a/Documentation/ABI/testing/sysfs-class-power
> > +++ b/Documentation/ABI/testing/sysfs-class-power
> > @@ -190,7 +190,7 @@ Description:
> > Valid values: "Unknown", "Good", "Overheat", "Dead",
> > "Over voltage", "Unspecified failure", "Cold",
> > "Watchdog timer expire", "Safety timer expire",
> > - "Over current"
> > + "Over current", "Warm", "Cool", "Hot"
> >
> > What: /sys/class/power_supply/<supply_name>/precharge_current
> > Date: June 2017
> > diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
> > index f37ad4eae60b..d0d549611794 100644
> > --- a/drivers/power/supply/power_supply_sysfs.c
> > +++ b/drivers/power/supply/power_supply_sysfs.c
> > @@ -61,7 +61,7 @@ static const char * const power_supply_charge_type_text[] = {
> > static const char * const power_supply_health_text[] = {
> > "Unknown", "Good", "Overheat", "Dead", "Over voltage",
> > "Unspecified failure", "Cold", "Watchdog timer expire",
> > - "Safety timer expire", "Over current"
> > + "Safety timer expire", "Over current", "Warm", "Cool", "Hot"
> > };
> >
> > static const char * const power_supply_technology_text[] = {
> > diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
> > index 28413f737e7d..bd0d3225f245 100644
> > --- a/include/linux/power_supply.h
> > +++ b/include/linux/power_supply.h
> > @@ -61,6 +61,9 @@ enum {
> > POWER_SUPPLY_HEALTH_WATCHDOG_TIMER_EXPIRE,
> > POWER_SUPPLY_HEALTH_SAFETY_TIMER_EXPIRE,
> > POWER_SUPPLY_HEALTH_OVERCURRENT,
> > + POWER_SUPPLY_HEALTH_WARM,
> > + POWER_SUPPLY_HEALTH_COOL,
> > + POWER_SUPPLY_HEALTH_HOT,
> > };
> >
> > enum {
> > --
> > 2.25.0
> >


2020-03-10 20:17:14

by Dan Murphy

[permalink] [raw]
Subject: Re: [PATCH v4 2/4] power_supply: Add additional health properties to the header

Hello

On 3/6/20 5:55 PM, Sandeep Patil wrote:
> Hi Sebastian,
>
> On Fri, Jan 17, 2020 at 02:06:58AM +0100, Sebastian Reichel wrote:
>> Hi,
>>
>> On Thu, Jan 16, 2020 at 11:50:37AM -0600, Dan Murphy wrote:
>>> Add HEALTH_WARM, HEALTH_COOL and HEALTH_HOT to the health enum.
>>>
>>> Signed-off-by: Dan Murphy <[email protected]>
>>> ---
>> Looks good. But I will not merge it without a user and have comments
>> for the driver.
> Android has been looking for these properties for a while now [1].
> It was added[2] when we saw that the manufacturers were implementing these
> properties in the driver. I didn't know the properties were absent upstream
> until yesterday. Somebody pointed out in our ongoing effort to make sure
> all core kernel changes that android depends on are present upstream.
>
> I think those values are also propagated in application facing APIs in
> Android (but I am not sure yet, let me know if that's something you want
> to find out).
>
> I wanted to chime in and present you a 'user' for this if that helps.

We have re-submitted the BQ25150/155 driver that would be the user and
we have 2 more for review that will use the new definitions

Dan

2020-03-10 21:33:30

by Sebastian Reichel

[permalink] [raw]
Subject: Re: [PATCH v4 2/4] power_supply: Add additional health properties to the header

Hi Sandeep,

On Fri, Mar 06, 2020 at 03:55:48PM -0800, Sandeep Patil wrote:
> On Fri, Jan 17, 2020 at 02:06:58AM +0100, Sebastian Reichel wrote:
> > Hi,
> >
> > On Thu, Jan 16, 2020 at 11:50:37AM -0600, Dan Murphy wrote:
> > > Add HEALTH_WARM, HEALTH_COOL and HEALTH_HOT to the health enum.
> > >
> > > Signed-off-by: Dan Murphy <[email protected]>
> > > ---
> >
> > Looks good. But I will not merge it without a user and have comments
> > for the driver.
>
> Android has been looking for these properties for a while now [1].
> It was added[2] when we saw that the manufacturers were implementing these
> properties in the driver. I didn't know the properties were absent upstream
> until yesterday. Somebody pointed out in our ongoing effort to make sure
> all core kernel changes that android depends on are present upstream.
>
> I think those values are also propagated in application facing APIs in
> Android (but I am not sure yet, let me know if that's something you want
> to find out).
>
> I wanted to chime in and present you a 'user' for this if that helps.

With user I meant an upstream kernel driver, which exposes the
values. But thanks for the pointer. This should be mentioned in
the patch description, also the fact that the status values are
directly taken from JEITA spec.

> Cc: [email protected]
>
> - ssp
>
> 1. https://android.googlesource.com/platform/system/core/+/refs/heads/master/healthd/BatteryMonitor.cpp#162
> 2. https://android-review.googlesource.com/c/platform/system/core/+/414481

-- Sebastian


Attachments:
(No filename) (1.54 kB)
signature.asc (849.00 B)
Download all attachments

2020-03-11 11:36:25

by Dan Murphy

[permalink] [raw]
Subject: Re: [PATCH v4 2/4] power_supply: Add additional health properties to the header

Sebastian

On 3/10/20 4:30 PM, Sebastian Reichel wrote:
> Hi Sandeep,
>
> On Fri, Mar 06, 2020 at 03:55:48PM -0800, Sandeep Patil wrote:
>> On Fri, Jan 17, 2020 at 02:06:58AM +0100, Sebastian Reichel wrote:
>>> Hi,
>>>
>>> On Thu, Jan 16, 2020 at 11:50:37AM -0600, Dan Murphy wrote:
>>>> Add HEALTH_WARM, HEALTH_COOL and HEALTH_HOT to the health enum.
>>>>
>>>> Signed-off-by: Dan Murphy <[email protected]>
>>>> ---
>>> Looks good. But I will not merge it without a user and have comments
>>> for the driver.
>> Android has been looking for these properties for a while now [1].
>> It was added[2] when we saw that the manufacturers were implementing these
>> properties in the driver. I didn't know the properties were absent upstream
>> until yesterday. Somebody pointed out in our ongoing effort to make sure
>> all core kernel changes that android depends on are present upstream.
>>
>> I think those values are also propagated in application facing APIs in
>> Android (but I am not sure yet, let me know if that's something you want
>> to find out).
>>
>> I wanted to chime in and present you a 'user' for this if that helps.
> With user I meant an upstream kernel driver, which exposes the
> values. But thanks for the pointer. This should be mentioned in
> the patch description, also the fact that the status values are
> directly taken from JEITA spec.

I mentioned the JEITA in the cover letter but I guess you would like the
description in the commit message as well

Dan


2020-03-11 16:44:43

by Ricardo Rivera-Matos

[permalink] [raw]
Subject: Re: [EXTERNAL] Re: [PATCH v4 2/4] power_supply: Add additional health properties to the header

Sebastian

On 3/11/20 6:29 AM, Dan Murphy wrote:
> Sebastian
>
> On 3/10/20 4:30 PM, Sebastian Reichel wrote:
>> Hi Sandeep,
>>
>> On Fri, Mar 06, 2020 at 03:55:48PM -0800, Sandeep Patil wrote:
>>> On Fri, Jan 17, 2020 at 02:06:58AM +0100, Sebastian Reichel wrote:
>>>> Hi,
>>>>
>>>> On Thu, Jan 16, 2020 at 11:50:37AM -0600, Dan Murphy wrote:
>>>>> Add HEALTH_WARM, HEALTH_COOL and HEALTH_HOT to the health enum.
>>>>>
>>>>> Signed-off-by: Dan Murphy <[email protected]>
>>>>> ---
>>>> Looks good. But I will not merge it without a user and have comments
>>>> for the driver.
>>> Android has been looking for these properties for a while now [1].
>>> It was added[2] when we saw that the manufacturers were implementing
>>> these
>>> properties in the driver. I didn't know the properties were absent
>>> upstream
>>> until yesterday. Somebody pointed out in our ongoing effort to make
>>> sure
>>> all core kernel changes that android depends on are present upstream.
>>>
>>> I think those values are also propagated in application facing APIs in
>>> Android (but I am not sure yet, let me know if that's something you
>>> want
>>> to find out).
>>>
>>> I wanted to chime in and present you a 'user' for this if that helps.
>> With user I meant an upstream kernel driver, which exposes the
>> values. But thanks for the pointer. This should be mentioned in
>> the patch description, also the fact that the status values are
>> directly taken from JEITA spec.
>
> I mentioned the JEITA in the cover letter but I guess you would like
> the description in the commit message as well
>
> Dan
>
>
I have added a note mentioning that the properties are taken from the
JEITA spec in the commit message and listing the bq2515x_charger driver
as a user. I am waiting for feedback on my other patches in the series
before sending you v5 patches.

Ricardo

2020-03-15 02:19:28

by Sandeep Patil

[permalink] [raw]
Subject: Re: [EXTERNAL] Re: [PATCH v4 2/4] power_supply: Add additional health properties to the header

On Wed, Mar 11, 2020 at 11:43:00AM -0500, Ricardo Rivera-Matos wrote:
> Sebastian
>
> On 3/11/20 6:29 AM, Dan Murphy wrote:
> > Sebastian
> >
> > On 3/10/20 4:30 PM, Sebastian Reichel wrote:
> > > Hi Sandeep,
> > >
> > > On Fri, Mar 06, 2020 at 03:55:48PM -0800, Sandeep Patil wrote:
> > > > On Fri, Jan 17, 2020 at 02:06:58AM +0100, Sebastian Reichel wrote:
> > > > > Hi,
> > > > >
> > > > > On Thu, Jan 16, 2020 at 11:50:37AM -0600, Dan Murphy wrote:
> > > > > > Add HEALTH_WARM, HEALTH_COOL and HEALTH_HOT to the health enum.
> > > > > >
> > > > > > Signed-off-by: Dan Murphy <[email protected]>
> > > > > > ---
> > > > > Looks good. But I will not merge it without a user and have comments
> > > > > for the driver.
> > > > Android has been looking for these properties for a while now [1].
> > > > It was added[2] when we saw that the manufacturers were
> > > > implementing these
> > > > properties in the driver. I didn't know the properties were
> > > > absent upstream
> > > > until yesterday. Somebody pointed out in our ongoing effort to
> > > > make sure
> > > > all core kernel changes that android depends on are present upstream.
> > > >
> > > > I think those values are also propagated in application facing APIs in
> > > > Android (but I am not sure yet, let me know if that's something
> > > > you want
> > > > to find out).
> > > >
> > > > I wanted to chime in and present you a 'user' for this if that helps.
> > > With user I meant an upstream kernel driver, which exposes the
> > > values. But thanks for the pointer. This should be mentioned in
> > > the patch description, also the fact that the status values are
> > > directly taken from JEITA spec.
> >
> > I mentioned the JEITA in the cover letter but I guess you would like the
> > description in the commit message as well
> >
> > Dan
> >
> >
> I have added a note mentioning that the properties are taken from the JEITA
> spec in the commit message and listing the bq2515x_charger driver as a user.
> I am waiting for feedback on my other patches in the series before sending
> you v5 patches.

Ricardo, I'll appreciate if you CC me if/when you send the v5 and followup.
I want to track this series so we can uncheck yet another out-of-tree
dependency that Android has.

Thanks for the quick followup.

- ssp

2020-04-01 00:25:21

by Sandeep Patil

[permalink] [raw]
Subject: Re: [PATCH v4 2/4] power_supply: Add additional health properties to the header

On Tue, Mar 10, 2020 at 03:09:37PM -0500, Dan Murphy wrote:
> Hello
>
> On 3/6/20 5:55 PM, Sandeep Patil wrote:
> > Hi Sebastian,
> >
> > On Fri, Jan 17, 2020 at 02:06:58AM +0100, Sebastian Reichel wrote:
> > > Hi,
> > >
> > > On Thu, Jan 16, 2020 at 11:50:37AM -0600, Dan Murphy wrote:
> > > > Add HEALTH_WARM, HEALTH_COOL and HEALTH_HOT to the health enum.
> > > >
> > > > Signed-off-by: Dan Murphy <[email protected]>
> > > > ---
> > > Looks good. But I will not merge it without a user and have comments
> > > for the driver.
> > Android has been looking for these properties for a while now [1].
> > It was added[2] when we saw that the manufacturers were implementing these
> > properties in the driver. I didn't know the properties were absent upstream
> > until yesterday. Somebody pointed out in our ongoing effort to make sure
> > all core kernel changes that android depends on are present upstream.
> >
> > I think those values are also propagated in application facing APIs in
> > Android (but I am not sure yet, let me know if that's something you want
> > to find out).
> >
> > I wanted to chime in and present you a 'user' for this if that helps.
>
> We have re-submitted the BQ25150/155 driver that would be the user and we
> have 2 more for review that will use the new definitions

Dan / Sebastian, I wasn't able to find the subsequent patches
that add the 3 health properties, so I'm assuming this patch is still
"out-of-tree".

I was hoping against that so we (Android) aren't unnecessarily diverging
from upstream. However, I have now just cherry-picked this patch[1] for the
next Android release.

Sebastian, since these appear in JEITA spec, will the same patch with
additions to the commit message saying so be enough?

Thanks,
- ssp

1. https://android-review.googlesource.com/c/kernel/common/+/1275596

>
> Dan
>