Support ROHM BD96801 scalable PMIC

ROHM BD96801 is a highly configurable automotive grade PMIC. Introduce DT bindings for the BD96801 regulators.

Signed-off-by Matti Vaittinen <[email protected]>

---
Revision history:
- No changes since RFCv1

 .../regulator/rohm,bd96801-regulator.yaml     | 69 +++++++++++++++++++
 1 file changed, 69 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/rohm,bd96801-regulator.yaml

Documentation/devicetree/bindings/regulator/rohm,bd96801-regulator.yaml new file +14 -0

new file mode 100644
index 000000000000..4015802a3d84
@@ -0,0 +1,69 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/regulator/rohm,bd96801-regulator.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ROHM BD96801 Power Management Integrated Circuit regulators
+
+maintainers:
+  - Matti Vaittinen <[email protected]>
+
+description: |
+  This module is part of the ROHM BD96801 MFD device. For more details
+  see Documentation/devicetree/bindings/mfd/rohm,bd96801-pmic.yaml.

Documentation/devicetree/bindings/regulator/rohm,bd96801-regulator.yaml new file +7 -0

+    type: object
+    description:
+      Properties for single BUCK regulator.
+    $ref: regulator.yaml#
+
+    properties:
+      regulator-name:

Quoted 7 lines · Matti Vaittinen

ROHM BD96801 is a highly configurable automotive grade PMIC. Introduce DT bindings for the BD96801 regulators.

Signed-off-by Matti Vaittinen <[email protected]>

---
Revision history:
- No changes since RFCv1

Subject: missing "regulator" prefix, as first.

Quoted 25 lines

  .../regulator/rohm,bd96801-regulator.yaml     | 69 +++++++++++++++++++
  1 file changed, 69 insertions(+)

create mode 100644 Documentation/devicetree/bindings/regulator/rohm,bd96801-regulator.yaml

Documentation/devicetree/bindings/regulator/rohm,bd96801-regulator.yaml new file +14 -0

new file mode 100644
index 000000000000..4015802a3d84
@@ -0,0 +1,69 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/regulator/rohm,bd96801-regulator.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ROHM BD96801 Power Management Integrated Circuit regulators
+
+maintainers:
+  - Matti Vaittinen <[email protected]>
+
+description: |
+  This module is part of the ROHM BD96801 MFD device. For more details
+  see Documentation/devicetree/bindings/mfd/rohm,bd96801-pmic.yaml.

Documentation/devicetree/bindings/regulator/rohm,bd96801-regulator.yaml line 24

Quoted 10 lines

Documentation/devicetree/bindings/regulator/rohm,bd96801-regulator.yaml new file +10 -0

+
+  The regulator controller is represented as a sub-node of the PMIC node
+  on the device tree.
+
+  Regulator nodes should be named to BUCK_<number> and LDO_<number>.
+  The valid names for BD96801 regulator nodes are
+  BUCK1, BUCK2, BUCK3, BUCK4, LDO5, LDO6, LDO7
+
+patternProperties:
+  "^LDO[5-7]$":

lowercase

Documentation/devicetree/bindings/regulator/rohm,bd96801-regulator.yaml line 28

Quoted 4 lines

Documentation/devicetree/bindings/regulator/rohm,bd96801-regulator.yaml new file +4 -0

+    type: object
+    description:
+      Properties for single LDO regulator.
+    $ref: regulator.yaml#

Missing unevaluatedProperties: false

Documentation/devicetree/bindings/regulator/rohm,bd96801-regulator.yaml line 34

Quoted 6 lines

Documentation/devicetree/bindings/regulator/rohm,bd96801-regulator.yaml new file +6 -0

+
+    properties:
+      regulator-name:
+        pattern: "^ldo[5-7]$"
+        description:
+          Name of the regulator. Should be "ldo5", ..., "ldo7"

Why do you enforce the name? The name should match board schematics, not regulator datasheet.

Documentation/devicetree/bindings/regulator/rohm,bd96801-regulator.yaml line 41

Quoted 7 lines

Documentation/devicetree/bindings/regulator/rohm,bd96801-regulator.yaml new file +7 -0

+      rohm,initial-voltage-microvolt:
+        description:
+          Initial voltage for regulator. Voltage can be tuned +/-150 mV from
+          this value. NOTE, This can be modified via I2C only when PMIC is in
+          STBY state.
+        minimum: 300000
+        maximum: 3300000

Hm, regulator min/max microvolts properties don't work for you? The initial will be just middle?

Quoted 63 lines · Krzysztof Kozlowski

Quoted 7 lines · Matti Vaittinen

ROHM BD96801 is a highly configurable automotive grade PMIC. Introduce DT bindings for the BD96801 regulators.

Signed-off-by Matti Vaittinen <[email protected]>

---
Revision history:
- No changes since RFCv1

Subject: missing "regulator" prefix, as first.

Quoted 25 lines

  .../regulator/rohm,bd96801-regulator.yaml     | 69 +++++++++++++++++++
  1 file changed, 69 insertions(+)

create mode 100644 Documentation/devicetree/bindings/regulator/rohm,bd96801-regulator.yaml

Documentation/devicetree/bindings/regulator/rohm,bd96801-regulator.yaml new file +14 -0

new file mode 100644
index 000000000000..4015802a3d84
@@ -0,0 +1,69 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/regulator/rohm,bd96801-regulator.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ROHM BD96801 Power Management Integrated Circuit regulators
+
+maintainers:
+  - Matti Vaittinen <[email protected]>
+
+description: |
+  This module is part of the ROHM BD96801 MFD device. For more details
+  see Documentation/devicetree/bindings/mfd/rohm,bd96801-pmic.yaml.

Documentation/devicetree/bindings/regulator/rohm,bd96801-regulator.yaml line 24

Quoted 10 lines

Documentation/devicetree/bindings/regulator/rohm,bd96801-regulator.yaml new file +10 -0

+
+  The regulator controller is represented as a sub-node of the PMIC node
+  on the device tree.
+
+  Regulator nodes should be named to BUCK_<number> and LDO_<number>.
+  The valid names for BD96801 regulator nodes are
+  BUCK1, BUCK2, BUCK3, BUCK4, LDO5, LDO6, LDO7
+
+patternProperties:
+  "^LDO[5-7]$":

lowercase

Documentation/devicetree/bindings/regulator/rohm,bd96801-regulator.yaml line 28

Quoted 4 lines

Documentation/devicetree/bindings/regulator/rohm,bd96801-regulator.yaml new file +4 -0

+    type: object
+    description:
+      Properties for single LDO regulator.
+    $ref: regulator.yaml#

Missing unevaluatedProperties: false

Documentation/devicetree/bindings/regulator/rohm,bd96801-regulator.yaml line 34

Quoted 6 lines

Documentation/devicetree/bindings/regulator/rohm,bd96801-regulator.yaml new file +6 -0

+
+    properties:
+      regulator-name:
+        pattern: "^ldo[5-7]$"
+        description:
+          Name of the regulator. Should be "ldo5", ..., "ldo7"

Quoted 2 lines

Why do you enforce the name? The name should match board schematics, not regulator datasheet.

If my memory serves me right, the slightly peculiar thing with the regulator core is it does matching of the regulators based on the names of the nodes. There was the regulator-compatible property, but I think it has been deprecated long ago.

https://elixir.bootlin.com/linux/latest/source/drivers/regulator/of_regulator.c#L380

Hence the regulators tend to have fixed names for the nodes. Unless there has been some recent changes I am not aware of...

Quoted 11 lines

Documentation/devicetree/bindings/regulator/rohm,bd96801-regulator.yaml line 41

Quoted 8 lines

Documentation/devicetree/bindings/regulator/rohm,bd96801-regulator.yaml new file +7 -0

+      rohm,initial-voltage-microvolt:
+        description:
+          Initial voltage for regulator. Voltage can be tuned +/-150 mV from
+          this value. NOTE, This can be modified via I2C only when PMIC is in
+          STBY state.
+        minimum: 300000
+        maximum: 3300000

Hm, regulator min/max microvolts properties don't work for you? The initial will be just middle?

I had not even thought of this!

I think this is a good idea. The problem I see is if the system where the PMIC is used will need to have 'initial power level' at start-up, which is near the one end of the allowed voltage area. (This because the "tuning"-range is quite narrow after the initial voltage is set). Wide allowed voltage range may be needed if the PMIC is reconfigured using the PMIC STBY state during the runtime.

Eg, sequence would look like:

Bootup:
PMIC STBY:
- initial value 'A' from DT
=> PMIC ACTIVE
- desired (early) voltages 'A' + 'tune'

...

Voltage state differing more than the 'tune' needed due to some runtime use-case:
=> PMIC STBY
- initial value 'B'
=> PMIC ACTIVE
- desired voltages 'B' + 'tune'

Now, if the 'A' can be 'far' from the mid point of the 'allowed voltages' -range.

I have no idea how valid this use-case is though. Once again, I work for a component vendor and don't get to see the forest from the trees... But sure I would like to enable as many possible use-cases as, well, possible :)

ROHM BD96801 is a highly configurable automotive grade PMIC. Introduce DT bindings for the BD96801 core.

Signed-off-by Matti Vaittinen <[email protected]>

---
Revision history:
RFCv1 => RFCv2:
- Document rohm,hw-timeout-ms
- Document rohm,wdg-action

---
 .../bindings/mfd/rohm,bd96801-pmic.yaml       | 171 ++++++++++++++++++
 1 file changed, 171 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/rohm,bd96801-pmic.yaml

Documentation/devicetree/bindings/mfd/rohm,bd96801-pmic.yaml new file +2 -0

new file mode 100644
index 000000000000..31ef787d6a8a
@@ -0,0 +1,171 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2

Documentation/devicetree/bindings/mfd/rohm,bd96801-pmic.yaml new file +14 -0

+    maxItems: 1
+
+  interrupts:
+    description:
+      The PMIC provides intb and errb IRQ lines. The errb IRQ line is used
+      for fatal IRQs which will cause the PMIC to shut down power outputs.
+      In many systems this will shut down the SoC contolling the PMIC and
+      connecting/handling the errb can be omitted. However, there are cases
+      where the SoC is not powered by the PMIC. In that case it may be
+      useful to connect the errb and handle errb events.
+    minItems: 1
+    maxItems: 2
+
+  interrupt-names:

Documentation/devicetree/bindings/mfd/rohm,bd96801-pmic.yaml line 57

Quoted 7 lines

Documentation/devicetree/bindings/mfd/rohm,bd96801-pmic.yaml new file +7 -0

+  rohm,wdg-action:
+    description:
+      Whether the watchdog failure must turn off the regulator power outputs or
+      just toggle the INTB line.
+    enum:
+      - prstb
+      - intb-only

This is second property controlling bite behavior. The other being:
https://lore.kernel.org/all/[email protected]/

Probably we need common property in watchdog.yaml.

On 12/04/2024 13:21, Matti Vaittinen wrote

Documentation/devicetree/bindings/mfd/rohm,bd96801-pmic.yaml line 47

Quoted 6 lines

Documentation/devicetree/bindings/mfd/rohm,bd96801-pmic.yaml new file +6 -0

+
+  rohm,hw-timeout-ms:
+    description:
+      Watchdog timeout value(s). First walue is timeout limit. Second value is
+      optional value for 'too early' watchdog ping if window timeout mode is
+      to be used.

Standard property timeout-sec does not work for you? It should allow two items as well.

Missing allOf

Morning Krzysztof,

Thanks again for the review/help!

Quoted 10 lines · Krzysztof Kozlowski

On 12/04/2024 13:21, Matti Vaittinen wrote

Quoted 8 lines

+
+  rohm,hw-timeout-ms:
+    description:
+      Watchdog timeout value(s). First walue is timeout limit. Second value is
+      optional value for 'too early' watchdog ping if window timeout mode is
+      to be used.

Quoted 2 lines

Standard property timeout-sec does not work for you? It should allow two items as well.

I don't think so. We need sub-second units. Furthermore, the timeout-sec (if I understand it correctly) updates the "timeout policy", which tells the expected ping-interval. This can be different from the "HW heart-beat" which tells the HW's ping expectation. Hence the "hw-" prefix.

Quoted 15 lines · Matti Vaittinen

Morning Krzysztof,

Thanks again for the review/help!

Quoted 10 lines · Krzysztof Kozlowski

On 12/04/2024 13:21, Matti Vaittinen wrote

Quoted 8 lines

+
+  rohm,hw-timeout-ms:
+    description:
+      Watchdog timeout value(s). First walue is timeout limit. Second value is
+      optional value for 'too early' watchdog ping if window timeout mode is
+      to be used.

Quoted 7 lines

Quoted 2 lines

Standard property timeout-sec does not work for you? It should allow two items as well.

I don't think so. We need sub-second units. Furthermore, the timeout-sec (if I understand it correctly) updates the "timeout policy", which tells the expected ping-interval. This can be different from the "HW heart-beat" which tells the HW's ping expectation. Hence the "hw-" prefix.

Oh, I just found out that this is an existing property. The ROHM BD9576/BD9573 do aleady use this. It seems I've had some discussion about it with Rob/Guenter when adding it. Frightening thing is that I didin't remember the discussion or that the property existed at all... Well, luckily we have lore :)

https://lore.kernel.org/all/[email protected]/#r

(I don't see the final conclusion in this discussion, it has probably been done on some later version of the series).

Quoted 16 lines · Matti Vaittinen

ROHM BD96801 is a highly configurable automotive grade PMIC. Introduce DT bindings for the BD96801 core.

Signed-off-by Matti Vaittinen <[email protected]>

---
Revision history:
RFCv1 => RFCv2:
- Document rohm,hw-timeout-ms
- Document rohm,wdg-action

---
  .../bindings/mfd/rohm,bd96801-pmic.yaml       | 171 ++++++++++++++++++
  1 file changed, 171 insertions(+)

create mode 100644 Documentation/devicetree/bindings/mfd/rohm,bd96801-pmic.yaml

Documentation/devicetree/bindings/mfd/rohm,bd96801-pmic.yaml +0 -0

Missing allOf and $ref to watchdog.yaml

Quoted 2 lines

Missing allOf and $ref to watchdog.yaml

Huh. The watchdog.yaml contains:

select:

   properties:
     $nodename:
       pattern: "^watchdog(@.*|-([0-9]|[1-9][0-9]+))?$"

properties:

   $nodename:
     pattern: "^(timer|watchdog)(@.*|-([0-9]|[1-9][0-9]+))?$"

This means the watchdog _must_ have own sub-node inside the PMIC node,

Quoted 10 lines · Matti Vaittinen

Quoted 2 lines

Missing allOf and $ref to watchdog.yaml

Huh. The watchdog.yaml contains:

select:

    properties:
      $nodename:
        pattern: "^watchdog(@.*|-([0-9]|[1-9][0-9]+))?$"

Quoted 6 lines

properties:

    $nodename:
      pattern: "^(timer|watchdog)(@.*|-([0-9]|[1-9][0-9]+))?$"

This means the watchdog _must_ have own sub-node inside the PMIC node,

Yes, that's a bit of a trouble.

Then instead maybe just add timeout-sec
with maxItems: 2.

Introduce driver for WDG block on ROHM BD96801 scalable PMIC.

This driver only supports watchdog with I2C feeding and delayed response detection. Whether the watchdog toggles PRSTB pin or just causes an interrupt can be configured via device-tree.

The BD96801 PMIC HW supports also window watchdog (too early feeding detection) and Q&A mode. These are not supported by this driver.

Signed-off-by Matti Vaittinen <[email protected]>

---
Revision history:
RFCv1 => RFCv2:
- remove always running
- add IRQ handling
- call emergency_restart()
- drop MODULE_ALIAS and add MODULE_DEVICE_TABLE

---
 drivers/watchdog/Kconfig       |  13 ++
 drivers/watchdog/Makefile      |   1 +
 drivers/watchdog/bd96801_wdt.c | 389 +++++++++++++++++++++++++++++++++
 3 files changed, 403 insertions(+)
 create mode 100644 drivers/watchdog/bd96801_wdt.c

drivers/watchdog/Kconfig +13 -0

index 6bee137cfbe0..d97e735e1faa 100644
@@ -181,6 +181,19 @@ config BD957XMUF_WATCHDOG
 	  watchdog. Alternatively say M to compile the driver as a module,
 	  which will be called bd9576_wdt.
 
+config BD96801_WATCHDOG
+	tristate "ROHM BD96801 PMIC Watchdog"
+	depends on MFD_ROHM_BD96801
+	select WATCHDOG_CORE
+	help
+	  Support for the watchdog in the ROHM BD96801 PMIC. Watchdog can be
+	  configured to only generate IRQ or to trigger system reset via reset
+	  pin.
+
+	  Say Y here to include support for the ROHM BD96801 watchdog.
+	  Alternatively say M to compile the driver as a module,
+	  which will be called bd96801_wdt.
+
 config CROS_EC_WATCHDOG
 	tristate "ChromeOS EC-based watchdog"
 	select WATCHDOG_CORE

drivers/watchdog/Makefile +1 -0

index 3710c218f05e..31bc94436c81 100644
@@ -217,6 +217,7 @@ obj-$(CONFIG_XEN_WDT) += xen_wdt.o
 
 # Architecture Independent
 obj-$(CONFIG_BD957XMUF_WATCHDOG) += bd9576_wdt.o
+obj-$(CONFIG_BD96801_WATCHDOG) += bd96801_wdt.o
 obj-$(CONFIG_CROS_EC_WATCHDOG) += cros_ec_wdt.o
 obj-$(CONFIG_DA9052_WATCHDOG) += da9052_wdt.o
 obj-$(CONFIG_DA9055_WATCHDOG) += da9055_wdt.o

drivers/watchdog/bd96801_wdt.c new file +88 -0

new file mode 100644
index 000000000000..08fab9a87aec
@@ -0,0 +1,389 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2024 ROHM Semiconductors
+ *
+ * ROHM BD96801 watchdog driver
+ */
+
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/mfd/rohm-bd96801.h>
+#include <linux/mfd/rohm-generic.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 BD96801_WD_TMO_SHORT_MASK	0x70
+#define BD96801_WD_RATIO_MASK		0x3
+#define BD96801_WD_TYPE_MASK		0x4
+#define BD96801_WD_TYPE_SLOW		0x4
+#define BD96801_WD_TYPE_WIN		0x0
+
+#define BD96801_WD_EN_MASK		0x3
+#define BD96801_WD_IF_EN		0x1
+#define BD96801_WD_QA_EN		0x2
+#define BD96801_WD_DISABLE		0x0
+
+#define BD96801_WD_ASSERT_MASK		0x8
+#define BD96801_WD_ASSERT_RST		0x8
+#define BD96801_WD_ASSERT_IRQ		0x0
+
+#define BD96801_WD_FEED_MASK		0x1
+#define BD96801_WD_FEED			0x1
+
+/* units in uS */
+#define FASTNG_MIN			3370
+#define BD96801_WDT_DEFAULT_MARGIN	6905120
+/* Unit is seconds */
+#define DEFAULT_TIMEOUT 30
+
+/*
+ * BD96801 WDG supports window mode so the TMO consists of SHORT and LONG
+ * timeout values. SHORT time is meaningfull only in window mode where feeding
+ * period shorter than SHORT would be an error. LONG time is used to detect if
+ * feeding is not occurring within given time limit (SoC SW hangs). The LONG
+ * timeout time is a multiple of (2, 4, 8 0r 16 times) the SHORT timeout.
+ */
+
+struct wdtbd96801 {
+	struct device		*dev;
+	struct regmap		*regmap;
+	struct watchdog_device	wdt;
+};
+
+static int bd96801_wdt_ping(struct watchdog_device *wdt)
+{
+	struct wdtbd96801 *w = watchdog_get_drvdata(wdt);
+
+	return regmap_update_bits(w->regmap, BD96801_REG_WD_FEED,
+				 BD96801_WD_FEED_MASK, BD96801_WD_FEED);
+}
+
+static int bd96801_wdt_start(struct watchdog_device *wdt)
+{
+	struct wdtbd96801 *w = watchdog_get_drvdata(wdt);
+	int ret;
+
+	ret = regmap_update_bits(w->regmap, BD96801_REG_WD_CONF,
+				 BD96801_WD_EN_MASK, BD96801_WD_IF_EN);
+
+	return ret;
+}
+
+static int bd96801_wdt_stop(struct watchdog_device *wdt)
+{
+	struct wdtbd96801 *w = watchdog_get_drvdata(wdt);
+
+	return regmap_update_bits(w->regmap, BD96801_REG_WD_CONF,
+				  BD96801_WD_EN_MASK, BD96801_WD_DISABLE);
+}
+
+static const struct watchdog_info bd96801_wdt_info = {

drivers/watchdog/bd96801_wdt.c new file +147 -0

+
+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, 4, 8, 16};
+
+	for (sel = 0; sel < ARRAY_SIZE(multipliers) &&
+	     multipliers[sel] * fast_val < *target; sel++)
+		;
+
+	if (sel == ARRAY_SIZE(multipliers))
+		return -EINVAL;
+
+	*slowsel = sel;
+	*target = multipliers[sel] * fast_val;
+
+	return 0;
+}
+
+static int find_closest_slow(int *target, int *slow_sel, int *fast_sel)
+{
+	static const int multipliers[] = {2, 4, 8, 16};
+	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;
+
+	*target = val;
+
+	return 0;
+}
+
+static int bd96801_set_wdt_mode(struct wdtbd96801 *w, int hw_margin,
+			       int hw_margin_min)
+{
+	int ret, fastng, slowng, type, reg, mask;
+	struct device *dev = w->dev;
+
+	/* convert to uS */
+	hw_margin *= 1000;
+	hw_margin_min *= 1000;
+	if (hw_margin_min) {
+		int min;
+
+		type = BD96801_WD_TYPE_WIN;
+		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;
+		}
+		w->wdt.min_hw_heartbeat_ms = min / 1000;
+	} else {
+		type = BD96801_WD_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;
+		}
+	}
+
+	w->wdt.max_hw_heartbeat_ms = hw_margin / 1000;
+
+	fastng <<= ffs(BD96801_WD_TMO_SHORT_MASK) - 1;
+
+	reg = slowng | fastng;
+	mask = BD96801_WD_RATIO_MASK | BD96801_WD_TMO_SHORT_MASK;
+	ret = regmap_update_bits(w->regmap, BD96801_REG_WD_TMO,
+				 mask, reg);
+	if (ret)
+		return ret;
+
+	ret = regmap_update_bits(w->regmap, BD96801_REG_WD_CONF,
+				 BD96801_WD_TYPE_MASK, type);
+
+	return ret;
+}
+
+static int bd96801_set_heartbeat_from_hw(struct wdtbd96801 *w,
+					 unsigned int conf_reg)
+{
+	int ret;
+	unsigned int val, sel, fast;
+
+	/*
+	 * The BD96801 supports a somewhat peculiar QA-mode, which we do not
+	 * support in this driver. If the QA-mode is enabled then we just
+	 * warn and bail-out.
+	 */
+	if ((conf_reg & BD96801_WD_EN_MASK) != BD96801_WD_IF_EN) {
+		dev_warn(w->dev, "watchdog set to Q&A mode - exiting\n");
+		return -EINVAL;
+	}
+
+	ret = regmap_read(w->regmap, BD96801_REG_WD_TMO, &val);
+	if (ret)
+		return ret;
+
+	sel = val & BD96801_WD_TMO_SHORT_MASK;
+	sel >>= ffs(BD96801_WD_TMO_SHORT_MASK) - 1;
+	fast = FASTNG_MIN << sel;
+
+	sel = (val & BD96801_WD_RATIO_MASK) + 1;
+	w->wdt.max_hw_heartbeat_ms = (fast << sel) / USEC_PER_MSEC;
+
+	if ((conf_reg & BD96801_WD_TYPE_MASK) == BD96801_WD_TYPE_WIN)
+		w->wdt.min_hw_heartbeat_ms = fast / USEC_PER_MSEC;
+

drivers/watchdog/bd96801_wdt.c new file +41 -0

+	if (count < 0 && count != -EINVAL)
+		return count;
+
+	if (count > 0) {
+		if (count > ARRAY_SIZE(hw_margin))
+			return -EINVAL;
+
+		ret = device_property_read_u32_array(w->dev->parent,
+						     "rohm,hw-timeout-ms",
+						     &hw_margin[0], count);
+		if (ret < 0)
+			return ret;
+
+		if (count == 1)
+			hw_margin_max = hw_margin[0];
+
+		if (count == 2) {
+			hw_margin_max = hw_margin[1];
+			hw_margin_min = hw_margin[0];
+		}
+	}
+
+	ret = bd96801_set_wdt_mode(w, hw_margin_max, hw_margin_min);
+	if (ret)
+		return ret;
+
+	ret = device_property_match_string(w->dev->parent, "rohm,wdg-action",
+					   "prstb");
+	if (ret >= 0) {
+		ret = regmap_update_bits(w->regmap, BD96801_REG_WD_CONF,
+				 BD96801_WD_ASSERT_MASK,
+				 BD96801_WD_ASSERT_RST);
+		return ret;
+	}
+
+	ret = device_property_match_string(w->dev->parent, "rohm,wdg-action",
+					   "intb-only");
+	if (ret >= 0) {
+		ret = regmap_update_bits(w->regmap, BD96801_REG_WD_CONF,
+				 BD96801_WD_ASSERT_MASK,
+				 BD96801_WD_ASSERT_IRQ);

drivers/watchdog/bd96801_wdt.c new file +83 -0

+	return IRQ_NONE;
+}
+
+static int bd96801_wdt_probe(struct platform_device *pdev)
+{
+	struct wdtbd96801 *w;
+	int ret, irq;
+	unsigned int val;
+
+	w = devm_kzalloc(&pdev->dev, sizeof(*w), GFP_KERNEL);
+	if (!w)
+		return -ENOMEM;
+
+	w->regmap = dev_get_regmap(pdev->dev.parent, NULL);
+	w->dev = &pdev->dev;
+
+	w->wdt.info = &bd96801_wdt_info;
+	w->wdt.ops =  &bd96801_wdt_ops;
+	w->wdt.parent = pdev->dev.parent;
+	w->wdt.timeout = DEFAULT_TIMEOUT;
+	watchdog_set_drvdata(&w->wdt, w);
+
+	ret = regmap_read(w->regmap, BD96801_REG_WD_CONF, &val);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret,
+				     "Failed to get the watchdog state\n");
+
+	/*
+	 * If the WDG is already enabled we assume it is configured by boot.
+	 * In this case we just update the hw-timeout based on values set to
+	 * the timeout / mode registers and leave the hardware configs
+	 * untouched.
+	 */
+	if ((val & BD96801_WD_EN_MASK) != BD96801_WD_DISABLE) {
+		dev_dbg(&pdev->dev, "watchdog was running during probe\n");
+		ret = bd96801_set_heartbeat_from_hw(w, val);
+		if (ret)
+			return ret;
+
+		set_bit(WDOG_HW_RUNNING, &w->wdt.status);
+	} else {
+		/* If WDG is not running so we will initializate it */
+		ret = init_wdg_hw(w);
+		if (ret)
+			return ret;
+	}
+
+	watchdog_init_timeout(&w->wdt, 0, pdev->dev.parent);
+	watchdog_set_nowayout(&w->wdt, nowayout);
+	watchdog_stop_on_reboot(&w->wdt);
+
+	irq = platform_get_irq_byname(pdev, "bd96801-wdg");
+	if (irq > 0) {
+		ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
+						bd96801_irq_hnd,
+						IRQF_ONESHOT,  "bd96801-wdg",
+						NULL);
+		if (ret)
+			return dev_err_probe(&pdev->dev, ret,
+					     "Failed to register IRQ\n");
+	}
+
+	return devm_watchdog_register_device(&pdev->dev, &w->wdt);
+}
+
+static const struct platform_device_id bd96801_wdt_id[] = {
+	{ "bd96801-wdt", },
+	{ }
+};
+MODULE_DEVICE_TABLE(platform, bd96801_wdt_id);
+
+static struct platform_driver bd96801_wdt = {
+	.driver = {
+		.name = "bd96801-wdt"
+	},
+	.probe = bd96801_wdt_probe,
+	.id_table = bd96801_wdt_id,
+};
+module_platform_driver(bd96801_wdt);
+
+MODULE_AUTHOR("Matti Vaittinen <[email protected]>");
+MODULE_DESCRIPTION("BD96801 watchdog driver");
+MODULE_LICENSE("GPL");

Signature · 14 lines

2.43.2


-- 
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 =] 

Quoted 25 lines · Matti Vaittinen

Introduce driver for WDG block on ROHM BD96801 scalable PMIC.

This driver only supports watchdog with I2C feeding and delayed response detection. Whether the watchdog toggles PRSTB pin or just causes an interrupt can be configured via device-tree.

The BD96801 PMIC HW supports also window watchdog (too early feeding detection) and Q&A mode. These are not supported by this driver.

Signed-off-by Matti Vaittinen <[email protected]>

---
Revision history:
RFCv1 => RFCv2:
- remove always running
- add IRQ handling
- call emergency_restart()
- drop MODULE_ALIAS and add MODULE_DEVICE_TABLE

---
  drivers/watchdog/Kconfig       |  13 ++
  drivers/watchdog/Makefile      |   1 +
  drivers/watchdog/bd96801_wdt.c | 389 +++++++++++++++++++++++++++++++++
  3 files changed, 403 insertions(+)

create mode 100644 drivers/watchdog/bd96801_wdt.c

drivers/watchdog/bd96801_wdt.c line 98

Quoted 7 lines

4

drivers/watchdog/bd96801_wdt.c new file +6 -0

+
+static const struct watchdog_ops bd96801_wdt_ops = {
+	.start		= bd96801_wdt_start,
+	.stop		= bd96801_wdt_stop,
+	.ping		= bd96801_wdt_ping,
+};

