Linux RTC
 help / color / mirror / Atom feed
* [PATCH v2 0/3] Samsung mfd/rtc driver alarm IRQ simplification
From: André Draszik @ 2025-11-20 14:38 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Lee Jones, Alexandre Belloni
  Cc: Peter Griffin, Tudor Ambarus, Will McVicker, Juan Yescas,
	Douglas Anderson, kernel-team, Kaustabh Chakraborty, linux-kernel,
	linux-samsung-soc, linux-rtc, André Draszik

Hi,

With the attached patches the Samsung s5m RTC driver is simplified a
little bit with regards to alarm IRQ acquisition.

The end result is that instead of having a list of IRQ numbers for each
variant (and a BUILD_BUG_ON() to ensure consistency), the RTC driver
queries the 'alarm' platform resource from the parent (mfd cell).

Additionally, we can drop a now-useless field from runtime data,
reducing memory consumption slightly.

The attached patches must be applied in-order as patch 2 without 1 will
fail at runtime, and patch 3 without 2 will fail at build time. I would
expect them all to go via the MFD tree. Alternatively, they could be
applied individually to the respective kernel trees during multiple
kernel release cycles, but that seems a needless complication and
delay.

Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
Changes in v2:
- rebase on top of https://lore.kernel.org/r/20251114-s2mpg10-chained-irq-v1-1-34ddfa49c4cd@linaro.org
- return struct regmap_irq_chip_data * in sec_irq_init() (Lee)
- collect tags
- Link to v1: https://lore.kernel.org/r/20251114-s5m-alarm-v1-0-c9b3bebae65f@linaro.org

---
André Draszik (3):
      mfd: sec: add rtc alarm IRQ as platform device resource
      rtc: s5m: query platform device IRQ resource for alarm IRQ
      mfd: sec: drop now unused struct sec_pmic_dev::irq_data

 drivers/mfd/sec-common.c         | 45 ++++++++++++++++++++--------
 drivers/mfd/sec-core.h           |  2 +-
 drivers/mfd/sec-irq.c            | 63 ++++++++++++++++++----------------------
 drivers/rtc/rtc-s5m.c            | 21 +++++---------
 include/linux/mfd/samsung/core.h |  1 -
 5 files changed, 71 insertions(+), 61 deletions(-)
---
base-commit: 9ad5de6d54f306b2bbf7ceb27e67a60c58a71224
change-id: 20251114-s5m-alarm-3de705ea53ce

Best regards,
-- 
André Draszik <andre.draszik@linaro.org>


^ permalink raw reply

* [PATCH v2 2/3] rtc: s5m: query platform device IRQ resource for alarm IRQ
From: André Draszik @ 2025-11-20 14:38 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Lee Jones, Alexandre Belloni
  Cc: Peter Griffin, Tudor Ambarus, Will McVicker, Juan Yescas,
	Douglas Anderson, kernel-team, Kaustabh Chakraborty, linux-kernel,
	linux-samsung-soc, linux-rtc, André Draszik
In-Reply-To: <20251120-s5m-alarm-v2-0-cc15f0e32161@linaro.org>

The core driver now exposes the alarm IRQ as a resource, so we can drop
the lookup from here to simplify the code and make adding support for
additional variants easier in this driver.

Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
 drivers/rtc/rtc-s5m.c | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/drivers/rtc/rtc-s5m.c b/drivers/rtc/rtc-s5m.c
index a7220b4d0e8dd35786b060e2a4106e2a39fe743f..c6ed5a4ca8a0e4554b1c88c879b01fc384735007 100644
--- a/drivers/rtc/rtc-s5m.c
+++ b/drivers/rtc/rtc-s5m.c
@@ -15,7 +15,6 @@
 #include <linux/rtc.h>
 #include <linux/platform_device.h>
 #include <linux/mfd/samsung/core.h>
-#include <linux/mfd/samsung/irq.h>
 #include <linux/mfd/samsung/rtc.h>
 #include <linux/mfd/samsung/s2mps14.h>
 
@@ -683,22 +682,18 @@ static int s5m_rtc_probe(struct platform_device *pdev)
 		case S2MPS15X:
 			regmap_cfg = &s2mps14_rtc_regmap_config;
 			info->regs = &s2mps15_rtc_regs;
-			alarm_irq = S2MPS14_IRQ_RTCA0;
 			break;
 		case S2MPS14X:
 			regmap_cfg = &s2mps14_rtc_regmap_config;
 			info->regs = &s2mps14_rtc_regs;
-			alarm_irq = S2MPS14_IRQ_RTCA0;
 			break;
 		case S2MPS13X:
 			regmap_cfg = &s2mps14_rtc_regmap_config;
 			info->regs = &s2mps13_rtc_regs;
-			alarm_irq = S2MPS14_IRQ_RTCA0;
 			break;
 		case S5M8767X:
 			regmap_cfg = &s5m_rtc_regmap_config;
 			info->regs = &s5m_rtc_regs;
-			alarm_irq = S5M8767_IRQ_RTCA1;
 			break;
 		default:
 			return dev_err_probe(&pdev->dev, -ENODEV,
@@ -719,7 +714,6 @@ static int s5m_rtc_probe(struct platform_device *pdev)
 					     "Failed to allocate regmap\n");
 	} else if (device_type == S2MPG10) {
 		info->regs = &s2mpg10_rtc_regs;
-		alarm_irq = S2MPG10_IRQ_RTCA0;
 	} else {
 		return dev_err_probe(&pdev->dev, -ENODEV,
 				     "Unsupported device type %d\n",
@@ -730,13 +724,14 @@ static int s5m_rtc_probe(struct platform_device *pdev)
 	info->s5m87xx = s5m87xx;
 	info->device_type = device_type;
 
-	if (s5m87xx->irq_data) {
-		info->irq = regmap_irq_get_virq(s5m87xx->irq_data, alarm_irq);
-		if (info->irq <= 0)
-			return dev_err_probe(&pdev->dev, -EINVAL,
-					     "Failed to get virtual IRQ %d\n",
-					     alarm_irq);
-	}
+	alarm_irq = platform_get_irq_byname_optional(pdev, "alarm");
+	if (alarm_irq > 0)
+		info->irq = alarm_irq;
+	else if (alarm_irq == -ENXIO)
+		info->irq = 0;
+	else
+		return dev_err_probe(&pdev->dev, alarm_irq ? : -EINVAL,
+				     "IRQ 'alarm' not found\n");
 
 	platform_set_drvdata(pdev, info);
 

-- 
2.52.0.rc1.455.g30608eb744-goog


^ permalink raw reply related

* [PATCH v2 1/3] mfd: sec: add rtc alarm IRQ as platform device resource
From: André Draszik @ 2025-11-20 14:38 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Lee Jones, Alexandre Belloni
  Cc: Peter Griffin, Tudor Ambarus, Will McVicker, Juan Yescas,
	Douglas Anderson, kernel-team, Kaustabh Chakraborty, linux-kernel,
	linux-samsung-soc, linux-rtc, André Draszik
In-Reply-To: <20251120-s5m-alarm-v2-0-cc15f0e32161@linaro.org>

By adding the RTC alarm IRQ to the MFD cell as a resource, the child
driver (rtc) can simply query that IRQ, instead of having a lookup
table itself.

This change therefore allows the child driver to be simplified with
regards to determining the alarm IRQ.

Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
 drivers/mfd/sec-common.c | 38 +++++++++++++++++++++++++++++---------
 1 file changed, 29 insertions(+), 9 deletions(-)

diff --git a/drivers/mfd/sec-common.c b/drivers/mfd/sec-common.c
index 42d55e70e34c8d7cd68cddaecc88017e259365b4..77370db52a7ba81234136b29f85892f4b197f429 100644
--- a/drivers/mfd/sec-common.c
+++ b/drivers/mfd/sec-common.c
@@ -23,9 +23,13 @@
 #include <linux/regmap.h>
 #include "sec-core.h"
 
+static const struct resource s5m8767_rtc_resources[] = {
+	DEFINE_RES_IRQ_NAMED(S5M8767_IRQ_RTCA1, "alarm"),
+};
+
 static const struct mfd_cell s5m8767_devs[] = {
 	MFD_CELL_NAME("s5m8767-pmic"),
-	MFD_CELL_NAME("s5m-rtc"),
+	MFD_CELL_RES("s5m-rtc", s5m8767_rtc_resources),
 	MFD_CELL_OF("s5m8767-clk", NULL, NULL, 0, 0, "samsung,s5m8767-clk"),
 };
 
@@ -33,50 +37,66 @@ static const struct mfd_cell s2dos05_devs[] = {
 	MFD_CELL_NAME("s2dos05-regulator"),
 };
 
+static const struct resource s2mpg10_rtc_resources[] = {
+	DEFINE_RES_IRQ_NAMED(S2MPG10_IRQ_RTCA0, "alarm"),
+};
+
 static const struct mfd_cell s2mpg10_devs[] = {
 	MFD_CELL_NAME("s2mpg10-meter"),
 	MFD_CELL_NAME("s2mpg10-regulator"),
-	MFD_CELL_NAME("s2mpg10-rtc"),
+	MFD_CELL_RES("s2mpg10-rtc", s2mpg10_rtc_resources),
 	MFD_CELL_OF("s2mpg10-clk", NULL, NULL, 0, 0, "samsung,s2mpg10-clk"),
 	MFD_CELL_OF("s2mpg10-gpio", NULL, NULL, 0, 0, "samsung,s2mpg10-gpio"),
 };
 
+static const struct resource s2mps11_rtc_resources[] = {
+	DEFINE_RES_IRQ_NAMED(S2MPS11_IRQ_RTCA0, "alarm"),
+};
+
 static const struct mfd_cell s2mps11_devs[] = {
 	MFD_CELL_NAME("s2mps11-regulator"),
-	MFD_CELL_NAME("s2mps14-rtc"),
+	MFD_CELL_RES("s2mps14-rtc", s2mps11_rtc_resources),
 	MFD_CELL_OF("s2mps11-clk", NULL, NULL, 0, 0, "samsung,s2mps11-clk"),
 };
 
+static const struct resource s2mps14_rtc_resources[] = {
+	DEFINE_RES_IRQ_NAMED(S2MPS14_IRQ_RTCA0, "alarm"),
+};
+
 static const struct mfd_cell s2mps13_devs[] = {
 	MFD_CELL_NAME("s2mps13-regulator"),
-	MFD_CELL_NAME("s2mps13-rtc"),
+	MFD_CELL_RES("s2mps13-rtc", s2mps14_rtc_resources),
 	MFD_CELL_OF("s2mps13-clk", NULL, NULL, 0, 0, "samsung,s2mps13-clk"),
 };
 
 static const struct mfd_cell s2mps14_devs[] = {
 	MFD_CELL_NAME("s2mps14-regulator"),
-	MFD_CELL_NAME("s2mps14-rtc"),
+	MFD_CELL_RES("s2mps14-rtc", s2mps14_rtc_resources),
 	MFD_CELL_OF("s2mps14-clk", NULL, NULL, 0, 0, "samsung,s2mps14-clk"),
 };
 
 static const struct mfd_cell s2mps15_devs[] = {
 	MFD_CELL_NAME("s2mps15-regulator"),
-	MFD_CELL_NAME("s2mps15-rtc"),
+	MFD_CELL_RES("s2mps15-rtc", s2mps14_rtc_resources),
 	MFD_CELL_OF("s2mps13-clk", NULL, NULL, 0, 0, "samsung,s2mps13-clk"),
 };
 
 static const struct mfd_cell s2mpa01_devs[] = {
 	MFD_CELL_NAME("s2mpa01-pmic"),
-	MFD_CELL_NAME("s2mps14-rtc"),
+	MFD_CELL_RES("s2mps14-rtc", s2mps14_rtc_resources),
 };
 
 static const struct mfd_cell s2mpu02_devs[] = {
 	MFD_CELL_NAME("s2mpu02-regulator"),
 };
 
+static const struct resource s2mpu05_rtc_resources[] = {
+	DEFINE_RES_IRQ_NAMED(S2MPU05_IRQ_RTCA0, "alarm"),
+};
+
 static const struct mfd_cell s2mpu05_devs[] = {
 	MFD_CELL_NAME("s2mpu05-regulator"),
-	MFD_CELL_NAME("s2mps15-rtc"),
+	MFD_CELL_RES("s2mps15-rtc", s2mpu05_rtc_resources),
 };
 
 static void sec_pmic_dump_rev(struct sec_pmic_dev *sec_pmic)
@@ -220,7 +240,7 @@ int sec_pmic_probe(struct device *dev, int device_type, unsigned int irq,
 				     sec_pmic->device_type);
 	}
 	ret = devm_mfd_add_devices(sec_pmic->dev, -1, sec_devs, num_sec_devs,
-				   NULL, 0, NULL);
+				   NULL, 0, regmap_irq_get_domain(sec_pmic->irq_data));
 	if (ret)
 		return ret;
 

-- 
2.52.0.rc1.455.g30608eb744-goog


^ permalink raw reply related

* [PATCH v2 3/3] mfd: sec: drop now unused struct sec_pmic_dev::irq_data
From: André Draszik @ 2025-11-20 14:38 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Lee Jones, Alexandre Belloni
  Cc: Peter Griffin, Tudor Ambarus, Will McVicker, Juan Yescas,
	Douglas Anderson, kernel-team, Kaustabh Chakraborty, linux-kernel,
	linux-samsung-soc, linux-rtc, André Draszik
In-Reply-To: <20251120-s5m-alarm-v2-0-cc15f0e32161@linaro.org>

This was used only to allow the s5m RTC driver to deal with the alarm
IRQ. That driver now uses a different approach to acquire that IRQ, and
::irq_data doesn't need to be kept around anymore.

Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
 drivers/mfd/sec-common.c         |  9 +++---
 drivers/mfd/sec-core.h           |  2 +-
 drivers/mfd/sec-irq.c            | 63 ++++++++++++++++++----------------------
 include/linux/mfd/samsung/core.h |  1 -
 4 files changed, 35 insertions(+), 40 deletions(-)

diff --git a/drivers/mfd/sec-common.c b/drivers/mfd/sec-common.c
index 77370db52a7ba81234136b29f85892f4b197f429..0021f9ae8484fd0afc2e47c813a953c91fa38546 100644
--- a/drivers/mfd/sec-common.c
+++ b/drivers/mfd/sec-common.c
@@ -163,6 +163,7 @@ sec_pmic_parse_dt_pdata(struct device *dev)
 int sec_pmic_probe(struct device *dev, int device_type, unsigned int irq,
 		   struct regmap *regmap, struct i2c_client *client)
 {
+	struct regmap_irq_chip_data *irq_data;
 	struct sec_platform_data *pdata;
 	const struct mfd_cell *sec_devs;
 	struct sec_pmic_dev *sec_pmic;
@@ -187,9 +188,9 @@ int sec_pmic_probe(struct device *dev, int device_type, unsigned int irq,
 
 	sec_pmic->pdata = pdata;
 
-	ret = sec_irq_init(sec_pmic);
-	if (ret)
-		return ret;
+	irq_data = sec_irq_init(sec_pmic);
+	if (IS_ERR(irq_data))
+		return PTR_ERR(irq_data);
 
 	pm_runtime_set_active(sec_pmic->dev);
 
@@ -240,7 +241,7 @@ int sec_pmic_probe(struct device *dev, int device_type, unsigned int irq,
 				     sec_pmic->device_type);
 	}
 	ret = devm_mfd_add_devices(sec_pmic->dev, -1, sec_devs, num_sec_devs,
-				   NULL, 0, regmap_irq_get_domain(sec_pmic->irq_data));
+				   NULL, 0, regmap_irq_get_domain(irq_data));
 	if (ret)
 		return ret;
 
diff --git a/drivers/mfd/sec-core.h b/drivers/mfd/sec-core.h
index 92c7558ab8b0de44a52e028eeb7998e38358cb4c..8d85c70c232612d1f7e5fb61b2acd25bf03a62e0 100644
--- a/drivers/mfd/sec-core.h
+++ b/drivers/mfd/sec-core.h
@@ -18,6 +18,6 @@ int sec_pmic_probe(struct device *dev, int device_type, unsigned int irq,
 		   struct regmap *regmap, struct i2c_client *client);
 void sec_pmic_shutdown(struct device *dev);
 
-int sec_irq_init(struct sec_pmic_dev *sec_pmic);
+struct regmap_irq_chip_data *sec_irq_init(struct sec_pmic_dev *sec_pmic);
 
 #endif /* __SEC_CORE_INT_H */
diff --git a/drivers/mfd/sec-irq.c b/drivers/mfd/sec-irq.c
index d992e41e716dcdc060421e1db8475523842a12be..96f53c3617da4cb54f650f9b98c0b934b823ceda 100644
--- a/drivers/mfd/sec-irq.c
+++ b/drivers/mfd/sec-irq.c
@@ -268,26 +268,28 @@ static const struct regmap_irq_chip s5m8767_irq_chip = {
 	.ack_base = S5M8767_REG_INT1,
 };
 
-static int s2mpg1x_add_chained_irq_chip(struct device *dev, struct regmap *regmap, int pirq,
-					struct regmap_irq_chip_data *parent,
-					const struct regmap_irq_chip *chip,
-					struct regmap_irq_chip_data **data)
+static struct regmap_irq_chip_data *
+s2mpg1x_add_chained_irq_chip(struct device *dev, struct regmap *regmap, int pirq,
+			     struct regmap_irq_chip_data *parent,
+			     const struct regmap_irq_chip *chip)
 {
+	struct regmap_irq_chip_data *data;
 	int irq, ret;
 
 	irq = regmap_irq_get_virq(parent, pirq);
 	if (irq < 0)
-		return dev_err_probe(dev, irq, "Failed to get parent vIRQ(%d) for chip %s\n", pirq,
-				     chip->name);
+		return dev_err_ptr_probe(dev, irq, "Failed to get parent vIRQ(%d) for chip %s\n",
+					 pirq, chip->name);
 
-	ret = devm_regmap_add_irq_chip(dev, regmap, irq, IRQF_ONESHOT | IRQF_SHARED, 0, chip, data);
+	ret = devm_regmap_add_irq_chip(dev, regmap, irq, IRQF_ONESHOT | IRQF_SHARED, 0, chip,
+				       &data);
 	if (ret)
-		return dev_err_probe(dev, ret, "Failed to add %s IRQ chip\n", chip->name);
+		return dev_err_ptr_probe(dev, ret, "Failed to add %s IRQ chip\n", chip->name);
 
-	return 0;
+	return data;
 }
 
-static int sec_irq_init_s2mpg1x(struct sec_pmic_dev *sec_pmic)
+static struct regmap_irq_chip_data *sec_irq_init_s2mpg1x(struct sec_pmic_dev *sec_pmic)
 {
 	const struct regmap_irq_chip *irq_chip, *chained_irq_chip;
 	struct regmap_irq_chip_data *irq_data;
@@ -302,27 +304,28 @@ static int sec_irq_init_s2mpg1x(struct sec_pmic_dev *sec_pmic)
 		chained_pirq = S2MPG10_COMMON_IRQ_PMIC;
 		break;
 	default:
-		return dev_err_probe(sec_pmic->dev, -EINVAL, "Unsupported device type %d\n",
-				     sec_pmic->device_type);
+		return dev_err_ptr_probe(sec_pmic->dev, -EINVAL, "Unsupported device type %d\n",
+					 sec_pmic->device_type);
 	};
 
 	regmap_common = dev_get_regmap(sec_pmic->dev, "common");
 	if (!regmap_common)
-		return dev_err_probe(sec_pmic->dev, -EINVAL, "No 'common' regmap %d\n",
-				     sec_pmic->device_type);
+		return dev_err_ptr_probe(sec_pmic->dev, -EINVAL, "No 'common' regmap %d\n",
+					 sec_pmic->device_type);
 
 	ret = devm_regmap_add_irq_chip(sec_pmic->dev, regmap_common, sec_pmic->irq, IRQF_ONESHOT, 0,
 				       irq_chip, &irq_data);
 	if (ret)
-		return dev_err_probe(sec_pmic->dev, ret, "Failed to add %s IRQ chip\n",
-				     irq_chip->name);
+		return dev_err_ptr_probe(sec_pmic->dev, ret, "Failed to add %s IRQ chip\n",
+					 irq_chip->name);
 
 	return s2mpg1x_add_chained_irq_chip(sec_pmic->dev, sec_pmic->regmap_pmic, chained_pirq,
-					    irq_data, chained_irq_chip, &sec_pmic->irq_data);
+					    irq_data, chained_irq_chip);
 }
 
