On 04/05/2022 15:55, Quentin Schulz wrote:
> From: Quentin Schulz <[email protected]>
>
> This patch adds documentation of device tree in YAML schema for the
> OV5675 CMOS image sensor from Omnivision.
>
> Cc: Quentin Schulz <[email protected]>
Don't Cc yourself in commits. This goes to the Git history, so
assumption is that the "other you" knows that you sent it. :)
> Signed-off-by: Quentin Schulz <[email protected]>
> ---
>
> v2:
> - fixed incorrect id,
> - fixed device tree example by adding missing dt-bindings headers,
> - fixed device tree example by using vcc_1v2 for dvdd supply, as requested
> in datasheet,
>
> .../bindings/media/i2c/ovti,ov5675.yaml | 139 ++++++++++++++++++
> MAINTAINERS | 1 +
> 2 files changed, 140 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov5675.yaml
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov5675.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov5675.yaml
> new file mode 100644
> index 000000000000..29df2f82c631
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov5675.yaml
> @@ -0,0 +1,139 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright (c) 2022 Theobroma Systems Design und Consulting GmbH
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/i2c/ovti,ov5675.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Omnivision OV5675 CMOS Sensor Device Tree Bindings
s/Device Tree Bindings//
> +
> +maintainers:
> + - Quentin Schulz <[email protected]>
> +
> +description: |-
> + The Omnivision OV5675 is a high performance, 1/5-inch, 5 megapixel, CMOS
> + image sensor that delivers 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. This chip is programmable
> + through I2C and two-wire SCCB. The sensor output is available via CSI-2
> + serial data output (up to 2-lane).
> +
> +properties:
> + compatible:
> + const: ovti,ov5675
> +
> + reg:
> + maxItems: 1
> +
> + clocks:
> + maxItems: 1
> +
> + clock-names:
> + description:
> + Input clock for the sensor.
> + items:
> + - const: xvclk
Just "xv" is preferred.
> +
> + clock-frequency:
> + description:
> + Frequency of the xvclk clock in Hertz.
> +
> + dovdd-supply:
> + description:
> + Definition of the regulator used as interface power supply.
> +
> + avdd-supply:
> + description:
> + Definition of the regulator used as analog power supply.
> +
> + dvdd-supply:
> + description:
> + Definition of the regulator used as digital power supply.
> +
> + reset-gpios:
> + description:
> + The phandle and specifier for the GPIO that controls sensor reset.
> + This corresponds to the hardware pin XSHUTDOWN which is physically
> + active low.
Needs maxItems
> +
> + port:
> + type: object
Open other bindings and compare how it is done there. This looks like
/schemas/graph.yaml#/$defs/port-base
> + additionalProperties: false
> + description:
> + A node containing an output port node with an endpoint definition
> + as documented in
> + Documentation/devicetree/bindings/media/video-interfaces.txt
> +
> + properties:
> + endpoint:
> + type: object
Missing ref
> +
> + properties:
> + data-lanes:
> + description: |-
No need for "|-"
> + The driver only supports 2-lane operation.
Please remove references to driver. It's not part of hardware.
> + items:
> + - const: 1
> + - const: 2
> +
> + link-frequencies:
> + $ref: /schemas/types.yaml#/definitions/uint64-array
The ref should be already provided by video-interfaces.
> + description:
> + Allowed data bus frequencies. 450000000Hz is supported by the driver.
Again, skip driver reference. However you need to describe the number of
items.
>
Best regards,
Krzysztof
Hi Krzysztof,
On 5/4/22 16:46, Krzysztof Kozlowski wrote:
> On 04/05/2022 15:55, Quentin Schulz wrote:
>> From: Quentin Schulz <[email protected]>
>>
>> This patch adds documentation of device tree in YAML schema for the
>> OV5675 CMOS image sensor from Omnivision.
>>
>> Cc: Quentin Schulz <[email protected]>
>
> Don't Cc yourself in commits. This goes to the Git history, so
> assumption is that the "other you" knows that you sent it. :)
>
It's a habit I've taken so that in the event I switch jobs, there's
still an address where I can be reached.
But I forgot the kernel as a .mailmap :) so I can avoid doing it.
>> Signed-off-by: Quentin Schulz <[email protected]>
>> ---
>>
>> v2:
>> - fixed incorrect id,
>> - fixed device tree example by adding missing dt-bindings headers,
>> - fixed device tree example by using vcc_1v2 for dvdd supply, as requested
>> in datasheet,
>>
>> .../bindings/media/i2c/ovti,ov5675.yaml | 139 ++++++++++++++++++
>> MAINTAINERS | 1 +
>> 2 files changed, 140 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov5675.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov5675.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov5675.yaml
>> new file mode 100644
>> index 000000000000..29df2f82c631
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov5675.yaml
>> @@ -0,0 +1,139 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +# Copyright (c) 2022 Theobroma Systems Design und Consulting GmbH
>> +%YAML 1.2
>> +---
>> +$id: https://urldefense.proofpoint.com/v2/url?u=http-3A__devicetree.org_schemas_media_i2c_ovti-2Cov5675.yaml-23&d=DwICaQ&c=_sEr5x9kUWhuk4_nFwjJtA&r=LYjLexDn7rXIzVmkNPvw5ymA1XTSqHGq8yBP6m6qZZ4njZguQhZhkI_-172IIy1t&m=5gh6rUPawGp8Fw7sPXVsvOViSUSnEVGWWB9RK_CSQCeBjoEfunHaI0w2GiB0zjq_&s=W_iQiFCa2XM4k350eNawjzJR2ZTUuSx-VM4E1iuknz4&e=
>> +$schema: https://urldefense.proofpoint.com/v2/url?u=http-3A__devicetree.org_meta-2Dschemas_core.yaml-23&d=DwICaQ&c=_sEr5x9kUWhuk4_nFwjJtA&r=LYjLexDn7rXIzVmkNPvw5ymA1XTSqHGq8yBP6m6qZZ4njZguQhZhkI_-172IIy1t&m=5gh6rUPawGp8Fw7sPXVsvOViSUSnEVGWWB9RK_CSQCeBjoEfunHaI0w2GiB0zjq_&s=nrri3o0sr-6AopqJCaHZ94dUfOwS8r1V7ybfSwD7v2M&e=
>> +
>> +title: Omnivision OV5675 CMOS Sensor Device Tree Bindings
>
> s/Device Tree Bindings//
>
>> +
>> +maintainers:
>> + - Quentin Schulz <[email protected]>
>> +
>> +description: |-
>> + The Omnivision OV5675 is a high performance, 1/5-inch, 5 megapixel, CMOS
>> + image sensor that delivers 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. This chip is programmable
>> + through I2C and two-wire SCCB. The sensor output is available via CSI-2
>> + serial data output (up to 2-lane).
>> +
>> +properties:
>> + compatible:
>> + const: ovti,ov5675
>> +
>> + reg:
>> + maxItems: 1
>> +
>> + clocks:
>> + maxItems: 1
>> +
>> + clock-names:
>> + description:
>> + Input clock for the sensor.
>> + items:
>> + - const: xvclk
>
> Just "xv" is preferred.
>
The name of the clock in the datasheet is XVCLK though. Wouldn't it be
confusing to describe HW by using names different from the datasheet?
>> +
>> + clock-frequency:
>> + description:
>> + Frequency of the xvclk clock in Hertz.
>> +
>> + dovdd-supply:
>> + description:
>> + Definition of the regulator used as interface power supply.
>> +
>> + avdd-supply:
>> + description:
>> + Definition of the regulator used as analog power supply.
>> +
>> + dvdd-supply:
>> + description:
>> + Definition of the regulator used as digital power supply.
>> +
>> + reset-gpios:
>> + description:
>> + The phandle and specifier for the GPIO that controls sensor reset.
>> + This corresponds to the hardware pin XSHUTDOWN which is physically
>> + active low.
>
> Needs maxItems
>
>> +
>> + port:
>> + type: object
>
> Open other bindings and compare how it is done there. This looks like
> /schemas/graph.yaml#/$defs/port-base
>
Did that but used an old kernel as base :/
>> + additionalProperties: false
>> + description:
>> + A node containing an output port node with an endpoint definition
>> + as documented in
>> + Documentation/devicetree/bindings/media/video-interfaces.txt
>> +
>> + properties:
>> + endpoint:
>> + type: object
>
> Missing ref
>
>> +
>> + properties:
>> + data-lanes:
>> + description: |-
>
> No need for "|-"
>
>> + The driver only supports 2-lane operation.
>
> Please remove references to driver. It's not part of hardware.
>
>> + items:
>> + - const: 1
>> + - const: 2
>> +
>> + link-frequencies:
>> + $ref: /schemas/types.yaml#/definitions/uint64-array
>
> The ref should be already provided by video-interfaces.
>
>> + description:
>> + Allowed data bus frequencies. 450000000Hz is supported by the driver.
>
> Again, skip driver reference. However you need to describe the number of
> items.
>
Currently, the driver is limited to 450 MHz link-freq and 2 data lanes,
while the HW advertises: "The OV5675 supports a MIPI interface of up to
2-lanes. The MIPI interface can be configured for 1/2-lane and each lane
is capable of a data transfer rate of up to 900 Mbps."
Was wondering what I am supposed to do in this situation as I see
Documentation/devicetree/bindings/media/i2c/ov8856.yaml mentioning
driver limitations in the dt-bindings.
Cheers,
Quentin