Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] dt-bindings: embedding raw image payloads in DT violates design principles - [Low] dt-bindings: logo-rotation uses non-standard string enum instead of integer --
commit 7c49827223c484baa96e766e2d19b7e319290579 Author: Max Pedraza <[email protected]> dt-bindings: display: add a device tree supplied boot logo This commit introduces a device tree binding for providing a boot logo from firmware, placing a "boot-logo-clut224" node under /chosen. It defines properties for dimensions, color lookup table, image data, positioning, and rotation. > diff --git a/Documentation/devicetree/bindings/display/boot-logo-clut224.yaml > b/Documentation/devicetree/bindings/display/boot-logo-clut224.yaml > new file mode 100644 > index 0000000000000..9dc34717639e9 > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/boot-logo-clut224.yaml > @@ -0,0 +1,135 @@ [ ... ] > + clut: > + $ref: /schemas/types.yaml#/definitions/uint8-array > + description: > + Colour lookup table, as consecutive red, green and blue bytes per > entry. > + The number of entries is derived from the property length and must not > + exceed 224. > + minItems: 3 > + maxItems: 672 > + > + data: > + $ref: /schemas/types.yaml#/definitions/uint8-array > + description: > + One byte per pixel, left to right and top to bottom, each byte being an > + index into the colour lookup table. The property length must be equal > to > + width multiplied by height. [Severity: Medium] Does this code introduce a regression by allowing large binary payloads to be embedded directly in the device tree? The Device Tree is intended for hardware description and minimal configuration, not for delivering large binary files. Embedding an uncompressed image directly via the 'data' property can permanently waste un-reclaimable kernel memory, as the unflattened DT remains in memory indefinitely. It also risks exceeding bootloader DTB size limits. While follow-up work might provide a 'memory-region' alternative, the schema still explicitly permits and supports embedding the binary payload directly via this property. [ ... ] > + logo-offset: > + $ref: /schemas/types.yaml#/definitions/int32-array > + description: > + X and Y displacement, in screen pixels, applied after the logo has been > + placed. > + Mostly useful together with a centred axis, to land the logo somewhere > + other than the middle of a panel whose usable area is not its centre. > + items: > + - description: X displacement > + minimum: -65535 > + maximum: 65535 > + - description: Y displacement > + minimum: -65535 > + maximum: 65535 > + > + logo-rotation: > + $ref: /schemas/types.yaml#/definitions/string > + description: > + Rotation applied to the logo before it is drawn. It turns the logo and > + not the screen, so a quarter turn swaps how much room the logo takes > + up, but logo-position and logo-offset stay in screen pixels either > + way. > + enum: [none, cw, ccw, ud] > + default: none [Severity: Low] Is this non-standard string enum a regression from established device tree conventions? Using a custom string enum ('none', 'cw', 'ccw', 'ud') violates the established convention for rotation. Existing bindings like panel-common.yaml and video-interface-devices.yaml dictate using a standard integer 'rotation' property with degree values (0, 90, 180, 270). This custom format fragments parsing logic in drivers. [ ... ] -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
