Re: [PATCH] reset: add exported __reset_control_get, return NULL if optional

From: Andy Shevchenko
Date: Tue Apr 04 2017 - 06:36:38 EST


On Tue, 2017-04-04 at 10:18 +0200, Philipp Zabel wrote:
> Rename the internal __reset_control_get/put functions to
> __reset_control_get/put_internal and add an exported
> __reset_control_get equivalent to __of_reset_control_get
> that takes a struct device parameter.
> This avoids the confusing call to __of_reset_control_get in
> the non-DT case and fixes the devm_reset_control_get_optional
> function to return NULL if RESET_CONTROLLER is enabled but
> dev->of_node == NULL.
>
> Fixes: bb475230b8e5 ("reset: make optional functions really optional")
> Reported-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> Cc: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> Cc: Ramiro Oliveira <Ramiro.Oliveira@xxxxxxxxxxxx>
> Signed-off-by: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx>

Tested-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>

(At least if fixes serial console on x86 boards with no legacy UART)

> ---
> Âdrivers/reset/core.cÂÂ| 22 ++++++++++++++++------
> Âinclude/linux/reset.h | 22 ++++++++++++++--------
> Â2 files changed, 30 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/reset/core.c b/drivers/reset/core.c
> index f1e5e65388bb5..cd739d2fa1603 100644
> --- a/drivers/reset/core.c
> +++ b/drivers/reset/core.c
> @@ -275,7 +275,7 @@ int reset_control_status(struct reset_control
> *rstc)
> Â}
> ÂEXPORT_SYMBOL_GPL(reset_control_status);
> Â
> -static struct reset_control *__reset_control_get(
> +static struct reset_control *__reset_control_get_internal(
> Â struct reset_controller_dev *rcdev,
> Â unsigned int index, bool shared)
> Â{
> @@ -308,7 +308,7 @@ static struct reset_control *__reset_control_get(
> Â return rstc;
> Â}
> Â
> -static void __reset_control_put(struct reset_control *rstc)
> +static void __reset_control_put_internal(struct reset_control *rstc)
> Â{
> Â lockdep_assert_held(&reset_list_mutex);
> Â
> @@ -377,7 +377,7 @@ struct reset_control
> *__of_reset_control_get(struct device_node *node,
> Â }
> Â
> Â /* reset_list_mutex also protects the rcdev's reset_control
> list */
> - rstc = __reset_control_get(rcdev, rstc_id, shared);
> + rstc = __reset_control_get_internal(rcdev, rstc_id, shared);
> Â
> Â mutex_unlock(&reset_list_mutex);
> Â
> @@ -385,6 +385,17 @@ struct reset_control
> *__of_reset_control_get(struct device_node *node,
> Â}
> ÂEXPORT_SYMBOL_GPL(__of_reset_control_get);
> Â
> +struct reset_control *__reset_control_get(struct device *dev, const
> char *id,
> + ÂÂint index, bool shared,
> bool optional)
> +{
> + if (dev->of_node)
> + return __of_reset_control_get(dev->of_node, id,
> index, shared,
> + ÂÂÂÂÂÂoptional);
> +
> + return optional ? NULL : ERR_PTR(-EINVAL);
> +}
> +EXPORT_SYMBOL_GPL(__reset_control_get);
> +
> Â/**
> Â * reset_control_put - free the reset controller
> Â * @rstc: reset controller
> @@ -396,7 +407,7 @@ void reset_control_put(struct reset_control *rstc)
> Â return;
> Â
> Â mutex_lock(&reset_list_mutex);
> - __reset_control_put(rstc);
> + __reset_control_put_internal(rstc);
> Â mutex_unlock(&reset_list_mutex);
> Â}
> ÂEXPORT_SYMBOL_GPL(reset_control_put);
> @@ -417,8 +428,7 @@ struct reset_control
> *__devm_reset_control_get(struct device *dev,
> Â if (!ptr)
> Â return ERR_PTR(-ENOMEM);
> Â
> - rstc = __of_reset_control_get(dev ? dev->of_node : NULL,
> - ÂÂÂÂÂÂid, index, shared, optional);
> + rstc = __reset_control_get(dev, id, index, shared, optional);
> Â if (!IS_ERR(rstc)) {
> Â *ptr = rstc;
> Â devres_add(dev, ptr);
> diff --git a/include/linux/reset.h b/include/linux/reset.h
> index 96fb139bdd08f..13d8681210d54 100644
> --- a/include/linux/reset.h
> +++ b/include/linux/reset.h
> @@ -15,6 +15,9 @@ int reset_control_status(struct reset_control
> *rstc);
> Âstruct reset_control *__of_reset_control_get(struct device_node
> *node,
> Â ÂÂÂÂÂconst char *id, int index, bool
> shared,
> Â ÂÂÂÂÂbool optional);
> +struct reset_control *__reset_control_get(struct device *dev, const
> char *id,
> + ÂÂint index, bool shared,
> + ÂÂbool optional);
> Âvoid reset_control_put(struct reset_control *rstc);
> Âstruct reset_control *__devm_reset_control_get(struct device *dev,
> Â ÂÂÂÂÂconst char *id, int index, bool
> shared,
> @@ -72,6 +75,13 @@ static inline struct reset_control
> *__of_reset_control_get(
> Â return optional ? NULL : ERR_PTR(-ENOTSUPP);
> Â}
> Â
> +static inline struct reset_control *__reset_control_get(
> + struct device *dev, const
> char *id,
> + int index, bool shared, bool
> optional)
> +{
> + return optional ? NULL : ERR_PTR(-ENOTSUPP);
> +}
> +
> Âstatic inline struct reset_control *__devm_reset_control_get(
> Â struct device *dev, const
> char *id,
> Â int index, bool shared, bool
> optional)
> @@ -102,8 +112,7 @@ __must_check reset_control_get_exclusive(struct
> device *dev, const char *id)
> Â#ifndef CONFIG_RESET_CONTROLLER
> Â WARN_ON(1);
> Â#endif
> - return __of_reset_control_get(dev ? dev->of_node : NULL, id,
> 0, false,
> -
> false);
> + return __reset_control_get(dev, id, 0, false, false);
> Â}
> Â
> Â/**
> @@ -131,22 +140,19 @@ __must_check reset_control_get_exclusive(struct
> device *dev, const char *id)
> Âstatic inline struct reset_control *reset_control_get_shared(
> Â struct device *dev, const
> char *id)
> Â{
> - return __of_reset_control_get(dev ? dev->of_node : NULL, id,
> 0, true,
> -
> false);
> + return __reset_control_get(dev, id, 0, true, false);
> Â}
> Â
> Âstatic inline struct reset_control
> *reset_control_get_optional_exclusive(
> Â struct device *dev, const
> char *id)
> Â{
> - return __of_reset_control_get(dev ? dev->of_node : NULL, id,
> 0, false,
> -
> true);
> + return __reset_control_get(dev, id, 0, false, true);
> Â}
> Â
> Âstatic inline struct reset_control
> *reset_control_get_optional_shared(
> Â struct device *dev, const
> char *id)
> Â{
> - return __of_reset_control_get(dev ? dev->of_node : NULL, id,
> 0, true,
> -
> true);
> + return __reset_control_get(dev, id, 0, true, true);
> Â}
> Â
> Â/**

--
Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
Intel Finland Oy