[PATCH] dt-bindings: Add schemas for simple-framebuffer

[PATCH] dt-bindings: Add schemas for simple-framebuffer

[Maxime Ripard]

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@samsung.com>
Cc: Chen-Yu Tsai <wens@csie.org>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Maxime Jourdan <mjourdan@baylibre.com>
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>

---

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).

I've tried to expand it, but I failed, using that patch:
http://code.bulix.org/mimu5z-634661?raw

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/amlogic,simple-framebuffer.txt b/Documentation/devicetree/bindings/display/amlogic,simple-framebuffer.txt
deleted file mode 100644
index aaa6c24c8e70..000000000000
--- a/Documentation/devicetree/bindings/display/amlogic,simple-framebuffer.txt
+++ /dev/null
@@ -1,33 +0,0 @@
-Meson specific Simple Framebuffer bindings
-
-This binding documents meson specific extensions to the simple-framebuffer
-bindings. The meson simplefb u-boot code relies on the devicetree containing
-pre-populated simplefb nodes.
-
-These extensions are intended so that u-boot can select the right node based
-on which pipeline is being used. As such they are solely intended for
-firmware / bootloader use, and the OS should ignore them.
-
-Required properties:
-- compatible: "amlogic,simple-framebuffer", "simple-framebuffer"
-- amlogic,pipeline, one of:
-  "vpu-cvbs"
-  "vpu-hdmi"
-
-Example:
-
-chosen {
-	#address-cells = <2>;
-	#size-cells = <2>;
-	ranges;
-
-	simplefb_hdmi: framebuffer-hdmi {
-		compatible = "amlogic,simple-framebuffer",
-			     "simple-framebuffer";
-		amlogic,pipeline = "vpu-hdmi";
-		clocks = <&clkc CLKID_HDMI_PCLK>,
-			 <&clkc CLKID_CLK81>,
-			 <&clkc CLKID_GCLK_VENCI_INT0>;
-		power-domains = <&pwrc_vpu>;
-	};
-};
diff --git a/Documentation/devicetree/bindings/display/simple-framebuffer-sunxi.txt b/Documentation/devicetree/bindings/display/simple-framebuffer-sunxi.txt
deleted file mode 100644
index d693b8dc9a62..000000000000
--- a/Documentation/devicetree/bindings/display/simple-framebuffer-sunxi.txt
+++ /dev/null
@@ -1,36 +0,0 @@
-Sunxi specific Simple Framebuffer bindings
-
-This binding documents sunxi specific extensions to the simple-framebuffer
-bindings. The sunxi simplefb u-boot code relies on the devicetree containing
-pre-populated simplefb nodes.
-
-These extensions are intended so that u-boot can select the right node based
-on which pipeline is being used. As such they are solely intended for
-firmware / bootloader use, and the OS should ignore them.
-
-Required properties:
-- compatible: "allwinner,simple-framebuffer"
-- allwinner,pipeline, one of:
-  "de_be0-lcd0"
-  "de_be1-lcd1"
-  "de_be0-lcd0-hdmi"
-  "de_be1-lcd1-hdmi"
-  "mixer0-lcd0"
-  "mixer0-lcd0-hdmi"
-  "mixer1-lcd1-hdmi"
-  "mixer1-lcd1-tve"
-
-Example:
-
-chosen {
-	#address-cells = <1>;
-	#size-cells = <1>;
-	ranges;
-
-	framebuffer@0 {
-		compatible = "allwinner,simple-framebuffer", "simple-framebuffer";
-		allwinner,pipeline = "de_be0-lcd0-hdmi";
-		clocks = <&pll5 1>, <&ahb_gates 36>, <&ahb_gates 43>,
-			 <&ahb_gates 44>;
-	};
-};
diff --git a/Documentation/devicetree/bindings/display/simple-framebuffer.txt b/Documentation/devicetree/bindings/display/simple-framebuffer.txt
deleted file mode 100644
index 5a9ce511be88..000000000000
--- a/Documentation/devicetree/bindings/display/simple-framebuffer.txt
+++ /dev/null
@@ -1,91 +0,0 @@
-Simple Framebuffer
-
-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's 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,
-see e.g. simple-framebuffer-sunxi.txt .
-
-Required properties:
-- compatible: "simple-framebuffer"
-- reg: Should contain the location and size of the framebuffer memory.
-- width: The width of the framebuffer in pixels.
-- height: The height of the framebuffer in pixels.
-- stride: The number of bytes in each line of the framebuffer.
-- format: The format of the framebuffer surface. Valid values are:
-  - r5g6b5 (16-bit pixels, d[15:11]=r, d[10:5]=g, d[4:0]=b).
-  - a8b8g8r8 (32-bit pixels, d[31:24]=a, d[23:16]=b, d[15:8]=g, d[7:0]=r).
-
-Optional properties:
-- clocks : List of clocks used by the framebuffer.
-- *-supply : Any number of regulators used by the framebuffer. These should
-	     be named according to the names in the device's design.
-
-  The above resources are expected to already be configured correctly.
-  The OS must ensure they are not modified or disabled while the simple
-  framebuffer remains active.
-
-- display : phandle pointing to the primary display hardware node
-
-Example:
-
-aliases {
-	display0 = &lcdc0;
-}
-
-chosen {
-	framebuffer0: framebuffer@1d385000 {
-		compatible = "simple-framebuffer";
-		reg = <0x1d385000 (1600 * 1200 * 2)>;
-		width = <1600>;
-		height = <1200>;
-		stride = <(1600 * 2)>;
-		format = "r5g6b5";
-		clocks = <&ahb_gates 36>, <&ahb_gates 43>, <&ahb_gates 44>;
-		lcd-supply = <&reg_dc1sw>;
-		display = <&lcdc0>;
-	};
-	stdout-path = "display0";
-};
-
-soc@1c00000 {
-	lcdc0: lcdc@1c0c000 {
-		compatible = "allwinner,sun4i-a10-lcdc";
-		...
-	};
-};
-
-
-*) Older devicetree files may have a compatible = "simple-framebuffer" node
-in a different place, operating systems must first enumerate any compatible
-nodes found under chosen and then check for other compatible nodes.
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)
+%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@samsung.com>
+  - Hans de Goede <hdegoede@redhat.com>
+
+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
+
+      - 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
+
+  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
+
+  amlogic,pipeline:
+    description: Pipeline used by the framebuffer on Amlogic SoCs
+    oneOf:
+      - const: vpu-cvbs
+      - const: vpu-hdmi
+
+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 = <&reg_dc1sw>;
+        display = <&lcdc0>;
+      };
+    };
+
+    soc@1c00000 {
+      lcdc0: lcdc@1c0c000 {
+        compatible = "allwinner,sun4i-a10-lcdc";
+      };
+    };
+
+...
-- 
2.20.1

[PATCH] dt-bindings: Add schemas for simple-framebuffer

[Maxime Ripard]

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@samsung.com>
Cc: Chen-Yu Tsai <wens@csie.org>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Maxime Jourdan <mjourdan@baylibre.com>
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>

---

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).

I've tried to expand it, but I failed, using that patch:
http://code.bulix.org/mimu5z-634661?raw

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/amlogic,simple-framebuffer.txt b/Documentation/devicetree/bindings/display/amlogic,simple-framebuffer.txt
deleted file mode 100644
index aaa6c24c8e70..000000000000
--- a/Documentation/devicetree/bindings/display/amlogic,simple-framebuffer.txt
+++ /dev/null
@@ -1,33 +0,0 @@
-Meson specific Simple Framebuffer bindings
-
-This binding documents meson specific extensions to the simple-framebuffer
-bindings. The meson simplefb u-boot code relies on the devicetree containing
-pre-populated simplefb nodes.
-
-These extensions are intended so that u-boot can select the right node based
-on which pipeline is being used. As such they are solely intended for
-firmware / bootloader use, and the OS should ignore them.
-
-Required properties:
-- compatible: "amlogic,simple-framebuffer", "simple-framebuffer"
-- amlogic,pipeline, one of:
-  "vpu-cvbs"
-  "vpu-hdmi"
-
-Example:
-
-chosen {
-	#address-cells = <2>;
-	#size-cells = <2>;
-	ranges;
-
-	simplefb_hdmi: framebuffer-hdmi {
-		compatible = "amlogic,simple-framebuffer",
-			     "simple-framebuffer";
-		amlogic,pipeline = "vpu-hdmi";
-		clocks = <&clkc CLKID_HDMI_PCLK>,
-			 <&clkc CLKID_CLK81>,
-			 <&clkc CLKID_GCLK_VENCI_INT0>;
-		power-domains = <&pwrc_vpu>;
-	};
-};
diff --git a/Documentation/devicetree/bindings/display/simple-framebuffer-sunxi.txt b/Documentation/devicetree/bindings/display/simple-framebuffer-sunxi.txt
deleted file mode 100644
index d693b8dc9a62..000000000000
--- a/Documentation/devicetree/bindings/display/simple-framebuffer-sunxi.txt
+++ /dev/null
@@ -1,36 +0,0 @@
-Sunxi specific Simple Framebuffer bindings
-
-This binding documents sunxi specific extensions to the simple-framebuffer
-bindings. The sunxi simplefb u-boot code relies on the devicetree containing
-pre-populated simplefb nodes.
-
-These extensions are intended so that u-boot can select the right node based
-on which pipeline is being used. As such they are solely intended for
-firmware / bootloader use, and the OS should ignore them.
-
-Required properties:
-- compatible: "allwinner,simple-framebuffer"
-- allwinner,pipeline, one of:
-  "de_be0-lcd0"
-  "de_be1-lcd1"
-  "de_be0-lcd0-hdmi"
-  "de_be1-lcd1-hdmi"
-  "mixer0-lcd0"
-  "mixer0-lcd0-hdmi"
-  "mixer1-lcd1-hdmi"
-  "mixer1-lcd1-tve"
-
-Example:
-
-chosen {
-	#address-cells = <1>;
-	#size-cells = <1>;
-	ranges;
-
-	framebuffer@0 {
-		compatible = "allwinner,simple-framebuffer", "simple-framebuffer";
-		allwinner,pipeline = "de_be0-lcd0-hdmi";
-		clocks = <&pll5 1>, <&ahb_gates 36>, <&ahb_gates 43>,
-			 <&ahb_gates 44>;
-	};
-};
diff --git a/Documentation/devicetree/bindings/display/simple-framebuffer.txt b/Documentation/devicetree/bindings/display/simple-framebuffer.txt
deleted file mode 100644
index 5a9ce511be88..000000000000
--- a/Documentation/devicetree/bindings/display/simple-framebuffer.txt
+++ /dev/null
@@ -1,91 +0,0 @@
-Simple Framebuffer
-
-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's 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,
-see e.g. simple-framebuffer-sunxi.txt .
-
-Required properties:
-- compatible: "simple-framebuffer"
-- reg: Should contain the location and size of the framebuffer memory.
-- width: The width of the framebuffer in pixels.
-- height: The height of the framebuffer in pixels.
-- stride: The number of bytes in each line of the framebuffer.
-- format: The format of the framebuffer surface. Valid values are:
-  - r5g6b5 (16-bit pixels, d[15:11]=r, d[10:5]=g, d[4:0]=b).
-  - a8b8g8r8 (32-bit pixels, d[31:24]=a, d[23:16]=b, d[15:8]=g, d[7:0]=r).
-
-Optional properties:
-- clocks : List of clocks used by the framebuffer.
-- *-supply : Any number of regulators used by the framebuffer. These should
-	     be named according to the names in the device's design.
-
-  The above resources are expected to already be configured correctly.
-  The OS must ensure they are not modified or disabled while the simple
-  framebuffer remains active.
-
-- display : phandle pointing to the primary display hardware node
-
-Example:
-
-aliases {
-	display0 = &lcdc0;
-}
-
-chosen {
-	framebuffer0: framebuffer@1d385000 {
-		compatible = "simple-framebuffer";
-		reg = <0x1d385000 (1600 * 1200 * 2)>;
-		width = <1600>;
-		height = <1200>;
-		stride = <(1600 * 2)>;
-		format = "r5g6b5";
-		clocks = <&ahb_gates 36>, <&ahb_gates 43>, <&ahb_gates 44>;
-		lcd-supply = <&reg_dc1sw>;
-		display = <&lcdc0>;
-	};
-	stdout-path = "display0";
-};
-
-soc@1c00000 {
-	lcdc0: lcdc@1c0c000 {
-		compatible = "allwinner,sun4i-a10-lcdc";
-		...
-	};
-};
-
-
-*) Older devicetree files may have a compatible = "simple-framebuffer" node
-in a different place, operating systems must first enumerate any compatible
-nodes found under chosen and then check for other compatible nodes.
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)
+%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@samsung.com>
+  - Hans de Goede <hdegoede@redhat.com>
+
+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
+
+      - 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
+
+  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
+
+  amlogic,pipeline:
+    description: Pipeline used by the framebuffer on Amlogic SoCs
+    oneOf:
+      - const: vpu-cvbs
+      - const: vpu-hdmi
+
+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 = <&reg_dc1sw>;
+        display = <&lcdc0>;
+      };
+    };
+
+    soc@1c00000 {
+      lcdc0: lcdc@1c0c000 {
+        compatible = "allwinner,sun4i-a10-lcdc";
+      };
+    };
+
+...
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] dt-bindings: Add schemas for simple-framebuffer

[Rob Herring]

On Mon, Mar 25, 2019 at 8:54 AM Maxime Ripard <maxime.ripard@bootlin.com> 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@samsung.com>
> Cc: Chen-Yu Tsai <wens@csie.org>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: Maxime Jourdan <mjourdan@baylibre.com>
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
>
> ---
>
> 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@samsung.com>
> +  - Hans de Goede <hdegoede@redhat.com>
> +
> +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 = <&reg_dc1sw>;
> +        display = <&lcdc0>;
> +      };
> +    };
> +
> +    soc@1c00000 {
> +      lcdc0: lcdc@1c0c000 {
> +        compatible = "allwinner,sun4i-a10-lcdc";
> +      };
> +    };
> +
> +...
> --
> 2.20.1
>

Re: [PATCH] dt-bindings: Add schemas for simple-framebuffer

[Rob Herring]

On Mon, Mar 25, 2019 at 8:54 AM Maxime Ripard <maxime.ripard@bootlin.com> 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@samsung.com>
> Cc: Chen-Yu Tsai <wens@csie.org>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: Maxime Jourdan <mjourdan@baylibre.com>
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
>
> ---
>
> 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@samsung.com>
> +  - Hans de Goede <hdegoede@redhat.com>
> +
> +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 = <&reg_dc1sw>;
> +        display = <&lcdc0>;
> +      };
> +    };
> +
> +    soc@1c00000 {
> +      lcdc0: lcdc@1c0c000 {
> +        compatible = "allwinner,sun4i-a10-lcdc";
> +      };
> +    };
> +
> +...
> --
> 2.20.1
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] dt-bindings: Add schemas for simple-framebuffer

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 2073 bytes --]

