[1/3] media: dt-bindings: Add description of OmniVision OG01A1B image sensor

Message ID 20240620124745.1265011-2-vladimir.zapolskiy@linaro.org (mailing list archive)
State New
Headers
Series media: i2c: og01a1b: Add OF support to OmniVision OG01A1B |

Commit Message

Vladimir Zapolskiy June 20, 2024, 12:47 p.m. UTC
  Add device tree bindings documentation for OmniVision OG01A1B image
sensor.

Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
---
 .../bindings/media/i2c/ovti,og01a1b.yaml      | 108 ++++++++++++++++++
 MAINTAINERS                                   |   1 +
 2 files changed, 109 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,og01a1b.yaml
  

Comments

Krzysztof Kozlowski June 20, 2024, 3:33 p.m. UTC | #1
On 20/06/2024 14:47, Vladimir Zapolskiy wrote:
> Add device tree bindings documentation for OmniVision OG01A1B image
> sensor.
> 
> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>


> +
> +maintainers:
> +  - Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
> +
> +description: |-

Do not need '|-' unless you need to preserve formatting.


> +  The OmniVision OG01A1B is black and white CMOS 1.3 Megapixel (1280x1024)
> +  image sensor controlled over an I2C-compatible SCCB bus.
> +  The sensor transmits images on a MIPI CSI-2 output interface with one or
> +  two data lanes.
> +
> +allOf:
> +  - $ref: /schemas/media/video-interface-devices.yaml#
> +
> +properties:
> +  compatible:
> +    const: ovti,og01a1b
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    description: External clock connected to the sensor system clock.

Drop description, obvious.

> +    maxItems: 1
> +
> +  clock-frequency:
> +    description: Frequency of the external clock to the sensor in Hz.

Drop property. Frequency comes from clock.

> +
> +  reset-gpios:
> +    description: Active low signal GPIO to control sensor.
> +    maxItems: 1
> +
> +  strobe-gpios:
> +    description: GPIO connected to the sensor strobe pad.
> +    maxItems: 1
> +
> +  avdd-supply:
> +    description: Analogue circuit voltage supply.
> +
> +  dovdd-supply:
> +    description: I/O circuit voltage supply.
> +
> +  dvdd-supply:
> +    description: Digital circuit voltage supply.
> +
> +  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:
> +            minItems: 1
> +            maxItems: 2
> +
> +          link-frequencies: true

This should not be needed.

> +
> +        required:
> +          - data-lanes
> +


Best regards,
Krzysztof
  
Sakari Ailus June 20, 2024, 8:04 p.m. UTC | #2
Hi Krzysztof,

On Thu, Jun 20, 2024 at 05:33:15PM +0200, Krzysztof Kozlowski wrote:
> On 20/06/2024 14:47, Vladimir Zapolskiy wrote:
> > +    properties:
> > +      endpoint:
> > +        $ref: /schemas/media/video-interfaces.yaml#
> > +        unevaluatedProperties: false
> > +
> > +        properties:
> > +          data-lanes:
> > +            minItems: 1
> > +            maxItems: 2
> > +
> > +          link-frequencies: true
> 
> This should not be needed.

Why?

Please see
<URL:https://hverkuil.home.xs4all.nl/spec/driver-api/camera-sensor.html#handling-clocks>.
  
Krzysztof Kozlowski June 20, 2024, 8:28 p.m. UTC | #3
On 20/06/2024 22:04, Sakari Ailus wrote:
> Hi Krzysztof,
> 
> On Thu, Jun 20, 2024 at 05:33:15PM +0200, Krzysztof Kozlowski wrote:
>> On 20/06/2024 14:47, Vladimir Zapolskiy wrote:
>>> +    properties:
>>> +      endpoint:
>>> +        $ref: /schemas/media/video-interfaces.yaml#
>>> +        unevaluatedProperties: false
>>> +
>>> +        properties:
>>> +          data-lanes:
>>> +            minItems: 1
>>> +            maxItems: 2
>>> +
>>> +          link-frequencies: true
>>
>> This should not be needed.
> 
> Why?

Because it is a no-op. Changes nothing, absolutely nothing. Property is
allowed from video-interfaces via unevaluatedProperties.

> 
> Please see
> <URL:https://hverkuil.home.xs4all.nl/spec/driver-api/camera-sensor.html#handling-clocks>.

This doe snot cover empty link-frequencies or I could not spot what you
are referring to.

Unless you are suggesting that this should be fixed into proper
frequencies? Then yes, Ack. Keep it with proper values.


Best regards,
Krzysztof
  
Sakari Ailus June 21, 2024, 5:12 p.m. UTC | #4
Hi Krzysztof,

On Thu, Jun 20, 2024 at 10:28:03PM +0200, Krzysztof Kozlowski wrote:
> On 20/06/2024 22:04, Sakari Ailus wrote:
> > Hi Krzysztof,
> > 
> > On Thu, Jun 20, 2024 at 05:33:15PM +0200, Krzysztof Kozlowski wrote:
> >> On 20/06/2024 14:47, Vladimir Zapolskiy wrote:
> >>> +    properties:
> >>> +      endpoint:
> >>> +        $ref: /schemas/media/video-interfaces.yaml#
> >>> +        unevaluatedProperties: false
> >>> +
> >>> +        properties:
> >>> +          data-lanes:
> >>> +            minItems: 1
> >>> +            maxItems: 2
> >>> +
> >>> +          link-frequencies: true
> >>
> >> This should not be needed.
> > 
> > Why?
> 
> Because it is a no-op. Changes nothing, absolutely nothing. Property is
> allowed from video-interfaces via unevaluatedProperties.

