[PATCH v7 0/4] reset: APIs to manage a list of resets

[PATCH v7 0/4] reset: APIs to manage a list of resets

[Philipp Zabel]

A set of patches to allow consumers to get and de/assert or trigger
a number of resets at the same time. A patch on top of Vivek's original
API extension is added to hide the reset_control_array behind a struct
reset_control so that the consumer doesn't have to care about the difference
between a singular reset control and a reset control controlling an array
of resets, except when requesting it.

This series also contains reset controls patches for dwc3-of-simple
and tegra pmc drivers.
A small patch is added in this series to correctly re-order the
resource handling in dwc3_of_simple_remove().

The series is tested on torvald's master branch the device tree
patches to enable usb on db820c.

Changes since v6:
 - Removed leftover reset_control_array_put stub.

Changes since v5:
 - Fixed devm/of_reset_control_array_get stub return values in the
   "reset: hide reset control arrays behind struct reset_control" patch.
 - Merged "reset: hide reset control arrays behind struct reset_control" patch
   into "reset: Add APIs to manage array of resets" patch, to avoid adding
   new API functions in one patch that are removed in the other.
 - Updated commit message of "soc/tegra: pmc: Use the new reset APIs to manage
   reset controllers" patch.
 - Dropped already merged "reset: use kref for reference counting" patch.

Changes since v4:
 - Added a patch to hide reset control arrays behind struct reset_control
   and adapted the consumer patches. This could be merged with the reset
   array API patch if we think this is a good idea.

Changes since v3:
 - Squashed of_reset_control_get_count() patch in the second patch that
   adds the reset control array APIs.
 - The error path after getting count through of_reset_control_get_count()
   now returns NULL pointer in case when 'optional' flag is true.
 - Added code in reset_control_array_assert() to deassert the
   already asserted resets in the error case.
 - Using of_reset_control_array_get_optional_exclusive() in dwc3 patch
   to request the reset control array.
 - Added a patch to fix the order in which resources are handled in
   dwc3_of_simple_remove() path.
 - Added tegra_powergate->reset to take care of single reset control
   passed from the client drivers.

Changes since v2:
 - Addressed comments to make APIs inline with gpiod API.
 - Moved number of reset controls in 'struct reset_control_array'
   so that the footprint is reduced.
 - of_reset_control_array_get() and devm_reset_control_array_get()
   now return pointer to the newly created reset control array.
 - Added comments to mention that the reset control array APIs don't
   guarantee any particular order when handling the reset controls.
 - Dropped 'name' from reset_control_array' since the interface is meant
   for a bunch of anonymous resets that can all be asserted or deasserted
   in arbitrary order.
 - Fixed returns for APIs reported by kbuild.
 - Fixed 'for' clause guards reported by kbuild.

Changes since v1:
 - Addressed comment for error handling in of_reset_control_get_count()
 - Added patch to manage reset controller array.
 - Rebased dwc3-of-simple changes based on the new set of APIs
   for reset control array.
 - Added a patch for soc/tegra/pmc driver to use the new set of
   reset control array APIs.

Vivek Gautam (4):
  reset: Add APIs to manage array of resets
  usb: dwc3: of-simple: Re-order resource handling in remove
  usb: dwc3: of-simple: Add support to get resets for the device
  soc/tegra: pmc: Use the new reset APIs to manage reset controllers

 drivers/reset/core.c              | 211 +++++++++++++++++++++++++++++++++++++-
 drivers/soc/tegra/pmc.c           |  82 ++++-----------
 drivers/usb/dwc3/dwc3-of-simple.c |  29 +++++-
 include/linux/reset.h             |  68 ++++++++++++
 4 files changed, 324 insertions(+), 66 deletions(-)

-- 
2.11.0

[PATCH v7 1/4] reset: Add APIs to manage array of resets

[Philipp Zabel]

From: Vivek Gautam <vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>

Many devices may want to request a bunch of resets and control them. So
it's better to manage them as an array. Add APIs to _get() an array of
reset_control, reusing the _assert(), _deassert(), and _reset() APIs for
single reset controls. Since reset controls already may control multiple
reset lines with a single hardware bit, from the user perspective, reset
control arrays are not at all different from single reset controls.
Note that these APIs don't guarantee that the reset lines managed in the
array are handled in any particular order.

Cc: Felipe Balbi <balbi-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Vivek Gautam <vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
[p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org: changed API to hide reset control arrays behind
 struct reset_control]
Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
Changes since v6:
 - Removed leftover reset_control_array_put stub.
---
 drivers/reset/core.c  | 211 +++++++++++++++++++++++++++++++++++++++++++++++++-
 include/linux/reset.h |  68 ++++++++++++++++
 2 files changed, 278 insertions(+), 1 deletion(-)

