[PATCH v4 0/5] Unify simple reset drivers

[PATCH v4 0/5] Unify simple reset drivers

[Philipp Zabel]

Hi,

This series introduces common reset ops for simple reset controllers.
"Simple" in this context means that they allow to directly control reset
lines by setting and clearing bits in a single register or in a contiguous
register range that is exclusive to the reset controller.

Parts of the sunxi driver and the socfpga, stm32, and zx2967 drivers are
merged into a single reset-simple driver.
The sunxi driver is kept around to register the early reset controllers,
but it reuses the exported reset_simple_ops.

Changes since v3 [1]:
 - Split the first patch into reset-simple addition (now patch 1)
   and reset-sunxi conversion (now patch 2). I'd really like to get
   an ok from the sunxi maintainers for the second one.
 - Rename reset_simple_set to reset_simple_update, as that function
   is used to both set and clear the control bit.
 - Changed reset-simple to only ever warn about the missing
   "altr,modrst-offset" property on socfpga, in case another compatible
   will use a non-zero default reg_offset in the future.
 - Dropped patch 5 ("reset: simple: read back to make sure changes are
   applied"). Its use is purely theoretical, and this series is about
   unifying the existing drivers.


[1] https://patchwork.kernel.org/patch/9903375/
    https://patchwork.kernel.org/patch/9903373/
    https://patchwork.kernel.org/patch/9903371/
    https://patchwork.kernel.org/patch/9903377/
    https://patchwork.kernel.org/patch/9903379/

regards
Philipp

Philipp Zabel (5):
  reset: add reset-simple to unify socfpga, stm32, sunxi, and zx2967
  reset: sunxi: use reset-simple driver
  reset: socfpga: use the reset-simple driver
  reset: stm32: use the reset-simple driver
  reset: zx2967: use the reset-simple driver

 MAINTAINERS                   |   1 -
 drivers/reset/Kconfig         |  24 ++----
 drivers/reset/Makefile        |   4 +-
 drivers/reset/reset-simple.c  | 186 ++++++++++++++++++++++++++++++++++++++++++
 drivers/reset/reset-simple.h  |  45 ++++++++++
 drivers/reset/reset-socfpga.c | 157 -----------------------------------
 drivers/reset/reset-stm32.c   | 108 ------------------------
 drivers/reset/reset-sunxi.c   | 104 ++---------------------
 drivers/reset/reset-zx2967.c  |  99 ----------------------
 9 files changed, 247 insertions(+), 481 deletions(-)
 create mode 100644 drivers/reset/reset-simple.c
 create mode 100644 drivers/reset/reset-simple.h
 delete mode 100644 drivers/reset/reset-socfpga.c
 delete mode 100644 drivers/reset/reset-stm32.c
 delete mode 100644 drivers/reset/reset-zx2967.c

-- 
2.11.0

[PATCH v4 1/5] reset: add reset-simple to unify socfpga, stm32, sunxi, and zx2967

[Philipp Zabel]

Copy reusable parts from the sunxi driver, to add a driver for simple
reset controllers with reset lines that can be controlled by toggling
bits in exclusive, contiguous register ranges using read-modify-write
cycles under a spinlock.

The following patches will replace compatible reset drivers with
reset-simple, extending it where necessary.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Alexandru Gagniuc <alex.g@adaptrum.com>
---
Changes since v3:
 - Split patch 1 into reset-simple and reset-sunxi parts.
 - Renamed reset_simple_set to reset_simple_update
---
 drivers/reset/Kconfig        |  10 ++++
 drivers/reset/Makefile       |   1 +
 drivers/reset/reset-simple.c | 134 +++++++++++++++++++++++++++++++++++++++++++
 drivers/reset/reset-simple.h |  41 +++++++++++++
 4 files changed, 186 insertions(+)
 create mode 100644 drivers/reset/reset-simple.c
 create mode 100644 drivers/reset/reset-simple.h

diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index 1c88af2947245..392e65187cd5a 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -75,6 +75,16 @@ config RESET_PISTACHIO
 	help
 	  This enables the reset driver for ImgTec Pistachio SoCs.
 
+config RESET_SIMPLE
+	bool "Simple Reset Controller Driver" if COMPILE_TEST
+	default ARCH_SUNXI
+	help
+	  This enables a simple reset controller driver for reset lines that
+	  that can be asserted and deasserted by toggling bits in a contiguous,
+	  exclusive register space.
+
+	  Currently this driver supports Allwinner SoCs.
+
 config RESET_SOCFPGA
 	bool "SoCFPGA Reset Driver" if COMPILE_TEST
 	default ARCH_SOCFPGA || ARCH_STRATIX10
diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index af1c15c330b30..ddd3eeb193567 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_RESET_LPC18XX) += reset-lpc18xx.o
 obj-$(CONFIG_RESET_MESON) += reset-meson.o
 obj-$(CONFIG_RESET_OXNAS) += reset-oxnas.o
 obj-$(CONFIG_RESET_PISTACHIO) += reset-pistachio.o
+obj-$(CONFIG_RESET_SIMPLE) += reset-simple.o
 obj-$(CONFIG_RESET_SOCFPGA) += reset-socfpga.o
 obj-$(CONFIG_RESET_STM32) += reset-stm32.o
 obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o
diff --git a/drivers/reset/reset-simple.c b/drivers/reset/reset-simple.c
new file mode 100644
index 0000000000000..a5119457cec61
--- /dev/null
+++ b/drivers/reset/reset-simple.c
@@ -0,0 +1,134 @@
+/*
+ * Simple Reset Controller Driver
+ *
+ * Copyright (C) 2017 Pengutronix, Philipp Zabel <kernel@pengutronix.de>
+ *
+ * Based on Allwinner SoCs Reset Controller driver
+ *
+ * Copyright 2013 Maxime Ripard
+ *
+ * Maxime Ripard <maxime.ripard@free-electrons.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/reset-controller.h>
+#include <linux/spinlock.h>
+
+#include "reset-simple.h"
+
+static inline struct reset_simple_data *
+to_reset_simple_data(struct reset_controller_dev *rcdev)
+{
+	return container_of(rcdev, struct reset_simple_data, rcdev);
+}
+
+static int reset_simple_update(struct reset_controller_dev *rcdev,
+			       unsigned long id, bool assert)
+{
+	struct reset_simple_data *data = to_reset_simple_data(rcdev);
+	int reg_width = sizeof(u32);
+	int bank = id / (reg_width * BITS_PER_BYTE);
+	int offset = id % (reg_width * BITS_PER_BYTE);
+	unsigned long flags;
+	u32 reg;
+
+	spin_lock_irqsave(&data->lock, flags);
+
+	reg = readl(data->membase + (bank * reg_width));
+	if (assert ^ data->active_low)
+		reg |= BIT(offset);
+	else
+		reg &= ~BIT(offset);
+	writel(reg, data->membase + (bank * reg_width));
+
+	spin_unlock_irqrestore(&data->lock, flags);
+
+	return 0;
+}
+
+static int reset_simple_assert(struct reset_controller_dev *rcdev,
+			       unsigned long id)
+{
+	return reset_simple_update(rcdev, id, true);
+}
+
+static int reset_simple_deassert(struct reset_controller_dev *rcdev,
+				 unsigned long id)
+{
+	return reset_simple_update(rcdev, id, false);
+}
+
+const struct reset_control_ops reset_simple_ops = {
+	.assert		= reset_simple_assert,
+	.deassert	= reset_simple_deassert,
+};
+
+/**
+ * struct reset_simple_devdata - simple reset controller properties
+ * @active_low: if true, bits are cleared to assert the reset. Otherwise, bits
+ *              are set to assert the reset.
+ */
+struct reset_simple_devdata {
+	bool active_low;
+};
+
+static const struct reset_simple_devdata reset_simple_active_low = {
+	.active_low = true,
+};
+
+static const struct of_device_id reset_simple_dt_ids[] = {
+	{ .compatible = "allwinner,sun6i-a31-clock-reset",
+		.data = &reset_simple_active_low },
+	{ /* sentinel */ },
+};
+
+static int reset_simple_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	const struct reset_simple_devdata *devdata;
+	struct reset_simple_data *data;
+	void __iomem *membase;
+	struct resource *res;
+
+	devdata = of_device_get_match_data(dev);
+
+	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	membase = devm_ioremap_resource(dev, res);
+	if (IS_ERR(membase))
+		return PTR_ERR(membase);
+
+	spin_lock_init(&data->lock);
+	data->membase = membase;
+	data->rcdev.owner = THIS_MODULE;
+	data->rcdev.nr_resets = resource_size(res) * BITS_PER_BYTE;
+	data->rcdev.ops = &reset_simple_ops;
+	data->rcdev.of_node = dev->of_node;
+
+	if (devdata)
+		data->active_low = devdata->active_low;
+
+	return devm_reset_controller_register(dev, &data->rcdev);
+}
+
+static struct platform_driver reset_simple_driver = {
+	.probe	= reset_simple_probe,
+	.driver = {
+		.name		= "simple-reset",
+		.of_match_table	= reset_simple_dt_ids,
+	},
+};
+builtin_platform_driver(reset_simple_driver);
diff --git a/drivers/reset/reset-simple.h b/drivers/reset/reset-simple.h
new file mode 100644
index 0000000000000..39af2014b5f12
--- /dev/null
+++ b/drivers/reset/reset-simple.h
@@ -0,0 +1,41 @@
+/*
+ * Simple Reset Controller ops
+ *
+ * Based on Allwinner SoCs Reset Controller driver
+ *
+ * Copyright 2013 Maxime Ripard
+ *
+ * Maxime Ripard <maxime.ripard@free-electrons.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#ifndef __RESET_SIMPLE_H__
+#define __RESET_SIMPLE_H__
+
+#include <linux/io.h>
+#include <linux/reset-controller.h>
+#include <linux/spinlock.h>
+
+/**
+ * struct reset_simple_data - driver data for simple reset controllers
+ * @lock: spinlock to protect registers during read-modify-write cycles
+ * @membase: memory mapped I/O register range
+ * @rcdev: reset controller device base structure
+ * @active_low: if true, bits are cleared to assert the reset. Otherwise, bits
+ *              are set to assert the reset. Note that this says nothing about
+ *              the voltage level of the actual reset line.
+ */
+struct reset_simple_data {
+	spinlock_t			lock;
+	void __iomem			*membase;
+	struct reset_controller_dev	rcdev;
+	bool				active_low;
+};
+
+extern const struct reset_control_ops reset_simple_ops;
+
+#endif /* __RESET_SIMPLE_H__ */
-- 
2.11.0