Hi,

On Mon, Mar 25, 2019 at 11:27:36AM -0500, Rob Herring wrote:
> On Mon, Mar 25, 2019 at 8:54 AM Maxime Ripard <maxime.ripard@bootlin.com> 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@samsung.com>
> > Cc: Chen-Yu Tsai <wens@csie.org>
> > Cc: Hans de Goede <hdegoede@redhat.com>
> > Cc: Maxime Jourdan <mjourdan@baylibre.com>
> > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> >
> > ---
> >
> > 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.

Maybe that's just cargo cult, but both the amlogic and the sunxi DT
have ranges in chosen. Shouldn't we need it so that the address is
decoded properly?

> > 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.

Ok, so the current patch would be good enough for now I guess.

> 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.

ACK.

I'll address the rest of your comments, thanks!
Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] dt-bindings: Add schemas for simple-framebuffer

[Rob Herring]

On Tue, Mar 26, 2019 at 4:35 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> Hi,
>
> On Mon, Mar 25, 2019 at 11:27:36AM -0500, Rob Herring wrote:
> > On Mon, Mar 25, 2019 at 8:54 AM Maxime Ripard <maxime.ripard@bootlin.com> 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@samsung.com>
> > > Cc: Chen-Yu Tsai <wens@csie.org>
> > > Cc: Hans de Goede <hdegoede@redhat.com>
> > > Cc: Maxime Jourdan <mjourdan@baylibre.com>
> > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> > >
> > > ---
> > >
> > > 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.
>
> Maybe that's just cargo cult, but both the amlogic and the sunxi DT
> have ranges in chosen. Shouldn't we need it so that the address is
> decoded properly?

