Re: [PATCH v2 1/4] drm: arm: Add DT bindings documentation for HDLCD driver.

[Rob Herring]

On Thu, Nov 12, 2015 at 4:42 AM, Liviu Dudau <Liviu.Dudau@xxxxxxx> wrote:
> On Wed, Nov 11, 2015 at 12:48:50PM -0600, Rob Herring wrote:
>> On Wed, Nov 11, 2015 at 04:06:47PM +0000, Liviu Dudau wrote:
>> > 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>
>>
>> Looks pretty good, but a few comments.
>>
>> > ---
>> >  .../devicetree/bindings/drm/arm/arm,hdlcd.txt      | 74 ++++++++++++++++++++++
>> >  1 file changed, 74 insertions(+)
>> >  create mode 100644 Documentation/devicetree/bindings/drm/arm/arm,hdlcd.txt
>> >
>> > diff --git a/Documentation/devicetree/bindings/drm/arm/arm,hdlcd.txt b/Documentation/devicetree/bindings/drm/arm/arm,hdlcd.txt
>> > new file mode 100644
>> > index 0000000..b57f1b9
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/drm/arm/arm,hdlcd.txt
>> > @@ -0,0 +1,74 @@
>> > +ARM HDLCD
>> > +
>> > +This is a display controller found on several development platforms produced
>> > +by ARM Ltd and in more modern of its' Fast Models. The HDLCD is an RGB
>> > +streamer that reads the data from a framebuffer and sends it to a single
>> > +digital encoder (DVI or HDMI).
>> > +
>> > +Required properties:
>> > +  - compatible: "arm,hdlcd"
>>
>> Kind of generic. Something more specific please.
>
> "There can be only one!" (hdlcd) :) This is going to be a "one version only" HW part.
> ARM has now switched to a new display hardware that has more features and a new name,
> and work on mainlining support for that will start once I get the HDLCD driver accepted.

So there is never going to be a single difference across platforms.
Variations in max clock for different FPGAs?


>> > +  - reg: Physical base address and length of the controller's registers.
>> > +    If a second pair of address and length values is present this specifies
>> > +    the presence of a DMA coherent memory area that the HDLCD can use as
>> > +    framebuffer instead of normal CMA memory.
>>
>> This is on-chip RAM or nornal system RAM? We already have bindings for
>> both.
>
> Juno has a set of TLX (ThinLinks) connectors on the board where an FPGA can be attached. On r1
> the code running on FPGA can even participate as an AXI master with full coherency. The FPGA
> has local memory that we want to share with the HDLCD to be used as a framebuffer.

So describe the memory region and then use a memory-region phandle to
the memory here.

>> > +  - interrupts: One interrupt used by the display controller to notify the
>> > +    interrupt controller when any of the interrupt sources programmed in
>> > +    the interrupt mask register have activated.
>> > +  - clocks: A list of phandle + clock-specifier pairs, one for each
>> > +    entry in 'clock-names'.
>> > +  - clock-names: A list of clock names. For HDLD it should contain:

typo: HDLD

>> > +      - "pxlclk" for the clock feeding the output PLL of the controller.
>> > +  - port: The HDLCD connection to an encoder chip. The connection is modelled

s/modelled/modeled/

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/