diff mbox

[V5,11/12] Documentation: Add DT bindings for ps8622/ps8625 bridge driver

Message ID 1405629839-12086-12-git-send-email-ajaykumar.rs@samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ajay Kumar July 17, 2014, 8:43 p.m. UTC
From: Vincent Palatin <vpalatin@chromium.org>

Add DT binding documentation for ps8622/ps8625 bridge driver.

Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
---
 .../devicetree/bindings/drm/bridge/ps8622.txt      |   21 ++++++++++++++++++++
 1 file changed, 21 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/drm/bridge/ps8622.txt

Comments

Ajay kumar July 17, 2014, 8:51 p.m. UTC | #1
+devicetree@vger.kernel.org

On Fri, Jul 18, 2014 at 2:13 AM, Ajay Kumar <ajaykumar.rs@samsung.com> wrote:
> From: Vincent Palatin <vpalatin@chromium.org>
>
> Add DT binding documentation for ps8622/ps8625 bridge driver.
>
> Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
> Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
> ---
>  .../devicetree/bindings/drm/bridge/ps8622.txt      |   21 ++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/drm/bridge/ps8622.txt
>
> diff --git a/Documentation/devicetree/bindings/drm/bridge/ps8622.txt b/Documentation/devicetree/bindings/drm/bridge/ps8622.txt
> new file mode 100644
> index 0000000..1d154ac
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/drm/bridge/ps8622.txt
> @@ -0,0 +1,21 @@
> +ps8622-bridge bindings
> +
> +Required properties:
> +       - compatible: "parade,ps8622" or "parade,ps8625"
> +       - reg: first i2c address of the bridge
> +       - sleep-gpios: OF device-tree gpio specification
> +       - reset-gpios: OF device-tree gpio specification
> +
> +Optional properties:
> +       - lane-count: number of DP lanes to use
> +       - use-external-pwm: backlight will be controlled by an external PWM
> +
> +Example:
> +       ps8622-bridge@48 {
> +               compatible = "parade,ps8622";
> +               reg = <0x48>;
> +               sleep-gpios = <&gpc3 6 1 0 0>;
> +               reset-gpios = <&gpc3 1 1 0 0>;
> +               display-timings = <&lcd_display_timings>;
> +               lane-count = <1>
> +       };
> --
> 1.7.9.5
>
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Thierry Reding July 21, 2014, 7:06 a.m. UTC | #2
On Fri, Jul 18, 2014 at 02:13:57AM +0530, Ajay Kumar wrote:
> From: Vincent Palatin <vpalatin@chromium.org>
> 
> Add DT binding documentation for ps8622/ps8625 bridge driver.
> 
> Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
> Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
> ---
>  .../devicetree/bindings/drm/bridge/ps8622.txt      |   21 ++++++++++++++++++++

This is the wrong directory. Bindings are supposed to be OS agnostic,
but DRM is (mostly) Linux-specific. video/bridge would be a better
subdirectory for this. Somebody really ought to be moving out the
existing bindings in the drm subdirectory to a more proper location.

>  1 file changed, 21 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/drm/bridge/ps8622.txt
> 
> diff --git a/Documentation/devicetree/bindings/drm/bridge/ps8622.txt b/Documentation/devicetree/bindings/drm/bridge/ps8622.txt
> new file mode 100644
> index 0000000..1d154ac
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/drm/bridge/ps8622.txt
> @@ -0,0 +1,21 @@
> +ps8622-bridge bindings
> +
> +Required properties:
> +	- compatible: "parade,ps8622" or "parade,ps8625"

Documentation/devicetree/bindings/vendor-prefixes.txt doesn't contain an
entry for parade yet.

> +	- reg: first i2c address of the bridge
> +	- sleep-gpios: OF device-tree gpio specification
> +	- reset-gpios: OF device-tree gpio specification
> +
> +Optional properties:
> +	- lane-count: number of DP lanes to use
> +	- use-external-pwm: backlight will be controlled by an external PWM

In case of an external backlight, don't you still need a way to control
it? Perhaps instead of using this boolean property you should make this
take a phandle to the real backlight? Like so:

	backlight {
		compatible = "pwm-backlight";
		...
	}

	bridge@48 {
		compatible = "parade,ps8622";
		...
		backlight = <&/backlight>;
	}

Then you can simply look up the backlight device when that property
exists and instantiate the built-in backlight otherwise.

