Hi Niklas, thanks for the patch On Fri, Nov 08, 2019 at 03:16:09AM +0100, Niklas Söderlund wrote: > Convert Renesas R-Car VIN bindings documentation to json-schema. > > As the examples in the bindings now can be tested add a new one which > describes how the both a parallel and a CSI-2 source can be connected on > Gen3 SoCs. > > Signed-off-by: Niklas Söderlund > --- > .../devicetree/bindings/media/renesas,vin.txt | 217 ---------- > .../bindings/media/renesas,vin.yaml | 409 ++++++++++++++++++ > 2 files changed, 409 insertions(+), 217 deletions(-) > delete mode 100644 Documentation/devicetree/bindings/media/renesas,vin.txt > create mode 100644 Documentation/devicetree/bindings/media/renesas,vin.yaml > > diff --git a/Documentation/devicetree/bindings/media/renesas,vin.txt b/Documentation/devicetree/bindings/media/renesas,vin.txt > deleted file mode 100644 > index 5eefd62ac5c5b9fd..0000000000000000 > --- a/Documentation/devicetree/bindings/media/renesas,vin.txt > +++ /dev/null > @@ -1,217 +0,0 @@ > -Renesas R-Car Video Input driver (rcar_vin) > -------------------------------------------- > - > -The rcar_vin device provides video input capabilities for the Renesas R-Car > -family of devices. > - > -Each VIN instance has a single parallel input that supports RGB and YUV video, > -with both external synchronization and BT.656 synchronization for the latter. > -Depending on the instance the VIN input is connected to external SoC pins, or > -on Gen3 and RZ/G2 platforms to a CSI-2 receiver. > - > - - compatible: Must be one or more of the following > - - "renesas,vin-r8a7743" for the R8A7743 device > - - "renesas,vin-r8a7744" for the R8A7744 device > - - "renesas,vin-r8a7745" for the R8A7745 device > - - "renesas,vin-r8a77470" for the R8A77470 device > - - "renesas,vin-r8a774a1" for the R8A774A1 device > - - "renesas,vin-r8a774b1" for the R8A774B1 device > - - "renesas,vin-r8a774c0" for the R8A774C0 device > - - "renesas,vin-r8a7778" for the R8A7778 device > - - "renesas,vin-r8a7779" for the R8A7779 device > - - "renesas,vin-r8a7790" for the R8A7790 device > - - "renesas,vin-r8a7791" for the R8A7791 device > - - "renesas,vin-r8a7792" for the R8A7792 device > - - "renesas,vin-r8a7793" for the R8A7793 device > - - "renesas,vin-r8a7794" for the R8A7794 device > - - "renesas,vin-r8a7795" for the R8A7795 device > - - "renesas,vin-r8a7796" for the R8A7796 device > - - "renesas,vin-r8a77965" for the R8A77965 device > - - "renesas,vin-r8a77970" for the R8A77970 device > - - "renesas,vin-r8a77980" for the R8A77980 device > - - "renesas,vin-r8a77990" for the R8A77990 device > - - "renesas,vin-r8a77995" for the R8A77995 device > - - "renesas,rcar-gen2-vin" for a generic R-Car Gen2 or RZ/G1 compatible > - device. > - > - When compatible with the generic version nodes must list the > - SoC-specific version corresponding to the platform first > - followed by the generic version. > - > - - reg: the register base and size for the device registers > - - interrupts: the interrupt for the device > - - clocks: Reference to the parent clock > - > -The per-board settings for Gen2 and RZ/G1 platforms: > - > -- port - sub-node describing a single endpoint connected to the VIN > - from external SoC pins as described in video-interfaces.txt[1]. > - Only the first one will be considered as each vin interface has one > - input port. > - > - - Optional properties for endpoint nodes: > - - hsync-active: see [1] for description. Default is active high. > - - vsync-active: see [1] for description. Default is active high. > - If both HSYNC and VSYNC polarities are not specified, embedded > - synchronization is selected. > - - field-active-even: see [1] for description. Default is active high. > - - bus-width: see [1] for description. The selected bus width depends on > - the SoC type and selected input image format. > - Valid values are: 8, 10, 12, 16, 24 and 32. > - - data-shift: see [1] for description. Valid values are 0 and 8. > - - data-enable-active: polarity of CLKENB signal, see [1] for > - description. Default is active high. > - > -The per-board settings for Gen3 and RZ/G2 platforms: > - > -Gen3 and RZ/G2 platforms can support both a single connected parallel input > -source from external SoC pins (port@0) and/or multiple parallel input sources > -from local SoC CSI-2 receivers (port@1) depending on SoC. > - > -- renesas,id - ID number of the VIN, VINx in the documentation. > -- ports > - - port@0 - sub-node describing a single endpoint connected to the VIN > - from external SoC pins as described in video-interfaces.txt[1]. > - Describing more than one endpoint in port@0 is invalid. Only VIN > - instances that are connected to external pins should have port@0. > - > - Endpoint nodes of port@0 support the optional properties listed in > - the Gen2 per-board settings description. > - > - - port@1 - sub-nodes describing one or more endpoints connected to > - the VIN from local SoC CSI-2 receivers. The endpoint numbers must > - use the following schema. > - > - - endpoint@0 - sub-node describing the endpoint connected to CSI20 > - - endpoint@1 - sub-node describing the endpoint connected to CSI21 > - - endpoint@2 - sub-node describing the endpoint connected to CSI40 > - - endpoint@3 - sub-node describing the endpoint connected to CSI41 > - > - Endpoint nodes of port@1 do not support any optional endpoint property. > - > -Device node example for Gen2 platforms > --------------------------------------- > - > - aliases { > - vin0 = &vin0; > - }; > - > - vin0: vin@e6ef0000 { > - compatible = "renesas,vin-r8a7790", "renesas,rcar-gen2-vin"; > - clocks = <&mstp8_clks R8A7790_CLK_VIN0>; > - reg = <0 0xe6ef0000 0 0x1000>; > - interrupts = <0 188 IRQ_TYPE_LEVEL_HIGH>; > - status = "disabled"; > - }; > - > -Board setup example for Gen2 platforms (vin1 composite video input) > -------------------------------------------------------------------- > - > -&i2c2 { > - status = "okay"; > - pinctrl-0 = <&i2c2_pins>; > - pinctrl-names = "default"; > - > - adv7180@20 { > - compatible = "adi,adv7180"; > - reg = <0x20>; > - remote = <&vin1>; > - > - port { > - adv7180: endpoint { > - bus-width = <8>; > - remote-endpoint = <&vin1ep0>; > - }; > - }; > - }; > -}; > - > -/* composite video input */ > -&vin1 { > - pinctrl-0 = <&vin1_pins>; > - pinctrl-names = "default"; > - > - status = "okay"; > - > - port { > - vin1ep0: endpoint { > - remote-endpoint = <&adv7180>; > - bus-width = <8>; > - }; > - }; > -}; > - > -Device node example for Gen3 platforms > --------------------------------------- > - > - vin0: video@e6ef0000 { > - compatible = "renesas,vin-r8a7795"; > - reg = <0 0xe6ef0000 0 0x1000>; > - interrupts = ; > - clocks = <&cpg CPG_MOD 811>; > - power-domains = <&sysc R8A7795_PD_ALWAYS_ON>; > - resets = <&cpg 811>; > - renesas,id = <0>; > - > - ports { > - #address-cells = <1>; > - #size-cells = <0>; > - > - port@1 { > - #address-cells = <1>; > - #size-cells = <0>; > - > - reg = <1>; > - > - vin0csi20: endpoint@0 { > - reg = <0>; > - remote-endpoint= <&csi20vin0>; > - }; > - vin0csi21: endpoint@1 { > - reg = <1>; > - remote-endpoint= <&csi21vin0>; > - }; > - vin0csi40: endpoint@2 { > - reg = <2>; > - remote-endpoint= <&csi40vin0>; > - }; > - }; > - }; > - }; > - > - csi20: csi2@fea80000 { > - compatible = "renesas,r8a7795-csi2"; > - reg = <0 0xfea80000 0 0x10000>; > - interrupts = ; > - clocks = <&cpg CPG_MOD 714>; > - power-domains = <&sysc R8A7795_PD_ALWAYS_ON>; > - resets = <&cpg 714>; > - > - ports { > - #address-cells = <1>; > - #size-cells = <0>; > - > - port@0 { > - reg = <0>; > - csi20_in: endpoint { > - clock-lanes = <0>; > - data-lanes = <1>; > - remote-endpoint = <&adv7482_txb>; > - }; > - }; > - > - port@1 { > - #address-cells = <1>; > - #size-cells = <0>; > - > - reg = <1>; > - > - csi20vin0: endpoint@0 { > - reg = <0>; > - remote-endpoint = <&vin0csi20>; > - }; > - }; > - }; > - }; > - > -[1] video-interfaces.txt common video media interface > diff --git a/Documentation/devicetree/bindings/media/renesas,vin.yaml b/Documentation/devicetree/bindings/media/renesas,vin.yaml > new file mode 100644 > index 0000000000000000..be261d9a98d62dcd > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/renesas,vin.yaml > @@ -0,0 +1,409 @@ > +# SPDX-License-Identifier: GPL-2.0 I think you should use GPL-2.0-only > +# Copyright (C) 2019 Renesas Electronics Corp. > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/media/renesas,vin.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Renesas R-Car Video Input (VIN) > + > +maintainers: > + - Niklas Söderlund I also put the mailing lists here, not sure if that's desired. > + > +description: |- > + The R-Car Video Input (VIN) device provides video input capabilities for the > + Renesas R-Car family of devices. > + > + Each VIN instance has a single parallel input that supports RGB and YUV video, > + with both external synchronization and BT.656 synchronization for the latter. > + Depending on the instance the VIN input is connected to external SoC pins, or > + on Gen3 and RZ/G2 platforms to a CSI-2 receiver. > + > +properties: > + compatible: > + oneOf: > + - items: > + - enum: > + - renesas,vin-r8a7743 # RZ/G1M > + - renesas,vin-r8a7744 # RZ/G1N > + - renesas,vin-r8a7745 # RZ/G1E > + - renesas,vin-r8a77470 # RZ/G1C > + - renesas,vin-r8a7790 # R-Car H2 > + - renesas,vin-r8a7791 # R-Car M2-W > + - renesas,vin-r8a7792 # R-Car V2H > + - renesas,vin-r8a7793 # R-Car M2-N > + - renesas,vin-r8a7794 # R-Car E2 > + - const: renesas,rcar-gen2-vin # Generic R-Car Gen2 or RZ/G1 > + > + - items: > + - enum: > + - renesas,vin-r8a774a1 # RZ/G2M > + - renesas,vin-r8a774b1 # RZ/G2N > + - renesas,vin-r8a774c0 # RZ/G2E > + - renesas,vin-r8a7778 # R-Car M1 > + - renesas,vin-r8a7779 # R-Car H1 > + - renesas,vin-r8a7795 # R-Car H3 > + - renesas,vin-r8a7796 # R-Car M3-W > + - renesas,vin-r8a77965 # R-Car M3-N > + - renesas,vin-r8a77970 # R-Car V3M > + - renesas,vin-r8a77980 # R-Car V3H > + - renesas,vin-r8a77990 # R-Car E3 > + - renesas,vin-r8a77995 # R-Car D3 > + > + reg: > + maxItems: 1 > + > + interrupts: > + maxItems: 1 > + > + clocks: > + maxItems: 1 > + > + power-domains: > + maxItems: 1 > + > + resets: > + maxItems: 1 > + > + #The per-board settings for Gen2 and RZ/G1 platforms: > + port: > + type: object > + description: |- > + A node containing a parallel input with a singel endpoint definitions as > + documented in > + Documentation/devicetree/bindings/media/video-interfaces.txt > + > + properties: > + reg: > + const: 1 Do you need reg for the single 'port' ? > + > + endpoint: > + type: object > + > + properties: > + hsync-active: > + description: |- > + Default is active high. If both HSYNC and VSYNC polarities are not > + specified, embedded synchronization is selected. > + enum: [ 0, 1 ] > + > + vsync-active: > + description: |- > + Default is active high. If both HSYNC and VSYNC polarities are not > + specified, embedded synchronization is selected. > + enum: [ 0, 1 ] > + > + field-active-even: > + description: Default is active high. > + enum: [ 0, 1 ] > + > + bus-width: > + enum: [ 8, 10, 12, 16, 24, 32 ] > + > + data-shift: > + enum: [ 0, 8 ] > + > + data-enable-active: > + description: Polarity of CLKENB signal, default is active high. > + enum: [ 0, 1 ] > + > + pclk-sample: > + enum: [ 0, 1 ] > + > + data-active: > + enum: [ 0, 1 ] > + > + remote-endpoint: > + maxItems: 1 > + > + required: > + - remote-endpoint > + > + additionalProperties: false > + > + additionalProperties: false > + > + #The per-board settings for Gen3 and RZ/G2 platforms: > + renesas,id: > + description: VIN channel number > + allOf: > + - $ref: /schemas/types.yaml#/definitions/uint32 > + - enum: [ 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15 ] > + > + ports: > + type: object > + description: |- > + A node containing input nodes with endpoint definitions as documented in > + Documentation/devicetree/bindings/media/video-interfaces.txt > + > + properties: > + port@0: > + type: object > + description: |- > + Input port node, single endpoint describing a parallel input source. > + > + properties: > + reg: > + const: 0 > + > + endpoint: > + type: object > + > + properties: > + hsync-active: > + description: |- > + Default is active high. If both HSYNC and VSYNC polarities are not > + specified, embedded synchronization is selected. > + enum: [ 0, 1 ] > + > + vsync-active: > + description: |- > + Default is active high. If both HSYNC and VSYNC polarities are not > + specified, embedded synchronization is selected. > + enum: [ 0, 1 ] > + > + field-active-even: > + description: Default is active high. > + enum: [ 0, 1 ] > + > + bus-width: > + enum: [ 8, 10, 12, 16, 24, 32 ] > + > + data-shift: > + enum: [ 0, 8 ] Why are those two not described ? I feel like or you either refrain from describing all poperties and point the reader to video-interfaces.txt, or you describe all of them, at least to provide an indication of the default if the property is not specified. > + > + data-enable-active: > + description: Polarity of CLKENB signal, default is active high. > + enum: [ 0, 1 ] > + > + pclk-sample: > + enum: [ 0, 1 ] > + > + data-active: > + enum: [ 0, 1 ] > + > + remote-endpoint: > + maxItems: 1 > + > + required: > + - remote-endpoint > + > + additionalProperties: false > + > + additionalProperties: false > + > + port@1: > + type: object > + description: |- > + Input port node, multiple endpoints describing all the R-Car CSI-2 > + modules connected the VIN. > + > + properties: > + '#address-cells': > + const: 1 > + > + '#size-cells': > + const: 0 > + > + reg: > + const: 1 > + > + endpoint@0: > + type: object > + description: Endpoint connected to CSI20. > + > + properties: > + reg: > + const: 0 > + > + remote-endpoint: > + maxItems: 1 > + > + required: > + - reg > + - remote-endpoint > + > + additionalProperties: false > + > + endpoint@1: > + type: object > + description: Endpoint connected to CSI21. > + > + properties: > + reg: > + const: 1 > + > + remote-endpoint: > + maxItems: 1 > + > + required: > + - reg > + - remote-endpoint > + > + additionalProperties: false > + > + endpoint@2: > + type: object > + description: Endpoint connected to CSI40. > + > + properties: > + reg: > + const: 2 > + > + remote-endpoint: > + maxItems: 1 > + > + required: > + - reg > + - remote-endpoint > + > + additionalProperties: false > + > + endpoint@3: > + type: object > + description: Endpoint connected to CSI41. > + > + properties: > + reg: > + const: 3 > + > + remote-endpoint: > + maxItems: 1 > + > + required: > + - reg > + - remote-endpoint Here and in the other endpoints of port@1, do you need required properties ? As I read in writing-schema.rts: "Unless noted otherwise, all properties are required." > + > + additionalProperties: false > + > + additionalProperties: false > + > +required: > + - compatible > + - reg > + - interrupts > + - clocks > + - power-domains > + - resets > + > +if: > + properties: > + compatible: > + contains: > + anyOf: > + - const: renesas,vin-r8a7778 > + - const: renesas,vin-r8a7779 > + - const: renesas,rcar-gen2-vin > +then: > + required: > + - port > +else: > + required: > + - renesas,id > + - ports > + > +additionalProperties: false > + > +examples: > + # Device node example for Gen2 platform > + - | > + #include > + #include > + #include > + > + vin1: vin@e6ef1000 { > + compatible = "renesas,vin-r8a7790", > + "renesas,rcar-gen2-vin"; > + reg = <0 0xe6ef1000 0 0x1000>; > + interrupts = ; > + clocks = <&cpg CPG_MOD 810>; > + power-domains = <&sysc R8A7790_PD_ALWAYS_ON>; > + resets = <&cpg 810>; > + > + port { > + vin1ep0: endpoint { > + remote-endpoint = <&adv7180>; > + bus-width = <8>; > + }; > + }; > + }; > + > + # Device node example for Gen3 platform with only CSI-2 > + - | > + #include > + #include > + #include > + > + vin0: video@e6ef0000 { > + compatible = "renesas,vin-r8a7795"; > + reg = <0 0xe6ef0000 0 0x1000>; > + interrupts = ; > + clocks = <&cpg CPG_MOD 811>; > + power-domains = <&sysc R8A7795_PD_ALWAYS_ON>; > + resets = <&cpg 811>; > + renesas,id = <0>; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@1 { > + #address-cells = <1>; > + #size-cells = <0>; > + > + reg = <1>; > + > + vin0csi20: endpoint@0 { > + reg = <0>; > + remote-endpoint= <&csi20vin0>; > + }; > + vin0csi40: endpoint@2 { > + reg = <2>; > + remote-endpoint= <&csi40vin0>; > + }; > + }; > + }; > + }; > + > + # Device node example for Gen3 platform with CSI-2 and parallel > + - | > + #include > + #include > + #include > + > + vin2: video@e6ef2000 { > + compatible = "renesas,vin-r8a77970"; > + reg = <0 0xe6ef2000 0 0x1000>; > + interrupts = ; > + clocks = <&cpg CPG_MOD 809>; > + power-domains = <&sysc R8A77970_PD_ALWAYS_ON>; > + resets = <&cpg 809>; > + renesas,id = <2>; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + > + vin2_in: endpoint { > + remote-endpoint = <&adv7612_out>; > + hsync-active = <0>; > + vsync-active = <0>; > + }; > + }; > + > + port@1 { > + #address-cells = <1>; > + #size-cells = <0>; > + > + reg = <1>; > + > + vin2csi40: endpoint@2 { > + reg = <2>; > + remote-endpoint = <&csi40vin2>; > + }; > + }; > + }; > + }; I can't really comment on the json-schema most complex parts, but from a device point of view, minors apart, it seems sane to me. Reviewed-by: Jacopo Mondi Thanks j > -- > 2.23.0 >