Re: [PATCHv3 7/9] arm: vt8500: doc: Add device tree bindings for arch-vt8500 devices

On Wed, 2012-08-22 at 15:07 -0600, Stephen Warren wrote:

On 08/21/2012 02:47 PM, Tony Prisk wrote:

quoted

Bindings for gpio, interrupt controller, power management controller,
timer, realtime clock, serial uart, ehci and uhci controllers and
framebuffer controllers used on the arch-vt8500 platform.

Framebuffer binding also specifies a 'display' node which is required
for determining the lcd panel data.

quoted

diff --git a/Documentation/devicetree/bindings/gpio/gpio_vt8500.txt b/Documentation/devicetree/bindings/gpio/gpio_vt8500.txt

quoted

+- #gpio-cells : should be <3>.
+	1) bank
+	2) pin number
+	3) flags

Should this enumerate what legal values are for flags, or point at a
standard document that does?

There currently are no supported flags - guess this was an oversight.
I'll take a look if there are any that we might require in the future
and implement them otherwise I'll drop the flags reference.

quoted

diff --git a/Documentation/devicetree/bindings/tty/serial/via,vt8500-uart.txt b/Documentation/devicetree/bindings/tty/serial/via,vt8500-uart.txt

quoted

+	uart@d8210000 {
+		compatible = "via,vt8500-uart";
+		reg = <0xd8210000 0x1040>;
+		interrupts = <47>;
+	};

How does the UART know what frequency its clock input is, in order to
calculate dividers? Should there be a clocks property to link to the
input clock, so the rate can be queried? If so, a reference to the
common clock binding, plus a specification of which clocks must be
listed in the clock property should be included here.

I didn't revisit the code that had already been posted in v1/v2 when I
added the common clock code and there was no clock handling code at that
point. The uart's are all clocked via a clkgen that outputs 24Mhz and
this was hardcoded in the driver.
I will correct this and add the appropriate device tree entries and
documentation.

quoted

diff --git a/Documentation/devicetree/bindings/video/via,vt8500-fb.txt b/Documentation/devicetree/bindings/video/via,vt8500-fb.txt

quoted

+VIA VT8500 Display
+-----------------------------------------------------
+Required properties:
+- xres : lcd panel horizontal resolution
+- yres : lcd panel vertical resolution
+- left-margin,
+- right-margin,
+- hsync-len: lcd panel horizontal timings in pixels
+- upper-margin,
+- lower-margin,
+- vsync-len: lcd panel verticals timings in pixels
+- bpp: lcd panel bit-depth.
+	<16> for RGB565, <32> for RGB888

Shouldn't this reference Sascha Hauer's binding document (although I
suppose it isn't checked in yet), and just document the additions? I
wonder if this binding should be written assuming Sascha's binding doc
will be checked in?

I haven't seen Sascha's binding yet - I'm not even sure its been agreed
upon but it does seem to be the most likely candidate so far. Hopefully
it will get accepted in this window, and I will have time to do exactly
as you commented, which was the plan from the start. This binding was
based on the most current one I had seen suggested at the time.


Given the additional comments for this patch set, and the required
changes I'll hold off posting v4 until everything is tidied up.

Regards

Tony Prisk