lkml.org 
[lkml]   [2021]   [Oct]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 2/2] reset: Add GPIO-based reset controller
From
Date


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@seco.com>
> ---
>
> 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@gmail.com>
> +S: Supported
> +F: drivers/reset/reset-gpio.c
> +
> GENERIC HDLC (WAN) DRIVERS
> M: Krzysztof Halasa <khc@pm.waw.pl>
> 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@seco.com>
> + *
> + * 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

\
 
 \ /
  Last update: 2021-10-25 20:18    [W:0.118 / U:0.612 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site