device_bind() with ofnode_null() tripping assert()?

[Jonas Karlman]

Hi Quentin,

On 2024-10-16 13:25, Quentin Schulz wrote:
> Hi all,
> 
> Not sure how to word this better so here goes nothing.
> 
> I recently wanted to debug some issues possibly related to DM which had 
> a bunch of debug() log messages I wanted to see. To see those, one needs 
> to define DEBUG. Once that's done, assert() calls are now built in.
> 
> It turns out that this means I cannot boot my board anymore because (at 
> least) one of the assert() fails (specifically an ofnode_valid()).
> 
> I've for now narrowed the one in SPL to dev_read_u32(protocol, 
> "arm,smc-id", &func_id) in smccc_agent.c, c.f. 
> https://elixir.bootlin.com/u-boot/v2024.10/source/drivers/firmware/scmi/smccc_agent.c#L91
> If I change ofnode_null() with ofnode_root() in the device_bind() 
> creating the scmi-base.0 device from the SCMI UCLASS, then it goes 
> further, c.f. 
> https://elixir.bootlin.com/u-boot/v2024.10/source/drivers/firmware/scmi/scmi_agent-uclass.c#L377
> 
> However, I then get another assert() being tripped later in U-Boot 
> proper (right after the Ethernet has been initialized and before I get 
> to the shell, no idea if it actually is related to Ethernet), which 
> makes it reset again.
> 
> So... I basically have no clue how to go forward with this. We've 
> started to discuss to decouple debug() log messages from assert() 
> (meaning they don't share the same knob) but that wouldn't fix the 
> issue. Should this assert() really be tripping? If not, how should we 
> handle that instead?

I have been hit with the such issue in lib/smbios.c due to assert().
Not sure if this is a correct fix or a bad workaround.

One thing that could make it easier to get a clue on what is causing the
assert() is to ensure debug/warn log happens before the assert() in e.g.
drivers/core/ofnode.c.

Have also seen similar ofnode related assert() reset issue in the past
with BOOTMETH_VBE enabled, due to a missing node or props.

diff --git a/lib/smbios.c b/lib/smbios.c
index 7c24ea129eb5..4aa8829a283d 100644
--- a/lib/smbios.c
+++ b/lib/smbios.c
@@ -7,6 +7,7 @@
 
 #define LOG_CATEGORY	LOGC_BOARD
 
+#include <display_options.h>
 #include <dm.h>
 #include <env.h>
 #include <linux/stringify.h>
@@ -630,7 +631,7 @@ ulong write_smbios_table(ulong addr)
 		ctx.subnode_name = NULL;
 		if (method->subnode_name) {
 			ctx.subnode_name = method->subnode_name;
-			if (IS_ENABLED(CONFIG_OF_CONTROL))
+			if (IS_ENABLED(CONFIG_OF_CONTROL) && ofnode_valid(parent_node))
 				ctx.node = ofnode_find_subnode(parent_node,
 							       method->subnode_name);
 		}

Regards,
Jonas

> 
> Cheers,
> Quentin