[PATCH v1 0/6] Support ROHM BD9576MUF and BD9573MUF PMICs

[PATCH v1 0/6] Support ROHM BD9576MUF and BD9573MUF PMICs

[Matti Vaittinen]

Initial support for ROHM BD9576MUF and BD9573MUF PMICs.

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.

Please note that this version of drivers is only tested on BD9576MUF
but according to the data-sheets the relevant parts of registers should
be same so drivers should also work on BD9573MUF.

This patch series includes MFD, watchdog and regulator drivers with
basic functionality such as:

- Enabling and pinging the watchdog
- configuring watchog timeout / window from device-tree
- reading regulator states/voltages
- enabling/disabling VOUT1 (VD50) when control mode B is used.

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.

---


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

 .../bindings/mfd/rohm,bd9576-pmic.yaml        | 129 +++++++
 .../regulator/rohm,bd9576-regulator.yaml      |  33 ++
 MAINTAINERS                                   |   4 +
 drivers/mfd/Kconfig                           |  11 +
 drivers/mfd/Makefile                          |   1 +
 drivers/mfd/rohm-bd9576.c                     | 130 +++++++
 drivers/regulator/Kconfig                     |  10 +
 drivers/regulator/Makefile                    |   1 +
 drivers/regulator/bd9576-regulator.c          | 337 ++++++++++++++++++
 drivers/watchdog/Kconfig                      |  13 +
 drivers/watchdog/Makefile                     |   1 +
 drivers/watchdog/bd9576_wdt.c                 | 295 +++++++++++++++
 include/linux/mfd/rohm-bd957x.h               |  61 ++++
 include/linux/mfd/rohm-generic.h              |   2 +
 14 files changed, 1028 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/rohm,bd9576-pmic.yaml
 create mode 100644 Documentation/devicetree/bindings/regulator/rohm,bd9576-regulator.yaml
 create mode 100644 drivers/mfd/rohm-bd9576.c
 create mode 100644 drivers/regulator/bd9576-regulator.c
 create mode 100644 drivers/watchdog/bd9576_wdt.c
 create mode 100644 include/linux/mfd/rohm-bd957x.h


base-commit: f4d51dffc6c01a9e94650d95ce0104964f8ae822
-- 
2.21.0


-- 
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 v1 1/6] 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>
---
 .../bindings/mfd/rohm,bd9576-pmic.yaml        | 129 ++++++++++++++++++
 1 file changed, 129 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..f17d4d621585
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/rohm,bd9576-pmic.yaml
@@ -0,0 +1,129 @@
+# 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
+
+  hw_margin_ms:
+    minimum: 4
+    maximum: 4416
+    description: Watchog timeout in milliseconds
+
+  rohm,hw-margin-min-ms:
+    minimum: 2
+    maximum: 220
+    description:
+      Watchdog on these ICs can be configured in a window mode where the ping
+      must come within certain time-window. Eg. too quick pinging will also
+      trigger timeout. Specify the minimum delay between pings if you wish to
+      use the window mode. Note, the maximum delay is internally configured as
+      a certain multiple of this value so maximum delay can be only up to 15
+      times this value. For example for 73 ms short ping value the maximum
+      timeout will be close to 1 sec.
+
+  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>;
+            hw_margin_ms = <30>;
+            rohm,hw-margin-min-ms = <4>;
+
+            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.21.0


-- 
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 v1 2/6] dt_bindings: regulator: Add ROHM BD9576MUF and BD9573MUF PMICs

[Matti Vaittinen]

    Add bindings for regulators on ROHM BD9576MUF and BD9573MUF PMICs.

Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
---
 .../regulator/rohm,bd9576-regulator.yaml      | 33 +++++++++++++++++++
 1 file changed, 33 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/rohm,bd9576-regulator.yaml

diff --git a/Documentation/devicetree/bindings/regulator/rohm,bd9576-regulator.yaml b/Documentation/devicetree/bindings/regulator/rohm,bd9576-regulator.yaml
new file mode 100644
index 000000000000..6df077b0db86
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/rohm,bd9576-regulator.yaml
@@ -0,0 +1,33 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/regulator/rohm,bd9576-regulator.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ROHM BD9576 and BD9573 Power Management Integrated Circuit regulators
+
+maintainers:
+  - Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
+
+description: |
+  This module is part of the ROHM BD9576 MFD device. For more details
+  see Documentation/devicetree/bindings/mfd/rohm,bd9576-pmic.yaml.
+
+  The regulator controller is represented as a sub-node of the PMIC node
+  on the device tree.
+
+  The valid names for BD9576 regulator nodes are
+  vd50, vd18, vdddr, vd10, voutl1, vouts1
+
+patternProperties:
+  "regulator-.+":
+    type: object
+    description:
+      Properties for single regulator.
+    $ref: "regulator.yaml#"
+
+    required:
+      - regulator-name
+
+    unevaluatedProperties: false
+additionalProperties: false
-- 
2.21.0


-- 
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 v1 3/6] 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        | 130 +++++++++++++++++++++++++++++++
 include/linux/mfd/rohm-bd957x.h  |  61 +++++++++++++++
 include/linux/mfd/rohm-generic.h |   2 +
 5 files changed, 205 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 33df0837ab41..e8275526818a 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1975,6 +1975,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 a60e5f835283..6ad33ccb35f0 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -260,6 +260,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..a23ded510209
--- /dev/null
+++ b/drivers/mfd/rohm-bd9576.c
@@ -0,0 +1,130 @@
+// 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[] = {
+	{
+		.range_min = BD957X_REG_SMRB_ASSERT,
+		.range_max = BD957X_REG_SMRB_ASSERT,
+	},
+	{
+		.range_min = BD957X_REG_PMIC_INTERNAL_STAT,
+		.range_max = BD957X_REG_PMIC_INTERNAL_STAT,
+	},
+	{
+		.range_min = BD957X_REG_INT_THERM_STAT,
+		.range_max = BD957X_REG_INT_THERM_STAT,
+	},
+	{
+		.range_min = BD957X_REG_INT_OVP_STAT,
+		.range_max = BD957X_REG_INT_SYS_STAT,
+	}, {
+		.range_min = BD957X_REG_INT_MAIN_STAT,
+		.range_max = 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 int chip_type;
+
+	chip_type = (unsigned int)(uintptr_t)
+		    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..4432e29d13e4
--- /dev/null
+++ b/include/linux/mfd/rohm-bd957x.h
@@ -0,0 +1,61 @@
+/* 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.21.0


-- 
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 v1 4/6] 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>
---
 drivers/watchdog/Kconfig      |  13 ++
 drivers/watchdog/Makefile     |   1 +
 drivers/watchdog/bd9576_wdt.c | 295 ++++++++++++++++++++++++++++++++++
 3 files changed, 309 insertions(+)
 create mode 100644 drivers/watchdog/bd9576_wdt.c

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index ab7aad5a1e69..d042a517a946 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 97bed1d3d97c..14d75f98e3df 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -208,6 +208,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..917c8c7ddeb1
--- /dev/null
+++ b/drivers/watchdog/bd9576_wdt.c
@@ -0,0 +1,295 @@
+// 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/delay.h>
+#include <linux/mfd/rohm-bd957x.h>
+#include <linux/module.h>
+#include <linux/gpio/consumer.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
+
+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, hw_margin_min;
+	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)) {
+		dev_err(dev, "getting watchdog-enable GPIO failed\n");
+		return PTR_ERR(priv->gpiod_en);
+	}
+
+	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)) {
+		dev_err(dev, "getting watchdog-ping GPIO failed\n");
+		return PTR_ERR(priv->gpiod_ping);
+	}
+
+	ret = of_property_read_u32(np,
+				   "hw_margin_ms", &hw_margin);
+	if (ret) {
+		if (ret != -EINVAL)
+			return ret;
+
+		hw_margin = BD957X_WDT_DEFAULT_MARGIN;
+	}
+
+	ret = of_property_read_u32(np, "rohm,hw-margin-min-ms", &hw_margin_min);
+	if (ret == -EINVAL)
+		hw_margin_min = 0;
+	else if (ret)
+		return ret;
+
+	ret = bd957x_set_wdt_mode(priv, hw_margin, 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;
+	priv->wdd.parent		= dev;
+	priv->wdd.timeout		= (hw_margin / 2) * 1000;
+
+	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.21.0


-- 
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 v1 5/6] regulator: Support ROHM BD9576MUF and BD9573MUF

[Matti Vaittinen]

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

The BD9576 includes some functional-safety features which are not
present on BD9573.

Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
---
 drivers/regulator/Kconfig            |  10 +
 drivers/regulator/Makefile           |   1 +
 drivers/regulator/bd9576-regulator.c | 337 +++++++++++++++++++++++++++
 3 files changed, 348 insertions(+)
 create mode 100644 drivers/regulator/bd9576-regulator.c

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index de17ef7e18f0..49e698c042b8 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -231,6 +231,16 @@ config REGULATOR_BD9571MWV
 	  This driver can also be built as a module. If so, the module
 	  will be called bd9571mwv-regulator.
 
+config REGULATOR_BD957XMUF
+	tristate "ROHM BD9576MUF and BD9573MUF Regulators"
+	depends on MFD_ROHM_BD957XMUF
+	help
+	  This driver supports voltage regulators on ROHM BD9576MUF and
+	  BD9573MUF PMICs.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called bd9576-regulator.
+
 config REGULATOR_CPCAP
 	tristate "Motorola CPCAP regulator"
 	depends on MFD_CPCAP
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index d8d3ecf526a8..92f50f12ca6d 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -32,6 +32,7 @@ obj-$(CONFIG_REGULATOR_BD70528) += bd70528-regulator.o
 obj-$(CONFIG_REGULATOR_BD71828) += bd71828-regulator.o
 obj-$(CONFIG_REGULATOR_BD718XX) += bd718x7-regulator.o
 obj-$(CONFIG_REGULATOR_BD9571MWV) += bd9571mwv-regulator.o
+obj-$(CONFIG_REGULATOR_BD957XMUF) += bd9576-regulator.o
 obj-$(CONFIG_REGULATOR_DA903X)	+= da903x-regulator.o
 obj-$(CONFIG_REGULATOR_DA9052)	+= da9052-regulator.o
 obj-$(CONFIG_REGULATOR_DA9055)	+= da9055-regulator.o
diff --git a/drivers/regulator/bd9576-regulator.c b/drivers/regulator/bd9576-regulator.c
new file mode 100644
index 000000000000..4137fa9a73bb
--- /dev/null
+++ b/drivers/regulator/bd9576-regulator.c
@@ -0,0 +1,337 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2020 ROHM Semiconductors
+// ROHM BD9576MUF/BD9573MUF regulator driver
+
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/gpio/consumer.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/mfd/rohm-bd957x.h>
+#include <linux/mfd/rohm-generic.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
+#include <linux/regulator/of_regulator.h>
+#include <linux/slab.h>
+
+#define BD957X_VOUTS1_VOLT	3300000
+#define BD957X_VOUTS4_BASE_VOLT	1030000
+#define BD957X_VOUTS34_NUM_VOLT	32
+
+static int vout1_volt_table[] = {5000000, 4900000, 4800000, 4700000, 4600000,
+				 4500000, 4500000, 4500000, 5000000, 5100000,
+				 5200000, 5300000, 5400000, 5500000, 5500000,
+				 5500000};
+
+static int vout2_volt_table[] = {1800000, 1780000, 1760000, 1740000, 1720000,
+				 1700000, 1680000, 1660000, 1800000, 1820000,
+				 1840000, 1860000, 1880000, 1900000, 1920000,
+				 1940000};
+
+static int voutl1_volt_table[] = {2500000, 2540000, 2580000, 2620000, 2660000,
+				  2700000, 2740000, 2780000, 2500000, 2460000,
+				  2420000, 2380000, 2340000, 2300000, 2260000,
+				  2220000};
+
+struct bd957x_regulator_data {
+	struct regulator_desc desc;
+	int base_voltage;
+};
+
+static int bd957x_vout34_list_voltage(struct regulator_dev *rdev,
+				      unsigned int selector)
+{
+	const struct regulator_desc *desc = rdev->desc;
+	int multiplier = selector & desc->vsel_mask & 0x7f;
+	int tune;
+
+	/* VOUT3 and 4 has 10mV step */
+	tune = multiplier * 10000;
+
+	if (!(selector & 0x80))
+		return desc->fixed_uV - tune;
+
+	return desc->fixed_uV + tune;
+}
+
+static int bd957x_list_voltage(struct regulator_dev *rdev,
+			       unsigned int selector)
+{
+	const struct regulator_desc *desc = rdev->desc;
+	int index = selector & desc->vsel_mask & 0x7f;
+
+	if (!(selector & 0x80))
+		index += desc->n_voltages/2;
+
+	if (index >= desc->n_voltages)
+		return -EINVAL;
+
+	return desc->volt_table[index];
+}
+
+static const struct regulator_ops bd957x_vout34_ops = {
+	.is_enabled = regulator_is_enabled_regmap,
+	.list_voltage = bd957x_vout34_list_voltage,
+	.get_voltage_sel = regulator_get_voltage_sel_regmap,
+};
+
+static const struct regulator_ops bd957X_vouts1_regulator_ops = {
+	.is_enabled = regulator_is_enabled_regmap,
+};
+
+static const struct regulator_ops bd957x_ops = {
+	.is_enabled = regulator_is_enabled_regmap,
+	.list_voltage = bd957x_list_voltage,
+	.get_voltage_sel = regulator_get_voltage_sel_regmap,
+};
+
+static struct bd957x_regulator_data bd9576_regulators[] = {
+	{
+		.desc = {
+			.name = "VD50",
+			.of_match = of_match_ptr("regulator-vd50"),
+			.regulators_node = of_match_ptr("regulators"),
+			.id = BD957X_VD50,
+			.type = REGULATOR_VOLTAGE,
+			.ops = &bd957x_ops,
+			.volt_table = &vout1_volt_table[0],
+			.n_voltages = ARRAY_SIZE(vout1_volt_table),
+			.vsel_reg = BD957X_REG_VOUT1_TUNE,
+			.vsel_mask = BD957X_MASK_VOUT1_TUNE,
+			.enable_reg = BD957X_REG_POW_TRIGGER1,
+			.enable_mask = BD957X_REGULATOR_EN_MASK,
+			.enable_val = BD957X_REGULATOR_DIS_VAL,
+			.enable_is_inverted = true,
+			.owner = THIS_MODULE,
+		},
+	},
+	{
+		.desc = {
+			.name = "VD18",
+			.of_match = of_match_ptr("regulator-vd18"),
+			.regulators_node = of_match_ptr("regulators"),
+			.id = BD957X_VD18,
+			.type = REGULATOR_VOLTAGE,
+			.ops = &bd957x_ops,
+			.volt_table = &vout2_volt_table[0],
+			.n_voltages = ARRAY_SIZE(vout2_volt_table),
+			.vsel_reg = BD957X_REG_VOUT2_TUNE,
+			.vsel_mask = BD957X_MASK_VOUT2_TUNE,
+			.enable_reg = BD957X_REG_POW_TRIGGER2,
+			.enable_mask = BD957X_REGULATOR_EN_MASK,
+			.enable_val = BD957X_REGULATOR_DIS_VAL,
+			.enable_is_inverted = true,
+			.owner = THIS_MODULE,
+		},
+	},
+	{
+		.desc = {
+			.name = "VDDDR",
+			.of_match = of_match_ptr("regulator-vdddr"),
+			.regulators_node = of_match_ptr("regulators"),
+			.id = BD957X_VDDDR,
+			.ops = &bd957x_vout34_ops,
+			.type = REGULATOR_VOLTAGE,
+			.n_voltages = BD957X_VOUTS34_NUM_VOLT,
+			.vsel_reg = BD957X_REG_VOUT3_TUNE,
+			.vsel_mask = BD957X_MASK_VOUT3_TUNE,
+			.enable_reg = BD957X_REG_POW_TRIGGER3,
+			.enable_mask = BD957X_REGULATOR_EN_MASK,
+			.enable_val = BD957X_REGULATOR_DIS_VAL,
+			.enable_is_inverted = true,
+			.owner = THIS_MODULE,
+		},
+	},
+	{
+		.desc = {
+			.name = "VD10",
+			.of_match = of_match_ptr("regulator-vd10"),
+			.regulators_node = of_match_ptr("regulators"),
+			.id = BD957X_VD10,
+			.ops = &bd957x_vout34_ops,
+			.type = REGULATOR_VOLTAGE,
+			.fixed_uV = BD957X_VOUTS4_BASE_VOLT,
+			.n_voltages = BD957X_VOUTS34_NUM_VOLT,
+			.vsel_reg = BD957X_REG_VOUT4_TUNE,
+			.vsel_mask = BD957X_MASK_VOUT4_TUNE,
+			.enable_reg = BD957X_REG_POW_TRIGGER4,
+			.enable_mask = BD957X_REGULATOR_EN_MASK,
+			.enable_val = BD957X_REGULATOR_DIS_VAL,
+			.enable_is_inverted = true,
+			.owner = THIS_MODULE,
+		},
+	},
+	{
+		.desc = {
+			.name = "VOUTL1",
+			.of_match = of_match_ptr("regulator-voutl1"),
+			.regulators_node = of_match_ptr("regulators"),
+			.id = BD957X_VOUTL1,
+			.ops = &bd957x_ops,
+			.type = REGULATOR_VOLTAGE,
+			.volt_table = &voutl1_volt_table[0],
+			.n_voltages = ARRAY_SIZE(voutl1_volt_table),
+			.vsel_reg = BD957X_REG_VOUTL1_TUNE,
+			.vsel_mask = BD957X_MASK_VOUTL1_TUNE,
+			.enable_reg = BD957X_REG_POW_TRIGGERL1,
+			.enable_mask = BD957X_REGULATOR_EN_MASK,
+			.enable_val = BD957X_REGULATOR_DIS_VAL,
+			.enable_is_inverted = true,
+			.owner = THIS_MODULE,
+		},
+	},
+	{
+		.desc = {
+			.name = "VOUTS1",
+			.of_match = of_match_ptr("regulator-vouts1"),
+			.regulators_node = of_match_ptr("regulators"),
+			.id = BD957X_VOUTS1,
+			.ops = &bd957X_vouts1_regulator_ops,
+			.type = REGULATOR_VOLTAGE,
+			.n_voltages = 1,
+			.fixed_uV = BD957X_VOUTS1_VOLT,
+			.enable_reg = BD957X_REG_POW_TRIGGERS1,
+			.enable_mask = BD957X_REGULATOR_EN_MASK,
+			.enable_val = BD957X_REGULATOR_DIS_VAL,
+			.enable_is_inverted = true,
+			.owner = THIS_MODULE,
+		},
+	},
+};
+
+static int bd957x_probe(struct platform_device *pdev)
+{
+	struct regmap *regmap;
+	struct regulator_config config = { 0 };
+	int i, err;
+	bool vout_mode, ddr_sel;
+	const struct bd957x_regulator_data *reg_data = &bd9576_regulators[0];
+	unsigned int num_reg_data = ARRAY_SIZE(bd9576_regulators);
+	enum rohm_chip_type chip = platform_get_device_id(pdev)->driver_data;
+
+	regmap = dev_get_regmap(pdev->dev.parent, NULL);
+	if (!regmap) {
+		dev_err(&pdev->dev, "No regmap\n");
+		return -EINVAL;
+	}
+	vout_mode = of_property_read_bool(pdev->dev.parent->of_node,
+					 "rohm,vout1-en-low");
+	if (vout_mode) {
+		struct gpio_desc *en;
+
+		dev_dbg(&pdev->dev, "GPIO controlled mode\n");
+
+		/* VOUT1 enable state judged by VOUT1_EN pin */
+		/* See if we have GPIO defined */
+		en = devm_gpiod_get_from_of_node(&pdev->dev,
+						 pdev->dev.parent->of_node,
+						 "rohm,vout1-en-gpios", 0,
+						 GPIOD_OUT_LOW, "vout1-en");
+		if (!IS_ERR(en)) {
+			/* VOUT1_OPS gpio ctrl */
+			/*
+			 * Regulator core prioritizes the ena_gpio over
+			 * enable/disable/is_enabled callbacks so no need to
+			 * clear them. We can still use same ops
+			 */
+			config.ena_gpiod = en;
+		} else {
+			/*
+			 * In theory it is possible someone wants to set
+			 * vout1-en LOW during OTP loading and set VOUT1 to be
+			 * controlled by GPIO - but control the GPIO from some
+			 * where else than this driver. For that to work we
+			 * should unset the is_enabled callback here.
+			 *
+			 * I believe such case where rohm,vout1-en-low is set
+			 * and vout1-en-gpios is not is likely to be a
+			 * misconfiguration. So let's just err out for now.
+			 */
+			dev_err(&pdev->dev,
+				"Failed to get VOUT1 control GPIO\n");
+			return PTR_ERR(en);
+		}
+	}
+
+	/*
+	 * If more than one PMIC needs to be controlled by same processor then
+	 * allocate the regulator data array here and use bd9576_regulators as
+	 * template. At the moment I see no such use-case so I spare some
+	 * bytes and use bd9576_regulators directly for non-constant configs
+	 * like DDR voltage selection.
+	 */
+	ddr_sel =  of_property_read_bool(pdev->dev.parent->of_node,
+					 "rohm,ddr-sel-low");
+	if (ddr_sel)
+		bd9576_regulators[2].desc.fixed_uV = 1350000;
+	else
+		bd9576_regulators[2].desc.fixed_uV = 1500000;
+
+	switch (chip) {
+	case ROHM_CHIP_TYPE_BD9576:
+		dev_dbg(&pdev->dev, "Found BD9576MUF\n");
+		break;
+	case ROHM_CHIP_TYPE_BD9573:
+		pr_info(&pdev->dev, "Found BD9573MUF\n");
+		break;
+	default:
+		dev_err(&pdev->dev, "Unsupported chip type\n");
+		err = -EINVAL;
+		goto err;
+	}
+
+	config.dev = pdev->dev.parent;
+	config.regmap = regmap;
+
+	for (i = 0; i < num_reg_data; i++) {
+
+		const struct regulator_desc *desc;
+		struct regulator_dev *rdev;
+		const struct bd957x_regulator_data *r;
+
+		r = &reg_data[i];
+		desc = &r->desc;
+
+		rdev = devm_regulator_register(&pdev->dev, desc, &config);
+		if (IS_ERR(rdev)) {
+			dev_err(&pdev->dev,
+				"failed to register %s regulator\n",
+				desc->name);
+			err = PTR_ERR(rdev);
+			goto err;
+		}
+		/*
+		 * Clear the VOUT1 GPIO setting - rest of the regulators do not
+		 * support GPIO control
+		 */
+		config.ena_gpiod = NULL;
+	}
+
+err:
+	return err;
+}
+
+static const struct platform_device_id bd957x_pmic_id[] = {
+	{ "bd9573-pmic", ROHM_CHIP_TYPE_BD9573 },
+	{ "bd9576-pmic", ROHM_CHIP_TYPE_BD9576 },
+	{ },
+};
+MODULE_DEVICE_TABLE(platform, bd957x_pmic_id);
+
+static struct platform_driver bd957x_regulator = {
+	.driver = {
+		.name = "bd957x-pmic",
+	},
+	.probe = bd957x_probe,
+	.id_table = bd957x_pmic_id,
+};
+
+module_platform_driver(bd957x_regulator);
+
+MODULE_AUTHOR("Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>");
+MODULE_DESCRIPTION("ROHM BD9576/BD9573 voltage regulator driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:bd957x-pmic");
-- 
2.21.0


-- 
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 v1 6/6] 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 b5cfab015bd6..630a23fc84f0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14956,16 +14956,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.21.0


-- 
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 v1 0/6] Support ROHM BD9576MUF and BD9573MUF PMICs

[Mark Brown]

On Thu, 17 Sep 2020 11:01:27 +0300, Matti Vaittinen wrote:
> Initial support for ROHM BD9576MUF and BD9573MUF PMICs.
> 
> 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.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next

Thanks!

[1/2] dt_bindings: regulator: Add ROHM BD9576MUF and BD9573MUF PMICs
      commit: fdb2f9ffc9f533ceef16666818557ea7b6edfe2a
[2/2] regulator: Support ROHM BD9576MUF and BD9573MUF
      commit: b014e9fae7e7de4329a7092ade4256982c5ce974

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

Re: [PATCH v1 4/6] wdt: Support wdt on ROHM BD9576MUF and BD9573MUF

[Guenter Roeck]

On 9/17/20 1:03 AM, Matti Vaittinen wrote:
> 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>
> ---
>  drivers/watchdog/Kconfig      |  13 ++
>  drivers/watchdog/Makefile     |   1 +
>  drivers/watchdog/bd9576_wdt.c | 295 ++++++++++++++++++++++++++++++++++
>  3 files changed, 309 insertions(+)
>  create mode 100644 drivers/watchdog/bd9576_wdt.c
> 
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index ab7aad5a1e69..d042a517a946 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 97bed1d3d97c..14d75f98e3df 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -208,6 +208,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..917c8c7ddeb1
> --- /dev/null
> +++ b/drivers/watchdog/bd9576_wdt.c
> @@ -0,0 +1,295 @@
> +// 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/delay.h>

Alphabetic include file order please.

> +#include <linux/mfd/rohm-bd957x.h>
> +#include <linux/module.h>
> +#include <linux/gpio/consumer.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
> +
> +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, hw_margin_min;
> +	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)) {

devm_gpiod_get_from_of_node() can return -EPROBE_DEFER in which case we don't want to see
an error message. I would suggest to use dev_err_probe().

> +		dev_err(dev, "getting watchdog-enable GPIO failed\n");
> +		return PTR_ERR(priv->gpiod_en);
> +	}
> +
> +	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)) {
> +		dev_err(dev, "getting watchdog-ping GPIO failed\n");
> +		return PTR_ERR(priv->gpiod_ping);
> +	}

Same as above.

> +
> +	ret = of_property_read_u32(np,
> +				   "hw_margin_ms", &hw_margin);

Line splits are arbitrary. Why is this "hw_margin_ms" and not "rohm,hw_margin_ms" ?

> +	if (ret) {
> +		if (ret != -EINVAL)
> +			return ret;
> +
> +		hw_margin = BD957X_WDT_DEFAULT_MARGIN;
> +	}
> +
> +	ret = of_property_read_u32(np, "rohm,hw-margin-min-ms", &hw_margin_min);
> +	if (ret == -EINVAL)
> +		hw_margin_min = 0;
> +	else if (ret)
> +		return ret;

Please use a single mechanism to handle -EINVAL after of_property_read_u32().

> +
> +	ret = bd957x_set_wdt_mode(priv, hw_margin, hw_margin_min);
> +	if (ret)
> +		return ret;
> +
> +	priv->always_running = of_property_read_bool(np,
> +						     "always-running");

Another arbitrary line split.

> +
> +	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;
> +	priv->wdd.parent		= dev;
> +	priv->wdd.timeout		= (hw_margin / 2) * 1000;
> +
> +	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");
> 

Re: [PATCH v1 4/6] wdt: Support wdt on ROHM BD9576MUF and BD9573MUF

[Vaittinen, Matti]

Thanks (again) for review Guenter!

On Thu, 2020-09-17 at 22:45 -0700, Guenter Roeck wrote:
> On 9/17/20 1:03 AM, Matti Vaittinen wrote:
> > 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>
> > ---
> >  drivers/watchdog/Kconfig      |  13 ++
> >  drivers/watchdog/Makefile     |   1 +
> >  drivers/watchdog/bd9576_wdt.c | 295
> > ++++++++++++++++++++++++++++++++++
> >  3 files changed, 309 insertions(+)
> >  create mode 100644 drivers/watchdog/bd9576_wdt.c

> 
> > +
> > +	ret = of_property_read_u32(np,
> > +				   "hw_margin_ms", &hw_margin);
> 
> Line splits are arbitrary. Why is this "hw_margin_ms" and not
> "rohm,hw_margin_ms" ?

"hw_margin_ms" is an existing binding for specifying the maximum TMO in
HW (if I understood it correctly). (It is used at least by the generig
GPIO watchdog) I thought it's better to not invent a new vendor
specific binding when we have a generic one.

https://elixir.bootlin.com/linux/v5.9-rc2/source/Documentation/devicetree/bindings/watchdog/gpio-wdt.txt

> 
> > +	if (ret) {
> > +		if (ret != -EINVAL)
> > +			return ret;
> > +
> > +		hw_margin = BD957X_WDT_DEFAULT_MARGIN;
> > +	}
> > +
> > +	ret = of_property_read_u32(np, "rohm,hw-margin-min-ms",
> > &hw_margin_min);
> > +	if (ret == -EINVAL)
> > +		hw_margin_min = 0;
> > +	else if (ret)
> > +		return ret;
> 
> Please use a single mechanism to handle -EINVAL after
> of_property_read_u32().

Sorry Guenter - I am probably a bit slow today but I am unsure if I
understand the suggestion. Do you mean something like:

hw_margin_min = 0;

ret = of_property_read_u32(np, "rohm,hw-margin-min-ms",
&hw_margin_min);
if (ret && ret != -EINVAL)
	return ret;

Other than that - all findings are clear to me. I'll craft a new
version but I'll wait for a while to see if Lee has time to send some
feedback on MFD so I could get both WDG and MFD fixes to same version
:)

Best Regards
	--Matti


--
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 for the translation Simon)

Re: [PATCH v1 1/6] dt_bindings: mfd: Add ROHM BD9576MUF and BD9573MUF PMICs

[Rob Herring]

On Thu, Sep 17, 2020 at 11:01:52AM +0300, Matti Vaittinen wrote:
> 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>
> ---
>  .../bindings/mfd/rohm,bd9576-pmic.yaml        | 129 ++++++++++++++++++
>  1 file changed, 129 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..f17d4d621585
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/rohm,bd9576-pmic.yaml
> @@ -0,0 +1,129 @@
> +# 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
> +
> +  hw_margin_ms:

Needs a vendor prefix.

s/_/-/

> +    minimum: 4
> +    maximum: 4416
> +    description: Watchog timeout in milliseconds

Maybe the words in the description should be in the property name as 
I don't see how 'h/w margin' relates to 'watchdog timeout'.

Is this a max and below is the min?:

> +
> +  rohm,hw-margin-min-ms:
> +    minimum: 2
> +    maximum: 220
> +    description:
> +      Watchdog on these ICs can be configured in a window mode where the ping
> +      must come within certain time-window. Eg. too quick pinging will also
> +      trigger timeout. Specify the minimum delay between pings if you wish to
> +      use the window mode. Note, the maximum delay is internally configured as
> +      a certain multiple of this value so maximum delay can be only up to 15
> +      times this value. For example for 73 ms short ping value the maximum
> +      timeout will be close to 1 sec.
> +
> +  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>;
> +            hw_margin_ms = <30>;
> +            rohm,hw-margin-min-ms = <4>;
> +
> +            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.21.0
> 
> 
> -- 
> 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 v1 4/6] wdt: Support wdt on ROHM BD9576MUF and BD9573MUF

[Guenter Roeck]

On 9/17/20 11:06 PM, Vaittinen, Matti wrote:
> Thanks (again) for review Guenter!
> 
> On Thu, 2020-09-17 at 22:45 -0700, Guenter Roeck wrote:
>> On 9/17/20 1:03 AM, Matti Vaittinen wrote:
>>> 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>
>>> ---
>>>  drivers/watchdog/Kconfig      |  13 ++
>>>  drivers/watchdog/Makefile     |   1 +
>>>  drivers/watchdog/bd9576_wdt.c | 295
>>> ++++++++++++++++++++++++++++++++++
>>>  3 files changed, 309 insertions(+)
>>>  create mode 100644 drivers/watchdog/bd9576_wdt.c
> 
>>
>>> +
>>> +	ret = of_property_read_u32(np,
>>> +				   "hw_margin_ms", &hw_margin);
>>
>> Line splits are arbitrary. Why is this "hw_margin_ms" and not
>> "rohm,hw_margin_ms" ?
> 
> "hw_margin_ms" is an existing binding for specifying the maximum TMO in
> HW (if I understood it correctly). (It is used at least by the generig
> GPIO watchdog) I thought it's better to not invent a new vendor
> specific binding when we have a generic one.
> 
> https://elixir.bootlin.com/linux/v5.9-rc2/source/Documentation/devicetree/bindings/watchdog/gpio-wdt.txt
> 
>>
>>> +	if (ret) {
>>> +		if (ret != -EINVAL)
>>> +			return ret;
>>> +
>>> +		hw_margin = BD957X_WDT_DEFAULT_MARGIN;
>>> +	}
>>> +
>>> +	ret = of_property_read_u32(np, "rohm,hw-margin-min-ms",
>>> &hw_margin_min);
>>> +	if (ret == -EINVAL)
>>> +		hw_margin_min = 0;
>>> +	else if (ret)
>>> +		return ret;
>>
>> Please use a single mechanism to handle -EINVAL after
>> of_property_read_u32().
> 
> Sorry Guenter - I am probably a bit slow today but I am unsure if I
> understand the suggestion. Do you mean something like:
> 

    if (ret) {
>>> +		if (ret != -EINVAL)
>>> +			return ret;
>>> +
>>> +		hw_margin = BD957X_WDT_DEFAULT_MARGIN;
>>> +	}

vs.

>>> +	if (ret == -EINVAL)
>>> +		hw_margin_min = 0;
>>> +	else if (ret)
>>> +		return ret;

is not very consistent to me.

Guenter

> hw_margin_min = 0;
> 
> ret = of_property_read_u32(np, "rohm,hw-margin-min-ms",
> &hw_margin_min);
> if (ret && ret != -EINVAL)
> 	return ret;
> 
> Other than that - all findings are clear to me. I'll craft a new
> version but I'll wait for a while to see if Lee has time to send some
> feedback on MFD so I could get both WDG and MFD fixes to same version
> :)
> 
> Best Regards
> 	--Matti
> 
> 
> --
> 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 for the translation Simon)
> 

Re: [PATCH v1 1/6] dt_bindings: mfd: Add ROHM BD9576MUF and BD9573MUF PMICs

[Vaittinen, Matti]

Thanks Rob for taking a look at this!

On Fri, 2020-09-18 at 11:28 -0600, Rob Herring wrote:
> On Thu, Sep 17, 2020 at 11:01:52AM +0300, Matti Vaittinen wrote:
> > 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>
> > ---
> >  .../bindings/mfd/rohm,bd9576-pmic.yaml        | 129
> > ++++++++++++++++++
> >  1 file changed, 129 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..f17d4d621585
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mfd/rohm,bd9576-pmic.yaml
> > @@ -0,0 +1,129 @@
> > +# 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
> > +
> > +  hw_margin_ms:
> 
> Needs a vendor prefix.
> 
> s/_/-/
> 
> > +    minimum: 4
> > +    maximum: 4416
> > +    description: Watchog timeout in milliseconds
> 
> Maybe the words in the description should be in the property name as 
> I don't see how 'h/w margin' relates to 'watchdog timeout'.

The hw_margin_ms is an existing property. As I wrote to Guenter:
"hw_margin_ms" is an existing binding for specifying the maximum TMO in
HW (if I understood it correctly). (It is used at least by the generig
GPIO watchdog) I thought it's better to not invent a new vendor
specific binding when we have a generic one.

https://elixir.bootlin.com/linux/v5.9-rc2/source/Documentation/devicetree/bindings/watchdog/gpio-wdt.txt

> 
> Is this a max and below is the min?:
> 
> > +
> > +  rohm,hw-margin-min-ms:
> > +    minimum: 2
> > +    maximum: 220
> > +    description:
> > +      Watchdog on these ICs can be configured in a window mode
> > where the ping
> > +      must come within certain time-window. Eg. too quick pinging
> > will also
> > +      trigger timeout. Specify the minimum delay between pings if
> > you wish to
> > +      use the window mode. Note, the maximum delay is internally
> > configured as
> > +      a certain multiple of this value so maximum delay can be
> > only up to 15
> > +      times this value. For example for 73 ms short ping value the
> > maximum
> > +      timeout will be close to 1 sec.

Yes. I didn't find existing property for "minimum time to ping WDG"
from existing properties. And I think it is not so common to have such
"ping window" in watchdog HW that it would warrant generic minimum tmo.
Hence vendor specific property for minimum tmo.

Would you still prefer me to introduce a new vendor specific property
for "max tmo"?

Best Regards
	Matti Vaittinen

Re: [PATCH v1 1/6] dt_bindings: mfd: Add ROHM BD9576MUF and BD9573MUF PMICs

[Rob Herring]

On Sat, Sep 19, 2020 at 5:46 AM Vaittinen, Matti
<Matti.Vaittinen@fi.rohmeurope.com> wrote:
>
> Thanks Rob for taking a look at this!
>
> On Fri, 2020-09-18 at 11:28 -0600, Rob Herring wrote:
> > On Thu, Sep 17, 2020 at 11:01:52AM +0300, Matti Vaittinen wrote:
> > > 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>
> > > ---
> > >  .../bindings/mfd/rohm,bd9576-pmic.yaml        | 129
> > > ++++++++++++++++++
> > >  1 file changed, 129 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..f17d4d621585
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/mfd/rohm,bd9576-pmic.yaml
> > > @@ -0,0 +1,129 @@
> > > +# 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
> > > +
> > > +  hw_margin_ms:
> >
> > Needs a vendor prefix.
> >
> > s/_/-/
> >
> > > +    minimum: 4
> > > +    maximum: 4416
> > > +    description: Watchog timeout in milliseconds
> >
> > Maybe the words in the description should be in the property name as
> > I don't see how 'h/w margin' relates to 'watchdog timeout'.
>
> The hw_margin_ms is an existing property. As I wrote to Guenter:
> "hw_margin_ms" is an existing binding for specifying the maximum TMO in
> HW (if I understood it correctly). (It is used at least by the generig
> GPIO watchdog) I thought it's better to not invent a new vendor
> specific binding when we have a generic one.
>
> https://elixir.bootlin.com/linux/v5.9-rc2/source/Documentation/devicetree/bindings/watchdog/gpio-wdt.txt

That one is odd and I haven't found an actual user of it. It would
make more sense as a collection of properties devices could use rather
than a virtual device.

I think I'd do something like 'watchdog-ping-time-msec' that can be
either '<min> <max>' or '<max>'.

Rob

Re: [PATCH v1 1/6] dt_bindings: mfd: Add ROHM BD9576MUF and BD9573MUF PMICs

[Vaittinen, Matti]

On Wed, 2020-09-23 at 08:27 -0600, Rob Herring wrote:
> On Sat, Sep 19, 2020 at 5:46 AM Vaittinen, Matti
> <Matti.Vaittinen@fi.rohmeurope.com> wrote:
> > Thanks Rob for taking a look at this!
> > 
> > On Fri, 2020-09-18 at 11:28 -0600, Rob Herring wrote:
> > > On Thu, Sep 17, 2020 at 11:01:52AM +0300, Matti Vaittinen wrote:
> > > > 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>
> > > > ---
> > > >  .../bindings/mfd/rohm,bd9576-pmic.yaml        | 129
> > > > ++++++++++++++++++
> > > >  1 file changed, 129 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..f17d4d621585
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/mfd/rohm,bd9576-
> > > > pmic.yaml
> > > > @@ -0,0 +1,129 @@
> > > > +# 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
> > > > +
> > > > +  hw_margin_ms:
> > > 
> > > Needs a vendor prefix.
> > > 
> > > s/_/-/
> > > 
> > > > +    minimum: 4
> > > > +    maximum: 4416
> > > > +    description: Watchog timeout in milliseconds
> > > 
> > > Maybe the words in the description should be in the property name
> > > as
> > > I don't see how 'h/w margin' relates to 'watchdog timeout'.
> > 
> > The hw_margin_ms is an existing property. As I wrote to Guenter:
> > "hw_margin_ms" is an existing binding for specifying the maximum
> > TMO in
> > HW (if I understood it correctly). (It is used at least by the
> > generig
> > GPIO watchdog) I thought it's better to not invent a new vendor
> > specific binding when we have a generic one.
> > 
> > https://elixir.bootlin.com/linux/v5.9-rc2/source/Documentation/devicetree/bindings/watchdog/gpio-wdt.txt
> 
> That one is odd and I haven't found an actual user of it. It would
> make more sense as a collection of properties devices could use
> rather
> than a virtual device.
> 
> I think I'd do something like 'watchdog-ping-time-msec' that can be
> either '<min> <max>' or '<max>'.