diff --git a/drivers/reset/core.c b/drivers/reset/core.c
index 0090784ff4105..c8fb4426b218a 100644
--- a/drivers/reset/core.c
+++ b/drivers/reset/core.c
@@ -43,11 +43,24 @@ struct reset_control {
 	unsigned int id;
 	struct kref refcnt;
 	bool shared;
+	bool array;
 	atomic_t deassert_count;
 	atomic_t triggered_count;
 };
 
 /**
+ * struct reset_control_array - an array of reset controls
+ * @base: reset control for compatibility with reset control API functions
+ * @num_rstcs: number of reset controls
+ * @rstc: array of reset controls
+ */
+struct reset_control_array {
+	struct reset_control base;
+	unsigned int num_rstcs;
+	struct reset_control *rstc[];
+};
+
+/**
  * 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
@@ -135,6 +148,65 @@ int devm_reset_controller_register(struct device *dev,
 }
 EXPORT_SYMBOL_GPL(devm_reset_controller_register);
 
+static inline struct reset_control_array *
+rstc_to_array(struct reset_control *rstc) {
+	return container_of(rstc, struct reset_control_array, base);
+}
+
+static int reset_control_array_reset(struct reset_control_array *resets)
+{
+	int ret, i;
+
+	for (i = 0; i < resets->num_rstcs; i++) {
+		ret = reset_control_reset(resets->rstc[i]);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int reset_control_array_assert(struct reset_control_array *resets)
+{
+	int ret, i;
+
+	for (i = 0; i < resets->num_rstcs; i++) {
+		ret = reset_control_assert(resets->rstc[i]);
+		if (ret)
+			goto err;
+	}
+
+	return 0;
+
+err:
+	while (i--)
+		reset_control_deassert(resets->rstc[i]);
+	return ret;
+}
+
+static int reset_control_array_deassert(struct reset_control_array *resets)
+{
+	int ret, i;
+
+	for (i = 0; i < resets->num_rstcs; i++) {
+		ret = reset_control_deassert(resets->rstc[i]);
+		if (ret)
+			goto err;
+	}
+
+	return 0;
+
+err:
+	while (i--)
+		reset_control_assert(resets->rstc[i]);
+	return ret;
+}
+
+static inline bool reset_control_is_array(struct reset_control *rstc)
+{
+	return rstc->array;
+}
+
 /**
  * reset_control_reset - reset the controlled device
  * @rstc: reset controller
@@ -158,6 +230,9 @@ int reset_control_reset(struct reset_control *rstc)
 	if (WARN_ON(IS_ERR(rstc)))
 		return -EINVAL;
 
+	if (reset_control_is_array(rstc))
+		return reset_control_array_reset(rstc_to_array(rstc));
+
 	if (!rstc->rcdev->ops->reset)
 		return -ENOTSUPP;
 
@@ -202,6 +277,9 @@ int reset_control_assert(struct reset_control *rstc)
 	if (WARN_ON(IS_ERR(rstc)))
 		return -EINVAL;
 
+	if (reset_control_is_array(rstc))
+		return reset_control_array_assert(rstc_to_array(rstc));
+
 	if (!rstc->rcdev->ops->assert)
 		return -ENOTSUPP;
 
@@ -240,6 +318,9 @@ int reset_control_deassert(struct reset_control *rstc)
 	if (WARN_ON(IS_ERR(rstc)))
 		return -EINVAL;
 
+	if (reset_control_is_array(rstc))
+		return reset_control_array_deassert(rstc_to_array(rstc));
+
 	if (!rstc->rcdev->ops->deassert)
 		return -ENOTSUPP;
 
@@ -266,7 +347,7 @@ int reset_control_status(struct reset_control *rstc)
 	if (!rstc)
 		return 0;
 
-	if (WARN_ON(IS_ERR(rstc)))
+	if (WARN_ON(IS_ERR(rstc)) || reset_control_is_array(rstc))
 		return -EINVAL;
 
 	if (rstc->rcdev->ops->status)
@@ -404,6 +485,16 @@ struct reset_control *__reset_control_get(struct device *dev, const char *id,
 }
 EXPORT_SYMBOL_GPL(__reset_control_get);
 
+static void reset_control_array_put(struct reset_control_array *resets)
+{
+	int i;
+
+	mutex_lock(&reset_list_mutex);
+	for (i = 0; i < resets->num_rstcs; i++)
+		__reset_control_put_internal(resets->rstc[i]);
+	mutex_unlock(&reset_list_mutex);
+}
+
 /**
  * reset_control_put - free the reset controller
  * @rstc: reset controller
@@ -413,6 +504,11 @@ void reset_control_put(struct reset_control *rstc)
 	if (IS_ERR_OR_NULL(rstc))
 		return;
 
+	if (reset_control_is_array(rstc)) {
+		reset_control_array_put(rstc_to_array(rstc));
+		return;
+	}
+
 	mutex_lock(&reset_list_mutex);
 	__reset_control_put_internal(rstc);
 	mutex_unlock(&reset_list_mutex);
@@ -472,3 +568,116 @@ int device_reset(struct device *dev)
 	return ret;
 }
 EXPORT_SYMBOL_GPL(device_reset);
+
+/**
+ * APIs to manage an array of reset controls.
+ */
+/**
+ * of_reset_control_get_count - Count number of resets available with a device
+ *
+ * @node: device node that contains 'resets'.
+ *
+ * Returns positive reset count on success, or error number on failure and
+ * on count being zero.
+ */
+static int of_reset_control_get_count(struct device_node *node)
+{
+	int count;
+
+	if (!node)
+		return -EINVAL;
+
+	count = of_count_phandle_with_args(node, "resets", "#reset-cells");
+	if (count == 0)
+		count = -ENOENT;
+
+	return count;
+}
+
+/**
+ * of_reset_control_array_get - Get a list of reset controls using
+ *				device node.
+ *
+ * @np: device node for the device that requests the reset controls array
+ * @shared: whether reset controls are shared or not
+ * @optional: whether it is optional to get the reset controls
+ *
+ * Returns pointer to allocated reset_control_array on success or
+ * error on failure
+ */
+struct reset_control *
+of_reset_control_array_get(struct device_node *np, bool shared, bool optional)
+{
+	struct reset_control_array *resets;
+	struct reset_control *rstc;
+	int num, i;
+
+	num = of_reset_control_get_count(np);
+	if (num < 0)
+		return optional ? NULL : ERR_PTR(num);
+
+	resets = kzalloc(sizeof(*resets) + sizeof(resets->rstc[0]) * num,
+			 GFP_KERNEL);
+	if (!resets)
+		return ERR_PTR(-ENOMEM);
+
+	for (i = 0; i < num; i++) {
+		rstc = __of_reset_control_get(np, NULL, i, shared, optional);
+		if (IS_ERR(rstc))
+			goto err_rst;
+		resets->rstc[i] = rstc;
+	}
+	resets->num_rstcs = num;
+	resets->base.array = true;
+
+	return &resets->base;
+
+err_rst:
+	mutex_lock(&reset_list_mutex);
+	while (--i >= 0)
+		__reset_control_put_internal(resets->rstc[i]);
+	mutex_unlock(&reset_list_mutex);
+
+	kfree(resets);
+
+	return rstc;
+}
+EXPORT_SYMBOL_GPL(of_reset_control_array_get);
+
+/**
+ * devm_reset_control_array_get - Resource managed reset control array get
+ *
+ * @dev: device that requests the list of reset controls
+ * @shared: whether reset controls are shared or not
+ * @optional: whether it is optional to get the reset controls
+ *
+ * The reset control array APIs are intended for a list of resets
+ * that just have to be asserted or deasserted, without any
+ * requirements on the order.
+ *
+ * Returns pointer to allocated reset_control_array on success or
+ * error on failure
+ */
+struct reset_control *
+devm_reset_control_array_get(struct device *dev, bool shared, bool optional)
+{
+	struct reset_control **devres;
+	struct reset_control *rstc;
+
+	devres = devres_alloc(devm_reset_control_release, sizeof(*devres),
+			      GFP_KERNEL);
+	if (!devres)
+		return ERR_PTR(-ENOMEM);
+
+	rstc = of_reset_control_array_get(dev->of_node, shared, optional);
+	if (IS_ERR(rstc)) {
+		devres_free(devres);
+		return rstc;
+	}
+
+	*devres = rstc;
+	devres_add(dev, devres);
+
+	return rstc;
+}
+EXPORT_SYMBOL_GPL(devm_reset_control_array_get);
diff --git a/include/linux/reset.h b/include/linux/reset.h
index 13d8681210d54..56463f37f3e67 100644
--- a/include/linux/reset.h
+++ b/include/linux/reset.h
@@ -25,6 +25,11 @@ struct reset_control *__devm_reset_control_get(struct device *dev,
 
 int __must_check device_reset(struct device *dev);
 
+struct reset_control *devm_reset_control_array_get(struct device *dev,
+						   bool shared, bool optional);
+struct reset_control *of_reset_control_array_get(struct device_node *np,
+						 bool shared, bool optional);
+
 static inline int device_reset_optional(struct device *dev)
 {
 	return device_reset(dev);
@@ -89,6 +94,18 @@ static inline struct reset_control *__devm_reset_control_get(
 	return optional ? NULL : ERR_PTR(-ENOTSUPP);
 }
 
+static inline struct reset_control *
+devm_reset_control_array_get(struct device *dev, bool shared, bool optional)
+{
+	return optional ? NULL : ERR_PTR(-ENOTSUPP);
+}
+
+static inline struct reset_control *
+of_reset_control_array_get(struct device_node *np, bool shared, bool optional)
+{
+	return optional ? NULL : ERR_PTR(-ENOTSUPP);
+}
+
 #endif /* CONFIG_RESET_CONTROLLER */
 
 /**
@@ -374,4 +391,55 @@ static inline struct reset_control *devm_reset_control_get_by_index(
 {
 	return devm_reset_control_get_exclusive_by_index(dev, index);
 }
+
+/*
+ * APIs to manage a list of reset controllers
+ */
+static inline struct reset_control *
+devm_reset_control_array_get_exclusive(struct device *dev)
+{
+	return devm_reset_control_array_get(dev, false, false);
+}
+
+static inline struct reset_control *
+devm_reset_control_array_get_shared(struct device *dev)
+{
+	return devm_reset_control_array_get(dev, true, false);
+}
+
+static inline struct reset_control *
+devm_reset_control_array_get_optional_exclusive(struct device *dev)
+{
+	return devm_reset_control_array_get(dev, false, true);
+}
+
+static inline struct reset_control *
+devm_reset_control_array_get_optional_shared(struct device *dev)
+{
+	return devm_reset_control_array_get(dev, true, true);
+}
+
+static inline struct reset_control *
+of_reset_control_array_get_exclusive(struct device_node *node)
+{
+	return of_reset_control_array_get(node, false, false);
+}
+
+static inline struct reset_control *
+of_reset_control_array_get_shared(struct device_node *node)
+{
+	return of_reset_control_array_get(node, true, false);
+}
+
+static inline struct reset_control *
+of_reset_control_array_get_optional_exclusive(struct device_node *node)
+{
+	return of_reset_control_array_get(node, false, true);
+}
+
+static inline struct reset_control *
+of_reset_control_array_get_optional_shared(struct device_node *node)
+{
+	return of_reset_control_array_get(node, true, true);
+}
 #endif
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v7 1/4] reset: Add APIs to manage array of resets

[Philipp Zabel]

From: Vivek Gautam <vivek.gautam@codeaurora.org>

Many devices may want to request a bunch of resets and control them. So
it's better to manage them as an array. Add APIs to _get() an array of
reset_control, reusing the _assert(), _deassert(), and _reset() APIs for
single reset controls. Since reset controls already may control multiple
reset lines with a single hardware bit, from the user perspective, reset
control arrays are not at all different from single reset controls.
Note that these APIs don't guarantee that the reset lines managed in the
array are handled in any particular order.

Cc: Felipe Balbi <balbi@kernel.org>
Cc: Jon Hunter <jonathanh@nvidia.com>
Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
[p.zabel@pengutronix.de: changed API to hide reset control arrays behind
 struct reset_control]
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
Changes since v6:
 - Removed leftover reset_control_array_put stub.
---
 drivers/reset/core.c  | 211 +++++++++++++++++++++++++++++++++++++++++++++++++-
 include/linux/reset.h |  68 ++++++++++++++++
 2 files changed, 278 insertions(+), 1 deletion(-)

diff --git a/drivers/reset/core.c b/drivers/reset/core.c
index 0090784ff4105..c8fb4426b218a 100644
--- a/drivers/reset/core.c
+++ b/drivers/reset/core.c
@@ -43,11 +43,24 @@ struct reset_control {
 	unsigned int id;
 	struct kref refcnt;
 	bool shared;
+	bool array;
 	atomic_t deassert_count;
 	atomic_t triggered_count;
 };
 
 /**
+ * struct reset_control_array - an array of reset controls
+ * @base: reset control for compatibility with reset control API functions
+ * @num_rstcs: number of reset controls
+ * @rstc: array of reset controls
+ */
+struct reset_control_array {
+	struct reset_control base;
+	unsigned int num_rstcs;
+	struct reset_control *rstc[];
+};
+
+/**
  * 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
@@ -135,6 +148,65 @@ int devm_reset_controller_register(struct device *dev,
 }
 EXPORT_SYMBOL_GPL(devm_reset_controller_register);
 
+static inline struct reset_control_array *
+rstc_to_array(struct reset_control *rstc) {
+	return container_of(rstc, struct reset_control_array, base);
+}
+
+static int reset_control_array_reset(struct reset_control_array *resets)
+{
+	int ret, i;
+
+	for (i = 0; i < resets->num_rstcs; i++) {
+		ret = reset_control_reset(resets->rstc[i]);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int reset_control_array_assert(struct reset_control_array *resets)
+{
+	int ret, i;
+
+	for (i = 0; i < resets->num_rstcs; i++) {
+		ret = reset_control_assert(resets->rstc[i]);
+		if (ret)
+			goto err;
+	}
+
+	return 0;
+
+err:
+	while (i--)
+		reset_control_deassert(resets->rstc[i]);
+	return ret;
+}
+
+static int reset_control_array_deassert(struct reset_control_array *resets)
+{
+	int ret, i;
+
+	for (i = 0; i < resets->num_rstcs; i++) {
+		ret = reset_control_deassert(resets->rstc[i]);
+		if (ret)
+			goto err;
+	}
+
+	return 0;
+
+err:
+	while (i--)
+		reset_control_assert(resets->rstc[i]);
+	return ret;
+}
+
+static inline bool reset_control_is_array(struct reset_control *rstc)
+{
+	return rstc->array;
+}
+
 /**
  * reset_control_reset - reset the controlled device
  * @rstc: reset controller
@@ -158,6 +230,9 @@ int reset_control_reset(struct reset_control *rstc)
 	if (WARN_ON(IS_ERR(rstc)))
 		return -EINVAL;
 
+	if (reset_control_is_array(rstc))
+		return reset_control_array_reset(rstc_to_array(rstc));
+
 	if (!rstc->rcdev->ops->reset)
 		return -ENOTSUPP;
 
@@ -202,6 +277,9 @@ int reset_control_assert(struct reset_control *rstc)
 	if (WARN_ON(IS_ERR(rstc)))
 		return -EINVAL;
 
+	if (reset_control_is_array(rstc))
+		return reset_control_array_assert(rstc_to_array(rstc));
+
 	if (!rstc->rcdev->ops->assert)
 		return -ENOTSUPP;
 
@@ -240,6 +318,9 @@ int reset_control_deassert(struct reset_control *rstc)
 	if (WARN_ON(IS_ERR(rstc)))
 		return -EINVAL;
 
+	if (reset_control_is_array(rstc))
+		return reset_control_array_deassert(rstc_to_array(rstc));
+
 	if (!rstc->rcdev->ops->deassert)
 		return -ENOTSUPP;
 
@@ -266,7 +347,7 @@ int reset_control_status(struct reset_control *rstc)
 	if (!rstc)
 		return 0;
 
-	if (WARN_ON(IS_ERR(rstc)))
+	if (WARN_ON(IS_ERR(rstc)) || reset_control_is_array(rstc))
 		return -EINVAL;
 
 	if (rstc->rcdev->ops->status)
@@ -404,6 +485,16 @@ struct reset_control *__reset_control_get(struct device *dev, const char *id,
 }
 EXPORT_SYMBOL_GPL(__reset_control_get);
 
+static void reset_control_array_put(struct reset_control_array *resets)
+{
+	int i;
+
+	mutex_lock(&reset_list_mutex);
+	for (i = 0; i < resets->num_rstcs; i++)
+		__reset_control_put_internal(resets->rstc[i]);
+	mutex_unlock(&reset_list_mutex);
+}
+
 /**
  * reset_control_put - free the reset controller
  * @rstc: reset controller
@@ -413,6 +504,11 @@ void reset_control_put(struct reset_control *rstc)
 	if (IS_ERR_OR_NULL(rstc))
 		return;
 
+	if (reset_control_is_array(rstc)) {
+		reset_control_array_put(rstc_to_array(rstc));
+		return;
+	}
+
 	mutex_lock(&reset_list_mutex);
 	__reset_control_put_internal(rstc);
 	mutex_unlock(&reset_list_mutex);
@@ -472,3 +568,116 @@ int device_reset(struct device *dev)
 	return ret;
 }
 EXPORT_SYMBOL_GPL(device_reset);
+
+/**
+ * APIs to manage an array of reset controls.
+ */
+/**
+ * of_reset_control_get_count - Count number of resets available with a device
+ *
+ * @node: device node that contains 'resets'.
+ *
+ * Returns positive reset count on success, or error number on failure and
+ * on count being zero.
+ */
+static int of_reset_control_get_count(struct device_node *node)
+{
+	int count;
+
+	if (!node)
+		return -EINVAL;
+
+	count = of_count_phandle_with_args(node, "resets", "#reset-cells");
+	if (count == 0)
+		count = -ENOENT;
+
+	return count;
+}
+
+/**
+ * of_reset_control_array_get - Get a list of reset controls using
+ *				device node.
+ *
+ * @np: device node for the device that requests the reset controls array
+ * @shared: whether reset controls are shared or not
+ * @optional: whether it is optional to get the reset controls
+ *
+ * Returns pointer to allocated reset_control_array on success or
+ * error on failure
+ */
+struct reset_control *
+of_reset_control_array_get(struct device_node *np, bool shared, bool optional)
+{
+	struct reset_control_array *resets;
+	struct reset_control *rstc;
+	int num, i;
+
+	num = of_reset_control_get_count(np);
+	if (num < 0)
+		return optional ? NULL : ERR_PTR(num);
+
+	resets = kzalloc(sizeof(*resets) + sizeof(resets->rstc[0]) * num,
+			 GFP_KERNEL);
+	if (!resets)
+		return ERR_PTR(-ENOMEM);
+
+	for (i = 0; i < num; i++) {
+		rstc = __of_reset_control_get(np, NULL, i, shared, optional);
+		if (IS_ERR(rstc))
+			goto err_rst;
+		resets->rstc[i] = rstc;
+	}
+	resets->num_rstcs = num;
+	resets->base.array = true;
+
+	return &resets->base;
+
+err_rst:
+	mutex_lock(&reset_list_mutex);
+	while (--i >= 0)
+		__reset_control_put_internal(resets->rstc[i]);
+	mutex_unlock(&reset_list_mutex);
+
+	kfree(resets);
+
+	return rstc;
+}
+EXPORT_SYMBOL_GPL(of_reset_control_array_get);
+
+/**
+ * devm_reset_control_array_get - Resource managed reset control array get
+ *
+ * @dev: device that requests the list of reset controls
+ * @shared: whether reset controls are shared or not
+ * @optional: whether it is optional to get the reset controls
+ *
+ * The reset control array APIs are intended for a list of resets
+ * that just have to be asserted or deasserted, without any
+ * requirements on the order.
+ *
+ * Returns pointer to allocated reset_control_array on success or
+ * error on failure
+ */
+struct reset_control *
+devm_reset_control_array_get(struct device *dev, bool shared, bool optional)
+{
+	struct reset_control **devres;
+	struct reset_control *rstc;
+
+	devres = devres_alloc(devm_reset_control_release, sizeof(*devres),
+			      GFP_KERNEL);
+	if (!devres)
+		return ERR_PTR(-ENOMEM);
+
+	rstc = of_reset_control_array_get(dev->of_node, shared, optional);
+	if (IS_ERR(rstc)) {
+		devres_free(devres);
+		return rstc;
+	}
+
+	*devres = rstc;
+	devres_add(dev, devres);
+
+	return rstc;
+}
+EXPORT_SYMBOL_GPL(devm_reset_control_array_get);
diff --git a/include/linux/reset.h b/include/linux/reset.h
index 13d8681210d54..56463f37f3e67 100644
--- a/include/linux/reset.h
+++ b/include/linux/reset.h
@@ -25,6 +25,11 @@ struct reset_control *__devm_reset_control_get(struct device *dev,
 
 int __must_check device_reset(struct device *dev);
 
+struct reset_control *devm_reset_control_array_get(struct device *dev,
+						   bool shared, bool optional);
+struct reset_control *of_reset_control_array_get(struct device_node *np,
+						 bool shared, bool optional);
+
 static inline int device_reset_optional(struct device *dev)
 {
 	return device_reset(dev);
@@ -89,6 +94,18 @@ static inline struct reset_control *__devm_reset_control_get(
 	return optional ? NULL : ERR_PTR(-ENOTSUPP);
 }
 
+static inline struct reset_control *
+devm_reset_control_array_get(struct device *dev, bool shared, bool optional)
+{
+	return optional ? NULL : ERR_PTR(-ENOTSUPP);
+}
+
+static inline struct reset_control *
+of_reset_control_array_get(struct device_node *np, bool shared, bool optional)
+{
+	return optional ? NULL : ERR_PTR(-ENOTSUPP);
+}
+
 #endif /* CONFIG_RESET_CONTROLLER */
 
 /**
@@ -374,4 +391,55 @@ static inline struct reset_control *devm_reset_control_get_by_index(
 {
 	return devm_reset_control_get_exclusive_by_index(dev, index);
 }
+
+/*
+ * APIs to manage a list of reset controllers
+ */
+static inline struct reset_control *
+devm_reset_control_array_get_exclusive(struct device *dev)
+{
+	return devm_reset_control_array_get(dev, false, false);
+}
+
+static inline struct reset_control *
+devm_reset_control_array_get_shared(struct device *dev)
+{
+	return devm_reset_control_array_get(dev, true, false);
+}
+
+static inline struct reset_control *
+devm_reset_control_array_get_optional_exclusive(struct device *dev)
+{
+	return devm_reset_control_array_get(dev, false, true);
+}
+
+static inline struct reset_control *
+devm_reset_control_array_get_optional_shared(struct device *dev)
+{
+	return devm_reset_control_array_get(dev, true, true);
+}
+
+static inline struct reset_control *
+of_reset_control_array_get_exclusive(struct device_node *node)
+{
+	return of_reset_control_array_get(node, false, false);
+}
+
+static inline struct reset_control *
+of_reset_control_array_get_shared(struct device_node *node)
+{
+	return of_reset_control_array_get(node, true, false);
+}
+
+static inline struct reset_control *
+of_reset_control_array_get_optional_exclusive(struct device_node *node)
+{
+	return of_reset_control_array_get(node, false, true);
+}
+
+static inline struct reset_control *
+of_reset_control_array_get_optional_shared(struct device_node *node)
+{
+	return of_reset_control_array_get(node, true, true);
+}
 #endif
-- 
2.11.0

[PATCH v7 2/4] usb: dwc3: of-simple: Re-order resource handling in remove

[Philipp Zabel]

From: Vivek Gautam <vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>

Move clock handling after of_platform_depopulate to achieve
a sequence that is reverse of the probe sequence.

Cc: Felipe Balbi <balbi-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Signed-off-by: Vivek Gautam <vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
---
No changes since v6.
---
 drivers/usb/dwc3/dwc3-of-simple.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-of-simple.c b/drivers/usb/dwc3/dwc3-of-simple.c
index fe414e7a9c78c..a9bac09d3750d 100644
--- a/drivers/usb/dwc3/dwc3-of-simple.c
+++ b/drivers/usb/dwc3/dwc3-of-simple.c
@@ -123,13 +123,13 @@ static int dwc3_of_simple_remove(struct platform_device *pdev)
 	struct device		*dev = &pdev->dev;
 	int			i;
 
+	of_platform_depopulate(dev);
+
 	for (i = 0; i < simple->num_clocks; i++) {
 		clk_disable_unprepare(simple->clks[i]);
 		clk_put(simple->clks[i]);
 	}
 
-	of_platform_depopulate(dev);

[PATCH v7 2/4] usb: dwc3: of-simple: Re-order resource handling in remove

[Philipp Zabel]

From: Vivek Gautam <vivek.gautam@codeaurora.org>

Move clock handling after of_platform_depopulate to achieve
a sequence that is reverse of the probe sequence.

Cc: Felipe Balbi <balbi@kernel.org>
Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
---
No changes since v6.
---
 drivers/usb/dwc3/dwc3-of-simple.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-of-simple.c b/drivers/usb/dwc3/dwc3-of-simple.c
index fe414e7a9c78c..a9bac09d3750d 100644
--- a/drivers/usb/dwc3/dwc3-of-simple.c
+++ b/drivers/usb/dwc3/dwc3-of-simple.c
@@ -123,13 +123,13 @@ static int dwc3_of_simple_remove(struct platform_device *pdev)
 	struct device		*dev = &pdev->dev;
 	int			i;
 
+	of_platform_depopulate(dev);
+
 	for (i = 0; i < simple->num_clocks; i++) {
 		clk_disable_unprepare(simple->clks[i]);
 		clk_put(simple->clks[i]);
 	}
 
-	of_platform_depopulate(dev);
-
 	pm_runtime_put_sync(dev);
 	pm_runtime_disable(dev);
 
-- 
2.11.0

[PATCH v7 3/4] usb: dwc3: of-simple: Add support to get resets for the device

[Philipp Zabel]

From: Vivek Gautam <vivek.gautam@codeaurora.org>

Add support to get a list of resets available for the device.
These resets must be kept de-asserted until the device is
in use.

Cc: Felipe Balbi <balbi@kernel.org>
Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
[p.zabel@pengutronix.de: switch to hidden reset control array]
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
No changes since v6.
---
 drivers/usb/dwc3/dwc3-of-simple.c | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-of-simple.c b/drivers/usb/dwc3/dwc3-of-simple.c
index a9bac09d3750d..23d2221bde9df 100644
--- a/drivers/usb/dwc3/dwc3-of-simple.c
+++ b/drivers/usb/dwc3/dwc3-of-simple.c
@@ -29,11 +29,13 @@
 #include <linux/of.h>
 #include <linux/of_platform.h>
 #include <linux/pm_runtime.h>
+#include <linux/reset.h>
 
 struct dwc3_of_simple {
 	struct device		*dev;
 	struct clk		**clks;
 	int			num_clocks;
+	struct reset_control	*resets;
 };
 
 static int dwc3_of_simple_clk_init(struct dwc3_of_simple *simple, int count)
@@ -96,9 +98,20 @@ static int dwc3_of_simple_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, simple);
 	simple->dev = dev;
 
+	simple->resets = of_reset_control_array_get_optional_exclusive(np);
+	if (IS_ERR(simple->resets)) {
+		ret = PTR_ERR(simple->resets);
+		dev_err(dev, "failed to get device resets, err=%d\n", ret);
+		return ret;
+	}
+
+	ret = reset_control_deassert(simple->resets);
+	if (ret)
+		goto err_resetc_put;
+
 	ret = dwc3_of_simple_clk_init(simple, of_clk_get_parent_count(np));
 	if (ret)
-		return ret;
+		goto err_resetc_assert;
 
 	ret = of_platform_populate(np, NULL, NULL, dev);
 	if (ret) {
@@ -107,7 +120,7 @@ static int dwc3_of_simple_probe(struct platform_device *pdev)
 			clk_put(simple->clks[i]);
 		}
 
-		return ret;
+		goto err_resetc_assert;
 	}
 
 	pm_runtime_set_active(dev);
@@ -115,6 +128,13 @@ static int dwc3_of_simple_probe(struct platform_device *pdev)
 	pm_runtime_get_sync(dev);
 
 	return 0;
+
+err_resetc_assert:
+	reset_control_assert(simple->resets);
+
+err_resetc_put:
+	reset_control_put(simple->resets);
+	return ret;
 }
 
 static int dwc3_of_simple_remove(struct platform_device *pdev)
