On 28.05.2018 12:18, Laurent Pinchart wrote: > Hi Maciej, > > Thank you for the patch. > > 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. > >> + - 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. Regards Andrzej > >> +[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>; >> + }; >> + }; >> + }; -- 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
