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

[Maxime Ripard <maxime.ripard@free-electrons.com>]

[-- Attachment #1: Type: text/plain, Size: 4765 bytes --]

On Tue, Nov 18, 2014 at 09:15:34AM +0100, Hans de Goede wrote:
> Hi,
> 
> On 11/17/2014 04:55 PM, Maxime Ripard 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@free-electrons.com> 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@redhat.com>
> >>>> ---
> >>>>  .../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:

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)

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

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

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]