* Re: [PATCH] media: dt-bindings: ov5645: Convert OV5645 binding to a schema
2022-09-13 16:02 [PATCH] media: dt-bindings: ov5645: Convert OV5645 binding to a schema Prabhakar
@ 2022-09-14 14:16 ` Rob Herring
2022-09-16 10:16 ` Lad, Prabhakar
2022-09-14 21:41 ` Tommaso Merciai
2022-09-16 11:52 ` Sakari Ailus
2 siblings, 1 reply; 9+ messages in thread
From: Rob Herring @ 2022-09-14 14:16 UTC (permalink / raw)
To: Prabhakar
Cc: Mauro Carvalho Chehab, Krzysztof Kozlowski, Laurent Pinchart,
Sakari Ailus, Hans Verkuil, linux-media, devicetree,
linux-kernel, Lad Prabhakar
On Tue, Sep 13, 2022 at 05:02:24PM +0100, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Convert the simple OV5645 Device Tree binding to json-schema.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> .../devicetree/bindings/media/i2c/ov5645.txt | 54 --------
> .../bindings/media/i2c/ovti,ov5645.yaml | 119 ++++++++++++++++++
> 2 files changed, 119 insertions(+), 54 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/media/i2c/ov5645.txt
> create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov5645.yaml
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/ov5645.txt b/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> deleted file mode 100644
> index 72ad992f77be..000000000000
> --- a/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> +++ /dev/null
> @@ -1,54 +0,0 @@
> -* Omnivision 1/4-Inch 5Mp CMOS Digital Image Sensor
> -
> -The Omnivision OV5645 is a 1/4-Inch CMOS active pixel digital image sensor with
> -an active array size of 2592H x 1944V. It is programmable through a serial I2C
> -interface.
> -
> -Required Properties:
> -- compatible: Value should be "ovti,ov5645".
> -- clocks: Reference to the xclk clock.
> -- clock-names: Should be "xclk".
> -- clock-frequency: Frequency of the xclk clock.
> -- enable-gpios: Chip enable GPIO. Polarity is GPIO_ACTIVE_HIGH. This corresponds
> - to the hardware pin PWDNB which is physically active low.
> -- reset-gpios: Chip reset GPIO. Polarity is GPIO_ACTIVE_LOW. This corresponds to
> - the hardware pin RESETB.
> -- vdddo-supply: Chip digital IO regulator.
> -- vdda-supply: Chip analog regulator.
> -- vddd-supply: Chip digital core regulator.
> -
> -The device node must contain one 'port' child node for its digital output
> -video port, in accordance with the video interface bindings defined in
> -Documentation/devicetree/bindings/media/video-interfaces.txt.
> -
> -Example:
> -
> - &i2c1 {
> - ...
> -
> - ov5645: ov5645@3c {
> - compatible = "ovti,ov5645";
> - reg = <0x3c>;
> -
> - enable-gpios = <&gpio1 6 GPIO_ACTIVE_HIGH>;
> - reset-gpios = <&gpio5 20 GPIO_ACTIVE_LOW>;
> - pinctrl-names = "default";
> - pinctrl-0 = <&camera_rear_default>;
> -
> - clocks = <&clks 200>;
> - clock-names = "xclk";
> - clock-frequency = <24000000>;
> -
> - vdddo-supply = <&camera_dovdd_1v8>;
> - vdda-supply = <&camera_avdd_2v8>;
> - vddd-supply = <&camera_dvdd_1v2>;
> -
> - port {
> - ov5645_ep: endpoint {
> - clock-lanes = <1>;
> - data-lanes = <0 2>;
> - remote-endpoint = <&csi0_ep>;
> - };
> - };
> - };
> - };
> diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov5645.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov5645.yaml
> new file mode 100644
> index 000000000000..7f407c988f87
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov5645.yaml
> @@ -0,0 +1,119 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/i2c/ovti,ov5645.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: OmniVision OV5645 Image Sensor Device Tree Bindings
> +
> +maintainers:
> + - Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> +
> +allOf:
> + - $ref: /schemas/media/video-interface-devices.yaml#
What are you using from here?
(Answer below)
> +
> +properties:
> + compatible:
> + const: ovti,ov5645
> +
> + reg:
> + maxItems: 1
> +
> + clocks:
> + description: XCLK Input Clock
> +
> + clock-names:
> + const: xclk
> +
> + clock-frequency:
> + description: Frequency of the xclk clock in Hz.
> +
> + vdda-supply:
> + description: Analog voltage supply, 2.8 volts
> +
> + vddd-supply:
> + description: Digital core voltage supply, 1.5 volts
> +
> + vdddo-supply:
> + description: Digital I/O voltage supply, 1.8 volts
> +
> + enable-gpios:
> + maxItems: 1
> + description:
> + Reference to the GPIO connected to the PWDNB pin, if any.
> +
> + reset-gpios:
> + maxItems: 1
> + description:
> + Reference to the GPIO connected to the RESETB pin, if any.
> +
> + port:
> + description: Digital Output Port
> + $ref: /schemas/graph.yaml#/$defs/port-base
> + additionalProperties: false
> +
> + properties:
> + endpoint:
> + $ref: /schemas/media/video-interfaces.yaml#
> + unevaluatedProperties: false
> +
> + properties:
> + clock-lanes:
> + const: 0
> +
> + bus-type:
> + const: 4
> +
> + data-lanes:
> + minItems: 1
> + maxItems: 2
> + items:
> + enum: [1, 2]
> +
> +required:
> + - compatible
> + - reg
> + - clocks
> + - clock-names
> + - clock-frequency
> + - vdda-supply
> + - vddd-supply
> + - vdddo-supply
> + - enable-gpios
> + - reset-gpios
> + - port
> +
> +additionalProperties: false
You are not because this prevents it. Either remove the ref or use
unevaluatedProperties.
Rob
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] media: dt-bindings: ov5645: Convert OV5645 binding to a schema
2022-09-14 14:16 ` Rob Herring
@ 2022-09-16 10:16 ` Lad, Prabhakar
0 siblings, 0 replies; 9+ messages in thread
From: Lad, Prabhakar @ 2022-09-16 10:16 UTC (permalink / raw)
To: Rob Herring
Cc: Mauro Carvalho Chehab, Krzysztof Kozlowski, Laurent Pinchart,
Sakari Ailus, Hans Verkuil, linux-media, devicetree,
linux-kernel, Lad Prabhakar
Hi Rob,
Thank you for the review.
On Wed, Sep 14, 2022 at 3:16 PM Rob Herring <robh@kernel.org> wrote:
>
> On Tue, Sep 13, 2022 at 05:02:24PM +0100, Prabhakar wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > Convert the simple OV5645 Device Tree binding to json-schema.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> > .../devicetree/bindings/media/i2c/ov5645.txt | 54 --------
> > .../bindings/media/i2c/ovti,ov5645.yaml | 119 ++++++++++++++++++
> > 2 files changed, 119 insertions(+), 54 deletions(-)
> > delete mode 100644 Documentation/devicetree/bindings/media/i2c/ov5645.txt
> > create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov5645.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/media/i2c/ov5645.txt b/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> > deleted file mode 100644
> > index 72ad992f77be..000000000000
> > --- a/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> > +++ /dev/null
> > @@ -1,54 +0,0 @@
> > -* Omnivision 1/4-Inch 5Mp CMOS Digital Image Sensor
> > -
> > -The Omnivision OV5645 is a 1/4-Inch CMOS active pixel digital image sensor with
> > -an active array size of 2592H x 1944V. It is programmable through a serial I2C
> > -interface.
> > -
> > -Required Properties:
> > -- compatible: Value should be "ovti,ov5645".
> > -- clocks: Reference to the xclk clock.
> > -- clock-names: Should be "xclk".
> > -- clock-frequency: Frequency of the xclk clock.
> > -- enable-gpios: Chip enable GPIO. Polarity is GPIO_ACTIVE_HIGH. This corresponds
> > - to the hardware pin PWDNB which is physically active low.
> > -- reset-gpios: Chip reset GPIO. Polarity is GPIO_ACTIVE_LOW. This corresponds to
> > - the hardware pin RESETB.
> > -- vdddo-supply: Chip digital IO regulator.
> > -- vdda-supply: Chip analog regulator.
> > -- vddd-supply: Chip digital core regulator.
> > -
> > -The device node must contain one 'port' child node for its digital output
> > -video port, in accordance with the video interface bindings defined in
> > -Documentation/devicetree/bindings/media/video-interfaces.txt.
> > -
> > -Example:
> > -
> > - &i2c1 {
> > - ...
> > -
> > - ov5645: ov5645@3c {
> > - compatible = "ovti,ov5645";
> > - reg = <0x3c>;
> > -
> > - enable-gpios = <&gpio1 6 GPIO_ACTIVE_HIGH>;
> > - reset-gpios = <&gpio5 20 GPIO_ACTIVE_LOW>;
> > - pinctrl-names = "default";
> > - pinctrl-0 = <&camera_rear_default>;
> > -
> > - clocks = <&clks 200>;
> > - clock-names = "xclk";
> > - clock-frequency = <24000000>;
> > -
> > - vdddo-supply = <&camera_dovdd_1v8>;
> > - vdda-supply = <&camera_avdd_2v8>;
> > - vddd-supply = <&camera_dvdd_1v2>;
> > -
> > - port {
> > - ov5645_ep: endpoint {
> > - clock-lanes = <1>;
> > - data-lanes = <0 2>;
> > - remote-endpoint = <&csi0_ep>;
> > - };
> > - };
> > - };
> > - };
> > diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov5645.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov5645.yaml
> > new file mode 100644
> > index 000000000000..7f407c988f87
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov5645.yaml
> > @@ -0,0 +1,119 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/media/i2c/ovti,ov5645.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: OmniVision OV5645 Image Sensor Device Tree Bindings
> > +
> > +maintainers:
> > + - Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > +
> > +allOf:
> > + - $ref: /schemas/media/video-interface-devices.yaml#
>
> What are you using from here?
>
> (Answer below)
>
> > +
> > +properties:
<snip>
> > + - port
> > +
> > +additionalProperties: false
>
> You are not because this prevents it. Either remove the ref or use
> unevaluatedProperties.
>
I'll drop the ref while sending the v2.
Cheers,
Prabhakar
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] media: dt-bindings: ov5645: Convert OV5645 binding to a schema
2022-09-13 16:02 [PATCH] media: dt-bindings: ov5645: Convert OV5645 binding to a schema Prabhakar
2022-09-14 14:16 ` Rob Herring
@ 2022-09-14 21:41 ` Tommaso Merciai
2022-09-16 10:24 ` Lad, Prabhakar
2022-09-16 11:52 ` Sakari Ailus
2 siblings, 1 reply; 9+ messages in thread
From: Tommaso Merciai @ 2022-09-14 21:41 UTC (permalink / raw)
To: Prabhakar
Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Laurent Pinchart, Sakari Ailus, Hans Verkuil, linux-media,
devicetree, linux-kernel, Lad Prabhakar
Hi Prabhakar,
On Tue, Sep 13, 2022 at 05:02:24PM +0100, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Convert the simple OV5645 Device Tree binding to json-schema.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> .../devicetree/bindings/media/i2c/ov5645.txt | 54 --------
> .../bindings/media/i2c/ovti,ov5645.yaml | 119 ++++++++++++++++++
> 2 files changed, 119 insertions(+), 54 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/media/i2c/ov5645.txt
> create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov5645.yaml
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/ov5645.txt b/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> deleted file mode 100644
> index 72ad992f77be..000000000000
> --- a/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> +++ /dev/null
> @@ -1,54 +0,0 @@
> -* Omnivision 1/4-Inch 5Mp CMOS Digital Image Sensor
> -
> -The Omnivision OV5645 is a 1/4-Inch CMOS active pixel digital image sensor with
> -an active array size of 2592H x 1944V. It is programmable through a serial I2C
> -interface.
> -
> -Required Properties:
> -- compatible: Value should be "ovti,ov5645".
> -- clocks: Reference to the xclk clock.
> -- clock-names: Should be "xclk".
> -- clock-frequency: Frequency of the xclk clock.
> -- enable-gpios: Chip enable GPIO. Polarity is GPIO_ACTIVE_HIGH. This corresponds
> - to the hardware pin PWDNB which is physically active low.
> -- reset-gpios: Chip reset GPIO. Polarity is GPIO_ACTIVE_LOW. This corresponds to
> - the hardware pin RESETB.
> -- vdddo-supply: Chip digital IO regulator.
> -- vdda-supply: Chip analog regulator.
> -- vddd-supply: Chip digital core regulator.
> -
> -The device node must contain one 'port' child node for its digital output
> -video port, in accordance with the video interface bindings defined in
> -Documentation/devicetree/bindings/media/video-interfaces.txt.
> -
> -Example:
> -
> - &i2c1 {
> - ...
> -
> - ov5645: ov5645@3c {
> - compatible = "ovti,ov5645";
> - reg = <0x3c>;
> -
> - enable-gpios = <&gpio1 6 GPIO_ACTIVE_HIGH>;
> - reset-gpios = <&gpio5 20 GPIO_ACTIVE_LOW>;
> - pinctrl-names = "default";
> - pinctrl-0 = <&camera_rear_default>;
> -
> - clocks = <&clks 200>;
> - clock-names = "xclk";
> - clock-frequency = <24000000>;
> -
> - vdddo-supply = <&camera_dovdd_1v8>;
> - vdda-supply = <&camera_avdd_2v8>;
> - vddd-supply = <&camera_dvdd_1v2>;
> -
> - port {
> - ov5645_ep: endpoint {
> - clock-lanes = <1>;
> - data-lanes = <0 2>;
> - remote-endpoint = <&csi0_ep>;
> - };
> - };
> - };
> - };
> diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov5645.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov5645.yaml
> new file mode 100644
> index 000000000000..7f407c988f87
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov5645.yaml
> @@ -0,0 +1,119 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/i2c/ovti,ov5645.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: OmniVision OV5645 Image Sensor Device Tree Bindings
> +
> +maintainers:
> + - Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> +
> +allOf:
> + - $ref: /schemas/media/video-interface-devices.yaml#
> +
> +properties:
> + compatible:
> + const: ovti,ov5645
> +
> + reg:
> + maxItems: 1
> +
> + clocks:
> + description: XCLK Input Clock
> +
> + clock-names:
> + const: xclk
> +
> + clock-frequency:
> + description: Frequency of the xclk clock in Hz.
> +
> + vdda-supply:
> + description: Analog voltage supply, 2.8 volts
> +
> + vddd-supply:
> + description: Digital core voltage supply, 1.5 volts
> +
> + vdddo-supply:
> + description: Digital I/O voltage supply, 1.8 volts
> +
> + enable-gpios:
> + maxItems: 1
> + description:
> + Reference to the GPIO connected to the PWDNB pin, if any.
> +
> + reset-gpios:
> + maxItems: 1
> + description:
> + Reference to the GPIO connected to the RESETB pin, if any.
> +
> + port:
> + description: Digital Output Port
> + $ref: /schemas/graph.yaml#/$defs/port-base
> + additionalProperties: false
> +
> + properties:
> + endpoint:
> + $ref: /schemas/media/video-interfaces.yaml#
> + unevaluatedProperties: false
> +
> + properties:
> + clock-lanes:
> + const: 0
> +
> + bus-type:
> + const: 4
> +
> + data-lanes:
> + minItems: 1
> + maxItems: 2
> + items:
> + enum: [1, 2]
> +
> +required:
> + - compatible
> + - reg
> + - clocks
> + - clock-names
> + - clock-frequency
> + - vdda-supply
> + - vddd-supply
> + - vdddo-supply
> + - enable-gpios
> + - reset-gpios
> + - port
I think we don't need all of these properties as required.
The only "really" required are:
- compatible
- reg
- clocks
- port
Regards,
Tommaso
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/gpio/gpio.h>
> +
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + camera@3c {
> + compatible = "ovti,ov5645";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_ov5645>;
> + reg = <0x3c>;
> + clocks = <&clks 1>;
> + clock-names = "xclk";
> + clock-frequency = <24000000>;
> + vdddo-supply = <&ov5645_vdddo_1v8>; /* 1.8v */
> + vdda-supply = <&ov5645_vdda_2v8>; /* 2.8v */
> + vddd-supply = <&ov5645_vddd_1v5>; /* 1.5v */
> + enable-gpios = <&gpio1 19 GPIO_ACTIVE_HIGH>;
> + reset-gpios = <&gpio1 20 GPIO_ACTIVE_LOW>;
> +
> + port {
> + ov5645_ep: endpoint {
> + remote-endpoint = <&csi0_ep>;
> + clock-lanes = <0>;
> + data-lanes = <1 2>;
> + };
> + };
> + };
> + };
> +...
> --
> 2.25.1
>
--
Tommaso Merciai
Embedded Linux Engineer
tommaso.merciai@amarulasolutions.com
__________________________________
Amarula Solutions SRL
Via Le Canevare 30, 31100 Treviso, Veneto, IT
T. +39 042 243 5310
info@amarulasolutions.com
www.amarulasolutions.com
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] media: dt-bindings: ov5645: Convert OV5645 binding to a schema
2022-09-14 21:41 ` Tommaso Merciai
@ 2022-09-16 10:24 ` Lad, Prabhakar
2022-09-16 10:28 ` Krzysztof Kozlowski
0 siblings, 1 reply; 9+ messages in thread
From: Lad, Prabhakar @ 2022-09-16 10:24 UTC (permalink / raw)
To: Tommaso Merciai
Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Laurent Pinchart, Sakari Ailus, Hans Verkuil, linux-media,
devicetree, linux-kernel, Lad Prabhakar
Hi Tommaso,
Thank you for the review.
On Wed, Sep 14, 2022 at 10:41 PM Tommaso Merciai
<tommaso.merciai@amarulasolutions.com> wrote:
>
> Hi Prabhakar,
>
> On Tue, Sep 13, 2022 at 05:02:24PM +0100, Prabhakar wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > Convert the simple OV5645 Device Tree binding to json-schema.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> > .../devicetree/bindings/media/i2c/ov5645.txt | 54 --------
> > .../bindings/media/i2c/ovti,ov5645.yaml | 119 ++++++++++++++++++
> > 2 files changed, 119 insertions(+), 54 deletions(-)
> > delete mode 100644 Documentation/devicetree/bindings/media/i2c/ov5645.txt
> > create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov5645.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/media/i2c/ov5645.txt b/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> > deleted file mode 100644
> > index 72ad992f77be..000000000000
> > --- a/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> > +++ /dev/null
> > @@ -1,54 +0,0 @@
> > -* Omnivision 1/4-Inch 5Mp CMOS Digital Image Sensor
> > -
> > -The Omnivision OV5645 is a 1/4-Inch CMOS active pixel digital image sensor with
> > -an active array size of 2592H x 1944V. It is programmable through a serial I2C
> > -interface.
> > -
> > -Required Properties:
> > -- compatible: Value should be "ovti,ov5645".
> > -- clocks: Reference to the xclk clock.
> > -- clock-names: Should be "xclk".
> > -- clock-frequency: Frequency of the xclk clock.
> > -- enable-gpios: Chip enable GPIO. Polarity is GPIO_ACTIVE_HIGH. This corresponds
> > - to the hardware pin PWDNB which is physically active low.
> > -- reset-gpios: Chip reset GPIO. Polarity is GPIO_ACTIVE_LOW. This corresponds to
> > - the hardware pin RESETB.
> > -- vdddo-supply: Chip digital IO regulator.
> > -- vdda-supply: Chip analog regulator.
> > -- vddd-supply: Chip digital core regulator.
> > -
> > -The device node must contain one 'port' child node for its digital output
> > -video port, in accordance with the video interface bindings defined in
> > -Documentation/devicetree/bindings/media/video-interfaces.txt.
> > -
> > -Example:
> > -
> > - &i2c1 {
> > - ...
> > -
> > - ov5645: ov5645@3c {
> > - compatible = "ovti,ov5645";
> > - reg = <0x3c>;
> > -
> > - enable-gpios = <&gpio1 6 GPIO_ACTIVE_HIGH>;
> > - reset-gpios = <&gpio5 20 GPIO_ACTIVE_LOW>;
> > - pinctrl-names = "default";
> > - pinctrl-0 = <&camera_rear_default>;
> > -
> > - clocks = <&clks 200>;
> > - clock-names = "xclk";
> > - clock-frequency = <24000000>;
> > -
> > - vdddo-supply = <&camera_dovdd_1v8>;
> > - vdda-supply = <&camera_avdd_2v8>;
> > - vddd-supply = <&camera_dvdd_1v2>;
> > -
> > - port {
> > - ov5645_ep: endpoint {
> > - clock-lanes = <1>;
> > - data-lanes = <0 2>;
> > - remote-endpoint = <&csi0_ep>;
> > - };
> > - };
> > - };
> > - };
> > diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov5645.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov5645.yaml
> > new file mode 100644
> > index 000000000000..7f407c988f87
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov5645.yaml
> > @@ -0,0 +1,119 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/media/i2c/ovti,ov5645.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: OmniVision OV5645 Image Sensor Device Tree Bindings
> > +
> > +maintainers:
> > + - Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > +
> > +allOf:
> > + - $ref: /schemas/media/video-interface-devices.yaml#
> > +
> > +properties:
> > + compatible:
> > + const: ovti,ov5645
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + clocks:
> > + description: XCLK Input Clock
> > +
> > + clock-names:
> > + const: xclk
> > +
> > + clock-frequency:
> > + description: Frequency of the xclk clock in Hz.
> > +
> > + vdda-supply:
> > + description: Analog voltage supply, 2.8 volts
> > +
> > + vddd-supply:
> > + description: Digital core voltage supply, 1.5 volts
> > +
> > + vdddo-supply:
> > + description: Digital I/O voltage supply, 1.8 volts
> > +
> > + enable-gpios:
> > + maxItems: 1
> > + description:
> > + Reference to the GPIO connected to the PWDNB pin, if any.
> > +
> > + reset-gpios:
> > + maxItems: 1
> > + description:
> > + Reference to the GPIO connected to the RESETB pin, if any.
> > +
> > + port:
> > + description: Digital Output Port
> > + $ref: /schemas/graph.yaml#/$defs/port-base
> > + additionalProperties: false
> > +
> > + properties:
> > + endpoint:
> > + $ref: /schemas/media/video-interfaces.yaml#
> > + unevaluatedProperties: false
> > +
> > + properties:
> > + clock-lanes:
> > + const: 0
> > +
> > + bus-type:
> > + const: 4
> > +
> > + data-lanes:
> > + minItems: 1
> > + maxItems: 2
> > + items:
> > + enum: [1, 2]
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - clocks
> > + - clock-names
> > + - clock-frequency
> > + - vdda-supply
> > + - vddd-supply
> > + - vdddo-supply
> > + - enable-gpios
> > + - reset-gpios
> > + - port
>
> I think we don't need all of these properties as required.
> The only "really" required are:
>
> - compatible
> - reg
> - clocks
> - port
>
Agreed, it's just the driver requirement hence the previous bindings
had marked them as required.
Cheers,
Prabhakar
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] media: dt-bindings: ov5645: Convert OV5645 binding to a schema
2022-09-16 10:24 ` Lad, Prabhakar
@ 2022-09-16 10:28 ` Krzysztof Kozlowski
2022-09-16 10:40 ` Lad, Prabhakar
0 siblings, 1 reply; 9+ messages in thread
From: Krzysztof Kozlowski @ 2022-09-16 10:28 UTC (permalink / raw)
To: Lad, Prabhakar, Tommaso Merciai
Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Laurent Pinchart, Sakari Ailus, Hans Verkuil, linux-media,
devicetree, linux-kernel, Lad Prabhakar
On 16/09/2022 11:24, Lad, Prabhakar wrote:
>>> +required:
>>> + - compatible
>>> + - reg
>>> + - clocks
>>> + - clock-names
>>> + - clock-frequency
>>> + - vdda-supply
>>> + - vddd-supply
>>> + - vdddo-supply
>>> + - enable-gpios
>>> + - reset-gpios
>>> + - port
>>
>> I think we don't need all of these properties as required.
>> The only "really" required are:
>>
>> - compatible
>> - reg
>> - clocks
>> - port
>>
> Agreed, it's just the driver requirement hence the previous bindings
> had marked them as required.
Please mention the change from pure conversion (with reason) in the
commit msg.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] media: dt-bindings: ov5645: Convert OV5645 binding to a schema
2022-09-16 10:28 ` Krzysztof Kozlowski
@ 2022-09-16 10:40 ` Lad, Prabhakar
0 siblings, 0 replies; 9+ messages in thread
From: Lad, Prabhakar @ 2022-09-16 10:40 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Tommaso Merciai, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Laurent Pinchart, Sakari Ailus,
Hans Verkuil, linux-media, devicetree, linux-kernel,
Lad Prabhakar
On Fri, Sep 16, 2022 at 11:28 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 16/09/2022 11:24, Lad, Prabhakar wrote:
> >>> +required:
> >>> + - compatible
> >>> + - reg
> >>> + - clocks
> >>> + - clock-names
> >>> + - clock-frequency
> >>> + - vdda-supply
> >>> + - vddd-supply
> >>> + - vdddo-supply
> >>> + - enable-gpios
> >>> + - reset-gpios
> >>> + - port
> >>
> >> I think we don't need all of these properties as required.
> >> The only "really" required are:
> >>
> >> - compatible
> >> - reg
> >> - clocks
> >> - port
> >>
> > Agreed, it's just the driver requirement hence the previous bindings
> > had marked them as required.
>
> Please mention the change from pure conversion (with reason) in the
> commit msg.
>
Sure will do.
Cheers,
Prabhakar
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] media: dt-bindings: ov5645: Convert OV5645 binding to a schema
2022-09-13 16:02 [PATCH] media: dt-bindings: ov5645: Convert OV5645 binding to a schema Prabhakar
2022-09-14 14:16 ` Rob Herring
2022-09-14 21:41 ` Tommaso Merciai
@ 2022-09-16 11:52 ` Sakari Ailus
2022-09-16 12:10 ` Lad, Prabhakar
2 siblings, 1 reply; 9+ messages in thread
From: Sakari Ailus @ 2022-09-16 11:52 UTC (permalink / raw)
To: Prabhakar
Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Laurent Pinchart, Hans Verkuil, linux-media, devicetree,
linux-kernel, Lad Prabhakar
Hi Prabhakar,
On Tue, Sep 13, 2022 at 05:02:24PM +0100, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Convert the simple OV5645 Device Tree binding to json-schema.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> .../devicetree/bindings/media/i2c/ov5645.txt | 54 --------
> .../bindings/media/i2c/ovti,ov5645.yaml | 119 ++++++++++++++++++
> 2 files changed, 119 insertions(+), 54 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/media/i2c/ov5645.txt
> create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov5645.yaml
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/ov5645.txt b/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> deleted file mode 100644
> index 72ad992f77be..000000000000
> --- a/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> +++ /dev/null
> @@ -1,54 +0,0 @@
> -* Omnivision 1/4-Inch 5Mp CMOS Digital Image Sensor
> -
> -The Omnivision OV5645 is a 1/4-Inch CMOS active pixel digital image sensor with
> -an active array size of 2592H x 1944V. It is programmable through a serial I2C
> -interface.
> -
> -Required Properties:
> -- compatible: Value should be "ovti,ov5645".
> -- clocks: Reference to the xclk clock.
> -- clock-names: Should be "xclk".
> -- clock-frequency: Frequency of the xclk clock.
> -- enable-gpios: Chip enable GPIO. Polarity is GPIO_ACTIVE_HIGH. This corresponds
> - to the hardware pin PWDNB which is physically active low.
> -- reset-gpios: Chip reset GPIO. Polarity is GPIO_ACTIVE_LOW. This corresponds to
> - the hardware pin RESETB.
> -- vdddo-supply: Chip digital IO regulator.
> -- vdda-supply: Chip analog regulator.
> -- vddd-supply: Chip digital core regulator.
> -
> -The device node must contain one 'port' child node for its digital output
> -video port, in accordance with the video interface bindings defined in
> -Documentation/devicetree/bindings/media/video-interfaces.txt.
> -
> -Example:
> -
> - &i2c1 {
> - ...
> -
> - ov5645: ov5645@3c {
> - compatible = "ovti,ov5645";
> - reg = <0x3c>;
> -
> - enable-gpios = <&gpio1 6 GPIO_ACTIVE_HIGH>;
> - reset-gpios = <&gpio5 20 GPIO_ACTIVE_LOW>;
> - pinctrl-names = "default";
> - pinctrl-0 = <&camera_rear_default>;
> -
> - clocks = <&clks 200>;
> - clock-names = "xclk";
> - clock-frequency = <24000000>;
> -
> - vdddo-supply = <&camera_dovdd_1v8>;
> - vdda-supply = <&camera_avdd_2v8>;
> - vddd-supply = <&camera_dvdd_1v2>;
> -
> - port {
> - ov5645_ep: endpoint {
> - clock-lanes = <1>;
> - data-lanes = <0 2>;
> - remote-endpoint = <&csi0_ep>;
> - };
> - };
> - };
> - };
> diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov5645.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov5645.yaml
> new file mode 100644
> index 000000000000..7f407c988f87
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov5645.yaml
> @@ -0,0 +1,119 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/i2c/ovti,ov5645.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: OmniVision OV5645 Image Sensor Device Tree Bindings
> +
> +maintainers:
> + - Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> +
> +allOf:
> + - $ref: /schemas/media/video-interface-devices.yaml#
> +
> +properties:
> + compatible:
> + const: ovti,ov5645
> +
> + reg:
> + maxItems: 1
> +
> + clocks:
> + description: XCLK Input Clock
> +
> + clock-names:
> + const: xclk
> +
> + clock-frequency:
> + description: Frequency of the xclk clock in Hz.
> +
> + vdda-supply:
> + description: Analog voltage supply, 2.8 volts
> +
> + vddd-supply:
> + description: Digital core voltage supply, 1.5 volts
> +
> + vdddo-supply:
> + description: Digital I/O voltage supply, 1.8 volts
> +
> + enable-gpios:
> + maxItems: 1
> + description:
> + Reference to the GPIO connected to the PWDNB pin, if any.
> +
> + reset-gpios:
> + maxItems: 1
> + description:
> + Reference to the GPIO connected to the RESETB pin, if any.
> +
> + port:
> + description: Digital Output Port
> + $ref: /schemas/graph.yaml#/$defs/port-base
> + additionalProperties: false
> +
> + properties:
> + endpoint:
> + $ref: /schemas/media/video-interfaces.yaml#
> + unevaluatedProperties: false
> +
> + properties:
> + clock-lanes:
> + const: 0
> +
> + bus-type:
> + const: 4
Please drop the two, they provide no information to the driver that already
knows this. (Unless of course, the driver supports other bus types these
bindings do not document. Some OV sensors do.) clock-lanes needs to go in
any case, and data-lanes are dependent on bus-type being 4 I presume.
bus-type is also not present in the example.
> +
> + data-lanes:
> + minItems: 1
> + maxItems: 2
> + items:
> + enum: [1, 2]
> +
> +required:
> + - compatible
> + - reg
> + - clocks
> + - clock-names
> + - clock-frequency
> + - vdda-supply
> + - vddd-supply
> + - vdddo-supply
> + - enable-gpios
> + - reset-gpios
> + - port
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/gpio/gpio.h>
> +
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + camera@3c {
> + compatible = "ovti,ov5645";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_ov5645>;
> + reg = <0x3c>;
> + clocks = <&clks 1>;
> + clock-names = "xclk";
> + clock-frequency = <24000000>;
> + vdddo-supply = <&ov5645_vdddo_1v8>; /* 1.8v */
> + vdda-supply = <&ov5645_vdda_2v8>; /* 2.8v */
> + vddd-supply = <&ov5645_vddd_1v5>; /* 1.5v */
> + enable-gpios = <&gpio1 19 GPIO_ACTIVE_HIGH>;
> + reset-gpios = <&gpio1 20 GPIO_ACTIVE_LOW>;
> +
> + port {
> + ov5645_ep: endpoint {
> + remote-endpoint = <&csi0_ep>;
> + clock-lanes = <0>;
> + data-lanes = <1 2>;
> + };
> + };
> + };
> + };
> +...
--
Kind regards,
Sakari Ailus
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] media: dt-bindings: ov5645: Convert OV5645 binding to a schema
2022-09-16 11:52 ` Sakari Ailus
@ 2022-09-16 12:10 ` Lad, Prabhakar
0 siblings, 0 replies; 9+ messages in thread
From: Lad, Prabhakar @ 2022-09-16 12:10 UTC (permalink / raw)
To: Sakari Ailus
Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Laurent Pinchart, Hans Verkuil, linux-media, devicetree,
linux-kernel, Lad Prabhakar
Hi Sakari,
Thank you for the review.
On Fri, Sep 16, 2022 at 12:52 PM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
>
> Hi Prabhakar,
>
> On Tue, Sep 13, 2022 at 05:02:24PM +0100, Prabhakar wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > Convert the simple OV5645 Device Tree binding to json-schema.
<snip>
> > + $ref: /schemas/media/video-interfaces.yaml#
> > + unevaluatedProperties: false
> > +
> > + properties:
> > + clock-lanes:
> > + const: 0
> > +
> > + bus-type:
> > + const: 4
>
> Please drop the two, they provide no information to the driver that already
> knows this. (Unless of course, the driver supports other bus types these
> bindings do not document. Some OV sensors do.) clock-lanes needs to go in
> any case, and data-lanes are dependent on bus-type being 4 I presume.
>
Agreed, I will drop the above two and send a v2.
> bus-type is also not present in the example.
>
Ahh, I missed that.
Cheers,
Prabhakar
^ permalink raw reply [flat|nested] 9+ messages in thread