Re: [PATCH 2/2] reset: Add GPIO-based reset controller

From: Sean Anderson
Date: Mon Oct 25 2021 - 14:18:01 EST




On 10/18/21 7:49 PM, Sean Anderson wrote:
This adds a driver to control GPIO-based resets using the reset
controller API. This allows using a common interface for devices which
may have both GPIO resets and reset controllers, depending on the
configuration. It also allows for easier sharing of reset GPIOs in the
case where one GPIO is a reset for multiple devices.

There are several properties for specifying pre/post-(de)assert delays.
This device can also use a "reset done" GPIO, for cases when such a GPIO
is provided by the device being reset. This can be useful when the
datasheet does not otherwise specify reset timings, or specifies a much
longer maximum reset delay than the typical delay.

There is one queue for waiters on done GPIOs. I don't anticipate there
being a penalty from this, since there will likely only be concurrent
waiters during startup. I believe that wait_event_idle_timeout is the
correct function to use here, but there are a lot of variants of this
function, so I am not completely sure it is the best.

Signed-off-by: Sean Anderson <sean.anderson@xxxxxxxx>
---

MAINTAINERS | 5 +
drivers/reset/Kconfig | 11 ++
drivers/reset/Makefile | 1 +
drivers/reset/reset-gpio.c | 223 +++++++++++++++++++++++++++++++++++++
4 files changed, 240 insertions(+)
create mode 100644 drivers/reset/reset-gpio.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 8d118d7957d2..0a54c4dd83d9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7813,6 +7813,11 @@ F: Documentation/i2c/muxes/i2c-mux-gpio.rst
F: drivers/i2c/muxes/i2c-mux-gpio.c
F: include/linux/platform_data/i2c-mux-gpio.h
+GENERIC GPIO RESET DRIVER
+M: Sean Anderson <seanga2@xxxxxxxxx>
+S: Supported
+F: drivers/reset/reset-gpio.c
+
GENERIC HDLC (WAN) DRIVERS
M: Krzysztof Halasa <khc@xxxxxxxxx>
S: Maintained
diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index be799a5abf8a..24888005baf8 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -66,6 +66,17 @@ config RESET_BRCMSTB_RESCAL
This enables the RESCAL reset controller for SATA, PCIe0, or PCIe1 on
BCM7216.
+config RESET_GPIO
+ tristate "GPIO reset controller"
+ depends on OF
+ help
+ This enables a generic controller for resets attached via GPIOs. It
+ may be used to add GPIO resets to drivers which expect a reset
+ controller. It supports adding delays and waiting for a "done" GPIO
+ to be asserted.
+
+ If compiled as module, it will be called reset-gpio.
+
config RESET_HSDK
bool "Synopsys HSDK Reset Driver"
depends on HAS_IOMEM
diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index 21d46d8869ff..f577ec16fd93 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_RESET_BCM6345) += reset-bcm6345.o
obj-$(CONFIG_RESET_BERLIN) += reset-berlin.o
obj-$(CONFIG_RESET_BRCMSTB) += reset-brcmstb.o
obj-$(CONFIG_RESET_BRCMSTB_RESCAL) += reset-brcmstb-rescal.o
+obj-$(CONFIG_RESET_GPIO) += reset-gpio.o
obj-$(CONFIG_RESET_HSDK) += reset-hsdk.o
obj-$(CONFIG_RESET_IMX7) += reset-imx7.o
obj-$(CONFIG_RESET_INTEL_GW) += reset-intel-gw.o
diff --git a/drivers/reset/reset-gpio.c b/drivers/reset/reset-gpio.c
new file mode 100644
index 000000000000..93d3dbb150e0
--- /dev/null
+++ b/drivers/reset/reset-gpio.c
@@ -0,0 +1,223 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2021 Sean Anderson <sean.anderson@xxxxxxxx>
+ *
+ * This driver controls GPIOs used to reset device(s). It may be used for when
+ * there is a need for more complex behavior than a simple reset-gpios
+ * property. It may also be used to unify code paths between device-based and
+ * gpio-based resets.
+ */
+
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/reset-controller.h>
+#include <linux/sched.h>
+#include <linux/wait.h>
+
+/**
+ * struct reset_gpio_priv - Private data for GPIO reset driver
+ * @rc: Reset controller for this driver
+ * @done_queue: Queue to wait for changes on done GPIOs. Events occur whenever
+ * the value of any done GPIO changes. Valid only when @done is
+ * non-%NULL.
+ * @reset: Array of gpios to use when (de)asserting resets
+ * @done: Array of gpios to determine whether a reset has finished; may be
+ * %NULL
+ * @done_timeout_jiffies: Timeout when waiting for a done GPIO to be asserted, in jiffies
+ * @post_assert_delay: Time to wait after asserting a reset, in us
+ * @post_deassert_delay: Time to wait after deasserting a reset, in us
+ */
+struct reset_gpio_priv {
+ struct reset_controller_dev rc;
+ struct wait_queue_head done_queue;
+ struct gpio_descs *reset;
+ struct gpio_descs *done;
+ unsigned long done_timeout_jiffies;
+ u32 pre_assert_delay;
+ u32 post_assert_delay;
+ u32 pre_deassert_delay;
+ u32 post_deassert_delay;
+};
+
+static inline struct reset_gpio_priv
+*rc_to_reset_gpio(struct reset_controller_dev *rc)
+{
+ return container_of(rc, struct reset_gpio_priv, rc);
+}
+
+static int reset_gpio_assert(struct reset_controller_dev *rc, unsigned long id)
+{
+ struct reset_gpio_priv *priv = rc_to_reset_gpio(rc);
+
+ if (priv->pre_assert_delay)
+ fsleep(priv->pre_assert_delay);
+ gpiod_set_value_cansleep(priv->reset->desc[id], 1);
+ if (priv->post_assert_delay)
+ fsleep(priv->post_assert_delay);
+ return 0;
+}
+
+static int reset_gpio_deassert(struct reset_controller_dev *rc,
+ unsigned long id)
+{
+ int ret = 0;
+ unsigned int remaining;
+ struct reset_gpio_priv *priv = rc_to_reset_gpio(rc);
+
+ if (priv->pre_deassert_delay)
+ fsleep(priv->pre_deassert_delay);
+ gpiod_set_value_cansleep(priv->reset->desc[id], 0);
+ if (priv->post_deassert_delay)
+ fsleep(priv->post_deassert_delay);
+
+ if (!priv->done)
+ return 0;
+
+ remaining = wait_event_idle_timeout(
+ priv->done_queue,
+ (ret = gpiod_get_value_cansleep(priv->done->desc[id])),
+ priv->done_timeout_jiffies);
+ dev_dbg(rc->dev, "%s: remaining=%u\n", __func__, remaining);
+ if (ret < 0)
+ return ret;
+ if (ret)
+ return 0;
+ return -ETIMEDOUT;
+}
+
+static int reset_gpio_reset(struct reset_controller_dev *rc, unsigned long id)
+{
+ int ret = reset_gpio_assert(rc, id);
+
+ if (!ret)
+ return ret;
+
+ return reset_gpio_deassert(rc, id);
+}
+
+static int reset_gpio_status(struct reset_controller_dev *rc, unsigned long id)
+{
+ struct reset_gpio_priv *priv = rc_to_reset_gpio(rc);
+
+ return gpiod_get_value_cansleep(priv->reset->desc[id]);
+}
+
+static const struct reset_control_ops reset_gpio_ops = {
+ .reset = reset_gpio_reset,
+ .assert = reset_gpio_assert,
+ .deassert = reset_gpio_deassert,
+ .status = reset_gpio_status,
+};
+
+static irqreturn_t reset_gpio_irq(int irq, void *data)
+{
+ struct reset_gpio_priv *priv = data;
+
+ wake_up(&priv->done_queue);
+ return IRQ_HANDLED;
+}
+
+static int reset_gpio_probe(struct platform_device *pdev)
+{
+ int ret;
+ struct device *dev = &pdev->dev;
+ struct device_node *np = dev->of_node;
+ struct reset_gpio_priv *priv;
+ u32 done_timeout_us;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+ platform_set_drvdata(pdev, priv);
+
+ /* A short macro to reduce repetitive error handling */
+#define read_delay(propname, val) do { \
+ ret = of_property_read_u32(dev->of_node, (propname), &(val)); \
+ if (ret == -EINVAL) \
+ (val) = 0; \
+ else if (ret) \
+ return dev_err_probe(dev, ret, \
+ "Could not read %s\n", propname); \
+} while (0)
+
+ read_delay("pre-assert-us", priv->pre_assert_delay);
+ read_delay("post-assert-us", priv->post_assert_delay);
+ read_delay("pre-deassert-us", priv->pre_deassert_delay);
+ read_delay("post-deassert-us", priv->post_deassert_delay);
+
+ ret = of_property_read_u32(np, "done-timeout-us", &done_timeout_us);
+ if (ret == -EINVAL) {
+ if (priv->post_deassert_delay)
+ done_timeout_us = 10 * priv->post_deassert_delay;
+ else
+ done_timeout_us = 1000;
+ } else if (ret)
+ return dev_err_probe(dev, ret,
+ "Could not read done timeout\n");
+ priv->done_timeout_jiffies = usecs_to_jiffies(done_timeout_us);
+
+ priv->reset = devm_gpiod_get_array(dev, "reset", GPIOD_OUT_HIGH);
+ if (IS_ERR(priv->reset))
+ return dev_err_probe(dev, PTR_ERR(priv->reset),
+ "Could not get reset gpios\n");
+
+ priv->done = devm_gpiod_get_array_optional(dev, "done",
+ GPIOD_IN);
+ if (IS_ERR(priv->done))
+ return dev_err_probe(dev, PTR_ERR(priv->done),
+ "Could not get done gpios\n");
+ if (priv->done) {
+ int i;
+
+ if (priv->reset->ndescs != priv->done->ndescs)
+ return dev_err_probe(dev, -EINVAL,
+ "Number of reset and done gpios does not match\n");
+ init_waitqueue_head(&priv->done_queue);
+ for (i = 0; i < priv->done->ndescs; i++) {
+ ret = gpiod_to_irq(priv->done->desc[i]);
+ if (ret < 0)
+ return dev_err_probe(dev, ret,
+ "Could not convert GPIO to IRQ\n");
+
+ ret = devm_request_irq(dev, ret, reset_gpio_irq,
+ IRQF_SHARED, dev_name(dev),
+ priv);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "Could not request IRQ\n");
+ }
+ }
+
+ priv->rc.ops = &reset_gpio_ops;
+ priv->rc.owner = THIS_MODULE;
+ priv->rc.dev = dev;
+ priv->rc.of_node = np;
+ priv->rc.nr_resets = priv->reset->ndescs;
+ ret = devm_reset_controller_register(dev, &priv->rc);
+ if (!ret)
+ dev_info(dev, "probed with %u resets\n", priv->reset->ndescs);
+ return ret;
+}
+
+static const struct of_device_id reset_gpio_of_match[] = {
+ { .compatible = "gpio-reset", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, reset_gpio_of_match);
+
+static struct platform_driver reset_gpio_driver = {
+ .probe = reset_gpio_probe,
+ .driver = {
+ .name = "gpio-reset",
+ .of_match_table = of_match_ptr(reset_gpio_of_match),
+ },
+};
+module_platform_driver(reset_gpio_driver);
+
+MODULE_ALIAS("platform:gpio-reset");
+MODULE_DESCRIPTION("Generic GPIO reset driver");
+MODULE_LICENSE("GPL v2");


ping?

Philipp, should I be CCing anyone else? MAINTAINERS only lists you and vger...

--Sean