[RFC PATCH v2 0/3] Add reset control for mfd syscon devices

[RFC PATCH v2 0/3] Add reset control for mfd syscon devices

[Jeremy Kerr]

This RFC series adds a facility for syscon devices to control a reset
line when probed; we have instances of simple register-only syscon
resources that need deassertion of a reset line for the register set to
be accessible.

Rather than requiring a specific driver to implement this, it'd be nice
to use the generic syscon device and the generic resets linkage to do
so.

Any comments/queries/etc are most welcome.

Cheers,


Jeremy
---
v2:
 * do reset control in the early of_syscon_register() path, rather than
   the platform device init, which isn't used.
 * consequently, add regmap infrastructure to attach a reset
   controller, in a similar way to attaching clocks

Jeremy Kerr (3):
  dt-bindings: mfd/syscon: Add resets property
  regmap: mmio: allow reset control in a MMIO regmap
  mfd: syscon: allow reset control for syscon devices

 .../devicetree/bindings/mfd/syscon.yaml       |  3 +++
 drivers/base/regmap/regmap-mmio.c             | 22 +++++++++++++++
 drivers/mfd/syscon.c                          | 27 ++++++++++++++-----
 include/linux/regmap.h                        |  3 +++
 4 files changed, 49 insertions(+), 6 deletions(-)

-- 
2.35.1

[RFC PATCH v2 1/3] dt-bindings: mfd/syscon: Add resets property

[Jeremy Kerr]

Simple syscon devices may require deassertion of a reset signal in order
to access their register set. This change adds the `resets` property from
reset.yaml#/properties/resets (referenced through core.yaml), specifying
a maxItems of 1 for a single (optional) reset descriptor.

This will allow a future change to the syscon driver to implement reset
control.

Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
Acked-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/mfd/syscon.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/syscon.yaml b/Documentation/devicetree/bindings/mfd/syscon.yaml
index 4e4baf53796d..9dc5984d9147 100644
--- a/Documentation/devicetree/bindings/mfd/syscon.yaml
+++ b/Documentation/devicetree/bindings/mfd/syscon.yaml
@@ -86,6 +86,9 @@ properties:
       on the device.
     enum: [1, 2, 4, 8]
 
+  resets:
+    maxItems: 1
+
   hwlocks:
     maxItems: 1
     description:
-- 
2.35.1

[RFC PATCH v2 2/3] regmap: mmio: allow reset control in a MMIO regmap

[Jeremy Kerr]

A syscon device may need to be taken out of reset before functioning -
this change adds a facility to attach a reset control to a mmio regmap,
and performs the necessary deassert/assert operations on the reset
controller on attach/detach.

Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
---
 drivers/base/regmap/regmap-mmio.c | 22 ++++++++++++++++++++++
 include/linux/regmap.h            |  3 +++
 2 files changed, 25 insertions(+)

diff --git a/drivers/base/regmap/regmap-mmio.c b/drivers/base/regmap/regmap-mmio.c
index 3ccdd86a97e7..e2611de73b42 100644
--- a/drivers/base/regmap/regmap-mmio.c
+++ b/drivers/base/regmap/regmap-mmio.c
@@ -8,6 +8,7 @@
 #include <linux/err.h>
 #include <linux/io.h>
 #include <linux/module.h>
+#include <linux/reset.h>
 #include <linux/regmap.h>
 #include <linux/slab.h>
 #include <linux/swab.h>
@@ -22,6 +23,8 @@ struct regmap_mmio_context {
 	bool attached_clk;
 	struct clk *clk;
 
+	struct reset_control *reset;
+
 	void (*reg_write)(struct regmap_mmio_context *ctx,
 			  unsigned int reg, unsigned int val);
 	unsigned int (*reg_read)(struct regmap_mmio_context *ctx,
@@ -633,4 +636,23 @@ void regmap_mmio_detach_clk(struct regmap *map)
 }
 EXPORT_SYMBOL_GPL(regmap_mmio_detach_clk);
 
+int regmap_mmio_attach_reset(struct regmap *map, struct reset_control *reset)
+{
+	struct regmap_mmio_context *ctx = map->bus_context;
+
+	ctx->reset = reset;
+
+	return reset_control_deassert(ctx->reset);
+}
+EXPORT_SYMBOL_GPL(regmap_mmio_attach_reset);
+
+void regmap_mmio_detach_reset(struct regmap *map)
+{
+	struct regmap_mmio_context *ctx = map->bus_context;
+
+	reset_control_assert(ctx->reset);
+	ctx->reset = NULL;
+}
+EXPORT_SYMBOL_GPL(regmap_mmio_detach_reset);
+
 MODULE_LICENSE("GPL v2");
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index ca3434dca3a0..b0c7a747c06f 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -34,6 +34,7 @@ struct spmi_device;
 struct regmap;
 struct regmap_range_cfg;
 struct regmap_field;
+struct reset_control;
 struct snd_ac97;
 struct sdw_slave;
 
@@ -1150,6 +1151,8 @@ bool regmap_ac97_default_volatile(struct device *dev, unsigned int reg);
 
 int regmap_mmio_attach_clk(struct regmap *map, struct clk *clk);
 void regmap_mmio_detach_clk(struct regmap *map);
+int regmap_mmio_attach_reset(struct regmap *map, struct reset_control *reset);
+void regmap_mmio_detach_reset(struct regmap *map);
 void regmap_exit(struct regmap *map);
 int regmap_reinit_cache(struct regmap *map,
 			const struct regmap_config *config);
-- 
2.35.1

[RFC PATCH v2 3/3] mfd: syscon: allow reset control for syscon devices

[Jeremy Kerr]

Simple syscon devices may require deassertion of a reset signal in order
to access their register set. Rather than requiring a custom driver to
implement this, we can use the generic "resets" specifiers to link a
reset line to the syscon.

This change adds an optional reset line to the syscon device
description, and code to perform the deassertion/assertion on
probe/remove.

Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>

---
v2:
 * do reset control in the early of_syscon_register() path, rather than
   the platform device init, which isn't used.
---
 drivers/mfd/syscon.c | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c
index bdb2ce7ff03b..6b99488d05a6 100644
--- a/drivers/mfd/syscon.c
+++ b/drivers/mfd/syscon.c
@@ -20,6 +20,7 @@
 #include <linux/platform_data/syscon.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
+#include <linux/reset.h>
 #include <linux/mfd/syscon.h>
 #include <linux/slab.h>
 
@@ -31,6 +32,7 @@ static LIST_HEAD(syscon_list);
 struct syscon {
 	struct device_node *np;
 	struct regmap *regmap;
+	struct reset_control *reset;
 	struct list_head list;
 };
 
@@ -40,7 +42,7 @@ static const struct regmap_config syscon_regmap_config = {
 	.reg_stride = 4,
 };
 
-static struct syscon *of_syscon_register(struct device_node *np, bool check_clk)
+static struct syscon *of_syscon_register(struct device_node *np, bool check_res)
 {
 	struct clk *clk;
 	struct syscon *syscon;
@@ -50,6 +52,7 @@ static struct syscon *of_syscon_register(struct device_node *np, bool check_clk)
 	int ret;
 	struct regmap_config syscon_config = syscon_regmap_config;
 	struct resource res;
+	struct reset_control *reset;
 
 	syscon = kzalloc(sizeof(*syscon), GFP_KERNEL);
 	if (!syscon)
@@ -114,7 +117,7 @@ static struct syscon *of_syscon_register(struct device_node *np, bool check_clk)
 		goto err_regmap;
 	}
 
-	if (check_clk) {
+	if (check_res) {
 		clk = of_clk_get(np, 0);
 		if (IS_ERR(clk)) {
 			ret = PTR_ERR(clk);
@@ -124,7 +127,17 @@ static struct syscon *of_syscon_register(struct device_node *np, bool check_clk)
 		} else {
 			ret = regmap_mmio_attach_clk(regmap, clk);
 			if (ret)
-				goto err_attach;
+				goto err_attach_clk;
+		}
+
+		reset = of_reset_control_get_optional_exclusive(np, NULL);
+		if (IS_ERR(reset)) {
+			ret = PTR_ERR(reset);
+			goto err_attach_clk;
+		} else if (reset) {
+			ret = regmap_mmio_attach_reset(regmap, reset);
+			if (ret)
+				goto err_attach_reset;
 		}
 	}
 
@@ -137,7 +150,9 @@ static struct syscon *of_syscon_register(struct device_node *np, bool check_clk)
 
 	return syscon;
 
-err_attach:
+err_attach_reset:
+	reset_control_put(reset);
+err_attach_clk:
 	if (!IS_ERR(clk))
 		clk_put(clk);
 err_clk:
@@ -150,7 +165,7 @@ static struct syscon *of_syscon_register(struct device_node *np, bool check_clk)
 }
 
 static struct regmap *device_node_get_regmap(struct device_node *np,
-					     bool check_clk)
+					     bool check_res)
 {
 	struct syscon *entry, *syscon = NULL;
 
@@ -165,7 +180,7 @@ static struct regmap *device_node_get_regmap(struct device_node *np,
 	spin_unlock(&syscon_list_slock);
 
 	if (!syscon)
-		syscon = of_syscon_register(np, check_clk);
+		syscon = of_syscon_register(np, check_res);
 
 	if (IS_ERR(syscon))
 		return ERR_CAST(syscon);
-- 
2.35.1

Re: [RFC PATCH v2 0/3] Add reset control for mfd syscon devices

[Arnd Bergmann]

On Fri, Dec 9, 2022, at 02:33, Jeremy Kerr wrote:
> This RFC series adds a facility for syscon devices to control a reset
> line when probed; we have instances of simple register-only syscon
> resources that need deassertion of a reset line for the register set to
> be accessible.
>
> Rather than requiring a specific driver to implement this, it'd be nice
> to use the generic syscon device and the generic resets linkage to do
> so.
>
> Any comments/queries/etc are most welcome.

The syscon side looks good to me,

Reviewed-by: Arnd Bergmann <arnd@arndb.de>

Re: [RFC PATCH v2 2/3] regmap: mmio: allow reset control in a MMIO regmap

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 509 bytes --]

On Fri, Dec 09, 2022 at 09:33:08AM +0800, Jeremy Kerr wrote:
> A syscon device may need to be taken out of reset before functioning -
> this change adds a facility to attach a reset control to a mmio regmap,
> and performs the necessary deassert/assert operations on the reset
> controller on attach/detach.

Managment of reset feels like something that should be done at a higher
level - typically reset also implies losing all the register contents
which means it should be somewhere above the cache layer.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [RFC PATCH v2 2/3] regmap: mmio: allow reset control in a MMIO regmap

[Jeremy Kerr]

Hi Mark,

> > A syscon device may need to be taken out of reset before functioning
> > - this change adds a facility to attach a reset control to a mmio
> > regmap, and performs the necessary deassert/assert operations on the
> > reset controller on attach/detach.
> 
> Managment of reset feels like something that should be done at a
> higher level - typically reset also implies losing all the register
> contents which means it should be somewhere above the cache layer.

Yep, that makes sense. I'll rework to do the reset controller handling
in the syscon layer instead - unless there are any objections to that?

Cheers,


Jeremy