Re: [PATCH v2 09/17] mfd: Support for ROHM BD71815 PMIC core

Hello Lee,

Thanks again for the review!

On Mon, 2021-01-25 at 14:10 +0000, Lee Jones wrote:

On Tue, 19 Jan 2021, Matti Vaittinen wrote:

quoted

Add core support for ROHM BD71815 Power Management IC.

The IC integrates regulators, a battery charger with a coulomb
counter,
a real-time clock (RTC), clock gate and general-purpose outputs
(GPO).

Signed-off-by: Matti Vaittinen <redacted>
---
Changes since v1:
  - Used BIT() for better readability
  - removed some unused definitions

 drivers/mfd/Kconfig              |  15 +-
 drivers/mfd/rohm-bd71828.c       | 416 +++++++++++++++++++++--
 include/linux/mfd/rohm-bd71815.h | 561
+++++++++++++++++++++++++++++++
 include/linux/mfd/rohm-bd71828.h |   3 +
 4 files changed, 952 insertions(+), 43 deletions(-)
 create mode 100644 include/linux/mfd/rohm-bd71815.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index bdfce7b15621..59bfacb91898 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig

@@ -1984,19 +1984,20 @@ config MFD_ROHM_BD70528
 	  charger.
 
 config MFD_ROHM_BD71828
-	tristate "ROHM BD71828 Power Management IC"
+	tristate "ROHM BD71828 and BD71815 Power Management IC"
 	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 Power
-	  Management IC. BD71828GW is a single-chip power management IC

for
-	  battery-powered portable devices. The IC integrates 7 buck
-	  converters, 7 LDOs, and a 1500 mA single-cell linear charger.
-	  Also included is a Coulomb counter, a real-time clock (RTC),
and
-	  a 32.768 kHz clock gate.
+	  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.
 
 config MFD_STM32_LPTIMER
 	tristate "Support for STM32 Low-Power Timer"

diff --git a/drivers/mfd/rohm-bd71828.c b/drivers/mfd/rohm-

bd71828.c
index 210261d026f2..28b82477ce4c 100644

--- a/drivers/mfd/rohm-bd71828.c
+++ b/drivers/mfd/rohm-bd71828.c

@@ -2,7 +2,7 @@
 //
 // Copyright (C) 2019 ROHM Semiconductors
 //
-// ROHM BD71828 PMIC driver
+// ROHM BD71828/BD71815 PMIC driver
 
 #include <linux/gpio_keys.h>
 #include <linux/i2c.h>

@@ -11,7 +11,9 @@
 #include <linux/ioport.h>
 #include <linux/irq.h>
 #include <linux/mfd/core.h>
+#include <linux/mfd/rohm-bd71815.h>
 #include <linux/mfd/rohm-bd71828.h>
+#include <linux/mfd/rohm-generic.h>
 #include <linux/module.h>
 #include <linux/of_device.h>
 #include <linux/regmap.h>

@@ -29,12 +31,102 @@ static struct gpio_keys_platform_data

bd71828_powerkey_data = {
 	.name = "bd71828-pwrkey",
 };
 
