[PATCH RESEND v6 16/17] power: supply: bd71828-power: Support ROHM BD72720

Matti Vaittinen posted 17 patches 1 month, 3 weeks ago
[PATCH RESEND v6 16/17] power: supply: bd71828-power: Support ROHM BD72720
Posted by Matti Vaittinen 1 month, 3 weeks ago
From: Matti Vaittinen <mazziesaccount@gmail.com>

The ROHM BD72720 is a power management IC with a charger and coulomb
counter block which is closely related to the charger / coulomb counter
found from the BD71815, BD71828, BD71879 which are all supported by the
bd71828-power driver. Due to the similarities it makes sense to support
also the BD72720 with the same driver.

Add basic support for the charger logic on ROHM BD72720.

Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>

---
Revision history:
 v2 => :
 - No changes

 RFCv1 => v2:
 - Support using 9-bit register addresses (offset of 0x100) with the
   BD72720
 - Simplify probe and IC data as we don't need two regmaps
 - Drop two BD72720 specific functions as we no longer need different
   regmap for it.

Note: This patch depends on the series: "power: supply: add charger for
BD71828" by Andreas:
https://lore.kernel.org/all/20250918-bd71828-charger-v5-0-851164839c28@kemnade.info/

NOTE: Fuel-gauging is not supported. You can find an unmaintained
downstream reference-driver with a fuel-gauge example from:
https://github.com/RohmSemiconductor/Linux-Kernel-PMIC-Drivers/releases/tag/bd72720-reference-driver-v1
---
 drivers/power/supply/bd71828-power.c | 134 +++++++++++++++++++++++----
 1 file changed, 116 insertions(+), 18 deletions(-)

diff --git a/drivers/power/supply/bd71828-power.c b/drivers/power/supply/bd71828-power.c
index ce73c0f48397..438e220a9cb7 100644
--- a/drivers/power/supply/bd71828-power.c
+++ b/drivers/power/supply/bd71828-power.c
@@ -5,6 +5,7 @@
 #include <linux/kernel.h>
 #include <linux/mfd/rohm-bd71815.h>
 #include <linux/mfd/rohm-bd71828.h>
+#include <linux/mfd/rohm-bd72720.h>
 #include <linux/module.h>
 #include <linux/mod_devicetable.h>
 #include <linux/platform_device.h>
