Add device-tree binding documentation for OV4689 image sensor driver,
and the relevant MAINTAINERS entries.
Signed-off-by: Mikhail Rudenko <[email protected]>
---
.../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 <[email protected]>
+
+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
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ ov4689: camera@36 {
+ compatible = "ovti,ov4689";
+ reg = <0x36>;
+
+ clocks = <&ov4689_clk>;
+ clock-names = "xvclk";
+
+ avdd-supply = <&ov4689_avdd>;
+ dovdd-supply = <&ov4689_dovdd>;
+ dvdd-supply = <&ov4689_dvdd>;
+
+ powerdown-gpios = <&pio 107 GPIO_ACTIVE_LOW>;
+ reset-gpios = <&pio 109 GPIO_ACTIVE_LOW>;
+
+ orientation = <2>;
+ rotation = <0>;
+
+ port {
+ wcam_out: endpoint {
+ remote-endpoint = <&mipi_in_wcam>;
+ data-lanes = <1 2 3 4>;
+ link-frequencies = /bits/ 64 <500000000>;
+ };
+ };
+ };
+ };
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index f468864fd268..63c4844f26e6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14523,6 +14523,13 @@ S: Maintained
T: git git://linuxtv.org/media_tree.git
F: drivers/media/i2c/ov2740.c
+OMNIVISION OV4689 SENSOR DRIVER
+M: Mikhail Rudenko <[email protected]>
+L: [email protected]
+S: Maintained
+T: git git://linuxtv.org/media_tree.git
+F: Documentation/devicetree/bindings/media/i2c/ovti,ov4689.yaml
+
OMNIVISION OV5640 SENSOR DRIVER
M: Steve Longerbeam <[email protected]>
L: [email protected]
--
2.37.3
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 <[email protected]>
Too many "media" prefixes in the subject. Also you duplicated dt
bindings as prefix and commit msg (skip the latter).
> ---
> .../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 <[email protected]>
> +
> +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.
> +
> + 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.
> + 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
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 <[email protected]>
> ---
> .../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 <[email protected]>
> +
> +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
Regards,
Tommaso
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/gpio/gpio.h>
> +
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + ov4689: camera@36 {
> + compatible = "ovti,ov4689";
> + reg = <0x36>;
> +
> + clocks = <&ov4689_clk>;
> + clock-names = "xvclk";
> +
> + avdd-supply = <&ov4689_avdd>;
> + dovdd-supply = <&ov4689_dovdd>;
> + dvdd-supply = <&ov4689_dvdd>;
> +
> + powerdown-gpios = <&pio 107 GPIO_ACTIVE_LOW>;
> + reset-gpios = <&pio 109 GPIO_ACTIVE_LOW>;
> +
> + orientation = <2>;
> + rotation = <0>;
> +
> + port {
> + wcam_out: endpoint {
> + remote-endpoint = <&mipi_in_wcam>;
> + data-lanes = <1 2 3 4>;
> + link-frequencies = /bits/ 64 <500000000>;
> + };
> + };
> + };
> + };
> +
> +...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f468864fd268..63c4844f26e6 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -14523,6 +14523,13 @@ S: Maintained
> T: git git://linuxtv.org/media_tree.git
> F: drivers/media/i2c/ov2740.c
>
> +OMNIVISION OV4689 SENSOR DRIVER
> +M: Mikhail Rudenko <[email protected]>
> +L: [email protected]
> +S: Maintained
> +T: git git://linuxtv.org/media_tree.git
> +F: Documentation/devicetree/bindings/media/i2c/ovti,ov4689.yaml
> +
> OMNIVISION OV5640 SENSOR DRIVER
> M: Steve Longerbeam <[email protected]>
> L: [email protected]
> --
> 2.37.3
>
--
Tommaso Merciai
Embedded Linux Engineer
[email protected]
__________________________________
Amarula Solutions SRL
Via Le Canevare 30, 31100 Treviso, Veneto, IT
T. +39 042 243 5310
[email protected]
http://www.amarulasolutions.com
Hi Tommaso,
On 2022-09-13 at 16:05 +02, Tommaso Merciai <[email protected]> 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 <[email protected]>
>> ---
>> .../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 <[email protected]>
>> +
>> +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
Hi Krzysztof,
On 2022-09-12 at 12:55 +02, Krzysztof Kozlowski <[email protected]> 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 <[email protected]>
>
> 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 <[email protected]>
>> +
>> +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
On 15/09/2022 13:16, Mikhail Rudenko wrote:
>
> 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?
Ah, sorry, I meant in the subject. You already have dt-bindings as
prefix, so no "DT bindings" at the end. Just:
media: dt-bindings: i2c: document OV4689
>
>>> ---
>>> .../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 <[email protected]>
>>> +
>>> +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)?
Yes, usually clock-names for one entry does not make sense.
>
>>> +
>>> + 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?
No.
https://elixir.bootlin.com/linux/v6.0-rc1/source/Documentation/devicetree/bindings/gpio/gpio-consumer-common.yaml
Best regards,
Krzysztof
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 <[email protected]> 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 <[email protected]>
> >> ---
> >> .../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 <[email protected]>
> >> +
> >> +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/[email protected]/
Take a look and let me know.
Thanks,
Tommaso
>
> > Regards,
> > Tommaso
>
> --
> Best regards,
> Mikhail Rudenko
--
Tommaso Merciai
Embedded Linux Engineer
[email protected]
__________________________________
Amarula Solutions SRL
Via Le Canevare 30, 31100 Treviso, Veneto, IT
T. +39 042 243 5310
[email protected]
http://www.amarulasolutions.com
On 2022-09-16 at 15:15 +02, Tommaso Merciai <[email protected]> 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 <[email protected]> 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 <[email protected]>
>> >> ---
>> >> .../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 <[email protected]>
>> >> +
>> >> +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/[email protected]/
>
> 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
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 <[email protected]> wrote:
> > On Thu, Sep 15, 2022 at 11:11:57PM +0300, Mikhail Rudenko wrote:
> >> On 2022-09-13 at 16:05 +02, Tommaso Merciai <[email protected]> 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 <[email protected]>
> >> >> ---
> >> >> .../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 <[email protected]>
> >> >> +
> >> >> +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/[email protected]/
> >
> > 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