[PATCH v2 u-boot 0/2] reset: Add get/assert/deassert/release for bulk of reset signals

[PATCH v2 u-boot 0/2] reset: Add get/assert/deassert/release for bulk of reset signals

[Neil Armstrong]

This patch adds a "bulk" API to the reset API in order to get/deassert/
assert/release a group of reset signals associated with a device.

This bulk API will avoid adding a copy of the same code to manage
a group of reset signals in drivers.

This serie and the "clk bulk" serie is necessary and suggested by Marek Vasut
for the "Add USB Support for Amlogic Meson GXL SoCs" to avoid adding the same
code to manage a bulk of clocks and resets in another driver.
I will push a RFC code to update the other drivers having a similar
code pattern when these patches are merged.

The second patch adds the bulk reset API tests for the sandbox test suite.

Changes since v1 :
 - Add sandbox tests

Neil Armstrong (2):
  reset: Add get/assert/deassert/release for bulk of reset signals
  reset: add sandbox test for bulk API

 arch/sandbox/include/asm/reset.h   |  4 ++
 drivers/reset/reset-uclass.c       | 60 +++++++++++++++++++++++
 drivers/reset/sandbox-reset-test.c | 29 +++++++++++
 drivers/reset/sandbox-reset.c      |  2 +-
 include/reset.h                    | 99 ++++++++++++++++++++++++++++++++++++++
 test/dm/reset.c                    | 33 +++++++++++++
 6 files changed, 226 insertions(+), 1 deletion(-)

-- 
2.7.4

[PATCH v2 u-boot 1/2] reset: Add get/assert/deassert/release for bulk of reset signals

[Neil Armstrong]

This patch adds a "bulk" API to the reset API in order to get/deassert/
assert/release a group of reset signals associated with a device.

This bulk API will avoid adding a copy of the same code to manage
a group of reset signals in drivers.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/reset/reset-uclass.c | 60 +++++++++++++++++++++++++++
 include/reset.h              | 99 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 159 insertions(+)

diff --git a/drivers/reset/reset-uclass.c b/drivers/reset/reset-uclass.c
index 307a297..9a5c9c9 100644
--- a/drivers/reset/reset-uclass.c
+++ b/drivers/reset/reset-uclass.c
@@ -81,6 +81,40 @@ int reset_get_by_index(struct udevice *dev, int index,
 	return 0;
 }
 
+int reset_get_bulk(struct udevice *dev, struct reset_ctl_bulk *bulk)
+{
+	int i, ret, err, count;
+	
+	bulk->count = 0;
+
+	count = dev_count_phandle_with_args(dev, "resets", "#reset-cells");
+	if (!count)
+		return 0;
+
+	bulk->resets = devm_kcalloc(dev, count, sizeof(struct reset_ctl),
+				    GFP_KERNEL);
+	if (!bulk->resets)
+		return -ENOMEM;
+
+	for (i = 0; i < count; i++) {
+		ret = reset_get_by_index(dev, i, &bulk->resets[i]);
+		if (ret < 0)
+			goto bulk_get_err;
+
+		++bulk->count;
+	}
+
+	return 0;
+
+bulk_get_err:
+	err = reset_release_all(bulk->resets, bulk->count);
+	if (err)
+		debug("%s: could release all resets for %p\n",
+		      __func__, dev);
+
+	return ret;
+}
+
 int reset_get_by_name(struct udevice *dev, const char *name,
 		     struct reset_ctl *reset_ctl)
 {
@@ -126,6 +160,19 @@ int reset_assert(struct reset_ctl *reset_ctl)
 	return ops->rst_assert(reset_ctl);
 }
 
+int reset_assert_bulk(struct reset_ctl_bulk *bulk)
+{
+	int i, ret;
+
+	for (i = 0; i < bulk->count; i++) {
+		ret = reset_assert(&bulk->resets[i]);
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
+}
+
 int reset_deassert(struct reset_ctl *reset_ctl)
 {
 	struct reset_ops *ops = reset_dev_ops(reset_ctl->dev);
@@ -135,6 +182,19 @@ int reset_deassert(struct reset_ctl *reset_ctl)
 	return ops->rst_deassert(reset_ctl);
 }
 
+int reset_deassert_bulk(struct reset_ctl_bulk *bulk)
+{
+	int i, ret;
+
+	for (i = 0; i < bulk->count; i++) {
+		ret = reset_deassert(&bulk->resets[i]);
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
+}
+
 int reset_release_all(struct reset_ctl *reset_ctl, int count)
 {
 	int i, ret;
diff --git a/include/reset.h b/include/reset.h
index 7185ade..d38f176 100644
--- a/include/reset.h
+++ b/include/reset.h
@@ -60,6 +60,24 @@ struct reset_ctl {
 	unsigned long id;
 };
 
+/**
+ * struct reset_ctl_bulk - A handle to (allowing control of) a bulk of reset
+ * signals.
+ *
+ * Clients provide storage for the reset control bulk. The content of the
+ * structure is managed solely by the reset API. A reset control bulk struct is
+ * initialized by "get"ing the reset control bulk struct.
+ * The reset control bulk struct is passed to all other bulk reset APIs to apply
+ * the API to all the reset signals in the bulk struct.
+ *
+ * @resets: An array of reset signal handles handles.
+ * @count: The number of reset signal handles in the reset array.
+ */
+struct reset_ctl_bulk {
+	struct reset_ctl *resets;
+	unsigned int count;
+};
+
 #ifdef CONFIG_DM_RESET
 /**
  * reset_get_by_index - Get/request a reset signal by integer index.
@@ -81,6 +99,22 @@ int reset_get_by_index(struct udevice *dev, int index,
 		       struct reset_ctl *reset_ctl);
 
 /**
+ * reset_get_bulk - Get/request all reset signals of a device.
+ *
+ * This looks up and requests all reset signals of the client device; each
+ * device is assumed to have n reset signals associated with it somehow,
+ * and this function finds and requests all of them in a separate structure.
+ * The mapping of client device reset signals indices to provider reset signals
+ * may be via device-tree properties, board-provided mapping tables, or some
+ * other mechanism.
+ *
+ * @dev:	The client device.
+ * @bulk	A pointer to a reset control bulk struct to initialize.
+ * @return 0 if OK, or a negative error code.
+ */
+int reset_get_bulk(struct udevice *dev, struct reset_ctl_bulk *bulk);
+
+/**
  * reset_get_by_name - Get/request a reset signal by name.
  *
  * This looks up and requests a reset signal. The name is relative to the
@@ -132,6 +166,21 @@ int reset_free(struct reset_ctl *reset_ctl);
 int reset_assert(struct reset_ctl *reset_ctl);
 
 /**
+ * reset_assert_bulk - Assert all reset signals in a reset control bulk struct.
+ *
+ * This function will assert the specified reset signals in a reset control
+ * bulk struct, thus resetting the affected HW module(s). Depending on the
+ * reset controller hardware, the reset signals will either stay asserted
+ * until reset_deassert_bulk() is called, or the hardware may autonomously
+ * clear the reset signals itself.
+ *
+ * @bulk:	A reset control bulk struct that was previously successfully
+ *		requested by reset_get_bulk().
+ * @return 0 if OK, or a negative error code.
+ */
+int reset_assert_bulk(struct reset_ctl_bulk *bulk);
+
+/**
  * reset_deassert - Deassert a reset signal.
  *
  * This function will deassert the specified reset signal, thus releasing the
@@ -145,6 +194,20 @@ int reset_assert(struct reset_ctl *reset_ctl);
 int reset_deassert(struct reset_ctl *reset_ctl);
 
 /**
+ * reset_deassert_bulk - Deassert all reset signals in a reset control bulk
+ * struct.
+ *
+ * This function will deassert the specified reset signals in a reset control
+ * bulk struct, thus releasing the affected HW modules() from reset, and
+ * allowing them to continue normal operation.
+ *
+ * @bulk:	A reset control bulk struct that was previously successfully
+ *		requested by reset_get_bulk().
+ * @return 0 if OK, or a negative error code.
+ */
+int reset_deassert_bulk(struct reset_ctl_bulk *bulk);
+
+/**
  * reset_release_all - Assert/Free an array of previously requested resets.
  *
  * For each reset contained in the reset array, this function will check if
@@ -156,6 +219,23 @@ int reset_deassert(struct reset_ctl *reset_ctl);
  * @return 0 if OK, or a negative error code.
  */
 int reset_release_all(struct reset_ctl *reset_ctl, int count);
+
+/**
+ * reset_release_bulk - Assert/Free an array of previously requested reset
+ * signals in a reset control bulk struct.
+ *
+ * For each reset contained in the reset control bulk struct, this function
+ * will check if reset has been previously requested and then will assert
+ * and free it.
+ *
+ * @bulk:	A reset control bulk struct that was previously successfully
+ *		requested by reset_get_bulk().
+ * @return 0 if OK, or a negative error code.
+ */
+static inline int reset_release_bulk(struct reset_ctl_bulk *bulk)
+{
+	return reset_release_all(bulk->resets, bulk->count);
+}
 #else
 static inline int reset_get_by_index(struct udevice *dev, int index,
 				     struct reset_ctl *reset_ctl)
@@ -163,6 +243,11 @@ static inline int reset_get_by_index(struct udevice *dev, int index,
 	return -ENOTSUPP;
 }
 
+static inline int reset_get_bulk(struct udevice *dev, struct clk_bulk *bulk)
+{
+	return -ENOTSUPP;
+}
+
 static inline int reset_get_by_name(struct udevice *dev, const char *name,
 				    struct reset_ctl *reset_ctl)
 {
@@ -179,16 +264,30 @@ static inline int reset_assert(struct reset_ctl *reset_ctl)
 	return 0;
 }
 
+static inline int reset_assert_bulk(struct reset_ctl_bulk *bulk)
+{
+	return 0;
+}
+
 static inline int reset_deassert(struct reset_ctl *reset_ctl)
 {
 	return 0;
 }
 
+static inline int reset_deassert_bulk(struct reset_ctl_bulk *bulk)
+{
+	return 0;
+}
+
 static inline int reset_release_all(struct reset_ctl *reset_ctl, int count)
 {
 	return 0;
 }
 
+static inline int reset_release_bulk(struct clk_bulk *bulk)
+{
+	return 0;
+}
 #endif
 
 #endif
-- 
2.7.4

[PATCH v2 u-boot 1/2] reset: Add get/assert/deassert/release for bulk of reset signals

[Simon Glass]

On 3 April 2018 at 17:40, Neil Armstrong <narmstrong@baylibre.com> wrote:
> This patch adds a "bulk" API to the reset API in order to get/deassert/
> assert/release a group of reset signals associated with a device.
>
> This bulk API will avoid adding a copy of the same code to manage
> a group of reset signals in drivers.
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  drivers/reset/reset-uclass.c | 60 +++++++++++++++++++++++++++
>  include/reset.h              | 99 ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 159 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot, v2, u-boot, 1/2] reset: Add get/assert/deassert/release for bulk of reset signals

[Tom Rini]

On Tue, Apr 03, 2018 at 11:40:50AM +0200, Neil Armstrong wrote:

> This patch adds a "bulk" API to the reset API in order to get/deassert/
> assert/release a group of reset signals associated with a device.
> 
> This bulk API will avoid adding a copy of the same code to manage
> a group of reset signals in drivers.
> 
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-amlogic/attachments/20180411/d0fe1857/attachment.sig>

[U-Boot, v2, u-boot, 1/2] reset: Add get/assert/deassert/release for bulk of reset signals

[Marek Vasut]

On 04/11/2018 04:05 PM, Tom Rini wrote:
> On Tue, Apr 03, 2018 at 11:40:50AM +0200, Neil Armstrong wrote:
> 
>> This patch adds a "bulk" API to the reset API in order to get/deassert/
>> assert/release a group of reset signals associated with a device.
>>
>> This bulk API will avoid adding a copy of the same code to manage
>> a group of reset signals in drivers.
>>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> Applied to u-boot/master, thanks!

Breaks building of designware_i2c.c and also isn't checkpatch clean.

ERROR: trailing whitespace
#90: FILE: drivers/reset/reset-uclass.c:87:
+^I$

total: 1 errors, 0 warnings, 0 checks, 229 lines checked

-- 
Best regards,
Marek Vasut

[U-Boot, v2, u-boot, 1/2] reset: Add get/assert/deassert/release for bulk of reset signals

[Tom Rini]

On Sat, Apr 14, 2018 at 02:11:38AM +0200, Marek Vasut wrote:
> On 04/11/2018 04:05 PM, Tom Rini wrote:
> > On Tue, Apr 03, 2018 at 11:40:50AM +0200, Neil Armstrong wrote:
> > 
> >> This patch adds a "bulk" API to the reset API in order to get/deassert/
> >> assert/release a group of reset signals associated with a device.
> >>
> >> This bulk API will avoid adding a copy of the same code to manage
> >> a group of reset signals in drivers.
> >>
> >> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> >> Reviewed-by: Simon Glass <sjg@chromium.org>
> > 
> > Applied to u-boot/master, thanks!
> 
> Breaks building of designware_i2c.c and also isn't checkpatch clean.

For which platform?

> ERROR: trailing whitespace
> #90: FILE: drivers/reset/reset-uclass.c:87:
> +^I$
> 
> total: 1 errors, 0 warnings, 0 checks, 229 lines checked

Yes, I probably ought to see about getting checkpatch to fixup issues
like that automatically.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-amlogic/attachments/20180414/c9a79a89/attachment.sig>

[U-Boot] [PATCH v2 u-boot 1/2] reset: Add get/assert/deassert/release for bulk of reset signals

[Jean-Jacques Hiblot]

Hi Neil,

while trying to test your patches on dwc3, I ran into a compilation issue.

On 03/04/2018 11:40, Neil Armstrong wrote:
> This patch adds a "bulk" API to the reset API in order to get/deassert/
> assert/release a group of reset signals associated with a device.
>
> This bulk API will avoid adding a copy of the same code to manage
> a group of reset signals in drivers.
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>   drivers/reset/reset-uclass.c | 60 +++++++++++++++++++++++++++
>   include/reset.h              | 99 ++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 159 insertions(+)
>
> diff --git a/drivers/reset/reset-uclass.c b/drivers/reset/reset-uclass.c
> index 307a297..9a5c9c9 100644
> --- a/drivers/reset/reset-uclass.c
> +++ b/drivers/reset/reset-uclass.c
> @@ -81,6 +81,40 @@ int reset_get_by_index(struct udevice *dev, int index,
>   	return 0;
>   }
>   
> +int reset_get_bulk(struct udevice *dev, struct reset_ctl_bulk *bulk)
> +{
> +	int i, ret, err, count;
> +	
> +	bulk->count = 0;
> +
> +	count = dev_count_phandle_with_args(dev, "resets", "#reset-cells");
> +	if (!count)
> +		return 0;
> +
> +	bulk->resets = devm_kcalloc(dev, count, sizeof(struct reset_ctl),
> +				    GFP_KERNEL);
> +	if (!bulk->resets)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < count; i++) {
> +		ret = reset_get_by_index(dev, i, &bulk->resets[i]);
> +		if (ret < 0)
> +			goto bulk_get_err;
> +
> +		++bulk->count;
> +	}
> +
> +	return 0;
> +
> +bulk_get_err:
> +	err = reset_release_all(bulk->resets, bulk->count);
> +	if (err)
> +		debug("%s: could release all resets for %p\n",
> +		      __func__, dev);
> +
> +	return ret;
> +}
> +
>   int reset_get_by_name(struct udevice *dev, const char *name,
>   		     struct reset_ctl *reset_ctl)
>   {
> @@ -126,6 +160,19 @@ int reset_assert(struct reset_ctl *reset_ctl)
>   	return ops->rst_assert(reset_ctl);
>   }
>   
> +int reset_assert_bulk(struct reset_ctl_bulk *bulk)
> +{
> +	int i, ret;
> +
> +	for (i = 0; i < bulk->count; i++) {
> +		ret = reset_assert(&bulk->resets[i]);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
>   int reset_deassert(struct reset_ctl *reset_ctl)
>   {
>   	struct reset_ops *ops = reset_dev_ops(reset_ctl->dev);
> @@ -135,6 +182,19 @@ int reset_deassert(struct reset_ctl *reset_ctl)
>   	return ops->rst_deassert(reset_ctl);
>   }
>   
> +int reset_deassert_bulk(struct reset_ctl_bulk *bulk)
> +{
> +	int i, ret;
> +
> +	for (i = 0; i < bulk->count; i++) {
> +		ret = reset_deassert(&bulk->resets[i]);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
>   int reset_release_all(struct reset_ctl *reset_ctl, int count)
>   {
>   	int i, ret;
> diff --git a/include/reset.h b/include/reset.h
> index 7185ade..d38f176 100644
> --- a/include/reset.h
> +++ b/include/reset.h
> @@ -60,6 +60,24 @@ struct reset_ctl {
>   	unsigned long id;
>   };
>   
> +/**
> + * struct reset_ctl_bulk - A handle to (allowing control of) a bulk of reset
> + * signals.
> + *
> + * Clients provide storage for the reset control bulk. The content of the
> + * structure is managed solely by the reset API. A reset control bulk struct is
> + * initialized by "get"ing the reset control bulk struct.
> + * The reset control bulk struct is passed to all other bulk reset APIs to apply
> + * the API to all the reset signals in the bulk struct.
> + *
> + * @resets: An array of reset signal handles handles.
> + * @count: The number of reset signal handles in the reset array.
> + */
> +struct reset_ctl_bulk {
> +	struct reset_ctl *resets;
> +	unsigned int count;
> +};
> +
>   #ifdef CONFIG_DM_RESET
>   /**
>    * reset_get_by_index - Get/request a reset signal by integer index.
> @@ -81,6 +99,22 @@ int reset_get_by_index(struct udevice *dev, int index,
>   		       struct reset_ctl *reset_ctl);
>   
>   /**
> + * reset_get_bulk - Get/request all reset signals of a device.
> + *
> + * This looks up and requests all reset signals of the client device; each
> + * device is assumed to have n reset signals associated with it somehow,
> + * and this function finds and requests all of them in a separate structure.
> + * The mapping of client device reset signals indices to provider reset signals
> + * may be via device-tree properties, board-provided mapping tables, or some
> + * other mechanism.
> + *
> + * @dev:	The client device.
> + * @bulk	A pointer to a reset control bulk struct to initialize.
> + * @return 0 if OK, or a negative error code.
> + */
> +int reset_get_bulk(struct udevice *dev, struct reset_ctl_bulk *bulk);
> +
> +/**
>    * reset_get_by_name - Get/request a reset signal by name.
>    *
>    * This looks up and requests a reset signal. The name is relative to the
> @@ -132,6 +166,21 @@ int reset_free(struct reset_ctl *reset_ctl);
>   int reset_assert(struct reset_ctl *reset_ctl);
>   
>   /**
> + * reset_assert_bulk - Assert all reset signals in a reset control bulk struct.
> + *
> + * This function will assert the specified reset signals in a reset control
> + * bulk struct, thus resetting the affected HW module(s). Depending on the
> + * reset controller hardware, the reset signals will either stay asserted
> + * until reset_deassert_bulk() is called, or the hardware may autonomously
> + * clear the reset signals itself.
> + *
> + * @bulk:	A reset control bulk struct that was previously successfully
> + *		requested by reset_get_bulk().
> + * @return 0 if OK, or a negative error code.
> + */
> +int reset_assert_bulk(struct reset_ctl_bulk *bulk);
> +
> +/**
>    * reset_deassert - Deassert a reset signal.
>    *
>    * This function will deassert the specified reset signal, thus releasing the
> @@ -145,6 +194,20 @@ int reset_assert(struct reset_ctl *reset_ctl);
>   int reset_deassert(struct reset_ctl *reset_ctl);
>   
>   /**
> + * reset_deassert_bulk - Deassert all reset signals in a reset control bulk
> + * struct.
> + *
> + * This function will deassert the specified reset signals in a reset control
> + * bulk struct, thus releasing the affected HW modules() from reset, and
> + * allowing them to continue normal operation.
> + *
> + * @bulk:	A reset control bulk struct that was previously successfully
> + *		requested by reset_get_bulk().
> + * @return 0 if OK, or a negative error code.
> + */
> +int reset_deassert_bulk(struct reset_ctl_bulk *bulk);
> +
> +/**
>    * reset_release_all - Assert/Free an array of previously requested resets.
>    *
>    * For each reset contained in the reset array, this function will check if
> @@ -156,6 +219,23 @@ int reset_deassert(struct reset_ctl *reset_ctl);
>    * @return 0 if OK, or a negative error code.
>    */
>   int reset_release_all(struct reset_ctl *reset_ctl, int count);
> +
> +/**
> + * reset_release_bulk - Assert/Free an array of previously requested reset
> + * signals in a reset control bulk struct.
> + *
> + * For each reset contained in the reset control bulk struct, this function
> + * will check if reset has been previously requested and then will assert
> + * and free it.
> + *
> + * @bulk:	A reset control bulk struct that was previously successfully
> + *		requested by reset_get_bulk().
> + * @return 0 if OK, or a negative error code.
> + */
> +static inline int reset_release_bulk(struct reset_ctl_bulk *bulk)
> +{
> +	return reset_release_all(bulk->resets, bulk->count);
> +}
>   #else
>   static inline int reset_get_by_index(struct udevice *dev, int index,
>   				     struct reset_ctl *reset_ctl)
> @@ -163,6 +243,11 @@ static inline int reset_get_by_index(struct udevice *dev, int index,
>   	return -ENOTSUPP;
>   }
>   
> +static inline int reset_get_bulk(struct udevice *dev, struct clk_bulk *bulk)
Should be struct reset_ctl_bulk instead of struct clk_bulk
> +{
> +	return -ENOTSUPP;
> +}
> +
>   static inline int reset_get_by_name(struct udevice *dev, const char *name,
>   				    struct reset_ctl *reset_ctl)
>   {
> @@ -179,16 +264,30 @@ static inline int reset_assert(struct reset_ctl *reset_ctl)
>   	return 0;
>   }
>   
> +static inline int reset_assert_bulk(struct reset_ctl_bulk *bulk)
> +{
> +	return 0;
> +}
> +
>   static inline int reset_deassert(struct reset_ctl *reset_ctl)
>   {
>   	return 0;
>   }
>   
> +static inline int reset_deassert_bulk(struct reset_ctl_bulk *bulk)
> +{
> +	return 0;
> +}
> +
>   static inline int reset_release_all(struct reset_ctl *reset_ctl, int count)
>   {
>   	return 0;
>   }
>   
> +static inline int reset_release_bulk(struct clk_bulk *bulk)
ditto.

Jean-Jacques
> +{
> +	return 0;
> +}
>   #endif
>   
>   #endif

[U-Boot] [PATCH v2 u-boot 1/2] reset: Add get/assert/deassert/release for bulk of reset signals

[Neil Armstrong]

On 11/04/2018 17:49, Jean-Jacques Hiblot wrote:
> Hi Neil,
> 
> while trying to test your patches on dwc3, I ran into a compilation issue.
> 
> On 03/04/2018 11:40, Neil Armstrong wrote:
>> This patch adds a "bulk" API to the reset API in order to get/deassert/
>> assert/release a group of reset signals associated with a device.
>>
>> This bulk API will avoid adding a copy of the same code to manage
>> a group of reset signals in drivers.
>>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>> ? drivers/reset/reset-uclass.c | 60 +++++++++++++++++++++++++++
>> ? include/reset.h????????????? | 99 ++++++++++++++++++++++++++++++++++++++++++++
>> ? 2 files changed, 159 insertions(+)
>>
>> diff --git a/drivers/reset/reset-uclass.c b/drivers/reset/reset-uclass.c
>> index 307a297..9a5c9c9 100644
>> --- a/drivers/reset/reset-uclass.c
>> +++ b/drivers/reset/reset-uclass.c
>> @@ -81,6 +81,40 @@ int reset_get_by_index(struct udevice *dev, int index,
>> ????? return 0;
>> ? }
>> ? +int reset_get_bulk(struct udevice *dev, struct reset_ctl_bulk *bulk)
>> +{
>> +??? int i, ret, err, count;
>> +???
>> +??? bulk->count = 0;
>> +
>> +??? count = dev_count_phandle_with_args(dev, "resets", "#reset-cells");
>> +??? if (!count)
>> +??????? return 0;
>> +
>> +??? bulk->resets = devm_kcalloc(dev, count, sizeof(struct reset_ctl),
>> +??????????????????? GFP_KERNEL);
>> +??? if (!bulk->resets)
>> +??????? return -ENOMEM;
>> +
>> +??? for (i = 0; i < count; i++) {
>> +??????? ret = reset_get_by_index(dev, i, &bulk->resets[i]);
>> +??????? if (ret < 0)
>> +??????????? goto bulk_get_err;
>> +
>> +??????? ++bulk->count;
>> +??? }
>> +
>> +??? return 0;
>> +
>> +bulk_get_err:
>> +??? err = reset_release_all(bulk->resets, bulk->count);
>> +??? if (err)
>> +??????? debug("%s: could release all resets for %p\n",
>> +????????????? __func__, dev);
>> +
>> +??? return ret;
>> +}
>> +
>> ? int reset_get_by_name(struct udevice *dev, const char *name,
>> ?????????????? struct reset_ctl *reset_ctl)
>> ? {
>> @@ -126,6 +160,19 @@ int reset_assert(struct reset_ctl *reset_ctl)
>> ????? return ops->rst_assert(reset_ctl);
>> ? }
>> ? +int reset_assert_bulk(struct reset_ctl_bulk *bulk)
>> +{
>> +??? int i, ret;
>> +
>> +??? for (i = 0; i < bulk->count; i++) {
>> +??????? ret = reset_assert(&bulk->resets[i]);
>> +??????? if (ret < 0)
>> +??????????? return ret;
>> +??? }
>> +
>> +??? return 0;
>> +}
>> +
>> ? int reset_deassert(struct reset_ctl *reset_ctl)
>> ? {
>> ????? struct reset_ops *ops = reset_dev_ops(reset_ctl->dev);
>> @@ -135,6 +182,19 @@ int reset_deassert(struct reset_ctl *reset_ctl)
>> ????? return ops->rst_deassert(reset_ctl);
>> ? }
>> ? +int reset_deassert_bulk(struct reset_ctl_bulk *bulk)
>> +{
>> +??? int i, ret;
>> +
>> +??? for (i = 0; i < bulk->count; i++) {
>> +??????? ret = reset_deassert(&bulk->resets[i]);
>> +??????? if (ret < 0)
>> +??????????? return ret;
>> +??? }
>> +
>> +??? return 0;
>> +}
>> +
>> ? int reset_release_all(struct reset_ctl *reset_ctl, int count)
>> ? {
>> ????? int i, ret;
>> diff --git a/include/reset.h b/include/reset.h
>> index 7185ade..d38f176 100644
>> --- a/include/reset.h
>> +++ b/include/reset.h
>> @@ -60,6 +60,24 @@ struct reset_ctl {
>> ????? unsigned long id;
>> ? };
>> ? +/**
>> + * struct reset_ctl_bulk - A handle to (allowing control of) a bulk of reset
>> + * signals.
>> + *
>> + * Clients provide storage for the reset control bulk. The content of the
>> + * structure is managed solely by the reset API. A reset control bulk struct is
>> + * initialized by "get"ing the reset control bulk struct.
>> + * The reset control bulk struct is passed to all other bulk reset APIs to apply
>> + * the API to all the reset signals in the bulk struct.
>> + *
>> + * @resets: An array of reset signal handles handles.
>> + * @count: The number of reset signal handles in the reset array.
>> + */
>> +struct reset_ctl_bulk {
>> +??? struct reset_ctl *resets;
>> +??? unsigned int count;
>> +};
>> +
>> ? #ifdef CONFIG_DM_RESET
>> ? /**
>> ?? * reset_get_by_index - Get/request a reset signal by integer index.
>> @@ -81,6 +99,22 @@ int reset_get_by_index(struct udevice *dev, int index,
>> ???????????????? struct reset_ctl *reset_ctl);
>> ? ? /**
>> + * reset_get_bulk - Get/request all reset signals of a device.
>> + *
>> + * This looks up and requests all reset signals of the client device; each
>> + * device is assumed to have n reset signals associated with it somehow,
>> + * and this function finds and requests all of them in a separate structure.
>> + * The mapping of client device reset signals indices to provider reset signals
>> + * may be via device-tree properties, board-provided mapping tables, or some
>> + * other mechanism.
>> + *
>> + * @dev:??? The client device.
>> + * @bulk??? A pointer to a reset control bulk struct to initialize.
>> + * @return 0 if OK, or a negative error code.
>> + */
>> +int reset_get_bulk(struct udevice *dev, struct reset_ctl_bulk *bulk);
>> +
>> +/**
>> ?? * reset_get_by_name - Get/request a reset signal by name.
>> ?? *
>> ?? * This looks up and requests a reset signal. The name is relative to the
>> @@ -132,6 +166,21 @@ int reset_free(struct reset_ctl *reset_ctl);
>> ? int reset_assert(struct reset_ctl *reset_ctl);
>> ? ? /**
>> + * reset_assert_bulk - Assert all reset signals in a reset control bulk struct.
>> + *
>> + * This function will assert the specified reset signals in a reset control
>> + * bulk struct, thus resetting the affected HW module(s). Depending on the
>> + * reset controller hardware, the reset signals will either stay asserted
>> + * until reset_deassert_bulk() is called, or the hardware may autonomously
>> + * clear the reset signals itself.
>> + *
>> + * @bulk:??? A reset control bulk struct that was previously successfully
>> + *??????? requested by reset_get_bulk().
>> + * @return 0 if OK, or a negative error code.
>> + */
>> +int reset_assert_bulk(struct reset_ctl_bulk *bulk);
>> +
>> +/**
>> ?? * reset_deassert - Deassert a reset signal.
>> ?? *
>> ?? * This function will deassert the specified reset signal, thus releasing the
>> @@ -145,6 +194,20 @@ int reset_assert(struct reset_ctl *reset_ctl);
>> ? int reset_deassert(struct reset_ctl *reset_ctl);
>> ? ? /**
>> + * reset_deassert_bulk - Deassert all reset signals in a reset control bulk
>> + * struct.
>> + *
>> + * This function will deassert the specified reset signals in a reset control
>> + * bulk struct, thus releasing the affected HW modules() from reset, and
>> + * allowing them to continue normal operation.
>> + *
>> + * @bulk:??? A reset control bulk struct that was previously successfully
>> + *??????? requested by reset_get_bulk().
>> + * @return 0 if OK, or a negative error code.
>> + */
>> +int reset_deassert_bulk(struct reset_ctl_bulk *bulk);
>> +
>> +/**
>> ?? * reset_release_all - Assert/Free an array of previously requested resets.
>> ?? *
>> ?? * For each reset contained in the reset array, this function will check if
>> @@ -156,6 +219,23 @@ int reset_deassert(struct reset_ctl *reset_ctl);
>> ?? * @return 0 if OK, or a negative error code.
>> ?? */
>> ? int reset_release_all(struct reset_ctl *reset_ctl, int count);
>> +
>> +/**
>> + * reset_release_bulk - Assert/Free an array of previously requested reset
>> + * signals in a reset control bulk struct.
>> + *
>> + * For each reset contained in the reset control bulk struct, this function
>> + * will check if reset has been previously requested and then will assert
>> + * and free it.
>> + *
>> + * @bulk:??? A reset control bulk struct that was previously successfully
>> + *??????? requested by reset_get_bulk().
>> + * @return 0 if OK, or a negative error code.
>> + */
>> +static inline int reset_release_bulk(struct reset_ctl_bulk *bulk)
>> +{
>> +??? return reset_release_all(bulk->resets, bulk->count);
>> +}
>> ? #else
>> ? static inline int reset_get_by_index(struct udevice *dev, int index,
>> ?????????????????????? struct reset_ctl *reset_ctl)
>> @@ -163,6 +243,11 @@ static inline int reset_get_by_index(struct udevice *dev, int index,
>> ????? return -ENOTSUPP;
>> ? }
>> ? +static inline int reset_get_bulk(struct udevice *dev, struct clk_bulk *bulk)
> Should be struct reset_ctl_bulk instead of struct clk_bulk
>> +{
>> +??? return -ENOTSUPP;
>> +}
>> +
>> ? static inline int reset_get_by_name(struct udevice *dev, const char *name,
>> ????????????????????? struct reset_ctl *reset_ctl)
>> ? {
>> @@ -179,16 +264,30 @@ static inline int reset_assert(struct reset_ctl *reset_ctl)
>> ????? return 0;
>> ? }
>> ? +static inline int reset_assert_bulk(struct reset_ctl_bulk *bulk)
>> +{
>> +??? return 0;
>> +}
>> +
>> ? static inline int reset_deassert(struct reset_ctl *reset_ctl)
>> ? {
>> ????? return 0;
>> ? }
>> ? +static inline int reset_deassert_bulk(struct reset_ctl_bulk *bulk)
>> +{
>> +??? return 0;
>> +}
>> +
>> ? static inline int reset_release_all(struct reset_ctl *reset_ctl, int count)
>> ? {
>> ????? return 0;
>> ? }
>> ? +static inline int reset_release_bulk(struct clk_bulk *bulk)
> ditto.
> 
> Jean-Jacques
>> +{
>> +??? return 0;
>> +}
>> ? #endif
>> ? ? #endif
> 

You are right, I will push a fix ASAP.

Neil

[PATCH v2 u-boot 2/2] reset: add sandbox test for bulk API

[Neil Armstrong]

This patch adds the bulk reset API tests for the sandbox test suite.

Unlike the main test, it also check the "other" reset signal using the bulk API
and checks if the resets are correctly asserted/deasserted.

To allow the bulk API to work, and avoid changing the DT, the number of resets
of the sandbox reset controller has been bumped to 101 for the "other" reset
line to be valid.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 arch/sandbox/include/asm/reset.h   |  4 ++++
 drivers/reset/sandbox-reset-test.c | 29 +++++++++++++++++++++++++++++
 drivers/reset/sandbox-reset.c      |  2 +-
 test/dm/reset.c                    | 33 +++++++++++++++++++++++++++++++++
 4 files changed, 67 insertions(+), 1 deletion(-)

diff --git a/arch/sandbox/include/asm/reset.h b/arch/sandbox/include/asm/reset.h
index 7146aa5..0cd7702 100644
--- a/arch/sandbox/include/asm/reset.h
+++ b/arch/sandbox/include/asm/reset.h
@@ -14,8 +14,12 @@ struct udevice;
 int sandbox_reset_query(struct udevice *dev, unsigned long id);
 
 int sandbox_reset_test_get(struct udevice *dev);
+int sandbox_reset_test_get_bulk(struct udevice *dev);
 int sandbox_reset_test_assert(struct udevice *dev);
+int sandbox_reset_test_assert_bulk(struct udevice *dev);
 int sandbox_reset_test_deassert(struct udevice *dev);
+int sandbox_reset_test_deassert_bulk(struct udevice *dev);
 int sandbox_reset_test_free(struct udevice *dev);
+int sandbox_reset_test_release_bulk(struct udevice *dev);
 
 #endif
diff --git a/drivers/reset/sandbox-reset-test.c b/drivers/reset/sandbox-reset-test.c
index e37d6c9..f0ceaa0 100644
--- a/drivers/reset/sandbox-reset-test.c
+++ b/drivers/reset/sandbox-reset-test.c
@@ -12,6 +12,7 @@
 
 struct sandbox_reset_test {
 	struct reset_ctl ctl;
+	struct reset_ctl_bulk bulk;
 };
 
 int sandbox_reset_test_get(struct udevice *dev)
@@ -21,6 +22,13 @@ int sandbox_reset_test_get(struct udevice *dev)
 	return reset_get_by_name(dev, "test", &sbrt->ctl);
 }
 
+int sandbox_reset_test_get_bulk(struct udevice *dev)
+{
+	struct sandbox_reset_test *sbrt = dev_get_priv(dev);
+
+	return reset_get_bulk(dev, &sbrt->bulk);
+}
+
 int sandbox_reset_test_assert(struct udevice *dev)
 {
 	struct sandbox_reset_test *sbrt = dev_get_priv(dev);
@@ -28,6 +36,13 @@ int sandbox_reset_test_assert(struct udevice *dev)
 	return reset_assert(&sbrt->ctl);
 }
 
+int sandbox_reset_test_assert_bulk(struct udevice *dev)
+{
+	struct sandbox_reset_test *sbrt = dev_get_priv(dev);
+
+	return reset_assert_bulk(&sbrt->bulk);
+}
+
 int sandbox_reset_test_deassert(struct udevice *dev)
 {
 	struct sandbox_reset_test *sbrt = dev_get_priv(dev);
@@ -35,6 +50,13 @@ int sandbox_reset_test_deassert(struct udevice *dev)
 	return reset_deassert(&sbrt->ctl);
 }
 
+int sandbox_reset_test_deassert_bulk(struct udevice *dev)
+{
+	struct sandbox_reset_test *sbrt = dev_get_priv(dev);
+
+	return reset_deassert_bulk(&sbrt->bulk);
+}
+
 int sandbox_reset_test_free(struct udevice *dev)
 {
 	struct sandbox_reset_test *sbrt = dev_get_priv(dev);
@@ -42,6 +64,13 @@ int sandbox_reset_test_free(struct udevice *dev)
 	return reset_free(&sbrt->ctl);
 }
 
+int sandbox_reset_test_release_bulk(struct udevice *dev)
+{
+	struct sandbox_reset_test *sbrt = dev_get_priv(dev);
+
+	return reset_release_bulk(&sbrt->bulk);
+}
+
 static const struct udevice_id sandbox_reset_test_ids[] = {
 	{ .compatible = "sandbox,reset-ctl-test" },
 	{ }
diff --git a/drivers/reset/sandbox-reset.c b/drivers/reset/sandbox-reset.c
index 4258af5..c310749 100644
--- a/drivers/reset/sandbox-reset.c
+++ b/drivers/reset/sandbox-reset.c
@@ -10,7 +10,7 @@
 #include <asm/io.h>
 #include <asm/reset.h>
 
-#define SANDBOX_RESET_SIGNALS 3
+#define SANDBOX_RESET_SIGNALS 101
 
 struct sandbox_reset_signal {
 	bool asserted;
diff --git a/test/dm/reset.c b/test/dm/reset.c
index 0ae8031..8dc0023 100644
--- a/test/dm/reset.c
+++ b/test/dm/reset.c
@@ -13,6 +13,9 @@
 /* This must match the specifier for mbox-names="test" in the DT node */
 #define TEST_RESET_ID 2
 
+/* This is the other reset phandle specifier handled by bulk */
+#define OTHER_RESET_ID 2
+
 static int dm_test_reset(struct unit_test_state *uts)
 {
 	struct udevice *dev_reset;
@@ -37,3 +40,33 @@ static int dm_test_reset(struct unit_test_state *uts)
 	return 0;
 }
 DM_TEST(dm_test_reset, DM_TESTF_SCAN_FDT);
+
+static int dm_test_reset_bulk(struct unit_test_state *uts)
+{
+	struct udevice *dev_reset;
+	struct udevice *dev_test;
+
+	ut_assertok(uclass_get_device_by_name(UCLASS_RESET, "reset-ctl",
+					      &dev_reset));
+	ut_asserteq(0, sandbox_reset_query(dev_reset, TEST_RESET_ID));
+	ut_asserteq(0, sandbox_reset_query(dev_reset, OTHER_RESET_ID));
+
+	ut_assertok(uclass_get_device_by_name(UCLASS_MISC, "reset-ctl-test",
+					      &dev_test));
+	ut_assertok(sandbox_reset_test_get_bulk(dev_test));
+
+	ut_assertok(sandbox_reset_test_assert_bulk(dev_test));
+	ut_asserteq(1, sandbox_reset_query(dev_reset, TEST_RESET_ID));
+	ut_asserteq(1, sandbox_reset_query(dev_reset, OTHER_RESET_ID));
+
+	ut_assertok(sandbox_reset_test_deassert_bulk(dev_test));
+	ut_asserteq(0, sandbox_reset_query(dev_reset, TEST_RESET_ID));
+	ut_asserteq(0, sandbox_reset_query(dev_reset, OTHER_RESET_ID));
+
+	ut_assertok(sandbox_reset_test_release_bulk(dev_test));
+	ut_asserteq(1, sandbox_reset_query(dev_reset, TEST_RESET_ID));
+	ut_asserteq(1, sandbox_reset_query(dev_reset, OTHER_RESET_ID));
+
+	return 0;
+}
+DM_TEST(dm_test_reset_bulk, DM_TESTF_SCAN_FDT);
-- 
2.7.4

[PATCH v2 u-boot 2/2] reset: add sandbox test for bulk API

[Simon Glass]

On 3 April 2018 at 17:40, Neil Armstrong <narmstrong@baylibre.com> wrote:
> This patch adds the bulk reset API tests for the sandbox test suite.
>
> Unlike the main test, it also check the "other" reset signal using the bulk API
> and checks if the resets are correctly asserted/deasserted.
>
> To allow the bulk API to work, and avoid changing the DT, the number of resets
> of the sandbox reset controller has been bumped to 101 for the "other" reset
> line to be valid.

Does it need to be 101, or would, say, 5 be enough?

>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  arch/sandbox/include/asm/reset.h   |  4 ++++
>  drivers/reset/sandbox-reset-test.c | 29 +++++++++++++++++++++++++++++
>  drivers/reset/sandbox-reset.c      |  2 +-
>  test/dm/reset.c                    | 33 +++++++++++++++++++++++++++++++++
>  4 files changed, 67 insertions(+), 1 deletion(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[PATCH v2 u-boot 2/2] reset: add sandbox test for bulk API

[Neil Armstrong]

On 03/04/2018 19:53, Simon Glass wrote:
> On 3 April 2018 at 17:40, Neil Armstrong <narmstrong@baylibre.com> wrote:
>> This patch adds the bulk reset API tests for the sandbox test suite.
>>
>> Unlike the main test, it also check the "other" reset signal using the bulk API
>> and checks if the resets are correctly asserted/deasserted.
>>
>> To allow the bulk API to work, and avoid changing the DT, the number of resets
>> of the sandbox reset controller has been bumped to 101 for the "other" reset
>> line to be valid.
> 
> Does it need to be 101, or would, say, 5 be enough?

It could stay at 3, but a dts change should be needed since it declares :

resets = <&resetc 100>, <&resetc 2>;
reset-names = "other", "test";

> 
>>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>>  arch/sandbox/include/asm/reset.h   |  4 ++++
>>  drivers/reset/sandbox-reset-test.c | 29 +++++++++++++++++++++++++++++
>>  drivers/reset/sandbox-reset.c      |  2 +-
>>  test/dm/reset.c                    | 33 +++++++++++++++++++++++++++++++++
>>  4 files changed, 67 insertions(+), 1 deletion(-)
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 

[PATCH v2 u-boot 2/2] reset: add sandbox test for bulk API

[Simon Glass]

Hi Neil,

On 4 April 2018 at 04:45, Neil Armstrong <narmstrong@baylibre.com> wrote:
> On 03/04/2018 19:53, Simon Glass wrote:
>> On 3 April 2018 at 17:40, Neil Armstrong <narmstrong@baylibre.com> wrote:
>>> This patch adds the bulk reset API tests for the sandbox test suite.
>>>
>>> Unlike the main test, it also check the "other" reset signal using the bulk API
>>> and checks if the resets are correctly asserted/deasserted.
>>>
>>> To allow the bulk API to work, and avoid changing the DT, the number of resets
>>> of the sandbox reset controller has been bumped to 101 for the "other" reset
>>> line to be valid.
>>
>> Does it need to be 101, or would, say, 5 be enough?
>
> It could stay at 3, but a dts change should be needed since it declares :
>
> resets = <&resetc 100>, <&resetc 2>;
> reset-names = "other", "test";

Well that is fine if you want to change test.dts - you just need to
make sure you keep the intent.

But you've explained the reason, and it's fine to keep this patch as
is, if you like.

Regards,
Simon

[U-Boot,v2,u-boot,2/2] reset: add sandbox test for bulk API

[Tom Rini]

On Tue, Apr 03, 2018 at 11:40:51AM +0200, Neil Armstrong wrote:

> This patch adds the bulk reset API tests for the sandbox test suite.
> 
> Unlike the main test, it also check the "other" reset signal using the bulk API
> and checks if the resets are correctly asserted/deasserted.
> 
> To allow the bulk API to work, and avoid changing the DT, the number of resets
> of the sandbox reset controller has been bumped to 101 for the "other" reset
> line to be valid.
> 
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-amlogic/attachments/20180411/38185354/attachment.sig>