[PATCH v4 1/2] Documentation: Add GPIO reset binding to reset binding documentation

[PATCH v4 1/2] Documentation: Add GPIO reset binding to reset binding documentation

[Philipp Zabel]

This patch adds documentation clarifying the reset GPIO bindings most
commonly in use (reset-gpios and <name>-reset-gpios properties).

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 Documentation/devicetree/bindings/reset/reset.txt | 26 +++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/reset/reset.txt b/Documentation/devicetree/bindings/reset/reset.txt
index 31db6ff..51f9e35 100644
--- a/Documentation/devicetree/bindings/reset/reset.txt
+++ b/Documentation/devicetree/bindings/reset/reset.txt
@@ -2,8 +2,8 @@
 
 This binding is intended to represent the hardware reset signals present
 internally in most IC (SoC, FPGA, ...) designs. Reset signals for whole
-standalone chips are most likely better represented as GPIOs, although there
-are likely to be exceptions to this rule.
+standalone chips are most likely better represented as GPIOs, ideally using a
+common scheme as described below.
 
 Hardware blocks typically receive a reset signal. This signal is generated by
 a reset provider (e.g. power management or clock module) and received by a
@@ -56,6 +56,20 @@ reset-names:	List of reset signal name strings sorted in the same order as
 		the resets property. Consumers drivers will use reset-names to
 		match reset signal names with reset specifiers.
 
+= GPIO Reset consumers =
+
+For the common case of reset lines controlled by GPIOs, the GPIO binding
+documented in devicetree/bindings/gpio/gpio.txt should be used:
+
+Required properties:
+reset-gpios or		Reset GPIO using standard GPIO bindings,
+<name>-reset-gpios:	optionally named to specify the reset line
+
+Optional properties:
+reset-boot-asserted or		Boolean. If set, the corresponding reset is
+<name>-reset-boot-asserted:	initially asserted and should be kept that way
+				until released by the driver.
+
 For example:
 
 	device {
@@ -65,6 +79,14 @@ For example:
 
 This represents a device with a single reset signal named "reset".
 
+	device2 {
+		reset-gpios = <&gpio3 15 GPIO_ACTIVE_LOW>;
+		reset-boot-asserted;
+	};
+
+This represents a device with a single reset signal, controlled
+by an active-low GPIO, which is initally kept in reset.
+
 	bus {
 		resets = <&rst 10> <&rst 11> <&rst 12> <&rst 11>;
 		reset-names = "i2s1", "i2s2", "dma", "mixer";
-- 
1.9.1

[PATCH v4 2/2] reset: Add GPIO support to reset controller framework

[Philipp Zabel]

This adds support for GPIO controlled reset pins on peripheral ICs to the reset
controller framework. Currently there is no support for specifying a delay
between assertion and de-assertion of the reset signal, so this has to be
handled by the drivers.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
Changes since v3:
 - Rebased onto v3.15-rc1
 - Turned gpiod_get error into debug message, we don't want this to trigger
   for optional resets if no GPIO is given.
---
 drivers/reset/Kconfig      |  17 ++++++
 drivers/reset/Makefile     |   9 ++-
 drivers/reset/core.c       |  29 +++++-----
 drivers/reset/gpio-reset.c | 137 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/reset/reset-core.h |  48 ++++++++++++++++
 5 files changed, 225 insertions(+), 15 deletions(-)
 create mode 100644 drivers/reset/gpio-reset.c
 create mode 100644 drivers/reset/reset-core.h

diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index 0615f50..26a1d24 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -10,6 +10,23 @@ menuconfig RESET_CONTROLLER
 	  This framework is designed to abstract reset handling of devices
 	  via GPIOs or SoC-internal reset controller modules.
 
+	  If the device tree contains any resets or reset-gpio properties,
+	  this probably should be enabled.
+
 	  If unsure, say no.
 
+if RESET_CONTROLLER
+
+menuconfig RESET_GPIO
+	bool "GPIO Reset Support"
+	depends on GPIOLIB
+	default y if GPIOLIB
+	help
+	  GPIO Reset Controller support.
+
+	  This option lets the reset controller framework handle reset lines
+	  connected to GPIOs.
+
+endif
+
 source "drivers/reset/sti/Kconfig"
diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index 4f60caf..44c96b3 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -1,3 +1,10 @@
-obj-$(CONFIG_RESET_CONTROLLER) += core.o
+reset-core-objs := core.o
+
+obj-$(CONFIG_RESET_CONTROLLER) += reset-core.o
+
+ifeq ($(CONFIG_RESET_GPIO),y)
+  reset-core-objs += gpio-reset.o
+endif
+
 obj-$(CONFIG_ARCH_SUNXI) += reset-sunxi.o
 obj-$(CONFIG_ARCH_STI) += sti/
diff --git a/drivers/reset/core.c b/drivers/reset/core.c
index baeaf82..6bfd2d2 100644
--- a/drivers/reset/core.c
+++ b/drivers/reset/core.c
@@ -18,23 +18,12 @@
 #include <linux/reset-controller.h>
 #include <linux/slab.h>
 
+#include "reset-core.h"
+
 static DEFINE_MUTEX(reset_controller_list_mutex);
 static LIST_HEAD(reset_controller_list);
 
 /**
- * struct reset_control - a reset control
- * @rcdev: a pointer to the reset controller device
- *         this reset control belongs to
- * @id: ID of the reset controller in the reset
- *      controller device
- */
-struct reset_control {
-	struct reset_controller_dev *rcdev;
-	struct device *dev;
-	unsigned int id;
-};
-
-/**
  * of_reset_simple_xlate - translate reset_spec to the reset line number
  * @rcdev: a pointer to the reset controller device
  * @reset_spec: reset line specifier as found in the device tree
@@ -149,6 +138,8 @@ struct reset_control *of_reset_control_get(struct device_node *node,
 						 "reset-names", id);
 	ret = of_parse_phandle_with_args(node, "resets", "#reset-cells",
 					 index, &args);
+	if (index == -EINVAL)
+		return ERR_PTR(-ENOENT);
 	if (ret)
 		return ERR_PTR(ret);
 
@@ -209,6 +200,13 @@ struct reset_control *reset_control_get(struct device *dev, const char *id)
 	if (!IS_ERR(rstc))
 		rstc->dev = dev;
 
+	/*
+	 * If there is no dedicated reset controller device, check if we have
+	 * a reset line controlled by a GPIO instead.
+	 */
+	if (PTR_ERR(rstc) == -ENOENT)
+		return gpio_reset_control_get(dev, id);
+
 	return rstc;
 }
 EXPORT_SYMBOL_GPL(reset_control_get);
@@ -223,7 +221,10 @@ void reset_control_put(struct reset_control *rstc)
 	if (IS_ERR(rstc))
 		return;
 
-	module_put(rstc->rcdev->owner);
+	if (reset_control_is_gpio(rstc))
+		gpio_reset_control_put(rstc);
+	else
+		module_put(rstc->rcdev->owner);
 	kfree(rstc);
 }
 EXPORT_SYMBOL_GPL(reset_control_put);
diff --git a/drivers/reset/gpio-reset.c b/drivers/reset/gpio-reset.c
new file mode 100644
index 0000000..4b12136
--- /dev/null
+++ b/drivers/reset/gpio-reset.c
@@ -0,0 +1,137 @@
+/*
+ * GPIO Reset Controller
+ *
+ * Copyright 2013 Philipp Zabel, Pengutronix
+ *
+ * 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/gpio.h>
+#include <linux/gpio/consumer.h>
+#include <linux/reset-controller.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+
+#include "reset-core.h"
+
+/*
+ * Global GPIO reset controller
+ */
+static struct reset_controller_dev *gpio_rcdev;
+
+static int gpio_reset_set(struct reset_controller_dev *rcdev,
+			  unsigned int gpio, int asserted)
+{
+	struct gpio_desc *gpiod = gpio_to_desc(gpio);
+
+	if (!gpiod)
+		return -EINVAL;
+
+	gpiod_set_value_cansleep(gpiod, asserted);
+
+	return 0;
+}
+
+static int gpio_reset_assert(struct reset_controller_dev *rcdev,
+		unsigned long id)
+{
+	return gpio_reset_set(rcdev, id, 1);
+}
+
+static int gpio_reset_deassert(struct reset_controller_dev *rcdev,
+		unsigned long id)
+{
+	return gpio_reset_set(rcdev, id, 0);
+}
+
+static struct reset_control_ops gpio_reset_ops = {
+	.assert = gpio_reset_assert,
+	.deassert = gpio_reset_deassert,
+};
+
+struct reset_controller_dev *reset_get_gpio_rcdev(void)
+{
+	if (gpio_rcdev)
+		return gpio_rcdev;
+
+	gpio_rcdev = kzalloc(sizeof(*gpio_rcdev), GFP_KERNEL);
+	if (!gpio_rcdev)
+		return NULL;
+
+	gpio_rcdev->owner = THIS_MODULE;
+	gpio_rcdev->ops = &gpio_reset_ops;
+
+	reset_controller_register(gpio_rcdev);
+
+	return gpio_rcdev;
+}
+
+struct reset_control *gpio_reset_control_get(struct device *dev, const char *id)
+{
+	const char *assert_prop = "reset-boot-asserted";
+	const char *gpio_con_id = "reset";
+	struct reset_controller_dev *rcdev;
+	struct reset_control *rstc;
+	struct gpio_desc *gpiod;
+	char scratch[48];
+	bool asserted;
+	int ret;
+
+	if (id) {
+		snprintf(scratch, 48, "%s-reset-boot-asserted", id);
+		assert_prop = scratch;
+	}
+
+	asserted = of_property_read_bool(dev->of_node, assert_prop);
+
+	if (id) {
+		snprintf(scratch, 48, "%s-reset", id);
+		gpio_con_id = scratch;
+	}
+
+	gpiod = gpiod_get(dev, gpio_con_id);
+	if (IS_ERR(gpiod)) {
+		dev_dbg(dev, "Failed to get GPIO reset: %ld\n", PTR_ERR(gpiod));
+		return ERR_CAST(gpiod);
+	}
+
+	ret = gpiod_direction_output(gpiod, asserted);
+	if (ret < 0)
+		goto err_put;
+
+	ret = -ENOMEM;
+	rcdev = reset_get_gpio_rcdev();
+	if (!rcdev)
+		goto err_put;
+
+	rstc = kzalloc(sizeof(*rstc), GFP_KERNEL);
+	if (!rstc)
+		goto err_put;
+
+	rstc->dev = dev;
+	rstc->rcdev = rcdev;
+	rstc->id = desc_to_gpio(gpiod);
+
+	return rstc;
+
+err_put:
+	gpiod_put(gpiod);
+	return ERR_PTR(ret);
+}
+
+bool reset_control_is_gpio(struct reset_control *rstc)
+{
+	return rstc->rcdev == gpio_rcdev;
+}
+
+void gpio_reset_control_put(struct reset_control *rstc)
+{
+	struct gpio_desc *gpiod = gpio_to_desc(rstc->id);
+
+	if (gpiod)
+		gpiod_put(gpiod);
+}
diff --git a/drivers/reset/reset-core.h b/drivers/reset/reset-core.h
new file mode 100644
index 0000000..ef50100
--- /dev/null
+++ b/drivers/reset/reset-core.h
@@ -0,0 +1,48 @@
+/*
+ * Reset Controller framework
+ *
+ * Copyright 2013 Philipp Zabel, Pengutronix
+ *
+ * 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_CORE_H__
+#define __RESET_CORE_H__
+
+/**
+ * struct reset_control - a reset control
+ * @rcdev: a pointer to the reset controller device
+ *         this reset control belongs to
+ * @id: ID of the reset controller in the reset
+ *      controller device
+ */
+struct reset_control {
+	struct reset_controller_dev *rcdev;
+	struct device *dev;
+	unsigned int id;
+};
+
+#if IS_ENABLED(CONFIG_RESET_GPIO)
+struct reset_control *gpio_reset_control_get(struct device *dev,
+					     const char *id);
+bool reset_control_is_gpio(struct reset_control *rstc);
+void gpio_reset_control_put(struct reset_control *rstc);
+#else
+static inline struct reset_control *gpio_reset_control_get(struct device *dev,
+						    const char *id)
+{
+	return -ENOSYS;
+}
+
+static inline bool reset_control_is_gpio(struct reset_control *rstc)
+{
+	return false;
+}
+
+static inline void gpio_reset_control_put(struct reset_control *rstc) { }
+#endif
+
+#endif
-- 
1.9.1

