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

[spanda at codeaurora.org]

On 2018-05-16 21:57, Stephen Boyd wrote:
> Quoting Sandeep Panda (2018-05-14 22:52:42)
>> 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..b82bb56
>> --- /dev/null
>> +++ 
>> b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt
>> @@ -0,0 +1,81 @@
>> +Optional properties:
>> +- interrupts: Specifier for the SN65DSI86 interrupt line.
>> +- hpd-gpios: OF device-tree gpio specifications for HPD pin.
>> +
>> +- 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: OF device-tree clock specification for refclk input. The 
>> reference
> 
> What is "OF device-tree .* specification" providing? This is all an OF
> device-tree specification.
Ok. i will remove OF device tree, only keep specification for refclk 
input.
> 
>> +  clock rate must be 12 MHz, 19.2 MHz, 26 MHz, 27 MHz or 38.4 MHz.
>> +
>> +Required nodes:
>> +
>> +This device has two video ports. Their connections are modelled using 
>> the
>> +OF graph bindings specified in 
>> Documentation/devicetree/bindings/graph.txt.
>> +
>> +- Video port 0 for DSI input
>> +- Video port 1 for eDP output
>> +
>> +Example
>> +-------
>> +
>> +edp-bridge at 2d {
>> +       compatible = "ti,sn65dsi86";
>> +       #address-cells = <1>;
>> +       #size-cells = <0>;
>> +       reg = <0x2d>;
>> +
>> +       enable-gpios = <&msmgpio 33 GPIO_ACTIVE_HIGH>;
>> +       interrupt-parent = <&gpio3>;
>> +       interrupts = <4 IRQ_TYPE_EDGE_FALLING>;
>> +
>> +       vccio-supply = <&pm8916_l17>;
>> +       vcca-supply = <&pm8916_l6>;
>> +       vpll-supply = <&pm8916_l17>;
>> +       vcc-supply = <&pm8916_l6>;
>> +
>> +       clock-names = "refclk";
>> +       clocks = <&input_refclk>;
>> +
>> +       ports {
>> +               #address-cells = <1>;
>> +               #size-cells = <0>;
>> +
>> +               port at 0 {
>> +                       reg = <0>;
>> +
>> +                       edp_bridge_in: endpoint {
>> +                               remote-endpoint = <&dsi_out>;
> 
> How do we know the number of lanes that are connected and if there's 
> one
> channel (A) or two channels (A and B)? Would there be two endpoints in
> that case?
Currently we have hard coded the number of lanes to 4 in driver. And 
regarding supporting 2 DSI input channels dt-binding, we are planning to 
add those entries once we upload a patchset support dual dsi 
configuration in bridge driver.
> 
>> +                       };
>> +               };
>> +
>> +               port at 1 {
>> +                       reg = <1>;
>> +
>> +                       edp_bridge_out: endpoint {
>> +                               remote-endpoint = <&edp_panel_in>;
> 
> The hardware looks to support some sort of lane renumbering scheme,
> where the eDP logical lane 0 can be routed through a different pin than
> MLP/N0, same for logical lane 1, etc. I don't have a use case for this
> right now, but I hope that it could be added somewhere in the binding 
> as
> an optional property to describe this lane remapping feature. It also
> has some sort of lane polarity inversion feature. Perhaps there needs 
> to
> be a lane-config property that does this remapping and inversion with
> two cells.
Ok. I will add this feature to dt binding.
> 
> 	lane-config = <0 0>, /* Lane 0 logical is lane 0 phys (!inv) */
> 	              <1 0>, /* Lane 1 logical is lane 1 phys (!inv) */
> 		      <2 0>, /* Lane 2 logical is lane 2 phys (!inv) */
> 		      <3 0>; /* Lane 3 logical is lane 3 phys (!inv) */
> 
> Or
> 
> 	lane-config = <2 1>, /* Lane 2 logical is lane 0 phys (inv) */
> 	              <1 0>, /* Lane 1 logical is lane 1 phys (!inv) */
> 		      <3 1>, /* Lane 3 logical is lane 2 phys (inv) */
> 		      <0 0>; /* Lane 0 logical is lane 3 phys (!inv) */
> 
> 
>> +                       };
>> +               };
>> +       };