linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] power_supply: Add additional health properties to the header
@ 2019-09-30 14:31 Dan Murphy
  2019-09-30 14:31 ` [PATCH 2/3] dt: power: Add the bq2515x family dt bindings Dan Murphy
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Dan Murphy @ 2019-09-30 14:31 UTC (permalink / raw)
  To: sre, linux-pm; +Cc: linux-kernel, Dan Murphy

Add HEALTH_WARM, HEALTH_COOL and HEALTH_HOT to the health enum.

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
 include/linux/power_supply.h | 3 +++
 1 file changed, 3 insertions(+)

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.22.0.214.g8dca754b1e


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

* [PATCH 2/3] dt: power: Add the bq2515x family dt bindings
  2019-09-30 14:31 [PATCH 1/3] power_supply: Add additional health properties to the header Dan Murphy
@ 2019-09-30 14:31 ` Dan Murphy
  2019-10-20 12:30   ` Sebastian Reichel
  2019-09-30 14:31 ` [PATCH 3/3] power: supply: bq2515x: Introduce the bq2515x family Dan Murphy
  2019-10-20 12:25 ` [PATCH 1/3] power_supply: Add additional health properties to the header Sebastian Reichel
  2 siblings, 1 reply; 11+ messages in thread
From: Dan Murphy @ 2019-09-30 14:31 UTC (permalink / raw)
  To: sre, linux-pm; +Cc: linux-kernel, Dan Murphy

Add the bindings for the bq25150 and bq25155 500mA
 charging IC 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 <dmurphy@ti.com>
---
 .../bindings/power/supply/bq2515x.txt         | 48 +++++++++++++++++++
 1 file changed, 48 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/supply/bq2515x.txt

diff --git a/Documentation/devicetree/bindings/power/supply/bq2515x.txt b/Documentation/devicetree/bindings/power/supply/bq2515x.txt
new file mode 100644
index 000000000000..98203ab53d44
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/supply/bq2515x.txt
@@ -0,0 +1,48 @@
+Binding for TI bq25150 500-mA Linear charger
+
+The BQ25150 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.
+
+Required Properties:
+	- compatible: Should contain one of the following:
+		"ti,bq25150"
+		"ti,bq25155"
+	- reg: int I2c address 0x6b
+	- #address-cells: 1;
+	- #size-cells: = 0;
+
+Optional Properties:
+	- ti,battery-regulation-voltage: integer, maximum charging voltage in mV.
+		Values range from 3600->4600mV
+	- ti,charge-current: integer, maximum charging current in mA.
+		Values range from 50->600mA
+	- reset-gpios: GPIO used for Master reset
+	- low-power-gpios: GPIO used for low power mode of IC.
+	- charge-enable-gpios: GPIO used to turn on and off charging.
+	- pg-gpios: 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.
+
+Example
+bq25150@6b {
+	compatible = "ti,bq25150";
+	reg = <0x6b>;
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	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>;
+
+	ti,charge-current = <300>;
+	ti,battery-regulation-voltage = <4200>;
+};
+
+Reference Datasheet:
+http://www.ti.com/lit/ds/symlink/bq25150.pdf
+http://www.ti.com/lit/ds/symlink/bq25155.pdf
-- 
2.22.0.214.g8dca754b1e


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

* [PATCH 3/3] power: supply: bq2515x: Introduce the bq2515x family
  2019-09-30 14:31 [PATCH 1/3] power_supply: Add additional health properties to the header Dan Murphy
  2019-09-30 14:31 ` [PATCH 2/3] dt: power: Add the bq2515x family dt bindings Dan Murphy
@ 2019-09-30 14:31 ` Dan Murphy
  2019-10-20 12:15   ` Sebastian Reichel
  2019-10-20 12:43   ` Sebastian Reichel
  2019-10-20 12:25 ` [PATCH 1/3] power_supply: Add additional health properties to the header Sebastian Reichel
  2 siblings, 2 replies; 11+ messages in thread
From: Dan Murphy @ 2019-09-30 14:31 UTC (permalink / raw)
  To: sre, linux-pm; +Cc: linux-kernel, Dan Murphy

Introduce the bq25150 and bq25155 supply chargers.

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

diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index c84a7b1caeb6..d6228516fc70 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..211a8b8e44d9
--- /dev/null
+++ b/drivers/power/supply/bq2515x_charger.c
@@ -0,0 +1,743 @@
+// 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	3600
+#define BQ2515X_VBAT_REG_MAX	4600
+#define BQ2515X_VBAT_REG_MIN	3600
+
+#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[] = {
+	50, 100, 150, 200, 300, 400, 500, 600
+};
+
+/* initial field values, converted to register values */
+struct bq2515x_init_data {
+	u8 ichg;	/* charge current		*/
+	u8 vreg;	/* regulation voltage		*/
+	u8 iterm;	/* termination current		*/
+	u8 iprechg;	/* precharge current		*/
+	u8 sysvmin;	/* minimum system voltage limit */
+	u8 ilim;	/* ILIM current contol		*/
+};
+
+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 intval;
+	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;
+
+	intval = (ichg_measurement * 100 / BQ2515X_DIVISOR) * ichg_multiplier;
+	return intval / 100;
+}
+
+static int get_const_charge_voltage(struct bq2515x_device *bq2515x)
+{
+	int ret;
+	int intval;
+	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;
+	intval = ((vbat_measurement * 10000) / BQ2515X_DIVISOR) * 6;
+	return intval / 10;
+}
+
+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 * 10;
+}
+
+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) / 10;
+
+	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_OVERHEAT;
+			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, };
+	int ret = -EINVAL;
+
+	bq2515x->mains = devm_power_supply_register(bq2515x->dev,
+						    &bq2515x_mains_desc,
+						    &psy_cfg);
+	if (IS_ERR(bq2515x->mains))
+		return ret;
+
+	bq2515x->battery = devm_power_supply_register(bq2515x->dev,
+						      &bq2515x_battery_desc,
+						      &psy_cfg);
+	if (IS_ERR(bq2515x->battery)) {
+		power_supply_unregister(bq2515x->mains);
+		return ret;
+	}
+
+	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 (bq2515x->init_data.vreg)
+		ret = bq2515x_set_batt_reg(bq2515x, bq2515x->init_data.vreg);
+
+	return ret;
+}
+
+static int bq2515x_read_properties(struct bq2515x_device *bq2515x)
+{
+	int ret;
+
+	ret = device_property_read_u8(bq2515x->dev, "ti,charge-current",
+				      &bq2515x->init_data.ichg);
+	if (ret)
+		goto fail;
+
+	ret = device_property_read_u8(bq2515x->dev,
+				      "ti,battery-regulation-voltage",
+				      &bq2515x->init_data.vreg);
+	if (ret)
+		goto fail;
+
+	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 (IS_ERR(bq2515x->reset_gpio))
+		dev_info(bq2515x->dev, "reset GPIO not defined");
+
+	bq2515x->lp_gpio = devm_gpiod_get_optional(bq2515x->dev, "low-power",
+						   GPIOD_OUT_LOW);
+	if (IS_ERR(bq2515x->lp_gpio))
+		dev_info(bq2515x->dev, "LP GPIO not defined");
+
+	bq2515x->ce_gpio = devm_gpiod_get_optional(bq2515x->dev,
+						   "charge-enable",
+						   GPIOD_OUT_HIGH);
+	if (IS_ERR(bq2515x->ce_gpio))
+		dev_info(bq2515x->dev, "Charge enable GPIO not defined");
+
+fail:
+	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\n");
+		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 = of_match_ptr(bq2515x_of_match),
+	},
+	.probe = bq2515x_probe,
+	.id_table = bq2515x_i2c_ids,
+};
+module_i2c_driver(bq2515x_driver);
+
+MODULE_AUTHOR("Dan Murphy <dmurphy@ti.com>");
+MODULE_DESCRIPTION("BQ2515X charger driver");
+MODULE_LICENSE("GPL v2");
-- 
2.22.0.214.g8dca754b1e


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

* Re: [PATCH 3/3] power: supply: bq2515x: Introduce the bq2515x family
  2019-09-30 14:31 ` [PATCH 3/3] power: supply: bq2515x: Introduce the bq2515x family Dan Murphy
@ 2019-10-20 12:15   ` Sebastian Reichel
  2019-10-23 19:02     ` Dan Murphy
  2019-10-20 12:43   ` Sebastian Reichel
  1 sibling, 1 reply; 11+ messages in thread
