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

[Simon Glass]

Hi Kishon,

On Thu, 13 May 2021 at 00:30, 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:
> >>
> >> 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?
>
> Patch 1 updates sandbox_reset_test_get_devm() in sandbox-reset-test.c.
> That's the only test I saw was using optional reset APIs. Do you think
> any thing else needs to be updated?

My point is that if you are changing the logic of reset_request(), you
should add a test for it and you need to update the docs.

Do you have someone at TI that can work through this with you? I am
not the maintainer of this subsystem, nor the expert here, but I feel
it could use a review with someone else as well, to really nut out all
the issues once and for all.

Regards,
Simon