[PATCH 1/1] lib: utils/reset: error handling in fdt_reset_init()

[Anup Patel]

On Tue, Oct 26, 2021 at 3:38 PM Heinrich Schuchardt
<heinrich.schuchardt at canonical.com> wrote:
>
> On 10/26/21 11:47, Anup Patel wrote:
> > On Tue, Oct 26, 2021 at 1:55 PM Heinrich Schuchardt
> > <heinrich.schuchardt at canonical.com> wrote:
> >>
> >> The initialization of a reset driver may fail for various reasons, like
> >> a PMIC based reset driver not finding the required I2C driver. The return
> >> code of the init routine may take other error values than -ENODEV.
> >>
> >> If the initialization of a reset driver fails, this should not lead to the
> >> board hanging. It is enough that the reset driver does not call
> >> sbi_system_reset_add_device() to avoid invoking the driver for a device
> >> that could not be initialized.
> >>
> >> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
> >> ---
> >>   lib/utils/reset/fdt_reset.c | 9 +++++----
> >>   1 file changed, 5 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/lib/utils/reset/fdt_reset.c b/lib/utils/reset/fdt_reset.c
> >> index 92f37b0..e1d6fdc 100644
> >> --- a/lib/utils/reset/fdt_reset.c
> >> +++ b/lib/utils/reset/fdt_reset.c
> >> @@ -46,10 +46,11 @@ int fdt_reset_init(void)
> >>
> >>                  if (drv->init) {
> >>                          rc = drv->init(fdt, noff, match);
> >> -                       if (rc == SBI_ENODEV)
> >> -                               continue;
> >> -                       if (rc)
> >> -                               return rc;
> >> +                       /*
> >> +                        * The driver will not call
> >> +                        * sbi_system_reset_add_device() in case of an error.
> >> +                        * Hence any error shall be ignored here.
> >> +                        */
> >
> > Just a minor suggestion, how about having an error print
> > for "rc && rc != SBI_ENODEV" ?
>
> fdt_reset_init() is called in sbi_platform_early_init(). This is before
> sbi_console_init(). So you don't have a console yet.
>
> Can we move the fdt_reset_init() call from generic_early_init() to
> generic_final_init()?

Yes, It is better to call fdt_reset_init() from generic_final_init().

Maybe call it as the first thing in generic_final_init() for
cold_boot == true ?

Regards,
Anup

>
> Best regards
>
> Heinrich
>
> >
> >>                  }
> >>          }
> >>
> >> --
> >> 2.32.0
> >>
> >>
> >> --
> >> opensbi mailing list
> >> opensbi at lists.infradead.org
> >> http://lists.infradead.org/mailman/listinfo/opensbi
> >
> > Otherwise it looks good to me.
> >
> > Reviewed-by: Anup Patel <anup.patel at wdc.com>
> >
> > Regards,
> > Anup
> >
>