| Message ID | 1389030903-27684-1-git-send-email-p.zabel@pengutronix.de |
|---|---|
| State | Superseded, archived |
| Headers | show |
Hi Philipp, On Mon, Jan 06, 2014 at 06:55:03PM +0100, Philipp Zabel wrote: > 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 v1: > - Since (<name>-)reset-gpios is already a common pattern, I have dropped the > additional reset-gpio-names property. > - A boolean (<name>-)reset-boot-asserted is added to keep GPIOs asserted when > requesting them. > - Rebased onto Maxime Ripard's of_reset_control_get patch > --- > drivers/reset/Kconfig | 17 ++++++ > drivers/reset/Makefile | 9 ++- > drivers/reset/core.c | 29 +++++----- > drivers/reset/gpio-reset.c | 137 +++++++++++++++++++++++++++++++++++++++++++++ > drivers/reset/reset-core.h | 47 ++++++++++++++++ > 5 files changed, 224 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 c9d04f7..a6b12a9 100644 > --- a/drivers/reset/Kconfig > +++ b/drivers/reset/Kconfig > @@ -10,4 +10,21 @@ 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 > diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile > index cc29832..764bb74 100644 > --- a/drivers/reset/Makefile > +++ b/drivers/reset/Makefile > @@ -1,2 +1,9 @@ > -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 > 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..78cd023 > --- /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-initially-asserted"; I guess you meant reset-boot-asserted here, right? > + const char *gpio_con_id = "reset"; > + struct reset_controller_dev *rcdev; > + struct reset_control *rstc; > + struct gpio_desc *gpiod; > + bool asserted = false; > + char scratch[48]; > + 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_err(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; What happens if the GPIO is active low? I see no way in your binding and driver to give that information, and that would change the behaviour quite a bit. Thanks! Maxime
Hi Maxime, Am Montag, den 06.01.2014, 19:14 +0100 schrieb Maxime Ripard: > > +struct reset_control *gpio_reset_control_get(struct device *dev, const char *id) > > +{ > > + const char *assert_prop = "reset-initially-asserted"; > > I guess you meant reset-boot-asserted here, right? Yes, thank you. > > + const char *gpio_con_id = "reset"; > > + struct reset_controller_dev *rcdev; > > + struct reset_control *rstc; > > + struct gpio_desc *gpiod; > > + bool asserted = false; > > + char scratch[48]; > > + 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_err(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; > > What happens if the GPIO is active low? I see no way in your binding > and driver to give that information, and that would change the > behaviour quite a bit. I was under the (wrong) impression that gpiod_direction_output takes a logical value as gpiod_set_value does. Will fix that. regards Philipp -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Philipp, On Tue, Jan 07, 2014 at 10:52:21AM +0100, Philipp Zabel wrote: > > > + const char *gpio_con_id = "reset"; > > > + struct reset_controller_dev *rcdev; > > > + struct reset_control *rstc; > > > + struct gpio_desc *gpiod; > > > + bool asserted = false; > > > + char scratch[48]; > > > + 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_err(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; > > > > What happens if the GPIO is active low? I see no way in your binding > > and driver to give that information, and that would change the > > behaviour quite a bit. > > I was under the (wrong) impression that gpiod_direction_output takes a > logical value as gpiod_set_value does. Will fix that. I don't think gpiod_set_value does either unfortunately. Maxime
Am Dienstag, den 07.01.2014, 12:19 +0100 schrieb Maxime Ripard: > Hi Philipp, > > On Tue, Jan 07, 2014 at 10:52:21AM +0100, Philipp Zabel wrote: > > > > + const char *gpio_con_id = "reset"; > > > > + struct reset_controller_dev *rcdev; > > > > + struct reset_control *rstc; > > > > + struct gpio_desc *gpiod; > > > > + bool asserted = false; > > > > + char scratch[48]; > > > > + 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_err(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; > > > > > > What happens if the GPIO is active low? I see no way in your binding > > > and driver to give that information, and that would change the > > > behaviour quite a bit. > > > > I was under the (wrong) impression that gpiod_direction_output takes a > > logical value as gpiod_set_value does. Will fix that. > > I don't think gpiod_set_value does either unfortunately. It does, and Alexandre (added to Cc:) suggested that gpiod_direction_output probably should, too. I'll send a patch. regards Philipp -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jan 07, 2014 at 12:28:09PM +0100, Philipp Zabel wrote: > Am Dienstag, den 07.01.2014, 12:19 +0100 schrieb Maxime Ripard: > > > I was under the (wrong) impression that gpiod_direction_output takes a > > > logical value as gpiod_set_value does. Will fix that. > > > > I don't think gpiod_set_value does either unfortunately. > > It does, and Alexandre (added to Cc:) suggested that > gpiod_direction_output probably should, too. I'll send a patch. Ah, yes, I was looking at 3.12, and this has been changed in 3.13. Nevermind then. Thanks! Maxime
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";
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> --- Changes since v1: - Since (<name>-)reset-gpios is already a common pattern, I have dropped the additional reset-gpio-names property. - A boolean (<name>-)reset-boot-asserted is added to keep GPIOs asserted when requesting them. --- Documentation/devicetree/bindings/reset/reset.txt | 26 +++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-)