Hello Inki,

On 12/03/2015 06:30 AM, Inki Dae wrote:
> This patch updates a ports node binding for panel.
> 
> With this, dp node can have a ports node which describes
> a remote endpoint node that can be connected to panel or bridge
> node.
> 
> Signed-off-by: Inki Dae <inki.dae at samsung.com>
> ---
>  .../bindings/display/exynos/exynos_dp.txt          | 28 
> ++++++++++++++++++----
>  1 file changed, 24 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/exynos/exynos_dp.txt 
> b/Documentation/devicetree/bindings/display/exynos/exynos_dp.txt
> index 64693f2..15b52cb 100644
> --- a/Documentation/devicetree/bindings/display/exynos/exynos_dp.txt
> +++ b/Documentation/devicetree/bindings/display/exynos/exynos_dp.txt
> @@ -66,8 +66,15 @@ Optional properties for dp-controller:
>               Hotplug detect GPIO.
>                       Indicates which GPIO should be used for hotplug
>                       detection
> -     -video interfaces: Device node can contain video interface port
> -                         nodes according to [1].
> +Video interfaces:
> +  Device node can contain video interface port nodes according to [1].
> +  The following are properties specific to those nodes:
> +
> +  endpoint node connected to bridge or panel node:
> +   - remote-endpoint: specifies the endpoint in panel or bridge node.
> +                   This node is required in all kinds of exynos dp
> +                   to represent the connection between dp and bridge
> +                   ,or dp and panel.
>

This is nice but I think the DT binding should also document that it uses
a phandle to define the connection with the panel (but explain that is
deprecated). If only so people looking at a DTS and then going to the DT
binding can understand why there is two ways to define the same.

>  [1]: Documentation/devicetree/bindings/media/video-interfaces.txt
>  
> @@ -111,9 +118,22 @@ Board Specific portion:
>               };
>  
>               ports {
> -                     port at 0 {
> +                     #address-cells = <1>;
> +                     #size-cells = <0>;
> +

These two properties are only needed when there is more than 2 ports and
a reg property is used to number the port nodes but I don't think that's
the case for Exynos DP and certainly is not the case in this example so
I think you should just remove them.

> +                     port {
>                               dp_out: endpoint {
> -                                     remote-endpoint = <&bridge_in>;
> +                                     remote-endpoint = <&dp_in>;
> +                             };
> +                     };
> +             };
> +
> +             panel at 0 {
> +                     reg = <0>;
> +                     ...
> +                     port {
> +                             dp_in: endpoint {
> +                                     remote-endpoint = <&dp_out>;
>                               };
>                       };
>               };
> 

The rest looks good to me so with the two changes feel free to add:

Reviewed-by: Javier Martinez Canillas <javier at osg.samsung.com>

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Reply via email to