Re: [PATCH v3 1/2] video: ARM CLCD: Add DT support

On Mon, 2013-09-16 at 20:52 +0100, Stephen Warren wrote:

I think the binding should either always use names as the key, or use
indices in interrupts as the key. Hence, I'd word that more like:

- interrupt-names: either the single entry "combined" representing a
			combined interrupt output (CLCDINTR), or the
			four entries "mbe", "vcomp", "lnbu", "fuf"
			representing the individual CLCDMBEINTR,
			CLCDVCOMPINTR, CLCDLNBUINTR, CLCDFUFINTR
			interrupts.
- interrupts: contains an interrupt specifier for each entry in
		 interrupt-names.

Cool, works for me.

quoted

+- arm,pl11x,panel-data-pads: array of 24 cells, each of them describing
+				a function of one of the CLD pads,
+				starting from 0 up to 23; each pad can
+				be described by one of the following values:
+	- 0: reserved (not connected)
+	- 0x100-0x107: color upper STN panel data 0 to 7

...

I assume those are the raw values that go into the HW?

quoted

+				Example sets of values for standard
+				panel interfaces:
+	- PL110 single colour STN panel:
+			<0x107 0x106 0x105 0x104 0x103 0x102 0x101 0x100>,
+			<0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0>;

The indentation of the introductory text seems a little odd. 

It follows the first paragraph of this property description. But I may
re-format all this stuff. Really looking forward some the formal
syntax :-)

Do we really need so many examples?

They cover all standard cases. If I was to write a tree for a new
platform not knowing anything about CLCD, I think I'd appreciate this
and don't believe this extra kB or two is a problem. Do you?

quoted

+Optional properties:
+
+- arm,pl11x,framebuffer-base: a pair of two values, address and size,
+				defining the framebuffer to be used;
+				to be used only if it is *not*
+				part of normal memory, as described
+				in /memory node

If the framebuffer is part of /memory, what happens then? Is the address
not fixed (so the HW isn't yet set up) and hence a driver should
allocate it?

Yes, if it wants to display anything :-) And as this is a normal and
expected behaviour, I don't think it deserves a note in the
documentation. I'm open to any suggestions that would make the wording
above emphasize the "weirdness" of situations requiring the property.

Paweł