Is there no way to setup a timeout to the WDOG device from userspace?

drivers/watchdog/bd96801_wdt.c line 255

Quoted 8 lines

drivers/watchdog/bd96801_wdt.c new file +8 -0

+
+static int init_wdg_hw(struct wdtbd96801 *w)
+{
+	u32 hw_margin[2];
+	int count, ret;
+	u32 hw_margin_max = BD96801_WDT_DEFAULT_MARGIN, hw_margin_min = 0;
+
+	count = device_property_count_u32(w->dev->parent, "rohm,hw-timeout-ms");

Why is that timeout need to be configured from a devicce-tree property? set_timeout/get_timeout can't be done for this device?

Quoted 44 lines

drivers/watchdog/bd96801_wdt.c new file +44 -0

+	if (count < 0 && count != -EINVAL)
+		return count;
+
+	if (count > 0) {
+		if (count > ARRAY_SIZE(hw_margin))
+			return -EINVAL;
+
+		ret = device_property_read_u32_array(w->dev->parent,
+						     "rohm,hw-timeout-ms",
+						     &hw_margin[0], count);
+		if (ret < 0)
+			return ret;
+
+		if (count == 1)
+			hw_margin_max = hw_margin[0];
+
+		if (count == 2) {
+			hw_margin_max = hw_margin[1];
+			hw_margin_min = hw_margin[0];
+		}
+	}
+
+	ret = bd96801_set_wdt_mode(w, hw_margin_max, hw_margin_min);
+	if (ret)
+		return ret;
+
+	ret = device_property_match_string(w->dev->parent, "rohm,wdg-action",
+					   "prstb");
+	if (ret >= 0) {
+		ret = regmap_update_bits(w->regmap, BD96801_REG_WD_CONF,
+				 BD96801_WD_ASSERT_MASK,
+				 BD96801_WD_ASSERT_RST);
+		return ret;
+	}
+
+	ret = device_property_match_string(w->dev->parent, "rohm,wdg-action",
+					   "intb-only");
+	if (ret >= 0) {
+		ret = regmap_update_bits(w->regmap, BD96801_REG_WD_CONF,
+				 BD96801_WD_ASSERT_MASK,
+				 BD96801_WD_ASSERT_IRQ);
+		return ret;
+	}
+

drivers/watchdog/bd96801_wdt.c line 306

Quoted 7 lines

drivers/watchdog/bd96801_wdt.c new file +7 -0

+	return 0;
+}
+
+extern void emergency_restart(void);
+static irqreturn_t bd96801_irq_hnd(int irq, void *data)
+{
+	emergency_restart();

In case of a full system hang will this function get executed?

Review

Quoted 2 lines

(quoted diff) +2 -0

+	return IRQ_NONE;
+}

