Re: [PATCH v7 2/3] media: dt-bindings: media: i2c: Add bindings for TW9900

Rob Herring
2023-10-17T19:48:54Z

On Mon, Oct 16, 2023 at 03:58:32PM +0200, Mehdi Djait wrote:
> The Techwell TW9900 is a video decoder supporting multiple input
> standards, such as PAL and NTSC, and outputs a BT.656 video
> signal.
>
> It's designed to be low-power, posesses some features such as a
> programmable comb-filter, and automatic input standard detection
>
> Signed-off-by: Mehdi Djait <mehdi.djait@bootlin.com>
> ---
> V6->V7:
> - added powerdown-gpios and input ports
> - used 4 spaces for example identation
>
> V5->V6:
> - This commit had a "Reviewed-by: Rob Herring <robh@kernel.org>" Tag but
> decided not to collect it because the last Iteration was more than 2
> years ago
> - removed SECAM from the mentioned standards
> - changed maintainer
>
> V4->V5:
> - renamed the file to match the compatible string, and referenced
> the graph.yaml schema
>
> V3->V4:
> - add the missing reset-gpios node to the binding
>
> V2->V3:
> - fix the example not compiling due to a typo in the reset-gpios
> node.
>
> .../bindings/media/i2c/techwell,tw9900.yaml | 82 +++++++++++++++++++
> 1 file changed, 82 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/i2c/techwell,tw9900.yaml
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/techwell,tw9900.yaml b/Documentation/devicetree/bindings/media/i2c/techwell,tw9900.yaml
> new file mode 100644
> index 000000000000..244289a9a3e0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/techwell,tw9900.yaml
> @@ -0,0 +1,82 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/i2c/techwell,tw9900.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Techwell TW9900 NTSC/PAL video decoder
> +
> +maintainers:
> + - Mehdi Djait <mehdi.djait@bootlin.com>
> +
> +description:
> + The tw9900 is a multi-standard video decoder, supporting NTSC, PAL standards
> + with auto-detection features.
> +
> +properties:
> + compatible:
> + const: techwell,tw9900
> +
> + reg:
> + maxItems: 1
> +
> + vdd-supply:
> + description: VDD power supply
> +
> + reset-gpios:
> + description: GPIO descriptor for the RESET input pin
> + maxItems: 1
> +
> + powerdown-gpios:
> + description: GPIO descriptor for the POWERDOWN input pin
> + maxItems: 1
> +
> + ports:
> + $ref: /schemas/graph.yaml#/properties/ports
> + properties:
> + port@3:
> + $ref: /schemas/graph.yaml#/properties/port
> + description: Video port for the decoder output.
> +
> + patternProperties:
> + "^port@[0-2]$":
> + $ref: /schemas/graph.yaml#/properties/port
> + description: Analog video input port.

You need to define each port (0, 1, 2). Are those each component or
composite, s-video, and component? The latter would make more sense as I
don't think there would be a need to route each component to different
place. It's logically just 1 stream.

> +
> + required:
> + - port@3
> +
> +required:
> + - compatible
> + - ports
> + - reg
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/gpio/gpio.h>
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + video-decoder@44 {
> + compatible = "techwell,tw9900";
> + reg = <0x44>;
> +
> + vdd-supply = <&tw9900_supply>;
> + reset-gpios = <&gpio2 5 GPIO_ACTIVE_LOW>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@3 {
> + reg = <3>;
> + endpoint {
> + remote-endpoint = <&vip_in>;
> + };
> + };
> + };
> + };
> + };
> --
> 2.41.0
>