| Message ID | 20200511110611.3142-7-ricardo.canuelo@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Convert adi,adv7511.txt DT bindings to yaml | expand |
Hi Ricardo, Thank you for the patch. On Mon, May 11, 2020 at 01:06:11PM +0200, Ricardo Cañuelo wrote: > Convert the ADV7511/11w/13/33/35 DT bindings to json-schema. The > original binding has been split into two files: adi,adv7511.yaml for > ADV7511/11W/13 and adi,adv7533.yaml for ADV7533/35. > > Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com> > --- > .../bindings/display/bridge/adi,adv7511.txt | 143 ----------- > .../bindings/display/bridge/adi,adv7511.yaml | 230 ++++++++++++++++++ > .../bindings/display/bridge/adi,adv7533.yaml | 166 +++++++++++++ > 3 files changed, 396 insertions(+), 143 deletions(-) > delete mode 100644 Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt > create mode 100644 Documentation/devicetree/bindings/display/bridge/adi,adv7511.yaml > create mode 100644 Documentation/devicetree/bindings/display/bridge/adi,adv7533.yaml > > diff --git a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt > deleted file mode 100644 > index 659523f538bf..000000000000 > --- a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt > +++ /dev/null > @@ -1,143 +0,0 @@ > -Analog Devices ADV7511(W)/13/33/35 HDMI Encoders > ------------------------------------------------- > - > -The ADV7511, ADV7511W, ADV7513, ADV7533 and ADV7535 are HDMI audio and video > -transmitters compatible with HDMI 1.4 and DVI 1.0. They support color space > -conversion, S/PDIF, CEC and HDCP. ADV7533/5 supports the DSI interface for input > -pixels, while the others support RGB interface. > - > -Required properties: > - > -- compatible: Should be one of: > - "adi,adv7511" > - "adi,adv7511w" > - "adi,adv7513" > - "adi,adv7533" > - "adi,adv7535" > - > -- reg: I2C slave addresses > - The ADV7511 internal registers are split into four pages exposed through > - different I2C addresses, creating four register maps. Each map has it own > - I2C address and acts as a standard slave device on the I2C bus. The main > - address is mandatory, others are optional and revert to defaults if not > - specified. > - > - > -The ADV7511 supports a large number of input data formats that differ by their > -color depth, color format, clock mode, bit justification and random > -arrangement of components on the data bus. The combination of the following > -properties describe the input and map directly to the video input tables of the > -ADV7511 datasheet that document all the supported combinations. > - > -- adi,input-depth: Number of bits per color component at the input (8, 10 or > - 12). > -- adi,input-colorspace: The input color space, one of "rgb", "yuv422" or > - "yuv444". > -- adi,input-clock: The input clock type, one of "1x" (one clock cycle per > - pixel), "2x" (two clock cycles per pixel), "ddr" (one clock cycle per pixel, > - data driven on both edges). > - > -The following input format properties are required except in "rgb 1x" and > -"yuv444 1x" modes, in which case they must not be specified. > - > -- adi,input-style: The input components arrangement variant (1, 2 or 3), as > - listed in the input format tables in the datasheet. > -- adi,input-justification: The input bit justification ("left", "evenly", > - "right"). > - > -- avdd-supply: A 1.8V supply that powers up the AVDD pin on the chip. > -- dvdd-supply: A 1.8V supply that powers up the DVDD pin on the chip. > -- pvdd-supply: A 1.8V supply that powers up the PVDD pin on the chip. > -- dvdd-3v-supply: A 3.3V supply that powers up the pin called DVDD_3V > - on the chip. > -- bgvdd-supply: A 1.8V supply that powers up the BGVDD pin. This is > - needed only for ADV7511. > - > -The following properties are required for ADV7533 and ADV7535: > - > -- adi,dsi-lanes: Number of DSI data lanes connected to the DSI host. It should > - be one of 1, 2, 3 or 4. > -- a2vdd-supply: 1.8V supply that powers up the A2VDD pin on the chip. > -- v3p3-supply: A 3.3V supply that powers up the V3P3 pin on the chip. > -- v1p2-supply: A supply that powers up the V1P2 pin on the chip. It can be > - either 1.2V or 1.8V for ADV7533 but only 1.8V for ADV7535. > - > -Optional properties: > - > -- interrupts: Specifier for the ADV7511 interrupt > -- pd-gpios: Specifier for the GPIO connected to the power down signal > - > -- adi,clock-delay: Video data clock delay relative to the pixel clock, in ps > - (-1200 ps .. 1600 ps). Defaults to no delay. > -- adi,embedded-sync: The input uses synchronization signals embedded in the > - data stream (similar to BT.656). Defaults to separate H/V synchronization > - signals. > -- adi,disable-timing-generator: Only for ADV7533 and ADV7535. Disables the > - internal timing generator. The chip will rely on the sync signals in the > - DSI data lanes, rather than generate its own timings for HDMI output. > -- clocks: from common clock binding: reference to the CEC clock. > -- clock-names: from common clock binding: must be "cec". > -- reg-names : Names of maps with programmable addresses. > - It can contain any map needing a non-default address. > - Possible maps names are : "main", "edid", "cec", "packet" > - > -Required nodes: > - > -The ADV7511 has two video ports. Their connections are modelled using the OF > -graph bindings specified in Documentation/devicetree/bindings/graph.txt. > - > -- Video port 0 for the RGB, YUV or DSI input. In the case of ADV7533/5, the > - remote endpoint phandle should be a reference to a valid mipi_dsi_host device > - node. > -- Video port 1 for the HDMI output > -- Audio port 2 for the HDMI audio input > - > - > -Example > -------- > - > - adv7511w: hdmi@39 { > - compatible = "adi,adv7511w"; > - /* > - * The EDID page will be accessible on address 0x66 on the I2C > - * bus. All other maps continue to use their default addresses. > - */ > - reg = <0x39>, <0x66>; > - reg-names = "main", "edid"; > - interrupt-parent = <&gpio3>; > - interrupts = <29 IRQ_TYPE_EDGE_FALLING>; > - clocks = <&cec_clock>; > - clock-names = "cec"; > - > - adi,input-depth = <8>; > - adi,input-colorspace = "rgb"; > - adi,input-clock = "1x"; > - adi,input-style = <1>; > - adi,input-justification = "evenly"; > - > - ports { > - #address-cells = <1>; > - #size-cells = <0>; > - > - port@0 { > - reg = <0>; > - adv7511w_in: endpoint { > - remote-endpoint = <&dpi_out>; > - }; > - }; > - > - port@1 { > - reg = <1>; > - adv7511_out: endpoint { > - remote-endpoint = <&hdmi_connector_in>; > - }; > - }; > - > - port@2 { > - reg = <2>; > - codec_endpoint: endpoint { > - remote-endpoint = <&i2s0_cpu_endpoint>; > - }; > - }; > - }; > - }; > diff --git a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.yaml b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.yaml > new file mode 100644 > index 000000000000..a306adba105f > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.yaml > @@ -0,0 +1,233 @@ > +# SPDX-License-Identifier: GPL-2.0-only > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/bridge/adi,adv7511.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Analog Devices ADV7511/11W/13 HDMI Encoders > + > +maintainers: > + - Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + > +description: | > + The ADV7511, ADV7511W and ADV7513 are HDMI audio and video > + transmitters compatible with HDMI 1.4 and DVI 1.0. They support color > + space conversion, S/PDIF, CEC and HDCP. They support RGB input > + interface. I would write the last sentence as "The transmitter input is parallel RGB or YUV data." as YUV is also supported. > + > +properties: > + compatible: > + enum: > + - adi,adv7511 > + - adi,adv7511w > + - adi,adv7513 > + > + reg: > + description: | > + I2C slave addresses. > + > + The ADV7511/11W/13 internal registers are split into four pages > + exposed through different I2C addresses, creating four register > + maps. Each map has it own I2C address and acts as a standard slave > + device on the I2C bus. The main address is mandatory, others are > + optional and revert to defaults if not specified. > + minItems: 1 > + maxItems: 4 > + > + reg-names: > + description: > + Names of maps with programmable addresses. It can contain any map > + needing a non-default address. > + minItems: 1 > + items: > + - const: main > + - const: edid > + - const: cec > + - const: packet > + > + clocks: > + description: Reference to the CEC clock. > + maxItems: 1 > + > + clock-names: > + const: cec > + > + interrupts: > + maxItems: 1 > + > + pd-gpios: > + description: GPIO connected to the power down signal. > + maxItems: 1 > + > + avdd-supply: > + description: A 1.8V supply that powers up the AVDD pin. > + > + dvdd-supply: > + description: A 1.8V supply that powers up the DVDD pin. > + > + pvdd-supply: > + description: A 1.8V supply that powers up the PVDD pin. > + > + dvdd-3v-supply: > + description: A 3.3V supply that powers up the DVDD_3V pin. > + > + bgvdd-supply: > + description: A 1.8V supply that powers up the BGVDD pin. > + > + adi,input-depth: > + description: Number of bits per color component at the input. > + allOf: > + - $ref: /schemas/types.yaml#/definitions/uint32 > + - enum: [ 8, 10, 12 ] > + > + adi,input-colorspace: > + description: Input color space. > + allOf: > + - $ref: /schemas/types.yaml#/definitions/string > + - enum: [ rgb, yuv422, yuv444 ] Isn't string implied ? Can't you write adi,input-colorspace: description: Input color space. enum: [ rgb, yuv422, yuv444 ] Same for the other properties below. > + > + adi,input-clock: > + description: | > + Input clock type. > + "1x": one clock cycle per pixel > + "2x": two clock cycles per pixel > + "dd": one clock cycle per pixel, data driven on both edges > + allOf: > + - $ref: /schemas/types.yaml#/definitions/string > + - enum: [ 1x, 2x, dd ] > + > + adi,clock-delay: > + description: > + Video data clock delay relative to the pixel clock, in ps > + (-1200ps .. 1600 ps). > + allOf: > + - $ref: /schemas/types.yaml#/definitions/uint32 > + - default: 0 > + > + adi,embedded-sync: > + description: > + The input uses synchronization signals embedded in the data > + stream (similar to BT.656). Defaults to 0 (separate H/V > + synchronization signals). > + allOf: > + - $ref: /schemas/types.yaml#/definitions/uint32 > + - enum: [ 0, 1 ] > + - default: 0 This be a boolean property (it is read as a bool by the driver, the property being absent means false, the property being present means true). > + > + adi,input-style: > + description: > + Input components arrangement variant as listed in the input > + format tables in the datasheet. > + allOf: > + - $ref: /schemas/types.yaml#/definitions/uint32 > + - enum: [ 1, 2, 3 ] > + > + adi,input-justification: > + description: Input bit justification. > + allOf: > + - $ref: /schemas/types.yaml#/definitions/string > + - enum: [ left, evenly, right ] > + > + ports: > + description: > + The ADV7511(W)/13 has two video ports and one audio port. This node > + models their connections as documented in > + Documentation/devicetree/bindings/media/video-interfaces.txt > + Documentation/devicetree/bindings/graph.txt > + type: object > + properties: > + port@0: > + description: Video port for the RGB, YUV or DSI input. s/RGB, YUV or DSI/RGB or YUV/ > + type: object > + > + port@1: > + description: Video port for the HDMI output. > + type: object > + > + port@2: > + description: Audio port for the HDMI output. > + type: object > + > +# adi,input-colorspace and adi,input-clock are required except in > +# "rgb 1x" and "yuv444 1x" modes, in which case they must not be > +# specified. > +if: > + not: > + properties: > + adi,input-colorspace: > + contains: > + enum: [ rgb, yuv444 ] > + adi,input-clock: > + contains: > + const: 1x As both properties take a single value, I think you can omit "contains:". > + > +then: > + required: > + - adi,input-style > + - adi,input-justification > + > +else: > + properties: > + adi,input-style: false > + adi,input-justification: false > + > + > +required: > + - compatible > + - reg > + - ports > + - adi,input-depth > + - adi,input-colorspace > + - adi,input-clock Shouldn't the power supplies be required ? > + > +examples: > + - | > + #include <dt-bindings/interrupt-controller/irq.h> > + > + adv7511w: hdmi@39 { > + compatible = "adi,adv7511w"; > + /* > + * The EDID page will be accessible on address 0x66 on the I2C > + * bus. All other maps continue to use their default addresses. > + */ > + reg = <0x39>, <0x66>; > + reg-names = "main", "edid"; > + interrupt-parent = <&gpio3>; > + interrupts = <29 IRQ_TYPE_EDGE_FALLING>; > + clocks = <&cec_clock>; > + clock-names = "cec"; > + > + adi,input-depth = <8>; > + adi,input-colorspace = "yuv422"; > + adi,input-clock = "1x"; > + > + adi,input-style = <3>; > + adi,input-justification = "right"; > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + adv7511w_in: endpoint { > + remote-endpoint = <&dpi_out>; > + }; > + }; > + > + port@1 { > + reg = <1>; > + adv7511_out: endpoint { > + remote-endpoint = <&hdmi_connector_in>; > + }; > + }; > + > + port@2 { > + reg = <2>; > + codec_endpoint: endpoint { > + remote-endpoint = <&i2s0_cpu_endpoint>; > + }; > + }; > + }; > + }; > + > +... > diff --git a/Documentation/devicetree/bindings/display/bridge/adi,adv7533.yaml b/Documentation/devicetree/bindings/display/bridge/adi,adv7533.yaml > new file mode 100644 > index 000000000000..dfcc63dfc5c5 > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7533.yaml > @@ -0,0 +1,166 @@ > +# SPDX-License-Identifier: GPL-2.0-only > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/bridge/adi,adv7533.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Analog Devices ADV7533/35 HDMI Encoders > + > +maintainers: > + - Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + > +description: | > + The ADV7533 and ADV7535 are HDMI audio and video transmitters > + compatible with HDMI 1.4 and DVI 1.0. They support color space > + conversion, S/PDIF, CEC and HDCP. They support DSI for input pixels. I would write the last sentence as "The transmitter input is MIPI DSI.". > + > +properties: > + compatible: > + enum: > + - adi,adv7533 > + - adi,adv7535 > + > + reg: > + description: | > + I2C slave addresses. > + > + The ADV7533/35 internal registers are split into four pages > + exposed through different I2C addresses, creating four register > + maps. Each map has it own I2C address and acts as a standard slave > + device on the I2C bus. The main address is mandatory, others are > + optional and revert to defaults if not specified. > + minItems: 1 > + maxItems: 4 > + > + reg-names: > + description: > + Names of maps with programmable addresses. It can contain any map > + needing a non-default address. > + minItems: 1 > + items: > + - const: main > + - const: edid > + - const: cec > + - const: packet > + > + clocks: > + description: Reference to the CEC clock. > + maxItems: 1 > + > + clock-names: > + const: cec > + > + interrupts: > + maxItems: 1 > + > + pd-gpios: > + description: GPIO connected to the power down signal. > + maxItems: 1 > + > + avdd-supply: > + description: A 1.8V supply that powers up the AVDD pin. > + > + dvdd-supply: > + description: A 1.8V supply that powers up the DVDD pin. > + > + pvdd-supply: > + description: A 1.8V supply that powers up the PVDD pin. > + > + a2vdd-supply: > + description: A 1.8V supply that powers up the A2VDD pin. > + > + v3p3-supply: > + description: A 3.3V supply that powers up the V3P3 pin. > + > + v1p2-supply: > + description: > + A supply that powers up the V1P2 pin. It can be either 1.2V > + or 1.8V for ADV7533 but only 1.8V for ADV7535. > + > + adi,disable-timing-generator: > + description: > + Disables the interal timing generator. The chip will rely on the s/interal/internal/ > + sync signals in the DSI data lanes, rather than generate its own s/generate/generating/ > + timings for HDMI output. > + type: boolean > + > + adi,dsi-lanes: > + description: Number of DSI data lanes connected to the DSI host. > + allOf: > + - $ref: /schemas/types.yaml#/definitions/uint32 > + - enum: [ 1, 2, 3, 4 ] > + > + ports: > + description: > + The ADV7533/35 has two video ports and one audio port. This node > + models their connections as documented in > + Documentation/devicetree/bindings/media/video-interfaces.txt > + Documentation/devicetree/bindings/graph.txt > + type: object > + properties: > + port@0: > + description: > + Video port for the RGB, YUV or DSI input. The remote endpoint s/RGB, YUV or // > + phandle should be a reference to a valid mipi_dsi_host_device. > + type: object > + > + port@1: > + description: Video port for the HDMI output. > + type: object > + > + port@2: > + description: Audio port for the HDMI output. > + type: object > + > +required: > + - compatible > + - reg > + - ports > + - adi,dsi-lanes Shouldn't the power supplies be required ? > + > +examples: > + - | > + #include <dt-bindings/interrupt-controller/irq.h> > + > + adv7533: hdmi@39 { > + compatible = "adi,adv7533"; > + /* > + * The EDID page will be accessible on address 0x66 on the I2C > + * bus. All other maps continue to use their default addresses. > + */ > + reg = <0x39>, <0x66>; > + reg-names = "main", "edid"; > + interrupt-parent = <&gpio3>; > + interrupts = <29 IRQ_TYPE_EDGE_FALLING>; > + clocks = <&cec_clock>; > + clock-names = "cec"; > + adi,dsi-lanes = <4>; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + adv7511w_in: endpoint { > + remote-endpoint = <&dpi_out>; > + }; > + }; > + > + port@1 { > + reg = <1>; > + adv7511_out: endpoint { > + remote-endpoint = <&hdmi_connector_in>; > + }; > + }; The name of the two endpoints doesn't match the adv7533. The remote endpoint phandle for port 0 should have dsi in its name. > + > + port@2 { > + reg = <2>; > + codec_endpoint: endpoint { > + remote-endpoint = <&i2s0_cpu_endpoint>; > + }; > + }; > + }; > + }; > + > +...
Hi Laurent, thanks for the thorough review. Some comments below: On jue 14-05-2020 04:54:12, Laurent Pinchart wrote: > > +description: | > > + The ADV7511, ADV7511W and ADV7513 are HDMI audio and video > > + transmitters compatible with HDMI 1.4 and DVI 1.0. They support color > > + space conversion, S/PDIF, CEC and HDCP. They support RGB input > > + interface. > > I would write the last sentence as "The transmitter input is parallel > RGB or YUV data." as YUV is also supported. Ok. > > + adi,input-colorspace: > > + description: Input color space. > > + allOf: > > + - $ref: /schemas/types.yaml#/definitions/string > > + - enum: [ rgb, yuv422, yuv444 ] > > Isn't string implied ? Can't you write > > adi,input-colorspace: > description: Input color space. > enum: [ rgb, yuv422, yuv444 ] example-schema.yaml says that Vendor specific properties have slightly different schema requirements than common properties. They must have at least a type definition and 'description'. However, dt_binding_check doesn't seem to enforce this rule for string properties, and I saw a couple of vendor-specific string properties in other bindings that don't define the type either, so I'm going to follow your suggestion but only for string properties, the rest need a type definition. I noticed I can remove the "allOf" keywords from these too. > > + adi,embedded-sync: > > + description: > > + The input uses synchronization signals embedded in the data > > + stream (similar to BT.656). Defaults to 0 (separate H/V > > + synchronization signals). > > + allOf: > > + - $ref: /schemas/types.yaml#/definitions/uint32 > > + - enum: [ 0, 1 ] > > + - default: 0 > > This be a boolean property (it is read as a bool by the driver, the > property being absent means false, the property being present means > true). You're completely right. > > + ports: > > + description: > > + The ADV7511(W)/13 has two video ports and one audio port. This node > > + models their connections as documented in > > + Documentation/devicetree/bindings/media/video-interfaces.txt > > + Documentation/devicetree/bindings/graph.txt > > + type: object > > + properties: > > + port@0: > > + description: Video port for the RGB, YUV or DSI input. > > s/RGB, YUV or DSI/RGB or YUV/ Ok. > > +if: > > + not: > > + properties: > > + adi,input-colorspace: > > + contains: > > + enum: [ rgb, yuv444 ] > > + adi,input-clock: > > + contains: > > + const: 1x > > As both properties take a single value, I think you can omit > "contains:". I think it's required here, removing it makes the test fail. > > +required: > > + - compatible > > + - reg > > + - ports > > + - adi,input-depth > > + - adi,input-colorspace > > + - adi,input-clock > > Shouldn't the power supplies be required ? Good question. The original binding is not completely clear on this. There's a "Required properties" section at the top that doesn't include the supplies, the supplies block for the ADV7511/11w/13 looks like a gray area in that regard, while the same block for ADV7533/35 is more explicit in that they are required, and they are not listed in the "Optional properties" section. However, most of the DTs that use this binding don't define any supplies. And AFAICT from looking at the code, regulator_get() will use a dummy regulator and show a warning for every expected regulator that's not defined in the DT. If we want to be more strict and require the definition of all the supplies, there will be many more DTs changes in the series, and I'm not sure I'll be able to do that in a reasonable amount of time. I'm looking at them and it's not always clear which regulators to use or if they are even defined. > > +description: | > > + The ADV7533 and ADV7535 are HDMI audio and video transmitters > > + compatible with HDMI 1.4 and DVI 1.0. They support color space > > + conversion, S/PDIF, CEC and HDCP. They support DSI for input pixels. > > I would write the last sentence as "The transmitter input is MIPI DSI.". > > ... > > > + Disables the interal timing generator. The chip will rely on the > > s/interal/internal/ > > > + sync signals in the DSI data lanes, rather than generate its own > > s/generate/generating/ > > ... > > > + properties: > > + port@0: > > + description: > > + Video port for the RGB, YUV or DSI input. The remote endpoint > > s/RGB, YUV or // Ok. > > + ports { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + port@0 { > > + reg = <0>; > > + adv7511w_in: endpoint { > > + remote-endpoint = <&dpi_out>; > > + }; > > + }; > > + > > + port@1 { > > + reg = <1>; > > + adv7511_out: endpoint { > > + remote-endpoint = <&hdmi_connector_in>; > > + }; > > + }; > > The name of the two endpoints doesn't match the adv7533. The remote > endpoint phandle for port 0 should have dsi in its name. Oops, thanks for catching these. Cheers, Ricardo
Hi Ricardo, On Thu, May 14, 2020 at 11:36:17AM +0200, Ricardo Cañuelo wrote: > Hi Laurent, thanks for the thorough review. Some comments below: > > On jue 14-05-2020 04:54:12, Laurent Pinchart wrote: > > > +description: | > > > + The ADV7511, ADV7511W and ADV7513 are HDMI audio and video > > > + transmitters compatible with HDMI 1.4 and DVI 1.0. They support color > > > + space conversion, S/PDIF, CEC and HDCP. They support RGB input > > > + interface. > > > > I would write the last sentence as "The transmitter input is parallel > > RGB or YUV data." as YUV is also supported. > > Ok. > > > > + adi,input-colorspace: > > > + description: Input color space. > > > + allOf: > > > + - $ref: /schemas/types.yaml#/definitions/string > > > + - enum: [ rgb, yuv422, yuv444 ] > > > > Isn't string implied ? Can't you write > > > > adi,input-colorspace: > > description: Input color space. > > enum: [ rgb, yuv422, yuv444 ] > > example-schema.yaml says that > > Vendor specific properties have slightly different schema > requirements than common properties. They must have at least a type > definition and 'description'. > > However, dt_binding_check doesn't seem to enforce this rule for string > properties, and I saw a couple of vendor-specific string properties in > other bindings that don't define the type either, so I'm going to follow > your suggestion but only for string properties, the rest need a type > definition. I'll defer to Rob to tell the law here :-) > I noticed I can remove the "allOf" keywords from these too. > > > > + adi,embedded-sync: > > > + description: > > > + The input uses synchronization signals embedded in the data > > > + stream (similar to BT.656). Defaults to 0 (separate H/V > > > + synchronization signals). > > > + allOf: > > > + - $ref: /schemas/types.yaml#/definitions/uint32 > > > + - enum: [ 0, 1 ] > > > + - default: 0 > > > > This be a boolean property (it is read as a bool by the driver, the > > property being absent means false, the property being present means > > true). > > You're completely right. > > > > + ports: > > > + description: > > > + The ADV7511(W)/13 has two video ports and one audio port. This node > > > + models their connections as documented in > > > + Documentation/devicetree/bindings/media/video-interfaces.txt > > > + Documentation/devicetree/bindings/graph.txt > > > + type: object > > > + properties: > > > + port@0: > > > + description: Video port for the RGB, YUV or DSI input. > > > > s/RGB, YUV or DSI/RGB or YUV/ > > Ok. > > > > +if: > > > + not: > > > + properties: > > > + adi,input-colorspace: > > > + contains: > > > + enum: [ rgb, yuv444 ] > > > + adi,input-clock: > > > + contains: > > > + const: 1x > > > > As both properties take a single value, I think you can omit > > "contains:". > > I think it's required here, removing it makes the test fail. I thought the following could work: if: not: properties: adi,input-colorspace: enum: [ rgb, yuv444 ] adi,input-clock: items: - const: 1x But no big deal, contains: is ok too. > > > +required: > > > + - compatible > > > + - reg > > > + - ports > > > + - adi,input-depth > > > + - adi,input-colorspace > > > + - adi,input-clock > > > > Shouldn't the power supplies be required ? > > Good question. The original binding is not completely clear on > this. There's a "Required properties" section at the top that doesn't > include the supplies, the supplies block for the ADV7511/11w/13 looks > like a gray area in that regard, while the same block for ADV7533/35 is > more explicit in that they are required, and they are not listed in the > "Optional properties" section. > > However, most of the DTs that use this binding don't define any > supplies. And AFAICT from looking at the code, regulator_get() will use > a dummy regulator and show a warning for every expected regulator that's > not defined in the DT. > > If we want to be more strict and require the definition of all the > supplies, there will be many more DTs changes in the series, and I'm not > sure I'll be able to do that in a reasonable amount of time. I'm looking > at them and it's not always clear which regulators to use or if they are > even defined. We can decouple the two though (I think). The bindings should reflect what we consider right, and the dts files could be fixed on top. > > > +description: | > > > + The ADV7533 and ADV7535 are HDMI audio and video transmitters > > > + compatible with HDMI 1.4 and DVI 1.0. They support color space > > > + conversion, S/PDIF, CEC and HDCP. They support DSI for input pixels. > > > > I would write the last sentence as "The transmitter input is MIPI DSI.". > > > > ... > > > > > + Disables the interal timing generator. The chip will rely on the > > > > s/interal/internal/ > > > > > + sync signals in the DSI data lanes, rather than generate its own > > > > s/generate/generating/ > > > > ... > > > > > + properties: > > > + port@0: > > > + description: > > > + Video port for the RGB, YUV or DSI input. The remote endpoint > > > > s/RGB, YUV or // > > Ok. > > > > + ports { > > > + #address-cells = <1>; > > > + #size-cells = <0>; > > > + > > > + port@0 { > > > + reg = <0>; > > > + adv7511w_in: endpoint { > > > + remote-endpoint = <&dpi_out>; > > > + }; > > > + }; > > > + > > > + port@1 { > > > + reg = <1>; > > > + adv7511_out: endpoint { > > > + remote-endpoint = <&hdmi_connector_in>; > > > + }; > > > + }; > > > > The name of the two endpoints doesn't match the adv7533. The remote > > endpoint phandle for port 0 should have dsi in its name. > > Oops, thanks for catching these.
On Thu, May 14, 2020 at 06:22:39PM +0300, Laurent Pinchart wrote: > Hi Ricardo, > > On Thu, May 14, 2020 at 11:36:17AM +0200, Ricardo Cañuelo wrote: > > Hi Laurent, thanks for the thorough review. Some comments below: > > > > On jue 14-05-2020 04:54:12, Laurent Pinchart wrote: > > > > +description: | > > > > + The ADV7511, ADV7511W and ADV7513 are HDMI audio and video > > > > + transmitters compatible with HDMI 1.4 and DVI 1.0. They support color > > > > + space conversion, S/PDIF, CEC and HDCP. They support RGB input > > > > + interface. > > > > > > I would write the last sentence as "The transmitter input is parallel > > > RGB or YUV data." as YUV is also supported. > > > > Ok. > > > > > > + adi,input-colorspace: > > > > + description: Input color space. > > > > + allOf: > > > > + - $ref: /schemas/types.yaml#/definitions/string > > > > + - enum: [ rgb, yuv422, yuv444 ] > > > > > > Isn't string implied ? Can't you write > > > > > > adi,input-colorspace: > > > description: Input color space. > > > enum: [ rgb, yuv422, yuv444 ] > > > > example-schema.yaml says that > > > > Vendor specific properties have slightly different schema > > requirements than common properties. They must have at least a type > > definition and 'description'. > > > > However, dt_binding_check doesn't seem to enforce this rule for string > > properties, and I saw a couple of vendor-specific string properties in > > other bindings that don't define the type either, so I'm going to follow > > your suggestion but only for string properties, the rest need a type > > definition. > > I'll defer to Rob to tell the law here :-) Yes, if you have a string with defined values, then a type isn't needed. That only applies to strings as numeric values need a size. > > > I noticed I can remove the "allOf" keywords from these too. Yes, that's a recent change. Both forms still work. > > > > + adi,embedded-sync: > > > > + description: > > > > + The input uses synchronization signals embedded in the data > > > > + stream (similar to BT.656). Defaults to 0 (separate H/V > > > > + synchronization signals). > > > > + allOf: > > > > + - $ref: /schemas/types.yaml#/definitions/uint32 > > > > + - enum: [ 0, 1 ] > > > > + - default: 0 > > > > > > This be a boolean property (it is read as a bool by the driver, the > > > property being absent means false, the property being present means > > > true). > > > > You're completely right. > > > > > > + ports: > > > > + description: > > > > + The ADV7511(W)/13 has two video ports and one audio port. This node > > > > + models their connections as documented in > > > > + Documentation/devicetree/bindings/media/video-interfaces.txt > > > > + Documentation/devicetree/bindings/graph.txt > > > > + type: object > > > > + properties: > > > > + port@0: > > > > + description: Video port for the RGB, YUV or DSI input. > > > > > > s/RGB, YUV or DSI/RGB or YUV/ > > > > Ok. > > > > > > +if: > > > > + not: > > > > + properties: > > > > + adi,input-colorspace: > > > > + contains: > > > > + enum: [ rgb, yuv444 ] > > > > + adi,input-clock: > > > > + contains: > > > > + const: 1x > > > > > > As both properties take a single value, I think you can omit > > > "contains:". > > > > I think it's required here, removing it makes the test fail. > > I thought the following could work: > > if: > not: > properties: > adi,input-colorspace: > enum: [ rgb, yuv444 ] > adi,input-clock: > items: > - const: 1x > > But no big deal, contains: is ok too. In theory the above should work. However, this is probably a case where we don't fix-up the properties. If you look at the DT yaml encoding, everything is an array (as dtc doesn't know). For schemas, the tooling expands scalars to arrays. Rob
Hi Laurent, On jue 14-05-2020 18:22:39, Laurent Pinchart wrote: > > If we want to be more strict and require the definition of all the > > supplies, there will be many more DTs changes in the series, and I'm not > > sure I'll be able to do that in a reasonable amount of time. I'm looking > > at them and it's not always clear which regulators to use or if they are > > even defined. > > We can decouple the two though (I think). The bindings should reflect > what we consider right, and the dts files could be fixed on top. Do you have a suggestion on how to do this? If we decouple the two tasks most of the work would be searching for DTs to fix and finding a way to fix each one of them, and unless I do this _before_ the binding conversion I'll get a lot of dtbs_check errors. The binding conversion itself is done, if we go this route the only additional change would be to make the supplies required. Cheers, Ricardo
Hi Ricardo, On Mon, May 25, 2020 at 09:43:35AM +0200, Ricardo Cañuelo wrote: > On jue 14-05-2020 18:22:39, Laurent Pinchart wrote: > > > If we want to be more strict and require the definition of all the > > > supplies, there will be many more DTs changes in the series, and I'm not > > > sure I'll be able to do that in a reasonable amount of time. I'm looking > > > at them and it's not always clear which regulators to use or if they are > > > even defined. > > > > We can decouple the two though (I think). The bindings should reflect > > what we consider right, and the dts files could be fixed on top. > > Do you have a suggestion on how to do this? If we decouple the two > tasks most of the work would be searching for DTs to fix and finding a > way to fix each one of them, and unless I do this _before_ the binding > conversion I'll get a lot of dtbs_check errors. Rob should answer this question as it will be his decision, but I've personally never considered non-compliant DT sources to be an obstacle to bindings conversion to YAML. The DT sources should be fixed, but I don't see it as a prerequisite (although it's a good practice). > The binding conversion itself is done, if we go this route the only > additional change would be to make the supplies required.
Hi Laurent, On Tue, May 26, 2020 at 3:44 AM Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > On Mon, May 25, 2020 at 09:43:35AM +0200, Ricardo Cañuelo wrote: > > On jue 14-05-2020 18:22:39, Laurent Pinchart wrote: > > > > If we want to be more strict and require the definition of all the > > > > supplies, there will be many more DTs changes in the series, and I'm not > > > > sure I'll be able to do that in a reasonable amount of time. I'm looking > > > > at them and it's not always clear which regulators to use or if they are > > > > even defined. > > > > > > We can decouple the two though (I think). The bindings should reflect > > > what we consider right, and the dts files could be fixed on top. > > > > Do you have a suggestion on how to do this? If we decouple the two > > tasks most of the work would be searching for DTs to fix and finding a > > way to fix each one of them, and unless I do this _before_ the binding > > conversion I'll get a lot of dtbs_check errors. > > Rob should answer this question as it will be his decision, but I've > personally never considered non-compliant DT sources to be an obstacle > to bindings conversion to YAML. The DT sources should be fixed, but I > don't see it as a prerequisite (although it's a good practice). I do my best to avoid introducing regressions when the binding conversions go upstream. FTR, hence patches 1-3 are already in v5.7-rc7. Gr{oetje,eeting}s, Geert
Hi Geert, On Tue, May 26, 2020 at 09:03:09AM +0200, Geert Uytterhoeven wrote: > On Tue, May 26, 2020 at 3:44 AM Laurent Pinchart wrote: > > On Mon, May 25, 2020 at 09:43:35AM +0200, Ricardo Cañuelo wrote: > > > On jue 14-05-2020 18:22:39, Laurent Pinchart wrote: > > > > > If we want to be more strict and require the definition of all the > > > > > supplies, there will be many more DTs changes in the series, and I'm not > > > > > sure I'll be able to do that in a reasonable amount of time. I'm looking > > > > > at them and it's not always clear which regulators to use or if they are > > > > > even defined. > > > > > > > > We can decouple the two though (I think). The bindings should reflect > > > > what we consider right, and the dts files could be fixed on top. > > > > > > Do you have a suggestion on how to do this? If we decouple the two > > > tasks most of the work would be searching for DTs to fix and finding a > > > way to fix each one of them, and unless I do this _before_ the binding > > > conversion I'll get a lot of dtbs_check errors. > > > > Rob should answer this question as it will be his decision, but I've > > personally never considered non-compliant DT sources to be an obstacle > > to bindings conversion to YAML. The DT sources should be fixed, but I > > don't see it as a prerequisite (although it's a good practice). > > I do my best to avoid introducing regressions when the binding conversions > go upstream. Please note that we're not talking about runtime regressions, as drivers are not updated. It's "only" dtbs_check that would produce new errors. > FTR, hence patches 1-3 are already in v5.7-rc7.
Hi Laurent, On Tue, May 26, 2020 at 12:12 PM Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > On Tue, May 26, 2020 at 09:03:09AM +0200, Geert Uytterhoeven wrote: > > On Tue, May 26, 2020 at 3:44 AM Laurent Pinchart wrote: > > > On Mon, May 25, 2020 at 09:43:35AM +0200, Ricardo Cañuelo wrote: > > > > On jue 14-05-2020 18:22:39, Laurent Pinchart wrote: > > > > > > If we want to be more strict and require the definition of all the > > > > > > supplies, there will be many more DTs changes in the series, and I'm not > > > > > > sure I'll be able to do that in a reasonable amount of time. I'm looking > > > > > > at them and it's not always clear which regulators to use or if they are > > > > > > even defined. > > > > > > > > > > We can decouple the two though (I think). The bindings should reflect > > > > > what we consider right, and the dts files could be fixed on top. > > > > > > > > Do you have a suggestion on how to do this? If we decouple the two > > > > tasks most of the work would be searching for DTs to fix and finding a > > > > way to fix each one of them, and unless I do this _before_ the binding > > > > conversion I'll get a lot of dtbs_check errors. > > > > > > Rob should answer this question as it will be his decision, but I've > > > personally never considered non-compliant DT sources to be an obstacle > > > to bindings conversion to YAML. The DT sources should be fixed, but I > > > don't see it as a prerequisite (although it's a good practice). > > > > I do my best to avoid introducing regressions when the binding conversions > > go upstream. > > Please note that we're not talking about runtime regressions, as drivers > are not updated. It's "only" dtbs_check that would produce new errors. Exactly. I was talking about "make dtbs_check" regressions, too. Gr{oetje,eeting}s, Geert
On Tue, 2020-05-26 at 12:39 +0200, Geert Uytterhoeven wrote: > Hi Laurent, > > On Tue, May 26, 2020 at 12:12 PM Laurent Pinchart > <laurent.pinchart@ideasonboard.com> wrote: > > On Tue, May 26, 2020 at 09:03:09AM +0200, Geert Uytterhoeven wrote: > > > On Tue, May 26, 2020 at 3:44 AM Laurent Pinchart wrote: > > > > On Mon, May 25, 2020 at 09:43:35AM +0200, Ricardo Cañuelo wrote: > > > > > On jue 14-05-2020 18:22:39, Laurent Pinchart wrote: > > > > > > > If we want to be more strict and require the definition of all the > > > > > > > supplies, there will be many more DTs changes in the series, and I'm not > > > > > > > sure I'll be able to do that in a reasonable amount of time. I'm looking > > > > > > > at them and it's not always clear which regulators to use or if they are > > > > > > > even defined. > > > > > > > > > > > > We can decouple the two though (I think). The bindings should reflect > > > > > > what we consider right, and the dts files could be fixed on top. > > > > > > > > > > Do you have a suggestion on how to do this? If we decouple the two > > > > > tasks most of the work would be searching for DTs to fix and finding a > > > > > way to fix each one of them, and unless I do this _before_ the binding > > > > > conversion I'll get a lot of dtbs_check errors. > > > > > > > > Rob should answer this question as it will be his decision, but I've > > > > personally never considered non-compliant DT sources to be an obstacle > > > > to bindings conversion to YAML. The DT sources should be fixed, but I > > > > don't see it as a prerequisite (although it's a good practice). > > > > > > I do my best to avoid introducing regressions when the binding conversions > > > go upstream. > > > > Please note that we're not talking about runtime regressions, as drivers > > are not updated. It's "only" dtbs_check that would produce new errors. > > Exactly. I was talking about "make dtbs_check" regressions, too. > A "make dtbs_check" failure, due to some foo.dts that fails the check due to a correct YAML conversion, can't be considered a regression. I strongly agree with Laurent here, we want to convert the bindings to YAML and we can't consider non-compliant DT sources to prevent them. Regards, Ezequiel
On Tue, May 26, 2020 at 1:03 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Laurent, > > On Tue, May 26, 2020 at 3:44 AM Laurent Pinchart > <laurent.pinchart@ideasonboard.com> wrote: > > On Mon, May 25, 2020 at 09:43:35AM +0200, Ricardo Cañuelo wrote: > > > On jue 14-05-2020 18:22:39, Laurent Pinchart wrote: > > > > > If we want to be more strict and require the definition of all the > > > > > supplies, there will be many more DTs changes in the series, and I'm not > > > > > sure I'll be able to do that in a reasonable amount of time. I'm looking > > > > > at them and it's not always clear which regulators to use or if they are > > > > > even defined. > > > > > > > > We can decouple the two though (I think). The bindings should reflect > > > > what we consider right, and the dts files could be fixed on top. > > > > > > Do you have a suggestion on how to do this? If we decouple the two > > > tasks most of the work would be searching for DTs to fix and finding a > > > way to fix each one of them, and unless I do this _before_ the binding > > > conversion I'll get a lot of dtbs_check errors. > > > > Rob should answer this question as it will be his decision, but I've > > personally never considered non-compliant DT sources to be an obstacle > > to bindings conversion to YAML. The DT sources should be fixed, but I > > don't see it as a prerequisite (although it's a good practice). There's currently no requirement that binding schema don't introduce warnings in dts files. That should change when/if we get to a warning free state (probably per platform/family). I don't think we're close on any platform? (If we are, I'd like to start tracking that). It is good to pay attention to the warnings you get though as the schema may not be doing what you expect or the binding really doesn't match reality. > I do my best to avoid introducing regressions when the binding conversions > go upstream. Meaning you fix the dts files or massage the schema to match? If we just adjust schema to match, what's the point in this effort? We should find things wrong or ill defined. Rob
Hi Rob, On Wed, May 27, 2020 at 7:30 PM Rob Herring <robh+dt@kernel.org> wrote: > On Tue, May 26, 2020 at 1:03 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > On Tue, May 26, 2020 at 3:44 AM Laurent Pinchart > > <laurent.pinchart@ideasonboard.com> wrote: > > > On Mon, May 25, 2020 at 09:43:35AM +0200, Ricardo Cañuelo wrote: > > > > On jue 14-05-2020 18:22:39, Laurent Pinchart wrote: > > > > > > If we want to be more strict and require the definition of all the > > > > > > supplies, there will be many more DTs changes in the series, and I'm not > > > > > > sure I'll be able to do that in a reasonable amount of time. I'm looking > > > > > > at them and it's not always clear which regulators to use or if they are > > > > > > even defined. > > > > > > > > > > We can decouple the two though (I think). The bindings should reflect > > > > > what we consider right, and the dts files could be fixed on top. > > > > > > > > Do you have a suggestion on how to do this? If we decouple the two > > > > tasks most of the work would be searching for DTs to fix and finding a > > > > way to fix each one of them, and unless I do this _before_ the binding > > > > conversion I'll get a lot of dtbs_check errors. > > > > > > Rob should answer this question as it will be his decision, but I've > > > personally never considered non-compliant DT sources to be an obstacle > > > to bindings conversion to YAML. The DT sources should be fixed, but I > > > don't see it as a prerequisite (although it's a good practice). > > There's currently no requirement that binding schema don't introduce > warnings in dts files. That should change when/if we get to a warning > free state (probably per platform/family). I don't think we're close > on any platform? (If we are, I'd like to start tracking that). It is > good to pay attention to the warnings you get though as the schema may > not be doing what you expect or the binding really doesn't match > reality. OK. > > I do my best to avoid introducing regressions when the binding conversions > > go upstream. > > Meaning you fix the dts files or massage the schema to match? If we > just adjust schema to match, what's the point in this effort? We > should find things wrong or ill defined. I fix up DTS files, and fast-track those fixes, so they appear upstream before the DT binding conversion, where possible. Gr{oetje,eeting}s, Geert
Hi all, On mié 27-05-2020 20:18:07, Geert Uytterhoeven wrote: > > There's currently no requirement that binding schema don't introduce > > warnings in dts files. That should change when/if we get to a warning > > free state (probably per platform/family). I don't think we're close > > on any platform? (If we are, I'd like to start tracking that). It is > > good to pay attention to the warnings you get though as the schema may > > not be doing what you expect or the binding really doesn't match > > reality. > > OK. > > > > I do my best to avoid introducing regressions when the binding conversions > > > go upstream. > > > > Meaning you fix the dts files or massage the schema to match? If we > > just adjust schema to match, what's the point in this effort? We > > should find things wrong or ill defined. > > I fix up DTS files, and fast-track those fixes, so they appear upstream before > the DT binding conversion, where possible. Thank you everyone for pitching in and for clarifying the procedure, I'll prepare a new series with the changes that Laurent proposed (without the patches that Geert already merged). Cheers, Ricardo
diff --git a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt deleted file mode 100644 index 659523f538bf..000000000000 --- a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt +++ /dev/null @@ -1,143 +0,0 @@ -Analog Devices ADV7511(W)/13/33/35 HDMI Encoders ------------------------------------------------- - -The ADV7511, ADV7511W, ADV7513, ADV7533 and ADV7535 are HDMI audio and video -transmitters compatible with HDMI 1.4 and DVI 1.0. They support color space -conversion, S/PDIF, CEC and HDCP. ADV7533/5 supports the DSI interface for input -pixels, while the others support RGB interface. - -Required properties: - -- compatible: Should be one of: - "adi,adv7511" - "adi,adv7511w" - "adi,adv7513" - "adi,adv7533" - "adi,adv7535" - -- reg: I2C slave addresses - The ADV7511 internal registers are split into four pages exposed through - different I2C addresses, creating four register maps. Each map has it own - I2C address and acts as a standard slave device on the I2C bus. The main - address is mandatory, others are optional and revert to defaults if not - specified. - - -The ADV7511 supports a large number of input data formats that differ by their -color depth, color format, clock mode, bit justification and random -arrangement of components on the data bus. The combination of the following -properties describe the input and map directly to the video input tables of the -ADV7511 datasheet that document all the supported combinations. - -- adi,input-depth: Number of bits per color component at the input (8, 10 or - 12). -- adi,input-colorspace: The input color space, one of "rgb", "yuv422" or - "yuv444". -- adi,input-clock: The input clock type, one of "1x" (one clock cycle per - pixel), "2x" (two clock cycles per pixel), "ddr" (one clock cycle per pixel, - data driven on both edges). - -The following input format properties are required except in "rgb 1x" and -"yuv444 1x" modes, in which case they must not be specified. - -- adi,input-style: The input components arrangement variant (1, 2 or 3), as - listed in the input format tables in the datasheet. -- adi,input-justification: The input bit justification ("left", "evenly", - "right"). - -- avdd-supply: A 1.8V supply that powers up the AVDD pin on the chip. -- dvdd-supply: A 1.8V supply that powers up the DVDD pin on the chip. -- pvdd-supply: A 1.8V supply that powers up the PVDD pin on the chip. -- dvdd-3v-supply: A 3.3V supply that powers up the pin called DVDD_3V - on the chip. -- bgvdd-supply: A 1.8V supply that powers up the BGVDD pin. This is - needed only for ADV7511. - -The following properties are required for ADV7533 and ADV7535: - -- adi,dsi-lanes: Number of DSI data lanes connected to the DSI host. It should - be one of 1, 2, 3 or 4. -- a2vdd-supply: 1.8V supply that powers up the A2VDD pin on the chip. -- v3p3-supply: A 3.3V supply that powers up the V3P3 pin on the chip. -- v1p2-supply: A supply that powers up the V1P2 pin on the chip. It can be - either 1.2V or 1.8V for ADV7533 but only 1.8V for ADV7535. - -Optional properties: - -- interrupts: Specifier for the ADV7511 interrupt -- pd-gpios: Specifier for the GPIO connected to the power down signal - -- adi,clock-delay: Video data clock delay relative to the pixel clock, in ps - (-1200 ps .. 1600 ps). Defaults to no delay. -- adi,embedded-sync: The input uses synchronization signals embedded in the - data stream (similar to BT.656). Defaults to separate H/V synchronization - signals. -- adi,disable-timing-generator: Only for ADV7533 and ADV7535. Disables the - internal timing generator. The chip will rely on the sync signals in the - DSI data lanes, rather than generate its own timings for HDMI output. -- clocks: from common clock binding: reference to the CEC clock. -- clock-names: from common clock binding: must be "cec". -- reg-names : Names of maps with programmable addresses. - It can contain any map needing a non-default address. - Possible maps names are : "main", "edid", "cec", "packet" - -Required nodes: - -The ADV7511 has two video ports. Their connections are modelled using the OF -graph bindings specified in Documentation/devicetree/bindings/graph.txt. - -- Video port 0 for the RGB, YUV or DSI input. In the case of ADV7533/5, the - remote endpoint phandle should be a reference to a valid mipi_dsi_host device - node. -- Video port 1 for the HDMI output -- Audio port 2 for the HDMI audio input - - -Example -------- - - adv7511w: hdmi@39 { - compatible = "adi,adv7511w"; - /* - * The EDID page will be accessible on address 0x66 on the I2C - * bus. All other maps continue to use their default addresses. - */ - reg = <0x39>, <0x66>; - reg-names = "main", "edid"; - interrupt-parent = <&gpio3>; - interrupts = <29 IRQ_TYPE_EDGE_FALLING>; - clocks = <&cec_clock>; - clock-names = "cec"; - - adi,input-depth = <8>; - adi,input-colorspace = "rgb"; - adi,input-clock = "1x"; - adi,input-style = <1>; - adi,input-justification = "evenly"; - - ports { - #address-cells = <1>; - #size-cells = <0>; - - port@0 { - reg = <0>; - adv7511w_in: endpoint { - remote-endpoint = <&dpi_out>; - }; - }; - - port@1 { - reg = <1>; - adv7511_out: endpoint { - remote-endpoint = <&hdmi_connector_in>; - }; - }; - - port@2 { - reg = <2>; - codec_endpoint: endpoint { - remote-endpoint = <&i2s0_cpu_endpoint>; - }; - }; - }; - }; diff --git a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.yaml b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.yaml new file mode 100644 index 000000000000..a306adba105f --- /dev/null +++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.yaml @@ -0,0 +1,233 @@ +# SPDX-License-Identifier: GPL-2.0-only +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/bridge/adi,adv7511.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Analog Devices ADV7511/11W/13 HDMI Encoders + +maintainers: + - Laurent Pinchart <laurent.pinchart@ideasonboard.com> + +description: | + The ADV7511, ADV7511W and ADV7513 are HDMI audio and video + transmitters compatible with HDMI 1.4 and DVI 1.0. They support color + space conversion, S/PDIF, CEC and HDCP. They support RGB input + interface. + +properties: + compatible: + enum: + - adi,adv7511 + - adi,adv7511w + - adi,adv7513 + + reg: + description: | + I2C slave addresses. + + The ADV7511/11W/13 internal registers are split into four pages + exposed through different I2C addresses, creating four register + maps. Each map has it own I2C address and acts as a standard slave + device on the I2C bus. The main address is mandatory, others are + optional and revert to defaults if not specified. + minItems: 1 + maxItems: 4 + + reg-names: + description: + Names of maps with programmable addresses. It can contain any map + needing a non-default address. + minItems: 1 + items: + - const: main + - const: edid + - const: cec + - const: packet + + clocks: + description: Reference to the CEC clock. + maxItems: 1 + + clock-names: + const: cec + + interrupts: + maxItems: 1 + + pd-gpios: + description: GPIO connected to the power down signal. + maxItems: 1 + + avdd-supply: + description: A 1.8V supply that powers up the AVDD pin. + + dvdd-supply: + description: A 1.8V supply that powers up the DVDD pin. + + pvdd-supply: + description: A 1.8V supply that powers up the PVDD pin. + + dvdd-3v-supply: + description: A 3.3V supply that powers up the DVDD_3V pin. + + bgvdd-supply: + description: A 1.8V supply that powers up the BGVDD pin. + + adi,input-depth: + description: Number of bits per color component at the input. + allOf: + - $ref: /schemas/types.yaml#/definitions/uint32 + - enum: [ 8, 10, 12 ] + + adi,input-colorspace: + description: Input color space. + allOf: + - $ref: /schemas/types.yaml#/definitions/string + - enum: [ rgb, yuv422, yuv444 ] + + adi,input-clock: + description: | + Input clock type. + "1x": one clock cycle per pixel + "2x": two clock cycles per pixel + "dd": one clock cycle per pixel, data driven on both edges + allOf: + - $ref: /schemas/types.yaml#/definitions/string + - enum: [ 1x, 2x, dd ] + + adi,clock-delay: + description: + Video data clock delay relative to the pixel clock, in ps + (-1200ps .. 1600 ps). + allOf: + - $ref: /schemas/types.yaml#/definitions/uint32 + - default: 0 + + adi,embedded-sync: + description: + The input uses synchronization signals embedded in the data + stream (similar to BT.656). Defaults to 0 (separate H/V + synchronization signals). + allOf: + - $ref: /schemas/types.yaml#/definitions/uint32 + - enum: [ 0, 1 ] + - default: 0 + + adi,input-style: + description: + Input components arrangement variant as listed in the input + format tables in the datasheet. + allOf: + - $ref: /schemas/types.yaml#/definitions/uint32 + - enum: [ 1, 2, 3 ] + + adi,input-justification: + description: Input bit justification. + allOf: + - $ref: /schemas/types.yaml#/definitions/string + - enum: [ left, evenly, right ] + + ports: + description: + The ADV7511(W)/13 has two video ports and one audio port. This node + models their connections as documented in + Documentation/devicetree/bindings/media/video-interfaces.txt + Documentation/devicetree/bindings/graph.txt + type: object + properties: + port@0: + description: Video port for the RGB, YUV or DSI input. + type: object + + port@1: + description: Video port for the HDMI output. + type: object + + port@2: + description: Audio port for the HDMI output. + type: object + +# adi,input-colorspace and adi,input-clock are required except in +# "rgb 1x" and "yuv444 1x" modes, in which case they must not be +# specified. +if: + not: + properties: + adi,input-colorspace: + contains: + enum: [ rgb, yuv444 ] + adi,input-clock: + contains: + const: 1x + +then: + required: + - adi,input-style + - adi,input-justification + +else: + properties: + adi,input-style: false + adi,input-justification: false + + +required: + - compatible + - reg + - ports + - adi,input-depth + - adi,input-colorspace + - adi,input-clock + +examples: + - | + #include <dt-bindings/interrupt-controller/irq.h> + + adv7511w: hdmi@39 { + compatible = "adi,adv7511w"; + /* + * The EDID page will be accessible on address 0x66 on the I2C + * bus. All other maps continue to use their default addresses. + */ + reg = <0x39>, <0x66>; + reg-names = "main", "edid"; + interrupt-parent = <&gpio3>; + interrupts = <29 IRQ_TYPE_EDGE_FALLING>; + clocks = <&cec_clock>; + clock-names = "cec"; + + adi,input-depth = <8>; + adi,input-colorspace = "yuv422"; + adi,input-clock = "1x"; + + adi,input-style = <3>; + adi,input-justification = "right"; + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + adv7511w_in: endpoint { + remote-endpoint = <&dpi_out>; + }; + }; + + port@1 { + reg = <1>; + adv7511_out: endpoint { + remote-endpoint = <&hdmi_connector_in>; + }; + }; + + port@2 { + reg = <2>; + codec_endpoint: endpoint { + remote-endpoint = <&i2s0_cpu_endpoint>; + }; + }; + }; + }; + +... diff --git a/Documentation/devicetree/bindings/display/bridge/adi,adv7533.yaml b/Documentation/devicetree/bindings/display/bridge/adi,adv7533.yaml new file mode 100644 index 000000000000..dfcc63dfc5c5 --- /dev/null +++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7533.yaml @@ -0,0 +1,166 @@ +# SPDX-License-Identifier: GPL-2.0-only +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/bridge/adi,adv7533.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Analog Devices ADV7533/35 HDMI Encoders + +maintainers: + - Laurent Pinchart <laurent.pinchart@ideasonboard.com> + +description: | + The ADV7533 and ADV7535 are HDMI audio and video transmitters + compatible with HDMI 1.4 and DVI 1.0. They support color space + conversion, S/PDIF, CEC and HDCP. They support DSI for input pixels. + +properties: + compatible: + enum: + - adi,adv7533 + - adi,adv7535 + + reg: + description: | + I2C slave addresses. + + The ADV7533/35 internal registers are split into four pages + exposed through different I2C addresses, creating four register + maps. Each map has it own I2C address and acts as a standard slave + device on the I2C bus. The main address is mandatory, others are + optional and revert to defaults if not specified. + minItems: 1 + maxItems: 4 + + reg-names: + description: + Names of maps with programmable addresses. It can contain any map + needing a non-default address. + minItems: 1 + items: + - const: main + - const: edid + - const: cec + - const: packet + + clocks: + description: Reference to the CEC clock. + maxItems: 1 + + clock-names: + const: cec + + interrupts: + maxItems: 1 + + pd-gpios: + description: GPIO connected to the power down signal. + maxItems: 1 + + avdd-supply: + description: A 1.8V supply that powers up the AVDD pin. + + dvdd-supply: + description: A 1.8V supply that powers up the DVDD pin. + + pvdd-supply: + description: A 1.8V supply that powers up the PVDD pin. + + a2vdd-supply: + description: A 1.8V supply that powers up the A2VDD pin. + + v3p3-supply: + description: A 3.3V supply that powers up the V3P3 pin. + + v1p2-supply: + description: + A supply that powers up the V1P2 pin. It can be either 1.2V + or 1.8V for ADV7533 but only 1.8V for ADV7535. + + adi,disable-timing-generator: + description: + Disables the interal timing generator. The chip will rely on the + sync signals in the DSI data lanes, rather than generate its own + timings for HDMI output. + type: boolean + + adi,dsi-lanes: + description: Number of DSI data lanes connected to the DSI host. + allOf: + - $ref: /schemas/types.yaml#/definitions/uint32 + - enum: [ 1, 2, 3, 4 ] + + ports: + description: + The ADV7533/35 has two video ports and one audio port. This node + models their connections as documented in + Documentation/devicetree/bindings/media/video-interfaces.txt + Documentation/devicetree/bindings/graph.txt + type: object + properties: + port@0: + description: + Video port for the RGB, YUV or DSI input. The remote endpoint + phandle should be a reference to a valid mipi_dsi_host_device. + type: object + + port@1: + description: Video port for the HDMI output. + type: object + + port@2: + description: Audio port for the HDMI output. + type: object + +required: + - compatible + - reg + - ports + - adi,dsi-lanes + +examples: + - | + #include <dt-bindings/interrupt-controller/irq.h> + + adv7533: hdmi@39 { + compatible = "adi,adv7533"; + /* + * The EDID page will be accessible on address 0x66 on the I2C + * bus. All other maps continue to use their default addresses. + */ + reg = <0x39>, <0x66>; + reg-names = "main", "edid"; + interrupt-parent = <&gpio3>; + interrupts = <29 IRQ_TYPE_EDGE_FALLING>; + clocks = <&cec_clock>; + clock-names = "cec"; + adi,dsi-lanes = <4>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + adv7511w_in: endpoint { + remote-endpoint = <&dpi_out>; + }; + }; + + port@1 { + reg = <1>; + adv7511_out: endpoint { + remote-endpoint = <&hdmi_connector_in>; + }; + }; + + port@2 { + reg = <2>; + codec_endpoint: endpoint { + remote-endpoint = <&i2s0_cpu_endpoint>; + }; + }; + }; + }; + +...
Convert the ADV7511/11w/13/33/35 DT bindings to json-schema. The original binding has been split into two files: adi,adv7511.yaml for ADV7511/11W/13 and adi,adv7533.yaml for ADV7533/35. Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com> --- .../bindings/display/bridge/adi,adv7511.txt | 143 ----------- .../bindings/display/bridge/adi,adv7511.yaml | 230 ++++++++++++++++++ .../bindings/display/bridge/adi,adv7533.yaml | 166 +++++++++++++ 3 files changed, 396 insertions(+), 143 deletions(-) delete mode 100644 Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt create mode 100644 Documentation/devicetree/bindings/display/bridge/adi,adv7511.yaml create mode 100644 Documentation/devicetree/bindings/display/bridge/adi,adv7533.yaml