@@ -130,6 +150,9 @@ static int dwc3_of_simple_remove(struct platform_device *pdev)
 		clk_put(simple->clks[i]);
 	}
 
+	reset_control_assert(simple->resets);
+	reset_control_put(simple->resets);
+
 	pm_runtime_put_sync(dev);
 	pm_runtime_disable(dev);
 
-- 
2.11.0

[PATCH v7 4/4] soc/tegra: pmc: Use the new reset APIs to manage reset controllers

[Philipp Zabel]

From: Vivek Gautam <vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>

Make use of of_reset_control_array_get_exclusive() to manage
an array of reset controllers available with the device.

Cc: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Cc: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Vivek Gautam <vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
[p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org: switch to hidden reset control array]
Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
No changes since v6.
---
 drivers/soc/tegra/pmc.c | 82 ++++++++++++-------------------------------------
 1 file changed, 20 insertions(+), 62 deletions(-)

diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index e233dd5dcab3d..749b218147a19 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -124,8 +124,7 @@ struct tegra_powergate {
 	unsigned int id;
 	struct clk **clks;
 	unsigned int num_clks;
-	struct reset_control **resets;
-	unsigned int num_resets;
+	struct reset_control *reset;
 };
 
 struct tegra_io_pad_soc {
@@ -348,32 +347,14 @@ static int tegra_powergate_enable_clocks(struct tegra_powergate *pg)
 	return err;
 }
 
