Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties

From: Thierry Reding <thierry.reding@gmail.com>
To: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: devicetree@vger.kernel.org, linux-samsung-soc@vger.kernel.org,
	seanpaul@google.com, daniel.vetter@ffwll.ch, joshi@samsung.com,
	dri-devel@lists.freedesktop.org,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	ajaynumb@gmail.com, prashanth.g@samsung.com,
	Ajay Kumar <ajaykumar.rs@samsung.com>
Subject: Re: [PATCH V7 11/12] Documentation: bridge: Add documentation for ps8622 DT properties
Date: Mon, 22 Sep 2014 09:54:25 +0200	[thread overview]
Message-ID: <20140922075424.GB1470@ulmo> (raw)
In-Reply-To: <5419760A.7020908@ti.com>

[-- Attachment #1.1: Type: text/plain, Size: 2711 bytes --]

On Wed, Sep 17, 2014 at 02:52:42PM +0300, Tomi Valkeinen 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@samsung.com>
> > ---
> >  .../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?
> 
> > +
> > +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.

I disagree. Why would we want to burden all devices with a bloated
binding and drivers with parsing a complex graph when it's not even
known that it will be necessary? Evidently this device works fine
using the current binding. Just because there are bindings to describe
ports in a generic way doesn't mean it has to be applied everywhere.
After all the concept of ports/endpoints applies to non-video devices
too, yet we don't require the bindings for those devices to add ports
or endpoints nodes.

Also it won't be very difficult to extend the binding in a backwards
compatible way if that becomes necessary.

One thing that I'd like to see in this binding, though, is how to hook
up the bridge to a panel. However I'm still catching up on mail after
vacation, so perhaps this has already been discussed further down the
thread.

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel