linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RFC 0/3] reset: Requesting pre-deasserted, auto-reasserting reset controls via devres
@ 2024-06-21 14:45 Philipp Zabel
  2024-06-21 14:45 ` [PATCH RFC 1/3] reset: replace boolean parameters with flags parameter Philipp Zabel
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Philipp Zabel @ 2024-06-21 14:45 UTC (permalink / raw)
  To: Kunihiko Hayashi, Masami Hiramatsu, Lad Prabhakar,
	Geert Uytterhoeven, Uwe Kleine-König
  Cc: linux-kernel, linux-arm-kernel, kernel, Philipp Zabel

There is a recurring pattern of drivers requesting a reset control and
deasserting the reset during probe, followed by registering a reset
assertion via devm_add_action_or_reset().

We can simplify this by providing devm_reset_control_get_*_deasserted()
helpers that return an already deasserted reset control, similarly to
devm_clk_get_enabled().

This doesn't remove a lot of boilerplate at each instance, but there are
quite a few of them now.

regards
Philipp

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
Philipp Zabel (3):
      reset: replace boolean parameters with flags parameter
      reset: Add devres helpers to request pre-deasserted reset controls
      reset: uniphier-glue: Use devm_reset_control_bulk_get_shared_deasserted()

 drivers/reset/core.c                | 103 ++++++++++----
 drivers/reset/reset-uniphier-glue.c |  24 +---
 include/linux/reset.h               | 274 ++++++++++++++++++++++++++++--------
 3 files changed, 289 insertions(+), 112 deletions(-)
---
base-commit: 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0
change-id: 20240621-reset-get-deasserted-5185a8e4a706

Best regards,
-- 
Philipp Zabel <p.zabel@pengutronix.de>



^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH RFC 1/3] reset: replace boolean parameters with flags parameter
  2024-06-21 14:45 [PATCH RFC 0/3] reset: Requesting pre-deasserted, auto-reasserting reset controls via devres Philipp Zabel
@ 2024-06-21 14:45 ` Philipp Zabel
  2024-06-22  7:47   ` Uwe Kleine-König
  2024-06-21 14:45 ` [PATCH RFC 2/3] reset: Add devres helpers to request pre-deasserted reset controls Philipp Zabel
  2024-06-21 14:45 ` [PATCH RFC 3/3] reset: uniphier-glue: Use devm_reset_control_bulk_get_shared_deasserted() Philipp Zabel
  2 siblings, 1 reply; 8+ messages in thread
From: Philipp Zabel @ 2024-06-21 14:45 UTC (permalink / raw)
  To: Kunihiko Hayashi, Masami Hiramatsu, Lad Prabhakar,
	Geert Uytterhoeven, Uwe Kleine-König
  Cc: linux-kernel, linux-arm-kernel, kernel, Philipp Zabel

Introduce enum reset_control_flags and replace the list of boolean
parameters to the internal reset_control_get functions with a single
flags parameter, before adding more boolean options.

The separate boolean parameters have been shown to be error prone in
the past. See for example commit a57f68ddc886 ("reset: Fix devm bulk
optional exclusive control getter").

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/reset/core.c  |  59 +++++++++---------
 include/linux/reset.h | 161 +++++++++++++++++++++++++++++++-------------------
 2 files changed, 129 insertions(+), 91 deletions(-)

diff --git a/drivers/reset/core.c b/drivers/reset/core.c
index dba74e857be6..3658fe089147 100644
--- a/drivers/reset/core.c
+++ b/drivers/reset/core.c
@@ -773,8 +773,10 @@ EXPORT_SYMBOL_GPL(reset_control_bulk_release);
 
 static struct reset_control *
 __reset_control_get_internal(struct reset_controller_dev *rcdev,
-			     unsigned int index, bool shared, bool acquired)
+			     unsigned int index, enum reset_control_flags flags)
 {
+	bool shared = flags & RESET_CONTROL_FLAGS_BIT_SHARED;
+	bool acquired = flags & RESET_CONTROL_FLAGS_BIT_ACQUIRED;
 	struct reset_control *rstc;
 
 	lockdep_assert_held(&reset_list_mutex);
@@ -999,8 +1001,9 @@ static struct reset_controller_dev *__reset_find_rcdev(const struct of_phandle_a
 
 struct reset_control *
 __of_reset_control_get(struct device_node *node, const char *id, int index,
-		       bool shared, bool optional, bool acquired)
+		       enum reset_control_flags flags)
 {
+	bool optional = flags & RESET_CONTROL_FLAGS_BIT_OPTIONAL;
 	bool gpio_fallback = false;
 	struct reset_control *rstc;
 	struct reset_controller_dev *rcdev;
@@ -1065,7 +1068,7 @@ __of_reset_control_get(struct device_node *node, const char *id, int index,
 	}
 
 	/* reset_list_mutex also protects the rcdev's reset_control list */
-	rstc = __reset_control_get_internal(rcdev, rstc_id, shared, acquired);
+	rstc = __reset_control_get_internal(rcdev, rstc_id, flags);
 
 out_unlock:
 	mutex_unlock(&reset_list_mutex);
@@ -1096,8 +1099,9 @@ __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 acquired)
+				enum reset_control_flags flags)
 {
+	bool optional = flags & RESET_CONTROL_FLAGS_BIT_OPTIONAL;
 	const struct reset_control_lookup *lookup;
 	struct reset_controller_dev *rcdev;
 	const char *dev_id = dev_name(dev);
@@ -1123,7 +1127,7 @@ __reset_control_get_from_lookup(struct device *dev, const char *con_id,
 
 			rstc = __reset_control_get_internal(rcdev,
 							    lookup->index,
-							    shared, acquired);
+							    flags);
 			mutex_unlock(&reset_list_mutex);
 			break;
 		}
@@ -1138,30 +1142,29 @@ __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,
-					  bool acquired)
+					  int index, enum reset_control_flags flags)
 {
+	bool shared = flags & RESET_CONTROL_FLAGS_BIT_SHARED;
+	bool acquired = flags & RESET_CONTROL_FLAGS_BIT_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, acquired);
+		return __of_reset_control_get(dev->of_node, id, index, flags);
 
-	return __reset_control_get_from_lookup(dev, id, shared, optional,
-					       acquired);
+	return __reset_control_get_from_lookup(dev, id, flags);
 }
 EXPORT_SYMBOL_GPL(__reset_control_get);
 
 int __reset_control_bulk_get(struct device *dev, int num_rstcs,
 			     struct reset_control_bulk_data *rstcs,
-			     bool shared, bool optional, bool acquired)
+			     enum reset_control_flags flags)
 {
 	int ret, i;
 
 	for (i = 0; i < num_rstcs; i++) {
-		rstcs[i].rstc = __reset_control_get(dev, rstcs[i].id, 0,
-						    shared, optional, acquired);
+		rstcs[i].rstc = __reset_control_get(dev, rstcs[i].id, 0, flags);
 		if (IS_ERR(rstcs[i].rstc)) {
 			ret = PTR_ERR(rstcs[i].rstc);
 			goto err;
@@ -1231,7 +1234,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 acquired)
+			 enum reset_control_flags flags)
 {
 	struct reset_control **ptr, *rstc;
 
@@ -1240,7 +1243,7 @@ __devm_reset_control_get(struct device *dev, const char *id, int index,
 	if (!ptr)
 		return ERR_PTR(-ENOMEM);
 
-	rstc = __reset_control_get(dev, id, index, shared, optional, acquired);
+	rstc = __reset_control_get(dev, id, index, flags);
 	if (IS_ERR_OR_NULL(rstc)) {
 		devres_free(ptr);
 		return rstc;
@@ -1267,7 +1270,7 @@ static void devm_reset_control_bulk_release(struct device *dev, void *res)
 
 int __devm_reset_control_bulk_get(struct device *dev, int num_rstcs,
 				  struct reset_control_bulk_data *rstcs,
-				  bool shared, bool optional, bool acquired)
+				  enum reset_control_flags flags)
 {
 	struct reset_control_bulk_devres *ptr;
 	int ret;
@@ -1277,7 +1280,7 @@ int __devm_reset_control_bulk_get(struct device *dev, int num_rstcs,
 	if (!ptr)
 		return -ENOMEM;
 
-	ret = __reset_control_bulk_get(dev, num_rstcs, rstcs, shared, optional, acquired);
+	ret = __reset_control_bulk_get(dev, num_rstcs, rstcs, flags);
 	if (ret < 0) {
 		devres_free(ptr);
 		return ret;
@@ -1303,6 +1306,7 @@ EXPORT_SYMBOL_GPL(__devm_reset_control_bulk_get);
  */
 int __device_reset(struct device *dev, bool optional)
 {
+	enum reset_control_flags flags;
 	struct reset_control *rstc;
 	int ret;
 
@@ -1318,7 +1322,8 @@ int __device_reset(struct device *dev, bool optional)
 	}
 #endif
 
-	rstc = __reset_control_get(dev, NULL, 0, 0, optional, true);
+	flags = optional ? RESET_CONTROL_OPTIONAL_EXCLUSIVE : RESET_CONTROL_EXCLUSIVE;
+	rstc = __reset_control_get(dev, NULL, 0, flags);
 	if (IS_ERR(rstc))
 		return PTR_ERR(rstc);
 
@@ -1361,17 +1366,14 @@ static int of_reset_control_get_count(struct device_node *node)
  *				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
- * @acquired: only one reset control may be acquired for a given controller
- *            and ID
+ * @flags: whether reset controls are shared, optional, acquired
  *
  * Returns pointer to allocated reset_control on success or error on failure
  */
 struct reset_control *
-of_reset_control_array_get(struct device_node *np, bool shared, bool optional,
-			   bool acquired)
+of_reset_control_array_get(struct device_node *np, enum reset_control_flags flags)
 {
+	bool optional = flags & RESET_CONTROL_FLAGS_BIT_OPTIONAL;
 	struct reset_control_array *resets;
 	struct reset_control *rstc;
 	int num, i;
@@ -1386,8 +1388,7 @@ of_reset_control_array_get(struct device_node *np, bool shared, bool optional,
 	resets->num_rstcs = num;
 
 	for (i = 0; i < num; i++) {
-		rstc = __of_reset_control_get(np, NULL, i, shared, optional,
-					      acquired);
+		rstc = __of_reset_control_get(np, NULL, i, flags);
 		if (IS_ERR(rstc))
 			goto err_rst;
 		resets->rstc[i] = rstc;
@@ -1422,7 +1423,7 @@ EXPORT_SYMBOL_GPL(of_reset_control_array_get);
  * Returns pointer to allocated reset_control on success or error on failure
  */
 struct reset_control *
-devm_reset_control_array_get(struct device *dev, bool shared, bool optional)
+devm_reset_control_array_get(struct device *dev, enum reset_control_flags flags)
 {
 	struct reset_control **ptr, *rstc;
 
@@ -1431,7 +1432,7 @@ devm_reset_control_array_get(struct device *dev, bool shared, bool optional)
 	if (!ptr)
 		return ERR_PTR(-ENOMEM);
 
-	rstc = of_reset_control_array_get(dev->of_node, shared, optional, true);
+	rstc = of_reset_control_array_get(dev->of_node, flags);
 	if (IS_ERR_OR_NULL(rstc)) {
 		devres_free(ptr);
 		return rstc;
diff --git a/include/linux/reset.h b/include/linux/reset.h
index 514ddf003efc..99296af98f81 100644
--- a/include/linux/reset.h
+++ b/include/linux/reset.h
@@ -25,6 +25,33 @@ struct reset_control_bulk_data {
 	struct reset_control		*rstc;
 };
 
+#define RESET_CONTROL_FLAGS_BIT_SHARED		BIT(0)	/* not exclusive */
+#define RESET_CONTROL_FLAGS_BIT_OPTIONAL	BIT(1)
+#define RESET_CONTROL_FLAGS_BIT_ACQUIRED	BIT(2)	/* iff exclusive, not released */
+
+/**
+ * enum reset_control_flags - Flags that can be passed to the reset_control_get functions
+ *                    to determine the type of reset control.
+ *                    These values cannot be OR'd.
+ *
+ * @RESET_CONTROL_EXCLUSIVE:				exclusive, acquired,
+ * @RESET_CONTROL_EXCLUSIVE_RELEASED:			exclusive, released,
+ * @RESET_CONTROL_SHARED:				shared
+ * @RESET_CONTROL_OPTIONAL_EXCLUSIVE:			optional, exclusive, acquired
+ * @RESET_CONTROL_OPTIONAL_EXCLUSIVE_RELEASED:		optional, exclusive, released
+ * @RESET_CONTROL_OPTIONAL_SHARED:			optional, shared
+ */
+enum reset_control_flags {
+	RESET_CONTROL_EXCLUSIVE				= RESET_CONTROL_FLAGS_BIT_ACQUIRED,
+	RESET_CONTROL_EXCLUSIVE_RELEASED		= 0,
+	RESET_CONTROL_SHARED				= RESET_CONTROL_FLAGS_BIT_SHARED,
+	RESET_CONTROL_OPTIONAL_EXCLUSIVE		= RESET_CONTROL_FLAGS_BIT_OPTIONAL |
+							  RESET_CONTROL_FLAGS_BIT_ACQUIRED,
+	RESET_CONTROL_OPTIONAL_EXCLUSIVE_RELEASED	= RESET_CONTROL_FLAGS_BIT_OPTIONAL,
+	RESET_CONTROL_OPTIONAL_SHARED			= RESET_CONTROL_FLAGS_BIT_OPTIONAL |
+							  RESET_CONTROL_FLAGS_BIT_SHARED,
+};
+
 #ifdef CONFIG_RESET_CONTROLLER
 
 int reset_control_reset(struct reset_control *rstc);
@@ -42,30 +69,25 @@ int reset_control_bulk_acquire(int num_rstcs, struct reset_control_bulk_data *rs
 void reset_control_bulk_release(int num_rstcs, struct reset_control_bulk_data *rstcs);
 
 struct reset_control *__of_reset_control_get(struct device_node *node,
-				     const char *id, int index, bool shared,
-				     bool optional, bool acquired);
+				     const char *id, int index, enum reset_control_flags flags);
 struct reset_control *__reset_control_get(struct device *dev, const char *id,
-					  int index, bool shared,
-					  bool optional, bool acquired);
+					  int index, enum reset_control_flags flags);
 void reset_control_put(struct reset_control *rstc);
 int __reset_control_bulk_get(struct device *dev, int num_rstcs,
 			     struct reset_control_bulk_data *rstcs,
-			     bool shared, bool optional, bool acquired);
+			     enum reset_control_flags flags);
 void reset_control_bulk_put(int num_rstcs, struct reset_control_bulk_data *rstcs);
 
 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 acquired);
+				     const char *id, int index, enum reset_control_flags flags);
 int __devm_reset_control_bulk_get(struct device *dev, int num_rstcs,
 				  struct reset_control_bulk_data *rstcs,
-				  bool shared, bool optional, bool acquired);
+				  enum reset_control_flags flags);
 
 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 acquired);
+						   enum reset_control_flags flags);
+struct reset_control *of_reset_control_array_get(struct device_node *np, enum reset_control_flags);
 
 int reset_control_get_count(struct device *dev);
 
@@ -116,17 +138,19 @@ 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 acquired)
+					const char *id, int index, enum reset_control_flags flags)
 {
+	bool optional = flags & RESET_CONTROL_FLAGS_BIT_OPTIONAL;
+
 	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,
-					bool acquired)
+					int index, enum reset_control_flags flags)
 {
+	bool optional = flags & RESET_CONTROL_FLAGS_BIT_OPTIONAL;
+
 	return optional ? NULL : ERR_PTR(-ENOTSUPP);
 }
 
@@ -162,8 +186,10 @@ reset_control_bulk_release(int num_rstcs, struct reset_control_bulk_data *rstcs)
 static inline int
 __reset_control_bulk_get(struct device *dev, int num_rstcs,
 			 struct reset_control_bulk_data *rstcs,
-			 bool shared, bool optional, bool acquired)
+			 enum reset_control_flags flags)
 {
+	bool optional = flags & RESET_CONTROL_FLAGS_BIT_OPTIONAL;
+
 	return optional ? 0 : -EOPNOTSUPP;
 }
 
@@ -174,30 +200,36 @@ reset_control_bulk_put(int num_rstcs, struct reset_control_bulk_data *rstcs)
 
 static inline struct reset_control *__devm_reset_control_get(
 					struct device *dev, const char *id,
-					int index, bool shared, bool optional,
-					bool acquired)
+					int index, enum reset_control_flags flags)
 {
+	bool optional = flags & RESET_CONTROL_FLAGS_BIT_OPTIONAL;
+
 	return optional ? NULL : ERR_PTR(-ENOTSUPP);
 }
 
 static inline int
 __devm_reset_control_bulk_get(struct device *dev, int num_rstcs,
 			      struct reset_control_bulk_data *rstcs,
-			      bool shared, bool optional, bool acquired)
+			      enum reset_control_flags flags)
 {
+	bool optional = flags & RESET_CONTROL_FLAGS_BIT_OPTIONAL;
+
 	return optional ? 0 : -EOPNOTSUPP;
 }
 
 static inline struct reset_control *
-devm_reset_control_array_get(struct device *dev, bool shared, bool optional)
+devm_reset_control_array_get(struct device *dev, enum reset_control_flags flags)
 {
+	bool optional = flags & RESET_CONTROL_FLAGS_BIT_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,
-			   bool acquired)
+of_reset_control_array_get(struct device_node *np, enum reset_control_flags flags)
 {
+	bool optional = flags & RESET_CONTROL_FLAGS_BIT_OPTIONAL;
+
 	return optional ? NULL : ERR_PTR(-ENOTSUPP);
 }
 
@@ -236,7 +268,7 @@ 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, true);
+	return __reset_control_get(dev, id, 0, RESET_CONTROL_EXCLUSIVE);
 }
 
 /**
@@ -253,7 +285,7 @@ static inline int __must_check
 reset_control_bulk_get_exclusive(struct device *dev, int num_rstcs,
 				 struct reset_control_bulk_data *rstcs)
 {
-	return __reset_control_bulk_get(dev, num_rstcs, rstcs, false, false, true);
+	return __reset_control_bulk_get(dev, num_rstcs, rstcs, RESET_CONTROL_EXCLUSIVE);
 }
 
 /**
@@ -274,7 +306,7 @@ 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);
+	return __reset_control_get(dev, id, 0, RESET_CONTROL_EXCLUSIVE_RELEASED);
 }
 
 /**
@@ -295,7 +327,7 @@ static inline int __must_check
 reset_control_bulk_get_exclusive_released(struct device *dev, int num_rstcs,
 					  struct reset_control_bulk_data *rstcs)
 {
-	return __reset_control_bulk_get(dev, num_rstcs, rstcs, false, false, false);
+	return __reset_control_bulk_get(dev, num_rstcs, rstcs, RESET_CONTROL_EXCLUSIVE_RELEASED);
 }
 
 /**
@@ -316,7 +348,8 @@ static inline int __must_check
 reset_control_bulk_get_optional_exclusive_released(struct device *dev, int num_rstcs,
 						   struct reset_control_bulk_data *rstcs)
 {
-	return __reset_control_bulk_get(dev, num_rstcs, rstcs, false, true, false);
+	return __reset_control_bulk_get(dev, num_rstcs, rstcs,
+					RESET_CONTROL_OPTIONAL_EXCLUSIVE_RELEASED);
 }
 
 /**
@@ -344,7 +377,7 @@ reset_control_bulk_get_optional_exclusive_released(struct device *dev, int num_r
 static inline struct reset_control *reset_control_get_shared(
 					struct device *dev, const char *id)
 {
-	return __reset_control_get(dev, id, 0, true, false, false);
+	return __reset_control_get(dev, id, 0, RESET_CONTROL_SHARED);
 }
 
 /**
@@ -361,7 +394,7 @@ static inline int __must_check
 reset_control_bulk_get_shared(struct device *dev, int num_rstcs,
 			      struct reset_control_bulk_data *rstcs)
 {
-	return __reset_control_bulk_get(dev, num_rstcs, rstcs, true, false, false);
+	return __reset_control_bulk_get(dev, num_rstcs, rstcs, RESET_CONTROL_SHARED);
 }
 
 /**
@@ -378,7 +411,7 @@ reset_control_bulk_get_shared(struct device *dev, int num_rstcs,
 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, true);
+	return __reset_control_get(dev, id, 0, RESET_CONTROL_OPTIONAL_EXCLUSIVE);
 }
 
 /**
@@ -398,7 +431,7 @@ static inline int __must_check
 reset_control_bulk_get_optional_exclusive(struct device *dev, int num_rstcs,
 					  struct reset_control_bulk_data *rstcs)
 {
-	return __reset_control_bulk_get(dev, num_rstcs, rstcs, false, true, true);
+	return __reset_control_bulk_get(dev, num_rstcs, rstcs, RESET_CONTROL_OPTIONAL_EXCLUSIVE);
 }
 
 /**
@@ -415,7 +448,7 @@ reset_control_bulk_get_optional_exclusive(struct device *dev, int num_rstcs,
 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, false);
+	return __reset_control_get(dev, id, 0, RESET_CONTROL_OPTIONAL_SHARED);
 }
 
 /**
@@ -435,7 +468,7 @@ static inline int __must_check
 reset_control_bulk_get_optional_shared(struct device *dev, int num_rstcs,
 				       struct reset_control_bulk_data *rstcs)
 {
-	return __reset_control_bulk_get(dev, num_rstcs, rstcs, true, true, false);
+	return __reset_control_bulk_get(dev, num_rstcs, rstcs, RESET_CONTROL_OPTIONAL_SHARED);
 }
 
 /**
@@ -451,7 +484,7 @@ reset_control_bulk_get_optional_shared(struct device *dev, int num_rstcs,
 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, true);
+	return __of_reset_control_get(node, id, 0, RESET_CONTROL_EXCLUSIVE);
 }
 
 /**
@@ -471,7 +504,7 @@ static inline struct reset_control *of_reset_control_get_exclusive(
 static inline struct reset_control *of_reset_control_get_optional_exclusive(
 				struct device_node *node, const char *id)
 {
-	return __of_reset_control_get(node, id, 0, false, true, true);
+	return __of_reset_control_get(node, id, 0, RESET_CONTROL_OPTIONAL_EXCLUSIVE);
 }
 
 /**
@@ -496,7 +529,7 @@ static inline struct reset_control *of_reset_control_get_optional_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, false);
+	return __of_reset_control_get(node, id, 0, RESET_CONTROL_SHARED);
 }
 
 /**
@@ -513,7 +546,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, true);
+	return __of_reset_control_get(node, NULL, index, RESET_CONTROL_EXCLUSIVE);
 }
 
 /**
@@ -541,7 +574,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, false);
+	return __of_reset_control_get(node, NULL, index, RESET_CONTROL_SHARED);
 }
 
 /**
@@ -560,7 +593,7 @@ 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, true);
+	return __devm_reset_control_get(dev, id, 0, RESET_CONTROL_EXCLUSIVE);
 }
 
 /**
@@ -580,7 +613,8 @@ static inline int __must_check
 devm_reset_control_bulk_get_exclusive(struct device *dev, int num_rstcs,
 				      struct reset_control_bulk_data *rstcs)
 {
-	return __devm_reset_control_bulk_get(dev, num_rstcs, rstcs, false, false, true);
+	return __devm_reset_control_bulk_get(dev, num_rstcs, rstcs,
+					     RESET_CONTROL_EXCLUSIVE);
 }
 
 /**
@@ -599,7 +633,7 @@ 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);
+	return __devm_reset_control_get(dev, id, 0, RESET_CONTROL_EXCLUSIVE_RELEASED);
 }
 
 /**
@@ -619,7 +653,8 @@ static inline int __must_check
 devm_reset_control_bulk_get_exclusive_released(struct device *dev, int num_rstcs,
 					       struct reset_control_bulk_data *rstcs)
 {
-	return __devm_reset_control_bulk_get(dev, num_rstcs, rstcs, false, false, false);
+	return __devm_reset_control_bulk_get(dev, num_rstcs, rstcs,
+					     RESET_CONTROL_EXCLUSIVE_RELEASED);
 }
 
 /**
@@ -638,7 +673,7 @@ static inline struct reset_control *
 __must_check devm_reset_control_get_optional_exclusive_released(struct device *dev,
 								const char *id)
 {
-	return __devm_reset_control_get(dev, id, 0, false, true, false);
+	return __devm_reset_control_get(dev, id, 0, RESET_CONTROL_OPTIONAL_EXCLUSIVE_RELEASED);
 }
 
 /**
@@ -658,7 +693,8 @@ static inline int __must_check
 devm_reset_control_bulk_get_optional_exclusive_released(struct device *dev, int num_rstcs,
 							struct reset_control_bulk_data *rstcs)
 {
-	return __devm_reset_control_bulk_get(dev, num_rstcs, rstcs, false, true, false);
+	return __devm_reset_control_bulk_get(dev, num_rstcs, rstcs,
+					     RESET_CONTROL_OPTIONAL_EXCLUSIVE_RELEASED);
 }
 
 /**
@@ -673,7 +709,7 @@ devm_reset_control_bulk_get_optional_exclusive_released(struct device *dev, int
 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, false);
+	return __devm_reset_control_get(dev, id, 0, RESET_CONTROL_SHARED);
 }
 
 /**
@@ -693,7 +729,7 @@ static inline int __must_check
 devm_reset_control_bulk_get_shared(struct device *dev, int num_rstcs,
 				   struct reset_control_bulk_data *rstcs)
 {
-	return __devm_reset_control_bulk_get(dev, num_rstcs, rstcs, true, false, false);
+	return __devm_reset_control_bulk_get(dev, num_rstcs, rstcs, RESET_CONTROL_SHARED);
 }
 
 /**
@@ -711,7 +747,7 @@ devm_reset_control_bulk_get_shared(struct device *dev, int num_rstcs,
 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, true);
+	return __devm_reset_control_get(dev, id, 0, RESET_CONTROL_OPTIONAL_EXCLUSIVE);
 }
 
 /**
@@ -731,7 +767,8 @@ static inline int __must_check
 devm_reset_control_bulk_get_optional_exclusive(struct device *dev, int num_rstcs,
 					       struct reset_control_bulk_data *rstcs)
 {
-	return __devm_reset_control_bulk_get(dev, num_rstcs, rstcs, false, true, true);
+	return __devm_reset_control_bulk_get(dev, num_rstcs, rstcs,
+					     RESET_CONTROL_OPTIONAL_EXCLUSIVE);
 }
 
 /**
@@ -749,7 +786,7 @@ devm_reset_control_bulk_get_optional_exclusive(struct device *dev, int num_rstcs
 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, false);
+	return __devm_reset_control_get(dev, id, 0, RESET_CONTROL_OPTIONAL_SHARED);
 }
 
 /**
@@ -769,7 +806,7 @@ static inline int __must_check
 devm_reset_control_bulk_get_optional_shared(struct device *dev, int num_rstcs,
 					    struct reset_control_bulk_data *rstcs)
 {
-	return __devm_reset_control_bulk_get(dev, num_rstcs, rstcs, true, true, false);
+	return __devm_reset_control_bulk_get(dev, num_rstcs, rstcs, RESET_CONTROL_OPTIONAL_SHARED);
 }
 
 /**
@@ -787,7 +824,7 @@ devm_reset_control_bulk_get_optional_shared(struct device *dev, int num_rstcs,
 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, true);
+	return __devm_reset_control_get(dev, NULL, index, RESET_CONTROL_EXCLUSIVE);
 }
 
 /**
@@ -803,7 +840,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, false);
+	return __devm_reset_control_get(dev, NULL, index, RESET_CONTROL_SHARED);
 }
 
 /*
@@ -851,54 +888,54 @@ static inline struct reset_control *devm_reset_control_get_by_index(
 static inline struct reset_control *
 devm_reset_control_array_get_exclusive(struct device *dev)
 {
-	return devm_reset_control_array_get(dev, false, false);
+	return devm_reset_control_array_get(dev, RESET_CONTROL_EXCLUSIVE);
 }
 
 static inline struct reset_control *
 devm_reset_control_array_get_shared(struct device *dev)
 {
-	return devm_reset_control_array_get(dev, true, false);
+	return devm_reset_control_array_get(dev, RESET_CONTROL_SHARED);
 }
 
 static inline struct reset_control *
 devm_reset_control_array_get_optional_exclusive(struct device *dev)
 {
-	return devm_reset_control_array_get(dev, false, true);
+	return devm_reset_control_array_get(dev, RESET_CONTROL_OPTIONAL_EXCLUSIVE);
 }
 
 static inline struct reset_control *
 devm_reset_control_array_get_optional_shared(struct device *dev)
 {
-	return devm_reset_control_array_get(dev, true, true);
+	return devm_reset_control_array_get(dev, RESET_CONTROL_OPTIONAL_SHARED);
 }
 
 static inline struct reset_control *
 of_reset_control_array_get_exclusive(struct device_node *node)
 {
-	return of_reset_control_array_get(node, false, false, true);
+	return of_reset_control_array_get(node, RESET_CONTROL_EXCLUSIVE);
 }
 
 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);
+	return of_reset_control_array_get(node, RESET_CONTROL_EXCLUSIVE_RELEASED);
 }
 
 static inline struct reset_control *
 of_reset_control_array_get_shared(struct device_node *node)
 {
-	return of_reset_control_array_get(node, true, false, true);
+	return of_reset_control_array_get(node, RESET_CONTROL_SHARED);
 }
 
 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, true);
+	return of_reset_control_array_get(node, RESET_CONTROL_OPTIONAL_EXCLUSIVE);
 }
 
 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, true);
+	return of_reset_control_array_get(node, RESET_CONTROL_OPTIONAL_SHARED);
 }
 #endif

-- 
2.39.2



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH RFC 2/3] reset: Add devres helpers to request pre-deasserted reset controls
  2024-06-21 14:45 [PATCH RFC 0/3] reset: Requesting pre-deasserted, auto-reasserting reset controls via devres Philipp Zabel
  2024-06-21 14:45 ` [PATCH RFC 1/3] reset: replace boolean parameters with flags parameter Philipp Zabel
