Skip to contentSkip to conversation

dt-bindings: simplefb: Drop the advice about using a specific path for nodes

PATCH Superseded Hans de Goede 4 participants 16 messages Nov 17, 2014 – Nov 21, 2014

Subsystems (3)

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

cover dt-bindings: simplefb: Drop the advice about using a specific path for nodes

Hi All,

Review of the u-boot sunxi simplefb patches has lead to the decision that u-boot should not use a specific path to find the nodes as this goes contrary to how devicetree usually works.

Instead a platform specific compatible + properties should be used for this.

This series updates the simplefb bindings reflect this.

Regards,

Hans

#1 lore raw

PATCH 1/2 dt-bindings: simplefb: Drop the advice about using a specific path for nodes

This goes contrary to how devicetree usually works, so drop it. Instead if the firmware needs to be able to find a specific node it should use a platform specific compatible + properties for this.

---
 Documentation/devicetree/bindings/video/simple-framebuffer.txt | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)
Documentation/devicetree/bindings/video/simple-framebuffer.txt +4 -6
index f8fb7e6..4474ef6 100644
@@ -31,12 +31,10 @@ 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, they should be named
-"framebuffer#-<output>", e.g. "framebuffer0-hdmi". The output should be
-included in the name since different outputs typically require different
-clocks and the clocks are part of the pre-populated nodes. The firmware must
-rename the nodes to the standard "framebuffer@<address>" name using the
-runtime chosen address when enabling the nodes.
+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"
Signature · 1 line
2.1.0
#2 lore raw

PATCH 1/2 dt-bindings: simplefb: Drop the advice about using a specific path for nodes

Quoted 5 lines · Hans de Goede

This goes contrary to how devicetree usually works, so drop it. Instead if the firmware needs to be able to find a specific node it should use a platform specific compatible + properties for this.

Quoted 28 lines
---
 Documentation/devicetree/bindings/video/simple-framebuffer.txt | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)
Documentation/devicetree/bindings/video/simple-framebuffer.txt +4 -7
index f8fb7e6..4474ef6 100644
@@ -31,12 +31,10 @@ 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, they should be named
-"framebuffer#-<output>", e.g. "framebuffer0-hdmi". The output should be
-included in the name since different outputs typically require different
-clocks and the clocks are part of the pre-populated nodes. The firmware must
-rename the nodes to the standard "framebuffer@<address>" name using the
-runtime chosen address when enabling the nodes.
+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"
--

2.1.0

#3 lore raw

PATCH 2/2 dt-bindings: simplefb-sunxi: Add sunxi simplefb extensions

In reply to Hans de Goede

If pre-filled framebuffer nodes are used, the firmware may need extra properties to find the right node. This documents the properties to use for this on sunxi platforms.

---
 .../bindings/video/simple-framebuffer-sunxi.txt    | 33 ++++++++++++++++++++++
 1 file changed, 33 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt
Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt new file +33 -0
new file mode 100644
index 0000000..84ca264
@@ -0,0 +1,33 @@
+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 and output is being used. As such they are solely intended
+for firmware / bootloader use, and the OS should ignore them.
+
+Required properties:
+- compatible: "sunxi,framebuffer"
+- sunxi,pipeline:
+  <0> for the de_be0 -> lcdc0 -> output pipeline
+  <1> for the de_be1 -> lcdc1 -> output pipeline
+- sunxi,output: One of: "hdmi", "lcd", "vga", and "composite"  
+
+Example:
+
+chosen {
+	#address-cells = <1>;
+	#size-cells = <1>;
+	ranges;
+
+	framebuffer at 0 {
+		compatible = "sunxi,framebuffer", "simple-framebuffer";
+		sunxi,pipeline = <0>;
+		sunxi,output = "hdmi";
+		clocks = <&pll5 1>, <&ahb_gates 36>, <&ahb_gates 43>,
+			 <&ahb_gates 44>;
+		status = "disabled";
+	};
+};
Signature · 1 line
2.1.0
#4 lore raw

PATCH 2/2 dt-bindings: simplefb-sunxi: Add sunxi simplefb extensions

Quoted 5 lines · Hans de Goede

If pre-filled framebuffer nodes are used, the firmware may need extra properties to find the right node. This documents the properties to use for this on sunxi platforms.

