[PATCH v3 0/2] dt-bindings: simplefb: Drop the advice about using a specific path for nodes

[PATCH v3 0/2] dt-bindings: simplefb: Drop the advice about using a specific path for nodes

[Hans de Goede]

Hi Tomi,

So it turns out we needed a v3, as I mistakenly went with "sunxi," as vendor
prefix for the allwinner specific properties, but the registered vendor prefix
for allwinner is "allwinnner,", this version fixes this.

Changes in v3: Use proper "allwinnner," for the compatible string and vendor
specific properties.

Changes in v2: Changed the simplefb-sunxi bindings to use a single
sunxi,pipeline string property instead of a sunxi,pipeline int and a
sunxi,output string property.

This patch set is all acked up, and a fix to the binding changes you've
already queued for 3.19, so please queue these 2 patches for 3.19.

Thanks & Regards,

Hans

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

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

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Acked-by: Grant Likely <grant.likely@linaro.org>
---
 Documentation/devicetree/bindings/video/simple-framebuffer.txt | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/video/simple-framebuffer.txt b/Documentation/devicetree/bindings/video/simple-framebuffer.txt
index f8fb7e6..4474ef6 100644
--- a/Documentation/devicetree/bindings/video/simple-framebuffer.txt
+++ b/Documentation/devicetree/bindings/video/simple-framebuffer.txt
@@ -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

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

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

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Acked-by: Grant Likely <grant.likely@linaro.org>
---
 .../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..c46ba64
--- /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 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"
+
+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>;
+		status = "disabled";
+	};
+};
-- 
2.1.0

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

[Tomi Valkeinen]

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

On 18/11/14 13:10, 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>
> Acked-by: Grant Likely <grant.likely@linaro.org>
> ---
>  .../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..c46ba64
> --- /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 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:

Sorry my ignorance, but what's sunxi and what's allwinner? Both names
are mixed here.

 Tomi



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

Re: [PATCH v3 0/2] dt-bindings: simplefb: Drop the advice about using a specific path for nodes

[Tomi Valkeinen]

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

Hi,

On 18/11/14 13:10, Hans de Goede wrote:
> Hi Tomi,
> 
> So it turns out we needed a v3, as I mistakenly went with "sunxi," as vendor
> prefix for the allwinner specific properties, but the registered vendor prefix
> for allwinner is "allwinnner,", this version fixes this.
> 
> Changes in v3: Use proper "allwinnner," for the compatible string and vendor
> specific properties.
> 
> Changes in v2: Changed the simplefb-sunxi bindings to use a single
> sunxi,pipeline string property instead of a sunxi,pipeline int and a
> sunxi,output string property.
> 
> This patch set is all acked up, and a fix to the binding changes you've
> already queued for 3.19, so please queue these 2 patches for 3.19.

The discussions about this have continued in the earlier version of this
series. Is this series still ok for everyone?

The series looks fine to me, and has Grant's acks, so I'll merge it
tomorrow to fbdev tree if no one objects (and my comment for the patch 2
about sunxi/allwinner was just my ignorance).

 Tomi



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

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

[Hans de Goede]

Hi,

On 11/25/2014 01:32 PM, Tomi Valkeinen wrote:
> On 18/11/14 13:10, 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>
>> Acked-by: Grant Likely <grant.likely@linaro.org>
>> ---
>>   .../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..c46ba64
>> --- /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 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:
>
> Sorry my ignorance, but what's sunxi and what's allwinner? Both names
> are mixed here.

sunxi is the sun#i SoCs from Allwinner, Allwinner is the manufacturer and the
SoC "code" names used everywhere in the kernel for their SoCs are sun4i, sun5i,
sun6i, etc. Most people refer to these SoCs as sunxi. This is also what the
linux-sunxi mailinglist in the Cc is about.

The official devicetree vendor prefix for Allwinner is allwinner, hence the
allwinner in the compatible name, see e.g. also

Documentation/devicetree/bindings/input/sun4i-lradc-keys.txt

Which also uses sunxi / sun4i everywhere except in the compatible vendor prefix.

Regards,

Hans

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

[Tomi Valkeinen]

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

On 25/11/14 14:52, Hans de Goede wrote:

>>> +Required properties:
>>> +- compatible: "allwinner,simple-framebuffer"
>>> +- allwinner,pipeline, one of:
>>
>> Sorry my ignorance, but what's sunxi and what's allwinner? Both names
>> are mixed here.
> 
> sunxi is the sun#i SoCs from Allwinner, Allwinner is the manufacturer
> and the
> SoC "code" names used everywhere in the kernel for their SoCs are sun4i,
> sun5i,
> sun6i, etc. Most people refer to these SoCs as sunxi. This is also what the
> linux-sunxi mailinglist in the Cc is about.
> 
> The official devicetree vendor prefix for Allwinner is allwinner, hence the
> allwinner in the compatible name, see e.g. also
> 
> Documentation/devicetree/bindings/input/sun4i-lradc-keys.txt
> 
> Which also uses sunxi / sun4i everywhere except in the compatible vendor
> prefix.

Alright, thanks for explanation.

Shouldn't the compatible then be "allwinner,sunxi-simple-framebuffer",
to differentiate from some other SoC Allwinner has or might create in
the future? That is, presuming you're confident enough that a single
compatible string covers all the current and forthcoming sunxi SoCs.

Perhaps simplefb is a bit special case, but I usually feel better if the
compatible string is defined in a more specific manner. In this case I'd
have:

allwinner,sun4i-simple-framebuffer
allwinner,sun5i-simple-framebuffer
allwinner,sun6i-simple-framebuffer

so that if sun7i has totally different display controller, there would
be no conflict.

 Tomi



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

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

[Hans de Goede]

Hi,

On 11/25/2014 02:02 PM, Tomi Valkeinen wrote:
> On 25/11/14 14:52, Hans de Goede wrote:
>
>>>> +Required properties:
>>>> +- compatible: "allwinner,simple-framebuffer"
>>>> +- allwinner,pipeline, one of:
>>>
>>> Sorry my ignorance, but what's sunxi and what's allwinner? Both names
>>> are mixed here.
>>
>> sunxi is the sun#i SoCs from Allwinner, Allwinner is the manufacturer
>> and the
>> SoC "code" names used everywhere in the kernel for their SoCs are sun4i,
>> sun5i,
>> sun6i, etc. Most people refer to these SoCs as sunxi. This is also what the
>> linux-sunxi mailinglist in the Cc is about.
>>
>> The official devicetree vendor prefix for Allwinner is allwinner, hence the
>> allwinner in the compatible name, see e.g. also
>>
>> Documentation/devicetree/bindings/input/sun4i-lradc-keys.txt
>>
>> Which also uses sunxi / sun4i everywhere except in the compatible vendor
>> prefix.
>
> Alright, thanks for explanation.
>
> Shouldn't the compatible then be "allwinner,sunxi-simple-framebuffer",
> to differentiate from some other SoC Allwinner has or might create in
> the future? That is, presuming you're confident enough that a single
> compatible string covers all the current and forthcoming sunxi SoCs.

This was discussed in an earlier thread, we (Ian Campbell, Grant and me)
decided to settle on allwinner,simple-framebuffer to make it clear that
these are allwinner extensions to the standard simple-framebuffer bindings,
and that the node otherwise is simple-framebuffer compatible.

We were afraid that e.g. sun4i-simple-framebuffer would signal that it
is not a normal simple-framebuffer node, so we decided to go with just
the allwinner, prefix to indicate that it uses allwinner specific
extensions.

Regards,

Hans

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

[Tomi Valkeinen]

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

On 25/11/14 15:21, Hans de Goede wrote:

>> Shouldn't the compatible then be "allwinner,sunxi-simple-framebuffer",
>> to differentiate from some other SoC Allwinner has or might create in
>> the future? That is, presuming you're confident enough that a single
>> compatible string covers all the current and forthcoming sunxi SoCs.
> 
> This was discussed in an earlier thread, we (Ian Campbell, Grant and me)

Okay. Sorry for not having time at the moment to follow the discussions
properly. =)

> decided to settle on allwinner,simple-framebuffer to make it clear that
> these are allwinner extensions to the standard simple-framebuffer bindings,
> and that the node otherwise is simple-framebuffer compatible.
> 
> We were afraid that e.g. sun4i-simple-framebuffer would signal that it
> is not a normal simple-framebuffer node, so we decided to go with just
> the allwinner, prefix to indicate that it uses allwinner specific
> extensions.