[PATCH v4 2/5] reset: sunxi: use reset-simple driver

[Philipp Zabel]

Use the newly created copies in the reset-simple driver to replace the
sunxi platform driver code and reset operations. The separate sunxi
driver still remains to register the early reset controllers, but it
reuses the reset operations in reset-simple.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Alexandru Gagniuc <alex.g@adaptrum.com>
---
Changes since v3:
 - Split patch 1 into reset-simple and reset-sunxi parts.
---
 drivers/reset/Kconfig       |   1 +
 drivers/reset/reset-sunxi.c | 104 +++-----------------------------------------
 2 files changed, 7 insertions(+), 98 deletions(-)

diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index 392e65187cd5a..a14c0fe498771 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -100,6 +100,7 @@ config RESET_STM32
 config RESET_SUNXI
 	bool "Allwinner SoCs Reset Driver" if COMPILE_TEST && !ARCH_SUNXI
 	default ARCH_SUNXI
+	select RESET_SIMPLE
 	help
 	  This enables the reset driver for Allwinner SoCs.
 
diff --git a/drivers/reset/reset-sunxi.c b/drivers/reset/reset-sunxi.c
index 2c7dd1fd08df6..db9a1a75523f4 100644
--- a/drivers/reset/reset-sunxi.c
+++ b/drivers/reset/reset-sunxi.c
@@ -22,64 +22,11 @@
 #include <linux/spinlock.h>
 #include <linux/types.h>
 
-struct sunxi_reset_data {
-	spinlock_t			lock;
-	void __iomem			*membase;
-	struct reset_controller_dev	rcdev;
-};
-
-static int sunxi_reset_assert(struct reset_controller_dev *rcdev,
-			      unsigned long id)
-{
-	struct sunxi_reset_data *data = container_of(rcdev,
-						     struct sunxi_reset_data,
-						     rcdev);
-	int reg_width = sizeof(u32);
-	int bank = id / (reg_width * BITS_PER_BYTE);
-	int offset = id % (reg_width * BITS_PER_BYTE);
-	unsigned long flags;
-	u32 reg;
-
-	spin_lock_irqsave(&data->lock, flags);
-
-	reg = readl(data->membase + (bank * reg_width));
-	writel(reg & ~BIT(offset), data->membase + (bank * reg_width));
-
-	spin_unlock_irqrestore(&data->lock, flags);
-
-	return 0;
-}
-
-static int sunxi_reset_deassert(struct reset_controller_dev *rcdev,
-				unsigned long id)
-{
-	struct sunxi_reset_data *data = container_of(rcdev,
-						     struct sunxi_reset_data,
-						     rcdev);
-	int reg_width = sizeof(u32);
-	int bank = id / (reg_width * BITS_PER_BYTE);
-	int offset = id % (reg_width * BITS_PER_BYTE);
-	unsigned long flags;
-	u32 reg;
-
-	spin_lock_irqsave(&data->lock, flags);
-
-	reg = readl(data->membase + (bank * reg_width));
-	writel(reg | BIT(offset), data->membase + (bank * reg_width));
-
-	spin_unlock_irqrestore(&data->lock, flags);
-
-	return 0;
-}
-
-static const struct reset_control_ops sunxi_reset_ops = {
-	.assert		= sunxi_reset_assert,
-	.deassert	= sunxi_reset_deassert,
-};
+#include "reset-simple.h"
 
 static int sunxi_reset_init(struct device_node *np)
 {
-	struct sunxi_reset_data *data;
+	struct reset_simple_data *data;
 	struct resource res;
 	resource_size_t size;
 	int ret;
@@ -108,8 +55,9 @@ static int sunxi_reset_init(struct device_node *np)
 
 	data->rcdev.owner = THIS_MODULE;
 	data->rcdev.nr_resets = size * 8;
-	data->rcdev.ops = &sunxi_reset_ops;
+	data->rcdev.ops = &reset_simple_ops;
 	data->rcdev.of_node = np;
+	data->active_low = true;
 
 	return reset_controller_register(&data->rcdev);
 
@@ -122,6 +70,8 @@ static int sunxi_reset_init(struct device_node *np)
  * These are the reset controller we need to initialize early on in
  * our system, before we can even think of using a regular device
  * driver for it.
+ * The controllers that we can register through the regular device
+ * model are handled by the simple reset driver directly.
  */
 static const struct of_device_id sunxi_early_reset_dt_ids[] __initconst = {
 	{ .compatible = "allwinner,sun6i-a31-ahb1-reset", },
@@ -135,45 +85,3 @@ void __init sun6i_reset_init(void)
 	for_each_matching_node(np, sunxi_early_reset_dt_ids)
 		sunxi_reset_init(np);
 }
-
-/*
- * And these are the controllers we can register through the regular
- * device model.
- */
-static const struct of_device_id sunxi_reset_dt_ids[] = {
-	 { .compatible = "allwinner,sun6i-a31-clock-reset", },
-	 { /* sentinel */ },
-};
-
-static int sunxi_reset_probe(struct platform_device *pdev)
-{
-	struct sunxi_reset_data *data;
-	struct resource *res;
-
-	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
-	if (!data)
-		return -ENOMEM;
-
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	data->membase = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(data->membase))
-		return PTR_ERR(data->membase);
-
-	spin_lock_init(&data->lock);
-
-	data->rcdev.owner = THIS_MODULE;
-	data->rcdev.nr_resets = resource_size(res) * 8;
-	data->rcdev.ops = &sunxi_reset_ops;
-	data->rcdev.of_node = pdev->dev.of_node;
-
-	return devm_reset_controller_register(&pdev->dev, &data->rcdev);
-}
-
-static struct platform_driver sunxi_reset_driver = {
-	.probe	= sunxi_reset_probe,
-	.driver = {
-		.name		= "sunxi-reset",
-		.of_match_table	= sunxi_reset_dt_ids,
-	},
-};
-builtin_platform_driver(sunxi_reset_driver);
-- 
2.11.0

[PATCH v4 3/5] reset: socfpga: use the reset-simple driver

[Philipp Zabel]

Add reset line status readback, inverted status support, and socfpga
device tree quirks to the simple reset driver, and use it to replace
the socfpga driver.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
Changes since v3:
 - Rebased onto reset/next
 - Only warn about missing altr,modrst-offset property on socfpga
---
 drivers/reset/Kconfig         |  10 +--
 drivers/reset/Makefile        |   1 -
 drivers/reset/reset-simple.c  |  51 +++++++++++++-
 drivers/reset/reset-simple.h  |   4 ++
 drivers/reset/reset-socfpga.c | 157 ------------------------------------------
 5 files changed, 56 insertions(+), 167 deletions(-)
 delete mode 100644 drivers/reset/reset-socfpga.c

diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index a14c0fe498771..2d6770b210ed7 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -77,19 +77,13 @@ config RESET_PISTACHIO
 
 config RESET_SIMPLE
 	bool "Simple Reset Controller Driver" if COMPILE_TEST
-	default ARCH_SUNXI
+	default ARCH_SOCFPGA || ARCH_STRATIX10 || ARCH_SUNXI
 	help
 	  This enables a simple reset controller driver for reset lines that
 	  that can be asserted and deasserted by toggling bits in a contiguous,
 	  exclusive register space.
 
-	  Currently this driver supports Allwinner SoCs.
-
-config RESET_SOCFPGA
-	bool "SoCFPGA Reset Driver" if COMPILE_TEST
-	default ARCH_SOCFPGA || ARCH_STRATIX10
-	help
-	  This enables the reset controller driver for Altera SoCFPGAs.
+	  Currently this driver supports Altera SoCFPGAs and Allwinner SoCs.
 
 config RESET_STM32
 	bool "STM32 Reset Driver" if COMPILE_TEST
diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index ddd3eeb193567..3bf3b4e6a9fd2 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -13,7 +13,6 @@ obj-$(CONFIG_RESET_MESON) += reset-meson.o
 obj-$(CONFIG_RESET_OXNAS) += reset-oxnas.o
 obj-$(CONFIG_RESET_PISTACHIO) += reset-pistachio.o
 obj-$(CONFIG_RESET_SIMPLE) += reset-simple.o
-obj-$(CONFIG_RESET_SOCFPGA) += reset-socfpga.o
 obj-$(CONFIG_RESET_STM32) += reset-stm32.o
 obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o
 obj-$(CONFIG_RESET_TI_SCI) += reset-ti-sci.o
diff --git a/drivers/reset/reset-simple.c b/drivers/reset/reset-simple.c
index a5119457cec61..98ff0f924948e 100644
--- a/drivers/reset/reset-simple.c
+++ b/drivers/reset/reset-simple.c
@@ -68,25 +68,58 @@ static int reset_simple_deassert(struct reset_controller_dev *rcdev,
 	return reset_simple_update(rcdev, id, false);
 }
 
+static int reset_simple_status(struct reset_controller_dev *rcdev,
+			       unsigned long id)
+{
+	struct reset_simple_data *data = to_reset_simple_data(rcdev);
+	int reg_width = sizeof(u32);
+	int bank = id / (reg_width * BITS_PER_BYTE);
+	int offset = id % (reg_width * BITS_PER_BYTE);
+	u32 reg;
+
+	reg = readl(data->membase + (bank * reg_width));
+
+	return !(reg & BIT(offset)) ^ !data->status_active_low;
+}
+
 const struct reset_control_ops reset_simple_ops = {
 	.assert		= reset_simple_assert,
 	.deassert	= reset_simple_deassert,
+	.status		= reset_simple_status,
 };
 
 /**
  * struct reset_simple_devdata - simple reset controller properties
+ * @reg_offset: offset between base address and first reset register.
+ * @nr_resets: number of resets. If not set, default to resource size in bits.
  * @active_low: if true, bits are cleared to assert the reset. Otherwise, bits
  *              are set to assert the reset.
+ * @status_active_low: if true, bits read back as cleared while the reset is
+ *                     asserted. Otherwise, bits read back as set while the
+ *                     reset is asserted.
  */
 struct reset_simple_devdata {
+	u32 reg_offset;
+	u32 nr_resets;
 	bool active_low;
+	bool status_active_low;
+};
+
+#define SOCFPGA_NR_BANKS	8
+
+static const struct reset_simple_devdata reset_simple_socfpga = {
+	.reg_offset = 0x10,
+	.nr_resets = SOCFPGA_NR_BANKS * 32,
+	.status_active_low = true,
 };
 
 static const struct reset_simple_devdata reset_simple_active_low = {
 	.active_low = true,
+	.status_active_low = true,
 };
 
 static const struct of_device_id reset_simple_dt_ids[] = {
+	{ .compatible = "altr,rst-mgr", .data = &reset_simple_socfpga },
 	{ .compatible = "allwinner,sun6i-a31-clock-reset",
 		.data = &reset_simple_active_low },
 	{ /* sentinel */ },
@@ -99,6 +132,7 @@ static int reset_simple_probe(struct platform_device *pdev)
 	struct reset_simple_data *data;
 	void __iomem *membase;
 	struct resource *res;
+	u32 reg_offset = 0;
 
 	devdata = of_device_get_match_data(dev);
 
@@ -118,8 +152,23 @@ static int reset_simple_probe(struct platform_device *pdev)
 	data->rcdev.ops = &reset_simple_ops;
 	data->rcdev.of_node = dev->of_node;
 
-	if (devdata)
+	if (devdata) {
+		reg_offset = devdata->reg_offset;
+		if (devdata->nr_resets)
+			data->rcdev.nr_resets = devdata->nr_resets;
 		data->active_low = devdata->active_low;
+		data->status_active_low = devdata->status_active_low;
+	}
+
+	if (devdata == &reset_simple_socfpga &&
+	    of_property_read_u32(dev->of_node, "altr,modrst-offset",
+				 &reg_offset)) {
+		dev_warn(dev,
+			 "missing altr,modrst-offset property, assuming 0x%x!\n",
+			 reg_offset);
+	}
+
+	data->membase += reg_offset;
 
 	return devm_reset_controller_register(dev, &data->rcdev);
 }
diff --git a/drivers/reset/reset-simple.h b/drivers/reset/reset-simple.h
index 39af2014b5f12..8a496022baeff 100644
--- a/drivers/reset/reset-simple.h
+++ b/drivers/reset/reset-simple.h
@@ -28,12 +28,16 @@
  * @active_low: if true, bits are cleared to assert the reset. Otherwise, bits
  *              are set to assert the reset. Note that this says nothing about
  *              the voltage level of the actual reset line.
+ * @status_active_low: if true, bits read back as cleared while the reset is
+ *                     asserted. Otherwise, bits read back as set while the
+ *                     reset is asserted.
  */
 struct reset_simple_data {
 	spinlock_t			lock;
 	void __iomem			*membase;
 	struct reset_controller_dev	rcdev;
 	bool				active_low;
+	bool				status_active_low;
 };
 
 extern const struct reset_control_ops reset_simple_ops;
diff --git a/drivers/reset/reset-socfpga.c b/drivers/reset/reset-socfpga.c
deleted file mode 100644
index 3907bbc9c6cf7..0000000000000
--- a/drivers/reset/reset-socfpga.c
+++ /dev/null
@@ -1,157 +0,0 @@
-/*
- * Socfpga Reset Controller Driver
- *
- * Copyright 2014 Steffen Trumtrar <s.trumtrar@pengutronix.de>
- *
- * based on
- * Allwinner SoCs Reset Controller driver
- *
- * Copyright 2013 Maxime Ripard
- *
- * Maxime Ripard <maxime.ripard@free-electrons.com>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- */
-
-#include <linux/err.h>
-#include <linux/io.h>
-#include <linux/init.h>
-#include <linux/of.h>
-#include <linux/platform_device.h>
-#include <linux/reset-controller.h>
-#include <linux/spinlock.h>
-#include <linux/types.h>
-
-#define BANK_INCREMENT		4
-#define NR_BANKS		8
-
-struct socfpga_reset_data {
-	spinlock_t			lock;
-	void __iomem			*membase;
-	struct reset_controller_dev	rcdev;
-};
-
-static int socfpga_reset_assert(struct reset_controller_dev *rcdev,
-				unsigned long id)
-{
-	struct socfpga_reset_data *data = container_of(rcdev,
-						     struct socfpga_reset_data,
-						     rcdev);
-	int reg_width = sizeof(u32);
-	int bank = id / (reg_width * BITS_PER_BYTE);
-	int offset = id % (reg_width * BITS_PER_BYTE);
-	unsigned long flags;
-	u32 reg;
-
-	spin_lock_irqsave(&data->lock, flags);
-
-	reg = readl(data->membase + (bank * BANK_INCREMENT));
-	writel(reg | BIT(offset), data->membase + (bank * BANK_INCREMENT));
-	spin_unlock_irqrestore(&data->lock, flags);
-
-	return 0;
-}
-
-static int socfpga_reset_deassert(struct reset_controller_dev *rcdev,
-				  unsigned long id)
-{
-	struct socfpga_reset_data *data = container_of(rcdev,
-						     struct socfpga_reset_data,
-						     rcdev);
-
-	int reg_width = sizeof(u32);
-	int bank = id / (reg_width * BITS_PER_BYTE);
-	int offset = id % (reg_width * BITS_PER_BYTE);
-	unsigned long flags;
-	u32 reg;
-
-	spin_lock_irqsave(&data->lock, flags);
-
-	reg = readl(data->membase + (bank * BANK_INCREMENT));
-	writel(reg & ~BIT(offset), data->membase + (bank * BANK_INCREMENT));
-
-	spin_unlock_irqrestore(&data->lock, flags);
-
-	return 0;
-}
-
-static int socfpga_reset_status(struct reset_controller_dev *rcdev,
-				unsigned long id)
-{
-	struct socfpga_reset_data *data = container_of(rcdev,
-						struct socfpga_reset_data, rcdev);
-	int reg_width = sizeof(u32);
-	int bank = id / (reg_width * BITS_PER_BYTE);
-	int offset = id % (reg_width * BITS_PER_BYTE);
-	u32 reg;
-
-	reg = readl(data->membase + (bank * BANK_INCREMENT));
-
-	return !(reg & BIT(offset));
-}
-
-static const struct reset_control_ops socfpga_reset_ops = {
-	.assert		= socfpga_reset_assert,
-	.deassert	= socfpga_reset_deassert,
-	.status		= socfpga_reset_status,
-};
-
-static int socfpga_reset_probe(struct platform_device *pdev)
-{
-	struct socfpga_reset_data *data;
-	struct resource *res;
-	struct device *dev = &pdev->dev;
-	struct device_node *np = dev->of_node;
-	u32 modrst_offset;
-
-	/*
-	 * The binding was mainlined without the required property.
-	 * Do not continue, when we encounter an old DT.
-	 */
-	if (!of_find_property(pdev->dev.of_node, "#reset-cells", NULL)) {
-		dev_err(&pdev->dev, "%pOF missing #reset-cells property\n",
-			pdev->dev.of_node);
-		return -EINVAL;
-	}
-
-	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
-	if (!data)
-		return -ENOMEM;
-
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	data->membase = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(data->membase))
-		return PTR_ERR(data->membase);
-
-	if (of_property_read_u32(np, "altr,modrst-offset", &modrst_offset)) {
-		dev_warn(dev, "missing altr,modrst-offset property, assuming 0x10!\n");
-		modrst_offset = 0x10;
-	}
-	data->membase += modrst_offset;
-
-	spin_lock_init(&data->lock);
-
-	data->rcdev.owner = THIS_MODULE;
-	data->rcdev.nr_resets = NR_BANKS * (sizeof(u32) * BITS_PER_BYTE);
-	data->rcdev.ops = &socfpga_reset_ops;
-	data->rcdev.of_node = pdev->dev.of_node;
-
-	return devm_reset_controller_register(dev, &data->rcdev);
-}
-
-static const struct of_device_id socfpga_reset_dt_ids[] = {
-	{ .compatible = "altr,rst-mgr", },
-	{ /* sentinel */ },
-};
-
-static struct platform_driver socfpga_reset_driver = {
-	.probe	= socfpga_reset_probe,
-	.driver = {
-		.name		= "socfpga-reset",
-		.of_match_table	= socfpga_reset_dt_ids,
-	},
-};
-builtin_platform_driver(socfpga_reset_driver);
-- 
2.11.0