-int sec_irq_init(struct sec_pmic_dev *sec_pmic)
+struct regmap_irq_chip_data *sec_irq_init(struct sec_pmic_dev *sec_pmic)
 {
+	struct regmap_irq_chip_data *sec_irq_chip_data;
 	const struct regmap_irq_chip *sec_irq_chip;
 	int ret;
 
@@ -331,7 +334,7 @@ int sec_irq_init(struct sec_pmic_dev *sec_pmic)
 		sec_irq_chip = &s5m8767_irq_chip;
 		break;
 	case S2DOS05:
-		return 0;
+		return NULL;
 	case S2MPA01:
 		sec_irq_chip = &s2mps14_irq_chip;
 		break;
@@ -356,30 +359,22 @@ int sec_irq_init(struct sec_pmic_dev *sec_pmic)
 		sec_irq_chip = &s2mpu05_irq_chip;
 		break;
 	default:
-		return dev_err_probe(sec_pmic->dev, -EINVAL,
-				     "Unsupported device type %d\n",
-				     sec_pmic->device_type);
+		return dev_err_ptr_probe(sec_pmic->dev, -EINVAL, "Unsupported device type %d\n",
+					 sec_pmic->device_type);
 	}
 
 	if (!sec_pmic->irq) {
 		dev_warn(sec_pmic->dev,
 			 "No interrupt specified, no interrupts\n");
-		return 0;
+		return NULL;
 	}
 
 	ret = devm_regmap_add_irq_chip(sec_pmic->dev, sec_pmic->regmap_pmic,
 				       sec_pmic->irq, IRQF_ONESHOT,
-				       0, sec_irq_chip, &sec_pmic->irq_data);
+				       0, sec_irq_chip, &sec_irq_chip_data);
 	if (ret)
-		return dev_err_probe(sec_pmic->dev, ret,
-				     "Failed to add %s IRQ chip\n",
-				     sec_irq_chip->name);
+		return dev_err_ptr_probe(sec_pmic->dev, ret, "Failed to add %s IRQ chip\n",
+					 sec_irq_chip->name);
 
-	/*
-	 * The rtc-s5m driver requests S2MPS14_IRQ_RTCA0 also for S2MPS11
-	 * so the interrupt number must be consistent.
-	 */
-	BUILD_BUG_ON(((enum s2mps14_irq)S2MPS11_IRQ_RTCA0) != S2MPS14_IRQ_RTCA0);
-
-	return 0;
+	return sec_irq_chip_data;
 }
