[PATCH 1/2] reset: Do not return NULL on error for devm_reset_control_get_optional()

[Simon Glass]

Hi Kishon,

On Tue, 11 May 2021 at 00:26, Kishon Vijay Abraham I <kishon at ti.com> wrote:
>
> Hi Simon,
>
> On 07/05/21 10:04 pm, Simon Glass wrote:
> > Hi Kishon,
> >
> > On Fri, 7 May 2021 at 05:02, Kishon Vijay Abraham I <kishon at ti.com> wrote:
> >>
> >> In order for client to know whether it was able to successfully get a
> >> reset controller or not, do not return NULL on error for
> >> devm_reset_control_get_optional() and
> >> devm_reset_bulk_get_optional_by_node().
> >>
> >> Signed-off-by: Kishon Vijay Abraham I <kishon at ti.com>
> >> ---
> >>  drivers/reset/reset-uclass.c       | 16 ++--------------
> >>  drivers/reset/sandbox-reset-test.c |  2 +-
> >>  2 files changed, 3 insertions(+), 15 deletions(-)
> >>
> >> diff --git a/drivers/reset/reset-uclass.c b/drivers/reset/reset-uclass.c
> >> index ac89eaf098..906f58ce3d 100644
> >> --- a/drivers/reset/reset-uclass.c
> >> +++ b/drivers/reset/reset-uclass.c
> >> @@ -299,12 +299,7 @@ struct reset_ctl *devm_reset_control_get(struct udevice *dev, const char *id)
> >>  struct reset_ctl *devm_reset_control_get_optional(struct udevice *dev,
> >>                                                   const char *id)
> >>  {
> >> -       struct reset_ctl *r = devm_reset_control_get(dev, id);
> >> -
> >> -       if (IS_ERR(r))
> >> -               return NULL;
> >> -
> >> -       return r;
> >> +       return devm_reset_control_get(dev, id);
> >>  }
> >
> > We need to get some updates to the API (function comments in the
> > header) here. I'm not sure what the intent is.
>
> right, that has to be fixed.
> >
> > I thought these functions were going to return a valid (but possibly
> > empty) reset_ctl always?
>
> I thought about it again and felt it might not be correct to return
> reset_ctl always. The reset control is optional only if the consumer
> node doesn't have populated reset properties.
>
> If we always return valid reset_ctl possibly with it's dev member
> initialized or not initialized, it would not be possible to tell it's
> not initialized because of the absence of reset property or due to some
> other valid error.

reset_valid() should check if dev is NULL or not, like with clock and
GPIO. Is that enough?

Regards,
Simon