[PATCH v4 4/5] reset: stm32: use the reset-simple driver

[Philipp Zabel]

The reset-simple driver can be used without changes.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
Acked-by: Gabriel Fernandez <gabriel.fernandez@st.com>
---
 drivers/reset/Kconfig        |  11 ++---
 drivers/reset/Makefile       |   1 -
 drivers/reset/reset-simple.c |   1 +
 drivers/reset/reset-stm32.c  | 108 -------------------------------------------
 4 files changed, 4 insertions(+), 117 deletions(-)
 delete mode 100644 drivers/reset/reset-stm32.c

diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index 2d6770b210ed7..9bdb4fe1b03e6 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -77,19 +77,14 @@ config RESET_PISTACHIO
 
 config RESET_SIMPLE
 	bool "Simple Reset Controller Driver" if COMPILE_TEST
-	default ARCH_SOCFPGA || ARCH_STRATIX10 || ARCH_SUNXI
+	default ARCH_SOCFPGA || ARCH_STM32 || ARCH_STRATIX10 || ARCH_SUNXI
 	help
 	  This enables a simple reset controller driver for reset lines that
 	  that can be asserted and deasserted by toggling bits in a contiguous,
 	  exclusive register space.
 
-	  Currently this driver supports Altera SoCFPGAs and Allwinner SoCs.
-
-config RESET_STM32
-	bool "STM32 Reset Driver" if COMPILE_TEST
-	default ARCH_STM32
-	help
-	  This enables the RCC reset controller driver for STM32 MCUs.
+	  Currently this driver supports Altera SoCFPGAs, the RCC reset
+	  controller in STM32 MCUs, and Allwinner SoCs.
 
 config RESET_SUNXI
 	bool "Allwinner SoCs Reset Driver" if COMPILE_TEST && !ARCH_SUNXI
diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index 3bf3b4e6a9fd2..98dd1eccc820c 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -13,7 +13,6 @@ obj-$(CONFIG_RESET_MESON) += reset-meson.o
 obj-$(CONFIG_RESET_OXNAS) += reset-oxnas.o
 obj-$(CONFIG_RESET_PISTACHIO) += reset-pistachio.o
 obj-$(CONFIG_RESET_SIMPLE) += reset-simple.o
-obj-$(CONFIG_RESET_STM32) += reset-stm32.o
 obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o
 obj-$(CONFIG_RESET_TI_SCI) += reset-ti-sci.o
 obj-$(CONFIG_RESET_TI_SYSCON) += reset-ti-syscon.o
