[PATCH 2/2] reset: Let reset API's handle gracefully if reset_ctl is -ENODATA

[Simon Glass]

Hi Kishon,

On Fri, 7 May 2021 at 05:02, Kishon Vijay Abraham I <kishon at ti.com> wrote:
>
> Let reset API's (like reset_assert(), reset_deassert(), ..) handle
> gracefully if the argument reset_ctl is -ENODATA. This is for seamlessly
> handling client drivers which get optional reset controller using
> devm_reset_control_get_optional().
>
> Signed-off-by: Kishon Vijay Abraham I <kishon at ti.com>
> ---
>  drivers/reset/reset-uclass.c | 35 +++++++++++++++++++++++++++++------
>  1 file changed, 29 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/reset/reset-uclass.c b/drivers/reset/reset-uclass.c
> index 906f58ce3d..077ca956f4 100644
> --- a/drivers/reset/reset-uclass.c
> +++ b/drivers/reset/reset-uclass.c
> @@ -162,7 +162,12 @@ int reset_get_by_name(struct udevice *dev, const char *name,
>
>  int reset_request(struct reset_ctl *reset_ctl)
>  {
> -       struct reset_ops *ops = reset_dev_ops(reset_ctl->dev);
> +       struct reset_ops *ops;
> +
> +       if (IS_ERR(reset_ctl) && PTR_ERR(reset_ctl) == -ENODATA)
> +               return 0;

This should call reset_valid(), not do things here. Also I thought we
came to the conclusion that reset_ctl would always be a valid pointer?

In any case, if you change the logic of these function, you must also
change the header, otherwise there is no docs.

Finally, can you please update the tests?

> +
> +       ops = reset_dev_ops(reset_ctl->dev);
>
>         debug("%s(reset_ctl=%p)\n", __func__, reset_ctl);
>
> @@ -171,7 +176,12 @@ int reset_request(struct reset_ctl *reset_ctl)
>
>  int reset_free(struct reset_ctl *reset_ctl)
>  {
> -       struct reset_ops *ops = reset_dev_ops(reset_ctl->dev);
> +       struct reset_ops *ops;
> +
> +       if (IS_ERR(reset_ctl) && PTR_ERR(reset_ctl) == -ENODATA)
> +               return 0;
> +
> +       ops = reset_dev_ops(reset_ctl->dev);
>
>         debug("%s(reset_ctl=%p)\n", __func__, reset_ctl);
>
> @@ -180,7 +190,12 @@ int reset_free(struct reset_ctl *reset_ctl)
>
>  int reset_assert(struct reset_ctl *reset_ctl)
>  {
> -       struct reset_ops *ops = reset_dev_ops(reset_ctl->dev);
> +       struct reset_ops *ops;
> +
> +       if (IS_ERR(reset_ctl) && PTR_ERR(reset_ctl) == -ENODATA)
> +               return 0;
> +
> +       ops = reset_dev_ops(reset_ctl->dev);
>
>         debug("%s(reset_ctl=%p)\n", __func__, reset_ctl);
>
> @@ -202,7 +217,12 @@ int reset_assert_bulk(struct reset_ctl_bulk *bulk)
>
>  int reset_deassert(struct reset_ctl *reset_ctl)
>  {
> -       struct reset_ops *ops = reset_dev_ops(reset_ctl->dev);
> +       struct reset_ops *ops;
> +
> +       if (IS_ERR(reset_ctl) && PTR_ERR(reset_ctl) == -ENODATA)
> +               return 0;
> +
> +       ops = reset_dev_ops(reset_ctl->dev);
>
>         debug("%s(reset_ctl=%p)\n", __func__, reset_ctl);
>
> @@ -224,9 +244,12 @@ int reset_deassert_bulk(struct reset_ctl_bulk *bulk)
>
>  int reset_status(struct reset_ctl *reset_ctl)
>  {
> -       struct reset_ops *ops = reset_dev_ops(reset_ctl->dev);
> +       struct reset_ops *ops;
>
> -       debug("%s(reset_ctl=%p)\n", __func__, reset_ctl);
> +       if (IS_ERR(reset_ctl) && PTR_ERR(reset_ctl) == -ENODATA)
> +               return 0;
> +
> +       ops = reset_dev_ops(reset_ctl->dev);
>
>         return ops->rst_status(reset_ctl);
>  }
> --
> 2.17.1
>

Regards,
Simon