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

[Sean Anderson <seanga2@gmail.com>]

On 9/16/20 9:09 PM, Simon Glass wrote:
> Hi Sean,
> 
> On Sat, 12 Sep 2020 at 15:46, Sean Anderson <seanga2@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@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?

--Sean