@@ -51,12 +52,14 @@ struct pwr_regs {
 	unsigned int chg_state;
 	unsigned int bat_temp;
 	unsigned int dcin_stat;
+	unsigned int dcin_online_mask;
 	unsigned int dcin_collapse_limit;
 	unsigned int chg_set1;
 	unsigned int chg_en;
 	unsigned int vbat_alm_limit_u;
 	unsigned int conf;
 	unsigned int vdcin;
+	unsigned int vdcin_himask;
 };
 
 static const struct pwr_regs pwr_regs_bd71828 = {
@@ -67,12 +70,14 @@ static const struct pwr_regs pwr_regs_bd71828 = {
 	.chg_state = BD71828_REG_CHG_STATE,
 	.bat_temp = BD71828_REG_BAT_TEMP,
 	.dcin_stat = BD71828_REG_DCIN_STAT,
+	.dcin_online_mask = BD7182x_MASK_DCIN_DET,
 	.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,
 	.conf = BD71828_REG_CONF,
 	.vdcin = BD71828_REG_VDCIN_U,
+	.vdcin_himask = BD7182x_MASK_VDCIN_U,
 };
 
 static const struct pwr_regs pwr_regs_bd71815 = {
@@ -85,6 +90,7 @@ static const struct pwr_regs pwr_regs_bd71815 = {
 	.chg_state = BD71815_REG_CHG_STATE,
 	.bat_temp = BD71815_REG_BAT_TEMP,
 	.dcin_stat = BD71815_REG_DCIN_STAT,
+	.dcin_online_mask = BD7182x_MASK_DCIN_DET,
 	.dcin_collapse_limit = BD71815_REG_DCIN_CLPS,
 	.chg_set1 = BD71815_REG_CHG_SET1,
 	.chg_en   = BD71815_REG_CHG_SET1,
@@ -92,6 +98,31 @@ static const struct pwr_regs pwr_regs_bd71815 = {
 	.conf = BD71815_REG_CONF,
 
 	.vdcin = BD71815_REG_VM_DCIN_U,
+	.vdcin_himask = BD7182x_MASK_VDCIN_U,
+};
+
+static struct pwr_regs pwr_regs_bd72720 = {
+	.vbat_avg = BD72720_REG_VM_SA_VBAT_U,
+	.ibat = BD72720_REG_CC_CURCD_U,
+	.ibat_avg = BD72720_REG_CC_SA_CURCD_U,
+	.btemp_vth = BD72720_REG_VM_BTMP_U,
+	/*
+	 * Note, state 0x40 IMP_CHK. not documented
+	 * on other variants but was still handled in
+	 * existing code. No memory traces as to why.
+	 */
+	.chg_state = BD72720_REG_CHG_STATE,
+	.bat_temp = BD72720_REG_CHG_BAT_TEMP_STAT,
+	.dcin_stat = BD72720_REG_INT_VBUS_SRC,
+	.dcin_online_mask = BD72720_MASK_DCIN_DET,
+	.dcin_collapse_limit = -1, /* Automatic. Setting not supported */
+	.chg_set1 = BD72720_REG_CHG_SET_1,
+	.chg_en = BD72720_REG_CHG_EN,
+	/* 15mV note in data-sheet */
+	.vbat_alm_limit_u = BD72720_REG_ALM_VBAT_TH_U,
+	.conf = BD72720_REG_CONF, /* o XSTB, only PON. Seprate slave addr */
+	.vdcin = BD72720_REG_VM_VBUS_U, /* 10 bits not 11 as with other ICs */
+	.vdcin_himask = BD72720_MASK_VDCIN_U,
 };
 
 struct bd71828_power {
@@ -298,7 +329,7 @@ static int get_chg_online(struct bd71828_power *pwr, int *chg_online)
 		dev_err(pwr->dev, "Failed to read DCIN status\n");
 		return ret;
 	}
-	*chg_online = ((r & BD7182x_MASK_DCIN_DET) != 0);
+	*chg_online = ((r & pwr->regs->dcin_online_mask) != 0);
 
 	return 0;
 }
@@ -329,8 +360,8 @@ static int bd71828_bat_inserted(struct bd71828_power *pwr)
 	ret = val & BD7182x_MASK_CONF_PON;
 
 	if (ret)
-		regmap_update_bits(pwr->regmap, pwr->regs->conf,
-				   BD7182x_MASK_CONF_PON, 0);
+		if (regmap_update_bits(pwr->regmap, pwr->regs->conf, BD7182x_MASK_CONF_PON, 0))
+			dev_err(pwr->dev, "Failed to write CONF register\n");
 
 	return ret;
 }
@@ -358,11 +389,13 @@ 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;
+	if (pwr->regs->dcin_collapse_limit != (unsigned int)-1) {
+		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);
@@ -419,7 +452,7 @@ static int bd71828_charger_get_property(struct power_supply *psy,
 		break;
 	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
 		ret = bd7182x_read16_himask(pwr, pwr->regs->vdcin,
-					    BD7182x_MASK_VDCIN_U, &tmp);
+					    pwr->regs->vdcin_himask, &tmp);
 		if (ret)
 			return ret;
 
@@ -630,6 +663,9 @@ 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(vbus_curr_limit, "VBUS current limited")
+BD_ISR_DUMMY(vsys_ov_res, "VSYS over-voltage cleared")
+BD_ISR_DUMMY(vsys_ov_det, "VSYS over-voltage")
 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")
@@ -878,6 +914,51 @@ static int bd7182x_get_irqs(struct platform_device *pdev,
 		BDIRQ("bd71828-temp-125-over", bd71828_temp_vf125_det),
 		BDIRQ("bd71828-temp-125-under", bd71828_temp_vf125_res),
 	};
+	static const struct bd7182x_irq_res bd72720_irqs[] = {
+		BDIRQ("bd72720_int_vbus_rmv", BD_ISR_NAME(dcin_removed)),
+		BDIRQ("bd72720_int_vbus_det", bd7182x_dcin_detected),
+		BDIRQ("bd72720_int_vbus_mon_res", BD_ISR_NAME(dcin_mon_res)),
+		BDIRQ("bd72720_int_vbus_mon_det", BD_ISR_NAME(dcin_mon_det)),
+		BDIRQ("bd72720_int_vsys_mon_res", BD_ISR_NAME(vsys_mon_res)),
+		BDIRQ("bd72720_int_vsys_mon_det", BD_ISR_NAME(vsys_mon_det)),
+		BDIRQ("bd72720_int_vsys_uv_res", BD_ISR_NAME(vsys_uv_res)),
+		BDIRQ("bd72720_int_vsys_uv_det", BD_ISR_NAME(vsys_uv_det)),
+		BDIRQ("bd72720_int_vsys_lo_res", BD_ISR_NAME(vsys_low_res)),
+		BDIRQ("bd72720_int_vsys_lo_det", BD_ISR_NAME(vsys_low_det)),
+		BDIRQ("bd72720_int_vsys_ov_res", BD_ISR_NAME(vsys_ov_res)),
+		BDIRQ("bd72720_int_vsys_ov_det", BD_ISR_NAME(vsys_ov_det)),
+		BDIRQ("bd72720_int_bat_ilim", BD_ISR_NAME(vbus_curr_limit)),
+		BDIRQ("bd72720_int_chg_done", bd718x7_chg_done),
+		BDIRQ("bd72720_int_extemp_tout", BD_ISR_NAME(chg_wdg_temp)),
+		BDIRQ("bd72720_int_chg_wdt_exp", BD_ISR_NAME(chg_wdg)),
+		BDIRQ("bd72720_int_bat_mnt_out", BD_ISR_NAME(rechg_res)),
+		BDIRQ("bd72720_int_bat_mnt_in", BD_ISR_NAME(rechg_det)),
+		BDIRQ("bd72720_int_chg_trns", BD_ISR_NAME(chg_state_changed)),
+
+		BDIRQ("bd72720_int_vbat_mon_res", BD_ISR_NAME(bat_mon_res)),
+		BDIRQ("bd72720_int_vbat_mon_det", BD_ISR_NAME(bat_mon)),
+		BDIRQ("bd72720_int_vbat_sht_res", BD_ISR_NAME(bat_short_res)),
+		BDIRQ("bd72720_int_vbat_sht_det", BD_ISR_NAME(bat_short)),
+		BDIRQ("bd72720_int_vbat_lo_res", BD_ISR_NAME(bat_low_res)),
+		BDIRQ("bd72720_int_vbat_lo_det", BD_ISR_NAME(bat_low)),
+		BDIRQ("bd72720_int_vbat_ov_res", BD_ISR_NAME(bat_ov_res)),
+		BDIRQ("bd72720_int_vbat_ov_det", BD_ISR_NAME(bat_ov)),
+		BDIRQ("bd72720_int_bat_rmv", BD_ISR_NAME(bat_removed)),
+		BDIRQ("bd72720_int_bat_det", BD_ISR_NAME(bat_det)),
+		BDIRQ("bd72720_int_dbat_det", BD_ISR_NAME(bat_dead)),
+		BDIRQ("bd72720_int_bat_temp_trns", BD_ISR_NAME(temp_transit)),
+		BDIRQ("bd72720_int_lobtmp_res", BD_ISR_NAME(temp_bat_low_res)),
+		BDIRQ("bd72720_int_lobtmp_det", BD_ISR_NAME(temp_bat_low)),
+		BDIRQ("bd72720_int_ovbtmp_res", BD_ISR_NAME(temp_bat_hi_res)),
+		BDIRQ("bd72720_int_ovbtmp_det", BD_ISR_NAME(temp_bat_hi)),
+		BDIRQ("bd72720_int_ocur1_res", BD_ISR_NAME(bat_oc1_res)),
+		BDIRQ("bd72720_int_ocur1_det", BD_ISR_NAME(bat_oc1)),
+		BDIRQ("bd72720_int_ocur2_res", BD_ISR_NAME(bat_oc2_res)),
+		BDIRQ("bd72720_int_ocur2_det", BD_ISR_NAME(bat_oc2)),
+		BDIRQ("bd72720_int_ocur3_res", BD_ISR_NAME(bat_oc3_res)),
+		BDIRQ("bd72720_int_ocur3_det", BD_ISR_NAME(bat_oc3)),
+		BDIRQ("bd72720_int_cc_mon2_det", BD_ISR_NAME(bat_cc_mon)),
+	};
 	int num_irqs;
 	const struct bd7182x_irq_res *irqs;
 
@@ -890,6 +971,10 @@ static int bd7182x_get_irqs(struct platform_device *pdev,
 		irqs = &bd71815_irqs[0];
 		num_irqs = ARRAY_SIZE(bd71815_irqs);
 		break;
+	case ROHM_CHIP_TYPE_BD72720:
+		irqs = &bd72720_irqs[0];
+		num_irqs = ARRAY_SIZE(bd72720_irqs);
+		break;
 	default:
 		return -EINVAL;
 	}
@@ -958,21 +1043,27 @@ static int bd71828_power_probe(struct platform_device *pdev)
 	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;
+	/*
+	 * The BD72720 MFD device registers two regmaps. Power-supply driver
+	 * uses the "wrap-map", which provides access to both of the I2C slave
+	 * addresses used by the BD72720
+	 */
 	pwr->chip_type = platform_get_device_id(pdev)->driver_data;
+	if (pwr->chip_type != ROHM_CHIP_TYPE_BD72720)
+		pwr->regmap = dev_get_regmap(pdev->dev.parent, NULL);
+	else
+		pwr->regmap = dev_get_regmap(pdev->dev.parent, "wrap-map");
+	if (!pwr->regmap) {
+		dev_err(&pdev->dev, "No parent regmap\n");
+		return -EINVAL;
+	}
+
+	pwr->dev = &pdev->dev;
 
 	switch (pwr->chip_type) {
 	case ROHM_CHIP_TYPE_BD71828:
@@ -985,6 +1076,12 @@ static int bd71828_power_probe(struct platform_device *pdev)
 		pwr->get_temp = bd71815_get_temp;
 		pwr->regs = &pwr_regs_bd71815;
 		break;
+	case ROHM_CHIP_TYPE_BD72720:
+		pwr->bat_inserted = bd71828_bat_inserted;
+		pwr->regs = &pwr_regs_bd72720;
+		pwr->get_temp = bd71828_get_temp;
+		dev_dbg(pwr->dev, "Found ROHM BD72720\n");
+		break;
 	default:
 		dev_err(pwr->dev, "Unknown PMIC\n");
 		return -EINVAL;
@@ -1030,6 +1127,7 @@ static int bd71828_power_probe(struct platform_device *pdev)
 static const struct platform_device_id bd71828_charger_id[] = {
 	{ "bd71815-power", ROHM_CHIP_TYPE_BD71815 },
 	{ "bd71828-power", ROHM_CHIP_TYPE_BD71828 },
+	{ "bd72720-power", ROHM_CHIP_TYPE_BD72720 },
 	{ },
 };
 MODULE_DEVICE_TABLE(platform, bd71828_charger_id);
-- 
2.52.0

Re: [PATCH RESEND v6 16/17] power: supply: bd71828-power: Support ROHM BD72720
Posted by Sebastian Reichel 3 weeks, 6 days ago
Hi,

On Mon, Dec 15, 2025 at 03:21:19PM +0200, Matti Vaittinen wrote:
> From: Matti Vaittinen <mazziesaccount@gmail.com>
> 
> The ROHM BD72720 is a power management IC with a charger and coulomb
> counter block which is closely related to the charger / coulomb counter
> found from the BD71815, BD71828, BD71879 which are all supported by the
> bd71828-power driver. Due to the similarities it makes sense to support
> also the BD72720 with the same driver.
> 
> Add basic support for the charger logic on ROHM BD72720.
> 
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
> 
> ---
> Revision history:
>  v2 => :
>  - No changes
> 
>  RFCv1 => v2:
>  - Support using 9-bit register addresses (offset of 0x100) with the
>    BD72720
>  - Simplify probe and IC data as we don't need two regmaps
>  - Drop two BD72720 specific functions as we no longer need different
>    regmap for it.
> 
> Note: This patch depends on the series: "power: supply: add charger for
> BD71828" by Andreas:
> https://lore.kernel.org/all/20250918-bd71828-charger-v5-0-851164839c28@kemnade.info/

That should be in v6.19?

> NOTE: Fuel-gauging is not supported. You can find an unmaintained
> downstream reference-driver with a fuel-gauge example from:
> https://github.com/RohmSemiconductor/Linux-Kernel-PMIC-Drivers/releases/tag/bd72720-reference-driver-v1
> ---
>  drivers/power/supply/bd71828-power.c | 134 +++++++++++++++++++++++----
>  1 file changed, 116 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/power/supply/bd71828-power.c b/drivers/power/supply/bd71828-power.c
> index ce73c0f48397..438e220a9cb7 100644
> --- a/drivers/power/supply/bd71828-power.c
> +++ b/drivers/power/supply/bd71828-power.c
> @@ -5,6 +5,7 @@
>  #include <linux/kernel.h>
>  #include <linux/mfd/rohm-bd71815.h>
>  #include <linux/mfd/rohm-bd71828.h>
> +#include <linux/mfd/rohm-bd72720.h>
>  #include <linux/module.h>
>  #include <linux/mod_devicetable.h>
>  #include <linux/platform_device.h>
> @@ -51,12 +52,14 @@ struct pwr_regs {
>  	unsigned int chg_state;
>  	unsigned int bat_temp;
>  	unsigned int dcin_stat;
> +	unsigned int dcin_online_mask;
>  	unsigned int dcin_collapse_limit;
>  	unsigned int chg_set1;
>  	unsigned int chg_en;
>  	unsigned int vbat_alm_limit_u;
>  	unsigned int conf;
>  	unsigned int vdcin;
> +	unsigned int vdcin_himask;
>  };
>  
>  static const struct pwr_regs pwr_regs_bd71828 = {
> @@ -67,12 +70,14 @@ static const struct pwr_regs pwr_regs_bd71828 = {
>  	.chg_state = BD71828_REG_CHG_STATE,
>  	.bat_temp = BD71828_REG_BAT_TEMP,
>  	.dcin_stat = BD71828_REG_DCIN_STAT,
> +	.dcin_online_mask = BD7182x_MASK_DCIN_DET,
>  	.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,
>  	.conf = BD71828_REG_CONF,
>  	.vdcin = BD71828_REG_VDCIN_U,
> +	.vdcin_himask = BD7182x_MASK_VDCIN_U,
>  };
>  
>  static const struct pwr_regs pwr_regs_bd71815 = {
> @@ -85,6 +90,7 @@ static const struct pwr_regs pwr_regs_bd71815 = {
>  	.chg_state = BD71815_REG_CHG_STATE,
>  	.bat_temp = BD71815_REG_BAT_TEMP,
>  	.dcin_stat = BD71815_REG_DCIN_STAT,
> +	.dcin_online_mask = BD7182x_MASK_DCIN_DET,
>  	.dcin_collapse_limit = BD71815_REG_DCIN_CLPS,
>  	.chg_set1 = BD71815_REG_CHG_SET1,
>  	.chg_en   = BD71815_REG_CHG_SET1,
> @@ -92,6 +98,31 @@ static const struct pwr_regs pwr_regs_bd71815 = {
>  	.conf = BD71815_REG_CONF,
>  
>  	.vdcin = BD71815_REG_VM_DCIN_U,
> +	.vdcin_himask = BD7182x_MASK_VDCIN_U,
> +};
> +
> +static struct pwr_regs pwr_regs_bd72720 = {
> +	.vbat_avg = BD72720_REG_VM_SA_VBAT_U,
> +	.ibat = BD72720_REG_CC_CURCD_U,
> +	.ibat_avg = BD72720_REG_CC_SA_CURCD_U,
> +	.btemp_vth = BD72720_REG_VM_BTMP_U,
> +	/*
> +	 * Note, state 0x40 IMP_CHK. not documented
> +	 * on other variants but was still handled in
> +	 * existing code. No memory traces as to why.
> +	 */
> +	.chg_state = BD72720_REG_CHG_STATE,
> +	.bat_temp = BD72720_REG_CHG_BAT_TEMP_STAT,
> +	.dcin_stat = BD72720_REG_INT_VBUS_SRC,
> +	.dcin_online_mask = BD72720_MASK_DCIN_DET,
> +	.dcin_collapse_limit = -1, /* Automatic. Setting not supported */
> +	.chg_set1 = BD72720_REG_CHG_SET_1,
> +	.chg_en = BD72720_REG_CHG_EN,
> +	/* 15mV note in data-sheet */
> +	.vbat_alm_limit_u = BD72720_REG_ALM_VBAT_TH_U,
> +	.conf = BD72720_REG_CONF, /* o XSTB, only PON. Seprate slave addr */
> +	.vdcin = BD72720_REG_VM_VBUS_U, /* 10 bits not 11 as with other ICs */
> +	.vdcin_himask = BD72720_MASK_VDCIN_U,
>  };
>  
>  struct bd71828_power {
> @@ -298,7 +329,7 @@ static int get_chg_online(struct bd71828_power *pwr, int *chg_online)
>  		dev_err(pwr->dev, "Failed to read DCIN status\n");
>  		return ret;
>  	}
> -	*chg_online = ((r & BD7182x_MASK_DCIN_DET) != 0);
> +	*chg_online = ((r & pwr->regs->dcin_online_mask) != 0);
>  
>  	return 0;
>  }
> @@ -329,8 +360,8 @@ static int bd71828_bat_inserted(struct bd71828_power *pwr)
>  	ret = val & BD7182x_MASK_CONF_PON;
>  
>  	if (ret)
> -		regmap_update_bits(pwr->regmap, pwr->regs->conf,
> -				   BD7182x_MASK_CONF_PON, 0);
> +		if (regmap_update_bits(pwr->regmap, pwr->regs->conf, BD7182x_MASK_CONF_PON, 0))
> +			dev_err(pwr->dev, "Failed to write CONF register\n");
>  
>  	return ret;
>  }
> @@ -358,11 +389,13 @@ 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;
> +	if (pwr->regs->dcin_collapse_limit != (unsigned int)-1) {
> +		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);
> @@ -419,7 +452,7 @@ static int bd71828_charger_get_property(struct power_supply *psy,
>  		break;
>  	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
>  		ret = bd7182x_read16_himask(pwr, pwr->regs->vdcin,
> -					    BD7182x_MASK_VDCIN_U, &tmp);
> +					    pwr->regs->vdcin_himask, &tmp);
>  		if (ret)
>  			return ret;
>  
> @@ -630,6 +663,9 @@ 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(vbus_curr_limit, "VBUS current limited")
> +BD_ISR_DUMMY(vsys_ov_res, "VSYS over-voltage cleared")
> +BD_ISR_DUMMY(vsys_ov_det, "VSYS over-voltage")
>  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")
> @@ -878,6 +914,51 @@ static int bd7182x_get_irqs(struct platform_device *pdev,
>  		BDIRQ("bd71828-temp-125-over", bd71828_temp_vf125_det),
>  		BDIRQ("bd71828-temp-125-under", bd71828_temp_vf125_res),
>  	};
> +	static const struct bd7182x_irq_res bd72720_irqs[] = {
> +		BDIRQ("bd72720_int_vbus_rmv", BD_ISR_NAME(dcin_removed)),
> +		BDIRQ("bd72720_int_vbus_det", bd7182x_dcin_detected),
> +		BDIRQ("bd72720_int_vbus_mon_res", BD_ISR_NAME(dcin_mon_res)),
> +		BDIRQ("bd72720_int_vbus_mon_det", BD_ISR_NAME(dcin_mon_det)),
> +		BDIRQ("bd72720_int_vsys_mon_res", BD_ISR_NAME(vsys_mon_res)),
> +		BDIRQ("bd72720_int_vsys_mon_det", BD_ISR_NAME(vsys_mon_det)),
> +		BDIRQ("bd72720_int_vsys_uv_res", BD_ISR_NAME(vsys_uv_res)),
> +		BDIRQ("bd72720_int_vsys_uv_det", BD_ISR_NAME(vsys_uv_det)),
> +		BDIRQ("bd72720_int_vsys_lo_res", BD_ISR_NAME(vsys_low_res)),
> +		BDIRQ("bd72720_int_vsys_lo_det", BD_ISR_NAME(vsys_low_det)),
> +		BDIRQ("bd72720_int_vsys_ov_res", BD_ISR_NAME(vsys_ov_res)),
> +		BDIRQ("bd72720_int_vsys_ov_det", BD_ISR_NAME(vsys_ov_det)),
> +		BDIRQ("bd72720_int_bat_ilim", BD_ISR_NAME(vbus_curr_limit)),
> +		BDIRQ("bd72720_int_chg_done", bd718x7_chg_done),
> +		BDIRQ("bd72720_int_extemp_tout", BD_ISR_NAME(chg_wdg_temp)),
> +		BDIRQ("bd72720_int_chg_wdt_exp", BD_ISR_NAME(chg_wdg)),
> +		BDIRQ("bd72720_int_bat_mnt_out", BD_ISR_NAME(rechg_res)),
> +		BDIRQ("bd72720_int_bat_mnt_in", BD_ISR_NAME(rechg_det)),
> +		BDIRQ("bd72720_int_chg_trns", BD_ISR_NAME(chg_state_changed)),
> +
> +		BDIRQ("bd72720_int_vbat_mon_res", BD_ISR_NAME(bat_mon_res)),
> +		BDIRQ("bd72720_int_vbat_mon_det", BD_ISR_NAME(bat_mon)),
> +		BDIRQ("bd72720_int_vbat_sht_res", BD_ISR_NAME(bat_short_res)),
> +		BDIRQ("bd72720_int_vbat_sht_det", BD_ISR_NAME(bat_short)),
> +		BDIRQ("bd72720_int_vbat_lo_res", BD_ISR_NAME(bat_low_res)),
> +		BDIRQ("bd72720_int_vbat_lo_det", BD_ISR_NAME(bat_low)),
> +		BDIRQ("bd72720_int_vbat_ov_res", BD_ISR_NAME(bat_ov_res)),
> +		BDIRQ("bd72720_int_vbat_ov_det", BD_ISR_NAME(bat_ov)),
> +		BDIRQ("bd72720_int_bat_rmv", BD_ISR_NAME(bat_removed)),
> +		BDIRQ("bd72720_int_bat_det", BD_ISR_NAME(bat_det)),
> +		BDIRQ("bd72720_int_dbat_det", BD_ISR_NAME(bat_dead)),
> +		BDIRQ("bd72720_int_bat_temp_trns", BD_ISR_NAME(temp_transit)),
> +		BDIRQ("bd72720_int_lobtmp_res", BD_ISR_NAME(temp_bat_low_res)),
> +		BDIRQ("bd72720_int_lobtmp_det", BD_ISR_NAME(temp_bat_low)),
> +		BDIRQ("bd72720_int_ovbtmp_res", BD_ISR_NAME(temp_bat_hi_res)),
> +		BDIRQ("bd72720_int_ovbtmp_det", BD_ISR_NAME(temp_bat_hi)),
> +		BDIRQ("bd72720_int_ocur1_res", BD_ISR_NAME(bat_oc1_res)),
> +		BDIRQ("bd72720_int_ocur1_det", BD_ISR_NAME(bat_oc1)),
> +		BDIRQ("bd72720_int_ocur2_res", BD_ISR_NAME(bat_oc2_res)),
> +		BDIRQ("bd72720_int_ocur2_det", BD_ISR_NAME(bat_oc2)),
> +		BDIRQ("bd72720_int_ocur3_res", BD_ISR_NAME(bat_oc3_res)),
> +		BDIRQ("bd72720_int_ocur3_det", BD_ISR_NAME(bat_oc3)),
> +		BDIRQ("bd72720_int_cc_mon2_det", BD_ISR_NAME(bat_cc_mon)),
> +	};
>  	int num_irqs;
>  	const struct bd7182x_irq_res *irqs;
>  
> @@ -890,6 +971,10 @@ static int bd7182x_get_irqs(struct platform_device *pdev,
>  		irqs = &bd71815_irqs[0];
>  		num_irqs = ARRAY_SIZE(bd71815_irqs);
>  		break;
> +	case ROHM_CHIP_TYPE_BD72720:
> +		irqs = &bd72720_irqs[0];
> +		num_irqs = ARRAY_SIZE(bd72720_irqs);
> +		break;
>  	default:
>  		return -EINVAL;
>  	}
> @@ -958,21 +1043,27 @@ static int bd71828_power_probe(struct platform_device *pdev)
>  	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;
> +	/*
> +	 * The BD72720 MFD device registers two regmaps. Power-supply driver
> +	 * uses the "wrap-map", which provides access to both of the I2C slave
> +	 * addresses used by the BD72720
> +	 */
>  	pwr->chip_type = platform_get_device_id(pdev)->driver_data;
> +	if (pwr->chip_type != ROHM_CHIP_TYPE_BD72720)
> +		pwr->regmap = dev_get_regmap(pdev->dev.parent, NULL);
> +	else
> +		pwr->regmap = dev_get_regmap(pdev->dev.parent, "wrap-map");
> +	if (!pwr->regmap) {
> +		dev_err(&pdev->dev, "No parent regmap\n");
> +		return -EINVAL;
> +	}

return dev_err_probe(&pdev->dev, -EINVAL, "No parent regmap\n");

Otherwise LGTM.

Greetings,

-- Sebastian

> +
> +	pwr->dev = &pdev->dev;
>  
>  	switch (pwr->chip_type) {
>  	case ROHM_CHIP_TYPE_BD71828:
> @@ -985,6 +1076,12 @@ static int bd71828_power_probe(struct platform_device *pdev)
>  		pwr->get_temp = bd71815_get_temp;
>  		pwr->regs = &pwr_regs_bd71815;
>  		break;
> +	case ROHM_CHIP_TYPE_BD72720:
> +		pwr->bat_inserted = bd71828_bat_inserted;
> +		pwr->regs = &pwr_regs_bd72720;
> +		pwr->get_temp = bd71828_get_temp;
> +		dev_dbg(pwr->dev, "Found ROHM BD72720\n");
> +		break;
>  	default:
>  		dev_err(pwr->dev, "Unknown PMIC\n");
>  		return -EINVAL;
> @@ -1030,6 +1127,7 @@ static int bd71828_power_probe(struct platform_device *pdev)
>  static const struct platform_device_id bd71828_charger_id[] = {
>  	{ "bd71815-power", ROHM_CHIP_TYPE_BD71815 },
>  	{ "bd71828-power", ROHM_CHIP_TYPE_BD71828 },
> +	{ "bd72720-power", ROHM_CHIP_TYPE_BD72720 },
>  	{ },
>  };
>  MODULE_DEVICE_TABLE(platform, bd71828_charger_id);
> -- 
> 2.52.0
> 


Re: [PATCH RESEND v6 16/17] power: supply: bd71828-power: Support ROHM BD72720
Posted by Matti Vaittinen 3 weeks, 6 days ago
On 12/01/2026 02:51, Sebastian Reichel wrote:
> Hi,
> 
> On Mon, Dec 15, 2025 at 03:21:19PM +0200, Matti Vaittinen wrote:
>> From: Matti Vaittinen <mazziesaccount@gmail.com>
>>
>> The ROHM BD72720 is a power management IC with a charger and coulomb
>> counter block which is closely related to the charger / coulomb counter
>> found from the BD71815, BD71828, BD71879 which are all supported by the
>> bd71828-power driver. Due to the similarities it makes sense to support
>> also the BD72720 with the same driver.
>>
>> Add basic support for the charger logic on ROHM BD72720.
>>
>> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
>>
>> ---
>> Revision history:
>>   v2 => :
>>   - No changes
>>
>>   RFCv1 => v2:
>>   - Support using 9-bit register addresses (offset of 0x100) with the
>>     BD72720
>>   - Simplify probe and IC data as we don't need two regmaps
>>   - Drop two BD72720 specific functions as we no longer need different
>>     regmap for it.
>>
>> Note: This patch depends on the series: "power: supply: add charger for
>> BD71828" by Andreas:
>> https://lore.kernel.org/all/20250918-bd71828-charger-v5-0-851164839c28@kemnade.info/
> 
> That should be in v6.19?

Ah, right. As Andreas also stated, yes. This is no longer relevant. I 
simply forgot to clean-up the note from this patch.

>> @@ -958,21 +1043,27 @@ static int bd71828_power_probe(struct platform_device *pdev)
>>   	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;
>> +	/*
>> +	 * The BD72720 MFD device registers two regmaps. Power-supply driver
>> +	 * uses the "wrap-map", which provides access to both of the I2C slave
>> +	 * addresses used by the BD72720
>> +	 */
>>   	pwr->chip_type = platform_get_device_id(pdev)->driver_data;
>> +	if (pwr->chip_type != ROHM_CHIP_TYPE_BD72720)
>> +		pwr->regmap = dev_get_regmap(pdev->dev.parent, NULL);
>> +	else
>> +		pwr->regmap = dev_get_regmap(pdev->dev.parent, "wrap-map");
>> +	if (!pwr->regmap) {
>> +		dev_err(&pdev->dev, "No parent regmap\n");
>> +		return -EINVAL;
>> +	}
> 
> return dev_err_probe(&pdev->dev, -EINVAL, "No parent regmap\n");
> 
> Otherwise LGTM.

Thanks Sebastian! I appreciate the review!

This driver uses dev_err_probe() only in cases where the error to be 
returned is not hard-coded. The design dates back to when I was first 
introduced to the dev_err_probe() - and using pattern like:

ret = -EINVAL;
if (ret == -EPROBE_DEFER)
  ...

(which results from calling the dev_err_probe with hard-coded error) 
felt very repulsive to me.

I have since quit resisting the 'use dev_err_probe() for all probe error 
paths' -policy (since Resistance is futile - and because there are other 
benefits besides the deferred probe handling) - but I suppose we should 
clean-up also the other similar cases in this driver (I see at least one 
other occurrence in the chip_type-check below). Is it Ok to merge this 
as is, and do a follow-up patch to clean-up all the occurrences? If yes, 
then this might go "as-is" via MFD, together with the other stuff, right?

I can also re-spin this with the print fixed and:
  - add new patch to fix the other occurrence.
  - meld the fix for existing print in this patch.

Just please let me know your preference.



>>   	switch (pwr->chip_type) {
>>   	case ROHM_CHIP_TYPE_BD71828:
>> @@ -985,6 +1076,12 @@ static int bd71828_power_probe(struct platform_device *pdev)
>>   		pwr->get_temp = bd71815_get_temp;
>>   		pwr->regs = &pwr_regs_bd71815;
>>   		break;
>> +	case ROHM_CHIP_TYPE_BD72720:
>> +		pwr->bat_inserted = bd71828_bat_inserted;
>> +		pwr->regs = &pwr_regs_bd72720;
>> +		pwr->get_temp = bd71828_get_temp;
>> +		dev_dbg(pwr->dev, "Found ROHM BD72720\n");
>> +		break;
>>   	default:
>>   		dev_err(pwr->dev, "Unknown PMIC\n");
>>   		return -EINVAL;

The other occurrence --^

>> @@ -1030,6 +1127,7 @@ static int bd71828_power_probe(struct platform_device *pdev)
>>   static const struct platform_device_id bd71828_charger_id[] = {
>>   	{ "bd71815-power", ROHM_CHIP_TYPE_BD71815 },
>>   	{ "bd71828-power", ROHM_CHIP_TYPE_BD71828 },
>> +	{ "bd72720-power", ROHM_CHIP_TYPE_BD72720 },
>>   	{ },
>>   };
>>   MODULE_DEVICE_TABLE(platform, bd71828_charger_id);
>> -- 
>> 2.52.0

Yours,
	-- Matti


-- 
---
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~
Re: [PATCH RESEND v6 16/17] power: supply: bd71828-power: Support ROHM BD72720
Posted by Andreas Kemnade 3 weeks, 6 days ago
On Mon, 12 Jan 2026 01:51:58 +0100
Sebastian Reichel <sebastian.reichel@collabora.com> wrote:

> Hi,
> 
> On Mon, Dec 15, 2025 at 03:21:19PM +0200, Matti Vaittinen wrote:
> > From: Matti Vaittinen <mazziesaccount@gmail.com>
> > 
> > The ROHM BD72720 is a power management IC with a charger and coulomb
> > counter block which is closely related to the charger / coulomb counter
> > found from the BD71815, BD71828, BD71879 which are all supported by the
> > bd71828-power driver. Due to the similarities it makes sense to support
> > also the BD72720 with the same driver.
> > 
> > Add basic support for the charger logic on ROHM BD72720.
> > 
> > Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
> > 
> > ---
> > Revision history:
> >  v2 => :
> >  - No changes
> > 
> >  RFCv1 => v2:
> >  - Support using 9-bit register addresses (offset of 0x100) with the
> >    BD72720
> >  - Simplify probe and IC data as we don't need two regmaps
> >  - Drop two BD72720 specific functions as we no longer need different
> >    regmap for it.
> > 
> > Note: This patch depends on the series: "power: supply: add charger for
> > BD71828" by Andreas:
> > https://lore.kernel.org/all/20250918-bd71828-charger-v5-0-851164839c28@kemnade.info/  
> 
> That should be in v6.19?
> 
yes, it is.
Just this note survived...

Regards,
Andreas