Hi Rob,
Thanks for your feedback.
On 2020-07-13 09:44:49 -0600, Rob Herring wrote:
> On Mon, Jul 13, 2020 at 01:18:43PM +0200, Niklas Söderlund wrote:
> > Convert ADV7604 video decoder documentation to json-schema.
> >
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>
> > ---
> > .../devicetree/bindings/media/i2c/adv7604.txt | 87 ---------
> > .../bindings/media/i2c/adv7604.yaml | 171 ++++++++++++++++++
> > 2 files changed, 171 insertions(+), 87 deletions(-)
> > delete mode 100644 Documentation/devicetree/bindings/media/i2c/adv7604.txt
> > create mode 100644 Documentation/devicetree/bindings/media/i2c/adv7604.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/media/i2c/adv7604.txt b/Documentation/devicetree/bindings/media/i2c/adv7604.txt
> > deleted file mode 100644
> > index 8c7cdc7cd7aaebc8..0000000000000000
> > --- a/Documentation/devicetree/bindings/media/i2c/adv7604.txt
> > +++ /dev/null
> > @@ -1,87 +0,0 @@
> > -* Analog Devices ADV7604/11/12 video decoder with HDMI receiver
> > -
> > -The ADV7604 and ADV7611/12 are multiformat video decoders with an integrated
> > -HDMI receiver. The ADV7604 has four multiplexed HDMI inputs and one analog
> > -input, and the ADV7611 has one HDMI input and no analog input. The 7612 is
> > -similar to the 7611 but has 2 HDMI inputs.
> > -
> > -These device tree bindings support the ADV7611/12 only at the moment.
> > -
> > -Required Properties:
> > -
> > - - compatible: Must contain one of the following
> > - - "adi,adv7611" for the ADV7611
> > - - "adi,adv7612" for the ADV7612
> > -
> > - - reg: I2C slave addresses
> > - The ADV76xx has up to thirteen 256-byte maps that can be accessed via the
> > - main I2C ports. 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 device node must contain one 'port' child node per device input and output
> > -port, in accordance with the video interface bindings defined in
> > -Documentation/devicetree/bindings/media/video-interfaces.txt. The port nodes
> > -are numbered as follows.
> > -
> > - Port ADV7611 ADV7612
> > -------------------------------------------------------------
> > - HDMI 0 0, 1
> > - Digital output 1 2
> > -
> > -The digital output port node must contain at least one endpoint.
> > -
> > -Optional Properties:
> > -
> > - - hpd-gpios: References to the GPIOs that control the HDMI hot-plug
> > - detection pins, one per HDMI input. The active flag indicates the GPIO
> > - level that enables hot-plug detection.
> > - - reset-gpios: Reference to the GPIO connected to the device's reset pin.
> > - - default-input: Select which input is selected after reset.
> > - - reg-names : Names of maps with programmable addresses.
> > - It can contain any map needing a non-default address.
> > - Possible maps names are :
> > - "main", "avlink", "cec", "infoframe", "esdp", "dpp", "afe",
> > - "rep", "edid", "hdmi", "test", "cp", "vdp"
> > -
> > -Optional Endpoint Properties:
> > -
> > - The following three properties are defined in video-interfaces.txt and are
> > - valid for source endpoints only.
> > -
> > - - hsync-active: Horizontal synchronization polarity. Defaults to active low.
> > - - vsync-active: Vertical synchronization polarity. Defaults to active low.
> > - - pclk-sample: Pixel clock polarity. Defaults to output on the falling edge.
> > -
> > - If none of hsync-active, vsync-active and pclk-sample is specified the
> > - endpoint will use embedded BT.656 synchronization.
> > -
> > -Example:
> > -
> > - hdmi_receiver@4c {
> > - compatible = "adi,adv7611";
> > - /*
> > - * The edid page will be accessible @ 0x66 on the I2C bus. All
> > - * other maps will retain their default addresses.
> > - */
> > - reg = <0x4c>, <0x66>;
> > - reg-names = "main", "edid";
> > -
> > - reset-gpios = <&ioexp 0 GPIO_ACTIVE_LOW>;
> > - hpd-gpios = <&ioexp 2 GPIO_ACTIVE_HIGH>;
> > -
> > - #address-cells = <1>;
> > - #size-cells = <0>;
> > -
> > - default-input = <0>;
> > -
> > - port@0 {
> > - reg = <0>;
> > - };
> > - port@1 {
> > - reg = <1>;
> > - hdmi_in: endpoint {
> > - remote-endpoint = <&ccdc_in>;
> > - };
> > - };
> > - };
> > diff --git a/Documentation/devicetree/bindings/media/i2c/adv7604.yaml b/Documentation/devicetree/bindings/media/i2c/adv7604.yaml
> > new file mode 100644
> > index 0000000000000000..d354fd8e0cc88d93
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/i2c/adv7604.yaml
> > @@ -0,0 +1,171 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/media/i2c/adv7604.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Analog Devices ADV7604/11/12 video decoder with HDMI receiver
> > +
> > +maintainers:
> > + - Hans Verkuil <hverkuil-cisco@xxxxxxxxx>
> > +
> > +description:
> > + The ADV7604 and ADV7611/12 are multiformat video decoders with an integrated
> > + HDMI receiver. The ADV7604 has four multiplexed HDMI inputs and one analog
> > + input, and the ADV7611 has one HDMI input and no analog input. The 7612 is
> > + similar to the 7611 but has 2 HDMI inputs.
> > +
> > + These device tree bindings support the ADV7611/12 only at the moment.
> > +
> > +properties:
> > + compatible:
> > + items:
> > + - enum:
> > + - adi,adv7611
> > + - adi,adv7612
> > +
> > + reg:
> > + minItems: 1
> > + maxItems: 13
> > +
> > + reg-names:
> > + items:
> > + - enum: [ main, avlink, cec, infoframe, esdp, dpp, afe, rep, edid, hdmi, test, cp, vdp ]
> > + - enum: [ main, avlink, cec, infoframe, esdp, dpp, afe, rep, edid, hdmi, test, cp, vdp ]
> > + - enum: [ main, avlink, cec, infoframe, esdp, dpp, afe, rep, edid, hdmi, test, cp, vdp ]
> > + - enum: [ main, avlink, cec, infoframe, esdp, dpp, afe, rep, edid, hdmi, test, cp, vdp ]
> > + - enum: [ main, avlink, cec, infoframe, esdp, dpp, afe, rep, edid, hdmi, test, cp, vdp ]
> > + - enum: [ main, avlink, cec, infoframe, esdp, dpp, afe, rep, edid, hdmi, test, cp, vdp ]
> > + - enum: [ main, avlink, cec, infoframe, esdp, dpp, afe, rep, edid, hdmi, test, cp, vdp ]
> > + - enum: [ main, avlink, cec, infoframe, esdp, dpp, afe, rep, edid, hdmi, test, cp, vdp ]
> > + - enum: [ main, avlink, cec, infoframe, esdp, dpp, afe, rep, edid, hdmi, test, cp, vdp ]
> > + - enum: [ main, avlink, cec, infoframe, esdp, dpp, afe, rep, edid, hdmi, test, cp, vdp ]
> > + - enum: [ main, avlink, cec, infoframe, esdp, dpp, afe, rep, edid, hdmi, test, cp, vdp ]
> > + - enum: [ main, avlink, cec, infoframe, esdp, dpp, afe, rep, edid, hdmi, test, cp, vdp ]
> > + - enum: [ main, avlink, cec, infoframe, esdp, dpp, afe, rep, edid, hdmi, test, cp, vdp ]
>
> This can be simplified to:
>
> items:
> enum: [ main, avlink, cec, infoframe, esdp, dpp, afe, rep, edid, hdmi, test, cp, vdp ]
I tried this at first,
properties:
reg-names:
items:
enum: [ main, avlink, cec, infoframe, esdp, dpp, afe, rep, edid, hdmi, test, cp, vdp ]
But running the dt_binding_check on it fails with,
$ make dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/media/i2c/adv7604.yaml
CHKDT Documentation/devicetree/bindings/media/i2c/adv7604.yaml
/mnt/build/linux/Documentation/devicetree/bindings/media/i2c/adv7604.yaml: properties:reg-names:items: {'enum': ['main', 'avlink', 'cec', 'infoframe', 'esdp', 'dpp', 'afe', 'rep', 'edid', 'hdmi', 'test', 'cp', 'vdp']} is not of type 'array'
make[1]: *** [Documentation/devicetree/bindings/Makefile:20: Documentation/devicetree/bindings/media/i2c/adv7604.example.dts] Error 1
make: *** [Makefile:1334: dt_binding_check] Error 2
What am I missing? I'm using latest master of dt-schema [1] if it makes
any difference.
1. 1330cddcba757bee ("schemas: Make additionalProperties/unevaluatedProperties explicit")
>
> (Make items a schema applying to all items, rather than a list)
>
> Though we may want to enforce 'main' is first with:
>
> minItems: 1
> maxItems: 13
> items:
> - const: main
>
> These 2 can be combined under an 'allOf'.
I also had this idea by dropping main from the list of enums for all but
the first one. But I can't find any argument for why main needs to go
first, but I agree it feels like the right thing.
>
> > +
> > + reset-gpios:
> > + maxItems: 1
> > +
> > + hpd-gpios:
> > + minItems: 1
> > + description:
> > + References to the GPIOs that control the HDMI hot-plug detection pins,
> > + one per HDMI input. The active flag indicates the GPIO level that
> > + enables hot-plug detection.
> > +
> > + default-input:
> > + maxItems: 1
> > + description:
> > + Select which input is selected after reset.
> > +
> > + ports:
> > + type: object
> > + description:
> > + A node containing input and output port nodes with endpoint definitions
> > + as documented in
> > + Documentation/devicetree/bindings/media/video-interfaces.txt
> > +
> > +required:
> > + - compatible
> > + - reg
> > +
> > +allOf:
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + const: adi,adv7611
> > + then:
> > + properties:
> > + ports:
> > + properties:
> > + '#address-cells':
> > + const: 1
> > + '#size-cells':
> > + const: 0
> > + port@0:
> > + type: object
> > + description: Input port
> > + port@1:
> > + type: object
> > + description: Output port
> > +
> > + required:
> > + - port@1
> > +
> > + additionalProperties: false
> > +
> > + required:
> > + - ports
> > +
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + const: adi,adv7612
> > + then:
> > + properties:
> > + ports:
> > + properties:
> > + '#address-cells':
> > + const: 1
> > + '#size-cells':
> > + const: 0
> > + port@2:
> > + type: object
> > + description: Output port
> > +
> > + patternProperties:
> > + "^port@[0-1]$":
> > + type: object
> > + description: Input port
> > +
> > + required:
> > + - port@2
> > +
> > + additionalProperties: false
> > +
> > + required:
> > + - ports
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/gpio/gpio.h>
> > +
> > + i2c {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + hdmi_receiver@4c {
> > + compatible = "adi,adv7611";
> > + /*
> > + * The edid page will be accessible @ 0x66 on the I2C bus. All
> > + * other maps will retain their default addresses.
> > + */
> > + reg = <0x4c>, <0x66>;
> > + reg-names = "main", "edid";
> > +
> > + reset-gpios = <&ioexp 0 GPIO_ACTIVE_LOW>;
> > + hpd-gpios = <&ioexp 2 GPIO_ACTIVE_HIGH>;
> > + default-input = <0>;
> > +
> > + ports {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + port@0 {
> > + reg = <0>;
> > + };
> > +
> > + port@1 {
> > + reg = <1>;
> > + hdmi_in: endpoint {
> > + remote-endpoint = <&ccdc_in>;
> > + };
> > + };
> > + };
> > +
> > +
> > + };
> > + };
> > --
> > 2.27.0
> >
--
Regards,
Niklas Söderlund
