[PATCH 1/5] reset: add acquired/released state for exclusive reset controls

[PATCH 1/5] reset: add acquired/released state for exclusive reset controls

[Thierry Reding]

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

There are cases where a driver needs explicit control over a reset line
that is exclusively conneted to its device, but this control has to be
temporarily handed over to the power domain controller to handle reset
requirements during power transitions.
Allow multiple exclusive reset controls to be requested in 'released'
state for the same physical reset line, only one of which can be
acquired at the same time.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/reset/core.c  | 139 ++++++++++++++++++++++++++++++++++++++----
 include/linux/reset.h |  93 ++++++++++++++++++++++------
 2 files changed, 200 insertions(+), 32 deletions(-)

diff --git a/drivers/reset/core.c b/drivers/reset/core.c
index 9582efb70025..1e8a42b16f23 100644
--- a/drivers/reset/core.c
+++ b/drivers/reset/core.c
@@ -34,6 +34,7 @@ static LIST_HEAD(reset_lookup_list);
  * @id: ID of the reset controller in the reset
  *      controller device
  * @refcnt: Number of gets of this reset_control
+ * @acquired: Only one reset_control may be acquired for a given rcdev and id.
  * @shared: Is this a shared (1), or an exclusive (0) reset_control?
  * @deassert_cnt: Number of times this reset line has been deasserted
  * @triggered_count: Number of times this reset line has been reset. Currently
@@ -45,6 +46,7 @@ struct reset_control {
 	struct list_head list;
 	unsigned int id;
 	struct kref refcnt;
+	bool acquired;
 	bool shared;
 	bool array;
 	atomic_t deassert_count;
@@ -63,6 +65,17 @@ struct reset_control_array {
 	struct reset_control *rstc[];
 };
 
+static const char *rcdev_name(struct reset_controller_dev *rcdev)
+{
+	if (rcdev->dev)
+		return dev_name(rcdev->dev);
+
+	if (rcdev->of_node)
+		return rcdev->of_node->full_name;
+
+	return NULL;
+}
+
 /**
  * of_reset_simple_xlate - translate reset_spec to the reset line number
  * @rcdev: a pointer to the reset controller device
@@ -272,6 +285,9 @@ int reset_control_reset(struct reset_control *rstc)
 
 		if (atomic_inc_return(&rstc->triggered_count) != 1)
 			return 0;
+	} else {
+		if (!rstc->acquired)
+			return -EPERM;
 	}
 
 	ret = rstc->rcdev->ops->reset(rstc->rcdev, rstc->id);
@@ -334,6 +350,12 @@ int reset_control_assert(struct reset_control *rstc)
 		 */
 		if (!rstc->rcdev->ops->assert)
 			return -ENOTSUPP;
+
+		if (!rstc->acquired) {
+			WARN(1, "reset %s (ID: %u) is not acquired\n",
+			     rcdev_name(rstc->rcdev), rstc->id);
+			return -EPERM;
+		}
 	}
 
 	return rstc->rcdev->ops->assert(rstc->rcdev, rstc->id);
@@ -369,6 +391,12 @@ int reset_control_deassert(struct reset_control *rstc)
 
 		if (atomic_inc_return(&rstc->deassert_count) != 1)
 			return 0;
+	} else {
+		if (!rstc->acquired) {
+			WARN(1, "reset %s (ID: %u) is not acquired\n",
+			     rcdev_name(rstc->rcdev), rstc->id);
+			return -EPERM;
+		}
 	}
 
 	/*
@@ -406,9 +434,81 @@ int reset_control_status(struct reset_control *rstc)
 }
 EXPORT_SYMBOL_GPL(reset_control_status);
 
+/**
+ * reset_control_acquire() - acquires a reset control for exclusive use
+ * @rstc: reset control
+ *
+ * This is used to explicitly acquire a reset control for exclusive use. Note
+ * that exclusive resets are requested as acquired by default. In order for a
+ * second consumer to be able to control the reset, the first consumer has to
+ * release it first. Typically the easiest way to achieve this is to call the
+ * reset_control_get_exclusive_released() to obtain an instance of the reset
+ * control. Such reset controls are not acquired by default.
+ *
+ * Consumers implementing shared access to an exclusive reset need to follow
+ * a specific protocol in order to work together. Before consumers can change
+ * a reset they must acquire exclusive access using reset_control_acquire().
+ * After they are done operating the reset, they must release exclusive access
+ * with a call to reset_control_release(). Consumers are not granted exclusive
+ * access to the reset as long as another consumer hasn't released a reset.
+ *
+ * See also: reset_control_release()
+ */
+int reset_control_acquire(struct reset_control *rstc)
+{
+	struct reset_control *rc;
+
+	if (!rstc)
+		return 0;
+
+	if (WARN_ON(IS_ERR(rstc)))
+		return -EINVAL;
+
+	mutex_lock(&reset_list_mutex);
+
+	if (rstc->acquired) {
+		mutex_unlock(&reset_list_mutex);
+		return 0;
+	}
+
+	list_for_each_entry(rc, &rstc->rcdev->reset_control_head, list) {
+		if (rstc != rc && rstc->id == rc->id) {
+			if (rc->acquired) {
+				mutex_unlock(&reset_list_mutex);
+				return -EBUSY;
+			}
+		}
+	}
+
+	rstc->acquired = true;
+
+	mutex_unlock(&reset_list_mutex);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(reset_control_acquire);
+
+/**
+ * reset_control_release() - releases exclusive access to a reset control
+ * @rstc: reset control
+ *
+ * Releases exclusive access right to a reset control previously obtained by a
+ * call to reset_control_acquire(). Until a consumer calls this function, no
+ * other consumers will be granted exclusive access.
+ *
+ * See also: reset_control_acquire()
+ */
+void reset_control_release(struct reset_control *rstc)
+{
+	if (!rstc || WARN_ON(IS_ERR(rstc)))
+		return;
+
+	rstc->acquired = false;
+}
+EXPORT_SYMBOL_GPL(reset_control_release);
+
 static struct reset_control *__reset_control_get_internal(
 				struct reset_controller_dev *rcdev,
-				unsigned int index, bool shared)
+				unsigned int index, bool shared, bool acquired)
 {
 	struct reset_control *rstc;
 
@@ -416,6 +516,14 @@ static struct reset_control *__reset_control_get_internal(
 
 	list_for_each_entry(rstc, &rcdev->reset_control_head, list) {
 		if (rstc->id == index) {
+			/*
+			 * Allow creating a secondary exclusive reset_control
+			 * that is initially not acquired for an already
+			 * controlled reset line.
+			 */
+			if (!rstc->shared && !shared && !acquired)
+				break;
+
 			if (WARN_ON(!rstc->shared || !shared))
 				return ERR_PTR(-EBUSY);
 
@@ -434,6 +542,7 @@ static struct reset_control *__reset_control_get_internal(
 	list_add(&rstc->list, &rcdev->reset_control_head);
 	rstc->id = index;
 	kref_init(&rstc->refcnt);
+	rstc->acquired = acquired;
 	rstc->shared = shared;
 
 	return rstc;
@@ -461,7 +570,7 @@ static void __reset_control_put_internal(struct reset_control *rstc)
 
 struct reset_control *__of_reset_control_get(struct device_node *node,
 				     const char *id, int index, bool shared,
-				     bool optional)
+				     bool optional, bool acquired)
 {
 	struct reset_control *rstc;
 	struct reset_controller_dev *r, *rcdev;
@@ -514,7 +623,7 @@ struct reset_control *__of_reset_control_get(struct device_node *node,
 	}
 
 	/* reset_list_mutex also protects the rcdev's reset_control list */
-	rstc = __reset_control_get_internal(rcdev, rstc_id, shared);
+	rstc = __reset_control_get_internal(rcdev, rstc_id, shared, acquired);
 
 out:
 	mutex_unlock(&reset_list_mutex);
@@ -544,7 +653,7 @@ __reset_controller_by_name(const char *name)
 
 static struct reset_control *
 __reset_control_get_from_lookup(struct device *dev, const char *con_id,
-				bool shared, bool optional)
+				bool shared, bool optional, bool acquired)
 {
 	const struct reset_control_lookup *lookup;
 	struct reset_controller_dev *rcdev;
@@ -574,7 +683,7 @@ __reset_control_get_from_lookup(struct device *dev, const char *con_id,
 
 			rstc = __reset_control_get_internal(rcdev,
 							    lookup->index,
-							    shared);
+							    shared, acquired);
 			mutex_unlock(&reset_list_mutex);
 			break;
 		}
@@ -589,13 +698,18 @@ __reset_control_get_from_lookup(struct device *dev, const char *con_id,
 }
 
 struct reset_control *__reset_control_get(struct device *dev, const char *id,
-					  int index, bool shared, bool optional)
+					  int index, bool shared, bool optional,
+					  bool acquired)
 {
+	if (WARN_ON(shared && acquired))
+		return ERR_PTR(-EINVAL);
+
 	if (dev->of_node)
 		return __of_reset_control_get(dev->of_node, id, index, shared,
-					      optional);
+					      optional, acquired);
 
-	return __reset_control_get_from_lookup(dev, id, shared, optional);
+	return __reset_control_get_from_lookup(dev, id, shared, optional,
+					       acquired);
 }
 EXPORT_SYMBOL_GPL(__reset_control_get);
 
@@ -636,7 +750,7 @@ static void devm_reset_control_release(struct device *dev, void *res)
 
 struct reset_control *__devm_reset_control_get(struct device *dev,
 				     const char *id, int index, bool shared,
-				     bool optional)
+				     bool optional, bool acquired)
 {
 	struct reset_control **ptr, *rstc;
 
@@ -645,7 +759,7 @@ struct reset_control *__devm_reset_control_get(struct device *dev,
 	if (!ptr)
 		return ERR_PTR(-ENOMEM);
 
-	rstc = __reset_control_get(dev, id, index, shared, optional);
+	rstc = __reset_control_get(dev, id, index, shared, optional, acquired);
 	if (!IS_ERR(rstc)) {
 		*ptr = rstc;
 		devres_add(dev, ptr);
@@ -672,7 +786,7 @@ int __device_reset(struct device *dev, bool optional)
 	struct reset_control *rstc;
 	int ret;
 
-	rstc = __reset_control_get(dev, NULL, 0, 0, optional);
+	rstc = __reset_control_get(dev, NULL, 0, 0, optional, true);
 	if (IS_ERR(rstc))
 		return PTR_ERR(rstc);
 
@@ -736,7 +850,8 @@ of_reset_control_array_get(struct device_node *np, bool shared, bool optional)
 		return ERR_PTR(-ENOMEM);
 
 	for (i = 0; i < num; i++) {
-		rstc = __of_reset_control_get(np, NULL, i, shared, optional);
+		rstc = __of_reset_control_get(np, NULL, i, shared, optional,
+					      true);
 		if (IS_ERR(rstc))
 			goto err_rst;
 		resets->rstc[i] = rstc;
diff --git a/include/linux/reset.h b/include/linux/reset.h
index c1901b61ca30..ea9a8a1ce4b1 100644
--- a/include/linux/reset.h
+++ b/include/linux/reset.h
@@ -14,18 +14,20 @@ int reset_control_reset(struct reset_control *rstc);
 int reset_control_assert(struct reset_control *rstc);
 int reset_control_deassert(struct reset_control *rstc);
 int reset_control_status(struct reset_control *rstc);
+int reset_control_acquire(struct reset_control *rstc);
+void reset_control_release(struct reset_control *rstc);
 
 struct reset_control *__of_reset_control_get(struct device_node *node,
 				     const char *id, int index, bool shared,
-				     bool optional);
+				     bool optional, bool acquired);
 struct reset_control *__reset_control_get(struct device *dev, const char *id,
 					  int index, bool shared,
-					  bool optional);
+					  bool optional, bool acquired);
 void reset_control_put(struct reset_control *rstc);
 int __device_reset(struct device *dev, bool optional);
 struct reset_control *__devm_reset_control_get(struct device *dev,
 				     const char *id, int index, bool shared,
-				     bool optional);
+				     bool optional, bool acquired);
 
 struct reset_control *devm_reset_control_array_get(struct device *dev,
 						   bool shared, bool optional);
@@ -56,6 +58,15 @@ static inline int reset_control_status(struct reset_control *rstc)
 	return 0;
 }
 
+static inline int reset_control_acquire(struct reset_control *rstc)
+{
+	return 0;
+}
+
+static inline void reset_control_release(struct reset_control *rstc)
+{
+}
+
 static inline void reset_control_put(struct reset_control *rstc)
 {
 }
@@ -68,21 +79,23 @@ static inline int __device_reset(struct device *dev, bool optional)
 static inline struct reset_control *__of_reset_control_get(
 					struct device_node *node,
 					const char *id, int index, bool shared,
-					bool optional)
+					bool optional, bool acquired)
 {
 	return optional ? NULL : ERR_PTR(-ENOTSUPP);
 }
 
 static inline struct reset_control *__reset_control_get(
 					struct device *dev, const char *id,
-					int index, bool shared, bool optional)
+					int index, bool shared, bool optional,
+					bool acquired)
 {
 	return optional ? NULL : ERR_PTR(-ENOTSUPP);
 }
 
 static inline struct reset_control *__devm_reset_control_get(
 					struct device *dev, const char *id,
-					int index, bool shared, bool optional)
+					int index, bool shared, bool optional,
+					bool acquired)
 {
 	return optional ? NULL : ERR_PTR(-ENOTSUPP);
 }
@@ -134,7 +147,28 @@ static inline int device_reset_optional(struct device *dev)
 static inline struct reset_control *
 __must_check reset_control_get_exclusive(struct device *dev, const char *id)
 {
-	return __reset_control_get(dev, id, 0, false, false);
+	return __reset_control_get(dev, id, 0, false, false, true);
+}
+
+/**
+ * reset_control_get_exclusive_released - Lookup and obtain a temoprarily
+ *                                        exclusive reference to a reset
+ *                                        controller.
+ * @dev: device to be reset by the controller
+ * @id: reset line name
+ *
+ * Returns a struct reset_control or IS_ERR() condition containing errno.
+ * reset-controls returned by this function must be acquired via
+ * reset_control_acquire() before they can be used and should be released
+ * via reset_control_release() afterwards.
+ *
+ * Use of id names is optional.
+ */
+static inline struct reset_control *
+__must_check reset_control_get_exclusive_released(struct device *dev,
+						  const char *id)
+{
+	return __reset_control_get(dev, id, 0, false, false, false);
 }
 
 /**
@@ -162,19 +196,19 @@ __must_check reset_control_get_exclusive(struct device *dev, const char *id)
 static inline struct reset_control *reset_control_get_shared(
 					struct device *dev, const char *id)
 {
-	return __reset_control_get(dev, id, 0, true, false);
+	return __reset_control_get(dev, id, 0, true, false, false);
 }
 
 static inline struct reset_control *reset_control_get_optional_exclusive(
 					struct device *dev, const char *id)
 {
-	return __reset_control_get(dev, id, 0, false, true);
+	return __reset_control_get(dev, id, 0, false, true, true);
 }
 
 static inline struct reset_control *reset_control_get_optional_shared(
 					struct device *dev, const char *id)
 {
-	return __reset_control_get(dev, id, 0, true, true);
+	return __reset_control_get(dev, id, 0, true, true, false);
 }
 
 /**
@@ -190,7 +224,7 @@ static inline struct reset_control *reset_control_get_optional_shared(
 static inline struct reset_control *of_reset_control_get_exclusive(
 				struct device_node *node, const char *id)
 {
-	return __of_reset_control_get(node, id, 0, false, false);
+	return __of_reset_control_get(node, id, 0, false, false, true);
 }
 
 /**
@@ -215,7 +249,7 @@ static inline struct reset_control *of_reset_control_get_exclusive(
 static inline struct reset_control *of_reset_control_get_shared(
 				struct device_node *node, const char *id)
 {
-	return __of_reset_control_get(node, id, 0, true, false);
+	return __of_reset_control_get(node, id, 0, true, false, false);
 }
 
 /**
@@ -232,7 +266,7 @@ static inline struct reset_control *of_reset_control_get_shared(
 static inline struct reset_control *of_reset_control_get_exclusive_by_index(
 					struct device_node *node, int index)
 {
-	return __of_reset_control_get(node, NULL, index, false, false);
+	return __of_reset_control_get(node, NULL, index, false, false, true);
 }
 
 /**
@@ -260,7 +294,7 @@ static inline struct reset_control *of_reset_control_get_exclusive_by_index(
 static inline struct reset_control *of_reset_control_get_shared_by_index(
 					struct device_node *node, int index)
 {
-	return __of_reset_control_get(node, NULL, index, true, false);
+	return __of_reset_control_get(node, NULL, index, true, false, false);
 }
 
 /**
@@ -279,7 +313,26 @@ static inline struct reset_control *
 __must_check devm_reset_control_get_exclusive(struct device *dev,
 					      const char *id)
 {
-	return __devm_reset_control_get(dev, id, 0, false, false);
+	return __devm_reset_control_get(dev, id, 0, false, false, true);
+}
+
+/**
+ * devm_reset_control_get_exclusive_released - resource managed
+ *                                             reset_control_get_exclusive_released()
+ * @dev: device to be reset by the controller
+ * @id: reset line name
+ *
+ * Managed reset_control_get_exclusive_released(). For reset controllers
+ * returned from this function, reset_control_put() is called automatically on
+ * driver detach.
+ *
+ * See reset_control_get_exclusive_released() for more information.
+ */
+static inline struct reset_control *
+__must_check devm_reset_control_get_exclusive_released(struct device *dev,
+						       const char *id)
+{
+	return __devm_reset_control_get(dev, id, 0, false, false, false);
 }
 
 /**
@@ -294,19 +347,19 @@ __must_check devm_reset_control_get_exclusive(struct device *dev,
 static inline struct reset_control *devm_reset_control_get_shared(
 					struct device *dev, const char *id)
 {
-	return __devm_reset_control_get(dev, id, 0, true, false);
+	return __devm_reset_control_get(dev, id, 0, true, false, false);
 }
 
 static inline struct reset_control *devm_reset_control_get_optional_exclusive(
 					struct device *dev, const char *id)
 {
-	return __devm_reset_control_get(dev, id, 0, false, true);
+	return __devm_reset_control_get(dev, id, 0, false, true, true);
 }
 
 static inline struct reset_control *devm_reset_control_get_optional_shared(
 					struct device *dev, const char *id)
 {
-	return __devm_reset_control_get(dev, id, 0, true, true);
+	return __devm_reset_control_get(dev, id, 0, true, true, false);
 }
 
 /**
@@ -324,7 +377,7 @@ static inline struct reset_control *devm_reset_control_get_optional_shared(
 static inline struct reset_control *
 devm_reset_control_get_exclusive_by_index(struct device *dev, int index)
 {
-	return __devm_reset_control_get(dev, NULL, index, false, false);
+	return __devm_reset_control_get(dev, NULL, index, false, false, true);
 }
 
 /**
@@ -340,7 +393,7 @@ devm_reset_control_get_exclusive_by_index(struct device *dev, int index)
 static inline struct reset_control *
 devm_reset_control_get_shared_by_index(struct device *dev, int index)
 {
-	return __devm_reset_control_get(dev, NULL, index, true, false);
+	return __devm_reset_control_get(dev, NULL, index, true, false, false);
 }
 
 /*
-- 
2.20.1

[PATCH 2/5] reset: Add acquired flag to of_reset_control_array_get()

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

In order to be able to request an array of reset controls in acquired or
released mode, add the acquired flag to of_reset_control_array_get() and
pass the flag to subsequent calls of __of_reset_control_get().

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/reset/core.c              |  9 ++++++---
 drivers/usb/dwc3/dwc3-of-simple.c |  3 ++-
 include/linux/reset.h             | 14 ++++++++------
 3 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/reset/core.c b/drivers/reset/core.c
index 1e8a42b16f23..f94da91c22af 100644
--- a/drivers/reset/core.c
+++ b/drivers/reset/core.c
@@ -830,12 +830,15 @@ static int of_reset_control_get_count(struct device_node *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
+ * @acquired: only one reset control may be acquired for a given controller
+ *            and ID
  *
  * 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)
+of_reset_control_array_get(struct device_node *np, bool shared, bool optional,
+			   bool acquired)
 {
 	struct reset_control_array *resets;
 	struct reset_control *rstc;
@@ -851,7 +854,7 @@ of_reset_control_array_get(struct device_node *np, bool shared, bool optional)
 
 	for (i = 0; i < num; i++) {
 		rstc = __of_reset_control_get(np, NULL, i, shared, optional,
-					      true);
+					      acquired);
 		if (IS_ERR(rstc))
 			goto err_rst;
 		resets->rstc[i] = rstc;
@@ -898,7 +901,7 @@ devm_reset_control_array_get(struct device *dev, bool shared, bool optional)
 	if (!devres)
 		return ERR_PTR(-ENOMEM);
 
-	rstc = of_reset_control_array_get(dev->of_node, shared, optional);
+	rstc = of_reset_control_array_get(dev->of_node, shared, optional, true);
 	if (IS_ERR(rstc)) {
 		devres_free(devres);
 		return rstc;
diff --git a/drivers/usb/dwc3/dwc3-of-simple.c b/drivers/usb/dwc3/dwc3-of-simple.c
index 4c2771c5e727..67ce2037472d 100644
--- a/drivers/usb/dwc3/dwc3-of-simple.c
+++ b/drivers/usb/dwc3/dwc3-of-simple.c
@@ -107,7 +107,8 @@ static int dwc3_of_simple_probe(struct platform_device *pdev)
 		simple->pulse_resets = true;
 	}
 
-	simple->resets = of_reset_control_array_get(np, shared_resets, true);
+	simple->resets = of_reset_control_array_get(np, shared_resets, true,
+						    true);
 	if (IS_ERR(simple->resets)) {
 		ret = PTR_ERR(simple->resets);
 		dev_err(dev, "failed to get device resets, err=%d\n", ret);
diff --git a/include/linux/reset.h b/include/linux/reset.h
index ea9a8a1ce4b1..a01b32bf51d4 100644
--- a/include/linux/reset.h
+++ b/include/linux/reset.h
@@ -32,7 +32,8 @@ struct reset_control *__devm_reset_control_get(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);
+						 bool shared, bool optional,
+						 bool acquired);
 
 int reset_control_get_count(struct device *dev);
 
@@ -107,7 +108,8 @@ devm_reset_control_array_get(struct device *dev, bool shared, bool optional)
 }
 
 static inline struct reset_control *
-of_reset_control_array_get(struct device_node *np, bool shared, bool optional)
+of_reset_control_array_get(struct device_node *np, bool shared, bool optional,
+			   bool acquired)
 {
 	return optional ? NULL : ERR_PTR(-ENOTSUPP);
 }
@@ -465,24 +467,24 @@ devm_reset_control_array_get_optional_shared(struct device *dev)
 static inline struct reset_control *
 of_reset_control_array_get_exclusive(struct device_node *node)
 {
-	return of_reset_control_array_get(node, false, false);
+	return of_reset_control_array_get(node, false, false, true);
 }
 
 static inline struct reset_control *
 of_reset_control_array_get_shared(struct device_node *node)
 {
-	return of_reset_control_array_get(node, true, false);
+	return of_reset_control_array_get(node, true, false, true);
 }
 
 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);
+	return of_reset_control_array_get(node, false, true, 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);
+	return of_reset_control_array_get(node, true, true, true);
 }
 #endif
-- 
2.20.1

[PATCH 3/5] reset: Add acquire/release support for arrays

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Add implementations that apply acquire and release operations to all
reset controls part of a reset control array.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/reset/core.c  | 36 +++++++++++++++++++++++++++++++++++-
 include/linux/reset.h |  6 ++++++
 2 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/drivers/reset/core.c b/drivers/reset/core.c
index f94da91c22af..81ea77cba123 100644
--- a/drivers/reset/core.c
+++ b/drivers/reset/core.c
@@ -245,6 +245,34 @@ static int reset_control_array_deassert(struct reset_control_array *resets)
 	return ret;
 }
 
+static int reset_control_array_acquire(struct reset_control_array *resets)
+{
+	unsigned int i;
+	int err;
+
+	for (i = 0; i < resets->num_rstcs; i++) {
+		err = reset_control_acquire(resets->rstc[i]);
+		if (err < 0)
+			goto release;
+	}
+
+	return 0;
+
+release:
+	while (i--)
+		reset_control_release(resets->rstc[i]);
+
+	return err;
+}
+
+static void reset_control_array_release(struct reset_control_array *resets)
+{
+	unsigned int i;
+
+	for (i = 0; i < resets->num_rstcs; i++)
+		reset_control_release(resets->rstc[i]);
+}
+
 static inline bool reset_control_is_array(struct reset_control *rstc)
 {
 	return rstc->array;
@@ -464,6 +492,9 @@ int reset_control_acquire(struct reset_control *rstc)
 	if (WARN_ON(IS_ERR(rstc)))
 		return -EINVAL;
 
+	if (reset_control_is_array(rstc))
+		return reset_control_array_acquire(rstc_to_array(rstc));
+
 	mutex_lock(&reset_list_mutex);
 
 	if (rstc->acquired) {
@@ -502,7 +533,10 @@ void reset_control_release(struct reset_control *rstc)
 	if (!rstc || WARN_ON(IS_ERR(rstc)))
 		return;
 
-	rstc->acquired = false;
+	if (reset_control_is_array(rstc))
+		reset_control_array_release(rstc_to_array(rstc));
+	else
+		rstc->acquired = false;
 }
 EXPORT_SYMBOL_GPL(reset_control_release);
 
diff --git a/include/linux/reset.h b/include/linux/reset.h
index a01b32bf51d4..95d555c2130a 100644
--- a/include/linux/reset.h
+++ b/include/linux/reset.h
@@ -470,6 +470,12 @@ of_reset_control_array_get_exclusive(struct device_node *node)
 	return of_reset_control_array_get(node, false, false, true);
 }
 
+static inline struct reset_control *
+of_reset_control_array_get_exclusive_released(struct device_node *node)
+{
+	return of_reset_control_array_get(node, false, false, false);
+}
+
 static inline struct reset_control *
 of_reset_control_array_get_shared(struct device_node *node)
 {
-- 
2.20.1

[PATCH 4/5] soc/tegra: pmc: Implement acquire/release for resets

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

By implementing the acquire/release protocol, the resets can be shared
with other drivers that also adhere to this protocol. This will be used
for example by the SOR driver to put hardware into a known good state,
irrespective of whether or not the power domain can be reset.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/soc/tegra/pmc.c | 39 +++++++++++++++++++++++++++++++++------
 1 file changed, 33 insertions(+), 6 deletions(-)

diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index 0df258518693..0c5f79528e5f 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -656,10 +656,15 @@ static int tegra_genpd_power_on(struct generic_pm_domain *domain)
 	int err;
 
 	err = tegra_powergate_power_up(pg, true);
-	if (err)
+	if (err) {
 		dev_err(dev, "failed to turn on PM domain %s: %d\n",
 			pg->genpd.name, err);
+		goto out;
+	}
 
+	reset_control_release(pg->reset);
+
+out:
 	return err;
 }
 
@@ -669,10 +674,18 @@ static int tegra_genpd_power_off(struct generic_pm_domain *domain)
 	struct device *dev = pg->pmc->dev;
 	int err;
 
+	err = reset_control_acquire(pg->reset);
+	if (err < 0) {
+		pr_err("failed to acquire resets: %d\n", err);
+		return err;
+	}
+
 	err = tegra_powergate_power_down(pg);
-	if (err)
+	if (err) {
 		dev_err(dev, "failed to turn off PM domain %s: %d\n",
 			pg->genpd.name, err);
+		reset_control_release(pg->reset);
+	}
 
 	return err;
 }
@@ -937,20 +950,34 @@ static int tegra_powergate_of_get_resets(struct tegra_powergate *pg,
 	struct device *dev = pg->pmc->dev;
 	int err;
 
-	pg->reset = of_reset_control_array_get_exclusive(np);
+	pg->reset = of_reset_control_array_get_exclusive_released(np);
 	if (IS_ERR(pg->reset)) {
 		err = PTR_ERR(pg->reset);
 		dev_err(dev, "failed to get device resets: %d\n", err);
 		return err;
 	}
 
-	if (off)
+	err = reset_control_acquire(pg->reset);
+	if (err < 0) {
+		pr_err("failed to acquire resets: %d\n", err);
+		goto out;
+	}
+
+	if (off) {
 		err = reset_control_assert(pg->reset);
-	else
+	} else {
 		err = reset_control_deassert(pg->reset);
+		if (err < 0)
+			goto out;
 
-	if (err)
+		reset_control_release(pg->reset);
+	}
+
+out:
+	if (err) {
+		reset_control_release(pg->reset);
 		reset_control_put(pg->reset);
+	}
 
 	return err;
 }
-- 
2.20.1

[PATCH 5/5] drm/tegra: sor: Implement acquire/release for reset

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

By implementing the acquire/release protocol, the SOR reset can be
shared with other drivers that also adhere to this protocol, such as the
PMC driver that uses the same reset as part of the powergate and -ungate
implementation using generic power domains.

Runtime PM makes sure that the operations are executed in the right
order, and the reset core has error handling and WARNs in place to make
sure the acquire/release protocol is followed.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/tegra/sor.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
index 40057106f5f3..5be5a0817dfe 100644
--- a/drivers/gpu/drm/tegra/sor.c
+++ b/drivers/gpu/drm/tegra/sor.c
@@ -2871,6 +2871,13 @@ static int tegra_sor_init(struct host1x_client *client)
 	 * kernel is possible.
 	 */
 	if (sor->rst) {
+		err = reset_control_acquire(sor->rst);
+		if (err < 0) {
+			dev_err(sor->dev, "failed to acquire SOR reset: %d\n",
+				err);
+			return err;
+		}
+
 		err = reset_control_assert(sor->rst);
 		if (err < 0) {
 			dev_err(sor->dev, "failed to assert SOR reset: %d\n",
@@ -2894,6 +2901,8 @@ static int tegra_sor_init(struct host1x_client *client)
 				err);
 			return err;
 		}
+
+		reset_control_release(sor->rst);
 	}
 
 	err = clk_prepare_enable(sor->clk_safe);
@@ -3331,7 +3340,7 @@ static int tegra_sor_probe(struct platform_device *pdev)
 		goto remove;
 	}
 
-	sor->rst = devm_reset_control_get(&pdev->dev, "sor");
+	sor->rst = devm_reset_control_get_exclusive_released(&pdev->dev, "sor");
 	if (IS_ERR(sor->rst)) {
 		err = PTR_ERR(sor->rst);
 
@@ -3519,6 +3528,8 @@ static int tegra_sor_suspend(struct device *dev)
 			dev_err(dev, "failed to assert reset: %d\n", err);
 			return err;
 		}
+
+		reset_control_release(sor->rst);
 	}
 
 	usleep_range(1000, 2000);
@@ -3542,9 +3553,17 @@ static int tegra_sor_resume(struct device *dev)
 	usleep_range(1000, 2000);
 
 	if (sor->rst) {
+		err = reset_control_acquire(sor->rst);
+		if (err < 0) {
+			dev_err(dev, "failed to acquire reset: %d\n", err);
+			clk_disable_unprepare(sor->clk);
+			return err;
+		}
+
 		err = reset_control_deassert(sor->rst);
 		if (err < 0) {
 			dev_err(dev, "failed to deassert reset: %d\n", err);
+			reset_control_release(sor->rst);
 			clk_disable_unprepare(sor->clk);
 			return err;
 		}
-- 
2.20.1

Re: [PATCH 1/5] reset: add acquired/released state for exclusive reset controls

[Thierry Reding]

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

On Thu, Feb 21, 2019 at 04:25:53PM +0100, Thierry Reding wrote:
> From: Philipp Zabel <p.zabel@pengutronix.de>
> 
> There are cases where a driver needs explicit control over a reset line
> that is exclusively conneted to its device, but this control has to be
> temporarily handed over to the power domain controller to handle reset
> requirements during power transitions.
> Allow multiple exclusive reset controls to be requested in 'released'
> state for the same physical reset line, only one of which can be
> acquired at the same time.
> 
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  drivers/reset/core.c  | 139 ++++++++++++++++++++++++++++++++++++++----
>  include/linux/reset.h |  93 ++++++++++++++++++++++------
>  2 files changed, 200 insertions(+), 32 deletions(-)

Hi Philipp,

the bulk of this is unchanged relative to what you had posted
originally. I squashed in the few things that we had already discussed
earlier (EINVAL -> EPERM) and a couple of minor fixes for issues that I
found while working with this.

Attached is my fixup patch which contains all the changes I made on top
of your version and that I squashed into this.

Thierry

--- >8 ---
commit aa618d0b63eec676d9ea8db91a4c5fdc9330fc6b
Author: Thierry Reding <treding@nvidia.com>
Date:   Mon Feb 18 11:32:46 2019 +0100

    fixup! reset: add acquired/released state for exclusive reset controls

diff --git a/drivers/reset/core.c b/drivers/reset/core.c
index c6a7a4474142..1e8a42b16f23 100644
--- a/drivers/reset/core.c
+++ b/drivers/reset/core.c
@@ -65,6 +65,17 @@ struct reset_control_array {
 	struct reset_control *rstc[];
 };
 
+static const char *rcdev_name(struct reset_controller_dev *rcdev)
+{
+	if (rcdev->dev)
+		return dev_name(rcdev->dev);
+
+	if (rcdev->of_node)
+		return rcdev->of_node->full_name;
+
+	return NULL;
+}
+
 /**
  * of_reset_simple_xlate - translate reset_spec to the reset line number
  * @rcdev: a pointer to the reset controller device
@@ -276,7 +287,7 @@ int reset_control_reset(struct reset_control *rstc)
 			return 0;
 	} else {
 		if (!rstc->acquired)
-			return -EINVAL;
+			return -EPERM;
 	}
 
 	ret = rstc->rcdev->ops->reset(rstc->rcdev, rstc->id);
@@ -340,8 +351,11 @@ int reset_control_assert(struct reset_control *rstc)
 		if (!rstc->rcdev->ops->assert)
 			return -ENOTSUPP;
 
-		if (!rstc->acquired)
-			return -EINVAL;
+		if (!rstc->acquired) {
+			WARN(1, "reset %s (ID: %u) is not acquired\n",
+			     rcdev_name(rstc->rcdev), rstc->id);
+			return -EPERM;
+		}
 	}
 
 	return rstc->rcdev->ops->assert(rstc->rcdev, rstc->id);
@@ -378,8 +392,11 @@ int reset_control_deassert(struct reset_control *rstc)
 		if (atomic_inc_return(&rstc->deassert_count) != 1)
 			return 0;
 	} else {
-		if (!rstc->acquired)
-			return -EINVAL;
+		if (!rstc->acquired) {
+			WARN(1, "reset %s (ID: %u) is not acquired\n",
+			     rcdev_name(rstc->rcdev), rstc->id);
+			return -EPERM;
+		}
 	}
 
 	/*
@@ -417,15 +434,43 @@ int reset_control_status(struct reset_control *rstc)
 }
 EXPORT_SYMBOL_GPL(reset_control_status);
 
+/**
+ * reset_control_acquire() - acquires a reset control for exclusive use
+ * @rstc: reset control
+ *
+ * This is used to explicitly acquire a reset control for exclusive use. Note
+ * that exclusive resets are requested as acquired by default. In order for a
+ * second consumer to be able to control the reset, the first consumer has to
+ * release it first. Typically the easiest way to achieve this is to call the
+ * reset_control_get_exclusive_released() to obtain an instance of the reset
+ * control. Such reset controls are not acquired by default.
+ *
+ * Consumers implementing shared access to an exclusive reset need to follow
+ * a specific protocol in order to work together. Before consumers can change
+ * a reset they must acquire exclusive access using reset_control_acquire().
+ * After they are done operating the reset, they must release exclusive access
+ * with a call to reset_control_release(). Consumers are not granted exclusive
+ * access to the reset as long as another consumer hasn't released a reset.
+ *
+ * See also: reset_control_release()
+ */
 int reset_control_acquire(struct reset_control *rstc)
 {
 	struct reset_control *rc;
 
-	if (!rstc || rstc->acquired)
+	if (!rstc)
 		return 0;
 
+	if (WARN_ON(IS_ERR(rstc)))
+		return -EINVAL;
+
 	mutex_lock(&reset_list_mutex);
 
+	if (rstc->acquired) {
+		mutex_unlock(&reset_list_mutex);
+		return 0;
+	}
+
 	list_for_each_entry(rc, &rstc->rcdev->reset_control_head, list) {
 		if (rstc != rc && rstc->id == rc->id) {
 			if (rc->acquired) {
@@ -435,13 +480,28 @@ int reset_control_acquire(struct reset_control *rstc)
 		}
 	}
 
+	rstc->acquired = true;
+
 	mutex_unlock(&reset_list_mutex);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(reset_control_acquire);
 
+/**
+ * reset_control_release() - releases exclusive access to a reset control
+ * @rstc: reset control
+ *
+ * Releases exclusive access right to a reset control previously obtained by a
+ * call to reset_control_acquire(). Until a consumer calls this function, no
+ * other consumers will be granted exclusive access.
+ *
+ * See also: reset_control_acquire()
+ */
 void reset_control_release(struct reset_control *rstc)
 {
+	if (!rstc || WARN_ON(IS_ERR(rstc)))
+		return;
+
 	rstc->acquired = false;
 }
 EXPORT_SYMBOL_GPL(reset_control_release);

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

Re: [PATCH 1/5] reset: add acquired/released state for exclusive reset controls

[Thierry Reding]

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

On Thu, Feb 21, 2019 at 04:28:58PM +0100, Thierry Reding wrote:
> On Thu, Feb 21, 2019 at 04:25:53PM +0100, Thierry Reding wrote:
> > From: Philipp Zabel <p.zabel@pengutronix.de>
> > 
> > There are cases where a driver needs explicit control over a reset line
> > that is exclusively conneted to its device, but this control has to be
> > temporarily handed over to the power domain controller to handle reset
> > requirements during power transitions.
> > Allow multiple exclusive reset controls to be requested in 'released'
> > state for the same physical reset line, only one of which can be
> > acquired at the same time.
> > 
> > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> >  drivers/reset/core.c  | 139 ++++++++++++++++++++++++++++++++++++++----
> >  include/linux/reset.h |  93 ++++++++++++++++++++++------
> >  2 files changed, 200 insertions(+), 32 deletions(-)
> 
> Hi Philipp,
> 
> the bulk of this is unchanged relative to what you had posted
> originally. I squashed in the few things that we had already discussed
> earlier (EINVAL -> EPERM) and a couple of minor fixes for issues that I
> found while working with this.
> 
> Attached is my fixup patch which contains all the changes I made on top
> of your version and that I squashed into this.
> 
> Thierry

Hi Philipp,

do you have any further comments on this series?

Thierry

> --- >8 ---
> commit aa618d0b63eec676d9ea8db91a4c5fdc9330fc6b
> Author: Thierry Reding <treding@nvidia.com>
> Date:   Mon Feb 18 11:32:46 2019 +0100
> 
>     fixup! reset: add acquired/released state for exclusive reset controls
> 
> diff --git a/drivers/reset/core.c b/drivers/reset/core.c
> index c6a7a4474142..1e8a42b16f23 100644
> --- a/drivers/reset/core.c
> +++ b/drivers/reset/core.c
> @@ -65,6 +65,17 @@ struct reset_control_array {
>  	struct reset_control *rstc[];
>  };
>  
> +static const char *rcdev_name(struct reset_controller_dev *rcdev)
> +{
> +	if (rcdev->dev)
> +		return dev_name(rcdev->dev);
> +
> +	if (rcdev->of_node)
> +		return rcdev->of_node->full_name;
> +
> +	return NULL;
> +}
> +
>  /**
>   * of_reset_simple_xlate - translate reset_spec to the reset line number
>   * @rcdev: a pointer to the reset controller device
> @@ -276,7 +287,7 @@ int reset_control_reset(struct reset_control *rstc)
>  			return 0;
>  	} else {
>  		if (!rstc->acquired)
> -			return -EINVAL;
> +			return -EPERM;
>  	}
>  
>  	ret = rstc->rcdev->ops->reset(rstc->rcdev, rstc->id);
> @@ -340,8 +351,11 @@ int reset_control_assert(struct reset_control *rstc)
>  		if (!rstc->rcdev->ops->assert)
>  			return -ENOTSUPP;
>  
> -		if (!rstc->acquired)
> -			return -EINVAL;
> +		if (!rstc->acquired) {
> +			WARN(1, "reset %s (ID: %u) is not acquired\n",
> +			     rcdev_name(rstc->rcdev), rstc->id);
> +			return -EPERM;
> +		}
>  	}
>  
>  	return rstc->rcdev->ops->assert(rstc->rcdev, rstc->id);
> @@ -378,8 +392,11 @@ int reset_control_deassert(struct reset_control *rstc)
>  		if (atomic_inc_return(&rstc->deassert_count) != 1)
>  			return 0;
>  	} else {
> -		if (!rstc->acquired)
> -			return -EINVAL;
> +		if (!rstc->acquired) {
> +			WARN(1, "reset %s (ID: %u) is not acquired\n",
> +			     rcdev_name(rstc->rcdev), rstc->id);
> +			return -EPERM;
> +		}
>  	}
>  
>  	/*
> @@ -417,15 +434,43 @@ int reset_control_status(struct reset_control *rstc)
>  }
>  EXPORT_SYMBOL_GPL(reset_control_status);
>  
> +/**
> + * reset_control_acquire() - acquires a reset control for exclusive use
> + * @rstc: reset control
> + *
> + * This is used to explicitly acquire a reset control for exclusive use. Note
> + * that exclusive resets are requested as acquired by default. In order for a
> + * second consumer to be able to control the reset, the first consumer has to
> + * release it first. Typically the easiest way to achieve this is to call the
> + * reset_control_get_exclusive_released() to obtain an instance of the reset
> + * control. Such reset controls are not acquired by default.
> + *
> + * Consumers implementing shared access to an exclusive reset need to follow
> + * a specific protocol in order to work together. Before consumers can change
> + * a reset they must acquire exclusive access using reset_control_acquire().
> + * After they are done operating the reset, they must release exclusive access
> + * with a call to reset_control_release(). Consumers are not granted exclusive
> + * access to the reset as long as another consumer hasn't released a reset.
> + *
> + * See also: reset_control_release()
> + */
>  int reset_control_acquire(struct reset_control *rstc)
>  {
>  	struct reset_control *rc;
>  
> -	if (!rstc || rstc->acquired)
> +	if (!rstc)
>  		return 0;
>  
> +	if (WARN_ON(IS_ERR(rstc)))
> +		return -EINVAL;
> +
>  	mutex_lock(&reset_list_mutex);
>  
> +	if (rstc->acquired) {
> +		mutex_unlock(&reset_list_mutex);
> +		return 0;
> +	}
> +
>  	list_for_each_entry(rc, &rstc->rcdev->reset_control_head, list) {
>  		if (rstc != rc && rstc->id == rc->id) {
>  			if (rc->acquired) {
> @@ -435,13 +480,28 @@ int reset_control_acquire(struct reset_control *rstc)
>  		}
>  	}
>  
> +	rstc->acquired = true;
> +
>  	mutex_unlock(&reset_list_mutex);
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(reset_control_acquire);
>  
> +/**
> + * reset_control_release() - releases exclusive access to a reset control
> + * @rstc: reset control
> + *
> + * Releases exclusive access right to a reset control previously obtained by a
> + * call to reset_control_acquire(). Until a consumer calls this function, no
> + * other consumers will be granted exclusive access.
> + *
> + * See also: reset_control_acquire()
> + */
>  void reset_control_release(struct reset_control *rstc)
>  {
> +	if (!rstc || WARN_ON(IS_ERR(rstc)))
> +		return;
> +
>  	rstc->acquired = false;
>  }
>  EXPORT_SYMBOL_GPL(reset_control_release);



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

Re: [PATCH 1/5] reset: add acquired/released state for exclusive reset controls

[Philipp Zabel]

Hi Thierry,

On Mon, 2019-03-18 at 10:12 +0100, Thierry Reding wrote:
> On Thu, Feb 21, 2019 at 04:28:58PM +0100, Thierry Reding wrote:
> > On Thu, Feb 21, 2019 at 04:25:53PM +0100, Thierry Reding wrote:
> > > From: Philipp Zabel <p.zabel@pengutronix.de>
> > > 
> > > There are cases where a driver needs explicit control over a reset line
> > > that is exclusively conneted to its device, but this control has to be
> > > temporarily handed over to the power domain controller to handle reset
> > > requirements during power transitions.
> > > Allow multiple exclusive reset controls to be requested in 'released'
> > > state for the same physical reset line, only one of which can be
> > > acquired at the same time.
> > > 
> > > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> > > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > > ---
> > >  drivers/reset/core.c  | 139 ++++++++++++++++++++++++++++++++++++++----
> > >  include/linux/reset.h |  93 ++++++++++++++++++++++------
> > >  2 files changed, 200 insertions(+), 32 deletions(-)
> > 
> > Hi Philipp,
> > 
> > the bulk of this is unchanged relative to what you had posted
> > originally. I squashed in the few things that we had already discussed
> > earlier (EINVAL -> EPERM) and a couple of minor fixes for issues that I
> > found while working with this.
> > 
> > Attached is my fixup patch which contains all the changes I made on top
> > of your version and that I squashed into this.
> > 
> > Thierry
> 
> Hi Philipp,
> 
> do you have any further comments on this series?

Sorry for the delay, I'll have a closer look tomorrow. I obviously don't
disagree on the implementation and I appreciate the added documentation.

As for how to merge this, would you be fine with me providing a stable
branch that contains the first three patches? That could then go into
both reset/next and tegra trees.

regards
Philipp

Re: [PATCH 1/5] reset: add acquired/released state for exclusive reset controls

[Thierry Reding]

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

On Mon, Mar 18, 2019 at 05:40:50PM +0100, Philipp Zabel wrote:
> Hi Thierry,
> 
> On Mon, 2019-03-18 at 10:12 +0100, Thierry Reding wrote:
> > On Thu, Feb 21, 2019 at 04:28:58PM +0100, Thierry Reding wrote:
> > > On Thu, Feb 21, 2019 at 04:25:53PM +0100, Thierry Reding wrote:
> > > > From: Philipp Zabel <p.zabel@pengutronix.de>
> > > > 
> > > > There are cases where a driver needs explicit control over a reset line
> > > > that is exclusively conneted to its device, but this control has to be
> > > > temporarily handed over to the power domain controller to handle reset
> > > > requirements during power transitions.
> > > > Allow multiple exclusive reset controls to be requested in 'released'
> > > > state for the same physical reset line, only one of which can be
> > > > acquired at the same time.
> > > > 
> > > > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> > > > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > > > ---
> > > >  drivers/reset/core.c  | 139 ++++++++++++++++++++++++++++++++++++++----
> > > >  include/linux/reset.h |  93 ++++++++++++++++++++++------
> > > >  2 files changed, 200 insertions(+), 32 deletions(-)
> > > 
> > > Hi Philipp,
> > > 
> > > the bulk of this is unchanged relative to what you had posted
> > > originally. I squashed in the few things that we had already discussed
> > > earlier (EINVAL -> EPERM) and a couple of minor fixes for issues that I
> > > found while working with this.
> > > 
> > > Attached is my fixup patch which contains all the changes I made on top
> > > of your version and that I squashed into this.
> > > 
> > > Thierry
> > 
> > Hi Philipp,
> > 
> > do you have any further comments on this series?
> 
> Sorry for the delay, I'll have a closer look tomorrow. I obviously don't
> disagree on the implementation and I appreciate the added documentation.
> 
> As for how to merge this, would you be fine with me providing a stable
> branch that contains the first three patches? That could then go into
> both reset/next and tegra trees.

Yeah, that's fine with me.

Thanks,
Thierry

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

Re: [PATCH 1/5] reset: add acquired/released state for exclusive reset controls

[Philipp Zabel]

Hi Thierry,

On Thu, 2019-02-21 at 16:28 +0100, Thierry Reding wrote:
> On Thu, Feb 21, 2019 at 04:25:53PM +0100, Thierry Reding wrote:
> > From: Philipp Zabel <p.zabel@pengutronix.de>
> > 
> > There are cases where a driver needs explicit control over a reset line
> > that is exclusively conneted to its device, but this control has to be
> > temporarily handed over to the power domain controller to handle reset
> > requirements during power transitions.
> > Allow multiple exclusive reset controls to be requested in 'released'
> > state for the same physical reset line, only one of which can be
> > acquired at the same time.
> > 
> > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> >  drivers/reset/core.c  | 139 ++++++++++++++++++++++++++++++++++++++----
> >  include/linux/reset.h |  93 ++++++++++++++++++++++------
> >  2 files changed, 200 insertions(+), 32 deletions(-)
> 
> Hi Philipp,
> 
> the bulk of this is unchanged relative to what you had posted
> originally. I squashed in the few things that we had already discussed
> earlier (EINVAL -> EPERM) and a couple of minor fixes for issues that I
> found while working with this.
> 
> Attached is my fixup patch which contains all the changes I made on top
> of your version and that I squashed into this.

I have no further comments on this, I find all these changes either
necessary or useful.

regards
Philipp

Re: [PATCH 2/5] reset: Add acquired flag to of_reset_control_array_get()

[Philipp Zabel]

On Thu, 2019-02-21 at 16:25 +0100, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> In order to be able to request an array of reset controls in acquired or
> released mode, add the acquired flag to of_reset_control_array_get() and
> pass the flag to subsequent calls of __of_reset_control_get().
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  drivers/reset/core.c              |  9 ++++++---
>  drivers/usb/dwc3/dwc3-of-simple.c |  3 ++-
>  include/linux/reset.h             | 14 ++++++++------
>  3 files changed, 16 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/reset/core.c b/drivers/reset/core.c
> index 1e8a42b16f23..f94da91c22af 100644
> --- a/drivers/reset/core.c
> +++ b/drivers/reset/core.c
> @@ -830,12 +830,15 @@ static int of_reset_control_get_count(struct device_node *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
> + * @acquired: only one reset control may be acquired for a given controller
> + *            and ID
>   *
>   * 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)
> +of_reset_control_array_get(struct device_node *np, bool shared, bool optional,
> +			   bool acquired)
>  {
>  	struct reset_control_array *resets;
>  	struct reset_control *rstc;
> @@ -851,7 +854,7 @@ of_reset_control_array_get(struct device_node *np, bool shared, bool optional)
>  
>  	for (i = 0; i < num; i++) {
>  		rstc = __of_reset_control_get(np, NULL, i, shared, optional,
> -					      true);
> +					      acquired);
>  		if (IS_ERR(rstc))
>  			goto err_rst;
>  		resets->rstc[i] = rstc;
> @@ -898,7 +901,7 @@ devm_reset_control_array_get(struct device *dev, bool shared, bool optional)
>  	if (!devres)
>  		return ERR_PTR(-ENOMEM);
>  
> -	rstc = of_reset_control_array_get(dev->of_node, shared, optional);
> +	rstc = of_reset_control_array_get(dev->of_node, shared, optional, true);
>  	if (IS_ERR(rstc)) {
>  		devres_free(devres);
>  		return rstc;
> diff --git a/drivers/usb/dwc3/dwc3-of-simple.c b/drivers/usb/dwc3/dwc3-of-simple.c
> index 4c2771c5e727..67ce2037472d 100644
> --- a/drivers/usb/dwc3/dwc3-of-simple.c
> +++ b/drivers/usb/dwc3/dwc3-of-simple.c
> @@ -107,7 +107,8 @@ static int dwc3_of_simple_probe(struct platform_device *pdev)
>  		simple->pulse_resets = true;
>  	}
>  
> -	simple->resets = of_reset_control_array_get(np, shared_resets, true);
> +	simple->resets = of_reset_control_array_get(np, shared_resets, true,
> +						    true);

Felipe, could I get your acked-by to merge this through the reset tree?

dwc3-of-simple is a bit of a special case because it calls
of_reset_control_array_get directly instead of through the
of_reset_control_array_get_shared/exclusive wrappers.

regards
Philipp

Re: [PATCH 3/5] reset: Add acquire/release support for arrays

[Philipp Zabel]

On Thu, 2019-02-21 at 16:25 +0100, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> Add implementations that apply acquire and release operations to all
> reset controls part of a reset control array.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>

Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>

> ---
>  drivers/reset/core.c  | 36 +++++++++++++++++++++++++++++++++++-
>  include/linux/reset.h |  6 ++++++
>  2 files changed, 41 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/reset/core.c b/drivers/reset/core.c
> index f94da91c22af..81ea77cba123 100644
> --- a/drivers/reset/core.c
> +++ b/drivers/reset/core.c
> @@ -245,6 +245,34 @@ static int reset_control_array_deassert(struct reset_control_array *resets)
>  	return ret;
>  }
>  
> +static int reset_control_array_acquire(struct reset_control_array *resets)
> +{
> +	unsigned int i;
> +	int err;
> +
> +	for (i = 0; i < resets->num_rstcs; i++) {
> +		err = reset_control_acquire(resets->rstc[i]);
> +		if (err < 0)
> +			goto release;
> +	}

This could be done under a single mutex lock by extracting a free
version of of reset_control_acquire. Just an idea though, not sure if
that would have any measurable benefit.

regards
Philipp

Re: [PATCH 4/5] soc/tegra: pmc: Implement acquire/release for resets

[Philipp Zabel]

On Thu, 2019-02-21 at 16:25 +0100, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> By implementing the acquire/release protocol, the resets can be shared
> with other drivers that also adhere to this protocol. This will be used
> for example by the SOR driver to put hardware into a known good state,
> irrespective of whether or not the power domain can be reset.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  drivers/soc/tegra/pmc.c | 39 +++++++++++++++++++++++++++++++++------
>  1 file changed, 33 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
> index 0df258518693..0c5f79528e5f 100644
> --- a/drivers/soc/tegra/pmc.c
> +++ b/drivers/soc/tegra/pmc.c
> @@ -656,10 +656,15 @@ static int tegra_genpd_power_on(struct generic_pm_domain *domain)
>  	int err;
>  
>  	err = tegra_powergate_power_up(pg, true);
> -	if (err)
> +	if (err) {
>  		dev_err(dev, "failed to turn on PM domain %s: %d\n",
>  			pg->genpd.name, err);
> +		goto out;
> +	}
>  
> +	reset_control_release(pg->reset);
> +
> +out:
>  	return err;
>  }
>  
> @@ -669,10 +674,18 @@ static int tegra_genpd_power_off(struct generic_pm_domain *domain)
>  	struct device *dev = pg->pmc->dev;
>  	int err;
>  
> +	err = reset_control_acquire(pg->reset);
> +	if (err < 0) {
> +		pr_err("failed to acquire resets: %d\n", err);
> +		return err;
> +	}
> +
>  	err = tegra_powergate_power_down(pg);
> -	if (err)
> +	if (err) {
>  		dev_err(dev, "failed to turn off PM domain %s: %d\n",
>  			pg->genpd.name, err);
> +		reset_control_release(pg->reset);
> +	}
>  
>  	return err;
>  }
> @@ -937,20 +950,34 @@ static int tegra_powergate_of_get_resets(struct tegra_powergate *pg,
>  	struct device *dev = pg->pmc->dev;
>  	int err;
>  
> -	pg->reset = of_reset_control_array_get_exclusive(np);
> +	pg->reset = of_reset_control_array_get_exclusive_released(np);
>  	if (IS_ERR(pg->reset)) {
>  		err = PTR_ERR(pg->reset);
>  		dev_err(dev, "failed to get device resets: %d\n", err);
>  		return err;
>  	}
>  
> -	if (off)
> +	err = reset_control_acquire(pg->reset);
> +	if (err < 0) {
> +		pr_err("failed to acquire resets: %d\n", err);
> +		goto out;
> +	}
> +
> +	if (off) {
>  		err = reset_control_assert(pg->reset);
> -	else
> +	} else {
>  		err = reset_control_deassert(pg->reset);
> +		if (err < 0)
> +			goto out;
>  
> -	if (err)
> +		reset_control_release(pg->reset);
> +	}
> +
> +out:
> +	if (err) {
> +		reset_control_release(pg->reset);
>  		reset_control_put(pg->reset);
> +	}
>  
>  	return err;
>  }

Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>

regards
Philipp

Re: [PATCH 5/5] drm/tegra: sor: Implement acquire/release for reset

[Philipp Zabel]

On Thu, 2019-02-21 at 16:25 +0100, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> By implementing the acquire/release protocol, the SOR reset can be
> shared with other drivers that also adhere to this protocol, such as the
> PMC driver that uses the same reset as part of the powergate and -ungate
> implementation using generic power domains.
> 
> Runtime PM makes sure that the operations are executed in the right
> order, and the reset core has error handling and WARNs in place to make
> sure the acquire/release protocol is followed.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  drivers/gpu/drm/tegra/sor.c | 21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
> index 40057106f5f3..5be5a0817dfe 100644
> --- a/drivers/gpu/drm/tegra/sor.c
> +++ b/drivers/gpu/drm/tegra/sor.c
> @@ -2871,6 +2871,13 @@ static int tegra_sor_init(struct host1x_client *client)
>  	 * kernel is possible.
>  	 */
>  	if (sor->rst) {
> +		err = reset_control_acquire(sor->rst);
> +		if (err < 0) {
> +			dev_err(sor->dev, "failed to acquire SOR reset: %d\n",
> +				err);
> +			return err;
> +		}
> +
>  		err = reset_control_assert(sor->rst);
>  		if (err < 0) {
>  			dev_err(sor->dev, "failed to assert SOR reset: %d\n",
> @@ -2894,6 +2901,8 @@ static int tegra_sor_init(struct host1x_client *client)
>  				err);
>  			return err;
>  		}
> +
> +		reset_control_release(sor->rst);
>  	}
>  
>  	err = clk_prepare_enable(sor->clk_safe);
> @@ -3331,7 +3340,7 @@ static int tegra_sor_probe(struct platform_device *pdev)
>  		goto remove;
>  	}
>  
> -	sor->rst = devm_reset_control_get(&pdev->dev, "sor");
> +	sor->rst = devm_reset_control_get_exclusive_released(&pdev->dev, "sor");
>  	if (IS_ERR(sor->rst)) {
>  		err = PTR_ERR(sor->rst);
>  
> @@ -3519,6 +3528,8 @@ static int tegra_sor_suspend(struct device *dev)
>  			dev_err(dev, "failed to assert reset: %d\n", err);
>  			return err;
>  		}
> +
> +		reset_control_release(sor->rst);
>  	}
>  
>  	usleep_range(1000, 2000);
> @@ -3542,9 +3553,17 @@ static int tegra_sor_resume(struct device *dev)
>  	usleep_range(1000, 2000);
>  
>  	if (sor->rst) {
> +		err = reset_control_acquire(sor->rst);
> +		if (err < 0) {
> +			dev_err(dev, "failed to acquire reset: %d\n", err);
> +			clk_disable_unprepare(sor->clk);
> +			return err;
> +		}
> +
>  		err = reset_control_deassert(sor->rst);
>  		if (err < 0) {
>  			dev_err(dev, "failed to deassert reset: %d\n", err);
> +			reset_control_release(sor->rst);
>  			clk_disable_unprepare(sor->clk);
>  			return err;
>  		}

Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>

regards
Philipp

Re: [PATCH 2/5] reset: Add acquired flag to of_reset_control_array_get()

[Felipe Balbi]

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

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

> On Thu, 2019-02-21 at 16:25 +0100, Thierry Reding wrote:
>> From: Thierry Reding <treding@nvidia.com>
>> 
>> In order to be able to request an array of reset controls in acquired or
>> released mode, add the acquired flag to of_reset_control_array_get() and
>> pass the flag to subsequent calls of __of_reset_control_get().
>> 
>> Signed-off-by: Thierry Reding <treding@nvidia.com>
>> ---
>>  drivers/reset/core.c              |  9 ++++++---
>>  drivers/usb/dwc3/dwc3-of-simple.c |  3 ++-
>>  include/linux/reset.h             | 14 ++++++++------
>>  3 files changed, 16 insertions(+), 10 deletions(-)
>> 
>> diff --git a/drivers/reset/core.c b/drivers/reset/core.c
>> index 1e8a42b16f23..f94da91c22af 100644
>> --- a/drivers/reset/core.c
>> +++ b/drivers/reset/core.c
>> @@ -830,12 +830,15 @@ static int of_reset_control_get_count(struct device_node *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
>> + * @acquired: only one reset control may be acquired for a given controller
>> + *            and ID
>>   *
>>   * 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)
>> +of_reset_control_array_get(struct device_node *np, bool shared, bool optional,
>> +			   bool acquired)
>>  {
>>  	struct reset_control_array *resets;
>>  	struct reset_control *rstc;
>> @@ -851,7 +854,7 @@ of_reset_control_array_get(struct device_node *np, bool shared, bool optional)
>>  
>>  	for (i = 0; i < num; i++) {
>>  		rstc = __of_reset_control_get(np, NULL, i, shared, optional,
>> -					      true);
>> +					      acquired);
>>  		if (IS_ERR(rstc))
>>  			goto err_rst;
>>  		resets->rstc[i] = rstc;
>> @@ -898,7 +901,7 @@ devm_reset_control_array_get(struct device *dev, bool shared, bool optional)
>>  	if (!devres)
>>  		return ERR_PTR(-ENOMEM);
>>  
>> -	rstc = of_reset_control_array_get(dev->of_node, shared, optional);
>> +	rstc = of_reset_control_array_get(dev->of_node, shared, optional, true);
>>  	if (IS_ERR(rstc)) {
>>  		devres_free(devres);
>>  		return rstc;
>> diff --git a/drivers/usb/dwc3/dwc3-of-simple.c b/drivers/usb/dwc3/dwc3-of-simple.c
>> index 4c2771c5e727..67ce2037472d 100644
>> --- a/drivers/usb/dwc3/dwc3-of-simple.c
>> +++ b/drivers/usb/dwc3/dwc3-of-simple.c
>> @@ -107,7 +107,8 @@ static int dwc3_of_simple_probe(struct platform_device *pdev)
>>  		simple->pulse_resets = true;
>>  	}
>>  
>> -	simple->resets = of_reset_control_array_get(np, shared_resets, true);
>> +	simple->resets = of_reset_control_array_get(np, shared_resets, true,
>> +						    true);
>
> Felipe, could I get your acked-by to merge this through the reset tree?
>
> dwc3-of-simple is a bit of a special case because it calls
> of_reset_control_array_get directly instead of through the
> of_reset_control_array_get_shared/exclusive wrappers.

Sure thing:

Acked-by: Felipe Balbi <felipe.balbi@linux.intel.com>

-- 
balbi

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

Re: [PATCH 2/5] reset: Add acquired flag to of_reset_control_array_get()

[Philipp Zabel]

On Wed, 2019-03-20 at 08:51 +0200, Felipe Balbi wrote:
> Philipp Zabel <p.zabel@pengutronix.de> writes:
[...]
> > Felipe, could I get your acked-by to merge this through the reset tree?
> > 
> > dwc3-of-simple is a bit of a special case because it calls
> > of_reset_control_array_get directly instead of through the
> > of_reset_control_array_get_shared/exclusive wrappers.
> 
> Sure thing:
> 
> Acked-by: Felipe Balbi <felipe.balbi@linux.intel.com>

Thank you, I've added patches 1-3 to reset/next and named a stable
branch that could be used as a dependency for patches 4-5:

  git://git.pengutronix.de/pza/linux reset/acquire

regards
Philipp