Re: [PATCH v2 1/2] media: dt-bindings: media: i2c: document OV4689 DT bindings

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

 



Hi Tommaso,

On 2022-09-13 at 16:05 +02, Tommaso Merciai <tommaso.merciai@xxxxxxxxxxxxxxxxxxxx> wrote:
> Hi Mikhail,
>
> On Sun, Sep 11, 2022 at 11:01:34PM +0300, Mikhail Rudenko wrote:
>> Add device-tree binding documentation for OV4689 image sensor driver,
>> and the relevant MAINTAINERS entries.
>>
>> Signed-off-by: Mikhail Rudenko <mike.rudenko@xxxxxxxxx>
>> ---
>>  .../bindings/media/i2c/ovti,ov4689.yaml       | 141 ++++++++++++++++++
>>  MAINTAINERS                                   |   7 +
>>  2 files changed, 148 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov4689.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov4689.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov4689.yaml
>> new file mode 100644
>> index 000000000000..376330b5572a
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov4689.yaml
>> @@ -0,0 +1,141 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/media/i2c/ovti,ov4689.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Omnivision OV4689 CMOS
>> +
>> +maintainers:
>> +  - Mikhail Rudenko <mike.rudenko@xxxxxxxxx>
>> +
>> +description: |
>> +  The Omnivision OV4689 is a high performance, 1/3-inch, 4 megapixel
>> +  image sensor. Ihis chip supports high frame rate speeds up to 90 fps
>> +  at 2688x1520 resolution. It is programmable through an I2C
>> +  interface, and sensor output is sent via 1/2/4 lane MIPI CSI-2
>> +  connection.
>> +
>> +allOf:
>> +  - $ref: /schemas/media/video-interface-devices.yaml#
>> +
>> +properties:
>> +  compatible:
>> +    const: ovti,ov4689
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  clocks:
>> +    description:
>> +      External clock (XVCLK) for the sensor, 6-64 MHz
>> +    maxItems: 1
>> +
>> +  clock-names: true
>> +
>> +  dovdd-supply:
>> +    description:
>> +      Digital I/O voltage supply, 1.7-3.0 V
>> +
>> +  avdd-supply:
>> +    description:
>> +      Analog voltage supply, 2.6-3.0 V
>> +
>> +  dvdd-supply:
>> +    description:
>> +      Digital core voltage supply, 1.1-1.3 V
>> +
>> +  powerdown-gpios:
>> +    maxItems: 1
>> +    description:
>> +      GPIO connected to the powerdown pin (active low)
>> +
>> +  reset-gpios:
>> +    maxItems: 1
>> +    description:
>> +      GPIO connected to the reset pin (active low)
>> +
>> +  orientation: true
>> +
>> +  rotation: true
>> +
>> +  port:
>> +    $ref: /schemas/graph.yaml#/$defs/port-base
>> +    additionalProperties: false
>> +    description:
>> +      Output port node, single endpoint describing the CSI-2 transmitter
>> +
>> +    properties:
>> +      endpoint:
>> +        $ref: /schemas/media/video-interfaces.yaml#
>> +        unevaluatedProperties: false
>> +
>> +        properties:
>> +          data-lanes:
>> +            oneOf:
>> +              - items:
>> +                  - const: 1
>> +                  - const: 2
>> +                  - const: 3
>> +                  - const: 4
>> +              - items:
>> +                  - const: 1
>> +                  - const: 2
>> +              - items:
>> +                  - const: 1
>> +          link-frequencies: true
>> +
>> +        required:
>> +          - data-lanes
>> +          - link-frequencies
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - clocks
>> +  - clock-names
>> +  - dovdd-supply
>> +  - avdd-supply
>> +  - dvdd-supply
>> +  - powerdown-gpios
>> +  - reset-gpios
>> +  - port
>
> I think we don't need all of these entries as required.
> The only let me say "really" required are:
>
> - compatible
> - reg
> - clocks
> - port

Thanks for the review! I agree that the driver may be modified to work
without powerdown and reset gpios and they are not required for sensor
operation. On contrary, supplies are obviously required. Of course, linux
provides dummy regulators if supplies are missing from device tree, but
I though the intention was to document hardware, not implementation
details. What do think of this?

> Regards,
> Tommaso

--
Best regards,
Mikhail Rudenko



[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