Re: [PATCH v4 2/2] reset: Add GPIO support to reset controller framework

[One Thousand Gnomes]

> This adds support for GPIO controlled reset pins on peripheral ICs to the reset
> controller framework. Currently there is no support for specifying a delay
> between assertion and de-assertion of the reset signal, so this has to be
> handled by the drivers.

Lots of GPIO controllers are doing posted writes to the fabric so surely
your delay will be wrong ?


Alan

Re: [PATCH v4 2/2] reset: Add GPIO support to reset controller framework

[Philipp Zabel]

Hi Alan,

Am Montag, den 14.04.2014, 11:57 +0100 schrieb One Thousand Gnomes:
> > This adds support for GPIO controlled reset pins on peripheral ICs to the reset
> > controller framework. Currently there is no support for specifying a delay
> > between assertion and de-assertion of the reset signal, so this has to be
> > handled by the drivers.
> 
> Lots of GPIO controllers are doing posted writes to the fabric so surely
> your delay will be wrong ?

I haven't seen hardware yet where this is an issue, but it's a valid
point. I can also imagine boards where the GPIO is connected to some
board specific timed reset logic or where the reset length has to be
altered just due to capacitance.

regards
Philipp

Re: [PATCH v4 1/2] Documentation: Add GPIO reset binding to reset binding documentation

[Maxime Ripard]

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

On Mon, Apr 14, 2014 at 10:21:48AM +0200, Philipp Zabel wrote:
> This patch adds documentation clarifying the reset GPIO bindings most
> commonly in use (reset-gpios and <name>-reset-gpios properties).
> 
> Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/reset/reset.txt | 26 +++++++++++++++++++++--
>  1 file changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/reset/reset.txt b/Documentation/devicetree/bindings/reset/reset.txt
> index 31db6ff..51f9e35 100644
> --- a/Documentation/devicetree/bindings/reset/reset.txt
> +++ b/Documentation/devicetree/bindings/reset/reset.txt
> @@ -2,8 +2,8 @@
>  
>  This binding is intended to represent the hardware reset signals present
>  internally in most IC (SoC, FPGA, ...) designs. Reset signals for whole
> -standalone chips are most likely better represented as GPIOs, although there
> -are likely to be exceptions to this rule.
> +standalone chips are most likely better represented as GPIOs, ideally using a
> +common scheme as described below.
>  
>  Hardware blocks typically receive a reset signal. This signal is generated by
>  a reset provider (e.g. power management or clock module) and received by a
> @@ -56,6 +56,20 @@ reset-names:	List of reset signal name strings sorted in the same order as
>  		the resets property. Consumers drivers will use reset-names to
>  		match reset signal names with reset specifiers.
>  
> += GPIO Reset consumers =
> +
> +For the common case of reset lines controlled by GPIOs, the GPIO binding
> +documented in devicetree/bindings/gpio/gpio.txt should be used:
> +
> +Required properties:
> +reset-gpios or		Reset GPIO using standard GPIO bindings,
> +<name>-reset-gpios:	optionally named to specify the reset line
> +
> +Optional properties:
> +reset-boot-asserted or		Boolean. If set, the corresponding reset is
> +<name>-reset-boot-asserted:	initially asserted and should be kept that way
> +				until released by the driver.
> +

I still feel like we should really treat gpios like just another reset
controller, ie. using the resets property.

