[PATCH RESEND v6 0/4] Support ROHM BD9576MUF and BD9573MUF PMICs

[PATCH RESEND v6 0/4] Support ROHM BD9576MUF and BD9573MUF PMICs

[Matti Vaittinen]

Initial support for ROHM BD9576MUF and BD9573MUF PMICs.

Resending in case it fell through the cracks :) Lee, any chance getting
this in MFD? (Also, this still lacks of your ack).

These PMICs are primarily intended to be used to power the R-Car family
processors. BD9576MUF includes some additional safety features the
BD9573MUF does not have. This initial version of drivers does not
utilize these features and for now the SW behaviour is identical.

This patch series includes MFD and watchdog drivers. Regulator part was
already applied.

- Enabling and pinging the watchdog
- configuring watchog timeout / window from device-tree

This patch series does not bring interrupt support. BD9576MUF and BD9573MUF
are designed to keep the IRQ line low for whole duration of error
condition. IRQ can't be 'acked'. So proper IRQ support would require
some IRQ limiter implementation (delayed unmask?) in order to not hog
the CPU.

Changelog v6:
  - Fixed watchdog timeout

Changelog v5:
  - rebased on top of v5.10-rc2
  - few styling fixes in MFD as suggested by Lee

Changelog v4:
  - rebased on top of 5.10-rc1
  - Fix typo (repeated word maximum) from the DT binding doc

Changelog v3:
  - use only one binding to specify watchdog time-out window.

Changelog v2:
  - dropped already applied regulator part
  - dt_bindings: Fix case for regulator-names in the example
  - watchdod: unify probe error check and revise includes

---

Matti Vaittinen (4):
  dt_bindings: mfd: Add ROHM BD9576MUF and BD9573MUF PMICs
  mfd: Support ROHM BD9576MUF and BD9573MUF
  wdt: Support wdt on ROHM BD9576MUF and BD9573MUF
  MAINTAINERS: Add ROHM BD9576MUF and BD9573MUF drivers

 .../bindings/mfd/rohm,bd9576-pmic.yaml        | 123 ++++++++
 MAINTAINERS                                   |   4 +
 drivers/mfd/Kconfig                           |  11 +
 drivers/mfd/Makefile                          |   1 +
 drivers/mfd/rohm-bd9576.c                     | 108 +++++++
 drivers/watchdog/Kconfig                      |  13 +
 drivers/watchdog/Makefile                     |   1 +
 drivers/watchdog/bd9576_wdt.c                 | 291 ++++++++++++++++++
 include/linux/mfd/rohm-bd957x.h               |  59 ++++
 include/linux/mfd/rohm-generic.h              |   2 +
 10 files changed, 613 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/rohm,bd9576-pmic.yaml
 create mode 100644 drivers/mfd/rohm-bd9576.c
 create mode 100644 drivers/watchdog/bd9576_wdt.c
 create mode 100644 include/linux/mfd/rohm-bd957x.h


base-commit: 3cea11cd5e3b00d91caf0b4730194039b45c5891
-- 
2.25.4


-- 
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =] 

[PATCH RESEND v6 1/4] dt_bindings: mfd: Add ROHM BD9576MUF and BD9573MUF PMICs

[Matti Vaittinen]

Add bindings for ROHM BD9576MUF and BD9573MUF PMICs. These
PMICs are primarily intended to be used to power the R-Car series
processors. They provide 6 power outputs, safety features and a
watchdog with two functional modes.

Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../bindings/mfd/rohm,bd9576-pmic.yaml        | 123 ++++++++++++++++++
 1 file changed, 123 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/rohm,bd9576-pmic.yaml

diff --git a/Documentation/devicetree/bindings/mfd/rohm,bd9576-pmic.yaml b/Documentation/devicetree/bindings/mfd/rohm,bd9576-pmic.yaml
new file mode 100644
index 000000000000..6483860da955
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/rohm,bd9576-pmic.yaml
@@ -0,0 +1,123 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/rohm,bd9576-pmic.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ROHM BD9576MUF and BD9573MUF Power Management Integrated Circuit bindings
+
+maintainers:
+  - Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
+
+description: |
+  BD9576MUF and BD9573MUF are power management ICs primarily intended for
+  powering the R-Car series processors.
+  The IC provides 6 power outputs with configurable sequencing and safety
+  monitoring. A watchdog logic with slow ping/windowed modes is also included.
+
+properties:
+  compatible:
+    enum:
+      - rohm,bd9576
+      - rohm,bd9573
+
+  reg:
+    description:
+      I2C slave address.
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  rohm,vout1-en-low:
+    description:
+      BD9576 and BD9573 VOUT1 regulator enable state can be individually
+      controlled by a GPIO. This is dictated by state of vout1-en pin during
+      the PMIC startup. If vout1-en is LOW during PMIC startup then the VOUT1
+      enable sate is controlled via this pin. Set this property if vout1-en
+      is wired to be down at PMIC start-up.
+    type: boolean
+
+  rohm,vout1-en-gpios:
+    description:
+      GPIO specifier to specify the GPIO connected to vout1-en for vout1 ON/OFF
+      state control.
+    maxItems: 1
+
+  rohm,ddr-sel-low:
+    description:
+      The BD9576 and BD9573 output voltage for DDR can be selected by setting
+      the ddr-sel pin low or high. Set this property if ddr-sel is grounded.
+    type: boolean
+
+  rohm,watchdog-enable-gpios:
+    description: The GPIO line used to enable the watchdog.
+    maxItems: 1
+
+  rohm,watchdog-ping-gpios:
+    description: The GPIO line used to ping the watchdog.
+    maxItems: 1
+
+  rohm,hw-timeout-ms:
+    maxItems: 2
+    description:
+      Watchog timeout in milliseconds. If single value is given it is
+      the maximum timeout. Eg. if pinging watchdog is not done within this time
+      limit the watchdog will be triggered. If two values are given watchdog
+      is configured in "window mode". Then first value is limit for short-ping
+      Eg. if watchdog is pinged sooner than that the watchdog will trigger.
+      When two values is given the second value is the maximum timeout.
+      # (HW) minimum for short timeout is 2ms, maximum 220 ms.
+      # (HW) minimum for max timeout is 4ms, maximum 4416 ms.
+
+  regulators:
+    $ref: ../regulator/rohm,bd9576-regulator.yaml
+    description:
+      List of child nodes that specify the regulators.
+
+required:
+  - compatible
+  - reg
+  - regulators
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/leds/common.h>
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        pmic: pmic@30 {
+            compatible = "rohm,bd9576";
+            reg = <0x30>;
+            rohm,vout1-en-low;
+            rohm,vout1-en-gpios = <&gpio2 6 GPIO_ACTIVE_HIGH>;
+            rohm,ddr-sel-low;
+            rohm,watchdog-enable-gpios = <&gpio2 6 GPIO_ACTIVE_HIGH>;
+            rohm,watchdog-ping-gpios = <&gpio2 7 GPIO_ACTIVE_HIGH>;
+            rohm,hw-timeout-ms = <150>, <2300>;
+
+            regulators {
+                boost1: regulator-vd50 {
+                    regulator-name = "VD50";
+                };
+                buck1: regulator-vd18 {
+                    regulator-name = "VD18";
+                };
+                buck2: regulator-vdddr {
+                    regulator-name = "VDDDR";
+                };
+                buck3: regulator-vd10 {
+                    regulator-name = "VD10";
+                };
+                ldo: regulator-voutl1 {
+                    regulator-name = "VOUTL1";
+                };
+                sw: regulator-vouts1 {
+                    regulator-name = "VOUTS1";
+                };
+            };
+        };
+    };
-- 
2.25.4


-- 
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =] 

[PATCH RESEND v6 2/4] mfd: Support ROHM BD9576MUF and BD9573MUF

[Matti Vaittinen]

Add core support for ROHM BD9576MUF and BD9573MUF PMICs which are
mainly used to power the R-Car series processors.

Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
---
 drivers/mfd/Kconfig              |  11 ++++
 drivers/mfd/Makefile             |   1 +
 drivers/mfd/rohm-bd9576.c        | 108 +++++++++++++++++++++++++++++++
 include/linux/mfd/rohm-bd957x.h  |  59 +++++++++++++++++
 include/linux/mfd/rohm-generic.h |   2 +
 5 files changed, 181 insertions(+)
 create mode 100644 drivers/mfd/rohm-bd9576.c
 create mode 100644 include/linux/mfd/rohm-bd957x.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 8b99a13669bf..dcb2b14a570e 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -2010,6 +2010,17 @@ config MFD_ROHM_BD71828
 	  Also included is a Coulomb counter, a real-time clock (RTC), and
 	  a 32.768 kHz clock gate.
 
