Hi Ajay, On 10/10/2012 01:08 AM, Ajay Kumar wrote: > Add documentation for the DT bindings in exynos display port driver. > > Signed-off-by: Ajay Kumar<ajaykumar...@samsung.com> > --- > .../devicetree/bindings/video/exynos_dp.txt | 83 > ++++++++++++++++++++ > 1 files changed, 83 insertions(+), 0 deletions(-) > create mode 100644 Documentation/devicetree/bindings/video/exynos_dp.txt > > diff --git a/Documentation/devicetree/bindings/video/exynos_dp.txt > b/Documentation/devicetree/bindings/video/exynos_dp.txt > new file mode 100644 > index 0000000..a021963 > --- /dev/null > +++ b/Documentation/devicetree/bindings/video/exynos_dp.txt > @@ -0,0 +1,83 @@ > +Exynos display port driver should configure the display port interface > +based on the type of panel connected to it.
The bindings are supposed to describe devices, not drivers. So it might be better to say: "The Exynos display port interface should be configured based on the type of panel connected to it." >From this documentation it is not clear which properties are required and which are optional for each node. I think, in the property names dashes should be used, rather than underscores. Dashes are much more common among existing bindings. > +We use two nodes: > + -dptx_phy node > + -display-port-controller node > + > +For the dp-phy initialization, we use a dptx_phy node. > +Required properties for dptx_phy: > + -compatible: > + Should be "samsung,dp-phy". Is there a separate dptx-phy driver that is being matched with this compatible property ? Is the dptx-node going to be referenced by other nodes than display-port-controller ? If not, then you could likely drop the compatible property entirely and make the dptx-phy node a child node of display-port-controller, i.e. display-port-controller { compatible = "samsung,exynos5-dp"; reg = <0x145b0000 0x10000>; interrupts =<10 3>; interrupt-parent =<&combiner>; dptx-phy { reg = <0x10040720>; samsung,enable_mask = <1>; }; }; Then the driver could just look for a child node named "dptx-phy" with e.g. of_find_node_by_name(). > + -samsung,dptx_phy_reg: I think it's fine to use just 'reg' instead of this vendor specific name. > + Base address of DP PHY register. > + -samsung,enable_mask: > + The bit-mask used to enable/disable DP PHY. > + > +For the Panel initialization, we read data from display-port-controller node. > +Required properties for display-port-controller: > + -compatible: > + Should be "samsung,exynos5-dp". > + -reg: > + physical base address of the controller and length > + of memory mapped region. > + -interrupts: > + Interrupt combiner values. > + -interrupt-parent: > + phandle to Interrupt combiner node. > + -samsung,dp_phy: > + phandle to dptx_phy node. > + -samsung,color_space: > + input video data format. > + COLOR_RGB = 0, COLOR_YCBCR422 = 1, COLOR_YCBCR444 = 2 > + -samsung,dynamic_range: > + dynamic range for input video data. > + VESA = 0, CEA = 1 > + -samsung,ycbcr_coeff: > + YCbCr co-efficients for input video. > + COLOR_YCBCR601 = 0, COLOR_YCBCR709 = 1 > + -samsung,color_depth: > + Number of bits per colour component. > + COLOR_6 = 0, COLOR_8 = 1, COLOR_10 = 2, COLOR_12 = 3 > + -samsung,link_rate: > + link rate supported by the panel. > + LINK_RATE_1_62GBPS = 0x6, LINK_RATE_2_70GBPS = 0x0A > + -samsung,lane_count: > + number of lanes supported by the panel. > + LANE_COUNT1 = 1, LANE_COUNT2 = 2, LANE_COUNT4 = 4 > + -samsung,interlaced: > + Interlace scan mode. > + Progressive if defined, Interlaced if not defined > + -samsung,v_sync_polarity: > + VSYNC polarity configuration. > + High if defined, Low if not defined > + -samsung,h_sync_polarity: > + HSYNC polarity configuration. > + High if defined, Low if not defined So there is no common video bindings for things like these two ? In V4L2 we decided to use vsync-active, hsync-active [1], the video timings bindings [2] use hsync-active-high, hsync-active-high boolean properties. Perhaps it is worth to pick some of those standard definitions and use instead of the vendor specific ones ? > + > +Example: > + > +SOC specific portion: > + dptx_phy: dptx_phy@0x10040720 { > + compatible = "samsung,dp-phy"; > + samsung,dptx_phy_reg =<0x10040720>; reg = <0x10040720>; > + samsung,enable_mask =<1>; > + }; > + > + display-port-controller { > + compatible = "samsung,exynos5-dp"; > + reg =<0x145B0000 0x10000>; I think lower case is preferred. > + interrupts =<10 3>; > + interrupt-parent =<&combiner>; > + samsung,dp_phy =<&dptx_phy>; > + }; > + > +Board Specific portion: > + display-port-controller { > + samsung,color_space =<0>; > + samsung,dynamic_range =<0>; > + samsung,ycbcr_coeff =<0>; > + samsung,color_depth =<1>; > + samsung,link_rate =<0x0a>; > + samsung,lane_count =<2>; > + }; Thanks, Sylwester [1] http://www.mail-archive.com/linux-media@vger.kernel.org/msg52743.html [2] http://www.mail-archive.com/linux-media@vger.kernel.org/msg53323.html _______________________________________________ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss