| Message ID | 20230913192811.1946423-1-festevam@gmail.com |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | [v4] media: dt-bindings: Add OV5642 | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On Wed, Sep 13, 2023 at 04:28:11PM -0300, Fabio Estevam wrote: > From: Fabio Estevam <festevam@denx.de> > > 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. Many of these Omnivision sensors have the same supplies and same GPIO connections, so why can't they share the binding? There are some differences I've seen that boil down to just what the driver decided to require or not, but that shouldn't really vary. See commit 44ade291b77c ("media: dt-bindings: Merge OV5695 into OV5693 binding") for example. In any case, the binding looks fine and any merging can be done after. Reviewed-by: Rob Herring <robh@kernel.org> > > Signed-off-by: Fabio Estevam <festevam@denx.de> > --- > Changes since v3: > - Make propeties conditional on parallel verus MIPI CSI-2. (Sakari) > - Remove clock-names as there is a single clock. > - Pass bus-type. > > .../bindings/media/i2c/ovti,ov5642.yaml | 141 ++++++++++++++++++ > .../devicetree/bindings/trivial-devices.yaml | 2 - > 2 files changed, 141 insertions(+), 2 deletions(-) > create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov5642.yaml
Hi Rob, On Thu, Sep 14, 2023 at 09:59:35AM -0500, Rob Herring wrote: > On Wed, Sep 13, 2023 at 04:28:11PM -0300, Fabio Estevam wrote: > > From: Fabio Estevam <festevam@denx.de> > > > > 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. > > Many of these Omnivision sensors have the same supplies and same GPIO > connections, so why can't they share the binding? There are some > differences I've seen that boil down to just what the driver decided to > require or not, but that shouldn't really vary. See commit 44ade291b77c > ("media: dt-bindings: Merge OV5695 into OV5693 binding") for example. > > In any case, the binding looks fine and any merging can be done after. The sensors differ from interface capabilities (one or more of parallel, CSI-2 D-PHY and CSI-2 C-PHY), number of lanes / data lines etc. For historical reasons, many have different defaults for vsync and hsync polarities, for instance. I suppose some of these would reasonably be taken into account, if not in a single file, in a couple of files. CSI-2 connected sensors are the most common these days. The device for which this patch added the bindings appears to be 15 years old.
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..01f8b2b3fd17 --- /dev/null +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov5642.yaml @@ -0,0 +1,141 @@ +# 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 + + 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. + + port: + $ref: /schemas/graph.yaml#/$defs/port-base + description: | + Video output port. + + properties: + endpoint: + $ref: /schemas/media/video-interfaces.yaml# + unevaluatedProperties: false + + properties: + bus-type: + enum: [5, 6] + + bus-width: + enum: [8, 10] + default: 10 + + data-shift: + enum: [0, 2] + default: 0 + + hsync-active: + enum: [0, 1] + default: 1 + + vsync-active: + enum: [0, 1] + default: 1 + + pclk-sample: + enum: [0, 1] + default: 1 + + allOf: + - if: + properties: + bus-type: + const: 6 + then: + properties: + hsync-active: false + vsync-active: false + + - if: + properties: + bus-width: + const: 10 + then: + properties: + data-shift: + const: 0 + + required: + - bus-type + + additionalProperties: false + +required: + - compatible + - reg + - clocks + - port + +additionalProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + #include <dt-bindings/media/video-interfaces.h> + + i2c { + #address-cells = <1>; + #size-cells = <0>; + + camera@3c { + compatible = "ovti,ov5642"; + reg = <0x3c>; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_ov5642>; + clocks = <&clk_ext_camera>; + 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 { + ov5642_to_parallel: endpoint { + bus-type = <MEDIA_BUS_TYPE_PARALLEL>; + remote-endpoint = <¶llel_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 cd58179ae337..c665018f55b9 100644 --- a/Documentation/devicetree/bindings/trivial-devices.yaml +++ b/Documentation/devicetree/bindings/trivial-devices.yaml @@ -311,8 +311,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