On Mon, Nov 17, 2014 at 3:55 PM, Maxime Ripard
<maxime.ripard@xxxxxxxxxxxxxxxxxx> wrote:
> On Mon, Nov 17, 2014 at 01:39:18PM +0000, Grant Likely wrote:
>> On Mon, Nov 17, 2014 at 12:47 PM, Maxime Ripard
>> <maxime.ripard@xxxxxxxxxxxxxxxxxx> wrote:
>> > On Mon, Nov 17, 2014 at 12:34:46PM +0100, Hans de Goede wrote:
>> >> 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 <hdegoede@xxxxxxxxxx>
>> >> ---
>> >> .../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", 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.
> 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.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