Wouldn't

compatible = "allwinner,sun4i-simple-framebuffer", "simple-framebuffer";

tell that it's a simple-framebuffer, with allwinner's sun4i extensions?

I guess you can have just "allwinner,simple-framebuffer", and then if a
new Allwinner SoC has a totally different display controller, the
documentation would specify that this property is for that SoC, and this
another property is for that another SoC. But isn't the compatible
string what's supposed to use in cases like this?

And if the new SoC is not sunxi, but some totally other family, there's
need for a new compatible string anyway, as
"simple-framebuffer-sunxi.txt" is for sunxi only.

 Tomi



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

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

[Hans de Goede]

Hi,

On 11/25/2014 02:38 PM, Tomi Valkeinen wrote:
> On 25/11/14 15:21, Hans de Goede wrote:
>
>>> Shouldn't the compatible then be "allwinner,sunxi-simple-framebuffer",
>>> to differentiate from some other SoC Allwinner has or might create in
>>> the future? That is, presuming you're confident enough that a single
>>> compatible string covers all the current and forthcoming sunxi SoCs.
>>
>> This was discussed in an earlier thread, we (Ian Campbell, Grant and me)
>
> Okay. Sorry for not having time at the moment to follow the discussions
> properly. =)
>
>> decided to settle on allwinner,simple-framebuffer to make it clear that
>> these are allwinner extensions to the standard simple-framebuffer bindings,
>> and that the node otherwise is simple-framebuffer compatible.
>>
>> We were afraid that e.g. sun4i-simple-framebuffer would signal that it
>> is not a normal simple-framebuffer node, so we decided to go with just
>> the allwinner, prefix to indicate that it uses allwinner specific
>> extensions.
>
> Wouldn't
>
> compatible = "allwinner,sun4i-simple-framebuffer", "simple-framebuffer";
>
> tell that it's a simple-framebuffer, with allwinner's sun4i extensions?
>
> I guess you can have just "allwinner,simple-framebuffer", and then if a
> new Allwinner SoC has a totally different display controller, the
> documentation would specify that this property is for that SoC, and this
> another property is for that another SoC. But isn't the compatible
> string what's supposed to use in cases like this?

The only soc specific thing in the binding is the pipeline property string
values, and we can always add new values to that, the rest is all generic,
as simplefb is generic.

As said Ian Campbell, Grant and me have decided on using this, and currently
patches are already queued up for both the dts files and u-boot to use this,
so unless there are really strong reasons to change it at this point I would
prefer to keep this as is.

Regards,

Hans

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

[Tomi Valkeinen]

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

On 25/11/14 15:45, Hans de Goede wrote:

>> Wouldn't
>>
>> compatible = "allwinner,sun4i-simple-framebuffer", "simple-framebuffer";
>>
>> tell that it's a simple-framebuffer, with allwinner's sun4i extensions?
>>
>> I guess you can have just "allwinner,simple-framebuffer", and then if a
>> new Allwinner SoC has a totally different display controller, the
>> documentation would specify that this property is for that SoC, and this
>> another property is for that another SoC. But isn't the compatible
>> string what's supposed to use in cases like this?
> 
> The only soc specific thing in the binding is the pipeline property string
> values, and we can always add new values to that, the rest is all generic,
> as simplefb is generic.

The thing I don't understand is that the compatible string states that
"this covers all Allwinner SoCs", even if we have no idea what kind of
SoCs those may be. And if it covers all kinds of SoCs, then it might as
well be fully generic, not Allwinner specific.

And if it's not fully generic, then having it cover all possible
Allwinner SoCs doesn't make sense either.

> As said Ian Campbell, Grant and me have decided on using this, and
> currently
> patches are already queued up for both the dts files and u-boot to use
> this,
> so unless there are really strong reasons to change it at this point I
> would
> prefer to keep this as is.

Ok. Well, as I said, it does not look correct to me, but if everybody
else agrees on it (and I see I didn't get any replies during the night),
I'll be applying this today.

 Tomi



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