Re: [RFC][PATCH 1/2] dt/bindings: display: Add DT bindings for Mali Display Processors.

[Liviu Dudau]

On Fri, Apr 01, 2016 at 05:47:57PM +0100, Mark Rutland wrote:
> On Fri, Apr 01, 2016 at 05:21:51PM +0100, Liviu Dudau wrote:
> > Add DT bindings documentation for the Mali Display Processor. The bindings
> > describe the Mali DP500, DP550 and DP650 processors from ARM Ltd.
> > 
> > Cc: Rob Herring <robh+dt@xxxxxxxxxx>
> > Cc: Pawel Moll <pawel.moll@xxxxxxx>
> > Cc: Mark Rutland <mark.rutland@xxxxxxx>
> > Cc: Ian Campbell <ijc+devicetree@xxxxxxxxxxxxxx>
> > Cc: Kumar Gala <galak@xxxxxxxxxxxxxx>
> > 
> > Signed-off-by: Liviu Dudau <Liviu.Dudau@xxxxxxx>
> > ---
> >  .../devicetree/bindings/display/arm,malidp.txt     | 65 ++++++++++++++++++++++
> >  1 file changed, 65 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/display/arm,malidp.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/display/arm,malidp.txt b/Documentation/devicetree/bindings/display/arm,malidp.txt
> > new file mode 100644
> > index 0000000..ed70de3
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/arm,malidp.txt
> > @@ -0,0 +1,65 @@
> > +ARM Mali-DP
> > +
> > +The following bindings apply to a family of Display Processors sold as
> > +licensable IP by ARM Ltd. The bindings describe the Mali DP500, DP550 and
> > +DP650 processors that offer multiple composition layers, support for
> > +rotation and scaling output.
> > +
> > +Required properties:
> > +  - compatible: should be one of
> > +	"arm,mali-dp500"
> > +	"arm,mali-dp550"
> > +	"arm,mali-dp650"
> > +    depending on the particular implementation present in the hardware
> > +  - reg: Physical base address and size of the block of registers used by
> > +    the processor.
> > +  - interrupts: Interrupt list, as defined in ../interrupt-controller/interrupts.txt,
> > +    interrupt client nodes.
> > +  - interrupt-names: name of the engine inside the processor that will
> > +    use the corresponding interrupt. Should be one of "DE" or "SE".
> 
> s/be one of/contain/, s/or/and/, judging by the example.

Yes, I need to rephrase that as my intent was to explain that each of the names can
be either "DE" or "SE", but all the reviews so far tripped over this one.

> 
> Otherwise this generally looks fine to me.

Thanks for reviewing this!
Liviu

> 
> Mark.
> 
> > +  - clocks: A list of phandle + clock-specifier pairs, one for each entry
> > +    in 'clock-names'
> > +  - clock-names: A list of clock names. It should contain:
> > +      - "pclk": for the APB interface clock
> > +      - "aclk": for the AXI interface clock
> > +      - "mclk": for the main processor clock
> > +      - "pxlclk": for the pixel clock feeding the output PLL of the processor.
> > +  - arm,malidp-output-port-lines: Array of u8 values describing the number
> > +    of output lines per channel (R, G and B).
> > +
> > +Required sub-nodes:
> > +  - port: The Mali DP connection to an encoder input port. The connection
> > +    is modelled using the OF graph bindings specified in
> > +    Documentation/devicetree/bindings/graph.txt
> > +
> > +Optional properties:
> > +  - memory-region: phandle to a node describing memory (see
> > +    Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt)
> > +    to be used for the framebuffer; if not present, the framebuffer may
> > +    be located anywhere in memory.
> > +
> > +
> > +Example:
> > +
> > +/ {
> > +	...
> > +
> > +	dp0: malidp@6f200000 {
> > +		compatible = "arm,mali-dp650";
> > +		reg = <0 0x6f200000 0 0x20000>;
> > +		memory-region = <&display_reserved>;
> > +		interrupts = <0 168 IRQ_TYPE_LEVEL_HIGH>,
> > +			     <0 168 IRQ_TYPE_LEVEL_HIGH>;
> > +		interrupt-names = "DE", "SE";
> > +		clocks = <&oscclk2>, <&fpgaosc0>, <&fpgaosc1>, <&fpgaosc1>;
> > +		clock-names = "pxlclk", "mclk", "aclk", "pclk";
> > +		arm,malidp-output-port-lines = /bits/ 8 <8 8 8>;
> > +		port {
> > +			dp0_output: endpoint {
> > +				remote-endpoint = <&tda998x_2_input>;
> > +			};
> > +		};
> > +	};
> > +
> > +	...
> > +};
> > -- 
> > 2.7.1
> > 
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    Â\_(ã)_/Â