Re: [PATCH 2/2] dt-bindings: simplefb-sunxi: Add sunxi simplefb extensions

Hi,

On 11/18/2014 09:47 AM, Ian Campbell wrote:

On Tue, 2014-11-18 at 09:15 +0100, Hans de Goede wrote:

quoted

Hi,

On 11/17/2014 04:55 PM, Maxime Ripard wrote:

quoted

On Mon, Nov 17, 2014 at 01:39:18PM +0000, Grant Likely wrote:

quoted

On Mon, Nov 17, 2014 at 12:47 PM, Maxime Ripard
[off-list ref] wrote:

quoted

On Mon, Nov 17, 2014 at 12:34:46PM +0100, Hans de Goede wrote:

quoted

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.

Signed-off-by: Hans de Goede <redacted>
---
 .../bindings/video/simple-framebuffer-sunxi.txt    | 33 ++++++++++++++++++++++
 1 file changed, 33 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt

diff --git a/Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt b/Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt
new file mode 100644
index 0000000..84ca264
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt

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

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

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.

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

Regards,

Hans