From: Sebastian Reichel @ 2019-10-20 12:15 UTC (permalink / raw)
  To: Dan Murphy; +Cc: linux-pm, linux-kernel

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

Hi Dan,

On Mon, Sep 30, 2019 at 09:31:37AM -0500, Dan Murphy wrote:
> Introduce the bq25150 and bq25155 supply chargers.
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
>  drivers/power/supply/Kconfig           |   8 +
>  drivers/power/supply/Makefile          |   1 +
>  drivers/power/supply/bq2515x_charger.c | 743 +++++++++++++++++++++++++
>  3 files changed, 752 insertions(+)
>  create mode 100644 drivers/power/supply/bq2515x_charger.c
> 
> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
> index c84a7b1caeb6..d6228516fc70 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..211a8b8e44d9
> --- /dev/null
> +++ b/drivers/power/supply/bq2515x_charger.c
> @@ -0,0 +1,743 @@
> +// 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	3600
> +#define BQ2515X_VBAT_REG_MAX	4600
> +#define BQ2515X_VBAT_REG_MIN	3600
> +
> +#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[] = {
> +	50, 100, 150, 200, 300, 400, 500, 600
> +};
> +
> +/* initial field values, converted to register values */
> +struct bq2515x_init_data {
> +	u8 ichg;	/* charge current		*/
> +	u8 vreg;	/* regulation voltage		*/
> +	u8 iterm;	/* termination current		*/
> +	u8 iprechg;	/* precharge current		*/
> +	u8 sysvmin;	/* minimum system voltage limit */
> +	u8 ilim;	/* ILIM current contol		*/
> +};
> +
> +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 intval;
> +	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;
> +
> +	intval = (ichg_measurement * 100 / BQ2515X_DIVISOR) * ichg_multiplier;
> +	return intval / 100;
> +}
> +
> +static int get_const_charge_voltage(struct bq2515x_device *bq2515x)
> +{
> +	int ret;
> +	int intval;
> +	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;
> +	intval = ((vbat_measurement * 10000) / BQ2515X_DIVISOR) * 6;
> +	return intval / 10;
> +}
> +
> +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 * 10;
> +}
> +
> +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) / 10;
> +
> +	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_OVERHEAT;
> +			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, };
> +	int ret = -EINVAL;
> +
> +	bq2515x->mains = devm_power_supply_register(bq2515x->dev,
> +						    &bq2515x_mains_desc,
> +						    &psy_cfg);
> +	if (IS_ERR(bq2515x->mains))
> +		return ret;
> +
> +	bq2515x->battery = devm_power_supply_register(bq2515x->dev,
> +						      &bq2515x_battery_desc,
> +						      &psy_cfg);
> +	if (IS_ERR(bq2515x->battery)) {
> +		power_supply_unregister(bq2515x->mains);

you registered the mains power-supply with devm_ prefix, so it
will be removed automatically.

> +		return ret;
> +	}
> +
> +	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 (bq2515x->init_data.vreg)
> +		ret = bq2515x_set_batt_reg(bq2515x, bq2515x->init_data.vreg);

This throws away potential error code from bq2515x_set_ilim_lvl().

