Messages in this thread Patch in this message | | | Subject | Re: [PATCH 2/2] reset: Add GPIO-based reset controller | From | Philipp Zabel <> | Date | Tue, 02 Nov 2021 12:26:09 +0100 |
| |
Hi Sean,
On Mon, 2021-10-25 at 14:17 -0400, Sean Anderson wrote: [...] > ping? > > Philipp, should I be CCing anyone else? MAINTAINERS only lists you and vger...
That is still correct. Maybe it is time to request a dedicated reset driver mailing list for better visibility.
I was on vacation for a bit, but thankfully Rob already pointed you in the right direction. I would like this to live in the core and continue to use the established reset-gpios bindings.
I'm not sure whether adding a full reset_controller_dev for GPIO resets is necessary. Maybe there is a good reasond for it, but I found adding a gpio_desc pointer to struct reset_control and implementing non-shared GPIO resets in the core would be trivial - I prototyped this at some point (untested, see below).
To support shared resets, the already requested gpio_desc would have to be found and used to retrieve the corresponding requested reset_control from a list. This could probably be achieved by exporting gpiod_find() from gpiolib.
----------8<---------- diff --git a/drivers/reset/core.c b/drivers/reset/core.c index 61e688882643..b82acb38deb5 100644 --- a/drivers/reset/core.c +++ b/drivers/reset/core.c @@ -7,6 +7,8 @@ #include <linux/atomic.h> #include <linux/device.h> #include <linux/err.h> +#include <linux/gpio.h> +#include <linux/gpio/consumer.h> #include <linux/export.h> #include <linux/kernel.h> #include <linux/kref.h> @@ -26,6 +28,8 @@ static LIST_HEAD(reset_lookup_list); * struct reset_control - a reset control * @rcdev: a pointer to the reset controller device * this reset control belongs to + * @gpio: a pointer to the gpio_desc controlling the + * reset line, alternative to rcdev and id. * @list: list entry for the rcdev's reset controller list * @id: ID of the reset controller in the reset * controller device @@ -41,6 +45,7 @@ static LIST_HEAD(reset_lookup_list); struct reset_control { struct reset_controller_dev *rcdev; struct list_head list; + struct gpio_desc *gpio; unsigned int id; struct kref refcnt; bool acquired; @@ -330,7 +335,8 @@ int reset_control_reset(struct reset_control *rstc) if (!rstc) return 0; - if (WARN_ON(IS_ERR(rstc))) + if (WARN_ON(IS_ERR(rstc)) || + WARN_ON(rstc->gpio)) return -EINVAL; if (reset_control_is_array(rstc)) @@ -458,7 +464,14 @@ int reset_control_assert(struct reset_control *rstc) if (atomic_dec_return(&rstc->deassert_count) != 0) return 0; + } + + if (rstc->gpio) { + gpiod_set_value(rstc->gpio, 1); + return 0; + } + if (rstc->shared) { /* * Shared reset controls allow the reset line to be in any state * after this call, so doing nothing is a valid option. @@ -551,6 +564,11 @@ int reset_control_deassert(struct reset_control *rstc) } } + if (rstc->gpio) { + gpiod_set_value(rstc->gpio, 0); + return 0; + } + /* * If the reset controller does not implement .deassert(), we assume * that it handles self-deasserting reset lines via .reset(). In that @@ -559,7 +577,7 @@ int reset_control_deassert(struct reset_control *rstc) * return -ENOTSUPP. */ if (!rstc->rcdev->ops->deassert) - return 0; + return -ENOTSUPP; return rstc->rcdev->ops->deassert(rstc->rcdev, rstc->id); } @@ -609,8 +627,10 @@ int reset_control_status(struct reset_control *rstc) if (WARN_ON(IS_ERR(rstc)) || reset_control_is_array(rstc)) return -EINVAL; - if (rstc->rcdev->ops->status) + if (rstc->rcdev && rstc->rcdev->ops->status) return rstc->rcdev->ops->status(rstc->rcdev, rstc->id); + if (rstc->gpio) + return gpiod_get_value(rstc->gpio); return -ENOTSUPP; } @@ -796,9 +816,11 @@ static void __reset_control_release(struct kref *kref) lockdep_assert_held(&reset_list_mutex); - module_put(rstc->rcdev->owner); + if (rstc->rcdev) { + module_put(rstc->rcdev->owner); - list_del(&rstc->list); + list_del(&rstc->list); + } kfree(rstc); } @@ -935,19 +957,66 @@ __reset_control_get_from_lookup(struct device *dev, const char *con_id, return rstc; } +static struct reset_control *__reset_control_get_gpio(struct device *dev, + const char *id, + bool optional, + bool acquired) +{ + struct reset_control *rstc; + struct gpio_desc *gpio; + char prop[32]; + + if (id) + snprintf(prop, sizeof(prop), "%s-reset", id); + gpio = gpiod_get_optional(dev, id ? prop : "reset", GPIOD_ASIS); + if (IS_ERR(gpio)) + return ERR_CAST(gpio); + if (!gpio) + return optional ? NULL : ERR_PTR(-ENOENT); + + rstc = kzalloc(sizeof(*rstc), GFP_KERNEL); + if (!rstc) { + gpiod_put(gpio); + return ERR_PTR(-ENOMEM); + } + + /* + * Set the reset GPIO to output, deasserted, unless the GPIO is already + * set to output, asserted. + */ + if (!(gpiod_get_direction(gpio) == GPIOF_DIR_OUT && + gpiod_get_value(gpio) == 1)) + gpiod_direction_output(gpio, 0); + + rstc->gpio = gpio; + rstc->refcnt = 1; + rstc->acquired = acquired; + rstc->shared = false; + + return rstc; +} + struct reset_control *__reset_control_get(struct device *dev, const char *id, int index, bool shared, bool optional, bool acquired) { + struct reset_control *rstc = ERR_PTR(-ENOENT); + if (WARN_ON(shared && acquired)) return ERR_PTR(-EINVAL); if (dev->of_node) - return __of_reset_control_get(dev->of_node, id, index, shared, + rstc = __of_reset_control_get(dev->of_node, id, index, shared, optional, acquired); + if ((rstc == NULL || PTR_ERR(rstc) == -ENOENT) && index == 0 && + shared == 0) + rstc = __reset_control_get_gpio(dev, id, optional, acquired); + + if (rstc == NULL || PTR_ERR(rstc) == -ENOENT) + rstc = __reset_control_get_from_lookup(dev, id, shared, + optional, acquired); - return __reset_control_get_from_lookup(dev, id, shared, optional, - acquired); + return rstc; } EXPORT_SYMBOL_GPL(__reset_control_get); ---------->8---------- regards Philipp
| |