Re: [PATCH v2 1/3] media: dt-bindings: ov5675: document YAML binding

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Krzysztof,

On 5/4/22 16:46, Krzysztof Kozlowski wrote:
On 04/05/2022 15:55, Quentin Schulz wrote:
From: Quentin Schulz <quentin.schulz@xxxxxxxxxxxxxxxxxxxxx>

This patch adds documentation of device tree in YAML schema for the
OV5675 CMOS image sensor from Omnivision.

Cc: Quentin Schulz <foss+kernel@xxxxxxxxx>

Don't Cc yourself in commits. This goes to the Git history, so
assumption is that the "other you" knows that you sent it. :)


It's a habit I've taken so that in the event I switch jobs, there's still an address where I can be reached.

But I forgot the kernel as a .mailmap :) so I can avoid doing it.

Signed-off-by: Quentin Schulz <quentin.schulz@xxxxxxxxxxxxxxxxxxxxx>
---

v2:
  - fixed incorrect id,
  - fixed device tree example by adding missing dt-bindings headers,
  - fixed device tree example by using vcc_1v2 for dvdd supply, as requested
  in datasheet,

  .../bindings/media/i2c/ovti,ov5675.yaml       | 139 ++++++++++++++++++
  MAINTAINERS                                   |   1 +
  2 files changed, 140 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov5675.yaml

diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov5675.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov5675.yaml
new file mode 100644
index 000000000000..29df2f82c631
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov5675.yaml
@@ -0,0 +1,139 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (c) 2022 Theobroma Systems Design und Consulting GmbH
+%YAML 1.2
+---
+$id: https://urldefense.proofpoint.com/v2/url?u=http-3A__devicetree.org_schemas_media_i2c_ovti-2Cov5675.yaml-23&d=DwICaQ&c=_sEr5x9kUWhuk4_nFwjJtA&r=LYjLexDn7rXIzVmkNPvw5ymA1XTSqHGq8yBP6m6qZZ4njZguQhZhkI_-172IIy1t&m=5gh6rUPawGp8Fw7sPXVsvOViSUSnEVGWWB9RK_CSQCeBjoEfunHaI0w2GiB0zjq_&s=W_iQiFCa2XM4k350eNawjzJR2ZTUuSx-VM4E1iuknz4&e=
+$schema: https://urldefense.proofpoint.com/v2/url?u=http-3A__devicetree.org_meta-2Dschemas_core.yaml-23&d=DwICaQ&c=_sEr5x9kUWhuk4_nFwjJtA&r=LYjLexDn7rXIzVmkNPvw5ymA1XTSqHGq8yBP6m6qZZ4njZguQhZhkI_-172IIy1t&m=5gh6rUPawGp8Fw7sPXVsvOViSUSnEVGWWB9RK_CSQCeBjoEfunHaI0w2GiB0zjq_&s=nrri3o0sr-6AopqJCaHZ94dUfOwS8r1V7ybfSwD7v2M&e=
+
+title: Omnivision OV5675 CMOS Sensor Device Tree Bindings

s/Device Tree Bindings//

+
+maintainers:
+  - Quentin Schulz <quentin.schulz@xxxxxxxxxxxxxxxxxxxxx>
+
+description: |-
+  The Omnivision OV5675 is a high performance, 1/5-inch, 5 megapixel, CMOS
+  image sensor that delivers 2592x1944 at 30fps. It provides full-frame,
+  sub-sampled, and windowed 10-bit MIPI images in various formats via the
+  Serial Camera Control Bus (SCCB) interface. This chip is programmable
+  through I2C and two-wire SCCB. The sensor output is available via CSI-2
+  serial data output (up to 2-lane).
+
+properties:
+  compatible:
+    const: ovti,ov5675
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    description:
+      Input clock for the sensor.
+    items:
+      - const: xvclk

Just "xv" is preferred.


The name of the clock in the datasheet is XVCLK though. Wouldn't it be confusing to describe HW by using names different from the datasheet?

+
+  clock-frequency:
+    description:
+      Frequency of the xvclk clock in Hertz.
+
+  dovdd-supply:
+    description:
+      Definition of the regulator used as interface power supply.
+
+  avdd-supply:
+    description:
+      Definition of the regulator used as analog power supply.
+
+  dvdd-supply:
+    description:
+      Definition of the regulator used as digital power supply.
+
+  reset-gpios:
+    description:
+      The phandle and specifier for the GPIO that controls sensor reset.
+      This corresponds to the hardware pin XSHUTDOWN which is physically
+      active low.

Needs maxItems

+
+  port:
+    type: object

Open other bindings and compare how it is done there. This looks like
/schemas/graph.yaml#/$defs/port-base


Did that but used an old kernel as base :/

+    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

Missing ref

+
+        properties:
+          data-lanes:
+            description: |-

No need for "|-"

+              The driver only supports 2-lane operation.

Please remove references to driver. It's not part of hardware.

+            items:
+              - const: 1
+              - const: 2
+
+          link-frequencies:
+            $ref: /schemas/types.yaml#/definitions/uint64-array

The ref should be already provided by video-interfaces.

+            description:
+              Allowed data bus frequencies. 450000000Hz is supported by the driver.

Again, skip driver reference. However you need to describe the number of
items.


Currently, the driver is limited to 450 MHz link-freq and 2 data lanes, while the HW advertises: "The OV5675 supports a MIPI interface of up to 2-lanes. The MIPI interface can be configured for 1/2-lane and each lane

is capable of a data transfer rate of up to 900 Mbps."

Was wondering what I am supposed to do in this situation as I see Documentation/devicetree/bindings/media/i2c/ov8856.yaml mentioning driver limitations in the dt-bindings.

Cheers,
Quentin



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux