[PATCH 0/2] power: supply: add charger for BD71828

[PATCH 0/2] power: supply: add charger for BD71828

[Andreas Kemnade,,,]

Add basic charger which does just read out simple registers without
doing any sophisticated things. 

This is a stripped down version of
https://lore.kernel.org/lkml/dbd97c1b0d715aa35a8b4d79741e433d97c562aa.1637061794.git.matti.vaittinen@fi.rohmeurope.com/

That version includes all the bells-and-whistles you might imagine
around coloumb counter handling and capacity measurement which includes
changes no the power supply core.
Rather do a step by step approach to keep that reviewable.

Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
Andreas Kemnade (1):
      power: supply: Add bd718(15/28/78) charger driver

Matti Vaittinen (1):
      mfd: bd71828, bd71815 prepare for power-supply support

 drivers/mfd/rohm-bd71828.c           |   44 +-
 drivers/power/supply/Kconfig         |   11 +
 drivers/power/supply/Makefile        |    1 +
 drivers/power/supply/bd71828-power.c | 1144 ++++++++++++++++++++++++++++++++++
 include/linux/mfd/rohm-bd71828.h     |   65 ++
 include/linux/mfd/rohm-generic.h     |    2 +
 6 files changed, 1258 insertions(+), 9 deletions(-)
---
base-commit: 8f5ae30d69d7543eee0d70083daf4de8fe15d585
change-id: 20250816-bd71828-charger-9d187346f734

Best regards,
-- 
Andreas Kemnade,,, <andreas@kemnade.info>

[PATCH 1/2] mfd: bd71828, bd71815 prepare for power-supply support

[Andreas Kemnade,,,]

From: Matti Vaittinen <mazziesaccount@gmail.com>

Add core support for ROHM BD718(15/28/78) PMIC's charger blocks.

Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
 drivers/mfd/rohm-bd71828.c       | 44 +++++++++++++++++++++------
 include/linux/mfd/rohm-bd71828.h | 65 ++++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/rohm-generic.h |  2 ++
 3 files changed, 102 insertions(+), 9 deletions(-)