diff --git a/drivers/reset/reset-simple.c b/drivers/reset/reset-simple.c
index 98ff0f924948e..273039ef02e64 100644
--- a/drivers/reset/reset-simple.c
+++ b/drivers/reset/reset-simple.c
@@ -120,6 +120,7 @@ static const struct reset_simple_devdata reset_simple_active_low = {
 
 static const struct of_device_id reset_simple_dt_ids[] = {
 	{ .compatible = "altr,rst-mgr", .data = &reset_simple_socfpga },
+	{ .compatible = "st,stm32-rcc", },
 	{ .compatible = "allwinner,sun6i-a31-clock-reset",
 		.data = &reset_simple_active_low },
 	{ /* sentinel */ },
diff --git a/drivers/reset/reset-stm32.c b/drivers/reset/reset-stm32.c
deleted file mode 100644
index 3a7c8527e66af..0000000000000
--- a/drivers/reset/reset-stm32.c
+++ /dev/null
@@ -1,108 +0,0 @@
-/*
- * Copyright (C) Maxime Coquelin 2015
- * Author:  Maxime Coquelin <mcoquelin.stm32@gmail.com>
- * License terms:  GNU General Public License (GPL), version 2
- *
- * Heavily based on sunxi driver from Maxime Ripard.
- */
-
-#include <linux/err.h>
-#include <linux/io.h>
-#include <linux/of.h>
-#include <linux/of_address.h>
-#include <linux/platform_device.h>
-#include <linux/reset-controller.h>
-#include <linux/slab.h>
-#include <linux/spinlock.h>
-#include <linux/types.h>
-
-struct stm32_reset_data {
-	spinlock_t			lock;
-	void __iomem			*membase;
-	struct reset_controller_dev	rcdev;
-};
-
-static int stm32_reset_assert(struct reset_controller_dev *rcdev,
-			      unsigned long id)
-{
-	struct stm32_reset_data *data = container_of(rcdev,
-						     struct stm32_reset_data,
-						     rcdev);
-	int bank = id / BITS_PER_LONG;
-	int offset = id % BITS_PER_LONG;
-	unsigned long flags;
-	u32 reg;
-
-	spin_lock_irqsave(&data->lock, flags);
-
-	reg = readl(data->membase + (bank * 4));
-	writel(reg | BIT(offset), data->membase + (bank * 4));
-
-	spin_unlock_irqrestore(&data->lock, flags);
-
-	return 0;
-}
-
-static int stm32_reset_deassert(struct reset_controller_dev *rcdev,
-				unsigned long id)
-{
-	struct stm32_reset_data *data = container_of(rcdev,
-						     struct stm32_reset_data,
-						     rcdev);
-	int bank = id / BITS_PER_LONG;
-	int offset = id % BITS_PER_LONG;
-	unsigned long flags;
-	u32 reg;
-
-	spin_lock_irqsave(&data->lock, flags);
-
-	reg = readl(data->membase + (bank * 4));
-	writel(reg & ~BIT(offset), data->membase + (bank * 4));
-
-	spin_unlock_irqrestore(&data->lock, flags);
-
-	return 0;
-}
-
-static const struct reset_control_ops stm32_reset_ops = {
-	.assert		= stm32_reset_assert,
-	.deassert	= stm32_reset_deassert,
-};
-
-static const struct of_device_id stm32_reset_dt_ids[] = {
-	 { .compatible = "st,stm32-rcc", },
-	 { /* sentinel */ },
-};
-
-static int stm32_reset_probe(struct platform_device *pdev)
-{
-	struct stm32_reset_data *data;
-	struct resource *res;
-
-	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
-	if (!data)
-		return -ENOMEM;
-
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	data->membase = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(data->membase))
-		return PTR_ERR(data->membase);
-
-	spin_lock_init(&data->lock);
-
-	data->rcdev.owner = THIS_MODULE;
-	data->rcdev.nr_resets = resource_size(res) * 8;
-	data->rcdev.ops = &stm32_reset_ops;
-	data->rcdev.of_node = pdev->dev.of_node;
-
-	return devm_reset_controller_register(&pdev->dev, &data->rcdev);
-}
-
-static struct platform_driver stm32_reset_driver = {
-	.probe	= stm32_reset_probe,
-	.driver = {
-		.name		= "stm32-rcc-reset",
-		.of_match_table	= stm32_reset_dt_ids,
-	},
-};
-builtin_platform_driver(stm32_reset_driver);
-- 
2.11.0

[PATCH v4 5/5] reset: zx2967: use the reset-simple driver

[Philipp Zabel]

The reset-simple driver can be used without changes.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Alexandru Gagniuc <alex.g@adaptrum.com>
---
 MAINTAINERS                  |  1 -
 drivers/reset/Kconfig        | 10 +----
 drivers/reset/Makefile       |  1 -
 drivers/reset/reset-simple.c |  2 +
 drivers/reset/reset-zx2967.c | 99 --------------------------------------------
 5 files changed, 4 insertions(+), 109 deletions(-)
 delete mode 100644 drivers/reset/reset-zx2967.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 3b9887b3644ef..d48d8a238fae1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2124,7 +2124,6 @@ F:	drivers/gpio/gpio-zx.c
 F:	drivers/i2c/busses/i2c-zx2967.c
 F:	drivers/mmc/host/dw_mmc-zx.*
 F:	drivers/pinctrl/zte/
-F:	drivers/reset/reset-zx2967.c
 F:	drivers/soc/zte/
 F:	drivers/thermal/zx2967_thermal.c
 F:	drivers/watchdog/zx2967_wdt.c
diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index 9bdb4fe1b03e6..de0090c911cd6 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -77,14 +77,14 @@ config RESET_PISTACHIO
 
 config RESET_SIMPLE
 	bool "Simple Reset Controller Driver" if COMPILE_TEST
-	default ARCH_SOCFPGA || ARCH_STM32 || ARCH_STRATIX10 || ARCH_SUNXI
+	default ARCH_SOCFPGA || ARCH_STM32 || ARCH_STRATIX10 || ARCH_SUNXI || ARCH_ZX
 	help
 	  This enables a simple reset controller driver for reset lines that
 	  that can be asserted and deasserted by toggling bits in a contiguous,
 	  exclusive register space.
 
 	  Currently this driver supports Altera SoCFPGAs, the RCC reset
-	  controller in STM32 MCUs, and Allwinner SoCs.
+	  controller in STM32 MCUs, Allwinner SoCs, and ZTE's zx2967 family.
 
 config RESET_SUNXI
 	bool "Allwinner SoCs Reset Driver" if COMPILE_TEST && !ARCH_SUNXI
@@ -121,12 +121,6 @@ config RESET_UNIPHIER
 	  Say Y if you want to control reset signals provided by System Control
 	  block, Media I/O block, Peripheral Block.
 
-config RESET_ZX2967
-	bool "ZTE ZX2967 Reset Driver"
-	depends on ARCH_ZX || COMPILE_TEST
-	help
-	  This enables the reset controller driver for ZTE's zx2967 family.
-
 config RESET_ZYNQ
 	bool "ZYNQ Reset Driver" if COMPILE_TEST
 	default ARCH_ZYNQ
diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index 98dd1eccc820c..48b64d63b75aa 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -17,6 +17,5 @@ obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o
 obj-$(CONFIG_RESET_TI_SCI) += reset-ti-sci.o
 obj-$(CONFIG_RESET_TI_SYSCON) += reset-ti-syscon.o
 obj-$(CONFIG_RESET_UNIPHIER) += reset-uniphier.o
-obj-$(CONFIG_RESET_ZX2967) += reset-zx2967.o
 obj-$(CONFIG_RESET_ZYNQ) += reset-zynq.o
 
diff --git a/drivers/reset/reset-simple.c b/drivers/reset/reset-simple.c
index 273039ef02e64..a63f96d92035e 100644
--- a/drivers/reset/reset-simple.c
+++ b/drivers/reset/reset-simple.c
@@ -123,6 +123,8 @@ static const struct of_device_id reset_simple_dt_ids[] = {
 	{ .compatible = "st,stm32-rcc", },
 	{ .compatible = "allwinner,sun6i-a31-clock-reset",
 		.data = &reset_simple_active_low },
+	{ .compatible = "zte,zx296718-reset",
+		.data = &reset_simple_active_low },
 	{ /* sentinel */ },
 };
 
diff --git a/drivers/reset/reset-zx2967.c b/drivers/reset/reset-zx2967.c
deleted file mode 100644
index 4f319f7753d4d..0000000000000
--- a/drivers/reset/reset-zx2967.c
+++ /dev/null
@@ -1,99 +0,0 @@
-/*
- * ZTE's zx2967 family reset controller driver
- *
- * Copyright (C) 2017 ZTE Ltd.
- *
- * Author: Baoyou Xie <baoyou.xie@linaro.org>
- *
- * License terms: GNU General Public License (GPL) version 2
- */
-
-#include <linux/of_address.h>
-#include <linux/platform_device.h>
-#include <linux/reset-controller.h>
-
-struct zx2967_reset {
-	void __iomem			*reg_base;
-	spinlock_t			lock;
-	struct reset_controller_dev	rcdev;
-};
-
-static int zx2967_reset_act(struct reset_controller_dev *rcdev,
-			    unsigned long id, bool assert)
-{
-	struct zx2967_reset *reset = NULL;
-	int bank = id / 32;
-	int offset = id % 32;
-	u32 reg;
-	unsigned long flags;
-
-	reset = container_of(rcdev, struct zx2967_reset, rcdev);
-
-	spin_lock_irqsave(&reset->lock, flags);
-
-	reg = readl_relaxed(reset->reg_base + (bank * 4));
-	if (assert)
-		reg &= ~BIT(offset);
-	else
-		reg |= BIT(offset);
-	writel_relaxed(reg, reset->reg_base + (bank * 4));
-
-	spin_unlock_irqrestore(&reset->lock, flags);
-
-	return 0;
-}
-
-static int zx2967_reset_assert(struct reset_controller_dev *rcdev,
-			       unsigned long id)
-{
-	return zx2967_reset_act(rcdev, id, true);
-}
-
-static int zx2967_reset_deassert(struct reset_controller_dev *rcdev,
-				 unsigned long id)
-{
-	return zx2967_reset_act(rcdev, id, false);
-}
-
-static const struct reset_control_ops zx2967_reset_ops = {
-	.assert		= zx2967_reset_assert,
-	.deassert	= zx2967_reset_deassert,
-};
-
-static int zx2967_reset_probe(struct platform_device *pdev)
-{
-	struct zx2967_reset *reset;
-	struct resource *res;
-
-	reset = devm_kzalloc(&pdev->dev, sizeof(*reset), GFP_KERNEL);
-	if (!reset)
-		return -ENOMEM;
-
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	reset->reg_base = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(reset->reg_base))
-		return PTR_ERR(reset->reg_base);
-
-	spin_lock_init(&reset->lock);
-
-	reset->rcdev.owner = THIS_MODULE;
-	reset->rcdev.nr_resets = resource_size(res) * 8;
-	reset->rcdev.ops = &zx2967_reset_ops;
-	reset->rcdev.of_node = pdev->dev.of_node;
-
-	return devm_reset_controller_register(&pdev->dev, &reset->rcdev);
-}
-
-static const struct of_device_id zx2967_reset_dt_ids[] = {
-	 { .compatible = "zte,zx296718-reset", },
-	 {},
-};
-
-static struct platform_driver zx2967_reset_driver = {
-	.probe	= zx2967_reset_probe,
-	.driver = {
-		.name		= "zx2967-reset",
-		.of_match_table	= zx2967_reset_dt_ids,
-	},
-};
-builtin_platform_driver(zx2967_reset_driver);
-- 
2.11.0

Re: [PATCH v4 2/5] reset: sunxi: use reset-simple driver

[Chen-Yu Tsai]

On Tue, Oct 17, 2017 at 9:03 PM, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> Use the newly created copies in the reset-simple driver to replace the
> sunxi platform driver code and reset operations. The separate sunxi
> driver still remains to register the early reset controllers, but it
> reuses the reset operations in reset-simple.
>
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> Reviewed-by: Alexandru Gagniuc <alex.g@adaptrum.com>

Reviewed-by: Chen-Yu Tsai <wens@csie.org>

Though to be honest this won't get much testing...

We've converted all sunxi SoCs over to the new sunxi-ng clk dt-bindings
and drivers, which include the reset controls as well.

Re: [PATCH v4 1/5] reset: add reset-simple to unify socfpga, stm32, sunxi, and zx2967

[Chen-Yu Tsai]

On Tue, Oct 17, 2017 at 9:03 PM, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> Copy reusable parts from the sunxi driver, to add a driver for simple
> reset controllers with reset lines that can be controlled by toggling
> bits in exclusive, contiguous register ranges using read-modify-write
> cycles under a spinlock.
>
> The following patches will replace compatible reset drivers with
> reset-simple, extending it where necessary.
>
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> Reviewed-by: Alexandru Gagniuc <alex.g@adaptrum.com>