> +	return ret;
> +}
> +
> +static int bq2515x_read_properties(struct bq2515x_device *bq2515x)
> +{
> +	int ret;
> +
> +	ret = device_property_read_u8(bq2515x->dev, "ti,charge-current",
> +				      &bq2515x->init_data.ichg);
> +	if (ret)
> +		goto fail;
> +
> +	ret = device_property_read_u8(bq2515x->dev,
> +				      "ti,battery-regulation-voltage",
> +				      &bq2515x->init_data.vreg);
> +	if (ret)
> +		goto fail;

The above properties are marked as optional in the DT bindings
document.

> +	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 (IS_ERR(bq2515x->reset_gpio))
> +		dev_info(bq2515x->dev, "reset GPIO not defined");
> +
> +	bq2515x->lp_gpio = devm_gpiod_get_optional(bq2515x->dev, "low-power",
> +						   GPIOD_OUT_LOW);
> +	if (IS_ERR(bq2515x->lp_gpio))
> +		dev_info(bq2515x->dev, "LP GPIO not defined");
> +
> +	bq2515x->ce_gpio = devm_gpiod_get_optional(bq2515x->dev,
> +						   "charge-enable",
> +						   GPIOD_OUT_HIGH);
> +	if (IS_ERR(bq2515x->ce_gpio))
> +		dev_info(bq2515x->dev, "Charge enable GPIO not defined");

The GPIO errors should not be ignored, especially since you
might get a -EPROBE_DEFER.

> +fail:
> +	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\n");
> +		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 = of_match_ptr(bq2515x_of_match),

You need to do one of those:

 * remove of_match_ptr and specify bq2515x_of_match directly
 * put the OF table in #ifdef CONFIG_OF
 * mark the OF table as __maybe_unused

> +	},
> +	.probe = bq2515x_probe,
> +	.id_table = bq2515x_i2c_ids,
> +};
> +module_i2c_driver(bq2515x_driver);
> +
> +MODULE_AUTHOR("Dan Murphy <dmurphy@ti.com>");
> +MODULE_DESCRIPTION("BQ2515X charger driver");
> +MODULE_LICENSE("GPL v2");
> -- 
> 2.22.0.214.g8dca754b1e
> 

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

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

* Re: [PATCH 1/3] power_supply: Add additional health properties to the header
  2019-09-30 14:31 [PATCH 1/3] power_supply: Add additional health properties to the header Dan Murphy
  2019-09-30 14:31 ` [PATCH 2/3] dt: power: Add the bq2515x family dt bindings Dan Murphy
  2019-09-30 14:31 ` [PATCH 3/3] power: supply: bq2515x: Introduce the bq2515x family Dan Murphy
@ 2019-10-20 12:25 ` Sebastian Reichel
  2019-10-21 19:35   ` Dan Murphy
  2 siblings, 1 reply; 11+ messages in thread
From: Sebastian Reichel @ 2019-10-20 12:25 UTC (permalink / raw)
  To: Dan Murphy; +Cc: linux-pm, linux-kernel

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

Hi,

On Mon, Sep 30, 2019 at 09:31:35AM -0500, Dan Murphy wrote:
> Add HEALTH_WARM, HEALTH_COOL and HEALTH_HOT to the health enum.

You used OVERHEAT instead of HOT in the implementation,
which makes sense to me, so please drop HOT here. Also
this needs to be documented in the ABI documentation:

Documentation/ABI/testing/sysfs-class-power

-- Sebastian

> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
>  include/linux/power_supply.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> 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.22.0.214.g8dca754b1e
> 

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

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

* Re: [PATCH 2/3] dt: power: Add the bq2515x family dt bindings
  2019-09-30 14:31 ` [PATCH 2/3] dt: power: Add the bq2515x family dt bindings Dan Murphy
@ 2019-10-20 12:30   ` Sebastian Reichel
  2019-10-21 19:36     ` Dan Murphy
  0 siblings, 1 reply; 11+ messages in thread
From: Sebastian Reichel @ 2019-10-20 12:30 UTC (permalink / raw)
  To: Dan Murphy; +Cc: linux-pm, linux-kernel

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

Hi Dan,

On Mon, Sep 30, 2019 at 09:31:36AM -0500, Dan Murphy wrote:
> Add the bindings for the bq25150 and bq25155 500mA
>  charging IC 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 <dmurphy@ti.com>
> ---
>  .../bindings/power/supply/bq2515x.txt         | 48 +++++++++++++++++++

This should use the new YAML binding style.

>  1 file changed, 48 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/power/supply/bq2515x.txt
> 
> diff --git a/Documentation/devicetree/bindings/power/supply/bq2515x.txt b/Documentation/devicetree/bindings/power/supply/bq2515x.txt
> new file mode 100644
> index 000000000000..98203ab53d44
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/supply/bq2515x.txt
> @@ -0,0 +1,48 @@
> +Binding for TI bq25150 500-mA Linear charger
> +
> +The BQ25150 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.
> +
> +Required Properties:
> +	- compatible: Should contain one of the following:
> +		"ti,bq25150"
> +		"ti,bq25155"
> +	- reg: int I2c address 0x6b
> +	- #address-cells: 1;
> +	- #size-cells: = 0;
> +
> +Optional Properties:
> +	- ti,battery-regulation-voltage: integer, maximum charging voltage in mV.
> +		Values range from 3600->4600mV
> +	- ti,charge-current: integer, maximum charging current in mA.
> +		Values range from 50->600mA

Those values should be in uV and uA. Also the values basically
describe the dumb battery, so please use this binding:

Documentation/devicetree/bindings/power/supply/battery.txt

> +	- reset-gpios: GPIO used for Master reset
> +	- low-power-gpios: GPIO used for low power mode of IC.
> +	- charge-enable-gpios: GPIO used to turn on and off charging.
> +	- pg-gpios: 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.
> +
> +Example
> +bq25150@6b {

charger@6b

> +	compatible = "ti,bq25150";
> +	reg = <0x6b>;
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +
> +	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>;
> +
> +	ti,charge-current = <300>;
> +	ti,battery-regulation-voltage = <4200>;
> +};
> +
> +Reference Datasheet:
> +http://www.ti.com/lit/ds/symlink/bq25150.pdf
> +http://www.ti.com/lit/ds/symlink/bq25155.pdf
> -- 
> 2.22.0.214.g8dca754b1e
> 

-- Sebastian

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

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

* Re: [PATCH 3/3] power: supply: bq2515x: Introduce the bq2515x family
  2019-09-30 14:31 ` [PATCH 3/3] power: supply: bq2515x: Introduce the bq2515x family Dan Murphy
  2019-10-20 12:15   ` Sebastian Reichel
@ 2019-10-20 12:43   ` Sebastian Reichel
  1 sibling, 0 replies; 11+ messages in thread