I understand that you chose this pattern to be pretty much compatible
with what have been done so fare, bu I don't see how to fulfill that
goal completely, since most of the devices are actually using
reset-gpios, but some are using other names too (including
reset-gpio).

So if we can't be fully backward compatible, I don't see the benefit
of being inconsistent with how reset controllers are used in general,
and more globally on how gpios are tied for regulators for example.

That would also make reset-gpio a regular reset driver, instead of
adding logic to the core itself to handle this special case.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v4 1/2] Documentation: Add GPIO reset binding to reset binding documentation

[Philipp Zabel]

Hi Maxime,

Am Donnerstag, den 17.04.2014, 13:58 +0200 schrieb Maxime Ripard:
> I still feel like we should really treat gpios like just another reset
> controller, ie. using the resets property.

I now feel like we really shouldn't. If we do anything but use the
generic gpio bindings for reset gpios, we might force every OS or
bootloader using the device tree to implement some kind of reset
controller framework, even for hardware that only has gpio resets.

> I understand that you chose this pattern to be pretty much compatible
> with what have been done so fare, bu I don't see how to fulfill that
> goal completely, since most of the devices are actually using
> reset-gpios, but some are using other names too (including
> reset-gpio).

Yes, that is a problem that applies to all gpios, not only to reset
gpios, though.

> So if we can't be fully backward compatible, I don't see the benefit
> of being inconsistent with how reset controllers are used in general,

See above.

> and more globally on how gpios are tied for regulators for example.

Reset controllers are rather more similar to gpio or interrupt
controllers. Those also have a number of entities per device node,
as opposed to regulators.

> That would also make reset-gpio a regular reset driver, instead of
> adding logic to the core itself to handle this special case.

Which is what I initially tried and moved away from.

regards
Philipp

Re: [PATCH v4 1/2] Documentation: Add GPIO reset binding to reset binding documentation

[Maxime Ripard]

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

On Wed, Apr 30, 2014 at 10:54:22AM +0200, Philipp Zabel wrote:
> Hi Maxime,
> 
> Am Donnerstag, den 17.04.2014, 13:58 +0200 schrieb Maxime Ripard:
> > I still feel like we should really treat gpios like just another reset
> > controller, ie. using the resets property.
> 
> I now feel like we really shouldn't. If we do anything but use the
> generic gpio bindings for reset gpios, we might force every OS or
> bootloader using the device tree to implement some kind of reset
> controller framework, even for hardware that only has gpio resets.

Well, we pretty much introduced that requirement already whenever we
got this framework in the first place.

You can now expect pretty much anything, even a CPU or a timer to have
a reset property, so you have to handle them anyway.

So I'd be much more in favor of consistency both with other frameworks
and within the reset framework itself.

> > I understand that you chose this pattern to be pretty much compatible
> > with what have been done so fare, bu I don't see how to fulfill that
> > goal completely, since most of the devices are actually using
> > reset-gpios, but some are using other names too (including
> > reset-gpio).
> 
> Yes, that is a problem that applies to all gpios, not only to reset
> gpios, though.

Indeed, but you're the only one I can think of that have tried to
factor the gpio handling in the framework after the facts. All the
other either have no standards and let the driver deal with it, or
have explicitly asked for a given property name.

> > So if we can't be fully backward compatible, I don't see the benefit
> > of being inconsistent with how reset controllers are used in general,
> 
> See above.
> 
> > and more globally on how gpios are tied for regulators for example.
> 
> Reset controllers are rather more similar to gpio or interrupt
> controllers. Those also have a number of entities per device node,
> as opposed to regulators.

Well, that depends on the bindings actually. The fact that the reset
controller can handle several reset lines is dependent of the
controller itself. And I believe we can say pretty much the same about
the regulators.

> > That would also make reset-gpio a regular reset driver, instead of
> > adding logic to the core itself to handle this special case.
> 
> Which is what I initially tried and moved away from.

Yeah, I know.

Overall, I'd very much like the DT maintainers to step in on
this. Especially after the discussion we had again yesterday about the
stable DT bindings things.

Stable DT bindings only works if the DT maintainers can do some
arbitration in this case.

Mark? Ping?

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]