Re: [PATCH v5 08/16] mfd: bd71828: Support ROHM BD72720
From: Matti Vaittinen
Date: Thu Nov 27 2025 - 02:46:13 EST
On 26/11/2025 16:28, Lee Jones wrote:
On Thu, 20 Nov 2025, Matti Vaittinen wrote:
From: Matti Vaittinen <mazziesaccount@xxxxxxxxx>
The ROHM BD72720 is a power management IC which continues the BD71828
family of PMICs. Similarly to the BD71815 and BD71828, the BD72720
integrates regulators, charger, RTC, clock gate and GPIOs.
The main difference to the earlier PMICs is that the BD72720 has two
different I2C slave addresses. In addition to the registers behind the
'main I2C address', most of the charger (and to some extent LED) control
is done via registers behind a 'secondary I2C slave address', 0x4c.
Signed-off-by: Matti Vaittinen <mazziesaccount@xxxxxxxxx>
---
Revision history:
v2 =>:
- no changes
RFCv1 => v2: (Mostly addressed comments from Lee and Andreas)
- Use stacked regmaps to avoid platform data and the tango with
multiple regmaps in the power-supply driver
- Use regmap_reg_range()
- make it clear bd72720_irq_type_base is an array
- tab-out definitions in the bd72720 header
- minor styling
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@xxxxxxxxxxxx/
There are some new variants being planned. Most notably, the BD73900
should be almost identical to the BD72720 - for everything else except
the charger block.
---
drivers/mfd/Kconfig | 18 +-
drivers/mfd/rohm-bd71828.c | 488 +++++++++++++++++++++++-
include/linux/mfd/rohm-bd72720.h | 634 +++++++++++++++++++++++++++++++
include/linux/mfd/rohm-generic.h | 1 +
4 files changed, 1126 insertions(+), 15 deletions(-)
create mode 100644 include/linux/mfd/rohm-bd72720.h
// snip
diff --git a/drivers/mfd/rohm-bd71828.c b/drivers/mfd/rohm-bd71828.c
index 2a43005b67ee..2e546aa60ffd 100644
--- a/drivers/mfd/rohm-bd71828.c
+++ b/drivers/mfd/rohm-bd71828.c
@@ -2,7 +2,7 @@
//
// Copyright (C) 2019 ROHM Semiconductors
//
-// ROHM BD71828/BD71815 PMIC driver
+// ROHM BD718[15/28/79] and BD72720 PMIC driver
Looks like this header format slipped in.
I would appreciate a follow-up patch to change it to standard C
multi-line format (except the SPDX line).
Sure.
// snip
+static int regmap_write_wrapper(void *context, unsigned int reg, unsigned int val)
+{
+ struct bd72720_regmaps *maps = context;
+
+ if (reg < 0x100)
Define this to something human readable please.
Some kind of PAGE or BOUNDARY. Perhaps something better.
I will use 'BD72720_SECONDARY_I2C_REG_OFFSET'. A tad long, but it's not used on a long lines.
+ return regmap_write(maps->map1_4b, reg, val);
+
+ reg = BD72720_REG_UNWRAP(reg);
+
+ return regmap_write(maps->map2_4c, reg, val);
+}
// snip
+ return (struct regmap *)secondary_i2c;
*shudders* -- that's a hack, right!
/me does some grepping around ...
Shouldn't this be:
return ERR_CAST(secondary_i2c);
I didn't know about the ERR_CAST(). Thanks for going the extra mile and looking
+ }
//snip
diff --git a/include/linux/mfd/rohm-bd72720.h b/include/linux/mfd/rohm-bd72720.h
new file mode 100644
index 000000000000..42fcf8f81b2f
--- /dev/null
+++ b/include/linux/mfd/rohm-bd72720.h
@@ -0,0 +1,634 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright 2024 ROHM Semiconductors.
Still out of date.
Ah. Indeed. Thanks.
// snip+ * Author: Matti Vaittinen <matti.vaittinen@xxxxxxxxxxxxxxxxx>
+ */
+
+
+/* BD72720 interrupts */
+#define BD72720_INT_LONGPUSH_MASK BIT(0)
+#define BD72720_INT_MIDPUSH_MASK BIT(1)
+#define BD72720_INT_SHORTPUSH_MASK BIT(2)
+#define BD72720_INT_PUSH_MASK BIT(3)
+#define BD72720_INT_HALL_DET_MASK BIT(4)
+#define BD72720_INT_HALL_TGL_MASK BIT(5)
+#define BD72720_INT_WDOG_MASK BIT(6)
+#define BD72720_INT_SWRESET_MASK BIT(7)
+#define BD72720_INT_SEQ_DONE_MASK BIT(0)
+#define BD72720_INT_PGFAULT_MASK BIT(4)
+#define BD72720_INT_BUCK1_DVS_MASK BIT(0)
+#define BD72720_INT_BUCK2_DVS_MASK BIT(1)
+#define BD72720_INT_BUCK3_DVS_MASK BIT(2)
+#define BD72720_INT_BUCK4_DVS_MASK BIT(3)
+#define BD72720_INT_BUCK5_DVS_MASK BIT(4)
+#define BD72720_INT_BUCK6_DVS_MASK BIT(5)
+#define BD72720_INT_BUCK7_DVS_MASK BIT(6)
+#define BD72720_INT_BUCK8_DVS_MASK BIT(7)
+#define BD72720_INT_BUCK9_DVS_MASK BIT(0)
+#define BD72720_INT_BUCK10_DVS_MASK BIT(1)
+#define BD72720_INT_LDO1_DVS_MASK BIT(4)
+#define BD72720_INT_LDO2_DVS_MASK BIT(5)
+#define BD72720_INT_LDO3_DVS_MASK BIT(6)
+#define BD72720_INT_LDO4_DVS_MASK BIT(7)
+#define BD72720_INT_VBUS_RMV_MASK BIT(0)
+#define BD72720_INT_VBUS_DET_MASK BIT(1)
+#define BD72720_INT_VBUS_MON_RES_MASK BIT(2)
+#define BD72720_INT_VBUS_MON_DET_MASK BIT(3)
+#define BD72720_INT_VSYS_MON_RES_MASK BIT(0)
+#define BD72720_INT_VSYS_MON_DET_MASK BIT(1)
+#define BD72720_INT_VSYS_UV_RES_MASK BIT(2)
+#define BD72720_INT_VSYS_UV_DET_MASK BIT(3)
+#define BD72720_INT_VSYS_LO_RES_MASK BIT(4)
+#define BD72720_INT_VSYS_LO_DET_MASK BIT(5)
+#define BD72720_INT_VSYS_OV_RES_MASK BIT(6)
+#define BD72720_INT_VSYS_OV_DET_MASK BIT(7)
+#define BD72720_INT_BAT_ILIM_MASK BIT(0)
+#define BD72720_INT_CHG_DONE_MASK BIT(1)
+#define BD72720_INT_EXTEMP_TOUT_MASK BIT(2)
+#define BD72720_INT_CHG_WDT_EXP_MASK BIT(3)
+#define BD72720_INT_BAT_MNT_OUT_MASK BIT(4)
+#define BD72720_INT_BAT_MNT_IN_MASK BIT(5)
+#define BD72720_INT_CHG_TRNS_MASK BIT(7)
+#define BD72720_INT_VBAT_MON_RES_MASK BIT(0)
+#define BD72720_INT_VBAT_MON_DET_MASK BIT(1)
+#define BD72720_INT_VBAT_SHT_RES_MASK BIT(2)
+#define BD72720_INT_VBAT_SHT_DET_MASK BIT(3)
+#define BD72720_INT_VBAT_LO_RES_MASK BIT(4)
+#define BD72720_INT_VBAT_LO_DET_MASK BIT(5)
+#define BD72720_INT_VBAT_OV_RES_MASK BIT(6)
+#define BD72720_INT_VBAT_OV_DET_MASK BIT(7)
+#define BD72720_INT_BAT_RMV_MASK BIT(0)
+#define BD72720_INT_BAT_DET_MASK BIT(1)
+#define BD72720_INT_DBAT_DET_MASK BIT(2)
+#define BD72720_INT_BAT_TEMP_TRNS_MASK BIT(3)
+#define BD72720_INT_LOBTMP_RES_MASK BIT(4)
+#define BD72720_INT_LOBTMP_DET_MASK BIT(5)
+#define BD72720_INT_OVBTMP_RES_MASK BIT(6)
+#define BD72720_INT_OVBTMP_DET_MASK BIT(7)
+#define BD72720_INT_OCUR1_RES_MASK BIT(0)
+#define BD72720_INT_OCUR1_DET_MASK BIT(1)
+#define BD72720_INT_OCUR2_RES_MASK BIT(2)
+#define BD72720_INT_OCUR2_DET_MASK BIT(3)
+#define BD72720_INT_OCUR3_RES_MASK BIT(4)
+#define BD72720_INT_OCUR3_DET_MASK BIT(5)
+#define BD72720_INT_CC_MON1_DET_MASK BIT(0)
+#define BD72720_INT_CC_MON2_DET_MASK BIT(1)
+#define BD72720_INT_CC_MON3_DET_MASK BIT(2)
+#define BD72720_INT_GPIO1_IN_MASK BIT(4)
+#define BD72720_INT_GPIO2_IN_MASK BIT(5)
+#define BD72720_INT_VF125_RES_MASK BIT(0)
+#define BD72720_INT_VF125_DET_MASK BIT(1)
+#define BD72720_INT_VF_RES_MASK BIT(2)
+#define BD72720_INT_VF_DET_MASK BIT(3)
+#define BD72720_INT_RTC0_MASK BIT(4)
+#define BD72720_INT_RTC1_MASK BIT(5)
+#define BD72720_INT_RTC2_MASK BIT(6)
I'd be able to sleep better if these all lined up.
Hm. I think they are when this is applied?
Thanks for the review! I agree with all the comments I didn't comment on. I'll prepare v6 fixing these :)
Yours,
-- Matti
---
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~