diff --git a/drivers/mfd/rohm-bd71828.c b/drivers/mfd/rohm-bd71828.c
index a14b7aa69c3c61d51f2aeeae9afdf222310d63e3..84a64c3b9c9f52e663855c89ed78ede9a7c21f55 100644
--- a/drivers/mfd/rohm-bd71828.c
+++ b/drivers/mfd/rohm-bd71828.c
@@ -45,8 +45,8 @@ static const struct resource bd71828_rtc_irqs[] = {
 
 static const struct resource bd71815_power_irqs[] = {
 	DEFINE_RES_IRQ_NAMED(BD71815_INT_DCIN_RMV, "bd71815-dcin-rmv"),
-	DEFINE_RES_IRQ_NAMED(BD71815_INT_CLPS_OUT, "bd71815-clps-out"),
-	DEFINE_RES_IRQ_NAMED(BD71815_INT_CLPS_IN, "bd71815-clps-in"),
+	DEFINE_RES_IRQ_NAMED(BD71815_INT_CLPS_OUT, "bd71815-dcin-clps-out"),
+	DEFINE_RES_IRQ_NAMED(BD71815_INT_CLPS_IN, "bd71815-dcin-clps-in"),
 	DEFINE_RES_IRQ_NAMED(BD71815_INT_DCIN_OVP_RES, "bd71815-dcin-ovp-res"),
 	DEFINE_RES_IRQ_NAMED(BD71815_INT_DCIN_OVP_DET, "bd71815-dcin-ovp-det"),
 	DEFINE_RES_IRQ_NAMED(BD71815_INT_DCIN_MON_RES, "bd71815-dcin-mon-res"),
@@ -56,7 +56,7 @@ static const struct resource bd71815_power_irqs[] = {
 	DEFINE_RES_IRQ_NAMED(BD71815_INT_VSYS_LOW_RES, "bd71815-vsys-low-res"),
 	DEFINE_RES_IRQ_NAMED(BD71815_INT_VSYS_LOW_DET, "bd71815-vsys-low-det"),
 	DEFINE_RES_IRQ_NAMED(BD71815_INT_VSYS_MON_RES, "bd71815-vsys-mon-res"),
-	DEFINE_RES_IRQ_NAMED(BD71815_INT_VSYS_MON_RES, "bd71815-vsys-mon-det"),
+	DEFINE_RES_IRQ_NAMED(BD71815_INT_VSYS_MON_DET, "bd71815-vsys-mon-det"),
 	DEFINE_RES_IRQ_NAMED(BD71815_INT_CHG_WDG_TEMP, "bd71815-chg-wdg-temp"),
 	DEFINE_RES_IRQ_NAMED(BD71815_INT_CHG_WDG_TIME, "bd71815-chg-wdg"),
 	DEFINE_RES_IRQ_NAMED(BD71815_INT_CHG_RECHARGE_RES, "bd71815-rechg-res"),
@@ -87,10 +87,10 @@ static const struct resource bd71815_power_irqs[] = {
 	DEFINE_RES_IRQ_NAMED(BD71815_INT_BAT_OVER_CURR_2_DET, "bd71815-bat-oc2-det"),
 	DEFINE_RES_IRQ_NAMED(BD71815_INT_BAT_OVER_CURR_3_RES, "bd71815-bat-oc3-res"),
 	DEFINE_RES_IRQ_NAMED(BD71815_INT_BAT_OVER_CURR_3_DET, "bd71815-bat-oc3-det"),
-	DEFINE_RES_IRQ_NAMED(BD71815_INT_TEMP_BAT_LOW_RES, "bd71815-bat-low-res"),
-	DEFINE_RES_IRQ_NAMED(BD71815_INT_TEMP_BAT_LOW_DET, "bd71815-bat-low-det"),
-	DEFINE_RES_IRQ_NAMED(BD71815_INT_TEMP_BAT_HI_RES, "bd71815-bat-hi-res"),
-	DEFINE_RES_IRQ_NAMED(BD71815_INT_TEMP_BAT_HI_DET, "bd71815-bat-hi-det"),
+	DEFINE_RES_IRQ_NAMED(BD71815_INT_TEMP_BAT_LOW_RES, "bd71815-temp-bat-low-res"),
+	DEFINE_RES_IRQ_NAMED(BD71815_INT_TEMP_BAT_LOW_DET, "bd71815-temp-bat-low-det"),
+	DEFINE_RES_IRQ_NAMED(BD71815_INT_TEMP_BAT_HI_RES, "bd71815-temp-bat-hi-res"),
+	DEFINE_RES_IRQ_NAMED(BD71815_INT_TEMP_BAT_HI_DET, "bd71815-temp-bat-hi-det"),
 };
 
 static const struct mfd_cell bd71815_mfd_cells[] = {
@@ -109,7 +109,30 @@ static const struct mfd_cell bd71815_mfd_cells[] = {
 	},
 };
 
-static const struct mfd_cell bd71828_mfd_cells[] = {
+static const struct resource bd71828_power_irqs[] = {
+	DEFINE_RES_IRQ_NAMED(BD71828_INT_CHG_TOPOFF_TO_DONE,
+			     "bd71828-chg-done"),
+	DEFINE_RES_IRQ_NAMED(BD71828_INT_DCIN_DET, "bd71828-pwr-dcin-in"),
+	DEFINE_RES_IRQ_NAMED(BD71828_INT_DCIN_RMV, "bd71828-pwr-dcin-out"),
+	DEFINE_RES_IRQ_NAMED(BD71828_INT_BAT_LOW_VOLT_RES,
+			     "bd71828-vbat-normal"),
+	DEFINE_RES_IRQ_NAMED(BD71828_INT_BAT_LOW_VOLT_DET, "bd71828-vbat-low"),
+	DEFINE_RES_IRQ_NAMED(BD71828_INT_TEMP_BAT_HI_DET, "bd71828-btemp-hi"),
+	DEFINE_RES_IRQ_NAMED(BD71828_INT_TEMP_BAT_HI_RES, "bd71828-btemp-cool"),
+	DEFINE_RES_IRQ_NAMED(BD71828_INT_TEMP_BAT_LOW_DET, "bd71828-btemp-lo"),
+	DEFINE_RES_IRQ_NAMED(BD71828_INT_TEMP_BAT_LOW_RES,
+			     "bd71828-btemp-warm"),
+	DEFINE_RES_IRQ_NAMED(BD71828_INT_TEMP_CHIP_OVER_VF_DET,
+			     "bd71828-temp-hi"),
+	DEFINE_RES_IRQ_NAMED(BD71828_INT_TEMP_CHIP_OVER_VF_RES,
+			     "bd71828-temp-norm"),
+	DEFINE_RES_IRQ_NAMED(BD71828_INT_TEMP_CHIP_OVER_125_DET,
+			     "bd71828-temp-125-over"),
+	DEFINE_RES_IRQ_NAMED(BD71828_INT_TEMP_CHIP_OVER_125_RES,
+			     "bd71828-temp-125-under"),
+};
+
+static struct mfd_cell bd71828_mfd_cells[] = {
 	{ .name = "bd71828-pmic", },
 	{ .name = "bd71828-gpio", },
 	{ .name = "bd71828-led", .of_compatible = "rohm,bd71828-leds" },
@@ -118,8 +141,11 @@ static const struct mfd_cell bd71828_mfd_cells[] = {
 	 * BD70528 clock gate are the register address and mask.
 	 */
 	{ .name = "bd71828-clk", },
-	{ .name = "bd71827-power", },
 	{
+		.name = "bd71828-power",
+		.resources = bd71828_power_irqs,
+		.num_resources = ARRAY_SIZE(bd71828_power_irqs),
+	}, {
 		.name = "bd71828-rtc",
 		.resources = bd71828_rtc_irqs,
 		.num_resources = ARRAY_SIZE(bd71828_rtc_irqs),
diff --git a/include/linux/mfd/rohm-bd71828.h b/include/linux/mfd/rohm-bd71828.h
index ce786c96404a3dc9d5124ffbbd507df89ca0e5ba..a34991984caa8724e925f1c59de4bcfa543ae411 100644
--- a/include/linux/mfd/rohm-bd71828.h
+++ b/include/linux/mfd/rohm-bd71828.h
@@ -189,6 +189,71 @@ enum {
 /* Charger/Battey */
 #define BD71828_REG_CHG_STATE		0x65
 #define BD71828_REG_CHG_FULL		0xd2
+#define BD71828_REG_CHG_EN		0x6F
+#define BD71828_REG_DCIN_STAT		0x68
+#define BD71828_MASK_DCIN_DET		0x01
+#define BD71828_REG_VDCIN_U		0x9c
+#define BD71828_MASK_CHG_EN		0x01
+#define BD71828_CHG_MASK_DCIN_U		0x0f
+#define BD71828_REG_BAT_STAT		0x67
+#define BD71828_REG_BAT_TEMP		0x6c
+#define BD71828_MASK_BAT_TEMP		0x07
+#define BD71828_BAT_TEMP_OPEN		0x07
+#define BD71828_MASK_BAT_DET		0x20
+#define BD71828_MASK_BAT_DET_DONE	0x10
+#define BD71828_REG_CHG_STATE		0x65
+#define BD71828_REG_VBAT_U		0x8c
+#define BD71828_MASK_VBAT_U		0x0f
+#define BD71828_REG_VBAT_REX_AVG_U	0x92
+
+#define BD71828_REG_OCV_PWRON_U		0x8A
+
+#define BD71828_REG_VBAT_MIN_AVG_U	0x8e
+#define BD71828_REG_VBAT_MIN_AVG_L	0x8f
+
+#define BD71828_REG_CC_CNT3		0xb5
+#define BD71828_REG_CC_CNT2		0xb6
+#define BD71828_REG_CC_CNT1		0xb7
+#define BD71828_REG_CC_CNT0		0xb8
+#define BD71828_REG_CC_CURCD_AVG_U	0xb2
+#define BD71828_MASK_CC_CURCD_AVG_U	0x3f
+#define BD71828_MASK_CC_CUR_DIR		0x80
+#define BD71828_REG_VM_BTMP_U		0xa1
+#define BD71828_REG_VM_BTMP_L		0xa2
+#define BD71828_MASK_VM_BTMP_U		0x0f
+#define BD71828_REG_COULOMB_CTRL	0xc4
+#define BD71828_REG_COULOMB_CTRL2	0xd2
+#define BD71828_MASK_REX_CC_CLR		0x01
+#define BD71828_MASK_FULL_CC_CLR	0x10
+#define BD71828_REG_CC_CNT_FULL3	0xbd
+#define BD71828_REG_CC_CNT_CHG3		0xc1
+
+#define BD71828_REG_VBAT_INITIAL1_U	0x86
+#define BD71828_REG_VBAT_INITIAL1_L	0x87
+
+#define BD71828_REG_VBAT_INITIAL2_U	0x88
+#define BD71828_REG_VBAT_INITIAL2_L	0x89
+
+#define BD71828_REG_IBAT_U		0xb0
+#define BD71828_REG_IBAT_L		0xb1
+
+#define BD71828_REG_IBAT_AVG_U		0xb2
+#define BD71828_REG_IBAT_AVG_L		0xb3
+
+#define BD71828_REG_VSYS_AVG_U		0x96
+#define BD71828_REG_VSYS_AVG_L		0x97
+#define BD71828_REG_VSYS_MIN_AVG_U	0x98
+#define BD71828_REG_VSYS_MIN_AVG_L	0x99
+#define BD71828_REG_CHG_SET1		0x75
+#define BD71828_REG_ALM_VBAT_LIMIT_U	0xaa
+#define BD71828_REG_BATCAP_MON_LIMIT_U	0xcc
+#define BD71828_REG_CONF		0x64
+
+#define BD71828_REG_DCIN_CLPS		0x71
+
+#define BD71828_REG_MEAS_CLEAR		0xaf
+
+
 
 /* LEDs */
 #define BD71828_REG_LED_CTRL		0x4A
diff --git a/include/linux/mfd/rohm-generic.h b/include/linux/mfd/rohm-generic.h
index 579e8dcfcca41d2680283819684a1014617d0d4b..5e9d0da380ec0fc3245bee998c791a162a34e3fa 100644
--- a/include/linux/mfd/rohm-generic.h
+++ b/include/linux/mfd/rohm-generic.h
@@ -13,9 +13,11 @@ enum rohm_chip_type {
 	ROHM_CHIP_TYPE_BD9574,
 	ROHM_CHIP_TYPE_BD9576,
 	ROHM_CHIP_TYPE_BD71815,
+	ROHM_CHIP_TYPE_BD71827,
 	ROHM_CHIP_TYPE_BD71828,
 	ROHM_CHIP_TYPE_BD71837,
 	ROHM_CHIP_TYPE_BD71847,
+	ROHM_CHIP_TYPE_BD71878,
 	ROHM_CHIP_TYPE_BD96801,
 	ROHM_CHIP_TYPE_BD96802,
 	ROHM_CHIP_TYPE_BD96805,

-- 
2.39.5

[PATCH 2/2] power: supply: Add bd718(15/28/78) charger driver

[Andreas Kemnade]

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.5

Re: [PATCH 2/2] power: supply: Add bd718(15/28/78) charger driver

[Krzysztof Kozlowski]

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

Re: [PATCH 2/2] power: supply: Add bd718(15/28/78) charger driver

[Andreas Kemnade]

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

Re: [PATCH 2/2] power: supply: Add bd718(15/28/78) charger driver

[Krzysztof Kozlowski]

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

Re: [PATCH 2/2] power: supply: Add bd718(15/28/78) charger driver

[kernel test robot]

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

Re: [PATCH 1/2] mfd: bd71828, bd71815 prepare for power-supply support

[Matti Vaittinen]

Thanks a ton for picking up from where I left it Andreas! :) I _really_ 
do love seeing this proceeding. This has haunted me and I actually still 
have a JIRA item pending for this work - although that's one of those 
items which might never get done - I just couldn't delete it after 
putting quite bit of effort to this :(

I hope that effort benefits someone!

Just one comment:

On 16/08/2025 22:19, Andreas Kemnade,,, wrote:
> From: Matti Vaittinen <mazziesaccount@gmail.com>
> 
> Add core support for ROHM BD718(15/28/78) PMIC's charger blocks.
> 
> Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> ---
>   drivers/mfd/rohm-bd71828.c       | 44 +++++++++++++++++++++------
>   include/linux/mfd/rohm-bd71828.h | 65 ++++++++++++++++++++++++++++++++++++++++
>   include/linux/mfd/rohm-generic.h |  2 ++
>   3 files changed, 102 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/mfd/rohm-bd71828.c b/drivers/mfd/rohm-bd71828.c
> index a14b7aa69c3c61d51f2aeeae9afdf222310d63e3..84a64c3b9c9f52e663855c89ed78ede9a7c21f55 100644
> --- a/drivers/mfd/rohm-bd71828.c
> +++ b/drivers/mfd/rohm-bd71828.c
> @@ -45,8 +45,8 @@ static const struct resource bd71828_rtc_irqs[] = {
>   
>   static const struct resource bd71815_power_irqs[] = {
>   	DEFINE_RES_IRQ_NAMED(BD71815_INT_DCIN_RMV, "bd71815-dcin-rmv"),
> -	DEFINE_RES_IRQ_NAMED(BD71815_INT_CLPS_OUT, "bd71815-clps-out"),
> -	DEFINE_RES_IRQ_NAMED(BD71815_INT_CLPS_IN, "bd71815-clps-in"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_CLPS_OUT, "bd71815-dcin-clps-out"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_CLPS_IN, "bd71815-dcin-clps-in"),
>   	DEFINE_RES_IRQ_NAMED(BD71815_INT_DCIN_OVP_RES, "bd71815-dcin-ovp-res"),
>   	DEFINE_RES_IRQ_NAMED(BD71815_INT_DCIN_OVP_DET, "bd71815-dcin-ovp-det"),
>   	DEFINE_RES_IRQ_NAMED(BD71815_INT_DCIN_MON_RES, "bd71815-dcin-mon-res"),
> @@ -56,7 +56,7 @@ static const struct resource bd71815_power_irqs[] = {
>   	DEFINE_RES_IRQ_NAMED(BD71815_INT_VSYS_LOW_RES, "bd71815-vsys-low-res"),
>   	DEFINE_RES_IRQ_NAMED(BD71815_INT_VSYS_LOW_DET, "bd71815-vsys-low-det"),
>   	DEFINE_RES_IRQ_NAMED(BD71815_INT_VSYS_MON_RES, "bd71815-vsys-mon-res"),
> -	DEFINE_RES_IRQ_NAMED(BD71815_INT_VSYS_MON_RES, "bd71815-vsys-mon-det"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_VSYS_MON_DET, "bd71815-vsys-mon-det"),
>   	DEFINE_RES_IRQ_NAMED(BD71815_INT_CHG_WDG_TEMP, "bd71815-chg-wdg-temp"),
>   	DEFINE_RES_IRQ_NAMED(BD71815_INT_CHG_WDG_TIME, "bd71815-chg-wdg"),
>   	DEFINE_RES_IRQ_NAMED(BD71815_INT_CHG_RECHARGE_RES, "bd71815-rechg-res"),
> @@ -87,10 +87,10 @@ static const struct resource bd71815_power_irqs[] = {
>   	DEFINE_RES_IRQ_NAMED(BD71815_INT_BAT_OVER_CURR_2_DET, "bd71815-bat-oc2-det"),
>   	DEFINE_RES_IRQ_NAMED(BD71815_INT_BAT_OVER_CURR_3_RES, "bd71815-bat-oc3-res"),
>   	DEFINE_RES_IRQ_NAMED(BD71815_INT_BAT_OVER_CURR_3_DET, "bd71815-bat-oc3-det"),
> -	DEFINE_RES_IRQ_NAMED(BD71815_INT_TEMP_BAT_LOW_RES, "bd71815-bat-low-res"),
> -	DEFINE_RES_IRQ_NAMED(BD71815_INT_TEMP_BAT_LOW_DET, "bd71815-bat-low-det"),
> -	DEFINE_RES_IRQ_NAMED(BD71815_INT_TEMP_BAT_HI_RES, "bd71815-bat-hi-res"),
> -	DEFINE_RES_IRQ_NAMED(BD71815_INT_TEMP_BAT_HI_DET, "bd71815-bat-hi-det"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_TEMP_BAT_LOW_RES, "bd71815-temp-bat-low-res"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_TEMP_BAT_LOW_DET, "bd71815-temp-bat-low-det"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_TEMP_BAT_HI_RES, "bd71815-temp-bat-hi-res"),
> +	DEFINE_RES_IRQ_NAMED(BD71815_INT_TEMP_BAT_HI_DET, "bd71815-temp-bat-hi-det"),
>   };
>   
>   static const struct mfd_cell bd71815_mfd_cells[] = {
> @@ -109,7 +109,30 @@ static const struct mfd_cell bd71815_mfd_cells[] = {
>   	},
>   };
>   
> -static const struct mfd_cell bd71828_mfd_cells[] = {
> +static const struct resource bd71828_power_irqs[] = {
> +	DEFINE_RES_IRQ_NAMED(BD71828_INT_CHG_TOPOFF_TO_DONE,
> +			     "bd71828-chg-done"),
> +	DEFINE_RES_IRQ_NAMED(BD71828_INT_DCIN_DET, "bd71828-pwr-dcin-in"),
> +	DEFINE_RES_IRQ_NAMED(BD71828_INT_DCIN_RMV, "bd71828-pwr-dcin-out"),
> +	DEFINE_RES_IRQ_NAMED(BD71828_INT_BAT_LOW_VOLT_RES,
> +			     "bd71828-vbat-normal"),
> +	DEFINE_RES_IRQ_NAMED(BD71828_INT_BAT_LOW_VOLT_DET, "bd71828-vbat-low"),
> +	DEFINE_RES_IRQ_NAMED(BD71828_INT_TEMP_BAT_HI_DET, "bd71828-btemp-hi"),
> +	DEFINE_RES_IRQ_NAMED(BD71828_INT_TEMP_BAT_HI_RES, "bd71828-btemp-cool"),
> +	DEFINE_RES_IRQ_NAMED(BD71828_INT_TEMP_BAT_LOW_DET, "bd71828-btemp-lo"),
> +	DEFINE_RES_IRQ_NAMED(BD71828_INT_TEMP_BAT_LOW_RES,
> +			     "bd71828-btemp-warm"),
> +	DEFINE_RES_IRQ_NAMED(BD71828_INT_TEMP_CHIP_OVER_VF_DET,
> +			     "bd71828-temp-hi"),
> +	DEFINE_RES_IRQ_NAMED(BD71828_INT_TEMP_CHIP_OVER_VF_RES,
> +			     "bd71828-temp-norm"),
> +	DEFINE_RES_IRQ_NAMED(BD71828_INT_TEMP_CHIP_OVER_125_DET,
> +			     "bd71828-temp-125-over"),
> +	DEFINE_RES_IRQ_NAMED(BD71828_INT_TEMP_CHIP_OVER_125_RES,
> +			     "bd71828-temp-125-under"),
> +};
> +
> +static struct mfd_cell bd71828_mfd_cells[] = {
>   	{ .name = "bd71828-pmic", },
>   	{ .name = "bd71828-gpio", },
>   	{ .name = "bd71828-led", .of_compatible = "rohm,bd71828-leds" },
> @@ -118,8 +141,11 @@ static const struct mfd_cell bd71828_mfd_cells[] = {
>   	 * BD70528 clock gate are the register address and mask.
>   	 */
>   	{ .name = "bd71828-clk", },
> -	{ .name = "bd71827-power", },
>   	{
> +		.name = "bd71828-power",
> +		.resources = bd71828_power_irqs,
> +		.num_resources = ARRAY_SIZE(bd71828_power_irqs),
> +	}, {
>   		.name = "bd71828-rtc",
>   		.resources = bd71828_rtc_irqs,
>   		.num_resources = ARRAY_SIZE(bd71828_rtc_irqs),
> diff --git a/include/linux/mfd/rohm-bd71828.h b/include/linux/mfd/rohm-bd71828.h
> index ce786c96404a3dc9d5124ffbbd507df89ca0e5ba..a34991984caa8724e925f1c59de4bcfa543ae411 100644
> --- a/include/linux/mfd/rohm-bd71828.h
> +++ b/include/linux/mfd/rohm-bd71828.h
> @@ -189,6 +189,71 @@ enum {
>   /* Charger/Battey */
>   #define BD71828_REG_CHG_STATE		0x65
>   #define BD71828_REG_CHG_FULL		0xd2
> +#define BD71828_REG_CHG_EN		0x6F
> +#define BD71828_REG_DCIN_STAT		0x68
> +#define BD71828_MASK_DCIN_DET		0x01
> +#define BD71828_REG_VDCIN_U		0x9c
> +#define BD71828_MASK_CHG_EN		0x01
> +#define BD71828_CHG_MASK_DCIN_U		0x0f
> +#define BD71828_REG_BAT_STAT		0x67
> +#define BD71828_REG_BAT_TEMP		0x6c
> +#define BD71828_MASK_BAT_TEMP		0x07
> +#define BD71828_BAT_TEMP_OPEN		0x07
> +#define BD71828_MASK_BAT_DET		0x20
> +#define BD71828_MASK_BAT_DET_DONE	0x10
> +#define BD71828_REG_CHG_STATE		0x65
> +#define BD71828_REG_VBAT_U		0x8c
> +#define BD71828_MASK_VBAT_U		0x0f
> +#define BD71828_REG_VBAT_REX_AVG_U	0x92
> +
> +#define BD71828_REG_OCV_PWRON_U		0x8A
> +
> +#define BD71828_REG_VBAT_MIN_AVG_U	0x8e
> +#define BD71828_REG_VBAT_MIN_AVG_L	0x8f
> +
> +#define BD71828_REG_CC_CNT3		0xb5
> +#define BD71828_REG_CC_CNT2		0xb6
> +#define BD71828_REG_CC_CNT1		0xb7
> +#define BD71828_REG_CC_CNT0		0xb8
> +#define BD71828_REG_CC_CURCD_AVG_U	0xb2
> +#define BD71828_MASK_CC_CURCD_AVG_U	0x3f
> +#define BD71828_MASK_CC_CUR_DIR		0x80
> +#define BD71828_REG_VM_BTMP_U		0xa1
> +#define BD71828_REG_VM_BTMP_L		0xa2
> +#define BD71828_MASK_VM_BTMP_U		0x0f
> +#define BD71828_REG_COULOMB_CTRL	0xc4
> +#define BD71828_REG_COULOMB_CTRL2	0xd2
> +#define BD71828_MASK_REX_CC_CLR		0x01
> +#define BD71828_MASK_FULL_CC_CLR	0x10
> +#define BD71828_REG_CC_CNT_FULL3	0xbd
> +#define BD71828_REG_CC_CNT_CHG3		0xc1
> +
> +#define BD71828_REG_VBAT_INITIAL1_U	0x86
> +#define BD71828_REG_VBAT_INITIAL1_L	0x87
> +
> +#define BD71828_REG_VBAT_INITIAL2_U	0x88
> +#define BD71828_REG_VBAT_INITIAL2_L	0x89
> +
> +#define BD71828_REG_IBAT_U		0xb0
> +#define BD71828_REG_IBAT_L		0xb1
> +
> +#define BD71828_REG_IBAT_AVG_U		0xb2
> +#define BD71828_REG_IBAT_AVG_L		0xb3
> +
> +#define BD71828_REG_VSYS_AVG_U		0x96
> +#define BD71828_REG_VSYS_AVG_L		0x97
> +#define BD71828_REG_VSYS_MIN_AVG_U	0x98
> +#define BD71828_REG_VSYS_MIN_AVG_L	0x99
> +#define BD71828_REG_CHG_SET1		0x75
> +#define BD71828_REG_ALM_VBAT_LIMIT_U	0xaa
> +#define BD71828_REG_BATCAP_MON_LIMIT_U	0xcc
> +#define BD71828_REG_CONF		0x64
> +
> +#define BD71828_REG_DCIN_CLPS		0x71
> +
> +#define BD71828_REG_MEAS_CLEAR		0xaf
> +
> +
>   
>   /* LEDs */
>   #define BD71828_REG_LED_CTRL		0x4A
> diff --git a/include/linux/mfd/rohm-generic.h b/include/linux/mfd/rohm-generic.h
> index 579e8dcfcca41d2680283819684a1014617d0d4b..5e9d0da380ec0fc3245bee998c791a162a34e3fa 100644
> --- a/include/linux/mfd/rohm-generic.h
> +++ b/include/linux/mfd/rohm-generic.h
> @@ -13,9 +13,11 @@ enum rohm_chip_type {
>   	ROHM_CHIP_TYPE_BD9574,
>   	ROHM_CHIP_TYPE_BD9576,
>   	ROHM_CHIP_TYPE_BD71815,
> +	ROHM_CHIP_TYPE_BD71827,

Reading you drop the BD71827 support (which sounds like the right thing 
to do) - do we still need this?

>   	ROHM_CHIP_TYPE_BD71828,
>   	ROHM_CHIP_TYPE_BD71837,
>   	ROHM_CHIP_TYPE_BD71847,
> +	ROHM_CHIP_TYPE_BD71878,
>   	ROHM_CHIP_TYPE_BD96801,
>   	ROHM_CHIP_TYPE_BD96802,
>   	ROHM_CHIP_TYPE_BD96805,
> 

Yours,
	-- Matti

Re: [PATCH 2/2] power: supply: Add bd718(15/28/78) charger driver

[Matti Vaittinen]

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

Re: [PATCH 1/2] mfd: bd71828, bd71815 prepare for power-supply support

[Andreas Kemnade]

Hi Matti,

Am Mon, 18 Aug 2025 08:49:27 +0300
schrieb Matti Vaittinen <mazziesaccount@gmail.com>:

> > diff --git a/include/linux/mfd/rohm-generic.h b/include/linux/mfd/rohm-generic.h
> > index 579e8dcfcca41d2680283819684a1014617d0d4b..5e9d0da380ec0fc3245bee998c791a162a34e3fa 100644
> > --- a/include/linux/mfd/rohm-generic.h
> > +++ b/include/linux/mfd/rohm-generic.h
> > @@ -13,9 +13,11 @@ enum rohm_chip_type {
> >   	ROHM_CHIP_TYPE_BD9574,
> >   	ROHM_CHIP_TYPE_BD9576,
> >   	ROHM_CHIP_TYPE_BD71815,
> > +	ROHM_CHIP_TYPE_BD71827,  
> 
> Reading you drop the BD71827 support (which sounds like the right thing 
> to do) - do we still need this?

we do not. It just flipped through in all that harmless devices.

Regards,
Andreas

Re: [PATCH 2/2] power: supply: Add bd718(15/28/78) charger driver

[Andreas Kemnade]

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

Re: [PATCH 2/2] power: supply: Add bd718(15/28/78) charger driver

[Matti Vaittinen]

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

Re: [PATCH 2/2] power: supply: Add bd718(15/28/78) charger driver

[Matti Vaittinen]

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

Re: [PATCH 2/2] power: supply: Add bd718(15/28/78) charger driver

[Matti Vaittinen]

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");
> 

Re: [PATCH 2/2] power: supply: Add bd718(15/28/78) charger driver

[Andreas Kemnade]

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

Re: [PATCH 2/2] power: supply: Add bd718(15/28/78) charger driver

[Dan Carpenter]

Hi 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

Re: [PATCH 2/2] power: supply: Add bd718(15/28/78) charger driver

[Andreas Kemnade]

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

Re: [PATCH 2/2] power: supply: Add bd718(15/28/78) charger driver

[Matti Vaittinen]

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

Re: [PATCH 2/2] power: supply: Add bd718(15/28/78) charger driver

[Andreas Kemnade]

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