From: Sebastian Reichel @ 2019-10-20 12:43 UTC (permalink / raw)
  To: Dan Murphy; +Cc: linux-pm, linux-kernel

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

Hi,

Here are some more notes.

On Mon, Sep 30, 2019 at 09:31:37AM -0500, Dan Murphy wrote:
> Introduce the bq25150 and bq25155 supply chargers.
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---

[...]

> +/* initial field values, converted to register values */
> +struct bq2515x_init_data {
> +	u8 ichg;	/* charge current		*/
> +	u8 vreg;	/* regulation voltage		*/

The following 4 entries seem to be unused.

> +	u8 iterm;	/* termination current		*/
> +	u8 iprechg;	/* precharge current		*/
> +	u8 sysvmin;	/* minimum system voltage limit */
> +	u8 ilim;	/* ILIM current contol		*/
> +};

[...]

> +static int get_const_charge_current(struct bq2515x_device *bq2515x)
> +{
> +	int ret;
> +	int intval;
> +	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;
> +
> +	intval = (ichg_measurement * 100 / BQ2515X_DIVISOR) * ichg_multiplier;
> +	return intval / 100;
> +}
> [...]

I just checked the calculation and you return mA. This is incorrect,
since the power-supply ABI uses µA. Please also re-check the other
calculations. You can find some information about the power-supply
subsystem here:

Documentation/power/power_supply_class.rst

-- Sebastian

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

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

* Re: [PATCH 1/3] power_supply: Add additional health properties to the header
  2019-10-20 12:25 ` [PATCH 1/3] power_supply: Add additional health properties to the header Sebastian Reichel
@ 2019-10-21 19:35   ` Dan Murphy
  0 siblings, 0 replies; 11+ messages in thread
From: Dan Murphy @ 2019-10-21 19:35 UTC (permalink / raw)
  To: Sebastian Reichel; +Cc: linux-pm, linux-kernel

Sebastian

On 10/20/19 7:25 AM, Sebastian Reichel wrote:
> Hi,
>
> On Mon, Sep 30, 2019 at 09:31:35AM -0500, Dan Murphy wrote:
>> Add HEALTH_WARM, HEALTH_COOL and HEALTH_HOT to the health enum.
> You used OVERHEAT instead of HOT in the implementation,
> which makes sense to me, so please drop HOT here. Also
> this needs to be documented in the ABI documentation:

Hmm.  To me OVERHEAT and HOT can mean two different things.  I will 
check the implementation but I would

prefer to use HOT.

If a battery or charger is HOT that may be expected to be within the 
heat limits of the device being monitored but that it is hot.

Overheating means the device is above the expected upper thermal limit.  
Or over the heat limit.

The user may want to take action within the hot range to cool the device 
or may expect the device to run hot without over heating.


