Re: [PATCH v2 1/3] dt-bindings: Add doc for the ingenic-drm driver

From: Paul Cercueil
Date: Sat Mar 16 2019 - 17:58:15 EST


Hi Sam,

Le sam. 16 mars 2019 à 21:59, Sam Ravnborg <sam@xxxxxxxxxxxx> a écrit :
Hi Paul.

On Fri, Mar 15, 2019 at 06:06:59PM +0100, Paul Cercueil wrote:
Add documentation for the devicetree bindings of the DRM driver for the
JZ47xx family of SoCs from Ingenic.

Signed-off-by: Paul Cercueil <paul@xxxxxxxxxxxxxxx>
Tested-by: Artur Rojek <contact@xxxxxxxxxxxxxx>
---

Notes:
v2: Remove ingenic,panel property.

.../devicetree/bindings/display/ingenic,drm.txt | 29 ++++++++++++++++++++++
1 file changed, 29 insertions(+)
create mode 100644 Documentation/devicetree/bindings/display/ingenic,drm.txt

diff --git a/Documentation/devicetree/bindings/display/ingenic,drm.txt b/Documentation/devicetree/bindings/display/ingenic,drm.txt
new file mode 100644
index 000000000000..f57a4ae0f7ac
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/ingenic,drm.txt
@@ -0,0 +1,29 @@
+Ingenic JZ47xx DRM driver
+
+Required properties:
+- compatible: one of:
+ * ingenic,jz4740-drm
+ * ingenic,jz4725b-drm
+- reg: LCD registers location and length
+- clocks: LCD pixclock and device clock specifiers.
+ The device clock is only required on the JZ4740.
+- clock-names: "lcd_pclk" and "lcd"
+- interrupts: Specifies the interrupt line the LCD controller is connected to.
+
+Optional properties:
+- ingenic,lcd-mode: LCD mode to use with the display panel.
+ See <dt-bindings/display/ingenic,drm.h> for all the
+ possible values.

The value of the ingenic,lcd-mode depends on the panel used.
So this looks like a panel property and not a display property.

The panel should be specified using an OF graph binding
in a port sub-node.
The remote-endpoint will specifify the panel properties
and if the panel requires something special from the display driver,
the panle should specify this.
So one can change panel without touching the display node
in the DT.

Then the 'ingenic,lcd-mode' property would be in the panel node,
even though it's not used by the panel driver but by the DRM driver?
That sounds strange to me.

+
+Example:
+
+lcd: lcd-controller@13050000 {
+ compatible = "ingenic,jz4725b-drm";
+ reg = <0x13050000 0x1000>;
+
+ interrupt-parent = <&intc>;
+ interrupts = <31>;
+
+ clocks = <&cgu JZ4725B_CLK_LCD>;
+ clock-names = "lcd";
+};

As hinted above the panel is not specified above and to follow the
modern way to do so would be to use a OF graph binding.

It's done this way in the DRM driver now, but I didn't update the
example. I will update it in the next version.

-Paul