[v4,2/2] power: supply: Add STC3117 fuel gauge unit driver

Commit Message

Bhavin Sharma Nov. 27, 2024, 3:19 p.m. UTC

Adds initial support for the STC3117 fuel gauge.

The driver provides functionality to monitor key parameters including:
- Voltage
- Current
- State of Charge (SOC)
- Temperature
- Status

Signed-off-by: Hardevsinh Palaniya <[e-mail]>
Signed-off-by: Bhavin Sharma <[e-mail]>
---
 MAINTAINERS                               |   8 +
 drivers/power/supply/Kconfig              |   7 +
 drivers/power/supply/Makefile             |   1 +
 drivers/power/supply/stc3117_fuel_gauge.c | 625 ++++++++++++++++++++++
 4 files changed, 641 insertions(+)
 create mode 100644 drivers/power/supply/stc3117_fuel_gauge.c

+++ b/drivers/power/supply/stc3117_fuel_gauge.c
@@ -0,0 +-2,622 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * stc3117_fuel_gauge.c - STMicroelectronics STC3117 Fuel Gauge Driver
+ *
+ * Copyright (c) 2024 Silicon Signals Pvt Ltd.
+ * Author:      Bhavin Sharma <[e-mail]>
+ *              Hardevsinh Palaniya <[e-mail]>
+ */
+
+#include <linux/i2c.h>
+#include <linux/workqueue.h>
+#include <linux/power_supply.h>
+#include <linux/regmap.h>
+#include <linux/crc8.h>
+
+#define STC3117_ADDR_MODE                       0x00
+#define STC3117_ADDR_CTRL                       0x01
+#define STC3117_ADDR_SOC_L                      0x02
+#define STC3117_ADDR_SOC_H                      0x03
+#define STC3117_ADDR_COUNTER_L                  0x04
+#define STC3117_ADDR_COUNTER_H                  0x05
+#define STC3117_ADDR_CURRENT_L                  0x06
+#define STC3117_ADDR_CURRENT_H                  0x07
+#define STC3117_ADDR_VOLTAGE_L                  0x08
+#define STC3117_ADDR_VOLTAGE_H                  0x09
+#define STC3117_ADDR_TEMPERATURE                0x0A
+#define STC3117_ADDR_AVG_CURRENT_L              0X0B
+#define STC3117_ADDR_AVG_CURRENT_H              0X0C
+#define STC3117_ADDR_OCV_L                      0X0D
+#define STC3117_ADDR_OCV_H                      0X0E
+#define STC3117_ADDR_CC_CNF_L                   0X0F
+#define STC3117_ADDR_CC_CNF_H                   0X10
+#define STC3117_ADDR_VM_CNF_L                   0X11
+#define STC3117_ADDR_VM_CNF_H                   0X12
+#define STC3117_ADDR_ALARM_SOC                  0X13
+#define STC3117_ADDR_ALARM_VOLTAGE              0X14
+#define STC3117_ADDR_ID                         0X18
+#define STC3117_ADDR_CC_ADJ_L			0X1B
+#define STC3117_ADDR_CC_ADJ_H			0X1C
+#define STC3117_ADDR_VM_ADJ_L			0X1D
+#define STC3117_ADDR_VM_ADJ_H			0X1E
+#define STC3117_ADDR_RAM			0x20
+#define STC3117_ADDR_OCV_TABLE			0x30
+#define STC3117_ADDR_SOC_TABLE			0x30
+
+/************ Bit mask definition ************/
+#define STC3117_ID			        0x16
+#define STC3117_MIXED_MODE			0x00
+#define STC3117_VMODE				BIT(0)
+#define STC3117_GG_RUN				BIT(4)
+#define STC3117_CC_MODE				BIT(5)
+#define STC3117_BATFAIL			BIT(3)
+#define STC3117_PORDET				BIT(4)
+
+#define STC3117_RAM_SIZE			16
+#define STC3117_OCV_TABLE_SIZE			16
+#define STC3117_RAM_TESTWORD			0x53A9
+#define STC3117_SOFT_RESET                      0x11
+#define STC3117_NOMINAL_CAPACITY		2600
+
+#define VOLTAGE_LSB_VALUE			9011
+#define CURRENT_LSB_VALUE			24084
+#define RSENSE					10
+#define BATT_CAPACITY				1800
+#define BATTERY_RINT				60
+#define APP_CUTOFF_VOLTAGE			2500
+#define BATT_CHG_VOLTAGE			4250
+#define BATT_MIN_VOLTAGE			3300
+#define MAX_HRSOC				51200
+#define MAX_SOC				1000
+#define CHG_MIN_CURRENT			200
+#define CHG_END_CURRENT			20
+#define APP_MIN_CURRENT			(-5)
+#define BATTERY_FULL				95
+#define CRC8_POLYNOMIAL			0x07
+#define CRC8_INIT				0x00
+#define CRC8_TABLE_SIZE			256
+
+DECLARE_CRC8_TABLE(stc3117_crc_table);
+
+enum stc3117_state {
+	STC3117_INIT,
+	STC3117_RUNNING,
+	STC3117_POWERDN,
+};
+
+enum stc3117_status {
+	BATT_LOWBATT = -2,
+	BATT_DISCHARG,
+	BATT_IDLE,
+	BATT_FULCHARG,
+	BATT_ENDCHARG,
+	BATT_CHARGING,
+};
+
+/* Default OCV curve Li-ion battery */
+static const int OCVValue[16] = {
+    3400, 3582, 3669, 3676, 3699, 3737, 3757, 3774,
+    3804, 3844, 3936, 3984, 4028, 4131, 4246, 4320
+};
+
+static union stc3117_internal_ram {
+	u8 ram_bytes[STC3117_RAM_SIZE];
+	struct {
+	u16 TestWord;   /* 0-1    Bytes */
+	u16 HRSOC;      /* 2-3    Bytes */
+	u16 CC_cnf;     /* 4-5    Bytes */
+	u16 VM_cnf;     /* 6-7    Bytes */
+	u8 soc;         /* 8      Byte  */
+	u8 state;       /* 9      Byte  */
+	u8 unused[5];   /* 10-14  Bytes */
+	u8 CRC;         /* 15     Byte  */
+	} reg;
+} ram_data;
+
+struct stc3117_data {
+	struct i2c_client	*client;
+	struct regmap		*regmap;
+	struct delayed_work update_work;
+	struct power_supply	*battery;
+
+	u8 SOCValue[16];
+	int CC_cnf;
+	int VM_cnf;
+	int CC_adj;
+	int VM_adj;
+	int AvgCurrent;
+	int AvgVoltage;
+	int Current;
+	int Voltage;
+	int Temp;
+	int SOC;
+	int OCV;
+	int HRSOC;
+	int Presence;
+	int Battery_state;
+};
+
+static int STC3117_convert(int value, int factor)
+{
+	value = (value * factor) / 4096;
+	return value;
+}
+
+static int stc3117_get_battery_data(struct stc3117_data *data)
+{
+	u8 reg_list[16];
+	u8 data_adjust[4];
+	int value, mode;
+
+	regmap_bulk_read(data->regmap, STC3117_ADDR_MODE, reg_list, sizeof(reg_list));
+
+	/* SOC */
+	value = (reg_list[3] << 8) + reg_list[2];
+	data->HRSOC = value;
+	data->SOC = (value * 10 + 256) / 512;
+
+	/* cureent */
+	value = (reg_list[7] << 8) + reg_list[6];
+	data->Current = STC3117_convert(value, CURRENT_LSB_VALUE / 10);
+
+	/* Voltage */
+	value = (reg_list[9] << 8) + reg_list[8];
+	data->Voltage = STC3117_convert(value, VOLTAGE_LSB_VALUE);  /* result in mV */
+
+	/* Temp */
+	data->Temp = reg_list[10];
+
+	/* Avg current */
+	value = (reg_list[12] << 8) + reg_list[11];
+	regmap_read(data->regmap, STC3117_ADDR_MODE, &mode);
+	if (!(mode & STC3117_VMODE)) {
+		value = STC3117_convert(value, CURRENT_LSB_VALUE / 10);
+		value = value / 4;
+	} else {
+		value = STC3117_convert(value, 36 * STC3117_NOMINAL_CAPACITY);
+	}
+	data->AvgCurrent = value;  /* result in mA */
+
+	/* OCV */
+	value = (reg_list[14] << 8) + reg_list[13];
+	value = STC3117_convert(value, VOLTAGE_LSB_VALUE);  /* result in mV */
+	value = (value + 2) / 4;
+	data->OCV = value;
+
+	/* CC & VM adjustment counters */
+	regmap_bulk_read(data->regmap, STC3117_ADDR_CC_ADJ_L, data_adjust, sizeof(data_adjust));
+	value = (data_adjust[1] << 8) + data_adjust[0];
+	data->CC_adj = value;
+
+	value = (data_adjust[3] << 8) + data_adjust[2];
+	data->VM_adj = value;
+
+	return 0;
+}
+
+static int ram_write(struct stc3117_data *data)
+{
+	int ret;
+
+	ret = regmap_bulk_write(data->regmap, STC3117_ADDR_RAM, ram_data.ram_bytes, STC3117_RAM_SIZE);
+	if (ret)
+		return -EINVAL;
+
+	return 0;
+};
+
+static int stc3117_update_battery_status(struct stc3117_data *data)
+{
+	switch (data->Battery_state) {
+	case BATT_CHARGING:
+		if (data->AvgCurrent < CHG_MIN_CURRENT)
+			data->Battery_state = BATT_ENDCHARG;
+		break;
+
+	case BATT_ENDCHARG:
+		if (data->Current > CHG_MIN_CURRENT)
+			data->Battery_state = BATT_CHARGING;
+		else if (data->AvgCurrent < CHG_END_CURRENT)
+			data->Battery_state = BATT_IDLE;
+		else if ((data->Current > CHG_END_CURRENT) && (data->Voltage > BATT_CHG_VOLTAGE))
+			data->Battery_state = BATT_FULCHARG;
+		break;
+
+	case BATT_FULCHARG:
+		if ((data->Current > CHG_MIN_CURRENT))
+			data->Battery_state = BATT_CHARGING;
+		else if (data->AvgCurrent < CHG_END_CURRENT) {
+			if (data->AvgVoltage > BATT_CHG_VOLTAGE)
+			{
+				regmap_write(data->regmap, STC3117_ADDR_SOC_H, (MAX_HRSOC >> 8 & 0xFF));
+				regmap_write(data->regmap, STC3117_ADDR_SOC_L, (MAX_HRSOC & 0xFF));
+	       			data->SOC = MAX_SOC;
+			}
+			data->Battery_state = BATT_IDLE;
+		}
+		break;
+
+	case BATT_IDLE:
+		if (data->Current > CHG_END_CURRENT)
+			data->Battery_state = BATT_CHARGING;
+		else if (data->Current < APP_MIN_CURRENT)
+			data->Battery_state = BATT_DISCHARG;
+		break;
+
+	case BATT_DISCHARG:
+		if (data->Current > APP_MIN_CURRENT)
+			data->Battery_state = BATT_IDLE;
+		else if (data->AvgVoltage < BATT_MIN_VOLTAGE)
+			data->Battery_state = BATT_LOWBATT;
+		break;
+
+	case BATT_LOWBATT:
+	  	if (data->AvgVoltage > (BATT_MIN_VOLTAGE + 50))
+			data->Battery_state = BATT_IDLE;
+		break;
+
+	default:
+		data->Battery_state = BATT_IDLE;
+	}
+
+	return 0;
+}
+
+static int ram_read(struct stc3117_data *data)
+{
+	int ret;
+
+	ret = regmap_bulk_read(data->regmap, STC3117_ADDR_RAM, ram_data.ram_bytes, STC3117_RAM_SIZE);
+	if (ret)
+		return -EINVAL;
+
+	return 0;
+};
+
+static int stc3117_set_para(struct stc3117_data *data)
+{
+	int ret;
+
+	ret = regmap_write(data->regmap, STC3117_ADDR_MODE, STC3117_VMODE);
+
+	for (int i = 0; i < STC3117_OCV_TABLE_SIZE; i++)
+		regmap_write(data->regmap, STC3117_ADDR_OCV_TABLE + i, OCVValue[i] * 100 / 55);
+
+	if (data->SOCValue[1] != 0)
+		regmap_bulk_write(data->regmap, STC3117_ADDR_SOC_TABLE, data->SOCValue, STC3117_OCV_TABLE_SIZE);
+	ret |= regmap_write(data->regmap, STC3117_ADDR_CC_CNF_H,
+						(ram_data.reg.CC_cnf >> 8) & 0xFF);
+	ret |= regmap_write(data->regmap, STC3117_ADDR_CC_CNF_L,
+						ram_data.reg.CC_cnf & 0xFF);
+	ret |= regmap_write(data->regmap, STC3117_ADDR_VM_CNF_H,
+						(ram_data.reg.VM_cnf >> 8) & 0xFF);
+	ret |= regmap_write(data->regmap, STC3117_ADDR_VM_CNF_L,
+						ram_data.reg.VM_cnf & 0xFF);
+	ret |= regmap_write(data->regmap, STC3117_ADDR_CTRL, 0x03);
+
+	ret |= regmap_write(data->regmap, STC3117_ADDR_MODE, STC3117_MIXED_MODE | STC3117_GG_RUN);
+
+	return ret;
+};
+
+static int stc3117_init(struct stc3117_data *data)
+{
+	int ID, ret;
+	int ctrl;
+	int ocv_m, ocv_l;
+
+	regmap_read(data->regmap, STC3117_ADDR_ID, &ID);
+	if (ID != STC3117_ID)
+		return -EINVAL;
+
+	data->CC_cnf = (BATT_CAPACITY * RSENSE * 250 + 6194) / 12389;
+	data->VM_cnf = (BATT_CAPACITY * 200 * 50 + 24444) / 48889;
+
+	/* Battery has not been removed */
+
+	data->Presence = 1;
+
+	/* Read RAM data */
+	ret = ram_read(data);
+	if (ret)
+		return -EINVAL;
+
+	if ((ram_data.reg.TestWord != STC3117_RAM_TESTWORD) ||
+                (crc8(stc3117_crc_table, ram_data.ram_bytes, STC3117_RAM_SIZE, CRC8_INIT)) != 0)
+	{
+		ram_data.reg.TestWord = STC3117_RAM_TESTWORD;
+		ram_data.reg.CC_cnf = data->CC_cnf;
+		ram_data.reg.VM_cnf = data->VM_cnf;
+		ram_data.reg.CRC = crc8(stc3117_crc_table, ram_data.ram_bytes,
+                	                        STC3117_RAM_SIZE - 1, CRC8_INIT);
+
+		ret = regmap_read(data->regmap, STC3117_ADDR_OCV_H, &ocv_m);
+
+		ret |= regmap_read(data->regmap, STC3117_ADDR_OCV_L, &ocv_l);
+
+		ret |= stc3117_set_para(data);
+
+		ret |= regmap_write(data->regmap, STC3117_ADDR_OCV_H, ocv_m);
+
+		ret |= regmap_write(data->regmap, STC3117_ADDR_OCV_L, ocv_l);
+		if (ret)
+			return -EINVAL;
+	} else {
+		ret = regmap_read(data->regmap, STC3117_ADDR_CTRL, &ctrl);
+		if ((ctrl & STC3117_BATFAIL) != 0  || (ctrl & STC3117_PORDET) != 0)
+		{
+			ret = regmap_read(data->regmap, STC3117_ADDR_OCV_H, &ocv_m);
+
+			ret |= regmap_read(data->regmap, STC3117_ADDR_OCV_L, &ocv_l);
+
+			ret |= stc3117_set_para(data);
+
+			ret |= regmap_write(data->regmap, STC3117_ADDR_OCV_H, ocv_m);
+
+			ret |= regmap_write(data->regmap, STC3117_ADDR_OCV_L, ocv_l);
+			if (ret)
+				return -EINVAL;
+		} else {
+			ret = stc3117_set_para(data);
+			if (ret)
+				return -EINVAL;
+			regmap_write(data->regmap, STC3117_ADDR_SOC_H, (ram_data.reg.HRSOC >> 8 & 0xFF));
+			regmap_write(data->regmap, STC3117_ADDR_SOC_L, (ram_data.reg.HRSOC & 0xFF));
+		}
+	}
+
+	ram_data.reg.state = STC3117_INIT;
+	ram_data.reg.CRC = crc8(stc3117_crc_table, ram_data.ram_bytes,
+						STC3117_RAM_SIZE - 1, CRC8_INIT);
+	ret = ram_write(data);
+	if (ret)
+		return -EINVAL;
+
+	data->Battery_state = BATT_IDLE;
+
+	return 0;
+};
+
+static int stc3117_task(struct stc3117_data *data)
+{
+	int ID, mode, ret;
+	int count_l, count_m;
+	int ocv_l, ocv_m;
+
+	regmap_read(data->regmap, STC3117_ADDR_ID, &ID);
+	if (ID != STC3117_ID) {
+		data->Presence = 0;
+		return -EINVAL;
+	}
+
+	stc3117_get_battery_data(data);
+
+	/* Read RAM data */
+	ret = ram_read(data);
+	if (ret)
+		return -EINVAL;
+
+	if ((ram_data.reg.TestWord != STC3117_RAM_TESTWORD) ||
+			(crc8(stc3117_crc_table, ram_data.ram_bytes, STC3117_RAM_SIZE, CRC8_INIT) != 0))
+	{
+		ram_data.reg.TestWord = STC3117_RAM_TESTWORD; 
+		ram_data.reg.CC_cnf = data->CC_cnf;
+		ram_data.reg.VM_cnf = data->VM_cnf;
+		ram_data.reg.CRC = crc8(stc3117_crc_table, ram_data.ram_bytes,
+                                STC3117_RAM_SIZE - 1, CRC8_INIT);
+		ram_data.reg.state = STC3117_INIT;
+	}
+
+	/* check battery presence status */
+	regmap_read(data->regmap, STC3117_ADDR_CTRL, &mode);
+	if ((mode & STC3117_BATFAIL) != 0)
+	{
+		data->Presence = 0;
+		ram_data.reg.TestWord = 0;
+		ram_data.reg.state = STC3117_INIT;
+		ret = ram_write(data);
+		if (ret)
+			return -EINVAL;
+		regmap_write(data->regmap, STC3117_ADDR_CTRL, STC3117_PORDET);
+	}
+
+	data->Presence = 1;
+
+	 regmap_read(data->regmap, STC3117_ADDR_MODE, &mode);
+	if ((mode & STC3117_GG_RUN) == 0)
+	{
+		if (ram_data.reg.state > STC3117_INIT) {
+			ret = stc3117_set_para(data);
+			if (ret)
+				return -EINVAL;
+			regmap_write(data->regmap, STC3117_ADDR_SOC_L, (ram_data.reg.HRSOC & 0xFF));
+			regmap_write(data->regmap, STC3117_ADDR_SOC_H, (ram_data.reg.HRSOC >> 8 & 0xFF));
+		} else {
+			ret = regmap_read(data->regmap, STC3117_ADDR_OCV_H, &ocv_m);
+
+			ret |= regmap_read(data->regmap, STC3117_ADDR_OCV_L, &ocv_l);
+
+			ret |= stc3117_set_para(data);
+
+			ret |= regmap_write(data->regmap, STC3117_ADDR_OCV_H, ocv_m);
+
+			ret |= regmap_write(data->regmap, STC3117_ADDR_OCV_L, ocv_l);
+			if (ret)
+				return -EINVAL;
+		}
+		ram_data.reg.state = STC3117_INIT;
+	}
+
+	regmap_read(data->regmap, STC3117_ADDR_COUNTER_L, &count_l);
+	regmap_read(data->regmap, STC3117_ADDR_COUNTER_H, &count_m);
+
+	count_m = (count_m << 8) + count_l;
+
+	/* INIT state, wait for current & temperature value available: */
+	if (ram_data.reg.state == STC3117_INIT && count_m > 4) {
+		data->AvgVoltage = data->Voltage;
+		data->AvgCurrent = data->Current;
+		ram_data.reg.state = STC3117_RUNNING;
+	}
+
+	if (ram_data.reg.state != STC3117_RUNNING)
+	{
+    		data->Current = 0;
+        	data->Temp = 250;
+	} else {
+		if (data->Voltage < APP_CUTOFF_VOLTAGE)
+			data->SOC = 0;
+
+		if (mode & STC3117_VMODE) {
+			data->AvgCurrent = 0;
+			data->Current = 0;
+		} else {
+		       stc3117_update_battery_status(data);
+		}
+	}
+
+	ram_data.reg.HRSOC = data->HRSOC;
+	ram_data.reg.soc = (data->SOC + 5) / 10;
+	ram_data.reg.CRC = crc8(stc3117_crc_table, ram_data.ram_bytes,
+        	        STC3117_RAM_SIZE - 1, CRC8_INIT);
+
+	ret = ram_write(data);
+	if (ret)
+		return -EINVAL;
+	return 0;
+};
+
+static void fuel_gauge_update_work(struct work_struct *work)
+{
+	struct stc3117_data *data = container_of(to_delayed_work(work), struct stc3117_data, update_work);
+
+	stc3117_task(data);
+
+	/* Schedule the work to run again in 2 seconds */
+	schedule_delayed_work(&data->update_work, msecs_to_jiffies(2000));
+}
+
+static int stc3117_get_property(struct power_supply *psy,
+				enum power_supply_property psp, union power_supply_propval *val)
+{
+	struct stc3117_data *data = power_supply_get_drvdata(psy);
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_STATUS:
+		if (data->SOC > BATTERY_FULL)
+			val->intval = POWER_SUPPLY_STATUS_FULL;
+		if (data->Current < 0)
+			val->intval = POWER_SUPPLY_STATUS_CHARGING;
+		else if (data->Current > 0)
+			val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
+		else
+			val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING;
+		break;
+	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+		val->intval = data->Voltage;
+		break;
+	case POWER_SUPPLY_PROP_CURRENT_NOW:
+		val->intval = data->Current;
+		break;
+	case POWER_SUPPLY_PROP_CAPACITY:
+		val->intval = data->SOC;
+		break;
+	case POWER_SUPPLY_PROP_TEMP:
+		val->intval = data->Temp;
+		break;
+	case POWER_SUPPLY_PROP_PRESENT:
+		val->intval = data->Presence;
+		break;
+	default:
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static enum power_supply_property stc3117_battery_props[] = {
+	POWER_SUPPLY_PROP_STATUS,
+	POWER_SUPPLY_PROP_VOLTAGE_NOW,
+	POWER_SUPPLY_PROP_CURRENT_NOW,
+	POWER_SUPPLY_PROP_CAPACITY,
+	POWER_SUPPLY_PROP_TEMP,
+	POWER_SUPPLY_PROP_PRESENT,
+};
+
+static const struct power_supply_desc stc3117_battery_desc = {
+	.name = "stc3117-battery",
+	.type = POWER_SUPPLY_TYPE_BATTERY,
+	.get_property = stc3117_get_property,
+	.properties = stc3117_battery_props,
+	.num_properties = ARRAY_SIZE(stc3117_battery_props),
+};
+
+static const struct regmap_config stc3117_regmap_config = {
+	.reg_bits       = 8,
+	.val_bits       = 8,
+};
+
+static int stc3117_probe(struct i2c_client *client)
+{
+	struct stc3117_data *data;
+	struct power_supply_config psy_cfg = {};
+	int ret;
+
+	data = devm_kzalloc(&client->dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	data->client = client;
+	data->regmap = devm_regmap_init_i2c(client, &stc3117_regmap_config);
+	if (IS_ERR(data->regmap))
+		return PTR_ERR(data->regmap);
+
+	i2c_set_clientdata(client, data);
+	psy_cfg.drv_data = data;
+
+	crc8_populate_msb(stc3117_crc_table, CRC8_POLYNOMIAL);
+
+	ret = stc3117_init(data);
+	if (ret)
+		dev_err_probe(&client->dev, ret, "failed to initialization of stc3117\n");
+
+	INIT_DELAYED_WORK(&data->update_work, fuel_gauge_update_work);
+
+	schedule_delayed_work(&data->update_work, 0);
+
+	data->battery = devm_power_supply_register(&client->dev,
+						   &stc3117_battery_desc, &psy_cfg);
+	if (IS_ERR(data->battery))
+		dev_err_probe(&client->dev, PTR_ERR(data->battery), "failed to register battery\n");
+
+	return 0;
+}
+
+static const struct i2c_device_id stc3117_id[] = {
+	{"stc3117", 0},
+	{},
+};
+MODULE_DEVICE_TABLE(i2c, stc3117_id);
+
+static const struct of_device_id stc3117_of_match[] = {
+	{ .compatible = "st,stc3117" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, stc3117_of_match);
+
+static struct i2c_driver stc3117_i2c_driver = {
+	.driver = {
+		.name = "stc3117_i2c_driver",
+		.of_match_table = stc3117_of_match,
+	},
+	.probe = stc3117_probe,
+	.id_table = stc3117_id,
+};
+
+module_i2c_driver(stc3117_i2c_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Bhavin Sharma <[e-mail]>");
+MODULE_AUTHOR("Hardevsinh Palaniya <[e-mail]>");
+MODULE_DESCRIPTION("STC3117 Fuel Gauge Driver");

Comments

Krzysztof Kozlowski Nov. 27, 2024, 6:24 p.m. UTC | #1

On 27/11/2024 16:19, Bhavin Sharma wrote:
> Adds initial support for the STC3117 fuel gauge.
> 
> The driver provides functionality to monitor key parameters including:
> - Voltage
> - Current
> - State of Charge (SOC)
> - Temperature
> - Status
> 
> Signed-off-by: Hardevsinh Palaniya <[e-mail]>
> Signed-off-by: Bhavin Sharma <[e-mail]>
> ---
>  MAINTAINERS                               |   8 +
>  drivers/power/supply/Kconfig              |   7 +
>  drivers/power/supply/Makefile             |   1 +
>  drivers/power/supply/stc3117_fuel_gauge.c | 625 ++++++++++++++++++++++
>  4 files changed, 641 insertions(+)
>  create mode 100644 drivers/power/supply/stc3117_fuel_gauge.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 82161bc70b51..42c1af29eddb 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -21855,6 +21855,14 @@ T:	git git://linuxtv.org/media_tree.git
>  F:	Documentation/devicetree/bindings/media/i2c/st,st-mipid02.yaml
>  F:	drivers/media/i2c/st-mipid02.c
>  
> +ST STC3117 FUEL GAUGE DRIVER
> +M:	Hardevsinh Palaniya <[e-mail]>
> +M:	Bhavin Sharma <[e-mail]>
> +L:	[e-mail]
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/power/supply/st,stc3117.yaml
> +F:	drivers/power/supply/stc3117_fuel_gauge.c
> +
>  ST STM32 FIREWALL
>  M:	Gatien Chevallier <[e-mail]>
>  S:	Maintained
> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
> index bcfa63fb9f1e..6ad968fa1f69 100644
> --- a/drivers/power/supply/Kconfig
> +++ b/drivers/power/supply/Kconfig
> @@ -908,6 +908,13 @@ config FUEL_GAUGE_SC27XX
>  	  Say Y here to enable support for fuel gauge with SC27XX
>  	  PMIC chips.
>  
> +config FUEL_GAUGE_STC3117
> +	tristate "STMicroelectronics STC3117 fuel gauge driver"
> +	depends on I2C
> +	help
> +	  Say Y here to enable support for fuel gauge with STC3117
> +	  PMIC chips.
> +
>  config CHARGER_UCS1002
>  	tristate "Microchip UCS1002 USB Port Power Controller"
>  	depends on I2C
> diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
> index 8dcb41545317..aea3d35f27f3 100644
> --- a/drivers/power/supply/Makefile
> +++ b/drivers/power/supply/Makefile
> @@ -107,6 +107,7 @@ obj-$(CONFIG_CHARGER_CROS_USBPD)	+= cros_usbpd-charger.o
>  obj-$(CONFIG_CHARGER_CROS_PCHG)	+= cros_peripheral_charger.o
>  obj-$(CONFIG_CHARGER_SC2731)	+= sc2731_charger.o
>  obj-$(CONFIG_FUEL_GAUGE_SC27XX)	+= sc27xx_fuel_gauge.o
> +obj-$(CONFIG_FUEL_GAUGE_STC3117)	+= stc3117_fuel_gauge.o
>  obj-$(CONFIG_CHARGER_UCS1002)	+= ucs1002_power.o
>  obj-$(CONFIG_CHARGER_BD99954)	+= bd99954-charger.o
>  obj-$(CONFIG_CHARGER_WILCO)	+= wilco-charger.o
> diff --git a/drivers/power/supply/stc3117_fuel_gauge.c b/drivers/power/supply/stc3117_fuel_gauge.c
> new file mode 100644
> index 000000000000..99291bb9250f
> --- /dev/null
> +++ b/drivers/power/supply/stc3117_fuel_gauge.c
> @@ -0,0 +-2,622 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * stc3117_fuel_gauge.c - STMicroelectronics STC3117 Fuel Gauge Driver
> + *
> + * Copyright (c) 2024 Silicon Signals Pvt Ltd.
> + * Author:      Bhavin Sharma <[e-mail]>
> + *              Hardevsinh Palaniya <[e-mail]>
> + */
> +
> +#include <linux/i2c.h>
> +#include <linux/workqueue.h>
> +#include <linux/power_supply.h>
> +#include <linux/regmap.h>
> +#include <linux/crc8.h>
> +
> +#define STC3117_ADDR_MODE                       0x00
> +#define STC3117_ADDR_CTRL                       0x01
> +#define STC3117_ADDR_SOC_L                      0x02
> +#define STC3117_ADDR_SOC_H                      0x03
> +#define STC3117_ADDR_COUNTER_L                  0x04
> +#define STC3117_ADDR_COUNTER_H                  0x05
> +#define STC3117_ADDR_CURRENT_L                  0x06
> +#define STC3117_ADDR_CURRENT_H                  0x07
> +#define STC3117_ADDR_VOLTAGE_L                  0x08
> +#define STC3117_ADDR_VOLTAGE_H                  0x09
> +#define STC3117_ADDR_TEMPERATURE                0x0A
> +#define STC3117_ADDR_AVG_CURRENT_L              0X0B
> +#define STC3117_ADDR_AVG_CURRENT_H              0X0C
> +#define STC3117_ADDR_OCV_L                      0X0D
> +#define STC3117_ADDR_OCV_H                      0X0E
> +#define STC3117_ADDR_CC_CNF_L                   0X0F
> +#define STC3117_ADDR_CC_CNF_H                   0X10
> +#define STC3117_ADDR_VM_CNF_L                   0X11
> +#define STC3117_ADDR_VM_CNF_H                   0X12
> +#define STC3117_ADDR_ALARM_SOC                  0X13
> +#define STC3117_ADDR_ALARM_VOLTAGE              0X14
> +#define STC3117_ADDR_ID                         0X18
> +#define STC3117_ADDR_CC_ADJ_L			0X1B
> +#define STC3117_ADDR_CC_ADJ_H			0X1C
> +#define STC3117_ADDR_VM_ADJ_L			0X1D
> +#define STC3117_ADDR_VM_ADJ_H			0X1E
> +#define STC3117_ADDR_RAM			0x20
> +#define STC3117_ADDR_OCV_TABLE			0x30
> +#define STC3117_ADDR_SOC_TABLE			0x30
> +
> +/************ Bit mask definition ************/
> +#define STC3117_ID			        0x16
> +#define STC3117_MIXED_MODE			0x00
> +#define STC3117_VMODE				BIT(0)
> +#define STC3117_GG_RUN				BIT(4)
> +#define STC3117_CC_MODE				BIT(5)
> +#define STC3117_BATFAIL			BIT(3)
> +#define STC3117_PORDET				BIT(4)
> +
> +#define STC3117_RAM_SIZE			16
> +#define STC3117_OCV_TABLE_SIZE			16
> +#define STC3117_RAM_TESTWORD			0x53A9
> +#define STC3117_SOFT_RESET                      0x11
> +#define STC3117_NOMINAL_CAPACITY		2600
> +
> +#define VOLTAGE_LSB_VALUE			9011
> +#define CURRENT_LSB_VALUE			24084
> +#define RSENSE					10
> +#define BATT_CAPACITY				1800
> +#define BATTERY_RINT				60
> +#define APP_CUTOFF_VOLTAGE			2500
> +#define BATT_CHG_VOLTAGE			4250
> +#define BATT_MIN_VOLTAGE			3300
> +#define MAX_HRSOC				51200
> +#define MAX_SOC				1000
> +#define CHG_MIN_CURRENT			200
> +#define CHG_END_CURRENT			20
> +#define APP_MIN_CURRENT			(-5)
> +#define BATTERY_FULL				95
> +#define CRC8_POLYNOMIAL			0x07
> +#define CRC8_INIT				0x00
> +#define CRC8_TABLE_SIZE			256
> +
> +DECLARE_CRC8_TABLE(stc3117_crc_table);
> +
> +enum stc3117_state {
> +	STC3117_INIT,
> +	STC3117_RUNNING,
> +	STC3117_POWERDN,
> +};
> +
> +enum stc3117_status {
> +	BATT_LOWBATT = -2,
> +	BATT_DISCHARG,
> +	BATT_IDLE,
> +	BATT_FULCHARG,
> +	BATT_ENDCHARG,
> +	BATT_CHARGING,
> +};
> +
> +/* Default OCV curve Li-ion battery */
> +static const int OCVValue[16] = {
> +    3400, 3582, 3669, 3676, 3699, 3737, 3757, 3774,
> +    3804, 3844, 3936, 3984, 4028, 4131, 4246, 4320
> +};
> +
> +static union stc3117_internal_ram {
> +	u8 ram_bytes[STC3117_RAM_SIZE];
> +	struct {
> +	u16 TestWord;   /* 0-1    Bytes */
> +	u16 HRSOC;      /* 2-3    Bytes */
> +	u16 CC_cnf;     /* 4-5    Bytes */
> +	u16 VM_cnf;     /* 6-7    Bytes */
> +	u8 soc;         /* 8      Byte  */
> +	u8 state;       /* 9      Byte  */
> +	u8 unused[5];   /* 10-14  Bytes */
> +	u8 CRC;         /* 15     Byte  */
> +	} reg;
> +} ram_data;
> +
> +struct stc3117_data {
> +	struct i2c_client	*client;
> +	struct regmap		*regmap;
> +	struct delayed_work update_work;
> +	struct power_supply	*battery;
> +
> +	u8 SOCValue[16];
> +	int CC_cnf;
> +	int VM_cnf;
> +	int CC_adj;
> +	int VM_adj;
> +	int AvgCurrent;
> +	int AvgVoltage;
> +	int Current;
> +	int Voltage;
> +	int Temp;
> +	int SOC;
> +	int OCV;
> +	int HRSOC;
> +	int Presence;
> +	int Battery_state;


That's some Windows coding style... You need to clean up everything here
to match Linux Coding style.


> +};
> +




> +	i2c_set_clientdata(client, data);
> +	psy_cfg.drv_data = data;
> +
> +	crc8_populate_msb(stc3117_crc_table, CRC8_POLYNOMIAL);
> +
> +	ret = stc3117_init(data);
> +	if (ret)
> +		dev_err_probe(&client->dev, ret, "failed to initialization of stc3117\n");
> +
> +	INIT_DELAYED_WORK(&data->update_work, fuel_gauge_update_work);
> +
> +	schedule_delayed_work(&data->update_work, 0);
> +
> +	data->battery = devm_power_supply_register(&client->dev,
> +						   &stc3117_battery_desc, &psy_cfg);
> +	if (IS_ERR(data->battery))
> +		dev_err_probe(&client->dev, PTR_ERR(data->battery), "failed to register battery\n");
> +
You ignored (again!) received comments. In multiple places. Go back to
previous email and carefully read commetns.

One more thing:

Please wrap code according to coding style (checkpatch is not a coding
style description, but only a tool).


Best regards,
Krzysztof

Bhavin Sharma Nov. 28, 2024, 8:44 a.m. UTC | #2

Hi Krzysztof,
 
Thank you for your review and feedback.
 
> > +struct stc3117_data {
> > +     struct i2c_client       *client;
> > +     struct regmap           *regmap;
> > +     struct delayed_work update_work;
> > +     struct power_supply     *battery;
> > +
> > +     u8 SOCValue[16];
> > +     int CC_cnf;
> > +     int VM_cnf;
> > +     int CC_adj;
> > +     int VM_adj;
> > +     int AvgCurrent;
> > +     int AvgVoltage;
> > +     int Current;
> > +     int Voltage;
> > +     int Temp;
> > +     int SOC;
> > +     int OCV;
> > +     int HRSOC;
> > +     int Presence;
> > +     int Battery_state;
>
> That's some Windows coding style... You need to clean up everything here
> to match Linux Coding style.
 
Could you clarify what specific changes are required here to align with the Linux
coding style?
 
I am not sure what exactly needs to be changed here.
 
> > +     data->battery = devm_power_supply_register(&client->dev,
> > +                                                &stc3117_battery_desc, &psy_cfg);
> > +     if (IS_ERR(data->battery))
> > +             dev_err_probe(&client->dev, PTR_ERR(data->battery), "failed to register battery\n");
> > +
> You ignored (again!) received comments. In multiple places. Go back to
> previous email and carefully read commetns.
 
Sebastian suggested using dev_err_probe, while you mentioned using dev_err.
so what should i follow ?
 
> One more thing:
> 
> Please wrap code according to coding style (checkpatch is not a coding
> style description, but only a tool).
 
Could you recommend an example driver from the kernel source tree that 
follows the expected coding style? This would help me ensure compliance.
 
Best Regards,
Bhavin

Krzysztof Kozlowski Nov. 28, 2024, 8:55 a.m. UTC | #3

On 28/11/2024 09:44, Bhavin Sharma wrote:
> Hi Krzysztof,
>  
> Thank you for your review and feedback.
>  
>>> +struct stc3117_data {
>>> +     struct i2c_client       *client;
>>> +     struct regmap           *regmap;
>>> +     struct delayed_work update_work;
>>> +     struct power_supply     *battery;
>>> +
>>> +     u8 SOCValue[16];
>>> +     int CC_cnf;
>>> +     int VM_cnf;
>>> +     int CC_adj;
>>> +     int VM_adj;
>>> +     int AvgCurrent;
>>> +     int AvgVoltage;
>>> +     int Current;
>>> +     int Voltage;
>>> +     int Temp;
>>> +     int SOC;
>>> +     int OCV;
>>> +     int HRSOC;
>>> +     int Presence;
>>> +     int Battery_state;
>>
>> That's some Windows coding style... You need to clean up everything here
>> to match Linux Coding style.
>  
> Could you clarify what specific changes are required here to align with the Linux
> coding style?


Entire. Go one by one: "Breaking long lines and strings" - not
implemented. "Naming" - not implemented. Then go with every point. You
are making here some sort of shortcut - ignoring coding style, not
reading it and insisting on me to provide you exact things to change.
No, that's way too many things. You are supposed to read the coding style.

>  
> I am not sure what exactly needs to be changed here.
>  
>>> +     data->battery = devm_power_supply_register(&client->dev,
>>> +                                                &stc3117_battery_desc, &psy_cfg);
>>> +     if (IS_ERR(data->battery))
>>> +             dev_err_probe(&client->dev, PTR_ERR(data->battery), "failed to register battery\n");
>>> +
>> You ignored (again!) received comments. In multiple places. Go back to
>> previous email and carefully read commetns.
>  
> Sebastian suggested using dev_err_probe, while you mentioned using dev_err.
> so what should i follow ?


No. That's not true. Read comments again. I am not happy that after
pointing out you still insist and force me to re-iterate the same.
That's my last reply in this matter:

comment was:
"return dev_err_probe(dev, PTR_ERR(stc_sply), "failed to register
battery\n");"

Where do you have "return" statement?

What about all my other comments? You are supposed to reply inline and
acknowledge each of such comment. That's the only way I believe you will
really do what we ask you to do.

> 
>> One more thing:
>>  
>> Please wrap code according to coding style (checkpatch is not a coding
>> style description, but only a tool).
>  
> Could you recommend an example driver from the kernel source tree that 
> follows the expected coding style? This would help me ensure compliance.


`git log -- path` will tell give you the latest drivers..


>  
> Best Regards,
> Bhavin
>  


Trim your replies and do not top-post. All this copied stuff below is
making things just difficult to read.

> 
> 
> 
> 
> 
> 
> 
> ________________________________________
> From: Krzysztof Kozlowski <[e-mail]>
> Sent: Wednesday, November 27, 2024 11:54 PM
> To: Bhavin Sharma <[e-mail]>; [e-mail] <[e-mail]>; [e-mail] <[e-mail]>; [e-mail] <[e-mail]>; [e-mail] <[e-mail]>
> Cc: Hardevsinh Palaniya <[e-mail]>; [e-mail] <[e-mail]>; [e-mail] <[e-mail]>; linux-


All this must be removed.


Best regards,
Krzysztof

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index 82161bc70b51..42c1af29eddb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -21855,6 +21855,14 @@  T:	git git://linuxtv.org/media_tree.git
 F:	Documentation/devicetree/bindings/media/i2c/st,st-mipid02.yaml
 F:	drivers/media/i2c/st-mipid02.c
 
+ST STC3117 FUEL GAUGE DRIVER
+M:	Hardevsinh Palaniya <[e-mail]>
+M:	Bhavin Sharma <[e-mail]>
+L:	[e-mail]
+S:	Maintained
+F:	Documentation/devicetree/bindings/power/supply/st,stc3117.yaml
+F:	drivers/power/supply/stc3117_fuel_gauge.c
+
 ST STM32 FIREWALL
 M:	Gatien Chevallier <[e-mail]>
 S:	Maintained
diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index bcfa63fb9f1e..6ad968fa1f69 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -908,6 +908,13 @@  config FUEL_GAUGE_SC27XX
 	  Say Y here to enable support for fuel gauge with SC27XX
 	  PMIC chips.
 
+config FUEL_GAUGE_STC3117
+	tristate "STMicroelectronics STC3117 fuel gauge driver"
+	depends on I2C
+	help
+	  Say Y here to enable support for fuel gauge with STC3117
+	  PMIC chips.
+
 config CHARGER_UCS1002
 	tristate "Microchip UCS1002 USB Port Power Controller"
 	depends on I2C
diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
index 8dcb41545317..aea3d35f27f3 100644
--- a/drivers/power/supply/Makefile
+++ b/drivers/power/supply/Makefile
@@ -107,6 +107,7 @@  obj-$(CONFIG_CHARGER_CROS_USBPD)	+= cros_usbpd-charger.o
 obj-$(CONFIG_CHARGER_CROS_PCHG)	+= cros_peripheral_charger.o
 obj-$(CONFIG_CHARGER_SC2731)	+= sc2731_charger.o
 obj-$(CONFIG_FUEL_GAUGE_SC27XX)	+= sc27xx_fuel_gauge.o
+obj-$(CONFIG_FUEL_GAUGE_STC3117)	+= stc3117_fuel_gauge.o
 obj-$(CONFIG_CHARGER_UCS1002)	+= ucs1002_power.o
 obj-$(CONFIG_CHARGER_BD99954)	+= bd99954-charger.o
 obj-$(CONFIG_CHARGER_WILCO)	+= wilco-charger.o
diff --git a/drivers/power/supply/stc3117_fuel_gauge.c b/drivers/power/supply/stc3117_fuel_gauge.c
new file mode 100644
index 000000000000..99291bb9250f
--- /dev/null