Your suggestion looks good to me. If we introduce such then it would
make sense to add handling for this in GPIO watchdog too.

What I do wonder is how "hw_margin_ms" is unused? I see it is a
required property for GPIO watchdog. The GPIO WDG probe seems to
actually error out if reading this property fails with any error. I
would assume the GPIO WDG is used somewhere? Hence I am a bit afraid of
touching it. Breaking existing setups would not be nice.

Guenter - how do you see this? Should we leave GPIO WDG as it is,
convert it to use this new binding Rob suggested - or support both the
old and new (at least for some time) in the driver - and possibly print
a warning when old is used?

Best Regards
	Matti Vaittinen

Re: [PATCH v1 1/6] dt_bindings: mfd: Add ROHM BD9576MUF and BD9573MUF PMICs

[Vaittinen, Matti]

Hi dee Ho peeps!

On Thu, 2020-09-24 at 09:12 +0300, Matti Vaittinen wrote:
> On Wed, 2020-09-23 at 08:27 -0600, Rob Herring wrote:
> > On Sat, Sep 19, 2020 at 5:46 AM Vaittinen, Matti
> > <Matti.Vaittinen@fi.rohmeurope.com> wrote:
> > > Thanks Rob for taking a look at this!
> > > 
> > > On Fri, 2020-09-18 at 11:28 -0600, Rob Herring wrote:
> > > > On Thu, Sep 17, 2020 at 11:01:52AM +0300, Matti Vaittinen
> > > > wrote:
> > > > > 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>
> > > > > ---
> > > > >  .../bindings/mfd/rohm,bd9576-pmic.yaml        | 129
> > > > > ++++++++++++++++++
> > > > >  1 file changed, 129 insertions(+)
> > > > >  create mode 100644
> > > > > Documentation/devicetree/bindings/mfd/rohm,bd9576-pmic.yaml

// Snip

> > > > > +
> > > > > +  hw_margin_ms:
> > > > 
> > > > Needs a vendor prefix.
> > > > 
> > > > s/_/-/
> > > > 
> > > > > +    minimum: 4
> > > > > +    maximum: 4416
> > > > > +    description: Watchog timeout in milliseconds
> > > > 
> > > > Maybe the words in the description should be in the property
> > > > name
> > > > as
> > > > I don't see how 'h/w margin' relates to 'watchdog timeout'.
> > > 
> > > The hw_margin_ms is an existing property. As I wrote to Guenter:
> > > "hw_margin_ms" is an existing binding for specifying the maximum
> > > TMO in
> > > HW (if I understood it correctly). (It is used at least by the
> > > generig
> > > GPIO watchdog) I thought it's better to not invent a new vendor
> > > specific binding when we have a generic one.
> > > 
> > > https://elixir.bootlin.com/linux/v5.9-rc2/source/Documentation/devicetree/bindings/watchdog/gpio-wdt.txt
> > 
> > That one is odd and I haven't found an actual user of it. It would
> > make more sense as a collection of properties devices could use
> > rather
> > than a virtual device.
> > 
> > I think I'd do something like 'watchdog-ping-time-msec' that can be
> > either '<min> <max>' or '<max>'.
> 
> Your suggestion looks good to me. If we introduce such then it would
> make sense to add handling for this in GPIO watchdog too.
> 
> What I do wonder is how "hw_margin_ms" is unused? I see it is a
> required property for GPIO watchdog. The GPIO WDG probe seems to
> actually error out if reading this property fails with any error. I
> would assume the GPIO WDG is used somewhere? Hence I am a bit afraid
> of
> touching it. Breaking existing setups would not be nice.
> 
> Guenter - how do you see this? Should we leave GPIO WDG as it is,
> convert it to use this new binding Rob suggested - or support both
> the
> old and new (at least for some time) in the driver - and possibly
> print
> a warning when old is used?

And one thing more - I don't think the 'watchdog-ping-time-msec' is
best candidate as it sounds like the time when one should ping the WDG
(SW feature). We already have the timeout-sec defined for that. This
new property is to configure/advertice the HW time limit - Eg, time
when WDG takes action if it has not been pinged (for max) or takes
action if WDG is pinged too quickly (min). Thus I liked hw-margin
better than ping-time. (For example with hw-margin <500ms> ... <4000ms>
the ping-time 1000 ms would be just fine.

Couple of things I would like to get opinions for ... 

1. Corect location for this binding - and should it be vendor specific
or generic?
 - I wonder if I should put this new property in rohm-bd9576.yaml? 
 - Should it be vendor specific? 
 - Or should I put it in watchdog.yaml and make it generic?

I think it should be generic as many wdg chips implement the timeout
configuration.

2. Should we extend WDG core to parse this property if it is placed in
watchdog.yaml?
2a) And should we extend watchdog core to call the driver callback for
setting timeout if it finds the <max> tmo?
2b) Should we extend driver IF to allow callback for setting min tmo?
2c) Current tmo setting callback uses units of seconds. Should we
support setting TMO in ms? I think it might make sense for few specific
Linux setups. (I know people use Linux for things that are almost RT -
no matter how clever that is. So some might benefit from sub-second
scale wdg window).

Thoughts?

Best Regards
	Matti Vaittinen


--
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 for the translation Simon)