Add charger driver for ROHM BD718(15/28/78) PMIC charger block.
It is a stripped down version of the driver here:
https://lore.kernel.org/lkml/dbd97c1b0d715aa35a8b4d79741e433d97c562aa.1637061794.git.matti.vaittinen@fi.rohmeurope.com/
For the ease of review and to do a step-by-step approach remove all the
coloumb counter related stuff and do not sneak in BD71827 support.
Changes besides that:
Replace the custom property by a standard one and do not use megaohms
for the current sense resistor.
Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
drivers/power/supply/Kconfig | 11 +
drivers/power/supply/Makefile | 1 +
drivers/power/supply/bd71828-power.c | 1144 ++++++++++++++++++++++++++++++++++
3 files changed, 1156 insertions(+)
diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index 79ddb006e2dad6bf96b71ed570a37c006b5f9433..f3429f0aecf5a17fbaa52ee76899657181429a48 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -974,6 +974,17 @@ config CHARGER_UCS1002
Say Y to enable support for Microchip UCS1002 Programmable
USB Port Power Controller with Charger Emulation.
+config CHARGER_BD71828
+ tristate "Power-supply driver for ROHM BD71828 and BD71815 PMIC"
+ depends on MFD_ROHM_BD71828
+ select POWER_SIMPLE_GAUGE
+ help
+ Say Y here to enable support for charger, battery and fuel gauge
+ in ROHM BD71815, BD71817, ROHM BD71828 power management
+ ICs. This driver gets various bits of information about battery
+ and charger states and also implements fuel gauge based on
+ coulomb counters on PMIC.
+
config CHARGER_BD99954
tristate "ROHM bd99954 charger driver"
depends on I2C
diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
index f943c9150b326d41ff241f82610f70298635eb08..c6520a11f021c872f01250ae54eb4c63018cd428 100644
--- a/drivers/power/supply/Makefile
+++ b/drivers/power/supply/Makefile
@@ -113,6 +113,7 @@ 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_BD71828) += bd71828-power.o
obj-$(CONFIG_CHARGER_BD99954) += bd99954-charger.o
obj-$(CONFIG_CHARGER_WILCO) += wilco-charger.o
obj-$(CONFIG_RN5T618_POWER) += rn5t618_power.o
diff --git a/drivers/power/supply/bd71828-power.c b/drivers/power/supply/bd71828-power.c
new file mode 100644
index 0000000000000000000000000000000000000000..f2686c7eadf0515856d60123f57bca59b33bbd10
--- /dev/null
+++ b/drivers/power/supply/bd71828-power.c
@@ -0,0 +1,1144 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * bd71828-power.c
+ * @file ROHM BD71815, BD71828 and BD71878 Charger driver
+ *
+ * Copyright 2021.
+ */
+
+#include <linux/delay.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/mfd/rohm-bd71815.h>
+#include <linux/mfd/rohm-bd71828.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/power_supply.h>
+#include <linux/slab.h>
+#include <linux/uaccess.h>
+
+/* common defines */
+#define BD7182x_MASK_VBAT_U 0x1f
+#define BD7182x_MASK_VDCIN_U 0x0f
+#define BD7182x_MASK_IBAT_U 0x3f
+#define BD7182x_MASK_CURDIR_DISCHG 0x80
+#define BD7182x_MASK_CC_CCNTD_HI 0x0FFF
+#define BD7182x_MASK_CC_CCNTD 0x0FFFFFFF
+#define BD7182x_MASK_CHG_STATE 0x7f
+#define BD7182x_MASK_CC_FULL_CLR 0x10
+#define BD7182x_MASK_BAT_TEMP 0x07
+#define BD7182x_MASK_DCIN_DET BIT(0)
+#define BD7182x_MASK_CONF_PON BIT(0)
+#define BD71815_MASK_CONF_XSTB BIT(1)
+#define BD7182x_MASK_BAT_STAT 0x3f
+#define BD7182x_MASK_DCIN_STAT 0x07
+
+#define BD7182x_MASK_CCNTRST 0x80
+#define BD7182x_MASK_CCNTENB 0x40
+#define BD7182x_MASK_CCCALIB 0x20
+#define BD7182x_MASK_WDT_AUTO 0x40
+#define BD7182x_MASK_VBAT_ALM_LIMIT_U 0x01
+#define BD7182x_MASK_CHG_EN 0x01
+
+#define BD7182x_DCIN_COLLAPSE_DEFAULT 0x36
+
+/* Measured min and max value clear bits */
+#define BD718XX_MASK_VSYS_MIN_AVG_CLR 0x10
+
+#define JITTER_DEFAULT 3000
+#define MAX_CURRENT_DEFAULT 890000 /* uA */
+#define AC_NAME "bd71828_ac"
+#define BAT_NAME "bd71828_bat"
+
+/*
+ * VBAT Low voltage detection Threshold
+ * 0x00D4*16mV = 212*0.016 = 3.392v
+ */
+#define VBAT_LOW_TH 0x00D4
+
+#define THR_RELAX_CURRENT_DEFAULT 5 /* mA */
+#define THR_RELAX_TIME_DEFAULT (60 * 60) /* sec. */
+
+#define DGRD_CYC_CAP_DEFAULT 88 /* 1 micro Ah */
+
+#define DGRD_TEMP_H_DEFAULT 450 /* 0.1 degrees C */
+#define DGRD_TEMP_M_DEFAULT 250 /* 0.1 degrees C */
+#define DGRD_TEMP_L_DEFAULT 50 /* 0.1 degrees C */
+#define DGRD_TEMP_VL_DEFAULT 0 /* 0.1 degrees C */
+
+#define SOC_EST_MAX_NUM_DEFAULT 5
+#define PWRCTRL_NORMAL 0x22
+#define PWRCTRL_RESET 0x23
+
+/*
+ * Originally we relied upon a fixed size table of OCV and VDR params.
+ * However the exising linux power-supply batinfo interface for getting the OCV
+ * values from DT does not have fixed amount of OCV values. Thus we use fixed
+ * parameter amount only for values provided as module params - and use this
+ * only as maximum number of parameters when values come from DT.
+ */
+#define NUM_BAT_PARAMS 23
+#define MAX_NUM_VDR_VALUES NUM_BAT_PARAMS
+
+struct pwr_regs {
+ int used_init_regs;
+ u8 vbat_init;
+ u8 vbat_init2;
+ u8 vbat_init3;
+ u8 vbat_avg;
+ u8 ibat;
+ u8 ibat_avg;
+ u8 meas_clear;
+ u8 vsys_min_avg;
+ u8 btemp_vth;
+ u8 chg_state;
+ u8 coulomb3;
+ u8 coulomb2;
+ u8 coulomb1;
+ u8 coulomb0;
+ u8 coulomb_ctrl;
+ u8 vbat_rex_avg;
+ u8 coulomb_full3;
+ u8 cc_full_clr;
+ u8 coulomb_chg3;
+ u8 bat_temp;
+ u8 dcin_stat;
+ u8 dcin_collapse_limit;
+ u8 chg_set1;
+ u8 chg_en;
+ u8 vbat_alm_limit_u;
+ u8 batcap_mon_limit_u;
+ u8 conf;
+ u8 vdcin;
+};
+
+static const struct pwr_regs pwr_regs_bd71828 = {
+ .vbat_init = BD71828_REG_VBAT_INITIAL1_U,
+ .vbat_init2 = BD71828_REG_VBAT_INITIAL2_U,
+ .vbat_init3 = BD71828_REG_OCV_PWRON_U,
+ .used_init_regs = 3,
+ .vbat_avg = BD71828_REG_VBAT_U,
+ .ibat = BD71828_REG_IBAT_U,
+ .ibat_avg = BD71828_REG_IBAT_AVG_U,
+ .meas_clear = BD71828_REG_MEAS_CLEAR,
+ .vsys_min_avg = BD71828_REG_VSYS_MIN_AVG_U,
+ .btemp_vth = BD71828_REG_VM_BTMP_U,
+ .chg_state = BD71828_REG_CHG_STATE,
+ .coulomb3 = BD71828_REG_CC_CNT3,
+ .coulomb2 = BD71828_REG_CC_CNT2,
+ .coulomb1 = BD71828_REG_CC_CNT1,
+ .coulomb0 = BD71828_REG_CC_CNT0,
+ .coulomb_ctrl = BD71828_REG_COULOMB_CTRL,
+ .vbat_rex_avg = BD71828_REG_VBAT_REX_AVG_U,
+ .coulomb_full3 = BD71828_REG_CC_CNT_FULL3,
+ .cc_full_clr = BD71828_REG_COULOMB_CTRL2,
+ .coulomb_chg3 = BD71828_REG_CC_CNT_CHG3,
+ .bat_temp = BD71828_REG_BAT_TEMP,
+ .dcin_stat = BD71828_REG_DCIN_STAT,
+ .dcin_collapse_limit = BD71828_REG_DCIN_CLPS,
+ .chg_set1 = BD71828_REG_CHG_SET1,
+ .chg_en = BD71828_REG_CHG_EN,
+ .vbat_alm_limit_u = BD71828_REG_ALM_VBAT_LIMIT_U,
+ .batcap_mon_limit_u = BD71828_REG_BATCAP_MON_LIMIT_U,
+ .conf = BD71828_REG_CONF,
+ .vdcin = BD71828_REG_VDCIN_U,
+};
+
+static const struct pwr_regs pwr_regs_bd71815 = {
+ .vbat_init = BD71815_REG_VM_OCV_PRE_U,
+ .vbat_init2 = BD71815_REG_VM_OCV_PST_U,
+ .used_init_regs = 2,
+ .vbat_avg = BD71815_REG_VM_SA_VBAT_U,
+ /* BD71815 does not have separate current and current avg */
+ .ibat = BD71815_REG_CC_CURCD_U,
+ .ibat_avg = BD71815_REG_CC_CURCD_U,
+
+ .meas_clear = BD71815_REG_VM_SA_MINMAX_CLR,
+ .vsys_min_avg = BD71815_REG_VM_SA_VSYS_MIN_U,
+ .btemp_vth = BD71815_REG_VM_BTMP,
+ .chg_state = BD71815_REG_CHG_STATE,
+ .coulomb3 = BD71815_REG_CC_CCNTD_3,
+ .coulomb2 = BD71815_REG_CC_CCNTD_2,
+ .coulomb1 = BD71815_REG_CC_CCNTD_1,
+ .coulomb0 = BD71815_REG_CC_CCNTD_0,
+ .coulomb_ctrl = BD71815_REG_CC_CTRL,
+ .vbat_rex_avg = BD71815_REG_REX_SA_VBAT_U,
+ .coulomb_full3 = BD71815_REG_FULL_CCNTD_3,
+ .cc_full_clr = BD71815_REG_FULL_CTRL,
+ .coulomb_chg3 = BD71815_REG_CCNTD_CHG_3,
+ .bat_temp = BD71815_REG_BAT_TEMP,
+ .dcin_stat = BD71815_REG_DCIN_STAT,
+ .dcin_collapse_limit = BD71815_REG_DCIN_CLPS,
+ .chg_set1 = BD71815_REG_CHG_SET1,
+ .chg_en = BD71815_REG_CHG_SET1,
+ .vbat_alm_limit_u = BD71815_REG_ALM_VBAT_TH_U,
+ .batcap_mon_limit_u = BD71815_REG_CC_BATCAP1_TH_U,
+ .conf = BD71815_REG_CONF,
+
+ .vdcin = BD71815_REG_VM_DCIN_U,
+};
+
+struct bd71828_power {
+ struct regmap *regmap;
+ enum rohm_chip_type chip_type;
+ struct device *dev;
+ struct power_supply *ac;
+ struct power_supply *bat;
+
+ const struct pwr_regs *regs;
+ /* Reg val to uA */
+ int curr_factor;
+ int rsens;
+ int (*get_temp)(struct bd71828_power *pwr, int *temp);
+ int (*bat_inserted)(struct bd71828_power *pwr);
+};
+
+static int bd7182x_write16(struct bd71828_power *pwr, int reg, uint16_t val)
+{
+ __be16 tmp;
+
+ tmp = cpu_to_be16(val);
+
+ return regmap_bulk_write(pwr->regmap, reg, &tmp, sizeof(tmp));
+}
+
+static int bd7182x_read16_himask(struct bd71828_power *pwr, int reg, int himask,
+ uint16_t *val)
+{
+ struct regmap *regmap = pwr->regmap;
+ int ret;
+ __be16 rvals;
+ u8 *tmp = (u8 *) &rvals;
+
+ ret = regmap_bulk_read(regmap, reg, &rvals, sizeof(*val));
+ if (!ret) {
+ *tmp &= himask;
+ *val = be16_to_cpu(rvals);
+ }
+ return ret;
+}
+
+static int bd71828_get_vbat(struct bd71828_power *pwr, int *vcell)
+{
+ uint16_t tmp_vcell;
+ int ret;
+
+ ret = bd7182x_read16_himask(pwr, pwr->regs->vbat_avg,
+ BD7182x_MASK_VBAT_U, &tmp_vcell);
+ if (ret)
+ dev_err(pwr->dev, "Failed to read battery average voltage\n");
+ else
+ *vcell = ((int)tmp_vcell) * 1000;
+
+ return ret;
+}
+
+static int bd71828_get_current_ds_adc(struct bd71828_power *pwr, int *curr, int *curr_avg)
+{
+ __be16 tmp_curr;
+ char *tmp = (char *)&tmp_curr;
+ int dir = 1;
+ int regs[] = { pwr->regs->ibat, pwr->regs->ibat_avg };
+ int *vals[] = { curr, curr_avg };
+ int ret, i;
+
+ for (dir = 1, i = 0; i < ARRAY_SIZE(regs); i++) {
+ ret = regmap_bulk_read(pwr->regmap, regs[i], &tmp_curr,
+ sizeof(tmp_curr));
+ if (ret)
+ break;
+
+ if (*tmp & BD7182x_MASK_CURDIR_DISCHG)
+ dir = -1;
+
+ *tmp &= BD7182x_MASK_IBAT_U;
+ tmp_curr = be16_to_cpu(tmp_curr);
+
+ *vals[i] = dir * ((int)tmp_curr) * pwr->curr_factor;
+ }
+
+ return ret;
+}
+
+/* Unit is tenths of degree C */
+static int bd71815_get_temp(struct bd71828_power *pwr, int *temp)
+{
+ struct regmap *regmap = pwr->regmap;
+ int ret;
+ int t;
+
+ ret = regmap_read(regmap, pwr->regs->btemp_vth, &t);
+ t = 200 - t;
+
+ if (ret || t > 200) {
+ dev_err(pwr->dev, "Failed to read battery temperature\n");
+ *temp = 2000;
+ } else {
+ *temp = t * 10;
+ }
+
+ return ret;
+}
+
+/* Unit is tenths of degree C */
+static int bd71828_get_temp(struct bd71828_power *pwr, int *temp)
+{
+ uint16_t t;
+ int ret;
+ int tmp = 200 * 10000;
+
+ ret = bd7182x_read16_himask(pwr, pwr->regs->btemp_vth,
+ BD71828_MASK_VM_BTMP_U, &t);
+ if (ret || t > 3200)
+ dev_err(pwr->dev,
+ "Failed to read system min average voltage\n");
+
+ tmp -= 625ULL * (unsigned int)t;
+ *temp = tmp / 1000;
+
+ return ret;
+}
+
+static int bd71828_charge_status(struct bd71828_power *pwr,
+ int *s, int *h)
+{
+ unsigned int state;
+ int status, health;
+ int ret = 1;
+
+ ret = regmap_read(pwr->regmap, pwr->regs->chg_state, &state);
+ if (ret)
+ dev_err(pwr->dev, "charger status reading failed (%d)\n", ret);
+
+ state &= BD7182x_MASK_CHG_STATE;
+
+ dev_dbg(pwr->dev, "CHG_STATE %d\n", state);
+
+ switch (state) {
+ case 0x00:
+ ret = 0;
+ status = POWER_SUPPLY_STATUS_DISCHARGING;
+ health = POWER_SUPPLY_HEALTH_GOOD;
+ break;
+ case 0x01:
+ case 0x02:
+ case 0x03:
+ case 0x0E:
+ status = POWER_SUPPLY_STATUS_CHARGING;
+ health = POWER_SUPPLY_HEALTH_GOOD;
+ break;
+ case 0x0F:
+ ret = 0;
+ status = POWER_SUPPLY_STATUS_FULL;
+ health = POWER_SUPPLY_HEALTH_GOOD;
+ break;
+ case 0x10:
+ case 0x11:
+ case 0x12:
+ case 0x13:
+ case 0x14:
+ case 0x20:
+ case 0x21:
+ case 0x22:
+ case 0x23:
+ case 0x24:
+ ret = 0;
+ status = POWER_SUPPLY_STATUS_NOT_CHARGING;
+ health = POWER_SUPPLY_HEALTH_OVERHEAT;
+ break;
+ case 0x30:
+ case 0x31:
+ case 0x32:
+ case 0x40:
+ ret = 0;
+ status = POWER_SUPPLY_STATUS_DISCHARGING;
+ health = POWER_SUPPLY_HEALTH_GOOD;
+ break;
+ case 0x7f:
+ default:
+ ret = 0;
+ status = POWER_SUPPLY_STATUS_NOT_CHARGING;
+ health = POWER_SUPPLY_HEALTH_DEAD;
+ break;
+ }
+
+ if (s)
+ *s = status;
+ if (h)
+ *h = health;
+
+ return ret;
+}
+
+static int get_chg_online(struct bd71828_power *pwr, int *chg_online)
+{
+ int r, ret;
+
+ ret = regmap_read(pwr->regmap, pwr->regs->dcin_stat, &r);
+ if (ret) {
+ dev_err(pwr->dev, "Failed to read DCIN status\n");
+ return ret;
+ }
+ *chg_online = ((r & BD7182x_MASK_DCIN_DET) != 0);
+
+ return 0;
+}
+
+static int get_bat_online(struct bd71828_power *pwr, int *bat_online)
+{
+ int r, ret;
+
+#define BAT_OPEN 0x7
+ ret = regmap_read(pwr->regmap, pwr->regs->bat_temp, &r);
+ if (ret) {
+ dev_err(pwr->dev, "Failed to read battery temperature\n");
+ return ret;
+ }
+ *bat_online = ((r & BD7182x_MASK_BAT_TEMP) != BAT_OPEN);
+
+ return 0;
+}
+
+static int bd71828_bat_inserted(struct bd71828_power *pwr)
+{
+ int ret, val;
+
+ ret = regmap_read(pwr->regmap, pwr->regs->conf, &val);
+ if (ret) {
+ dev_err(pwr->dev, "Failed to read CONF register\n");
+ return 0;
+ }
+ ret = val & BD7182x_MASK_CONF_PON;
+
+ if (ret)
+ regmap_update_bits(pwr->regmap, pwr->regs->conf,
+ BD7182x_MASK_CONF_PON, 0);
+
+ return ret;
+}
+
+static int bd71815_bat_inserted(struct bd71828_power *pwr)
+{
+ int ret, val;
+
+ ret = regmap_read(pwr->regmap, pwr->regs->conf, &val);
+ if (ret) {
+ dev_err(pwr->dev, "Failed to read CONF register\n");
+ return ret;
+ }
+
+ ret = !(val & BD71815_MASK_CONF_XSTB);
+ if (ret)
+ regmap_write(pwr->regmap, pwr->regs->conf, val |
+ BD71815_MASK_CONF_XSTB);
+
+ return ret;
+}
+
+static int bd71828_init_hardware(struct bd71828_power *pwr)
+{
+ int ret;
+
+ /* TODO: Collapse limit should come from device-tree ? */
+ ret = regmap_write(pwr->regmap, pwr->regs->dcin_collapse_limit,
+ BD7182x_DCIN_COLLAPSE_DEFAULT);
+ if (ret) {
+ dev_err(pwr->dev, "Failed to write DCIN collapse limit\n");
+ return ret;
+ }
+
+ ret = pwr->bat_inserted(pwr);
+ if (ret < 0)
+ return ret;
+
+ if (ret) {
+
+ /* WDT_FST auto set */
+ ret = regmap_update_bits(pwr->regmap, pwr->regs->chg_set1,
+ BD7182x_MASK_WDT_AUTO,
+ BD7182x_MASK_WDT_AUTO);
+ if (ret)
+ return ret;
+
+ ret = bd7182x_write16(pwr, pwr->regs->vbat_alm_limit_u,
+ VBAT_LOW_TH);
+ if (ret)
+ return ret;
+
+ /*
+ * On BD71815 "we mask the power-state" from relax detection.
+ * I am unsure what the impact of the power-state would be if
+ * we didn't - but this is what the vendor driver did - and
+ * that driver has been used in few projects so I just assume
+ * this is needed.
+ */
+ if (pwr->chip_type == ROHM_CHIP_TYPE_BD71815) {
+ ret = regmap_set_bits(pwr->regmap,
+ BD71815_REG_REX_CTRL_1,
+ REX_PMU_STATE_MASK);
+ if (ret)
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
+static int bd71828_charger_get_property(struct power_supply *psy,
+ enum power_supply_property psp,
+ union power_supply_propval *val)
+{
+ struct bd71828_power *pwr = dev_get_drvdata(psy->dev.parent);
+ u32 vot;
+ uint16_t tmp;
+ int online;
+ int ret;
+
+ switch (psp) {
+ case POWER_SUPPLY_PROP_ONLINE:
+ ret = get_chg_online(pwr, &online);
+ if (!ret)
+ val->intval = online;
+ break;
+ case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+ ret = bd7182x_read16_himask(pwr, pwr->regs->vdcin,
+ BD7182x_MASK_VDCIN_U, &tmp);
+ if (ret)
+ return ret;
+
+ vot = tmp;
+ /* 5 milli volt steps */
+ val->intval = 5000 * vot;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int bd71828_battery_get_property(struct power_supply *psy,
+ enum power_supply_property psp,
+ union power_supply_propval *val)
+{
+ struct bd71828_power *pwr = dev_get_drvdata(psy->dev.parent);
+ int ret = 0;
+ int status, health, tmp, curr, curr_avg, chg_en;
+
+ if (psp == POWER_SUPPLY_PROP_STATUS || psp == POWER_SUPPLY_PROP_HEALTH
+ || psp == POWER_SUPPLY_PROP_CHARGE_TYPE)
+ ret = bd71828_charge_status(pwr, &status, &health);
+ else if (psp == POWER_SUPPLY_PROP_CURRENT_AVG ||
+ psp == POWER_SUPPLY_PROP_CURRENT_NOW)
+ ret = bd71828_get_current_ds_adc(pwr, &curr, &curr_avg);
+ if (ret)
+ return ret;
+
+ switch (psp) {
+ case POWER_SUPPLY_PROP_STATUS:
+ val->intval = status;
+ break;
+ case POWER_SUPPLY_PROP_HEALTH:
+ val->intval = health;
+ break;
+ case POWER_SUPPLY_PROP_CHARGE_TYPE:
+ if (status == POWER_SUPPLY_STATUS_CHARGING)
+ val->intval = POWER_SUPPLY_CHARGE_TYPE_FAST;
+ else
+ val->intval = POWER_SUPPLY_CHARGE_TYPE_NONE;
+ break;
+ case POWER_SUPPLY_PROP_PRESENT:
+ ret = get_bat_online(pwr, &tmp);
+ if (!ret)
+ val->intval = tmp;
+ break;
+ case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+ ret = bd71828_get_vbat(pwr, &tmp);
+ val->intval = tmp;
+ break;
+ case POWER_SUPPLY_PROP_TECHNOLOGY:
+ val->intval = POWER_SUPPLY_TECHNOLOGY_LION;
+ break;
+ case POWER_SUPPLY_PROP_CURRENT_AVG:
+ val->intval = curr_avg;
+ break;
+ case POWER_SUPPLY_PROP_CURRENT_NOW:
+ val->intval = curr;
+ break;
+ case POWER_SUPPLY_PROP_CURRENT_MAX:
+ val->intval = MAX_CURRENT_DEFAULT;
+ break;
+ case POWER_SUPPLY_PROP_TEMP:
+ ret = pwr->get_temp(pwr, &val->intval);
+ break;
+ case POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR:
+ ret = regmap_read(pwr->regmap, pwr->regs->chg_en, &chg_en);
+ if (ret)
+ return ret;
+
+ val->intval = (chg_en & BD7182x_MASK_CHG_EN) ?
+ POWER_SUPPLY_CHARGE_BEHAVIOUR_AUTO :
+ POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE;
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
+
+ return ret;
+}
+
+static int bd71828_battery_set_property(struct power_supply *psy,
+ enum power_supply_property psp,
+ const union power_supply_propval *val)
+{
+ struct bd71828_power *pwr = dev_get_drvdata(psy->dev.parent);
+ int ret = 0;
+
+ switch (psp) {
+ case POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR:
+ if (val->intval == POWER_SUPPLY_CHARGE_BEHAVIOUR_AUTO)
+ ret = regmap_update_bits(pwr->regmap, pwr->regs->chg_en,
+ BD7182x_MASK_CHG_EN,
+ BD7182x_MASK_CHG_EN);
+ else
+ ret = regmap_update_bits(pwr->regmap, pwr->regs->chg_en,
+ BD7182x_MASK_CHG_EN,
+ 0);
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return ret;
+}
+
+static int bd71828_battery_property_is_writeable(struct power_supply *psy,
+ enum power_supply_property psp)
+{
+ switch (psp) {
+ case POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR:
+ return true;
+ default:
+ return false;
+ }
+}
+
+
+/** @brief ac properties */
+static const enum power_supply_property bd71828_charger_props[] = {
+ POWER_SUPPLY_PROP_ONLINE,
+ POWER_SUPPLY_PROP_VOLTAGE_NOW,
+};
+
+static const enum power_supply_property bd71828_battery_props[] = {
+ POWER_SUPPLY_PROP_STATUS,
+ POWER_SUPPLY_PROP_HEALTH,
+ POWER_SUPPLY_PROP_VOLTAGE_NOW,
+ POWER_SUPPLY_PROP_HEALTH,
+ POWER_SUPPLY_PROP_PRESENT,
+ POWER_SUPPLY_PROP_TECHNOLOGY,
+ POWER_SUPPLY_PROP_CURRENT_AVG,
+ POWER_SUPPLY_PROP_CURRENT_NOW,
+ POWER_SUPPLY_PROP_CURRENT_MAX,
+ POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR,
+};
+
+/** @brief powers supplied by bd71828_ac */
+static char *bd71828_ac_supplied_to[] = {
+ BAT_NAME,
+};
+
+static const struct power_supply_desc bd71828_ac_desc = {
+ .name = AC_NAME,
+ .type = POWER_SUPPLY_TYPE_MAINS,
+ .properties = bd71828_charger_props,
+ .num_properties = ARRAY_SIZE(bd71828_charger_props),
+ .get_property = bd71828_charger_get_property,
+};
+
+static const struct power_supply_desc bd71828_bat_desc = {
+ .name = BAT_NAME,
+ .type = POWER_SUPPLY_TYPE_BATTERY,
+ .charge_behaviours = BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_AUTO) |
+ BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE),
+ .properties = bd71828_battery_props,
+ .num_properties = ARRAY_SIZE(bd71828_battery_props),
+ .get_property = bd71828_battery_get_property,
+ .set_property = bd71828_battery_set_property,
+ .property_is_writeable = bd71828_battery_property_is_writeable,
+};
+
+#define RSENS_CURR 10000000LLU
+
+#define BD_ISR_NAME(name) \
+bd7181x_##name##_isr
+
+#define BD_ISR_BAT(name, print, run_gauge) \
+static irqreturn_t BD_ISR_NAME(name)(int irq, void *data) \
+{ \
+ struct bd71828_power *pwr = (struct bd71828_power *)data; \
+ \
+ dev_dbg(pwr->dev, "%s\n", print); \
+ power_supply_changed(pwr->bat); \
+ \
+ return IRQ_HANDLED; \
+}
+
+#define BD_ISR_AC(name, print, run_gauge) \
+static irqreturn_t BD_ISR_NAME(name)(int irq, void *data) \
+{ \
+ struct bd71828_power *pwr = (struct bd71828_power *)data; \
+ \
+ power_supply_changed(pwr->ac); \
+ dev_dbg(pwr->dev, "%s\n", print); \
+ power_supply_changed(pwr->bat); \
+ \
+ return IRQ_HANDLED; \
+}
+
+#define BD_ISR_DUMMY(name, print) \
+static irqreturn_t BD_ISR_NAME(name)(int irq, void *data) \
+{ \
+ struct bd71828_power *pwr = (struct bd71828_power *)data; \
+ \
+ dev_dbg(pwr->dev, "%s\n", print); \
+ \
+ return IRQ_HANDLED; \
+}
+
+BD_ISR_BAT(chg_state_changed, "CHG state changed", true)
+/* DCIN voltage changes */
+BD_ISR_AC(dcin_removed, "DCIN removed", true)
+BD_ISR_AC(clps_out, "DCIN voltage back to normal", true)
+BD_ISR_AC(clps_in, "DCIN voltage collapsed", false)
+BD_ISR_AC(dcin_ovp_res, "DCIN voltage normal", true)
+BD_ISR_AC(dcin_ovp_det, "DCIN OVER VOLTAGE", true)
+
+BD_ISR_DUMMY(dcin_mon_det, "DCIN voltage below threshold")
+BD_ISR_DUMMY(dcin_mon_res, "DCIN voltage above threshold")
+
+BD_ISR_DUMMY(vsys_uv_res, "VSYS under-voltage cleared")
+BD_ISR_DUMMY(vsys_uv_det, "VSYS under-voltage")
+BD_ISR_DUMMY(vsys_low_res, "'VSYS low' cleared")
+BD_ISR_DUMMY(vsys_low_det, "VSYS low")
+BD_ISR_DUMMY(vsys_mon_res, "VSYS mon - resumed")
+BD_ISR_DUMMY(vsys_mon_det, "VSYS mon - detected")
+BD_ISR_BAT(chg_wdg_temp, "charger temperature watchdog triggered", true)
+BD_ISR_BAT(chg_wdg, "charging watchdog triggered", true)
+BD_ISR_BAT(bat_removed, "Battery removed", true)
+BD_ISR_BAT(bat_det, "Battery detected", true)
+/* TODO: Verify the meaning of these interrupts */
+BD_ISR_BAT(rechg_det, "Recharging", true)
+BD_ISR_BAT(rechg_res, "Recharge ending", true)
+BD_ISR_DUMMY(temp_transit, "Temperature transition")
+BD_ISR_BAT(therm_rmv, "bd71815-therm-rmv", false)
+BD_ISR_BAT(therm_det, "bd71815-therm-det", true)
+BD_ISR_BAT(bat_dead, "bd71815-bat-dead", false)
+BD_ISR_BAT(bat_short_res, "bd71815-bat-short-res", true)
+BD_ISR_BAT(bat_short, "bd71815-bat-short-det", false)
+BD_ISR_BAT(bat_low_res, "bd71815-bat-low-res", true)
+BD_ISR_BAT(bat_low, "bd71815-bat-low-det", true)
+BD_ISR_BAT(bat_ov_res, "bd71815-bat-over-res", true)
+/* What should we do here? */
+BD_ISR_BAT(bat_ov, "bd71815-bat-over-det", false)
+BD_ISR_BAT(bat_mon_res, "bd71815-bat-mon-res", true)
+BD_ISR_BAT(bat_mon, "bd71815-bat-mon-det", true)
+BD_ISR_BAT(bat_cc_mon, "bd71815-bat-cc-mon2", false)
+BD_ISR_BAT(bat_oc1_res, "bd71815-bat-oc1-res", true)
+BD_ISR_BAT(bat_oc1, "bd71815-bat-oc1-det", false)
+BD_ISR_BAT(bat_oc2_res, "bd71815-bat-oc2-res", true)
+BD_ISR_BAT(bat_oc2, "bd71815-bat-oc2-det", false)
+BD_ISR_BAT(bat_oc3_res, "bd71815-bat-oc3-res", true)
+BD_ISR_BAT(bat_oc3, "bd71815-bat-oc3-det", false)
+BD_ISR_BAT(temp_bat_low_res, "bd71815-temp-bat-low-res", true)
+BD_ISR_BAT(temp_bat_low, "bd71815-temp-bat-low-det", true)
+BD_ISR_BAT(temp_bat_hi_res, "bd71815-temp-bat-hi-res", true)
+BD_ISR_BAT(temp_bat_hi, "bd71815-temp-bat-hi-det", true)
+
+static irqreturn_t bd7182x_dcin_removed(int irq, void *data)
+{
+ struct bd71828_power *pwr = (struct bd71828_power *)data;
+
+ power_supply_changed(pwr->ac);
+ dev_dbg(pwr->dev, "DCIN removed\n");
+
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t bd718x7_chg_done(int irq, void *data)
+{
+ struct bd71828_power *pwr = (struct bd71828_power *)data;
+
+ power_supply_changed(pwr->bat);
+
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t bd7182x_dcin_detected(int irq, void *data)
+{
+ struct bd71828_power *pwr = (struct bd71828_power *)data;
+
+ dev_dbg(pwr->dev, "DCIN inserted\n");
+ power_supply_changed(pwr->ac);
+
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t bd71828_vbat_low_res(int irq, void *data)
+{
+ struct bd71828_power *pwr = (struct bd71828_power *)data;
+
+ dev_dbg(pwr->dev, "VBAT LOW Resumed\n");
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t bd71828_vbat_low_det(int irq, void *data)
+{
+ struct bd71828_power *pwr = (struct bd71828_power *)data;
+
+ dev_dbg(pwr->dev, "VBAT LOW Detected\n");
+
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t bd71828_temp_bat_hi_det(int irq, void *data)
+{
+ struct bd71828_power *pwr = (struct bd71828_power *)data;
+
+ dev_warn(pwr->dev, "Overtemp Detected\n");
+ power_supply_changed(pwr->bat);
+
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t bd71828_temp_bat_hi_res(int irq, void *data)
+{
+ struct bd71828_power *pwr = (struct bd71828_power *)data;
+
+ dev_dbg(pwr->dev, "Overtemp Resumed\n");
+ power_supply_changed(pwr->bat);
+
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t bd71828_temp_bat_low_det(int irq, void *data)
+{
+ struct bd71828_power *pwr = (struct bd71828_power *)data;
+
+ dev_dbg(pwr->dev, "Lowtemp Detected\n");
+ power_supply_changed(pwr->bat);
+
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t bd71828_temp_bat_low_res(int irq, void *data)
+{
+ struct bd71828_power *pwr = (struct bd71828_power *)data;
+
+ dev_dbg(pwr->dev, "Lowtemp Resumed\n");
+ power_supply_changed(pwr->bat);
+
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t bd71828_temp_vf_det(int irq, void *data)
+{
+ struct bd71828_power *pwr = (struct bd71828_power *)data;
+
+ dev_dbg(pwr->dev, "VF Detected\n");
+ power_supply_changed(pwr->bat);
+
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t bd71828_temp_vf_res(int irq, void *data)
+{
+ struct bd71828_power *pwr = (struct bd71828_power *)data;
+
+ dev_dbg(pwr->dev, "VF Resumed\n");
+ power_supply_changed(pwr->bat);
+
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t bd71828_temp_vf125_det(int irq, void *data)
+{
+ struct bd71828_power *pwr = (struct bd71828_power *)data;
+
+ dev_dbg(pwr->dev, "VF125 Detected\n");
+ power_supply_changed(pwr->bat);
+
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t bd71828_temp_vf125_res(int irq, void *data)
+{
+ struct bd71828_power *pwr = (struct bd71828_power *)data;
+
+ dev_dbg(pwr->dev, "VF125 Resumed\n");
+ power_supply_changed(pwr->bat);
+
+ return IRQ_HANDLED;
+}
+
+struct bd7182x_irq_res {
+ const char *name;
+ irq_handler_t handler;
+};
+
+#define BDIRQ(na, hn) { .name = (na), .handler = (hn) }
+
+static int bd7182x_get_irqs(struct platform_device *pdev,
+ struct bd71828_power *pwr)
+{
+ int i, irq, ret;
+ static const struct bd7182x_irq_res bd71815_irqs[] = {
+ BDIRQ("bd71815-dcin-rmv", BD_ISR_NAME(dcin_removed)),
+ BDIRQ("bd71815-dcin-clps-out", BD_ISR_NAME(clps_out)),
+ BDIRQ("bd71815-dcin-clps-in", BD_ISR_NAME(clps_in)),
+ BDIRQ("bd71815-dcin-ovp-res", BD_ISR_NAME(dcin_ovp_res)),
+ BDIRQ("bd71815-dcin-ovp-det", BD_ISR_NAME(dcin_ovp_det)),
+ BDIRQ("bd71815-dcin-mon-res", BD_ISR_NAME(dcin_mon_res)),
+ BDIRQ("bd71815-dcin-mon-det", BD_ISR_NAME(dcin_mon_det)),
+
+ BDIRQ("bd71815-vsys-uv-res", BD_ISR_NAME(vsys_uv_res)),
+ BDIRQ("bd71815-vsys-uv-det", BD_ISR_NAME(vsys_uv_det)),
+ BDIRQ("bd71815-vsys-low-res", BD_ISR_NAME(vsys_low_res)),
+ BDIRQ("bd71815-vsys-low-det", BD_ISR_NAME(vsys_low_det)),
+ BDIRQ("bd71815-vsys-mon-res", BD_ISR_NAME(vsys_mon_res)),
+ BDIRQ("bd71815-vsys-mon-det", BD_ISR_NAME(vsys_mon_det)),
+ BDIRQ("bd71815-chg-wdg-temp", BD_ISR_NAME(chg_wdg_temp)),
+ BDIRQ("bd71815-chg-wdg", BD_ISR_NAME(chg_wdg)),
+ BDIRQ("bd71815-rechg-det", BD_ISR_NAME(rechg_det)),
+ BDIRQ("bd71815-rechg-res", BD_ISR_NAME(rechg_res)),
+ BDIRQ("bd71815-ranged-temp-transit", BD_ISR_NAME(temp_transit)),
+ BDIRQ("bd71815-chg-state-change", BD_ISR_NAME(chg_state_changed)),
+ BDIRQ("bd71815-bat-temp-normal", bd71828_temp_bat_hi_res),
+ BDIRQ("bd71815-bat-temp-erange", bd71828_temp_bat_hi_det),
+ BDIRQ("bd71815-bat-rmv", BD_ISR_NAME(bat_removed)),
+ BDIRQ("bd71815-bat-det", BD_ISR_NAME(bat_det)),
+
+ /* Add ISRs for these */
+ BDIRQ("bd71815-therm-rmv", BD_ISR_NAME(therm_rmv)),
+ BDIRQ("bd71815-therm-det", BD_ISR_NAME(therm_det)),
+ BDIRQ("bd71815-bat-dead", BD_ISR_NAME(bat_dead)),
+ BDIRQ("bd71815-bat-short-res", BD_ISR_NAME(bat_short_res)),
+ BDIRQ("bd71815-bat-short-det", BD_ISR_NAME(bat_short)),
+ BDIRQ("bd71815-bat-low-res", BD_ISR_NAME(bat_low_res)),
+ BDIRQ("bd71815-bat-low-det", BD_ISR_NAME(bat_low)),
+ BDIRQ("bd71815-bat-over-res", BD_ISR_NAME(bat_ov_res)),
+ BDIRQ("bd71815-bat-over-det", BD_ISR_NAME(bat_ov)),
+ BDIRQ("bd71815-bat-mon-res", BD_ISR_NAME(bat_mon_res)),
+ BDIRQ("bd71815-bat-mon-det", BD_ISR_NAME(bat_mon)),
+ /* cc-mon 1 & 3 ? */
+ BDIRQ("bd71815-bat-cc-mon2", BD_ISR_NAME(bat_cc_mon)),
+ BDIRQ("bd71815-bat-oc1-res", BD_ISR_NAME(bat_oc1_res)),
+ BDIRQ("bd71815-bat-oc1-det", BD_ISR_NAME(bat_oc1)),
+ BDIRQ("bd71815-bat-oc2-res", BD_ISR_NAME(bat_oc2_res)),
+ BDIRQ("bd71815-bat-oc2-det", BD_ISR_NAME(bat_oc2)),
+ BDIRQ("bd71815-bat-oc3-res", BD_ISR_NAME(bat_oc3_res)),
+ BDIRQ("bd71815-bat-oc3-det", BD_ISR_NAME(bat_oc3)),
+ BDIRQ("bd71815-temp-bat-low-res", BD_ISR_NAME(temp_bat_low_res)),
+ BDIRQ("bd71815-temp-bat-low-det", BD_ISR_NAME(temp_bat_low)),
+ BDIRQ("bd71815-temp-bat-hi-res", BD_ISR_NAME(temp_bat_hi_res)),
+ BDIRQ("bd71815-temp-bat-hi-det", BD_ISR_NAME(temp_bat_hi)),
+ /*
+ * TODO: add rest of the IRQs and re-check the handling.
+ * Check the bd71815-bat-cc-mon1, bd71815-bat-cc-mon3,
+ * bd71815-bat-low-res, bd71815-bat-low-det,
+ * bd71815-bat-hi-res, bd71815-bat-hi-det.
+ */
+ };
+ static const struct bd7182x_irq_res bd71828_irqs[] = {
+ BDIRQ("bd71828-chg-done", bd718x7_chg_done),
+ BDIRQ("bd71828-pwr-dcin-in", bd7182x_dcin_detected),
+ BDIRQ("bd71828-pwr-dcin-out", bd7182x_dcin_removed),
+ BDIRQ("bd71828-vbat-normal", bd71828_vbat_low_res),
+ BDIRQ("bd71828-vbat-low", bd71828_vbat_low_det),
+ BDIRQ("bd71828-btemp-hi", bd71828_temp_bat_hi_det),
+ BDIRQ("bd71828-btemp-cool", bd71828_temp_bat_hi_res),
+ BDIRQ("bd71828-btemp-lo", bd71828_temp_bat_low_det),
+ BDIRQ("bd71828-btemp-warm", bd71828_temp_bat_low_res),
+ BDIRQ("bd71828-temp-hi", bd71828_temp_vf_det),
+ BDIRQ("bd71828-temp-norm", bd71828_temp_vf_res),
+ BDIRQ("bd71828-temp-125-over", bd71828_temp_vf125_det),
+ BDIRQ("bd71828-temp-125-under", bd71828_temp_vf125_res),
+ };
+ int num_irqs;
+ const struct bd7182x_irq_res *irqs;
+
+
+ switch (pwr->chip_type) {
+ case ROHM_CHIP_TYPE_BD71828:
+ irqs = &bd71828_irqs[0];
+ num_irqs = ARRAY_SIZE(bd71828_irqs);
+ break;
+ case ROHM_CHIP_TYPE_BD71815:
+ irqs = &bd71815_irqs[0];
+ num_irqs = ARRAY_SIZE(bd71815_irqs);
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ for (i = 0; i < num_irqs; i++) {
+ irq = platform_get_irq_byname(pdev, irqs[i].name);
+
+ ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
+ irqs[i].handler, 0,
+ irqs[i].name, pwr);
+ if (ret)
+ break;
+ }
+
+ return ret;
+}
+
+#define RSENS_DEFAULT_30MOHM 30000 /* 30 mOhm in uOhms*/
+
+static int bd7182x_get_rsens(struct bd71828_power *pwr)
+{
+ u64 tmp = RSENS_CURR;
+ int rsens_ohm = RSENS_DEFAULT_30MOHM;
+ struct fwnode_handle *node = NULL;
+
+ if (pwr->dev->parent)
+ node = dev_fwnode(pwr->dev->parent);
+
+ if (node) {
+ int ret;
+ uint32_t rs;
+
+ ret = fwnode_property_read_u32(node,
+ "rohm,charger-sense-resistor-micro-ohms",
+ &rs);
+ if (ret) {
+ if (ret == -EINVAL) {
+ rs = RSENS_DEFAULT_30MOHM;
+ } else {
+ dev_err(pwr->dev, "Bad RSENS dt property\n");
+ return ret;
+ }
+ }
+ if (!rs) {
+ dev_err(pwr->dev, "Bad RSENS value\n");
+ return -EINVAL;
+ }
+
+ rsens_ohm = (int)rs;
+ }
+
+ /* Reg val to uA */
+ do_div(tmp, rsens_ohm);
+
+ pwr->curr_factor = tmp;
+ pwr->rsens = rsens_ohm;
+ dev_dbg(pwr->dev, "Setting rsens to %u micro ohm\n", pwr->rsens);
+ dev_dbg(pwr->dev, "Setting curr-factor to %u\n", pwr->curr_factor);
+ return 0;
+}
+
+static int bd71828_power_probe(struct platform_device *pdev)
+{
+ struct bd71828_power *pwr;
+ struct power_supply_config ac_cfg = {};
+ struct power_supply_config bat_cfg = {};
+ int ret;
+ struct regmap *regmap;
+
+ regmap = dev_get_regmap(pdev->dev.parent, NULL);
+ if (!regmap) {
+ dev_err(&pdev->dev, "No parent regmap\n");
+ return -EINVAL;
+ }
+
+ pwr = devm_kzalloc(&pdev->dev, sizeof(*pwr), GFP_KERNEL);
+ if (!pwr)
+ return -ENOMEM;
+
+ pwr->regmap = regmap;
+ pwr->dev = &pdev->dev;
+ pwr->chip_type = platform_get_device_id(pdev)->driver_data;
+
+ switch (pwr->chip_type) {
+ case ROHM_CHIP_TYPE_BD71828:
+ pwr->bat_inserted = bd71828_bat_inserted;
+ pwr->get_temp = bd71828_get_temp;
+ pwr->regs = &pwr_regs_bd71828;
+ dev_dbg(pwr->dev, "Found ROHM BD71828\n");
+ break;
+ case ROHM_CHIP_TYPE_BD71815:
+ pwr->bat_inserted = bd71815_bat_inserted;
+ pwr->get_temp = bd71815_get_temp;
+ pwr->regs = &pwr_regs_bd71815;
+ dev_dbg(pwr->dev, "Found ROHM BD71815\n");
+ break;
+ default:
+ dev_err(pwr->dev, "Unknown PMIC\n");
+ return -EINVAL;
+ }
+
+ ret = bd7182x_get_rsens(pwr);
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret, "sense resistor missing\n");
+
+ dev_set_drvdata(&pdev->dev, pwr);
+ bd71828_init_hardware(pwr);
+
+ bat_cfg.drv_data = pwr;
+ bat_cfg.fwnode = dev_fwnode(&pdev->dev);
+
+ ac_cfg.supplied_to = bd71828_ac_supplied_to;
+ ac_cfg.num_supplicants = ARRAY_SIZE(bd71828_ac_supplied_to);
+ ac_cfg.drv_data = pwr;
+
+ pwr->ac = devm_power_supply_register(&pdev->dev, &bd71828_ac_desc,
+ &ac_cfg);
+ if (IS_ERR(pwr->ac)) {
+ return dev_err_probe(&pdev->dev, PTR_ERR(pwr->ac),
+ "failed to register ac\n");
+ }
+
+ pwr->bat = devm_power_supply_register(&pdev->dev, &bd71828_bat_desc,
+ &bat_cfg);
+ if (IS_ERR(pwr->bat)) {
+ return dev_err_probe(&pdev->dev, PTR_ERR(pwr->bat),
+ "failed to register bat\n");
+ }
+
+ ret = bd7182x_get_irqs(pdev, pwr);
+ if (ret) {
+ return dev_err_probe(&pdev->dev, ret, "failed to request IRQs");
+ };
+
+ /* Configure wakeup capable */
+ device_set_wakeup_capable(pwr->dev, 1);
+ device_set_wakeup_enable(pwr->dev, 1);
+
+ return 0;
+}
+
+static const struct platform_device_id bd71828_charger_id[] = {
+ { "bd71815-power", ROHM_CHIP_TYPE_BD71815 },
+ { "bd71828-power", ROHM_CHIP_TYPE_BD71828 },
+ { },
+};
+MODULE_DEVICE_TABLE(platform, bd71828_charger_id);
+
+static struct platform_driver bd71828_power_driver = {
+ .driver = {
+ .name = "bd718xx-power",
+ },
+ .probe = bd71828_power_probe,
+ .id_table = bd71828_charger_id,
+};
+
+module_platform_driver(bd71828_power_driver);
+MODULE_ALIAS("platform:bd718xx-power");
+
+MODULE_AUTHOR("Cong Pham <cpham2403@gmail.com>");
+MODULE_DESCRIPTION("ROHM BD718(15/17/28/78) PMIC Battery Charger driver");
+MODULE_LICENSE("GPL");
--
2.39.5Hi Andreas,
kernel test robot noticed the following build warnings:
url: https://github.com/intel-lab-lkp/linux/commits/Andreas-Kemnade/mfd-bd71828-bd71815-prepare-for-power-supply-support/20250817-032146
base: 8f5ae30d69d7543eee0d70083daf4de8fe15d585
patch link: https://lore.kernel.org/r/20250816-bd71828-charger-v1-2-71b11bde5c73%40kemnade.info
patch subject: [PATCH 2/2] power: supply: Add bd718(15/28/78) charger driver
config: loongarch-randconfig-r073-20250818 (https://download.01.org/0day-ci/archive/20250819/202508191303.UzPStkjj-lkp@intel.com/config)
compiler: loongarch64-linux-gcc (GCC) 15.1.0
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202508191303.UzPStkjj-lkp@intel.com/
smatch warnings:
drivers/power/supply/bd71828-power.c:298 bd71828_get_temp() error: uninitialized symbol 't'.
drivers/power/supply/bd71828-power.c:559 bd71828_battery_get_property() error: uninitialized symbol 'tmp'.
vim +/t +298 drivers/power/supply/bd71828-power.c
c57d31029550a0 Andreas Kemnade 2025-08-16 286 static int bd71828_get_temp(struct bd71828_power *pwr, int *temp)
c57d31029550a0 Andreas Kemnade 2025-08-16 287 {
c57d31029550a0 Andreas Kemnade 2025-08-16 288 uint16_t t;
c57d31029550a0 Andreas Kemnade 2025-08-16 289 int ret;
c57d31029550a0 Andreas Kemnade 2025-08-16 290 int tmp = 200 * 10000;
c57d31029550a0 Andreas Kemnade 2025-08-16 291
c57d31029550a0 Andreas Kemnade 2025-08-16 292 ret = bd7182x_read16_himask(pwr, pwr->regs->btemp_vth,
c57d31029550a0 Andreas Kemnade 2025-08-16 293 BD71828_MASK_VM_BTMP_U, &t);
c57d31029550a0 Andreas Kemnade 2025-08-16 294 if (ret || t > 3200)
c57d31029550a0 Andreas Kemnade 2025-08-16 295 dev_err(pwr->dev,
c57d31029550a0 Andreas Kemnade 2025-08-16 296 "Failed to read system min average voltage\n");
We should return the error here.
c57d31029550a0 Andreas Kemnade 2025-08-16 297
c57d31029550a0 Andreas Kemnade 2025-08-16 @298 tmp -= 625ULL * (unsigned int)t;
If bd7182x_read16_himask() fails then t is uninitialized or invalid.
c57d31029550a0 Andreas Kemnade 2025-08-16 299 *temp = tmp / 1000;
c57d31029550a0 Andreas Kemnade 2025-08-16 300
c57d31029550a0 Andreas Kemnade 2025-08-16 301 return ret;
c57d31029550a0 Andreas Kemnade 2025-08-16 302 }
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On 16/08/2025 22:19, Andreas Kemnade wrote:
> Add charger driver for ROHM BD718(15/28/78) PMIC charger block.
> It is a stripped down version of the driver here:
> https://lore.kernel.org/lkml/dbd97c1b0d715aa35a8b4d79741e433d97c562aa.1637061794.git.matti.vaittinen@fi.rohmeurope.com/
>
> For the ease of review and to do a step-by-step approach remove all the
> coloumb counter related stuff and do not sneak in BD71827 support.
>
> Changes besides that:
> Replace the custom property by a standard one and do not use megaohms
> for the current sense resistor.
>
> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> ---
> drivers/power/supply/Kconfig | 11 +
> drivers/power/supply/Makefile | 1 +
> drivers/power/supply/bd71828-power.c | 1144 ++++++++++++++++++++++++++++++++++
> 3 files changed, 1156 insertions(+)
>
> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
> index 79ddb006e2dad6bf96b71ed570a37c006b5f9433..f3429f0aecf5a17fbaa52ee76899657181429a48 100644
> --- a/drivers/power/supply/Kconfig
> +++ b/drivers/power/supply/Kconfig
> @@ -974,6 +974,17 @@ config CHARGER_UCS1002
> Say Y to enable support for Microchip UCS1002 Programmable
> USB Port Power Controller with Charger Emulation.
>
> +config CHARGER_BD71828
> + tristate "Power-supply driver for ROHM BD71828 and BD71815 PMIC"
> + depends on MFD_ROHM_BD71828
> + select POWER_SIMPLE_GAUGE
> + help
> + Say Y here to enable support for charger, battery and fuel gauge
Maybe drop the fuel-gauge?
> + in ROHM BD71815, BD71817, ROHM BD71828 power management
> + ICs. This driver gets various bits of information about battery
> + and charger states and also implements fuel gauge based on
> + coulomb counters on PMIC.
> +
> config CHARGER_BD99954
> tristate "ROHM bd99954 charger driver"
> depends on I2C
> diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
> index f943c9150b326d41ff241f82610f70298635eb08..c6520a11f021c872f01250ae54eb4c63018cd428 100644
> --- a/drivers/power/supply/Makefile
> +++ b/drivers/power/supply/Makefile
> @@ -113,6 +113,7 @@ 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_BD71828) += bd71828-power.o
> obj-$(CONFIG_CHARGER_BD99954) += bd99954-charger.o
> obj-$(CONFIG_CHARGER_WILCO) += wilco-charger.o
> obj-$(CONFIG_RN5T618_POWER) += rn5t618_power.o
> diff --git a/drivers/power/supply/bd71828-power.c b/drivers/power/supply/bd71828-power.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..f2686c7eadf0515856d60123f57bca59b33bbd10
> --- /dev/null
> +++ b/drivers/power/supply/bd71828-power.c
> @@ -0,0 +1,1144 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * bd71828-power.c
> + * @file ROHM BD71815, BD71828 and BD71878 Charger driver
> + *
> + * Copyright 2021.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/mfd/rohm-bd71815.h>
> +#include <linux/mfd/rohm-bd71828.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/power_supply.h>
> +#include <linux/slab.h>
> +#include <linux/uaccess.h>
> +
> +/* common defines */
> +#define BD7182x_MASK_VBAT_U 0x1f
> +#define BD7182x_MASK_VDCIN_U 0x0f
> +#define BD7182x_MASK_IBAT_U 0x3f
> +#define BD7182x_MASK_CURDIR_DISCHG 0x80
> +#define BD7182x_MASK_CC_CCNTD_HI 0x0FFF
> +#define BD7182x_MASK_CC_CCNTD 0x0FFFFFFF
> +#define BD7182x_MASK_CHG_STATE 0x7f
> +#define BD7182x_MASK_CC_FULL_CLR 0x10
> +#define BD7182x_MASK_BAT_TEMP 0x07
> +#define BD7182x_MASK_DCIN_DET BIT(0)
> +#define BD7182x_MASK_CONF_PON BIT(0)
> +#define BD71815_MASK_CONF_XSTB BIT(1)
> +#define BD7182x_MASK_BAT_STAT 0x3f
> +#define BD7182x_MASK_DCIN_STAT 0x07
> +
> +#define BD7182x_MASK_CCNTRST 0x80
> +#define BD7182x_MASK_CCNTENB 0x40
> +#define BD7182x_MASK_CCCALIB 0x20
I suppose unused defines could get cleared. (At least the CC related
ones)...
> +#define BD7182x_MASK_WDT_AUTO 0x40
> +#define BD7182x_MASK_VBAT_ALM_LIMIT_U 0x01
> +#define BD7182x_MASK_CHG_EN 0x01
> +
> +#define BD7182x_DCIN_COLLAPSE_DEFAULT 0x36
> +
> +/* Measured min and max value clear bits */
> +#define BD718XX_MASK_VSYS_MIN_AVG_CLR 0x10
> +
> +#define JITTER_DEFAULT 3000
...Also the loop jitter
> +#define MAX_CURRENT_DEFAULT 890000 /* uA */
> +#define AC_NAME "bd71828_ac"
> +#define BAT_NAME "bd71828_bat"
> +
> +/*
> + * VBAT Low voltage detection Threshold
> + * 0x00D4*16mV = 212*0.016 = 3.392v
> + */
> +#define VBAT_LOW_TH 0x00D4
> +
> +#define THR_RELAX_CURRENT_DEFAULT 5 /* mA */
> +#define THR_RELAX_TIME_DEFAULT (60 * 60) /* sec. */
> +
> +#define DGRD_CYC_CAP_DEFAULT 88 /* 1 micro Ah */
> +
> +#define DGRD_TEMP_H_DEFAULT 450 /* 0.1 degrees C */
> +#define DGRD_TEMP_M_DEFAULT 250 /* 0.1 degrees C */
> +#define DGRD_TEMP_L_DEFAULT 50 /* 0.1 degrees C */
> +#define DGRD_TEMP_VL_DEFAULT 0 /* 0.1 degrees C */
> +
> +#define SOC_EST_MAX_NUM_DEFAULT 5
> +#define PWRCTRL_NORMAL 0x22
> +#define PWRCTRL_RESET 0x23
> +
> +/*
> + * Originally we relied upon a fixed size table of OCV and VDR params.
> + * However the exising linux power-supply batinfo interface for getting the OCV
> + * values from DT does not have fixed amount of OCV values. Thus we use fixed
> + * parameter amount only for values provided as module params - and use this
> + * only as maximum number of parameters when values come from DT.
> + */
> +#define NUM_BAT_PARAMS 23
> +#define MAX_NUM_VDR_VALUES NUM_BAT_PARAMS
> +
> +struct pwr_regs {
> + int used_init_regs;
> + u8 vbat_init;
> + u8 vbat_init2;
> + u8 vbat_init3;
> + u8 vbat_avg;
> + u8 ibat;
> + u8 ibat_avg;
> + u8 meas_clear;
> + u8 vsys_min_avg;
> + u8 btemp_vth;
> + u8 chg_state;
> + u8 coulomb3;
> + u8 coulomb2;
> + u8 coulomb1;
> + u8 coulomb0;
> + u8 coulomb_ctrl;
> + u8 vbat_rex_avg;
> + u8 coulomb_full3;
> + u8 cc_full_clr;
> + u8 coulomb_chg3;
The coulomb counter and vbat_rex related entries could be dropped as
they're now unused. Actually, dropping all unused members would simplify
this quite a bit.
> + u8 bat_temp;
> + u8 dcin_stat;
> + u8 dcin_collapse_limit;
> + u8 chg_set1;
> + u8 chg_en;
> + u8 vbat_alm_limit_u;
> + u8 batcap_mon_limit_u;
> + u8 conf;
> + u8 vdcin;
> +};
> +
...
> +
> +static int bd71828_battery_set_property(struct power_supply *psy,
> + enum power_supply_property psp,
> + const union power_supply_propval *val)
> +{
> + struct bd71828_power *pwr = dev_get_drvdata(psy->dev.parent);
> + int ret = 0;
> +
> + switch (psp) {
> + case POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR:
> + if (val->intval == POWER_SUPPLY_CHARGE_BEHAVIOUR_AUTO)
> + ret = regmap_update_bits(pwr->regmap, pwr->regs->chg_en,
> + BD7182x_MASK_CHG_EN,
> + BD7182x_MASK_CHG_EN);
> + else
> + ret = regmap_update_bits(pwr->regmap, pwr->regs->chg_en,
> + BD7182x_MASK_CHG_EN,
> + 0);
Nice! I didn't know about the POWER_SUPPLY_CHARGE_BEHAVIOUR_AUTO.
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return ret;
> +}
> +
...
> +
> +static int bd71828_power_probe(struct platform_device *pdev)
> +{
> + struct bd71828_power *pwr;
> + struct power_supply_config ac_cfg = {};
> + struct power_supply_config bat_cfg = {};
> + int ret;
> + struct regmap *regmap;
> +
> + regmap = dev_get_regmap(pdev->dev.parent, NULL);
> + if (!regmap) {
> + dev_err(&pdev->dev, "No parent regmap\n");
> + return -EINVAL;
> + }
> +
> + pwr = devm_kzalloc(&pdev->dev, sizeof(*pwr), GFP_KERNEL);
> + if (!pwr)
> + return -ENOMEM;
> +
> + pwr->regmap = regmap;
> + pwr->dev = &pdev->dev;
> + pwr->chip_type = platform_get_device_id(pdev)->driver_data;
> +
> + switch (pwr->chip_type) {
> + case ROHM_CHIP_TYPE_BD71828:
> + pwr->bat_inserted = bd71828_bat_inserted;
> + pwr->get_temp = bd71828_get_temp;
> + pwr->regs = &pwr_regs_bd71828;
> + dev_dbg(pwr->dev, "Found ROHM BD71828\n");
> + break;
> + case ROHM_CHIP_TYPE_BD71815:
> + pwr->bat_inserted = bd71815_bat_inserted;
> + pwr->get_temp = bd71815_get_temp;
> + pwr->regs = &pwr_regs_bd71815;
> + dev_dbg(pwr->dev, "Found ROHM BD71815\n");
> + break;
> + default:
> + dev_err(pwr->dev, "Unknown PMIC\n");
> + return -EINVAL;
> + }
> +
> + ret = bd7182x_get_rsens(pwr);
> + if (ret)
> + return dev_err_probe(&pdev->dev, ret, "sense resistor missing\n");
> +
> + dev_set_drvdata(&pdev->dev, pwr);
> + bd71828_init_hardware(pwr);
> +
> + bat_cfg.drv_data = pwr;
> + bat_cfg.fwnode = dev_fwnode(&pdev->dev);
> +
> + ac_cfg.supplied_to = bd71828_ac_supplied_to;
> + ac_cfg.num_supplicants = ARRAY_SIZE(bd71828_ac_supplied_to);
> + ac_cfg.drv_data = pwr;
> +
> + pwr->ac = devm_power_supply_register(&pdev->dev, &bd71828_ac_desc,
> + &ac_cfg);
> + if (IS_ERR(pwr->ac)) {
> + return dev_err_probe(&pdev->dev, PTR_ERR(pwr->ac),
> + "failed to register ac\n");
> + }
> +
> + pwr->bat = devm_power_supply_register(&pdev->dev, &bd71828_bat_desc,
> + &bat_cfg);
> + if (IS_ERR(pwr->bat)) {
> + return dev_err_probe(&pdev->dev, PTR_ERR(pwr->bat),
> + "failed to register bat\n");
> + }
> +
> + ret = bd7182x_get_irqs(pdev, pwr);
> + if (ret) {
> + return dev_err_probe(&pdev->dev, ret, "failed to request IRQs");
> + };
Can drop the {}
> +
> + /* Configure wakeup capable */
> + device_set_wakeup_capable(pwr->dev, 1);
> + device_set_wakeup_enable(pwr->dev, 1);
> +
> + return 0;
> +}
> +
> +static const struct platform_device_id bd71828_charger_id[] = {
> + { "bd71815-power", ROHM_CHIP_TYPE_BD71815 },
> + { "bd71828-power", ROHM_CHIP_TYPE_BD71828 },
> + { },
> +};
> +MODULE_DEVICE_TABLE(platform, bd71828_charger_id);
> +
> +static struct platform_driver bd71828_power_driver = {
> + .driver = {
> + .name = "bd718xx-power",
> + },
> + .probe = bd71828_power_probe,
> + .id_table = bd71828_charger_id,
> +};
> +
> +module_platform_driver(bd71828_power_driver);
> +MODULE_ALIAS("platform:bd718xx-power");
> +
> +MODULE_AUTHOR("Cong Pham <cpham2403@gmail.com>");
> +MODULE_DESCRIPTION("ROHM BD718(15/17/28/78) PMIC Battery Charger driver");
> +MODULE_LICENSE("GPL");
>
Am Mon, 18 Aug 2025 13:33:05 +0300
schrieb Matti Vaittinen <mazziesaccount@gmail.com>:
[...]
> > +#define BD7182x_MASK_CCNTRST 0x80
> > +#define BD7182x_MASK_CCNTENB 0x40
> > +#define BD7182x_MASK_CCCALIB 0x20
>
> I suppose unused defines could get cleared. (At least the CC related
> ones)...
>
Hmm, I don't think having unused register defines is a problem at least
that is not an uncommon thing.
> > +#define BD7182x_MASK_WDT_AUTO 0x40
> > +#define BD7182x_MASK_VBAT_ALM_LIMIT_U 0x01
> > +#define BD7182x_MASK_CHG_EN 0x01
> > +
> > +#define BD7182x_DCIN_COLLAPSE_DEFAULT 0x36
> > +
> > +/* Measured min and max value clear bits */
> > +#define BD718XX_MASK_VSYS_MIN_AVG_CLR 0x10
> > +
> > +#define JITTER_DEFAULT 3000
>
> ...Also the loop jitter
>
yes, there I agree.
> > +#define MAX_CURRENT_DEFAULT 890000 /* uA */
> > +#define AC_NAME "bd71828_ac"
> > +#define BAT_NAME "bd71828_bat"
> > +
> > +/*
> > + * VBAT Low voltage detection Threshold
> > + * 0x00D4*16mV = 212*0.016 = 3.392v
> > + */
> > +#define VBAT_LOW_TH 0x00D4
> > +
> > +#define THR_RELAX_CURRENT_DEFAULT 5 /* mA */
> > +#define THR_RELAX_TIME_DEFAULT (60 * 60) /* sec. */
> > +
> > +#define DGRD_CYC_CAP_DEFAULT 88 /* 1 micro Ah */
> > +
> > +#define DGRD_TEMP_H_DEFAULT 450 /* 0.1 degrees C */
> > +#define DGRD_TEMP_M_DEFAULT 250 /* 0.1 degrees C */
> > +#define DGRD_TEMP_L_DEFAULT 50 /* 0.1 degrees C */
> > +#define DGRD_TEMP_VL_DEFAULT 0 /* 0.1 degrees C */
> > +
> > +#define SOC_EST_MAX_NUM_DEFAULT 5
> > +#define PWRCTRL_NORMAL 0x22
> > +#define PWRCTRL_RESET 0x23
> > +
> > +/*
> > + * Originally we relied upon a fixed size table of OCV and VDR params.
> > + * However the exising linux power-supply batinfo interface for getting the OCV
> > + * values from DT does not have fixed amount of OCV values. Thus we use fixed
> > + * parameter amount only for values provided as module params - and use this
> > + * only as maximum number of parameters when values come from DT.
> > + */
> > +#define NUM_BAT_PARAMS 23
> > +#define MAX_NUM_VDR_VALUES NUM_BAT_PARAMS
> > +
> > +struct pwr_regs {
> > + int used_init_regs;
> > + u8 vbat_init;
> > + u8 vbat_init2;
> > + u8 vbat_init3;
> > + u8 vbat_avg;
> > + u8 ibat;
> > + u8 ibat_avg;
> > + u8 meas_clear;
> > + u8 vsys_min_avg;
> > + u8 btemp_vth;
> > + u8 chg_state;
> > + u8 coulomb3;
> > + u8 coulomb2;
> > + u8 coulomb1;
> > + u8 coulomb0;
> > + u8 coulomb_ctrl;
> > + u8 vbat_rex_avg;
> > + u8 coulomb_full3;
> > + u8 cc_full_clr;
> > + u8 coulomb_chg3;
>
> The coulomb counter and vbat_rex related entries could be dropped as
> they're now unused. Actually, dropping all unused members would simplify
> this quite a bit.
>
hmm, removing it just to add it back tomorrow again... that are not so
much bytes. I would not remove them.
Regards,
Andreas
Hi Andreas,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 8f5ae30d69d7543eee0d70083daf4de8fe15d585]
url: https://github.com/intel-lab-lkp/linux/commits/Andreas-Kemnade/mfd-bd71828-bd71815-prepare-for-power-supply-support/20250817-032146
base: 8f5ae30d69d7543eee0d70083daf4de8fe15d585
patch link: https://lore.kernel.org/r/20250816-bd71828-charger-v1-2-71b11bde5c73%40kemnade.info
patch subject: [PATCH 2/2] power: supply: Add bd718(15/28/78) charger driver
config: m68k-randconfig-r111-20250818 (https://download.01.org/0day-ci/archive/20250818/202508180501.24kGNF9b-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 12.5.0
reproduce: (https://download.01.org/0day-ci/archive/20250818/202508180501.24kGNF9b-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202508180501.24kGNF9b-lkp@intel.com/
sparse warnings: (new ones prefixed by >>)
>> drivers/power/supply/bd71828-power.c:257:26: sparse: sparse: incorrect type in assignment (different base types) @@ expected restricted __be16 [addressable] [usertype] tmp_curr @@ got unsigned short [usertype] @@
drivers/power/supply/bd71828-power.c:257:26: sparse: expected restricted __be16 [addressable] [usertype] tmp_curr
drivers/power/supply/bd71828-power.c:257:26: sparse: got unsigned short [usertype]
>> drivers/power/supply/bd71828-power.c:259:36: sparse: sparse: cast from restricted __be16
vim +257 drivers/power/supply/bd71828-power.c
237
238 static int bd71828_get_current_ds_adc(struct bd71828_power *pwr, int *curr, int *curr_avg)
239 {
240 __be16 tmp_curr;
241 char *tmp = (char *)&tmp_curr;
242 int dir = 1;
243 int regs[] = { pwr->regs->ibat, pwr->regs->ibat_avg };
244 int *vals[] = { curr, curr_avg };
245 int ret, i;
246
247 for (dir = 1, i = 0; i < ARRAY_SIZE(regs); i++) {
248 ret = regmap_bulk_read(pwr->regmap, regs[i], &tmp_curr,
249 sizeof(tmp_curr));
250 if (ret)
251 break;
252
253 if (*tmp & BD7182x_MASK_CURDIR_DISCHG)
254 dir = -1;
255
256 *tmp &= BD7182x_MASK_IBAT_U;
> 257 tmp_curr = be16_to_cpu(tmp_curr);
258
> 259 *vals[i] = dir * ((int)tmp_curr) * pwr->curr_factor;
260 }
261
262 return ret;
263 }
264
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On 16/08/2025 21:19, Andreas Kemnade wrote:
> Add charger driver for ROHM BD718(15/28/78) PMIC charger block.
> It is a stripped down version of the driver here:
> https://lore.kernel.org/lkml/dbd97c1b0d715aa35a8b4d79741e433d97c562aa.1637061794.git.matti.vaittinen@fi.rohmeurope.com/
Why are you duplicating the driver? Why original cannot be used?
...
> +
> +#define RSENS_DEFAULT_30MOHM 30000 /* 30 mOhm in uOhms*/
> +
> +static int bd7182x_get_rsens(struct bd71828_power *pwr)
> +{
> + u64 tmp = RSENS_CURR;
> + int rsens_ohm = RSENS_DEFAULT_30MOHM;
> + struct fwnode_handle *node = NULL;
> +
> + if (pwr->dev->parent)
> + node = dev_fwnode(pwr->dev->parent);
> +
> + if (node) {
> + int ret;
> + uint32_t rs;
> +
> + ret = fwnode_property_read_u32(node,
> + "rohm,charger-sense-resistor-micro-ohms",
Hm? Are you writing ACPI or DT driver?
> + &rs);
> + if (ret) {
> + if (ret == -EINVAL) {
> + rs = RSENS_DEFAULT_30MOHM;
> + } else {
> + dev_err(pwr->dev, "Bad RSENS dt property\n");
> + return ret;
> + }
> + }
> + if (!rs) {
> + dev_err(pwr->dev, "Bad RSENS value\n");
> + return -EINVAL;
> + }
> +
> + rsens_ohm = (int)rs;
> + }
> +
> + /* Reg val to uA */
> + do_div(tmp, rsens_ohm);
> +
> + pwr->curr_factor = tmp;
> + pwr->rsens = rsens_ohm;
> + dev_dbg(pwr->dev, "Setting rsens to %u micro ohm\n", pwr->rsens);
> + dev_dbg(pwr->dev, "Setting curr-factor to %u\n", pwr->curr_factor);
> + return 0;
> +}
> +
> +static int bd71828_power_probe(struct platform_device *pdev)
> +{
> + struct bd71828_power *pwr;
> + struct power_supply_config ac_cfg = {};
> + struct power_supply_config bat_cfg = {};
> + int ret;
> + struct regmap *regmap;
> +
> + regmap = dev_get_regmap(pdev->dev.parent, NULL);
> + if (!regmap) {
> + dev_err(&pdev->dev, "No parent regmap\n");
> + return -EINVAL;
> + }
> +
> + pwr = devm_kzalloc(&pdev->dev, sizeof(*pwr), GFP_KERNEL);
> + if (!pwr)
> + return -ENOMEM;
> +
> + pwr->regmap = regmap;
> + pwr->dev = &pdev->dev;
> + pwr->chip_type = platform_get_device_id(pdev)->driver_data;
> +
> + switch (pwr->chip_type) {
> + case ROHM_CHIP_TYPE_BD71828:
> + pwr->bat_inserted = bd71828_bat_inserted;
> + pwr->get_temp = bd71828_get_temp;
> + pwr->regs = &pwr_regs_bd71828;
> + dev_dbg(pwr->dev, "Found ROHM BD71828\n");
This is pretty useless debug. You do not use here autodetection, so
there is no "found" case. It's straightforward bind.
> + break;
> + case ROHM_CHIP_TYPE_BD71815:
> + pwr->bat_inserted = bd71815_bat_inserted;
> + pwr->get_temp = bd71815_get_temp;
> + pwr->regs = &pwr_regs_bd71815;
> + dev_dbg(pwr->dev, "Found ROHM BD71815\n");
Same here, drop.
> + break;
> + default:
> + dev_err(pwr->dev, "Unknown PMIC\n");
> + return -EINVAL;
> + }
> +
> + ret = bd7182x_get_rsens(pwr);
> + if (ret)
> + return dev_err_probe(&pdev->dev, ret, "sense resistor missing\n");
> +
> + dev_set_drvdata(&pdev->dev, pwr);
> + bd71828_init_hardware(pwr);
> +
> + bat_cfg.drv_data = pwr;
> + bat_cfg.fwnode = dev_fwnode(&pdev->dev);
> +
> + ac_cfg.supplied_to = bd71828_ac_supplied_to;
> + ac_cfg.num_supplicants = ARRAY_SIZE(bd71828_ac_supplied_to);
> + ac_cfg.drv_data = pwr;
> +
> + pwr->ac = devm_power_supply_register(&pdev->dev, &bd71828_ac_desc,
> + &ac_cfg);
> + if (IS_ERR(pwr->ac)) {
> + return dev_err_probe(&pdev->dev, PTR_ERR(pwr->ac),
> + "failed to register ac\n");
> + }
> +
> + pwr->bat = devm_power_supply_register(&pdev->dev, &bd71828_bat_desc,
> + &bat_cfg);
> + if (IS_ERR(pwr->bat)) {
> + return dev_err_probe(&pdev->dev, PTR_ERR(pwr->bat),
> + "failed to register bat\n");
> + }
> +
> + ret = bd7182x_get_irqs(pdev, pwr);
> + if (ret) {
Please run scripts/checkpatch.pl on the patches and fix reported
warnings. After that, run also 'scripts/checkpatch.pl --strict' on the
patches and (probably) fix more warnings. Some warnings can be ignored,
especially from --strict run, but the code here looks like it needs a
fix. Feel free to get in touch if the warning is not clear.
Drop {}
This applies to other places as well.
> + return dev_err_probe(&pdev->dev, ret, "failed to request IRQs");
> + };
> +
> + /* Configure wakeup capable */
> + device_set_wakeup_capable(pwr->dev, 1);
> + device_set_wakeup_enable(pwr->dev, 1);
> +
> + return 0;
> +}
> +
> +static const struct platform_device_id bd71828_charger_id[] = {
> + { "bd71815-power", ROHM_CHIP_TYPE_BD71815 },
> + { "bd71828-power", ROHM_CHIP_TYPE_BD71828 },
> + { },
> +};
> +MODULE_DEVICE_TABLE(platform, bd71828_charger_id);
> +
> +static struct platform_driver bd71828_power_driver = {
> + .driver = {
> + .name = "bd718xx-power",
> + },
> + .probe = bd71828_power_probe,
> + .id_table = bd71828_charger_id,
> +};
> +
> +module_platform_driver(bd71828_power_driver);
> +MODULE_ALIAS("platform:bd718xx-power");
Drop module alias, incorrect name anyway and you already have proper
aliases from table.
Best regards,
Krzysztof
Am Sun, 17 Aug 2025 07:58:35 +0200
schrieb Krzysztof Kozlowski <krzk@kernel.org>:
> On 16/08/2025 21:19, Andreas Kemnade wrote:
> > Add charger driver for ROHM BD718(15/28/78) PMIC charger block.
> > It is a stripped down version of the driver here:
> > https://lore.kernel.org/lkml/dbd97c1b0d715aa35a8b4d79741e433d97c562aa.1637061794.git.matti.vaittinen@fi.rohmeurope.com/
>
> Why are you duplicating the driver? Why original cannot be used?
>
>
I am not duplicating the driver. That patch series never went in. I am
stripping it down to let things go in step by step. I have also talked
with Sebastian about this. And he also prefers a step by step approach
to have it more easily reviewed.
I also do not have the infrastructure to test things like capacity
degradation over time. There is non-trivial rebasing work involved, so
I even do not feel confident submitting such at all.
> ...
>
> > +
> > +#define RSENS_DEFAULT_30MOHM 30000 /* 30 mOhm in uOhms*/
> > +
> > +static int bd7182x_get_rsens(struct bd71828_power *pwr)
> > +{
> > + u64 tmp = RSENS_CURR;
> > + int rsens_ohm = RSENS_DEFAULT_30MOHM;
> > + struct fwnode_handle *node = NULL;
> > +
> > + if (pwr->dev->parent)
> > + node = dev_fwnode(pwr->dev->parent);
> > +
> > + if (node) {
> > + int ret;
> > + uint32_t rs;
> > +
> > + ret = fwnode_property_read_u32(node,
> > + "rohm,charger-sense-resistor-micro-ohms",
>
> Hm? Are you writing ACPI or DT driver?
>
I am writing a driver for a platform device which gets information via
dt properties. The property is defined in
Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml
[...]
> > + pwr->bat = devm_power_supply_register(&pdev->dev, &bd71828_bat_desc,
> > + &bat_cfg);
> > + if (IS_ERR(pwr->bat)) {
> > + return dev_err_probe(&pdev->dev, PTR_ERR(pwr->bat),
> > + "failed to register bat\n");
> > + }
> > +
> > + ret = bd7182x_get_irqs(pdev, pwr);
> > + if (ret) {
>
> Please run scripts/checkpatch.pl on the patches and fix reported
> warnings. After that, run also 'scripts/checkpatch.pl --strict' on the
> patches and (probably) fix more warnings. Some warnings can be ignored,
> especially from --strict run, but the code here looks like it needs a
> fix. Feel free to get in touch if the warning is not clear.
>
> Drop {}
>
> This applies to other places as well.
>
ok, I have forgotten the --strict. And {} around multiline things do
not trigger anything at my brain, even if it just a single statement.
BTW: Is there any way to mark warnings as handled if they can be
ignored, so that I do not see them in subsequent submissions?
Regards,
Andreas
On 17/08/2025 10:11, Andreas Kemnade wrote: > Am Sun, 17 Aug 2025 07:58:35 +0200 > schrieb Krzysztof Kozlowski <krzk@kernel.org>: > >> On 16/08/2025 21:19, Andreas Kemnade wrote: >>> Add charger driver for ROHM BD718(15/28/78) PMIC charger block. >>> It is a stripped down version of the driver here: >>> https://lore.kernel.org/lkml/dbd97c1b0d715aa35a8b4d79741e433d97c562aa.1637061794.git.matti.vaittinen@fi.rohmeurope.com/ >> >> Why are you duplicating the driver? Why original cannot be used? >> >> > I am not duplicating the driver. That patch series never went in. I am > stripping it down to let things go in step by step. I have also talked > with Sebastian about this. And he also prefers a step by step approach > to have it more easily reviewed. > I also do not have the infrastructure to test things like capacity > degradation over time. There is non-trivial rebasing work involved, so > I even do not feel confident submitting such at all. OK, but if you refer to other work, then also please explain why this is stripped down. Best regards, Krzysztof
On 17/08/2025 11:13, Krzysztof Kozlowski wrote: > On 17/08/2025 10:11, Andreas Kemnade wrote: >> Am Sun, 17 Aug 2025 07:58:35 +0200 >> schrieb Krzysztof Kozlowski <krzk@kernel.org>: >> >>> On 16/08/2025 21:19, Andreas Kemnade wrote: >>>> Add charger driver for ROHM BD718(15/28/78) PMIC charger block. >>>> It is a stripped down version of the driver here: >>>> https://lore.kernel.org/lkml/dbd97c1b0d715aa35a8b4d79741e433d97c562aa.1637061794.git.matti.vaittinen@fi.rohmeurope.com/ >>> >>> Why are you duplicating the driver? Why original cannot be used? >>> >>> >> I am not duplicating the driver. That patch series never went in. I am >> stripping it down to let things go in step by step. I have also talked >> with Sebastian about this. And he also prefers a step by step approach >> to have it more easily reviewed. >> I also do not have the infrastructure to test things like capacity >> degradation over time. There is non-trivial rebasing work involved, so >> I even do not feel confident submitting such at all. > > > OK, but if you refer to other work, then also please explain why this is > stripped down. First of all, thanks a ton Andreas for continuing this work which I never managed to finish! Battery fuel-gauging with coulomb-counter is hard. I believe we can get some results with the original RFC code - but it requires quite a bit of effort. AFAIR, there are (at least) 4 "pain-points". 1. Lack of persistent storage for charging cycles. For proper fuel-gauging, we would need information about battery aging. The PMIC has nothing to store the charging cycle counter when power is cut. That'd require some user-space solution which could store the cycle information in a persistent storage && tell it to the driver at start-up. I don't know if there is open-source userspace solution for this. 2. Battery parameters. This is the real problem. In order to make the fuel-gauging work, the driver needs proper battery information. I wrote the original driver to be able to retrieve the data from a static-battery DT node - but I have a feeling the device-vendor using this PMIC provided battery-info via module parameters. I am not sure if those parameters can be recovered - and as Andreas said, defining them is not easy task. By minimum we would need the OCV-tables and some aging + temperature degradation effects (or VDR-tables which ROHM uses for it's zero-correction algorithm - but AFAIR, defining those VDR tables is not widely known information). 3. ADC offset. The coulomb-counter operates by measuring and integrating voltage-drop over known Rsense resistor. If (when) the ADC has some measurement offset, it will produce a systematic error which accumulates over time. Hence a calibration is required. The BD718[15/28] have an ADC calibration routine, but AFAIR, there was some limitations. I don't remember all the dirty details, but it probably didn't work too well if current consumption was varying during the calibration(?). I think running the calibration is not supported by the driver. 4. Maintaining all this. The fuel-gauging is maths which uses quite a few of battery parameters. Pinpointing an error from parameters, algorithm(s) or hardware is far from trivial because errors can specific to the very battery/system they were detected at. There are probably more problems (some of which I have forgotten, and some of which I haven't even hit yet). TLDR; It'd be hard to do accurate fuel-gauging without proper battery information and some extra work. We could probably get some rough estimates about the capacity - but implementing it only makes sense if there is someone really using it. Charger control on the other hand makes some sense. [It at least allows Andreas to charge his eReader using solar-power when on a biking hiking! How cool is that? ;)] So, dropping fuel-gauge (for now), and upstreaming the rest seems like a very good approach to me. Thanks for CC'in me Andreas. I don't have much time to work on this (as I never do), but please keep me in loop and let me know if I can help... I can at very least review things :) Thanks again for working with this! (Ps. Are you joining ELCE in Amsterdam? It'd be nice to see you there if you do). Yours, -- Matti
Hi Matti,
Am Mon, 18 Aug 2025 09:34:02 +0300
schrieb Matti Vaittinen <mazziesaccount@gmail.com>:
> On 17/08/2025 11:13, Krzysztof Kozlowski wrote:
> > On 17/08/2025 10:11, Andreas Kemnade wrote:
> >> Am Sun, 17 Aug 2025 07:58:35 +0200
> >> schrieb Krzysztof Kozlowski <krzk@kernel.org>:
> >>
> >>> On 16/08/2025 21:19, Andreas Kemnade wrote:
> >>>> Add charger driver for ROHM BD718(15/28/78) PMIC charger block.
> >>>> It is a stripped down version of the driver here:
> >>>> https://lore.kernel.org/lkml/dbd97c1b0d715aa35a8b4d79741e433d97c562aa.1637061794.git.matti.vaittinen@fi.rohmeurope.com/
> >>>
> >>> Why are you duplicating the driver? Why original cannot be used?
> >>>
> >>>
> >> I am not duplicating the driver. That patch series never went in. I am
> >> stripping it down to let things go in step by step. I have also talked
> >> with Sebastian about this. And he also prefers a step by step approach
> >> to have it more easily reviewed.
> >> I also do not have the infrastructure to test things like capacity
> >> degradation over time. There is non-trivial rebasing work involved, so
> >> I even do not feel confident submitting such at all.
> >
> >
> > OK, but if you refer to other work, then also please explain why this is
> > stripped down.
>
> First of all, thanks a ton Andreas for continuing this work which I
> never managed to finish!
>
> Battery fuel-gauging with coulomb-counter is hard. I believe we can get
> some results with the original RFC code - but it requires quite a bit of
> effort. AFAIR, there are (at least) 4 "pain-points".
>
Newest rebase I have is for 6.15. Yes, capacity calculation is hard.
Even the ugly-patched Kobo vendor driver has some surprises. It once
says battery is empty, then I put in charger, rebooted into debian,
Vbat = 4.1V even with charger detached.
I think the fuel-gauging stuff itself should go in a step by step
approach. I am wondering how sophisticated other drivers and hardware
are.
The rn5t618/rc5t619 mainline driver just uses raw coloumb counter
values and there is no compensation for anything. Some hardware does
more sophisticated things itself.
> 1. Lack of persistent storage for charging cycles. For proper
> fuel-gauging, we would need information about battery aging. The PMIC
> has nothing to store the charging cycle counter when power is cut.
> That'd require some user-space solution which could store the cycle
> information in a persistent storage && tell it to the driver at
> start-up. I don't know if there is open-source userspace solution for this.
>
I do not think so, and you will have trouble if you have dual-boot or
from some alternative boot media involved. The BQ27000 stuff has afaik
hw calculation of battery capacity to deal with this.
> 2. Battery parameters. This is the real problem. In order to make the
> fuel-gauging work, the driver needs proper battery information. I wrote
> the original driver to be able to retrieve the data from a
> static-battery DT node - but I have a feeling the device-vendor using
> this PMIC provided battery-info via module parameters. I am not sure if
> those parameters can be recovered - and as Andreas said, defining them
> is not easy task. By minimum we would need the OCV-tables and some aging
> + temperature degradation effects (or VDR-tables which ROHM uses for
> it's zero-correction algorithm - but AFAIR, defining those VDR tables is
> not widely known information).
>
Kobo kernels have these tables as part of the driver, the right one is
selected via an index in the NTX_HWCONFIG blob provided by the
bootloader to the kernel. So that is not necessarily a problem. It
could be translated into dt.
static int ocv_table_28_PR284983N[23] = {
//4200000, 4162288, 4110762, 4066502, 4025265, 3988454, 3955695, 3926323, 3900244, 3876035, 3834038, 3809386, 3794093, 3782718, 3774483, 3768044, 3748158, 3728750, 3704388, 3675577, 3650676, 3463852, 2768530
4200000, 4166349, 4114949, 4072016, 4031575, 3995353, 3963956, 3935650, 3910161, 3883395, 3845310, 3817535, 3801354, 3789708, 3781393, 3774994, 3765230, 3749035, 3726707, 3699147, 3671953, 3607301, 3148394
};
static int vdr_table_h_28_PR284983N[23] = {
//100, 100, 101, 101, 102, 102, 103, 103, 104, 104, 105, 106, 106, 107, 107, 108, 108, 109, 110, 112, 124, 157, 786
100, 100, 101, 102, 102, 105, 106, 107, 112, 108, 108, 105, 105, 108, 110, 110, 110, 111, 112, 114, 120, 131, 620
};
static int vdr_table_m_28_PR284983N[23] = {
//100, 100, 101, 101, 102, 102, 103, 103, 104, 104, 105, 102, 100, 100, 102, 103, 103, 105, 108, 112, 124, 157, 586
100, 100, 103, 106, 110, 114, 115, 119, 122, 122, 115, 113, 112, 114, 117, 124, 126, 123, 122, 126, 140, 156, 558
};
static int vdr_table_l_28_PR284983N[23] = {
//100, 100, 103, 105, 110, 110, 113, 112, 112, 112, 105, 110, 110, 111, 122, 131, 138, 143, 150, 166, 242, 354, 357
100, 100, 105, 110, 114, 117, 121, 125, 126, 122, 116, 114, 115, 118, 124, 132, 140, 148, 156, 170, 210, 355, 579
};
static int vdr_table_vl_28_PR284983N[23] = {
//100, 100, 103, 106, 108, 111, 114, 117, 118, 115, 108, 106, 108, 113, 115, 114, 118, 125, 144, 159, 204, 361, 874
100, 100, 109, 115, 118, 123, 127, 130, 140, 139, 134, 130, 128, 138, 140, 150, 154, 164, 178, 204, 271, 362, 352
};
> 3. ADC offset. The coulomb-counter operates by measuring and integrating
> voltage-drop over known Rsense resistor. If (when) the ADC has some
> measurement offset, it will produce a systematic error which accumulates
> over time. Hence a calibration is required. The BD718[15/28] have an ADC
> calibration routine, but AFAIR, there was some limitations. I don't
> remember all the dirty details, but it probably didn't work too well if
> current consumption was varying during the calibration(?). I think
> running the calibration is not supported by the driver.
>
Yes, that is a pain.
[...]
> TLDR; It'd be hard to do accurate fuel-gauging without proper
battery
> information and some extra work. We could probably get some rough
> estimates about the capacity - but implementing it only makes sense if
> there is someone really using it. Charger control on the other hand
> makes some sense. [It at least allows Andreas to charge his eReader
> using solar-power when on a biking hiking! How cool is that? ;)]
>
And using a hub dynamo.
For now I have a user space script to help me, probably moving that into
input_current_limit.
But it is really nice to see if things are charging or are discharging
unusually fast.
It is a pity that such power sources are not taken into consideration
in standards or charges much. Around 20 year ago or something, I could
just attach a Thinkpad to a solar panel, there was a smooth transition
between discharging a litte (displaying battery discharging time in
weeks) and more ore less charging. Today often the recommendation is to
put somehow another battery in between. But that is technically
somehow nonsense. You need a buffer for power and another one in the
row.
Regards,
Andreas
On 18/08/2025 11:36, Andreas Kemnade wrote:
> Hi Matti,
>
> Am Mon, 18 Aug 2025 09:34:02 +0300
> schrieb Matti Vaittinen <mazziesaccount@gmail.com>:
>
>> On 17/08/2025 11:13, Krzysztof Kozlowski wrote:
>>> On 17/08/2025 10:11, Andreas Kemnade wrote:
>>>> Am Sun, 17 Aug 2025 07:58:35 +0200
>>>> schrieb Krzysztof Kozlowski <krzk@kernel.org>:
>>>>
>>>>> On 16/08/2025 21:19, Andreas Kemnade wrote:
> Newest rebase I have is for 6.15. Yes, capacity calculation is hard.
Just a thing to note. I've drafted some support for another variant, on
top of the v6.6. Just pushed the latest version of that to:
https://github.com/M-Vaittinen/linux/tree/bd72720-on-6.6-rohmpower
It may have something useful for you (or then it doesn't). Following
could perhaps be checked:
8c00ee888283 ("regulator: bd71828: Use platform device id")
0ba48e3a48d4 ("mfd: bd71828: Add IPRE register")
f6caf815fc2f ("power: supply: bd71828: Support setting trickle charge
current")
56197c1819e5 ("dt-bindings: Add trickle-charge upper limit")
af7500d7f278 ("mfd: bd71828: Definition for fast charge term current
register")
98401932fb75 ("mfd: bd71815: Add EXTMOS_EN mask")
e508c94159d8 ("mfd: bd71828: Add charge profile control masks")
e751bf502e29 ("power: supply: bd71828: Support setting charging profiles")
AND
b84792488191 ("power: supply: bd71827-power: Fix pre- and trickle charge
currents")
2f952952cecd ("power: supply: bd71827-charger: Fix print")
These hopefully are already done:
c4fe777755ef ("power: supply: bd71828: Fix Rsense resistor value")
34d9261706b2 ("dt-bindings: mfd: bd71828: Fix sense resistor values")
Rest may be just noise related to this new IC.
Yours,
-- Matti
On 18/08/2025 11:36, Andreas Kemnade wrote:
> Hi Matti,
>
> Am Mon, 18 Aug 2025 09:34:02 +0300
> schrieb Matti Vaittinen <mazziesaccount@gmail.com>:
>
>> On 17/08/2025 11:13, Krzysztof Kozlowski wrote:
>>> On 17/08/2025 10:11, Andreas Kemnade wrote:
>>>> Am Sun, 17 Aug 2025 07:58:35 +0200
>>>> schrieb Krzysztof Kozlowski <krzk@kernel.org>:
>>>>
>>>>> On 16/08/2025 21:19, Andreas Kemnade wrote:
>>>>>> Add charger driver for ROHM BD718(15/28/78) PMIC charger block.
>>>>>> It is a stripped down version of the driver here:
>>>>>> https://lore.kernel.org/lkml/dbd97c1b0d715aa35a8b4d79741e433d97c562aa.1637061794.git.matti.vaittinen@fi.rohmeurope.com/
>>>>>
>>>>> Why are you duplicating the driver? Why original cannot be used?
>>>>>
>>>>>
>>>> I am not duplicating the driver. That patch series never went in. I am
>>>> stripping it down to let things go in step by step. I have also talked
>>>> with Sebastian about this. And he also prefers a step by step approach
>>>> to have it more easily reviewed.
>>>> I also do not have the infrastructure to test things like capacity
>>>> degradation over time. There is non-trivial rebasing work involved, so
>>>> I even do not feel confident submitting such at all.
>>>
>>>
>>> OK, but if you refer to other work, then also please explain why this is
>>> stripped down.
>>
>> First of all, thanks a ton Andreas for continuing this work which I
>> never managed to finish!
>>
>> Battery fuel-gauging with coulomb-counter is hard. I believe we can get
>> some results with the original RFC code - but it requires quite a bit of
>> effort. AFAIR, there are (at least) 4 "pain-points".
>>
> Newest rebase I have is for 6.15. Yes, capacity calculation is hard.
> Even the ugly-patched Kobo vendor driver has some surprises. It once
> says battery is empty, then I put in charger, rebooted into debian,
> Vbat = 4.1V even with charger detached.
:/
> I think the fuel-gauging stuff itself should go in a step by step
> approach.
I agree.
> I am wondering how sophisticated other drivers and hardware
> are.
I have no deep knowledge on this (either). I remember having some
(email) discussions with Linus W about Samsung's chargers / batteries...
My understanding is that there are very different levels of
"sophistication", both in HW and in SW. I really find this fascinating.
Unfortunately, there has also been infamous exploding batteries and
other less pleasant events. Hence this is also slightly dangerous area.
> The rn5t618/rc5t619 mainline driver just uses raw coloumb counter
> values and there is no compensation for anything. Some hardware does
> more sophisticated things itself.
Yes.
>> 1. Lack of persistent storage for charging cycles. For proper
>> fuel-gauging, we would need information about battery aging. The PMIC
>> has nothing to store the charging cycle counter when power is cut.
>> That'd require some user-space solution which could store the cycle
>> information in a persistent storage && tell it to the driver at
>> start-up. I don't know if there is open-source userspace solution for this.
>>
> I do not think so, and you will have trouble if you have dual-boot or
> from some alternative boot media involved.
I didn't even think about it. So, even with persistent PMIC areas, if
software is doing the charging count book-keeping, it won't be great for
a generic design. (May work Ok with an embedded device which is likely
to not get booted with other flavours of software).
> The BQ27000 stuff has afaik
> hw calculation of battery capacity to deal with this.
>
>> 2. Battery parameters. This is the real problem. In order to make the
>> fuel-gauging work, the driver needs proper battery information. I wrote
>> the original driver to be able to retrieve the data from a
>> static-battery DT node - but I have a feeling the device-vendor using
>> this PMIC provided battery-info via module parameters. I am not sure if
>> those parameters can be recovered - and as Andreas said, defining them
>> is not easy task. By minimum we would need the OCV-tables and some aging
>> + temperature degradation effects (or VDR-tables which ROHM uses for
>> it's zero-correction algorithm - but AFAIR, defining those VDR tables is
>> not widely known information).
>>
> Kobo kernels have these tables as part of the driver, the right one is
> selected via an index in the NTX_HWCONFIG blob provided by the
> bootloader to the kernel. So that is not necessarily a problem. It
> could be translated into dt.
>
> static int ocv_table_28_PR284983N[23] = {
> //4200000, 4162288, 4110762, 4066502, 4025265, 3988454, 3955695, 3926323, 3900244, 3876035, 3834038, 3809386, 3794093, 3782718, 3774483, 3768044, 3748158, 3728750, 3704388, 3675577, 3650676, 3463852, 2768530
> 4200000, 4166349, 4114949, 4072016, 4031575, 3995353, 3963956, 3935650, 3910161, 3883395, 3845310, 3817535, 3801354, 3789708, 3781393, 3774994, 3765230, 3749035, 3726707, 3699147, 3671953, 3607301, 3148394
> };
>
> static int vdr_table_h_28_PR284983N[23] = {
> //100, 100, 101, 101, 102, 102, 103, 103, 104, 104, 105, 106, 106, 107, 107, 108, 108, 109, 110, 112, 124, 157, 786
> 100, 100, 101, 102, 102, 105, 106, 107, 112, 108, 108, 105, 105, 108, 110, 110, 110, 111, 112, 114, 120, 131, 620
> };
>
> static int vdr_table_m_28_PR284983N[23] = {
> //100, 100, 101, 101, 102, 102, 103, 103, 104, 104, 105, 102, 100, 100, 102, 103, 103, 105, 108, 112, 124, 157, 586
> 100, 100, 103, 106, 110, 114, 115, 119, 122, 122, 115, 113, 112, 114, 117, 124, 126, 123, 122, 126, 140, 156, 558
> };
>
> static int vdr_table_l_28_PR284983N[23] = {
> //100, 100, 103, 105, 110, 110, 113, 112, 112, 112, 105, 110, 110, 111, 122, 131, 138, 143, 150, 166, 242, 354, 357
> 100, 100, 105, 110, 114, 117, 121, 125, 126, 122, 116, 114, 115, 118, 124, 132, 140, 148, 156, 170, 210, 355, 579
> };
>
> static int vdr_table_vl_28_PR284983N[23] = {
> //100, 100, 103, 106, 108, 111, 114, 117, 118, 115, 108, 106, 108, 113, 115, 114, 118, 125, 144, 159, 204, 361, 874
> 100, 100, 109, 115, 118, 123, 127, 130, 140, 139, 134, 130, 128, 138, 140, 150, 154, 164, 178, 204, 271, 362, 352
> };
Oh, good. If we can get the right battery parameters from the vendor
driver, then the main problem gets solved. Although, multiple sets of
different VDR tables probably means, that there are variants with
different types of battery out there. I assume the bootloader can
somehow detect the battery type to provide the correct blob?
>
>> 3. ADC offset. The coulomb-counter operates by measuring and integrating
>> voltage-drop over known Rsense resistor. If (when) the ADC has some
>> measurement offset, it will produce a systematic error which accumulates
>> over time. Hence a calibration is required. The BD718[15/28] have an ADC
>> calibration routine, but AFAIR, there was some limitations. I don't
>> remember all the dirty details, but it probably didn't work too well if
>> current consumption was varying during the calibration(?). I think
>> running the calibration is not supported by the driver.
>>
> Yes, that is a pain.
I am pretty sure I can dig the registers which initiate the ADC
calibration, but I don't have real devices with real battery to test it.
I can try to find that information if if you wish to experiment with it
though...
...The BD718xx had a magic "test register area" - where this calibration
stuff (amongst other, very hazardous things) resides. Problem is that
this "test register area" is implemented in a way, that it is behind
another I2C slave address, which can be enabled by a magic write
sequence. Enabling it from a generically usable driver can't really be
done. It would be hazardous if there was another device in the I2C with
the same slave address as the "test register area".
> [...]
>> TLDR; It'd be hard to do accurate fuel-gauging without proper
> battery
>> information and some extra work. We could probably get some rough
>> estimates about the capacity - but implementing it only makes sense if
>> there is someone really using it. Charger control on the other hand
>> makes some sense. [It at least allows Andreas to charge his eReader
>> using solar-power when on a biking hiking! How cool is that? ;)]
>>
> And using a hub dynamo.
> For now I have a user space script to help me, probably moving that into
> input_current_limit.
Sounds good to me.
> But it is really nice to see if things are charging or are discharging
> unusually fast.
> It is a pity that such power sources are not taken into consideration
> in standards or charges much. Around 20 year ago or something, I could
> just attach a Thinkpad to a solar panel, there was a smooth transition
> between discharging a litte (displaying battery discharging time in
> weeks) and more ore less charging. Today often the recommendation is to
> put somehow another battery in between. But that is technically
> somehow nonsense. You need a buffer for power and another one in the
> row.
Yours,
-- Matti
Am Mon, 18 Aug 2025 12:32:43 +0300
schrieb Matti Vaittinen <mazziesaccount@gmail.com>:
> > Kobo kernels have these tables as part of the driver, the right one is
> > selected via an index in the NTX_HWCONFIG blob provided by the
> > bootloader to the kernel. So that is not necessarily a problem. It
> > could be translated into dt.
> >
> > static int ocv_table_28_PR284983N[23] = {
> > //4200000, 4162288, 4110762, 4066502, 4025265, 3988454, 3955695, 3926323, 3900244, 3876035, 3834038, 3809386, 3794093, 3782718, 3774483, 3768044, 3748158, 3728750, 3704388, 3675577, 3650676, 3463852, 2768530
> > 4200000, 4166349, 4114949, 4072016, 4031575, 3995353, 3963956, 3935650, 3910161, 3883395, 3845310, 3817535, 3801354, 3789708, 3781393, 3774994, 3765230, 3749035, 3726707, 3699147, 3671953, 3607301, 3148394
> > };
> >
> > static int vdr_table_h_28_PR284983N[23] = {
> > //100, 100, 101, 101, 102, 102, 103, 103, 104, 104, 105, 106, 106, 107, 107, 108, 108, 109, 110, 112, 124, 157, 786
> > 100, 100, 101, 102, 102, 105, 106, 107, 112, 108, 108, 105, 105, 108, 110, 110, 110, 111, 112, 114, 120, 131, 620
> > };
> >
> > static int vdr_table_m_28_PR284983N[23] = {
> > //100, 100, 101, 101, 102, 102, 103, 103, 104, 104, 105, 102, 100, 100, 102, 103, 103, 105, 108, 112, 124, 157, 586
> > 100, 100, 103, 106, 110, 114, 115, 119, 122, 122, 115, 113, 112, 114, 117, 124, 126, 123, 122, 126, 140, 156, 558
> > };
> >
> > static int vdr_table_l_28_PR284983N[23] = {
> > //100, 100, 103, 105, 110, 110, 113, 112, 112, 112, 105, 110, 110, 111, 122, 131, 138, 143, 150, 166, 242, 354, 357
> > 100, 100, 105, 110, 114, 117, 121, 125, 126, 122, 116, 114, 115, 118, 124, 132, 140, 148, 156, 170, 210, 355, 579
> > };
> >
> > static int vdr_table_vl_28_PR284983N[23] = {
> > //100, 100, 103, 106, 108, 111, 114, 117, 118, 115, 108, 106, 108, 113, 115, 114, 118, 125, 144, 159, 204, 361, 874
> > 100, 100, 109, 115, 118, 123, 127, 130, 140, 139, 134, 130, 128, 138, 140, 150, 154, 164, 178, 204, 271, 362, 352
> > };
>
> Oh, good. If we can get the right battery parameters from the vendor
> driver, then the main problem gets solved. Although, multiple sets of
> different VDR tables probably means, that there are variants with
> different types of battery out there. I assume the bootloader can
> somehow detect the battery type to provide the correct blob?
Historically the Kobo devices ship said HWCONFIG blob apparently to use
the same kernel on multiple devices, then devicetree was invented and
used what was available. There is then a
switch(gptHWCFG->m_val.bBattery) {
...
ocv_table_default =
ocv_table_28_PR284983N;
So that all only means there
are several different batteries amoung the devices supported by that
kernel. From my guts feeling I wonder if the is_relaxed stuff is
properly working and I wonder whether a Kalman filter would give better
results, but that is all something for the future.
Regards,
Andreas
On 20/08/2025 19:05, Andreas Kemnade wrote:
> Am Mon, 18 Aug 2025 12:32:43 +0300
> schrieb Matti Vaittinen <mazziesaccount@gmail.com>:
>
>>> Kobo kernels have these tables as part of the driver, the right one is
>>> selected via an index in the NTX_HWCONFIG blob provided by the
>>> bootloader to the kernel. So that is not necessarily a problem. It
>>> could be translated into dt.
>>>
>>> static int ocv_table_28_PR284983N[23] = {
>>> //4200000, 4162288, 4110762, 4066502, 4025265, 3988454, 3955695, 3926323, 3900244, 3876035, 3834038, 3809386, 3794093, 3782718, 3774483, 3768044, 3748158, 3728750, 3704388, 3675577, 3650676, 3463852, 2768530
>>> 4200000, 4166349, 4114949, 4072016, 4031575, 3995353, 3963956, 3935650, 3910161, 3883395, 3845310, 3817535, 3801354, 3789708, 3781393, 3774994, 3765230, 3749035, 3726707, 3699147, 3671953, 3607301, 3148394
>>> };
>>>
>>> static int vdr_table_h_28_PR284983N[23] = {
>>> //100, 100, 101, 101, 102, 102, 103, 103, 104, 104, 105, 106, 106, 107, 107, 108, 108, 109, 110, 112, 124, 157, 786
>>> 100, 100, 101, 102, 102, 105, 106, 107, 112, 108, 108, 105, 105, 108, 110, 110, 110, 111, 112, 114, 120, 131, 620
>>> };
>>>
>>> static int vdr_table_m_28_PR284983N[23] = {
>>> //100, 100, 101, 101, 102, 102, 103, 103, 104, 104, 105, 102, 100, 100, 102, 103, 103, 105, 108, 112, 124, 157, 586
>>> 100, 100, 103, 106, 110, 114, 115, 119, 122, 122, 115, 113, 112, 114, 117, 124, 126, 123, 122, 126, 140, 156, 558
>>> };
>>>
>>> static int vdr_table_l_28_PR284983N[23] = {
>>> //100, 100, 103, 105, 110, 110, 113, 112, 112, 112, 105, 110, 110, 111, 122, 131, 138, 143, 150, 166, 242, 354, 357
>>> 100, 100, 105, 110, 114, 117, 121, 125, 126, 122, 116, 114, 115, 118, 124, 132, 140, 148, 156, 170, 210, 355, 579
>>> };
>>>
>>> static int vdr_table_vl_28_PR284983N[23] = {
>>> //100, 100, 103, 106, 108, 111, 114, 117, 118, 115, 108, 106, 108, 113, 115, 114, 118, 125, 144, 159, 204, 361, 874
>>> 100, 100, 109, 115, 118, 123, 127, 130, 140, 139, 134, 130, 128, 138, 140, 150, 154, 164, 178, 204, 271, 362, 352
>>> };
>>
>> Oh, good. If we can get the right battery parameters from the vendor
>> driver, then the main problem gets solved. Although, multiple sets of
>> different VDR tables probably means, that there are variants with
>> different types of battery out there. I assume the bootloader can
>> somehow detect the battery type to provide the correct blob?
>
> Historically the Kobo devices ship said HWCONFIG blob apparently to use
> the same kernel on multiple devices, then devicetree was invented and
> used what was available. There is then a
> switch(gptHWCFG->m_val.bBattery) {
> ...
> ocv_table_default =
> ocv_table_28_PR284983N;
>
>
>
> So that all only means there
> are several different batteries amoung the devices supported by that
> kernel.
Ah. So you believe the other batteries are used on other devices which
run the same kernel. Makes sense.
> From my guts feeling I wonder if the is_relaxed stuff is
> properly working and I wonder whether a Kalman filter would give better
> results, but that is all something for the future.
I believe your experience is stronger than mine (also) here :) I don't
really know the theory behind the 'relaxed battery' (or much of other
battery chemistry stuff). I was merely trusting the inventions of the HQ
engineers, who told me that the OCV tables can be used to adjust the
coulomb counter when the battery is 'relaxed'. 'Relaxed' here meaning
that it has not been charged (or a lot of current has not been drawn
from it) recently. AFAIR, most of the PMIC models had some hardware
support for detecting if the battery is in 'relaxed' state.
I admit it sounds like somewhat uncertain approach. I'd love to hear how
you think the filter would help. I suppose you think of applying some
filtering to the CC correction? Eg, 'smoothen' the CC resetting based on
relaxed OCV, by applying some filtering to the correction values? Sounds
cool! But... It does also sound the analysis about the impact of the
filtering will be hard.
The reason why I dropped the simple-gauge RFC is, that I don't even have
a BD71828 which is connected to a battery (and even with that the
testing would be hard and slow). I thought of trying to do some
simulation, but even that felt quite futile without some proper
battery-data. So, my work was largely just shooting blindly and
listening if some customer started screaming so loud it could be heard
in Finland ^_^;
I think the "ideology" of the fuel-gauging in the HQ was to accept some
of the errors and trust the VDR table based zero-correction to fix
things when battery was about to get empty. The thinking was that more
accurate battery status gets important only when the battery is getting
exhausted - and that's when the zero-correction kicks in.
Yours,
-- Matti
Am Thu, 21 Aug 2025 08:31:06 +0300
schrieb Matti Vaittinen <mazziesaccount@gmail.com>:
> On 20/08/2025 19:05, Andreas Kemnade wrote:
> > Am Mon, 18 Aug 2025 12:32:43 +0300
> > schrieb Matti Vaittinen <mazziesaccount@gmail.com>:
> >
> >>> Kobo kernels have these tables as part of the driver, the right one is
> >>> selected via an index in the NTX_HWCONFIG blob provided by the
> >>> bootloader to the kernel. So that is not necessarily a problem. It
> >>> could be translated into dt.
> >>>
> >>> static int ocv_table_28_PR284983N[23] = {
> >>> //4200000, 4162288, 4110762, 4066502, 4025265, 3988454, 3955695, 3926323, 3900244, 3876035, 3834038, 3809386, 3794093, 3782718, 3774483, 3768044, 3748158, 3728750, 3704388, 3675577, 3650676, 3463852, 2768530
> >>> 4200000, 4166349, 4114949, 4072016, 4031575, 3995353, 3963956, 3935650, 3910161, 3883395, 3845310, 3817535, 3801354, 3789708, 3781393, 3774994, 3765230, 3749035, 3726707, 3699147, 3671953, 3607301, 3148394
> >>> };
> >>>
> >>> static int vdr_table_h_28_PR284983N[23] = {
> >>> //100, 100, 101, 101, 102, 102, 103, 103, 104, 104, 105, 106, 106, 107, 107, 108, 108, 109, 110, 112, 124, 157, 786
> >>> 100, 100, 101, 102, 102, 105, 106, 107, 112, 108, 108, 105, 105, 108, 110, 110, 110, 111, 112, 114, 120, 131, 620
> >>> };
> >>>
> >>> static int vdr_table_m_28_PR284983N[23] = {
> >>> //100, 100, 101, 101, 102, 102, 103, 103, 104, 104, 105, 102, 100, 100, 102, 103, 103, 105, 108, 112, 124, 157, 586
> >>> 100, 100, 103, 106, 110, 114, 115, 119, 122, 122, 115, 113, 112, 114, 117, 124, 126, 123, 122, 126, 140, 156, 558
> >>> };
> >>>
> >>> static int vdr_table_l_28_PR284983N[23] = {
> >>> //100, 100, 103, 105, 110, 110, 113, 112, 112, 112, 105, 110, 110, 111, 122, 131, 138, 143, 150, 166, 242, 354, 357
> >>> 100, 100, 105, 110, 114, 117, 121, 125, 126, 122, 116, 114, 115, 118, 124, 132, 140, 148, 156, 170, 210, 355, 579
> >>> };
> >>>
> >>> static int vdr_table_vl_28_PR284983N[23] = {
> >>> //100, 100, 103, 106, 108, 111, 114, 117, 118, 115, 108, 106, 108, 113, 115, 114, 118, 125, 144, 159, 204, 361, 874
> >>> 100, 100, 109, 115, 118, 123, 127, 130, 140, 139, 134, 130, 128, 138, 140, 150, 154, 164, 178, 204, 271, 362, 352
> >>> };
> >>
> >> Oh, good. If we can get the right battery parameters from the vendor
> >> driver, then the main problem gets solved. Although, multiple sets of
> >> different VDR tables probably means, that there are variants with
> >> different types of battery out there. I assume the bootloader can
> >> somehow detect the battery type to provide the correct blob?
> >
> > Historically the Kobo devices ship said HWCONFIG blob apparently to use
> > the same kernel on multiple devices, then devicetree was invented and
> > used what was available. There is then a
> > switch(gptHWCFG->m_val.bBattery) {
> > ...
> > ocv_table_default =
> > ocv_table_28_PR284983N;
> >
> >
> >
> > So that all only means there
> > are several different batteries amoung the devices supported by that
> > kernel.
>
> Ah. So you believe the other batteries are used on other devices which
> run the same kernel. Makes sense.
>
> > From my guts feeling I wonder if the is_relaxed stuff is
> > properly working and I wonder whether a Kalman filter would give better
> > results, but that is all something for the future.
>
> I believe your experience is stronger than mine (also) here :) I don't
> really know the theory behind the 'relaxed battery' (or much of other
> battery chemistry stuff). I was merely trusting the inventions of the HQ
> engineers, who told me that the OCV tables can be used to adjust the
> coulomb counter when the battery is 'relaxed'. 'Relaxed' here meaning
> that it has not been charged (or a lot of current has not been drawn
> from it) recently. AFAIR, most of the PMIC models had some hardware
> support for detecting if the battery is in 'relaxed' state.
>
I am also no battery chemistry expert. But I have looked a lot at
battery voltage behavior for various reasons e.g. for simple things
like: does my charger behave? I do not believe in
in the concept of a boolean is_relaxed. You can always know something
about battery state by doing something like a table_lookup(vbat -
some_factor * ibat)
Depending on situation you have different accuracy. Your battery will
probably not be empty if e.g. your voltage is at 3.9V and you are not
doing excessive charging. I have seen many surprises. My pinephone
sometimes suddenly went off with battery state like 40%. I personally
feel more confident, if I can additionally see the battery voltage.
I have not debugged such issues much yet.
> I admit it sounds like somewhat uncertain approach. I'd love to hear how
> you think the filter would help. I suppose you think of applying some
> filtering to the CC correction? Eg, 'smoothen' the CC resetting based on
> relaxed OCV, by applying some filtering to the correction values? Sounds
> cool! But... It does also sound the analysis about the impact of the
> filtering will be hard.
The problems to solve look a bit like problems in inertial navigation
and there Kalman filters are used. There you have e.g. gyroscopes to
determine the rate of turn, producing drift like a coloumb counter.
Accelerometers can show your orientation if things are "relaxed" so not
much acceleration besides gravity. Magnetometers can be "unrelaxed" by
some magnetic disturbance.
The basic point is you can put the accuracy into the model. So you can
use the information like battery capacity can be estimated by
looking voltage/current by +/- 15%, so something produced by looking
at the gauge cannot be out of that range. On the other hand capacity can
not change faster than fuel gauge + estimated drift can produce.
You can spend a life time on optimizing such filters. The question is
how easily you can improve something by using them. The analysis of the
impact can be doing a bit more subjective by looking at how sane the
capacity display behaves, and whether there are less surprises.
More objectively, you can of course relax the battery as much as
possible and check if there are no illogical capacity changes or do
well defined charging/discharging and look what happens to see whether
capacity was correct.
Regards,
Andreas
© 2016 - 2025 Red Hat, Inc.