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