All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/4] reset: APIs to manage a list of resets
@ 2017-04-03 13:41 Vivek Gautam
  2017-04-03 13:41 ` [PATCH v2 1/4] reset: Add API to count number of reset available with device Vivek Gautam
                   ` (2 more replies)
  0 siblings, 3 replies; 26+ messages in thread
From: Vivek Gautam @ 2017-04-03 13:41 UTC (permalink / raw)
  To: linux-kernel, linux-tegra, linux-usb
  Cc: p.zabel, swarren, thierry.reding, balbi, Vivek Gautam

Set of patches to support getting and de/asserting a list (array)
of reset controllers available with the device.

These patches address the comments received for the earlier
posted series [1,2].
The users can pass a list of reset controllers and the number of
resets available in the driver or the ones using device tree can
request the total number of reset controllers available in the
device node and pass the information to reset_control_array_* APIs
to _get(), _assert() and _deassert() the list of resets.

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

-- Based on Torvald's master branch.
-- Tested usb on db820c target for dwc3-of-simple driver.
-- Build tested for tegra/pmc driver. Needs testing.

[1] https://lkml.org/lkml/2017/2/22/12
[2] https://lkml.org/lkml/2017/2/22/11

Vivek Gautam (4):
  reset: Add API to count number of reset available with device
  reset: Add APIs to manage array of resets
  usb: dwc3: of-simple: Add support to get resets for the device
  soc/tegra: pmc: Use the new reset APIs to manage reset controllers

 drivers/reset/core.c              | 191 ++++++++++++++++++++++++++++++++++++++
 drivers/soc/tegra/pmc.c           |  92 ++++++++----------
 drivers/usb/dwc3/dwc3-of-simple.c |  45 +++++++++
 include/linux/reset.h             | 114 +++++++++++++++++++++++
 4 files changed, 389 insertions(+), 53 deletions(-)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

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

* [PATCH v2 1/4] reset: Add API to count number of reset available with device
  2017-04-03 13:41 [PATCH v2 0/4] reset: APIs to manage a list of resets Vivek Gautam
@ 2017-04-03 13:41 ` Vivek Gautam
       [not found]   ` <1491226922-20307-2-git-send-email-vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
  2017-04-03 13:42 ` [PATCH v2 2/4] reset: Add APIs to manage array of resets Vivek Gautam
       [not found] ` <1491226922-20307-1-git-send-email-vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
  2 siblings, 1 reply; 26+ messages in thread
From: Vivek Gautam @ 2017-04-03 13:41 UTC (permalink / raw)
  To: linux-kernel, linux-tegra, linux-usb
  Cc: p.zabel, swarren, thierry.reding, balbi, Vivek Gautam

Count number of reset phandles available with the device node
to know the resets a given device has.

Cc: Philipp Zabel <p.zabel@pengutronix.de>
Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
---

Changes since v1:
 - Handling the error path by returning error code for failures
   and ENODEV for count equal to 0.
 - Moved the function to drivers instead of putting in the includes.

 drivers/reset/core.c  | 22 ++++++++++++++++++++++
 include/linux/reset.h |  6 ++++++
 2 files changed, 28 insertions(+)

diff --git a/drivers/reset/core.c b/drivers/reset/core.c
index f1e5e65388bb..66db061165cb 100644
--- a/drivers/reset/core.c
+++ b/drivers/reset/core.c
@@ -455,3 +455,25 @@ int device_reset(struct device *dev)
 	return ret;
 }
 EXPORT_SYMBOL_GPL(device_reset);
+
+/**
+ * of_reset_control_get_count - Count number of resets available with a device
+ *
+ * @node: device node that contains 'resets'.
+ *
+ * Returns positive reset count on success, or error number on failure and
+ * on count being zero.
+ */
+int of_reset_control_get_count(struct device_node *node)
+{
+	int count;
+
+	count = of_count_phandle_with_args(node, "resets", "#reset-cells");
+	if (count < 0)
+		return count;
+	if (count == 0)
+		return -ENODEV;
+
+	return count;
+}
+EXPORT_SYMBOL_GPL(of_reset_control_get_count);
diff --git a/include/linux/reset.h b/include/linux/reset.h
index 96fb139bdd08..d89556412ccc 100644
--- a/include/linux/reset.h
+++ b/include/linux/reset.h
@@ -21,6 +21,7 @@ struct reset_control *__devm_reset_control_get(struct device *dev,
 				     bool optional);
 
 int __must_check device_reset(struct device *dev);
+int of_reset_control_get_count(struct device_node *node);
 
 static inline int device_reset_optional(struct device *dev)
 {
@@ -79,6 +80,11 @@ static inline struct reset_control *__devm_reset_control_get(
 	return optional ? NULL : ERR_PTR(-ENOTSUPP);
 }
 
+static inline int of_reset_control_get_count(struct device_node *node);
+{
+	return -ENOTSUPP;
+}
+
 #endif /* CONFIG_RESET_CONTROLLER */
 
 /**
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

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

* [PATCH v2 2/4] reset: Add APIs to manage array of resets
  2017-04-03 13:41 [PATCH v2 0/4] reset: APIs to manage a list of resets Vivek Gautam
  2017-04-03 13:41 ` [PATCH v2 1/4] reset: Add API to count number of reset available with device Vivek Gautam
@ 2017-04-03 13:42 ` Vivek Gautam
       [not found]   ` <1491226922-20307-3-git-send-email-vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
       [not found] ` <1491226922-20307-1-git-send-email-vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
  2 siblings, 1 reply; 26+ messages in thread
From: Vivek Gautam @ 2017-04-03 13:42 UTC (permalink / raw)
  To: linux-kernel, linux-tegra, linux-usb
  Cc: p.zabel, swarren, thierry.reding, balbi, Vivek Gautam

Many devices may want to request a bunch of resets
and control them. So it's better to manage them as an
array. Add APIs to _get(), _assert(), and _deassert()
an array of reset_control.

Cc: Philipp Zabel <p.zabel@pengutronix.de>
Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
---

Changes since v1:
 - New patch added to the series.

 drivers/reset/core.c  | 169 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/reset.h | 108 ++++++++++++++++++++++++++++++++
 2 files changed, 277 insertions(+)

diff --git a/drivers/reset/core.c b/drivers/reset/core.c
index 66db061165cb..fb908aa4f69e 100644
--- a/drivers/reset/core.c
+++ b/drivers/reset/core.c
@@ -477,3 +477,172 @@ int of_reset_control_get_count(struct device_node *node)
 	return count;
 }
 EXPORT_SYMBOL_GPL(of_reset_control_get_count);
+
+/*
+ * APIs to manage an array of reset controllers
+ */
+/**
+ * reset_control_array_assert: assert a list of resets
+ *
+ * @resets: reset control array holding info about a list of resets
+ * @num_rsts: number of resets to be asserted.
+ *
+ * Returns 0 on success or error number on failure.
+ */
+int reset_control_array_assert(int num_rsts,
+				struct reset_control_array *resets)
+{
+	int ret, i;
+
+	if (WARN_ON(IS_ERR_OR_NULL(resets)))
+		return -EINVAL;
+
+	for (i = 0; i < num_rsts; i++) {
+		ret = reset_control_assert(resets[i].rst);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(reset_control_array_assert);
+
+/**
+ * reset_control_array_deassert: deassert a list of resets
+ *
+ * @resets: reset control array holding info about a list of resets
+ * @num_rsts: number of resets to be deasserted.
+ *
+ * Returns 0 on success or error number on failure.
+ */
+int reset_control_array_deassert(int num_rsts,
+				struct reset_control_array *resets)
+{
+	int ret, i;
+
+	if (WARN_ON(IS_ERR_OR_NULL(resets)))
+		return -EINVAL;
+
+	for (i = 0; i < num_rsts; i++)
+		ret = reset_control_deassert(resets[i].rst);
+		if (ret)
+			goto err;
+
+	return 0;
+
+err:
+	while (--i >= 0)
+		reset_control_assert(resets[i].rst);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(reset_control_array_deassert);
+
+/**
+ * reset_control_array_get - Get a list of reset controllers
+ *
+ * @dev: device that requests the list of reset controllers
+ * @num_rsts: number of reset controllers requested
+ * @resets: reset control array holding info about a list of resets
+ * @shared: whether reset controllers are shared or not
+ * @optional: whether it is optional to get the reset controllers
+ *
+ * Returns 0 on success or error number on failure
+ */
+static int reset_control_array_get(struct device *dev, int num_rsts,
+				struct reset_control_array *resets,
+				bool shared, bool optional)
+{
+	int ret, i;
+	struct reset_control *rstc;
+
+	for (i = 0; i < num_rsts; i++) {
+		rstc = __of_reset_control_get(dev ? dev->of_node : NULL,
+					resets[i].name, i, shared, optional);
+		if (IS_ERR(rstc)) {
+			ret = PTR_ERR(rstc);
+			goto err_rst;
+		}
+		resets[i].rst = rstc;
+	}
+
+	return 0;
+
+err_rst:
+	while (--i >= 0)
+		reset_control_put(resets[i].rst);
+	return ret;
+}
+
+static void devm_reset_control_array_release(struct device *dev, void *res)
+{
+	struct reset_control_devres *ptr = res;
+	int i;
+
+	for (i = 0; i < ptr->num_resets; i++)
+		reset_control_put(ptr->resets[i].rst);
+}
+
+/**
+ * devm_reset_control_array_get - Resource managed reset_control_array_get
+ *				  See reset_control_array_get() for more
+ *				  details
+ *
+ * Returns 0 on success or error number on failure
+ */
+int devm_reset_control_array_get(struct device *dev, int num_rsts,
+				struct reset_control_array *resets,
+				bool shared, bool optional)
+{
+	struct reset_control_devres *ptr;
+	int ret;
+
+	ptr = devres_alloc(devm_reset_control_array_release, sizeof(*ptr),
+			   GFP_KERNEL);
+	if (!ptr)
+		return -ENOMEM;
+
+	ret = reset_control_array_get(dev, num_rsts, resets,
+					shared, optional);
+	if (ret) {
+		ptr->resets = resets;
+		ptr->num_resets = num_rsts;
+		devres_add(dev, ptr);
+	} else {
+		devres_free(ptr);
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(devm_reset_control_array_get);
+
+int of_reset_control_array_get(struct device_node *node, int num_rsts,
+				struct reset_control_array *resets,
+				bool shared, bool optional)
+{
+	int ret, i;
+	struct reset_control *rstc;
+
+	for (i = 0; i < num_rsts; i++) {
+		rstc = __of_reset_control_get(node, NULL, i, shared, optional);
+		if (IS_ERR(rstc)) {
+			ret = PTR_ERR(rstc);
+			goto err_rst;
+		}
+		resets[i].rst = rstc;
+	}
+
+	return 0;
+
+err_rst:
+	while (--i >= 0)
+		reset_control_put(resets[i].rst);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(of_reset_control_array_get);
+
+void reset_control_array_put(int num, struct reset_control_array *resets)
+{
+	while (num--)
+		reset_control_put(resets[num].rst);
+}
+EXPORT_SYMBOL_GPL(reset_control_array_put);
diff --git a/include/linux/reset.h b/include/linux/reset.h
index d89556412ccc..d6ca70b9d634 100644
--- a/include/linux/reset.h
+++ b/include/linux/reset.h
@@ -5,6 +5,16 @@
 
 struct reset_control;
 
+struct reset_control_array {
+	struct reset_control *rst;
+	const char *name;
+};
+
+struct reset_control_devres {
+	struct reset_control_array *resets;
+	int num_resets;
+};
+
 #ifdef CONFIG_RESET_CONTROLLER
 
 int reset_control_reset(struct reset_control *rstc);
@@ -23,6 +33,18 @@ struct reset_control *__devm_reset_control_get(struct device *dev,
 int __must_check device_reset(struct device *dev);
 int of_reset_control_get_count(struct device_node *node);
 
+int reset_control_array_assert(int num_rsts,
+				struct reset_control_array *resets);
+int reset_control_array_deassert(int num_rsts,
+				struct reset_control_array *resets);
+int devm_reset_control_array_get(struct device *dev, int num_rsts,
+				struct reset_control_array *resets,
+				bool shared, bool optional);
+int of_reset_control_array_get(struct device_node *node, int num_rsts,
+				struct reset_control_array *resets,
+				bool shared, bool optional);
+void reset_control_array_put(int num, struct reset_control_array *resets);
+
 static inline int device_reset_optional(struct device *dev)
 {
 	return device_reset(dev);
@@ -85,6 +107,39 @@ static inline struct reset_control *__devm_reset_control_get(
 	return -ENOTSUPP;
 }
 
+static inline int reset_control_array_assert(int num_rsts,
+				struct reset_control_array *resets)
+{
+	return 0;
+}
+
+static inline int reset_control_array_deassert(int num_rsts,
+				struct reset_control_array *resets)
+{
+	return 0;
+}
+
+static inline
+int devm_reset_control_array_get(struct device *dev, int num_rsts,
+				struct reset_control_array *resets,
+				bool shared, bool optional)
+{
+	return optional ? NULL : ERR_PTR(-ENOTSUPP);
+}
+
+static inline
+int of_reset_control_array_get(struct device_node *node, int num_rsts,
+				struct reset_control_array *resets,
+				bool shared, bool optional)
+{
+	return optional ? NULL : ERR_PTR(-ENOTSUPP);
+}
+
+static inline
+void reset_control_array_put(int num, struct reset_control_array *resets)
+{
+}
+
 #endif /* CONFIG_RESET_CONTROLLER */
 
 /**
@@ -374,4 +429,57 @@ static inline struct reset_control *devm_reset_control_get_by_index(
 {
 	return devm_reset_control_get_exclusive_by_index(dev, index);
 }
+
+/*
+ * APIs to manage a list of reset controllers
+ */
+static inline
+int devm_reset_control_array_get_exclusive(struct device *dev, int num_rsts,
+					struct reset_control_array *resets)
+{
+	return devm_reset_control_array_get(dev, num_rsts,
+						resets, false, false);
+}
+
+static inline
+int devm_reset_control_array_get_shared(struct device *dev, int num_rsts,
+					struct reset_control_array *resets)
+{
+	return devm_reset_control_array_get(dev, num_rsts,
+						resets, true, false);
+}
+
+static inline
+int devm_reset_control_array_get_optional_exclusive(struct device *dev,
+					int num_rsts,
+					struct reset_control_array *resets)
+{
+	return devm_reset_control_array_get(dev, num_rsts,
+						resets, false, true);
+}
+
+static inline
+int devm_reset_control_array_get_optional_shared(struct device *dev,
+					int num_rsts,
+					struct reset_control_array *resets)
+{
+	return devm_reset_control_array_get(dev, num_rsts,
+						resets, true, true);
+}
+
+static inline
+int of_reset_control_array_get_exclusive(struct device_node *node,
+					int num_rsts,
+					struct reset_control_array *resets)
+{
+	return of_reset_control_array_get(node, num_rsts, resets, false, false);
+}
+
+static inline
+int of_reset_control_array_get_shared(struct device_node *node,
+					int num_rsts,
+					struct reset_control_array *resets)
+{
+	return of_reset_control_array_get(node, num_rsts, resets, true, false);
+}
 #endif
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

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

* [PATCH v2 3/4] usb: dwc3: of-simple: Add support to get resets for the device
  2017-04-03 13:41 [PATCH v2 0/4] reset: APIs to manage a list of resets Vivek Gautam
@ 2017-04-03 13:42     ` Vivek Gautam
  2017-04-03 13:42 ` [PATCH v2 2/4] reset: Add APIs to manage array of resets Vivek Gautam
       [not found] ` <1491226922-20307-1-git-send-email-vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
  2 siblings, 0 replies; 26+ messages in thread
From: Vivek Gautam @ 2017-04-03 13:42 UTC (permalink / raw)
  To: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA
  Cc: p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ, swarren-3lzwWm7+Weoh9ZMKESR00Q,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w,
	balbi-DgEjT+Ai2ygdnm+yROfE0A, Vivek Gautam

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

Cc: Felipe Balbi <balbi-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Signed-off-by: Vivek Gautam <vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
---

Changes since v1:
 - Using new APIs for reset control array.
 - Using the *_get_exclusive() API.
 - Added reset_control_array_put() to error and driver removal path.

 drivers/usb/dwc3/dwc3-of-simple.c | 45 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/drivers/usb/dwc3/dwc3-of-simple.c b/drivers/usb/dwc3/dwc3-of-simple.c
index fe414e7a9c78..96786f5ede0b 100644
--- a/drivers/usb/dwc3/dwc3-of-simple.c
+++ b/drivers/usb/dwc3/dwc3-of-simple.c
@@ -29,13 +29,48 @@
 #include <linux/of.h>
 #include <linux/of_platform.h>
 #include <linux/pm_runtime.h>
+#include <linux/reset.h>
 
 struct dwc3_of_simple {
 	struct device		*dev;
 	struct clk		**clks;
 	int			num_clocks;
+	int			num_resets;
+	struct reset_control_array *resets;
 };
 
+static int dwc3_of_simple_reset_init(struct dwc3_of_simple *simple, int count)
+{
+	struct device		*dev = simple->dev;
+	int			ret;
+
+	/* Not all platforms have resets, so don't return a failure */
+	if (count < 0)
+		return 0;
+
+	simple->num_resets = count;
+
+	simple->resets = devm_kcalloc(dev, count, sizeof(simple->resets),
+							GFP_KERNEL);
+	if (!simple->resets)
+		return -ENOMEM;
+
+	ret = of_reset_control_array_get_exclusive(dev->of_node, count,
+							simple->resets);
+	if (ret) {
+		dev_err(dev, "failed to get device resets\n");
+		return ret;
+	}
+
+	ret = reset_control_array_deassert(count, simple->resets);
+	if (ret) {
+		reset_control_array_put(count, simple->resets);
+		return ret;
+	}
+
+	return 0;
+}
+
 static int dwc3_of_simple_clk_init(struct dwc3_of_simple *simple, int count)
 {
 	struct device		*dev = simple->dev;
@@ -100,6 +135,10 @@ static int dwc3_of_simple_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	ret = dwc3_of_simple_reset_init(simple, of_reset_control_get_count(np));
+	if (ret)
+		return ret;
+
 	ret = of_platform_populate(np, NULL, NULL, dev);
 	if (ret) {
 		for (i = 0; i < simple->num_clocks; i++) {
@@ -107,6 +146,9 @@ static int dwc3_of_simple_probe(struct platform_device *pdev)
 			clk_put(simple->clks[i]);
 		}
 
+		reset_control_array_assert(simple->num_resets, simple->resets);
+		reset_control_array_put(simple->num_resets, simple->resets);
+
 		return ret;
 	}
 
@@ -128,6 +170,9 @@ static int dwc3_of_simple_remove(struct platform_device *pdev)
 		clk_put(simple->clks[i]);
 	}
 
+	reset_control_array_assert(simple->num_resets, simple->resets);
+	reset_control_array_put(simple->num_resets, simple->resets);
+
 	of_platform_depopulate(dev);
 
 	pm_runtime_put_sync(dev);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

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

* [PATCH v2 3/4] usb: dwc3: of-simple: Add support to get resets for the device
@ 2017-04-03 13:42     ` Vivek Gautam
  0 siblings, 0 replies; 26+ messages in thread
From: Vivek Gautam @ 2017-04-03 13:42 UTC (permalink / raw)
  To: linux-kernel, linux-tegra, linux-usb
  Cc: p.zabel, swarren, thierry.reding, balbi, Vivek Gautam

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

Cc: Felipe Balbi <balbi@kernel.org>
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
---

Changes since v1:
 - Using new APIs for reset control array.
 - Using the *_get_exclusive() API.
 - Added reset_control_array_put() to error and driver removal path.

 drivers/usb/dwc3/dwc3-of-simple.c | 45 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/drivers/usb/dwc3/dwc3-of-simple.c b/drivers/usb/dwc3/dwc3-of-simple.c
index fe414e7a9c78..96786f5ede0b 100644
--- a/drivers/usb/dwc3/dwc3-of-simple.c
+++ b/drivers/usb/dwc3/dwc3-of-simple.c
@@ -29,13 +29,48 @@
 #include <linux/of.h>
 #include <linux/of_platform.h>
 #include <linux/pm_runtime.h>
+#include <linux/reset.h>
 
 struct dwc3_of_simple {
 	struct device		*dev;
 	struct clk		**clks;
 	int			num_clocks;
+	int			num_resets;
+	struct reset_control_array *resets;
 };
 
+static int dwc3_of_simple_reset_init(struct dwc3_of_simple *simple, int count)
+{
+	struct device		*dev = simple->dev;
+	int			ret;
+
+	/* Not all platforms have resets, so don't return a failure */
+	if (count < 0)
+		return 0;
+
+	simple->num_resets = count;
+
+	simple->resets = devm_kcalloc(dev, count, sizeof(simple->resets),
+							GFP_KERNEL);
+	if (!simple->resets)
+		return -ENOMEM;
+
+	ret = of_reset_control_array_get_exclusive(dev->of_node, count,
+							simple->resets);
+	if (ret) {
+		dev_err(dev, "failed to get device resets\n");
+		return ret;
+	}
+
+	ret = reset_control_array_deassert(count, simple->resets);
+	if (ret) {
+		reset_control_array_put(count, simple->resets);
+		return ret;
+	}
+
+	return 0;
+}
+
 static int dwc3_of_simple_clk_init(struct dwc3_of_simple *simple, int count)
 {
 	struct device		*dev = simple->dev;
@@ -100,6 +135,10 @@ static int dwc3_of_simple_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	ret = dwc3_of_simple_reset_init(simple, of_reset_control_get_count(np));
+	if (ret)
+		return ret;
+
 	ret = of_platform_populate(np, NULL, NULL, dev);
 	if (ret) {
 		for (i = 0; i < simple->num_clocks; i++) {
@@ -107,6 +146,9 @@ static int dwc3_of_simple_probe(struct platform_device *pdev)
 			clk_put(simple->clks[i]);
 		}
 
+		reset_control_array_assert(simple->num_resets, simple->resets);
+		reset_control_array_put(simple->num_resets, simple->resets);
+
 		return ret;
 	}
 
@@ -128,6 +170,9 @@ static int dwc3_of_simple_remove(struct platform_device *pdev)
 		clk_put(simple->clks[i]);
 	}
 
+	reset_control_array_assert(simple->num_resets, simple->resets);
+	reset_control_array_put(simple->num_resets, simple->resets);
+
 	of_platform_depopulate(dev);
 
 	pm_runtime_put_sync(dev);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

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

* [PATCH v2 4/4] soc/tegra: pmc: Use the new reset APIs to manage reset controllers
  2017-04-03 13:41 [PATCH v2 0/4] reset: APIs to manage a list of resets Vivek Gautam
@ 2017-04-03 13:42     ` Vivek Gautam
  2017-04-03 13:42 ` [PATCH v2 2/4] reset: Add APIs to manage array of resets Vivek Gautam
       [not found] ` <1491226922-20307-1-git-send-email-vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
  2 siblings, 0 replies; 26+ messages in thread
From: Vivek Gautam @ 2017-04-03 13:42 UTC (permalink / raw)
  To: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA
  Cc: p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ, swarren-3lzwWm7+Weoh9ZMKESR00Q,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w,
	balbi-DgEjT+Ai2ygdnm+yROfE0A, Vivek Gautam, Thierry Reding

Make use of reset_control_array_*() set of APIs to manage
an array of reset controllers available with the device.

Cc: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Cc: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Signed-off-by: Vivek Gautam <vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
---

Changes since v1:
 - New patch added to the series to enable this driver use the
   newer set of APIs that manage reset control array.

 drivers/soc/tegra/pmc.c | 92 +++++++++++++++++++++----------------------------
 1 file changed, 39 insertions(+), 53 deletions(-)

diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index e233dd5dcab3..d06e624381aa 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -124,7 +124,7 @@ struct tegra_powergate {
 	unsigned int id;
 	struct clk **clks;
 	unsigned int num_clks;
-	struct reset_control **resets;
+	struct reset_control_array *resets;
 	unsigned int num_resets;
 };
 
@@ -348,32 +348,14 @@ static int tegra_powergate_enable_clocks(struct tegra_powergate *pg)
 	return err;
 }
 
-static int tegra_powergate_reset_assert(struct tegra_powergate *pg)
+static inline int tegra_powergate_reset_assert(struct tegra_powergate *pg)
 {
-	unsigned int i;
-	int err;
-
-	for (i = 0; i < pg->num_resets; i++) {
-		err = reset_control_assert(pg->resets[i]);
-		if (err)
-			return err;
-	}
-
-	return 0;
+	return reset_control_array_assert(pg->num_resets, pg->resets);
 }
 
-static int tegra_powergate_reset_deassert(struct tegra_powergate *pg)
+static inline int tegra_powergate_reset_deassert(struct tegra_powergate *pg)
 {
-	unsigned int i;
-	int err;
-
-	for (i = 0; i < pg->num_resets; i++) {
-		err = reset_control_deassert(pg->resets[i]);
-		if (err)
-			return err;
-	}
-
-	return 0;
+	return reset_control_array_deassert(pg->num_resets, pg->resets);
 }
 
 static int tegra_powergate_power_up(struct tegra_powergate *pg,
@@ -566,12 +548,24 @@ int tegra_powergate_sequence_power_up(unsigned int id, struct clk *clk,
 	pg.id = id;
 	pg.clks = &clk;
 	pg.num_clks = 1;
-	pg.resets = &rst;
 	pg.num_resets = 1;
 
+	pg.resets = kcalloc(1, sizeof(pg.resets), GFP_KERNEL);
+	if (!pg.resets)
+		return -ENOMEM;
+
+	pg.resets[0].rst = rst;
+
 	err = tegra_powergate_power_up(&pg, false);
-	if (err)
+	if (err) {
 		pr_err("failed to turn on partition %d: %d\n", id, err);
+		goto free_reset;
+	}
+
+	return 0;
+
+free_reset:
+	kfree(pg.resets);
 
 	return err;
 }
@@ -755,44 +749,38 @@ static int tegra_powergate_of_get_clks(struct tegra_powergate *pg,
 static int tegra_powergate_of_get_resets(struct tegra_powergate *pg,
 					 struct device_node *np, bool off)
 {
-	struct reset_control *rst;
-	unsigned int i, count;
+	unsigned int count;
 	int err;
 
-	count = of_count_phandle_with_args(np, "resets", "#reset-cells");
-	if (count == 0)
-		return -ENODEV;
+	count = of_reset_control_get_count(np);
+	if (count < 0)
+		return count;
 
-	pg->resets = kcalloc(count, sizeof(rst), GFP_KERNEL);
+	pg->resets = kcalloc(count, sizeof(pg->resets), GFP_KERNEL);
 	if (!pg->resets)
 		return -ENOMEM;
 
-	for (i = 0; i < count; i++) {
-		pg->resets[i] = of_reset_control_get_by_index(np, i);
-		if (IS_ERR(pg->resets[i])) {
-			err = PTR_ERR(pg->resets[i]);
-			goto error;
-		}
+	err = of_reset_control_array_get_exclusive(np, count, pg->resets);
+	if (err) {
+		pr_err("failed to get device resets\n");
+		goto free_reset;
+	}
 
-		if (off)
-			err = reset_control_assert(pg->resets[i]);
-		else
-			err = reset_control_deassert(pg->resets[i]);
+	if (off)
+		err = reset_control_array_assert(count, pg->resets);
+	else
+		err = reset_control_array_deassert(count, pg->resets);
 
-		if (err) {
-			reset_control_put(pg->resets[i]);
-			goto error;
-		}
-	}
+	if (err)
+		goto put_reset;
 
 	pg->num_resets = count;
 
 	return 0;
 
-error:
-	while (i--)
-		reset_control_put(pg->resets[i]);
-
+put_reset:
+	reset_control_array_put(count, pg->resets);
+free_reset:
 	kfree(pg->resets);
 
 	return err;
@@ -885,9 +873,7 @@ static void tegra_powergate_add(struct tegra_pmc *pmc, struct device_node *np)
 	pm_genpd_remove(&pg->genpd);
 
 remove_resets:
-	while (pg->num_resets--)
-		reset_control_put(pg->resets[pg->num_resets]);

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

* [PATCH v2 4/4] soc/tegra: pmc: Use the new reset APIs to manage reset controllers
@ 2017-04-03 13:42     ` Vivek Gautam
  0 siblings, 0 replies; 26+ messages in thread
From: Vivek Gautam @ 2017-04-03 13:42 UTC (permalink / raw)
  To: linux-kernel, linux-tegra, linux-usb
  Cc: p.zabel, swarren, thierry.reding, balbi, Vivek Gautam, Thierry Reding

Make use of reset_control_array_*() set of APIs to manage
an array of reset controllers available with the device.

Cc: Thierry Reding <treding@nvidia.com>
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
---

Changes since v1:
 - New patch added to the series to enable this driver use the
   newer set of APIs that manage reset control array.

 drivers/soc/tegra/pmc.c | 92 +++++++++++++++++++++----------------------------
 1 file changed, 39 insertions(+), 53 deletions(-)

diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index e233dd5dcab3..d06e624381aa 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -124,7 +124,7 @@ struct tegra_powergate {
 	unsigned int id;
 	struct clk **clks;
 	unsigned int num_clks;
-	struct reset_control **resets;
+	struct reset_control_array *resets;
 	unsigned int num_resets;
 };
 
@@ -348,32 +348,14 @@ static int tegra_powergate_enable_clocks(struct tegra_powergate *pg)
 	return err;
 }
 
-static int tegra_powergate_reset_assert(struct tegra_powergate *pg)
+static inline int tegra_powergate_reset_assert(struct tegra_powergate *pg)
 {
-	unsigned int i;
-	int err;
-
-	for (i = 0; i < pg->num_resets; i++) {
-		err = reset_control_assert(pg->resets[i]);
-		if (err)
-			return err;
-	}
-
-	return 0;
+	return reset_control_array_assert(pg->num_resets, pg->resets);
 }
 
-static int tegra_powergate_reset_deassert(struct tegra_powergate *pg)
+static inline int tegra_powergate_reset_deassert(struct tegra_powergate *pg)
 {
-	unsigned int i;
-	int err;
-
-	for (i = 0; i < pg->num_resets; i++) {
-		err = reset_control_deassert(pg->resets[i]);
-		if (err)
-			return err;
-	}
-
-	return 0;
+	return reset_control_array_deassert(pg->num_resets, pg->resets);
 }
 
 static int tegra_powergate_power_up(struct tegra_powergate *pg,
@@ -566,12 +548,24 @@ int tegra_powergate_sequence_power_up(unsigned int id, struct clk *clk,
 	pg.id = id;
 	pg.clks = &clk;
 	pg.num_clks = 1;
-	pg.resets = &rst;
 	pg.num_resets = 1;
 
+	pg.resets = kcalloc(1, sizeof(pg.resets), GFP_KERNEL);
+	if (!pg.resets)
+		return -ENOMEM;
+
+	pg.resets[0].rst = rst;
+
 	err = tegra_powergate_power_up(&pg, false);
-	if (err)
+	if (err) {
 		pr_err("failed to turn on partition %d: %d\n", id, err);
+		goto free_reset;
+	}
+
+	return 0;
+
+free_reset:
+	kfree(pg.resets);
 
 	return err;
 }
@@ -755,44 +749,38 @@ static int tegra_powergate_of_get_clks(struct tegra_powergate *pg,
 static int tegra_powergate_of_get_resets(struct tegra_powergate *pg,
 					 struct device_node *np, bool off)
 {
-	struct reset_control *rst;
-	unsigned int i, count;
+	unsigned int count;
 	int err;
 
-	count = of_count_phandle_with_args(np, "resets", "#reset-cells");
-	if (count == 0)
-		return -ENODEV;
+	count = of_reset_control_get_count(np);
+	if (count < 0)
+		return count;
 
-	pg->resets = kcalloc(count, sizeof(rst), GFP_KERNEL);
+	pg->resets = kcalloc(count, sizeof(pg->resets), GFP_KERNEL);
 	if (!pg->resets)
 		return -ENOMEM;
 
-	for (i = 0; i < count; i++) {
-		pg->resets[i] = of_reset_control_get_by_index(np, i);
-		if (IS_ERR(pg->resets[i])) {
-			err = PTR_ERR(pg->resets[i]);
-			goto error;
-		}
+	err = of_reset_control_array_get_exclusive(np, count, pg->resets);
+	if (err) {
+		pr_err("failed to get device resets\n");
+		goto free_reset;
+	}
 
-		if (off)
-			err = reset_control_assert(pg->resets[i]);
-		else
-			err = reset_control_deassert(pg->resets[i]);
+	if (off)
+		err = reset_control_array_assert(count, pg->resets);
+	else
+		err = reset_control_array_deassert(count, pg->resets);
 
-		if (err) {
-			reset_control_put(pg->resets[i]);
-			goto error;
-		}
-	}
+	if (err)
+		goto put_reset;
 
 	pg->num_resets = count;
 
 	return 0;
 
-error:
-	while (i--)
-		reset_control_put(pg->resets[i]);
-
+put_reset:
+	reset_control_array_put(count, pg->resets);
+free_reset:
 	kfree(pg->resets);
 
 	return err;
@@ -885,9 +873,7 @@ static void tegra_powergate_add(struct tegra_pmc *pmc, struct device_node *np)
 	pm_genpd_remove(&pg->genpd);
 
 remove_resets:
-	while (pg->num_resets--)
-		reset_control_put(pg->resets[pg->num_resets]);
-
+	reset_control_array_put(pg->num_resets, pg->resets);
 	kfree(pg->resets);
 
 remove_clks:
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

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

* Re: [PATCH v2 2/4] reset: Add APIs to manage array of resets
  2017-04-03 13:42 ` [PATCH v2 2/4] reset: Add APIs to manage array of resets Vivek Gautam
@ 2017-04-03 16:36       ` Philipp Zabel
  0 siblings, 0 replies; 26+ messages in thread
From: Philipp Zabel @ 2017-04-03 16:36 UTC (permalink / raw)
  To: Vivek Gautam
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA, swarren-3lzwWm7+Weoh9ZMKESR00Q,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w,
	balbi-DgEjT+Ai2ygdnm+yROfE0A

Hi Vivek,

thank you for the patch series. A few comments below: I'd like to reduce
the API surface a bit and include the counting and array allocation in
the _get functions, if possible.

On Mon, 2017-04-03 at 19:12 +0530, Vivek Gautam wrote:
> Many devices may want to request a bunch of resets
> and control them. So it's better to manage them as an
> array. Add APIs to _get(), _assert(), and _deassert()
> an array of reset_control.
> 
> Cc: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> Signed-off-by: Vivek Gautam <vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> ---
> 
> Changes since v1:
>  - New patch added to the series.
> 
>  drivers/reset/core.c  | 169 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/reset.h | 108 ++++++++++++++++++++++++++++++++
>  2 files changed, 277 insertions(+)
> 
> diff --git a/drivers/reset/core.c b/drivers/reset/core.c
> index 66db061165cb..fb908aa4f69e 100644
> --- a/drivers/reset/core.c
> +++ b/drivers/reset/core.c
> @@ -477,3 +477,172 @@ int of_reset_control_get_count(struct device_node *node)
>  	return count;
>  }
>  EXPORT_SYMBOL_GPL(of_reset_control_get_count);
> +
> +/*
> + * APIs to manage an array of reset controllers
> + */
> +/**
> + * reset_control_array_assert: assert a list of resets
> + *
> + * @resets: reset control array holding info about a list of resets
> + * @num_rsts: number of resets to be asserted.

This should mention the API doesn't make any guarantees that the reset
lines controlled by the reset array are asserted or deasserted in any
particular order.

> + * Returns 0 on success or error number on failure.
> + */
> +int reset_control_array_assert(int num_rsts,
> +				struct reset_control_array *resets)

I'd prefer to mirror the gpiod API a little, and to have the number
contained in the array structure, similar to struct gpio_descs:

struct reset_control_array {
	unsigned int num_rstcs;
	struct reset_control *rstc[];
};

int reset_control_array_assert(struct reset_control_array *resets)
{
	...

> +{
> +	int ret, i;
> +
> +	if (WARN_ON(IS_ERR_OR_NULL(resets)))
> +		return -EINVAL;
> +
> +	for (i = 0; i < num_rsts; i++) {
> +		ret = reset_control_assert(resets[i].rst);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(reset_control_array_assert);
> +
> +/**
> + * reset_control_array_deassert: deassert a list of resets
> + *
> + * @resets: reset control array holding info about a list of resets
> + * @num_rsts: number of resets to be deasserted.

Same here, no guarantees on the order in which the resets are
deasserted.

> + * Returns 0 on success or error number on failure.
> + */
> +int reset_control_array_deassert(int num_rsts,
> +				struct reset_control_array *resets)
> +{
> +	int ret, i;
> +
> +	if (WARN_ON(IS_ERR_OR_NULL(resets)))
> +		return -EINVAL;
> +
> +	for (i = 0; i < num_rsts; i++)
> +		ret = reset_control_deassert(resets[i].rst);
> +		if (ret)
> +			goto err;
> +
> +	return 0;
> +
> +err:
> +	while (--i >= 0)
> +		reset_control_assert(resets[i].rst);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(reset_control_array_deassert);
> +
> +/**
> + * reset_control_array_get - Get a list of reset controllers

A list of "reset controls".

> + *
> + * @dev: device that requests the list of reset controllers
> + * @num_rsts: number of reset controllers requested
> + * @resets: reset control array holding info about a list of resets
> + * @shared: whether reset controllers are shared or not
> + * @optional: whether it is optional to get the reset controllers
> + *

This should mention that the array API is intended for a list of resets
that just have to be asserted or deasserted, without any requirements on
the order.

> + * Returns 0 on success or error number on failure
> + */
> +static int reset_control_array_get(struct device *dev, int num_rsts,
> +				struct reset_control_array *resets,
> +				bool shared, bool optional)

Can we make this count and return a pointer to the newly created array?

static struct reset_controls *
reset_control_array_get(struct device *dev, bool shared, bool optional)
{
	...

That way the consumer doesn't have to care about counting and array
allocation.

> +{
> +	int ret, i;
> +	struct reset_control *rstc;
> +
> +	for (i = 0; i < num_rsts; i++) {
> +		rstc = __of_reset_control_get(dev ? dev->of_node : NULL,
> +					resets[i].name, i, shared, optional);
> +		if (IS_ERR(rstc)) {
> +			ret = PTR_ERR(rstc);
> +			goto err_rst;
> +		}
> +		resets[i].rst = rstc;
> +	}
> +
> +	return 0;
> +
> +err_rst:
> +	while (--i >= 0)
> +		reset_control_put(resets[i].rst);
> +	return ret;
> +}
> +
> +static void devm_reset_control_array_release(struct device *dev, void *res)
> +{
> +	struct reset_control_devres *ptr = res;
> +	int i;
> +
> +	for (i = 0; i < ptr->num_resets; i++)
> +		reset_control_put(ptr->resets[i].rst);
> +}
> +
> +/**
> + * devm_reset_control_array_get - Resource managed reset_control_array_get
> + *				  See reset_control_array_get() for more
> + *				  details
> + *
> + * Returns 0 on success or error number on failure
> + */
> +int devm_reset_control_array_get(struct device *dev, int num_rsts,
> +				struct reset_control_array *resets,
> +				bool shared, bool optional)

Same here.

> +{
> +	struct reset_control_devres *ptr;
> +	int ret;
> +
> +	ptr = devres_alloc(devm_reset_control_array_release, sizeof(*ptr),
> +			   GFP_KERNEL);
> +	if (!ptr)
> +		return -ENOMEM;
> +
> +	ret = reset_control_array_get(dev, num_rsts, resets,
> +					shared, optional);
> +	if (ret) {
> +		ptr->resets = resets;
> +		ptr->num_resets = num_rsts;
> +		devres_add(dev, ptr);
> +	} else {
> +		devres_free(ptr);
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(devm_reset_control_array_get);
> +
> +int of_reset_control_array_get(struct device_node *node, int num_rsts,
> +				struct reset_control_array *resets,
> +				bool shared, bool optional)

And here.

> +{
> +	int ret, i;
> +	struct reset_control *rstc;
> +
> +	for (i = 0; i < num_rsts; i++) {
> +		rstc = __of_reset_control_get(node, NULL, i, shared, optional);
> +		if (IS_ERR(rstc)) {
> +			ret = PTR_ERR(rstc);
> +			goto err_rst;
> +		}
> +		resets[i].rst = rstc;
> +	}
> +
> +	return 0;
> +
> +err_rst:
> +	while (--i >= 0)
> +		reset_control_put(resets[i].rst);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(of_reset_control_array_get);
> +
> +void reset_control_array_put(int num, struct reset_control_array *resets)

This could drop the num then.

> +{
> +	while (num--)
> +		reset_control_put(resets[num].rst);
> +}
> +EXPORT_SYMBOL_GPL(reset_control_array_put);
> diff --git a/include/linux/reset.h b/include/linux/reset.h
> index d89556412ccc..d6ca70b9d634 100644
> --- a/include/linux/reset.h
> +++ b/include/linux/reset.h
> @@ -5,6 +5,16 @@
>  
>  struct reset_control;
>  
> +struct reset_control_array {
> +	struct reset_control *rst;
> +	const char *name;

Drop the name. This interface is only good for a bunch of anonymous
resets that can all be asserted or deasserted in arbitrary order. If the
reset lines have to be known individually, this is probably the wrong
interface.

> +};
> +
> +struct reset_control_devres {
> +	struct reset_control_array *resets;
> +	int num_resets;
> +};
> +
>  #ifdef CONFIG_RESET_CONTROLLER
>  
>  int reset_control_reset(struct reset_control *rstc);
> @@ -23,6 +33,18 @@ struct reset_control *__devm_reset_control_get(struct device *dev,
>  int __must_check device_reset(struct device *dev);
>  int of_reset_control_get_count(struct device_node *node);
>  
> +int reset_control_array_assert(int num_rsts,
> +				struct reset_control_array *resets);
> +int reset_control_array_deassert(int num_rsts,
> +				struct reset_control_array *resets);
> +int devm_reset_control_array_get(struct device *dev, int num_rsts,
> +				struct reset_control_array *resets,
> +				bool shared, bool optional);
> +int of_reset_control_array_get(struct device_node *node, int num_rsts,
> +				struct reset_control_array *resets,
> +				bool shared, bool optional);
> +void reset_control_array_put(int num, struct reset_control_array *resets);
> +
>  static inline int device_reset_optional(struct device *dev)
>  {
>  	return device_reset(dev);
> @@ -85,6 +107,39 @@ static inline struct reset_control *__devm_reset_control_get(
>  	return -ENOTSUPP;
>  }
>  
> +static inline int reset_control_array_assert(int num_rsts,
> +				struct reset_control_array *resets)
> +{
> +	return 0;
> +}
> +
> +static inline int reset_control_array_deassert(int num_rsts,
> +				struct reset_control_array *resets)
> +{
> +	return 0;
> +}

Is this missing a stub for reset_control_array_get?

> +static inline
> +int devm_reset_control_array_get(struct device *dev, int num_rsts,
> +				struct reset_control_array *resets,
> +				bool shared, bool optional)
> +{
> +	return optional ? NULL : ERR_PTR(-ENOTSUPP);
> +}
> +
> +static inline
> +int of_reset_control_array_get(struct device_node *node, int num_rsts,
> +				struct reset_control_array *resets,
> +				bool shared, bool optional)
> +{
> +	return optional ? NULL : ERR_PTR(-ENOTSUPP);
> +}
> +
> +static inline
> +void reset_control_array_put(int num, struct reset_control_array *resets)
> +{
> +}
> +
>  #endif /* CONFIG_RESET_CONTROLLER */
>  
>  /**
> @@ -374,4 +429,57 @@ static inline struct reset_control *devm_reset_control_get_by_index(
>  {
>  	return devm_reset_control_get_exclusive_by_index(dev, index);
>  }
> +
> +/*
> + * APIs to manage a list of reset controllers
> + */
> +static inline
> +int devm_reset_control_array_get_exclusive(struct device *dev, int num_rsts,
> +					struct reset_control_array *resets)
> +{
> +	return devm_reset_control_array_get(dev, num_rsts,
> +						resets, false, false);
> +}
> +
> +static inline
> +int devm_reset_control_array_get_shared(struct device *dev, int num_rsts,
> +					struct reset_control_array *resets)
> +{
> +	return devm_reset_control_array_get(dev, num_rsts,
> +						resets, true, false);
> +}
> +
> +static inline
> +int devm_reset_control_array_get_optional_exclusive(struct device *dev,
> +					int num_rsts,
> +					struct reset_control_array *resets)
> +{
> +	return devm_reset_control_array_get(dev, num_rsts,
> +						resets, false, true);
> +}
> +
> +static inline
> +int devm_reset_control_array_get_optional_shared(struct device *dev,
> +					int num_rsts,
> +					struct reset_control_array *resets)
> +{
> +	return devm_reset_control_array_get(dev, num_rsts,
> +						resets, true, true);
> +}
> +
> +static inline
> +int of_reset_control_array_get_exclusive(struct device_node *node,
> +					int num_rsts,
> +					struct reset_control_array *resets)
> +{
> +	return of_reset_control_array_get(node, num_rsts, resets, false, false);
> +}
> +
> +static inline
> +int of_reset_control_array_get_shared(struct device_node *node,
> +					int num_rsts,
> +					struct reset_control_array *resets)
> +{
> +	return of_reset_control_array_get(node, num_rsts, resets, true, false);
> +}
>  #endif

regards
Philipp

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

* Re: [PATCH v2 2/4] reset: Add APIs to manage array of resets
@ 2017-04-03 16:36       ` Philipp Zabel
  0 siblings, 0 replies; 26+ messages in thread
From: Philipp Zabel @ 2017-04-03 16:36 UTC (permalink / raw)
  To: Vivek Gautam
  Cc: linux-kernel, linux-tegra, linux-usb, swarren, thierry.reding, balbi

Hi Vivek,

thank you for the patch series. A few comments below: I'd like to reduce
the API surface a bit and include the counting and array allocation in
the _get functions, if possible.

On Mon, 2017-04-03 at 19:12 +0530, Vivek Gautam wrote:
> Many devices may want to request a bunch of resets
> and control them. So it's better to manage them as an
> array. Add APIs to _get(), _assert(), and _deassert()
> an array of reset_control.
> 
> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
> ---
> 
> Changes since v1:
>  - New patch added to the series.
> 
>  drivers/reset/core.c  | 169 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/reset.h | 108 ++++++++++++++++++++++++++++++++
>  2 files changed, 277 insertions(+)
> 
> diff --git a/drivers/reset/core.c b/drivers/reset/core.c
> index 66db061165cb..fb908aa4f69e 100644
> --- a/drivers/reset/core.c
> +++ b/drivers/reset/core.c
> @@ -477,3 +477,172 @@ int of_reset_control_get_count(struct device_node *node)
>  	return count;
>  }
>  EXPORT_SYMBOL_GPL(of_reset_control_get_count);
> +
> +/*
> + * APIs to manage an array of reset controllers
> + */
> +/**
> + * reset_control_array_assert: assert a list of resets
> + *
> + * @resets: reset control array holding info about a list of resets
> + * @num_rsts: number of resets to be asserted.

This should mention the API doesn't make any guarantees that the reset
lines controlled by the reset array are asserted or deasserted in any
particular order.

> + * Returns 0 on success or error number on failure.
> + */
> +int reset_control_array_assert(int num_rsts,
> +				struct reset_control_array *resets)

I'd prefer to mirror the gpiod API a little, and to have the number
contained in the array structure, similar to struct gpio_descs:

struct reset_control_array {
	unsigned int num_rstcs;
	struct reset_control *rstc[];
};

int reset_control_array_assert(struct reset_control_array *resets)
{
	...

> +{
> +	int ret, i;
> +
> +	if (WARN_ON(IS_ERR_OR_NULL(resets)))
> +		return -EINVAL;
> +
> +	for (i = 0; i < num_rsts; i++) {
> +		ret = reset_control_assert(resets[i].rst);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(reset_control_array_assert);
> +
> +/**
> + * reset_control_array_deassert: deassert a list of resets
> + *
> + * @resets: reset control array holding info about a list of resets
> + * @num_rsts: number of resets to be deasserted.

Same here, no guarantees on the order in which the resets are
deasserted.

> + * Returns 0 on success or error number on failure.
> + */
> +int reset_control_array_deassert(int num_rsts,
> +				struct reset_control_array *resets)
> +{
> +	int ret, i;
> +
> +	if (WARN_ON(IS_ERR_OR_NULL(resets)))
> +		return -EINVAL;
> +
> +	for (i = 0; i < num_rsts; i++)
> +		ret = reset_control_deassert(resets[i].rst);
> +		if (ret)
> +			goto err;
> +
> +	return 0;
> +
> +err:
> +	while (--i >= 0)
> +		reset_control_assert(resets[i].rst);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(reset_control_array_deassert);
> +
> +/**
> + * reset_control_array_get - Get a list of reset controllers

A list of "reset controls".

> + *
> + * @dev: device that requests the list of reset controllers
> + * @num_rsts: number of reset controllers requested
> + * @resets: reset control array holding info about a list of resets
> + * @shared: whether reset controllers are shared or not
> + * @optional: whether it is optional to get the reset controllers
> + *

This should mention that the array API is intended for a list of resets
that just have to be asserted or deasserted, without any requirements on
the order.

> + * Returns 0 on success or error number on failure
> + */
> +static int reset_control_array_get(struct device *dev, int num_rsts,
> +				struct reset_control_array *resets,
> +				bool shared, bool optional)

Can we make this count and return a pointer to the newly created array?

static struct reset_controls *
reset_control_array_get(struct device *dev, bool shared, bool optional)
{
	...

That way the consumer doesn't have to care about counting and array
allocation.

> +{
> +	int ret, i;
> +	struct reset_control *rstc;
> +
> +	for (i = 0; i < num_rsts; i++) {
> +		rstc = __of_reset_control_get(dev ? dev->of_node : NULL,
> +					resets[i].name, i, shared, optional);
> +		if (IS_ERR(rstc)) {
> +			ret = PTR_ERR(rstc);
> +			goto err_rst;
> +		}
> +		resets[i].rst = rstc;
> +	}
> +
> +	return 0;
> +
> +err_rst:
> +	while (--i >= 0)
> +		reset_control_put(resets[i].rst);
> +	return ret;
> +}
> +
> +static void devm_reset_control_array_release(struct device *dev, void *res)
> +{
> +	struct reset_control_devres *ptr = res;
> +	int i;
> +
> +	for (i = 0; i < ptr->num_resets; i++)
> +		reset_control_put(ptr->resets[i].rst);
> +}
> +
> +/**
> + * devm_reset_control_array_get - Resource managed reset_control_array_get
> + *				  See reset_control_array_get() for more
> + *				  details
> + *
> + * Returns 0 on success or error number on failure
> + */
> +int devm_reset_control_array_get(struct device *dev, int num_rsts,
> +				struct reset_control_array *resets,
> +				bool shared, bool optional)

Same here.

> +{
> +	struct reset_control_devres *ptr;
> +	int ret;
> +
> +	ptr = devres_alloc(devm_reset_control_array_release, sizeof(*ptr),
> +			   GFP_KERNEL);
> +	if (!ptr)
> +		return -ENOMEM;
> +
> +	ret = reset_control_array_get(dev, num_rsts, resets,
> +					shared, optional);
> +	if (ret) {
> +		ptr->resets = resets;
> +		ptr->num_resets = num_rsts;
> +		devres_add(dev, ptr);
> +	} else {
> +		devres_free(ptr);
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(devm_reset_control_array_get);
> +
> +int of_reset_control_array_get(struct device_node *node, int num_rsts,
> +				struct reset_control_array *resets,
> +				bool shared, bool optional)

And here.

> +{
> +	int ret, i;
> +	struct reset_control *rstc;
> +
> +	for (i = 0; i < num_rsts; i++) {
> +		rstc = __of_reset_control_get(node, NULL, i, shared, optional);
> +		if (IS_ERR(rstc)) {
> +			ret = PTR_ERR(rstc);
> +			goto err_rst;
> +		}
> +		resets[i].rst = rstc;
> +	}
> +
> +	return 0;
> +
> +err_rst:
> +	while (--i >= 0)
> +		reset_control_put(resets[i].rst);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(of_reset_control_array_get);
> +
> +void reset_control_array_put(int num, struct reset_control_array *resets)

This could drop the num then.

> +{
> +	while (num--)
> +		reset_control_put(resets[num].rst);
> +}
> +EXPORT_SYMBOL_GPL(reset_control_array_put);
> diff --git a/include/linux/reset.h b/include/linux/reset.h
> index d89556412ccc..d6ca70b9d634 100644
> --- a/include/linux/reset.h
> +++ b/include/linux/reset.h
> @@ -5,6 +5,16 @@
>  
>  struct reset_control;
>  
> +struct reset_control_array {
> +	struct reset_control *rst;
> +	const char *name;

Drop the name. This interface is only good for a bunch of anonymous
resets that can all be asserted or deasserted in arbitrary order. If the
reset lines have to be known individually, this is probably the wrong
interface.

> +};
> +
> +struct reset_control_devres {
> +	struct reset_control_array *resets;
> +	int num_resets;
> +};
> +
>  #ifdef CONFIG_RESET_CONTROLLER
>  
>  int reset_control_reset(struct reset_control *rstc);
> @@ -23,6 +33,18 @@ struct reset_control *__devm_reset_control_get(struct device *dev,
>  int __must_check device_reset(struct device *dev);
>  int of_reset_control_get_count(struct device_node *node);
>  
> +int reset_control_array_assert(int num_rsts,
> +				struct reset_control_array *resets);
> +int reset_control_array_deassert(int num_rsts,
> +				struct reset_control_array *resets);
> +int devm_reset_control_array_get(struct device *dev, int num_rsts,
> +				struct reset_control_array *resets,
> +				bool shared, bool optional);
> +int of_reset_control_array_get(struct device_node *node, int num_rsts,
> +				struct reset_control_array *resets,
> +				bool shared, bool optional);
> +void reset_control_array_put(int num, struct reset_control_array *resets);
> +
>  static inline int device_reset_optional(struct device *dev)
>  {
>  	return device_reset(dev);
> @@ -85,6 +107,39 @@ static inline struct reset_control *__devm_reset_control_get(
>  	return -ENOTSUPP;
>  }
>  
> +static inline int reset_control_array_assert(int num_rsts,
> +				struct reset_control_array *resets)
> +{
> +	return 0;
> +}
> +
> +static inline int reset_control_array_deassert(int num_rsts,
> +				struct reset_control_array *resets)
> +{
> +	return 0;
> +}

Is this missing a stub for reset_control_array_get?

> +static inline
> +int devm_reset_control_array_get(struct device *dev, int num_rsts,
> +				struct reset_control_array *resets,
> +				bool shared, bool optional)
> +{
> +	return optional ? NULL : ERR_PTR(-ENOTSUPP);
> +}
> +
> +static inline
> +int of_reset_control_array_get(struct device_node *node, int num_rsts,
> +				struct reset_control_array *resets,
> +				bool shared, bool optional)
> +{
> +	return optional ? NULL : ERR_PTR(-ENOTSUPP);
> +}
> +
> +static inline
> +void reset_control_array_put(int num, struct reset_control_array *resets)
> +{
> +}
> +
>  #endif /* CONFIG_RESET_CONTROLLER */
>  
>  /**
> @@ -374,4 +429,57 @@ static inline struct reset_control *devm_reset_control_get_by_index(
>  {
>  	return devm_reset_control_get_exclusive_by_index(dev, index);
>  }
> +
> +/*
> + * APIs to manage a list of reset controllers
> + */
> +static inline
> +int devm_reset_control_array_get_exclusive(struct device *dev, int num_rsts,
> +					struct reset_control_array *resets)
> +{
> +	return devm_reset_control_array_get(dev, num_rsts,
> +						resets, false, false);
> +}
> +
> +static inline
> +int devm_reset_control_array_get_shared(struct device *dev, int num_rsts,
> +					struct reset_control_array *resets)
> +{
> +	return devm_reset_control_array_get(dev, num_rsts,
> +						resets, true, false);
> +}
> +
> +static inline
> +int devm_reset_control_array_get_optional_exclusive(struct device *dev,
> +					int num_rsts,
> +					struct reset_control_array *resets)
> +{
> +	return devm_reset_control_array_get(dev, num_rsts,
> +						resets, false, true);
> +}
> +
> +static inline
> +int devm_reset_control_array_get_optional_shared(struct device *dev,
> +					int num_rsts,
> +					struct reset_control_array *resets)
> +{
> +	return devm_reset_control_array_get(dev, num_rsts,
> +						resets, true, true);
> +}
> +
> +static inline
> +int of_reset_control_array_get_exclusive(struct device_node *node,
> +					int num_rsts,
> +					struct reset_control_array *resets)
> +{
> +	return of_reset_control_array_get(node, num_rsts, resets, false, false);
> +}
> +
> +static inline
> +int of_reset_control_array_get_shared(struct device_node *node,
> +					int num_rsts,
> +					struct reset_control_array *resets)
> +{
> +	return of_reset_control_array_get(node, num_rsts, resets, true, false);
> +}
>  #endif

regards
Philipp

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

* Re: [PATCH v2 2/4] reset: Add APIs to manage array of resets
  2017-04-03 13:42 ` [PATCH v2 2/4] reset: Add APIs to manage array of resets Vivek Gautam
@ 2017-04-04  4:12       ` kbuild test robot
  0 siblings, 0 replies; 26+ messages in thread
From: kbuild test robot @ 2017-04-04  4:12 UTC (permalink / raw)
  Cc: kbuild-all-JC7UmRfGjtg, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA, p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ,
	swarren-3lzwWm7+Weoh9ZMKESR00Q,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w,
	balbi-DgEjT+Ai2ygdnm+yROfE0A, Vivek Gautam

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

Hi Vivek,

[auto build test WARNING on balbi-usb/next]
[also build test WARNING on v4.11-rc5 next-20170403]
[cannot apply to pza/reset/next]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Vivek-Gautam/reset-APIs-to-manage-a-list-of-resets/20170404-111639
base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next
config: x86_64-randconfig-x008-201714 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   drivers/reset/core.c: In function 'reset_control_array_deassert':
   drivers/reset/core.c:526:2: warning: this 'for' clause does not guard... [-Wmisleading-indentation]
     for (i = 0; i < num_rsts; i++)
     ^~~
   drivers/reset/core.c:528:3: note: ...this statement, but the latter is misleadingly indented as if it is guarded by the 'for'
      if (ret)
      ^~
>> drivers/reset/core.c:531:9: warning: 'ret' may be used uninitialized in this function [-Wmaybe-uninitialized]
     return 0;
            ^

vim +/ret +531 drivers/reset/core.c

   520	{
   521		int ret, i;
   522	
   523		if (WARN_ON(IS_ERR_OR_NULL(resets)))
   524			return -EINVAL;
   525	
 > 526		for (i = 0; i < num_rsts; i++)
   527			ret = reset_control_deassert(resets[i].rst);
   528			if (ret)
   529				goto err;
   530	
 > 531		return 0;
   532	
   533	err:
   534		while (--i >= 0)

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 25352 bytes --]

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

* Re: [PATCH v2 2/4] reset: Add APIs to manage array of resets
@ 2017-04-04  4:12       ` kbuild test robot
  0 siblings, 0 replies; 26+ messages in thread
From: kbuild test robot @ 2017-04-04  4:12 UTC (permalink / raw)
  To: Vivek Gautam
  Cc: kbuild-all, linux-kernel, linux-tegra, linux-usb, p.zabel,
	swarren, thierry.reding, balbi, Vivek Gautam

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

Hi Vivek,

[auto build test WARNING on balbi-usb/next]
[also build test WARNING on v4.11-rc5 next-20170403]
[cannot apply to pza/reset/next]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Vivek-Gautam/reset-APIs-to-manage-a-list-of-resets/20170404-111639
base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next
config: x86_64-randconfig-x008-201714 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   drivers/reset/core.c: In function 'reset_control_array_deassert':
   drivers/reset/core.c:526:2: warning: this 'for' clause does not guard... [-Wmisleading-indentation]
     for (i = 0; i < num_rsts; i++)
     ^~~
   drivers/reset/core.c:528:3: note: ...this statement, but the latter is misleadingly indented as if it is guarded by the 'for'
      if (ret)
      ^~
>> drivers/reset/core.c:531:9: warning: 'ret' may be used uninitialized in this function [-Wmaybe-uninitialized]
     return 0;
            ^

vim +/ret +531 drivers/reset/core.c

   520	{
   521		int ret, i;
   522	
   523		if (WARN_ON(IS_ERR_OR_NULL(resets)))
   524			return -EINVAL;
   525	
 > 526		for (i = 0; i < num_rsts; i++)
   527			ret = reset_control_deassert(resets[i].rst);
   528			if (ret)
   529				goto err;
   530	
 > 531		return 0;
   532	
   533	err:
   534		while (--i >= 0)

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 25352 bytes --]

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

* Re: [PATCH v2 2/4] reset: Add APIs to manage array of resets
  2017-04-03 13:42 ` [PATCH v2 2/4] reset: Add APIs to manage array of resets Vivek Gautam
@ 2017-04-04  4:14       ` kbuild test robot
  0 siblings, 0 replies; 26+ messages in thread
From: kbuild test robot @ 2017-04-04  4:14 UTC (permalink / raw)
  Cc: kbuild-all-JC7UmRfGjtg, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA, p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ,
	swarren-3lzwWm7+Weoh9ZMKESR00Q,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w,
	balbi-DgEjT+Ai2ygdnm+yROfE0A, Vivek Gautam

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

Hi Vivek,

[auto build test WARNING on balbi-usb/next]
[also build test WARNING on v4.11-rc5 next-20170403]
[cannot apply to pza/reset/next]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Vivek-Gautam/reset-APIs-to-manage-a-list-of-resets/20170404-111639
base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next
config: x86_64-randconfig-x004-201714 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   drivers/reset/core.c: In function 'reset_control_array_deassert':
>> drivers/reset/core.c:526:2: warning: this 'for' clause does not guard... [-Wmisleading-indentation]
     for (i = 0; i < num_rsts; i++)
     ^~~
   drivers/reset/core.c:528:3: note: ...this statement, but the latter is misleadingly indented as if it is guarded by the 'for'
      if (ret)
      ^~

vim +/for +526 drivers/reset/core.c

   510	/**
   511	 * reset_control_array_deassert: deassert a list of resets
   512	 *
   513	 * @resets: reset control array holding info about a list of resets
   514	 * @num_rsts: number of resets to be deasserted.
   515	 *
   516	 * Returns 0 on success or error number on failure.
   517	 */
   518	int reset_control_array_deassert(int num_rsts,
   519					struct reset_control_array *resets)
   520	{
   521		int ret, i;
   522	
   523		if (WARN_ON(IS_ERR_OR_NULL(resets)))
   524			return -EINVAL;
   525	
 > 526		for (i = 0; i < num_rsts; i++)
   527			ret = reset_control_deassert(resets[i].rst);
   528			if (ret)
   529				goto err;
   530	
   531		return 0;
   532	
   533	err:
   534		while (--i >= 0)

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28383 bytes --]

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

* Re: [PATCH v2 2/4] reset: Add APIs to manage array of resets
@ 2017-04-04  4:14       ` kbuild test robot
  0 siblings, 0 replies; 26+ messages in thread
From: kbuild test robot @ 2017-04-04  4:14 UTC (permalink / raw)
  To: Vivek Gautam
  Cc: kbuild-all, linux-kernel, linux-tegra, linux-usb, p.zabel,
	swarren, thierry.reding, balbi, Vivek Gautam

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

Hi Vivek,

[auto build test WARNING on balbi-usb/next]
[also build test WARNING on v4.11-rc5 next-20170403]
[cannot apply to pza/reset/next]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Vivek-Gautam/reset-APIs-to-manage-a-list-of-resets/20170404-111639
base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next
config: x86_64-randconfig-x004-201714 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   drivers/reset/core.c: In function 'reset_control_array_deassert':
>> drivers/reset/core.c:526:2: warning: this 'for' clause does not guard... [-Wmisleading-indentation]
     for (i = 0; i < num_rsts; i++)
     ^~~
   drivers/reset/core.c:528:3: note: ...this statement, but the latter is misleadingly indented as if it is guarded by the 'for'
      if (ret)
      ^~

vim +/for +526 drivers/reset/core.c

   510	/**
   511	 * reset_control_array_deassert: deassert a list of resets
   512	 *
   513	 * @resets: reset control array holding info about a list of resets
   514	 * @num_rsts: number of resets to be deasserted.
   515	 *
   516	 * Returns 0 on success or error number on failure.
   517	 */
   518	int reset_control_array_deassert(int num_rsts,
   519					struct reset_control_array *resets)
   520	{
   521		int ret, i;
   522	
   523		if (WARN_ON(IS_ERR_OR_NULL(resets)))
   524			return -EINVAL;
   525	
 > 526		for (i = 0; i < num_rsts; i++)
   527			ret = reset_control_deassert(resets[i].rst);
   528			if (ret)
   529				goto err;
   530	
   531		return 0;
   532	
   533	err:
   534		while (--i >= 0)

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28383 bytes --]

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

* Re: [PATCH v2 1/4] reset: Add API to count number of reset available with device
  2017-04-03 13:41 ` [PATCH v2 1/4] reset: Add API to count number of reset available with device Vivek Gautam
@ 2017-04-04  4:22       ` kbuild test robot
  0 siblings, 0 replies; 26+ messages in thread
From: kbuild test robot @ 2017-04-04  4:22 UTC (permalink / raw)
  Cc: kbuild-all-JC7UmRfGjtg, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA, p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ,
	swarren-3lzwWm7+Weoh9ZMKESR00Q,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w,
	balbi-DgEjT+Ai2ygdnm+yROfE0A, Vivek Gautam

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

Hi Vivek,

[auto build test ERROR on balbi-usb/next]
[also build test ERROR on v4.11-rc5 next-20170403]
[cannot apply to pza/reset/next]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Vivek-Gautam/reset-APIs-to-manage-a-list-of-resets/20170404-111639
base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next
config: i386-randconfig-r0-201714 (attached as .config)
compiler: gcc-5 (Debian 5.4.1-2) 5.4.1 20160904
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   In file included from drivers/usb/dwc2/platform.c:48:0:
>> include/linux/reset.h:84:1: error: expected identifier or '(' before '{' token
    {
    ^
   include/linux/reset.h:83:19: warning: 'of_reset_control_get_count' declared 'static' but never defined [-Wunused-function]
    static inline int of_reset_control_get_count(struct device_node *node);
                      ^

vim +84 include/linux/reset.h

    78						int index, bool shared, bool optional)
    79	{
    80		return optional ? NULL : ERR_PTR(-ENOTSUPP);
    81	}
    82	
    83	static inline int of_reset_control_get_count(struct device_node *node);
  > 84	{
    85		return -ENOTSUPP;
    86	}
    87	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 29765 bytes --]

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

* Re: [PATCH v2 1/4] reset: Add API to count number of reset available with device
@ 2017-04-04  4:22       ` kbuild test robot
  0 siblings, 0 replies; 26+ messages in thread
From: kbuild test robot @ 2017-04-04  4:22 UTC (permalink / raw)
  To: Vivek Gautam
  Cc: kbuild-all, linux-kernel, linux-tegra, linux-usb, p.zabel,
	swarren, thierry.reding, balbi, Vivek Gautam

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

Hi Vivek,

[auto build test ERROR on balbi-usb/next]
[also build test ERROR on v4.11-rc5 next-20170403]
[cannot apply to pza/reset/next]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Vivek-Gautam/reset-APIs-to-manage-a-list-of-resets/20170404-111639
base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next
config: i386-randconfig-r0-201714 (attached as .config)
compiler: gcc-5 (Debian 5.4.1-2) 5.4.1 20160904
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   In file included from drivers/usb/dwc2/platform.c:48:0:
>> include/linux/reset.h:84:1: error: expected identifier or '(' before '{' token
    {
    ^
   include/linux/reset.h:83:19: warning: 'of_reset_control_get_count' declared 'static' but never defined [-Wunused-function]
    static inline int of_reset_control_get_count(struct device_node *node);
                      ^

vim +84 include/linux/reset.h

    78						int index, bool shared, bool optional)
    79	{
    80		return optional ? NULL : ERR_PTR(-ENOTSUPP);
    81	}
    82	
    83	static inline int of_reset_control_get_count(struct device_node *node);
  > 84	{
    85		return -ENOTSUPP;
    86	}
    87	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 29765 bytes --]

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

* Re: [PATCH v2 2/4] reset: Add APIs to manage array of resets
  2017-04-04  4:14       ` kbuild test robot
@ 2017-04-04  4:23           ` Vivek Gautam
  -1 siblings, 0 replies; 26+ messages in thread
From: Vivek Gautam @ 2017-04-04  4:23 UTC (permalink / raw)
  To: kbuild test robot
  Cc: kbuild-all-JC7UmRfGjtg, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA, p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ,
	swarren-3lzwWm7+Weoh9ZMKESR00Q,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w,
	balbi-DgEjT+Ai2ygdnm+yROfE0A



On 04/04/2017 09:44 AM, kbuild test robot wrote:
> Hi Vivek,
>
> [auto build test WARNING on balbi-usb/next]
> [also build test WARNING on v4.11-rc5 next-20170403]
> [cannot apply to pza/reset/next]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url:    https://github.com/0day-ci/linux/commits/Vivek-Gautam/reset-APIs-to-manage-a-list-of-resets/20170404-111639
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next
> config: x86_64-randconfig-x004-201714 (attached as .config)
> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
> reproduce:
>          # save the attached .config to linux build tree
>          make ARCH=x86_64
>
> All warnings (new ones prefixed by >>):
>
>     drivers/reset/core.c: In function 'reset_control_array_deassert':
>>> drivers/reset/core.c:526:2: warning: this 'for' clause does not guard... [-Wmisleading-indentation]
>       for (i = 0; i < num_rsts; i++)
>       ^~~
>     drivers/reset/core.c:528:3: note: ...this statement, but the latter is misleadingly indented as if it is guarded by the 'for'
>        if (ret)
>        ^~

Right, gonna fix these warnings in the next version of the patch.
Thanks

> vim +/for +526 drivers/reset/core.c
>
>     510	/**
>     511	 * reset_control_array_deassert: deassert a list of resets
>     512	 *
>     513	 * @resets: reset control array holding info about a list of resets
>     514	 * @num_rsts: number of resets to be deasserted.
>     515	 *
>     516	 * Returns 0 on success or error number on failure.
>     517	 */
>     518	int reset_control_array_deassert(int num_rsts,
>     519					struct reset_control_array *resets)
>     520	{
>     521		int ret, i;
>     522	
>     523		if (WARN_ON(IS_ERR_OR_NULL(resets)))
>     524			return -EINVAL;
>     525	
>   > 526		for (i = 0; i < num_rsts; i++)
>     527			ret = reset_control_deassert(resets[i].rst);
>     528			if (ret)
>     529				goto err;
>     530	
>     531		return 0;
>     532	
>     533	err:
>     534		while (--i >= 0)
>
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

BRs
Vivek

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

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

* Re: [PATCH v2 2/4] reset: Add APIs to manage array of resets
@ 2017-04-04  4:23           ` Vivek Gautam
  0 siblings, 0 replies; 26+ messages in thread
From: Vivek Gautam @ 2017-04-04  4:23 UTC (permalink / raw)
  To: kbuild test robot
  Cc: kbuild-all, linux-kernel, linux-tegra, linux-usb, p.zabel,
	swarren, thierry.reding, balbi



On 04/04/2017 09:44 AM, kbuild test robot wrote:
> Hi Vivek,
>
> [auto build test WARNING on balbi-usb/next]
> [also build test WARNING on v4.11-rc5 next-20170403]
> [cannot apply to pza/reset/next]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url:    https://github.com/0day-ci/linux/commits/Vivek-Gautam/reset-APIs-to-manage-a-list-of-resets/20170404-111639
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next
> config: x86_64-randconfig-x004-201714 (attached as .config)
> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
> reproduce:
>          # save the attached .config to linux build tree
>          make ARCH=x86_64
>
> All warnings (new ones prefixed by >>):
>
>     drivers/reset/core.c: In function 'reset_control_array_deassert':
>>> drivers/reset/core.c:526:2: warning: this 'for' clause does not guard... [-Wmisleading-indentation]
>       for (i = 0; i < num_rsts; i++)
>       ^~~
>     drivers/reset/core.c:528:3: note: ...this statement, but the latter is misleadingly indented as if it is guarded by the 'for'
>        if (ret)
>        ^~

Right, gonna fix these warnings in the next version of the patch.
Thanks

> vim +/for +526 drivers/reset/core.c
>
>     510	/**
>     511	 * reset_control_array_deassert: deassert a list of resets
>     512	 *
>     513	 * @resets: reset control array holding info about a list of resets
>     514	 * @num_rsts: number of resets to be deasserted.
>     515	 *
>     516	 * Returns 0 on success or error number on failure.
>     517	 */
>     518	int reset_control_array_deassert(int num_rsts,
>     519					struct reset_control_array *resets)
>     520	{
>     521		int ret, i;
>     522	
>     523		if (WARN_ON(IS_ERR_OR_NULL(resets)))
>     524			return -EINVAL;
>     525	
>   > 526		for (i = 0; i < num_rsts; i++)
>     527			ret = reset_control_deassert(resets[i].rst);
>     528			if (ret)
>     529				goto err;
>     530	
>     531		return 0;
>     532	
>     533	err:
>     534		while (--i >= 0)
>
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

BRs
Vivek

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

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

* Re: [PATCH v2 3/4] usb: dwc3: of-simple: Add support to get resets for the device
  2017-04-03 13:42     ` Vivek Gautam
@ 2017-04-04  5:20         ` kbuild test robot
  -1 siblings, 0 replies; 26+ messages in thread
From: kbuild test robot @ 2017-04-04  5:20 UTC (permalink / raw)
  Cc: kbuild-all-JC7UmRfGjtg, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA, p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ,
	swarren-3lzwWm7+Weoh9ZMKESR00Q,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w,
	balbi-DgEjT+Ai2ygdnm+yROfE0A, Vivek Gautam

Hi Vivek,

[auto build test WARNING on balbi-usb/next]
[also build test WARNING on v4.11-rc5 next-20170403]
[cannot apply to pza/reset/next]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Vivek-Gautam/reset-APIs-to-manage-a-list-of-resets/20170404-111639
base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next


coccinelle warnings: (new ones prefixed by >>)

>> drivers/usb/dwc3/dwc3-of-simple.c:53:43-49: ERROR: application of sizeof to pointer

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

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

* [PATCH] usb: dwc3: of-simple: fix noderef.cocci warnings
  2017-04-03 13:42     ` Vivek Gautam
@ 2017-04-04  5:20         ` kbuild test robot
  -1 siblings, 0 replies; 26+ messages in thread
From: kbuild test robot @ 2017-04-04  5:20 UTC (permalink / raw)
  Cc: kbuild-all-JC7UmRfGjtg, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA, p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ,
	swarren-3lzwWm7+Weoh9ZMKESR00Q,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w,
	balbi-DgEjT+Ai2ygdnm+yROfE0A, Vivek Gautam

drivers/usb/dwc3/dwc3-of-simple.c:53:43-49: ERROR: application of sizeof to pointer

 sizeof when applied to a pointer typed expression gives the size of
 the pointer

Generated by: scripts/coccinelle/misc/noderef.cocci

CC: Vivek Gautam <vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Signed-off-by: Fengguang Wu <fengguang.wu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---

 dwc3-of-simple.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/usb/dwc3/dwc3-of-simple.c
+++ b/drivers/usb/dwc3/dwc3-of-simple.c
@@ -50,7 +50,7 @@ static int dwc3_of_simple_reset_init(str
 
 	simple->num_resets = count;
 
-	simple->resets = devm_kcalloc(dev, count, sizeof(simple->resets),
+	simple->resets = devm_kcalloc(dev, count, sizeof(*simple->resets),
 							GFP_KERNEL);
 	if (!simple->resets)
 		return -ENOMEM;

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

* Re: [PATCH v2 3/4] usb: dwc3: of-simple: Add support to get resets for the device
@ 2017-04-04  5:20         ` kbuild test robot
  0 siblings, 0 replies; 26+ messages in thread
From: kbuild test robot @ 2017-04-04  5:20 UTC (permalink / raw)
  To: Vivek Gautam
  Cc: kbuild-all, linux-kernel, linux-tegra, linux-usb, p.zabel,
	swarren, thierry.reding, balbi, Vivek Gautam

Hi Vivek,

[auto build test WARNING on balbi-usb/next]
[also build test WARNING on v4.11-rc5 next-20170403]
[cannot apply to pza/reset/next]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Vivek-Gautam/reset-APIs-to-manage-a-list-of-resets/20170404-111639
base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next


coccinelle warnings: (new ones prefixed by >>)

>> drivers/usb/dwc3/dwc3-of-simple.c:53:43-49: ERROR: application of sizeof to pointer

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

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

* [PATCH] usb: dwc3: of-simple: fix noderef.cocci warnings
@ 2017-04-04  5:20         ` kbuild test robot
  0 siblings, 0 replies; 26+ messages in thread
From: kbuild test robot @ 2017-04-04  5:20 UTC (permalink / raw)
  To: Vivek Gautam
  Cc: kbuild-all, linux-kernel, linux-tegra, linux-usb, p.zabel,
	swarren, thierry.reding, balbi, Vivek Gautam

drivers/usb/dwc3/dwc3-of-simple.c:53:43-49: ERROR: application of sizeof to pointer

 sizeof when applied to a pointer typed expression gives the size of
 the pointer

Generated by: scripts/coccinelle/misc/noderef.cocci

CC: Vivek Gautam <vivek.gautam@codeaurora.org>
Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
---

 dwc3-of-simple.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/usb/dwc3/dwc3-of-simple.c
+++ b/drivers/usb/dwc3/dwc3-of-simple.c
@@ -50,7 +50,7 @@ static int dwc3_of_simple_reset_init(str
 
 	simple->num_resets = count;
 
-	simple->resets = devm_kcalloc(dev, count, sizeof(simple->resets),
+	simple->resets = devm_kcalloc(dev, count, sizeof(*simple->resets),
 							GFP_KERNEL);
 	if (!simple->resets)
 		return -ENOMEM;

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

* Re: [PATCH v2 3/4] usb: dwc3: of-simple: Add support to get resets for the device
  2017-04-03 13:42     ` Vivek Gautam
@ 2017-04-04  5:34       ` kbuild test robot
  -1 siblings, 0 replies; 26+ messages in thread
From: kbuild test robot @ 2017-04-04  5:34 UTC (permalink / raw)
  Cc: kbuild-all, linux-kernel, linux-tegra, linux-usb, p.zabel,
	swarren, thierry.reding, balbi, Vivek Gautam

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

Hi Vivek,

[auto build test WARNING on balbi-usb/next]
[also build test WARNING on v4.11-rc5 next-20170403]
[cannot apply to pza/reset/next]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Vivek-Gautam/reset-APIs-to-manage-a-list-of-resets/20170404-111639
base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next
config: i386-randconfig-r0-201714 (attached as .config)
compiler: gcc-5 (Debian 5.4.1-2) 5.4.1 20160904
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All warnings (new ones prefixed by >>):

   In file included from drivers/usb/dwc3/dwc3-of-simple.c:32:0:
   include/linux/reset.h:106:1: error: expected identifier or '(' before '{' token
    {
    ^
   include/linux/reset.h: In function 'devm_reset_control_array_get':
   include/linux/reset.h:127:9: warning: return makes integer from pointer without a cast [-Wint-conversion]
     return optional ? NULL : ERR_PTR(-ENOTSUPP);
            ^
   include/linux/reset.h: In function 'of_reset_control_array_get':
   include/linux/reset.h:135:9: warning: return makes integer from pointer without a cast [-Wint-conversion]
     return optional ? NULL : ERR_PTR(-ENOTSUPP);
            ^
   drivers/usb/dwc3/dwc3-of-simple.c: At top level:
>> include/linux/reset.h:105:19: warning: 'of_reset_control_get_count' used but never defined
    static inline int of_reset_control_get_count(struct device_node *node);
                      ^

vim +/of_reset_control_get_count +105 include/linux/reset.h

bb475230 Ramiro Oliveira 2017-01-13   99  					struct device *dev, const char *id,
bb475230 Ramiro Oliveira 2017-01-13  100  					int index, bool shared, bool optional)
5bcd0b7f Axel Lin        2015-09-01  101  {
0ca10b60 Philipp Zabel   2017-03-20  102  	return optional ? NULL : ERR_PTR(-ENOTSUPP);
5bcd0b7f Axel Lin        2015-09-01  103  }
5bcd0b7f Axel Lin        2015-09-01  104  
045cc544 Vivek Gautam    2017-04-03 @105  static inline int of_reset_control_get_count(struct device_node *node);
045cc544 Vivek Gautam    2017-04-03  106  {
045cc544 Vivek Gautam    2017-04-03  107  	return -ENOTSUPP;
045cc544 Vivek Gautam    2017-04-03  108  }
045cc544 Vivek Gautam    2017-04-03  109  
d5652c65 Vivek Gautam    2017-04-03  110  static inline int reset_control_array_assert(int num_rsts,
d5652c65 Vivek Gautam    2017-04-03  111  				struct reset_control_array *resets)
d5652c65 Vivek Gautam    2017-04-03  112  {
d5652c65 Vivek Gautam    2017-04-03  113  	return 0;
d5652c65 Vivek Gautam    2017-04-03  114  }
d5652c65 Vivek Gautam    2017-04-03  115  
d5652c65 Vivek Gautam    2017-04-03  116  static inline int reset_control_array_deassert(int num_rsts,
d5652c65 Vivek Gautam    2017-04-03  117  				struct reset_control_array *resets)
d5652c65 Vivek Gautam    2017-04-03  118  {
d5652c65 Vivek Gautam    2017-04-03  119  	return 0;
d5652c65 Vivek Gautam    2017-04-03  120  }
d5652c65 Vivek Gautam    2017-04-03  121  
d5652c65 Vivek Gautam    2017-04-03  122  static inline
d5652c65 Vivek Gautam    2017-04-03  123  int devm_reset_control_array_get(struct device *dev, int num_rsts,
d5652c65 Vivek Gautam    2017-04-03  124  				struct reset_control_array *resets,
d5652c65 Vivek Gautam    2017-04-03  125  				bool shared, bool optional)
d5652c65 Vivek Gautam    2017-04-03  126  {
d5652c65 Vivek Gautam    2017-04-03 @127  	return optional ? NULL : ERR_PTR(-ENOTSUPP);
d5652c65 Vivek Gautam    2017-04-03  128  }
d5652c65 Vivek Gautam    2017-04-03  129  
d5652c65 Vivek Gautam    2017-04-03  130  static inline

:::::: The code at line 105 was first introduced by commit
:::::: 045cc544119a744cda92da0644066f18cb3760b4 reset: Add API to count number of reset available with device

:::::: TO: Vivek Gautam <vivek.gautam@codeaurora.org>
:::::: CC: 0day robot <fengguang.wu@intel.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 29765 bytes --]

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

* Re: [PATCH v2 3/4] usb: dwc3: of-simple: Add support to get resets for the device
@ 2017-04-04  5:34       ` kbuild test robot
  0 siblings, 0 replies; 26+ messages in thread
From: kbuild test robot @ 2017-04-04  5:34 UTC (permalink / raw)
  To: Vivek Gautam
  Cc: kbuild-all, linux-kernel, linux-tegra, linux-usb, p.zabel,
	swarren, thierry.reding, balbi, Vivek Gautam

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

Hi Vivek,

[auto build test WARNING on balbi-usb/next]
[also build test WARNING on v4.11-rc5 next-20170403]
[cannot apply to pza/reset/next]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Vivek-Gautam/reset-APIs-to-manage-a-list-of-resets/20170404-111639
base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next
config: i386-randconfig-r0-201714 (attached as .config)
compiler: gcc-5 (Debian 5.4.1-2) 5.4.1 20160904
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All warnings (new ones prefixed by >>):

   In file included from drivers/usb/dwc3/dwc3-of-simple.c:32:0:
   include/linux/reset.h:106:1: error: expected identifier or '(' before '{' token
    {
    ^
   include/linux/reset.h: In function 'devm_reset_control_array_get':
   include/linux/reset.h:127:9: warning: return makes integer from pointer without a cast [-Wint-conversion]
     return optional ? NULL : ERR_PTR(-ENOTSUPP);
            ^
   include/linux/reset.h: In function 'of_reset_control_array_get':
   include/linux/reset.h:135:9: warning: return makes integer from pointer without a cast [-Wint-conversion]
     return optional ? NULL : ERR_PTR(-ENOTSUPP);
            ^
   drivers/usb/dwc3/dwc3-of-simple.c: At top level:
>> include/linux/reset.h:105:19: warning: 'of_reset_control_get_count' used but never defined
    static inline int of_reset_control_get_count(struct device_node *node);
                      ^

vim +/of_reset_control_get_count +105 include/linux/reset.h

bb475230 Ramiro Oliveira 2017-01-13   99  					struct device *dev, const char *id,
bb475230 Ramiro Oliveira 2017-01-13  100  					int index, bool shared, bool optional)
5bcd0b7f Axel Lin        2015-09-01  101  {
0ca10b60 Philipp Zabel   2017-03-20  102  	return optional ? NULL : ERR_PTR(-ENOTSUPP);
5bcd0b7f Axel Lin        2015-09-01  103  }
5bcd0b7f Axel Lin        2015-09-01  104  
045cc544 Vivek Gautam    2017-04-03 @105  static inline int of_reset_control_get_count(struct device_node *node);
045cc544 Vivek Gautam    2017-04-03  106  {
045cc544 Vivek Gautam    2017-04-03  107  	return -ENOTSUPP;
045cc544 Vivek Gautam    2017-04-03  108  }
045cc544 Vivek Gautam    2017-04-03  109  
d5652c65 Vivek Gautam    2017-04-03  110  static inline int reset_control_array_assert(int num_rsts,
d5652c65 Vivek Gautam    2017-04-03  111  				struct reset_control_array *resets)
d5652c65 Vivek Gautam    2017-04-03  112  {
d5652c65 Vivek Gautam    2017-04-03  113  	return 0;
d5652c65 Vivek Gautam    2017-04-03  114  }
d5652c65 Vivek Gautam    2017-04-03  115  
d5652c65 Vivek Gautam    2017-04-03  116  static inline int reset_control_array_deassert(int num_rsts,
d5652c65 Vivek Gautam    2017-04-03  117  				struct reset_control_array *resets)
d5652c65 Vivek Gautam    2017-04-03  118  {
d5652c65 Vivek Gautam    2017-04-03  119  	return 0;
d5652c65 Vivek Gautam    2017-04-03  120  }
d5652c65 Vivek Gautam    2017-04-03  121  
d5652c65 Vivek Gautam    2017-04-03  122  static inline
d5652c65 Vivek Gautam    2017-04-03  123  int devm_reset_control_array_get(struct device *dev, int num_rsts,
d5652c65 Vivek Gautam    2017-04-03  124  				struct reset_control_array *resets,
d5652c65 Vivek Gautam    2017-04-03  125  				bool shared, bool optional)
d5652c65 Vivek Gautam    2017-04-03  126  {
d5652c65 Vivek Gautam    2017-04-03 @127  	return optional ? NULL : ERR_PTR(-ENOTSUPP);
d5652c65 Vivek Gautam    2017-04-03  128  }
d5652c65 Vivek Gautam    2017-04-03  129  
d5652c65 Vivek Gautam    2017-04-03  130  static inline

:::::: The code at line 105 was first introduced by commit
:::::: 045cc544119a744cda92da0644066f18cb3760b4 reset: Add API to count number of reset available with device

:::::: TO: Vivek Gautam <vivek.gautam@codeaurora.org>
:::::: CC: 0day robot <fengguang.wu@intel.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 29765 bytes --]

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

* Re: [PATCH v2 2/4] reset: Add APIs to manage array of resets
  2017-04-03 16:36       ` Philipp Zabel
  (?)
@ 2017-04-04 10:39       ` Vivek Gautam
  2017-04-04 12:47         ` Philipp Zabel
  -1 siblings, 1 reply; 26+ messages in thread
From: Vivek Gautam @ 2017-04-04 10:39 UTC (permalink / raw)
  To: Philipp Zabel
  Cc: linux-kernel, linux-tegra, linux-usb, swarren, thierry.reding, balbi

Hi Philipp,


On 04/03/2017 10:06 PM, Philipp Zabel wrote:
> Hi Vivek,
>
> thank you for the patch series. A few comments below: I'd like to reduce
> the API surface a bit and include the counting and array allocation in
> the _get functions, if possible.

Thank you for reviewing the patch. Please find my comments inline.

>
> On Mon, 2017-04-03 at 19:12 +0530, Vivek Gautam wrote:
>> Many devices may want to request a bunch of resets
>> and control them. So it's better to manage them as an
>> array. Add APIs to _get(), _assert(), and _deassert()
>> an array of reset_control.
>>
>> Cc: Philipp Zabel <p.zabel@pengutronix.de>
>> Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
>> ---
>>
>> Changes since v1:
>>   - New patch added to the series.
>>
>>   drivers/reset/core.c  | 169 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>   include/linux/reset.h | 108 ++++++++++++++++++++++++++++++++
>>   2 files changed, 277 insertions(+)
>>
>> diff --git a/drivers/reset/core.c b/drivers/reset/core.c
>> index 66db061165cb..fb908aa4f69e 100644
>> --- a/drivers/reset/core.c
>> +++ b/drivers/reset/core.c
>> @@ -477,3 +477,172 @@ int of_reset_control_get_count(struct device_node *node)
>>   	return count;
>>   }
>>   EXPORT_SYMBOL_GPL(of_reset_control_get_count);
>> +
>> +/*
>> + * APIs to manage an array of reset controllers
>> + */
>> +/**
>> + * reset_control_array_assert: assert a list of resets
>> + *
>> + * @resets: reset control array holding info about a list of resets
>> + * @num_rsts: number of resets to be asserted.
> This should mention the API doesn't make any guarantees that the reset
> lines controlled by the reset array are asserted or deasserted in any
> particular order.

Sure, will add this comment.

>
>> + * Returns 0 on success or error number on failure.
>> + */
>> +int reset_control_array_assert(int num_rsts,
>> +				struct reset_control_array *resets)
> I'd prefer to mirror the gpiod API a little, and to have the number
> contained in the array structure, similar to struct gpio_descs:
>
> struct reset_control_array {
> 	unsigned int num_rstcs;
> 	struct reset_control *rstc[];
> };
>
> int reset_control_array_assert(struct reset_control_array *resets)
> {
> 	...

Alright, i can update this.
I took regulator_bulk interface as the reference, but now i see
gpio_descs has a smaller footprint.

>> +{
>> +	int ret, i;
>> +
>> +	if (WARN_ON(IS_ERR_OR_NULL(resets)))
>> +		return -EINVAL;
>> +
>> +	for (i = 0; i < num_rsts; i++) {
>> +		ret = reset_control_assert(resets[i].rst);
>> +		if (ret)
>> +			return ret;
>> +	}
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(reset_control_array_assert);
>> +
>> +/**
>> + * reset_control_array_deassert: deassert a list of resets
>> + *
>> + * @resets: reset control array holding info about a list of resets
>> + * @num_rsts: number of resets to be deasserted.
> Same here, no guarantees on the order in which the resets are
> deasserted.

sure.

>
>> + * Returns 0 on success or error number on failure.
>> + */
>> +int reset_control_array_deassert(int num_rsts,
>> +				struct reset_control_array *resets)
>> +{
>> +	int ret, i;
>> +
>> +	if (WARN_ON(IS_ERR_OR_NULL(resets)))
>> +		return -EINVAL;
>> +
>> +	for (i = 0; i < num_rsts; i++)
>> +		ret = reset_control_deassert(resets[i].rst);
>> +		if (ret)
>> +			goto err;
>> +
>> +	return 0;
>> +
>> +err:
>> +	while (--i >= 0)
>> +		reset_control_assert(resets[i].rst);
>> +	return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(reset_control_array_deassert);
>> +
>> +/**
>> + * reset_control_array_get - Get a list of reset controllers
> A list of "reset controls".

right, will update this.

>
>> + *
>> + * @dev: device that requests the list of reset controllers
>> + * @num_rsts: number of reset controllers requested
>> + * @resets: reset control array holding info about a list of resets
>> + * @shared: whether reset controllers are shared or not
>> + * @optional: whether it is optional to get the reset controllers
>> + *
> This should mention that the array API is intended for a list of resets
> that just have to be asserted or deasserted, without any requirements on
> the order.

Sure, will mention that.

>
>> + * Returns 0 on success or error number on failure
>> + */
>> +static int reset_control_array_get(struct device *dev, int num_rsts,
>> +				struct reset_control_array *resets,
>> +				bool shared, bool optional)
> Can we make this count and return a pointer to the newly created array?
>
> static struct reset_controls *
> reset_control_array_get(struct device *dev, bool shared, bool optional)
> {
> 	...
>
> That way the consumer doesn't have to care about counting and array
> allocation.

Just trying to think again in line with the regulator bulk APIs.
Can't a user provide a list of reset controls as data and thus
request reset controls with a "id" and num_resets available.

e.g.
    const char * const rst_names[] = {
      "rst1", "rst2" ...
    };
    xyz_data = {
      .resets = rst_names;
      .num = ARRAY_SIZE(rst_names);
    };
and then the driver making use of reset_control_array APIs
to request this list of reset controls and assert/deassert them.

I am assuming this case when one user driver is used across a bunch
of platforms with different number of resets available.
We may not want to rely solely on the device tree entries, since
the resets can be mandatory in few cases and we may want to
return failure.

>
>> +{
>> +	int ret, i;
>> +	struct reset_control *rstc;
>> +
>> +	for (i = 0; i < num_rsts; i++) {
>> +		rstc = __of_reset_control_get(dev ? dev->of_node : NULL,
>> +					resets[i].name, i, shared, optional);
>> +		if (IS_ERR(rstc)) {
>> +			ret = PTR_ERR(rstc);
>> +			goto err_rst;
>> +		}
>> +		resets[i].rst = rstc;
>> +	}
>> +
>> +	return 0;
>> +
>> +err_rst:
>> +	while (--i >= 0)
>> +		reset_control_put(resets[i].rst);
>> +	return ret;
>> +}
>> +
>> +static void devm_reset_control_array_release(struct device *dev, void *res)
>> +{
>> +	struct reset_control_devres *ptr = res;
>> +	int i;
>> +
>> +	for (i = 0; i < ptr->num_resets; i++)
>> +		reset_control_put(ptr->resets[i].rst);
>> +}
>> +
>> +/**
>> + * devm_reset_control_array_get - Resource managed reset_control_array_get
>> + *				  See reset_control_array_get() for more
>> + *				  details
>> + *
>> + * Returns 0 on success or error number on failure
>> + */
>> +int devm_reset_control_array_get(struct device *dev, int num_rsts,
>> +				struct reset_control_array *resets,
>> +				bool shared, bool optional)
> Same here.
>
>> +{
>> +	struct reset_control_devres *ptr;
>> +	int ret;
>> +
>> +	ptr = devres_alloc(devm_reset_control_array_release, sizeof(*ptr),
>> +			   GFP_KERNEL);
>> +	if (!ptr)
>> +		return -ENOMEM;
>> +
>> +	ret = reset_control_array_get(dev, num_rsts, resets,
>> +					shared, optional);
>> +	if (ret) {
>> +		ptr->resets = resets;
>> +		ptr->num_resets = num_rsts;
>> +		devres_add(dev, ptr);
>> +	} else {
>> +		devres_free(ptr);
>> +	}
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(devm_reset_control_array_get);
>> +
>> +int of_reset_control_array_get(struct device_node *node, int num_rsts,
>> +				struct reset_control_array *resets,
>> +				bool shared, bool optional)
> And here.
>
>> +{
>> +	int ret, i;
>> +	struct reset_control *rstc;
>> +
>> +	for (i = 0; i < num_rsts; i++) {
>> +		rstc = __of_reset_control_get(node, NULL, i, shared, optional);
>> +		if (IS_ERR(rstc)) {
>> +			ret = PTR_ERR(rstc);
>> +			goto err_rst;
>> +		}
>> +		resets[i].rst = rstc;
>> +	}
>> +
>> +	return 0;
>> +
>> +err_rst:
>> +	while (--i >= 0)
>> +		reset_control_put(resets[i].rst);
>> +	return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(of_reset_control_array_get);
>> +
>> +void reset_control_array_put(int num, struct reset_control_array *resets)
> This could drop the num then.

Sure.

>
>> +{
>> +	while (num--)
>> +		reset_control_put(resets[num].rst);
>> +}
>> +EXPORT_SYMBOL_GPL(reset_control_array_put);
>> diff --git a/include/linux/reset.h b/include/linux/reset.h
>> index d89556412ccc..d6ca70b9d634 100644
>> --- a/include/linux/reset.h
>> +++ b/include/linux/reset.h
>> @@ -5,6 +5,16 @@
>>   
>>   struct reset_control;
>>   
>> +struct reset_control_array {
>> +	struct reset_control *rst;
>> +	const char *name;
> Drop the name. This interface is only good for a bunch of anonymous
> resets that can all be asserted or deasserted in arbitrary order. If the
> reset lines have to be known individually, this is probably the wrong
> interface.

Sure, will drop 'name' and have the reset_control_array structure
as suggested above.

>
>> +};
>> +
>> +struct reset_control_devres {
>> +	struct reset_control_array *resets;
>> +	int num_resets;
>> +};
>> +
>>   #ifdef CONFIG_RESET_CONTROLLER
>>   
>>   int reset_control_reset(struct reset_control *rstc);
>> @@ -23,6 +33,18 @@ struct reset_control *__devm_reset_control_get(struct device *dev,
>>   int __must_check device_reset(struct device *dev);
>>   int of_reset_control_get_count(struct device_node *node);
>>   
>> +int reset_control_array_assert(int num_rsts,
>> +				struct reset_control_array *resets);
>> +int reset_control_array_deassert(int num_rsts,
>> +				struct reset_control_array *resets);
>> +int devm_reset_control_array_get(struct device *dev, int num_rsts,
>> +				struct reset_control_array *resets,
>> +				bool shared, bool optional);
>> +int of_reset_control_array_get(struct device_node *node, int num_rsts,
>> +				struct reset_control_array *resets,
>> +				bool shared, bool optional);
>> +void reset_control_array_put(int num, struct reset_control_array *resets);
>> +
>>   static inline int device_reset_optional(struct device *dev)
>>   {
>>   	return device_reset(dev);
>> @@ -85,6 +107,39 @@ static inline struct reset_control *__devm_reset_control_get(
>>   	return -ENOTSUPP;
>>   }
>>   
>> +static inline int reset_control_array_assert(int num_rsts,
>> +				struct reset_control_array *resets)
>> +{
>> +	return 0;
>> +}
>> +
>> +static inline int reset_control_array_deassert(int num_rsts,
>> +				struct reset_control_array *resets)
>> +{
>> +	return 0;
>> +}
> Is this missing a stub for reset_control_array_get?

No, that's supposed to be a static function.

>
>> +static inline
>> +int devm_reset_control_array_get(struct device *dev, int num_rsts,
>> +				struct reset_control_array *resets,
>> +				bool shared, bool optional)
>> +{
>> +	return optional ? NULL : ERR_PTR(-ENOTSUPP);
>> +}
>> +
>> +static inline
>> +int of_reset_control_array_get(struct device_node *node, int num_rsts,
>> +				struct reset_control_array *resets,
>> +				bool shared, bool optional)
>> +{
>> +	return optional ? NULL : ERR_PTR(-ENOTSUPP);
>> +}
>> +
>> +static inline
>> +void reset_control_array_put(int num, struct reset_control_array *resets)
>> +{
>> +}
>> +
>>   #endif /* CONFIG_RESET_CONTROLLER */
>>   
>>   /**
>> @@ -374,4 +429,57 @@ static inline struct reset_control *devm_reset_control_get_by_index(
>>   {
>>   	return devm_reset_control_get_exclusive_by_index(dev, index);
>>   }
>> +
>> +/*
>> + * APIs to manage a list of reset controllers
>> + */
>> +static inline
>> +int devm_reset_control_array_get_exclusive(struct device *dev, int num_rsts,
>> +					struct reset_control_array *resets)
>> +{
>> +	return devm_reset_control_array_get(dev, num_rsts,
>> +						resets, false, false);
>> +}
>> +
>> +static inline
>> +int devm_reset_control_array_get_shared(struct device *dev, int num_rsts,
>> +					struct reset_control_array *resets)
>> +{
>> +	return devm_reset_control_array_get(dev, num_rsts,
>> +						resets, true, false);
>> +}
>> +
>> +static inline
>> +int devm_reset_control_array_get_optional_exclusive(struct device *dev,
>> +					int num_rsts,
>> +					struct reset_control_array *resets)
>> +{
>> +	return devm_reset_control_array_get(dev, num_rsts,
>> +						resets, false, true);
>> +}
>> +
>> +static inline
>> +int devm_reset_control_array_get_optional_shared(struct device *dev,
>> +					int num_rsts,
>> +					struct reset_control_array *resets)
>> +{
>> +	return devm_reset_control_array_get(dev, num_rsts,
>> +						resets, true, true);
>> +}
>> +
>> +static inline
>> +int of_reset_control_array_get_exclusive(struct device_node *node,
>> +					int num_rsts,
>> +					struct reset_control_array *resets)
>> +{
>> +	return of_reset_control_array_get(node, num_rsts, resets, false, false);
>> +}
>> +
>> +static inline
>> +int of_reset_control_array_get_shared(struct device_node *node,
>> +					int num_rsts,
>> +					struct reset_control_array *resets)
>> +{
>> +	return of_reset_control_array_get(node, num_rsts, resets, true, false);
>> +}
>>   #endif
> regards
> Philipp
>

Best Regards
Vivek

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

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

* Re: [PATCH v2 2/4] reset: Add APIs to manage array of resets
  2017-04-04 10:39       ` Vivek Gautam
@ 2017-04-04 12:47         ` Philipp Zabel
  2017-04-05  6:50           ` Vivek Gautam
  0 siblings, 1 reply; 26+ messages in thread
From: Philipp Zabel @ 2017-04-04 12:47 UTC (permalink / raw)
  To: Vivek Gautam
  Cc: linux-kernel, linux-tegra, linux-usb, swarren, thierry.reding, balbi

Hi Vivek,

On Tue, 2017-04-04 at 16:09 +0530, Vivek Gautam wrote:
[...]
> > I'd prefer to mirror the gpiod API a little, and to have the number
> > contained in the array structure, similar to struct gpio_descs:
[...]
> Alright, i can update this.
> I took regulator_bulk interface as the reference, but now i see
> gpio_descs has a smaller footprint.

Ok, understood.
I think the smaller footprint API is more convenient for the user.

[...]
> >> + * Returns 0 on success or error number on failure
> >> + */
> >> +static int reset_control_array_get(struct device *dev, int num_rsts,
> >> +				struct reset_control_array *resets,
> >> +				bool shared, bool optional)
> > Can we make this count and return a pointer to the newly created array?
> >
> > static struct reset_controls *
> > reset_control_array_get(struct device *dev, bool shared, bool optional)
> > {
> > 	...
> >
> > That way the consumer doesn't have to care about counting and array
> > allocation.
> 
> Just trying to think again in line with the regulator bulk APIs.
> Can't a user provide a list of reset controls as data and thus
> request reset controls with a "id" and num_resets available.
> 
> e.g.
>     const char * const rst_names[] = {
>       "rst1", "rst2" ...
>     };
>     xyz_data = {
>       .resets = rst_names;
>       .num = ARRAY_SIZE(rst_names);
>     };
> and then the driver making use of reset_control_array APIs
> to request this list of reset controls and assert/deassert them.
> 
> I am assuming this case when one user driver is used across a bunch
> of platforms with different number of resets available.
> We may not want to rely solely on the device tree entries, since
> the resets can be mandatory in few cases and we may want to
> return failure.

The way I understood the array API was as a method of handling a list of
anonymous resets, specified as

	resets = <&reset 1>, <&reset 2>, <&reset 3>;
	// reset-names = "rst1", "rst2", "rst3"; /* don't care */

in the device tree.

Now if you want to handle a partial list of those as an unordered list,
with special consideration for others, that could be added as a separate
API when there is use for it. But I expect that most potential users of
this array API will not have to make such a distinction.

> >> @@ -85,6 +107,39 @@ static inline struct reset_control *__devm_reset_control_get(
> >>   	return -ENOTSUPP;
> >>   }
> >>   
> >> +static inline int reset_control_array_assert(int num_rsts,
> >> +				struct reset_control_array *resets)
> >> +{
> >> +	return 0;
> >> +}
> >> +
> >> +static inline int reset_control_array_deassert(int num_rsts,
> >> +				struct reset_control_array *resets)
> >> +{
> >> +	return 0;
> >> +}
> > Is this missing a stub for reset_control_array_get?
> 
> No, that's supposed to be a static function.

Oh right, it is static. Please ignore.

regards
Philipp

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

* Re: [PATCH v2 2/4] reset: Add APIs to manage array of resets
  2017-04-04 12:47         ` Philipp Zabel
@ 2017-04-05  6:50           ` Vivek Gautam
  0 siblings, 0 replies; 26+ messages in thread
From: Vivek Gautam @ 2017-04-05  6:50 UTC (permalink / raw)
  To: Philipp Zabel
  Cc: linux-kernel, linux-tegra, linux-usb, swarren, thierry.reding, balbi

Hi Philipp,


On 04/04/2017 06:17 PM, Philipp Zabel wrote:
> Hi Vivek,
>
> On Tue, 2017-04-04 at 16:09 +0530, Vivek Gautam wrote:
> [...]
>>> I'd prefer to mirror the gpiod API a little, and to have the number
>>> contained in the array structure, similar to struct gpio_descs:
> [...]
>> Alright, i can update this.
>> I took regulator_bulk interface as the reference, but now i see
>> gpio_descs has a smaller footprint.
> Ok, understood.
> I think the smaller footprint API is more convenient for the user.
>
> [...]
>>>> + * Returns 0 on success or error number on failure
>>>> + */
>>>> +static int reset_control_array_get(struct device *dev, int num_rsts,
>>>> +				struct reset_control_array *resets,
>>>> +				bool shared, bool optional)
>>> Can we make this count and return a pointer to the newly created array?
>>>
>>> static struct reset_controls *
>>> reset_control_array_get(struct device *dev, bool shared, bool optional)
>>> {
>>> 	...
>>>
>>> That way the consumer doesn't have to care about counting and array
>>> allocation.
>> Just trying to think again in line with the regulator bulk APIs.
>> Can't a user provide a list of reset controls as data and thus
>> request reset controls with a "id" and num_resets available.
>>
>> e.g.
>>      const char * const rst_names[] = {
>>        "rst1", "rst2" ...
>>      };
>>      xyz_data = {
>>        .resets = rst_names;
>>        .num = ARRAY_SIZE(rst_names);
>>      };
>> and then the driver making use of reset_control_array APIs
>> to request this list of reset controls and assert/deassert them.
>>
>> I am assuming this case when one user driver is used across a bunch
>> of platforms with different number of resets available.
>> We may not want to rely solely on the device tree entries, since
>> the resets can be mandatory in few cases and we may want to
>> return failure.
> The way I understood the array API was as a method of handling a list of
> anonymous resets, specified as
>
> 	resets = <&reset 1>, <&reset 2>, <&reset 3>;
> 	// reset-names = "rst1", "rst2", "rst3"; /* don't care */
>
> in the device tree.
>
> Now if you want to handle a partial list of those as an unordered list,
> with special consideration for others, that could be added as a separate
> API when there is use for it. But I expect that most potential users of
> this array API will not have to make such a distinction.

Alright, I will modify the array APIs as suggested to handle an
unordered list of resets passed from the device tree.
As you said, we can handle the special cases when the need arise.

>
>>>> @@ -85,6 +107,39 @@ static inline struct reset_control *__devm_reset_control_get(
>>>>    	return -ENOTSUPP;
>>>>    }
>>>>    
>>>> +static inline int reset_control_array_assert(int num_rsts,
>>>> +				struct reset_control_array *resets)
>>>> +{
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +static inline int reset_control_array_deassert(int num_rsts,
>>>> +				struct reset_control_array *resets)
>>>> +{
>>>> +	return 0;
>>>> +}
>>> Is this missing a stub for reset_control_array_get?
>> No, that's supposed to be a static function.
> Oh right, it is static. Please ignore.

Thanks

> regards
> Philipp
>

Best Regards
Vivek

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

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

end of thread, other threads:[~2017-04-05  6:50 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-03 13:41 [PATCH v2 0/4] reset: APIs to manage a list of resets Vivek Gautam
2017-04-03 13:41 ` [PATCH v2 1/4] reset: Add API to count number of reset available with device Vivek Gautam
     [not found]   ` <1491226922-20307-2-git-send-email-vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2017-04-04  4:22     ` kbuild test robot
2017-04-04  4:22       ` kbuild test robot
2017-04-03 13:42 ` [PATCH v2 2/4] reset: Add APIs to manage array of resets Vivek Gautam
     [not found]   ` <1491226922-20307-3-git-send-email-vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2017-04-03 16:36     ` Philipp Zabel
2017-04-03 16:36       ` Philipp Zabel
2017-04-04 10:39       ` Vivek Gautam
2017-04-04 12:47         ` Philipp Zabel
2017-04-05  6:50           ` Vivek Gautam
2017-04-04  4:12     ` kbuild test robot
2017-04-04  4:12       ` kbuild test robot
2017-04-04  4:14     ` kbuild test robot
2017-04-04  4:14       ` kbuild test robot
     [not found]       ` <201704041257.siy13sYK%fengguang.wu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2017-04-04  4:23         ` Vivek Gautam
2017-04-04  4:23           ` Vivek Gautam
     [not found] ` <1491226922-20307-1-git-send-email-vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2017-04-03 13:42   ` [PATCH v2 3/4] usb: dwc3: of-simple: Add support to get resets for the device Vivek Gautam
2017-04-03 13:42     ` Vivek Gautam
     [not found]     ` <1491226922-20307-4-git-send-email-vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2017-04-04  5:20       ` kbuild test robot
2017-04-04  5:20         ` kbuild test robot
2017-04-04  5:20       ` [PATCH] usb: dwc3: of-simple: fix noderef.cocci warnings kbuild test robot
2017-04-04  5:20         ` kbuild test robot
2017-04-04  5:34     ` [PATCH v2 3/4] usb: dwc3: of-simple: Add support to get resets for the device kbuild test robot
2017-04-04  5:34       ` kbuild test robot
2017-04-03 13:42   ` [PATCH v2 4/4] soc/tegra: pmc: Use the new reset APIs to manage reset controllers Vivek Gautam
2017-04-03 13:42     ` Vivek Gautam

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.