On Wed, Sep 17, 2014 at 07:22:05PM +0300, Tomi Valkeinen wrote:
> On 17/09/14 17:29, Ajay kumar wrote:
> > Hi Tomi,
> >
> > Thanks for your comments.
> >
> > On Wed, Sep 17, 2014 at 5:22 PM, Tomi Valkeinen <tomi.valkeinen@xxxxxx> wrote:
> >> On 27/08/14 17:39, Ajay Kumar wrote:
> >>> Add documentation for DT properties supported by ps8622/ps8625
> >>> eDP-LVDS converter.
> >>>
> >>> Signed-off-by: Ajay Kumar <ajaykumar.rs@xxxxxxxxxxx>
> >>> ---
> >>> .../devicetree/bindings/video/bridge/ps8622.txt | 20 ++++++++++++++++++++
> >>> 1 file changed, 20 insertions(+)
> >>> create mode 100644 Documentation/devicetree/bindings/video/bridge/ps8622.txt
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/video/bridge/ps8622.txt b/Documentation/devicetree/bindings/video/bridge/ps8622.txt
> >>> new file mode 100644
> >>> index 0000000..0ec8172
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/video/bridge/ps8622.txt
> >>> @@ -0,0 +1,20 @@
> >>> +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 for PD_ pin.
> >>> + - reset-gpios: OF device-tree gpio specification for RST_ pin.
> >>> +
> >>> +Optional properties:
> >>> + - lane-count: number of DP lanes to use
> >>> + - use-external-pwm: backlight will be controlled by an external PWM
> >>
> >> What does this mean? That the backlight support from ps8625 is not used?
> >> If so, maybe "disable-pwm" or something?
> > "use-external-pwm" or "disable-bridge-pwm" would be better.
>
> Well, the properties are about the bridge. "use-external-pwm" means that
> the bridge uses an external PWM, which, if I understood correctly, is
> not what the property is about.
>
> "disable-bridge-pwm" is ok, but the "bridge" there is extra. The
> properties are about the bridge, so it's implicit.
>
> >>> +
> >>> +Example:
> >>> + lvds-bridge@48 {
> >>> + compatible = "parade,ps8622";
> >>> + reg = <0x48>;
> >>> + sleep-gpios = <&gpc3 6 1 0 0>;
> >>> + reset-gpios = <&gpc3 1 1 0 0>;
> >>> + lane-count = <1>;
> >>> + };
> >>>
> >>
> >> I wish all new display component bindings would use the video
> >> ports/endpoints to describe the connections. It will be very difficult
> >> to improve the display driver model later if we're missing such critical
> >> pieces from the DT bindings.
> > Why do we need a complex graph when it can be handled using a simple phandle?
>
> Maybe in your case you can handle it with simple phandle. Can you
> guarantee that it's enough for everyone, on all platforms?
Nobody can guarantee that. An interesting effect that DT ABI stability
has had is that people now try to come up with overly generic concepts
to describe devices in an attempt to make them more future-proof. I
don't think we're doing ourselves a favour with that approach.
> The point of the ports/endpoint graph is to also support more
> complicated scenarios.
But the scenario will always remain the same for this bridge. There will
always be an input signal and a translation to some output signal along
with some parameters to control how that translation happens. More
complicated scenarios will likely need to be represented differently at
a higher level than the bridge.
> If you now create ps8622 bindings that do not
> support those graphs, the no one else using ps8622 can use
> ports/endpoint graphs either.
That's not true. The binding can easily be extended in a backwards-
compatible way later on (should it really prove necessary).
Thierry
Attachment:
pgpDSDTbSV0kJ.pgp
Description: PGP signature
