[PATCH 0/5] Add new features for SC27XX fuel gauge driver

[PATCH 0/5] Add new features for SC27XX fuel gauge driver

[Baolin Wang]

This patch set adds some new features for SC27XX fuel gauge driver.

1. Read calibration data from eFuse device to calibrate fuel gauge.
2. Add low voltage alarm to adjust the battery capacity in lower
voltage stage.
3. Add power management interfaces
4. Save last optimized battery capacity to be used as the initial
battery capacity if system is not first power-on.

Baolin Wang (2):
  dt-bindings: power: supply: Add nvmem properties to calibrate FGU
  power: supply: sc27xx: Add fuel gauge calibration

Yuanjiang Yu (3):
  power: supply: sc27xx: Add fuel gauge low voltage alarm
  power: supply: sc27xx: Add suspend/resume interfaces
  power: supply: sc27xx: Save last battery capacity

 .../devicetree/bindings/power/supply/sc27xx-fg.txt |    4 +
 drivers/power/supply/sc27xx_fuel_gauge.c           |  453 +++++++++++++++++++-
 2 files changed, 444 insertions(+), 13 deletions(-)

-- 
1.7.9.5

[PATCH 1/5] dt-bindings: power: supply: Add nvmem properties to calibrate FGU

[Baolin Wang]

Add nvmem properties to calibrate FGU from eFuse controller.

Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
 .../devicetree/bindings/power/supply/sc27xx-fg.txt |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/power/supply/sc27xx-fg.txt b/Documentation/devicetree/bindings/power/supply/sc27xx-fg.txt
index 98a2400..fc35ac5 100644
--- a/Documentation/devicetree/bindings/power/supply/sc27xx-fg.txt
+++ b/Documentation/devicetree/bindings/power/supply/sc27xx-fg.txt
@@ -11,6 +11,8 @@ Required properties:
 - battery-detect-gpios: GPIO for battery detection.
 - io-channels: Specify the IIO ADC channel to get temperature.
 - io-channel-names: Should be "bat-temp".
+- nvmem-cells: A phandle to the calibration cells provided by eFuse device.
+- nvmem-cell-names: Should be "fgu_calib".
 - monitored-battery: Phandle of battery characteristics devicetree node.
   See Documentation/devicetree/bindings/power/supply/battery.txt
 
@@ -47,6 +49,8 @@ Example:
 			battery-detect-gpios = <&pmic_eic 9 GPIO_ACTIVE_HIGH>;
 			io-channels = <&pmic_adc 5>;
 			io-channel-names = "bat-temp";
+			nvmem-cells = <&fgu_calib>;
+			nvmem-cell-names = "fgu_calib";
 			monitored-battery = <&bat>;
 		};
 	};
-- 
1.7.9.5

Re: [PATCH 1/5] dt-bindings: power: supply: Add nvmem properties to calibrate FGU

[Rob Herring]

On Wed, 14 Nov 2018 17:07:04 +0800, Baolin Wang wrote:
> Add nvmem properties to calibrate FGU from eFuse controller.
> 
> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
> ---
>  .../devicetree/bindings/power/supply/sc27xx-fg.txt |    4 ++++
>  1 file changed, 4 insertions(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

[PATCH 2/5] power: supply: sc27xx: Add fuel gauge calibration

[Baolin Wang]

This patch adds support to read calibration values from the eFuse controller
to calibrate the ADC values corresponding to current and voltage, which can
make the current and voltage data more accurate.

Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
 drivers/power/supply/sc27xx_fuel_gauge.c |   62 ++++++++++++++++++++++++------
 1 file changed, 51 insertions(+), 11 deletions(-)

diff --git a/drivers/power/supply/sc27xx_fuel_gauge.c b/drivers/power/supply/sc27xx_fuel_gauge.c
index 8613584..66f4db2 100644
--- a/drivers/power/supply/sc27xx_fuel_gauge.c
+++ b/drivers/power/supply/sc27xx_fuel_gauge.c
@@ -6,10 +6,12 @@
 #include <linux/interrupt.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/nvmem-consumer.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/power_supply.h>
 #include <linux/regmap.h>
+#include <linux/slab.h>
 
 /* PMIC global control registers definition */
 #define SC27XX_MODULE_EN0		0xc08
@@ -39,8 +41,6 @@
 #define SC27XX_FGU_CLBCNT_MASK		GENMASK(15, 0)
 #define SC27XX_FGU_CLBCNT_SHIFT		16
 
-#define SC27XX_FGU_1000MV_ADC		686
-#define SC27XX_FGU_1000MA_ADC		1372
 #define SC27XX_FGU_CUR_BASIC_ADC	8192
 #define SC27XX_FGU_SAMPLE_HZ		2
 
@@ -59,6 +59,8 @@
  * @init_clbcnt: the initial coulomb counter
  * @max_volt: the maximum constant input voltage in millivolt
  * @table_len: the capacity table length
+ * @cur_1000ma_adc: ADC value corresponding to 1000 mA
+ * @vol_1000mv_adc: ADC value corresponding to 1000 mV
  * @cap_table: capacity table with corresponding ocv
  */
 struct sc27xx_fgu_data {
@@ -76,6 +78,8 @@ struct sc27xx_fgu_data {
 	int init_clbcnt;
 	int max_volt;
 	int table_len;
+	int cur_1000ma_adc;
+	int vol_1000mv_adc;
 	struct power_supply_battery_ocv_table *cap_table;
 };
 
@@ -86,14 +90,14 @@ struct sc27xx_fgu_data {
 	"sc2723_charger",
 };
 
-static int sc27xx_fgu_adc_to_current(int adc)
+static int sc27xx_fgu_adc_to_current(struct sc27xx_fgu_data *data, int adc)
 {
-	return DIV_ROUND_CLOSEST(adc * 1000, SC27XX_FGU_1000MA_ADC);
+	return DIV_ROUND_CLOSEST(adc * 1000, data->cur_1000ma_adc);
 }
 
-static int sc27xx_fgu_adc_to_voltage(int adc)
+static int sc27xx_fgu_adc_to_voltage(struct sc27xx_fgu_data *data, int adc)
 {
-	return DIV_ROUND_CLOSEST(adc * 1000, SC27XX_FGU_1000MV_ADC);
+	return DIV_ROUND_CLOSEST(adc * 1000, data->vol_1000mv_adc);
 }
 
 /*
@@ -116,7 +120,7 @@ static int sc27xx_fgu_get_boot_capacity(struct sc27xx_fgu_data *data, int *cap)
 		return ret;
 
 	cur <<= 1;
-	oci = sc27xx_fgu_adc_to_current(cur - SC27XX_FGU_CUR_BASIC_ADC);
+	oci = sc27xx_fgu_adc_to_current(data, cur - SC27XX_FGU_CUR_BASIC_ADC);
 
 	/*
 	 * Should get the OCV from SC27XX_FGU_POCV register at the system
@@ -127,7 +131,7 @@ static int sc27xx_fgu_get_boot_capacity(struct sc27xx_fgu_data *data, int *cap)
 	if (ret)
 		return ret;
 
-	volt = sc27xx_fgu_adc_to_voltage(volt);
+	volt = sc27xx_fgu_adc_to_voltage(data, volt);
 	ocv = volt * 1000 - oci * data->internal_resist;
 
 	/*
@@ -201,7 +205,7 @@ static int sc27xx_fgu_get_capacity(struct sc27xx_fgu_data *data, int *cap)
 	 * as 100 to improve the precision.
 	 */
 	temp = DIV_ROUND_CLOSEST(delta_clbcnt, 360);
-	temp = sc27xx_fgu_adc_to_current(temp);
+	temp = sc27xx_fgu_adc_to_current(data, temp);
 
 	/*
 	 * Convert to capacity percent of the battery total capacity,
@@ -225,7 +229,7 @@ static int sc27xx_fgu_get_vbat_vol(struct sc27xx_fgu_data *data, int *val)
 	 * It is ADC values reading from registers which need to convert to
 	 * corresponding voltage values.
 	 */
-	*val = sc27xx_fgu_adc_to_voltage(vol);
+	*val = sc27xx_fgu_adc_to_voltage(data, vol);
 
 	return 0;
 }
@@ -242,7 +246,7 @@ static int sc27xx_fgu_get_current(struct sc27xx_fgu_data *data, int *val)
 	 * It is ADC values reading from registers which need to convert to
 	 * corresponding current values.
 	 */
-	*val = sc27xx_fgu_adc_to_current(cur - SC27XX_FGU_CUR_BASIC_ADC);
+	*val = sc27xx_fgu_adc_to_current(data, cur - SC27XX_FGU_CUR_BASIC_ADC);
 
 	return 0;
 }
@@ -469,6 +473,38 @@ static int sc27xx_fgu_cap_to_clbcnt(struct sc27xx_fgu_data *data, int capacity)
 	return DIV_ROUND_CLOSEST(cur_cap * 36, 10);
 }
 
+static int sc27xx_fgu_calibration(struct sc27xx_fgu_data *data)
+{
+	struct nvmem_cell *cell;
+	int calib_data, cal_4200mv;
+	void *buf;
+	size_t len;
+
+	cell = nvmem_cell_get(data->dev, "fgu_calib");
+	if (IS_ERR(cell))
+		return PTR_ERR(cell);
+
+	buf = nvmem_cell_read(cell, &len);
+	nvmem_cell_put(cell);
+
+	if (IS_ERR(buf))
+		return PTR_ERR(buf);
+
+	memcpy(&calib_data, buf, min(len, sizeof(u32)));
+
+	/*
+	 * Get the ADC value corresponding to 4200 mV from eFuse controller
+	 * according to below formula. Then convert to ADC values corresponding
+	 * to 1000 mV and 1000 mA.
+	 */
+	cal_4200mv = (calib_data & 0x1ff) + 6963 - 4096 - 256;
+	data->vol_1000mv_adc = DIV_ROUND_CLOSEST(cal_4200mv * 10, 42);
+	data->cur_1000ma_adc = data->vol_1000mv_adc * 4;
+
+	kfree(buf);
+	return 0;
+}
+
 static int sc27xx_fgu_hw_init(struct sc27xx_fgu_data *data)
 {
 	struct power_supply_battery_info info = { };
@@ -503,6 +539,10 @@ static int sc27xx_fgu_hw_init(struct sc27xx_fgu_data *data)
 
 	power_supply_put_battery_info(data->battery, &info);
 
+	ret = sc27xx_fgu_calibration(data);
+	if (ret)
+		return ret;
+
 	/* Enable the FGU module */
 	ret = regmap_update_bits(data->regmap, SC27XX_MODULE_EN0,
 				 SC27XX_FGU_EN, SC27XX_FGU_EN);
-- 
1.7.9.5

[PATCH 3/5] power: supply: sc27xx: Add fuel gauge low voltage alarm

[Baolin Wang]

From: Yuanjiang Yu <yuanjiang.yu@unisoc.com>

Add low voltage alarm support to make sure the battery capacity
more accurate in lower voltage stage.

Signed-off-by: Yuanjiang Yu <yuanjiang.yu@unisoc.com>
Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
 drivers/power/supply/sc27xx_fuel_gauge.c |  171 +++++++++++++++++++++++++++++-
 1 file changed, 170 insertions(+), 1 deletion(-)

diff --git a/drivers/power/supply/sc27xx_fuel_gauge.c b/drivers/power/supply/sc27xx_fuel_gauge.c
index 66f4db2..962d0f8 100644
--- a/drivers/power/supply/sc27xx_fuel_gauge.c
+++ b/drivers/power/supply/sc27xx_fuel_gauge.c
@@ -31,8 +31,11 @@
 #define SC27XX_FGU_OCV			0x24
 #define SC27XX_FGU_POCV			0x28
 #define SC27XX_FGU_CURRENT		0x2c
+#define SC27XX_FGU_LOW_OVERLOAD		0x34
 #define SC27XX_FGU_CLBCNT_SETH		0x50
 #define SC27XX_FGU_CLBCNT_SETL		0x54
+#define SC27XX_FGU_CLBCNT_DELTH		0x58
+#define SC27XX_FGU_CLBCNT_DELTL		0x5c
 #define SC27XX_FGU_CLBCNT_VALH		0x68
 #define SC27XX_FGU_CLBCNT_VALL		0x6c
 #define SC27XX_FGU_CLBCNT_QMAXL		0x74
@@ -40,6 +43,11 @@
 #define SC27XX_WRITE_SELCLB_EN		BIT(0)
 #define SC27XX_FGU_CLBCNT_MASK		GENMASK(15, 0)
 #define SC27XX_FGU_CLBCNT_SHIFT		16
+#define SC27XX_FGU_LOW_OVERLOAD_MASK	GENMASK(12, 0)
+
+#define SC27XX_FGU_INT_MASK		GENMASK(9, 0)
+#define SC27XX_FGU_LOW_OVERLOAD_INT	BIT(0)
+#define SC27XX_FGU_CLBCNT_DELTA_INT	BIT(2)
 
 #define SC27XX_FGU_CUR_BASIC_ADC	8192
 #define SC27XX_FGU_SAMPLE_HZ		2
@@ -56,8 +64,10 @@
  * @internal_resist: the battery internal resistance in mOhm
  * @total_cap: the total capacity of the battery in mAh
  * @init_cap: the initial capacity of the battery in mAh
+ * @alarm_cap: the alarm capacity
  * @init_clbcnt: the initial coulomb counter
  * @max_volt: the maximum constant input voltage in millivolt
+ * @min_volt: the minimum drained battery voltage in microvolt
  * @table_len: the capacity table length
  * @cur_1000ma_adc: ADC value corresponding to 1000 mA
  * @vol_1000mv_adc: ADC value corresponding to 1000 mV
@@ -75,14 +85,18 @@ struct sc27xx_fgu_data {
 	int internal_resist;
 	int total_cap;
 	int init_cap;
+	int alarm_cap;
 	int init_clbcnt;
 	int max_volt;
+	int min_volt;
 	int table_len;
 	int cur_1000ma_adc;
 	int vol_1000mv_adc;
 	struct power_supply_battery_ocv_table *cap_table;
 };
 
+static int sc27xx_fgu_cap_to_clbcnt(struct sc27xx_fgu_data *data, int capacity);
+
 static const char * const sc27xx_charger_supply_name[] = {
 	"sc2731_charger",
 	"sc2720_charger",
@@ -100,6 +114,11 @@ static int sc27xx_fgu_adc_to_voltage(struct sc27xx_fgu_data *data, int adc)
 	return DIV_ROUND_CLOSEST(adc * 1000, data->vol_1000mv_adc);
 }
 
+static int sc27xx_fgu_voltage_to_adc(struct sc27xx_fgu_data *data, int vol)
+{
+	return DIV_ROUND_CLOSEST(vol * data->vol_1000mv_adc, 1000);
+}
+
 /*
  * When system boots on, we can not read battery capacity from coulomb
  * registers, since now the coulomb registers are invalid. So we should
@@ -428,6 +447,92 @@ static void sc27xx_fgu_external_power_changed(struct power_supply *psy)
 	.external_power_changed	= sc27xx_fgu_external_power_changed,
 };
 
+static void sc27xx_fgu_adjust_cap(struct sc27xx_fgu_data *data, int cap)
+{
+	data->init_cap = cap;
+	data->init_clbcnt = sc27xx_fgu_cap_to_clbcnt(data, data->init_cap);
+}
+
+static irqreturn_t sc27xx_fgu_interrupt(int irq, void *dev_id)
+{
+	struct sc27xx_fgu_data *data = dev_id;
+	int ret, cap, ocv, adc;
+	u32 status;
+
+	mutex_lock(&data->lock);
+
+	ret = regmap_read(data->regmap, data->base + SC27XX_FGU_INT_STS,
+			  &status);
+	if (ret)
+		goto out;
+
+	ret = regmap_update_bits(data->regmap, data->base + SC27XX_FGU_INT_CLR,
+				 status, status);
+	if (ret)
+		goto out;
+
+	/*
+	 * When low overload voltage interrupt happens, we should calibrate the
+	 * battery capacity in lower voltage stage.
+	 */
+	if (!(status & SC27XX_FGU_LOW_OVERLOAD_INT))
+		goto out;
+
+	ret = sc27xx_fgu_get_capacity(data, &cap);
+	if (ret)
+		goto out;
+
+	ret = sc27xx_fgu_get_vbat_ocv(data, &ocv);
+	if (ret)
+		goto out;
+
+	/*
+	 * If current OCV value is less than the minimum OCV value in OCV table,
+	 * which means now battery capacity is 0%, and we should adjust the
+	 * inititial capacity to 0.
+	 */
+	if (ocv <= data->cap_table[data->table_len - 1].ocv) {
+		sc27xx_fgu_adjust_cap(data, 0);
+	} else if (ocv <= data->min_volt) {
+		/*
+		 * If current OCV value is less than the low alarm voltage, but
+		 * current capacity is larger than the alarm capacity, we should
+		 * adjust the inititial capacity to alarm capacity.
+		 */
+		if (cap > data->alarm_cap) {
+			sc27xx_fgu_adjust_cap(data, data->alarm_cap);
+		} else if (cap <= 0) {
+			int cur_cap;
+
+			/*
+			 * If current capacity is equal with 0 or less than 0
+			 * (some error occurs), we should adjust inititial
+			 * capacity to the capacity corresponding to current OCV
+			 * value.
+			 */
+			cur_cap = power_supply_ocv2cap_simple(data->cap_table,
+							      data->table_len,
+							      ocv);
+			sc27xx_fgu_adjust_cap(data, cur_cap);
+		}
+
+		/*
+		 * After adjusting the battery capacity, we should set the
+		 * lowest alarm voltage instead.
+		 */
+		data->min_volt = data->cap_table[data->table_len - 1].ocv;
+		adc = sc27xx_fgu_voltage_to_adc(data, data->min_volt / 1000);
+		regmap_update_bits(data->regmap, data->base + SC27XX_FGU_LOW_OVERLOAD,
+				   SC27XX_FGU_LOW_OVERLOAD_MASK, adc);
+	}
+
+out:
+	mutex_unlock(&data->lock);
+
+	power_supply_changed(data->battery);
+	return IRQ_HANDLED;
+}
+
 static irqreturn_t sc27xx_fgu_bat_detection(int irq, void *dev_id)
 {
 	struct sc27xx_fgu_data *data = dev_id;
@@ -509,7 +614,7 @@ static int sc27xx_fgu_hw_init(struct sc27xx_fgu_data *data)
 {
 	struct power_supply_battery_info info = { };
 	struct power_supply_battery_ocv_table *table;
-	int ret;
+	int ret, delta_clbcnt, alarm_adc;
 
 	ret = power_supply_get_battery_info(data->battery, &info);
 	if (ret) {
@@ -520,6 +625,7 @@ static int sc27xx_fgu_hw_init(struct sc27xx_fgu_data *data)
 	data->total_cap = info.charge_full_design_uah / 1000;
 	data->max_volt = info.constant_charge_voltage_max_uv / 1000;
 	data->internal_resist = info.factory_internal_resistance_uohm / 1000;
+	data->min_volt = info.voltage_min_design_uv;
 
 	/*
 	 * For SC27XX fuel gauge device, we only use one ocv-capacity
@@ -537,6 +643,10 @@ static int sc27xx_fgu_hw_init(struct sc27xx_fgu_data *data)
 		return -ENOMEM;
 	}
 
+	data->alarm_cap = power_supply_ocv2cap_simple(data->cap_table,
+						      data->table_len,
+						      data->min_volt);
+
 	power_supply_put_battery_info(data->battery, &info);
 
 	ret = sc27xx_fgu_calibration(data);
@@ -559,6 +669,50 @@ static int sc27xx_fgu_hw_init(struct sc27xx_fgu_data *data)
 		goto disable_fgu;
 	}
 
+	ret = regmap_update_bits(data->regmap, data->base + SC27XX_FGU_INT_CLR,
+				 SC27XX_FGU_INT_MASK, SC27XX_FGU_INT_MASK);
+	if (ret) {
+		dev_err(data->dev, "failed to clear interrupt status\n");
+		goto disable_clk;
+	}
+
+	/*
+	 * Set the voltage low overload threshold, which means when the battery
+	 * voltage is lower than this threshold, the controller will generate
+	 * one interrupt to notify.
+	 */
+	alarm_adc = sc27xx_fgu_voltage_to_adc(data, data->min_volt / 1000);
+	ret = regmap_update_bits(data->regmap, data->base + SC27XX_FGU_LOW_OVERLOAD,
+				 SC27XX_FGU_LOW_OVERLOAD_MASK, alarm_adc);
+	if (ret) {
+		dev_err(data->dev, "failed to set fgu low overload\n");
+		goto disable_clk;
+	}
+
+	/*
+	 * Set the coulomb counter delta threshold, that means when the coulomb
+	 * counter change is multiples of the delta threshold, the controller
+	 * will generate one interrupt to notify the users to update the battery
+	 * capacity. Now we set the delta threshold as a counter value of 1%
+	 * capacity.
+	 */
+	delta_clbcnt = sc27xx_fgu_cap_to_clbcnt(data, 1);
+
+	ret = regmap_update_bits(data->regmap, data->base + SC27XX_FGU_CLBCNT_DELTL,
+				 SC27XX_FGU_CLBCNT_MASK, delta_clbcnt);
+	if (ret) {
+		dev_err(data->dev, "failed to set low delta coulomb counter\n");
+		goto disable_clk;
+	}
+
+	ret = regmap_update_bits(data->regmap, data->base + SC27XX_FGU_CLBCNT_DELTH,
+				 SC27XX_FGU_CLBCNT_MASK,
+				 delta_clbcnt >> SC27XX_FGU_CLBCNT_SHIFT);
+	if (ret) {
+		dev_err(data->dev, "failed to set high delta coulomb counter\n");
+		goto disable_clk;
+	}
+
 	/*
 	 * Get the boot battery capacity when system powers on, which is used to
 	 * initialize the coulomb counter. After that, we can read the coulomb
@@ -658,6 +812,21 @@ static int sc27xx_fgu_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
+		dev_err(&pdev->dev, "no irq resource specified\n");
+		return irq;
+	}
+
+	ret = devm_request_threaded_irq(data->dev, irq, NULL,
+					sc27xx_fgu_interrupt,
+					IRQF_NO_SUSPEND | IRQF_ONESHOT,
+					pdev->name, data);
+	if (ret) {
+		dev_err(data->dev, "failed to request fgu IRQ\n");
+		return ret;
+	}
+
 	irq = gpiod_to_irq(data->gpiod);
 	if (irq < 0) {
 		dev_err(&pdev->dev, "failed to translate GPIO to IRQ\n");
-- 
1.7.9.5

Re: [PATCH 3/5] power: supply: sc27xx: Add fuel gauge low voltage alarm

[Pavel Machek]

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

On Wed 2018-11-14 17:07:06, Baolin Wang wrote:
> From: Yuanjiang Yu <yuanjiang.yu@unisoc.com>
> 
> Add low voltage alarm support to make sure the battery capacity
> more accurate in lower voltage stage.
> 
> Signed-off-by: Yuanjiang Yu <yuanjiang.yu@unisoc.com>
> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>

Should we also shut down the system when that happens, as battery is
empty?

Or is there any lower threshold when we should do the shutdown?
								Pavel
								
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

Re: [PATCH 3/5] power: supply: sc27xx: Add fuel gauge low voltage alarm

[Baolin Wang]

Hi Pavel,
On Mon, 26 Nov 2018 at 05:45, Pavel Machek <pavel@ucw.cz> wrote:
>
> On Wed 2018-11-14 17:07:06, Baolin Wang wrote:
> > From: Yuanjiang Yu <yuanjiang.yu@unisoc.com>
> >
> > Add low voltage alarm support to make sure the battery capacity
> > more accurate in lower voltage stage.
> >
> > Signed-off-by: Yuanjiang Yu <yuanjiang.yu@unisoc.com>
> > Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
>
> Should we also shut down the system when that happens, as battery is
> empty?
>
> Or is there any lower threshold when we should do the shutdown?

We do not shutdown the system when battery is empty in fuel gauge
driver, instead we should do that in charger manager service or other
upper layers. In fuel gauge driver, we should just supply the accurate
battery capacity for upper layer.

-- 
Baolin Wang
Best Regards

[PATCH 4/5] power: supply: sc27xx: Add suspend/resume interfaces

[Baolin Wang]

From: Yuanjiang Yu <yuanjiang.yu@unisoc.com>

Add fuel gauge platform suspend and resume interfaces. In suspend state,
we should enable the low voltage and coulomb counter threshold interrupts
to wake up system to calibrate the battery capacity in lower voltage stage.

Signed-off-by: Yuanjiang Yu <yuanjiang.yu@unisoc.com>
Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
 drivers/power/supply/sc27xx_fuel_gauge.c |   77 ++++++++++++++++++++++++++++++
 1 file changed, 77 insertions(+)

diff --git a/drivers/power/supply/sc27xx_fuel_gauge.c b/drivers/power/supply/sc27xx_fuel_gauge.c
index 962d0f8..8c52e29 100644
--- a/drivers/power/supply/sc27xx_fuel_gauge.c
+++ b/drivers/power/supply/sc27xx_fuel_gauge.c
@@ -789,6 +789,7 @@ static int sc27xx_fgu_probe(struct platform_device *pdev)
 	data->bat_present = !!ret;
 	mutex_init(&data->lock);
 	data->dev = &pdev->dev;
+	platform_set_drvdata(pdev, data);
 
 	fgu_cfg.drv_data = data;
 	fgu_cfg.of_node = np;
@@ -846,6 +847,81 @@ static int sc27xx_fgu_probe(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_PM_SLEEP
+static int sc27xx_fgu_resume(struct device *dev)
+{
+	struct sc27xx_fgu_data *data = dev_get_drvdata(dev);
+	int ret;
+
+	ret = regmap_update_bits(data->regmap, data->base + SC27XX_FGU_INT_EN,
+				 SC27XX_FGU_LOW_OVERLOAD_INT |
+				 SC27XX_FGU_CLBCNT_DELTA_INT, 0);
+	if (ret) {
+		dev_err(data->dev, "failed to disable fgu interrupts\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static int sc27xx_fgu_suspend(struct device *dev)
+{
+	struct sc27xx_fgu_data *data = dev_get_drvdata(dev);
+	int ret, status, ocv;
+
+	ret = sc27xx_fgu_get_status(data, &status);
+	if (ret)
+		return ret;
+
+	/*
+	 * If we are charging, then no need to enable the FGU interrupts to
+	 * adjust the battery capacity.
+	 */
+	if (status != POWER_SUPPLY_STATUS_NOT_CHARGING)
+		return 0;
+
+	ret = regmap_update_bits(data->regmap, data->base + SC27XX_FGU_INT_EN,
+				 SC27XX_FGU_LOW_OVERLOAD_INT,
+				 SC27XX_FGU_LOW_OVERLOAD_INT);
+	if (ret) {
+		dev_err(data->dev, "failed to enable low voltage interrupt\n");
+		return ret;
+	}
+
+	ret = sc27xx_fgu_get_vbat_ocv(data, &ocv);
+	if (ret)
+		goto disable_int;
+
+	/*
+	 * If current OCV is less than the minimum voltage, we should enable the
+	 * coulomb counter threshold interrupt to notify events to adjust the
+	 * battery capacity.
+	 */
+	if (ocv < data->min_volt) {
+		ret = regmap_update_bits(data->regmap,
+					 data->base + SC27XX_FGU_INT_EN,
+					 SC27XX_FGU_CLBCNT_DELTA_INT,
+					 SC27XX_FGU_CLBCNT_DELTA_INT);
+		if (ret) {
+			dev_err(data->dev,
+				"failed to enable coulomb threshold int\n");
+			goto disable_int;
+		}
+	}
+
+	return 0;
+
+disable_int:
+	regmap_update_bits(data->regmap, data->base + SC27XX_FGU_INT_EN,
+			   SC27XX_FGU_LOW_OVERLOAD_INT, 0);
+	return ret;
+}
+#endif
+
+static const struct dev_pm_ops sc27xx_fgu_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(sc27xx_fgu_suspend, sc27xx_fgu_resume)
+};
+
 static const struct of_device_id sc27xx_fgu_of_match[] = {
 	{ .compatible = "sprd,sc2731-fgu", },
 	{ }
@@ -856,6 +932,7 @@ static int sc27xx_fgu_probe(struct platform_device *pdev)
 	.driver = {
 		.name = "sc27xx-fgu",
 		.of_match_table = sc27xx_fgu_of_match,
+		.pm = &sc27xx_fgu_pm_ops,
 	}
 };
 
-- 
1.7.9.5

[PATCH 5/5] power: supply: sc27xx: Save last battery capacity

[Baolin Wang]

From: Yuanjiang Yu <yuanjiang.yu@unisoc.com>

Our charger manager can optimize the battery capacity periodically, so
we can save last battery capacity into registers. Then next system
power-on, we can read the last saved battery capacity as the initial
battery capacity, which can make the battery capacity more accurate.

Signed-off-by: Yuanjiang Yu <yuanjiang.yu@unisoc.com>
Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
 drivers/power/supply/sc27xx_fuel_gauge.c |  143 +++++++++++++++++++++++++++++-
 1 file changed, 142 insertions(+), 1 deletion(-)

diff --git a/drivers/power/supply/sc27xx_fuel_gauge.c b/drivers/power/supply/sc27xx_fuel_gauge.c
index 8c52e29..181a8f7 100644
--- a/drivers/power/supply/sc27xx_fuel_gauge.c
+++ b/drivers/power/supply/sc27xx_fuel_gauge.c
@@ -39,6 +39,9 @@
 #define SC27XX_FGU_CLBCNT_VALH		0x68
 #define SC27XX_FGU_CLBCNT_VALL		0x6c
 #define SC27XX_FGU_CLBCNT_QMAXL		0x74
+#define SC27XX_FGU_USER_AREA_SET	0xa0
+#define SC27XX_FGU_USER_AREA_CLEAR	0xa4
+#define SC27XX_FGU_USER_AREA_STATUS	0xa8
 
 #define SC27XX_WRITE_SELCLB_EN		BIT(0)
 #define SC27XX_FGU_CLBCNT_MASK		GENMASK(15, 0)
@@ -49,6 +52,14 @@
 #define SC27XX_FGU_LOW_OVERLOAD_INT	BIT(0)
 #define SC27XX_FGU_CLBCNT_DELTA_INT	BIT(2)
 
+#define SC27XX_FGU_MODE_AREA_MASK	GENMASK(15, 12)
+#define SC27XX_FGU_CAP_AREA_MASK	GENMASK(11, 0)
+#define SC27XX_FGU_MODE_AREA_SHIFT	12
+
+#define SC27XX_FGU_FIRST_POWERTON	GENMASK(3, 0)
+#define SC27XX_FGU_DEFAULT_CAP		GENMASK(11, 0)
+#define SC27XX_FGU_NORMAIL_POWERTON	0x5
+
 #define SC27XX_FGU_CUR_BASIC_ADC	8192
 #define SC27XX_FGU_SAMPLE_HZ		2
 
@@ -119,6 +130,80 @@ static int sc27xx_fgu_voltage_to_adc(struct sc27xx_fgu_data *data, int vol)
 	return DIV_ROUND_CLOSEST(vol * data->vol_1000mv_adc, 1000);
 }
 