Introduce driver for WDG block on ROHM BD96801 scalable PMIC.

This driver only supports watchdog with I2C feeding and delayed response detection. Whether the watchdog toggles PRSTB pin or just causes an interrupt can be configured via device-tree.

The BD96801 PMIC HW supports also window watchdog (too early feeding detection) and Q&A mode. These are not supported by this driver.

Signed-off-by Matti Vaittinen <[email protected]>

---
Revision history:
RFCv1 => RFCv2:
- remove always running
- add IRQ handling
- call emergency_restart()
- drop MODULE_ALIAS and add MODULE_DEVICE_TABLE

---
  drivers/watchdog/Kconfig       |  13 ++
  drivers/watchdog/Makefile      |   1 +
  drivers/watchdog/bd96801_wdt.c | 389 +++++++++++++++++++++++++++++++++
  3 files changed, 403 insertions(+)

create mode 100644 drivers/watchdog/bd96801_wdt.c

drivers/watchdog/Kconfig +13 -0

index 6bee137cfbe0..d97e735e1faa 100644
@@ -181,6 +181,19 @@ config BD957XMUF_WATCHDOG
  	  watchdog. Alternatively say M to compile the driver as a module,
  	  which will be called bd9576_wdt.
 
+config BD96801_WATCHDOG
+	tristate "ROHM BD96801 PMIC Watchdog"
+	depends on MFD_ROHM_BD96801
+	select WATCHDOG_CORE
+	help
+	  Support for the watchdog in the ROHM BD96801 PMIC. Watchdog can be
+	  configured to only generate IRQ or to trigger system reset via reset
+	  pin.
+
+	  Say Y here to include support for the ROHM BD96801 watchdog.
+	  Alternatively say M to compile the driver as a module,
+	  which will be called bd96801_wdt.
+
  config CROS_EC_WATCHDOG
  	tristate "ChromeOS EC-based watchdog"
  	select WATCHDOG_CORE

