[PATCH] reset: Put back *_optional variants

[Hans de Goede]

Hi,

On 26-05-16 03:15, John Youn wrote:
> Prior to commit 6c96f05c8bb8 ("reset: Make [of_]reset_control_get[_foo]
> functions wrappers"), the optional variants returned -ENOTSUPP when
> CONFIG_RESET_CONTROLLER was not set. This patch reverts to this
> behavior. Otherwise those calls will return -EINVAL causing users to
> think that an error occurred when CONFIG_RESET_CONTROLLER is not set.
>
> Fixes: 6c96f05c8bb8 ("reset: Make [of_]reset_control_get[_foo] functions wrappers")
> Signed-off-by: John Youn <johnyoun at synopsys.com>
> ---
>
> Hi Philipp, Hans,
>
> The commit referenced above breaks an upcoming patch for the dwc2
> driver that adds an optional reset control.
>
> https://marc.info/?l=linux-usb&m=146161328211584&w=2
>
> I've attempted to add the optional variants back the way they were
> working before. Let me know if I need to do anything else to fix it or
> if it should be done another way.
>
> Regards,
> John

Hmm, I don't like all the extra code your patch adds just to fix
a return value...

Looking at the code before my "reset: Make [of_]reset_control_get[_foo]
functions wrappers" patch, all the dev*_get* functions were
returning ENOTSUPP except for [devm_]reset_control_get, so following
your logic we should also change the of_reset_control_get_by_index
variant to return -ENOTSUP.

Or better, simply make them all return -ENOTSUP, that seems both
consistent and more KISS to me, this would mean an error code
change for [devm_]reset_control_get, but will fix all the other
getters from having a changed error-code, and I would callers
of [devm_]reset_control_get to not care which error code they
get, except for -EPROBE_DEFER.

So IMHO the following change would be a better way to fix this:

--- a/include/linux/reset.h
+++ b/include/linux/reset.h
@@ -65,14 +65,14 @@ static inline struct reset_control *__of_reset_control_get(
                                         struct device_node *node,
                                         const char *id, int index, int shared)
  {
-   return ERR_PTR(-EINVAL);
+ return ERR_PTR(-ENOTSUPP);
  }

  static inline struct reset_control *__devm_reset_control_get(
                                         struct device *dev,
                                         const char *id, int index, int shared)
  {
-   return ERR_PTR(-EINVAL);
+ return ERR_PTR(-ENOTSUPP);
  }

  #endif /* CONFIG_RESET_CONTROLLER */



Regards,

Hans



>
>  include/linux/reset.h | 33 +++++++++++++++++++++++++++++++--
>  1 file changed, 31 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/reset.h b/include/linux/reset.h
> index ec0306ce..739d33d 100644
> --- a/include/linux/reset.h
> +++ b/include/linux/reset.h
> @@ -14,10 +14,24 @@ int reset_control_status(struct reset_control *rstc);
>
>  struct reset_control *__of_reset_control_get(struct device_node *node,
>  				     const char *id, int index, int shared);
> +
> +struct reset_control *__of_reset_control_get_optional(struct device_node *node,
> +				     const char *id, int index, int shared)
> +{
> +	return __of_reset_control_get(node, id, index, shared);
> +}
> +
>  void reset_control_put(struct reset_control *rstc);
>  struct reset_control *__devm_reset_control_get(struct device *dev,
>  				     const char *id, int index, int shared);
>
> +static inline struct reset_control *__devm_reset_control_get_optional(
> +					struct device *dev,
> +					const char *id, int index, int shared)
> +{
> +	return __devm_reset_control_get(dev, id, index, shared);
> +}
> +
>  int __must_check device_reset(struct device *dev);
>
>  static inline int device_reset_optional(struct device *dev)
> @@ -74,6 +88,13 @@ static inline struct reset_control *__of_reset_control_get(
>  	return ERR_PTR(-EINVAL);
>  }
>
> +static inline struct reset_control *__of_reset_control_get_optional(
> +					struct device_node *node,
> +					const char *id, int index, int shared)
> +{
> +	return ERR_PTR(-ENOTSUPP);
> +}
> +
>  static inline struct reset_control *__devm_reset_control_get(
>  					struct device *dev,
>  					const char *id, int index, int shared)
> @@ -81,6 +102,13 @@ static inline struct reset_control *__devm_reset_control_get(
>  	return ERR_PTR(-EINVAL);
>  }
>
> +static inline struct reset_control *__devm_reset_control_get_optional(
> +					struct device *dev,
> +					const char *id, int index, int shared)
> +{
> +	return ERR_PTR(-ENOTSUPP);
> +}
> +
>  #endif /* CONFIG_RESET_CONTROLLER */
>
>  /**
> @@ -110,7 +138,8 @@ static inline struct reset_control *__must_check reset_control_get(
>  static inline struct reset_control *reset_control_get_optional(
>  					struct device *dev, const char *id)
>  {
> -	return __of_reset_control_get(dev ? dev->of_node : NULL, id, 0, 0);
> +	return __of_reset_control_get_optional(dev ? dev->of_node : NULL,
> +					       id, 0, 0);
>  }
>
>  /**
> @@ -194,7 +223,7 @@ static inline struct reset_control *__must_check devm_reset_control_get(
>  static inline struct reset_control *devm_reset_control_get_optional(
>  					struct device *dev, const char *id)
>  {
> -	return __devm_reset_control_get(dev, id, 0, 0);
> +	return __devm_reset_control_get_optional(dev, id, 0, 0);
>  }
>
>  /**
>