Hi Mark, On Mon, 7 Feb 2022 at 14:20, Mark Kettenis <mark.kette...@xs4all.nl> wrote: > > > From: Simon Glass <s...@chromium.org> > > Date: Mon, 7 Feb 2022 13:22:22 -0700 > > > > Hi Mark, > > > > On Sat, 5 Feb 2022 at 16:10, Mark Kettenis <kette...@openbsd.org> wrote: > > > > > > The stdout-path property in the device tree does not necessarily > > > point at a serial device. The code that binds the device if it > > > isn't marked to be bound before relocation does not check whether > > > the device really is a serial device. So check that its uclass is > > > UCLASS_SERIAL before probing it. > > > > > > Signed-off-by: Mark Kettenis <kette...@openbsd.org> > > > --- > > > drivers/serial/serial-uclass.c | 3 ++- > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > This would be better as an assert() to avoid code bloat. Under what > > circumstances was this wrong? > > I'm passing a debive tree to U-Boot that has stdout-path set to > "/chosen/framebuffer" to indicate that output should go to the > framebuffer instead of the serial port. This is a node with a > "simple-framebuffer" compatible, which means the list_bind_fdt() call > binds the simple_video (simplefb.c) driver and stores a pointer to its > struct udevice into devp. I've verified that the uclass is indeed > UCLASS_VIDEO at that point with my device tree. > > Since the function returns 0, the caller of this function thinks we > returned a valid UCLASS_SERIAL device and when u-boot starts using it > as such it crashes (without printing anything since there is no > console output device yet). > > So an assert won't work. We should simply return an error an run the > fallback code below that looks for the serial port with the specified > index. That way U-Boot continues to work as expected (with output on > both the serial port and the framebuffer). But once my OS is running, > it knows to pick the framebuffer console and all's fine. > > I haven't looked at the binary growth, but it can't be much. But if > you think it is a problem, maybe we should make this code that binds > the console driver optional?
OK I see. It likely isn't much, but in SPL it is still growth. Worth checking. Can you perhaps create a 'console' alias so that the line above finds the correct one? But I'm a bit unsure of how you get the serial console to work if serial_check_stdout() fails? Is the serial driver not marked for pre-reloc use? There is something odd going on. Regards, Simon