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.rs@xxxxxxxxxxx>
> ---
> .../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@xxxxxxxxxxxxxxx/msg52743.html
[2] http://www.mail-archive.com/linux-media@xxxxxxxxxxxxxxx/msg53323.html
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
