Hi Ramiro,
Am Donnerstag, den 12.01.2017, 18:34 +0000 schrieb Ramiro Oliveira:
> "The *_get_optional_* functions weren't really optional so this patch
> makes them really optional.
>
> These *_get_optional_* functions will now return NULL instead of an error
> if no matching reset phandle is found in the DT, and all the
> reset_control_* functions now accept NULL rstc pointers.
>
> Signed-off-by: Ramiro Oliveira <ramiro.olive...@synopsys.com>
> ---
> drivers/reset/core.c | 59
> ++++++++++++++++++++++++++++++++++++++++-----------
> include/linux/reset.h | 45 ++++++++++++++++++++++-----------------
> 2 files changed, 73 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/reset/core.c b/drivers/reset/core.c
> index 272c1e4ecb5c..7b679b0bfef8 100644
> --- a/drivers/reset/core.c
> +++ b/drivers/reset/core.c
> @@ -143,12 +143,18 @@ EXPORT_SYMBOL_GPL(devm_reset_controller_register);
> * a no-op.
> * Consumers must not use reset_control_(de)assert on shared reset lines when
> * reset_control_reset has been used.
> + *
> + * If rstc is NULL it is an optional reset and the function will just
> + * return 0.
> */
> int reset_control_reset(struct reset_control *rstc)
> {
> int ret;
>
> - if (WARN_ON(IS_ERR_OR_NULL(rstc)))
> + if (!rstc)
> + return 0;
> +
> + if (WARN_ON(IS_ERR(rstc)))
> return -EINVAL;
>
> if (!rstc->rcdev->ops->reset)
> @@ -182,10 +188,17 @@ EXPORT_SYMBOL_GPL(reset_control_reset);
> * internal state to be reset, but must be prepared for this to happen.
> * Consumers must not use reset_control_reset on shared reset lines when
> * reset_control_(de)assert has been used.
> + * return 0.
> + *
> + * If rstc is NULL it is an optional reset and the function will just
> + * return 0.
> */
> int reset_control_assert(struct reset_control *rstc)
> {
> - if (WARN_ON(IS_ERR_OR_NULL(rstc)))
> + if (!rstc)
> + return 0;
> +
> + if (WARN_ON(IS_ERR(rstc)))
> return -EINVAL;
>
> if (!rstc->rcdev->ops->assert)
> @@ -213,10 +226,17 @@ EXPORT_SYMBOL_GPL(reset_control_assert);
> * After calling this function, the reset is guaranteed to be deasserted.
> * Consumers must not use reset_control_reset on shared reset lines when
> * reset_control_(de)assert has been used.
> + * return 0.
> + *
> + * If rstc is NULL it is an optional reset and the function will just
> + * return 0.
> */
> int reset_control_deassert(struct reset_control *rstc)
> {
> - if (WARN_ON(IS_ERR_OR_NULL(rstc)))
> + if (!rstc)
> + return 0;
> +
> + if (WARN_ON(IS_ERR(rstc)))
> return -EINVAL;
>
> if (!rstc->rcdev->ops->deassert)
> @@ -237,12 +257,15 @@ EXPORT_SYMBOL_GPL(reset_control_deassert);
> /**
> * reset_control_status - returns a negative errno if not supported, a
> * positive value if the reset line is asserted, or zero if the reset
> - * line is not asserted.
> + * line is not asserted or if the desc is NULL (optional reset).
> * @rstc: reset controller
> */
> int reset_control_status(struct reset_control *rstc)
> {
> - if (WARN_ON(IS_ERR_OR_NULL(rstc)))
> + if (!rstc)
> + return 0;
> +
> + if (WARN_ON(IS_ERR(rstc)))
> return -EINVAL;
>
> if (rstc->rcdev->ops->status)
> @@ -299,7 +322,8 @@ static void __reset_control_put(struct reset_control
> *rstc)
> }
>
> struct reset_control *__of_reset_control_get(struct device_node *node,
> - const char *id, int index, bool shared)
> + const char *id, int index, bool shared,
> + bool optional)
> {
> struct reset_control *rstc;
> struct reset_controller_dev *r, *rcdev;
> @@ -313,14 +337,24 @@ struct reset_control *__of_reset_control_get(struct
> device_node *node,
> if (id) {
> index = of_property_match_string(node,
> "reset-names", id);
> - if (index < 0)
> - return ERR_PTR(-ENOENT);
> + if (index < 0) {
> + if (optional)
> + return NULL;
> + else
> + return (index == -EILSEQ) ?
> + ERR_PTR(index) : ERR_PTR(-ENOENT);
That is not what I meant. I think reset_control_get should return an
error on EILSEQ even if optional is true, since that means there are
resets specified in the DT, but the reset-names property is broken.
How about:
if (index == -EILSEQ)
return ERR_PTR(index);
if (index < 0)
return optional ? NULL : ERR_PTR(-ENOENT);
> + }
> }
>
> ret = of_parse_phandle_with_args(node, "resets", "#reset-cells",
> index, &args);
> - if (ret)
> - return ERR_PTR(ret);
> + if (ret) {
> + if (optional)
> + return NULL;
> + else
> + return (index == -EINVAL) ?
> + ERR_PTR(index) : ERR_PTR(-ENOENT);
> + }
That is not what I meant either. If the phandle parsing fails with
of_phandle_iterator_next returning -EINVAL, that should be passed along
in the optional case: that happens when we've already found a matching
entry in reset-names, but the resets property is broken.
How about:
if (ret == -EINVAL)
return ERR_PTR(ret);
if (ret)
return optional ? NULL : ERR_PTR(ret);
regards
Philipp