Right, indeed.

This should be listeed as required instead.

> 
> > 
> > Please see
> > <URL:https://hverkuil.home.xs4all.nl/spec/driver-api/camera-sensor.html#handling-clocks>.
> 
> This doe snot cover empty link-frequencies or I could not spot what you
> are referring to.
> 
> Unless you are suggesting that this should be fixed into proper
> frequencies? Then yes, Ack. Keep it with proper values.
  
kernel test robot June 24, 2024, 3:25 p.m. UTC | #5
Hi Vladimir,

kernel test robot noticed the following build warnings:

[auto build test WARNING on media-tree/master]
[also build test WARNING on sailus-media-tree/master krzk-dt/for-next robh/for-next linus/master v6.10-rc5 next-20240621]
[cannot apply to sailus-media-tree/streams]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Vladimir-Zapolskiy/media-dt-bindings-Add-description-of-OmniVision-OG01A1B-image-sensor/20240624-161554
base:   git://linuxtv.org/media_tree.git master
patch link:    https://lore.kernel.org/r/20240620124745.1265011-2-vladimir.zapolskiy%40linaro.org
patch subject: [PATCH 1/3] media: dt-bindings: Add description of OmniVision OG01A1B image sensor
reproduce: (https://download.01.org/0day-ci/archive/20240624/202406242352.HSKkjAv2-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202406242352.HSKkjAv2-lkp@intel.com/

All warnings (new ones prefixed by >>):

   Warning: Documentation/devicetree/bindings/power/wakeup-source.txt references a file that doesn't exist: Documentation/devicetree/bindings/input/qcom,pm8xxx-keypad.txt
   Warning: Documentation/devicetree/bindings/regulator/siliconmitus,sm5703-regulator.yaml references a file that doesn't exist: Documentation/devicetree/bindings/mfd/siliconmitus,sm5703.yaml
   Warning: Documentation/devicetree/bindings/sound/fsl-asoc-card.txt references a file that doesn't exist: Documentation/devicetree/bindings/sound/fsl,asrc.txt
   Warning: Documentation/gpu/amdgpu/display/display-contributing.rst references a file that doesn't exist: Documentation/GPU/amdgpu/display/mpo-overview.rst
>> Warning: MAINTAINERS references a file that doesn't exist: Documentation/devicetree/bindings/media/i2c/og01a1b.yaml
   Using alabaster theme
  

Patch

diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,og01a1b.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,og01a1b.yaml
new file mode 100644
index 000000000000..5f4c9c4c1ee5
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ovti,og01a1b.yaml
@@ -0,0 +1,108 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (c) 2023-2024 Linaro Ltd.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/i2c/ovti,og01a1b.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: OmniVision OG01A1B Image Sensor
+
+maintainers:
+  - Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
+
+description: |-
+  The OmniVision OG01A1B is black and white CMOS 1.3 Megapixel (1280x1024)
+  image sensor controlled over an I2C-compatible SCCB bus.
+  The sensor transmits images on a MIPI CSI-2 output interface with one or
+  two data lanes.
+
+allOf:
+  - $ref: /schemas/media/video-interface-devices.yaml#
+
+properties:
+  compatible:
+    const: ovti,og01a1b
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    description: External clock connected to the sensor system clock.
+    maxItems: 1
+
+  clock-frequency:
+    description: Frequency of the external clock to the sensor in Hz.
+
+  reset-gpios:
+    description: Active low signal GPIO to control sensor.
+    maxItems: 1
+
+  strobe-gpios:
+    description: GPIO connected to the sensor strobe pad.
+    maxItems: 1
+
+  avdd-supply:
+    description: Analogue circuit voltage supply.
+
+  dovdd-supply:
+    description: I/O circuit voltage supply.
+
+  dvdd-supply:
+    description: Digital circuit voltage supply.
+
+  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:
+            minItems: 1
+            maxItems: 2
+
+          link-frequencies: true
+
+        required:
+          - data-lanes
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - port
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        sensor@60 {
+            compatible = "ovti,og01a1b";
+            reg = <0x60>;
+            clocks = <&clk 0>;
+            reset-gpios = <&gpio 117 GPIO_ACTIVE_LOW>;
+            avdd-supply = <&vreg_3v3>;
+            dovdd-supply = <&vreg_1p8>;
+            dvdd-supply = <&vreg_1p2>;
+
+            port {
+                og01a1b_ep: endpoint {
+                    remote-endpoint = <&csiphy_ep>;
+                    data-lanes = <1 2>;
+                    link-frequencies = /bits/ 64 <500000000>;
+                };
+            };
+        };
+    };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index d6c90161c7bf..eff16776a287 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16509,6 +16509,7 @@  OMNIVISION OG01A1B SENSOR DRIVER
 M:	Sakari Ailus <sakari.ailus@linux.intel.com>
 L:	linux-media@vger.kernel.org
 S:	Maintained
+F:	Documentation/devicetree/bindings/media/i2c/og01a1b.yaml
 F:	drivers/media/i2c/og01a1b.c
 
 OMNIVISION OV01A10 SENSOR DRIVER