@ 2024-06-21 14:45 ` Philipp Zabel
  2024-06-21 14:45 ` [PATCH RFC 3/3] reset: uniphier-glue: Use devm_reset_control_bulk_get_shared_deasserted() Philipp Zabel
  2 siblings, 0 replies; 8+ messages in thread
From: Philipp Zabel @ 2024-06-21 14:45 UTC (permalink / raw)
  To: Kunihiko Hayashi, Masami Hiramatsu, Lad Prabhakar,
	Geert Uytterhoeven, Uwe Kleine-König
  Cc: linux-kernel, linux-arm-kernel, kernel, Philipp Zabel

Add devres helpers

 - devm_reset_control_bulk_get_exclusive_deasserted
 - devm_reset_control_bulk_get_optional_exclusive_deasserted
 - devm_reset_control_bulk_get_optional_shared_deasserted
 - devm_reset_control_bulk_get_shared_deasserted
 - devm_reset_control_get_exclusive_deasserted
 - devm_reset_control_get_optional_exclusive_deasserted
 - devm_reset_control_get_optional_shared_deasserted
 - devm_reset_control_get_shared_deasserted

to request and immediately deassert reset controls. During cleanup,
reset_control_assert() will be called automatically on the returned
reset controls.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/reset/core.c  |  44 +++++++++++++++++++-
 include/linux/reset.h | 113 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 155 insertions(+), 2 deletions(-)