-static int tegra_powergate_reset_assert(struct tegra_powergate *pg)
+static inline int tegra_powergate_reset_assert(struct tegra_powergate *pg)
 {
-	unsigned int i;
-	int err;
-
-	for (i = 0; i < pg->num_resets; i++) {
-		err = reset_control_assert(pg->resets[i]);
-		if (err)
-			return err;
-	}
-
-	return 0;
+	return reset_control_assert(pg->reset);
 }
 
-static int tegra_powergate_reset_deassert(struct tegra_powergate *pg)
+static inline int tegra_powergate_reset_deassert(struct tegra_powergate *pg)
 {
-	unsigned int i;
-	int err;
-
-	for (i = 0; i < pg->num_resets; i++) {
-		err = reset_control_deassert(pg->resets[i]);
-		if (err)
-			return err;
-	}
-
-	return 0;
+	return reset_control_deassert(pg->reset);
 }
 
 static int tegra_powergate_power_up(struct tegra_powergate *pg,
@@ -566,8 +547,7 @@ int tegra_powergate_sequence_power_up(unsigned int id, struct clk *clk,
 	pg.id = id;
 	pg.clks = &clk;
 	pg.num_clks = 1;
-	pg.resets = &rst;
-	pg.num_resets = 1;
+	pg.reset = IS_ERR(rst) ? NULL : rst;
 
 	err = tegra_powergate_power_up(&pg, false);
 	if (err)
@@ -755,45 +735,26 @@ static int tegra_powergate_of_get_clks(struct tegra_powergate *pg,
 static int tegra_powergate_of_get_resets(struct tegra_powergate *pg,
 					 struct device_node *np, bool off)
 {
-	struct reset_control *rst;
-	unsigned int i, count;
 	int err;
 
-	count = of_count_phandle_with_args(np, "resets", "#reset-cells");
-	if (count == 0)
-		return -ENODEV;
-
-	pg->resets = kcalloc(count, sizeof(rst), GFP_KERNEL);
-	if (!pg->resets)
-		return -ENOMEM;
-
-	for (i = 0; i < count; i++) {
-		pg->resets[i] = of_reset_control_get_by_index(np, i);
-		if (IS_ERR(pg->resets[i])) {
-			err = PTR_ERR(pg->resets[i]);
-			goto error;
-		}
-
-		if (off)
-			err = reset_control_assert(pg->resets[i]);
-		else
-			err = reset_control_deassert(pg->resets[i]);
-
-		if (err) {
-			reset_control_put(pg->resets[i]);
-			goto error;
-		}
+	pg->reset = of_reset_control_array_get_exclusive(np);
+	if (IS_ERR(pg->reset)) {
+		pr_err("failed to get device resets\n");
+		return PTR_ERR(pg->reset);
 	}
 
-	pg->num_resets = count;
+	if (off)
+		err = reset_control_assert(pg->reset);
+	else
+		err = reset_control_deassert(pg->reset);
 
-	return 0;
+	if (err)
+		goto put_reset;
 
-error:
-	while (i--)
-		reset_control_put(pg->resets[i]);
+	return 0;
 
-	kfree(pg->resets);
+put_reset:
+	reset_control_put(pg->reset);
 
 	return err;
 }
@@ -885,10 +846,7 @@ static void tegra_powergate_add(struct tegra_pmc *pmc, struct device_node *np)
 	pm_genpd_remove(&pg->genpd);
 
 remove_resets:
-	while (pg->num_resets--)
-		reset_control_put(pg->resets[pg->num_resets]);

[PATCH v7 4/4] soc/tegra: pmc: Use the new reset APIs to manage reset controllers

[Philipp Zabel]

From: Vivek Gautam <vivek.gautam@codeaurora.org>

Make use of of_reset_control_array_get_exclusive() to manage
an array of reset controllers available with the device.

Cc: Jon Hunter <jonathanh@nvidia.com>
Cc: Thierry Reding <treding@nvidia.com>
Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
[p.zabel@pengutronix.de: switch to hidden reset control array]
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
No changes since v6.
---
 drivers/soc/tegra/pmc.c | 82 ++++++++++++-------------------------------------
 1 file changed, 20 insertions(+), 62 deletions(-)

diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index e233dd5dcab3d..749b218147a19 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -124,8 +124,7 @@ struct tegra_powergate {
 	unsigned int id;
 	struct clk **clks;
 	unsigned int num_clks;
-	struct reset_control **resets;
-	unsigned int num_resets;
+	struct reset_control *reset;
 };
 
 struct tegra_io_pad_soc {
@@ -348,32 +347,14 @@ static int tegra_powergate_enable_clocks(struct tegra_powergate *pg)
 	return err;
 }
 
-static int tegra_powergate_reset_assert(struct tegra_powergate *pg)
+static inline int tegra_powergate_reset_assert(struct tegra_powergate *pg)
 {
-	unsigned int i;
-	int err;
-
-	for (i = 0; i < pg->num_resets; i++) {
-		err = reset_control_assert(pg->resets[i]);
-		if (err)
-			return err;
-	}
-
-	return 0;
+	return reset_control_assert(pg->reset);
 }
 
-static int tegra_powergate_reset_deassert(struct tegra_powergate *pg)
+static inline int tegra_powergate_reset_deassert(struct tegra_powergate *pg)
 {
-	unsigned int i;
-	int err;
-
-	for (i = 0; i < pg->num_resets; i++) {
-		err = reset_control_deassert(pg->resets[i]);
-		if (err)
-			return err;
-	}
-
-	return 0;
+	return reset_control_deassert(pg->reset);
 }
 
 static int tegra_powergate_power_up(struct tegra_powergate *pg,
@@ -566,8 +547,7 @@ int tegra_powergate_sequence_power_up(unsigned int id, struct clk *clk,
 	pg.id = id;
 	pg.clks = &clk;
 	pg.num_clks = 1;
-	pg.resets = &rst;
-	pg.num_resets = 1;
+	pg.reset = IS_ERR(rst) ? NULL : rst;
 
 	err = tegra_powergate_power_up(&pg, false);
 	if (err)
@@ -755,45 +735,26 @@ static int tegra_powergate_of_get_clks(struct tegra_powergate *pg,
 static int tegra_powergate_of_get_resets(struct tegra_powergate *pg,
 					 struct device_node *np, bool off)
 {
-	struct reset_control *rst;
-	unsigned int i, count;
 	int err;
 
-	count = of_count_phandle_with_args(np, "resets", "#reset-cells");
-	if (count == 0)
-		return -ENODEV;
-
-	pg->resets = kcalloc(count, sizeof(rst), GFP_KERNEL);
-	if (!pg->resets)
-		return -ENOMEM;
-
-	for (i = 0; i < count; i++) {
-		pg->resets[i] = of_reset_control_get_by_index(np, i);
-		if (IS_ERR(pg->resets[i])) {
-			err = PTR_ERR(pg->resets[i]);
-			goto error;
-		}
-
-		if (off)
-			err = reset_control_assert(pg->resets[i]);
-		else
-			err = reset_control_deassert(pg->resets[i]);
-
-		if (err) {
-			reset_control_put(pg->resets[i]);
-			goto error;
-		}
+	pg->reset = of_reset_control_array_get_exclusive(np);
+	if (IS_ERR(pg->reset)) {
+		pr_err("failed to get device resets\n");
+		return PTR_ERR(pg->reset);
 	}
 
-	pg->num_resets = count;
+	if (off)
+		err = reset_control_assert(pg->reset);
+	else
+		err = reset_control_deassert(pg->reset);
 
-	return 0;
+	if (err)
+		goto put_reset;
 
-error:
-	while (i--)
-		reset_control_put(pg->resets[i]);
+	return 0;
 
-	kfree(pg->resets);
+put_reset:
+	reset_control_put(pg->reset);
 
 	return err;
 }
@@ -885,10 +846,7 @@ static void tegra_powergate_add(struct tegra_pmc *pmc, struct device_node *np)
 	pm_genpd_remove(&pg->genpd);
 
 remove_resets:
-	while (pg->num_resets--)
-		reset_control_put(pg->resets[pg->num_resets]);
-
-	kfree(pg->resets);
+	reset_control_put(pg->reset);
 
 remove_clks:
 	while (pg->num_clks--)
-- 
2.11.0

Re: [PATCH v7 2/4] usb: dwc3: of-simple: Re-order resource handling in remove

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 419 bytes --]


Hi,

Philipp Zabel <p.zabel@pengutronix.de> writes:
> From: Vivek Gautam <vivek.gautam@codeaurora.org>
>
> Move clock handling after of_platform_depopulate to achieve
> a sequence that is reverse of the probe sequence.
>
> Cc: Felipe Balbi <balbi@kernel.org>
> Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>

does this depend on the rest of the series? Doesn't seem like it does.

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH v7 3/4] usb: dwc3: of-simple: Add support to get resets for the device

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 578 bytes --]

Philipp Zabel <p.zabel@pengutronix.de> writes:

> From: Vivek Gautam <vivek.gautam@codeaurora.org>
>
> Add support to get a list of resets available for the device.
> These resets must be kept de-asserted until the device is
> in use.
>
> Cc: Felipe Balbi <balbi@kernel.org>
> Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
> [p.zabel@pengutronix.de: switch to hidden reset control array]
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>

now this seems like it depends on patch 1/4, right? Is the reset API
change going on v4.15?

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH v7 3/4] usb: dwc3: of-simple: Add support to get resets for the device

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 578 bytes --]

Philipp Zabel <p.zabel@pengutronix.de> writes:

> From: Vivek Gautam <vivek.gautam@codeaurora.org>
>
> Add support to get a list of resets available for the device.
> These resets must be kept de-asserted until the device is
> in use.
>
> Cc: Felipe Balbi <balbi@kernel.org>
> Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
> [p.zabel@pengutronix.de: switch to hidden reset control array]
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>

now this seems like it depends on patch 1/4, right? Is the reset API
change going on v4.15?

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH v7 3/4] usb: dwc3: of-simple: Add support to get resets for the device

[Philipp Zabel]

Hi Felipe,

On Thu, 2017-10-19 at 12:38 +0300, Felipe Balbi wrote:
> Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> writes:
> 
> > From: Vivek Gautam <vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> > 
> > Add support to get a list of resets available for the device.
> > These resets must be kept de-asserted until the device is
> > in use.
> > 
> > Cc: Felipe Balbi <balbi-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > Signed-off-by: Vivek Gautam <vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> > [p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org: switch to hidden reset control array]
> > Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> 
> now this seems like it depends on patch 1/4, right? Is the reset API
> change going on v4.15?

Thank you for the reminder. Patch 1 has made it into v4.14-rc1, commit
17c82e206d2a ("reset: Add APIs to manage array of resets"). The other
patches could be picked up for v4.15 without any dependency issues.

regards
Philipp

Re: [PATCH v7 3/4] usb: dwc3: of-simple: Add support to get resets for the device

[Philipp Zabel]

Hi Felipe,

On Thu, 2017-10-19 at 12:38 +0300, Felipe Balbi wrote:
> Philipp Zabel <p.zabel@pengutronix.de> writes:
> 
> > From: Vivek Gautam <vivek.gautam@codeaurora.org>
> > 
> > Add support to get a list of resets available for the device.
> > These resets must be kept de-asserted until the device is
> > in use.
> > 
> > Cc: Felipe Balbi <balbi@kernel.org>
> > Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
> > [p.zabel@pengutronix.de: switch to hidden reset control array]
> > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> 
> now this seems like it depends on patch 1/4, right? Is the reset API
> change going on v4.15?

Thank you for the reminder. Patch 1 has made it into v4.14-rc1, commit
17c82e206d2a ("reset: Add APIs to manage array of resets"). The other
patches could be picked up for v4.15 without any dependency issues.

regards
Philipp

Re: [PATCH v7 3/4] usb: dwc3: of-simple: Add support to get resets for the device

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1032 bytes --]


Hi,

Philipp Zabel <p.zabel@pengutronix.de> writes:
> Hi Felipe,
>
> On Thu, 2017-10-19 at 12:38 +0300, Felipe Balbi wrote:
>> Philipp Zabel <p.zabel@pengutronix.de> writes:
>> 
>> > From: Vivek Gautam <vivek.gautam@codeaurora.org>
>> > 
>> > Add support to get a list of resets available for the device.
>> > These resets must be kept de-asserted until the device is
>> > in use.
>> > 
>> > Cc: Felipe Balbi <balbi@kernel.org>
>> > Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
>> > [p.zabel@pengutronix.de: switch to hidden reset control array]
>> > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
>> 
>> now this seems like it depends on patch 1/4, right? Is the reset API
>> change going on v4.15?
>
> Thank you for the reminder. Patch 1 has made it into v4.14-rc1, commit
> 17c82e206d2a ("reset: Add APIs to manage array of resets"). The other
> patches could be picked up for v4.15 without any dependency issues.

Thanks, I'll put other dwc3 patches in my tree :-)

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH v7 3/4] usb: dwc3: of-simple: Add support to get resets for the device

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1396 bytes --]


Hi,

Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> writes:
> Hi Felipe,
>
> On Thu, 2017-10-19 at 12:38 +0300, Felipe Balbi wrote:
>> Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> writes:
>> 
>> > From: Vivek Gautam <vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>> > 
>> > Add support to get a list of resets available for the device.
>> > These resets must be kept de-asserted until the device is
>> > in use.
>> > 
>> > Cc: Felipe Balbi <balbi-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>> > Signed-off-by: Vivek Gautam <vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>> > [p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org: switch to hidden reset control array]
>> > Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
>> 
>> now this seems like it depends on patch 1/4, right? Is the reset API
>> change going on v4.15?
>
> Thank you for the reminder. Patch 1 has made it into v4.14-rc1, commit
> 17c82e206d2a ("reset: Add APIs to manage array of resets"). The other
> patches could be picked up for v4.15 without any dependency issues.

patch 2 applied fine. patch 3 fails to apply hunk 2.

checking file drivers/usb/dwc3/dwc3-of-simple.c
Hunk #1 succeeded at 28 (offset -1 lines).
Hunk #2 FAILED at 98.
1 out of 5 hunks FAILED


Care to rebase on my testing/next?

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH v7 3/4] usb: dwc3: of-simple: Add support to get resets for the device

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1217 bytes --]


Hi,

Philipp Zabel <p.zabel@pengutronix.de> writes:
> Hi Felipe,
>
> On Thu, 2017-10-19 at 12:38 +0300, Felipe Balbi wrote:
>> Philipp Zabel <p.zabel@pengutronix.de> writes:
>> 
>> > From: Vivek Gautam <vivek.gautam@codeaurora.org>
>> > 
>> > Add support to get a list of resets available for the device.
>> > These resets must be kept de-asserted until the device is
>> > in use.
>> > 
>> > Cc: Felipe Balbi <balbi@kernel.org>
>> > Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
>> > [p.zabel@pengutronix.de: switch to hidden reset control array]
>> > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
>> 
>> now this seems like it depends on patch 1/4, right? Is the reset API
>> change going on v4.15?
>
> Thank you for the reminder. Patch 1 has made it into v4.14-rc1, commit
> 17c82e206d2a ("reset: Add APIs to manage array of resets"). The other
> patches could be picked up for v4.15 without any dependency issues.

patch 2 applied fine. patch 3 fails to apply hunk 2.

checking file drivers/usb/dwc3/dwc3-of-simple.c
Hunk #1 succeeded at 28 (offset -1 lines).
Hunk #2 FAILED at 98.
1 out of 5 hunks FAILED


Care to rebase on my testing/next?

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH v7 3/4] usb: dwc3: of-simple: Add support to get resets for the device

[Philipp Zabel]

On Thu, 2017-10-19 at 14:31 +0300, Felipe Balbi wrote:
> Hi,
> 
> Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> writes:
> > Hi Felipe,
> > 
> > On Thu, 2017-10-19 at 12:38 +0300, Felipe Balbi wrote:
> > > Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> writes:
> > > 
> > > > From: Vivek Gautam <vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> > > > 
> > > > Add support to get a list of resets available for the device.
> > > > These resets must be kept de-asserted until the device is
> > > > in use.
> > > > 
> > > > Cc: Felipe Balbi <balbi-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > > > Signed-off-by: Vivek Gautam <vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> > > > [p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org: switch to hidden reset control array]
> > > > Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > > 
> > > now this seems like it depends on patch 1/4, right? Is the reset API
> > > change going on v4.15?
> > 
> > Thank you for the reminder. Patch 1 has made it into v4.14-rc1, commit
> > 17c82e206d2a ("reset: Add APIs to manage array of resets"). The other
> > patches could be picked up for v4.15 without any dependency issues.
> 
> patch 2 applied fine. patch 3 fails to apply hunk 2.
> 
> checking file drivers/usb/dwc3/dwc3-of-simple.c
> Hunk #1 succeeded at 28 (offset -1 lines).
> Hunk #2 FAILED at 98.
> 1 out of 5 hunks FAILED
> 
> 
> Care to rebase on my testing/next?

Thanks, I'll resend patch 3 separately.

regards
Philipp

Re: [PATCH v7 3/4] usb: dwc3: of-simple: Add support to get resets for the device

[Philipp Zabel]