+static bool sc27xx_fgu_is_first_poweron(struct sc27xx_fgu_data *data)
+{
+	int ret, status, cap, mode;
+
+	ret = regmap_read(data->regmap,
+			  data->base + SC27XX_FGU_USER_AREA_STATUS, &status);
+	if (ret)
+		return false;
+
+	/*
+	 * We use low 4 bits to save the last battery capacity and high 12 bits
+	 * to save the system boot mode.
+	 */
+	mode = (status & SC27XX_FGU_MODE_AREA_MASK) >> SC27XX_FGU_MODE_AREA_SHIFT;
+	cap = status & SC27XX_FGU_CAP_AREA_MASK;
+
+	/*
+	 * When FGU has been powered down, the user area registers became
+	 * default value (0xffff), which can be used to valid if the system is
+	 * first power on or not.
+	 */
+	if (mode == SC27XX_FGU_FIRST_POWERTON || cap == SC27XX_FGU_DEFAULT_CAP)
+		return true;
+
+	return false;
+}
+
+static int sc27xx_fgu_save_boot_mode(struct sc27xx_fgu_data *data,
+				     int boot_mode)
+{
+	int ret;
+
+	ret = regmap_update_bits(data->regmap,
+				 data->base + SC27XX_FGU_USER_AREA_CLEAR,
+				 SC27XX_FGU_MODE_AREA_MASK,
+				 SC27XX_FGU_MODE_AREA_MASK);
+	if (ret)
+		return ret;
+
+	return regmap_update_bits(data->regmap,
+				  data->base + SC27XX_FGU_USER_AREA_SET,
+				  SC27XX_FGU_MODE_AREA_MASK,
+				  boot_mode << SC27XX_FGU_MODE_AREA_SHIFT);
+}
+
+static int sc27xx_fgu_save_last_cap(struct sc27xx_fgu_data *data, int cap)
+{
+	int ret;
+
+	ret = regmap_update_bits(data->regmap,
+				 data->base + SC27XX_FGU_USER_AREA_CLEAR,
+				 SC27XX_FGU_CAP_AREA_MASK,
+				 SC27XX_FGU_CAP_AREA_MASK);
+	if (ret)
+		return ret;
+
+	return regmap_update_bits(data->regmap,
+				  data->base + SC27XX_FGU_USER_AREA_SET,
+				  SC27XX_FGU_CAP_AREA_MASK, cap);
+}
+
+static int sc27xx_fgu_read_last_cap(struct sc27xx_fgu_data *data, int *cap)
+{
+	int ret, value;
+
+	ret = regmap_read(data->regmap,
+			  data->base + SC27XX_FGU_USER_AREA_STATUS, &value);
+	if (ret)
+		return ret;
+
+	*cap = value & SC27XX_FGU_CAP_AREA_MASK;
+	return 0;
+}
+
 /*
  * When system boots on, we can not read battery capacity from coulomb
  * registers, since now the coulomb registers are invalid. So we should
@@ -128,6 +213,20 @@ static int sc27xx_fgu_voltage_to_adc(struct sc27xx_fgu_data *data, int vol)
 static int sc27xx_fgu_get_boot_capacity(struct sc27xx_fgu_data *data, int *cap)
 {
 	int volt, cur, oci, ocv, ret;
+	bool is_first_poweron = sc27xx_fgu_is_first_poweron(data);
+
+	/*
+	 * If system is not the first power on, we should use the last saved
+	 * battery capacity as the initial battery capacity. Otherwise we should
+	 * re-calculate the initial battery capacity.
+	 */
+	if (!is_first_poweron) {
+		ret = sc27xx_fgu_read_last_cap(data, cap);
+		if (ret)
+			return ret;
+
+		return sc27xx_fgu_save_boot_mode(data, SC27XX_FGU_NORMAIL_POWERTON);
+	}
 
 	/*
 	 * After system booting on, the SC27XX_FGU_CLBCNT_QMAXL register saved
@@ -160,7 +259,11 @@ static int sc27xx_fgu_get_boot_capacity(struct sc27xx_fgu_data *data, int *cap)
 	*cap = power_supply_ocv2cap_simple(data->cap_table, data->table_len,
 					   ocv);
 
-	return 0;
+	ret = sc27xx_fgu_save_last_cap(data, *cap);
+	if (ret)
+		return ret;
+
+	return sc27xx_fgu_save_boot_mode(data, SC27XX_FGU_NORMAIL_POWERTON);
 }
 
 static int sc27xx_fgu_set_clbcnt(struct sc27xx_fgu_data *data, int clbcnt)
@@ -418,6 +521,30 @@ static int sc27xx_fgu_get_property(struct power_supply *psy,
 	return ret;
 }
 
+static int sc27xx_fgu_set_property(struct power_supply *psy,
+				   enum power_supply_property psp,
+				   const union power_supply_propval *val)
+{
+	struct sc27xx_fgu_data *data = power_supply_get_drvdata(psy);
+	int ret;
+
+	mutex_lock(&data->lock);
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_CAPACITY:
+		ret = sc27xx_fgu_save_last_cap(data, val->intval);
+		if (ret < 0)
+			dev_err(data->dev, "failed to save battery capacity\n");
+		break;
+
+	default:
+		ret = -EINVAL;
+	}
+
+	mutex_unlock(&data->lock);
+	return ret;
+}
+
 static void sc27xx_fgu_external_power_changed(struct power_supply *psy)
 {
 	struct sc27xx_fgu_data *data = power_supply_get_drvdata(psy);
@@ -425,6 +552,18 @@ static void sc27xx_fgu_external_power_changed(struct power_supply *psy)
 	power_supply_changed(data->battery);
 }
 
+static int sc27xx_fgu_property_is_writeable(struct power_supply *psy,
+					    enum power_supply_property psp)
+{
+	switch (psp) {
+	case POWER_SUPPLY_PROP_CAPACITY:
+		return 1;
+
+	default:
+		return 0;
+	}
+}
+
 static enum power_supply_property sc27xx_fgu_props[] = {
 	POWER_SUPPLY_PROP_STATUS,
 	POWER_SUPPLY_PROP_HEALTH,
@@ -444,7 +583,9 @@ static void sc27xx_fgu_external_power_changed(struct power_supply *psy)
 	.properties		= sc27xx_fgu_props,
 	.num_properties		= ARRAY_SIZE(sc27xx_fgu_props),
 	.get_property		= sc27xx_fgu_get_property,
+	.set_property		= sc27xx_fgu_set_property,
 	.external_power_changed	= sc27xx_fgu_external_power_changed,
+	.property_is_writeable	= sc27xx_fgu_property_is_writeable,
 };
 
 static void sc27xx_fgu_adjust_cap(struct sc27xx_fgu_data *data, int cap)
-- 
1.7.9.5

Re: [PATCH 5/5] power: supply: sc27xx: Save last battery capacity

[Pavel Machek]

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

Hi!

> Our charger manager can optimize the battery capacity periodically, so
> we can save last battery capacity into registers. Then next system
> power-on, we can read the last saved battery capacity as the initial
> battery capacity, which can make the battery capacity more accurate.
> 
> Signed-off-by: Yuanjiang Yu <yuanjiang.yu@unisoc.com>
> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
> ---
>  drivers/power/supply/sc27xx_fuel_gauge.c |  143 +++++++++++++++++++++++++++++-
>  1 file changed, 142 insertions(+), 1 deletion(-)
> 

> +static int sc27xx_fgu_set_property(struct power_supply *psy,
> +				   enum power_supply_property psp,
> +				   const union power_supply_propval *val)
> +{
> +	struct sc27xx_fgu_data *data = power_supply_get_drvdata(psy);
> +	int ret;
> +
> +	mutex_lock(&data->lock);
> +
> +	switch (psp) {
> +	case POWER_SUPPLY_PROP_CAPACITY:
> +		ret = sc27xx_fgu_save_last_cap(data, val->intval);
> +		if (ret < 0)
> +			dev_err(data->dev, "failed to save battery capacity\n");
> +		break;
> +
> +	default:
> +		ret = -EINVAL;
> +	}
> +
> +	mutex_unlock(&data->lock);
> +	return ret;

if (psp != ....) return -EINVAL; And you can do that outside
lock...

Ok, OTOH this is easier to extend in future. Do you expect more
writable properties?

> +static int sc27xx_fgu_property_is_writeable(struct power_supply *psy,
> +					    enum power_supply_property psp)
> +{
> +	switch (psp) {
> +	case POWER_SUPPLY_PROP_CAPACITY:
> +		return 1;
> +
> +	default:
> +		return 0;
> +	}
> +}


Same here. return psp == POWER_SUPPLY_PROP_CAPACITY; really looks
strange written like this.

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

Re: [PATCH 5/5] power: supply: sc27xx: Save last battery capacity

[Baolin Wang]

Hi Pavel,
On Mon, 26 Nov 2018 at 05:48, Pavel Machek <pavel@ucw.cz> wrote:
>
> Hi!
>
> > Our charger manager can optimize the battery capacity periodically, so
> > we can save last battery capacity into registers. Then next system
> > power-on, we can read the last saved battery capacity as the initial
> > battery capacity, which can make the battery capacity more accurate.
> >
> > Signed-off-by: Yuanjiang Yu <yuanjiang.yu@unisoc.com>
> > Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
> > ---
> >  drivers/power/supply/sc27xx_fuel_gauge.c |  143 +++++++++++++++++++++++++++++-
> >  1 file changed, 142 insertions(+), 1 deletion(-)
> >
>
> > +static int sc27xx_fgu_set_property(struct power_supply *psy,
> > +                                enum power_supply_property psp,
> > +                                const union power_supply_propval *val)
> > +{
> > +     struct sc27xx_fgu_data *data = power_supply_get_drvdata(psy);
> > +     int ret;
> > +
> > +     mutex_lock(&data->lock);
> > +
> > +     switch (psp) {
> > +     case POWER_SUPPLY_PROP_CAPACITY:
> > +             ret = sc27xx_fgu_save_last_cap(data, val->intval);
> > +             if (ret < 0)
> > +                     dev_err(data->dev, "failed to save battery capacity\n");
> > +             break;
> > +
> > +     default:
> > +             ret = -EINVAL;
> > +     }
> > +
> > +     mutex_unlock(&data->lock);
> > +     return ret;
>
> if (psp != ....) return -EINVAL; And you can do that outside
> lock...
>
> Ok, OTOH this is easier to extend in future. Do you expect more
> writable properties?

Until now I think there are no writable properties, I think I can
change it like you suggested.

>
> > +static int sc27xx_fgu_property_is_writeable(struct power_supply *psy,
> > +                                         enum power_supply_property psp)
> > +{
> > +     switch (psp) {
> > +     case POWER_SUPPLY_PROP_CAPACITY:
> > +             return 1;
> > +
> > +     default:
> > +             return 0;
> > +     }
> > +}
>
>
> Same here. return psp == POWER_SUPPLY_PROP_CAPACITY; really looks
> strange written like this.

Sure. Thanks.

-- 
Baolin Wang
Best Regards

Re: [PATCH 0/5] Add new features for SC27XX fuel gauge driver

[Sebastian Reichel]

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

Hi,

On Wed, Nov 14, 2018 at 05:07:03PM +0800, Baolin Wang wrote:
> This patch set adds some new features for SC27XX fuel gauge driver.
> 
> 1. Read calibration data from eFuse device to calibrate fuel gauge.
> 2. Add low voltage alarm to adjust the battery capacity in lower
> voltage stage.
> 3. Add power management interfaces
> 4. Save last optimized battery capacity to be used as the initial
> battery capacity if system is not first power-on.
> 
> Baolin Wang (2):
>   dt-bindings: power: supply: Add nvmem properties to calibrate FGU
>   power: supply: sc27xx: Add fuel gauge calibration
> 
> Yuanjiang Yu (3):
>   power: supply: sc27xx: Add fuel gauge low voltage alarm
>   power: supply: sc27xx: Add suspend/resume interfaces
>   power: supply: sc27xx: Save last battery capacity
> 
>  .../devicetree/bindings/power/supply/sc27xx-fg.txt |    4 +
>  drivers/power/supply/sc27xx_fuel_gauge.c           |  453 +++++++++++++++++++-
>  2 files changed, 444 insertions(+), 13 deletions(-)

I applied patches 1-4 and skipped patch 5 due to pending changes.

-- Sebastian

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

Re: [PATCH 0/5] Add new features for SC27XX fuel gauge driver

[Baolin Wang]

Hi Sebastian,

On Thu, 6 Dec 2018 at 08:10, Sebastian Reichel
<sebastian.reichel@collabora.com> wrote:
>
> Hi,
>
> On Wed, Nov 14, 2018 at 05:07:03PM +0800, Baolin Wang wrote:
> > This patch set adds some new features for SC27XX fuel gauge driver.
> >
> > 1. Read calibration data from eFuse device to calibrate fuel gauge.
> > 2. Add low voltage alarm to adjust the battery capacity in lower
> > voltage stage.
> > 3. Add power management interfaces
> > 4. Save last optimized battery capacity to be used as the initial
> > battery capacity if system is not first power-on.
> >
> > Baolin Wang (2):
> >   dt-bindings: power: supply: Add nvmem properties to calibrate FGU
> >   power: supply: sc27xx: Add fuel gauge calibration
> >
> > Yuanjiang Yu (3):
> >   power: supply: sc27xx: Add fuel gauge low voltage alarm
> >   power: supply: sc27xx: Add suspend/resume interfaces
> >   power: supply: sc27xx: Save last battery capacity
> >
> >  .../devicetree/bindings/power/supply/sc27xx-fg.txt |    4 +
> >  drivers/power/supply/sc27xx_fuel_gauge.c           |  453 +++++++++++++++++++-
> >  2 files changed, 444 insertions(+), 13 deletions(-)
>
> I applied patches 1-4 and skipped patch 5 due to pending changes.

I will send new version with addressing comments in patch 5. Thanks.

-- 
Baolin Wang
Best Regards