I don't think so. If you have #{size,address}-cells then that should
be enough. Lack of ranges should just mean that any translation stops
at that level. But it is possible that the kernel code requires
translation to go all the way to the root for memory addresses. I
haven't checked.

Rob

Re: [PATCH] dt-bindings: Add schemas for simple-framebuffer

[Rob Herring]

On Tue, Mar 26, 2019 at 4:35 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> Hi,
>
> On Mon, Mar 25, 2019 at 11:27:36AM -0500, Rob Herring wrote:
> > On Mon, Mar 25, 2019 at 8:54 AM Maxime Ripard <maxime.ripard@bootlin.com> 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@samsung.com>
> > > Cc: Chen-Yu Tsai <wens@csie.org>
> > > Cc: Hans de Goede <hdegoede@redhat.com>
> > > Cc: Maxime Jourdan <mjourdan@baylibre.com>
> > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> > >
> > > ---
> > >
> > > 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.
>
> Maybe that's just cargo cult, but both the amlogic and the sunxi DT
> have ranges in chosen. Shouldn't we need it so that the address is
> decoded properly?

I don't think so. If you have #{size,address}-cells then that should
be enough. Lack of ranges should just mean that any translation stops
at that level. But it is possible that the kernel code requires
translation to go all the way to the root for memory addresses. I
haven't checked.

Rob

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] dt-bindings: Add schemas for simple-framebuffer

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 2167 bytes --]

Hi Rob,

On Wed, Mar 27, 2019 at 08:20:18AM -0500, Rob Herring wrote:
> On Tue, Mar 26, 2019 at 4:35 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > On Mon, Mar 25, 2019 at 11:27:36AM -0500, Rob Herring wrote:
> > > On Mon, Mar 25, 2019 at 8:54 AM Maxime Ripard <maxime.ripard@bootlin.com> 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@samsung.com>
> > > > Cc: Chen-Yu Tsai <wens@csie.org>
> > > > Cc: Hans de Goede <hdegoede@redhat.com>
> > > > Cc: Maxime Jourdan <mjourdan@baylibre.com>
> > > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> > > >
> > > > ---
> > > >
> > > > 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 just sent a github PR to do that.

> > Maybe that's just cargo cult, but both the amlogic and the sunxi DT
> > have ranges in chosen. Shouldn't we need it so that the address is
> > decoded properly?
>
> I don't think so. If you have #{size,address}-cells then that should
> be enough. Lack of ranges should just mean that any translation stops
> at that level. But it is possible that the kernel code requires
> translation to go all the way to the root for memory addresses. I
> haven't checked.

I tested it, and it looks like of_translate_address returns EINVAL if
ranges is missing. If that's not desired, then I can look into it.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel