Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 14 Sep 2020 11:53:53 -0600
From:   Rob Herring <robh@kernel.org>
To:     =?iso-8859-1?Q?K=E9vin_L'h=F4pital?= <kevin.lhopital@bootlin.com>
Cc:     linux-media@vger.kernel.org, mchehab@kernel.org,
        mark.rutland@arm.com, mripard@kernel.org, wens@csie.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org,
        paul.kocialkowski@bootlin.com, thomas.petazzoni@bootlin.com
Subject: Re: [PATCH v2 1/3] dt-bindings: media: i2c: Add documentation for
 ov8865
Message-ID: <20200914175353.GA4127535@bogus>
References: <20200828131134.12113-1-kevin.lhopital@bootlin.com>
 <20200828131134.12113-2-kevin.lhopital@bootlin.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20200828131134.12113-2-kevin.lhopital@bootlin.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, Aug 28, 2020 at 03:11:32PM +0200, K?vin L'h?pital wrote:
> Add a documentation for the sensor ov8865 from Omnivision.
> 
> Signed-off-by: K?vin L'h?pital <kevin.lhopital@bootlin.com>
> ---
>  .../devicetree/bindings/media/i2c/ov8865.yaml | 165 ++++++++++++++++++

Use the compatible string for the filename.

>  1 file changed, 165 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov8865.yaml
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/ov8865.yaml b/Documentation/devicetree/bindings/media/i2c/ov8865.yaml
> new file mode 100644
> index 000000000000..468ddfd48c7a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/ov8865.yaml
> @@ -0,0 +1,165 @@
> +# SPDX-License-Identifier: (GPL-2.0 0R BSD-2-Clause)
> +# Copyright (C) 2020 K?vin L'h?pital
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/i2c/ov8865.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Omnivision OV8865 Sensor Device Tree Bindings
> +
> +maintainers:
> +  - K?vin L'h?pital <kevin.lhopital@bootlin.com>
> +
> +description: |-
> +  The Omnivision OV8865 is a image sensor that supports up to 3264x2448. It
> +  provides a 10 bits output format and a MIPI CSI2 interface (up to 4-lane).
> +  This chip is programmable through I2C.
> +
> +properties:
> +  compatible:
> +    const: ovti,ov8865
> +
> +  reg:
> +    maxItems: 1
> +
> +  pinctrl-names:
> +    description:
> +      Default name for the ov8865 pinctrl.
> +
> +  pinctrl-0:
> +    description:
> +      Pinctrl for the ov8865.

You can drop pinctrl-*. They are automatically added.

> +
> +  clocks:
> +    maxItems: 1
> +
> +  clock-names:
> +    description:
> +      Input clock name for the sensor.
> +
> +    items:
> +      - const: xclk
> +
> +  avdd-supply:
> +    description:
> +      Definition of the regulator used as analog power supply.
> +
> +  dovdd-supply:
> +    description:
> +      Definition of the regulator used as interface power supply.
> +
> +  vdd2-supply:
> +    description:
> +      Definition of the regulator used as digital power supply
> +
> +  afvdd-supply:
> +    description:
> +      Definition of the regulator used as actuator power supply.
> +
> +  powerdown-gpios:
> +    description:
> +      reference to the GPIO connceted to the powerdown pin. This is an active
> +      low signal on the OV8865.
> +
> +  reset-gpios:
> +    description:
> +      Reference to the GPIO connected to the reset pin. This is an active low
> +      signal for the OV8865.
> +
> +  rotation:
> +    description:
> +      Position of the sensor, the valid values are 0 (sensor mounted upright)
> +      and 180 (sensor mounted upside down).

Constraints, not free form text:

enum: [ 0, 180 ]

> +
> +  port:
> +    type: object
> +    additionalProperties: false
> +    description:
> +      A node containing an output port node with an endpoint definition as
> +      documented in
> +      Documentation/devicetree/bindings/media/video-interfaces.txt
> +
> +    properties:
> +      endpoint:
> +        type: object
> +
> +        properties:
> +          data-lanes:
> +            description:
> +              The driver only supports four-lane operation.

Not relevant to the h/w description.

> +            items:
> +              - const: 1
> +              - const: 2
> +              - const: 3
> +              - const: 4
> +
> +          clock-lanes:
> +            items:
> +              const: 0

If fixed in the h/w, it doesn't need to be in DT.

> +
> +          bus-type:
> +            description:
> +              The driver only supports the MIPI CSI2
> +            items:
> +              const: 4

Both of the above comments apply here...

> +
> +        required:
> +          - data-lanes
> +          - clock-lanes
> +          - bus-type
> +
> +    required:
> +      - endpoint
> +
> +required:
> +  - compatible
> +  - reg
> +  - pinctrl-names
> +  - pinctrl-0
> +  - clocks
> +  - clock-names
> +  - avdd-supply
> +  - dovdd-supply
> +  - vdd2-supply
> +  - afvdd-supply
> +  - powerdown-gpios
> +  - reset-gpios
> +  - port
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/sun8i-a83t-ccu.h>
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    i2c2 {
> +         #address-cells = <1>;
> +         #size-cells = <0>;
> +          ov8865: camera@36 {
> +                 compatible = "ovti,ov8865";
> +                 reg = <0x36>;
> +                 pinctrl-names = "default";
> +                 pinctrl-0 = <&csi_mclk_pin>; 
> +                 clocks = <&ccu CLK_CSI_MCLK>;
> +                 clock-names ="xclk";
> +                 avdd-supply = <&reg_ov8865_avdd>;
> +                 dovdd-supply = <&reg_ov8865_dovdd>;
> +                 vdd2-supply = <&reg_ov8865_vdd2>;
> +                 afvdd-supply = <&reg_ov8865_afvdd>;
> +                 powerdown-gpios = <&pio 4 17 GPIO_ACTIVE_LOW>; /* PE17 */
> +                 reset-gpios = <&pio 4 16 GPIO_ACTIVE_LOW>; /* PE16 */
> +                 rotation = <180>;
> +
> +                 port {
> +                      ov8865_to_mipi_csi2: endpoint {
> +                                 remote-endpoint = <&mipi_csi2_from_ov8865>;
> +                                 data-lanes = <1 2 3 4>;
> +                                 clock-lanes = <0>;
> +                                 bus-type = <4>; /* V4L2_FWNODE_BUS_TYPE_CSI2_DPHY */
> +                      };
> +                 };
> +           };
> +    };
> +
> +...
> -- 
> 2.17.1
>