| Message ID | 1444997709-57293-1-git-send-email-ck.hu@mediatek.com |
|---|---|
| State | Under Review, archived |
| Headers | show |
On Fri, Oct 16, 2015 at 08:15:08PM +0800, CK Hu wrote: > From: Jitao Shi <jitao.shi@mediatek.com> > > Add documentation for DT properties supported by ps8640 > DSI-eDP converter. > > Signed-off-by: Jitao Shi <jitao.shi@mediatek.com> > --- > .../devicetree/bindings/video/bridge/ps8640.txt | 48 ++++++++++++++++++++ > 1 file changed, 48 insertions(+) > create mode 100644 Documentation/devicetree/bindings/video/bridge/ps8640.txt > > diff --git a/Documentation/devicetree/bindings/video/bridge/ps8640.txt b/Documentation/devicetree/bindings/video/bridge/ps8640.txt > new file mode 100644 > index 0000000..450b5df > --- /dev/null > +++ b/Documentation/devicetree/bindings/video/bridge/ps8640.txt > @@ -0,0 +1,48 @@ > +ps8640-bridge bindings > + > +Required properties: > + - compatible: "parade,ps8640" > + - reg: first i2c address of the bridge Ther can be multiple addresses? > + - power-gpios: OF device-tree gpio specification for power pin. > + - reset-gpios: OF device-tree gpio specification for reset pin. > + - mode-sel-gpios: OF device-tree gpio specification for mode-sel pin. > + - ps8640-1v2-supply: OF device-tree regulator specification for 1v2. > + - ps8640-3v3-supply: OF device-tree regulator specification for 3v3. There's no need for the "ps8640-" prefix on these two. Please drop it. > +Optional properties: > + - video interfaces: Device node can contain video interface port > + nodes for panel according to [1]. Replace this part with: The device node can contain video interface port nodes per the video-interfaces binding [1]. Thanks, Mark. > + > +[1]: Documentation/devicetree/bindings/media/video-interfaces.txt > + > +Example: > + edp-bridge@18 { > + compatible = "parade,ps8640"; > + reg = <0x18>; > + power-gpios = <&pio 116 GPIO_ACTIVE_HIGH>; > + reset-gpios = <&pio 115 GPIO_ACTIVE_HIGH>; > + mode-sel-gpios = <&pio 92 GPIO_ACTIVE_HIGH>; > + ps8640-1v2-supply = <&ps8640_fixed_1v2>; > + ps8640-3v3-supply = <&mt6397_vgp2_reg>; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + > + ps8640_in: endpoint { > + remote-endpoint = <&dsi0_out>; > + }; > + }; > + > + port@1 { > + reg = <1>; > + > + ps8640_out: endpoint { > + remote-endpoint = <&panel_in>; > + }; > + }; > + }; > + }; > -- > 1.7.9.5 > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> + /* FIXME - use of_graph_get_port_by_id(np, 1) on newer kernels */ > + in_ep = of_graph_get_next_endpoint(np, NULL); Huh? > + edidp = of_get_property(np, "edid", &size); This property wasn't mentioned in the binding document. Please describe it. If it's from a more generic binding, refer to that from the binding document. Thanks, Mark. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi CK, there is a typo in the subject: s/Dcumentation/Documentation/. Am Freitag, den 16.10.2015, 20:15 +0800 schrieb CK Hu: > From: Jitao Shi <jitao.shi@mediatek.com> > > Add documentation for DT properties supported by ps8640 > DSI-eDP converter. > > Signed-off-by: Jitao Shi <jitao.shi@mediatek.com> [...] > + - ps8640-1v2-supply: OF device-tree regulator specification for 1v2. > + - ps8640-3v3-supply: OF device-tree regulator specification for 3v3. The ps8640- part of the regulator supply property names is redundant. The PS8622 driver uses vdd12-supply as its regulator. Should we strive for consistency here? Or if you have access to the datasheet, how are these inputs called there? > + > +Optional properties: > + - video interfaces: Device node can contain video interface port > + nodes for panel according to [1]. > + > +[1]: Documentation/devicetree/bindings/media/video-interfaces.txt It should be documented here that port@0 is the input port and port@1 is the output port. best regards Philipp -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Oct 16, 2015 at 7:15 AM, CK Hu <ck.hu@mediatek.com> wrote: > From: Jitao Shi <jitao.shi@mediatek.com> > > Add documentation for DT properties supported by ps8640 > DSI-eDP converter. > > Signed-off-by: Jitao Shi <jitao.shi@mediatek.com> > --- > .../devicetree/bindings/video/bridge/ps8640.txt | 48 ++++++++++++++++++++ Please move to bindings/display/bridge/. > 1 file changed, 48 insertions(+) > create mode 100644 Documentation/devicetree/bindings/video/bridge/ps8640.txt > > diff --git a/Documentation/devicetree/bindings/video/bridge/ps8640.txt b/Documentation/devicetree/bindings/video/bridge/ps8640.txt > new file mode 100644 > index 0000000..450b5df > --- /dev/null > +++ b/Documentation/devicetree/bindings/video/bridge/ps8640.txt > @@ -0,0 +1,48 @@ > +ps8640-bridge bindings > + > +Required properties: > + - compatible: "parade,ps8640" > + - reg: first i2c address of the bridge > + - power-gpios: OF device-tree gpio specification for power pin. > + - reset-gpios: OF device-tree gpio specification for reset pin. > + - mode-sel-gpios: OF device-tree gpio specification for mode-sel pin. > + - ps8640-1v2-supply: OF device-tree regulator specification for 1v2. > + - ps8640-3v3-supply: OF device-tree regulator specification for 3v3. As others have said, I would drop the part number and name them based on the supply name (e.g. vdd?, vcore?). > + > +Optional properties: > + - video interfaces: Device node can contain video interface port > + nodes for panel according to [1]. I don't think this should be optional. > + > +[1]: Documentation/devicetree/bindings/media/video-interfaces.txt > + > +Example: > + edp-bridge@18 { > + compatible = "parade,ps8640"; > + reg = <0x18>; > + power-gpios = <&pio 116 GPIO_ACTIVE_HIGH>; > + reset-gpios = <&pio 115 GPIO_ACTIVE_HIGH>; > + mode-sel-gpios = <&pio 92 GPIO_ACTIVE_HIGH>; > + ps8640-1v2-supply = <&ps8640_fixed_1v2>; > + ps8640-3v3-supply = <&mt6397_vgp2_reg>; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + > + ps8640_in: endpoint { > + remote-endpoint = <&dsi0_out>; > + }; > + }; > + > + port@1 { > + reg = <1>; > + > + ps8640_out: endpoint { > + remote-endpoint = <&panel_in>; > + }; > + }; > + }; > + }; > -- > 1.7.9.5 > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Oct 16, 2015 at 8:06 AM, Mark Rutland <mark.rutland@arm.com> wrote: >> + /* FIXME - use of_graph_get_port_by_id(np, 1) on newer kernels */ >> + in_ep = of_graph_get_next_endpoint(np, NULL); > > Huh? > >> + edidp = of_get_property(np, "edid", &size); > > This property wasn't mentioned in the binding document. > > Please describe it. If it's from a more generic binding, refer to that > from the binding document. It should be generic, but currently documented in individual drivers. I also think this is not the right location in the DT for this property. It should be part of the panel or connector node instead. Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/video/bridge/ps8640.txt b/Documentation/devicetree/bindings/video/bridge/ps8640.txt new file mode 100644 index 0000000..450b5df --- /dev/null +++ b/Documentation/devicetree/bindings/video/bridge/ps8640.txt @@ -0,0 +1,48 @@ +ps8640-bridge bindings + +Required properties: + - compatible: "parade,ps8640" + - reg: first i2c address of the bridge + - power-gpios: OF device-tree gpio specification for power pin. + - reset-gpios: OF device-tree gpio specification for reset pin. + - mode-sel-gpios: OF device-tree gpio specification for mode-sel pin. + - ps8640-1v2-supply: OF device-tree regulator specification for 1v2. + - ps8640-3v3-supply: OF device-tree regulator specification for 3v3. + +Optional properties: + - video interfaces: Device node can contain video interface port + nodes for panel according to [1]. + +[1]: Documentation/devicetree/bindings/media/video-interfaces.txt + +Example: + edp-bridge@18 { + compatible = "parade,ps8640"; + reg = <0x18>; + power-gpios = <&pio 116 GPIO_ACTIVE_HIGH>; + reset-gpios = <&pio 115 GPIO_ACTIVE_HIGH>; + mode-sel-gpios = <&pio 92 GPIO_ACTIVE_HIGH>; + ps8640-1v2-supply = <&ps8640_fixed_1v2>; + ps8640-3v3-supply = <&mt6397_vgp2_reg>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + + ps8640_in: endpoint { + remote-endpoint = <&dsi0_out>; + }; + }; + + port@1 { + reg = <1>; + + ps8640_out: endpoint { + remote-endpoint = <&panel_in>; + }; + }; + }; + };