[v3] dt-bindings: media: Add OV5642

Message ID	20230802160326.293420-1-festevam@denx.de (mailing list archive)
State	New, archived
Headers	show Return-Path: <linux-media-owner@vger.kernel.org> sender: festevam@denx.de) by phobos.denx.de (Postfix) with ESMTPSA id B40328686A; Wed, 2 Aug 2023 18:03:39 +0200 (CEST) From: Fabio Estevam <festevam@denx.de> To: robh+dt@kernel.org Cc: krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, mchehab@kernel.org, linux-media@vger.kernel.org, Fabio Estevam <festevam@denx.de>, Conor Dooley <conor.dooley@microchip.com> Subject: [PATCH v3] dt-bindings: media: Add OV5642 Date: Wed, 2 Aug 2023 13:03:26 -0300 Message-Id: <20230802160326.293420-1-festevam@denx.de> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: bulk
Series	[v3] dt-bindings: media: Add OV5642 \| expand [v3] dt-bindings: media: Add OV5642

Fabio Estevam Aug. 2, 2023, 4:03 p.m. UTC

As explained in the description text from trivial-devices.yaml:

"This is a list of trivial I2C and SPI devices that have simple device tree
bindings, consisting only of a compatible field, an address and possibly an
interrupt line."

A camera device does not fall into this category as it needs other
properties such as regulators, reset and powerdown GPIOs, clocks,
media endpoint.

Remove the OV5642 entry from trivial-devices.yaml and add its own
ovti,ov5642.yaml.

Signed-off-by: Fabio Estevam <festevam@denx.de>
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
---
Changes since v2:
- Made ovti,ov5642.yaml dual-licensed (Conor)
- Fixed whitespace warning (Conor)
- Squased both patches (Conor)
- Added Conor's Reviewed-by tag.
- Added linux-media on Cc.

 .../bindings/media/i2c/ovti,ov5642.yaml       | 118 ++++++++++++++++++
 .../devicetree/bindings/trivial-devices.yaml  |   2 -
 2 files changed, 118 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov5642.yaml

Sakari Ailus Sept. 13, 2023, 3:40 p.m. UTC | #1

Hi Fabio,

On Wed, Aug 02, 2023 at 01:03:26PM -0300, Fabio Estevam wrote:
> As explained in the description text from trivial-devices.yaml:
> 
> "This is a list of trivial I2C and SPI devices that have simple device tree
> bindings, consisting only of a compatible field, an address and possibly an
> interrupt line."
> 
> A camera device does not fall into this category as it needs other
> properties such as regulators, reset and powerdown GPIOs, clocks,
> media endpoint.
> 
> Remove the OV5642 entry from trivial-devices.yaml and add its own
> ovti,ov5642.yaml.
> 
> Signed-off-by: Fabio Estevam <festevam@denx.de>
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> ---
> Changes since v2:
> - Made ovti,ov5642.yaml dual-licensed (Conor)
> - Fixed whitespace warning (Conor)
> - Squased both patches (Conor)
> - Added Conor's Reviewed-by tag.
> - Added linux-media on Cc.
> 
>  .../bindings/media/i2c/ovti,ov5642.yaml       | 118 ++++++++++++++++++
>  .../devicetree/bindings/trivial-devices.yaml  |   2 -
>  2 files changed, 118 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov5642.yaml
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov5642.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov5642.yaml
> new file mode 100644
> index 000000000000..b48f1bc6aca4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov5642.yaml
> @@ -0,0 +1,118 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/i2c/ovti,ov5642.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: OmniVision OV5642 Image Sensor
> +
> +maintainers:
> +  - Fabio Estevam <festevam@gmail.com>
> +
> +allOf:
> +  - $ref: /schemas/media/video-interface-devices.yaml#
> +
> +properties:
> +  compatible:
> +    const: ovti,ov5642
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    description: XCLK Input Clock
> +
> +  clock-names:
> +    const: xclk
> +
> +  AVDD-supply:
> +    description: Analog voltage supply, 2.8V.
> +
> +  DVDD-supply:
> +    description: Digital core voltage supply, 1.5V.
> +
> +  DOVDD-supply:
> +    description: Digital I/O voltage supply, 1.8V.
> +
> +  powerdown-gpios:
> +    maxItems: 1
> +    description: Reference to the GPIO connected to the powerdown pin, if any.
> +
> +  reset-gpios:
> +    maxItems: 1
> +    description: Reference to the GPIO connected to the reset pin, if any.
> +
> +  rotation:
> +    enum:
> +      - 0
> +      - 180
> +
> +  port:
> +    description: Digital Output Port
> +    $ref: /schemas/graph.yaml#/$defs/port-base
> +    additionalProperties: false
> +
> +    properties:
> +      endpoint:
> +        $ref: /schemas/media/video-interfaces.yaml#
> +        unevaluatedProperties: false
> +
> +        properties:
> +          clock-lanes:
> +            const: 0
> +
> +          data-lanes:
> +            minItems: 1
> +            maxItems: 2
> +            items:
> +              enum: [1, 2]
> +
> +          bus-width:
> +            enum: [8, 10]
> +
> +          data-shift:
> +            enum: [0, 2]

You seem to have properties here that are specific to both parallel and
CSI-2 buses. Which one does the sensor use?

> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - port
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +      #include <dt-bindings/gpio/gpio.h>
> +
> +      i2c {
> +          #address-cells = <1>;
> +          #size-cells = <0>;
> +
> +          camera@3c {
> +              compatible = "ovti,ov5642";
> +              pinctrl-names = "default";
> +              pinctrl-0 = <&pinctrl_ov5642>;
> +              reg = <0x3c>;
> +              clocks = <&clk_ext_camera>;
> +              clock-names = "xclk";
> +              DOVDD-supply = <&vgen4_reg>;
> +              AVDD-supply = <&vgen3_reg>;
> +              DVDD-supply = <&vgen2_reg>;
> +              powerdown-gpios = <&gpio1 19 GPIO_ACTIVE_HIGH>;
> +              reset-gpios = <&gpio1 20 GPIO_ACTIVE_LOW>;
> +
> +              port {
> +                  /* Parallel bus endpoint */
> +                  ov5642_to_parallel: endpoint {
> +                      remote-endpoint = <&parallel_from_ov5642>;
> +                      bus-width = <8>;
> +                      data-shift = <2>; /* lines 9:2 are used */
> +                      hsync-active = <0>;
> +                      vsync-active = <0>;
> +                      pclk-sample = <1>;
> +                  };
> +              };
> +          };
> +      };
> diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml
> index 40bc475ee7e1..ab1423a4aa7f 100644
> --- a/Documentation/devicetree/bindings/trivial-devices.yaml
> +++ b/Documentation/devicetree/bindings/trivial-devices.yaml
> @@ -313,8 +313,6 @@ properties:
>            - nuvoton,w83773g
>              # OKI ML86V7667 video decoder
>            - oki,ml86v7667
> -            # OV5642: Color CMOS QSXGA (5-megapixel) Image Sensor with OmniBSI and Embedded TrueFocus
> -          - ovti,ov5642
>              # 48-Lane, 12-Port PCI Express Gen 2 (5.0 GT/s) Switch
>            - plx,pex8648
>              # Pulsedlight LIDAR range-finding sensor

Sakari Ailus Sept. 13, 2023, 3:56 p.m. UTC | #2

Hi Fabio,

On Wed, Aug 02, 2023 at 01:03:26PM -0300, Fabio Estevam wrote:
> As explained in the description text from trivial-devices.yaml:
> 
> "This is a list of trivial I2C and SPI devices that have simple device tree
> bindings, consisting only of a compatible field, an address and possibly an
> interrupt line."
> 
> A camera device does not fall into this category as it needs other
> properties such as regulators, reset and powerdown GPIOs, clocks,
> media endpoint.
> 
> Remove the OV5642 entry from trivial-devices.yaml and add its own
> ovti,ov5642.yaml.

One more little thing. Do you have a driver for this device? In upstream
there doesn't seem to be any.

Fabio Estevam Sept. 13, 2023, 4:02 p.m. UTC | #3

Hi Sakari,

On 13/09/2023 12:40, Sakari Ailus wrote:

> You seem to have properties here that are specific to both parallel and
> CSI-2 buses. Which one does the sensor use?

The OV5642 sensor can support both parallel and MIPI CSI-2 buses.

Fabio Estevam Sept. 13, 2023, 4:04 p.m. UTC | #4

Hi Sakari,

On 13/09/2023 12:56, Sakari Ailus wrote:

> One more little thing. Do you have a driver for this device? In 
> upstream
> there doesn't seem to be any.

Correct. There is no driver for OV5642 upstream.

The DT folks asked me to document the OV5642 binding even without an 
existing driver.

Sakari Ailus Sept. 13, 2023, 4:10 p.m. UTC | #5

Hi Fabio,

On Wed, Sep 13, 2023 at 01:02:25PM -0300, Fabio Estevam wrote:
> Hi Sakari,
> 
> On 13/09/2023 12:40, Sakari Ailus wrote:
> 
> > You seem to have properties here that are specific to both parallel and
> > CSI-2 buses. Which one does the sensor use?
> 
> The OV5642 sensor can support both parallel and MIPI CSI-2 buses.

Then these properties should be conditional to that. Please see an example
in Documentation/devicetree/bindings/media/i2c/ovti,ov772x.yaml .

You can omit clock-lanes in any case as there's just a single one (0).

Conor Dooley Sept. 13, 2023, 6:26 p.m. UTC | #6

On Wed, Sep 13, 2023 at 01:04:06PM -0300, Fabio Estevam wrote:
> Hi Sakari,
> 
> On 13/09/2023 12:56, Sakari Ailus wrote:
> 
> > One more little thing. Do you have a driver for this device? In upstream
> > there doesn't seem to be any.
> 
> Correct. There is no driver for OV5642 upstream.
> 
> The DT folks asked me to document the OV5642 binding even without an
> existing driver.

IIRC, Fabio wanted to delete it from trivial-devices, and Krzysztof and
I both felt it was more suitable to document it properly rather than
delete it.

Sakari Ailus Sept. 14, 2023, 10:18 a.m. UTC | #7

On Wed, Sep 13, 2023 at 07:26:48PM +0100, Conor Dooley wrote:
> On Wed, Sep 13, 2023 at 01:04:06PM -0300, Fabio Estevam wrote:
> > Hi Sakari,
> > 
> > On 13/09/2023 12:56, Sakari Ailus wrote:
> > 
> > > One more little thing. Do you have a driver for this device? In upstream
> > > there doesn't seem to be any.
> > 
> > Correct. There is no driver for OV5642 upstream.
> > 
> > The DT folks asked me to document the OV5642 binding even without an
> > existing driver.
> 
> IIRC, Fabio wanted to delete it from trivial-devices, and Krzysztof and
> I both felt it was more suitable to document it properly rather than
> delete it.

Ack, works for me.

[v3] dt-bindings: media: Add OV5642

Commit Message

Comments

Patch