On Thu, 2017-10-19 at 14:31 +0300, Felipe Balbi wrote:
> Hi,
> 
> Philipp Zabel <p.zabel@pengutronix.de> writes:
> > Hi Felipe,
> > 
> > On Thu, 2017-10-19 at 12:38 +0300, Felipe Balbi wrote:
> > > Philipp Zabel <p.zabel@pengutronix.de> writes:
> > > 
> > > > From: Vivek Gautam <vivek.gautam@codeaurora.org>
> > > > 
> > > > Add support to get a list of resets available for the device.
> > > > These resets must be kept de-asserted until the device is
> > > > in use.
> > > > 
> > > > Cc: Felipe Balbi <balbi@kernel.org>
> > > > Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
> > > > [p.zabel@pengutronix.de: switch to hidden reset control array]
> > > > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> > > 
> > > now this seems like it depends on patch 1/4, right? Is the reset API
> > > change going on v4.15?
> > 
> > Thank you for the reminder. Patch 1 has made it into v4.14-rc1, commit
> > 17c82e206d2a ("reset: Add APIs to manage array of resets"). The other
> > patches could be picked up for v4.15 without any dependency issues.
> 
> patch 2 applied fine. patch 3 fails to apply hunk 2.
> 
> checking file drivers/usb/dwc3/dwc3-of-simple.c
> Hunk #1 succeeded at 28 (offset -1 lines).
> Hunk #2 FAILED at 98.
> 1 out of 5 hunks FAILED
> 
> 
> Care to rebase on my testing/next?

Thanks, I'll resend patch 3 separately.

regards
Philipp

Re: [PATCH v7 4/4] soc/tegra: pmc: Use the new reset APIs to manage reset controllers

[Philipp Zabel]

Hi Jon, Thierry,

On Wed, 2017-07-19 at 17:59 +0200, Philipp Zabel wrote:
> From: Vivek Gautam <vivek.gautam@codeaurora.org>
> 
> Make use of of_reset_control_array_get_exclusive() to manage
> an array of reset controllers available with the device.
> 
> Cc: Jon Hunter <jonathanh@nvidia.com>
> Cc: Thierry Reding <treding@nvidia.com>
> Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
> [p.zabel@pengutronix.de: switch to hidden reset control array]
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>

will you pick this up now that the prerequisite patch 1 is contained in
master?
Please let me know if there are any issues with this patch.

regards
Philipp

