Hi Andrzej, On Wednesday, 30 May 2018 12:59:12 EEST Andrzej Hajda wrote: > On 28.05.2018 12:18, Laurent Pinchart wrote: > > On Monday, 28 May 2018 12:47:11 EEST Maciej Purski wrote: > >> The patch adds bindings to Toshiba DSI/LVDS bridge TC358764. > >> Bindings describe power supplies, reset gpio and video interfaces. > >> > >> Signed-off-by: Andrzej Hajda <a.hajda@xxxxxxxxxxx> > >> Signed-off-by: Maciej Purski <m.purski@xxxxxxxxxxx> > >> --- > >> > >> .../bindings/display/bridge/toshiba,tc358764.txt | 42 ++++++++++++++++ > >> 1 file changed, 42 insertions(+) > >> create mode 100644 > >> > >> Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt > >> > >> diff --git > >> a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt > >> b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt > >> new > >> file mode 100644 > >> index 0000000..d09bdc2 > >> --- /dev/null > >> +++ > >> b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358764.txt > >> @@ -0,0 +1,42 @@ > >> +TC358764 MIPI-DSI to LVDS panel bridge > >> + > >> +Required properties: > >> + - compatible: "toshiba,tc358764" > >> + - reg: the virtual channel number of a DSI peripheral > >> + - vddc-supply: core voltage supply > >> + - vddio-supply: I/O voltage supply > >> + - vddmipi-supply: MIPI voltage supply > >> + - vddlvds133-supply: LVDS1 3.3V voltage supply > >> + - vddlvds112-supply: LVDS1 1.2V voltage supply > > > > That's a lot of power supplies. Could some of them be merged together ? > > See https://patchwork.freedesktop.org/patch/216058/ for an earlier > > discussion on the same subject. > > Specs says about 3 supply voltage values: > - 1.2V - digital core, DSI-RX PHY > - 1.8-3.3V - digital I/O > - 3.3V - LVDS-TX PHY > > So I guess it should be minimal number of supplies. Natural candidates: > > - vddc-supply: core voltage supply, 1.2V > - vddio-supply: I/O voltage supply, 1.8V or 3.3V > - vddlvds-supply: LVDS1/2 voltage supply, 3.3V > > I have changed name of the latest supply to be more consistent with > other supplies, and changed 1.8-3.3 (which incorrectly suggest voltage > range), to more precise voltage alternative. This looks fine to me. > >> + - reset-gpios: a GPIO spec for the reset pin > >> + > >> +The device node can contain zero to two 'port' child nodes, each with > >> one > >> +child > >> +'endpoint' node, according to the bindings defined in [1]. > >> +The following are properties specific to those nodes. > >> + > >> +port: > >> + - reg: (required) can be 0 for DSI port or 1 for LVDS port; > > > > This seems pretty vague to me. It could be read as meaning that ports are > > completely optional, and that the port number you list can be used, but > > that something else could be used to. > > > > Let's make the port nodes mandatory. I propose the following. > > > > Required nodes: > > > > The TC358764 has DSI and LVDS ports whose connections are described using > > the OF graph bindings defined in > > Documentation/devicetree/bindings/graph.txt. The device node must contain > > one 'port' child node per DSI and LVDS port. The port nodes are numbered > > as follows. > > > > Port Number > > ------------------------------------------------------------------- > > DSI Input 0 > > LVDS Output 1 > > > > Each port node must contain endpoint nodes describing the hardware > > connections. > > Since the bridge is controlled via DSI bus, DSI input port is not necessary. I don't agree with this. Regardless of how the bridge is controlled, I think we should always use ports to describe the data connections. Otherwise it would get more complicated for display controller drivers to use different types of bridges. > >> +[1]: Documentation/devicetree/bindings/media/video-interfaces.txt > >> + > >> +Example: > >> + > >> + bridge@0 { > >> + reg = <0>; > >> + compatible = "toshiba,tc358764"; > >> + vddc-supply = <&vcc_1v2_reg>; > >> + vddio-supply = <&vcc_1v8_reg>; > >> + vddmipi-supply = <&vcc_1v2_reg>; > >> + vddlvds133-supply = <&vcc_3v3_reg>; > >> + vddlvds112-supply = <&vcc_1v2_reg>; > >> + reset-gpios = <&gpd1 6 GPIO_ACTIVE_LOW>; > >> + #address-cells = <1>; > >> + #size-cells = <0>; > >> + port@1 { > >> + reg = <1>; > >> + lvds_ep: endpoint { > >> + remote-endpoint = <&panel_ep>; > >> + }; > >> + }; > >> + }; -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
