On 2022-09-16 at 15:15 +02, Tommaso Merciai <tommaso.merciai@xxxxxxxxxxxxxxxxxxxx> wrote: > Hi Mikhail, > > On Thu, Sep 15, 2022 at 11:11:57PM +0300, Mikhail Rudenko wrote: >> >> 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? > > We have already discuss on this on the following thread sometimes ago :) > > https://www.patchwork.linux-fancy.com/project/linux-fancy/patch/20220630134835.592521-6-tommaso.merciai@xxxxxxxxxxxxxxxxxxxx/ > > Take a look and let me know. Okay, if there already is a consensus regarding this matter, I'll make the regulators optional in v3. -- Best regards, Mikhail Rudenko