> Documentation/ABI/testing/sysfs-class-power

ACK

Dan



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

* Re: [PATCH 2/3] dt: power: Add the bq2515x family dt bindings
  2019-10-20 12:30   ` Sebastian Reichel
@ 2019-10-21 19:36     ` Dan Murphy
  0 siblings, 0 replies; 11+ messages in thread
From: Dan Murphy @ 2019-10-21 19:36 UTC (permalink / raw)
  To: Sebastian Reichel; +Cc: linux-pm, linux-kernel

Sebastian

On 10/20/19 7:30 AM, Sebastian Reichel wrote:
> Hi Dan,
>
> On Mon, Sep 30, 2019 at 09:31:36AM -0500, Dan Murphy wrote:
>> Add the bindings for the bq25150 and bq25155 500mA
>>   charging IC 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 <dmurphy@ti.com>
>> ---
>>   .../bindings/power/supply/bq2515x.txt         | 48 +++++++++++++++++++
> This should use the new YAML binding style.


Ack


>
>>   1 file changed, 48 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/power/supply/bq2515x.txt
>>
>> diff --git a/Documentation/devicetree/bindings/power/supply/bq2515x.txt b/Documentation/devicetree/bindings/power/supply/bq2515x.txt
>> new file mode 100644
>> index 000000000000..98203ab53d44
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/power/supply/bq2515x.txt
>> @@ -0,0 +1,48 @@
>> +Binding for TI bq25150 500-mA Linear charger
>> +
>> +The BQ25150 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.
>> +
>> +Required Properties:
>> +	- compatible: Should contain one of the following:
>> +		"ti,bq25150"
>> +		"ti,bq25155"
>> +	- reg: int I2c address 0x6b
>> +	- #address-cells: 1;
>> +	- #size-cells: = 0;
>> +
>> +Optional Properties:
>> +	- ti,battery-regulation-voltage: integer, maximum charging voltage in mV.
>> +		Values range from 3600->4600mV
>> +	- ti,charge-current: integer, maximum charging current in mA.
>> +		Values range from 50->600mA
> Those values should be in uV and uA. Also the values basically
> describe the dumb battery, so please use this binding:
>
> Documentation/devicetree/bindings/power/supply/battery.txt

Ack


