On 03/12/2019 12:16, Yuti Amonkar wrote: > Document the bindings used for the Cadence MHDP DPI/DP bridge in > yaml format. > > Signed-off-by: Yuti Amonkar <yamonkar@xxxxxxxxxxx> Couple of comments bellow. > --- > .../bindings/display/bridge/cdns,mhdp.yaml | 101 +++++++++++++++++++++ > 1 file changed, 101 insertions(+) > create mode 100644 Documentation/devicetree/bindings/display/bridge/cdns,mhdp.yaml > > diff --git a/Documentation/devicetree/bindings/display/bridge/cdns,mhdp.yaml b/Documentation/devicetree/bindings/display/bridge/cdns,mhdp.yaml > new file mode 100644 > index 0000000..1739f2e > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/bridge/cdns,mhdp.yaml > @@ -0,0 +1,101 @@ > +%YAML 1.2 > +--- > +$id: "http://devicetree.org/schemas/display/bridge/cdns,mhdp.yaml#"; > +$schema: "http://devicetree.org/meta-schemas/core.yaml#"; > + > +title: Cadence MHDP bridge > + > +maintainers: > + - Swapnil Jakhade <sjakhade@xxxxxxxxxxx> > + - Yuti Amonkar <yamonkar@xxxxxxxxxxx> > + > +properties: > + compatible: > + items: > + - const: ti,j721e-mhdp8546 > + - const: cdns,mhdp8546 > + > + clocks: > + items: ^^^^^^ No need for this, but "maxItems: 1" would probably make sense. > + descrption: Typo in the key word. Please try "make dt_binding_check" (see Documentation/devicetree/writing-schema.rst) to run a formal check on your binding. > + DP bridge clock, it's used by the IP to know how to translate a number of > + clock cycles into a time (which is used to comply with DP standard timings > + and delays). > + > + reg: Add these here to make it explicit that there can be either 1 or 2 items. maxItems: 2 minItems: 1 > + items: ^^^^^^ This could probably be removed too. > + - description: > + Register block of mhdptx abp registers upto PHY mapped area(AUX_CONFIG_P). > + The AUX and PMA registers are mapped to associated phy driver. > + - description: > + Register block for DSS_EDP0_INTG_CFG_VP registers in case of TI J7 SoCs. It looks like the dt_binding_check accepts two descriptions, but I wonder how easy it is to understand that they are referring to the two reg items. Maybe we should add reg-names property to make things more explicit. For instance "mhdptx" and "j721e-intg". > + > + phys: > + description: see the Documentation/devicetree/bindings/phy/phy-cadence-torrent.yaml > + > + phy-names: > + const: dpphy > + > + interrupts: > + maxItems: 1 > + > + power-domains: > + maxItems: 1 > + description: phandle to the associated power domain > + > + ports: > + type: object > + description: > + Ports as described in Documentation/devictree/bindings/graph.txt > + properties: I think you need "#address-cells": const: 1 "#size-cells": const: 0 here, and you should make them "requred". > + port@0: > + description: > + input port representing the DP bridge input > + > + port@1: > + description: > + output port representing the DP bridge output > + required: > + - port@0 > + - port@1 > + > +required: > + - compatible > + - clocks > + - reg > + - phys > + - phy-names > + - ports > + > +additionalProperties: false > + > +examples: > + - | > + mhdp: dp-bridge@f0fb000000 { > + compatible = "ti,j721e-mhdp8546", "cdns,mhdp8546"; > + reg = <0xf0 0xfb000000 0x0 0x1000000>, > + <0xf0 0xfc000000 0x0 0x2000000>; > + clocks = <&mhdp_clock>; > + phys = <&dp_phy>; > + phy-names = "dpphy"; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + dp_bridge_input: endpoint { > + remote-endpoint = <&xxx_dpi_output>; > + }; > + }; > + > + port@1 { > + reg = <1>; > + dp_bridge_output: endpoint { > + remote-endpoint = <&xxx_dp_connector_input>; > + }; > + }; > + }; > + }; > +... > -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
