[PATCH] media: dt-bindings: Merge OV5695 into OV5693 binding

[PATCH] media: dt-bindings: Merge OV5695 into OV5693 binding

[Rob Herring]

The OV5695 binding is almost the same as the OV5693 binding. The only
difference is 'clock-names' is defined for OV5695. However, the lack of
clock-names is an omission as the Linux OV5693 driver expects the same
'xvclk' clock name.

'link-frequencies' is required by OV5693, but not OV5695. Just drop it
from being required. Expressing it conditionally would be ugly. It
shouldn't really be required either as the driver only supports 1
frequency anyways.

The rockchip-isp1 binding example is missing required properties, so it
has to be updated as well.

Signed-off-by: Rob Herring <robh@kernel.org>
---
 .../devicetree/bindings/media/i2c/ov5695.txt  | 41 -------------------
 .../bindings/media/i2c/ovti,ov5693.yaml       | 19 +++++----
 .../bindings/media/rockchip-isp1.yaml         |  1 +
 3 files changed, 13 insertions(+), 48 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/media/i2c/ov5695.txt

diff --git a/Documentation/devicetree/bindings/media/i2c/ov5695.txt b/Documentation/devicetree/bindings/media/i2c/ov5695.txt
deleted file mode 100644
index 640a63717d96..000000000000
--- a/Documentation/devicetree/bindings/media/i2c/ov5695.txt
+++ /dev/null
@@ -1,41 +0,0 @@
-* Omnivision OV5695 MIPI CSI-2 sensor
-
-Required Properties:
-- compatible: shall be "ovti,ov5695"
-- clocks: reference to the xvclk input clock
-- clock-names: shall be "xvclk"
-- avdd-supply: Analog voltage supply, 2.8 volts
-- dovdd-supply: Digital I/O voltage supply, 1.8 volts
-- dvdd-supply: Digital core voltage supply, 1.2 volts
-- reset-gpios: Low active reset gpio
-
-The device node shall contain one 'port' child node with an
-'endpoint' subnode for its digital output video port,
-in accordance with the video interface bindings defined in
-Documentation/devicetree/bindings/media/video-interfaces.txt.
-The endpoint optional property 'data-lanes' shall be "<1 2>".
-
-Example:
-&i2c7 {
-	ov5695: camera-sensor@36 {
-		compatible = "ovti,ov5695";
-		reg = <0x36>;
-		pinctrl-names = "default";
-		pinctrl-0 = <&clk_24m_cam>;
-
-		clocks = <&cru SCLK_TESTCLKOUT1>;
-		clock-names = "xvclk";
-
-		avdd-supply = <&pp2800_cam>;
-		dovdd-supply = <&pp1800>;
-		dvdd-supply = <&pp1250_cam>;
-		reset-gpios = <&gpio2 5 GPIO_ACTIVE_LOW>;
-
-		port {
-			wcam_out: endpoint {
-				remote-endpoint = <&mipi_in_wcam>;
-				data-lanes = <1 2>;
-			};
-		};
-	};
-};
diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml
index 359dc08440a8..a3d73a87d797 100644
--- a/Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml
+++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml
@@ -5,26 +5,29 @@
 $id: http://devicetree.org/schemas/media/i2c/ovti,ov5693.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: Omnivision OV5693 CMOS Sensor
+title: Omnivision OV5693/OV5695 CMOS Sensors
 
 maintainers:
   - Tommaso Merciai <tommaso.merciai@amarulasolutions.com>
 
 description: |
-  The Omnivision OV5693 is a high performance, 1/4-inch, 5 megapixel, CMOS
-  image sensor that delivers 2592x1944 at 30fps. It provides full-frame,
+  The Omnivision OV5693/OV5695 are high performance, 1/4-inch, 5 megapixel, CMOS
+  image sensors that deliver 2592x1944 at 30fps. It provides full-frame,
   sub-sampled, and windowed 10-bit MIPI images in various formats via the
   Serial Camera Control Bus (SCCB) interface.
 
-  OV5693 is controlled via I2C and two-wire Serial Camera Control Bus (SCCB).
-  The sensor output is available via CSI-2 serial data output (up to 2-lane).
+  OV5693/OV5695 are controlled via I2C and two-wire Serial Camera Control Bus
+  (SCCB). The sensor output is available via CSI-2 serial data output (up to
+  2-lane).
 
 allOf:
   - $ref: /schemas/media/video-interface-devices.yaml#
 
 properties:
   compatible:
-    const: ovti,ov5693
+    enum:
+      - ovti,ov5693
+      - ovti,ov5695
 
   reg:
     maxItems: 1
@@ -34,6 +37,9 @@ properties:
       System input clock (aka XVCLK). From 6 to 27 MHz.
     maxItems: 1
 
+  clock-names:
+    const: xvclk
+
   dovdd-supply:
     description:
       Digital I/O voltage supply, 1.8V.
@@ -72,7 +78,6 @@ properties:
 
         required:
           - data-lanes
-          - link-frequencies
 
 required:
   - compatible
diff --git a/Documentation/devicetree/bindings/media/rockchip-isp1.yaml b/Documentation/devicetree/bindings/media/rockchip-isp1.yaml
index 0bad7e640148..e466dff8286d 100644
--- a/Documentation/devicetree/bindings/media/rockchip-isp1.yaml
+++ b/Documentation/devicetree/bindings/media/rockchip-isp1.yaml
@@ -199,6 +199,7 @@ examples:
             wcam: camera@36 {
                 compatible = "ovti,ov5695";
                 reg = <0x36>;
+                clocks = <&cru SCLK_TESTCLKOUT1>;
 
                 port {
                     wcam_out: endpoint {
-- 
2.40.1

Re: [PATCH] media: dt-bindings: Merge OV5695 into OV5693 binding

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 5580 bytes --]

On Fri, Jul 07, 2023 at 03:06:04PM -0600, Rob Herring wrote:
> The OV5695 binding is almost the same as the OV5693 binding. The only
> difference is 'clock-names' is defined for OV5695. However, the lack of
> clock-names is an omission as the Linux OV5693 driver expects the same
> 'xvclk' clock name.
> 
> 'link-frequencies' is required by OV5693, but not OV5695. Just drop it
> from being required. Expressing it conditionally would be ugly. It
> shouldn't really be required either as the driver only supports 1
> frequency anyways.

I suppose the intent here is something like "the driver only supports 1
frequency and never bothers to read the property"?

Either way,
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

Cheers,
Conor.

> 
> The rockchip-isp1 binding example is missing required properties, so it
> has to be updated as well.
> 
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
>  .../devicetree/bindings/media/i2c/ov5695.txt  | 41 -------------------
>  .../bindings/media/i2c/ovti,ov5693.yaml       | 19 +++++----
>  .../bindings/media/rockchip-isp1.yaml         |  1 +
>  3 files changed, 13 insertions(+), 48 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/media/i2c/ov5695.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/ov5695.txt b/Documentation/devicetree/bindings/media/i2c/ov5695.txt
> deleted file mode 100644
> index 640a63717d96..000000000000
> --- a/Documentation/devicetree/bindings/media/i2c/ov5695.txt
> +++ /dev/null
> @@ -1,41 +0,0 @@
> -* Omnivision OV5695 MIPI CSI-2 sensor
> -
> -Required Properties:
> -- compatible: shall be "ovti,ov5695"
> -- clocks: reference to the xvclk input clock
> -- clock-names: shall be "xvclk"
> -- avdd-supply: Analog voltage supply, 2.8 volts
> -- dovdd-supply: Digital I/O voltage supply, 1.8 volts
> -- dvdd-supply: Digital core voltage supply, 1.2 volts
> -- reset-gpios: Low active reset gpio
> -
> -The device node shall contain one 'port' child node with an
> -'endpoint' subnode for its digital output video port,
> -in accordance with the video interface bindings defined in
> -Documentation/devicetree/bindings/media/video-interfaces.txt.
> -The endpoint optional property 'data-lanes' shall be "<1 2>".
> -
> -Example:
> -&i2c7 {
> -	ov5695: camera-sensor@36 {
> -		compatible = "ovti,ov5695";
> -		reg = <0x36>;
> -		pinctrl-names = "default";
> -		pinctrl-0 = <&clk_24m_cam>;
> -
> -		clocks = <&cru SCLK_TESTCLKOUT1>;
> -		clock-names = "xvclk";
> -
> -		avdd-supply = <&pp2800_cam>;
> -		dovdd-supply = <&pp1800>;
> -		dvdd-supply = <&pp1250_cam>;
> -		reset-gpios = <&gpio2 5 GPIO_ACTIVE_LOW>;
> -
> -		port {
> -			wcam_out: endpoint {
> -				remote-endpoint = <&mipi_in_wcam>;
> -				data-lanes = <1 2>;
> -			};
> -		};
> -	};
> -};
> diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml
> index 359dc08440a8..a3d73a87d797 100644
> --- a/Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml
> +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml
> @@ -5,26 +5,29 @@
>  $id: http://devicetree.org/schemas/media/i2c/ovti,ov5693.yaml#
>  $schema: http://devicetree.org/meta-schemas/core.yaml#
>  
> -title: Omnivision OV5693 CMOS Sensor
> +title: Omnivision OV5693/OV5695 CMOS Sensors
>  
>  maintainers:
>    - Tommaso Merciai <tommaso.merciai@amarulasolutions.com>
>  
>  description: |
> -  The Omnivision OV5693 is a high performance, 1/4-inch, 5 megapixel, CMOS
> -  image sensor that delivers 2592x1944 at 30fps. It provides full-frame,
> +  The Omnivision OV5693/OV5695 are high performance, 1/4-inch, 5 megapixel, CMOS
> +  image sensors that deliver 2592x1944 at 30fps. It provides full-frame,
>    sub-sampled, and windowed 10-bit MIPI images in various formats via the
>    Serial Camera Control Bus (SCCB) interface.
>  
> -  OV5693 is controlled via I2C and two-wire Serial Camera Control Bus (SCCB).
> -  The sensor output is available via CSI-2 serial data output (up to 2-lane).
> +  OV5693/OV5695 are controlled via I2C and two-wire Serial Camera Control Bus
> +  (SCCB). The sensor output is available via CSI-2 serial data output (up to
> +  2-lane).
>  
>  allOf:
>    - $ref: /schemas/media/video-interface-devices.yaml#
>  
>  properties:
>    compatible:
> -    const: ovti,ov5693
> +    enum:
> +      - ovti,ov5693
> +      - ovti,ov5695
>  
>    reg:
>      maxItems: 1
> @@ -34,6 +37,9 @@ properties:
>        System input clock (aka XVCLK). From 6 to 27 MHz.
>      maxItems: 1
>  
> +  clock-names:
> +    const: xvclk
> +
>    dovdd-supply:
>      description:
>        Digital I/O voltage supply, 1.8V.
> @@ -72,7 +78,6 @@ properties:
>  
>          required:
>            - data-lanes
> -          - link-frequencies
>  
>  required:
>    - compatible
> diff --git a/Documentation/devicetree/bindings/media/rockchip-isp1.yaml b/Documentation/devicetree/bindings/media/rockchip-isp1.yaml
> index 0bad7e640148..e466dff8286d 100644
> --- a/Documentation/devicetree/bindings/media/rockchip-isp1.yaml
> +++ b/Documentation/devicetree/bindings/media/rockchip-isp1.yaml
> @@ -199,6 +199,7 @@ examples:
>              wcam: camera@36 {
>                  compatible = "ovti,ov5695";
>                  reg = <0x36>;
> +                clocks = <&cru SCLK_TESTCLKOUT1>;
>  
>                  port {
>                      wcam_out: endpoint {
> -- 
> 2.40.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] media: dt-bindings: Merge OV5695 into OV5693 binding

[Rob Herring]

On Mon, Jul 10, 2023 at 11:45 AM Conor Dooley <conor@kernel.org> wrote:
>
> On Fri, Jul 07, 2023 at 03:06:04PM -0600, Rob Herring wrote:
> > The OV5695 binding is almost the same as the OV5693 binding. The only
> > difference is 'clock-names' is defined for OV5695. However, the lack of
> > clock-names is an omission as the Linux OV5693 driver expects the same
> > 'xvclk' clock name.
> >
> > 'link-frequencies' is required by OV5693, but not OV5695. Just drop it
> > from being required. Expressing it conditionally would be ugly. It
> > shouldn't really be required either as the driver only supports 1
> > frequency anyways.
>
> I suppose the intent here is something like "the driver only supports 1
> frequency and never bothers to read the property"?

It does read it and fails if it doesn't match. I don't really think
the driver should if there is only 1 freq. I don't know if it's that
the hw only supports 1 frequency or a driver limitation. If the h/w,
then the property is pointless.


> Either way,
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
>
> Cheers,
> Conor.
>
> >
> > The rockchip-isp1 binding example is missing required properties, so it
> > has to be updated as well.
> >
> > Signed-off-by: Rob Herring <robh@kernel.org>
> > ---
> >  .../devicetree/bindings/media/i2c/ov5695.txt  | 41 -------------------
> >  .../bindings/media/i2c/ovti,ov5693.yaml       | 19 +++++----
> >  .../bindings/media/rockchip-isp1.yaml         |  1 +
> >  3 files changed, 13 insertions(+), 48 deletions(-)
> >  delete mode 100644 Documentation/devicetree/bindings/media/i2c/ov5695.txt
> >
> > diff --git a/Documentation/devicetree/bindings/media/i2c/ov5695.txt b/Documentation/devicetree/bindings/media/i2c/ov5695.txt
> > deleted file mode 100644
> > index 640a63717d96..000000000000
> > --- a/Documentation/devicetree/bindings/media/i2c/ov5695.txt
> > +++ /dev/null
> > @@ -1,41 +0,0 @@
> > -* Omnivision OV5695 MIPI CSI-2 sensor
> > -
> > -Required Properties:
> > -- compatible: shall be "ovti,ov5695"
> > -- clocks: reference to the xvclk input clock
> > -- clock-names: shall be "xvclk"
> > -- avdd-supply: Analog voltage supply, 2.8 volts
> > -- dovdd-supply: Digital I/O voltage supply, 1.8 volts
> > -- dvdd-supply: Digital core voltage supply, 1.2 volts
> > -- reset-gpios: Low active reset gpio
> > -
> > -The device node shall contain one 'port' child node with an
> > -'endpoint' subnode for its digital output video port,
> > -in accordance with the video interface bindings defined in
> > -Documentation/devicetree/bindings/media/video-interfaces.txt.
> > -The endpoint optional property 'data-lanes' shall be "<1 2>".
> > -
> > -Example:
> > -&i2c7 {
> > -     ov5695: camera-sensor@36 {
> > -             compatible = "ovti,ov5695";
> > -             reg = <0x36>;
> > -             pinctrl-names = "default";
> > -             pinctrl-0 = <&clk_24m_cam>;
> > -
> > -             clocks = <&cru SCLK_TESTCLKOUT1>;
> > -             clock-names = "xvclk";
> > -
> > -             avdd-supply = <&pp2800_cam>;
> > -             dovdd-supply = <&pp1800>;
> > -             dvdd-supply = <&pp1250_cam>;
> > -             reset-gpios = <&gpio2 5 GPIO_ACTIVE_LOW>;
> > -
> > -             port {
> > -                     wcam_out: endpoint {
> > -                             remote-endpoint = <&mipi_in_wcam>;
> > -                             data-lanes = <1 2>;
> > -                     };
> > -             };
> > -     };
> > -};
> > diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml
> > index 359dc08440a8..a3d73a87d797 100644
> > --- a/Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml
> > +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml
> > @@ -5,26 +5,29 @@
> >  $id: http://devicetree.org/schemas/media/i2c/ovti,ov5693.yaml#
> >  $schema: http://devicetree.org/meta-schemas/core.yaml#
> >
> > -title: Omnivision OV5693 CMOS Sensor
> > +title: Omnivision OV5693/OV5695 CMOS Sensors
> >
> >  maintainers:
> >    - Tommaso Merciai <tommaso.merciai@amarulasolutions.com>
> >
> >  description: |
> > -  The Omnivision OV5693 is a high performance, 1/4-inch, 5 megapixel, CMOS
> > -  image sensor that delivers 2592x1944 at 30fps. It provides full-frame,
> > +  The Omnivision OV5693/OV5695 are high performance, 1/4-inch, 5 megapixel, CMOS
> > +  image sensors that deliver 2592x1944 at 30fps. It provides full-frame,
> >    sub-sampled, and windowed 10-bit MIPI images in various formats via the
> >    Serial Camera Control Bus (SCCB) interface.
> >
> > -  OV5693 is controlled via I2C and two-wire Serial Camera Control Bus (SCCB).
> > -  The sensor output is available via CSI-2 serial data output (up to 2-lane).
> > +  OV5693/OV5695 are controlled via I2C and two-wire Serial Camera Control Bus
> > +  (SCCB). The sensor output is available via CSI-2 serial data output (up to
> > +  2-lane).
> >
> >  allOf:
> >    - $ref: /schemas/media/video-interface-devices.yaml#
> >
> >  properties:
> >    compatible:
> > -    const: ovti,ov5693
> > +    enum:
> > +      - ovti,ov5693
> > +      - ovti,ov5695
> >
> >    reg:
> >      maxItems: 1
> > @@ -34,6 +37,9 @@ properties:
> >        System input clock (aka XVCLK). From 6 to 27 MHz.
> >      maxItems: 1
> >
> > +  clock-names:
> > +    const: xvclk
> > +
> >    dovdd-supply:
> >      description:
> >        Digital I/O voltage supply, 1.8V.
> > @@ -72,7 +78,6 @@ properties:
> >
> >          required:
> >            - data-lanes
> > -          - link-frequencies
> >
> >  required:
> >    - compatible
> > diff --git a/Documentation/devicetree/bindings/media/rockchip-isp1.yaml b/Documentation/devicetree/bindings/media/rockchip-isp1.yaml
> > index 0bad7e640148..e466dff8286d 100644
> > --- a/Documentation/devicetree/bindings/media/rockchip-isp1.yaml
> > +++ b/Documentation/devicetree/bindings/media/rockchip-isp1.yaml
> > @@ -199,6 +199,7 @@ examples:
> >              wcam: camera@36 {
> >                  compatible = "ovti,ov5695";
> >                  reg = <0x36>;
> > +                clocks = <&cru SCLK_TESTCLKOUT1>;
> >
> >                  port {
> >                      wcam_out: endpoint {
> > --
> > 2.40.1
> >

Re: [PATCH] media: dt-bindings: Merge OV5695 into OV5693 binding

[Dan Scally]

Hello both

On 10/07/2023 18:57, Rob Herring wrote:
> On Mon, Jul 10, 2023 at 11:45 AM Conor Dooley <conor@kernel.org> wrote:
>> On Fri, Jul 07, 2023 at 03:06:04PM -0600, Rob Herring wrote:
>>> The OV5695 binding is almost the same as the OV5693 binding. The only
>>> difference is 'clock-names' is defined for OV5695. However, the lack of
>>> clock-names is an omission as the Linux OV5693 driver expects the same
>>> 'xvclk' clock name.
>>>
>>> 'link-frequencies' is required by OV5693, but not OV5695. Just drop it
>>> from being required. Expressing it conditionally would be ugly. It
>>> shouldn't really be required either as the driver only supports 1
>>> frequency anyways.
>> I suppose the intent here is something like "the driver only supports 1
>> frequency and never bothers to read the property"?
> It does read it and fails if it doesn't match. I don't really think
> the driver should if there is only 1 freq. I don't know if it's that
> the hw only supports 1 frequency or a driver limitation. If the h/w,
> then the property is pointless.


It's a driver limitation. If someone needed to run a different link frequency they could add 
settings to the driver to configure the PLLs differently.

>
>
>> Either way,
>> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
>>
>> Cheers,
>> Conor.
>>
>>> The rockchip-isp1 binding example is missing required properties, so it
>>> has to be updated as well.
>>>
>>> Signed-off-by: Rob Herring <robh@kernel.org>
>>> ---
>>>   .../devicetree/bindings/media/i2c/ov5695.txt  | 41 -------------------
>>>   .../bindings/media/i2c/ovti,ov5693.yaml       | 19 +++++----
>>>   .../bindings/media/rockchip-isp1.yaml         |  1 +
>>>   3 files changed, 13 insertions(+), 48 deletions(-)
>>>   delete mode 100644 Documentation/devicetree/bindings/media/i2c/ov5695.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/media/i2c/ov5695.txt b/Documentation/devicetree/bindings/media/i2c/ov5695.txt
>>> deleted file mode 100644
>>> index 640a63717d96..000000000000
>>> --- a/Documentation/devicetree/bindings/media/i2c/ov5695.txt
>>> +++ /dev/null
>>> @@ -1,41 +0,0 @@
>>> -* Omnivision OV5695 MIPI CSI-2 sensor
>>> -
>>> -Required Properties:
>>> -- compatible: shall be "ovti,ov5695"
>>> -- clocks: reference to the xvclk input clock
>>> -- clock-names: shall be "xvclk"
>>> -- avdd-supply: Analog voltage supply, 2.8 volts
>>> -- dovdd-supply: Digital I/O voltage supply, 1.8 volts
>>> -- dvdd-supply: Digital core voltage supply, 1.2 volts
>>> -- reset-gpios: Low active reset gpio
>>> -
>>> -The device node shall contain one 'port' child node with an
>>> -'endpoint' subnode for its digital output video port,
>>> -in accordance with the video interface bindings defined in
>>> -Documentation/devicetree/bindings/media/video-interfaces.txt.
>>> -The endpoint optional property 'data-lanes' shall be "<1 2>".
>>> -
>>> -Example:
>>> -&i2c7 {
>>> -     ov5695: camera-sensor@36 {
>>> -             compatible = "ovti,ov5695";
>>> -             reg = <0x36>;
>>> -             pinctrl-names = "default";
>>> -             pinctrl-0 = <&clk_24m_cam>;
>>> -
>>> -             clocks = <&cru SCLK_TESTCLKOUT1>;
>>> -             clock-names = "xvclk";
>>> -
>>> -             avdd-supply = <&pp2800_cam>;
>>> -             dovdd-supply = <&pp1800>;
>>> -             dvdd-supply = <&pp1250_cam>;
>>> -             reset-gpios = <&gpio2 5 GPIO_ACTIVE_LOW>;
>>> -
>>> -             port {
>>> -                     wcam_out: endpoint {
>>> -                             remote-endpoint = <&mipi_in_wcam>;
>>> -                             data-lanes = <1 2>;
>>> -                     };
>>> -             };
>>> -     };
>>> -};
>>> diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml
>>> index 359dc08440a8..a3d73a87d797 100644
>>> --- a/Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml
>>> +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml
>>> @@ -5,26 +5,29 @@
>>>   $id: http://devicetree.org/schemas/media/i2c/ovti,ov5693.yaml#
>>>   $schema: http://devicetree.org/meta-schemas/core.yaml#
>>>
>>> -title: Omnivision OV5693 CMOS Sensor
>>> +title: Omnivision OV5693/OV5695 CMOS Sensors
>>>
>>>   maintainers:
>>>     - Tommaso Merciai <tommaso.merciai@amarulasolutions.com>
>>>
>>>   description: |
>>> -  The Omnivision OV5693 is a high performance, 1/4-inch, 5 megapixel, CMOS
>>> -  image sensor that delivers 2592x1944 at 30fps. It provides full-frame,
>>> +  The Omnivision OV5693/OV5695 are high performance, 1/4-inch, 5 megapixel, CMOS
>>> +  image sensors that deliver 2592x1944 at 30fps. It provides full-frame,
>>>     sub-sampled, and windowed 10-bit MIPI images in various formats via the
>>>     Serial Camera Control Bus (SCCB) interface.
>>>
>>> -  OV5693 is controlled via I2C and two-wire Serial Camera Control Bus (SCCB).
>>> -  The sensor output is available via CSI-2 serial data output (up to 2-lane).
>>> +  OV5693/OV5695 are controlled via I2C and two-wire Serial Camera Control Bus
>>> +  (SCCB). The sensor output is available via CSI-2 serial data output (up to
>>> +  2-lane).
>>>
>>>   allOf:
>>>     - $ref: /schemas/media/video-interface-devices.yaml#
>>>
>>>   properties:
>>>     compatible:
>>> -    const: ovti,ov5693
>>> +    enum:
>>> +      - ovti,ov5693
>>> +      - ovti,ov5695
>>>
>>>     reg:
>>>       maxItems: 1
>>> @@ -34,6 +37,9 @@ properties:
>>>         System input clock (aka XVCLK). From 6 to 27 MHz.
>>>       maxItems: 1
>>>
>>> +  clock-names:
>>> +    const: xvclk
>>> +
>>>     dovdd-supply:
>>>       description:
>>>         Digital I/O voltage supply, 1.8V.
>>> @@ -72,7 +78,6 @@ properties:
>>>
>>>           required:
>>>             - data-lanes
>>> -          - link-frequencies
>>>
>>>   required:
>>>     - compatible
>>> diff --git a/Documentation/devicetree/bindings/media/rockchip-isp1.yaml b/Documentation/devicetree/bindings/media/rockchip-isp1.yaml
>>> index 0bad7e640148..e466dff8286d 100644
>>> --- a/Documentation/devicetree/bindings/media/rockchip-isp1.yaml
>>> +++ b/Documentation/devicetree/bindings/media/rockchip-isp1.yaml
>>> @@ -199,6 +199,7 @@ examples:
>>>               wcam: camera@36 {
>>>                   compatible = "ovti,ov5695";
>>>                   reg = <0x36>;
>>> +                clocks = <&cru SCLK_TESTCLKOUT1>;
>>>
>>>                   port {
>>>                       wcam_out: endpoint {
>>> --
>>> 2.40.1
>>>

Re: [PATCH] media: dt-bindings: Merge OV5695 into OV5693 binding

[Sakari Ailus]

Hi Rob,

On Fri, Jul 07, 2023 at 03:06:04PM -0600, Rob Herring wrote:
> The OV5695 binding is almost the same as the OV5693 binding. The only
> difference is 'clock-names' is defined for OV5695. However, the lack of
> clock-names is an omission as the Linux OV5693 driver expects the same
> 'xvclk' clock name.
> 
> 'link-frequencies' is required by OV5693, but not OV5695. Just drop it
> from being required. Expressing it conditionally would be ugly. It
> shouldn't really be required either as the driver only supports 1
> frequency anyways.

The correct way to address this would appear to be to add link-frequencies
for both of these devices. I think I've seen one or two sensors of this
class (raw, CSI-2/parallel, external clock etc.) with link frequencies
documented as "fixed" --- which is probably a documentation issue more than
anything else.

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

-- 
Kind regards,

Sakari Ailus

Re: [PATCH] media: dt-bindings: Merge OV5695 into OV5693 binding

[Rob Herring]

On Mon, Jul 31, 2023 at 5:21 AM Sakari Ailus <sakari.ailus@iki.fi> wrote:
>
> Hi Rob,
>
> On Fri, Jul 07, 2023 at 03:06:04PM -0600, Rob Herring wrote:
> > The OV5695 binding is almost the same as the OV5693 binding. The only
> > difference is 'clock-names' is defined for OV5695. However, the lack of
> > clock-names is an omission as the Linux OV5693 driver expects the same
> > 'xvclk' clock name.
> >
> > 'link-frequencies' is required by OV5693, but not OV5695. Just drop it
> > from being required. Expressing it conditionally would be ugly. It
> > shouldn't really be required either as the driver only supports 1
> > frequency anyways.
>
> The correct way to address this would appear to be to add link-frequencies
> for both of these devices. I think I've seen one or two sensors of this
> class (raw, CSI-2/parallel, external clock etc.) with link frequencies
> documented as "fixed" --- which is probably a documentation issue more than
> anything else.

link-frequencies is already supported. It's just a question of being
required or not. Adding a property as required is an ABI break (if the
OS starts requiring the property).

Rob

Re: [PATCH] media: dt-bindings: Merge OV5695 into OV5693 binding

[Sakari Ailus]

Hi Rob,

On Tue, Aug 01, 2023 at 07:45:08AM -0600, Rob Herring wrote:
> On Mon, Jul 31, 2023 at 5:21 AM Sakari Ailus <sakari.ailus@iki.fi> wrote:
> >
> > Hi Rob,
> >
> > On Fri, Jul 07, 2023 at 03:06:04PM -0600, Rob Herring wrote:
> > > The OV5695 binding is almost the same as the OV5693 binding. The only
> > > difference is 'clock-names' is defined for OV5695. However, the lack of
> > > clock-names is an omission as the Linux OV5693 driver expects the same
> > > 'xvclk' clock name.
> > >
> > > 'link-frequencies' is required by OV5693, but not OV5695. Just drop it
> > > from being required. Expressing it conditionally would be ugly. It
> > > shouldn't really be required either as the driver only supports 1
> > > frequency anyways.
> >
> > The correct way to address this would appear to be to add link-frequencies
> > for both of these devices. I think I've seen one or two sensors of this
> > class (raw, CSI-2/parallel, external clock etc.) with link frequencies
> > documented as "fixed" --- which is probably a documentation issue more than
> > anything else.
> 
> link-frequencies is already supported. It's just a question of being
> required or not. Adding a property as required is an ABI break (if the
> OS starts requiring the property).

Currently the ov5693 driver requires a link-frequencies property, the
ov5695 driver doesn't care.

In this case the backwards compatible way would be to make it optional for
ov5695 and if it exists, then allow only those frequencies. It's a fairly
trivial driver, it only supports a single link frequency at the moment (as
well as a single external clock frequency).

At least link-frequencies should continue to be required for ov5693.

-- 
Regards,

Sakari Ailus