Re: [PATCH v2 2/6] video: logo: allow the boot logo to come from the device tree

[Màxim Pedraza Padilla]

El sáb, 8 ago 2026 a las 11:44, Helge Deller (<deller@xxxxxxxxxx>) escribió:
> This patch feels "stamped in" into the existing code.
> I do understand that it is by itself clean, but maybe merging it
> better with the existing code makes sense.

You were right. The placement is now a patch of its own at the front of the
series, with no device tree in it:

  struct fb_logo_placement {
s32 x, y; /* -1 centres on that axis */
s32 offset_x, offset_y;
  };

fb_center_logo is exactly {-1, -1}, so it stops being a second path, and the
device tree patch only fills the same structure in.

It also turned up a bug. fb_prepare_logo() and fb_show_logo_line() each open
coded the same decision, and my patch had made them disagree: the reservation
took the larger of the console position and the device tree one, the drawing
took only the device tree one. Both call one function now.

> why so many fb_logo_dt_read() calls?

Down to one, in fb_prepare_logo(). fb_show_logo() is only reached through
logo_shown == FBCON_LOGO_DRAW, which fbcon_prepare_logo() sets.

> I usually prefer if people use the IS_ENABLED(CONFIG_XXX) macro instead
> and put it at specific entry places

Done in both files, following your example.

Checked with CONFIG_OF=n on x86_64: neither object is left with an unresolved
of_* symbol, and the structure is dropped from .bss. That last part needed a
second IS_ENABLED() in an accessor that read it unguarded, or the compiler
could not prove the read dead. Also built a Cell config, where SPU_BASE makes
CONFIG_FB_LOGO_EXTRA real, to a linked vmlinux.

logo.c was the other file with an #ifdef around it. The rest of the series is
the binding, the reserved memory parsing and the host tool, none of it
conditional, so there was nothing else to convert.

One thing the reread turned up: fb_logo_axis() added a coordinate and an
offset, both straight from the device tree, in int, and the binding bounded
neither. A large pair wrapped instead of landing against an edge. It is done
in 64 bits now and both properties are bounded.

The series needs this, or dtbs_check rejects the node wherever it appears:

  https://github.com/devicetree-org/dt-schema/pull/204

Max