ov8858 device tree addition

ov8858 device tree addition

[Nicholas Roth]

All,

I've submitted a patch to linux-media that adds a driver for
the Omnivision OV8858 image sensor. Jacopo, a reviewer, asked
me to submit a device tree binding as well.

The relevant linux-media patch is entitled:
media: i2c: ov8858 Add driver for ov8858

Thanks,
-Nicholas

[PATCH] dt-bindings: media: Add Omnivision ov8858 binding

[Nicholas Roth]

Add a device tree binding for the Omnivision OV8858 image sensor.
The OV8858 is an 8 megapixel image sensor which provides images in RAW
format over MIPI CSI-2 data bus and is controlled through an
I2C-compatibile SCCB bus.

Tested on PinePhone Pro with libcamera cam and qcam.

Signed-off-by: Nicholas Roth <nicholas@rothemail.net>
---
 .../bindings/i2c/ovti,ov8858-i2c.yaml         | 78 +++++++++++++++++++
 1 file changed, 78 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/ovti,ov8858-i2c.yaml

diff --git a/Documentation/devicetree/bindings/i2c/ovti,ov8858-i2c.yaml b/Documentation/devicetree/bindings/i2c/ovti,ov8858-i2c.yaml
new file mode 100644
index 000000000000..1300c1b6fa17
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/ovti,ov8858-i2c.yaml
@@ -0,0 +1,78 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/i2c/i2c-imx.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Freescale Inter IC (I2C) and High Speed Inter IC (HS-I2C) for OV8858
+
+allOf:
+  - $ref: /schemas/i2c/i2c-controller.yaml#
+
+properties:
+  compatible: ovti,ov8858
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    const: xvclk
+
+  clock-frequency:
+    minimum: 24000000
+    default: 24000000
+    maximum: 24000000
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    i2c@ff110000 {
+        status = "okay";
+
+        clock-frequency = <400000>;
+        i2c-scl-rising-time-ns = <300>;
+        i2c-scl-falling-time-ns = <15>;
+        
+        pinctrl-0 = <&i2c1_xfer &cif_clkouta>;
+        
+        assigned-clocks = <&cru SCLK_CIF_OUT &cru SCLK_CIF_OUT_SRC>;
+        assigned-clock-parents = <&cru SCLK_CIF_OUT_SRC &cru PLL_GPLL>;
+        assigned-clock-rates = <19200000 0>;
+        ucam: camera@36 {
+            compatible = "ovti,ov8858";
+            reg = <0x36>;
+            pinctrl-names = "default";
+            pinctrl-0 = <&ucam_pdn &ucam_rst>;
+
+            clocks = <&cru SCLK_CIF_OUT>;
+            clock-names = "xvclk";
+
+            dovdd-supply = <&vcc1v8_dvp>;
+            /*XXX: also depends on vcca1v8_codec for I2C bus power */
+
+            reset-gpios = <&gpio1 RK_PA4 GPIO_ACTIVE_LOW>;
+            powerdown-gpios = <&gpio2 RK_PB4 GPIO_ACTIVE_LOW>;
+
+            rotation = <180>;
+
+            port {
+                ucam_out: endpoint {
+                    remote-endpoint = <&mipi_in_ucam>;
+                    data-lanes = <1 2 3 4>;
+                };
+            };
+        };
+    };
-- 
2.34.1

Re: [PATCH] dt-bindings: media: Add Omnivision ov8858 binding

[Krzysztof Kozlowski]

On 08/11/2022 04:38, Nicholas Roth wrote:
> Add a device tree binding for the Omnivision OV8858 image sensor.

Subject: drop redundant, second "binding".

Also - wrong CC list.

Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC.  It might happen, that command when run on an older
kernel, gives you outdated entries.  Therefore please be sure you base
your patches on recent Linux kernel.


> The OV8858 is an 8 megapixel image sensor which provides images in RAW
> format over MIPI CSI-2 data bus and is controlled through an
> I2C-compatibile SCCB bus.
> 
> Tested on PinePhone Pro with libcamera cam and qcam.
> 
> Signed-off-by: Nicholas Roth <nicholas@rothemail.net>
> ---
>  .../bindings/i2c/ovti,ov8858-i2c.yaml         | 78 +++++++++++++++++++

Where is the driver? If it was sent separately, you must resent entire
patchset.

>  1 file changed, 78 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/i2c/ovti,ov8858-i2c.yaml
> 
> diff --git a/Documentation/devicetree/bindings/i2c/ovti,ov8858-i2c.yaml b/Documentation/devicetree/bindings/i2c/ovti,ov8858-i2c.yaml
> new file mode 100644
> index 000000000000..1300c1b6fa17
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/ovti,ov8858-i2c.yaml

Filename matching the compatible.