Quoted 47 lines
---
 .../bindings/video/simple-framebuffer-sunxi.txt    | 33 ++++++++++++++++++++++
 1 file changed, 33 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt
Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt new file +33 -1
new file mode 100644
index 0000000..84ca264
@@ -0,0 +1,33 @@
+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 and output is being used. As such they are solely intended
+for firmware / bootloader use, and the OS should ignore them.
+
+Required properties:
+- compatible: "sunxi,framebuffer"
+- sunxi,pipeline:
+  <0> for the de_be0 -> lcdc0 -> output pipeline
+  <1> for the de_be1 -> lcdc1 -> output pipeline
+- sunxi,output: One of: "hdmi", "lcd", "vga", and "composite"
+
+Example:
+
+chosen {
+       #address-cells = <1>;
+       #size-cells = <1>;
+       ranges;
+
+       framebuffer at 0 {
+               compatible = "sunxi,framebuffer", "simple-framebuffer";
+               sunxi,pipeline = <0>;
+               sunxi,output = "hdmi";
+               clocks = <&pll5 1>, <&ahb_gates 36>, <&ahb_gates 43>,
+                        <&ahb_gates 44>;
+               status = "disabled";
+       };
+};
--

2.1.0

#5 lore raw

PATCH 2/2 dt-bindings: simplefb-sunxi: Add sunxi simplefb extensions

In reply to Hans de Goede

Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt line 15

Quoted 3 lines · Hans de Goede

+- sunxi,pipeline:

Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt new file +2 -0
+  <0> for the de_be0 -> lcdc0 -> output pipeline
+  <1> for the de_be1 -> lcdc1 -> output pipeline

Can we express this as a more descriptive string rather than a number, to better handle future developments?

Otherwise both patches in this series look good to me.

Quoted 18 lines

+- sunxi,output: One of: "hdmi", "lcd", "vga", and "composite"

Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt new file +17 -0
+
+Example:
+
+chosen {
+	#address-cells = <1>;
+	#size-cells = <1>;
+	ranges;
+
+	framebuffer at 0 {
+		compatible = "sunxi,framebuffer", "simple-framebuffer";
+		sunxi,pipeline = <0>;
+		sunxi,output = "hdmi";
+		clocks = <&pll5 1>, <&ahb_gates 36>, <&ahb_gates 43>,
+			 <&ahb_gates 44>;
+		status = "disabled";
+	};
+};
#6 lore raw

PATCH 2/2 dt-bindings: simplefb-sunxi: Add sunxi simplefb extensions

In reply to Hans de Goede
Quoted 36 lines · Hans de Goede

If pre-filled framebuffer nodes are used, the firmware may need extra properties to find the right node. This documents the properties to use for this on sunxi platforms.

