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

[Matti Vaittinen <mazziesaccount@gmail.com>]

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