| Message ID | 20220913160224.14951-1-prabhakar.mahadev-lad.rj@bp.renesas.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | media: dt-bindings: ov5645: Convert OV5645 binding to a schema | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | warning | total: 0 errors, 2 warnings, 119 lines checked |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success | |
| robh/checkpatch | warning | total: 0 errors, 2 warnings, 119 lines checked |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
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
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 >
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
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
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
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
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>; > + }; > + }; > + }; > + }; > +...
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
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 + +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>; + }; + }; + }; + }; +...