+config MFD_ROHM_BD957XMUF
+	tristate "ROHM BD9576MUF and BD9573MUF Power Management ICs"
+	depends on I2C=y
+	depends on OF
+	select REGMAP_I2C
+	select MFD_CORE
+	help
+	  Select this option to get support for the ROHM BD9576MUF and
+	  BD9573MUF Power Management ICs. BD9576 and BD9573 are primarily
+	  designed to be used to power R-Car series processors.
+
 config MFD_STM32_LPTIMER
 	tristate "Support for STM32 Low-Power Timer"
 	depends on (ARCH_STM32 && OF) || COMPILE_TEST
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 1780019d2474..837f68c9f336 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -261,6 +261,7 @@ obj-$(CONFIG_RAVE_SP_CORE)	+= rave-sp.o
 obj-$(CONFIG_MFD_ROHM_BD70528)	+= rohm-bd70528.o
 obj-$(CONFIG_MFD_ROHM_BD71828)	+= rohm-bd71828.o
 obj-$(CONFIG_MFD_ROHM_BD718XX)	+= rohm-bd718x7.o
+obj-$(CONFIG_MFD_ROHM_BD957XMUF)	+= rohm-bd9576.o
 obj-$(CONFIG_MFD_STMFX) 	+= stmfx.o
 obj-$(CONFIG_MFD_KHADAS_MCU) 	+= khadas-mcu.o
 
diff --git a/drivers/mfd/rohm-bd9576.c b/drivers/mfd/rohm-bd9576.c
new file mode 100644
index 000000000000..f4dd9e438427
--- /dev/null
+++ b/drivers/mfd/rohm-bd9576.c
@@ -0,0 +1,108 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+//
+// Copyright (C) 2020 ROHM Semiconductors
+//
+// ROHM BD9576MUF and BD9573MUF PMIC driver
+
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/ioport.h>
+#include <linux/irq.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/rohm-bd957x.h>
+#include <linux/mfd/rohm-generic.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/regmap.h>
+#include <linux/types.h>
+
+static struct mfd_cell bd9573_mfd_cells[] = {
+	{ .name = "bd9573-pmic", },
+	{ .name = "bd9576-wdt", },
+};
+
+static struct mfd_cell bd9576_mfd_cells[] = {
+	{ .name = "bd9576-pmic", },
+	{ .name = "bd9576-wdt", },
+};
+
+static const struct regmap_range volatile_ranges[] = {
+	regmap_reg_range(BD957X_REG_SMRB_ASSERT, BD957X_REG_SMRB_ASSERT),
+	regmap_reg_range(BD957X_REG_PMIC_INTERNAL_STAT,
+			 BD957X_REG_PMIC_INTERNAL_STAT),
+	regmap_reg_range(BD957X_REG_INT_THERM_STAT, BD957X_REG_INT_THERM_STAT),
+	regmap_reg_range(BD957X_REG_INT_OVP_STAT, BD957X_REG_INT_SYS_STAT),
+	regmap_reg_range(BD957X_REG_INT_MAIN_STAT, BD957X_REG_INT_MAIN_STAT),
+};
+
+static const struct regmap_access_table volatile_regs = {
+	.yes_ranges = &volatile_ranges[0],
+	.n_yes_ranges = ARRAY_SIZE(volatile_ranges),
+};
+
+static struct regmap_config bd957x_regmap = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.volatile_table = &volatile_regs,
+	.max_register = BD957X_MAX_REGISTER,
+	.cache_type = REGCACHE_RBTREE,
+};
+
+static int bd957x_i2c_probe(struct i2c_client *i2c,
+			     const struct i2c_device_id *id)
+{
+	int ret;
+	struct regmap *regmap;
+	struct mfd_cell *mfd;
+	int cells;
+	unsigned long chip_type;
+
+	chip_type = (unsigned long)of_device_get_match_data(&i2c->dev);
+
+	switch (chip_type) {
+	case ROHM_CHIP_TYPE_BD9576:
+		mfd = bd9576_mfd_cells;
+		cells = ARRAY_SIZE(bd9576_mfd_cells);
+		break;
+	case ROHM_CHIP_TYPE_BD9573:
+		mfd = bd9573_mfd_cells;
+		cells = ARRAY_SIZE(bd9573_mfd_cells);
+		break;
+	default:
+		dev_err(&i2c->dev, "Unknown device type");
+		return -EINVAL;
+	}
+
+	regmap = devm_regmap_init_i2c(i2c, &bd957x_regmap);
+	if (IS_ERR(regmap)) {
+		dev_err(&i2c->dev, "Failed to initialize Regmap\n");
+		return PTR_ERR(regmap);
+	}
+
+	ret = devm_mfd_add_devices(&i2c->dev, PLATFORM_DEVID_AUTO, mfd, cells,
+				   NULL, 0, NULL);
+	if (ret)
+		dev_err(&i2c->dev, "Failed to create subdevices\n");
+
+	return ret;
+}
+
+static const struct of_device_id bd957x_of_match[] = {
+	{ .compatible = "rohm,bd9576", .data = (void *)ROHM_CHIP_TYPE_BD9576, },
+	{ .compatible = "rohm,bd9573", .data = (void *)ROHM_CHIP_TYPE_BD9573, },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, bd957x_of_match);
+
+static struct i2c_driver bd957x_drv = {
+	.driver = {
+		.name = "rohm-bd957x",
+		.of_match_table = bd957x_of_match,
+	},
+	.probe = &bd957x_i2c_probe,
+};
+module_i2c_driver(bd957x_drv);
+
+MODULE_AUTHOR("Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>");
+MODULE_DESCRIPTION("ROHM BD9576MUF and BD9573MUF Power Management IC driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/mfd/rohm-bd957x.h b/include/linux/mfd/rohm-bd957x.h
new file mode 100644
index 000000000000..3e7ca6fe5d4f
--- /dev/null
+++ b/include/linux/mfd/rohm-bd957x.h
@@ -0,0 +1,59 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/* Copyright (C) 2020 ROHM Semiconductors */
+
+#ifndef __LINUX_MFD_BD957X_H__
+#define __LINUX_MFD_BD957X_H__
+
+enum {
+	BD957X_VD50,
+	BD957X_VD18,
+	BD957X_VDDDR,
+	BD957X_VD10,
+	BD957X_VOUTL1,
+	BD957X_VOUTS1,
+};
+
+#define BD957X_REG_SMRB_ASSERT		0x15
+#define BD957X_REG_PMIC_INTERNAL_STAT	0x20
+#define BD957X_REG_INT_THERM_STAT	0x23
+#define BD957X_REG_INT_THERM_MASK 0x24
+#define BD957X_REG_INT_OVP_STAT 0x25
+#define BD957X_REG_INT_SCP_STAT 0x26
+#define BD957X_REG_INT_OCP_STAT 0x27
+#define BD957X_REG_INT_OVD_STAT 0x28
+#define BD957X_REG_INT_UVD_STAT 0x29
+#define BD957X_REG_INT_UVP_STAT 0x2a
+#define BD957X_REG_INT_SYS_STAT 0x2b
+#define BD957X_REG_INT_SYS_MASK 0x2c
+#define BD957X_REG_INT_MAIN_STAT 0x30
+#define BD957X_REG_INT_MAIN_MASK 0x31
+
+#define BD957X_REG_WDT_CONF 0x16
+
+#define BD957X_REG_POW_TRIGGER1 0x41
+#define BD957X_REG_POW_TRIGGER2 0x42
+#define BD957X_REG_POW_TRIGGER3 0x43
+#define BD957X_REG_POW_TRIGGER4 0x44
+#define BD957X_REG_POW_TRIGGERL1 0x45
+#define BD957X_REG_POW_TRIGGERS1 0x46
+
+#define BD957X_REGULATOR_EN_MASK 0xff
+#define BD957X_REGULATOR_DIS_VAL 0xff
+
+#define BD957X_VSEL_REG_MASK	0xff
+
+#define BD957X_MASK_VOUT1_TUNE	0x87
+#define BD957X_MASK_VOUT2_TUNE	0x87
+#define BD957X_MASK_VOUT3_TUNE	0x1f
+#define BD957X_MASK_VOUT4_TUNE	0x1f
+#define BD957X_MASK_VOUTL1_TUNE	0x87
+
+#define BD957X_REG_VOUT1_TUNE	0x50
+#define BD957X_REG_VOUT2_TUNE	0x53
+#define BD957X_REG_VOUT3_TUNE	0x56
+#define BD957X_REG_VOUT4_TUNE	0x59
+#define BD957X_REG_VOUTL1_TUNE	0x5c
+
+#define BD957X_MAX_REGISTER 0x61
+
+#endif
diff --git a/include/linux/mfd/rohm-generic.h b/include/linux/mfd/rohm-generic.h
index 4283b5b33e04..58b4f1a0f4af 100644
--- a/include/linux/mfd/rohm-generic.h
+++ b/include/linux/mfd/rohm-generic.h
@@ -12,6 +12,8 @@ enum rohm_chip_type {
 	ROHM_CHIP_TYPE_BD71847,
 	ROHM_CHIP_TYPE_BD70528,
 	ROHM_CHIP_TYPE_BD71828,
+	ROHM_CHIP_TYPE_BD9576,
+	ROHM_CHIP_TYPE_BD9573,
 	ROHM_CHIP_TYPE_AMOUNT
 };
 
-- 
2.25.4


-- 
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =] 

[PATCH RESEND v6 3/4] wdt: Support wdt on ROHM BD9576MUF and BD9573MUF

