Re: [PATCH 1/2] dm: improve logging for missing uclass

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?

Cheers,
Julien



>
> Regards,
> SImon