linux-kernel - Re: [PATCH v7 1/5] dt-bindings: media: Add Allwinner A10 CSI binding

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20190820114849.GD5123@paasikivi.fi.intel.com>
Date:   Tue, 20 Aug 2019 14:48:49 +0300
From:   Sakari Ailus <sakari.ailus@...ux.intel.com>
To:     Maxime Ripard <mripard@...nel.org>
Cc:     Hans Verkuil <hans.verkuil@...co.com>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
        Laurent Pinchart <laurent.pinchart@...asonboard.com>,
        linux-media@...r.kernel.org, Chen-Yu Tsai <wens@...e.org>,
        linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        devicetree@...r.kernel.org, Mark Rutland <mark.rutland@....com>,
        Rob Herring <robh+dt@...nel.org>,
        Frank Rowand <frowand.list@...il.com>,
        Maxime Ripard <maxime.ripard@...tlin.com>,
        Rob Herring <robh@...nel.org>
Subject: Re: [PATCH v7 1/5] dt-bindings: media: Add Allwinner A10 CSI binding

Hi Maxime,

On Tue, Aug 20, 2019 at 01:24:32PM +0200, Maxime Ripard wrote:
> From: Maxime Ripard <maxime.ripard@...tlin.com>
> 
> The Allwinner A10 CMOS Sensor Interface is a camera capture interface also
> used in later (A10s, A13, A20, R8 and GR8) SoCs.
> 
> On some SoCs, like the A10, there's multiple instances of that controller,
> with one instance supporting more channels and having an ISP.
> 
> Reviewed-by: Rob Herring <robh@...nel.org>
> Signed-off-by: Maxime Ripard <maxime.ripard@...tlin.com>
> ---
>  Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml | 107 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 107 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml
> 
> diff --git a/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml
> new file mode 100644
> index 000000000000..9000bca344f9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml
> @@ -0,0 +1,107 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/arm/allwinner,sun4i-a10-csi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Allwinner A10 CMOS Sensor Interface (CSI) Device Tree Bindings
> +
> +maintainers:
> +  - Chen-Yu Tsai <wens@...e.org>
> +  - Maxime Ripard <maxime.ripard@...tlin.com>
> +
> +description: |-
> +  The Allwinner A10 and later has a CMOS Sensor Interface to retrieve
> +  frames from a parallel or BT656 sensor.
> +
> +properties:
> +  compatible:
> +    const: allwinner,sun7i-a20-csi0
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: The CSI interface clock
> +      - description: The CSI module clock
> +      - description: The CSI ISP clock
> +      - description: The CSI DRAM clock
> +
> +  clock-names:
> +    items:
> +      - const: bus
> +      - const: mod
> +      - const: isp
> +      - const: ram
> +
> +  resets:
> +    maxItems: 1
> +
> +  port:
> +    type: object
> +    additionalProperties: false
> +
> +    properties:
> +      endpoint:
> +        properties:
> +          bus-width:
> +            const: 8
> +            description: Number of data lines actively used.

Are other values supported? If not, you could omit this.

> +
> +          data-active: true
> +          hsync-active: true
> +          pclk-sample: true
> +          remote-endpoint: true
> +          vsync-active: true
> +
> +        required:
> +          - bus-width
> +          - data-active
> +          - hsync-active
> +          - pclk-sample
> +          - remote-endpoint
> +          - vsync-active

Some of these are not allowed in the Bt.656 mode (vsync-active and
hsync-active) while they're required in Bt.601 mode. Is there a way to tell
that in YAML-based bindings?

Similarly, video-interfaces.txt should be referenced from here, shouldn't
it?

> +
> +    required:
> +      - endpoint
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/clock/sun7i-a20-ccu.h>
> +    #include <dt-bindings/reset/sun4i-a10-ccu.h>
> +
> +    csi0: csi@...9000 {
> +        compatible = "allwinner,sun7i-a20-csi0";
> +        reg = <0x01c09000 0x1000>;
> +        interrupts = <GIC_SPI 42 IRQ_TYPE_LEVEL_HIGH>;
> +        clocks = <&ccu CLK_AHB_CSI0>, <&ccu CLK_CSI0>,
> +                 <&ccu CLK_CSI_SCLK>, <&ccu CLK_DRAM_CSI0>;
> +        clock-names = "bus", "mod", "isp", "ram";
> +        resets = <&ccu RST_CSI0>;
> +
> +        port {
> +            csi_from_ov5640: endpoint {
> +                remote-endpoint = <&ov5640_to_csi>;
> +                bus-width = <8>;
> +                hsync-active = <1>; /* Active high */
> +                vsync-active = <0>; /* Active low */
> +                data-active = <1>;  /* Active high */
> +                pclk-sample = <1>;  /* Rising */
> +            };
> +        };
> +    };
> +
> +...

-- 
Kind regards,

Sakari Ailus
sakari.ailus@...ux.intel.com