diff mbox

[v5,3/6] reset: hide reset control arrays behind struct reset_control

Message ID 20170601165203.15315-4-p.zabel@pengutronix.de (mailing list archive)
State Not Applicable, archived
Delegated to: Andy Gross
Headers show

Commit Message

Philipp Zabel June 1, 2017, 4:52 p.m. UTC
Reset controls already may control multiple reset lines with a single
hardware bit. So from the user perspective, reset control arrays are not
at all different from single reset controls.
Therefore, hide reset control arrays behind struct reset_control to
avoid having to introduce new API functions for array (de)assert/reset.

Cc: Vivek Gautam <vivek.gautam@codeaurora.org>
Cc: Jon Hunter <jonathanh@nvidia.com>
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/reset/core.c  | 225 ++++++++++++++++++++++++++------------------------
 include/linux/reset.h |  44 +++-------
 2 files changed, 128 insertions(+), 141 deletions(-)

Comments

kernel test robot June 1, 2017, 11:09 p.m. UTC | #1
Hi Philipp,

[auto build test ERROR on pza/reset/next]
[also build test ERROR on v4.12-rc3 next-20170601]
[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/Philipp-Zabel/reset-APIs-to-manage-a-list-of-resets/20170602-062536
base:   git://git.pengutronix.de/git/pza/linux reset/next
config: i386-randconfig-x074-06010927 (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=i386 

All errors (new ones prefixed by >>):

   In file included from drivers/usb/dwc2/platform.c:48:0:
   include/linux/reset.h: In function 'devm_reset_control_array_get_exclusive':
>> include/linux/reset.h:406:9: error: return from incompatible pointer type [-Werror=incompatible-pointer-types]
     return devm_reset_control_array_get(dev, false, false);
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/reset.h: In function 'devm_reset_control_array_get_shared':
   include/linux/reset.h:412:9: error: return from incompatible pointer type [-Werror=incompatible-pointer-types]
     return devm_reset_control_array_get(dev, true, false);
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/reset.h: In function 'devm_reset_control_array_get_optional_exclusive':
   include/linux/reset.h:418:9: error: return from incompatible pointer type [-Werror=incompatible-pointer-types]
     return devm_reset_control_array_get(dev, false, true);
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/reset.h: In function 'devm_reset_control_array_get_optional_shared':
   include/linux/reset.h:424:9: error: return from incompatible pointer type [-Werror=incompatible-pointer-types]
     return devm_reset_control_array_get(dev, true, true);
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/reset.h: In function 'of_reset_control_array_get_exclusive':
   include/linux/reset.h:430:9: error: return from incompatible pointer type [-Werror=incompatible-pointer-types]
     return of_reset_control_array_get(node, false, false);
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/reset.h: In function 'of_reset_control_array_get_shared':
   include/linux/reset.h:436:9: error: return from incompatible pointer type [-Werror=incompatible-pointer-types]
     return of_reset_control_array_get(node, true, false);
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/reset.h: In function 'of_reset_control_array_get_optional_exclusive':
   include/linux/reset.h:442:9: error: return from incompatible pointer type [-Werror=incompatible-pointer-types]
     return of_reset_control_array_get(node, false, true);
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/reset.h: In function 'of_reset_control_array_get_optional_shared':
   include/linux/reset.h:448:9: error: return from incompatible pointer type [-Werror=incompatible-pointer-types]
     return of_reset_control_array_get(node, true, true);
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors

vim +406 include/linux/reset.h

1b1b64c5 Vivek Gautam  2017-06-01  400  /*
1b1b64c5 Vivek Gautam  2017-06-01  401   * APIs to manage a list of reset controllers
1b1b64c5 Vivek Gautam  2017-06-01  402   */
1abcb861 Philipp Zabel 2017-06-01  403  static inline struct reset_control *
1b1b64c5 Vivek Gautam  2017-06-01  404  devm_reset_control_array_get_exclusive(struct device *dev)
1b1b64c5 Vivek Gautam  2017-06-01  405  {
1b1b64c5 Vivek Gautam  2017-06-01 @406  	return devm_reset_control_array_get(dev, false, false);
1b1b64c5 Vivek Gautam  2017-06-01  407  }
1b1b64c5 Vivek Gautam  2017-06-01  408  
1abcb861 Philipp Zabel 2017-06-01  409  static inline struct reset_control *

:::::: The code at line 406 was first introduced by commit
:::::: 1b1b64c55c5f63e050938df44e1ccee33cfeee94 reset: Add APIs to manage array of resets

:::::: 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
kernel test robot June 2, 2017, 12:12 a.m. UTC | #2
Hi Philipp,

[auto build test WARNING on pza/reset/next]
[also build test WARNING on v4.12-rc3 next-20170601]
[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/Philipp-Zabel/reset-APIs-to-manage-a-list-of-resets/20170602-062536
base:   git://git.pengutronix.de/git/pza/linux reset/next
config: x86_64-randconfig-i0-201722 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   In file included from drivers/i2c/busses/i2c-designware-platdrv.c:41:0:
   include/linux/reset.h: In function 'devm_reset_control_array_get_exclusive':
>> include/linux/reset.h:406:2: warning: return from incompatible pointer type
     return devm_reset_control_array_get(dev, false, false);
     ^
   include/linux/reset.h: In function 'devm_reset_control_array_get_shared':
   include/linux/reset.h:412:2: warning: return from incompatible pointer type
     return devm_reset_control_array_get(dev, true, false);
     ^
   include/linux/reset.h: In function 'devm_reset_control_array_get_optional_exclusive':
   include/linux/reset.h:418:2: warning: return from incompatible pointer type
     return devm_reset_control_array_get(dev, false, true);
     ^
   include/linux/reset.h: In function 'devm_reset_control_array_get_optional_shared':
   include/linux/reset.h:424:2: warning: return from incompatible pointer type
     return devm_reset_control_array_get(dev, true, true);
     ^
   include/linux/reset.h: In function 'of_reset_control_array_get_exclusive':
   include/linux/reset.h:430:2: warning: return from incompatible pointer type
     return of_reset_control_array_get(node, false, false);
     ^
   include/linux/reset.h: In function 'of_reset_control_array_get_shared':
   include/linux/reset.h:436:2: warning: return from incompatible pointer type
     return of_reset_control_array_get(node, true, false);
     ^
   include/linux/reset.h: In function 'of_reset_control_array_get_optional_exclusive':
   include/linux/reset.h:442:2: warning: return from incompatible pointer type
     return of_reset_control_array_get(node, false, true);
     ^
   include/linux/reset.h: In function 'of_reset_control_array_get_optional_shared':
   include/linux/reset.h:448:2: warning: return from incompatible pointer type
     return of_reset_control_array_get(node, true, true);
     ^

vim +406 include/linux/reset.h

a53e35db Lee Jones     2016-06-06  390  	return devm_reset_control_get_optional_exclusive(dev, id);
a53e35db Lee Jones     2016-06-06  391  
a53e35db Lee Jones     2016-06-06  392  }
a53e35db Lee Jones     2016-06-06  393  
a53e35db Lee Jones     2016-06-06  394  static inline struct reset_control *devm_reset_control_get_by_index(
a53e35db Lee Jones     2016-06-06  395  				struct device *dev, int index)
a53e35db Lee Jones     2016-06-06  396  {
a53e35db Lee Jones     2016-06-06  397  	return devm_reset_control_get_exclusive_by_index(dev, index);
a53e35db Lee Jones     2016-06-06  398  }
1b1b64c5 Vivek Gautam  2017-06-01  399  
1b1b64c5 Vivek Gautam  2017-06-01  400  /*
1b1b64c5 Vivek Gautam  2017-06-01  401   * APIs to manage a list of reset controllers
1b1b64c5 Vivek Gautam  2017-06-01  402   */
1abcb861 Philipp Zabel 2017-06-01  403  static inline struct reset_control *
1b1b64c5 Vivek Gautam  2017-06-01  404  devm_reset_control_array_get_exclusive(struct device *dev)
1b1b64c5 Vivek Gautam  2017-06-01  405  {
1b1b64c5 Vivek Gautam  2017-06-01 @406  	return devm_reset_control_array_get(dev, false, false);
1b1b64c5 Vivek Gautam  2017-06-01  407  }
1b1b64c5 Vivek Gautam  2017-06-01  408  
1abcb861 Philipp Zabel 2017-06-01  409  static inline struct reset_control *
1b1b64c5 Vivek Gautam  2017-06-01  410  devm_reset_control_array_get_shared(struct device *dev)
1b1b64c5 Vivek Gautam  2017-06-01  411  {
1b1b64c5 Vivek Gautam  2017-06-01  412  	return devm_reset_control_array_get(dev, true, false);
1b1b64c5 Vivek Gautam  2017-06-01  413  }
1b1b64c5 Vivek Gautam  2017-06-01  414  

:::::: The code at line 406 was first introduced by commit
:::::: 1b1b64c55c5f63e050938df44e1ccee33cfeee94 reset: Add APIs to manage array of resets

:::::: 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
Vivek Gautam June 13, 2017, 6:46 a.m. UTC | #3
Hi Philipp,


On 06/01/2017 10:22 PM, Philipp Zabel wrote:
> Reset controls already may control multiple reset lines with a single
> hardware bit. So from the user perspective, reset control arrays are not
> at all different from single reset controls.
> Therefore, hide reset control arrays behind struct reset_control to
> avoid having to introduce new API functions for array (de)assert/reset.
>
> Cc: Vivek Gautam <vivek.gautam@codeaurora.org>
> Cc: Jon Hunter <jonathanh@nvidia.com>
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
>   drivers/reset/core.c  | 225 ++++++++++++++++++++++++++------------------------
>   include/linux/reset.h |  44 +++-------
>   2 files changed, 128 insertions(+), 141 deletions(-)
>
> diff --git a/drivers/reset/core.c b/drivers/reset/core.c
> index 1747000757211..c8fb4426b218a 100644
> --- a/drivers/reset/core.c
> +++ b/drivers/reset/core.c
> @@ -43,11 +43,24 @@ struct reset_control {
>   	unsigned int id;
>   	struct kref refcnt;
>   	bool shared;
> +	bool array;
>   	atomic_t deassert_count;
>   	atomic_t triggered_count;
>   };
>   
>   /**
> + * struct reset_control_array - an array of reset controls
> + * @base: reset control for compatibility with reset control API functions
> + * @num_rstcs: number of reset controls
> + * @rstc: array of reset controls
> + */
> +struct reset_control_array {
> +	struct reset_control base;
> +	unsigned int num_rstcs;
> +	struct reset_control *rstc[];
> +};
> +
> +/**
>    * of_reset_simple_xlate - translate reset_spec to the reset line number
>    * @rcdev: a pointer to the reset controller device
>    * @reset_spec: reset line specifier as found in the device tree
> @@ -135,6 +148,65 @@ int devm_reset_controller_register(struct device *dev,
>   }
>   EXPORT_SYMBOL_GPL(devm_reset_controller_register);
>   
> +static inline struct reset_control_array *
> +rstc_to_array(struct reset_control *rstc) {
> +	return container_of(rstc, struct reset_control_array, base);
> +}
> +
> +static int reset_control_array_reset(struct reset_control_array *resets)
> +{
> +	int ret, i;
> +
> +	for (i = 0; i < resets->num_rstcs; i++) {
> +		ret = reset_control_reset(resets->rstc[i]);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int reset_control_array_assert(struct reset_control_array *resets)
> +{
> +	int ret, i;
> +
> +	for (i = 0; i < resets->num_rstcs; i++) {
> +		ret = reset_control_assert(resets->rstc[i]);
> +		if (ret)
> +			goto err;
> +	}
> +
> +	return 0;
> +
> +err:
> +	while (i--)
> +		reset_control_deassert(resets->rstc[i]);
> +	return ret;
> +}
> +
> +static int reset_control_array_deassert(struct reset_control_array *resets)
> +{
> +	int ret, i;
> +
> +	for (i = 0; i < resets->num_rstcs; i++) {
> +		ret = reset_control_deassert(resets->rstc[i]);
> +		if (ret)
> +			goto err;
> +	}
> +
> +	return 0;
> +
> +err:
> +	while (i--)
> +		reset_control_assert(resets->rstc[i]);
> +	return ret;
> +}
> +
> +static inline bool reset_control_is_array(struct reset_control *rstc)
> +{
> +	return rstc->array;
> +}
> +
>   /**
>    * reset_control_reset - reset the controlled device
>    * @rstc: reset controller
> @@ -158,6 +230,9 @@ int reset_control_reset(struct reset_control *rstc)
>   	if (WARN_ON(IS_ERR(rstc)))
>   		return -EINVAL;
>   
> +	if (reset_control_is_array(rstc))
> +		return reset_control_array_reset(rstc_to_array(rstc));
> +
>   	if (!rstc->rcdev->ops->reset)
>   		return -ENOTSUPP;
>   
> @@ -202,6 +277,9 @@ int reset_control_assert(struct reset_control *rstc)
>   	if (WARN_ON(IS_ERR(rstc)))
>   		return -EINVAL;
>   
> +	if (reset_control_is_array(rstc))
> +		return reset_control_array_assert(rstc_to_array(rstc));
> +
>   	if (!rstc->rcdev->ops->assert)
>   		return -ENOTSUPP;
>   
> @@ -240,6 +318,9 @@ int reset_control_deassert(struct reset_control *rstc)
>   	if (WARN_ON(IS_ERR(rstc)))
>   		return -EINVAL;
>   
> +	if (reset_control_is_array(rstc))
> +		return reset_control_array_deassert(rstc_to_array(rstc));
> +
>   	if (!rstc->rcdev->ops->deassert)
>   		return -ENOTSUPP;
>   
> @@ -266,7 +347,7 @@ int reset_control_status(struct reset_control *rstc)
>   	if (!rstc)
>   		return 0;
>   
> -	if (WARN_ON(IS_ERR(rstc)))
> +	if (WARN_ON(IS_ERR(rstc)) || reset_control_is_array(rstc))
>   		return -EINVAL;
>   
>   	if (rstc->rcdev->ops->status)
> @@ -404,6 +485,16 @@ struct reset_control *__reset_control_get(struct device *dev, const char *id,
>   }
>   EXPORT_SYMBOL_GPL(__reset_control_get);
>   
> +static void reset_control_array_put(struct reset_control_array *resets)
> +{
> +	int i;
> +
> +	mutex_lock(&reset_list_mutex);
> +	for (i = 0; i < resets->num_rstcs; i++)
> +		__reset_control_put_internal(resets->rstc[i]);
> +	mutex_unlock(&reset_list_mutex);
> +}
> +
>   /**
>    * reset_control_put - free the reset controller
>    * @rstc: reset controller
> @@ -413,6 +504,11 @@ void reset_control_put(struct reset_control *rstc)
>   	if (IS_ERR_OR_NULL(rstc))
>   		return;
>   
> +	if (reset_control_is_array(rstc)) {
> +		reset_control_array_put(rstc_to_array(rstc));
> +		return;
> +	}
> +
>   	mutex_lock(&reset_list_mutex);
>   	__reset_control_put_internal(rstc);
>   	mutex_unlock(&reset_list_mutex);
> @@ -499,81 +595,6 @@ static int of_reset_control_get_count(struct device_node *node)
>   }
>   
>   /**
> - * reset_control_array_assert: assert a list of resets
> - *
> - * @resets: reset control array holding info about the list of resets
> - *
> - * This API doesn't guarantee that the reset lines controlled by
> - * the reset array are asserted in any particular order.
> - *
> - * Returns 0 on success or error number on failure.
> - */
> -int reset_control_array_assert(struct reset_control_array *resets)
> -{
> -	int ret, i;
> -
> -	if (!resets)
> -		return 0;
> -
> -	if (IS_ERR(resets))
> -		return -EINVAL;
> -
> -	for (i = 0; i < resets->num_rstcs; i++) {
> -		ret = reset_control_assert(resets->rstc[i]);
> -		if (ret)
> -			goto err;
> -	}
> -
> -	return 0;
> -
> -err:
> -	while (i--)
> -		reset_control_deassert(resets->rstc[i]);
> -	return ret;
> -}
> -EXPORT_SYMBOL_GPL(reset_control_array_assert);
> -
> -/**
> - * reset_control_array_deassert: deassert a list of resets
> - *
> - * @resets: reset control array holding info about the list of resets
> - *
> - * This API doesn't guarantee that the reset lines controlled by
> - * the reset array are deasserted in any particular order.
> - *
> - * Returns 0 on success or error number on failure.
> - */
> -int reset_control_array_deassert(struct reset_control_array *resets)
> -{
> -	int ret, i;
> -
> -	if (!resets)
> -		return 0;
> -
> -	if (IS_ERR(resets))
> -		return -EINVAL;
> -
> -	for (i = 0; i < resets->num_rstcs; i++) {
> -		ret = reset_control_deassert(resets->rstc[i]);
> -		if (ret)
> -			goto err;
> -	}
> -
> -	return 0;
> -
> -err:
> -	while (i--)
> -		reset_control_assert(resets->rstc[i]);
> -	return ret;
> -}
> -EXPORT_SYMBOL_GPL(reset_control_array_deassert);
> -
> -static void devm_reset_control_array_release(struct device *dev, void *res)
> -{
> -	reset_control_array_put(*(struct reset_control_array **)res);
> -}
> -
> -/**
>    * of_reset_control_array_get - Get a list of reset controls using
>    *				device node.
>    *
> @@ -584,13 +605,12 @@ static void devm_reset_control_array_release(struct device *dev, void *res)
>    * Returns pointer to allocated reset_control_array on success or
>    * error on failure
>    */
> -struct reset_control_array *
> +struct reset_control *
>   of_reset_control_array_get(struct device_node *np, bool shared, bool optional)
>   {
>   	struct reset_control_array *resets;
>   	struct reset_control *rstc;
>   	int num, i;
> -	void *err;
>   
>   	num = of_reset_control_get_count(np);
>   	if (num < 0)
> @@ -603,23 +623,24 @@ of_reset_control_array_get(struct device_node *np, bool shared, bool optional)
>   
>   	for (i = 0; i < num; i++) {
>   		rstc = __of_reset_control_get(np, NULL, i, shared, optional);
> -		if (IS_ERR(rstc)) {
> -			err = ERR_CAST(rstc);
> +		if (IS_ERR(rstc))
>   			goto err_rst;
> -		}
>   		resets->rstc[i] = rstc;
>   	}
>   	resets->num_rstcs = num;
> +	resets->base.array = true;
>   
> -	return resets;
> +	return &resets->base;
>   
>   err_rst:
> +	mutex_lock(&reset_list_mutex);
>   	while (--i >= 0)
> -		reset_control_put(resets->rstc[i]);
> +		__reset_control_put_internal(resets->rstc[i]);
> +	mutex_unlock(&reset_list_mutex);
>   
>   	kfree(resets);
>   
> -	return err;
> +	return rstc;
>   }
>   EXPORT_SYMBOL_GPL(of_reset_control_array_get);
>   
> @@ -637,40 +658,26 @@ EXPORT_SYMBOL_GPL(of_reset_control_array_get);
>    * Returns pointer to allocated reset_control_array on success or
>    * error on failure
>    */
> -struct reset_control_array *
> +struct reset_control *
>   devm_reset_control_array_get(struct device *dev, bool shared, bool optional)
>   {
> -	struct reset_control_array **devres;
> -	struct reset_control_array *resets;
> +	struct reset_control **devres;
> +	struct reset_control *rstc;
>   
> -	devres = devres_alloc(devm_reset_control_array_release,
> -			      sizeof(*devres), GFP_KERNEL);
> +	devres = devres_alloc(devm_reset_control_release, sizeof(*devres),
> +			      GFP_KERNEL);
>   	if (!devres)
>   		return ERR_PTR(-ENOMEM);
>   
> -	resets = of_reset_control_array_get(dev->of_node, shared, optional);
> -	if (IS_ERR(resets)) {
> -		devres_free(resets);
> -		return resets;
> +	rstc = of_reset_control_array_get(dev->of_node, shared, optional);
> +	if (IS_ERR(rstc)) {
> +		devres_free(devres);
> +		return rstc;
>   	}
>   
> -	*devres = resets;
> +	*devres = rstc;
>   	devres_add(dev, devres);
>   
> -	return resets;
> +	return rstc;
>   }
>   EXPORT_SYMBOL_GPL(devm_reset_control_array_get);
> -
> -void reset_control_array_put(struct reset_control_array *resets)
> -{
> -	int i;
> -
> -	if (IS_ERR_OR_NULL(resets))
> -		return;
> -
> -	for (i = 0; i < resets->num_rstcs; i++)
> -		reset_control_put(resets->rstc[i]);
> -
> -	kfree(resets);
> -}
> -EXPORT_SYMBOL_GPL(reset_control_array_put);
> diff --git a/include/linux/reset.h b/include/linux/reset.h
> index df75fe50f765d..0f1be13e66e46 100644
> --- a/include/linux/reset.h
> +++ b/include/linux/reset.h
> @@ -5,11 +5,6 @@
>   
>   struct reset_control;
>   
> -struct reset_control_array {
> -	unsigned int num_rstcs;
> -	struct reset_control *rstc[];
> -};
> -
>   #ifdef CONFIG_RESET_CONTROLLER
>   
>   int reset_control_reset(struct reset_control *rstc);
> @@ -30,13 +25,10 @@ struct reset_control *__devm_reset_control_get(struct device *dev,
>   
>   int __must_check device_reset(struct device *dev);
>   
> -int reset_control_array_assert(struct reset_control_array *resets);
> -int reset_control_array_deassert(struct reset_control_array *resets);
> -struct reset_control_array *devm_reset_control_array_get(struct device *dev,
> -						bool shared, bool optional);
> -struct reset_control_array *of_reset_control_array_get(struct device_node *np,
> -						bool shared, bool optional);
> -void reset_control_array_put(struct reset_control_array *resets);
> +struct reset_control *devm_reset_control_array_get(struct device *dev,
> +						   bool shared, bool optional);
> +struct reset_control *of_reset_control_array_get(struct device_node *np,
> +						 bool shared, bool optional);
>   
>   static inline int device_reset_optional(struct device *dev)
>   {
> @@ -102,18 +94,6 @@ static inline struct reset_control *__devm_reset_control_get(
>   	return optional ? NULL : ERR_PTR(-ENOTSUPP);
>   }
>   
> -static inline
> -int reset_control_array_assert(struct reset_control_array *resets)
> -{
> -	return 0;
> -}
> -
> -static inline
> -int reset_control_array_deassert(struct reset_control_array *resets)
> -{
> -	return 0;
> -}
> -
>   static inline struct reset_control_array *

This has to return just 'struct reset_control *'.
That should resolve kbuild errors.

Rest looks good to me.
Reviewed-by: Vivek Gautam <vivek.gautam@codeaurora.org>

BRs
Vivek

>   devm_reset_control_array_get(struct device *dev, bool shared, bool optional)
>   {
> @@ -420,49 +400,49 @@ static inline struct reset_control *devm_reset_control_get_by_index(
>   /*
>    * APIs to manage a list of reset controllers
>    */
> -static inline struct reset_control_array *
> +static inline struct reset_control *
>   devm_reset_control_array_get_exclusive(struct device *dev)
>   {
>   	return devm_reset_control_array_get(dev, false, false);
>   }
>   
> -static inline struct reset_control_array *
> +static inline struct reset_control *
>   devm_reset_control_array_get_shared(struct device *dev)
>   {
>   	return devm_reset_control_array_get(dev, true, false);
>   }
>   
> -static inline struct reset_control_array *
> +static inline struct reset_control *
>   devm_reset_control_array_get_optional_exclusive(struct device *dev)
>   {
>   	return devm_reset_control_array_get(dev, false, true);
>   }
>   
> -static inline struct reset_control_array *
> +static inline struct reset_control *
>   devm_reset_control_array_get_optional_shared(struct device *dev)
>   {
>   	return devm_reset_control_array_get(dev, true, true);
>   }
>   
> -static inline struct reset_control_array *
> +static inline struct reset_control *
>   of_reset_control_array_get_exclusive(struct device_node *node)
>   {
>   	return of_reset_control_array_get(node, false, false);
>   }
>   
> -static inline struct reset_control_array *
> +static inline struct reset_control *
>   of_reset_control_array_get_shared(struct device_node *node)
>   {
>   	return of_reset_control_array_get(node, true, false);
>   }
>   
> -static inline struct reset_control_array *
> +static inline struct reset_control *
>   of_reset_control_array_get_optional_exclusive(struct device_node *node)
>   {
>   	return of_reset_control_array_get(node, false, true);
>   }
>   
> -static inline struct reset_control_array *
> +static inline struct reset_control *
>   of_reset_control_array_get_optional_shared(struct device_node *node)
>   {
>   	return of_reset_control_array_get(node, true, true);
Philipp Zabel June 19, 2017, 12:18 p.m. UTC | #4
Hi Vivek,

On Tue, 2017-06-13 at 12:16 +0530, Vivek Gautam wrote:
[...]
> > @@ -102,18 +94,6 @@ static inline struct reset_control *__devm_reset_control_get(
> >   	return optional ? NULL : ERR_PTR(-ENOTSUPP);
> >   }
> >   
> > -static inline
> > -int reset_control_array_assert(struct reset_control_array *resets)
> > -{
> > -	return 0;
> > -}
> > -
> > -static inline
> > -int reset_control_array_deassert(struct reset_control_array *resets)
> > -{
> > -	return 0;
> > -}
> > -
> >   static inline struct reset_control_array *
> 
> This has to return just 'struct reset_control *'.
> That should resolve kbuild errors.
>
> Rest looks good to me.
> Reviewed-by: Vivek Gautam <vivek.gautam@codeaurora.org>

Thanks, I had already fixed this locally, prompted by the kbuild test
robot. I'll send a v6. I would like to merge this patch into the first
"reset: Add APIs to manage array of resets" patch, if that's ok with
you.

regards
Philipp

--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vivek Gautam June 19, 2017, 1:06 p.m. UTC | #5
Hi Philipp,


On 06/19/2017 05:48 PM, Philipp Zabel wrote:
> Hi Vivek,
>
> On Tue, 2017-06-13 at 12:16 +0530, Vivek Gautam wrote:
> [...]
>>> @@ -102,18 +94,6 @@ static inline struct reset_control *__devm_reset_control_get(
>>>    	return optional ? NULL : ERR_PTR(-ENOTSUPP);
>>>    }
>>>    
>>> -static inline
>>> -int reset_control_array_assert(struct reset_control_array *resets)
>>> -{
>>> -	return 0;
>>> -}
>>> -
>>> -static inline
>>> -int reset_control_array_deassert(struct reset_control_array *resets)
>>> -{
>>> -	return 0;
>>> -}
>>> -
>>>    static inline struct reset_control_array *
>> This has to return just 'struct reset_control *'.
>> That should resolve kbuild errors.
>>
>> Rest looks good to me.
>> Reviewed-by: Vivek Gautam <vivek.gautam@codeaurora.org>
> Thanks, I had already fixed this locally, prompted by the kbuild test
> robot. I'll send a v6. I would like to merge this patch into the first
> "reset: Add APIs to manage array of resets" patch, if that's ok with
> you.

Thanks.
You are right. It makes more sense to merge these two patches
if Jon finds it okay to put reset_control_array behind reset_control.

I am cool with squashing this patch.


Best regards
Vivek

>
> regards
> Philipp
>
diff mbox

Patch

diff --git a/drivers/reset/core.c b/drivers/reset/core.c
index 1747000757211..c8fb4426b218a 100644
--- a/drivers/reset/core.c
+++ b/drivers/reset/core.c
@@ -43,11 +43,24 @@  struct reset_control {
 	unsigned int id;
 	struct kref refcnt;
 	bool shared;
+	bool array;
 	atomic_t deassert_count;
 	atomic_t triggered_count;
 };
 
 /**
+ * struct reset_control_array - an array of reset controls
+ * @base: reset control for compatibility with reset control API functions
+ * @num_rstcs: number of reset controls
+ * @rstc: array of reset controls
+ */
+struct reset_control_array {
+	struct reset_control base;
+	unsigned int num_rstcs;
+	struct reset_control *rstc[];
+};
+
+/**
  * of_reset_simple_xlate - translate reset_spec to the reset line number
  * @rcdev: a pointer to the reset controller device
  * @reset_spec: reset line specifier as found in the device tree
@@ -135,6 +148,65 @@  int devm_reset_controller_register(struct device *dev,
 }
 EXPORT_SYMBOL_GPL(devm_reset_controller_register);
 
+static inline struct reset_control_array *
+rstc_to_array(struct reset_control *rstc) {
+	return container_of(rstc, struct reset_control_array, base);
+}
+
+static int reset_control_array_reset(struct reset_control_array *resets)
+{
+	int ret, i;
+
+	for (i = 0; i < resets->num_rstcs; i++) {
+		ret = reset_control_reset(resets->rstc[i]);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int reset_control_array_assert(struct reset_control_array *resets)
+{
+	int ret, i;
+
+	for (i = 0; i < resets->num_rstcs; i++) {
+		ret = reset_control_assert(resets->rstc[i]);
+		if (ret)
+			goto err;
+	}
+
+	return 0;
+
+err:
+	while (i--)
+		reset_control_deassert(resets->rstc[i]);
+	return ret;
+}
+
+static int reset_control_array_deassert(struct reset_control_array *resets)
+{
+	int ret, i;
+
+	for (i = 0; i < resets->num_rstcs; i++) {
+		ret = reset_control_deassert(resets->rstc[i]);
+		if (ret)
+			goto err;
+	}
+
+	return 0;
+
+err:
+	while (i--)
+		reset_control_assert(resets->rstc[i]);
+	return ret;
+}
+
+static inline bool reset_control_is_array(struct reset_control *rstc)
+{
+	return rstc->array;
+}
+
 /**
  * reset_control_reset - reset the controlled device
  * @rstc: reset controller
@@ -158,6 +230,9 @@  int reset_control_reset(struct reset_control *rstc)
 	if (WARN_ON(IS_ERR(rstc)))
 		return -EINVAL;
 
+	if (reset_control_is_array(rstc))
+		return reset_control_array_reset(rstc_to_array(rstc));
+
 	if (!rstc->rcdev->ops->reset)
 		return -ENOTSUPP;
 
@@ -202,6 +277,9 @@  int reset_control_assert(struct reset_control *rstc)
 	if (WARN_ON(IS_ERR(rstc)))
 		return -EINVAL;
 
+	if (reset_control_is_array(rstc))
+		return reset_control_array_assert(rstc_to_array(rstc));
+
 	if (!rstc->rcdev->ops->assert)
 		return -ENOTSUPP;
 
@@ -240,6 +318,9 @@  int reset_control_deassert(struct reset_control *rstc)
 	if (WARN_ON(IS_ERR(rstc)))
 		return -EINVAL;
 
+	if (reset_control_is_array(rstc))
+		return reset_control_array_deassert(rstc_to_array(rstc));
+
 	if (!rstc->rcdev->ops->deassert)
 		return -ENOTSUPP;
 
@@ -266,7 +347,7 @@  int reset_control_status(struct reset_control *rstc)
 	if (!rstc)
 		return 0;
 
-	if (WARN_ON(IS_ERR(rstc)))
+	if (WARN_ON(IS_ERR(rstc)) || reset_control_is_array(rstc))
 		return -EINVAL;
 
 	if (rstc->rcdev->ops->status)
@@ -404,6 +485,16 @@  struct reset_control *__reset_control_get(struct device *dev, const char *id,
 }
 EXPORT_SYMBOL_GPL(__reset_control_get);
 
+static void reset_control_array_put(struct reset_control_array *resets)
+{
+	int i;
+
+	mutex_lock(&reset_list_mutex);
+	for (i = 0; i < resets->num_rstcs; i++)
+		__reset_control_put_internal(resets->rstc[i]);
+	mutex_unlock(&reset_list_mutex);
+}
+
 /**
  * reset_control_put - free the reset controller
  * @rstc: reset controller
@@ -413,6 +504,11 @@  void reset_control_put(struct reset_control *rstc)
 	if (IS_ERR_OR_NULL(rstc))
 		return;
 
+	if (reset_control_is_array(rstc)) {
+		reset_control_array_put(rstc_to_array(rstc));
+		return;
+	}
+
 	mutex_lock(&reset_list_mutex);
 	__reset_control_put_internal(rstc);
 	mutex_unlock(&reset_list_mutex);
@@ -499,81 +595,6 @@  static int of_reset_control_get_count(struct device_node *node)
 }
 
 /**
- * reset_control_array_assert: assert a list of resets
- *
- * @resets: reset control array holding info about the list of resets
- *
- * This API doesn't guarantee that the reset lines controlled by
- * the reset array are asserted in any particular order.
- *
- * Returns 0 on success or error number on failure.
- */
-int reset_control_array_assert(struct reset_control_array *resets)
-{
-	int ret, i;
-
-	if (!resets)
-		return 0;
-
-	if (IS_ERR(resets))
-		return -EINVAL;
-
-	for (i = 0; i < resets->num_rstcs; i++) {
-		ret = reset_control_assert(resets->rstc[i]);
-		if (ret)
-			goto err;
-	}
-
-	return 0;
-
-err:
-	while (i--)
-		reset_control_deassert(resets->rstc[i]);
-	return ret;
-}
-EXPORT_SYMBOL_GPL(reset_control_array_assert);
-
-/**
- * reset_control_array_deassert: deassert a list of resets
- *
- * @resets: reset control array holding info about the list of resets
- *
- * This API doesn't guarantee that the reset lines controlled by
- * the reset array are deasserted in any particular order.
- *
- * Returns 0 on success or error number on failure.
- */
-int reset_control_array_deassert(struct reset_control_array *resets)
-{
-	int ret, i;
-
-	if (!resets)
-		return 0;
-
-	if (IS_ERR(resets))
-		return -EINVAL;
-
-	for (i = 0; i < resets->num_rstcs; i++) {
-		ret = reset_control_deassert(resets->rstc[i]);
-		if (ret)
-			goto err;
-	}
-
-	return 0;
-
-err:
-	while (i--)
-		reset_control_assert(resets->rstc[i]);
-	return ret;
-}
-EXPORT_SYMBOL_GPL(reset_control_array_deassert);
-
-static void devm_reset_control_array_release(struct device *dev, void *res)
-{
-	reset_control_array_put(*(struct reset_control_array **)res);
-}
-
-/**
  * of_reset_control_array_get - Get a list of reset controls using
  *				device node.
  *
@@ -584,13 +605,12 @@  static void devm_reset_control_array_release(struct device *dev, void *res)
  * Returns pointer to allocated reset_control_array on success or
  * error on failure
  */
-struct reset_control_array *
+struct reset_control *
 of_reset_control_array_get(struct device_node *np, bool shared, bool optional)
 {
 	struct reset_control_array *resets;
 	struct reset_control *rstc;
 	int num, i;
-	void *err;
 
 	num = of_reset_control_get_count(np);
 	if (num < 0)
@@ -603,23 +623,24 @@  of_reset_control_array_get(struct device_node *np, bool shared, bool optional)
 
 	for (i = 0; i < num; i++) {
 		rstc = __of_reset_control_get(np, NULL, i, shared, optional);
-		if (IS_ERR(rstc)) {
-			err = ERR_CAST(rstc);
+		if (IS_ERR(rstc))
 			goto err_rst;
-		}
 		resets->rstc[i] = rstc;
 	}
 	resets->num_rstcs = num;
+	resets->base.array = true;
 
-	return resets;
+	return &resets->base;
 
 err_rst:
+	mutex_lock(&reset_list_mutex);
 	while (--i >= 0)
-		reset_control_put(resets->rstc[i]);
+		__reset_control_put_internal(resets->rstc[i]);
+	mutex_unlock(&reset_list_mutex);
 
 	kfree(resets);
 
-	return err;
+	return rstc;
 }
 EXPORT_SYMBOL_GPL(of_reset_control_array_get);
 
@@ -637,40 +658,26 @@  EXPORT_SYMBOL_GPL(of_reset_control_array_get);
  * Returns pointer to allocated reset_control_array on success or
  * error on failure
  */
-struct reset_control_array *
+struct reset_control *
 devm_reset_control_array_get(struct device *dev, bool shared, bool optional)
 {
-	struct reset_control_array **devres;
-	struct reset_control_array *resets;
+	struct reset_control **devres;
+	struct reset_control *rstc;
 
-	devres = devres_alloc(devm_reset_control_array_release,
-			      sizeof(*devres), GFP_KERNEL);
+	devres = devres_alloc(devm_reset_control_release, sizeof(*devres),
+			      GFP_KERNEL);
 	if (!devres)
 		return ERR_PTR(-ENOMEM);
 
-	resets = of_reset_control_array_get(dev->of_node, shared, optional);
-	if (IS_ERR(resets)) {
-		devres_free(resets);
-		return resets;
+	rstc = of_reset_control_array_get(dev->of_node, shared, optional);
+	if (IS_ERR(rstc)) {
+		devres_free(devres);
+		return rstc;
 	}
 
-	*devres = resets;
+	*devres = rstc;
 	devres_add(dev, devres);
 
-	return resets;
+	return rstc;
 }
 EXPORT_SYMBOL_GPL(devm_reset_control_array_get);
-
-void reset_control_array_put(struct reset_control_array *resets)
-{
-	int i;
-
-	if (IS_ERR_OR_NULL(resets))
-		return;
-
-	for (i = 0; i < resets->num_rstcs; i++)
-		reset_control_put(resets->rstc[i]);
-
-	kfree(resets);
-}
-EXPORT_SYMBOL_GPL(reset_control_array_put);
diff --git a/include/linux/reset.h b/include/linux/reset.h
index df75fe50f765d..0f1be13e66e46 100644
--- a/include/linux/reset.h
+++ b/include/linux/reset.h
@@ -5,11 +5,6 @@ 
 
 struct reset_control;
 
-struct reset_control_array {
-	unsigned int num_rstcs;
-	struct reset_control *rstc[];
-};
-
 #ifdef CONFIG_RESET_CONTROLLER
 
 int reset_control_reset(struct reset_control *rstc);
@@ -30,13 +25,10 @@  struct reset_control *__devm_reset_control_get(struct device *dev,
 
 int __must_check device_reset(struct device *dev);
 
-int reset_control_array_assert(struct reset_control_array *resets);
-int reset_control_array_deassert(struct reset_control_array *resets);
-struct reset_control_array *devm_reset_control_array_get(struct device *dev,
-						bool shared, bool optional);
-struct reset_control_array *of_reset_control_array_get(struct device_node *np,
-						bool shared, bool optional);
-void reset_control_array_put(struct reset_control_array *resets);
+struct reset_control *devm_reset_control_array_get(struct device *dev,
+						   bool shared, bool optional);
+struct reset_control *of_reset_control_array_get(struct device_node *np,
+						 bool shared, bool optional);
 
 static inline int device_reset_optional(struct device *dev)
 {
@@ -102,18 +94,6 @@  static inline struct reset_control *__devm_reset_control_get(
 	return optional ? NULL : ERR_PTR(-ENOTSUPP);
 }
 
-static inline
-int reset_control_array_assert(struct reset_control_array *resets)
-{
-	return 0;
-}
-
-static inline
-int reset_control_array_deassert(struct reset_control_array *resets)
-{
-	return 0;
-}
-
 static inline struct reset_control_array *
 devm_reset_control_array_get(struct device *dev, bool shared, bool optional)
 {
@@ -420,49 +400,49 @@  static inline struct reset_control *devm_reset_control_get_by_index(
 /*
  * APIs to manage a list of reset controllers
  */
-static inline struct reset_control_array *
+static inline struct reset_control *
 devm_reset_control_array_get_exclusive(struct device *dev)
 {
 	return devm_reset_control_array_get(dev, false, false);
 }
 
-static inline struct reset_control_array *
+static inline struct reset_control *
 devm_reset_control_array_get_shared(struct device *dev)
 {
 	return devm_reset_control_array_get(dev, true, false);
 }
 
-static inline struct reset_control_array *
+static inline struct reset_control *
 devm_reset_control_array_get_optional_exclusive(struct device *dev)
 {
 	return devm_reset_control_array_get(dev, false, true);
 }
 
-static inline struct reset_control_array *
+static inline struct reset_control *
 devm_reset_control_array_get_optional_shared(struct device *dev)
 {
 	return devm_reset_control_array_get(dev, true, true);
 }
 
-static inline struct reset_control_array *
+static inline struct reset_control *
 of_reset_control_array_get_exclusive(struct device_node *node)
 {
 	return of_reset_control_array_get(node, false, false);
 }
 
-static inline struct reset_control_array *
+static inline struct reset_control *
 of_reset_control_array_get_shared(struct device_node *node)
 {
 	return of_reset_control_array_get(node, true, false);
 }
 
-static inline struct reset_control_array *
+static inline struct reset_control *
 of_reset_control_array_get_optional_exclusive(struct device_node *node)
 {
 	return of_reset_control_array_get(node, false, true);
 }
 
-static inline struct reset_control_array *
+static inline struct reset_control *
 of_reset_control_array_get_optional_shared(struct device_node *node)
 {
 	return of_reset_control_array_get(node, true, true);