> @@ -0,0 +1,78 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/i2c/i2c-imx.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Freescale Inter IC (I2C) and High Speed Inter IC (HS-I2C) for OV8858
> +
> +allOf:
> +  - $ref: /schemas/i2c/i2c-controller.yaml#
> +
> +properties:
> +  compatible: ovti,ov8858
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  clock-names:
> +    const: xvclk
> +
> +  clock-frequency:
> +    minimum: 24000000
> +    default: 24000000
> +    maximum: 24000000

1. This should be then const instead of these tree... but:
2. Why it can work with only one frequency? Most of camera sensors take few.

> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    i2c@ff110000 {
> +        status = "okay";

Drop

> +
> +        clock-frequency = <400000>;
> +        i2c-scl-rising-time-ns = <300>;
> +        i2c-scl-falling-time-ns = <15>;
> +        
> +        pinctrl-0 = <&i2c1_xfer &cif_clkouta>;
> +        
> +        assigned-clocks = <&cru SCLK_CIF_OUT &cru SCLK_CIF_OUT_SRC>;
> +        assigned-clock-parents = <&cru SCLK_CIF_OUT_SRC &cru PLL_GPLL>;
> +        assigned-clock-rates = <19200000 0>;

Drop all above properties, not related.

> +        ucam: camera@36 {
> +            compatible = "ovti,ov8858";
> +            reg = <0x36>;
> +            pinctrl-names = "default";
> +            pinctrl-0 = <&ucam_pdn &ucam_rst>;
> +
> +            clocks = <&cru SCLK_CIF_OUT>;
> +            clock-names = "xvclk";
> +
> +            dovdd-supply = <&vcc1v8_dvp>;
> +            /*XXX: also depends on vcca1v8_codec for I2C bus power */

So your bindings are incomplete?
> +
> +            reset-gpios = <&gpio1 RK_PA4 GPIO_ACTIVE_LOW>;
> +            powerdown-gpios = <&gpio2 RK_PB4 GPIO_ACTIVE_LOW>;

Does not look like you tested the bindings. Please run `make
dt_binding_check` (see
Documentation/devicetree/bindings/writing-schema.rst for instructions).

Best regards,
Krzysztof

Re: [PATCH] dt-bindings: media: Add Omnivision ov8858 binding

[Rob Herring]

On Mon, 07 Nov 2022 21:38:13 -0600, Nicholas Roth wrote:
> Add a device tree binding for the Omnivision OV8858 image sensor.
> The OV8858 is an 8 megapixel image sensor which provides images in RAW
> format over MIPI CSI-2 data bus and is controlled through an
> I2C-compatibile SCCB bus.
> 
> Tested on PinePhone Pro with libcamera cam and qcam.
> 
> Signed-off-by: Nicholas Roth <nicholas@rothemail.net>
> ---
>  .../bindings/i2c/ovti,ov8858-i2c.yaml         | 78 +++++++++++++++++++
>  1 file changed, 78 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/i2c/ovti,ov8858-i2c.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/i2c/ovti,ov8858-i2c.yaml: properties:compatible: 'ovti,ov8858' is not of type 'object', 'boolean'
	from schema $id: http://json-schema.org/draft-07/schema#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/i2c/ovti,ov8858-i2c.yaml: 'maintainers' is a required property
	hint: Metaschema for devicetree binding documentation
	from schema $id: http://devicetree.org/meta-schemas/base.yaml#
./Documentation/devicetree/bindings/i2c/ovti,ov8858-i2c.yaml: $id: relative path/filename doesn't match actual path or filename
	expected: http://devicetree.org/schemas/i2c/ovti,ov8858-i2c.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/i2c/ovti,ov8858-i2c.yaml: ignoring, error in schema: properties: compatible
Error: Documentation/devicetree/bindings/i2c/ovti,ov8858-i2c.example.dts:27.37-38 syntax error
FATAL ERROR: Unable to parse input tree
make[1]: *** [scripts/Makefile.lib:406: Documentation/devicetree/bindings/i2c/ovti,ov8858-i2c.example.dtb] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1492: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

Re: [PATCH] dt-bindings: media: Add Omnivision ov8858 binding

[Nicholas Roth]

I just rebased onto the latest master and here's what I got:
(base) nroth@nroth-pc:~/linux$ perl scripts/get_maintainer.pl
Documentation/devicetree/bindings/media/i2c/ov8858.yaml
Nicholas Roth <nicholas@rothemail.net> (odd fixer:OMNIVISION OV8858
SENSOR DRIVER,commit_signer:1/1=100%,authored:1/1=100%,in file)
Mauro Carvalho Chehab <mchehab@kernel.org> (maintainer:MEDIA INPUT
INFRASTRUCTURE (V4L/DVB))
Rob Herring <robh+dt@kernel.org> (maintainer:OPEN FIRMWARE AND
FLATTENED DEVICE TREE BINDINGS)
Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
(maintainer:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS)
linux-media@vger.kernel.org (open list:OMNIVISION OV8858 SENSOR DRIVER)
devicetree@vger.kernel.org (open list:OPEN FIRMWARE AND FLATTENED
DEVICE TREE BINDINGS)
linux-kernel@vger.kernel.org (open list)

Is this just your way of saying that I forgot Mauro or was there
something else too? Thanks for the catch-- indeed, I did.

> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
Well, that's embarrassing... I forgot to commit before running
git-send-email. Also, great to know about the linter! Indeed, there
were still some errors on my latest branch. Thanks for bearing with me
for now :). Sending a v2...

On Tue, Nov 8, 2022 at 6:54 AM Rob Herring <robh@kernel.org> wrote:
>
>
> On Mon, 07 Nov 2022 21:38:13 -0600, Nicholas Roth wrote:
> > Add a device tree binding for the Omnivision OV8858 image sensor.
> > The OV8858 is an 8 megapixel image sensor which provides images in RAW
> > format over MIPI CSI-2 data bus and is controlled through an
> > I2C-compatibile SCCB bus.
> >
> > Tested on PinePhone Pro with libcamera cam and qcam.
> >
> > Signed-off-by: Nicholas Roth <nicholas@rothemail.net>
> > ---
> >  .../bindings/i2c/ovti,ov8858-i2c.yaml         | 78 +++++++++++++++++++
> >  1 file changed, 78 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/i2c/ovti,ov8858-i2c.yaml
> >
>
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
>
> yamllint warnings/errors:
>
> dtschema/dtc warnings/errors:
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/i2c/ovti,ov8858-i2c.yaml: properties:compatible: 'ovti,ov8858' is not of type 'object', 'boolean'
>         from schema $id: http://json-schema.org/draft-07/schema#
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/i2c/ovti,ov8858-i2c.yaml: 'maintainers' is a required property
>         hint: Metaschema for devicetree binding documentation
>         from schema $id: http://devicetree.org/meta-schemas/base.yaml#
> ./Documentation/devicetree/bindings/i2c/ovti,ov8858-i2c.yaml: $id: relative path/filename doesn't match actual path or filename
>         expected: http://devicetree.org/schemas/i2c/ovti,ov8858-i2c.yaml#
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/i2c/ovti,ov8858-i2c.yaml: ignoring, error in schema: properties: compatible
> Error: Documentation/devicetree/bindings/i2c/ovti,ov8858-i2c.example.dts:27.37-38 syntax error
> FATAL ERROR: Unable to parse input tree
> make[1]: *** [scripts/Makefile.lib:406: Documentation/devicetree/bindings/i2c/ovti,ov8858-i2c.example.dtb] Error 1
> make[1]: *** Waiting for unfinished jobs....
> make: *** [Makefile:1492: dt_binding_check] Error 2
>
> doc reference errors (make refcheckdocs):
>
> See https://patchwork.ozlabs.org/patch/
>
> This check can fail if there are any dependencies. The base for a patch
> series is generally the most recent rc1.
>
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
>
> pip3 install dtschema --upgrade
>
> Please check and re-submit.
>

Re: [PATCH] dt-bindings: media: Add Omnivision ov8858 binding

[Krzysztof Kozlowski]

On 09/11/2022 03:30, Nicholas Roth wrote:
> I just rebased onto the latest master and here's what I got:
> (base) nroth@nroth-pc:~/linux$ perl scripts/get_maintainer.pl
> Documentation/devicetree/bindings/media/i2c/ov8858.yaml
> Nicholas Roth <nicholas@rothemail.net> (odd fixer:OMNIVISION OV8858
> SENSOR DRIVER,commit_signer:1/1=100%,authored:1/1=100%,in file)
> Mauro Carvalho Chehab <mchehab@kernel.org> (maintainer:MEDIA INPUT
> INFRASTRUCTURE (V4L/DVB))

And he is not in Cc...

> Rob Herring <robh+dt@kernel.org> (maintainer:OPEN FIRMWARE AND
> FLATTENED DEVICE TREE BINDINGS)
> Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> (maintainer:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS)
> linux-media@vger.kernel.org (open list:OMNIVISION OV8858 SENSOR DRIVER)

Missing in Cc.

> devicetree@vger.kernel.org (open list:OPEN FIRMWARE AND FLATTENED
> DEVICE TREE BINDINGS)
> linux-kernel@vger.kernel.org (open list)

Missing in Cc.

> 
> Is this just your way of saying that I forgot Mauro or was there
> something else too? Thanks for the catch-- indeed, I did.

Mauro and lists.

I actually thought Sakari is listed as well, but it seems not, so if you
cc-ed Mauro and lists in v2, then it's good.

Best regards,
Krzysztof

ov8858 device tree addition

[Nicholas Roth]

Device tree entry for ov8858 image sensor.

Changes since v1:
* Fixed CC list
* Changed filename and path
* Added documentation for power GPIO
* Fixed linter errors