Skip to contentSkip to conversation

reset: make optional functions really optional

PATCH In review Dinh Nguyen 3 participants 6 messages Dec 3, 2018

Subsystems (2)
Stable branch Reset controller framework
lore
Share & tools
lore

Dinh Nguyen

author
#0 lore raw

PATCH 1/1 reset: make optional functions really optional

From: Ramiro Oliveira <[email protected]>

commit bb475230b8e59a547ab66ac3b02572df21a580e9 upstream.

The *_get_optional_* functions weren't really optional so this patch makes them really optional.

These *_get_optional_* functions will now return NULL instead of an error if no matching reset phandle is found in the DT, and all the reset_control_* functions now accept NULL rstc pointers.

Signed-off-by Ramiro Oliveira <[email protected]>
Signed-off-by Philipp Zabel <[email protected]>

---
Please apply to v4.9.y stable tree.

---
 drivers/reset/core.c  | 48 +++++++++++++++++++++++++++++++++----------
 include/linux/reset.h | 45 +++++++++++++++++++++++-----------------
 2 files changed, 63 insertions(+), 30 deletions(-)
drivers/reset/core.c +37 -11
index b8ae1dbd4c17..75821417b799 100644
@@ -135,11 +135,16 @@ EXPORT_SYMBOL_GPL(devm_reset_controller_register);
  * @rstc: reset controller
  *
  * Calling this on a shared reset controller is an error.
+ *
+ * If rstc is NULL it is an optional reset and the function will just
+ * return 0.
  */
 int reset_control_reset(struct reset_control *rstc)
 {
-	if (WARN_ON(IS_ERR_OR_NULL(rstc)) ||
-	    WARN_ON(rstc->shared))
+	if (!rstc)
+		return 0;
+
+	if (WARN_ON(IS_ERR(rstc)))
 		return -EINVAL;
 
 	if (rstc->rcdev->ops->reset)

@@ -159,10 +164,16 @@ EXPORT_SYMBOL_GPL(reset_control_reset);
  *
  * For shared reset controls a driver cannot expect the hw's registers and
  * internal state to be reset, but must be prepared for this to happen.
+ *
+ * If rstc is NULL it is an optional reset and the function will just
+ * return 0.
  */
 int reset_control_assert(struct reset_control *rstc)
 {
-	if (WARN_ON(IS_ERR_OR_NULL(rstc)))
+	if (!rstc)
+		return 0;
+
+	if (WARN_ON(IS_ERR(rstc)))
 		return -EINVAL;
 
 	if (!rstc->rcdev->ops->assert)

@@ -185,10 +196,16 @@ EXPORT_SYMBOL_GPL(reset_control_assert);
  * @rstc: reset controller
  *
  * After calling this function, the reset is guaranteed to be deasserted.
+ *
+ * If rstc is NULL it is an optional reset and the function will just
+ * return 0.
  */
 int reset_control_deassert(struct reset_control *rstc)
 {
-	if (WARN_ON(IS_ERR_OR_NULL(rstc)))
+	if (!rstc)
+		return 0;
+
+	if (WARN_ON(IS_ERR(rstc)))
 		return -EINVAL;
 
 	if (!rstc->rcdev->ops->deassert)

@@ -206,12 +223,15 @@ EXPORT_SYMBOL_GPL(reset_control_deassert);
 /**
  * reset_control_status - returns a negative errno if not supported, a
  * positive value if the reset line is asserted, or zero if the reset
- * line is not asserted.
+ * line is not asserted or if the desc is NULL (optional reset).
  * @rstc: reset controller
  */
 int reset_control_status(struct reset_control *rstc)
 {
-	if (WARN_ON(IS_ERR_OR_NULL(rstc)))
+	if (!rstc)
+		return 0;
+
+	if (WARN_ON(IS_ERR(rstc)))
 		return -EINVAL;
 
 	if (rstc->rcdev->ops->status)

@@ -268,7 +288,8 @@ static void __reset_control_put(struct reset_control *rstc)
 }
 
 struct reset_control *__of_reset_control_get(struct device_node *node,
-				     const char *id, int index, int shared)
+				     const char *id, int index, bool shared,
+				     bool optional)
 {
 	struct reset_control *rstc;
 	struct reset_controller_dev *r, *rcdev;

@@ -282,14 +303,18 @@ struct reset_control *__of_reset_control_get(struct device_node *node,
 	if (id) {
 		index = of_property_match_string(node,
 						 "reset-names", id);
+		if (index == -EILSEQ)
+			return ERR_PTR(index);
 		if (index < 0)
-			return ERR_PTR(-ENOENT);
+			return optional ? NULL : ERR_PTR(-ENOENT);
 	}
 
 	ret = of_parse_phandle_with_args(node, "resets", "#reset-cells",
 					 index, &args);
-	if (ret)
+	if (ret == -EINVAL)
 		return ERR_PTR(ret);
+	if (ret)
+		return optional ? NULL : ERR_PTR(ret);
 
 	mutex_lock(&reset_list_mutex);
 	rcdev = NULL;

@@ -348,7 +373,8 @@ 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, int shared)
+				     const char *id, int index, bool shared,
+				     bool optional)
 {
 	struct reset_control **ptr, *rstc;
 

@@ -358,7 +384,7 @@ struct reset_control *__devm_reset_control_get(struct device *dev,
 		return ERR_PTR(-ENOMEM);
 
 	rstc = __of_reset_control_get(dev ? dev->of_node : NULL,
-				      id, index, shared);
+				      id, index, shared, optional);
 	if (!IS_ERR(rstc)) {
 		*ptr = rstc;
 		devres_add(dev, ptr);
include/linux/reset.h +26 -19

144 lines not on this page · raw message

Signature · 1 line
2.17.1
maintainer
#1 lore raw
Quoted 7 lines · Dinh Nguyen

From: Ramiro Oliveira <[email protected]>

commit bb475230b8e59a547ab66ac3b02572df21a580e9 upstream.

The *_get_optional_* functions weren't really optional so this patch makes them really optional.

Quoted 8 lines

These *_get_optional_* functions will now return NULL instead of an error if no matching reset phandle is found in the DT, and all the reset_control_* functions now accept NULL rstc pointers.

Signed-off-by Ramiro Oliveira <[email protected]>
Signed-off-by Philipp Zabel <[email protected]>

---
Please apply to v4.9.y stable tree.

The original commit had a few upstream fixes:

bb6c7768385b reset: remove remaining WARN_ON() in <linux/reset.h> 1554bbd4ad40 reset: make device_reset_optional() really optional 62e24c5775ec reset: add exported __reset_control_get, return NULL if optional 0ca10b60ceeb reset: fix optional reset_control_get stubs to return NULL 4891486fb2c8 reset: core: fix reset_control_put

We'll need to backport them to 4.9 as well if you backport this commit.

--
Thanks,
Sasha

author
#2 lore raw
Quoted 19 lines · Sasha Levin
Quoted 7 lines · Dinh Nguyen

From: Ramiro Oliveira <[email protected]>

commit bb475230b8e59a547ab66ac3b02572df21a580e9 upstream.

The *_get_optional_* functions weren't really optional so this patch makes them really optional.

Quoted 8 lines

These *_get_optional_* functions will now return NULL instead of an error if no matching reset phandle is found in the DT, and all the reset_control_* functions now accept NULL rstc pointers.

Signed-off-by Ramiro Oliveira <[email protected]>
Signed-off-by Philipp Zabel <[email protected]>

---
Please apply to v4.9.y stable tree.

The original commit had a few upstream fixes:

Quoted 9 lines

bb6c7768385b reset: remove remaining WARN_ON() in <linux/reset.h> 1554bbd4ad40 reset: make device_reset_optional() really optional 62e24c5775ec reset: add exported __reset_control_get, return NULL if optional
0ca10b60ceeb reset: fix optional reset_control_get stubs to return NULL 4891486fb2c8 reset: core: fix reset_control_put

We'll need to backport them to 4.9 as well if you backport this commit.

Hmm..I was able to fix an ethernet driver load issue by just backporting this patch. Should I investigate backport of the other patches as well?

Dinh

maintainer
#3 lore raw
Quoted 29 lines · Dinh Nguyen
Quoted 26 lines · Sasha Levin
Quoted 7 lines · Dinh Nguyen

From: Ramiro Oliveira <[email protected]>

commit bb475230b8e59a547ab66ac3b02572df21a580e9 upstream.

The *_get_optional_* functions weren't really optional so this patch makes them really optional.

Quoted 8 lines

These *_get_optional_* functions will now return NULL instead of an error if no matching reset phandle is found in the DT, and all the reset_control_* functions now accept NULL rstc pointers.

Signed-off-by Ramiro Oliveira <[email protected]>
Signed-off-by Philipp Zabel <[email protected]>

---
Please apply to v4.9.y stable tree.

The original commit had a few upstream fixes:

bb6c7768385b reset: remove remaining WARN_ON() in <linux/reset.h> 1554bbd4ad40 reset: make device_reset_optional() really optional 62e24c5775ec reset: add exported __reset_control_get, return NULL if optional
0ca10b60ceeb reset: fix optional reset_control_get stubs to return NULL 4891486fb2c8 reset: core: fix reset_control_put

Quoted 5 lines
Quoted 2 lines

We'll need to backport them to 4.9 as well if you backport this commit.

Hmm..I was able to fix an ethernet driver load issue by just backporting this patch. Should I investigate backport of the other patches as well?

That patch will probably work on it's own, but if there are fixes for it then they should be backported as well.

--
Thanks,
Sasha

maintainer
#4 lore raw
In reply to Dinh Nguyen
Quoted 11 lines · Dinh Nguyen

From: Ramiro Oliveira <[email protected]>

commit bb475230b8e59a547ab66ac3b02572df21a580e9 upstream.

The *_get_optional_* functions weren't really optional so this patch makes them really optional.

These *_get_optional_* functions will now return NULL instead of an error if no matching reset phandle is found in the DT, and all the reset_control_* functions now accept NULL rstc pointers.

Quoted 8 lines
Signed-off-by Ramiro Oliveira <[email protected]>
Signed-off-by Philipp Zabel <[email protected]>

---
Please apply to v4.9.y stable tree.

---
 drivers/reset/core.c  | 48 +++++++++++++++++++++++++++++++++----------
 include/linux/reset.h | 45 +++++++++++++++++++++++-----------------
 2 files changed, 63 insertions(+), 30 deletions(-)

What problem is this solving? Why is this suddenly needed now?

thanks,

greg k-h

author
#5 lore raw
Quoted 21 lines · Greg KH
Quoted 11 lines · Dinh Nguyen

From: Ramiro Oliveira <[email protected]>

commit bb475230b8e59a547ab66ac3b02572df21a580e9 upstream.

The *_get_optional_* functions weren't really optional so this patch makes them really optional.

These *_get_optional_* functions will now return NULL instead of an error if no matching reset phandle is found in the DT, and all the reset_control_* functions now accept NULL rstc pointers.

Quoted 8 lines
Signed-off-by Ramiro Oliveira <[email protected]>
Signed-off-by Philipp Zabel <[email protected]>

---
Please apply to v4.9.y stable tree.

---
 drivers/reset/core.c  | 48 +++++++++++++++++++++++++++++++++----------
 include/linux/reset.h | 45 +++++++++++++++++++++++-----------------
 2 files changed, 63 insertions(+), 30 deletions(-)
Quoted 2 lines

What problem is this solving? Why is this suddenly needed now?

The v4.9-stable branch has this patch for stmmac ethernet driver:

"6f37f7b62baa net: stmmac: socfpga: add additional ocp reset line for Stratix10"

This patch calls devm_reset_control_get_optional(). This call ultimately fails without optional reset patch, causing the ethernet driver to fail to load.

Dinh