CC'ing Liam and Mark. On Fri, Sep 16, 2022 at 04:42:25PM +0300, Mikhail Rudenko wrote: > On 2022-09-16 at 15:15 +02, Tommaso Merciai <tommaso.merciai@xxxxxxxxxxxxxxxxxxxx> wrote: > > On Thu, Sep 15, 2022 at 11:11:57PM +0300, Mikhail Rudenko wrote: > >> On 2022-09-13 at 16:05 +02, Tommaso Merciai <tommaso.merciai@xxxxxxxxxxxxxxxxxxxx> wrote: > >> > 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. I always request the opposite during reviews :-) Could we get an authoritative answer from the maintainers of the regulator framework on this question, and document it somewhere ? -- Regards, Laurent Pinchart