diff --git a/drivers/reset/core.c b/drivers/reset/core.c
index 3658fe089147..cf4a16da9f34 100644
--- a/drivers/reset/core.c
+++ b/drivers/reset/core.c
@@ -1232,13 +1232,23 @@ static void devm_reset_control_release(struct device *dev, void *res)
 	reset_control_put(*(struct reset_control **)res);
 }
 
+static void devm_reset_control_release_deasserted(struct device *dev, void *res)
+{
+	struct reset_control *rstc = *(struct reset_control **)res;
+
+	reset_control_assert(rstc);
+	reset_control_put(rstc);
+}
+
 struct reset_control *
 __devm_reset_control_get(struct device *dev, const char *id, int index,
 			 enum reset_control_flags flags)
 {
 	struct reset_control **ptr, *rstc;
+	bool deasserted = flags & RESET_CONTROL_FLAGS_BIT_DEASSERTED;
 
-	ptr = devres_alloc(devm_reset_control_release, sizeof(*ptr),
+	ptr = devres_alloc(deasserted ? devm_reset_control_release_deasserted :
+			   devm_reset_control_release, sizeof(*ptr),
 			   GFP_KERNEL);
 	if (!ptr)
 		return ERR_PTR(-ENOMEM);
@@ -1249,6 +1259,17 @@ __devm_reset_control_get(struct device *dev, const char *id, int index,
 		return rstc;
 	}
 
+	if (deasserted) {
+		int ret;
+
+		ret = reset_control_deassert(rstc);
+		if (ret) {
+			reset_control_put(rstc);
+			devres_free(ptr);
+			return ERR_PTR(ret);
+		}
+	}
+
 	*ptr = rstc;
 	devres_add(dev, ptr);
 
@@ -1268,14 +1289,24 @@ static void devm_reset_control_bulk_release(struct device *dev, void *res)
 	reset_control_bulk_put(devres->num_rstcs, devres->rstcs);
 }
 
+static void devm_reset_control_bulk_release_deasserted(struct device *dev, void *res)
+{
+	struct reset_control_bulk_devres *devres = res;
+
+	reset_control_bulk_assert(devres->num_rstcs, devres->rstcs);
+	reset_control_bulk_put(devres->num_rstcs, devres->rstcs);
+}
+
 int __devm_reset_control_bulk_get(struct device *dev, int num_rstcs,
 				  struct reset_control_bulk_data *rstcs,
 				  enum reset_control_flags flags)
 {
 	struct reset_control_bulk_devres *ptr;
+	bool deasserted = flags & RESET_CONTROL_FLAGS_BIT_DEASSERTED;
 	int ret;
 
-	ptr = devres_alloc(devm_reset_control_bulk_release, sizeof(*ptr),
+	ptr = devres_alloc(deasserted ? devm_reset_control_bulk_release_deasserted :
+			   devm_reset_control_bulk_release, sizeof(*ptr),
 			   GFP_KERNEL);
 	if (!ptr)
 		return -ENOMEM;
@@ -1286,6 +1317,15 @@ int __devm_reset_control_bulk_get(struct device *dev, int num_rstcs,
 		return ret;
 	}
 
+	if (deasserted) {
+		ret = reset_control_bulk_deassert(num_rstcs, rstcs);
+		if (ret) {
+			reset_control_bulk_put(num_rstcs, rstcs);
+			devres_free(ptr);
+			return ret;
+		}
+	}
+
 	ptr->num_rstcs = num_rstcs;
 	ptr->rstcs = rstcs;
 	devres_add(dev, ptr);
diff --git a/include/linux/reset.h b/include/linux/reset.h
index 99296af98f81..2986ced69a02 100644
--- a/include/linux/reset.h
+++ b/include/linux/reset.h
@@ -28,6 +28,7 @@ struct reset_control_bulk_data {
 #define RESET_CONTROL_FLAGS_BIT_SHARED		BIT(0)	/* not exclusive */
 #define RESET_CONTROL_FLAGS_BIT_OPTIONAL	BIT(1)
 #define RESET_CONTROL_FLAGS_BIT_ACQUIRED	BIT(2)	/* iff exclusive, not released */
+#define RESET_CONTROL_FLAGS_BIT_DEASSERTED	BIT(3)
 
 /**
  * enum reset_control_flags - Flags that can be passed to the reset_control_get functions
@@ -35,21 +36,35 @@ struct reset_control_bulk_data {
  *                    These values cannot be OR'd.
  *
  * @RESET_CONTROL_EXCLUSIVE:				exclusive, acquired,
+ * @RESET_CONTROL_EXCLUSIVE_DEASSERTED:			exclusive, acquired, deasserted
  * @RESET_CONTROL_EXCLUSIVE_RELEASED:			exclusive, released,
  * @RESET_CONTROL_SHARED:				shared
+ * @RESET_CONTROL_SHARED_DEASSERTED:			shared, deasserted
  * @RESET_CONTROL_OPTIONAL_EXCLUSIVE:			optional, exclusive, acquired
+ * @RESET_CONTROL_OPTIONAL_EXCLUSIVE_DEASSERTED:	optional, exclusive, acquired, deasserted
  * @RESET_CONTROL_OPTIONAL_EXCLUSIVE_RELEASED:		optional, exclusive, released
  * @RESET_CONTROL_OPTIONAL_SHARED:			optional, shared
+ * @RESET_CONTROL_OPTIONAL_SHARED_DEASSERTED:		optional, shared, deasserted
  */
 enum reset_control_flags {
 	RESET_CONTROL_EXCLUSIVE				= RESET_CONTROL_FLAGS_BIT_ACQUIRED,
+	RESET_CONTROL_EXCLUSIVE_DEASSERTED		= RESET_CONTROL_FLAGS_BIT_ACQUIRED |
+							  RESET_CONTROL_FLAGS_BIT_DEASSERTED,
 	RESET_CONTROL_EXCLUSIVE_RELEASED		= 0,
 	RESET_CONTROL_SHARED				= RESET_CONTROL_FLAGS_BIT_SHARED,
+	RESET_CONTROL_SHARED_DEASSERTED			= RESET_CONTROL_FLAGS_BIT_SHARED |
+							  RESET_CONTROL_FLAGS_BIT_DEASSERTED,
 	RESET_CONTROL_OPTIONAL_EXCLUSIVE		= RESET_CONTROL_FLAGS_BIT_OPTIONAL |
 							  RESET_CONTROL_FLAGS_BIT_ACQUIRED,
+	RESET_CONTROL_OPTIONAL_EXCLUSIVE_DEASSERTED	= RESET_CONTROL_FLAGS_BIT_OPTIONAL |
+							  RESET_CONTROL_FLAGS_BIT_ACQUIRED |
+							  RESET_CONTROL_FLAGS_BIT_DEASSERTED,
 	RESET_CONTROL_OPTIONAL_EXCLUSIVE_RELEASED	= RESET_CONTROL_FLAGS_BIT_OPTIONAL,
 	RESET_CONTROL_OPTIONAL_SHARED			= RESET_CONTROL_FLAGS_BIT_OPTIONAL |
 							  RESET_CONTROL_FLAGS_BIT_SHARED,
+	RESET_CONTROL_OPTIONAL_SHARED_DEASSERTED	= RESET_CONTROL_FLAGS_BIT_OPTIONAL |
+							  RESET_CONTROL_FLAGS_BIT_SHARED |
+							  RESET_CONTROL_FLAGS_BIT_DEASSERTED,
 };
 
 #ifdef CONFIG_RESET_CONTROLLER
@@ -596,6 +611,25 @@ __must_check devm_reset_control_get_exclusive(struct device *dev,
 	return __devm_reset_control_get(dev, id, 0, RESET_CONTROL_EXCLUSIVE);
 }
 
+/**
+ * devm_reset_control_get_exclusive_deasserted - resource managed
+ *                                    reset_control_get_exclusive() +
+ *                                    reset_control_deassert()
+ * @dev: device to be reset by the controller
+ * @id: reset line name
+ *
+ * Managed reset_control_get_exclusive() + reset_control_deassert(). For reset
+ * controllers returned from this function, reset_control_assert() +
+ * reset_control_put() is called automatically on driver detach.
+ *
+ * See reset_control_get_exclusive() for more information.
+ */
+static inline struct reset_control * __must_check
+devm_reset_control_get_exclusive_deasserted(struct device *dev, const char *id)
+{
+	return __devm_reset_control_get(dev, id, 0, RESET_CONTROL_EXCLUSIVE_DEASSERTED);
+}
+
 /**
  * devm_reset_control_bulk_get_exclusive - resource managed
  *                                         reset_control_bulk_get_exclusive()
@@ -712,6 +746,25 @@ static inline struct reset_control *devm_reset_control_get_shared(
 	return __devm_reset_control_get(dev, id, 0, RESET_CONTROL_SHARED);
 }
 
+/**
+ * devm_reset_control_get_shared_deasserted - resource managed
+ *                                            reset_control_get_shared() +
+ *                                            reset_control_deassert()
+ * @dev: device to be reset by the controller
+ * @id: reset line name
+ *
+ * Managed reset_control_get_shared() + reset_control_deassert(). For reset
+ * controllers returned from this function, reset_control_assert() +
+ * reset_control_put() is called automatically on driver detach.
+ *
+ * See devm_reset_control_get_shared() for more information.
+ */
+static inline struct reset_control * __must_check
+devm_reset_control_get_shared_deasserted(struct device *dev, const char *id)
+{
+	return __devm_reset_control_get(dev, id, 0, RESET_CONTROL_SHARED_DEASSERTED);
+}
+
 /**
  * devm_reset_control_bulk_get_shared - resource managed
  *                                      reset_control_bulk_get_shared()
@@ -732,6 +785,28 @@ devm_reset_control_bulk_get_shared(struct device *dev, int num_rstcs,
 	return __devm_reset_control_bulk_get(dev, num_rstcs, rstcs, RESET_CONTROL_SHARED);
 }
 
+/**
+ * devm_reset_control_bulk_get_shared_deasserted - resource managed
+ *                                                 reset_control_bulk_get_shared() +
+ *                                                 reset_control_bulk_deassert()
+ * @dev: device to be reset by the controller
+ * @num_rstcs: number of entries in rstcs array
+ * @rstcs: array of struct reset_control_bulk_data with reset line names set
+ *
+ * Managed reset_control_bulk_get_shared() + reset_control_bulk_deassert(). For
+ * reset controllers returned from this function, reset_control_bulk_assert() +
+ * reset_control_bulk_put() are called automatically on driver detach.
+ *
+ * See devm_reset_control_bulk_get_shared() for more information.
+ */
+static inline int __must_check
+devm_reset_control_bulk_get_shared_deasserted(struct device *dev, int num_rstcs,
+					      struct reset_control_bulk_data *rstcs)
+{
+	return __devm_reset_control_bulk_get(dev, num_rstcs, rstcs,
+					     RESET_CONTROL_SHARED_DEASSERTED);
+}
+
 /**
  * devm_reset_control_get_optional_exclusive - resource managed
  *                                             reset_control_get_optional_exclusive()
@@ -750,6 +825,25 @@ static inline struct reset_control *devm_reset_control_get_optional_exclusive(
 	return __devm_reset_control_get(dev, id, 0, RESET_CONTROL_OPTIONAL_EXCLUSIVE);
 }
 
+/**
+ * devm_reset_control_get_optional_exclusive_deasserted - resource managed
+ *                                                        reset_control_get_optional_exclusive() +
+ *                                                        reset_control_deassert()
+ * @dev: device to be reset by the controller
+ * @id: reset line name
+ *
+ * Managed reset_control_get_optional_exclusive() + reset_control_deassert().
+ * For reset controllers returned from this function, reset_control_assert() +
+ * reset_control_put() is called automatically on driver detach.
+ *
+ * See devm_reset_control_get_optional_exclusive() for more information.
+ */
+static inline struct reset_control *
+devm_reset_control_get_optional_exclusive_deasserted(struct device *dev, const char *id)
+{
+	return __devm_reset_control_get(dev, id, 0, RESET_CONTROL_OPTIONAL_EXCLUSIVE_DEASSERTED);
+}
+
 /**
  * devm_reset_control_bulk_get_optional_exclusive - resource managed
  *                                                  reset_control_bulk_get_optional_exclusive()
@@ -789,6 +883,25 @@ static inline struct reset_control *devm_reset_control_get_optional_shared(
 	return __devm_reset_control_get(dev, id, 0, RESET_CONTROL_OPTIONAL_SHARED);
 }
 
+/**
+ * devm_reset_control_get_optional_shared_deasserted - resource managed
+ *                                                     reset_control_get_optional_shared() +
+ *                                                     reset_control_deassert()
+ * @dev: device to be reset by the controller
+ * @id: reset line name
+ *
+ * Managed reset_control_get_optional_shared() + reset_control_deassert(). For
+ * reset controllers returned from this function, reset_control_assert() +
+ * reset_control_put() is called automatically on driver detach.
+ *
+ * See devm_reset_control_get_optional_shared() for more information.
+ */
+static inline struct reset_control *
+devm_reset_control_get_optional_shared_deasserted(struct device *dev, const char *id)
+{
+	return __devm_reset_control_get(dev, id, 0, RESET_CONTROL_OPTIONAL_SHARED_DEASSERTED);
+}
+
 /**
  * devm_reset_control_bulk_get_optional_shared - resource managed
  *                                               reset_control_bulk_get_optional_shared()

-- 
2.39.2



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH RFC 3/3] reset: uniphier-glue: Use devm_reset_control_bulk_get_shared_deasserted()
  2024-06-21 14:45 [PATCH RFC 0/3] reset: Requesting pre-deasserted, auto-reasserting reset controls via devres Philipp Zabel
  2024-06-21 14:45 ` [PATCH RFC 1/3] reset: replace boolean parameters with flags parameter Philipp Zabel
  2024-06-21 14:45 ` [PATCH RFC 2/3] reset: Add devres helpers to request pre-deasserted reset controls Philipp Zabel
@ 2024-06-21 14:45 ` Philipp Zabel
  2 siblings, 0 replies; 8+ messages in thread
From: Philipp Zabel @ 2024-06-21 14:45 UTC (permalink / raw)
  To: Kunihiko Hayashi, Masami Hiramatsu, Lad Prabhakar,
	Geert Uytterhoeven, Uwe Kleine-König
  Cc: linux-kernel, linux-arm-kernel, kernel, Philipp Zabel

Replace the pattern devm_reset_control_bulk_get_shared() /
reset_control_bulk_deassert() / devm_add_action_or_reset()
with devm_reset_control_bulk_get_shared_deasserted() for
some reduction in boilerplate.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/reset/reset-uniphier-glue.c | 24 +++++-------------------
 1 file changed, 5 insertions(+), 19 deletions(-)

diff --git a/drivers/reset/reset-uniphier-glue.c b/drivers/reset/reset-uniphier-glue.c
index 5f9f2f7994c0..a2a262bf6bfc 100644
--- a/drivers/reset/reset-uniphier-glue.c
+++ b/drivers/reset/reset-uniphier-glue.c
@@ -35,13 +35,6 @@ static void uniphier_clk_disable(void *_priv)
 	clk_bulk_disable_unprepare(priv->data->nclks, priv->clk);
 }
 
-static void uniphier_rst_assert(void *_priv)
-{
-	struct uniphier_glue_reset_priv *priv = _priv;
-
-	reset_control_bulk_assert(priv->data->nrsts, priv->rst);
-}
-
 static int uniphier_glue_reset_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -68,13 +61,6 @@ static int uniphier_glue_reset_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	for (i = 0; i < priv->data->nrsts; i++)
-		priv->rst[i].id = priv->data->reset_names[i];
-	ret = devm_reset_control_bulk_get_shared(dev, priv->data->nrsts,
-						 priv->rst);
-	if (ret)
-		return ret;
-
 	ret = clk_bulk_prepare_enable(priv->data->nclks, priv->clk);
 	if (ret)
 		return ret;
@@ -83,11 +69,11 @@ static int uniphier_glue_reset_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	ret = reset_control_bulk_deassert(priv->data->nrsts, priv->rst);
-	if (ret)
-		return ret;
-
-	ret = devm_add_action_or_reset(dev, uniphier_rst_assert, priv);
+	for (i = 0; i < priv->data->nrsts; i++)
+		priv->rst[i].id = priv->data->reset_names[i];
+	ret = devm_reset_control_bulk_get_shared_deasserted(dev,
+							    priv->data->nrsts,
+							    priv->rst);
 	if (ret)
 		return ret;
 

-- 
2.39.2



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH RFC 1/3] reset: replace boolean parameters with flags parameter
  2024-06-21 14:45 ` [PATCH RFC 1/3] reset: replace boolean parameters with flags parameter Philipp Zabel
@ 2024-06-22  7:47   ` Uwe Kleine-König
  2024-06-26 16:17     ` Philipp Zabel
  0 siblings, 1 reply; 8+ messages in thread
From: Uwe Kleine-König @ 2024-06-22  7:47 UTC (permalink / raw)
  To: Philipp Zabel
  Cc: Kunihiko Hayashi, Masami Hiramatsu, Lad Prabhakar,
	Geert Uytterhoeven, linux-kernel, linux-arm-kernel, kernel

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

Hello Philipp,

I like the idea in general. Just a detail concern down below.

On Fri, Jun 21, 2024 at 04:45:02PM +0200, Philipp Zabel wrote:
> @@ -999,8 +1001,9 @@ static struct reset_controller_dev *__reset_find_rcdev(const struct of_phandle_a
>  
>  struct reset_control *
>  __of_reset_control_get(struct device_node *node, const char *id, int index,
> -		       bool shared, bool optional, bool acquired)
> +		       enum reset_control_flags flags)
>  {
> +	bool optional = flags & RESET_CONTROL_FLAGS_BIT_OPTIONAL;
>  	bool gpio_fallback = false;
>  	struct reset_control *rstc;
>  	struct reset_controller_dev *rcdev;
> @@ -1065,7 +1068,7 @@ __of_reset_control_get(struct device_node *node, const char *id, int index,
>  	}
>  
>  	/* reset_list_mutex also protects the rcdev's reset_control list */
> -	rstc = __reset_control_get_internal(rcdev, rstc_id, shared, acquired);
> +	rstc = __reset_control_get_internal(rcdev, rstc_id, flags);

If RESET_CONTROL_FLAGS_BIT_OPTIONAL was passed to
__of_reset_control_get(), you're forwarding it to
__reset_control_get_internal(). But the latter doesn't do anything with
that flag. I wonder if the API would be still less prone to error if
you'd filter out RESET_CONTROL_FLAGS_BIT_OPTIONAL before passing to
__reset_control_get_internal() and in __reset_control_get_internal() add
a check for unsupported flags.

>  out_unlock:
>  	mutex_unlock(&reset_list_mutex);
> @@ -1096,8 +1099,9 @@ __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 acquired)
> +				enum reset_control_flags flags)
>  {
> +	bool optional = flags & RESET_CONTROL_FLAGS_BIT_OPTIONAL;
>  	const struct reset_control_lookup *lookup;
>  	struct reset_controller_dev *rcdev;
>  	const char *dev_id = dev_name(dev);
> [...]
> @@ -1422,7 +1423,7 @@ EXPORT_SYMBOL_GPL(of_reset_control_array_get);
>   * Returns pointer to allocated reset_control on success or error on failure
>   */
>  struct reset_control *
> -devm_reset_control_array_get(struct device *dev, bool shared, bool optional)
> +devm_reset_control_array_get(struct device *dev, enum reset_control_flags flags)
>  {
>  	struct reset_control **ptr, *rstc;
>  
> @@ -1431,7 +1432,7 @@ devm_reset_control_array_get(struct device *dev, bool shared, bool optional)
>  	if (!ptr)
>  		return ERR_PTR(-ENOMEM);
>  
> -	rstc = of_reset_control_array_get(dev->of_node, shared, optional, true);
> +	rstc = of_reset_control_array_get(dev->of_node, flags);

Is it an error if the new devm_reset_control_array_get() is called
without RESET_CONTROL_FLAGS_BIT_ACQUIRED in flags?

>  	if (IS_ERR_OR_NULL(rstc)) {
>  		devres_free(ptr);
>  		return rstc;

Best regards
Uwe

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH RFC 1/3] reset: replace boolean parameters with flags parameter
  2024-06-22  7:47   ` Uwe Kleine-König
@ 2024-06-26 16:17     ` Philipp Zabel
  2024-09-25 14:25       ` Uwe Kleine-König
  0 siblings, 1 reply; 8+ messages in thread
From: Philipp Zabel @ 2024-06-26 16:17 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Kunihiko Hayashi, Masami Hiramatsu, Lad Prabhakar,
	Geert Uytterhoeven, linux-kernel, linux-arm-kernel, kernel

Hi Uwe,

On Sa, 2024-06-22 at 09:47 +0200, Uwe Kleine-König wrote:
> Hello Philipp,
> 
> I like the idea in general. Just a detail concern down below.

Thank you, much appreciated.

> On Fri, Jun 21, 2024 at 04:45:02PM +0200, Philipp Zabel wrote:
> > @@ -999,8 +1001,9 @@ static struct reset_controller_dev *__reset_find_rcdev(const struct of_phandle_a
> >  
> >  struct reset_control *
> >  __of_reset_control_get(struct device_node *node, const char *id, int index,
> > -		       bool shared, bool optional, bool acquired)
> > +		       enum reset_control_flags flags)
> >  {
> > +	bool optional = flags & RESET_CONTROL_FLAGS_BIT_OPTIONAL;
> >  	bool gpio_fallback = false;
> >  	struct reset_control *rstc;
> >  	struct reset_controller_dev *rcdev;
> > @@ -1065,7 +1068,7 @@ __of_reset_control_get(struct device_node *node, const char *id, int index,
> >  	}
> >  
> >  	/* reset_list_mutex also protects the rcdev's reset_control list */
> > -	rstc = __reset_control_get_internal(rcdev, rstc_id, shared, acquired);
> > +	rstc = __reset_control_get_internal(rcdev, rstc_id, flags);
> 
> If RESET_CONTROL_FLAGS_BIT_OPTIONAL was passed to
> __of_reset_control_get(), you're forwarding it to
> __reset_control_get_internal(). But the latter doesn't do anything with
> that flag. I wonder if the API would be still less prone to error if
> you'd filter out RESET_CONTROL_FLAGS_BIT_OPTIONAL before passing to
> __reset_control_get_internal() and in __reset_control_get_internal() add
> a check for unsupported flags.

Yes, I'll do that. For every enum value with the optional bit set,
there is a corresponding value without it.

> >  out_unlock:
> >  	mutex_unlock(&reset_list_mutex);
> > @@ -1096,8 +1099,9 @@ __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 acquired)
> > +				enum reset_control_flags flags)
> >  {
> > +	bool optional = flags & RESET_CONTROL_FLAGS_BIT_OPTIONAL;
> >  	const struct reset_control_lookup *lookup;
> >  	struct reset_controller_dev *rcdev;
> >  	const char *dev_id = dev_name(dev);
> > [...]
> > @@ -1422,7 +1423,7 @@ EXPORT_SYMBOL_GPL(of_reset_control_array_get);
> >   * Returns pointer to allocated reset_control on success or error on failure
> >   */
> >  struct reset_control *
> > -devm_reset_control_array_get(struct device *dev, bool shared, bool optional)
> > +devm_reset_control_array_get(struct device *dev, enum reset_control_flags flags)
> >  {
> >  	struct reset_control **ptr, *rstc;
> >  
> > @@ -1431,7 +1432,7 @@ devm_reset_control_array_get(struct device *dev, bool shared, bool optional)
> >  	if (!ptr)
> >  		return ERR_PTR(-ENOMEM);
> >  
> > -	rstc = of_reset_control_array_get(dev->of_node, shared, optional, true);
> > +	rstc = of_reset_control_array_get(dev->of_node, flags);
> 
> Is it an error if the new devm_reset_control_array_get() is called
> without RESET_CONTROL_FLAGS_BIT_ACQUIRED in flags?

I'd be inclined to consider this not-an-error.

There is one user of of_reset_control_array_get_exclusive_released(),
so it should work in theory. Of course nobody is using both devres and
the acquire/release API at the same time, and there is no
devm_reset_control_array_get_exclusive_released() wrapper.

regards
Philipp


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH RFC 1/3] reset: replace boolean parameters with flags parameter
  2024-06-26 16:17     ` Philipp Zabel
@ 2024-09-25 14:25       ` Uwe Kleine-König
  2024-09-25 16:43         ` Philipp Zabel
  0 siblings, 1 reply; 8+ messages in thread
From: Uwe Kleine-König @ 2024-09-25 14:25 UTC (permalink / raw)
  To: Philipp Zabel
  Cc: Kunihiko Hayashi, Masami Hiramatsu, Lad Prabhakar,
	Geert Uytterhoeven, linux-kernel, linux-arm-kernel, kernel

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

Hello Philipp,

On Wed, Jun 26, 2024 at 06:17:11PM +0200, Philipp Zabel wrote:
> On Sa, 2024-06-22 at 09:47 +0200, Uwe Kleine-König wrote:
> > On Fri, Jun 21, 2024 at 04:45:02PM +0200, Philipp Zabel wrote:
> > > @@ -999,8 +1001,9 @@ static struct reset_controller_dev *__reset_find_rcdev(const struct of_phandle_a
> > >  
> > >  struct reset_control *
> > >  __of_reset_control_get(struct device_node *node, const char *id, int index,
> > > -		       bool shared, bool optional, bool acquired)
> > > +		       enum reset_control_flags flags)
> > >  {
> > > +	bool optional = flags & RESET_CONTROL_FLAGS_BIT_OPTIONAL;
> > >  	bool gpio_fallback = false;
> > >  	struct reset_control *rstc;
> > >  	struct reset_controller_dev *rcdev;
> > > @@ -1065,7 +1068,7 @@ __of_reset_control_get(struct device_node *node, const char *id, int index,
> > >  	}
> > >  
> > >  	/* reset_list_mutex also protects the rcdev's reset_control list */
> > > -	rstc = __reset_control_get_internal(rcdev, rstc_id, shared, acquired);
> > > +	rstc = __reset_control_get_internal(rcdev, rstc_id, flags);
> > 
> > If RESET_CONTROL_FLAGS_BIT_OPTIONAL was passed to
> > __of_reset_control_get(), you're forwarding it to
> > __reset_control_get_internal(). But the latter doesn't do anything with
> > that flag. I wonder if the API would be still less prone to error if
> > you'd filter out RESET_CONTROL_FLAGS_BIT_OPTIONAL before passing to
> > __reset_control_get_internal() and in __reset_control_get_internal() add
> > a check for unsupported flags.
> 
> Yes, I'll do that. For every enum value with the optional bit set,
> there is a corresponding value without it.

Do you have this still on your todo list? I just review a pwm driver
that would benefit from devm_reset_control_get_exclusive_deasserted().

Best regards
Uwe

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH RFC 1/3] reset: replace boolean parameters with flags parameter
  2024-09-25 14:25       ` Uwe Kleine-König
@ 2024-09-25 16:43         ` Philipp Zabel
  0 siblings, 0 replies; 8+ messages in thread
From: Philipp Zabel @ 2024-09-25 16:43 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Kunihiko Hayashi, Masami Hiramatsu, Lad Prabhakar,
	Geert Uytterhoeven, linux-kernel, linux-arm-kernel, kernel

Hi Uwe,

On Mi, 2024-09-25 at 16:25 +0200, Uwe Kleine-König wrote:
> Hello Philipp,
> 
> On Wed, Jun 26, 2024 at 06:17:11PM +0200, Philipp Zabel wrote:
> > On Sa, 2024-06-22 at 09:47 +0200, Uwe Kleine-König wrote:
> > > On Fri, Jun 21, 2024 at 04:45:02PM +0200, Philipp Zabel wrote:
> > > > @@ -999,8 +1001,9 @@ static struct reset_controller_dev *__reset_find_rcdev(const struct of_phandle_a
> > > >  
> > > >  struct reset_control *
> > > >  __of_reset_control_get(struct device_node *node, const char *id, int index,
> > > > -		       bool shared, bool optional, bool acquired)
> > > > +		       enum reset_control_flags flags)
> > > >  {
> > > > +	bool optional = flags & RESET_CONTROL_FLAGS_BIT_OPTIONAL;
> > > >  	bool gpio_fallback = false;
> > > >  	struct reset_control *rstc;
> > > >  	struct reset_controller_dev *rcdev;
> > > > @@ -1065,7 +1068,7 @@ __of_reset_control_get(struct device_node *node, const char *id, int index,
> > > >  	}
> > > >  
> > > >  	/* reset_list_mutex also protects the rcdev's reset_control list */
> > > > -	rstc = __reset_control_get_internal(rcdev, rstc_id, shared, acquired);
> > > > +	rstc = __reset_control_get_internal(rcdev, rstc_id, flags);
> > > 
> > > If RESET_CONTROL_FLAGS_BIT_OPTIONAL was passed to
> > > __of_reset_control_get(), you're forwarding it to
> > > __reset_control_get_internal(). But the latter doesn't do anything with
> > > that flag. I wonder if the API would be still less prone to error if
> > > you'd filter out RESET_CONTROL_FLAGS_BIT_OPTIONAL before passing to
> > > __reset_control_get_internal() and in __reset_control_get_internal() add
> > > a check for unsupported flags.
> > 
> > Yes, I'll do that. For every enum value with the optional bit set,
> > there is a corresponding value without it.
> 
> Do you have this still on your todo list? I just review a pwm driver
> that would benefit from devm_reset_control_get_exclusive_deasserted().

Thanks for the reminder, just sent a v2.

regards
Philipp


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2024-09-25 16:46 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-21 14:45 [PATCH RFC 0/3] reset: Requesting pre-deasserted, auto-reasserting reset controls via devres Philipp Zabel
2024-06-21 14:45 ` [PATCH RFC 1/3] reset: replace boolean parameters with flags parameter Philipp Zabel
2024-06-22  7:47   ` Uwe Kleine-König
2024-06-26 16:17     ` Philipp Zabel
2024-09-25 14:25       ` Uwe Kleine-König
2024-09-25 16:43         ` Philipp Zabel
2024-06-21 14:45 ` [PATCH RFC 2/3] reset: Add devres helpers to request pre-deasserted reset controls Philipp Zabel
2024-06-21 14:45 ` [PATCH RFC 3/3] reset: uniphier-glue: Use devm_reset_control_bulk_get_shared_deasserted() Philipp Zabel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).