-static const struct resource rtc_irqs[] = {
+static const struct resource bd71815_rtc_irqs[] = {
+	DEFINE_RES_IRQ_NAMED(BD71815_INT_RTC0, "bd71815-rtc-alm-0"),
+	DEFINE_RES_IRQ_NAMED(BD71815_INT_RTC1, "bd71815-rtc-alm-1"),
+	DEFINE_RES_IRQ_NAMED(BD71815_INT_RTC2, "bd71815-rtc-alm-2"),
+};
+
+static const struct resource bd71828_rtc_irqs[] = {
 	DEFINE_RES_IRQ_NAMED(BD71828_INT_RTC0, "bd71828-rtc-alm-0"),
 	DEFINE_RES_IRQ_NAMED(BD71828_INT_RTC1, "bd71828-rtc-alm-1"),
 	DEFINE_RES_IRQ_NAMED(BD71828_INT_RTC2, "bd71828-rtc-alm-2"),
 };
 
+static 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-clps-out"),
+	DEFINE_RES_IRQ_NAMED(BD71815_INT_CLPS_IN, "bd71815-clps-in"),
+	DEFINE_RES_IRQ_NAMED(BD71815_INT_DCIN_OVP_RES, "bd71815-dcin-
ovp-res"),
+	DEFINE_RES_IRQ_NAMED(BD71815_INT_DCIN_OVP_DET, "bd71815-dcin-
ovp-det"),
+	DEFINE_RES_IRQ_NAMED(BD71815_INT_DCIN_MON_RES, "bd71815-dcin-
mon-res"),
+	DEFINE_RES_IRQ_NAMED(BD71815_INT_DCIN_MON_DET, "bd71815-dcin-
mon-det"),
+	DEFINE_RES_IRQ_NAMED(BD71815_INT_VSYS_UV_RES, "bd71815-vsys-uv-
res"),
+	DEFINE_RES_IRQ_NAMED(BD71815_INT_VSYS_UV_DET, "bd71815-vsys-uv-
det"),
+	DEFINE_RES_IRQ_NAMED(BD71815_INT_VSYS_LOW_RES, "bd71815-vsys-
low-res"),
+	DEFINE_RES_IRQ_NAMED(BD71815_INT_VSYS_LOW_DET, "bd71815-vsys-
low-det"),
+	DEFINE_RES_IRQ_NAMED(BD71815_INT_VSYS_MON_RES, "bd71815-vsys-
mon-res"),
+	DEFINE_RES_IRQ_NAMED(BD71815_INT_VSYS_MON_RES, "bd71815-vsys-
mon-det"),
+	DEFINE_RES_IRQ_NAMED(BD71815_INT_CHG_WDG_TEMP, "bd71815-chg-
wdg-temp"),
+	DEFINE_RES_IRQ_NAMED(BD71815_INT_CHG_WDG_TIME, "bd71815-chg-
wdg"),
+	DEFINE_RES_IRQ_NAMED(BD71815_INT_CHG_RECHARGE_RES, "bd71815-
rechg-res"),
+	DEFINE_RES_IRQ_NAMED(BD71815_INT_CHG_RECHARGE_DET, "bd71815-
rechg-det"),
+	DEFINE_RES_IRQ_NAMED(BD71815_INT_CHG_RANGED_TEMP_TRANSITION,
+			     "bd71815-ranged-temp-transit"),

The new line limit is 100.  Feel free to run these out.

I learn new things every day it seems. This change is more than
welcome!

quoted

+	if (!strncmp(mode, "open-drain", 10)) {
+		dev_dbg(dev, "configuring clk32kout as open-drain");

Do you *really* need these?

No. development leftover. Thanks for pointing that.

quoted

+		button_irq = 0;
+		dev_info(&i2c->dev, "BD71815 found\n");

Again, are these *really* useful to you and/or your users?

Besides, this device not *found* i.e. scanned/read and instantiated,
it has simply been matched from the local DTB.  It can still be
wrong.  You can probably omit them.

You're right. One can check the DT contents from /proc if he wants to
check what IC compatible was used. Thanks.

[...]

quoted

diff --git a/include/linux/mfd/rohm-bd71815.h

b/include/linux/mfd/rohm-bd71815.h
new file mode 100644
index 000000000000..8ee5874a5b73

--- /dev/null
+++ b/include/linux/mfd/rohm-bd71815.h

@@ -0,0 +1,561 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright 2014 Embest Technology Co. Ltd. Inc.

Jeeze!  Is this for the Amiga?

Nah. Long live NXP SOCs! ;) I think BD71815 was _originally_ developed
for i.MX6.

quoted

+ * Author: yanglsh@embest-tech.com
+ *
+ * 2020, 2021 Heavily modified by:

You should probably add a proper copyright.

Ok. I guess I can do so. Thanks!

quoted

+ *	 Matti Vaittinen [off-list ref]
+ */
+
+#ifndef _MFD_BD71815_H
+#define _MFD_BD71815_H
+
+#include <linux/regmap.h>

Best Regards
-- Matti Vaittinen