> +
> +Example:
> +	ps8622-bridge@48 {
> +		compatible = "parade,ps8622";
> +		reg = <0x48>;
> +		sleep-gpios = <&gpc3 6 1 0 0>;
> +		reset-gpios = <&gpc3 1 1 0 0>;
> +		display-timings = <&lcd_display_timings>;

Why is this specifying display-timings? It's not mentioned in the
binding above and I don't see why the bridge would need to provide
one since it's really a property of the panel.

Thierry
Ajay kumar July 21, 2014, 10:54 a.m. UTC | #3
Hi Thierry,

On Mon, Jul 21, 2014 at 12:36 PM, Thierry Reding
<thierry.reding@gmail.com> wrote:
> On Fri, Jul 18, 2014 at 02:13:57AM +0530, Ajay Kumar wrote:
>> From: Vincent Palatin <vpalatin@chromium.org>
>>
>> Add DT binding documentation for ps8622/ps8625 bridge driver.
>>
>> Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
>> Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
>> ---
>>  .../devicetree/bindings/drm/bridge/ps8622.txt      |   21 ++++++++++++++++++++
>
> This is the wrong directory. Bindings are supposed to be OS agnostic,
> but DRM is (mostly) Linux-specific. video/bridge would be a better
> subdirectory for this. Somebody really ought to be moving out the
> existing bindings in the drm subdirectory to a more proper location.
Ok. I will move them out into video/bridge.

>>  1 file changed, 21 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/drm/bridge/ps8622.txt
>>
>> diff --git a/Documentation/devicetree/bindings/drm/bridge/ps8622.txt b/Documentation/devicetree/bindings/drm/bridge/ps8622.txt
>> new file mode 100644
>> index 0000000..1d154ac
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/drm/bridge/ps8622.txt
>> @@ -0,0 +1,21 @@
>> +ps8622-bridge bindings
>> +
>> +Required properties:
>> +     - compatible: "parade,ps8622" or "parade,ps8625"
>
> Documentation/devicetree/bindings/vendor-prefixes.txt doesn't contain an
> entry for parade yet.
I will fix this.

>> +     - reg: first i2c address of the bridge
>> +     - sleep-gpios: OF device-tree gpio specification
>> +     - reset-gpios: OF device-tree gpio specification
>> +
>> +Optional properties:
>> +     - lane-count: number of DP lanes to use
>> +     - use-external-pwm: backlight will be controlled by an external PWM
>
> In case of an external backlight, don't you still need a way to control
> it? Perhaps instead of using this boolean property you should make this
> take a phandle to the real backlight? Like so:
>
>         backlight {
>                 compatible = "pwm-backlight";
>                 ...
>         }
>
>         bridge@48 {
>                 compatible = "parade,ps8622";
>                 ...
>                 backlight = <&/backlight>;
>         }
>
> Then you can simply look up the backlight device when that property
> exists and instantiate the built-in backlight otherwise.
Thanks for the pointer, this approach seems perfect!

>> +
>> +Example:
>> +     ps8622-bridge@48 {
>> +             compatible = "parade,ps8622";
>> +             reg = <0x48>;
>> +             sleep-gpios = <&gpc3 6 1 0 0>;
>> +             reset-gpios = <&gpc3 1 1 0 0>;
>> +             display-timings = <&lcd_display_timings>;
>
> Why is this specifying display-timings? It's not mentioned in the
> binding above and I don't see why the bridge would need to provide
> one since it's really a property of the panel.
Ahh. I was trying to copy bindings from previous patchset and I messed it up.
Will fix it.

Ajay
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/drm/bridge/ps8622.txt b/Documentation/devicetree/bindings/drm/bridge/ps8622.txt
new file mode 100644
index 0000000..1d154ac
--- /dev/null
+++ b/Documentation/devicetree/bindings/drm/bridge/ps8622.txt
@@ -0,0 +1,21 @@ 
+ps8622-bridge bindings
+
+Required properties:
+	- compatible: "parade,ps8622" or "parade,ps8625"
+	- reg: first i2c address of the bridge
+	- sleep-gpios: OF device-tree gpio specification
+	- reset-gpios: OF device-tree gpio specification
+
+Optional properties:
+	- lane-count: number of DP lanes to use
+	- use-external-pwm: backlight will be controlled by an external PWM
+
+Example:
+	ps8622-bridge@48 {
+		compatible = "parade,ps8622";
+		reg = <0x48>;
+		sleep-gpios = <&gpc3 6 1 0 0>;
+		reset-gpios = <&gpc3 1 1 0 0>;
+		display-timings = <&lcd_display_timings>;
+		lane-count = <1>
+	};