[Matti Vaittinen]

Add Watchdog support for ROHM BD9576MUF and BD9573MUF PMICs which are
mainly used to power the R-Car series processors. The watchdog is
pinged using a GPIO and enabled using another GPIO. Additionally
watchdog time-out can be configured to HW prior starting the watchdog.
Watchdog timeout can be configured to detect only delayed ping or in
a window mode where also too fast pings are detected.

Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/watchdog/Kconfig      |  13 ++
 drivers/watchdog/Makefile     |   1 +
 drivers/watchdog/bd9576_wdt.c | 291 ++++++++++++++++++++++++++++++++++
 3 files changed, 305 insertions(+)
 create mode 100644 drivers/watchdog/bd9576_wdt.c

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index fd7968635e6d..d5fec1bdf23e 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -172,6 +172,19 @@ config BD70528_WATCHDOG
 	  Alternatively say M to compile the driver as a module,
 	  which will be called bd70528_wdt.
 
+config BD957XMUF_WATCHDOG
+	tristate "ROHM BD9576MUF and BD9573MUF PMIC Watchdog"
+	depends on MFD_ROHM_BD957XMUF
+	select WATCHDOG_CORE
+	help
+	  Support for the watchdog in the ROHM BD9576 and BD9573 PMICs.
+	  These PMIC ICs contain watchdog block which can be configured
+	  to toggle reset line if SoC fails to ping watchdog via GPIO.
+
+	  Say Y here to include support for the ROHM BD9576 or BD9573
+	  watchdog. Alternatively say M to compile the driver as a module,
+	  which will be called bd9576_wdt.
+
 config DA9052_WATCHDOG
 	tristate "Dialog DA9052 Watchdog"
 	depends on PMIC_DA9052 || COMPILE_TEST
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 071a2e50be98..54f39883f3ac 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -209,6 +209,7 @@ obj-$(CONFIG_XEN_WDT) += xen_wdt.o
 
 # Architecture Independent
 obj-$(CONFIG_BD70528_WATCHDOG) += bd70528_wdt.o
+obj-$(CONFIG_BD957XMUF_WATCHDOG) += bd9576_wdt.o
 obj-$(CONFIG_DA9052_WATCHDOG) += da9052_wdt.o
 obj-$(CONFIG_DA9055_WATCHDOG) += da9055_wdt.o
 obj-$(CONFIG_DA9062_WATCHDOG) += da9062_wdt.o
diff --git a/drivers/watchdog/bd9576_wdt.c b/drivers/watchdog/bd9576_wdt.c
new file mode 100644
index 000000000000..0b6999f3b6e8
--- /dev/null
+++ b/drivers/watchdog/bd9576_wdt.c
@@ -0,0 +1,291 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2020 ROHM Semiconductors
+ *
+ * ROHM BD9576MUF and BD9573MUF Watchdog driver
+ */
+
+#include <linux/err.h>
+#include <linux/gpio/consumer.h>
+#include <linux/mfd/rohm-bd957x.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/watchdog.h>
+
+static bool nowayout;
+module_param(nowayout, bool, 0);
+MODULE_PARM_DESC(nowayout,
+		"Watchdog cannot be stopped once started (default=\"false\")");
+
+#define HW_MARGIN_MIN 2
+#define HW_MARGIN_MAX 4416
+#define BD957X_WDT_DEFAULT_MARGIN 4416
+#define WATCHDOG_TIMEOUT 30
+
+struct bd9576_wdt_priv {
+	struct gpio_desc	*gpiod_ping;
+	struct gpio_desc	*gpiod_en;
+	struct device		*dev;
+	struct regmap		*regmap;
+	bool			always_running;
+	struct watchdog_device	wdd;
+};
+
+static void bd9576_wdt_disable(struct bd9576_wdt_priv *priv)
+{
+	gpiod_set_value_cansleep(priv->gpiod_en, 0);
+}
+
+static int bd9576_wdt_ping(struct watchdog_device *wdd)
+{
+	struct bd9576_wdt_priv *priv = watchdog_get_drvdata(wdd);
+
+	/* Pulse */
+	gpiod_set_value_cansleep(priv->gpiod_ping, 1);
+	gpiod_set_value_cansleep(priv->gpiod_ping, 0);
+
+	return 0;
+}
+
+static int bd9576_wdt_start(struct watchdog_device *wdd)
+{
+	struct bd9576_wdt_priv *priv = watchdog_get_drvdata(wdd);
+
+	gpiod_set_value_cansleep(priv->gpiod_en, 1);
+
+	return bd9576_wdt_ping(wdd);
+}
+
+static int bd9576_wdt_stop(struct watchdog_device *wdd)
+{
+	struct bd9576_wdt_priv *priv = watchdog_get_drvdata(wdd);
+
+	if (!priv->always_running)
+		bd9576_wdt_disable(priv);
+	else
+		set_bit(WDOG_HW_RUNNING, &wdd->status);
+
+	return 0;
+}
+
+static const struct watchdog_info bd957x_wdt_ident = {
+	.options	= WDIOF_MAGICCLOSE | WDIOF_KEEPALIVEPING |
+			  WDIOF_SETTIMEOUT,
+	.identity	= "BD957x Watchdog",
+};
+
+static const struct watchdog_ops bd957x_wdt_ops = {
+	.owner		= THIS_MODULE,
+	.start		= bd9576_wdt_start,
+	.stop		= bd9576_wdt_stop,
+	.ping		= bd9576_wdt_ping,
+};
+
+/* Unit is hundreds of uS */
+#define FASTNG_MIN 23
+
+static int find_closest_fast(int target, int *sel, int *val)
+{
+	int i;
+	int window = FASTNG_MIN;
+
+	for (i = 0; i < 8 && window < target; i++)
+		window <<= 1;
+
+	*val = window;
+	*sel = i;
+
+	if (i == 8)
+		return -EINVAL;
+
+	return 0;
+
+}
+
+static int find_closest_slow_by_fast(int fast_val, int target, int *slowsel)
+{
+	int sel;
+	static const int multipliers[] = {2, 3, 7, 15};
+
+	for (sel = 0; sel < ARRAY_SIZE(multipliers) &&
+	     multipliers[sel] * fast_val < target; sel++)
+		;
+
+	if (sel == ARRAY_SIZE(multipliers))
+		return -EINVAL;
+
+	*slowsel = sel;
+
+	return 0;
+}
+
+static int find_closest_slow(int target, int *slow_sel, int *fast_sel)
+{
+	static const int multipliers[] = {2, 3, 7, 15};
+	int i, j;
+	int val = 0;
+	int window = FASTNG_MIN;
+
+	for (i = 0; i < 8; i++) {
+		for (j = 0; j < ARRAY_SIZE(multipliers); j++) {
+			int slow;
+
+			slow = window * multipliers[j];
+			if (slow >= target && (!val || slow < val)) {
+				val = slow;
+				*fast_sel = i;
+				*slow_sel = j;
+			}
+		}
+		window <<= 1;
+	}
+	if (!val)
+		return -EINVAL;
+
+	return 0;
+}
+
+#define BD957X_WDG_TYPE_WINDOW BIT(5)
+#define BD957X_WDG_TYPE_SLOW 0
+#define BD957X_WDG_TYPE_MASK BIT(5)
+#define BD957X_WDG_NG_RATIO_MASK 0x18
+#define BD957X_WDG_FASTNG_MASK 0x7
+
+static int bd957x_set_wdt_mode(struct bd9576_wdt_priv *priv, int hw_margin,
+			       int hw_margin_min)
+{
+	int ret, fastng, slowng, type, reg, mask;
+	struct device *dev = priv->dev;
+
+	/* convert to 100uS */
+	hw_margin *= 10;
+	hw_margin_min *= 10;
+	if (hw_margin_min) {
+		int min;
+
+		type = BD957X_WDG_TYPE_WINDOW;
+		dev_dbg(dev, "Setting type WINDOW 0x%x\n", type);
+		ret = find_closest_fast(hw_margin_min, &fastng, &min);
+		if (ret) {
+			dev_err(dev, "bad WDT window for fast timeout\n");
+			return ret;
+		}
+
+		ret = find_closest_slow_by_fast(min, hw_margin, &slowng);
+		if (ret) {
+			dev_err(dev, "bad WDT window\n");
+			return ret;
+		}
+
+	} else {
+		type = BD957X_WDG_TYPE_SLOW;
+		dev_dbg(dev, "Setting type SLOW 0x%x\n", type);
+		ret = find_closest_slow(hw_margin, &slowng, &fastng);
+		if (ret) {
+			dev_err(dev, "bad WDT window\n");
+			return ret;
+		}
+	}
+
+	slowng <<= ffs(BD957X_WDG_NG_RATIO_MASK) - 1;
+	reg = type | slowng | fastng;
+	mask = BD957X_WDG_TYPE_MASK | BD957X_WDG_NG_RATIO_MASK |
+	       BD957X_WDG_FASTNG_MASK;
+	ret = regmap_update_bits(priv->regmap, BD957X_REG_WDT_CONF,
+				 mask, reg);
+
+	return ret;
+}
+
+static int bd9576_wdt_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->parent->of_node;
+	struct bd9576_wdt_priv *priv;
+	u32 hw_margin[2];
+	u32 hw_margin_max = BD957X_WDT_DEFAULT_MARGIN, hw_margin_min = 0;
+	int ret;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, priv);
+
+	priv->dev = dev;
+	priv->regmap = dev_get_regmap(dev->parent, NULL);
+	if (!priv->regmap) {
+		dev_err(dev, "No regmap found\n");
+		return -ENODEV;
+	}
+
+	priv->gpiod_en = devm_gpiod_get_from_of_node(dev, dev->parent->of_node,
+						     "rohm,watchdog-enable-gpios",
+						     0, GPIOD_OUT_LOW,
+						     "watchdog-enable");
+	if (IS_ERR(priv->gpiod_en))
+		return dev_err_probe(dev, PTR_ERR(priv->gpiod_en),
+			      "getting watchdog-enable GPIO failed\n");
+
+	priv->gpiod_ping = devm_gpiod_get_from_of_node(dev, dev->parent->of_node,
+						     "rohm,watchdog-ping-gpios",
+						     0, GPIOD_OUT_LOW,
+						     "watchdog-ping");
+	if (IS_ERR(priv->gpiod_ping))
+		return dev_err_probe(dev, PTR_ERR(priv->gpiod_ping),
+				     "getting watchdog-ping GPIO failed\n");
+
+	ret = of_property_read_variable_u32_array(np, "rohm,hw-timeout-ms",
+						  &hw_margin[0], 1, 2);
+	if (ret < 0 && ret != -EINVAL)
+		return ret;
+
+	if (ret == 1)
+		hw_margin_max = hw_margin[0];
+
+	if (ret == 2) {
+		hw_margin_max = hw_margin[1];
+		hw_margin_min = hw_margin[0];
+	}
+
+	ret = bd957x_set_wdt_mode(priv, hw_margin_max, hw_margin_min);
+	if (ret)
+		return ret;
+
+	priv->always_running = of_property_read_bool(np, "always-running");
+
+	watchdog_set_drvdata(&priv->wdd, priv);
+
+	priv->wdd.info			= &bd957x_wdt_ident;
+	priv->wdd.ops			= &bd957x_wdt_ops;
+	priv->wdd.min_hw_heartbeat_ms	= hw_margin_min;
+	priv->wdd.max_hw_heartbeat_ms	= hw_margin_max;
+	priv->wdd.parent		= dev;
+	priv->wdd.timeout		= WATCHDOG_TIMEOUT;
+
+	watchdog_init_timeout(&priv->wdd, 0, dev);
+	watchdog_set_nowayout(&priv->wdd, nowayout);
+
+	watchdog_stop_on_reboot(&priv->wdd);
+
+	if (priv->always_running)
+		bd9576_wdt_start(&priv->wdd);
+
+	return devm_watchdog_register_device(dev, &priv->wdd);
+}
+
+static struct platform_driver bd9576_wdt_driver = {
+	.driver	= {
+		.name = "bd9576-wdt",
+	},
+	.probe	= bd9576_wdt_probe,
+};
+
+module_platform_driver(bd9576_wdt_driver);
+
+MODULE_AUTHOR("Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>");
+MODULE_DESCRIPTION("ROHM BD9576/BD9573 Watchdog driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:bd9576-wdt");
-- 
2.25.4