> ---
> No changes since v6.
> ---
>  drivers/soc/tegra/pmc.c | 82 ++++++++++++-------------------------------------
>  1 file changed, 20 insertions(+), 62 deletions(-)
> 
> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
> index e233dd5dcab3d..749b218147a19 100644
> --- a/drivers/soc/tegra/pmc.c
> +++ b/drivers/soc/tegra/pmc.c
> @@ -124,8 +124,7 @@ struct tegra_powergate {
>  	unsigned int id;
>  	struct clk **clks;
>  	unsigned int num_clks;
> -	struct reset_control **resets;
> -	unsigned int num_resets;
> +	struct reset_control *reset;
>  };
>  
>  struct tegra_io_pad_soc {
> @@ -348,32 +347,14 @@ static int tegra_powergate_enable_clocks(struct tegra_powergate *pg)
>  	return err;
>  }
>  
> -static int tegra_powergate_reset_assert(struct tegra_powergate *pg)
> +static inline int tegra_powergate_reset_assert(struct tegra_powergate *pg)
>  {
> -	unsigned int i;
> -	int err;
> -
> -	for (i = 0; i < pg->num_resets; i++) {
> -		err = reset_control_assert(pg->resets[i]);
> -		if (err)
> -			return err;
> -	}
> -
> -	return 0;
> +	return reset_control_assert(pg->reset);
>  }
>  
> -static int tegra_powergate_reset_deassert(struct tegra_powergate *pg)
> +static inline int tegra_powergate_reset_deassert(struct tegra_powergate *pg)
>  {
> -	unsigned int i;
> -	int err;
> -
> -	for (i = 0; i < pg->num_resets; i++) {
> -		err = reset_control_deassert(pg->resets[i]);
> -		if (err)
> -			return err;
> -	}
> -
> -	return 0;
> +	return reset_control_deassert(pg->reset);
>  }
>  
>  static int tegra_powergate_power_up(struct tegra_powergate *pg,
> @@ -566,8 +547,7 @@ int tegra_powergate_sequence_power_up(unsigned int id, struct clk *clk,
>  	pg.id = id;
>  	pg.clks = &clk;
>  	pg.num_clks = 1;
> -	pg.resets = &rst;
> -	pg.num_resets = 1;
> +	pg.reset = IS_ERR(rst) ? NULL : rst;
>  
>  	err = tegra_powergate_power_up(&pg, false);
>  	if (err)
> @@ -755,45 +735,26 @@ static int tegra_powergate_of_get_clks(struct tegra_powergate *pg,
>  static int tegra_powergate_of_get_resets(struct tegra_powergate *pg,
>  					 struct device_node *np, bool off)
>  {
> -	struct reset_control *rst;
> -	unsigned int i, count;
>  	int err;
>  
> -	count = of_count_phandle_with_args(np, "resets", "#reset-cells");
> -	if (count == 0)
> -		return -ENODEV;
> -
> -	pg->resets = kcalloc(count, sizeof(rst), GFP_KERNEL);
> -	if (!pg->resets)
> -		return -ENOMEM;
> -
> -	for (i = 0; i < count; i++) {
> -		pg->resets[i] = of_reset_control_get_by_index(np, i);
> -		if (IS_ERR(pg->resets[i])) {
> -			err = PTR_ERR(pg->resets[i]);
> -			goto error;
> -		}
> -
> -		if (off)
> -			err = reset_control_assert(pg->resets[i]);
> -		else
> -			err = reset_control_deassert(pg->resets[i]);
> -
> -		if (err) {
> -			reset_control_put(pg->resets[i]);
> -			goto error;
> -		}
> +	pg->reset = of_reset_control_array_get_exclusive(np);
> +	if (IS_ERR(pg->reset)) {
> +		pr_err("failed to get device resets\n");
> +		return PTR_ERR(pg->reset);
>  	}
>  
> -	pg->num_resets = count;
> +	if (off)
> +		err = reset_control_assert(pg->reset);
> +	else
> +		err = reset_control_deassert(pg->reset);
>  
> -	return 0;
> +	if (err)
> +		goto put_reset;
>  
> -error:
> -	while (i--)
> -		reset_control_put(pg->resets[i]);
> +	return 0;
>  
> -	kfree(pg->resets);
> +put_reset:
> +	reset_control_put(pg->reset);
>  
>  	return err;
>  }
> @@ -885,10 +846,7 @@ static void tegra_powergate_add(struct tegra_pmc *pmc, struct device_node *np)
>  	pm_genpd_remove(&pg->genpd);
>  
>  remove_resets:
> -	while (pg->num_resets--)
> -		reset_control_put(pg->resets[pg->num_resets]);
> -
> -	kfree(pg->resets);
> +	reset_control_put(pg->reset);
>  
>  remove_clks:
>  	while (pg->num_clks--)

Re: [PATCH v7 1/4] reset: Add APIs to manage array of resets

[Bjorn Andersson]

On Wed 19 Jul 08:59 PDT 2017, Philipp Zabel wrote:

> From: Vivek Gautam <vivek.gautam@codeaurora.org>
> 
> Many devices may want to request a bunch of resets and control them. So
> it's better to manage them as an array. Add APIs to _get() an array of
> reset_control, reusing the _assert(), _deassert(), and _reset() APIs for
> single reset controls. Since reset controls already may control multiple
> reset lines with a single hardware bit, from the user perspective, reset
> control arrays are not at all different from single reset controls.
> Note that these APIs don't guarantee that the reset lines managed in the
> array are handled in any particular order.
> 
> Cc: Felipe Balbi <balbi@kernel.org>
> Cc: Jon Hunter <jonathanh@nvidia.com>
> Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
> [p.zabel@pengutronix.de: changed API to hide reset control arrays behind
>  struct reset_control]
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>

This looks more or less identical to how regulators and clocks already
deals with resources in bulk; see regulator_bulk_data and clk_bulk_data
and their associated functions.

I would really like to see that you follow this model, to make it easier
for developers to work with and use the various subsystems.

Regards,
Bjorn

Re: [PATCH v7 1/4] reset: Add APIs to manage array of resets

[Philipp Zabel]

Hi,

On Thu, 2017-10-19 at 11:54 -0700, Bjorn Andersson wrote:
> On Wed 19 Jul 08:59 PDT 2017, Philipp Zabel wrote:
> 
> > From: Vivek Gautam <vivek.gautam@codeaurora.org>
> > 
> > Many devices may want to request a bunch of resets and control them. So
> > it's better to manage them as an array. Add APIs to _get() an array of
> > reset_control, reusing the _assert(), _deassert(), and _reset() APIs for
> > single reset controls. Since reset controls already may control multiple
> > reset lines with a single hardware bit, from the user perspective, reset
> > control arrays are not at all different from single reset controls.
> > Note that these APIs don't guarantee that the reset lines managed in the
> > array are handled in any particular order.
> > 
> > Cc: Felipe Balbi <balbi@kernel.org>
> > Cc: Jon Hunter <jonathanh@nvidia.com>
> > Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
> > [p.zabel@pengutronix.de: changed API to hide reset control arrays behind
> >  struct reset_control]
> > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> 
> This looks more or less identical to how regulators and clocks already
> deals with resources in bulk; see regulator_bulk_data and clk_bulk_data
> and their associated functions.
> 
> I would really like to see that you follow this model, to make it easier
> for developers to work with and use the various subsystems.

These APIs have two undesirable (in this case) properties; the driver
has to know the number of resets and their identifiers in advance, and
singular resets and bulk reset arrays can't be used interchangeably.
Both are not well suited to this use case, which is "triggering one or
any number of anonymous resets together".

I have nothing against adding a bulk API as well. There are already
users such as the pcie-qcom driver that could profit from it.

regards
Philipp

Re: [PATCH v7 4/4] soc/tegra: pmc: Use the new reset APIs to manage reset controllers

[Jon Hunter]

Hi Philipp,

On 19/10/17 16:17, Philipp Zabel wrote:
> Hi Jon, Thierry,
> 
> On Wed, 2017-07-19 at 17:59 +0200, Philipp Zabel wrote:
>> From: Vivek Gautam <vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>>
>> Make use of of_reset_control_array_get_exclusive() to manage
>> an array of reset controllers available with the device.
>>
>> Cc: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>> Cc: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>> Signed-off-by: Vivek Gautam <vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>> [p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org: switch to hidden reset control array]
>> Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> 
> will you pick this up now that the prerequisite patch 1 is contained in
> master?
> Please let me know if there are any issues with this patch.
> 
> regards
> Philipp
> 
>> ---
>> No changes since v6.
>> ---
>>  drivers/soc/tegra/pmc.c | 82 ++++++++++++-------------------------------------
>>  1 file changed, 20 insertions(+), 62 deletions(-)
>>
>> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
>> index e233dd5dcab3d..749b218147a19 100644
>> --- a/drivers/soc/tegra/pmc.c
>> +++ b/drivers/soc/tegra/pmc.c
>> @@ -124,8 +124,7 @@ struct tegra_powergate {
>>  	unsigned int id;
>>  	struct clk **clks;
>>  	unsigned int num_clks;
>> -	struct reset_control **resets;
>> -	unsigned int num_resets;
>> +	struct reset_control *reset;
>>  };
>>  
>>  struct tegra_io_pad_soc {
>> @@ -348,32 +347,14 @@ static int tegra_powergate_enable_clocks(struct tegra_powergate *pg)
>>  	return err;
>>  }
>>  
>> -static int tegra_powergate_reset_assert(struct tegra_powergate *pg)
>> +static inline int tegra_powergate_reset_assert(struct tegra_powergate *pg)
>>  {
>> -	unsigned int i;
>> -	int err;
>> -
>> -	for (i = 0; i < pg->num_resets; i++) {
>> -		err = reset_control_assert(pg->resets[i]);
>> -		if (err)
>> -			return err;
>> -	}
>> -
>> -	return 0;
>> +	return reset_control_assert(pg->reset);
>>  }
>>  
>> -static int tegra_powergate_reset_deassert(struct tegra_powergate *pg)
>> +static inline int tegra_powergate_reset_deassert(struct tegra_powergate *pg)
>>  {
>> -	unsigned int i;
>> -	int err;
>> -
>> -	for (i = 0; i < pg->num_resets; i++) {
>> -		err = reset_control_deassert(pg->resets[i]);
>> -		if (err)
>> -			return err;
>> -	}
>> -
>> -	return 0;
>> +	return reset_control_deassert(pg->reset);
>>  }

Nit-pick ... I think we should just get rid of this inline functions now
and just call reset_control_assert/deassert() where these are used.

>>  static int tegra_powergate_power_up(struct tegra_powergate *pg,
>> @@ -566,8 +547,7 @@ int tegra_powergate_sequence_power_up(unsigned int id, struct clk *clk,
>>  	pg.id = id;
>>  	pg.clks = &clk;
>>  	pg.num_clks = 1;
>> -	pg.resets = &rst;
>> -	pg.num_resets = 1;
>> +	pg.reset = IS_ERR(rst) ? NULL : rst;
>>  
>>  	err = tegra_powergate_power_up(&pg, false);
>>  	if (err)
>> @@ -755,45 +735,26 @@ static int tegra_powergate_of_get_clks(struct tegra_powergate *pg,
>>  static int tegra_powergate_of_get_resets(struct tegra_powergate *pg,
>>  					 struct device_node *np, bool off)
>>  {
>> -	struct reset_control *rst;
>> -	unsigned int i, count;
>>  	int err;
>>  
>> -	count = of_count_phandle_with_args(np, "resets", "#reset-cells");
>> -	if (count == 0)
>> -		return -ENODEV;
>> -
>> -	pg->resets = kcalloc(count, sizeof(rst), GFP_KERNEL);
>> -	if (!pg->resets)
>> -		return -ENOMEM;
>> -
>> -	for (i = 0; i < count; i++) {
>> -		pg->resets[i] = of_reset_control_get_by_index(np, i);
>> -		if (IS_ERR(pg->resets[i])) {
>> -			err = PTR_ERR(pg->resets[i]);
>> -			goto error;
>> -		}
>> -
>> -		if (off)
>> -			err = reset_control_assert(pg->resets[i]);
>> -		else
>> -			err = reset_control_deassert(pg->resets[i]);
>> -
>> -		if (err) {
>> -			reset_control_put(pg->resets[i]);
>> -			goto error;
>> -		}
>> +	pg->reset = of_reset_control_array_get_exclusive(np);
>> +	if (IS_ERR(pg->reset)) {
>> +		pr_err("failed to get device resets\n");
>> +		return PTR_ERR(pg->reset);
>>  	}
>>  
>> -	pg->num_resets = count;
>> +	if (off)
>> +		err = reset_control_assert(pg->reset);
>> +	else
>> +		err = reset_control_deassert(pg->reset);
>>  
>> -	return 0;
>> +	if (err)
>> +		goto put_reset;
>>  
>> -error:
>> -	while (i--)
>> -		reset_control_put(pg->resets[i]);
>> +	return 0;
>>  
>> -	kfree(pg->resets);
>> +put_reset:
>> +	reset_control_put(pg->reset);
>>  
>>  	return err;
>>  }
>> @@ -885,10 +846,7 @@ static void tegra_powergate_add(struct tegra_pmc *pmc, struct device_node *np)
>>  	pm_genpd_remove(&pg->genpd);
>>  
>>  remove_resets:
>> -	while (pg->num_resets--)
>> -		reset_control_put(pg->resets[pg->num_resets]);
>> -
>> -	kfree(pg->resets);
>> +	reset_control_put(pg->reset);
>>  
>>  remove_clks:
>>  	while (pg->num_clks--)

Otherwise ...

Acked-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

Thierry has just sent out his pull requests for v4.15 so we may have
missed this release :-(

Cheers
Jon

-- 
nvpublic

Re: [PATCH v7 4/4] soc/tegra: pmc: Use the new reset APIs to manage reset controllers

[Jon Hunter]

Hi Philipp,

On 19/10/17 16:17, Philipp Zabel wrote:
> Hi Jon, Thierry,
> 
> On Wed, 2017-07-19 at 17:59 +0200, Philipp Zabel wrote:
>> From: Vivek Gautam <vivek.gautam@codeaurora.org>
>>
>> Make use of of_reset_control_array_get_exclusive() to manage
>> an array of reset controllers available with the device.
>>
>> Cc: Jon Hunter <jonathanh@nvidia.com>
>> Cc: Thierry Reding <treding@nvidia.com>
>> Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
>> [p.zabel@pengutronix.de: switch to hidden reset control array]
>> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> 
> will you pick this up now that the prerequisite patch 1 is contained in
> master?
> Please let me know if there are any issues with this patch.
> 
> regards
> Philipp
> 
>> ---
>> No changes since v6.
>> ---
>>  drivers/soc/tegra/pmc.c | 82 ++++++++++++-------------------------------------
>>  1 file changed, 20 insertions(+), 62 deletions(-)
>>
>> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
>> index e233dd5dcab3d..749b218147a19 100644
>> --- a/drivers/soc/tegra/pmc.c
>> +++ b/drivers/soc/tegra/pmc.c
>> @@ -124,8 +124,7 @@ struct tegra_powergate {
>>  	unsigned int id;
>>  	struct clk **clks;
>>  	unsigned int num_clks;
>> -	struct reset_control **resets;
>> -	unsigned int num_resets;
>> +	struct reset_control *reset;
>>  };
>>  
>>  struct tegra_io_pad_soc {
>> @@ -348,32 +347,14 @@ static int tegra_powergate_enable_clocks(struct tegra_powergate *pg)
>>  	return err;
>>  }
>>  
>> -static int tegra_powergate_reset_assert(struct tegra_powergate *pg)
>> +static inline int tegra_powergate_reset_assert(struct tegra_powergate *pg)
>>  {
>> -	unsigned int i;
>> -	int err;
>> -
>> -	for (i = 0; i < pg->num_resets; i++) {
>> -		err = reset_control_assert(pg->resets[i]);
>> -		if (err)
>> -			return err;
>> -	}
>> -
>> -	return 0;
>> +	return reset_control_assert(pg->reset);
>>  }
>>  
>> -static int tegra_powergate_reset_deassert(struct tegra_powergate *pg)
>> +static inline int tegra_powergate_reset_deassert(struct tegra_powergate *pg)
>>  {
>> -	unsigned int i;
>> -	int err;
>> -
>> -	for (i = 0; i < pg->num_resets; i++) {
>> -		err = reset_control_deassert(pg->resets[i]);
>> -		if (err)
>> -			return err;
>> -	}
>> -
>> -	return 0;
>> +	return reset_control_deassert(pg->reset);
>>  }

Nit-pick ... I think we should just get rid of this inline functions now
and just call reset_control_assert/deassert() where these are used.

>>  static int tegra_powergate_power_up(struct tegra_powergate *pg,
>> @@ -566,8 +547,7 @@ int tegra_powergate_sequence_power_up(unsigned int id, struct clk *clk,
>>  	pg.id = id;
>>  	pg.clks = &clk;
>>  	pg.num_clks = 1;
>> -	pg.resets = &rst;
>> -	pg.num_resets = 1;
>> +	pg.reset = IS_ERR(rst) ? NULL : rst;
>>  
>>  	err = tegra_powergate_power_up(&pg, false);
>>  	if (err)
>> @@ -755,45 +735,26 @@ static int tegra_powergate_of_get_clks(struct tegra_powergate *pg,
>>  static int tegra_powergate_of_get_resets(struct tegra_powergate *pg,
>>  					 struct device_node *np, bool off)
>>  {
>> -	struct reset_control *rst;
>> -	unsigned int i, count;
>>  	int err;
>>  
>> -	count = of_count_phandle_with_args(np, "resets", "#reset-cells");
>> -	if (count == 0)
>> -		return -ENODEV;
>> -
>> -	pg->resets = kcalloc(count, sizeof(rst), GFP_KERNEL);
>> -	if (!pg->resets)
>> -		return -ENOMEM;
>> -
>> -	for (i = 0; i < count; i++) {
>> -		pg->resets[i] = of_reset_control_get_by_index(np, i);
>> -		if (IS_ERR(pg->resets[i])) {
>> -			err = PTR_ERR(pg->resets[i]);
>> -			goto error;
>> -		}
>> -
>> -		if (off)
>> -			err = reset_control_assert(pg->resets[i]);
>> -		else
>> -			err = reset_control_deassert(pg->resets[i]);
>> -
>> -		if (err) {
>> -			reset_control_put(pg->resets[i]);
>> -			goto error;
>> -		}
>> +	pg->reset = of_reset_control_array_get_exclusive(np);
>> +	if (IS_ERR(pg->reset)) {
>> +		pr_err("failed to get device resets\n");
>> +		return PTR_ERR(pg->reset);
>>  	}
>>  
>> -	pg->num_resets = count;
>> +	if (off)
>> +		err = reset_control_assert(pg->reset);
>> +	else
>> +		err = reset_control_deassert(pg->reset);
>>  
>> -	return 0;
>> +	if (err)
>> +		goto put_reset;
>>  
>> -error:
>> -	while (i--)
>> -		reset_control_put(pg->resets[i]);
>> +	return 0;
>>  
>> -	kfree(pg->resets);
>> +put_reset:
>> +	reset_control_put(pg->reset);
>>  
>>  	return err;
>>  }
>> @@ -885,10 +846,7 @@ static void tegra_powergate_add(struct tegra_pmc *pmc, struct device_node *np)
>>  	pm_genpd_remove(&pg->genpd);
>>  
>>  remove_resets:
>> -	while (pg->num_resets--)
>> -		reset_control_put(pg->resets[pg->num_resets]);
>> -
>> -	kfree(pg->resets);
>> +	reset_control_put(pg->reset);
>>  
>>  remove_clks:
>>  	while (pg->num_clks--)

Otherwise ...

Acked-by: Jon Hunter <jonathanh@nvidia.com>

Thierry has just sent out his pull requests for v4.15 so we may have
missed this release :-(

Cheers
Jon

-- 
nvpublic

Re: [PATCH v7 4/4] soc/tegra: pmc: Use the new reset APIs to manage reset controllers

[Philipp Zabel]

On Fri, Oct 20, 2017 at 04:51:24PM +0100, Jon Hunter wrote:
> Hi Philipp,
> 
> On 19/10/17 16:17, Philipp Zabel wrote:
> > Hi Jon, Thierry,
> > 
> > On Wed, 2017-07-19 at 17:59 +0200, Philipp Zabel wrote:
> >> From: Vivek Gautam <vivek.gautam@codeaurora.org>
> >>
> >> Make use of of_reset_control_array_get_exclusive() to manage
> >> an array of reset controllers available with the device.
> >>
> >> Cc: Jon Hunter <jonathanh@nvidia.com>
> >> Cc: Thierry Reding <treding@nvidia.com>
> >> Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
> >> [p.zabel@pengutronix.de: switch to hidden reset control array]
> >> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> > 
> > will you pick this up now that the prerequisite patch 1 is contained in
> > master?
> > Please let me know if there are any issues with this patch.
> > 
> > regards
> > Philipp
> > 
> >> ---
> >> No changes since v6.
> >> ---
> >>  drivers/soc/tegra/pmc.c | 82 ++++++++++++-------------------------------------
> >>  1 file changed, 20 insertions(+), 62 deletions(-)
> >>
> >> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
> >> index e233dd5dcab3d..749b218147a19 100644
> >> --- a/drivers/soc/tegra/pmc.c
> >> +++ b/drivers/soc/tegra/pmc.c
> >> @@ -124,8 +124,7 @@ struct tegra_powergate {
> >>  	unsigned int id;
> >>  	struct clk **clks;
> >>  	unsigned int num_clks;
> >> -	struct reset_control **resets;
> >> -	unsigned int num_resets;
> >> +	struct reset_control *reset;
> >>  };
> >>  
> >>  struct tegra_io_pad_soc {
> >> @@ -348,32 +347,14 @@ static int tegra_powergate_enable_clocks(struct tegra_powergate *pg)
> >>  	return err;
> >>  }
> >>  
> >> -static int tegra_powergate_reset_assert(struct tegra_powergate *pg)
> >> +static inline int tegra_powergate_reset_assert(struct tegra_powergate *pg)
> >>  {
> >> -	unsigned int i;
> >> -	int err;
> >> -
> >> -	for (i = 0; i < pg->num_resets; i++) {
> >> -		err = reset_control_assert(pg->resets[i]);
> >> -		if (err)
> >> -			return err;
> >> -	}
> >> -
> >> -	return 0;
> >> +	return reset_control_assert(pg->reset);
> >>  }
> >>  
> >> -static int tegra_powergate_reset_deassert(struct tegra_powergate *pg)
> >> +static inline int tegra_powergate_reset_deassert(struct tegra_powergate *pg)
> >>  {
> >> -	unsigned int i;
> >> -	int err;
> >> -
> >> -	for (i = 0; i < pg->num_resets; i++) {
> >> -		err = reset_control_deassert(pg->resets[i]);
> >> -		if (err)
> >> -			return err;
> >> -	}
> >> -
> >> -	return 0;
> >> +	return reset_control_deassert(pg->reset);
> >>  }
> 
> Nit-pick ... I think we should just get rid of this inline functions now
> and just call reset_control_assert/deassert() where these are used.
> 
[...]
> 
> Otherwise ...
> 
> Acked-by: Jon Hunter <jonathanh@nvidia.com>
> 
> Thierry has just sent out his pull requests for v4.15 so we may have
> missed this release :-(

Thanks, I'll drop the inline functions and send a new version, then.

regards
Philipp

Re: [PATCH v7 1/4] reset: Add APIs to manage array of resets

[Bjorn Andersson]

On Fri 20 Oct 05:20 PDT 2017, Philipp Zabel wrote:

> Hi,
> 
> On Thu, 2017-10-19 at 11:54 -0700, Bjorn Andersson wrote:
> > On Wed 19 Jul 08:59 PDT 2017, Philipp Zabel wrote:
> > 
> > > From: Vivek Gautam <vivek.gautam@codeaurora.org>
> > > 
> > > Many devices may want to request a bunch of resets and control them. So
> > > it's better to manage them as an array. Add APIs to _get() an array of
> > > reset_control, reusing the _assert(), _deassert(), and _reset() APIs for
> > > single reset controls. Since reset controls already may control multiple
> > > reset lines with a single hardware bit, from the user perspective, reset
> > > control arrays are not at all different from single reset controls.
> > > Note that these APIs don't guarantee that the reset lines managed in the
> > > array are handled in any particular order.
> > > 
> > > Cc: Felipe Balbi <balbi@kernel.org>
> > > Cc: Jon Hunter <jonathanh@nvidia.com>
> > > Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
> > > [p.zabel@pengutronix.de: changed API to hide reset control arrays behind
> > >  struct reset_control]
> > > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> > 
> > This looks more or less identical to how regulators and clocks already
> > deals with resources in bulk; see regulator_bulk_data and clk_bulk_data
> > and their associated functions.
> > 
> > I would really like to see that you follow this model, to make it easier
> > for developers to work with and use the various subsystems.
> 
> These APIs have two undesirable (in this case) properties; the driver
> has to know the number of resets and their identifiers in advance, and
> singular resets and bulk reset arrays can't be used interchangeably.

As a writer of device drivers as well as dts files I greatly appreciate
when this expectations is encoded in the kernel, so that it is clear
when the DT node is missing some resource - rather than having random
reboots because of spelling mistakes or variations between hardware
revisions.

We tend to express these things explicitly in the kernel, as magic
interfaces makes things harder to debug.

> Both are not well suited to this use case, which is "triggering one or
> any number of anonymous resets together".
> 

Triggering one is just a special case of N. 


But this does not change the fact that the reset framework interface
looks and function in a fundamentally different way than the clock and
regulator equivalents, which will be confusing - in particular since
most drivers will use 2 or 3 of these.

Regards,
Bjorn

Re: [PATCH v7 1/4] reset: Add APIs to manage array of resets

[Philipp Zabel]

Hi Bjorn,

On Wed, 2017-11-01 at 15:24 -0700, Bjorn Andersson wrote:
[...]
> > > This looks more or less identical to how regulators and clocks already
> > > deals with resources in bulk; see regulator_bulk_data and clk_bulk_data
> > > and their associated functions.
> > > 
> > > I would really like to see that you follow this model, to make it easier
> > > for developers to work with and use the various subsystems.
> > 
> > These APIs have two undesirable (in this case) properties; the driver
> > has to know the number of resets and their identifiers in advance, and
> > singular resets and bulk reset arrays can't be used interchangeably.
> 
> As a writer of device drivers as well as dts files I greatly appreciate
> when this expectations is encoded in the kernel, so that it is clear
> when the DT node is missing some resource - rather than having random
> reboots because of spelling mistakes or variations between hardware
> revisions.
> 
> We tend to express these things explicitly in the kernel, as magic
> interfaces makes things harder to debug.

I have no control over how most of those bindings are designed. While I
prefer bindings to explicitly specify resets by identifier where
possible, there are some generic bindings that just don't have this
information.
See for example the ohci/ehci-platform USB host drivers, which are used
for platform integration on various SoCs, or the Tegra pmc driver, which
has to handle resets for other peripherals when power gating.

Also, I currently don't see many drivers that would profit much from a
bulk API, as many drivers that request multiple reset controls have to
handle them individually anyway, to guarantee ordering or reset timings.
One candidate would be the pcie-qcom driver. If you are aware of more
potential users of a bulk API, please let me know.

> > Both are not well suited to this use case, which is "triggering one or
> > any number of anonymous resets together".
> > 
> 
> Triggering one is just a special case of N. 
>
> But this does not change the fact that the reset framework interface
> looks and function in a fundamentally different way than the clock and
> regulator equivalents, which will be confusing - in particular since
> most drivers will use 2 or 3 of these.

There is no way to make them work all exactly the same, as the resources
 themselves are used in different ways. Surely we should try to minimize
the API differences to allow transfer of expectations, but that should
not be the only goal.
For example neither clock nor regulator framework have support for
exclusive clocks/regulators that can be forced-off, and _optional
requests are handled differently in the gpio and regulator frameworks.

That being said, I'd be happy to add a bulk API if that actually helps
some drivers.

regards
Philipp

Re: [PATCH v7 4/4] soc/tegra: pmc: Use the new reset APIs to manage reset controllers

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 782 bytes --]

On Wed, Jul 19, 2017 at 05:59:08PM +0200, Philipp Zabel wrote:
> From: Vivek Gautam <vivek.gautam@codeaurora.org>
> 
> Make use of of_reset_control_array_get_exclusive() to manage
> an array of reset controllers available with the device.
> 
> Cc: Jon Hunter <jonathanh@nvidia.com>
> Cc: Thierry Reding <treding@nvidia.com>
> Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
> [p.zabel@pengutronix.de: switch to hidden reset control array]
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
> No changes since v6.
> ---
>  drivers/soc/tegra/pmc.c | 82 ++++++++++++-------------------------------------
>  1 file changed, 20 insertions(+), 62 deletions(-)

I've applied this with the static inline change that Jon suggested.

Thanks,
Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]