Reviewed-by: Chen-Yu Tsai <wens@csie.org>

Re: [PATCH v4 3/5] reset: socfpga: use the reset-simple driver

[Andre Przywara]

Hi,

On 17/10/17 14:03, Philipp Zabel wrote:
> Add reset line status readback, inverted status support, and socfpga
> device tree quirks to the simple reset driver, and use it to replace
> the socfpga driver.
> 
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
> Changes since v3:
>  - Rebased onto reset/next
>  - Only warn about missing altr,modrst-offset property on socfpga
> ---
>  drivers/reset/Kconfig         |  10 +--
>  drivers/reset/Makefile        |   1 -
>  drivers/reset/reset-simple.c  |  51 +++++++++++++-
>  drivers/reset/reset-simple.h  |   4 ++
>  drivers/reset/reset-socfpga.c | 157 ------------------------------------------
>  5 files changed, 56 insertions(+), 167 deletions(-)
>  delete mode 100644 drivers/reset/reset-socfpga.c
> 
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index a14c0fe498771..2d6770b210ed7 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -77,19 +77,13 @@ config RESET_PISTACHIO
>  
>  config RESET_SIMPLE
>  	bool "Simple Reset Controller Driver" if COMPILE_TEST
> -	default ARCH_SUNXI
> +	default ARCH_SOCFPGA || ARCH_STRATIX10 || ARCH_SUNXI
>  	help
>  	  This enables a simple reset controller driver for reset lines that
>  	  that can be asserted and deasserted by toggling bits in a contiguous,
>  	  exclusive register space.
>  
> -	  Currently this driver supports Allwinner SoCs.
> -
> -config RESET_SOCFPGA
> -	bool "SoCFPGA Reset Driver" if COMPILE_TEST
> -	default ARCH_SOCFPGA || ARCH_STRATIX10
> -	help
> -	  This enables the reset controller driver for Altera SoCFPGAs.
> +	  Currently this driver supports Altera SoCFPGAs and Allwinner SoCs.
>  
>  config RESET_STM32
>  	bool "STM32 Reset Driver" if COMPILE_TEST
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index ddd3eeb193567..3bf3b4e6a9fd2 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -13,7 +13,6 @@ obj-$(CONFIG_RESET_MESON) += reset-meson.o
>  obj-$(CONFIG_RESET_OXNAS) += reset-oxnas.o
>  obj-$(CONFIG_RESET_PISTACHIO) += reset-pistachio.o
>  obj-$(CONFIG_RESET_SIMPLE) += reset-simple.o
> -obj-$(CONFIG_RESET_SOCFPGA) += reset-socfpga.o
>  obj-$(CONFIG_RESET_STM32) += reset-stm32.o
>  obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o
>  obj-$(CONFIG_RESET_TI_SCI) += reset-ti-sci.o
> diff --git a/drivers/reset/reset-simple.c b/drivers/reset/reset-simple.c
> index a5119457cec61..98ff0f924948e 100644
> --- a/drivers/reset/reset-simple.c
> +++ b/drivers/reset/reset-simple.c
> @@ -68,25 +68,58 @@ static int reset_simple_deassert(struct reset_controller_dev *rcdev,
>  	return reset_simple_update(rcdev, id, false);
>  }
>  
> +static int reset_simple_status(struct reset_controller_dev *rcdev,
> +			       unsigned long id)
> +{
> +	struct reset_simple_data *data = to_reset_simple_data(rcdev);
> +	int reg_width = sizeof(u32);
> +	int bank = id / (reg_width * BITS_PER_BYTE);
> +	int offset = id % (reg_width * BITS_PER_BYTE);
> +	u32 reg;
> +
> +	reg = readl(data->membase + (bank * reg_width));
> +
> +	return !(reg & BIT(offset)) ^ !data->status_active_low;
> +}
> +
>  const struct reset_control_ops reset_simple_ops = {
>  	.assert		= reset_simple_assert,
>  	.deassert	= reset_simple_deassert,
> +	.status		= reset_simple_status,
>  };
>  
>  /**
>   * struct reset_simple_devdata - simple reset controller properties
> + * @reg_offset: offset between base address and first reset register.
> + * @nr_resets: number of resets. If not set, default to resource size in bits.
>   * @active_low: if true, bits are cleared to assert the reset. Otherwise, bits
>   *              are set to assert the reset.
> + * @status_active_low: if true, bits read back as cleared while the reset is
> + *                     asserted. Otherwise, bits read back as set while the
> + *                     reset is asserted.
>   */
>  struct reset_simple_devdata {
> +	u32 reg_offset;
> +	u32 nr_resets;
>  	bool active_low;
> +	bool status_active_low;
> +};
> +
> +#define SOCFPGA_NR_BANKS	8
> +
> +static const struct reset_simple_devdata reset_simple_socfpga = {
> +	.reg_offset = 0x10,
> +	.nr_resets = SOCFPGA_NR_BANKS * 32,
> +	.status_active_low = true,
>  };
>  
>  static const struct reset_simple_devdata reset_simple_active_low = {
>  	.active_low = true,
> +	.status_active_low = true,
>  };
>  
>  static const struct of_device_id reset_simple_dt_ids[] = {
> +	{ .compatible = "altr,rst-mgr", .data = &reset_simple_socfpga },
>  	{ .compatible = "allwinner,sun6i-a31-clock-reset",
>  		.data = &reset_simple_active_low },
>  	{ /* sentinel */ },
> @@ -99,6 +132,7 @@ static int reset_simple_probe(struct platform_device *pdev)
>  	struct reset_simple_data *data;
>  	void __iomem *membase;
>  	struct resource *res;
> +	u32 reg_offset = 0;
>  
>  	devdata = of_device_get_match_data(dev);
>  
> @@ -118,8 +152,23 @@ static int reset_simple_probe(struct platform_device *pdev)
>  	data->rcdev.ops = &reset_simple_ops;
>  	data->rcdev.of_node = dev->of_node;
>  
> -	if (devdata)
> +	if (devdata) {
> +		reg_offset = devdata->reg_offset;
> +		if (devdata->nr_resets)
> +			data->rcdev.nr_resets = devdata->nr_resets;
>  		data->active_low = devdata->active_low;
> +		data->status_active_low = devdata->status_active_low;
> +	}
> +
> +	if (devdata == &reset_simple_socfpga &&

Mmh, this pointer comparison looks a bit dodgy. Isn't
of_device_is_compatible() the right solution here? Also semantically, as
the property is tied to a certain compatible string (and not to our data
structure)?

> +	    of_property_read_u32(dev->of_node, "altr,modrst-offset",
> +				 &reg_offset)) {
> +		dev_warn(dev,
> +			 "missing altr,modrst-offset property, assuming 0x%x!\n",
> +			 reg_offset);

The existing driver falls back to 0x10 if no explicit property is found.
Should we copy this here for compatibility reasons? We could simply use:
"reg_offset = 0x10;" before the dev_warn().

The rest looks good to me.

Cheers,
Andre.

> +	}
> +
> +	data->membase += reg_offset;
>  
>  	return devm_reset_controller_register(dev, &data->rcdev);
>  }
> diff --git a/drivers/reset/reset-simple.h b/drivers/reset/reset-simple.h
> index 39af2014b5f12..8a496022baeff 100644
> --- a/drivers/reset/reset-simple.h
> +++ b/drivers/reset/reset-simple.h
> @@ -28,12 +28,16 @@
>   * @active_low: if true, bits are cleared to assert the reset. Otherwise, bits
>   *              are set to assert the reset. Note that this says nothing about
>   *              the voltage level of the actual reset line.
> + * @status_active_low: if true, bits read back as cleared while the reset is
> + *                     asserted. Otherwise, bits read back as set while the
> + *                     reset is asserted.
>   */
>  struct reset_simple_data {
>  	spinlock_t			lock;
>  	void __iomem			*membase;
>  	struct reset_controller_dev	rcdev;
>  	bool				active_low;
> +	bool				status_active_low;
>  };
>  
>  extern const struct reset_control_ops reset_simple_ops;
> diff --git a/drivers/reset/reset-socfpga.c b/drivers/reset/reset-socfpga.c
> deleted file mode 100644
> index 3907bbc9c6cf7..0000000000000
> --- a/drivers/reset/reset-socfpga.c
> +++ /dev/null
> @@ -1,157 +0,0 @@
> -/*
> - * Socfpga Reset Controller Driver
> - *
> - * Copyright 2014 Steffen Trumtrar <s.trumtrar@pengutronix.de>
> - *
> - * based on
> - * Allwinner SoCs Reset Controller driver
> - *
> - * Copyright 2013 Maxime Ripard
> - *
> - * Maxime Ripard <maxime.ripard@free-electrons.com>
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License as published by
> - * the Free Software Foundation; either version 2 of the License, or
> - * (at your option) any later version.
> - */
> -
> -#include <linux/err.h>
> -#include <linux/io.h>
> -#include <linux/init.h>
> -#include <linux/of.h>
> -#include <linux/platform_device.h>
> -#include <linux/reset-controller.h>
> -#include <linux/spinlock.h>
> -#include <linux/types.h>
> -
> -#define BANK_INCREMENT		4
> -#define NR_BANKS		8
> -
> -struct socfpga_reset_data {
> -	spinlock_t			lock;
> -	void __iomem			*membase;
> -	struct reset_controller_dev	rcdev;
> -};
> -
> -static int socfpga_reset_assert(struct reset_controller_dev *rcdev,
> -				unsigned long id)
> -{
> -	struct socfpga_reset_data *data = container_of(rcdev,
> -						     struct socfpga_reset_data,
> -						     rcdev);
> -	int reg_width = sizeof(u32);
> -	int bank = id / (reg_width * BITS_PER_BYTE);
> -	int offset = id % (reg_width * BITS_PER_BYTE);
> -	unsigned long flags;
> -	u32 reg;
> -
> -	spin_lock_irqsave(&data->lock, flags);
> -
> -	reg = readl(data->membase + (bank * BANK_INCREMENT));
> -	writel(reg | BIT(offset), data->membase + (bank * BANK_INCREMENT));
> -	spin_unlock_irqrestore(&data->lock, flags);
> -
> -	return 0;
> -}
> -
> -static int socfpga_reset_deassert(struct reset_controller_dev *rcdev,
> -				  unsigned long id)
> -{
> -	struct socfpga_reset_data *data = container_of(rcdev,
> -						     struct socfpga_reset_data,
> -						     rcdev);
> -
> -	int reg_width = sizeof(u32);
> -	int bank = id / (reg_width * BITS_PER_BYTE);
> -	int offset = id % (reg_width * BITS_PER_BYTE);
> -	unsigned long flags;
> -	u32 reg;
> -
> -	spin_lock_irqsave(&data->lock, flags);
> -
> -	reg = readl(data->membase + (bank * BANK_INCREMENT));
> -	writel(reg & ~BIT(offset), data->membase + (bank * BANK_INCREMENT));
> -
> -	spin_unlock_irqrestore(&data->lock, flags);
> -
> -	return 0;
> -}
> -
> -static int socfpga_reset_status(struct reset_controller_dev *rcdev,
> -				unsigned long id)
> -{
> -	struct socfpga_reset_data *data = container_of(rcdev,
> -						struct socfpga_reset_data, rcdev);
> -	int reg_width = sizeof(u32);
> -	int bank = id / (reg_width * BITS_PER_BYTE);
> -	int offset = id % (reg_width * BITS_PER_BYTE);
> -	u32 reg;
> -
> -	reg = readl(data->membase + (bank * BANK_INCREMENT));
> -
> -	return !(reg & BIT(offset));
> -}
> -
> -static const struct reset_control_ops socfpga_reset_ops = {
> -	.assert		= socfpga_reset_assert,
> -	.deassert	= socfpga_reset_deassert,
> -	.status		= socfpga_reset_status,
> -};
> -
> -static int socfpga_reset_probe(struct platform_device *pdev)
> -{
> -	struct socfpga_reset_data *data;
> -	struct resource *res;
> -	struct device *dev = &pdev->dev;
> -	struct device_node *np = dev->of_node;
> -	u32 modrst_offset;
> -
> -	/*
> -	 * The binding was mainlined without the required property.
> -	 * Do not continue, when we encounter an old DT.
> -	 */
> -	if (!of_find_property(pdev->dev.of_node, "#reset-cells", NULL)) {
> -		dev_err(&pdev->dev, "%pOF missing #reset-cells property\n",
> -			pdev->dev.of_node);
> -		return -EINVAL;
> -	}
> -
> -	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> -	if (!data)
> -		return -ENOMEM;
> -
> -	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	data->membase = devm_ioremap_resource(&pdev->dev, res);
> -	if (IS_ERR(data->membase))
> -		return PTR_ERR(data->membase);
> -
> -	if (of_property_read_u32(np, "altr,modrst-offset", &modrst_offset)) {
> -		dev_warn(dev, "missing altr,modrst-offset property, assuming 0x10!\n");
> -		modrst_offset = 0x10;
> -	}
> -	data->membase += modrst_offset;
> -
> -	spin_lock_init(&data->lock);
> -
> -	data->rcdev.owner = THIS_MODULE;
> -	data->rcdev.nr_resets = NR_BANKS * (sizeof(u32) * BITS_PER_BYTE);
> -	data->rcdev.ops = &socfpga_reset_ops;
> -	data->rcdev.of_node = pdev->dev.of_node;
> -
> -	return devm_reset_controller_register(dev, &data->rcdev);
> -}
> -
> -static const struct of_device_id socfpga_reset_dt_ids[] = {
> -	{ .compatible = "altr,rst-mgr", },
> -	{ /* sentinel */ },
> -};
> -
> -static struct platform_driver socfpga_reset_driver = {
> -	.probe	= socfpga_reset_probe,
> -	.driver = {
> -		.name		= "socfpga-reset",
> -		.of_match_table	= socfpga_reset_dt_ids,
> -	},
> -};
> -builtin_platform_driver(socfpga_reset_driver);
> 