>
>> +	- reset-gpios: GPIO used for Master reset
>> +	- low-power-gpios: GPIO used for low power mode of IC.
>> +	- charge-enable-gpios: GPIO used to turn on and off charging.
>> +	- pg-gpios: 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.
>> +
>> +Example
>> +bq25150@6b {
> charger@6b

Ack

Dan


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

* Re: [PATCH 3/3] power: supply: bq2515x: Introduce the bq2515x family
  2019-10-20 12:15   ` Sebastian Reichel
@ 2019-10-23 19:02     ` Dan Murphy
  0 siblings, 0 replies; 11+ messages in thread
From: Dan Murphy @ 2019-10-23 19:02 UTC (permalink / raw)
  To: Sebastian Reichel; +Cc: linux-pm, linux-kernel

Sebastian

On 10/20/19 7:15 AM, Sebastian Reichel wrote:
> Hi Dan,
>
> On Mon, Sep 30, 2019 at 09:31:37AM -0500, Dan Murphy wrote:
[...]
>> +
>> +static int bq2515x_power_supply_register(struct bq2515x_device *bq2515x)
>> +{
>> +	struct power_supply_config psy_cfg = { .drv_data = bq2515x, };
>> +	int ret = -EINVAL;
>> +
>> +	bq2515x->mains = devm_power_supply_register(bq2515x->dev,
>> +						    &bq2515x_mains_desc,
>> +						    &psy_cfg);
>> +	if (IS_ERR(bq2515x->mains))
>> +		return ret;
>> +
>> +	bq2515x->battery = devm_power_supply_register(bq2515x->dev,
>> +						      &bq2515x_battery_desc,
>> +						      &psy_cfg);
>> +	if (IS_ERR(bq2515x->battery)) {
>> +		power_supply_unregister(bq2515x->mains);
> you registered the mains power-supply with devm_ prefix, so it
> will be removed automatically.
Ack
>
>> +		return ret;
>> +	}
>> +
>> +	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 (bq2515x->init_data.vreg)
>> +		ret = bq2515x_set_batt_reg(bq2515x, bq2515x->init_data.vreg);
> This throws away potential error code from bq2515x_set_ilim_lvl().
Ack
>> +	return ret;
>> +}
>> +
>> +static int bq2515x_read_properties(struct bq2515x_device *bq2515x)
>> +{
>> +	int ret;
>> +
>> +	ret = device_property_read_u8(bq2515x->dev, "ti,charge-current",
>> +				      &bq2515x->init_data.ichg);
>> +	if (ret)
>> +		goto fail;
>> +
>> +	ret = device_property_read_u8(bq2515x->dev,
>> +				      "ti,battery-regulation-voltage",
>> +				      &bq2515x->init_data.vreg);
>> +	if (ret)
>> +		goto fail;
> The above properties are marked as optional in the DT bindings
> document.
ACK.  Will just set them to the data sheet default values
>
>> +	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 (IS_ERR(bq2515x->reset_gpio))
>> +		dev_info(bq2515x->dev, "reset GPIO not defined");
>> +
>> +	bq2515x->lp_gpio = devm_gpiod_get_optional(bq2515x->dev, "low-power",
>> +						   GPIOD_OUT_LOW);
>> +	if (IS_ERR(bq2515x->lp_gpio))
>> +		dev_info(bq2515x->dev, "LP GPIO not defined");
>> +
>> +	bq2515x->ce_gpio = devm_gpiod_get_optional(bq2515x->dev,
>> +						   "charge-enable",
>> +						   GPIOD_OUT_HIGH);
>> +	if (IS_ERR(bq2515x->ce_gpio))
>> +		dev_info(bq2515x->dev, "Charge enable GPIO not defined");
> The GPIO errors should not be ignored, especially since you
> might get a -EPROBE_DEFER.

ACK


>> +fail:
>> +	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\n");
>> +		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 = of_match_ptr(bq2515x_of_match),
> You need to do one of those:
>
>   * remove of_match_ptr and specify bq2515x_of_match directly
>   * put the OF table in #ifdef CONFIG_OF
>   * mark the OF table as __maybe_unused

Probably just do the first one.

Dan


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

* [PATCH 1/3] power_supply: Add additional health properties to the header
  2020-05-15 19:31 [PATCH 0/3] Add JEITA properties and introduce the bq2515x charger Dan Murphy
@ 2020-05-15 19:31 ` Dan Murphy
  0 siblings, 0 replies; 11+ messages in thread
From: Dan Murphy @ 2020-05-15 19:31 UTC (permalink / raw)
  To: sre
  Cc: sspatil, linux-pm, robh, linux-kernel, devicetree, Dan Murphy,
	Guru Das Srinagesh

Add HEALTH_WARM, HEALTH_COOL and HEALTH_HOT to the health enum.

HEALTH_WARM, HEALTH_COOL, and HEALTH_HOT properties are taken from the JEITA spec.

Signed-off-by: Dan Murphy <dmurphy@ti.com>

Tested-by: Guru Das Srinagesh <gurus@codeaurora.org>
---
 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 dcd5a71e6c67..8670e90c1d51 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.26.2


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

end of thread, other threads:[~2020-05-15 19:41 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-30 14:31 [PATCH 1/3] power_supply: Add additional health properties to the header Dan Murphy
2019-09-30 14:31 ` [PATCH 2/3] dt: power: Add the bq2515x family dt bindings Dan Murphy
2019-10-20 12:30   ` Sebastian Reichel
2019-10-21 19:36     ` Dan Murphy
2019-09-30 14:31 ` [PATCH 3/3] power: supply: bq2515x: Introduce the bq2515x family Dan Murphy
2019-10-20 12:15   ` Sebastian Reichel
2019-10-23 19:02     ` Dan Murphy
2019-10-20 12:43   ` Sebastian Reichel
2019-10-20 12:25 ` [PATCH 1/3] power_supply: Add additional health properties to the header Sebastian Reichel
2019-10-21 19:35   ` Dan Murphy
2020-05-15 19:31 [PATCH 0/3] Add JEITA properties and introduce the bq2515x charger Dan Murphy
2020-05-15 19:31 ` [PATCH 1/3] power_supply: Add additional health properties to the header Dan Murphy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).