| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <33880bc8-40af-42f0-b962-cc5214c7b330@gmail.com>
Date: Mon, 18 Aug 2025 13:33:05 +0300
From: Matti Vaittinen <mazziesaccount@...il.com>
To: Andreas Kemnade <andreas@...nade.info>, Lee Jones <lee@...nel.org>,
Sebastian Reichel <sre@...nel.org>
Cc: linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org
Subject: Re: [PATCH 2/2] power: supply: Add bd718(15/28/78) charger driver
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@...nade.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@...il.com>");
> +MODULE_DESCRIPTION("ROHM BD718(15/17/28/78) PMIC Battery Charger driver");
> +MODULE_LICENSE("GPL");
>
Powered by blists - more mailing lists