[PATCH 3/3] dt-bindings: exynos-dp: update ports node binding for panel

2015-12-04 Thread Inki Dae
Hi Rob,

2015년 12월 04일 08:38에 Rob Herring 이(가) 쓴 글:
> On Thu, Dec 03, 2015 at 06:30:10PM +0900, 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 
>> ---
>>  .../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.
> 
> Fix your punctuation.

Right. 

> 
>>  
>>  [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>;
> 
> You don't need these.

Ditto.

> 
>> +
>> +port {
>>  dp_out: endpoint {
>> -remote-endpoint = <_in>;
>> +remote-endpoint = <_in>;
>> +};
>> +};
>> +};
>> +
>> +panel at 0 {
>> +reg = <0>;
> 
> Drop the @0 and reg as you only have 1.

Ditto.

Thanks,
Inki Dae

> 
>> +...
>> +port {
>> +dp_in: endpoint {
>> +remote-endpoint = <_out>;
>>  };
>>  };
>>  };
>> -- 
>> 1.9.1
>>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 


[PATCH 3/3] dt-bindings: exynos-dp: update ports node binding for panel

2015-12-04 Thread Inki Dae
Hi Javier,

2015년 12월 03일 22:29에 Javier Martinez Canillas 이(가) 쓴 글:
> 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 
>> ---
>>  .../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.

Right. I found out that the dp can have only one port outbound. Will remove 
them.

> 
>> +port {
>>  dp_out: endpoint {
>> -remote-endpoint = <_in>;
>> +remote-endpoint = <_in>;
>> +};
>> +};
>> +};
>> +
>> +panel at 0 {
>> +reg = <0>;
>> +...
>> +port {
>> +dp_in: endpoint {
>> +remote-endpoint = <_out>;
>>  };
>>  };
>>  };
>>
> 
> The rest looks good to me so with the two changes feel free to add:
> 
> Reviewed-by: Javier Martinez Canillas 

Thanks,
Inki Dae

> 
> Best regards,
> 


[PATCH 3/3] dt-bindings: exynos-dp: update ports node binding for panel

2015-12-03 Thread Inki Dae
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 
---
 .../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.

 [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>;
+
+   port {
dp_out: endpoint {
-   remote-endpoint = <_in>;
+   remote-endpoint = <_in>;
+   };
+   };
+   };
+
+   panel at 0 {
+   reg = <0>;
+   ...
+   port {
+   dp_in: endpoint {
+   remote-endpoint = <_out>;
};
};
};
-- 
1.9.1



[PATCH 3/3] dt-bindings: exynos-dp: update ports node binding for panel

2015-12-03 Thread Rob Herring
On Thu, Dec 03, 2015 at 06:30:10PM +0900, 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 
> ---
>  .../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.

Fix your punctuation.

>  
>  [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>;

You don't need these.

> +
> + port {
>   dp_out: endpoint {
> - remote-endpoint = <_in>;
> + remote-endpoint = <_in>;
> + };
> + };
> + };
> +
> + panel at 0 {
> + reg = <0>;

Drop the @0 and reg as you only have 1.

> + ...
> + port {
> + dp_in: endpoint {
> + remote-endpoint = <_out>;
>   };
>   };
>   };
> -- 
> 1.9.1
> 


[PATCH 3/3] dt-bindings: exynos-dp: update ports node binding for panel

2015-12-03 Thread Javier Martinez Canillas
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 
> ---
>  .../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 = <_in>;
> + remote-endpoint = <_in>;
> + };
> + };
> + };
> +
> + panel at 0 {
> + reg = <0>;
> + ...
> + port {
> + dp_in: endpoint {
> + remote-endpoint = <_out>;
>   };
>   };
>   };
> 

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

Reviewed-by: Javier Martinez Canillas 

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