diff --git a/include/linux/mfd/samsung/core.h b/include/linux/mfd/samsung/core.h
index d785e101fe795a5d8f9cccf4ccc4232437e89416..c7c3c8cd8d5f99ef0cc3188e1c3b49031f4750f2 100644
--- a/include/linux/mfd/samsung/core.h
+++ b/include/linux/mfd/samsung/core.h
@@ -69,7 +69,6 @@ struct sec_pmic_dev {
 
 	int device_type;
 	int irq;
-	struct regmap_irq_chip_data *irq_data;
 };
 
 struct sec_platform_data {

-- 
2.52.0.rc1.455.g30608eb744-goog


^ permalink raw reply related

* Re: [PATCH 3/3] mfd: sec: drop now unused struct sec_pmic_dev::irq_data
From: André Draszik @ 2025-11-20 14:38 UTC (permalink / raw)
  To: Lee Jones
  Cc: Krzysztof Kozlowski, Alexandre Belloni, Peter Griffin,
	Tudor Ambarus, Will McVicker, Juan Yescas, Douglas Anderson,
	kernel-team, Kaustabh Chakraborty, linux-kernel,
	linux-samsung-soc, linux-rtc
In-Reply-To: <20251120103553.GZ1949330@google.com>

Hi Lee,

On Thu, 2025-11-20 at 10:35 +0000, Lee Jones wrote:
> On Fri, 14 Nov 2025, André Draszik wrote:
> 
> > This was used only to allow the s5m RTC driver to deal with the alarm
> > IRQ. That driver now uses a different approach to acquire that IRQ, and
> > ::irq_data doesn't need to be kept around anymore.
> > 
> > Signed-off-by: André Draszik <andre.draszik@linaro.org>
> > ---
> >  drivers/mfd/sec-common.c         |  5 +++--
> >  drivers/mfd/sec-core.h           |  2 +-
> >  drivers/mfd/sec-irq.c            | 10 ++--------
> >  include/linux/mfd/samsung/core.h |  1 -
> >  4 files changed, 6 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/mfd/sec-common.c b/drivers/mfd/sec-common.c
> > index 77370db52a7ba81234136b29f85892f4b197f429..794c4e5398e7dd1a816aff9a6559a6c19fec75a5 100644
> > --- a/drivers/mfd/sec-common.c
> > +++ b/drivers/mfd/sec-common.c
> > @@ -163,6 +163,7 @@ sec_pmic_parse_dt_pdata(struct device *dev)
> >  int sec_pmic_probe(struct device *dev, int device_type, unsigned int irq,
> >  		   struct regmap *regmap, struct i2c_client *client)
> >  {
> > +	struct regmap_irq_chip_data *irq_data;
> >  	struct sec_platform_data *pdata;
> >  	const struct mfd_cell *sec_devs;
> >  	struct sec_pmic_dev *sec_pmic;
> > @@ -187,7 +188,7 @@ int sec_pmic_probe(struct device *dev, int device_type, unsigned int irq,
> >  
> >  	sec_pmic->pdata = pdata;
> >  
> > -	ret = sec_irq_init(sec_pmic);
> > +	ret = sec_irq_init(sec_pmic, &irq_data);
> >  	if (ret)
> >  		return ret;
> >  
> > @@ -240,7 +241,7 @@ int sec_pmic_probe(struct device *dev, int device_type, unsigned int irq,
> >  				     sec_pmic->device_type);
> >  	}
> >  	ret = devm_mfd_add_devices(sec_pmic->dev, -1, sec_devs, num_sec_devs,
> > -				   NULL, 0, regmap_irq_get_domain(sec_pmic->irq_data));
> > +				   NULL, 0, regmap_irq_get_domain(irq_data));
> >  	if (ret)
> >  		return ret;
> >  
> > diff --git a/drivers/mfd/sec-core.h b/drivers/mfd/sec-core.h
> > index 92c7558ab8b0de44a52e028eeb7998e38358cb4c..c639180ea686f4308af3f872cb1d2209d201b2e7 100644
> > --- a/drivers/mfd/sec-core.h
> > +++ b/drivers/mfd/sec-core.h
> > @@ -18,6 +18,6 @@ int sec_pmic_probe(struct device *dev, int device_type, unsigned int irq,
> >  		   struct regmap *regmap, struct i2c_client *client);
> >  void sec_pmic_shutdown(struct device *dev);
> >  
> > -int sec_irq_init(struct sec_pmic_dev *sec_pmic);
> > +int sec_irq_init(struct sec_pmic_dev *sec_pmic, struct regmap_irq_chip_data **irq_data);
> >  
> >  #endif /* __SEC_CORE_INT_H */
> > diff --git a/drivers/mfd/sec-irq.c b/drivers/mfd/sec-irq.c
> > index c5c80b1ba104e6c5a55b442d2f10a8554201a961..05d4cc350a351d994e00ba08f5ce966d0d5c6a0b 100644
> > --- a/drivers/mfd/sec-irq.c
> > +++ b/drivers/mfd/sec-irq.c
> > @@ -253,7 +253,7 @@ static const struct regmap_irq_chip s5m8767_irq_chip = {
> >  	.ack_base = S5M8767_REG_INT1,
> >  };
> >  
> > -int sec_irq_init(struct sec_pmic_dev *sec_pmic)
> > +int sec_irq_init(struct sec_pmic_dev *sec_pmic, struct regmap_irq_chip_data **irq_data)
> 
> Instead of passing around pointers to pointers, why not return irq_data
> or NULL?

That was mainly to keep change smaller - I've updated the code as per
your suggestion in v2:
https://lore.kernel.org/r/20251120-s5m-alarm-v2-0-cc15f0e32161@linaro.org

Cheers,
Andre'

^ permalink raw reply

* Re: [PATCH v5 04/16] dt-bindings: battery: Voltage drop properties
From: Rob Herring (Arm) @ 2025-11-20 16:11 UTC (permalink / raw)
  To: Matti Vaittinen
  Cc: Pavel Machek, Alexandre Belloni, Linus Walleij, devicetree,
	Liam Girdwood, Sebastian Reichel, Mark Brown, linux-gpio,
	Andreas Kemnade, Michael Turquette, linux-pm, Krzysztof Kozlowski,
	linux-kernel, Stephen Boyd, Matti Vaittinen, linux-clk,
	Conor Dooley, Lee Jones, linux-rtc, linux-leds, Matti Vaittinen,
	Bartosz Golaszewski
In-Reply-To: <93768cba6688714756fca49cc57d46a111885863.1763625920.git.mazziesaccount@gmail.com>


On Thu, 20 Nov 2025 10:20:24 +0200, Matti Vaittinen wrote:
> From: Matti Vaittinen <mazziesaccount@gmail.com>
> 
> ROHM has developed a so called "zero-correction" -algorithm to improve
> the fuel-gauging accuracy close to the point where battery is depleted.
> This relies on battery specific "VDR" (voltage drop rate) tables, which
> are measured from the battery, and which describe the voltage drop rate.
> More thorough explanation about the "zero correction" and "VDR"
> parameters is here:
> https://lore.kernel.org/all/676253b9-ff69-7891-1f26-a8b5bb5a421b@fi.rohmeurope.com/
> 
> Document the VDR zero-correction specific battery properties used by the
> BD71815, BD71828, BD72720 and some other ROHM chargers. (Note, charger
> drivers aren't upstream yet).
> 
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
> 
> ---
> 
> Revision history:
>  v4 => v5:
>  - Move volt-drop parameters from rohm,vdr-battry,yaml to the
>    battery.yaml
>  - drop rohm, -prefix from volt-drop-* properties
>  - Drop the rohm,vdr-battry,yaml
>  - Add comment clarifying what the rohm,volt-drop-* properties are for
>    because this may no longer be obvious as they were moved to common
>    battery.yaml
>  - Drop Linus Walleij's rb-tag because the concept was changed
> 
>  v3 => v4:
>  - No changes
> 
>  v2 => v3:
>  - Constrain VDR threshold voltage to 48V
>  - Use standard '-bp' -suffix for the rohm,volt-drop-soc
> 
>  RFCv1 => v2:
>  - Add units to rohm,volt-drop-soc (tenths of %)
>  - Give real temperatures matching the VDR tables, instead of vague
>    'high', 'normal', 'low', 'very low'. (Add table of temperatures and
>    use number matching the right temperature index in the VDR table name).
>  - Fix typoed 'algorithm' in commit message.
> 
> The parameters are describing the battery voltage drop rates - so they
> are properties of the battery, not the charger. Thus they do not belong
> in the charger node.
> 
> The right place for them is the battery node, which is described by the
> generic "battery.yaml". There were some discussion whether these
> properties should be in their own file, or if they should be added to
> battery.yaml. Discussion can be found from:
> https://lore.kernel.org/all/52b99bf7-bfea-4cee-aa57-4c13e87eaa0d@gmail.com/
> This patch implements the volt-drop properties as generic (not vemdor
> specific) properties in the battery.yaml. It's worth noting that these
> properties are:
> 
>   - Meaningful only for those charger drivers which have the VDR
>     algorithm implemented. (And even though the algorithm is not charger
>     specific, AFAICS, it is currently only used by some ROHM PMIC
>     drivers).
>   - Technique of measuring the VDR tables for a battery is not widely
>     known. AFAICS, only folks at ROHM are measuring those for some
>     customer products. We do have those tables available for some of the
>     products, like Kobo e-readers though.
> ---
>  .../bindings/power/supply/battery.yaml        | 22 +++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>


^ permalink raw reply

* [PATCH v4 1/5] dt-bindings: rtc: nxp,pcf85363: add timestamp mode config
From: Lakshay Piplani @ 2025-11-21 12:11 UTC (permalink / raw)
  To: alexandre.belloni, linux-rtc, linux-kernel, robh, krzk+dt,
	conor+dt, devicetree, wim, linux, linux-watchdog
  Cc: vikash.bansal, priyanka.jain, shashank.rebbapragada,
	Lakshay Piplani

NXP PCF85263/PCF85363 provides three timestamp registers (TSR1-TSR3)
which latch the current time when a selected event occurs. Add a
vendor specific property, nxp,timestamp-mode, to select the event
source for each register.

Also introduce a new header 'pcf85363-tsr.h' to expose
macros for timestamp mode fields, improving readability
of device tree file.

Signed-off-by: Lakshay Piplani <lakshay.piplani@nxp.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
V3 -> V4:
- Added Reviewed-by tag in commit message (previously only in changelog) 
V2 -> V3:
- No changes in v3
- Added Reviewed-by: Rob Herring <robh@kernel.org>
V1 -> V2:
- Addressed review comments from Rob Herring:
  * use $ref: /schemas/types.yaml#/definitions/uint32-array
  * tuple form with exactly 3 items (TSR1/TSR2/TSR3), per items decimal enums
  * define 'nxp,timestamp-mode' clearly
  * drop watchdog related vendor properties
  * remove watchdog related vendor properties from i2c example

 .../devicetree/bindings/rtc/nxp,pcf85363.yaml | 23 ++++++++++++++-
 include/dt-bindings/rtc/pcf85363-tsr.h        | 28 +++++++++++++++++++
 2 files changed, 50 insertions(+), 1 deletion(-)
 create mode 100644 include/dt-bindings/rtc/pcf85363-tsr.h

diff --git a/Documentation/devicetree/bindings/rtc/nxp,pcf85363.yaml b/Documentation/devicetree/bindings/rtc/nxp,pcf85363.yaml
index 52aa3e2091e9..cf9c155162d6 100644
--- a/Documentation/devicetree/bindings/rtc/nxp,pcf85363.yaml
+++ b/Documentation/devicetree/bindings/rtc/nxp,pcf85363.yaml
@@ -4,7 +4,7 @@
 $id: http://devicetree.org/schemas/rtc/nxp,pcf85363.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: Philips PCF85263/PCF85363 Real Time Clock
+title: NXP PCF85263/PCF85363 Real Time Clock
 
 maintainers:
   - Alexandre Belloni <alexandre.belloni@bootlin.com>
@@ -39,6 +39,24 @@ properties:
   start-year: true
   wakeup-source: true
 
+  nxp,timestamp-mode:
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    items:
+      - enum: [0, 1, 2] # TSR1: NONE, FE, LE
+        description: TSR1 mode
+      - enum: [0, 1, 2, 3, 4, 5] # TSR2: NONE, FB, LB, LV, FE, LE
+        description: TSR2 mode
+      - enum: [0, 1, 2, 3] # TSR3: NONE, FB, LB, LV
+        description: TSR3 mode
+    description: |
+      Defines timestamp modes for TSR1, TSR2, and TSR3.
+      Use macros from <dt-bindings/rtc/pcf85363-tsr.h>.
+
+      Each value corresponds to a mode constant:
+        - TSR1: NONE, FE, LE
+        - TSR2: NONE, FB, LB, LV, FE, LE
+        - TSR3: NONE, FB, LB, LV
+
 required:
   - compatible
   - reg
@@ -47,6 +65,7 @@ additionalProperties: false
 
 examples:
   - |
+    #include <dt-bindings/rtc/pcf85363-tsr.h>
     i2c {
         #address-cells = <1>;
         #size-cells = <0>;
@@ -56,5 +75,7 @@ examples:
             reg = <0x51>;
             #clock-cells = <0>;
             quartz-load-femtofarads = <12500>;
+            wakeup-source;
+            nxp,timestamp-mode = <PCF85363_TSR1_FE PCF85363_TSR2_LB PCF85363_TSR3_LV>;
         };
     };
diff --git a/include/dt-bindings/rtc/pcf85363-tsr.h b/include/dt-bindings/rtc/pcf85363-tsr.h
new file mode 100644
index 000000000000..1fb5b9b3601e
--- /dev/null
+++ b/include/dt-bindings/rtc/pcf85363-tsr.h
@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
+/*
+ * Copyright 2025 NXP
+ */
+
+#ifndef _DT_BINDINGS_RTC_PCF85363_TSR_H
+#define _DT_BINDINGS_RTC_PCF85363_TSR_H
+
+/* TSR1 modes */
+#define PCF85363_TSR1_NONE 0x00
+#define PCF85363_TSR1_FE 0x01
+#define PCF85363_TSR1_LE 0x02
+
+/* TSR2 modes */
+#define PCF85363_TSR2_NONE 0x00
+#define PCF85363_TSR2_FB 0x01
+#define PCF85363_TSR2_LB 0x02
+#define PCF85363_TSR2_LV 0x03
+#define PCF85363_TSR2_FE 0x04
+#define PCF85363_TSR2_LE 0x05
+
+/* TSR3 modes */
+#define PCF85363_TSR3_NONE 0x00
+#define PCF85363_TSR3_FB 0x01
+#define PCF85363_TSR3_LB 0x02
+#define PCF85363_TSR3_LV 0x03
+
+#endif /* _DT_BINDINGS_RTC_PCF85363_TSR_H */
-- 
2.25.1


^ permalink raw reply related

* [PATCH v4 2/5] rtc: pcf85363: support reporting battery switch-over via RTC_VL
From: Lakshay Piplani @ 2025-11-21 12:11 UTC (permalink / raw)
  To: alexandre.belloni, linux-rtc, linux-kernel, robh, krzk+dt,
	conor+dt, devicetree, wim, linux, linux-watchdog
  Cc: vikash.bansal, priyanka.jain, shashank.rebbapragada,
	Lakshay Piplani
In-Reply-To: <20251121121137.3043764-1-lakshay.piplani@nxp.com>

Add battery switch-over reporting for PCF85263/PCF85363 using the standard
RTC_VL_* ioctl interface. When the backup supply takes over, the BSF flag
is exposed to userspace through RTC_VL_READ and can be cleared using
RTC_VL_CLR.

This allows applications to detect loss of main power without relying on
non-standard interfaces.

Signed-off-by: Lakshay Piplani <lakshay.piplani@nxp.com>
---
V3 -> V4:
- No changes in v4.
V2 -> V3:
- Split into separate patches as suggested:
  - Battery switch-over detection.
  - Timestamp recording for TS pin and battery switch-over events.
  - Offset calibration.
  - Watchdog timer (to be reviewed by watchdog maintainers).
- Dropped Alarm2 support
- Switched to rtc_add_group() for sysfs attributes
V1 -> V2:
- Watchdog related changes due to removal of vendor specific properties
  from device tree
  * remove vendor DT knobs (enable/timeout/stepsize/repeat)
  * use watchdog_init_timeout (with 10s default)
  * derive clock_sel from final timeout
  * default, repeat=true (repeat mode)
- Fixed uninitalised warning on 'ret' (reported by kernel test robot)
- Use dev_dbg instead of dev_info for debug related print messages
- Minor cleanup and comments.

 drivers/rtc/rtc-pcf85363.c | 49 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 47 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/rtc-pcf85363.c b/drivers/rtc/rtc-pcf85363.c
index 540042b9eec8..c03d5a65c5f7 100644
--- a/drivers/rtc/rtc-pcf85363.c
+++ b/drivers/rtc/rtc-pcf85363.c
@@ -14,6 +14,7 @@
 #include <linux/err.h>
 #include <linux/errno.h>
 #include <linux/bcd.h>
+#include <linux/device.h>
 #include <linux/of.h>
 #include <linux/regmap.h>
 
@@ -295,23 +296,67 @@ static int pcf85363_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 static irqreturn_t pcf85363_rtc_handle_irq(int irq, void *dev_id)
 {
 	struct pcf85363 *pcf85363 = i2c_get_clientdata(dev_id);
+	bool handled = false;
 	unsigned int flags;
 	int err;
 
 	err = regmap_read(pcf85363->regmap, CTRL_FLAGS, &flags);
+
 	if (err)
 		return IRQ_NONE;
 
+	if (flags) {
+		dev_dbg(&pcf85363->rtc->dev, "IRQ flags: 0x%02x%s%s\n",
+			flags, (flags & FLAGS_A1F) ? " [A1F]" : "",
+			(flags & FLAGS_BSF) ? " [BSF]" : "");
+	}
+
 	if (flags & FLAGS_A1F) {
 		rtc_update_irq(pcf85363->rtc, 1, RTC_IRQF | RTC_AF);
 		regmap_update_bits(pcf85363->regmap, CTRL_FLAGS, FLAGS_A1F, 0);
-		return IRQ_HANDLED;
+		handled = true;
 	}
 
-	return IRQ_NONE;
+	if (flags & FLAGS_BSF) {
+		regmap_update_bits(pcf85363->regmap, CTRL_FLAGS, FLAGS_BSF, 0);
+		handled = true;
+	}
+
+	return handled ? IRQ_HANDLED : IRQ_NONE;
+}
+
+static int pcf85363_rtc_ioctl(struct device *dev,
+			      unsigned int cmd, unsigned long arg)
+{
+	struct pcf85363 *pcf85363 = dev_get_drvdata(dev);
+	unsigned int val;
+	int ret;
+
+	switch (cmd) {
+	case RTC_VL_READ: {
+		u32 status = 0;
+
+		ret = regmap_read(pcf85363->regmap, CTRL_FLAGS, &val);
+
+		if (ret)
+			return ret;
+
+		if (val & FLAGS_BSF)
+			status |= RTC_VL_BACKUP_SWITCH;
+
+		return put_user(status, (u32 __user *)arg);
+	}
+
+	case RTC_VL_CLR:
+		return regmap_update_bits(pcf85363->regmap, CTRL_FLAGS, FLAGS_BSF, 0);
+
+	default:
+		return -ENOIOCTLCMD;
+	}
 }
 
 static const struct rtc_class_ops rtc_ops = {
+	.ioctl  = pcf85363_rtc_ioctl,
 	.read_time	= pcf85363_rtc_read_time,
 	.set_time	= pcf85363_rtc_set_time,
 	.read_alarm	= pcf85363_rtc_read_alarm,
-- 
2.25.1


^ permalink raw reply related

* [PATCH v4 3/5] rtc: pcf85363: add timestamp support with configurable timestamp mode
From: Lakshay Piplani @ 2025-11-21 12:11 UTC (permalink / raw)
  To: alexandre.belloni, linux-rtc, linux-kernel, robh, krzk+dt,
	conor+dt, devicetree, wim, linux, linux-watchdog
  Cc: vikash.bansal, priyanka.jain, shashank.rebbapragada,
	Lakshay Piplani
In-Reply-To: <20251121121137.3043764-1-lakshay.piplani@nxp.com>

Add support for the timestamp capture registers available on PCF85263 and
PCF85363. The registers latch the current time when selected events occur,
such as TS pin activation or battery switch-over.

The capture source can be configured via the nxp,timestamp-mode device
tree property, and latched values are exported through read-only sysfs
attributes.

Additionally:
- Use rtc_add_group() instead of sysfs_create_group() to register the
  timestamp attributes under the RTC class device (/sys/class/rtc/rtcX).
- Perform minor cleanups in the probe function for better readability.

Signed-off-by: Lakshay Piplani <lakshay.piplani@nxp.com>
---
V3 -> V4:
- No changes in v4.
V2 -> V3:
- Split into separate patches as suggested:
  - Battery switch-over detection.
  - Timestamp recording for TS pin and battery switch-over events.
  - Offset calibration.
  - Watchdog timer (to be reviewed by watchdog maintainers).
- Dropped Alarm2 support
- Switched to rtc_add_group() for sysfs attributes
V1 -> V2:
- Watchdog related changes due to removal of vendor specific properties
  from device tree
  * remove vendor DT knobs (enable/timeout/stepsize/repeat)
  * use watchdog_init_timeout (with 10s default)
  * derive clock_sel from final timeout
  * default, repeat=true (repeat mode)
- Fixed uninitalised warning on 'ret' (reported by kernel test robot)
- Use dev_dbg instead of dev_info for debug related print messages
- Minor cleanup and comments.

 drivers/rtc/rtc-pcf85363.c | 209 +++++++++++++++++++++++++++++++------
 1 file changed, 175 insertions(+), 34 deletions(-)

diff --git a/drivers/rtc/rtc-pcf85363.c b/drivers/rtc/rtc-pcf85363.c
index c03d5a65c5f7..e10e58f69012 100644
--- a/drivers/rtc/rtc-pcf85363.c
+++ b/drivers/rtc/rtc-pcf85363.c
@@ -16,6 +16,7 @@
 #include <linux/bcd.h>
 #include <linux/device.h>
 #include <linux/of.h>
+#include <linux/rtc.h>
 #include <linux/regmap.h>
 
 /*
@@ -101,19 +102,31 @@
 #define PIN_IO_INTA_OUT	2
 #define PIN_IO_INTA_HIZ	3
 
+#define PIN_IO_TSPM     GENMASK(3, 2)
+#define PIN_IO_TSIM     BIT(4)
+
 #define OSC_CAP_SEL	GENMASK(1, 0)
 #define OSC_CAP_6000	0x01
 #define OSC_CAP_12500	0x02
 
 #define STOP_EN_STOP	BIT(0)
+#define RTCM_BIT        BIT(4)
 
 #define RESET_CPR	0xa4
 
 #define NVRAM_SIZE	0x40
 
+#define TSR1_MASK       0x03
+#define TSR2_MASK       0x07
+#define TSR3_MASK       0x03
+#define TSR1_SHIFT      0
+#define TSR2_SHIFT      2
+#define TSR3_SHIFT      6
+
 struct pcf85363 {
 	struct rtc_device	*rtc;
 	struct regmap		*regmap;
+	u8 ts_valid_flags;
 };
 
 struct pcf85x63_config {
@@ -306,8 +319,11 @@ static irqreturn_t pcf85363_rtc_handle_irq(int irq, void *dev_id)
 		return IRQ_NONE;
 
 	if (flags) {
-		dev_dbg(&pcf85363->rtc->dev, "IRQ flags: 0x%02x%s%s\n",
+		dev_dbg(&pcf85363->rtc->dev, "IRQ flags: 0x%02x%s%s%s%s%s\n",
 			flags, (flags & FLAGS_A1F) ? " [A1F]" : "",
+			(flags & FLAGS_TSR1F) ? " [TSR1F]" : "",
+			(flags & FLAGS_TSR2F) ? " [TSR2F]" : "",
+			(flags & FLAGS_TSR3F) ? " [TSR3F]" : "",
 			(flags & FLAGS_BSF) ? " [BSF]" : "");
 	}
 
@@ -317,6 +333,24 @@ static irqreturn_t pcf85363_rtc_handle_irq(int irq, void *dev_id)
 		handled = true;
 	}
 
+	if (flags & FLAGS_TSR1F) {
+		regmap_update_bits(pcf85363->regmap, CTRL_FLAGS, FLAGS_TSR1F, 0);
+		pcf85363->ts_valid_flags |= FLAGS_TSR1F;
+		handled = true;
+	}
+
+	if (flags & FLAGS_TSR2F) {
+		regmap_update_bits(pcf85363->regmap, CTRL_FLAGS, FLAGS_TSR2F, 0);
+		pcf85363->ts_valid_flags |= FLAGS_TSR2F;
+		handled = true;
+	}
+
+	if (flags & FLAGS_TSR3F) {
+		regmap_update_bits(pcf85363->regmap, CTRL_FLAGS, FLAGS_TSR3F, 0);
+		pcf85363->ts_valid_flags |= FLAGS_TSR3F;
+		handled = true;
+	}
+
 	if (flags & FLAGS_BSF) {
 		regmap_update_bits(pcf85363->regmap, CTRL_FLAGS, FLAGS_BSF, 0);
 		handled = true;
@@ -424,11 +458,94 @@ static const struct pcf85x63_config pcf_85363_config = {
 	.num_nvram = 2
 };
 
+/*
+ * Reads 6 bytes of timestamp data starting at the given base register,
+ * converts them from BCD to binary, and formats the result into a
+ * human-readable string in "YYYY-MM-DD HH:MM:SS" format.
+ */
+static int pcf85363_read_timestamp(struct pcf85363 *pcf85363, u8 base_reg, char *buf)
+{
+	struct rtc_time tm;
+	u8 regs[6];
+	int ret;
+
+	ret = regmap_bulk_read(pcf85363->regmap, base_reg, regs, sizeof(regs));
+
+	if (ret)
+		return ret;
+
+	tm.tm_sec = bcd2bin(regs[0]);
+	tm.tm_min = bcd2bin(regs[1]);
+	tm.tm_hour = bcd2bin(regs[2]);
+	tm.tm_mday = bcd2bin(regs[3]);
+	tm.tm_mon = bcd2bin(regs[4]) - 1;
+	tm.tm_year = bcd2bin(regs[5]) + 100;
+
+	return sysfs_emit(buf, "%04d-%02d-%02d %02d:%02d:%02d\n",
+			  tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday,
+			  tm.tm_hour, tm.tm_min, tm.tm_sec);
+}
+
+/*
+ * Checks whether a specific timestamp flag is set. If so, reads and
+ * returns the formatted timestamp. Otherwise, returns "00-00-00 00:00:00".
+ */
+
+static ssize_t pcf85363_timestamp_show(struct device *dev, char *buf,
+				       u8 timestamp_flag, u8 base_reg)
+{
+	struct pcf85363 *pcf85363 = dev_get_drvdata(dev);
+
+	if (!(pcf85363->ts_valid_flags & timestamp_flag))
+		return sysfs_emit(buf, "00-00-00 00:00:00\n");
+
+	return pcf85363_read_timestamp(pcf85363, base_reg, buf);
+}
+
+static ssize_t timestamp1_show(struct device *dev,
+			       struct device_attribute *attr, char *buf)
+{
+	return pcf85363_timestamp_show(dev, buf, FLAGS_TSR1F, DT_TIMESTAMP1);
+}
+static DEVICE_ATTR_RO(timestamp1);
+
+static ssize_t timestamp2_show(struct device *dev,
+			       struct device_attribute *attr, char *buf)
+{
+	return pcf85363_timestamp_show(dev, buf, FLAGS_TSR2F, DT_TIMESTAMP2);
+}
+static DEVICE_ATTR_RO(timestamp2);
+
+static ssize_t timestamp3_show(struct device *dev,
+			       struct device_attribute *attr, char *buf)
+{
+	return pcf85363_timestamp_show(dev, buf, FLAGS_TSR3F, DT_TIMESTAMP3);
+}
+static DEVICE_ATTR_RO(timestamp3);
+
+static struct attribute *pcf85363_attrs[] = {
+	&dev_attr_timestamp1.attr,
+	&dev_attr_timestamp2.attr,
+	&dev_attr_timestamp3.attr,
+	NULL,
+};
+
+static const struct attribute_group pcf85363_attr_group = {
+	.attrs = pcf85363_attrs,
+};
+
 static int pcf85363_probe(struct i2c_client *client)
 {
-	struct pcf85363 *pcf85363;
 	const struct pcf85x63_config *config = &pcf_85363_config;
 	const void *data = of_device_get_match_data(&client->dev);
+	struct device *dev = &client->dev;
+	struct pcf85363 *pcf85363;
+	int irq_a = client->irq;
+	bool wakeup_source;
+	int ret, i, err;
+	u32 tsr_mode[3];
+	u8 val;
+
 	static struct nvmem_config nvmem_cfg[] = {
 		{
 			.name = "pcf85x63-",
@@ -446,25 +563,43 @@ static int pcf85363_probe(struct i2c_client *client)
 			.reg_write = pcf85363_nvram_write,
 		},
 	};
-	int ret, i, err;
-	bool wakeup_source;
 
 	if (data)
 		config = data;
 
-	pcf85363 = devm_kzalloc(&client->dev, sizeof(struct pcf85363),
-				GFP_KERNEL);
+	pcf85363 = devm_kzalloc(&client->dev, sizeof(*pcf85363), GFP_KERNEL);
 	if (!pcf85363)
 		return -ENOMEM;
 
+	pcf85363->ts_valid_flags = 0;
+
 	pcf85363->regmap = devm_regmap_init_i2c(client, &config->regmap);
-	if (IS_ERR(pcf85363->regmap)) {
-		dev_err(&client->dev, "regmap allocation failed\n");
-		return PTR_ERR(pcf85363->regmap);
-	}
+	if (IS_ERR(pcf85363->regmap))
+		return dev_err_probe(dev, PTR_ERR(pcf85363->regmap), "regmap init failed\n");
 
 	i2c_set_clientdata(client, pcf85363);
 
+	ret = regmap_update_bits(pcf85363->regmap, CTRL_FUNCTION, RTCM_BIT, 0);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to enable RTC mode\n");
+
+	if (!device_property_read_u32_array(dev, "nxp,timestamp-mode", tsr_mode, 3)) {
+		tsr_mode[0] &= TSR1_MASK;
+		tsr_mode[1] &= TSR2_MASK;
+		tsr_mode[2] &= TSR3_MASK;
+
+		val = (tsr_mode[2] << TSR3_SHIFT) |
+		      (tsr_mode[1] << TSR2_SHIFT) |
+		      (tsr_mode[0] << TSR1_SHIFT);
+
+		ret = regmap_write(pcf85363->regmap, DT_TS_MODE, val);
+		if (ret)
+			dev_warn(dev, "Failed to write timestamp mode register\n");
+
+		dev_dbg(dev, "Timestamp mode set: TSR1=0x%x TSR2=0x%x TSR3=0x%x\n",
+			tsr_mode[0], tsr_mode[1], tsr_mode[2]);
+	}
+
 	pcf85363->rtc = devm_rtc_allocate_device(&client->dev);
 	if (IS_ERR(pcf85363->rtc))
 		return PTR_ERR(pcf85363->rtc);
@@ -478,38 +613,44 @@ static int pcf85363_probe(struct i2c_client *client)
 	pcf85363->rtc->range_min = RTC_TIMESTAMP_BEGIN_2000;
 	pcf85363->rtc->range_max = RTC_TIMESTAMP_END_2099;
 
-	wakeup_source = device_property_read_bool(&client->dev,
-						  "wakeup-source");
-	if (client->irq > 0 || wakeup_source) {
-		regmap_write(pcf85363->regmap, CTRL_FLAGS, 0);
-		regmap_update_bits(pcf85363->regmap, CTRL_PIN_IO,
-				   PIN_IO_INTAPM, PIN_IO_INTA_OUT);
-	}
+	wakeup_source = device_property_read_bool(dev, "wakeup-source");
 
-	if (client->irq > 0) {
-		unsigned long irqflags = IRQF_TRIGGER_LOW;
+	ret = regmap_write(pcf85363->regmap, CTRL_FLAGS, 0x00);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to clear CTRL_FLAGS\n");
+
+	if (irq_a > 0) {
+		regmap_update_bits(pcf85363->regmap, CTRL_PIN_IO, PIN_IO_INTAPM, PIN_IO_INTA_OUT);
+		ret = devm_request_threaded_irq(dev, irq_a, NULL,
+						pcf85363_rtc_handle_irq,
+						IRQF_TRIGGER_LOW | IRQF_ONESHOT,
+						"pcf85363-inta", client);
 
-		if (dev_fwnode(&client->dev))
-			irqflags = 0;
-		ret = devm_request_threaded_irq(&client->dev, client->irq,
-						NULL, pcf85363_rtc_handle_irq,
-						irqflags | IRQF_ONESHOT,
-						"pcf85363", client);
 		if (ret) {
-			dev_warn(&client->dev,
-				 "unable to request IRQ, alarms disabled\n");
-			client->irq = 0;
+			dev_err_probe(dev, ret, "INTA IRQ request failed\n");
+			irq_a = 0;
+		} else {
+			regmap_write(pcf85363->regmap, CTRL_INTA_EN, INT_BSIE
+				     | INT_TSRIE);
 		}
 	}
 
-	if (client->irq > 0 || wakeup_source) {
-		device_init_wakeup(&client->dev, true);
-		set_bit(RTC_FEATURE_ALARM, pcf85363->rtc->features);
-	} else {
-		clear_bit(RTC_FEATURE_ALARM, pcf85363->rtc->features);
-	}
+	regmap_update_bits(pcf85363->regmap, CTRL_PIN_IO,
+			   PIN_IO_TSPM | PIN_IO_TSIM,
+			   PIN_IO_TSPM | PIN_IO_TSIM);
+
+	if (irq_a > 0 || wakeup_source)
+		device_init_wakeup(dev, true);
+
+	dev_set_drvdata(&pcf85363->rtc->dev, pcf85363);
+
+	ret = rtc_add_group(pcf85363->rtc, &pcf85363_attr_group);
+	if (ret)
+		return ret;
 
 	ret = devm_rtc_register_device(pcf85363->rtc);
+	if (ret)
+		return dev_err_probe(dev, ret, "RTC registration failed\n");
 
 	for (i = 0; i < config->num_nvram; i++) {
 		nvmem_cfg[i].priv = pcf85363;
-- 
2.25.1


^ permalink raw reply related

* [PATCH v4 4/5] rtc: pcf85363: add oscillator offset calibration support
From: Lakshay Piplani @ 2025-11-21 12:11 UTC (permalink / raw)
  To: alexandre.belloni, linux-rtc, linux-kernel, robh, krzk+dt,
	conor+dt, devicetree, wim, linux, linux-watchdog
  Cc: vikash.bansal, priyanka.jain, shashank.rebbapragada,
	Lakshay Piplani
In-Reply-To: <20251121121137.3043764-1-lakshay.piplani@nxp.com>

Expose the oscillator offset register of PCF85263/PCF85363 through the
rtc_class_ops read_offset and set_offset callbacks, allowing userspace
to apply frequency correction for drift compensation.

The correction mode defaults to normal mode (OFFM = 0), where each step
introduces an offset of approximately 2.170 ppm and corrections occur
every 4 hours.

Signed-off-by: Lakshay Piplani <lakshay.piplani@nxp.com>
---
V3 -> V4:
- No changes in v4.
V2 -> V3:
- Split into separate patches as suggested:
  - Battery switch-over detection.
  - Timestamp recording for TS pin and battery switch-over events.
  - Offset calibration.
  - Watchdog timer (to be reviewed by watchdog maintainers).
- Dropped Alarm2 support
- Switched to rtc_add_group() for sysfs attributes
V1 -> V2:
- Watchdog related changes due to removal of vendor specific properties
  from device tree
  * remove vendor DT knobs (enable/timeout/stepsize/repeat)
  * use watchdog_init_timeout (with 10s default)
  * derive clock_sel from final timeout
  * default, repeat=true (repeat mode)
- Fixed uninitalised warning on 'ret' (reported by kernel test robot)
- Use dev_dbg instead of dev_info for debug related print messages
- Minor cleanup and comments.

 drivers/rtc/rtc-pcf85363.c | 46 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/drivers/rtc/rtc-pcf85363.c b/drivers/rtc/rtc-pcf85363.c
index e10e58f69012..665bbbb169b0 100644
--- a/drivers/rtc/rtc-pcf85363.c
+++ b/drivers/rtc/rtc-pcf85363.c
@@ -123,6 +123,11 @@
 #define TSR2_SHIFT      2
 #define TSR3_SHIFT      6
 
+#define OFFSET_SIGN_BIT 7
+#define OFFSET_MINIMUM  -128
+#define OFFSET_MAXIMUM  127
+#define OFFSET_MASK     0xFF
+
 struct pcf85363 {
 	struct rtc_device	*rtc;
 	struct regmap		*regmap;
@@ -359,6 +364,45 @@ static irqreturn_t pcf85363_rtc_handle_irq(int irq, void *dev_id)
 	return handled ? IRQ_HANDLED : IRQ_NONE;
 }
 
+/*
+ * Read the current RTC offset from the CTRL_OFFSET
+ * register. This value is an 8-bit signed 2's complement
+ * value that corrects osciallator drift.
+ */
+static int pcf85363_read_offset(struct device *dev, long *offset)
+{
+	struct pcf85363 *pcf85363 = dev_get_drvdata(dev);
+	unsigned int val;
+	int ret;
+
+	ret = regmap_read(pcf85363->regmap, CTRL_OFFSET, &val);
+
+	if (ret)
+		return ret;
+
+	*offset = sign_extend32(val & OFFSET_MASK, OFFSET_SIGN_BIT);
+
+	return 0;
+}
+
+/*
+ * Write an oscillator offset correction value to
+ * the CTRL_OFFSET register. The valid range is
+ * -128 to 127 (8-bit signed), typically used to fine
+ * tune accuracy.
+ */
+static int pcf85363_set_offset(struct device *dev, long offset)
+{
+	struct pcf85363 *pcf85363 = dev_get_drvdata(dev);
+
+	if (offset < OFFSET_MINIMUM || offset > OFFSET_MAXIMUM) {
+		dev_warn(dev, "Offset out of range: %ld\n", offset);
+		return -ERANGE;
+	}
+
+	return regmap_write(pcf85363->regmap, CTRL_OFFSET, offset & OFFSET_MASK);
+}
+
 static int pcf85363_rtc_ioctl(struct device *dev,
 			      unsigned int cmd, unsigned long arg)
 {
@@ -396,6 +440,8 @@ static const struct rtc_class_ops rtc_ops = {
 	.read_alarm	= pcf85363_rtc_read_alarm,
 	.set_alarm	= pcf85363_rtc_set_alarm,
 	.alarm_irq_enable = pcf85363_rtc_alarm_irq_enable,
+	.read_offset = pcf85363_read_offset,
+	.set_offset = pcf85363_set_offset,
 };
 
 static int pcf85363_nvram_read(void *priv, unsigned int offset, void *val,
-- 
2.25.1


^ permalink raw reply related

* [PATCH v4 5/5] rtc: pcf85363: add watchdog support with configurable step size
From: Lakshay Piplani @ 2025-11-21 12:11 UTC (permalink / raw)
  To: alexandre.belloni, linux-rtc, linux-kernel, robh, krzk+dt,
	conor+dt, devicetree, wim, linux, linux-watchdog
  Cc: vikash.bansal, priyanka.jain, shashank.rebbapragada,
	Lakshay Piplani
In-Reply-To: <20251121121137.3043764-1-lakshay.piplani@nxp.com>

Add watchdog timer support to PCF85263/PCF85363 using the linux watchdog
subsystem. The driver programs the hardware watchdog timeout based on
the requested period.

Signed-off-by: Lakshay Piplani <lakshay.piplani@nxp.com>
---
V3 -> V4:
- Use watchdog_init_timeout(&wd->wdd, 0, dev) to allow devicetree or module parameter overrides; 
  fallback to WD_DEFAULT_TIMEOUT if not provided.
- Centralized clock selection logic in pcf85363_wdt_select_clock() and applied dynamically 
  whenever timeout changes.
- Removed unused repeat variable and simplified timeout handling for clarity.
V2 -> V3:
- Split into separate patches as suggested:
  - Battery switch-over detection.
  - Timestamp recording for TS pin and battery switch-over events.
  - Offset calibration.
  - Watchdog timer (to be reviewed by watchdog maintainers).
- Dropped Alarm2 support
- Switched to rtc_add_group() for sysfs attributes
V1 -> V2:
- Watchdog related changes due to removal of vendor specific properties
  from device tree
  * remove vendor DT knobs (enable/timeout/stepsize/repeat)
  * use watchdog_init_timeout (with 10s default)
  * derive clock_sel from final timeout
  * default, repeat=true (repeat mode)
- Fixed uninitalised warning on 'ret' (reported by kernel test robot)
- Use dev_dbg instead of dev_info for debug related print messages
- Minor cleanup and comments.

 drivers/rtc/rtc-pcf85363.c | 156 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 154 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/rtc-pcf85363.c b/drivers/rtc/rtc-pcf85363.c
index 665bbbb169b0..3eb87306c83c 100644
--- a/drivers/rtc/rtc-pcf85363.c
+++ b/drivers/rtc/rtc-pcf85363.c
@@ -5,6 +5,10 @@
  * Driver for NXP PCF85363 real-time clock.
  *
  * Copyright (C) 2017 Eric Nelson
+ *
+ * Copyright 2025 NXP
+ * Added support for timestamps, battery switch-over,
+ * watchdog, offset calibration.
  */
 #include <linux/module.h>
 #include <linux/i2c.h>
@@ -18,6 +22,7 @@
 #include <linux/of.h>
 #include <linux/rtc.h>
 #include <linux/regmap.h>
+#include <linux/watchdog.h>
 
 /*
  * Date/Time registers
@@ -128,6 +133,17 @@
 #define OFFSET_MAXIMUM  127
 #define OFFSET_MASK     0xFF
 
+#define WD_TIMEOUT_SHIFT        2
+#define WD_CLKSEL_MASK  GENMASK(1, 0)
+#define WD_CLKSEL_0_25HZ        0x00
+#define WD_CLKSEL_1HZ   0x01
+#define WD_CLKSEL_4HZ   0x02
+#define WD_CLKSEL_16HZ  0x03
+
+#define WD_DEFAULT_TIMEOUT  10
+#define WD_TIMEOUT_MIN	1
+#define WD_TIMEOUT_MAX	0x1F
+
 struct pcf85363 {
 	struct rtc_device	*rtc;
 	struct regmap		*regmap;
@@ -139,6 +155,14 @@ struct pcf85x63_config {
 	unsigned int num_nvram;
 };
 
+struct pcf85363_watchdog {
+	struct watchdog_device wdd;
+	struct regmap *regmap;
+	struct device *dev;
+	u8 timeout_val;
+	u8 clock_sel;
+};
+
 static int pcf85363_load_capacitance(struct pcf85363 *pcf85363, struct device_node *node)
 {
 	u32 load = 7000;
@@ -324,12 +348,13 @@ static irqreturn_t pcf85363_rtc_handle_irq(int irq, void *dev_id)
 		return IRQ_NONE;
 
 	if (flags) {
-		dev_dbg(&pcf85363->rtc->dev, "IRQ flags: 0x%02x%s%s%s%s%s\n",
+		dev_dbg(&pcf85363->rtc->dev, "IRQ flags: 0x%02x%s%s%s%s%s%s\n",
 			flags, (flags & FLAGS_A1F) ? " [A1F]" : "",
 			(flags & FLAGS_TSR1F) ? " [TSR1F]" : "",
 			(flags & FLAGS_TSR2F) ? " [TSR2F]" : "",
 			(flags & FLAGS_TSR3F) ? " [TSR3F]" : "",
-			(flags & FLAGS_BSF) ? " [BSF]" : "");
+			(flags & FLAGS_BSF) ? " [BSF]" : "",
+			(flags & FLAGS_WDF) ? " [WDF]" : "");
 	}
 
 	if (flags & FLAGS_A1F) {
@@ -361,6 +386,11 @@ static irqreturn_t pcf85363_rtc_handle_irq(int irq, void *dev_id)
 		handled = true;
 	}
 
+	if (flags & FLAGS_WDF) {
+		regmap_update_bits(pcf85363->regmap, CTRL_FLAGS, FLAGS_WDF, 0);
+		handled = true;
+	}
+
 	return handled ? IRQ_HANDLED : IRQ_NONE;
 }
 
@@ -504,6 +534,124 @@ static const struct pcf85x63_config pcf_85363_config = {
 	.num_nvram = 2
 };
 
+static void pcf85363_wdt_select_clock(struct pcf85363_watchdog *wd)
+{
+	unsigned int t = wd->wdd.timeout;
+
+	if (t <= 2)
+		wd->clock_sel = WD_CLKSEL_16HZ;
+	else if (t <= 8)
+		wd->clock_sel = WD_CLKSEL_4HZ;
+	else if (t <= 16)
+		wd->clock_sel = WD_CLKSEL_1HZ;
+	else
+		wd->clock_sel = WD_CLKSEL_0_25HZ;
+}
+
+/*
+ * This function sets the watchdog control register based on the timeout,
+ * clock selection and repeat mode settings. It prepares the value to
+ * write into the watchdog control register (CTRL_WDOG).
+ */
+static int pcf85363_wdt_reload(struct pcf85363_watchdog *wd)
+{
+	u8 val;
+
+	val = ((wd->timeout_val & WD_TIMEOUT_MAX) << WD_TIMEOUT_SHIFT) |
+	       (wd->clock_sel & WD_CLKSEL_MASK);
+
+	return regmap_write(wd->regmap, CTRL_WDOG, val);
+}
+
+static int pcf85363_wdt_start(struct watchdog_device *wdd)
+{
+	struct pcf85363_watchdog *wd = watchdog_get_drvdata(wdd);
+
+	return pcf85363_wdt_reload(wd);
+}
+
+static int pcf85363_wdt_stop(struct watchdog_device *wdd)
+{
+	struct pcf85363_watchdog *wd = watchdog_get_drvdata(wdd);
+
+	return regmap_write(wd->regmap, CTRL_WDOG, 0);
+}
+
+static int pcf85363_wdt_ping(struct watchdog_device *wdd)
+{
+	struct pcf85363_watchdog *wd = watchdog_get_drvdata(wdd);
+
+	regmap_update_bits(wd->regmap, CTRL_FLAGS, FLAGS_WDF, 0);
+
+	return pcf85363_wdt_reload(wd);
+}
+
+static int pcf85363_wdt_set_timeout(struct watchdog_device *wdd,
+				    unsigned int timeout)
+{
+	struct pcf85363_watchdog *wd = watchdog_get_drvdata(wdd);
+
+	wd->timeout_val = clamp(timeout, WD_TIMEOUT_MIN, WD_TIMEOUT_MAX);
+	wdd->timeout = wd->timeout_val;
+
+	pcf85363_wdt_select_clock(wd);
+
+	return pcf85363_wdt_reload(wd);
+}
+
+static const struct watchdog_info pcf85363_wdt_info = {
+	.identity = "PCF85363 Watchdog",
+	.options = WDIOF_KEEPALIVEPING | WDIOF_SETTIMEOUT,
+};
+
+static const struct watchdog_ops pcf85363_wdt_ops = {
+	.owner = THIS_MODULE,
+	.start = pcf85363_wdt_start,
+	.stop = pcf85363_wdt_stop,
+	.ping = pcf85363_wdt_ping,
+	.set_timeout = pcf85363_wdt_set_timeout,
+};
+
+static int pcf85363_watchdog_init(struct device *dev, struct regmap *regmap)
+{
+	struct pcf85363_watchdog *wd;
+	int ret;
+
+	if (!IS_ENABLED(CONFIG_WATCHDOG))
+		return 0;
+
+	wd = devm_kzalloc(dev, sizeof(*wd), GFP_KERNEL);
+	if (!wd)
+		return -ENOMEM;
+
+	wd->regmap = regmap;
+	wd->dev = dev;
+
+	wd->wdd.info = &pcf85363_wdt_info;
+	wd->wdd.ops = &pcf85363_wdt_ops;
+	wd->wdd.min_timeout = WD_TIMEOUT_MIN;
+	wd->wdd.max_timeout = WD_TIMEOUT_MAX;
+	wd->wdd.parent = dev;
+	wd->wdd.status = WATCHDOG_NOWAYOUT_INIT_STATUS;
+
+	ret = watchdog_init_timeout(&wd->wdd, 0, dev);
+	if (ret)
+		wd->wdd.timeout = WD_DEFAULT_TIMEOUT;
+
+	wd->timeout_val = wd->wdd.timeout;
+	pcf85363_wdt_select_clock(wd);
+
+	ret = regmap_update_bits(regmap, CTRL_FLAGS, FLAGS_WDF, 0);
+	if (ret) {
+		dev_err(dev, "failed to clear WDF:%d\n", ret);
+		return ret;
+	}
+
+	watchdog_set_drvdata(&wd->wdd, wd);
+
+	return devm_watchdog_register_device(dev, &wd->wdd);
+}
+
 /*
  * Reads 6 bytes of timestamp data starting at the given base register,
  * converts them from BCD to binary, and formats the result into a
@@ -685,6 +833,10 @@ static int pcf85363_probe(struct i2c_client *client)
 			   PIN_IO_TSPM | PIN_IO_TSIM,
 			   PIN_IO_TSPM | PIN_IO_TSIM);
 
+	ret = pcf85363_watchdog_init(dev, pcf85363->regmap);
+	if (ret)
+		dev_err_probe(dev, ret, "Watchdog init failed\n");
+
 	if (irq_a > 0 || wakeup_source)
 		device_init_wakeup(dev, true);
 
-- 
2.25.1


^ permalink raw reply related

* [PATCH] rtc: interface: Alarm race handling should not discard preceding error
From: Anthony Pighin (Nokia) @ 2025-11-25 17:35 UTC (permalink / raw)
  To: linux-rtc@vger.kernel.org; +Cc: alexandre.belloni@bootlin.com

Commit 795cda8338ea ("rtc: interface: Fix long-standing race when setting
alarm") should not discard any errors from the preceding validations.

Prior to that commit, if the alarm feature was disabled, or the
set_alarm failed, a meaningful error code would be returned to the
caller for further action.

After, more often than not, the __rtc_read_time will cause a success
return code instead, misleading the caller.

An example of this is when timer_enqueue is called for a rtc-abx080x
device. Since that driver does not clear the alarm feature bit, but
instead relies on the set_alarm operation to return invalid, the discard
of the return code causes very different behaviour; i.e.
    hwclock: select() to /dev/rtc0 to wait for clock tick timed out

Fixes: 795cda8338ea ("rtc: interface: Fix long-standing race when setting alarm")
Signed-off-by: Anthony Pighin <anthony.pighin@nokia.com>
---
 drivers/rtc/interface.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c
index b8b298efd9a9..1906f4884a83 100644
--- a/drivers/rtc/interface.c
+++ b/drivers/rtc/interface.c
@@ -457,7 +457,7 @@ static int __rtc_set_alarm(struct rtc_device *rtc, struct rtc_wkalrm *alarm)
         * are in, we can return -ETIME to signal that the timer has already
         * expired, which is true in both cases.
         */
-       if ((scheduled - now) <= 1) {
+       if (!err && (scheduled - now) <= 1) {
                err = __rtc_read_time(rtc, &tm);
                if (err)
                        return err;
--
2.43.0

^ permalink raw reply related

* [PATCH] rtc: abx80x: Disable alarm feature if no interrupt attached
From: Anthony Pighin (Nokia) @ 2025-11-25 18:00 UTC (permalink / raw)
  To: linux-rtc@vger.kernel.org; +Cc: alexandre.belloni@bootlin.com

Commit 795cda8338ea ("rtc: interface: Fix long-standing race when setting
alarm") exposed an issue where the rtc-abx80x driver does not clear the
alarm feature bit, but instead relies on the set_alarm operation to return
invalid.

For example, when a RTC_UIE_ON ioctl is handled, it should abort at the
feature validation. Instead, it proceeds to the rtc_timer_enqueue(),
which used to return an error from the set_alarm call. However,
following the race condition handling, which likely should not be
discarding predecing errors, a success condition is returned to the
ioctl() caller. This results in (for example):
    hwclock: select() to /dev/rtc0 to wait for clock tick timed out

Notwithstanding the validity of the race condition handling, if an interrupt
wasn't specified, or could not be attached, the driver should clear the
alarm feature bit.

Fixes: 718a820a303c ("rtc: abx80x: add alarm support")
Signed-off-by: Anthony Pighin <anthony.pighin@nokia.com>
---
 drivers/rtc/rtc-abx80x.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/rtc/rtc-abx80x.c b/drivers/rtc/rtc-abx80x.c
index 3fee27914ba8..5f3a3e60a19d 100644
--- a/drivers/rtc/rtc-abx80x.c
+++ b/drivers/rtc/rtc-abx80x.c
@@ -933,6 +933,8 @@ static int abx80x_probe(struct i2c_client *client)
                        client->irq = 0;
                }
        }
+       if (client->irq <= 0)
+               clear_bit(RTC_FEATURE_ALARM, priv->rtc->features);

        err = rtc_add_group(priv->rtc, &rtc_calib_attr_group);
        if (err) {
--
2.43.0

^ permalink raw reply related

* [PATCH] rtc: gamecube: Check the return value of ioremap()
From: Haotian Zhang @ 2025-11-26  1:20 UTC (permalink / raw)
  To: alexandre.belloni; +Cc: linkmauve, linux-rtc, linux-kernel, Haotian Zhang

The function ioremap() in gamecube_rtc_read_offset_from_sram() can fail
and return NULL, which is dereferenced without checking, leading to a
NULL pointer dereference.

Add a check for the return value of ioremap() and return -ENOMEM on
failure.

Fixes: 86559400b3ef ("rtc: gamecube: Add a RTC driver for the GameCube, Wii and Wii U")
Signed-off-by: Haotian Zhang <vulab@iscas.ac.cn>
---
 drivers/rtc/rtc-gamecube.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/rtc/rtc-gamecube.c b/drivers/rtc/rtc-gamecube.c
index c828bc8e05b9..cd7714437107 100644
--- a/drivers/rtc/rtc-gamecube.c
+++ b/drivers/rtc/rtc-gamecube.c
@@ -242,6 +242,10 @@ static int gamecube_rtc_read_offset_from_sram(struct priv *d)
 	}
 
 	hw_srnprot = ioremap(res.start, resource_size(&res));
+	if (!hw_srnprot) {
+		pr_err("Failed to ioremap hw_srnprot\n");
+		return -ENOMEM;
+	}
 	old = ioread32be(hw_srnprot);
 
 	/* TODO: figure out why we use this magic constant.  I obtained it by
-- 
2.50.1.windows.1


^ permalink raw reply related

* Re: PROBLEM: hwclock busted w/ M48T59 RTC (regression)
From: Nick Bowler @ 2025-11-26  3:18 UTC (permalink / raw)
  To: linux-kernel, regressions, linux-rtc; +Cc: Esben Haabendal, stable, sparclinux
In-Reply-To: <krmiwpwogrvpehlqdrugb5glcmsu54qpw3mteonqeqymrvzz37@dzt7mes7qgxt>

Any thoughts?

The problem is still present in 6.18-rc7 and reverting the commit
indicated below still fixes it.

I am also seeing the same failure on a totally different system with
Dallas DS1286 RTC, which is also fixed by reverting this commit.

Since the initial report this regression has been further backported
to all the remaining longterm kernel series.

Thanks,
  Nick

On Thu, Oct 23, 2025 at 12:45:13AM -0400, Nick Bowler wrote:
> Hi,
> 
> After a stable kernel update, the hwclock command seems no longer
> functional on my SPARC system with an ST M48T59Y-70PC1 RTC:
> 
>   # hwclock
>   [...long delay...]
>   hwclock: select() to /dev/rtc0 to wait for clock tick timed out
> 
> On prior kernels, there is no problem:
> 
>   # hwclock
>   2025-10-22 22:21:04.806992-04:00
> 
> I reproduced the same failure on 6.18-rc2 and bisected to this commit:
> 
>   commit 795cda8338eab036013314dbc0b04aae728880ab
>   Author: Esben Haabendal <esben@geanix.com>
>   Date:   Fri May 16 09:23:35 2025 +0200
>   
>       rtc: interface: Fix long-standing race when setting alarm
> 
> This commit was backported to all current 6.x stable branches,
> as well as 5.15.x, so they all have the same regression.
> 
> Reverting this commit on top of 6.18-rc2 corrects the problem.
> 
> Let me know if you need any more info!
> 
> Thanks,
>   Nick

^ permalink raw reply

* Re: [PATCH] rtc: gamecube: Check the return value of ioremap()
From: Link Mauve @ 2025-11-26  7:33 UTC (permalink / raw)
  To: Haotian Zhang; +Cc: alexandre.belloni, linkmauve, linux-rtc, linux-kernel
In-Reply-To: <20251126012019.1003-1-vulab@iscas.ac.cn>

[-- Attachment #1: Type: text/plain, Size: 1533 bytes --]

Good catch!

On Wed, Nov 26, 2025 at 09:20:19AM +0800, Haotian Zhang wrote:
> The function ioremap() in gamecube_rtc_read_offset_from_sram() can fail
> and return NULL, which is dereferenced without checking, leading to a
> NULL pointer dereference.
> 
> Add a check for the return value of ioremap() and return -ENOMEM on
> failure.
> 
> Fixes: 86559400b3ef ("rtc: gamecube: Add a RTC driver for the GameCube, Wii and Wii U")
> Signed-off-by: Haotian Zhang <vulab@iscas.ac.cn>
> ---
>  drivers/rtc/rtc-gamecube.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/rtc/rtc-gamecube.c b/drivers/rtc/rtc-gamecube.c
> index c828bc8e05b9..cd7714437107 100644
> --- a/drivers/rtc/rtc-gamecube.c
> +++ b/drivers/rtc/rtc-gamecube.c
> @@ -242,6 +242,10 @@ static int gamecube_rtc_read_offset_from_sram(struct priv *d)
>  	}
>  
>  	hw_srnprot = ioremap(res.start, resource_size(&res));
> +	if (!hw_srnprot) {
> +		pr_err("Failed to ioremap hw_srnprot\n");

The error messages on lines 240 and 276 start with a lowercase letter,
please use the same case for this message.  From a quick grep through
the kernel, it seems we use either lowercase or uppercase, but I’d
prefer to keep the case consistent in this driver at least.

> +		return -ENOMEM;
> +	}
>  	old = ioread32be(hw_srnprot);
>  
>  	/* TODO: figure out why we use this magic constant.  I obtained it by
> -- 
> 2.50.1.windows.1
> 

With that change:
Reviewed-by: Link Mauve <kernel@linkmauve.fr>

-- 
Link Mauve

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* [PATCH v2] rtc: gamecube: Check the return value of ioremap()
From: Haotian Zhang @ 2025-11-26  8:06 UTC (permalink / raw)
  To: alexandre.belloni; +Cc: linkmauve, linux-rtc, linux-kernel, Haotian Zhang
In-Reply-To: <20251126012019.1003-1-vulab@iscas.ac.cn>

The function ioremap() in gamecube_rtc_read_offset_from_sram() can fail
and return NULL, which is dereferenced without checking, leading to a
NULL pointer dereference.

Add a check for the return value of ioremap() and return -ENOMEM on
failure.

Fixes: 86559400b3ef ("rtc: gamecube: Add a RTC driver for the GameCube, Wii and Wii U")
Signed-off-by: Haotian Zhang <vulab@iscas.ac.cn>
---
Changes in v2:
  -Use lowercase for error message to match existing style.
---
 drivers/rtc/rtc-gamecube.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/rtc/rtc-gamecube.c b/drivers/rtc/rtc-gamecube.c
index c828bc8e05b9..045d5d45ab4b 100644
--- a/drivers/rtc/rtc-gamecube.c
+++ b/drivers/rtc/rtc-gamecube.c
@@ -242,6 +242,10 @@ static int gamecube_rtc_read_offset_from_sram(struct priv *d)
 	}
 
 	hw_srnprot = ioremap(res.start, resource_size(&res));
+	if (!hw_srnprot) {
+		pr_err("failed to ioremap hw_srnprot\n");
+		return -ENOMEM;
+	}
 	old = ioread32be(hw_srnprot);
 
 	/* TODO: figure out why we use this magic constant.  I obtained it by
-- 
2.50.1.windows.1


^ permalink raw reply related

* Re: [PATCH v2] rtc: gamecube: Check the return value of ioremap()
From: Link Mauve @ 2025-11-26  8:14 UTC (permalink / raw)
  To: Haotian Zhang; +Cc: alexandre.belloni, linkmauve, linux-rtc, linux-kernel
In-Reply-To: <20251126080625.1752-1-vulab@iscas.ac.cn>

[-- Attachment #1: Type: text/plain, Size: 1369 bytes --]

Hi,

On Wed, Nov 26, 2025 at 04:06:25PM +0800, Haotian Zhang wrote:
> The function ioremap() in gamecube_rtc_read_offset_from_sram() can fail
> and return NULL, which is dereferenced without checking, leading to a
> NULL pointer dereference.
> 
> Add a check for the return value of ioremap() and return -ENOMEM on
> failure.
> 
> Fixes: 86559400b3ef ("rtc: gamecube: Add a RTC driver for the GameCube, Wii and Wii U")
> Signed-off-by: Haotian Zhang <vulab@iscas.ac.cn>

You forgot to carry my R-b, but here it is again:
Reviewed-by: Link Mauve <kernel@linkmauve.fr>

> ---
> Changes in v2:
>   -Use lowercase for error message to match existing style.
> ---
>  drivers/rtc/rtc-gamecube.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/rtc/rtc-gamecube.c b/drivers/rtc/rtc-gamecube.c
> index c828bc8e05b9..045d5d45ab4b 100644
> --- a/drivers/rtc/rtc-gamecube.c
> +++ b/drivers/rtc/rtc-gamecube.c
> @@ -242,6 +242,10 @@ static int gamecube_rtc_read_offset_from_sram(struct priv *d)
>  	}
>  
>  	hw_srnprot = ioremap(res.start, resource_size(&res));
> +	if (!hw_srnprot) {
> +		pr_err("failed to ioremap hw_srnprot\n");
> +		return -ENOMEM;
> +	}
>  	old = ioread32be(hw_srnprot);
>  
>  	/* TODO: figure out why we use this magic constant.  I obtained it by
> -- 
> 2.50.1.windows.1
> 

-- 
Link Mauve

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* Re: PROBLEM: hwclock busted w/ M48T59 RTC (regression)
From: John Paul Adrian Glaubitz @ 2025-11-26  9:46 UTC (permalink / raw)
  To: Nick Bowler, linux-kernel, regressions, linux-rtc
  Cc: Esben Haabendal, stable, sparclinux, regressions
In-Reply-To: <gfwdg244bcmkv7l44fknfi4osd2b23unwaos7rnlirkdy2rrrt@yovd2vewdviv>

Hi Nick,

I have not used the regression tracker before, so let's give it a try:

#regzbot ^introduced: 795cda8338eab036013314dbc0b04aae728880ab

Adrian

On Tue, 2025-11-25 at 22:18 -0500, Nick Bowler wrote:
> Any thoughts?
> 
> The problem is still present in 6.18-rc7 and reverting the commit
> indicated below still fixes it.
> 
> I am also seeing the same failure on a totally different system with
> Dallas DS1286 RTC, which is also fixed by reverting this commit.
> 
> Since the initial report this regression has been further backported
> to all the remaining longterm kernel series.
> 
> Thanks,
>   Nick
> 
> On Thu, Oct 23, 2025 at 12:45:13AM -0400, Nick Bowler wrote:
> > Hi,
> > 
> > After a stable kernel update, the hwclock command seems no longer
> > functional on my SPARC system with an ST M48T59Y-70PC1 RTC:
> > 
> >   # hwclock
> >   [...long delay...]
> >   hwclock: select() to /dev/rtc0 to wait for clock tick timed out
> > 
> > On prior kernels, there is no problem:
> > 
> >   # hwclock
> >   2025-10-22 22:21:04.806992-04:00
> > 
> > I reproduced the same failure on 6.18-rc2 and bisected to this commit:
> > 
> >   commit 795cda8338eab036013314dbc0b04aae728880ab
> >   Author: Esben Haabendal <esben@geanix.com>
> >   Date:   Fri May 16 09:23:35 2025 +0200
> >   
> >       rtc: interface: Fix long-standing race when setting alarm
> > 
> > This commit was backported to all current 6.x stable branches,
> > as well as 5.15.x, so they all have the same regression.
> > 
> > Reverting this commit on top of 6.18-rc2 corrects the problem.
> > 
> > Let me know if you need any more info!
> > 
> > Thanks,
> >   Nick

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

^ permalink raw reply

* Re: [PATCH v2 0/3] Samsung mfd/rtc driver alarm IRQ simplification
From: Lee Jones @ 2025-11-26 11:29 UTC (permalink / raw)
  To: André Draszik
  Cc: Krzysztof Kozlowski, Alexandre Belloni, Peter Griffin,
	Tudor Ambarus, Will McVicker, Juan Yescas, Douglas Anderson,
	kernel-team, Kaustabh Chakraborty, linux-kernel,
	linux-samsung-soc, linux-rtc
In-Reply-To: <20251120-s5m-alarm-v2-0-cc15f0e32161@linaro.org>

On Thu, 20 Nov 2025, André Draszik wrote:

> Hi,
> 
> With the attached patches the Samsung s5m RTC driver is simplified a
> little bit with regards to alarm IRQ acquisition.
> 
> The end result is that instead of having a list of IRQ numbers for each
> variant (and a BUILD_BUG_ON() to ensure consistency), the RTC driver
> queries the 'alarm' platform resource from the parent (mfd cell).
> 
> Additionally, we can drop a now-useless field from runtime data,
> reducing memory consumption slightly.
> 
> The attached patches must be applied in-order as patch 2 without 1 will
> fail at runtime, and patch 3 without 2 will fail at build time. I would
> expect them all to go via the MFD tree. Alternatively, they could be
> applied individually to the respective kernel trees during multiple
> kernel release cycles, but that seems a needless complication and
> delay.
> 
> Signed-off-by: André Draszik <andre.draszik@linaro.org>
> ---
> Changes in v2:
> - rebase on top of https://lore.kernel.org/r/20251114-s2mpg10-chained-irq-v1-1-34ddfa49c4cd@linaro.org
> - return struct regmap_irq_chip_data * in sec_irq_init() (Lee)
> - collect tags
> - Link to v1: https://lore.kernel.org/r/20251114-s5m-alarm-v1-0-c9b3bebae65f@linaro.org
> 
> ---
> André Draszik (3):
>       mfd: sec: add rtc alarm IRQ as platform device resource
>       rtc: s5m: query platform device IRQ resource for alarm IRQ
>       mfd: sec: drop now unused struct sec_pmic_dev::irq_data
> 
>  drivers/mfd/sec-common.c         | 45 ++++++++++++++++++++--------
>  drivers/mfd/sec-core.h           |  2 +-
>  drivers/mfd/sec-irq.c            | 63 ++++++++++++++++++----------------------
>  drivers/rtc/rtc-s5m.c            | 21 +++++---------
>  include/linux/mfd/samsung/core.h |  1 -
>  5 files changed, 71 insertions(+), 61 deletions(-)

The MFD parts look okay to me.

Once we have the RTC Ack, I'll merge this and send out a PR.

-- 
Lee Jones [李琼斯]

^ permalink raw reply

* Re: [PATCH v2 0/3] Samsung mfd/rtc driver alarm IRQ simplification
From: André Draszik @ 2025-11-26 12:30 UTC (permalink / raw)
  To: Lee Jones
  Cc: Krzysztof Kozlowski, Alexandre Belloni, Peter Griffin,
	Tudor Ambarus, Will McVicker, Juan Yescas, Douglas Anderson,
	kernel-team, Kaustabh Chakraborty, linux-kernel,
	linux-samsung-soc, linux-rtc
In-Reply-To: <20251126112935.GA3070764@google.com>

Hi Lee,

On Wed, 2025-11-26 at 11:29 +0000, Lee Jones wrote:
> On Thu, 20 Nov 2025, André Draszik wrote:
> 
> > Hi,
> > 
> > With the attached patches the Samsung s5m RTC driver is simplified a
> > little bit with regards to alarm IRQ acquisition.
> > 
> > The end result is that instead of having a list of IRQ numbers for each
> > variant (and a BUILD_BUG_ON() to ensure consistency), the RTC driver
> > queries the 'alarm' platform resource from the parent (mfd cell).
> > 
> > Additionally, we can drop a now-useless field from runtime data,
> > reducing memory consumption slightly.
> > 
> > The attached patches must be applied in-order as patch 2 without 1 will
> > fail at runtime, and patch 3 without 2 will fail at build time. I would
> > expect them all to go via the MFD tree. Alternatively, they could be
> > applied individually to the respective kernel trees during multiple
> > kernel release cycles, but that seems a needless complication and
> > delay.
> > 
> > Signed-off-by: André Draszik <andre.draszik@linaro.org>
> > ---
> > Changes in v2:
> > - rebase on top of https://lore.kernel.org/r/20251114-s2mpg10-chained-irq-v1-1-34ddfa49c4cd@linaro.org
> > - return struct regmap_irq_chip_data * in sec_irq_init() (Lee)
> > - collect tags
> > - Link to v1: https://lore.kernel.org/r/20251114-s5m-alarm-v1-0-c9b3bebae65f@linaro.org
> > 
> > ---
> > André Draszik (3):
> >       mfd: sec: add rtc alarm IRQ as platform device resource
> >       rtc: s5m: query platform device IRQ resource for alarm IRQ
> >       mfd: sec: drop now unused struct sec_pmic_dev::irq_data
> > 
> >  drivers/mfd/sec-common.c         | 45 ++++++++++++++++++++--------
> >  drivers/mfd/sec-core.h           |  2 +-
> >  drivers/mfd/sec-irq.c            | 63 ++++++++++++++++++----------------------
> >  drivers/rtc/rtc-s5m.c            | 21 +++++---------
> >  include/linux/mfd/samsung/core.h |  1 -
> >  5 files changed, 71 insertions(+), 61 deletions(-)
> 
> The MFD parts look okay to me.
> 
> Once we have the RTC Ack, I'll merge this and send out a PR.

Thanks Lee. Alexandre kindly acked v1 in
https://lore.kernel.org/all/2025111415582194c6ee16@mail.local/
and there was no change to the RTC part in v2 hence I collected it
already.

So should be good to go?

Cheers,
Andre'


^ permalink raw reply

* Re: [PATCH v2 0/3] Samsung mfd/rtc driver alarm IRQ simplification
From: Lee Jones @ 2025-11-26 14:04 UTC (permalink / raw)
  To: André Draszik
  Cc: Krzysztof Kozlowski, Alexandre Belloni, Peter Griffin,
	Tudor Ambarus, Will McVicker, Juan Yescas, Douglas Anderson,
	kernel-team, Kaustabh Chakraborty, linux-kernel,
	linux-samsung-soc, linux-rtc
In-Reply-To: <20251126112935.GA3070764@google.com>

On Wed, 26 Nov 2025, Lee Jones wrote:

> On Thu, 20 Nov 2025, André Draszik wrote:
> 
> > Hi,
> > 
> > With the attached patches the Samsung s5m RTC driver is simplified a
> > little bit with regards to alarm IRQ acquisition.
> > 
> > The end result is that instead of having a list of IRQ numbers for each
> > variant (and a BUILD_BUG_ON() to ensure consistency), the RTC driver
> > queries the 'alarm' platform resource from the parent (mfd cell).
> > 
> > Additionally, we can drop a now-useless field from runtime data,
> > reducing memory consumption slightly.
> > 
> > The attached patches must be applied in-order as patch 2 without 1 will
> > fail at runtime, and patch 3 without 2 will fail at build time. I would
> > expect them all to go via the MFD tree. Alternatively, they could be
> > applied individually to the respective kernel trees during multiple
> > kernel release cycles, but that seems a needless complication and
> > delay.
> > 
> > Signed-off-by: André Draszik <andre.draszik@linaro.org>
> > ---
> > Changes in v2:
> > - rebase on top of https://lore.kernel.org/r/20251114-s2mpg10-chained-irq-v1-1-34ddfa49c4cd@linaro.org
> > - return struct regmap_irq_chip_data * in sec_irq_init() (Lee)
> > - collect tags
> > - Link to v1: https://lore.kernel.org/r/20251114-s5m-alarm-v1-0-c9b3bebae65f@linaro.org
> > 
> > ---
> > André Draszik (3):
> >       mfd: sec: add rtc alarm IRQ as platform device resource
> >       rtc: s5m: query platform device IRQ resource for alarm IRQ
> >       mfd: sec: drop now unused struct sec_pmic_dev::irq_data
> > 
> >  drivers/mfd/sec-common.c         | 45 ++++++++++++++++++++--------
> >  drivers/mfd/sec-core.h           |  2 +-
> >  drivers/mfd/sec-irq.c            | 63 ++++++++++++++++++----------------------
> >  drivers/rtc/rtc-s5m.c            | 21 +++++---------
> >  include/linux/mfd/samsung/core.h |  1 -
> >  5 files changed, 71 insertions(+), 61 deletions(-)
> 
> The MFD parts look okay to me.
> 
> Once we have the RTC Ack, I'll merge this and send out a PR.

Ah, I see it.  Apologies.

It's too late in the cycle to take this now anyway.

It's on my radar for when -rc1 is released.

-- 
Lee Jones [李琼斯]

^ permalink raw reply

* Re: [PATCH v5 08/16] mfd: bd71828: Support ROHM BD72720
From: Lee Jones @ 2025-11-26 14:28 UTC (permalink / raw)
  To: Matti Vaittinen
  Cc: Matti Vaittinen, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Sebastian Reichel, Liam Girdwood, Mark Brown,
	Michael Turquette, Stephen Boyd, Linus Walleij,
	Bartosz Golaszewski, Alexandre Belloni, linux-leds, devicetree,
	linux-kernel, linux-pm, linux-clk, linux-gpio, linux-rtc,
	Andreas Kemnade
In-Reply-To: <ffdc1c2f380959c792ad39817ba5e9cf4bbc1131.1763625920.git.mazziesaccount@gmail.com>

On Thu, 20 Nov 2025, Matti Vaittinen wrote:

> From: Matti Vaittinen <mazziesaccount@gmail.com>
> 
> 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@gmail.com>
> 
> ---
> 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@kemnade.info/
> 
> 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
> 
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 6cec1858947b..61e238b316f4 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -2211,20 +2211,22 @@ config MFD_ROHM_BD718XX
>  	  and emergency shut down as well as 32,768KHz clock output.
>  
>  config MFD_ROHM_BD71828
> -	tristate "ROHM BD71828 and BD71815 Power Management IC"
> +	tristate "ROHM BD718[15/28/79], BD72720 and BD73900 PMICs"
>  	depends on I2C=y
>  	depends on OF
>  	select REGMAP_I2C
>  	select REGMAP_IRQ
>  	select MFD_CORE
>  	help
> -	  Select this option to get support for the ROHM BD71828 and BD71815
> -	  Power Management ICs. BD71828GW and BD71815AGW are single-chip power
> -	  management ICs mainly for battery-powered portable devices.
> -	  The BD71828 integrates 7 buck converters and 7 LDOs. The BD71815
> -	  has 5 bucks, 7 LDOs, and a boost for driving LEDs. Both ICs provide
> -	  also a single-cell linear charger, a Coulomb counter, a real-time
> -	  clock (RTC), GPIOs and a 32.768 kHz clock gate.
> +	  Select this option to get support for the ROHM BD71815, BD71828,
> +	  BD71879, BD72720 and BD73900 Power Management ICs. These are
> +	  single-chip power management ICs mainly for battery-powered portable

Nit: Power Management ICs (PMIC)

> +	  devices.
> +	  The BD71815 has 5 bucks, 7 LDOs, and a boost for driving LEDs.
> +	  The BD718[28/79] have 7 buck converters and 7 LDOs.
> +	  The BD72720 and the BD73900 have 10 bucks and 11 LDOs.
> +	  All ICs provide a single-cell linear charger, a Coulomb counter,
> +	  a real-time clock (RTC), GPIOs and a 32.768 kHz clock gate.

Nit: Real-Time Clock (RTC)

>  
>  config MFD_ROHM_BD957XMUF
>  	tristate "ROHM BD9576MUF and BD9573MUF Power Management ICs"
> 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).

>  #include <linux/gpio_keys.h>
>  #include <linux/i2c.h>
> @@ -13,12 +13,29 @@
>  #include <linux/mfd/core.h>
>  #include <linux/mfd/rohm-bd71815.h>
>  #include <linux/mfd/rohm-bd71828.h>
> +#include <linux/mfd/rohm-bd72720.h>
>  #include <linux/mfd/rohm-generic.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/regmap.h>
>  #include <linux/types.h>
>  
> +#define BD72720_TYPED_IRQ_REG(_irq, _stat_offset, _mask, _type_offset)     \
> +	[_irq] = {							   \
> +		.reg_offset = (_stat_offset),				   \
> +		.mask = (_mask),					   \
> +		{							   \
> +			.type_reg_offset = (_type_offset),		   \
> +			.type_reg_mask = BD72720_GPIO_IRQ_TYPE_MASK,	   \
> +			.type_rising_val = BD72720_GPIO_IRQ_TYPE_RISING,   \
> +			.type_falling_val = BD72720_GPIO_IRQ_TYPE_FALLING, \
> +			.type_level_low_val = BD72720_GPIO_IRQ_TYPE_LOW,   \
> +			.type_level_high_val = BD72720_GPIO_IRQ_TYPE_HIGH, \
> +			.types_supported = IRQ_TYPE_EDGE_BOTH |		   \
> +				IRQ_TYPE_LEVEL_HIGH | IRQ_TYPE_LEVEL_LOW,  \
> +		},							   \
> +	}
> +
>  static struct gpio_keys_button button = {
>  	.code = KEY_POWER,
>  	.gpio = -1,
> @@ -43,6 +60,12 @@ static const struct resource bd71828_rtc_irqs[] = {
>  	DEFINE_RES_IRQ_NAMED(BD71828_INT_RTC2, "bd70528-rtc-alm-2"),
>  };
>  
> +static const struct resource bd72720_rtc_irqs[] = {
> +	DEFINE_RES_IRQ_NAMED(BD72720_INT_RTC0, "bd70528-rtc-alm-0"),
> +	DEFINE_RES_IRQ_NAMED(BD72720_INT_RTC1, "bd70528-rtc-alm-1"),
> +	DEFINE_RES_IRQ_NAMED(BD72720_INT_RTC2, "bd70528-rtc-alm-2"),
> +};
> +
>  static const struct resource bd71815_power_irqs[] = {
>  	DEFINE_RES_IRQ_NAMED(BD71815_INT_DCIN_RMV, "bd71815-dcin-rmv"),
>  	DEFINE_RES_IRQ_NAMED(BD71815_INT_CLPS_OUT, "bd71815-dcin-clps-out"),
> @@ -156,6 +179,74 @@ static struct mfd_cell bd71828_mfd_cells[] = {
>  	},
>  };
>  
> +static const struct resource bd72720_power_irqs[] = {
> +	DEFINE_RES_IRQ_NAMED(BD72720_INT_VBUS_RMV, "bd72720_int_vbus_rmv"),
> +	DEFINE_RES_IRQ_NAMED(BD72720_INT_VBUS_DET, "bd72720_int_vbus_det"),
> +	DEFINE_RES_IRQ_NAMED(BD72720_INT_VBUS_MON_RES, "bd72720_int_vbus_mon_res"),
> +	DEFINE_RES_IRQ_NAMED(BD72720_INT_VBUS_MON_DET, "bd72720_int_vbus_mon_det"),
> +	DEFINE_RES_IRQ_NAMED(BD72720_INT_VSYS_MON_RES, "bd72720_int_vsys_mon_res"),
> +	DEFINE_RES_IRQ_NAMED(BD72720_INT_VSYS_MON_DET, "bd72720_int_vsys_mon_det"),
> +	DEFINE_RES_IRQ_NAMED(BD72720_INT_VSYS_UV_RES, "bd72720_int_vsys_uv_res"),
> +	DEFINE_RES_IRQ_NAMED(BD72720_INT_VSYS_UV_DET, "bd72720_int_vsys_uv_det"),
> +	DEFINE_RES_IRQ_NAMED(BD72720_INT_VSYS_LO_RES, "bd72720_int_vsys_lo_res"),
> +	DEFINE_RES_IRQ_NAMED(BD72720_INT_VSYS_LO_DET, "bd72720_int_vsys_lo_det"),
> +	DEFINE_RES_IRQ_NAMED(BD72720_INT_VSYS_OV_RES, "bd72720_int_vsys_ov_res"),
> +	DEFINE_RES_IRQ_NAMED(BD72720_INT_VSYS_OV_DET, "bd72720_int_vsys_ov_det"),
> +	DEFINE_RES_IRQ_NAMED(BD72720_INT_BAT_ILIM, "bd72720_int_bat_ilim"),
> +	DEFINE_RES_IRQ_NAMED(BD72720_INT_CHG_DONE, "bd72720_int_chg_done"),
> +	DEFINE_RES_IRQ_NAMED(BD72720_INT_EXTEMP_TOUT, "bd72720_int_extemp_tout"),
> +	DEFINE_RES_IRQ_NAMED(BD72720_INT_CHG_WDT_EXP, "bd72720_int_chg_wdt_exp"),
> +	DEFINE_RES_IRQ_NAMED(BD72720_INT_BAT_MNT_OUT, "bd72720_int_bat_mnt_out"),
> +	DEFINE_RES_IRQ_NAMED(BD72720_INT_BAT_MNT_IN, "bd72720_int_bat_mnt_in"),
> +	DEFINE_RES_IRQ_NAMED(BD72720_INT_CHG_TRNS, "bd72720_int_chg_trns"),
> +
> +	DEFINE_RES_IRQ_NAMED(BD72720_INT_VBAT_MON_RES, "bd72720_int_vbat_mon_res"),
> +	DEFINE_RES_IRQ_NAMED(BD72720_INT_VBAT_MON_DET, "bd72720_int_vbat_mon_det"),
> +	DEFINE_RES_IRQ_NAMED(BD72720_INT_VBAT_SHT_RES, "bd72720_int_vbat_sht_res"),
> +	DEFINE_RES_IRQ_NAMED(BD72720_INT_VBAT_SHT_DET, "bd72720_int_vbat_sht_det"),
> +	DEFINE_RES_IRQ_NAMED(BD72720_INT_VBAT_LO_RES, "bd72720_int_vbat_lo_res"),
> +	DEFINE_RES_IRQ_NAMED(BD72720_INT_VBAT_LO_DET, "bd72720_int_vbat_lo_det"),
> +	DEFINE_RES_IRQ_NAMED(BD72720_INT_VBAT_OV_RES, "bd72720_int_vbat_ov_res"),
> +	DEFINE_RES_IRQ_NAMED(BD72720_INT_VBAT_OV_DET, "bd72720_int_vbat_ov_det"),
> +	DEFINE_RES_IRQ_NAMED(BD72720_INT_BAT_RMV, "bd72720_int_bat_rmv"),
> +	DEFINE_RES_IRQ_NAMED(BD72720_INT_BAT_DET, "bd72720_int_bat_det"),
> +	DEFINE_RES_IRQ_NAMED(BD72720_INT_DBAT_DET, "bd72720_int_dbat_det"),
> +	DEFINE_RES_IRQ_NAMED(BD72720_INT_BAT_TEMP_TRNS, "bd72720_int_bat_temp_trns"),
> +	DEFINE_RES_IRQ_NAMED(BD72720_INT_LOBTMP_RES, "bd72720_int_lobtmp_res"),
> +	DEFINE_RES_IRQ_NAMED(BD72720_INT_LOBTMP_DET, "bd72720_int_lobtmp_det"),
> +	DEFINE_RES_IRQ_NAMED(BD72720_INT_OVBTMP_RES, "bd72720_int_ovbtmp_res"),
> +	DEFINE_RES_IRQ_NAMED(BD72720_INT_OVBTMP_DET, "bd72720_int_ovbtmp_det"),
> +	DEFINE_RES_IRQ_NAMED(BD72720_INT_OCUR1_RES, "bd72720_int_ocur1_res"),
> +	DEFINE_RES_IRQ_NAMED(BD72720_INT_OCUR1_DET, "bd72720_int_ocur1_det"),
> +	DEFINE_RES_IRQ_NAMED(BD72720_INT_OCUR2_RES, "bd72720_int_ocur2_res"),
> +	DEFINE_RES_IRQ_NAMED(BD72720_INT_OCUR2_DET, "bd72720_int_ocur2_det"),
> +	DEFINE_RES_IRQ_NAMED(BD72720_INT_OCUR3_RES, "bd72720_int_ocur3_res"),
> +	DEFINE_RES_IRQ_NAMED(BD72720_INT_OCUR3_DET, "bd72720_int_ocur3_det"),
> +	DEFINE_RES_IRQ_NAMED(BD72720_INT_CC_MON1_DET, "bd72720_int_cc_mon1_det"),
> +	DEFINE_RES_IRQ_NAMED(BD72720_INT_CC_MON2_DET, "bd72720_int_cc_mon2_det"),
> +	DEFINE_RES_IRQ_NAMED(BD72720_INT_CC_MON3_DET, "bd72720_int_cc_mon3_det"),
> +};
> +
> +static const struct mfd_cell bd72720_mfd_cells[] = {
> +	{ .name = "bd72720-pmic", },
> +	{ .name = "bd72720-gpio", },
> +	{ .name = "bd72720-led", },
> +	{ .name = "bd72720-clk", },
> +	{
> +		.name = "bd72720-power",
> +		.resources = bd72720_power_irqs,
> +		.num_resources = ARRAY_SIZE(bd72720_power_irqs),
> +	}, {
> +		.name = "bd72720-rtc",
> +		.resources = bd72720_rtc_irqs,
> +		.num_resources = ARRAY_SIZE(bd72720_rtc_irqs),
> +	}, {
> +		.name = "gpio-keys",
> +		.platform_data = &bd71828_powerkey_data,
> +		.pdata_size = sizeof(bd71828_powerkey_data),
> +	},
> +};
> +
>  static const struct regmap_range bd71815_volatile_ranges[] = {
>  	regmap_reg_range(BD71815_REG_SEC, BD71815_REG_YEAR),
>  	regmap_reg_range(BD71815_REG_CONF, BD71815_REG_BAT_TEMP),
> @@ -180,6 +271,87 @@ static const struct regmap_range bd71828_volatile_ranges[] = {
>  	regmap_reg_range(BD71828_REG_INT_MAIN, BD71828_REG_IO_STAT),
>  };
>  
> +static const struct regmap_range bd72720_volatile_ranges_4b[] = {
> +	regmap_reg_range(BD72720_REG_RESETSRC_1, BD72720_REG_RESETSRC_2),
> +	regmap_reg_range(BD72720_REG_POWER_STATE, BD72720_REG_POWER_STATE),
> +	/* The state indicator bit changes when new state is reached */
> +	regmap_reg_range(BD72720_REG_PS_CTRL_1, BD72720_REG_PS_CTRL_1),
> +	regmap_reg_range(BD72720_REG_RCVNUM, BD72720_REG_RCVNUM),
> +	regmap_reg_range(BD72720_REG_CONF, BD72720_REG_HALL_STAT),
> +	regmap_reg_range(BD72720_REG_RTC_SEC, BD72720_REG_RTC_YEAR),
> +	regmap_reg_range(BD72720_REG_INT_LVL1_STAT, BD72720_REG_INT_ETC2_SRC),
> +};
> +
> +static const struct regmap_range bd72720_precious_ranges_4b[] = {
> +	regmap_reg_range(BD72720_REG_INT_LVL1_STAT, BD72720_REG_INT_ETC2_STAT),
> +};
> +
> +/*
> + * The BD72720 is an odd beast in that it contains two separate sets of
> + * registers, both starting from address 0x0. The twist is that these "pages"
> + * are behind different I2C slave addresses. Most of the registers are behind
> + * a slave address 0x4b, which will be used as the "main" address for this
> + * device.

New paragraph, new line?

> + * Most of the charger related registers are located behind slave address 0x4c.
> + * It is tempting to push the dealing with the charger registers and the extra
> + * 0x4c device in power-supply driver - but perhaps it's better for the sake of
> + * the cleaner re-use to deal with setting up all of the regmaps here.
> + * Furthermore, the LED stuff may need access to both of these devices.
> + *
> + * Instead of providing one of the regmaps to sub-devices in MFD platform data,
> + * we create one more 'wrapper regmap' with custom read/write operations. These
> + * custom accessors will select which of the 'real' regmaps to use, based on
> + * the register address.

As above.

> + * The register addresses are 8-bit, so we add offset 0x100 to the addresses
> + * behind the secondary slave 0x4c. The 'wrapper' regmap can then detect the
> + * correct slave address based on the register address and call regmap_write()
> + * and regmap_read() using correct 'real' regmap. This way the registers of
> + * both of the slaves can be accessed using one 'wrapper' regmap.
> + *
> + * NOTE: The added offsets mean that the defined addresses for slave 0x4c must
> + * be used through the 'wrapper' regmap because the offset must be stripped
> + * from the register addresses. The 0x4b can be accessed both indirectly using
> + * the 'wrapper' regmap, and directly using the 'real' regmap.
> + */
> +#define BD72720_SECONDARY_I2C_SLAVE 0x4c
> +
> +struct bd72720_regmaps {
> +	struct regmap *map1_4b;
> +	struct regmap *map2_4c;
> +};
> +
> +/* Translate the slave 0x4c wrapper register address to a real one */
> +#define BD72720_REG_UNWRAP(reg) ((reg) - 0x100)
> +
> +/* Ranges given to 'real' 0x4c regmap must use unwrapped addresses. */
> +#define BD72720_UNWRAP_REG_RANGE(startreg, endreg)					\
> +	regmap_reg_range(BD72720_REG_UNWRAP(startreg), BD72720_REG_UNWRAP(endreg))

'\n' here please.

> +static const struct regmap_range bd72720_volatile_ranges_4c[] = {
> +	/* Status information */
> +	BD72720_UNWRAP_REG_RANGE(BD72720_REG_CHG_STATE, BD72720_REG_CHG_EN),
> +	/*
> +	 * Under certain circumstances, write to some bits may be
> +	 * ignored
> +	 */
> +	BD72720_UNWRAP_REG_RANGE(BD72720_REG_CHG_CTRL, BD72720_REG_CHG_CTRL),
> +	/*
> +	 * TODO: Ensure this is used to advertise state, not (only?) to
> +	 * control it.
> +	 */
> +	BD72720_UNWRAP_REG_RANGE(BD72720_REG_VSYS_STATE_STAT, BD72720_REG_VSYS_STATE_STAT),
> +	/* Measured data */
> +	BD72720_UNWRAP_REG_RANGE(BD72720_REG_VM_VBAT_U, BD72720_REG_VM_VF_L),
> +	/* Self clearing bits */
> +	BD72720_UNWRAP_REG_RANGE(BD72720_REG_VM_VSYS_SA_MINMAX_CTRL,
> +				 BD72720_REG_VM_VSYS_SA_MINMAX_CTRL),
> +	/* Counters, self clearing bits */
> +	BD72720_UNWRAP_REG_RANGE(BD72720_REG_CC_CURCD_U, BD72720_REG_CC_CTRL),
> +	/* Self clearing bits */
> +	BD72720_UNWRAP_REG_RANGE(BD72720_REG_CC_CCNTD_CTRL, BD72720_REG_CC_CCNTD_CTRL),
> +	/* Self clearing bits */
> +	BD72720_UNWRAP_REG_RANGE(BD72720_REG_IMPCHK_CTRL, BD72720_REG_IMPCHK_CTRL),
> +};
> +
>  static const struct regmap_access_table bd71815_volatile_regs = {
>  	.yes_ranges = &bd71815_volatile_ranges[0],
>  	.n_yes_ranges = ARRAY_SIZE(bd71815_volatile_ranges),
> @@ -190,6 +362,21 @@ static const struct regmap_access_table bd71828_volatile_regs = {
>  	.n_yes_ranges = ARRAY_SIZE(bd71828_volatile_ranges),
>  };
>  
> +static const struct regmap_access_table bd72720_volatile_regs_4b = {
> +	.yes_ranges = &bd72720_volatile_ranges_4b[0],
> +	.n_yes_ranges = ARRAY_SIZE(bd72720_volatile_ranges_4b),
> +};
> +
> +static const struct regmap_access_table bd72720_precious_regs_4b = {
> +	.yes_ranges = &bd72720_precious_ranges_4b[0],
> +	.n_yes_ranges = ARRAY_SIZE(bd72720_precious_ranges_4b),
> +};
> +
> +static const struct regmap_access_table bd72720_volatile_regs_4c = {
> +	.yes_ranges = &bd72720_volatile_ranges_4c[0],
> +	.n_yes_ranges = ARRAY_SIZE(bd72720_volatile_ranges_4c),
> +};
> +
>  static const struct regmap_config bd71815_regmap = {
>  	.reg_bits = 8,
>  	.val_bits = 8,
> @@ -206,10 +393,79 @@ static const struct regmap_config bd71828_regmap = {
>  	.cache_type = REGCACHE_MAPLE,
>  };
>  
> +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.

> +		return regmap_write(maps->map1_4b, reg, val);
> +
> +	reg = BD72720_REG_UNWRAP(reg);
> +
> +	return regmap_write(maps->map2_4c, reg, val);
> +}
> +
> +static int regmap_read_wrapper(void *context, unsigned int reg, unsigned int *val)
> +{
> +	struct bd72720_regmaps *maps = context;
> +
> +	if (reg < 0x100)

Likewise.

> +		return regmap_read(maps->map1_4b, reg, val);
> +
> +	reg = BD72720_REG_UNWRAP(reg);
> +
> +	return regmap_read(maps->map2_4c, reg, val);
> +}
> +
> +static const struct regmap_config bd72720_wrapper_map_config = {
> +	.name = "wrap-map",
> +	.reg_bits = 9,
> +	.val_bits = 8,
> +	.max_register = BD72720_REG_IMPCHK_CTRL,
> +	/*
> +	 * We don't want to duplicate caches. It would be a bit faster to
> +	 * have the cache in this 'wrapper regmap', and not in the 'real
> +	 * regmaps' bd72720_regmap_4b and bd72720_regmap_4c below. This would
> +	 * require all the subdevices to use the wrapper-map in order to be
> +	 * able to benefit from the cache.
> +	 * Currently most of the sub-devices use only the same slave-address
> +	 * as this MFD driver. Now, because we don't add the offset to the
> +	 * registers belonging to this slave, those devices can use either the
> +	 * wrapper map, or the bd72720_regmap_4b directly. This means majority
> +	 * of our sub devices don't need to care which regmap they get using
> +	 * the dev_get_regmap(). This unifies the code between the BD72720 and
> +	 * those variants which don't have this 'multiple slave addresses'
> +	 * -hassle.
> +	 * So, for a small performance penalty, we simplify the code for the
> +	 * sub-devices by having the caches in the wrapped regmaps and not here.
> +	 */
> +	.cache_type = REGCACHE_NONE,
> +	.reg_write = regmap_write_wrapper,
> +	.reg_read = regmap_read_wrapper,
> +};
> +
> +static const struct regmap_config bd72720_regmap_4b = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.volatile_table = &bd72720_volatile_regs_4b,
> +	.precious_table = &bd72720_precious_regs_4b,
> +	.max_register = BD72720_REG_INT_ETC2_SRC,
> +	.cache_type = REGCACHE_MAPLE,
> +};
> +
> +static const struct regmap_config bd72720_regmap_4c = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.volatile_table = &bd72720_volatile_regs_4c,
> +	.max_register = BD72720_REG_UNWRAP(BD72720_REG_IMPCHK_CTRL),
> +	.cache_type = REGCACHE_MAPLE,
> +};
> +
>  /*
>   * Mapping of main IRQ register bits to sub-IRQ register offsets so that we can
>   * access corect sub-IRQ registers based on bits that are set in main IRQ
> - * register. BD71815 and BD71828 have same sub-register-block offests.
> + * register. BD71815 and BD71828 have same sub-register-block offests, the
> + * BD72720 has a different one.
>   */
>  
>  static unsigned int bit0_offsets[] = {11};		/* RTC IRQ */
> @@ -221,6 +477,15 @@ static unsigned int bit5_offsets[] = {3};		/* VSYS IRQ */
>  static unsigned int bit6_offsets[] = {1, 2};		/* DCIN IRQ */
>  static unsigned int bit7_offsets[] = {0};		/* BUCK IRQ */
>  
> +static unsigned int bd72720_bit0_offsets[] = {0, 1};	/* PS1 and PS2 */
> +static unsigned int bd72720_bit1_offsets[] = {2, 3};	/* DVS1 and DVS2 */
> +static unsigned int bd72720_bit2_offsets[] = {4};	/* VBUS */
> +static unsigned int bd72720_bit3_offsets[] = {5};	/* VSYS */
> +static unsigned int bd72720_bit4_offsets[] = {6};	/* CHG */
> +static unsigned int bd72720_bit5_offsets[] = {7, 8};	/* BAT1 and BAT2 */
> +static unsigned int bd72720_bit6_offsets[] = {9};	/* IBAT */
> +static unsigned int bd72720_bit7_offsets[] = {10, 11};	/* ETC1 and ETC2 */
> +
>  static const struct regmap_irq_sub_irq_map bd718xx_sub_irq_offsets[] = {
>  	REGMAP_IRQ_MAIN_REG_OFFSET(bit0_offsets),
>  	REGMAP_IRQ_MAIN_REG_OFFSET(bit1_offsets),
> @@ -232,6 +497,17 @@ static const struct regmap_irq_sub_irq_map bd718xx_sub_irq_offsets[] = {
>  	REGMAP_IRQ_MAIN_REG_OFFSET(bit7_offsets),
>  };
>  
> +static const struct regmap_irq_sub_irq_map bd72720_sub_irq_offsets[] = {
> +	REGMAP_IRQ_MAIN_REG_OFFSET(bd72720_bit0_offsets),
> +	REGMAP_IRQ_MAIN_REG_OFFSET(bd72720_bit1_offsets),
> +	REGMAP_IRQ_MAIN_REG_OFFSET(bd72720_bit2_offsets),
> +	REGMAP_IRQ_MAIN_REG_OFFSET(bd72720_bit3_offsets),
> +	REGMAP_IRQ_MAIN_REG_OFFSET(bd72720_bit4_offsets),
> +	REGMAP_IRQ_MAIN_REG_OFFSET(bd72720_bit5_offsets),
> +	REGMAP_IRQ_MAIN_REG_OFFSET(bd72720_bit6_offsets),
> +	REGMAP_IRQ_MAIN_REG_OFFSET(bd72720_bit7_offsets),
> +};
> +
>  static const struct regmap_irq bd71815_irqs[] = {
>  	REGMAP_IRQ_REG(BD71815_INT_BUCK1_OCP, 0, BD71815_INT_BUCK1_OCP_MASK),
>  	REGMAP_IRQ_REG(BD71815_INT_BUCK2_OCP, 0, BD71815_INT_BUCK2_OCP_MASK),
> @@ -405,6 +681,117 @@ static const struct regmap_irq bd71828_irqs[] = {
>  	REGMAP_IRQ_REG(BD71828_INT_RTC2, 11, BD71828_INT_RTC2_MASK),
>  };
>  
> +static const struct regmap_irq bd72720_irqs[] = {
> +	REGMAP_IRQ_REG(BD72720_INT_LONGPUSH, 0, BD72720_INT_LONGPUSH_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_MIDPUSH, 0, BD72720_INT_MIDPUSH_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_SHORTPUSH, 0, BD72720_INT_SHORTPUSH_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_PUSH, 0, BD72720_INT_PUSH_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_HALL_DET, 0, BD72720_INT_HALL_DET_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_HALL_TGL, 0, BD72720_INT_HALL_TGL_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_WDOG, 0, BD72720_INT_WDOG_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_SWRESET, 0, BD72720_INT_SWRESET_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_SEQ_DONE, 1, BD72720_INT_SEQ_DONE_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_PGFAULT, 1, BD72720_INT_PGFAULT_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_BUCK1_DVS, 2, BD72720_INT_BUCK1_DVS_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_BUCK2_DVS, 2, BD72720_INT_BUCK2_DVS_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_BUCK3_DVS, 2, BD72720_INT_BUCK3_DVS_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_BUCK4_DVS, 2, BD72720_INT_BUCK4_DVS_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_BUCK5_DVS, 2, BD72720_INT_BUCK5_DVS_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_BUCK6_DVS, 2, BD72720_INT_BUCK6_DVS_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_BUCK7_DVS, 2, BD72720_INT_BUCK7_DVS_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_BUCK8_DVS, 2, BD72720_INT_BUCK8_DVS_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_BUCK9_DVS, 3, BD72720_INT_BUCK9_DVS_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_BUCK10_DVS, 3, BD72720_INT_BUCK10_DVS_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_LDO1_DVS, 3, BD72720_INT_LDO1_DVS_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_LDO2_DVS, 3, BD72720_INT_LDO2_DVS_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_LDO3_DVS, 3, BD72720_INT_LDO3_DVS_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_LDO4_DVS, 3, BD72720_INT_LDO4_DVS_MASK),
> +
> +	REGMAP_IRQ_REG(BD72720_INT_VBUS_RMV, 4, BD72720_INT_VBUS_RMV_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_VBUS_DET, 4, BD72720_INT_VBUS_DET_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_VBUS_MON_RES, 4, BD72720_INT_VBUS_MON_RES_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_VBUS_MON_DET, 4, BD72720_INT_VBUS_MON_DET_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_VSYS_MON_RES, 5, BD72720_INT_VSYS_MON_RES_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_VSYS_MON_DET, 5, BD72720_INT_VSYS_MON_DET_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_VSYS_UV_RES, 5, BD72720_INT_VSYS_UV_RES_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_VSYS_UV_DET, 5, BD72720_INT_VSYS_UV_DET_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_VSYS_LO_RES, 5, BD72720_INT_VSYS_LO_RES_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_VSYS_LO_DET, 5, BD72720_INT_VSYS_LO_DET_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_VSYS_OV_RES, 5, BD72720_INT_VSYS_OV_RES_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_VSYS_OV_DET, 5, BD72720_INT_VSYS_OV_DET_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_BAT_ILIM, 6, BD72720_INT_BAT_ILIM_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_CHG_DONE, 6, BD72720_INT_CHG_DONE_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_EXTEMP_TOUT, 6, BD72720_INT_EXTEMP_TOUT_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_CHG_WDT_EXP, 6, BD72720_INT_CHG_WDT_EXP_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_BAT_MNT_OUT, 6, BD72720_INT_BAT_MNT_OUT_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_BAT_MNT_IN, 6, BD72720_INT_BAT_MNT_IN_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_CHG_TRNS, 6, BD72720_INT_CHG_TRNS_MASK),
> +
> +	REGMAP_IRQ_REG(BD72720_INT_VBAT_MON_RES, 7, BD72720_INT_VBAT_MON_RES_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_VBAT_MON_DET, 7, BD72720_INT_VBAT_MON_DET_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_VBAT_SHT_RES, 7, BD72720_INT_VBAT_SHT_RES_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_VBAT_SHT_DET, 7, BD72720_INT_VBAT_SHT_DET_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_VBAT_LO_RES, 7, BD72720_INT_VBAT_LO_RES_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_VBAT_LO_DET, 7, BD72720_INT_VBAT_LO_DET_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_VBAT_OV_RES, 7, BD72720_INT_VBAT_OV_RES_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_VBAT_OV_DET, 7, BD72720_INT_VBAT_OV_DET_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_BAT_RMV, 8, BD72720_INT_BAT_RMV_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_BAT_DET, 8, BD72720_INT_BAT_DET_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_DBAT_DET, 8, BD72720_INT_DBAT_DET_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_BAT_TEMP_TRNS, 8, BD72720_INT_BAT_TEMP_TRNS_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_LOBTMP_RES, 8, BD72720_INT_LOBTMP_RES_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_LOBTMP_DET, 8, BD72720_INT_LOBTMP_DET_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_OVBTMP_RES, 8, BD72720_INT_OVBTMP_RES_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_OVBTMP_DET, 8, BD72720_INT_OVBTMP_DET_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_OCUR1_RES, 9, BD72720_INT_OCUR1_RES_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_OCUR1_DET, 9, BD72720_INT_OCUR1_DET_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_OCUR2_RES, 9, BD72720_INT_OCUR2_RES_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_OCUR2_DET, 9, BD72720_INT_OCUR2_DET_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_OCUR3_RES, 9, BD72720_INT_OCUR3_RES_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_OCUR3_DET, 9, BD72720_INT_OCUR3_DET_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_CC_MON1_DET, 10, BD72720_INT_CC_MON1_DET_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_CC_MON2_DET, 10, BD72720_INT_CC_MON2_DET_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_CC_MON3_DET, 10, BD72720_INT_CC_MON3_DET_MASK),
> +/*
> + * The GPIO1_IN and GPIO2_IN IRQs are generated from the PMIC's GPIO1 and GPIO2
> + * pins. Eg, they may be wired to other devices which can then use the PMIC as
> + * an interrupt controller. The GPIO1 and GPIO2 can have the IRQ type
> + * specified. All of the types (falling, rising, and both edges as well as low
> + * and high levels) are supported.
> + */
> +	BD72720_TYPED_IRQ_REG(BD72720_INT_GPIO1_IN, 10, BD72720_INT_GPIO1_IN_MASK, 0),
> +	BD72720_TYPED_IRQ_REG(BD72720_INT_GPIO2_IN, 10, BD72720_INT_GPIO2_IN_MASK, 1),
> +	REGMAP_IRQ_REG(BD72720_INT_VF125_RES, 11, BD72720_INT_VF125_RES_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_VF125_DET, 11, BD72720_INT_VF125_DET_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_VF_RES, 11, BD72720_INT_VF_RES_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_VF_DET, 11, BD72720_INT_VF_DET_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_RTC0, 11, BD72720_INT_RTC0_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_RTC1, 11, BD72720_INT_RTC1_MASK),
> +	REGMAP_IRQ_REG(BD72720_INT_RTC2, 11, BD72720_INT_RTC2_MASK),
> +};
> +
> +static int bd72720_set_type_config(unsigned int **buf, unsigned int type,
> +				   const struct regmap_irq *irq_data,
> +				   int idx, void *irq_drv_data)
> +{
> +	const struct regmap_irq_type *t = &irq_data->type;
> +
> +	/*
> +	 * The regmap IRQ ecpects IRQ_TYPE_EDGE_BOTH to be written to register
> +	 * as logical OR of the type_falling_val and type_rising_val. This is
> +	 * not how the BD72720 implements this configuration, hence we need
> +	 * to handle this specific case separately.
> +	 */
> +	if (type == IRQ_TYPE_EDGE_BOTH) {
> +		buf[0][idx] &= ~t->type_reg_mask;
> +		buf[0][idx] |= BD72720_GPIO_IRQ_TYPE_BOTH;
> +
> +		return 0;
> +	}
> +
> +	return regmap_irq_set_type_config_simple(buf, type, irq_data, idx, irq_drv_data);
> +}
> +
>  static const struct regmap_irq_chip bd71828_irq_chip = {
>  	.name = "bd71828_irq",
>  	.main_status = BD71828_REG_INT_MAIN,
> @@ -437,6 +824,28 @@ static const struct regmap_irq_chip bd71815_irq_chip = {
>  	.irq_reg_stride = 1,
>  };
>  
> +static const unsigned int bd72720_irq_type_base[] = {BD72720_REG_GPIO1_CTRL};

I believe this should be: { BD72720_REG_GPIO1_CTRL };
> +
> +static const struct regmap_irq_chip bd72720_irq_chip = {
> +	.name = "bd72720_irq",
> +	.main_status = BD72720_REG_INT_LVL1_STAT,
> +	.irqs = &bd72720_irqs[0],
> +	.num_irqs = ARRAY_SIZE(bd72720_irqs),
> +	.status_base = BD72720_REG_INT_PS1_STAT,
> +	.unmask_base = BD72720_REG_INT_PS1_EN,
> +	.config_base = &bd72720_irq_type_base[0],
> +	.num_config_bases = 1,
> +	.num_config_regs = 2,
> +	.set_type_config = bd72720_set_type_config,
> +	.ack_base = BD72720_REG_INT_PS1_STAT,
> +	.init_ack_masked = true,
> +	.num_regs = 12,
> +	.num_main_regs = 1,
> +	.sub_reg_offsets = &bd72720_sub_irq_offsets[0],
> +	.num_main_status_bits = 8,
> +	.irq_reg_stride = 1,
> +};
> +
>  static int set_clk_mode(struct device *dev, struct regmap *regmap,
>  			int clkmode_reg)
>  {
> @@ -483,11 +892,40 @@ static void bd71828_remove_poweroff(void *data)
>  	pm_power_off = NULL;
>  }
>  
> +static struct regmap *bd72720_do_regmaps(struct i2c_client *i2c)
> +{
> +	struct bd72720_regmaps *maps;
> +	struct i2c_client *secondary_i2c;
> +
> +	secondary_i2c = devm_i2c_new_dummy_device(&i2c->dev, i2c->adapter,
> +						  BD72720_SECONDARY_I2C_SLAVE);
> +	if (IS_ERR(secondary_i2c)) {
> +		dev_err_probe(&i2c->dev, PTR_ERR(secondary_i2c),
> +			      "Failed to get secondary I2C\n");

You an use up to 100-char if it helps make things more readable.

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

> +	}
> +
> +	maps = devm_kzalloc(&i2c->dev, sizeof(*maps), GFP_KERNEL);
> +	if (!maps)
> +		return ERR_PTR(-ENOMEM);
> +
> +	maps->map1_4b = devm_regmap_init_i2c(i2c, &bd72720_regmap_4b);
> +	if (IS_ERR(maps->map1_4b))
> +		return maps->map1_4b;
> +
> +	maps->map2_4c = devm_regmap_init_i2c(secondary_i2c, &bd72720_regmap_4c);
> +	if (IS_ERR(maps->map2_4c))
> +		return maps->map2_4c;
> +
> +	return devm_regmap_init(&i2c->dev, NULL, maps, &bd72720_wrapper_map_config);
> +}
> +
>  static int bd71828_i2c_probe(struct i2c_client *i2c)
>  {
>  	struct regmap_irq_chip_data *irq_data;
>  	int ret;
> -	struct regmap *regmap;
> +	struct regmap *regmap = NULL;
>  	const struct regmap_config *regmap_config;
>  	const struct regmap_irq_chip *irqchip;
>  	unsigned int chip_type;
> @@ -495,6 +933,7 @@ static int bd71828_i2c_probe(struct i2c_client *i2c)
>  	int cells;
>  	int button_irq;
>  	int clkmode_reg;
> +	int main_lvl_mask_reg = 0, main_lvl_val = 0;
>  
>  	if (!i2c->irq) {
>  		dev_err(&i2c->dev, "No IRQ configured\n");
> @@ -526,16 +965,34 @@ static int bd71828_i2c_probe(struct i2c_client *i2c)
>  		 */
>  		button_irq = 0;
>  		break;
> +	case ROHM_CHIP_TYPE_BD72720:
> +	{
> +		mfd = bd72720_mfd_cells;
> +		cells = ARRAY_SIZE(bd72720_mfd_cells);
> +
> +		regmap = bd72720_do_regmaps(i2c);
> +		if (IS_ERR(regmap))
> +			return dev_err_probe(&i2c->dev, PTR_ERR(regmap),
> +				     "Failed to initialize Regmap\n");
> +
> +		irqchip = &bd72720_irq_chip;
> +		clkmode_reg = BD72720_REG_OUT32K;
> +		button_irq = BD72720_INT_SHORTPUSH;
> +		main_lvl_mask_reg = BD72720_REG_INT_LVL1_EN;
> +		main_lvl_val = BD72720_MASK_LVL1_EN_ALL;
> +		break;
> +	}
>  	default:
>  		dev_err(&i2c->dev, "Unknown device type");
>  		return -EINVAL;
>  	}
>  
> -	regmap = devm_regmap_init_i2c(i2c, regmap_config);
> -	if (IS_ERR(regmap))
> -		return dev_err_probe(&i2c->dev, PTR_ERR(regmap),
> +	if (!regmap) {
> +		regmap = devm_regmap_init_i2c(i2c, regmap_config);
> +		if (IS_ERR(regmap))
> +			return dev_err_probe(&i2c->dev, PTR_ERR(regmap),
>  				     "Failed to initialize Regmap\n");
> -
> +	}

'\n'

>  	ret = devm_regmap_add_irq_chip(&i2c->dev, regmap, i2c->irq,
>  				       IRQF_ONESHOT, 0, irqchip, &irq_data);
>  	if (ret)
> @@ -545,6 +1002,20 @@ static int bd71828_i2c_probe(struct i2c_client *i2c)
>  	dev_dbg(&i2c->dev, "Registered %d IRQs for chip\n",
>  		irqchip->num_irqs);
>  
> +	/*
> +	 * On some ICs the main IRQ register has corresponding mask register.
> +	 * This is not handled by the regmap IRQ. Let's enable all the main
> +	 * level IRQs here. Further writes to the main level MASK is not
> +	 * needed because masking is handled by the per IRQ 2.nd level MASK
> +	 * registers. 2.nd level masks are handled by the regmap IRQ.
> +	 */
> +	if (main_lvl_mask_reg) {
> +		ret = regmap_write(regmap, main_lvl_mask_reg, main_lvl_val);
> +		if (ret) {
> +			return dev_err_probe(&i2c->dev, ret,
> +					"Failed to enable main level IRQs\n");
> +		}
> +	}
>  	if (button_irq) {
>  		ret = regmap_irq_get_virq(irq_data, button_irq);
>  		if (ret < 0)
> @@ -586,6 +1057,9 @@ static const struct of_device_id bd71828_of_match[] = {
>  	}, {
>  		.compatible = "rohm,bd71815",
>  		.data = (void *)ROHM_CHIP_TYPE_BD71815,
> +	}, {
> +		.compatible = "rohm,bd72720",
> +		.data = (void *)ROHM_CHIP_TYPE_BD72720,
>  	 },
>  	{ },
>  };
> 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.

> + * Author: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> + */
> +
> +#ifndef _MFD_BD72720_H
> +#define _MFD_BD72720_H
> +
> +#include <linux/regmap.h>
> +
> +enum {
> +	BD72720_BUCK1,
> +	BD72720_BUCK2,
> +	BD72720_BUCK3,
> +	BD72720_BUCK4,
> +	BD72720_BUCK5,
> +	BD72720_BUCK6,
> +	BD72720_BUCK7,
> +	BD72720_BUCK8,
> +	BD72720_BUCK9,
> +	BD72720_BUCK10,
> +	BD72720_BUCK11,
> +	BD72720_LDO1,
> +	BD72720_LDO2,
> +	BD72720_LDO3,
> +	BD72720_LDO4,
> +	BD72720_LDO5,
> +	BD72720_LDO6,
> +	BD72720_LDO7,
> +	BD72720_LDO8,
> +	BD72720_LDO9,
> +	BD72720_LDO10,
> +	BD72720_LDO11,
> +	BD72720_REGULATOR_AMOUNT,
> +};
> +
> +/* 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.

> +
> +enum {
> +	/*
> +	 * The IRQs excluding GPIO1 and GPIO2 are ordered in a same way as the
> +	 * respective IRQ bits in status and mask registers are ordered.
> +	 *
> +	 * The BD72720_INT_GPIO1_IN and BD72720_INT_GPIO2_IN are IRQs which can
> +	 * be used by other devices. Let's have  GPIO1 and GPIO2 as first IRQs
> +	 * here so we can use the regmap-IRQ with standard device tree xlate
> +	 * while devices connected to the BD72720 IRQ input pins can refer to
> +	 * the first two interrupt numbers in their device tree. If we placed
> +	 * BD72720_INT_GPIO1_IN and BD72720_INT_GPIO2_IN after the CC_MON_DET
> +	 * interrupts (like they are in the registers), the devices using
> +	 * BD72720 as an IRQ parent should refer the interrupts starting with
> +	 * an offset which might not be trivial to understand.
> +	 */
> +	BD72720_INT_GPIO1_IN,
> +	BD72720_INT_GPIO2_IN,
> +	BD72720_INT_LONGPUSH,
> +	BD72720_INT_MIDPUSH,
> +	BD72720_INT_SHORTPUSH,
> +	BD72720_INT_PUSH,
> +	BD72720_INT_HALL_DET,
> +	BD72720_INT_HALL_TGL,
> +	BD72720_INT_WDOG,
> +	BD72720_INT_SWRESET,
> +	BD72720_INT_SEQ_DONE,
> +	BD72720_INT_PGFAULT,
> +	BD72720_INT_BUCK1_DVS,
> +	BD72720_INT_BUCK2_DVS,
> +	BD72720_INT_BUCK3_DVS,
> +	BD72720_INT_BUCK4_DVS,
> +	BD72720_INT_BUCK5_DVS,
> +	BD72720_INT_BUCK6_DVS,
> +	BD72720_INT_BUCK7_DVS,
> +	BD72720_INT_BUCK8_DVS,
> +	BD72720_INT_BUCK9_DVS,
> +	BD72720_INT_BUCK10_DVS,
> +	BD72720_INT_LDO1_DVS,
> +	BD72720_INT_LDO2_DVS,
> +	BD72720_INT_LDO3_DVS,
> +	BD72720_INT_LDO4_DVS,
> +	BD72720_INT_VBUS_RMV,
> +	BD72720_INT_VBUS_DET,
> +	BD72720_INT_VBUS_MON_RES,
> +	BD72720_INT_VBUS_MON_DET,
> +	BD72720_INT_VSYS_MON_RES,
> +	BD72720_INT_VSYS_MON_DET,
> +	BD72720_INT_VSYS_UV_RES,
> +	BD72720_INT_VSYS_UV_DET,
> +	BD72720_INT_VSYS_LO_RES,
> +	BD72720_INT_VSYS_LO_DET,
> +	BD72720_INT_VSYS_OV_RES,
> +	BD72720_INT_VSYS_OV_DET,
> +	BD72720_INT_BAT_ILIM,
> +	BD72720_INT_CHG_DONE,
> +	BD72720_INT_EXTEMP_TOUT,
> +	BD72720_INT_CHG_WDT_EXP,
> +	BD72720_INT_BAT_MNT_OUT,
> +	BD72720_INT_BAT_MNT_IN,
> +	BD72720_INT_CHG_TRNS,
> +	BD72720_INT_VBAT_MON_RES,
> +	BD72720_INT_VBAT_MON_DET,
> +	BD72720_INT_VBAT_SHT_RES,
> +	BD72720_INT_VBAT_SHT_DET,
> +	BD72720_INT_VBAT_LO_RES,
> +	BD72720_INT_VBAT_LO_DET,
> +	BD72720_INT_VBAT_OV_RES,
> +	BD72720_INT_VBAT_OV_DET,
> +	BD72720_INT_BAT_RMV,
> +	BD72720_INT_BAT_DET,
> +	BD72720_INT_DBAT_DET,
> +	BD72720_INT_BAT_TEMP_TRNS,
> +	BD72720_INT_LOBTMP_RES,
> +	BD72720_INT_LOBTMP_DET,
> +	BD72720_INT_OVBTMP_RES,
> +	BD72720_INT_OVBTMP_DET,
> +	BD72720_INT_OCUR1_RES,
> +	BD72720_INT_OCUR1_DET,
> +	BD72720_INT_OCUR2_RES,
> +	BD72720_INT_OCUR2_DET,
> +	BD72720_INT_OCUR3_RES,
> +	BD72720_INT_OCUR3_DET,
> +	BD72720_INT_CC_MON1_DET,
> +	BD72720_INT_CC_MON2_DET,
> +	BD72720_INT_CC_MON3_DET,
> +	BD72720_INT_VF125_RES,
> +	BD72720_INT_VF125_DET,
> +	BD72720_INT_VF_RES,
> +	BD72720_INT_VF_DET,
> +	BD72720_INT_RTC0,
> +	BD72720_INT_RTC1,
> +	BD72720_INT_RTC2,
> +};
> +
> +/*
> + * BD72720 Registers:
> + * The BD72720 has two sets of registers behind two different I2C slave
> + * addresses. "Common" registers being behind 0x4b, the charger registers
> + * being behind 0x4c.
> + */
> +/* Registers behind I2C slave 0x4b */
> +enum {
> +	BD72720_REG_PRODUCT_ID,
> +	BD72720_REG_MANUFACTURER_ID,
> +	BD72720_REG_PMIC_REV_NUM,
> +	BD72720_REG_NVM_REV_NUM,
> +	BD72720_REG_BOOTSRC		= 0x10,
> +	BD72720_REG_RESETSRC_1,
> +	BD72720_REG_RESETSRC_2,
> +	BD72720_REG_RESETSRC_3,
> +	BD72720_REG_RESETSRC_4,
> +	BD72720_REG_RESETSRC_5,
> +	BD72720_REG_RESETSRC_6,
> +	BD72720_REG_RESETSRC_7,
> +	BD72720_REG_POWER_STATE,
> +	BD72720_REG_PS_CFG,
> +	BD72720_REG_PS_CTRL_1,
> +	BD72720_REG_PS_CTRL_2,
> +	BD72720_REG_RCVCFG,
> +	BD72720_REG_RCVNUM,
> +	BD72720_REG_CRDCFG,
> +	BD72720_REG_REX_CTRL,
> +
> +	BD72720_REG_BUCK1_ON,
> +	BD72720_REG_BUCK1_MODE,
> +	/* Deep idle vsel */
> +	BD72720_REG_BUCK1_VSEL_DI,
> +	/* Idle vsel */
> +	BD72720_REG_BUCK1_VSEL_I,
> +	/* Suspend vsel */
> +	BD72720_REG_BUCK1_VSEL_S,
> +	/* Run boot vsel */
> +	BD72720_REG_BUCK1_VSEL_RB,
> +	/* Run0 ... run3 vsel */
> +	BD72720_REG_BUCK1_VSEL_RB0,
> +	BD72720_REG_BUCK1_VSEL_RB1,
> +	BD72720_REG_BUCK1_VSEL_RB2,
> +	BD72720_REG_BUCK1_VSEL_RB3,
> +
> +	BD72720_REG_BUCK2_ON,
> +	BD72720_REG_BUCK2_MODE,
> +	BD72720_REG_BUCK2_VSEL_DI,
> +	BD72720_REG_BUCK2_VSEL_I,
> +	BD72720_REG_BUCK2_VSEL_S,
> +	/* Run vsel */
> +	BD72720_REG_BUCK2_VSEL_R,
> +
> +	BD72720_REG_BUCK3_ON,
> +	BD72720_REG_BUCK3_MODE,
> +	BD72720_REG_BUCK3_VSEL_DI,
> +	BD72720_REG_BUCK3_VSEL_I,
> +	BD72720_REG_BUCK3_VSEL_S,
> +	BD72720_REG_BUCK3_VSEL_R,
> +
> +	BD72720_REG_BUCK4_ON,
> +	BD72720_REG_BUCK4_MODE,
> +	BD72720_REG_BUCK4_VSEL_DI,
> +	BD72720_REG_BUCK4_VSEL_I,
> +	BD72720_REG_BUCK4_VSEL_S,
> +	BD72720_REG_BUCK4_VSEL_R,
> +
> +	BD72720_REG_BUCK5_ON,
> +	BD72720_REG_BUCK5_MODE,
> +	BD72720_REG_BUCK5_VSEL,
> +
> +	BD72720_REG_BUCK6_ON,
> +	BD72720_REG_BUCK6_MODE,
> +	BD72720_REG_BUCK6_VSEL,
> +
> +	BD72720_REG_BUCK7_ON,
> +	BD72720_REG_BUCK7_MODE,
> +	BD72720_REG_BUCK7_VSEL,
> +
> +	BD72720_REG_BUCK8_ON,
> +	BD72720_REG_BUCK8_MODE,
> +	BD72720_REG_BUCK8_VSEL,
> +
> +	BD72720_REG_BUCK9_ON,
> +	BD72720_REG_BUCK9_MODE,
> +	BD72720_REG_BUCK9_VSEL,
> +
> +	BD72720_REG_BUCK10_ON,
> +	BD72720_REG_BUCK10_MODE,
> +	BD72720_REG_BUCK10_VSEL,
> +
> +	BD72720_REG_LDO1_ON,
> +	BD72720_REG_LDO1_MODE1,
> +	BD72720_REG_LDO1_MODE2,
> +	BD72720_REG_LDO1_VSEL_DI,
> +	BD72720_REG_LDO1_VSEL_I,
> +	BD72720_REG_LDO1_VSEL_S,
> +	BD72720_REG_LDO1_VSEL_RB,
> +	BD72720_REG_LDO1_VSEL_R0,
> +	BD72720_REG_LDO1_VSEL_R1,
> +	BD72720_REG_LDO1_VSEL_R2,
> +	BD72720_REG_LDO1_VSEL_R3,
> +
> +	BD72720_REG_LDO2_ON,
> +	BD72720_REG_LDO2_MODE,
> +	BD72720_REG_LDO2_VSEL_DI,
> +	BD72720_REG_LDO2_VSEL_I,
> +	BD72720_REG_LDO2_VSEL_S,
> +	BD72720_REG_LDO2_VSEL_R,
> +
> +	BD72720_REG_LDO3_ON,
> +	BD72720_REG_LDO3_MODE,
> +	BD72720_REG_LDO3_VSEL_DI,
> +	BD72720_REG_LDO3_VSEL_I,
> +	BD72720_REG_LDO3_VSEL_S,
> +	BD72720_REG_LDO3_VSEL_R,
> +
> +	BD72720_REG_LDO4_ON,
> +	BD72720_REG_LDO4_MODE,
> +	BD72720_REG_LDO4_VSEL_DI,
> +	BD72720_REG_LDO4_VSEL_I,
> +	BD72720_REG_LDO4_VSEL_S,
> +	BD72720_REG_LDO4_VSEL_R,
> +
> +	BD72720_REG_LDO5_ON,
> +	BD72720_REG_LDO5_MODE,
> +	BD72720_REG_LDO5_VSEL,
> +
> +	BD72720_REG_LDO6_ON,
> +	BD72720_REG_LDO6_MODE,
> +	BD72720_REG_LDO6_VSEL,
> +
> +	BD72720_REG_LDO7_ON,
> +	BD72720_REG_LDO7_MODE,
> +	BD72720_REG_LDO7_VSEL,
> +
> +	BD72720_REG_LDO8_ON,
> +	BD72720_REG_LDO8_MODE,
> +	BD72720_REG_LDO8_VSEL,
> +
> +	BD72720_REG_LDO9_ON,
> +	BD72720_REG_LDO9_MODE,
> +	BD72720_REG_LDO9_VSEL,
> +
> +	BD72720_REG_LDO10_ON,
> +	BD72720_REG_LDO10_MODE,
> +	BD72720_REG_LDO10_VSEL,
> +
> +	BD72720_REG_LDO11_ON,
> +	BD72720_REG_LDO11_MODE,
> +	BD72720_REG_LDO11_VSEL,
> +
> +	BD72720_REG_GPIO1_ON		= 0x8b,
> +	BD72720_REG_GPIO2_ON,
> +	BD72720_REG_GPIO3_ON,
> +	BD72720_REG_GPIO4_ON,
> +	BD72720_REG_GPIO5_ON,
> +
> +	BD72720_REG_GPIO1_CTRL,
> +	BD72720_REG_GPIO2_CTRL,
> +#define BD72720_GPIO_IRQ_TYPE_MASK	GENMASK(6, 4)
> +#define BD72720_GPIO_IRQ_TYPE_FALLING	0x0
> +#define BD72720_GPIO_IRQ_TYPE_RISING	0x1
> +#define BD72720_GPIO_IRQ_TYPE_BOTH	0x2
> +#define BD72720_GPIO_IRQ_TYPE_HIGH	0x3
> +#define BD72720_GPIO_IRQ_TYPE_LOW	0x4
> +	BD72720_REG_GPIO3_CTRL,
> +	BD72720_REG_GPIO4_CTRL,
> +	BD72720_REG_GPIO5_CTRL,
> +#define BD72720_GPIO_DRIVE_MASK		BIT(1)
> +#define BD72720_GPIO_HIGH		BIT(0)
> +
> +	BD72720_REG_EPDEN_CTRL,
> +	BD72720_REG_GATECNT_CTRL,
> +	BD72720_REG_LED_CTRL,
> +
> +	BD72720_REG_PWRON_CFG1,
> +	BD72720_REG_PWRON_CFG2,
> +
> +	BD72720_REG_OUT32K,
> +	BD72720_REG_CONF,
> +	BD72720_REG_HALL_STAT,
> +
> +	BD72720_REG_RTC_SEC		= 0xa0,
> +#define BD72720_REG_RTC_START		BD72720_REG_RTC_SEC
> +	BD72720_REG_RTC_MIN,
> +	BD72720_REG_RTC_HOUR,
> +	BD72720_REG_RTC_WEEK,
> +	BD72720_REG_RTC_DAY,
> +	BD72720_REG_RTC_MON,
> +	BD72720_REG_RTC_YEAR,
> +
> +	BD72720_REG_RTC_ALM0_SEC,
> +#define BD72720_REG_RTC_ALM_START	BD72720_REG_RTC_ALM0_SEC
> +	BD72720_REG_RTC_ALM0_MIN,
> +	BD72720_REG_RTC_ALM0_HOUR,
> +	BD72720_REG_RTC_ALM0_WEEK,
> +	BD72720_REG_RTC_ALM0_MON,
> +	BD72720_REG_RTC_ALM0_YEAR,
> +
> +	BD72720_REG_RTC_ALM1_SEC,
> +	BD72720_REG_RTC_ALM1_MIN,
> +	BD72720_REG_RTC_ALM1_HOUR,
> +	BD72720_REG_RTC_ALM1_WEEK,
> +	BD72720_REG_RTC_ALM1_MON,
> +	BD72720_REG_RTC_ALM1_YEAR,
> +
> +	BD72720_REG_RTC_ALM0_EN,
> +	BD72720_REG_RTC_ALM1_EN,
> +	BD72720_REG_RTC_ALM2,
> +
> +	BD72720_REG_INT_LVL1_EN		= 0xc0,
> +#define BD72720_MASK_LVL1_EN_ALL	GENMASK(7, 0)
> +	BD72720_REG_INT_PS1_EN,
> +	BD72720_REG_INT_PS2_EN,
> +	BD72720_REG_INT_DVS1_EN,
> +	BD72720_REG_INT_DVS2_EN,
> +	BD72720_REG_INT_VBUS_EN,
> +	BD72720_REG_INT_VSYS_EN,
> +	BD72720_REG_INT_CHG_EN,
> +	BD72720_REG_INT_BAT1_EN,
> +	BD72720_REG_INT_BAT2_EN,
> +	BD72720_REG_INT_IBAT_EN,
> +	BD72720_REG_INT_ETC1_EN,
> +	BD72720_REG_INT_ETC2_EN,
> +
> +	/*
> +	 * The _STAT registers inform IRQ line state, and are used to ack IRQ.
> +	 * The _SRC registers below indicate current state of the function
> +	 * connected to the line.
> +	 */
> +	BD72720_REG_INT_LVL1_STAT,
> +	BD72720_REG_INT_PS1_STAT,
> +	BD72720_REG_INT_PS2_STAT,
> +	BD72720_REG_INT_DVS1_STAT,
> +	BD72720_REG_INT_DVS2_STAT,
> +	BD72720_REG_INT_VBUS_STAT,
> +	BD72720_REG_INT_VSYS_STAT,
> +	BD72720_REG_INT_CHG_STAT,
> +	BD72720_REG_INT_BAT1_STAT,
> +	BD72720_REG_INT_BAT2_STAT,
> +	BD72720_REG_INT_IBAT_STAT,
> +	BD72720_REG_INT_ETC1_STAT,
> +	BD72720_REG_INT_ETC2_STAT,
> +
> +	BD72720_REG_INT_LVL1_SRC,
> +	BD72720_REG_INT_PS1_SRC,
> +	BD72720_REG_INT_PS2_SRC,
> +	BD72720_REG_INT_DVS1_SRC,
> +	BD72720_REG_INT_DVS2_SRC,
> +	BD72720_REG_INT_VBUS_SRC,
> +#define BD72720_MASK_DCIN_DET	BIT(1)
> +	BD72720_REG_INT_VSYS_SRC,
> +	BD72720_REG_INT_CHG_SRC,
> +	BD72720_REG_INT_BAT1_SRC,
> +	BD72720_REG_INT_BAT2_SRC,
> +	BD72720_REG_INT_IBAT_SRC,
> +	BD72720_REG_INT_ETC1_SRC,
> +	BD72720_REG_INT_ETC2_SRC,
> +};
> +
> +/* Register masks */
> +#define BD72720_MASK_DEEP_IDLE_EN	BIT(0)
> +#define BD72720_MASK_IDLE_EN		BIT(1)
> +#define BD72720_MASK_SUSPEND_EN		BIT(2)
> +#define BD72720_MASK_RUN_B_EN		BIT(3)
> +#define BD72720_MASK_RUN_0_EN		BIT(4)
> +#define BD72720_MASK_RUN_1_EN		BIT(5)
> +#define BD72720_MASK_RUN_2_EN		BIT(6)
> +#define BD72720_MASK_RUN_3_EN		BIT(7)
> +
> +#define BD72720_MASK_RAMP_UP_DELAY	GENMASK(7, 6)
> +#define BD72720_MASK_BUCK_VSEL		GENMASK(7, 0)
> +#define BD72720_MASK_LDO12346_VSEL	GENMASK(6, 0)
> +#define BD72720_MASK_LDO_VSEL		GENMASK(7, 0)
> +
> +#define BD72720_I2C4C_ADDR_OFFSET	0x100
> +
> +/* Registers behind I2C slave 0x4c */
> +enum {
> +	BD72720_REG_CHG_STATE = BD72720_I2C4C_ADDR_OFFSET,
> +	BD72720_REG_CHG_LAST_STATE,
> +	BD72720_REG_CHG_VBUS_STAT,
> +	BD72720_REG_CHG_VSYS_STAT,
> +	BD72720_REG_CHG_BAT_TEMP_STAT,
> +	BD72720_REG_CHG_WDT_STAT,
> +	BD72720_REG_CHG_ILIM_STAT,
> +	BD72720_REG_CHG_CHG_STAT,
> +	BD72720_REG_CHG_EN,
> +	BD72720_REG_CHG_INIT,
> +	BD72720_REG_CHG_CTRL,
> +	BD72720_REG_CHG_SET_1,
> +	BD72720_REG_CHG_SET_2,
> +	BD72720_REG_CHG_SET_3,
> +	BD72720_REG_CHG_VPRE,
> +	BD72720_REG_CHG_VBAT_1,
> +	BD72720_REG_CHG_VBAT_2,
> +	BD72720_REG_CHG_VBAT_3,
> +	BD72720_REG_CHG_VBAT_4,
> +	BD72720_REG_CHG_BAT_SET_1,
> +	BD72720_REG_CHG_BAT_SET_2,
> +	BD72720_REG_CHG_BAT_SET_3,
> +	BD72720_REG_CHG_IPRE,
> +	BD72720_REG_CHG_IFST_TERM,
> +	BD72720_REG_CHG_VSYS_REG,
> +	BD72720_REG_CHG_VBUS_SET,
> +	BD72720_REG_CHG_WDT_PRE,
> +	BD72720_REG_CHG_WDT_FST,
> +	BD72720_REG_CHG_LED_CTRL,
> +	BD72720_REG_CHG_CFG_1,
> +	BD72720_REG_CHG_IFST_1,
> +	BD72720_REG_CHG_IFST_2,
> +	BD72720_REG_CHG_IFST_3,
> +	BD72720_REG_CHG_IFST_4,
> +	BD72720_REG_CHG_S_CFG_1,
> +	BD72720_REG_CHG_S_CFG_2,
> +	BD72720_REG_RS_VBUS,
> +	BD72720_REG_RS_IBUS,
> +	BD72720_REG_RS_VSYS,
> +	BD72720_REG_VSYS_STATE_STAT,	/* 0x27 + offset*/
> +
> +	BD72720_REG_VM_VBAT_U		= BD72720_I2C4C_ADDR_OFFSET + 0x30,
> +	BD72720_REG_VM_VBAT_L,
> +	BD72720_REG_VM_OCV_PRE_U,
> +	BD72720_REG_VM_OCV_PRE_L,
> +	BD72720_REG_VM_OCV_PST_U,
> +	BD72720_REG_VM_OCV_PST_L,
> +	BD72720_REG_VM_OCV_PWRON_U,
> +	BD72720_REG_VM_OCV_PWRON_L,
> +	BD72720_REG_VM_DVBAT_IMP_U,
> +	BD72720_REG_VM_DVBAT_IMP_L,
> +	BD72720_REG_VM_SA_VBAT_U,
> +	BD72720_REG_VM_SA_VBAT_L,
> +	BD72720_REG_VM_SA_VBAT_MIN_U,
> +	BD72720_REG_VM_SA_VBAT_MIN_L,
> +	BD72720_REG_VM_SA_VBAT_MAX_U,
> +	BD72720_REG_VM_SA_VBAT_MAX_L,
> +	BD72720_REG_REX_SA_VBAT_U,
> +	BD72720_REG_REX_SA_VBAT_L,
> +	BD72720_REG_VM_VSYS_U,
> +	BD72720_REG_VM_VSYS_L,
> +	BD72720_REG_VM_SA_VSYS_U,
> +	BD72720_REG_VM_SA_VSYS_L,
> +	BD72720_REG_VM_SA_VSYS_MIN_U,
> +	BD72720_REG_VM_SA_VSYS_MIN_L,
> +	BD72720_REG_VM_SA_VSYS_MAX_U,
> +	BD72720_REG_VM_SA_VSYS_MAX_L,
> +	BD72720_REG_VM_SA2_VSYS_U,
> +	BD72720_REG_VM_SA2_VSYS_L,
> +	BD72720_REG_VM_VBUS_U,
> +#define BD72720_MASK_VDCIN_U	GENMASK(3, 0)
> +	BD72720_REG_VM_VBUS_L,
> +	BD72720_REG_VM_BATID_U,
> +	BD72720_REG_VM_BATID_L,
> +	BD72720_REG_VM_BATID_NOLOAD_U,
> +	BD72720_REG_VM_BATID_NOLOAD_L,
> +	BD72720_REG_VM_BATID_OFS_U,
> +	BD72720_REG_VM_BATID_OFS_L,
> +	BD72720_REG_VM_VTH_U,
> +	BD72720_REG_VM_VTH_L,
> +	BD72720_REG_VM_VTH_CORR_U,
> +	BD72720_REG_VM_VTH_CORR_L,
> +	BD72720_REG_VM_BTMP_U,
> +	BD72720_REG_VM_BTMP_L,
> +	BD72720_REG_VM_BTMP_IMP_U,
> +	BD72720_REG_VM_BTMP_IMP_L,
> +	BD72720_REG_VM_VF_U,
> +	BD72720_REG_VM_VF_L,
> +	BD72720_REG_VM_BATID_TH_U,
> +	BD72720_REG_VM_BATID_TH_L,
> +	BD72720_REG_VM_BTMP_OV_THR,
> +	BD72720_REG_VM_BTMP_OV_DUR,
> +	BD72720_REG_VM_BTMP_LO_THR,
> +	BD72720_REG_VM_BTMP_LO_DUR,
> +	BD72720_REG_ALM_VBAT_TH_U,
> +	BD72720_REG_ALM_VBAT_TH_L,
> +	BD72720_REG_ALM_VSYS_TH,
> +	BD72720_REG_ALM_VBUS_TH,
> +	BD72720_REG_ALM_VF_TH,
> +	BD72720_REG_VSYS_MAX,
> +	BD72720_REG_VSYS_MIN,
> +	BD72720_REG_VM_VSYS_SA_MINMAX_CTRL,
> +	BD72720_REG_VM_SA_CFG,		/* 0x6c + offset*/
> +
> +	BD72720_REG_CC_CURCD_U		= BD72720_I2C4C_ADDR_OFFSET + 0x70,
> +	BD72720_REG_CC_CURCD_L,
> +	BD72720_REG_CC_CURCD_IMP_U,
> +	BD72720_REG_CC_CURCD_IMP_L,
> +	BD72720_REG_CC_SA_CURCD_U,
> +	BD72720_REG_CC_SA_CURCD_L,
> +	BD72720_REG_CC_OCUR_MON,
> +	BD72720_REG_CC_CCNTD_3,
> +	BD72720_REG_CC_CCNTD_2,
> +	BD72720_REG_CC_CCNTD_1,
> +	BD72720_REG_CC_CCNTD_0,
> +	BD72720_REG_REX_CCNTD_3,
> +	BD72720_REG_REX_CCNTD_2,
> +	BD72720_REG_REX_CCNTD_1,
> +	BD72720_REG_REX_CCNTD_0,
> +	BD72720_REG_FULL_CCNTD_3,
> +	BD72720_REG_FULL_CCNTD_2,
> +	BD72720_REG_FULL_CCNTD_1,
> +	BD72720_REG_FULL_CCNTD_0,
> +	BD72720_REG_CCNTD_CHG_3,
> +	BD72720_REG_CCNTD_CHG_2,
> +	BD72720_REG_CC_STAT,
> +	BD72720_REG_CC_CTRL,
> +	BD72720_REG_CC_OCUR_THR_1,
> +	BD72720_REG_CC_OCUR_THR_2,
> +	BD72720_REG_CC_OCUR_THR_3,
> +	BD72720_REG_REX_CURCD_TH,
> +	BD72720_REG_CC_BATCAP1_TH_U,
> +	BD72720_REG_CC_BATCAP1_TH_L,
> +	BD72720_REG_CC_BATCAP2_TH_U,
> +	BD72720_REG_CC_BATCAP2_TH_L,
> +	BD72720_REG_CC_BATCAP3_TH_U,
> +	BD72720_REG_CC_BATCAP3_TH_L,
> +	BD72720_REG_CC_CCNTD_CTRL,
> +	BD72720_REG_CC_SA_CFG,		/* 0x92 + offset*/
> +	BD72720_REG_IMPCHK_CTRL		= BD72720_I2C4C_ADDR_OFFSET + 0xa0,
> +};
> +
> +#endif /* __LINUX_MFD_BD72720_H */
> diff --git a/include/linux/mfd/rohm-generic.h b/include/linux/mfd/rohm-generic.h
> index 579e8dcfcca4..0a284919a6c3 100644
> --- a/include/linux/mfd/rohm-generic.h
> +++ b/include/linux/mfd/rohm-generic.h
> @@ -16,6 +16,7 @@ enum rohm_chip_type {
>  	ROHM_CHIP_TYPE_BD71828,
>  	ROHM_CHIP_TYPE_BD71837,
>  	ROHM_CHIP_TYPE_BD71847,
> +	ROHM_CHIP_TYPE_BD72720,
>  	ROHM_CHIP_TYPE_BD96801,
>  	ROHM_CHIP_TYPE_BD96802,
>  	ROHM_CHIP_TYPE_BD96805,
> -- 
> 2.51.1
> 



-- 
Lee Jones [李琼斯]

^ permalink raw reply

* Re: [PATCH v5 07/16] mfd: rohm-bd71828: Use regmap_reg_range()
From: Lee Jones @ 2025-11-26 14:33 UTC (permalink / raw)
  To: Matti Vaittinen
  Cc: Matti Vaittinen, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Sebastian Reichel, Liam Girdwood, Mark Brown,
	Michael Turquette, Stephen Boyd, Linus Walleij,
	Bartosz Golaszewski, Alexandre Belloni, linux-leds, devicetree,
	linux-kernel, linux-pm, linux-clk, linux-gpio, linux-rtc,
	Andreas Kemnade
In-Reply-To: <b0c6256deb1388f0774b3c855c0614d363aa003b.1763625920.git.mazziesaccount@gmail.com>

On Thu, 20 Nov 2025, Matti Vaittinen wrote:

> From: Matti Vaittinen <mazziesaccount@gmail.com>
> 
> The regmap range tables tend to be somewhat verbose. Using the
> regmap_reg_range() can make the definitions slightly mode compact.
> 
> Tidy the regmap range tables by using the regmap_reg_range().
> 
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
> ---
> Revision history:
>  v2 => :
>  - no changes
>  RFCv1 => v2:
>  - New patch
> ---
>  drivers/mfd/rohm-bd71828.c | 64 +++++++++++---------------------------
>  1 file changed, 18 insertions(+), 46 deletions(-)

LGTM.

Once everything is in order, I plan to merge the set through MFD.

-- 
Lee Jones [李琼斯]

^ permalink raw reply

* Re: [PATCH v5 08/16] mfd: bd71828: Support ROHM BD72720
From: Matti Vaittinen @ 2025-11-27  7:46 UTC (permalink / raw)
  To: Lee Jones
  Cc: Matti Vaittinen, Pavel Machek, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Sebastian Reichel, Liam Girdwood, Mark Brown,
	Michael Turquette, Stephen Boyd, Linus Walleij,
	Bartosz Golaszewski, Alexandre Belloni, linux-leds, devicetree,
	linux-kernel, linux-pm, linux-clk, linux-gpio, linux-rtc,
	Andreas Kemnade
In-Reply-To: <20251126142809.GD3070764@google.com>

On 26/11/2025 16:28, Lee Jones wrote:
> On Thu, 20 Nov 2025, Matti Vaittinen wrote:
> 
>> From: Matti Vaittinen <mazziesaccount@gmail.com>
>>
>> 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@gmail.com>
>>
>> ---
>> 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@kemnade.info/
>>
>> 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.

>> + * Author: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
>> + */
>> +
// snip

>> +
>> +/* 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! ~~

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox