Messages in this thread | | | Subject | Re: [PATCH 2/2] reset: Add GPIO-based reset controller | From | Sean Anderson <> | Date | Mon, 25 Oct 2021 14:17:49 -0400 |
| |
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
| |