Hi Simon,
On Sun, 28 Sep 2014 10:18:59 -0600
Simon Glass <s...@chromium.org> wrote:
> Hi Masahiro,
>
> On 28 September 2014 09:54, Masahiro YAMADA <yamad...@jp.panasonic.com> wrote:
> > Simon,
> >
> >
> > 2014-09-29 0:17 GMT+09:00 Simon Glass <s...@chromium.org>:
> >> Hi Masahiro,
> >>
> >> On 28 September 2014 07:52, Masahiro Yamada <yamad...@jp.panasonic.com>
> >> wrote:
> >>> The function uclass_find() looks for a uclass in the linked
> >>> list of gd->uclass_root; gd->dm_root has nothing to do with
> >>> gd->uclass_root. Remove this confusing code.
> >>>
> >>> Signed-off-by: Masahiro Yamada <yamad...@jp.panasonic.com>
> >>> ---
> >>>
> >>> drivers/core/uclass.c | 2 --
> >>> 1 file changed, 2 deletions(-)
> >>>
> >>> diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c
> >>> index 901b06e..74df613 100644
> >>> --- a/drivers/core/uclass.c
> >>> +++ b/drivers/core/uclass.c
> >>> @@ -23,8 +23,6 @@ struct uclass *uclass_find(enum uclass_id key)
> >>> {
> >>> struct uclass *uc;
> >>>
> >>> - if (!gd->dm_root)
> >>> - return NULL;
> >>> /*
> >>> * TODO(s...@chromium.org): Optimise this, perhaps moving the
> >>> found
> >>> * node to the start of the list, or creating a linear array
> >>> mapping
> >>
> >> This came in in commit:
> >>
> >> c910e2e dm: Avoid accessing uclasses before they are ready
> >>
> >> Please see that (and the test that was added) for an explanation.
> >
> >
> > Commit c910e2e says:
> >
> > dm: Avoid accessing uclasses before they are ready
> >
> > Don't allow access to uclasses before they have been initialised.
> >
> >
> >
> > I still don't get it.
> > The log did not help me because it is not saying 'why'.
> >
> > What kind problem would happen if this check was dropped?
> >
> > gd->dm_root is set when the root device is bound.
> > At the first call of uclass_find(), it is true gd->dm_root is NULL,
> > but gd->uclass_root is also empty.
>
> But 'empty' means the list is empty. For it to be empty it must be
> initialised. But if you call the function before this list init
> happens, then it will just crash.
>
> We can use dm_root as an indicator that init has happened.
>
> >
> > This function, anyway, returns NULL.
> > The behavior is still the same, I think.
>
> You can try this by removing that code and seeing what the test does
> (dm_test_uclass_before_ready()).
>
So, you are checking gd->dm_root
to make sure gd->uclass_root has been initialized.
Understood what you are trying to do, but it is unfortunate.
OK, let's see what will happen if uclass_get() is called before dm_init().
It is true uclass_find() will return NULL safely,
but uclass_add() will crash, I think.
It looks like uclass_add() is trying to add a created uclass
to the uninitialized gd->uclass_root.
here,
list_add(&uc->sibling_node, &DM_UCLASS_ROOT_NON_CONST);
I think it is as dangeous as searching something in an uninitialized list.
Unfortunately, we cannot use LIST_HEAD(uclass_root)
because it is in the global data.
Best Regards
Masahiro Yamada
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
