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

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