Hi Krzysztof, On 2022-09-12 at 12:55 +02, Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> wrote: > On 11/09/2022 22:01, 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> > > Too many "media" prefixes in the subject. I see, will drop the first "media:" in v3. > Also you duplicated dt > bindings as prefix and commit msg (skip the latter). Just to be clear, do you mean dropping "device-tree binding" phrase from the commit message? >> --- >> .../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 > > This has to be strictly defined - which name you expect. Will fix in v3. Or maybe we should drop clock-names altogether and use devm_clk_get(&client->dev, NULL) in the driver instead (I've seen this approach in some existing drivers)? >> + >> + 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 > > You can skip here maxItems - it is defined by gpio-consumer-common. Ack, will fix in v3. Does this also apply to reset-gpios? >> + description: >> + GPIO connected to the powerdown pin (active low) >> + >> + reset-gpios: >> + maxItems: 1 >> + description: >> + GPIO connected to the reset pin (active low) >> + > > Best regards, > Krzysztof Thanks for review, -- Best regards, Mikhail Rudenko