drivers/watchdog/Makefile +1 -0

index 3710c218f05e..31bc94436c81 100644
@@ -217,6 +217,7 @@ obj-$(CONFIG_XEN_WDT) += xen_wdt.o
 
  # Architecture Independent
  obj-$(CONFIG_BD957XMUF_WATCHDOG) += bd9576_wdt.o
+obj-$(CONFIG_BD96801_WATCHDOG) += bd96801_wdt.o
  obj-$(CONFIG_CROS_EC_WATCHDOG) += cros_ec_wdt.o
  obj-$(CONFIG_DA9052_WATCHDOG) += da9052_wdt.o
  obj-$(CONFIG_DA9055_WATCHDOG) += da9055_wdt.o

drivers/watchdog/bd96801_wdt.c new file +0 -0

new file mode 100644
index 000000000000..08fab9a87aec
@@ -0,0 +1,389 @@

drivers/watchdog/bd96801_wdt.c new file +52 -0

+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, 4, 8, 16};
+
+	for (sel = 0; sel < ARRAY_SIZE(multipliers) &&
+	     multipliers[sel] * fast_val < *target; sel++)
+		;
+
+	if (sel == ARRAY_SIZE(multipliers))
+		return -EINVAL;
+
+	*slowsel = sel;
+	*target = multipliers[sel] * fast_val;
+
+	return 0;
+}
+
+static int find_closest_slow(int *target, int *slow_sel, int *fast_sel)
+{
+	static const int multipliers[] = {2, 4, 8, 16};
+	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;
+			}