Re: [PATCH v7 2/4] dt-bindings: drm/bridge: Document sn65dsi86 bridge bindings

[Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]

On Tue, May 29, 2018 at 08:41:42AM +0200, Andrzej Hajda wrote:
> On 24.05.2018 18:41, Sandeep Panda wrote:
> > Document the bindings used for the sn65dsi86 DSI to eDP bridge.
> >
> > Changes in v1:
> >  - Rephrase the dt-binding descriptions to be more inline with existing
> >    bindings (Andrzej Hajda).
> >  - Add missing dt-binding that are parsed by corresponding driver
> >    (Andrzej Hajda).
> >
> > Changes in v2:
> >  - Remove edp panel specific dt-binding entries. Only keep bridge
> >    specific entries (Sean Paul).
> >  - Remove custom-modes dt entry since its usage is removed from driver also (Sean Paul).
> >  - Remove is-pluggable dt entry since this will not be needed anymore (Sean Paul).
> >
> > Changes in v3:
> >  - Remove irq-gpio dt entry and instead populate is an interrupt
> >    property (Rob Herring).
> >
> > Changes in v4:
> >  - Add link to bridge chip datasheet (Stephen Boyd)
> >  - Add vpll and vcc regulator supply bindings (Stephen Boyd)
> >  - Add ref clk optional dt binding (Stephen Boyd)
> >  - Add gpio-controller optional dt binding (Stephen Boyd)
> >
> > Changes in v5:
> >  - Use clock property to specify the input refclk (Stephen Boyd).
> >  - Update gpio cell and pwm cell numbers (Stephen Boyd).
> >
> > Changes in v6:
> >  - Add property to mention the lane mapping scheme and polarity inversion
> >    (Stephen Boyd).
> >
> > Signed-off-by: Sandeep Panda <spanda@codeaurora.org>
> > ---
> >  .../bindings/display/bridge/ti,sn65dsi86.txt       | 89 ++++++++++++++++++++++
> >  1 file changed, 89 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt
> >
> > diff --git a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt
> > new file mode 100644
> > index 0000000..4a771a3
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt
> > @@ -0,0 +1,89 @@
> > +SN65DSI86 DSI to eDP bridge chip
> > +--------------------------------
> > +
> > +This is the binding for Texas Instruments SN65DSI86 bridge.
> > +http://www.ti.com/general/docs/lit/getliterature.tsp?genericPartNumber=sn65dsi86&fileType=pdf
> > +
> > +Required properties:
> > +- compatible: Must be "ti,sn65dsi86"
> > +- reg: i2c address of the chip, 0x2d as per datasheet
> > +- enable-gpios: OF device-tree gpio specification for bridge_en pin
> 
> info about active high should be added
> 
> > +
> > +- vccio-supply: A 1.8V supply that powers up the digital IOs.
> > +- vpll-supply: A 1.8V supply that powers up the displayport PLL.
> > +- vcca-supply: A 1.2V supply that powers up the analog circuits.
> > +- vcc-supply: A 1.2V supply that powers up the digital core.
> > +
> > +Optional properties:
> > +- interrupts: Specifier for the SN65DSI86 interrupt line.
> > +- hpd-gpios: Specifications for HPD gpio pin.
> 
> Again, please specify active level.

Having this property in the bridge node is strange. Also, does eDP 
normally have a HPD signal? If you are using this for DP, then this 
property goes in the connector node (or is absent if the bridge chip has 
a dedicated signal).

> > +
> > +- gpio-controller: Marks the device has a GPIO controller.
> > +- #gpio-cells    : Should be two. The first cell is the pin number and
> > +                   the second cell is used to specify flags.
> > +                   See ../../gpio/gpio.txt for more information.
> > +- #pwm-cells : Should be one. See ../../pwm/pwm.txt for description of
> > +               the cell formats.
> > +
> > +- clock-names: should be "refclk"
> > +- clocks: Specification for input reference clock. The reference
> > +	  clock rate must be 12 MHz, 19.2 MHz, 26 MHz, 27 MHz or 38.4 MHz.
> > +
> > +- lane-config: Specification to describe the logical to physical lane
> > +	       mapping scheme and polarity inversion of lanes.
> 
> Please describe this property, I guess it is about DSI lanes, and it
> should be exact(?) four pair of numbers, what are meaning and ranges of
> both numbers. What should be assumed if property is not present. Btw you
> can look into other bindings for reference, I guess there are already
> bindings having such property.

In fact, IIRC, some QCom bindings already have a property. Maybe it's 
the same. If so, don't describe it twice. Document in common location 
and just reference the common definition and add any constraints (like 
active level for a GPIO).

Is this DSI or eDP lanes?

Rob
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno