media: dt-bindings: Merge OV5695 into OV5693 binding

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

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
>

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
>>>

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