-- 
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =] 

[PATCH RESEND v6 4/4] MAINTAINERS: Add ROHM BD9576MUF and BD9573MUF drivers

[Matti Vaittinen]

Add maintainer entries for ROHM BD9576MUF and ROHM BD9573MUF drivers.
MFD, regulator and watchdog drivers were introduced for these PMICs.

Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
---
 MAINTAINERS | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index b516bb34a8d5..0bd788a94376 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15088,16 +15088,20 @@ F:	drivers/gpio/gpio-bd71828.c
 F:	drivers/mfd/rohm-bd70528.c
 F:	drivers/mfd/rohm-bd71828.c
 F:	drivers/mfd/rohm-bd718x7.c
+F:	drivers/mfd/rohm-bd9576.c
 F:	drivers/power/supply/bd70528-charger.c
 F:	drivers/regulator/bd70528-regulator.c
 F:	drivers/regulator/bd71828-regulator.c
 F:	drivers/regulator/bd718x7-regulator.c
+F:	drivers/regulator/bd9576-regulator.c
 F:	drivers/regulator/rohm-regulator.c
 F:	drivers/rtc/rtc-bd70528.c
 F:	drivers/watchdog/bd70528_wdt.c
+F:	drivers/watchdog/bd9576_wdt.c
 F:	include/linux/mfd/rohm-bd70528.h
 F:	include/linux/mfd/rohm-bd71828.h
 F:	include/linux/mfd/rohm-bd718x7.h
+F:	include/linux/mfd/rohm-bd957x.h
 F:	include/linux/mfd/rohm-generic.h
 F:	include/linux/mfd/rohm-shared.h
 
-- 
2.25.4


-- 
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =] 

Re: [PATCH RESEND v6 2/4] mfd: Support ROHM BD9576MUF and BD9573MUF

[Lee Jones]

On Mon, 23 Nov 2020, Matti Vaittinen wrote:

> Add core support for ROHM BD9576MUF and BD9573MUF PMICs which are
> mainly used to power the R-Car series processors.
> 
> Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> ---
>  drivers/mfd/Kconfig              |  11 ++++
>  drivers/mfd/Makefile             |   1 +
>  drivers/mfd/rohm-bd9576.c        | 108 +++++++++++++++++++++++++++++++
>  include/linux/mfd/rohm-bd957x.h  |  59 +++++++++++++++++
>  include/linux/mfd/rohm-generic.h |   2 +
>  5 files changed, 181 insertions(+)
>  create mode 100644 drivers/mfd/rohm-bd9576.c
>  create mode 100644 include/linux/mfd/rohm-bd957x.h

Looks like a possible candidate for "simple-mfd-i2c".

Could you look into that please?

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH RESEND v6 2/4] mfd: Support ROHM BD9576MUF and BD9573MUF

[Vaittinen, Matti]

Hello Lee,

On Fri, 2020-11-27 at 08:32 +0000, Lee Jones wrote:
> On Mon, 23 Nov 2020, Matti Vaittinen wrote:
> 
> > Add core support for ROHM BD9576MUF and BD9573MUF PMICs which are
> > mainly used to power the R-Car series processors.
> > 
> > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> > ---
> >  drivers/mfd/Kconfig              |  11 ++++
> >  drivers/mfd/Makefile             |   1 +
> >  drivers/mfd/rohm-bd9576.c        | 108
> > +++++++++++++++++++++++++++++++
> >  include/linux/mfd/rohm-bd957x.h  |  59 +++++++++++++++++
> >  include/linux/mfd/rohm-generic.h |   2 +
> >  5 files changed, 181 insertions(+)
> >  create mode 100644 drivers/mfd/rohm-bd9576.c
> >  create mode 100644 include/linux/mfd/rohm-bd957x.h
> 
> Looks like a possible candidate for "simple-mfd-i2c".
> 
> Could you look into that please?
> 
I must admit I didn't know about "simple-mfd-i2c". Good thing to know
when working with simple devices :) Is this a new thing?
I am unsure I understand the idea fully. Should users put all the
different regamp configs in this file and just add the device IDs with
pointer to correct config? (BD9576 and BD9573 need volatile ranges).
Also, does this mean each sub-device should have own node and own
compatible in DT to get correctly load and probed? I guess this would
need a buy-in from Rob too then.

By the way - for uneducated eyes like mine this does not look like it
has much to do with MFD as a device - here MFD reminds me of a simple-
bus on top of I2C.

Anyways, the BD9576 and BD9573 both have a few interrupts for OVD/UVD
conditions and I am expecting that I will be asked to provide the
regulator notifiers for those. Reason why I omitted the IRQs for now is
that the HW is designed to keep the IRQ asserted for whole error
duration so some delayed ack mechanism would be needed. I would like to
keep the door open for adding IRQs to MFD core.

Best Regards
	Matti

Re: [PATCH RESEND v6 2/4] mfd: Support ROHM BD9576MUF and BD9573MUF

[Lee Jones]

On Fri, 27 Nov 2020, Vaittinen, Matti wrote:

> Hello Lee,
> 
> On Fri, 2020-11-27 at 08:32 +0000, Lee Jones wrote:
> > On Mon, 23 Nov 2020, Matti Vaittinen wrote:
> > 
> > > Add core support for ROHM BD9576MUF and BD9573MUF PMICs which are
> > > mainly used to power the R-Car series processors.
> > > 
> > > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> > > ---
> > >  drivers/mfd/Kconfig              |  11 ++++
> > >  drivers/mfd/Makefile             |   1 +
> > >  drivers/mfd/rohm-bd9576.c        | 108
> > > +++++++++++++++++++++++++++++++
> > >  include/linux/mfd/rohm-bd957x.h  |  59 +++++++++++++++++
> > >  include/linux/mfd/rohm-generic.h |   2 +
> > >  5 files changed, 181 insertions(+)
> > >  create mode 100644 drivers/mfd/rohm-bd9576.c
> > >  create mode 100644 include/linux/mfd/rohm-bd957x.h
> > 
> > Looks like a possible candidate for "simple-mfd-i2c".
> > 
> > Could you look into that please?
> > 
> I must admit I didn't know about "simple-mfd-i2c". Good thing to know
> when working with simple devices :) Is this a new thing?

