From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sean Anderson Date: Wed, 16 Sep 2020 21:44:26 -0400 Subject: [PATCH 1/3] dev: Disambiguate errors in uclass_find In-Reply-To: References: <20200912214544.362594-1-seanga2@gmail.com> <20200912214544.362594-2-seanga2@gmail.com> Message-ID: <9164d6ac-7d85-a192-e0b8-649584e3420c@gmail.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 9/16/20 9:09 PM, Simon Glass wrote: > Hi Sean, > > On Sat, 12 Sep 2020 at 15:46, Sean Anderson 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 >> --- >> >> 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