---
 .../bindings/video/simple-framebuffer-sunxi.txt    | 33 ++++++++++++++++++++++
 1 file changed, 33 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt
Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt new file +20 -0
new file mode 100644
index 0000000..84ca264
@@ -0,0 +1,33 @@
+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 and output is being used. As such they are solely intended
+for firmware / bootloader use, and the OS should ignore them.
+
+Required properties:
+- compatible: "sunxi,framebuffer"
+- sunxi,pipeline:
+  <0> for the de_be0 -> lcdc0 -> output pipeline
+  <1> for the de_be1 -> lcdc1 -> output pipeline
+- sunxi,output: One of: "hdmi", "lcd", "vga", and "composite"  
+
+Example:
+
+chosen {

Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt line 30

Quoted 10 lines
Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt new file +10 -0
+	#address-cells = <1>;
+	#size-cells = <1>;
+	ranges;
+
+	framebuffer at 0 {
+		compatible = "sunxi,framebuffer", "simple-framebuffer";
+		sunxi,pipeline = <0>;
+		sunxi,output = "hdmi";
+		clocks = <&pll5 1>, <&ahb_gates 36>, <&ahb_gates 43>,
+			 <&ahb_gates 44>;

If we're going that way, then maybe having to specify clock-names would be better in order to know which clock is what?

Maxime

Signature · 10 lines
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141117/310b97ea/attachment.sig>
#7 lore raw

PATCH 2/2 dt-bindings: simplefb-sunxi: Add sunxi simplefb extensions

Hi,

Quoted 42 lines · Maxime Ripard
Quoted 41 lines · Hans de Goede

If pre-filled framebuffer nodes are used, the firmware may need extra properties to find the right node. This documents the properties to use for this on sunxi platforms.

---
 .../bindings/video/simple-framebuffer-sunxi.txt    | 33 ++++++++++++++++++++++
 1 file changed, 33 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt
Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt new file +25 -0
new file mode 100644
index 0000000..84ca264
@@ -0,0 +1,33 @@
+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 and output is being used. As such they are solely intended
+for firmware / bootloader use, and the OS should ignore them.
+
+Required properties:
+- compatible: "sunxi,framebuffer"
+- sunxi,pipeline:
+  <0> for the de_be0 -> lcdc0 -> output pipeline
+  <1> for the de_be1 -> lcdc1 -> output pipeline
+- sunxi,output: One of: "hdmi", "lcd", "vga", and "composite"  
+
+Example:
+
+chosen {
+	#address-cells = <1>;
+	#size-cells = <1>;
+	ranges;
+
+	framebuffer at 0 {
Quoted 8 lines

Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt line 30

Quoted 5 lines
Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt new file +5 -0
+		compatible = "sunxi,framebuffer", "simple-framebuffer";
+		sunxi,pipeline = <0>;
+		sunxi,output = "hdmi";
+		clocks = <&pll5 1>, <&ahb_gates 36>, <&ahb_gates 43>,
+			 <&ahb_gates 44>;

If we're going that way, then maybe having to specify clock-names would be better in order to know which clock is what?

I agree that using clock-names is more human readable then phandle's, but that is simply not how clocks are specified anywhere in dt.

Regards,

Hans

#8 lore raw

PATCH 2/2 dt-bindings: simplefb-sunxi: Add sunxi simplefb extensions

In reply to Maxime Ripard
Quoted 42 lines · Maxime Ripard
Quoted 41 lines · Hans de Goede

If pre-filled framebuffer nodes are used, the firmware may need extra properties to find the right node. This documents the properties to use for this on sunxi platforms.

---
 .../bindings/video/simple-framebuffer-sunxi.txt    | 33 ++++++++++++++++++++++
 1 file changed, 33 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt
Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt new file +25 -0
new file mode 100644
index 0000000..84ca264
@@ -0,0 +1,33 @@
+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 and output is being used. As such they are solely intended
+for firmware / bootloader use, and the OS should ignore them.
+
+Required properties:
+- compatible: "sunxi,framebuffer"
+- sunxi,pipeline:
+  <0> for the de_be0 -> lcdc0 -> output pipeline
+  <1> for the de_be1 -> lcdc1 -> output pipeline
+- sunxi,output: One of: "hdmi", "lcd", "vga", and "composite"
+
+Example:
+
+chosen {
+     #address-cells = <1>;
+     #size-cells = <1>;
+     ranges;
+
+     framebuffer at 0 {
Quoted 8 lines

Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt line 30

Quoted 5 lines
Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt new file +5 -0
+             compatible = "sunxi,framebuffer", "simple-framebuffer";
+             sunxi,pipeline = <0>;
+             sunxi,output = "hdmi";
+             clocks = <&pll5 1>, <&ahb_gates 36>, <&ahb_gates 43>,
+                      <&ahb_gates 44>;

If we're going that way, then maybe having to specify clock-names would be better in order to know which clock is what?

I wouldn't go that way with this binding since the driver has no need to differentiate between the clocks, and driver authors shouldn't be encouraged to do so. The purpose of the clocks in this node is only for itemizing dependencies, not for how to configure the clocks. Firmware shouldn't care at all about the clocks list, it only needs to find the correct pre-populated node to fill in and enable.

g.

#9 lore raw

PATCH 2/2 dt-bindings: simplefb-sunxi: Add sunxi simplefb extensions

Quoted 53 lines · Grant Likely
Quoted 42 lines · Maxime Ripard
Quoted 41 lines · Hans de Goede

If pre-filled framebuffer nodes are used, the firmware may need extra properties to find the right node. This documents the properties to use for this on sunxi platforms.

---
 .../bindings/video/simple-framebuffer-sunxi.txt    | 33 ++++++++++++++++++++++
 1 file changed, 33 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt
Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt new file +25 -0
new file mode 100644
index 0000000..84ca264
@@ -0,0 +1,33 @@
+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 and output is being used. As such they are solely intended
+for firmware / bootloader use, and the OS should ignore them.
+
+Required properties:
+- compatible: "sunxi,framebuffer"
+- sunxi,pipeline:
+  <0> for the de_be0 -> lcdc0 -> output pipeline
+  <1> for the de_be1 -> lcdc1 -> output pipeline
+- sunxi,output: One of: "hdmi", "lcd", "vga", and "composite"
+
+Example:
+
+chosen {
+     #address-cells = <1>;
+     #size-cells = <1>;
+     ranges;
+
+     framebuffer at 0 {
Quoted 8 lines

Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt line 30

Quoted 5 lines
Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt new file +5 -0
+             compatible = "sunxi,framebuffer", "simple-framebuffer";
+             sunxi,pipeline = <0>;
+             sunxi,output = "hdmi";
+             clocks = <&pll5 1>, <&ahb_gates 36>, <&ahb_gates 43>,
+                      <&ahb_gates 44>;

If we're going that way, then maybe having to specify clock-names would be better in order to know which clock is what?

Quoted 6 lines

I wouldn't go that way with this binding since the driver has no need to differentiate between the clocks, and driver authors shouldn't be encouraged to do so. The purpose of the clocks in this node is only for itemizing dependencies, not for how to configure the clocks. Firmware shouldn't care at all about the clocks list, it only needs to find the correct pre-populated node to fill in and enable.

Well, if we want to play the "DT as an ABI" stuff, you have no guarantee that in the future, simplefb will still be the driver bound to "sunxi,framebuffer", and that this driver doesn't need to differentiate which clocks it needs to protect and which are not needed. Especially with the combination of the output that you now hardcode.

Of course, if now we don't care at all about this ABI thing, it's completely fine for me. But I'll archive this email preciously.

Maxime

Signature · 10 lines
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141117/28d55089/attachment-0001.sig>
#10 lore raw

PATCH 2/2 dt-bindings: simplefb-sunxi: Add sunxi simplefb extensions

Hi,

Quoted 61 lines · Maxime Ripard
Quoted 53 lines · Grant Likely
Quoted 42 lines · Maxime Ripard
Quoted 41 lines · Hans de Goede

If pre-filled framebuffer nodes are used, the firmware may need extra properties to find the right node. This documents the properties to use for this on sunxi platforms.

---
 .../bindings/video/simple-framebuffer-sunxi.txt    | 33 ++++++++++++++++++++++
 1 file changed, 33 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt
Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt new file +25 -0
new file mode 100644
index 0000000..84ca264
@@ -0,0 +1,33 @@
+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 and output is being used. As such they are solely intended
+for firmware / bootloader use, and the OS should ignore them.
+
+Required properties:
+- compatible: "sunxi,framebuffer"
+- sunxi,pipeline:
+  <0> for the de_be0 -> lcdc0 -> output pipeline
+  <1> for the de_be1 -> lcdc1 -> output pipeline
+- sunxi,output: One of: "hdmi", "lcd", "vga", and "composite"
+
+Example:
+
+chosen {
+     #address-cells = <1>;
+     #size-cells = <1>;
+     ranges;
+
+     framebuffer at 0 {
Quoted 8 lines

Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt line 30

Quoted 5 lines
Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt new file +5 -0
+             compatible = "sunxi,framebuffer", "simple-framebuffer";
+             sunxi,pipeline = <0>;
+             sunxi,output = "hdmi";
+             clocks = <&pll5 1>, <&ahb_gates 36>, <&ahb_gates 43>,
+                      <&ahb_gates 44>;

If we're going that way, then maybe having to specify clock-names would be better in order to know which clock is what?

Quoted 6 lines

I wouldn't go that way with this binding since the driver has no need to differentiate between the clocks, and driver authors shouldn't be encouraged to do so. The purpose of the clocks in this node is only for itemizing dependencies, not for how to configure the clocks. Firmware shouldn't care at all about the clocks list, it only needs to find the correct pre-populated node to fill in and enable.

Quoted 3 lines

Well, if we want to play the "DT as an ABI" stuff, you have no guarantee that in the future, simplefb will still be the driver bound to "sunxi,framebuffer"

Huh? We do have that guarantee, or at least that a simplefb bindings compatible driver is, the "sunxi,framebuffer" bindings live in:

Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt

With the filename already being a big hint, further more it states that:

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

Which to me seems to guarantee that we're talking about a simple-framebuffer compatible node here. The scope of these extensions are very narrow, there only purpose is to allow the bootloader to find the right pre-populated simplefb node, everything else is specified by the simplefb bindings, not these extensions.

Regards,

Hans

#11 lore raw

PATCH 2/2 dt-bindings: simplefb-sunxi: Add sunxi simplefb extensions

Quoted 75 lines · Hans de Goede

Hi,

Quoted 61 lines · Maxime Ripard
Quoted 53 lines · Grant Likely
Quoted 42 lines · Maxime Ripard
Quoted 41 lines · Hans de Goede

If pre-filled framebuffer nodes are used, the firmware may need extra properties to find the right node. This documents the properties to use for this on sunxi platforms.

---
 .../bindings/video/simple-framebuffer-sunxi.txt    | 33 ++++++++++++++++++++++
 1 file changed, 33 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt
Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt new file +25 -0
new file mode 100644
index 0000000..84ca264
@@ -0,0 +1,33 @@
+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 and output is being used. As such they are solely intended
+for firmware / bootloader use, and the OS should ignore them.
+
+Required properties:
+- compatible: "sunxi,framebuffer"
+- sunxi,pipeline:
+  <0> for the de_be0 -> lcdc0 -> output pipeline
+  <1> for the de_be1 -> lcdc1 -> output pipeline
+- sunxi,output: One of: "hdmi", "lcd", "vga", and "composite"
+
+Example:
+
+chosen {
+     #address-cells = <1>;
+     #size-cells = <1>;
+     ranges;
+
+     framebuffer at 0 {
Quoted 8 lines

Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt line 30

Quoted 5 lines
Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt new file +5 -0
+             compatible = "sunxi,framebuffer", "simple-framebuffer";
+             sunxi,pipeline = <0>;
+             sunxi,output = "hdmi";
+             clocks = <&pll5 1>, <&ahb_gates 36>, <&ahb_gates 43>,
+                      <&ahb_gates 44>;

If we're going that way, then maybe having to specify clock-names would be better in order to know which clock is what?

Quoted 6 lines

I wouldn't go that way with this binding since the driver has no need to differentiate between the clocks, and driver authors shouldn't be encouraged to do so. The purpose of the clocks in this node is only for itemizing dependencies, not for how to configure the clocks. Firmware shouldn't care at all about the clocks list, it only needs to find the correct pre-populated node to fill in and enable.

Quoted 3 lines

Well, if we want to play the "DT as an ABI" stuff, you have no guarantee that in the future, simplefb will still be the driver bound to "sunxi,framebuffer"

Huh? We do have that guarantee, or at least that a simplefb bindings compatible driver is, the "sunxi,framebuffer" bindings live in:

Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt

With the filename already being a big hint, further more it states that:

Quoted 7 lines

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

There may be an argument here for using the slightly redundant compat string "allwinner,simple-framebuffer" in order to reinforce that this is an extension to simple-framebuffer, as opposed to a binding for a completely separate "non-simple" sunxi framebuffer driver which I think is what Maxime may be imagining.

Alternatively (or as well) perhaps these extension should be a series of extensions (one per platform) described in "appendices" of the main simple-framebuffer.txt. I know Grant expressed a preference for a separate document though and I think just using the more expressive name above would be sufficient, but thought I'd mention it as a possible option.

Ian.

#12 lore raw

PATCH 2/2 dt-bindings: simplefb-sunxi: Add sunxi simplefb extensions

Hi,

Quoted 90 lines · Ian Campbell
Quoted 75 lines · Hans de Goede

Hi,

Quoted 61 lines · Maxime Ripard
Quoted 53 lines · Grant Likely
Quoted 42 lines · Maxime Ripard
Quoted 41 lines · Hans de Goede

If pre-filled framebuffer nodes are used, the firmware may need extra properties to find the right node. This documents the properties to use for this on sunxi platforms.

---
 .../bindings/video/simple-framebuffer-sunxi.txt    | 33 ++++++++++++++++++++++
 1 file changed, 33 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt
Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt new file +25 -0
new file mode 100644
index 0000000..84ca264
@@ -0,0 +1,33 @@
+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 and output is being used. As such they are solely intended
+for firmware / bootloader use, and the OS should ignore them.
+
+Required properties:
+- compatible: "sunxi,framebuffer"
+- sunxi,pipeline:
+  <0> for the de_be0 -> lcdc0 -> output pipeline
+  <1> for the de_be1 -> lcdc1 -> output pipeline
+- sunxi,output: One of: "hdmi", "lcd", "vga", and "composite"
+
+Example:
+
+chosen {
+     #address-cells = <1>;
+     #size-cells = <1>;
+     ranges;
+
+     framebuffer at 0 {
Quoted 8 lines

Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt line 30

Quoted 5 lines
Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt new file +5 -0
+             compatible = "sunxi,framebuffer", "simple-framebuffer";
+             sunxi,pipeline = <0>;
+             sunxi,output = "hdmi";
+             clocks = <&pll5 1>, <&ahb_gates 36>, <&ahb_gates 43>,
+                      <&ahb_gates 44>;

If we're going that way, then maybe having to specify clock-names would be better in order to know which clock is what?

Quoted 6 lines

I wouldn't go that way with this binding since the driver has no need to differentiate between the clocks, and driver authors shouldn't be encouraged to do so. The purpose of the clocks in this node is only for itemizing dependencies, not for how to configure the clocks. Firmware shouldn't care at all about the clocks list, it only needs to find the correct pre-populated node to fill in and enable.

Quoted 3 lines

Well, if we want to play the "DT as an ABI" stuff, you have no guarantee that in the future, simplefb will still be the driver bound to "sunxi,framebuffer"

Huh? We do have that guarantee, or at least that a simplefb bindings compatible driver is, the "sunxi,framebuffer" bindings live in:

Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt

With the filename already being a big hint, further more it states that:

Quoted 7 lines

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

There may be an argument here for using the slightly redundant compat string "allwinner,simple-framebuffer" in order to reinforce that this is an extension to simple-framebuffer, as opposed to a binding for a completely separate "non-simple" sunxi framebuffer driver which I think is what Maxime may be imagining.

Quoted 6 lines

Alternatively (or as well) perhaps these extension should be a series of extensions (one per platform) described in "appendices" of the main simple-framebuffer.txt. I know Grant expressed a preference for a separate document though and I think just using the more expressive name above would be sufficient, but thought I'd mention it as a possible option.

That is a good suggestion, as discussed on irc, lets go with that.

Regards,

Hans

#13 lore raw

PATCH 2/2 dt-bindings: simplefb-sunxi: Add sunxi simplefb extensions

In reply to Hans de Goede
Quoted 68 lines · Hans de Goede

Hi,

Quoted 61 lines · Maxime Ripard
Quoted 53 lines · Grant Likely
Quoted 42 lines · Maxime Ripard
Quoted 41 lines · Hans de Goede

If pre-filled framebuffer nodes are used, the firmware may need extra properties to find the right node. This documents the properties to use for this on sunxi platforms.

---
 .../bindings/video/simple-framebuffer-sunxi.txt    | 33 ++++++++++++++++++++++
 1 file changed, 33 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt
Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt new file +25 -0
new file mode 100644
index 0000000..84ca264
@@ -0,0 +1,33 @@
+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 and output is being used. As such they are solely intended
+for firmware / bootloader use, and the OS should ignore them.
+
+Required properties:
+- compatible: "sunxi,framebuffer"
+- sunxi,pipeline:
+  <0> for the de_be0 -> lcdc0 -> output pipeline
+  <1> for the de_be1 -> lcdc1 -> output pipeline
+- sunxi,output: One of: "hdmi", "lcd", "vga", and "composite"
+
+Example:
+
+chosen {
+     #address-cells = <1>;
+     #size-cells = <1>;
+     ranges;
+
+     framebuffer at 0 {
Quoted 8 lines

Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt line 30

Quoted 5 lines
Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt new file +5 -0
+             compatible = "sunxi,framebuffer", "simple-framebuffer";
+             sunxi,pipeline = <0>;
+             sunxi,output = "hdmi";
+             clocks = <&pll5 1>, <&ahb_gates 36>, <&ahb_gates 43>,
+                      <&ahb_gates 44>;

If we're going that way, then maybe having to specify clock-names would be better in order to know which clock is what?

Quoted 6 lines

I wouldn't go that way with this binding since the driver has no need to differentiate between the clocks, and driver authors shouldn't be encouraged to do so. The purpose of the clocks in this node is only for itemizing dependencies, not for how to configure the clocks. Firmware shouldn't care at all about the clocks list, it only needs to find the correct pre-populated node to fill in and enable.

Quoted 3 lines

Well, if we want to play the "DT as an ABI" stuff, you have no guarantee that in the future, simplefb will still be the driver bound to "sunxi,framebuffer"

Quoted 6 lines

Huh? We do have that guarantee, or at least that a simplefb bindings compatible driver is, the "sunxi,framebuffer" bindings live in:

Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt

With the filename already being a big hint, further more it states that:

Yeah, well, that can change. The compatible itself can't, but we're free to move around the drivers however we see fit (as long as the user-space is not impacted)

Quoted 8 lines

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

Quoted 5 lines

Which to me seems to guarantee that we're talking about a simple-framebuffer compatible node here. The scope of these extensions are very narrow, there only purpose is to allow the bootloader to find the right pre-populated simplefb node, everything else is specified by the simplefb bindings, not these extensions.

My point was that what we might need to know these clocks at some point in the future. But if you don't believe we will, it's fine for me.

Maxime

Signature · 10 lines
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141118/d53efffa/attachment-0001.sig>
#14 lore raw

PATCH 2/2 dt-bindings: simplefb-sunxi: Add sunxi simplefb extensions

In reply to Maxime Ripard
Quoted 61 lines · Maxime Ripard
Quoted 53 lines · Grant Likely
Quoted 42 lines · Maxime Ripard
Quoted 41 lines · Hans de Goede

If pre-filled framebuffer nodes are used, the firmware may need extra properties to find the right node. This documents the properties to use for this on sunxi platforms.

---
 .../bindings/video/simple-framebuffer-sunxi.txt    | 33 ++++++++++++++++++++++
 1 file changed, 33 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt
Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt new file +25 -0
new file mode 100644
index 0000000..84ca264
@@ -0,0 +1,33 @@
+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 and output is being used. As such they are solely intended
+for firmware / bootloader use, and the OS should ignore them.
+
+Required properties:
+- compatible: "sunxi,framebuffer"
+- sunxi,pipeline:
+  <0> for the de_be0 -> lcdc0 -> output pipeline
+  <1> for the de_be1 -> lcdc1 -> output pipeline
+- sunxi,output: One of: "hdmi", "lcd", "vga", and "composite"
+
+Example:
+
+chosen {
+     #address-cells = <1>;
+     #size-cells = <1>;
+     ranges;
+
+     framebuffer at 0 {
Quoted 8 lines

Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt line 30

Quoted 5 lines
Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt new file +5 -0
+             compatible = "sunxi,framebuffer", "simple-framebuffer";
+             sunxi,pipeline = <0>;
+             sunxi,output = "hdmi";
+             clocks = <&pll5 1>, <&ahb_gates 36>, <&ahb_gates 43>,
+                      <&ahb_gates 44>;

If we're going that way, then maybe having to specify clock-names would be better in order to know which clock is what?

Quoted 6 lines

I wouldn't go that way with this binding since the driver has no need to differentiate between the clocks, and driver authors shouldn't be encouraged to do so. The purpose of the clocks in this node is only for itemizing dependencies, not for how to configure the clocks. Firmware shouldn't care at all about the clocks list, it only needs to find the correct pre-populated node to fill in and enable.

Quoted 6 lines

Well, if we want to play the "DT as an ABI" stuff, you have no guarantee that in the future, simplefb will still be the driver bound to "sunxi,framebuffer", and that this driver doesn't need to differentiate which clocks it needs to protect and which are not needed. Especially with the combination of the output that you now hardcode.

As long as the node claims compatibility with "simple-framebuffer", then it must abide by the simple framebuffer rules. It cannot depend on the driver having special knowledge about what to do with the clocks. It is fine for a driver that understands "sunxi,framebuffer" to enable additional features over and above simple-framebuffer, providing it doesn't break the vanilla "simple-framebuffer" interface.

The simple-framebuffer binding is clear that it will treat the clocks as dependencies only.

Quoted 2 lines

Of course, if now we don't care at all about this ABI thing, it's completely fine for me. But I'll archive this email preciously.

I do not in any way mean to say it is okay to break ABI.

Also, my comments above are advice that I would give to U-Boot authors, not something that should go in the binding. The simple-framebuffer binding doesn't need to know about the clock names at all, and if the assumption is that firmware will merely enable a pre-populated framebuffer node in the tree, then it probably doesn't need to parse the clocks property either. However, if it actually is better for the U-Boot implementation to have clocks with names in the node, then I'm not going to object.

g.

#15 lore raw

PATCH 2/2 dt-bindings: simplefb-sunxi: Add sunxi simplefb extensions

Quoted 77 lines · Grant Likely
Quoted 61 lines · Maxime Ripard
Quoted 53 lines · Grant Likely
Quoted 42 lines · Maxime Ripard
Quoted 41 lines · Hans de Goede

If pre-filled framebuffer nodes are used, the firmware may need extra properties to find the right node. This documents the properties to use for this on sunxi platforms.

---
 .../bindings/video/simple-framebuffer-sunxi.txt    | 33 ++++++++++++++++++++++
 1 file changed, 33 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt
Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt new file +25 -0
new file mode 100644
index 0000000..84ca264
@@ -0,0 +1,33 @@
+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 and output is being used. As such they are solely intended
+for firmware / bootloader use, and the OS should ignore them.
+
+Required properties:
+- compatible: "sunxi,framebuffer"
+- sunxi,pipeline:
+  <0> for the de_be0 -> lcdc0 -> output pipeline
+  <1> for the de_be1 -> lcdc1 -> output pipeline
+- sunxi,output: One of: "hdmi", "lcd", "vga", and "composite"
+
+Example:
+
+chosen {
+     #address-cells = <1>;
+     #size-cells = <1>;
+     ranges;
+
+     framebuffer at 0 {
Quoted 8 lines

Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt line 30

Quoted 5 lines
Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt new file +5 -0
+             compatible = "sunxi,framebuffer", "simple-framebuffer";
+             sunxi,pipeline = <0>;
+             sunxi,output = "hdmi";
+             clocks = <&pll5 1>, <&ahb_gates 36>, <&ahb_gates 43>,
+                      <&ahb_gates 44>;

If we're going that way, then maybe having to specify clock-names would be better in order to know which clock is what?

Quoted 6 lines

I wouldn't go that way with this binding since the driver has no need to differentiate between the clocks, and driver authors shouldn't be encouraged to do so. The purpose of the clocks in this node is only for itemizing dependencies, not for how to configure the clocks. Firmware shouldn't care at all about the clocks list, it only needs to find the correct pre-populated node to fill in and enable.

Quoted 6 lines

Well, if we want to play the "DT as an ABI" stuff, you have no guarantee that in the future, simplefb will still be the driver bound to "sunxi,framebuffer", and that this driver doesn't need to differentiate which clocks it needs to protect and which are not needed. Especially with the combination of the output that you now hardcode.

As long as the node claims compatibility with "simple-framebuffer", then it must abide by the simple framebuffer rules. It cannot depend on the driver having special knowledge about what to do with the clocks. It is fine for a driver that understands "sunxi,framebuffer" to enable additional features over and above simple-framebuffer, providing it doesn't break the vanilla "simple-framebuffer" interface.

Quoted 2 lines

The simple-framebuffer binding is clear that it will treat the clocks as dependencies only.

Isn't the compatible list precisely made to be able to define more specific behaviours if needed?

If I get a list with "sunxi,framebuffer", "simple-framebuffer", I've always thought that sunxi,framebuffer was defining exactly what the device was about, and that simple-framebuffer was only aimed at providing roughly the same functionnality, possibly with degraded features, and with a compatible DT bindings.

I don't see how if we require a clock-names list in sunxi,framebuffer that would break the binding of simple-framebuffer. It's just more information, that might prove useful in the future.

Quoted 5 lines
Quoted 2 lines

Of course, if now we don't care at all about this ABI thing, it's completely fine for me. But I'll archive this email preciously.

I do not in any way mean to say it is okay to break ABI.

Quoted 8 lines

Also, my comments above are advice that I would give to U-Boot authors, not something that should go in the binding. The simple-framebuffer binding doesn't need to know about the clock names at all, and if the assumption is that firmware will merely enable a pre-populated framebuffer node in the tree, then it probably doesn't need to parse the clocks property either. However, if it actually is better for the U-Boot implementation to have clocks with names in the node, then I'm not going to object.

I'm not speaking about u-boot here, but the kernel itself.

What if we want from that pipeline property to enable (or rather keep enabled) only a few clocks, and not all of them?

Maxime

Signature · 10 lines
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141121/0c9732aa/attachment.sig>