Yes, it's new.

> I am unsure I understand the idea fully. Should users put all the
> different regamp configs in this file and just add the device IDs with
> pointer to correct config? (BD9576 and BD9573 need volatile ranges).
> Also, does this mean each sub-device should have own node and own
> compatible in DT to get correctly load and probed? I guess this would
> need a buy-in from Rob too then.

You should describe the H/W in DT.

> By the way - for uneducated eyes like mine this does not look like it
> has much to do with MFD as a device - here MFD reminds me of a simple-
> bus on top of I2C.

This is for MFD devices where the parent does little more than create
a shared address space for child devices to operate on - like yours.

> Anyways, the BD9576 and BD9573 both have a few interrupts for OVD/UVD
> conditions and I am expecting that I will be asked to provide the
> regulator notifiers for those. Reason why I omitted the IRQs for now is
> that the HW is designed to keep the IRQ asserted for whole error
> duration so some delayed ack mechanism would be needed. I would like to
> keep the door open for adding IRQs to MFD core.

You mean to add an IRQ Domain?

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH RESEND v6 2/4] mfd: Support ROHM BD9576MUF and BD9573MUF

[Vaittinen, Matti]

Hello Lee,

On Wed, 2020-12-02 at 12:57 +0000, Lee Jones wrote:
> On Fri, 27 Nov 2020, Vaittinen, Matti wrote:
> 
> > Hello Lee,
> > 
> > On Fri, 2020-11-27 at 08:32 +0000, Lee Jones wrote:
> > > On Mon, 23 Nov 2020, Matti Vaittinen wrote:
> > > 
> > > > Add core support for ROHM BD9576MUF and BD9573MUF PMICs which
> > > > are
> > > > mainly used to power the R-Car series processors.
> > > > 
> > > > Signed-off-by: Matti Vaittinen <
> > > > matti.vaittinen@fi.rohmeurope.com>
> > > > ---
> > > >  drivers/mfd/Kconfig              |  11 ++++
> > > >  drivers/mfd/Makefile             |   1 +
> > > >  drivers/mfd/rohm-bd9576.c        | 108
> > > > +++++++++++++++++++++++++++++++
> > > >  include/linux/mfd/rohm-bd957x.h  |  59 +++++++++++++++++
> > > >  include/linux/mfd/rohm-generic.h |   2 +
> > > >  5 files changed, 181 insertions(+)
> > > >  create mode 100644 drivers/mfd/rohm-bd9576.c
> > > >  create mode 100644 include/linux/mfd/rohm-bd957x.h
> > > 
> > > Looks like a possible candidate for "simple-mfd-i2c".
> > > 
> > > Could you look into that please?
> > > 
> > I must admit I didn't know about "simple-mfd-i2c". Good thing to
> > know
> > when working with simple devices :) Is this a new thing?
> 
> Yes, it's new.
> 
> > I am unsure I understand the idea fully. Should users put all the
> > different regamp configs in this file and just add the device IDs
> > with
> > pointer to correct config? (BD9576 and BD9573 need volatile
> > ranges).
> > Also, does this mean each sub-device should have own node and own
> > compatible in DT to get correctly load and probed? I guess this
> > would
> > need a buy-in from Rob too then.
> 
> You should describe the H/W in DT.

Yes. And it is described. But I've occasionally received request from
DT guys to add some properties directly to MFD node and not to add own
sub-node. This is what is done for example with the BD71837/47 clocks -
there is no own node for clk - the clk properties are placed directly
in MFD node (as was requested by Stephen and Rob back then - I
originally had own node for clk). I really have no clear view on when
things warrant for own subnode and when they don't - but as far as I
can see using simple-mfd-i2c forces one to always have a sub-node /
device. Even just a empty node with nothing but the compatible even if
device does not need stuff from DT? Anyways, I think this is nice
addition for simple drivers.

> > By the way - for uneducated eyes like mine this does not look like
> > it
> > has much to do with MFD as a device - here MFD reminds me of a
> > simple-
> > bus on top of I2C.
> 
> This is for MFD devices where the parent does little more than create
> a shared address space for child devices to operate on - like yours.
> 
> > Anyways, the BD9576 and BD9573 both have a few interrupts for
> > OVD/UVD
> > conditions and I am expecting that I will be asked to provide the
> > regulator notifiers for those. Reason why I omitted the IRQs for
> > now is
> > that the HW is designed to keep the IRQ asserted for whole error
> > duration so some delayed ack mechanism would be needed. I would
> > like to
> > keep the door open for adding IRQs to MFD core.
> 
> You mean to add an IRQ Domain?

Yes. I planned to use regmap-irq and create irq chip in MFD when the
over / under voltage / temperature - notifications or watchdog IRQs are
needed. 

Best Regards,
	Matti Vaittinen

Re: [PATCH RESEND v6 2/4] mfd: Support ROHM BD9576MUF and BD9573MUF

[Vaittinen, Matti]

On Wed, 2020-12-02 at 12:57 +0000, Lee Jones wrote:
> On Fri, 27 Nov 2020, Vaittinen, Matti wrote:
> 
> > Hello Lee,
> > 
> > On Fri, 2020-11-27 at 08:32 +0000, Lee Jones wrote:
> > > On Mon, 23 Nov 2020, Matti Vaittinen wrote:
> > > 
> > > > Add core support for ROHM BD9576MUF and BD9573MUF PMICs which
> > > > are
> > > > mainly used to power the R-Car series processors.
> > > > 
> > > > Signed-off-by: Matti Vaittinen <
> > > > matti.vaittinen@fi.rohmeurope.com>
> > > > ---
> > > >  drivers/mfd/Kconfig              |  11 ++++
> > > >  drivers/mfd/Makefile             |   1 +
> > > >  drivers/mfd/rohm-bd9576.c        | 108
> > > > +++++++++++++++++++++++++++++++
> > > >  include/linux/mfd/rohm-bd957x.h  |  59 +++++++++++++++++
> > > >  include/linux/mfd/rohm-generic.h |   2 +
> > > >  5 files changed, 181 insertions(+)
> > > >  create mode 100644 drivers/mfd/rohm-bd9576.c
> > > >  create mode 100644 include/linux/mfd/rohm-bd957x.h
> > > 
> > > Looks like a possible candidate for "simple-mfd-i2c".
> > > 
> > > Could you look into that please?
> > > 
> > I must admit I didn't know about "simple-mfd-i2c". Good thing to
> > know
> > when working with simple devices :) Is this a new thing?
> 
> Yes, it's new.
> 
> > I am unsure I understand the idea fully. Should users put all the
> > different regamp configs in this file and just add the device IDs
> > with
> > pointer to correct config? (BD9576 and BD9573 need volatile
> > ranges).
> > Also, does this mean each sub-device should have own node and own
> > compatible in DT to get correctly load and probed? I guess this
> > would
> > need a buy-in from Rob too then.
> 
> You should describe the H/W in DT.

After re-reading this - do you mean one should describe for example the
register ranges in DT? I don't see code which parses the volatile
ranges or other regmap configs here. I assume no. I guess you replied
to the question whether each sub device would need own node and
compatible.

Best Regards
	Matti Vaittinen

Re: [PATCH RESEND v6 2/4] mfd: Support ROHM BD9576MUF and BD9573MUF

[Vaittinen, Matti]

Hi deee Ho Lee,

On Wed, 2020-12-02 at 15:32 +0200, Matti Vaittinen wrote:
> Hello Lee,
> 
> On Wed, 2020-12-02 at 12:57 +0000, Lee Jones wrote:
> > On Fri, 27 Nov 2020, Vaittinen, Matti wrote:
> > 
> > > Hello Lee,
> > > 
> > > On Fri, 2020-11-27 at 08:32 +0000, Lee Jones wrote:
> > > > On Mon, 23 Nov 2020, Matti Vaittinen wrote:
> > > > 
> > > > > Add core support for ROHM BD9576MUF and BD9573MUF PMICs which
> > > > > are
> > > > > mainly used to power the R-Car series processors.
> > > > > 
> > > > > Signed-off-by: Matti Vaittinen <
> > > > > matti.vaittinen@fi.rohmeurope.com>
> > > > > ---
> > > > >  drivers/mfd/Kconfig              |  11 ++++
> > > > >  drivers/mfd/Makefile             |   1 +
> > > > >  drivers/mfd/rohm-bd9576.c        | 108
> > > > > +++++++++++++++++++++++++++++++
> > > > >  include/linux/mfd/rohm-bd957x.h  |  59 +++++++++++++++++
> > > > >  include/linux/mfd/rohm-generic.h |   2 +
> > > > >  5 files changed, 181 insertions(+)
> > > > >  create mode 100644 drivers/mfd/rohm-bd9576.c
> > > > >  create mode 100644 include/linux/mfd/rohm-bd957x.h
> > > > 
> > > > Looks like a possible candidate for "simple-mfd-i2c".
> > > > 
> > > > Could you look into that please?
> > > > 
> > > I must admit I didn't know about "simple-mfd-i2c". Good thing to
> > > know
> > > when working with simple devices :) Is this a new thing?
> > 
> > Yes, it's new.
> > 
> > > I am unsure I understand the idea fully. Should users put all the
> > > different regamp configs in this file and just add the device IDs
> > > with
> > > pointer to correct config? (BD9576 and BD9573 need volatile
> > > ranges).
> > > Also, does this mean each sub-device should have own node and own
> > > compatible in DT to get correctly load and probed? I guess this
> > > would
> > > need a buy-in from Rob too then.
> > 
> > You should describe the H/W in DT.
> 
> Yes. And it is described. But I've occasionally received request from
> DT guys to add some properties directly to MFD node and not to add
> own
> sub-node. This is what is done for example with the BD71837/47 clocks
> -
> there is no own node for clk - the clk properties are placed directly
> in MFD node (as was requested by Stephen and Rob back then - I
> originally had own node for clk). I really have no clear view on when
> things warrant for own subnode and when they don't - but as far as I
> can see using simple-mfd-i2c forces one to always have a sub-node /
> device. Even just a empty node with nothing but the compatible even
> if
> device does not need stuff from DT? Anyways, I think this is nice
> addition for simple drivers.
> 
> > > By the way - for uneducated eyes like mine this does not look
> > > like
> > > it
> > > has much to do with MFD as a device - here MFD reminds me of a
> > > simple-
> > > bus on top of I2C.
> > 
> > This is for MFD devices where the parent does little more than
> > create
> > a shared address space for child devices to operate on - like
> > yours.
> > 
> > > Anyways, the BD9576 and BD9573 both have a few interrupts for
> > > OVD/UVD
> > > conditions and I am expecting that I will be asked to provide the
> > > regulator notifiers for those. Reason why I omitted the IRQs for
> > > now is
> > > that the HW is designed to keep the IRQ asserted for whole error
> > > duration so some delayed ack mechanism would be needed. I would
> > > like to
> > > keep the door open for adding IRQs to MFD core.
> > 
> > You mean to add an IRQ Domain?
> 
> Yes. I planned to use regmap-irq and create irq chip in MFD when the
> over / under voltage / temperature - notifications or watchdog IRQs
> are
> needed. 

I am sorry if I have missed your reply. The ROHM email had redirected
almost all patch emails to spam + I am not sure if some mails are
dropping :(

(I am considering moving to gmail - but I'd rather keep all mails in
one system and I can't transfer work mail traffic to gmail... I wonder
how others are managing the mails - which mail system you are using?)

I think this series is now pending the decision how to proceed with MFD
part. If you still want me to start with "simple-mfd-i2c", then I would
appreciate if you pointed me how you would like to see the regmap
configs added. Although I am quite positive this (eventually) ends up
being more than what simple-mfd-i2c is intended for (because at some
point people want to add the use of the interrupts).

Best Regards
	Matti Vaittinen

Re: [PATCH RESEND v6 2/4] mfd: Support ROHM BD9576MUF and BD9573MUF

[Vaittinen, Matti]

Hello Again peeps,

On Thu, 2020-12-17 at 12:04 +0200, Matti Vaittinen wrote:
> On Wed, 2020-12-02 at 15:32 +0200, Matti Vaittinen wrote:
> > Hello Lee,
> > 
> > On Wed, 2020-12-02 at 12:57 +0000, Lee Jones wrote:
> > > On Fri, 27 Nov 2020, Vaittinen, Matti wrote:
> > > 
> > > > Hello Lee,
> > > > 
> > > > On Fri, 2020-11-27 at 08:32 +0000, Lee Jones wrote:
> > > > > On Mon, 23 Nov 2020, Matti Vaittinen wrote:
> > > > > 
> > > > > > Add core support for ROHM BD9576MUF and BD9573MUF PMICs
> > > > > > which
> > > > > > are
> > > > > > mainly used to power the R-Car series processors.
> > > > > > 
> > > > > > Signed-off-by: Matti Vaittinen <
> > > > > > matti.vaittinen@fi.rohmeurope.com>
> > > > > > ---
> > > > > >  drivers/mfd/Kconfig              |  11 ++++
> > > > > >  drivers/mfd/Makefile             |   1 +
> > > > > >  drivers/mfd/rohm-bd9576.c        | 108
> > > > > > +++++++++++++++++++++++++++++++
> > > > > >  include/linux/mfd/rohm-bd957x.h  |  59 +++++++++++++++++
> > > > > >  include/linux/mfd/rohm-generic.h |   2 +
> > > > > >  5 files changed, 181 insertions(+)
> > > > > >  create mode 100644 drivers/mfd/rohm-bd9576.c
> > > > > >  create mode 100644 include/linux/mfd/rohm-bd957x.h
> > > > > 
> > > > > Looks like a possible candidate for "simple-mfd-i2c".
> > > > > 
> > > > > Could you look into that please?
> > > > > 
> > > > I must admit I didn't know about "simple-mfd-i2c". Good thing
> > > > to
> > > > know
> > > > when working with simple devices :) Is this a new thing?
> > > 
> > > Yes, it's new.
> > > 
> > > > I am unsure I understand the idea fully. Should users put all
> > > > the
> > > > different regamp configs in this file and just add the device
> > > > IDs
> > > > with
> > > > pointer to correct config? (BD9576 and BD9573 need volatile
> > > > ranges).
> > > > Also, does this mean each sub-device should have own node and
> > > > own
> > > > compatible in DT to get correctly load and probed? I guess this
> > > > would
> > > > need a buy-in from Rob too then.
> > > 
> > > You should describe the H/W in DT.
> > 
> > Yes. And it is described. But I've occasionally received request
> > from
> > DT guys to add some properties directly to MFD node and not to add
> > own
> > sub-node. This is what is done for example with the BD71837/47
> > clocks
> > -
> > there is no own node for clk - the clk properties are placed
> > directly
> > in MFD node (as was requested by Stephen and Rob back then - I
> > originally had own node for clk). I really have no clear view on
> > when
> > things warrant for own subnode and when they don't - but as far as
> > I
> > can see using simple-mfd-i2c forces one to always have a sub-node /
> > device. Even just a empty node with nothing but the compatible even
> > if
> > device does not need stuff from DT? Anyways, I think this is nice
> > addition for simple drivers.
> > 
> > > > By the way - for uneducated eyes like mine this does not look
> > > > like
> > > > it
> > > > has much to do with MFD as a device - here MFD reminds me of a
> > > > simple-
> > > > bus on top of I2C.
> > > 
> > > This is for MFD devices where the parent does little more than
> > > create
> > > a shared address space for child devices to operate on - like
> > > yours.
> > > 
> > > > Anyways, the BD9576 and BD9573 both have a few interrupts for
> > > > OVD/UVD
> > > > conditions and I am expecting that I will be asked to provide
> > > > the
> > > > regulator notifiers for those. Reason why I omitted the IRQs
> > > > for
> > > > now is
> > > > that the HW is designed to keep the IRQ asserted for whole
> > > > error
> > > > duration so some delayed ack mechanism would be needed. I would
> > > > like to
> > > > keep the door open for adding IRQs to MFD core.
> > > 
> > > You mean to add an IRQ Domain?
> > 
> > Yes. I planned to use regmap-irq and create irq chip in MFD when
> > the
> > over / under voltage / temperature - notifications or watchdog IRQs
> > are
> > needed. 
> 
> I am sorry if I have missed your reply. The ROHM email had redirected
> almost all patch emails to spam + I am not sure if some mails are
> dropping :(
> 
> (I am considering moving to gmail - but I'd rather keep all mails in
> one system and I can't transfer work mail traffic to gmail... I
> wonder
> how others are managing the mails - which mail system you are using?)
> 
> I think this series is now pending the decision how to proceed with
> MFD
> part. If you still want me to start with "simple-mfd-i2c", then I
> would
> appreciate if you pointed me how you would like to see the regmap
> configs added. Although I am quite positive this (eventually) ends up
> being more than what simple-mfd-i2c is intended for (because at some
> point people want to add the use of the interrupts).

Looking at this topic again. I kind of understand the idea of combining
bunch of MFD drivers into one file. Many of the ROHM PMIC MFD drivers
do provide same functionality. Regmap configs, regmap IRQ and MFD
cells. Some do also probe the device. So having own file for each IC is
likely to not scale well when more devices are supported (and I do hope
this will be the case also with the ROHM ICs).

What bugs me with the simple-mfd-i2c here is:
1. Requiring to have own compatibles for sub-devices (regulator and
WDG) to get them properly probed. (3 compatibles for 1 IC).
2. Requiring to have own DT node for WDG.
3. Supporting differences between BD9576 and BD9573 by having 6
compatibles for 2 ICs.
4. Adding interrupt support.

So ... How do you see adding BD9576/BD9573 MFD stuff in BD9571/(BD9574)
MFD driver? The data structures (regmap configs, MFD cells, regmap IRQ
portion when added) will be different but the functions and maybe
engineers looking at these may be common.

Is it just plain confusing to add core structures for technically
different ICs in same file - or is it way to avoid duplicating same
code in many files? I can try adding the BD9576/BD9573 to the BD9571
core - or I can do resend this as is (rebased on 5.11). I can also hack
this to be kicked by simple-mfd-i2c (although I have these strong
objections) - but I bet it will in the long run just lead to a sub-
optimal solution. When the BD9576/BD9573 logic blocks are re-used in
some "non simple" designs and re-using the sub-drivers is needed and/or
when IRQs are needed.

(BTW - I am currently working with BD71815/BD71817 - and after this
discussion I will add these in BD71828/BD71878 MFD core. I had created
new MFD file for them but this discussion has been a nice kick to the
better direction for me)

Best Regards
	Matti Vaittinen

Re: [PATCH RESEND v6 2/4] mfd: Support ROHM BD9576MUF and BD9573MUF

[Lee Jones]

On Tue, 29 Dec 2020, Vaittinen, Matti wrote:

> Hello Again peeps,
> 
> On Thu, 2020-12-17 at 12:04 +0200, Matti Vaittinen wrote:
> > On Wed, 2020-12-02 at 15:32 +0200, Matti Vaittinen wrote:
> > > Hello Lee,
> > > 
> > > On Wed, 2020-12-02 at 12:57 +0000, Lee Jones wrote:
> > > > On Fri, 27 Nov 2020, Vaittinen, Matti wrote:
> > > > 
> > > > > Hello Lee,
> > > > > 
> > > > > On Fri, 2020-11-27 at 08:32 +0000, Lee Jones wrote:
> > > > > > On Mon, 23 Nov 2020, Matti Vaittinen wrote:
> > > > > > 
> > > > > > > Add core support for ROHM BD9576MUF and BD9573MUF PMICs
> > > > > > > which
> > > > > > > are
> > > > > > > mainly used to power the R-Car series processors.
> > > > > > > 
> > > > > > > Signed-off-by: Matti Vaittinen <
> > > > > > > matti.vaittinen@fi.rohmeurope.com>
> > > > > > > ---
> > > > > > >  drivers/mfd/Kconfig              |  11 ++++
> > > > > > >  drivers/mfd/Makefile             |   1 +
> > > > > > >  drivers/mfd/rohm-bd9576.c        | 108
> > > > > > > +++++++++++++++++++++++++++++++
> > > > > > >  include/linux/mfd/rohm-bd957x.h  |  59 +++++++++++++++++
> > > > > > >  include/linux/mfd/rohm-generic.h |   2 +
> > > > > > >  5 files changed, 181 insertions(+)
> > > > > > >  create mode 100644 drivers/mfd/rohm-bd9576.c
> > > > > > >  create mode 100644 include/linux/mfd/rohm-bd957x.h
> > > > > > 
> > > > > > Looks like a possible candidate for "simple-mfd-i2c".
> > > > > > 
> > > > > > Could you look into that please?
> > > > > > 
> > > > > I must admit I didn't know about "simple-mfd-i2c". Good thing
> > > > > to
> > > > > know
> > > > > when working with simple devices :) Is this a new thing?
> > > > 
> > > > Yes, it's new.
> > > > 
> > > > > I am unsure I understand the idea fully. Should users put all
> > > > > the
> > > > > different regamp configs in this file and just add the device
> > > > > IDs
> > > > > with
> > > > > pointer to correct config? (BD9576 and BD9573 need volatile
> > > > > ranges).
> > > > > Also, does this mean each sub-device should have own node and
> > > > > own
> > > > > compatible in DT to get correctly load and probed? I guess this
> > > > > would
> > > > > need a buy-in from Rob too then.
> > > > 
> > > > You should describe the H/W in DT.
> > > 
> > > Yes. And it is described. But I've occasionally received request
> > > from
> > > DT guys to add some properties directly to MFD node and not to add
> > > own
> > > sub-node. This is what is done for example with the BD71837/47
> > > clocks
> > > -
> > > there is no own node for clk - the clk properties are placed
> > > directly
> > > in MFD node (as was requested by Stephen and Rob back then - I
> > > originally had own node for clk). I really have no clear view on
> > > when
> > > things warrant for own subnode and when they don't - but as far as
> > > I
> > > can see using simple-mfd-i2c forces one to always have a sub-node /
> > > device. Even just a empty node with nothing but the compatible even
> > > if
> > > device does not need stuff from DT? Anyways, I think this is nice
> > > addition for simple drivers.
> > > 
> > > > > By the way - for uneducated eyes like mine this does not look
> > > > > like
> > > > > it
> > > > > has much to do with MFD as a device - here MFD reminds me of a
> > > > > simple-
> > > > > bus on top of I2C.
> > > > 
> > > > This is for MFD devices where the parent does little more than
> > > > create
> > > > a shared address space for child devices to operate on - like
> > > > yours.
> > > > 
> > > > > Anyways, the BD9576 and BD9573 both have a few interrupts for
> > > > > OVD/UVD
> > > > > conditions and I am expecting that I will be asked to provide
> > > > > the
> > > > > regulator notifiers for those. Reason why I omitted the IRQs
> > > > > for
> > > > > now is
> > > > > that the HW is designed to keep the IRQ asserted for whole
> > > > > error
> > > > > duration so some delayed ack mechanism would be needed. I would
> > > > > like to
> > > > > keep the door open for adding IRQs to MFD core.
> > > > 
> > > > You mean to add an IRQ Domain?
> > > 
> > > Yes. I planned to use regmap-irq and create irq chip in MFD when
> > > the
> > > over / under voltage / temperature - notifications or watchdog IRQs
> > > are
> > > needed. 
> > 
> > I am sorry if I have missed your reply. The ROHM email had redirected
> > almost all patch emails to spam + I am not sure if some mails are
> > dropping :(
> > 
> > (I am considering moving to gmail - but I'd rather keep all mails in
> > one system and I can't transfer work mail traffic to gmail... I
> > wonder
> > how others are managing the mails - which mail system you are using?)
> > 
> > I think this series is now pending the decision how to proceed with
> > MFD
> > part. If you still want me to start with "simple-mfd-i2c", then I
> > would
> > appreciate if you pointed me how you would like to see the regmap
> > configs added. Although I am quite positive this (eventually) ends up
> > being more than what simple-mfd-i2c is intended for (because at some
> > point people want to add the use of the interrupts).
> 
> Looking at this topic again. I kind of understand the idea of combining
> bunch of MFD drivers into one file. Many of the ROHM PMIC MFD drivers
> do provide same functionality. Regmap configs, regmap IRQ and MFD
> cells. Some do also probe the device. So having own file for each IC is
> likely to not scale well when more devices are supported (and I do hope
> this will be the case also with the ROHM ICs).
> 
> What bugs me with the simple-mfd-i2c here is:
> 1. Requiring to have own compatibles for sub-devices (regulator and
> WDG) to get them properly probed. (3 compatibles for 1 IC).
> 2. Requiring to have own DT node for WDG.
> 3. Supporting differences between BD9576 and BD9573 by having 6
> compatibles for 2 ICs.
> 4. Adding interrupt support.

Linux sees each of these functions as separate devices which are
handled in different ways by isolated subsystems.  So yes, they each
require their own compatible string regardless of whether they share
the same physical piece of silicon or not.

> So ... How do you see adding BD9576/BD9573 MFD stuff in BD9571/(BD9574)
> MFD driver? The data structures (regmap configs, MFD cells, regmap IRQ
> portion when added) will be different but the functions and maybe
> engineers looking at these may be common.
> 
> Is it just plain confusing to add core structures for technically
> different ICs in same file - or is it way to avoid duplicating same
> code in many files? I can try adding the BD9576/BD9573 to the BD9571
> core - or I can do resend this as is (rebased on 5.11). I can also hack
> this to be kicked by simple-mfd-i2c (although I have these strong
> objections) - but I bet it will in the long run just lead to a sub-
> optimal solution. When the BD9576/BD9573 logic blocks are re-used in
> some "non simple" designs and re-using the sub-drivers is needed and/or
> when IRQs are needed.
> 
> (BTW - I am currently working with BD71815/BD71817 - and after this
> discussion I will add these in BD71828/BD71878 MFD core. I had created
> new MFD file for them but this discussion has been a nice kick to the
> better direction for me)

Everything will be a trade-off.

There will either be superflouous files or inflexible code.

You have to make the right decision for the driver and the subsystem.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH RESEND v6 2/4] mfd: Support ROHM BD9576MUF and BD9573MUF

[Vaittinen, Matti]

Hello Lee,

Nice to see you are back :)

