[PATCH 1/3] dev: Disambiguate errors in uclass_find

[Simon Glass]

Hi Sean,

On Wed, 16 Sep 2020 at 19:44, Sean Anderson <seanga2 at gmail.com> wrote:
>
> On 9/16/20 9:09 PM, Simon Glass wrote:
> > Hi Sean,
> >
> > On Sat, 12 Sep 2020 at 15:46, Sean Anderson <seanga2 at gmail.com> wrote:
> >>
> >> There are two cases where uclass_find can return an error. The second is
> >> when the uclass has not yet been init'd. The first is when the driver model
> >> has not been init'd (or has been only partially init'd) and there is no
> >> root uclass driver.
> >>
> >> If LOG_SYSLOG is enabled and LOG_DEFAULT_LEVEL is set to LOGL_DEBUG or
> >> higher, log_syslog_emit will try to call net_init before initf_dm has been
> >> called. This in turn (eventually) calls uclass_get for UCLASS_ETH.
> >>
> >> If the second error occurs, uclass_get should call uclass_add to create the
> >> uclass. If the first error occurs, then uclass_get should not call
> >> uclass_add (because the uclass list has not been initialized). However, the
> >> first error is expected when calling uclass_get for UCLASS_ROOT, so we add
> >> an exception.
> >>
> >> There are several alternative approaches to this patch. One option would be
> >> to duplicate the check against gd->dm_root in uclass_get and not change the
> >> behavior of uclass_find. I did not choose this approach because I think it
> >> it is less clear than the patch as-is. However, that is certainly
> >> subjective, and there is no other technical reason to do it this way.
> >>
> >> Another approach would have been to change log_syslog_emit to abort if it
> >> is called too early. I did not choose this approach because the check in
> >> uclass_find to see if gd->dm_root exists implies that functions in its
> >> file are allowed to be called at any time. There is an argument to be made
> >> that callers should ensure that they don't call certain functions too
> >> early. I think it is better to code defensively in these circumstances to
> >> make it easier to discover such errors.
> >>
> >> Signed-off-by: Sean Anderson <seanga2 at gmail.com>
> >> ---
> >>
> >>  drivers/core/uclass.c | 16 +++++++++++++++-
> >>  test/dm/core.c        |  2 +-
> >>  test/dm/test-main.c   |  2 +-
> >>  3 files changed, 17 insertions(+), 3 deletions(-)
> >
> > I'm not sure you can do this. You need to call dm_init() to get DM
> > running properly.
> >
> > So in this case I think we need to arrange for the log output to not
> > happen, before DM is inited.
>
> Ok, so do you think the second approach is better? E.g. adding
>
> if (!gd || !(gd->flags & GD_FLG_DEVINIT))
>         return 0;
>
> to the beginning of log_syslog_emit?

Yes. But how about we have a function in dm.h or similar that tells if
you that driver model is inited? It could be set quite early in
dm_init().

Regards,
Simon