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

From: Matti Vaittinen
Date: Mon Aug 18 2025 - 06:33:46 EST


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@xxxxxxxxxxxxxxxxx/

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@xxxxxxxxxxxx>
---
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@xxxxxxxxx>");
+MODULE_DESCRIPTION("ROHM BD718(15/17/28/78) PMIC Battery Charger driver");
+MODULE_LICENSE("GPL");