All of lore.kernel.org
 help / color / mirror / Atom feed
From: Geert Uytterhoeven <geert+renesas@glider.be>
To: Philipp Zabel <p.zabel@pengutronix.de>
Cc: Eric Auger <eric.auger@redhat.com>,
	Alex Williamson <alex.williamson@redhat.com>,
	devicetree@vger.kernel.org, linux-renesas-soc@vger.kernel.org,
	linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
	Geert Uytterhoeven <geert+renesas@glider.be>
Subject: [PATCH v2] reset: Exclusive resets must be dedicated to a single hardware block
Date: Thu, 27 Sep 2018 20:00:38 +0200	[thread overview]
Message-ID: <20180927180038.24599-1-geert+renesas@glider.be> (raw)

In some SoCs multiple hardware blocks may share a reset control.
The reset control API for shared resets will only assert such a reset
when the drivers for all hardware blocks agree.
The exclusive reset control API still allows to assert such a reset, but
that impacts all other hardware blocks sharing the reset.

While the kernel doc comments clearly state that the API for shared
resets applies to reset controls which are shared between hardware
blocks, the exact meaning of exclusive resets is not documented.
Fix the semantic ambiguity with respect to exclusive access vs.
exclusive reset lines by:
  1. Clarifying that exclusive resets really are intended for use with
     reset controls which are dedicated to a single hardware block,
  2. Ensuring that obtaining an exclusive reset control will fail if the
     reset is shared by multiple hardware blocks, for both DT-based and
     lookup-based reset controls.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
This is v2 of "[RFC] reset: Add support for dedicated reset controls":
  - Fix wrong variable in __reset_is_dedicated() loop,
  - Add missing of_node_put() in __of_reset_is_dedicated(),
  - Document that exclusive reset controls imply they are dedicated to a
    single hardware block,
  - Drop new dedicated flag and new API reset_control_get_dedicated(),
    as exclusive already implies dedicated,
  - Rename {__of_,}reset_is_dedicated() to {__of_,}reset_is_exclusive(),
  - Reword description.

Note: Exclusive lookup-based reset controls were not tested.
---
 drivers/reset/core.c  | 58 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/reset.h |  5 +++-
 2 files changed, 62 insertions(+), 1 deletion(-)

diff --git a/drivers/reset/core.c b/drivers/reset/core.c
index 225e34c56b94a2e3..2f5b61226c7964eb 100644
--- a/drivers/reset/core.c
+++ b/drivers/reset/core.c
@@ -459,6 +459,38 @@ static void __reset_control_put_internal(struct reset_control *rstc)
 	kref_put(&rstc->refcnt, __reset_control_release);
 }
 
+static bool __of_reset_is_exclusive(const struct device_node *node,
+				    const struct of_phandle_args args)
+{
+	struct of_phandle_args args2;
+	struct device_node *node2;
+	int index, ret;
+	bool eq;
+
+	for_each_node_with_property(node2, "resets") {
+		if (node == node2)
+			continue;
+
+		for (index = 0; ; index++) {
+			ret = of_parse_phandle_with_args(node2, "resets",
+							 "#reset-cells", index,
+							 &args2);
+			if (ret)
+				break;
+
+			eq = (args2.np == args.np &&
+			      args2.args_count == args.args_count &&
+			      !memcmp(args2.args, args.args,
+				      args.args_count * sizeof(args.args[0])));
+			of_node_put(args2.np);
+			if (eq)
+				return false;
+		}
+	}
+
+	return true;
+}
+
 struct reset_control *__of_reset_control_get(struct device_node *node,
 				     const char *id, int index, bool shared,
 				     bool optional)
@@ -514,6 +546,11 @@ struct reset_control *__of_reset_control_get(struct device_node *node,
 		return ERR_PTR(rstc_id);
 	}
 
+	if (!shared && !__of_reset_is_exclusive(node, args)) {
+		mutex_unlock(&reset_list_mutex);
+		return ERR_PTR(-EINVAL);
+	}
+
 	/* reset_list_mutex also protects the rcdev's reset_control list */
 	rstc = __reset_control_get_internal(rcdev, rstc_id, shared);
 
@@ -541,6 +578,22 @@ __reset_controller_by_name(const char *name)
 	return NULL;
 }
 
+static bool __reset_is_exclusive(const struct reset_control_lookup *lookup)
+{
+	const struct reset_control_lookup *lookup2;
+
+	list_for_each_entry(lookup2, &reset_lookup_list, list) {
+		if (lookup2 == lookup)
+			continue;
+
+		if (lookup2->provider == lookup->provider &&
+		    lookup2->index == lookup->index)
+			return false;
+	}
+
+	return true;
+}
+
 static struct reset_control *
 __reset_control_get_from_lookup(struct device *dev, const char *con_id,
 				bool shared, bool optional)
@@ -562,6 +615,11 @@ __reset_control_get_from_lookup(struct device *dev, const char *con_id,
 		if ((!con_id && !lookup->con_id) ||
 		    ((con_id && lookup->con_id) &&
 		     !strcmp(con_id, lookup->con_id))) {
+			if (!shared && !__reset_is_exclusive(lookup)) {
+				mutex_unlock(&reset_lookup_mutex);
+				return ERR_PTR(-EINVAL);
+			}
+
 			mutex_lock(&reset_list_mutex);
 			rcdev = __reset_controller_by_name(lookup->provider);
 			if (!rcdev) {
diff --git a/include/linux/reset.h b/include/linux/reset.h
index 29af6d6b2f4b8103..5881d2594761e48f 100644
--- a/include/linux/reset.h
+++ b/include/linux/reset.h
@@ -116,8 +116,11 @@ static inline int device_reset_optional(struct device *dev)
  * @id: reset line name
  *
  * Returns a struct reset_control or IS_ERR() condition containing errno.
- * If this function is called more than once for the same reset_control it will
+ * If this function is called more than once for the same reset control it will
  * return -EBUSY.
+ * This function is intended for use with reset controls which are dedicated
+ * to a single hardware block.  If called for a reset control shared among
+ * multiple hardware blocks, it will return -EINVAL.
  *
  * See reset_control_get_shared for details on shared references to
  * reset-controls.
-- 
2.17.1


             reply	other threads:[~2018-09-27 18:00 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-27 18:00 Geert Uytterhoeven [this message]
2018-10-04 10:14 ` Philipp Zabel
2018-10-05  8:55   ` Geert Uytterhoeven
2018-10-05  8:55     ` Geert Uytterhoeven
2018-10-05 12:31     ` Geert Uytterhoeven
2018-10-05 12:31       ` Geert Uytterhoeven
2018-10-05 15:16       ` Philipp Zabel
2018-10-05 15:16         ` Philipp Zabel
2018-10-08  9:59         ` Geert Uytterhoeven
2018-10-08  9:59           ` Geert Uytterhoeven
2018-10-08 10:57           ` Philipp Zabel
2018-10-08 10:57             ` Philipp Zabel
2018-10-08 11:47             ` Geert Uytterhoeven
2018-10-08 11:47               ` Geert Uytterhoeven
2018-10-08 12:57               ` Philipp Zabel
2018-10-08 12:57                 ` Philipp Zabel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180927180038.24599-1-geert+renesas@glider.be \
    --to=geert+renesas@glider.be \
    --cc=alex.williamson@redhat.com \
    --cc=devicetree@vger.kernel.org \
    --cc=eric.auger@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=p.zabel@pengutronix.de \
    --subject='Re: [PATCH v2] reset: Exclusive resets must be dedicated to a single hardware block' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.