Hi Julien, On Mon, 17 Aug 2026 at 01:33, Julien Stephan <[email protected]> wrote: > > Le ven. 7 août 2026 à 21:44, Simon Glass <[email protected]> a écrit : > > > > Hi Julien, > > > > On Fri, 7 Aug 2026 at 09:33, Julien Stephan <[email protected]> wrote: > > > > > > Le jeu. 6 août 2026 à 19:01, Simon Glass <[email protected]> a écrit : > > > > > > > > Hi Julien, > > > > > > > > On 2026-08-06T13:05:19, Julien Stephan <[email protected]> wrote: > > > > > dm: improve logging for missing uclass > > > > > > > > > > When a uclass definition is missing for an enabled driver, the board > > > > > cannot boot, and without any extra debug option enabled the following > > > > > error is displayed: > > > > > > > > > > initcall_run_r(): initcall initr_dm() failed ### ERROR ### Please > > > > > RESET the board ### > > > > > > > > > > There is a debug message using dm_warn(), which is not displayed by > > > > > default. Since this is a fatal error preventing the board from > > > > > booting, > > > > > the log level should be at least ERROR. > > > > > > > > > > Signed-off-by: Julien Stephan <[email protected]> > > > > > > > > > > drivers/core/uclass.c | 5 +++-- > > > > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > > > > > > diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c > > > > > @@ -60,8 +60,9 @@ static int uclass_add(enum uclass_id id, struct > > > > > uclass **ucp) > > > > > *ucp = NULL; > > > > > uc_drv = lists_uclass_lookup(id); > > > > > if (!uc_drv) { > > > > > - dm_warn("Cannot find uclass for id %d: please add the > > > > > UCLASS_DRIVER() declaration for this UCLASS_... id\n", > > > > > - id); > > > > > + log(LOGC_DM, LOGL_ERR, > > > > > + "Cannot find uclass for id %d: please add the > > > > > UCLASS_DRIVER() declaration for this UCLASS_... id\n", > > > > > + id); > > > > > > > > This file sets LOG_CATEGORY to LOGC_DM at the top, so log_err() would > > > > be cleaner: > > > > > > > > log_err("Cannot find uclass for id %d: please add the > > > > UCLASS_DRIVER() declaration for this UCLASS_... id\n", > > > > id); > > > > > > > > The idea with this was that by returning -EPFNOSUPPORT (unused > > > > elsewhere in U-Boot (he says...)) it would indicate this problem, > > > > without bloating the code with a large message. This code is used in > > > > SPL where code size is at a premium. I don't see the error number > > > > though, which I added here: > > > > > > > > 13123276806 initcall: Adjust the failure message and return value > > > > > > > > Hmm yes it was dropped in the initcall move back to the header file. > > > > > > > > We already have DM_WARN to help people with debugging, so perhaps > > > > instead we should just enable that by default in U-Boot proper? > > > > > > > > > > Hi Simon, > > > > > > I didn't think about the impact on SPL. I understand your point, but I > > > do feel a fatal error deserves a proper message. > > > > > > So what about reducing this long message to something shorter, using > > > log_err() as you suggested: > > > > > > log_err("uclass %d: no UCLASS_DRIVER()\n", id); > > > > > > That keeps a readable, greppable hint while cutting most of the SPL > > > size overhead. > > > > > > I didn't audit all the dm_warn() callers, but enabling DM_WARN by > > > default feels overkill. I suspect several of them are > > > genuinely non-fatal warnings. > > > > > > Otherwise, we could just drop this patch, since patch 2 now prints the > > > -EPFNOSUPPORT value on failure. > > > > > > What do you think? > > > > I tend to agree that a message would be better. But any SPL growth is > > painful...perhaps we could have a Kconfig to control just this case > > (default y in TPL/SPL/Proper) so people can turn it off for > > production? > > Hi Simon, > > Sorry for my late reply, I was on vacation last week. > > I've been thinking about this, and here is my suggestion for you to > review before I send a v2 (not tested yet): convert DM_WARN into a > choice with DM_NONE / DM_ERR / DM_WARN to control the verbosity of > driver model logs. Something like: > > drivers/core/Kconfig (same for the SPL/TPL variants): > > kconfig > choice > prompt "Driver model log verbosity" > depends on DM > default DM_ERR > help > Select which driver model messages are compiled into the binary. > Each level includes the levels below it. Lower verbosity saves code > size by compiling the message strings out (useful for production > builds), while higher verbosity helps with debugging. > > config DM_NONE > bool "No messages" > help > Compile out all driver model messages. This gives the smallest code > size, but a fatal condition gives no hint about its root cause. > > config DM_ERR > bool "Errors" > help > Emit driver model error messages via dm_err(), at ERROR level. > These report fatal conditions that typically prevent the board from > booting. Warnings are compiled out. > > config DM_WARN > bool "Warnings and errors" > help > Emit driver model warnings via dm_warn() (WARNING level) in addition > to errors. This is the most verbose and uses the most code space. > > Warnings may help with debugging, such as when expected devices do > not bind correctly. > > endchoice > > include/dm/util.h: > > -#if CONFIG_IS_ENABLED(DM_WARN) > +#if CONFIG_IS_ENABLED(DM_WARN) /* warnings and errors */ > #define dm_warn(fmt...) log(LOGC_DM, LOGL_WARNING, ##fmt) > -#else > +#define dm_err(fmt...) log(LOGC_DM, LOGL_ERR, ##fmt) > +#elif CONFIG_IS_ENABLED(DM_ERR) /* errors only */ > +#define dm_warn(fmt...) log(LOGC_DM, LOGL_DEBUG, ##fmt) > +#define dm_err(fmt...) log(LOGC_DM, LOGL_ERR, ##fmt) > +#else /* no messages */ > #define dm_warn(fmt...) log(LOGC_DM, LOGL_DEBUG, ##fmt) > +#define dm_err(fmt...) log(LOGC_DM, LOGL_DEBUG, ##fmt) > #endif > > This gives a reusable dm_err() mechanism, and existing dm_warn() > callers keep working: nothing selects DM_WARN, there is no #ifdef > CONFIG_DM_WARN in C (only the CONFIG_IS_ENABLED() above), and a choice > member named DM_WARN is still =y when selected. Only one defconfig > sets CONFIG_DM_WARN=y today, and it still resolves fine (I can refresh > it as part of a v2). > > One deliberate change: with default DM_ERR, driver model errors are > now emitted by default, whereas today they are silent unless DM_WARN > is enabled. > > While working on this I also noticed that a DM_DEBUG symbol already > exists, and it works completely differently: it adds -DDEBUG directly > in drivers/core/Makefile (proper only), so it's orthogonal to which > log strings get compiled in. My inclination is to leave DM_DEBUG as-is > for now and keep this series focused on the NONE/ERR/WARN choice; > folding it in as a top "debug" tier (and adding a dm_dbg()) could be a > follow-up, since it would change DM_DEBUG's scope and touch the > Makefile. Note that DM_DEBUG and DM_WARN also couldn't both be > selected if it became a choice member. > > So, what do you think? Is the DM_NONE / DM_ERR / DM_WARN choice the > right direction? And does leaving DM_DEBUG out of it for now (as a > possible follow-up) sound reasonable, or would you rather see it > unified into the choice from the start?
This seems good to me. You might be able to de-duplicate your code to just select which LOGL value to use, with your dm_warn/err() macros then using that. Regards, Simon
