Re: [PATCH v10 1/3] media: dt-bindings: ov8856: Document YAML bindings
From: Dongchun Zhu
Date: Tue May 05 2020 - 07:49:16 EST
Hi Robert,
Thanks for the update.
On Tue, 2020-05-05 at 12:01 +0200, Robert Foss wrote:
> From: Dongchun Zhu <dongchun.zhu@xxxxxxxxxxxx>
>
> This patch adds documentation of device tree in YAML schema for the
> OV8856 CMOS image sensor.
>
> Signed-off-by: Dongchun Zhu <dongchun.zhu@xxxxxxxxxxxx>
> Signed-off-by: Robert Foss <robert.foss@xxxxxxxxxx>
> Reviewed-by: Maxime Ripard <mripard@xxxxxxxxxx>
> ---
>
> - Changes since v9:
> * Remove remote-endpoint property
> * Marco: Make port property required again
> * Sakari: Remove Ben as a maintainer
> * Sakari: Replace data-lanes with const items
> * Sakari: Remove clock-lanes property
> * Sakari & Rob Herring: Change type of link-frequency
> work around dt-schema bug
>
> - Changes since v8:
> * Maxime: Added r-b
>
> - Changes since v7:
> * Marco: Make 'port' property optional
> * Maxime & Sakari: Add 'link-frequencies' property to dt binding
> * robher: Improve description for 'port' property
>
> - Changes since v6:
> * Marco: remove qcom specifics from DT example
>
> - Changes since v5:
> * Add assigned-clocks and assigned-clock-rates
> * robher: dt-schema errors
>
> - Changes since v4:
> * Fabio: Change reset-gpio to GPIO_ACTIVE_LOW, explain in description
> * Add clock-lanes property to example
> * robher: Fix syntax error in devicetree example
>
> - Changes since v3:
> * robher: Fix syntax error
> * robher: Removed maxItems
> * Fixes yaml 'make dt-binding-check' errors
>
> - Changes since v2:
>
> - Changes since v1:
> Fixes comments from Sakari, Tomasz
> * Add clock-frequency and link-frequencies in DT
>
> .../devicetree/bindings/media/i2c/ov8856.yaml | 142 ++++++++++++++++++
> MAINTAINERS | 1 +
> 2 files changed, 143 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/i2c/ov8856.yaml
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/ov8856.yaml b/Documentation/devicetree/bindings/media/i2c/ov8856.yaml
> new file mode 100644
> index 000000000000..d6af685ad3ca
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/ov8856.yaml
> @@ -0,0 +1,142 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright (c) 2019 MediaTek Inc.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/i2c/ov8856.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Omnivision OV8856 CMOS Sensor Device Tree Bindings
> +
> +maintainers:
> + - Dongchun Zhu <dongchun.zhu@xxxxxxxxxxxx>
> +
> +description: |-
> + The Omnivision OV8856 is a high performance, 1/4-inch, 8 megapixel, CMOS
> + image sensor that delivers 3264x2448 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 4-lane).
> +
> +properties:
> + compatible:
> + const: ovti,ov8856
> +
> + reg:
> + maxItems: 1
> +
> + clocks:
> + maxItems: 1
> +
> + clock-names:
> + description:
> + Input clock for the sensor.
> + items:
> + - const: xvclk
> +
> + 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.
> +
> + port:
> + type: object
> + 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
> +
> + properties:
> + data-lanes:
> + description: |-
> + The driver only supports four-lane operation.
> + items:
> + - const: 1
> + - const: 2
> + - const: 3
> + - const: 4
> +
> + link-frequencies:
> + allOf:
> + - $ref: /schemas/types.yaml#/definitions/uint64-array
> + description:
> + Allowed data bus frequencies. 360000000, 180000000 Hz or both
> + are supported by the driver.
> +
> +
> + required:
> + - link-frequencies
> +
> + required:
> + - endpoint
> +
> +required:
> + - compatible
> + - reg
> + - clocks
> + - clock-names
> + - clock-frequency
> + - dovdd-supply
> + - avdd-supply
> + - dvdd-supply
> + - reset-gpios
> + - port
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/gpio/gpio.h>
> +
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + ov8856: camera@10 {
> + compatible = "ovti,ov8856";
> + reg = <0x10>;
> +
> + reset-gpios = <&pio 111 GPIO_ACTIVE_LOW>;
Apologies for missing to follow the earlier discussion related to this.
I noticed the GPIO flag para and __ov8856_power_on() are aligned using
ACTIVE_LOW.
But from the datasheet, XSHUTDN pin is active-high for OV8856.
It means devm_gpiod_get API (in probe func) should use GPIOD_OUT_LOW to
initialize the GPIO as output with a value of 0.
Otherwise it should use GPIOD_OUT_HIGH.
There is one case for GPIO_ACTIVE_LOW setting:
https://patchwork.linuxtv.org/patch/63460/
https://patchwork.linuxtv.org/patch/63461/
Sakari, Tomasz, am I right?
> + pinctrl-names = "default";
> + pinctrl-0 = <&clk_24m_cam>;
> +
> + clocks = <&cam_osc>;
> + clock-names = "xvclk";
> + clock-frequency = <19200000>;
> +
> + avdd-supply = <&mt6358_vcama2_reg>;
> + dvdd-supply = <&mt6358_vcamd_reg>;
> + dovdd-supply = <&mt6358_vcamio_reg>;
> +
> + port {
> + wcam_out: endpoint {
> + remote-endpoint = <&mipi_in_wcam>;
> + data-lanes = <1 2 3 4>;
> + link-frequencies = /bits/ 64 <360000000>;
> + };
> + };
> + };
> + };
> +...
> \ No newline at end of file
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 26f281d9f32a..84b262afd13d 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -12489,6 +12489,7 @@ L: linux-media@xxxxxxxxxxxxxxx
> S: Maintained
> T: git git://linuxtv.org/media_tree.git
> F: drivers/media/i2c/ov8856.c
> +F: Documentation/devicetree/bindings/media/i2c/ov8856.yaml
>
Had you run parse-maintainers.pl?
The new item is supposed to be arranged in alphabetical order.
> OMNIVISION OV9640 SENSOR DRIVER
> M: Petr Cvek <petrcvekcz@xxxxxxxxx>