Re: [Patch v2 2/6] dt-bindings: display/ti: Move common dispc bindings to omap-dss.txt

From: Benoit Parrot <bparrot@ti.com>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Peter Ujfalusi <peter.ujfalusi@ti.com>,
	devicetree@vger.kernel.org,
	Tomi Valkeinen <tomi.valkeinen@ti.com>,
	Jyri Sarha <jsarha@ti.com>,
	dri-devel@lists.freedesktop.org
Subject: Re: [Patch v2 2/6] dt-bindings: display/ti: Move common dispc bindings to omap-dss.txt
Date: Fri, 27 Apr 2018 08:26:19 -0500	[thread overview]
Message-ID: <20180427132619.GA28125@ti.com> (raw)
In-Reply-To: <2666897.Gcu4TnYl7p@avalon>

Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote on Wed [2018-Apr-04 17:29:59 +0300]:
> Hi Benoit,
> 
> Thank you for the patch.
> 
> On Monday, 26 March 2018 19:21:24 EEST Benoit Parrot wrote:
> > Add common DISPC bindings into the top level bindings file.
> > Move common bindings here instead of having multiple copies of
> > the same information in all of the variant specific files.
> > 
> > Signed-off-by: Benoit Parrot <bparrot@ti.com>
> > Reviewed-by: Rob Herring <robh@kernel.org>
> > ---
> >  Documentation/devicetree/bindings/display/ti/ti,dra7-dss.txt  | 5 -----
> >  Documentation/devicetree/bindings/display/ti/ti,omap-dss.txt  | 7 +++++++
> >  Documentation/devicetree/bindings/display/ti/ti,omap2-dss.txt | 4 ----
> >  Documentation/devicetree/bindings/display/ti/ti,omap3-dss.txt | 4 ----
> >  Documentation/devicetree/bindings/display/ti/ti,omap4-dss.txt | 4 ----
> >  Documentation/devicetree/bindings/display/ti/ti,omap5-dss.txt | 4 ----
> >  6 files changed, 7 insertions(+), 21 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/display/ti/ti,dra7-dss.txt
> > b/Documentation/devicetree/bindings/display/ti/ti,dra7-dss.txt index
> > 91279f1060fe..c30f9ec189ed 100644
> > --- a/Documentation/devicetree/bindings/display/ti/ti,dra7-dss.txt
> > +++ b/Documentation/devicetree/bindings/display/ti/ti,dra7-dss.txt
> > @@ -47,11 +47,6 @@ Required properties:
> >  - clocks: handle to fclk
> >  - clock-names: "fck"
> > 
> > -Optional properties:
> > -- max-memory-bandwidth: Input memory (from main memory to dispc) bandwidth
> > limit -			in bytes per second
> > -
> > -
> >  HDMI
> >  ----
> > 
> > diff --git a/Documentation/devicetree/bindings/display/ti/ti,omap-dss.txt
> > b/Documentation/devicetree/bindings/display/ti/ti,omap-dss.txt index
> > e1ef29569338..249e588d7865 100644
> > --- a/Documentation/devicetree/bindings/display/ti/ti,omap-dss.txt
> > +++ b/Documentation/devicetree/bindings/display/ti/ti,omap-dss.txt
> > @@ -21,6 +21,13 @@ a RGB pixel stream to encoders.
> >  The encoder modules encode the received RGB pixel stream to a video output
> > like HDMI, MIPI DPI, etc.
> > 
> > +DISPC
> > +-----
> > +
> > +Optional properties:
> > +- max-memory-bandwidth: Input memory (from main memory to dispc) bandwidth
> > limit
> > +			in bytes per second
> > +
> 
> I like the idea, but I think you need to expand the description a bit. 
> ti,omap-dss.txt should explain how each module should be represented by a 
> child DT node in the DSS DT node, and this section should briefly explain what 
> the DISPC is (you can simply move the 4th paragraph of the file here) and that 
> it describes common properties.

Yes, I'll do that in the next revision.

> You could also move the description of other 
> DISPC properties here.

I had done that originally but Tomi pointed out that most of the other
properties are not common with all of the platform variants, some would
have multiple clocks or registers block or no hwmod components. I thought
it would more confusing to have some of those variant specific "required"
properties be shown here as optional.

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