On Mon, Mar 25, 2019 at 8:54 AM Maxime Ripard <maxime.ripard@xxxxxxxxxxx> wrote: > > The simple framebuffer is a binding that allows the bootloader to setup a > framebuffer, describe it in the Device Tree for the OS to pick it up and > use it as is. > > Replace the current binding by a schema to allow the validation tools to > check those nodes. > > Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@xxxxxxxxxxx> > Cc: Chen-Yu Tsai <wens@xxxxxxxx> > Cc: Hans de Goede <hdegoede@xxxxxxxxxx> > Cc: Maxime Jourdan <mjourdan@xxxxxxxxxxxx> > Signed-off-by: Maxime Ripard <maxime.ripard@xxxxxxxxxxx> > > --- > > Rob, > > Even though the node itself is properly described, I still end up with some > validation warnings when the chosen node is validated (basically > complaining that ranges, and the framebuffer nodes shouldn't be here, since > we haven't described them in the chosen schemas). Having 'ranges' is a bit strange because 'chosen' doesn't have an address. We can add #address-cells, #size-cells and framebuffer child node to the base chosen schema. > I've tried to expand it, but I failed, using that patch: > http://code.bulix.org/mimu5z-634661?raw That should work, but you will still get warnings from the core schema. Also, I guess you really want to match on the child compatible which isn't possible with the current tools. I think for now at least, we don't need to validate that simple-fb is a child of chosen. Maybe we could add a '$parent' property which the tools add like $nodename. I'd rather wait and see how frequently we need something like this. > I'm a bit lost right now about how to make those nodes being validated and > not report any warning anymore. I guess one way would be to expand the > chosen schemas instead. Let me know what you prefer. > --- > .../display/amlogic,simple-framebuffer.txt | 33 ---- > .../display/simple-framebuffer-sunxi.txt | 36 ---- > .../bindings/display/simple-framebuffer.txt | 91 ---------- > .../bindings/display/simple-framebuffer.yaml | 164 ++++++++++++++++++ > 4 files changed, 164 insertions(+), 160 deletions(-) > delete mode 100644 Documentation/devicetree/bindings/display/amlogic,simple-framebuffer.txt > delete mode 100644 Documentation/devicetree/bindings/display/simple-framebuffer-sunxi.txt > delete mode 100644 Documentation/devicetree/bindings/display/simple-framebuffer.txt > create mode 100644 Documentation/devicetree/bindings/display/simple-framebuffer.yaml [...] > diff --git a/Documentation/devicetree/bindings/display/simple-framebuffer.yaml b/Documentation/devicetree/bindings/display/simple-framebuffer.yaml > new file mode 100644 > index 000000000000..9f2245e3f5ba > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/simple-framebuffer.yaml > @@ -0,0 +1,164 @@ > +# SPDX-License-Identifier: (GPL-2.0+ OR BSD-2-Clause) Keep in mind the old files were implicitly GPL2 only. I imagine some of this is copied. Also, as gregkh says, are you sure you are okay with GPLv9? > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/simple-framebuffer.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Simple Framebuffer Device Tree Bindings > + > +maintainers: > + - Bartlomiej Zolnierkiewicz <b.zolnierkie@xxxxxxxxxxx> > + - Hans de Goede <hdegoede@xxxxxxxxxx> > + > +description: |+ > + A simple frame-buffer describes a frame-buffer setup by firmware or > + the bootloader, with the assumption that the display hardware has > + already been set up to scan out from the memory pointed to by the > + reg property. > + > + Since simplefb nodes represent runtime information they must be > + sub-nodes of the chosen node (*). Simplefb nodes must be named > + framebuffer@<address>. > + > + If the devicetree contains nodes for the display hardware used by a > + simplefb, then the simplefb node must contain a property called > + display, which contains a phandle pointing to the primary display > + hw node, so that the OS knows which simplefb to disable when handing > + over control to a driver for the real hardware. The bindings for the > + hw nodes must specify which node is considered the primary node. > + > + It is advised to add display# aliases to help the OS determine how > + to number things. If display# aliases are used, then if the simplefb > + node contains a display property then the /aliases/display# path > + must point to the display hw node the display property points to, > + otherwise it must point directly to the simplefb node. > + > + If a simplefb node represents the preferred console for user > + interaction, then the chosen node stdout-path property should point > + to it, or to the primary display hw node, as with display# > + aliases. If display aliases are used then it should be set to the > + alias instead. > + > + It is advised that devicetree files contain pre-filled, disabled > + framebuffer nodes, so that the firmware only needs to update the > + mode information and enable them. This way if e.g. later on support > + for more display clocks get added, the simplefb nodes will already > + contain this info and the firmware does not need to be updated. > + > + If pre-filled framebuffer nodes are used, the firmware may need > + extra information to find the right node. In that case an extra > + platform specific compatible and platform specific properties should > + be used and documented. > + > +properties: > + compatible: > + oneOf: > + - items: > + - const: allwinner,simple-framebuffer > + - const: simple-framebuffer > + > + - items: > + - const: amlogic,simple-framebuffer > + - const: simple-framebuffer I'd write this as: - enum: - allwinner,simple-framebuffer - amlogic,simple-framebuffer - const: simple-framebuffer > + > + - const: simple-framebuffer > + > + reg: > + description: Location and size of the framebuffer memory > + > + clocks: > + description: List of clocks used by the framebuffer. > + > + power-domains: > + description: List of power domains used by the framebuffer. > + > + width: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: Width of the framebuffer in pixels > + > + height: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: Height of the framebuffer in pixels > + > + stride: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: Number of bytes of a line in the framebuffer > + > + format: > + description: Format of the framebuffer > + oneOf: > + - const: r5g6b5 > + description: 16-bit pixels, d[15:11]=r, d[10:5]=g, d[4:0]=b > + - const: a8b8g8r8 > + description: 32-bit pixels, d[31:24]=a, d[23:16]=b, d[15:8]=g, d[7:0]=r This can be an enum rather than oneOf and a bunch of const. > + > + display: > + $ref: /schemas/types.yaml#/definitions/phandle > + description: Primary display hardware node > + > + allwinner,pipeline: > + description: Pipeline used by the framebuffer on Allwinner SoCs > + oneOf: > + - const: de_be0-lcd0 > + - const: de_be0-lcd0-hdmi > + - const: de_be0-lcd0-tve0 > + - const: de_be1-lcd0 > + - const: de_be1-lcd1-hdmi > + - const: de_fe0-de_be0-lcd0 > + - const: de_fe0-de_be0-lcd0-hdmi > + - const: de_fe0-de_be0-lcd0-tve0 > + - const: mixer0-lcd0 > + - const: mixer0-lcd0-hdmi > + - const: mixer1-lcd1-hdmi > + - const: mixer1-lcd1-tve Same here. > + > + amlogic,pipeline: > + description: Pipeline used by the framebuffer on Amlogic SoCs > + oneOf: > + - const: vpu-cvbs > + - const: vpu-hdmi Same here. > + > +patternProperties: > + "^[a-zA-Z0-9-]+-supply$": > + $ref: /schemas/types.yaml#/definitions/phandle > + description: > + Regulators used by the framebuffer. These should be named > + according to the names in the device design. > + > +required: > + # The binding requires also reg, width, height, stride and format, > + # but usually they will be filled by the bootloader. > + - compatible > + > +additionalProperties: false > + > +examples: > + - | > + aliases { > + display0 = &lcdc0; > + }; > + > + chosen { > + #address-cells = <1>; > + #size-cells = <1>; > + stdout-path = "display0"; > + framebuffer0: framebuffer@1d385000 { > + compatible = "simple-framebuffer"; > + reg = <0x1d385000 3840000>; > + width = <1600>; > + height = <1200>; > + stride = <3200>; > + format = "r5g6b5"; > + clocks = <&ahb_gates 36>, <&ahb_gates 43>, <&ahb_gates 44>; > + lcd-supply = <®_dc1sw>; > + display = <&lcdc0>; > + }; > + }; > + > + soc@1c00000 { > + lcdc0: lcdc@1c0c000 { > + compatible = "allwinner,sun4i-a10-lcdc"; > + }; > + }; > + > +... > -- > 2.20.1 >