Re: [PATCH v4 3/5] reset: socfpga: use the reset-simple driver

[Philipp Zabel]

On Wed, 2017-10-18 at 14:00 +0100, Andre Przywara wrote:
> Hi,

Thank you for the review.

> On 17/10/17 14:03, Philipp Zabel wrote:
> > Add reset line status readback, inverted status support, and socfpga
> > device tree quirks to the simple reset driver, and use it to replace
> > the socfpga driver.
> > 
> > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> > ---
> > Changes since v3:
> >  - Rebased onto reset/next
> >  - Only warn about missing altr,modrst-offset property on socfpga
> > ---
> >  drivers/reset/Kconfig         |  10 +--
> >  drivers/reset/Makefile        |   1 -
> >  drivers/reset/reset-simple.c  |  51 +++++++++++++-
> >  drivers/reset/reset-simple.h  |   4 ++
> >  drivers/reset/reset-socfpga.c | 157 ------------------------------------------
> >  5 files changed, 56 insertions(+), 167 deletions(-)
> >  delete mode 100644 drivers/reset/reset-socfpga.c
> > 
[...]
> > diff --git a/drivers/reset/reset-simple.c b/drivers/reset/reset-simple.c
> > index a5119457cec61..98ff0f924948e 100644
> > --- a/drivers/reset/reset-simple.c
> > +++ b/drivers/reset/reset-simple.c
> > @@ -68,25 +68,58 @@ static int reset_simple_deassert(struct reset_controller_dev *rcdev,
> >  	return reset_simple_update(rcdev, id, false);
> >  }
> >  
> > +static int reset_simple_status(struct reset_controller_dev *rcdev,
> > +			       unsigned long id)
> > +{
> > +	struct reset_simple_data *data = to_reset_simple_data(rcdev);
> > +	int reg_width = sizeof(u32);
> > +	int bank = id / (reg_width * BITS_PER_BYTE);
> > +	int offset = id % (reg_width * BITS_PER_BYTE);
> > +	u32 reg;
> > +
> > +	reg = readl(data->membase + (bank * reg_width));
> > +
> > +	return !(reg & BIT(offset)) ^ !data->status_active_low;
> > +}
> > +
> >  const struct reset_control_ops reset_simple_ops = {
> >  	.assert		= reset_simple_assert,
> >  	.deassert	= reset_simple_deassert,
> > +	.status		= reset_simple_status,
> >  };
> >  
> >  /**
> >   * struct reset_simple_devdata - simple reset controller properties
> > + * @reg_offset: offset between base address and first reset register.
> > + * @nr_resets: number of resets. If not set, default to resource size in bits.
> >   * @active_low: if true, bits are cleared to assert the reset. Otherwise, bits
> >   *              are set to assert the reset.
> > + * @status_active_low: if true, bits read back as cleared while the reset is
> > + *                     asserted. Otherwise, bits read back as set while the
> > + *                     reset is asserted.
> >   */
> >  struct reset_simple_devdata {
> > +	u32 reg_offset;
> > +	u32 nr_resets;
> >  	bool active_low;
> > +	bool status_active_low;
> > +};
> > +
> > +#define SOCFPGA_NR_BANKS	8
> > +
> > +static const struct reset_simple_devdata reset_simple_socfpga = {
> > +	.reg_offset = 0x10,

Here reset_simple_socfpga.reg_offset is set to the default of 0x10.

> > +	.nr_resets = SOCFPGA_NR_BANKS * 32,
> > +	.status_active_low = true,
> >  };
> >  
> >  static const struct reset_simple_devdata reset_simple_active_low = {
> >  	.active_low = true,
> > +	.status_active_low = true,
> >  };
> >  
> >  static const struct of_device_id reset_simple_dt_ids[] = {
> > +	{ .compatible = "altr,rst-mgr", .data = &reset_simple_socfpga },
> >  	{ .compatible = "allwinner,sun6i-a31-clock-reset",
> >  		.data = &reset_simple_active_low },
> >  	{ /* sentinel */ },
> > @@ -99,6 +132,7 @@ static int reset_simple_probe(struct platform_device *pdev)
> >  	struct reset_simple_data *data;
> >  	void __iomem *membase;
> >  	struct resource *res;
> > +	u32 reg_offset = 0;
> >  
> >  	devdata = of_device_get_match_data(dev);
> >  
> > @@ -118,8 +152,23 @@ static int reset_simple_probe(struct platform_device *pdev)
> >  	data->rcdev.ops = &reset_simple_ops;
> >  	data->rcdev.of_node = dev->of_node;
> >  
> > -	if (devdata)
> > +	if (devdata) {
> > +		reg_offset = devdata->reg_offset;

And here reg_offset is set to the default of 0x10 on socfpga.

> > +		if (devdata->nr_resets)
> > +			data->rcdev.nr_resets = devdata->nr_resets;
> >  		data->active_low = devdata->active_low;
> > +		data->status_active_low = devdata->status_active_low;
> > +	}
> > +
> > +	if (devdata == &reset_simple_socfpga &&
> 
> Mmh, this pointer comparison looks a bit dodgy. Isn't
> of_device_is_compatible() the right solution here?

My thinking was, the of_device_is_compatible is already called inside
of_device_get_match_data, so why call it again and not reuse the result?

> Also semantically, as the property is tied to a certain compatible
> string (and not to our data structure)?

I agree with this, though. I'll change this line to say:

+	if (of_device_is_compatible(dev->of_node, "altr,rst-mgr") &&

instead.

> > +	    of_property_read_u32(dev->of_node, "altr,modrst-offset",
> > +		dev_warn(dev,
> > +			 "missing altr,modrst-offset property, assuming 0x%x!\n",
> > +			 reg_offset);
> 
> The existing driver falls back to 0x10 if no explicit property is found.
> Should we copy this here for compatibility reasons? We could simply use:
> "reg_offset = 0x10;" before the dev_warn().

See above.

regards
Philipp

Re: [PATCH v4 3/5] reset: socfpga: use the reset-simple driver

[Andre Przywara]

Hi,

On 18/10/17 14:50, Philipp Zabel wrote:
> On Wed, 2017-10-18 at 14:00 +0100, Andre Przywara wrote:
>> Hi,
> 
> Thank you for the review.
> 
>> On 17/10/17 14:03, Philipp Zabel wrote:
>>> Add reset line status readback, inverted status support, and socfpga
>>> device tree quirks to the simple reset driver, and use it to replace
>>> the socfpga driver.
>>>
>>> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
>>> ---
>>> Changes since v3:
>>>  - Rebased onto reset/next
>>>  - Only warn about missing altr,modrst-offset property on socfpga
>>> ---
>>>  drivers/reset/Kconfig         |  10 +--
>>>  drivers/reset/Makefile        |   1 -
>>>  drivers/reset/reset-simple.c  |  51 +++++++++++++-
>>>  drivers/reset/reset-simple.h  |   4 ++
>>>  drivers/reset/reset-socfpga.c | 157 ------------------------------------------
>>>  5 files changed, 56 insertions(+), 167 deletions(-)
>>>  delete mode 100644 drivers/reset/reset-socfpga.c
>>>
> [...]
>>> diff --git a/drivers/reset/reset-simple.c b/drivers/reset/reset-simple.c
>>> index a5119457cec61..98ff0f924948e 100644
>>> --- a/drivers/reset/reset-simple.c
>>> +++ b/drivers/reset/reset-simple.c
>>> @@ -68,25 +68,58 @@ static int reset_simple_deassert(struct reset_controller_dev *rcdev,
>>>  	return reset_simple_update(rcdev, id, false);
>>>  }
>>>  
>>> +static int reset_simple_status(struct reset_controller_dev *rcdev,
>>> +			       unsigned long id)
>>> +{
>>> +	struct reset_simple_data *data = to_reset_simple_data(rcdev);
>>> +	int reg_width = sizeof(u32);
>>> +	int bank = id / (reg_width * BITS_PER_BYTE);
>>> +	int offset = id % (reg_width * BITS_PER_BYTE);
>>> +	u32 reg;
>>> +
>>> +	reg = readl(data->membase + (bank * reg_width));
>>> +
>>> +	return !(reg & BIT(offset)) ^ !data->status_active_low;
>>> +}
>>> +
>>>  const struct reset_control_ops reset_simple_ops = {
>>>  	.assert		= reset_simple_assert,
>>>  	.deassert	= reset_simple_deassert,
>>> +	.status		= reset_simple_status,
>>>  };
>>>  
>>>  /**
>>>   * struct reset_simple_devdata - simple reset controller properties
>>> + * @reg_offset: offset between base address and first reset register.
>>> + * @nr_resets: number of resets. If not set, default to resource size in bits.
>>>   * @active_low: if true, bits are cleared to assert the reset. Otherwise, bits
>>>   *              are set to assert the reset.
>>> + * @status_active_low: if true, bits read back as cleared while the reset is
>>> + *                     asserted. Otherwise, bits read back as set while the
>>> + *                     reset is asserted.
>>>   */
>>>  struct reset_simple_devdata {
>>> +	u32 reg_offset;
>>> +	u32 nr_resets;
>>>  	bool active_low;
>>> +	bool status_active_low;
>>> +};
>>> +
>>> +#define SOCFPGA_NR_BANKS	8
>>> +
>>> +static const struct reset_simple_devdata reset_simple_socfpga = {
>>> +	.reg_offset = 0x10,
> 
> Here reset_simple_socfpga.reg_offset is set to the default of 0x10.

Ah, right. Sorry, I missed that.

> 
>>> +	.nr_resets = SOCFPGA_NR_BANKS * 32,
>>> +	.status_active_low = true,
>>>  };
>>>  
>>>  static const struct reset_simple_devdata reset_simple_active_low = {
>>>  	.active_low = true,
>>> +	.status_active_low = true,
>>>  };
>>>  
>>>  static const struct of_device_id reset_simple_dt_ids[] = {
>>> +	{ .compatible = "altr,rst-mgr", .data = &reset_simple_socfpga },
>>>  	{ .compatible = "allwinner,sun6i-a31-clock-reset",
>>>  		.data = &reset_simple_active_low },
>>>  	{ /* sentinel */ },
>>> @@ -99,6 +132,7 @@ static int reset_simple_probe(struct platform_device *pdev)
>>>  	struct reset_simple_data *data;
>>>  	void __iomem *membase;
>>>  	struct resource *res;
>>> +	u32 reg_offset = 0;
>>>  
>>>  	devdata = of_device_get_match_data(dev);
>>>  
>>> @@ -118,8 +152,23 @@ static int reset_simple_probe(struct platform_device *pdev)
>>>  	data->rcdev.ops = &reset_simple_ops;
>>>  	data->rcdev.of_node = dev->of_node;
>>>  
>>> -	if (devdata)
>>> +	if (devdata) {
>>> +		reg_offset = devdata->reg_offset;
> 
> And here reg_offset is set to the default of 0x10 on socfpga.
> 
>>> +		if (devdata->nr_resets)
>>> +			data->rcdev.nr_resets = devdata->nr_resets;
>>>  		data->active_low = devdata->active_low;
>>> +		data->status_active_low = devdata->status_active_low;
>>> +	}
>>> +
>>> +	if (devdata == &reset_simple_socfpga &&
>>
>> Mmh, this pointer comparison looks a bit dodgy. Isn't
>> of_device_is_compatible() the right solution here?
> 
> My thinking was, the of_device_is_compatible is already called inside
> of_device_get_match_data, so why call it again and not reuse the result?
> 
>> Also semantically, as the property is tied to a certain compatible
>> string (and not to our data structure)?
> 
> I agree with this, though. I'll change this line to say:
> 
> +	if (of_device_is_compatible(dev->of_node, "altr,rst-mgr") &&

Yes, thank you, that is what I was after.

> 
> instead.
> 
>>> +	    of_property_read_u32(dev->of_node, "altr,modrst-offset",
>>> +		dev_warn(dev,
>>> +			 "missing altr,modrst-offset property, assuming 0x%x!\n",
>>> +			 reg_offset);
>>
>> The existing driver falls back to 0x10 if no explicit property is found.
>> Should we copy this here for compatibility reasons? We could simply use:
>> "reg_offset = 0x10;" before the dev_warn().

Giving the binding documentation a second look, I see that
altr,modrst-offset is a *required* property. So shall we actually use
the opportunity here to stop our too generous behaviour of "you are
missing this required property, but let me fix this up for you anyway
this (one more) time" and revert to a more strict return -ENODEV
instead? Otherwise we just proliferate wrong DTs.
Also this would remove this .reg_offset field from our struct.

Cheers,
Andre.