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

[Ajay kumar]

Hi Thierry,

On Mon, Jul 21, 2014 at 12:36 PM, Thierry Reding
<thierry.reding at gmail.com> wrote:
> On Fri, Jul 18, 2014 at 02:13:57AM +0530, Ajay Kumar wrote:
>> From: Vincent Palatin <vpalatin at chromium.org>
>>
>> Add DT binding documentation for ps8622/ps8625 bridge driver.
>>
>> Signed-off-by: Vincent Palatin <vpalatin at chromium.org>
>> Signed-off-by: Ajay Kumar <ajaykumar.rs at 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 at 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 at 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