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

Sean Anderson
2020-09-12T21:45:42Z

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(-)

diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c
index c3f1b73cd6..2e098034c9 100644
--- a/drivers/core/uclass.c
+++ b/drivers/core/uclass.c
@@ -19,6 +19,7 @@
#include <dm/uclass.h>
#include <dm/uclass-internal.h>
#include <dm/util.h>
+#include <linux/err.h>

DECLARE_GLOBAL_DATA_PTR;

@@ -27,7 +28,7 @@ struct uclass *uclass_find(enum uclass_id key)
struct uclass *uc;

if (!gd->dm_root)
- return NULL;
+ return ERR_PTR(-EAGAIN);
/*
* TODO(sjg at chromium.org): Optimise this, perhaps moving the found
* node to the start of the list, or creating a linear array mapping
@@ -144,6 +145,19 @@ int uclass_get(enum uclass_id id, struct uclass **ucp)

*ucp = NULL;
uc = uclass_find(id);
+ if (IS_ERR(uc)) {
+ /*
+ * If we're getting the root uclass, then uclass_find will fail
+ * with -EAGAIN (since it thinks there's no uclass list to
+ * search), but we need to add it anyway (since otherwise it
+ * would never be created).
+ */
+ if (PTR_ERR(uc) == -EAGAIN && id == UCLASS_ROOT)
+ uc = NULL;
+ else
+ return PTR_ERR(uc);
+ }
+
if (!uc)
return uclass_add(id, ucp);
*ucp = uc;
diff --git a/test/dm/core.c b/test/dm/core.c
index 8ed5bf7370..a4e0ac06f9 100644
--- a/test/dm/core.c
+++ b/test/dm/core.c
@@ -733,7 +733,7 @@ static int dm_test_uclass_before_ready(struct unit_test_state *uts)
gd->dm_root_f = NULL;
memset(&gd->uclass_root, '\0', sizeof(gd->uclass_root));

- ut_asserteq_ptr(NULL, uclass_find(UCLASS_TEST));
+ ut_asserteq_ptr(ERR_PTR(-EAGAIN), uclass_find(UCLASS_TEST));

return 0;
}
diff --git a/test/dm/test-main.c b/test/dm/test-main.c
index 38b7b1481a..6ff07a5d32 100644
--- a/test/dm/test-main.c
+++ b/test/dm/test-main.c
@@ -70,7 +70,7 @@ static int dm_test_destroy(struct unit_test_state *uts)
* check that here before we call uclass_find_device().
*/
uc = uclass_find(id);
- if (!uc)
+ if (IS_ERR_OR_NULL(uc))
continue;
ut_assertok(uclass_destroy(uc));
}
--
2.28.0