On Thu, 2021-01-14 at 10:00 +0000, Lee Jones wrote:
> On Tue, 29 Dec 2020, Vaittinen, Matti wrote:
> 
> > Hello Again peeps,
> > 
> > On Thu, 2020-12-17 at 12:04 +0200, Matti Vaittinen wrote:
> > > On Wed, 2020-12-02 at 15:32 +0200, Matti Vaittinen wrote:
> > > > Hello Lee,
> > > > 
> > > > On Wed, 2020-12-02 at 12:57 +0000, Lee Jones wrote:
> > > > > On Fri, 27 Nov 2020, Vaittinen, Matti wrote:
> > > > > 
> > > > > > Hello Lee,
> > > > > > 
> > > > > > On Fri, 2020-11-27 at 08:32 +0000, Lee Jones wrote:
> > > > > > > On Mon, 23 Nov 2020, Matti Vaittinen wrote:
> > > > > > > 
> > > > > > > > Add core support for ROHM BD9576MUF and BD9573MUF PMICs
> > > > > > > > which
> > > > > > > > are
> > > > > > > > mainly used to power the R-Car series processors.
> > > > > > > > 
> > > > > > > > Signed-off-by: Matti Vaittinen <
> > > > > > > > matti.vaittinen@fi.rohmeurope.com>
> > > > > > > > ---
> > > > > > > >  drivers/mfd/Kconfig              |  11 ++++
> > > > > > > >  drivers/mfd/Makefile             |   1 +
> > > > > > > >  drivers/mfd/rohm-bd9576.c        | 108
> > > > > > > > +++++++++++++++++++++++++++++++
> > > > > > > >  include/linux/mfd/rohm-bd957x.h  |  59
> > > > > > > > +++++++++++++++++
> > > > > > > >  include/linux/mfd/rohm-generic.h |   2 +
> > > > > > > >  5 files changed, 181 insertions(+)
> > > > > > > >  create mode 100644 drivers/mfd/rohm-bd9576.c
> > > > > > > >  create mode 100644 include/linux/mfd/rohm-bd957x.h
> > > > > > > 
> > > > > > > Looks like a possible candidate for "simple-mfd-i2c".
> > > > > > > 
> > > > > > > Could you look into that please?
> > > > > > > 
> > > > > > I must admit I didn't know about "simple-mfd-i2c". Good
> > > > > > thing
> > > > > > to
> > > > > > know
> > > > > > when working with simple devices :) Is this a new thing?
> > > > > 
> > > > > Yes, it's new.
> > > > > 
> > > > > > I am unsure I understand the idea fully. Should users put
> > > > > > all
> > > > > > the
> > > > > > different regamp configs in this file and just add the
> > > > > > device
> > > > > > IDs
> > > > > > with
> > > > > > pointer to correct config? (BD9576 and BD9573 need volatile
> > > > > > ranges).
> > > > > > Also, does this mean each sub-device should have own node
> > > > > > and
> > > > > > own
> > > > > > compatible in DT to get correctly load and probed? I guess
> > > > > > this
> > > > > > would
> > > > > > need a buy-in from Rob too then.
> > > > > 
> > > > > You should describe the H/W in DT.
> > > > 
> > > > Yes. And it is described. But I've occasionally received
> > > > request
> > > > from
> > > > DT guys to add some properties directly to MFD node and not to
> > > > add
> > > > own
> > > > sub-node. This is what is done for example with the BD71837/47
> > > > clocks
> > > > -
> > > > there is no own node for clk - the clk properties are placed
> > > > directly
> > > > in MFD node (as was requested by Stephen and Rob back then - I
> > > > originally had own node for clk). I really have no clear view
> > > > on
> > > > when
> > > > things warrant for own subnode and when they don't - but as far
> > > > as
> > > > I
> > > > can see using simple-mfd-i2c forces one to always have a sub-
> > > > node /
> > > > device. Even just a empty node with nothing but the compatible
> > > > even
> > > > if
> > > > device does not need stuff from DT? Anyways, I think this is
> > > > nice
> > > > addition for simple drivers.
> > > > 
> > > > > > By the way - for uneducated eyes like mine this does not
> > > > > > look
> > > > > > like
> > > > > > it
> > > > > > has much to do with MFD as a device - here MFD reminds me
> > > > > > of a
> > > > > > simple-
> > > > > > bus on top of I2C.
> > > > > 
> > > > > This is for MFD devices where the parent does little more
> > > > > than
> > > > > create
> > > > > a shared address space for child devices to operate on - like
> > > > > yours.
> > > > > 
> > > > > > Anyways, the BD9576 and BD9573 both have a few interrupts
> > > > > > for
> > > > > > OVD/UVD
> > > > > > conditions and I am expecting that I will be asked to
> > > > > > provide
> > > > > > the
> > > > > > regulator notifiers for those. Reason why I omitted the
> > > > > > IRQs
> > > > > > for
> > > > > > now is
> > > > > > that the HW is designed to keep the IRQ asserted for whole
> > > > > > error
> > > > > > duration so some delayed ack mechanism would be needed. I
> > > > > > would
> > > > > > like to
> > > > > > keep the door open for adding IRQs to MFD core.
> > > > > 
> > > > > You mean to add an IRQ Domain?
> > > > 
> > > > Yes. I planned to use regmap-irq and create irq chip in MFD
> > > > when
> > > > the
> > > > over / under voltage / temperature - notifications or watchdog
> > > > IRQs
> > > > are
> > > > needed. 
> > > 
> > > I am sorry if I have missed your reply. The ROHM email had
> > > redirected
> > > almost all patch emails to spam + I am not sure if some mails are
> > > dropping :(
> > > 
> > > (I am considering moving to gmail - but I'd rather keep all mails
> > > in
> > > one system and I can't transfer work mail traffic to gmail... I
> > > wonder
> > > how others are managing the mails - which mail system you are
> > > using?)
> > > 
> > > I think this series is now pending the decision how to proceed
> > > with
> > > MFD
> > > part. If you still want me to start with "simple-mfd-i2c", then I
> > > would
> > > appreciate if you pointed me how you would like to see the regmap
> > > configs added. Although I am quite positive this (eventually)
> > > ends up
> > > being more than what simple-mfd-i2c is intended for (because at
> > > some
> > > point people want to add the use of the interrupts).
> > 
> > Looking at this topic again. I kind of understand the idea of
> > combining
> > bunch of MFD drivers into one file. Many of the ROHM PMIC MFD
> > drivers
> > do provide same functionality. Regmap configs, regmap IRQ and MFD
> > cells. Some do also probe the device. So having own file for each
> > IC is
> > likely to not scale well when more devices are supported (and I do
> > hope
> > this will be the case also with the ROHM ICs).
> > 
> > What bugs me with the simple-mfd-i2c here is:
> > 1. Requiring to have own compatibles for sub-devices (regulator and
> > WDG) to get them properly probed. (3 compatibles for 1 IC).
> > 2. Requiring to have own DT node for WDG.
> > 3. Supporting differences between BD9576 and BD9573 by having 6
> > compatibles for 2 ICs.
> > 4. Adding interrupt support.
> 
> Linux sees each of these functions as separate devices which are
> handled in different ways by isolated subsystems.  So yes, they each
> require their own compatible string regardless of whether they share
> the same physical piece of silicon or not.

My understanding is that this is exactly why we have MFD? To bring all
the functions under one multifunctional device and to handle things
which are common to all blocks (like IRQs). Besides, like you  know
(better than me) we don't need additional compatibles or dummy dt-nodes 
when MFD instantiates the sub-devices.

> > So ... How do you see adding BD9576/BD9573 MFD stuff in
> > BD9571/(BD9574)
> > MFD driver? The data structures (regmap configs, MFD cells, regmap
> > IRQ
> > portion when added) will be different but the functions and maybe
> > engineers looking at these may be common.
> > 
> > Is it just plain confusing to add core structures for technically
> > different ICs in same file - or is it way to avoid duplicating same
> > code in many files? I can try adding the BD9576/BD9573 to the
> > BD9571
> > core - or I can do resend this as is (rebased on 5.11). I can also
> > hack
> > this to be kicked by simple-mfd-i2c (although I have these strong
> > objections) - but I bet it will in the long run just lead to a sub-
> > optimal solution. When the BD9576/BD9573 logic blocks are re-used
> > in
> > some "non simple" designs and re-using the sub-drivers is needed
> > and/or
> > when IRQs are needed.
> > 
> > (BTW - I am currently working with BD71815/BD71817 - and after this
> > discussion I will add these in BD71828/BD71878 MFD core. I had
> > created
> > new MFD file for them but this discussion has been a nice kick to
> > the
> > better direction for me)
> 
> Everything will be a trade-off.

Yes. The BD71815 is in my opinion something we can and should handle
with same MFD driver as BD71828. 

The BD9576/BD9573 I'd still go with own MFD (as was in this series).
But if you as the maintainer absolutely require me to rework it in some
direction (regardless of my reasoning here) - please let me know. I
would like to get this in a shape that rest of the drivers can also be
accepted in-tree. I think this has been waiting for the decision what
to do with the MFD for a while now.

> There will either be superflouous files or inflexible code.
> 
> You have to make the right decision for the driver and the subsystem.

I think I have explained what I think of but I need you to decide what
is best for the subsystem.


Best regards
	Matti Vaittinen