| Message ID | 20220630134835.592521-6-tommaso.merciai@amarulasolutions.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v5,1/6] media: ov5693: count num_supplies using array_size | expand |
Hi Tommaso, Krzysztof, This has been reviewed by Krzysztof already, so I guess it's fine, but let me ask anyway On Thu, Jun 30, 2022 at 03:48:34PM +0200, Tommaso Merciai wrote: > Add documentation of device tree in YAML schema for the OV5693 > CMOS image sensor from Omnivision > > Signed-off-by: Tommaso Merciai <tommaso.merciai@amarulasolutions.com> > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > --- > Changes since v1: > - Fix allOf position as suggested by Krzysztof > - Remove port description as suggested by Krzysztof > - Fix EOF as suggested by Krzysztof > > Changes since v2: > - Fix commit body as suggested by Krzysztof > > Changes since v3: > - Add reviewed-by tags, suggested by Jacopo, Krzysztof > > Changes since v4: > - Remove wrong Sakari reviewed-by tag, suggested by Krzysztof, Sakari > > .../bindings/media/i2c/ovti,ov5693.yaml | 106 ++++++++++++++++++ > MAINTAINERS | 1 + > 2 files changed, 107 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml > > diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml > new file mode 100644 > index 000000000000..b83c9fc04023 > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml > @@ -0,0 +1,106 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +# Copyright (c) 2022 Amarulasolutions > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/media/i2c/ovti,ov5693.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Omnivision OV5693 CMOS Sensor > + > +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, > + 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). > + > +allOf: > + - $ref: /schemas/media/video-interface-devices.yaml# > + > +properties: > + compatible: > + const: ovti,ov5693 > + > + reg: > + maxItems: 1 > + > + clocks: > + description: > + System input clock (aka XVCLK). From 6 to 27 MHz. > + maxItems: 1 > + > + dovdd-supply: > + description: > + Digital I/O voltage supply, 1.8V. > + > + avdd-supply: > + description: > + Analog voltage supply, 2.8V. > + > + dvdd-supply: > + description: > + Digital core voltage supply, 1.2V. > + > + reset-gpios: > + description: > + The phandle and specifier for the GPIO that controls sensor reset. > + This corresponds to the hardware pin XSHUTDN which is physically > + active low. > + maxItems: 1 > + > +required: > + - compatible > + - reg > + - clocks > + - dovdd-supply > + - avdd-supply > + - dvdd-supply Should supplies be made mandatory ? Sensors are often powered by fixed rails. Do we want DTS writers to create "fixed-regulators" for all of them ? The fact the regulator framework creates dummies if there's no entry in .dts for a regulator makes me think it's fine to have them optional, but I understand how Linux works should not be an indication of how a bindings should look like. > + - port > + > +unevaluatedProperties: false > + > +examples: > + - | > + #include <dt-bindings/clock/px30-cru.h> > + #include <dt-bindings/gpio/gpio.h> > + #include <dt-bindings/pinctrl/rockchip.h> > + > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + > + ov5693: camera@36 { > + compatible = "ovti,ov5693"; > + reg = <0x36>; > + > + reset-gpios = <&gpio2 RK_PB1 GPIO_ACTIVE_LOW>; > + pinctrl-names = "default"; > + pinctrl-0 = <&cif_clkout_m0>; > + > + clocks = <&cru SCLK_CIF_OUT>; > + assigned-clocks = <&cru SCLK_CIF_OUT>; > + assigned-clock-rates = <19200000>; > + > + avdd-supply = <&vcc_1v8>; > + dvdd-supply = <&vcc_1v2>; > + dovdd-supply = <&vcc_2v8>; > + > + rotation = <90>; > + orientation = <0>; > + > + port { > + ucam_out: endpoint { > + remote-endpoint = <&mipi_in_ucam>; > + data-lanes = <1 2>; > + link-frequencies = /bits/ 64 <450000000>; > + }; > + }; > + }; > + }; > + > +... > diff --git a/MAINTAINERS b/MAINTAINERS > index 1fc9ead83d2a..844307cb20c4 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -14719,6 +14719,7 @@ M: Daniel Scally <djrscally@gmail.com> > L: linux-media@vger.kernel.org > S: Maintained > T: git git://linuxtv.org/media_tree.git > +F: Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml > F: drivers/media/i2c/ov5693.c > > OMNIVISION OV5695 SENSOR DRIVER > -- > 2.25.1 >
Hi Jacopo, Thanks for your review. On Mon, Jul 11, 2022 at 11:36:59AM +0200, Jacopo Mondi wrote: > Hi Tommaso, Krzysztof, > > This has been reviewed by Krzysztof already, so I guess it's fine, > but let me ask anyway > > On Thu, Jun 30, 2022 at 03:48:34PM +0200, Tommaso Merciai wrote: > > Add documentation of device tree in YAML schema for the OV5693 > > CMOS image sensor from Omnivision > > > > Signed-off-by: Tommaso Merciai <tommaso.merciai@amarulasolutions.com> > > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > > --- > > Changes since v1: > > - Fix allOf position as suggested by Krzysztof > > - Remove port description as suggested by Krzysztof > > - Fix EOF as suggested by Krzysztof > > > > Changes since v2: > > - Fix commit body as suggested by Krzysztof > > > > Changes since v3: > > - Add reviewed-by tags, suggested by Jacopo, Krzysztof > > > > Changes since v4: > > - Remove wrong Sakari reviewed-by tag, suggested by Krzysztof, Sakari > > > > .../bindings/media/i2c/ovti,ov5693.yaml | 106 ++++++++++++++++++ > > MAINTAINERS | 1 + > > 2 files changed, 107 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml > > > > diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml > > new file mode 100644 > > index 000000000000..b83c9fc04023 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml > > @@ -0,0 +1,106 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +# Copyright (c) 2022 Amarulasolutions > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/media/i2c/ovti,ov5693.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Omnivision OV5693 CMOS Sensor > > + > > +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, > > + 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). > > + > > +allOf: > > + - $ref: /schemas/media/video-interface-devices.yaml# > > + > > +properties: > > + compatible: > > + const: ovti,ov5693 > > + > > + reg: > > + maxItems: 1 > > + > > + clocks: > > + description: > > + System input clock (aka XVCLK). From 6 to 27 MHz. > > + maxItems: 1 > > + > > + dovdd-supply: > > + description: > > + Digital I/O voltage supply, 1.8V. > > + > > + avdd-supply: > > + description: > > + Analog voltage supply, 2.8V. > > + > > + dvdd-supply: > > + description: > > + Digital core voltage supply, 1.2V. > > + > > + reset-gpios: > > + description: > > + The phandle and specifier for the GPIO that controls sensor reset. > > + This corresponds to the hardware pin XSHUTDN which is physically > > + active low. > > + maxItems: 1 > > + > > +required: > > + - compatible > > + - reg > > + - clocks > > + - dovdd-supply > > + - avdd-supply > > + - dvdd-supply > > Should supplies be made mandatory ? Sensors are often powered by fixed > rails. Do we want DTS writers to create "fixed-regulators" for all of > them ? The fact the regulator framework creates dummies if there's no > entry in .dts for a regulator makes me think it's fine to have them > optional, but I understand how Linux works should not be an indication > of how a bindings should look like. You are right, this depends on hw design and yes in many cases sensors are powered by fixed rails. But let me say, I see some design in wich I have to handle these signals and in fact are mandatory. I check also in others binding's doc like: - Documentation/devicetree/bindings/media/i2c/ovti,ov5640.yaml - Documentation/devicetree/bindings/media/i2c/ovti,ov02a10.yaml - Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml ... These keep this information. Anyway, You suggest to drop off: - dovdd-supply - avdd-supply - dvdd-supply
Hi Tommaso On Mon, Jul 11, 2022 at 01:11:08PM +0200, Tommaso Merciai wrote: > Hi Jacopo, > Thanks for your review. > > On Mon, Jul 11, 2022 at 11:36:59AM +0200, Jacopo Mondi wrote: > > Hi Tommaso, Krzysztof, > > > > This has been reviewed by Krzysztof already, so I guess it's fine, > > but let me ask anyway > > > > On Thu, Jun 30, 2022 at 03:48:34PM +0200, Tommaso Merciai wrote: > > > Add documentation of device tree in YAML schema for the OV5693 > > > CMOS image sensor from Omnivision > > > > > > Signed-off-by: Tommaso Merciai <tommaso.merciai@amarulasolutions.com> > > > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > > > --- > > > Changes since v1: > > > - Fix allOf position as suggested by Krzysztof > > > - Remove port description as suggested by Krzysztof > > > - Fix EOF as suggested by Krzysztof > > > > > > Changes since v2: > > > - Fix commit body as suggested by Krzysztof > > > > > > Changes since v3: > > > - Add reviewed-by tags, suggested by Jacopo, Krzysztof > > > > > > Changes since v4: > > > - Remove wrong Sakari reviewed-by tag, suggested by Krzysztof, Sakari > > > > > > .../bindings/media/i2c/ovti,ov5693.yaml | 106 ++++++++++++++++++ > > > MAINTAINERS | 1 + > > > 2 files changed, 107 insertions(+) > > > create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml > > > > > > diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml > > > new file mode 100644 > > > index 000000000000..b83c9fc04023 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml > > > @@ -0,0 +1,106 @@ > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > +# Copyright (c) 2022 Amarulasolutions > > > +%YAML 1.2 > > > +--- > > > +$id: http://devicetree.org/schemas/media/i2c/ovti,ov5693.yaml# > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > + > > > +title: Omnivision OV5693 CMOS Sensor > > > + > > > +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, > > > + 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). > > > + > > > +allOf: > > > + - $ref: /schemas/media/video-interface-devices.yaml# > > > + > > > +properties: > > > + compatible: > > > + const: ovti,ov5693 > > > + > > > + reg: > > > + maxItems: 1 > > > + > > > + clocks: > > > + description: > > > + System input clock (aka XVCLK). From 6 to 27 MHz. > > > + maxItems: 1 > > > + > > > + dovdd-supply: > > > + description: > > > + Digital I/O voltage supply, 1.8V. > > > + > > > + avdd-supply: > > > + description: > > > + Analog voltage supply, 2.8V. > > > + > > > + dvdd-supply: > > > + description: > > > + Digital core voltage supply, 1.2V. > > > + > > > + reset-gpios: > > > + description: > > > + The phandle and specifier for the GPIO that controls sensor reset. > > > + This corresponds to the hardware pin XSHUTDN which is physically > > > + active low. > > > + maxItems: 1 > > > + > > > +required: > > > + - compatible > > > + - reg > > > + - clocks > > > + - dovdd-supply > > > + - avdd-supply > > > + - dvdd-supply > > > > Should supplies be made mandatory ? Sensors are often powered by fixed > > rails. Do we want DTS writers to create "fixed-regulators" for all of > > them ? The fact the regulator framework creates dummies if there's no > > entry in .dts for a regulator makes me think it's fine to have them > > optional, but I understand how Linux works should not be an indication > > of how a bindings should look like. > > You are right, this depends on hw design and yes in many cases sensors are > powered by fixed rails. > But let me say, I see some design in wich I have to handle these signals and > in fact are mandatory. It's fine if you have to handle them, my question is it if it should be -mandatory- to specify them > > I check also in others binding's doc like: > > - Documentation/devicetree/bindings/media/i2c/ovti,ov5640.yaml > - Documentation/devicetree/bindings/media/i2c/ovti,ov02a10.yaml > - Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml > ... > > These keep this information. > > Anyway, You suggest to drop off: > > - dovdd-supply > - avdd-supply > - dvdd-supply > > From required properties, right? Yes, I wonder if they should be required. As usual there's a bunch of different styles in media/i2c/ and it's not always easy to distinguish which ones are actually intended from the ones which are instead the result of copying the existing. > > Tommmaso > > > > > > + - port > > > + > > > +unevaluatedProperties: false > > > + > > > +examples: > > > + - | > > > + #include <dt-bindings/clock/px30-cru.h> > > > + #include <dt-bindings/gpio/gpio.h> > > > + #include <dt-bindings/pinctrl/rockchip.h> > > > + > > > + i2c { > > > + #address-cells = <1>; > > > + #size-cells = <0>; > > > + > > > + ov5693: camera@36 { > > > + compatible = "ovti,ov5693"; > > > + reg = <0x36>; > > > + > > > + reset-gpios = <&gpio2 RK_PB1 GPIO_ACTIVE_LOW>; > > > + pinctrl-names = "default"; > > > + pinctrl-0 = <&cif_clkout_m0>; > > > + > > > + clocks = <&cru SCLK_CIF_OUT>; > > > + assigned-clocks = <&cru SCLK_CIF_OUT>; > > > + assigned-clock-rates = <19200000>; > > > + > > > + avdd-supply = <&vcc_1v8>; > > > + dvdd-supply = <&vcc_1v2>; > > > + dovdd-supply = <&vcc_2v8>; > > > + > > > + rotation = <90>; > > > + orientation = <0>; > > > + > > > + port { > > > + ucam_out: endpoint { > > > + remote-endpoint = <&mipi_in_ucam>; > > > + data-lanes = <1 2>; > > > + link-frequencies = /bits/ 64 <450000000>; > > > + }; > > > + }; > > > + }; > > > + }; > > > + > > > +... > > > diff --git a/MAINTAINERS b/MAINTAINERS > > > index 1fc9ead83d2a..844307cb20c4 100644 > > > --- a/MAINTAINERS > > > +++ b/MAINTAINERS > > > @@ -14719,6 +14719,7 @@ M: Daniel Scally <djrscally@gmail.com> > > > L: linux-media@vger.kernel.org > > > S: Maintained > > > T: git git://linuxtv.org/media_tree.git > > > +F: Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml > > > F: drivers/media/i2c/ov5693.c > > > > > > OMNIVISION OV5695 SENSOR DRIVER > > > -- > > > 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
On Mon, Jul 11, 2022 at 02:36:29PM +0200, Jacopo Mondi wrote: > Hi Tommaso > > On Mon, Jul 11, 2022 at 01:11:08PM +0200, Tommaso Merciai wrote: > > Hi Jacopo, > > Thanks for your review. > > > > On Mon, Jul 11, 2022 at 11:36:59AM +0200, Jacopo Mondi wrote: > > > Hi Tommaso, Krzysztof, > > > > > > This has been reviewed by Krzysztof already, so I guess it's fine, > > > but let me ask anyway > > > > > > On Thu, Jun 30, 2022 at 03:48:34PM +0200, Tommaso Merciai wrote: > > > > Add documentation of device tree in YAML schema for the OV5693 > > > > CMOS image sensor from Omnivision > > > > > > > > Signed-off-by: Tommaso Merciai <tommaso.merciai@amarulasolutions.com> > > > > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > > > > --- > > > > Changes since v1: > > > > - Fix allOf position as suggested by Krzysztof > > > > - Remove port description as suggested by Krzysztof > > > > - Fix EOF as suggested by Krzysztof > > > > > > > > Changes since v2: > > > > - Fix commit body as suggested by Krzysztof > > > > > > > > Changes since v3: > > > > - Add reviewed-by tags, suggested by Jacopo, Krzysztof > > > > > > > > Changes since v4: > > > > - Remove wrong Sakari reviewed-by tag, suggested by Krzysztof, Sakari > > > > > > > > .../bindings/media/i2c/ovti,ov5693.yaml | 106 ++++++++++++++++++ > > > > MAINTAINERS | 1 + > > > > 2 files changed, 107 insertions(+) > > > > create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml > > > > > > > > diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml > > > > new file mode 100644 > > > > index 000000000000..b83c9fc04023 > > > > --- /dev/null > > > > +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml > > > > @@ -0,0 +1,106 @@ > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > > +# Copyright (c) 2022 Amarulasolutions > > > > +%YAML 1.2 > > > > +--- > > > > +$id: http://devicetree.org/schemas/media/i2c/ovti,ov5693.yaml# > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > + > > > > +title: Omnivision OV5693 CMOS Sensor > > > > + > > > > +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, > > > > + 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). > > > > + > > > > +allOf: > > > > + - $ref: /schemas/media/video-interface-devices.yaml# > > > > + > > > > +properties: > > > > + compatible: > > > > + const: ovti,ov5693 > > > > + > > > > + reg: > > > > + maxItems: 1 > > > > + > > > > + clocks: > > > > + description: > > > > + System input clock (aka XVCLK). From 6 to 27 MHz. > > > > + maxItems: 1 > > > > + > > > > + dovdd-supply: > > > > + description: > > > > + Digital I/O voltage supply, 1.8V. > > > > + > > > > + avdd-supply: > > > > + description: > > > > + Analog voltage supply, 2.8V. > > > > + > > > > + dvdd-supply: > > > > + description: > > > > + Digital core voltage supply, 1.2V. > > > > + > > > > + reset-gpios: > > > > + description: > > > > + The phandle and specifier for the GPIO that controls sensor reset. > > > > + This corresponds to the hardware pin XSHUTDN which is physically > > > > + active low. > > > > + maxItems: 1 > > > > + > > > > +required: > > > > + - compatible > > > > + - reg > > > > + - clocks > > > > + - dovdd-supply > > > > + - avdd-supply > > > > + - dvdd-supply > > > > > > Should supplies be made mandatory ? Sensors are often powered by fixed > > > rails. Do we want DTS writers to create "fixed-regulators" for all of > > > them ? The fact the regulator framework creates dummies if there's no > > > entry in .dts for a regulator makes me think it's fine to have them > > > optional, but I understand how Linux works should not be an indication > > > of how a bindings should look like. > > > > You are right, this depends on hw design and yes in many cases sensors are > > powered by fixed rails. > > But let me say, I see some design in wich I have to handle these signals and > > in fact are mandatory. > > It's fine if you have to handle them, my question is it if it should > be -mandatory- to specify them > > > > > I check also in others binding's doc like: > > > > - Documentation/devicetree/bindings/media/i2c/ovti,ov5640.yaml > > - Documentation/devicetree/bindings/media/i2c/ovti,ov02a10.yaml > > - Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml > > ... > > > > These keep this information. > > > > Anyway, You suggest to drop off: > > > > - dovdd-supply > > - avdd-supply > > - dvdd-supply > > > > From required properties, right? > > Yes, I wonder if they should be required. As usual there's a > bunch of different styles in media/i2c/ and it's not always easy to > distinguish which ones are actually intended from the ones which are > instead the result of copying the existing. Got it. Let me know if we need v6 with your suggestion. Tommaso > > > > > > Tommmaso > > > > > > > > > + - port > > > > + > > > > +unevaluatedProperties: false > > > > + > > > > +examples: > > > > + - | > > > > + #include <dt-bindings/clock/px30-cru.h> > > > > + #include <dt-bindings/gpio/gpio.h> > > > > + #include <dt-bindings/pinctrl/rockchip.h> > > > > + > > > > + i2c { > > > > + #address-cells = <1>; > > > > + #size-cells = <0>; > > > > + > > > > + ov5693: camera@36 { > > > > + compatible = "ovti,ov5693"; > > > > + reg = <0x36>; > > > > + > > > > + reset-gpios = <&gpio2 RK_PB1 GPIO_ACTIVE_LOW>; > > > > + pinctrl-names = "default"; > > > > + pinctrl-0 = <&cif_clkout_m0>; > > > > + > > > > + clocks = <&cru SCLK_CIF_OUT>; > > > > + assigned-clocks = <&cru SCLK_CIF_OUT>; > > > > + assigned-clock-rates = <19200000>; > > > > + > > > > + avdd-supply = <&vcc_1v8>; > > > > + dvdd-supply = <&vcc_1v2>; > > > > + dovdd-supply = <&vcc_2v8>; > > > > + > > > > + rotation = <90>; > > > > + orientation = <0>; > > > > + > > > > + port { > > > > + ucam_out: endpoint { > > > > + remote-endpoint = <&mipi_in_ucam>; > > > > + data-lanes = <1 2>; > > > > + link-frequencies = /bits/ 64 <450000000>; > > > > + }; > > > > + }; > > > > + }; > > > > + }; > > > > + > > > > +... > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > > > index 1fc9ead83d2a..844307cb20c4 100644 > > > > --- a/MAINTAINERS > > > > +++ b/MAINTAINERS > > > > @@ -14719,6 +14719,7 @@ M: Daniel Scally <djrscally@gmail.com> > > > > L: linux-media@vger.kernel.org > > > > S: Maintained > > > > T: git git://linuxtv.org/media_tree.git > > > > +F: Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml > > > > F: drivers/media/i2c/ov5693.c > > > > > > > > OMNIVISION OV5695 SENSOR DRIVER > > > > -- > > > > 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
Hi Krzysztof could you have a look at the below question ? If no need to resend from Tommaso I think the series could be collected for v5.20. On Mon, Jul 11, 2022 at 11:37:05AM +0200, Jacopo Mondi wrote: > Hi Tommaso, Krzysztof, > > This has been reviewed by Krzysztof already, so I guess it's fine, > but let me ask anyway > > On Thu, Jun 30, 2022 at 03:48:34PM +0200, Tommaso Merciai wrote: > > Add documentation of device tree in YAML schema for the OV5693 > > CMOS image sensor from Omnivision > > > > Signed-off-by: Tommaso Merciai <tommaso.merciai@amarulasolutions.com> > > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > > --- > > Changes since v1: > > - Fix allOf position as suggested by Krzysztof > > - Remove port description as suggested by Krzysztof > > - Fix EOF as suggested by Krzysztof > > > > Changes since v2: > > - Fix commit body as suggested by Krzysztof > > > > Changes since v3: > > - Add reviewed-by tags, suggested by Jacopo, Krzysztof > > > > Changes since v4: > > - Remove wrong Sakari reviewed-by tag, suggested by Krzysztof, Sakari > > > > .../bindings/media/i2c/ovti,ov5693.yaml | 106 ++++++++++++++++++ > > MAINTAINERS | 1 + > > 2 files changed, 107 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml > > > > diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml > > new file mode 100644 > > index 000000000000..b83c9fc04023 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml > > @@ -0,0 +1,106 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +# Copyright (c) 2022 Amarulasolutions > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/media/i2c/ovti,ov5693.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Omnivision OV5693 CMOS Sensor > > + > > +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, > > + 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). > > + > > +allOf: > > + - $ref: /schemas/media/video-interface-devices.yaml# > > + > > +properties: > > + compatible: > > + const: ovti,ov5693 > > + > > + reg: > > + maxItems: 1 > > + > > + clocks: > > + description: > > + System input clock (aka XVCLK). From 6 to 27 MHz. > > + maxItems: 1 > > + > > + dovdd-supply: > > + description: > > + Digital I/O voltage supply, 1.8V. > > + > > + avdd-supply: > > + description: > > + Analog voltage supply, 2.8V. > > + > > + dvdd-supply: > > + description: > > + Digital core voltage supply, 1.2V. > > + > > + reset-gpios: > > + description: > > + The phandle and specifier for the GPIO that controls sensor reset. > > + This corresponds to the hardware pin XSHUTDN which is physically > > + active low. > > + maxItems: 1 > > + > > +required: > > + - compatible > > + - reg > > + - clocks > > + - dovdd-supply > > + - avdd-supply > > + - dvdd-supply > > Should supplies be made mandatory ? Sensors are often powered by fixed > rails. Do we want DTS writers to create "fixed-regulators" for all of > them ? The fact the regulator framework creates dummies if there's no > entry in .dts for a regulator makes me think it's fine to have them > optional, but I understand how Linux works should not be an indication > of how a bindings should look like. > This question ^ :) Thanks j > > + - port > > + > > +unevaluatedProperties: false > > + > > +examples: > > + - | > > + #include <dt-bindings/clock/px30-cru.h> > > + #include <dt-bindings/gpio/gpio.h> > > + #include <dt-bindings/pinctrl/rockchip.h> > > + > > + i2c { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + ov5693: camera@36 { > > + compatible = "ovti,ov5693"; > > + reg = <0x36>; > > + > > + reset-gpios = <&gpio2 RK_PB1 GPIO_ACTIVE_LOW>; > > + pinctrl-names = "default"; > > + pinctrl-0 = <&cif_clkout_m0>; > > + > > + clocks = <&cru SCLK_CIF_OUT>; > > + assigned-clocks = <&cru SCLK_CIF_OUT>; > > + assigned-clock-rates = <19200000>; > > + > > + avdd-supply = <&vcc_1v8>; > > + dvdd-supply = <&vcc_1v2>; > > + dovdd-supply = <&vcc_2v8>; > > + > > + rotation = <90>; > > + orientation = <0>; > > + > > + port { > > + ucam_out: endpoint { > > + remote-endpoint = <&mipi_in_ucam>; > > + data-lanes = <1 2>; > > + link-frequencies = /bits/ 64 <450000000>; > > + }; > > + }; > > + }; > > + }; > > + > > +... > > diff --git a/MAINTAINERS b/MAINTAINERS > > index 1fc9ead83d2a..844307cb20c4 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -14719,6 +14719,7 @@ M: Daniel Scally <djrscally@gmail.com> > > L: linux-media@vger.kernel.org > > S: Maintained > > T: git git://linuxtv.org/media_tree.git > > +F: Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml > > F: drivers/media/i2c/ov5693.c > > > > OMNIVISION OV5695 SENSOR DRIVER > > -- > > 2.25.1 > >
On 12/07/2022 17:25, Jacopo Mondi wrote: > Hi Krzysztof > could you have a look at the below question ? Sorry, there was a bunch of quoted text without end. When you reply under quote, please remove the rest of the quote. None of us have a lot of time to waste on scrolling emails... > > If no need to resend from Tommaso I think the series could be > collected for v5.20. > > On Mon, Jul 11, 2022 at 11:37:05AM +0200, Jacopo Mondi wrote: >> Hi Tommaso, Krzysztof, >> >> This has been reviewed by Krzysztof already, so I guess it's fine, >> but let me ask anyway >> >> On Thu, Jun 30, 2022 at 03:48:34PM +0200, Tommaso Merciai wrote: >>> Add documentation of device tree in YAML schema for the OV5693 >>> CMOS image sensor from Omnivision >>> >>> Signed-off-by: Tommaso Merciai <tommaso.merciai@amarulasolutions.com> >>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >>> --- >>> Changes since v1: >>> - Fix allOf position as suggested by Krzysztof >>> - Remove port description as suggested by Krzysztof >>> - Fix EOF as suggested by Krzysztof >>> >>> Changes since v2: >>> - Fix commit body as suggested by Krzysztof >>> >>> Changes since v3: >>> - Add reviewed-by tags, suggested by Jacopo, Krzysztof >>> >>> Changes since v4: >>> - Remove wrong Sakari reviewed-by tag, suggested by Krzysztof, Sakari >>> >>> .../bindings/media/i2c/ovti,ov5693.yaml | 106 ++++++++++++++++++ >>> MAINTAINERS | 1 + >>> 2 files changed, 107 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml >>> >>> diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml >>> new file mode 100644 >>> index 000000000000..b83c9fc04023 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml >>> @@ -0,0 +1,106 @@ >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>> +# Copyright (c) 2022 Amarulasolutions >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/media/i2c/ovti,ov5693.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: Omnivision OV5693 CMOS Sensor >>> + >>> +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, >>> + 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). >>> + >>> +allOf: >>> + - $ref: /schemas/media/video-interface-devices.yaml# >>> + >>> +properties: >>> + compatible: >>> + const: ovti,ov5693 >>> + >>> + reg: >>> + maxItems: 1 >>> + >>> + clocks: >>> + description: >>> + System input clock (aka XVCLK). From 6 to 27 MHz. >>> + maxItems: 1 >>> + >>> + dovdd-supply: >>> + description: >>> + Digital I/O voltage supply, 1.8V. >>> + >>> + avdd-supply: >>> + description: >>> + Analog voltage supply, 2.8V. >>> + >>> + dvdd-supply: >>> + description: >>> + Digital core voltage supply, 1.2V. >>> + >>> + reset-gpios: >>> + description: >>> + The phandle and specifier for the GPIO that controls sensor reset. >>> + This corresponds to the hardware pin XSHUTDN which is physically >>> + active low. >>> + maxItems: 1 >>> + >>> +required: >>> + - compatible >>> + - reg >>> + - clocks >>> + - dovdd-supply >>> + - avdd-supply >>> + - dvdd-supply >> >> Should supplies be made mandatory ? Sensors are often powered by fixed >> rails. Do we want DTS writers to create "fixed-regulators" for all of >> them ? The fact the regulator framework creates dummies if there's no >> entry in .dts for a regulator makes me think it's fine to have them >> optional, but I understand how Linux works should not be an indication >> of how a bindings should look like. >> > > This question ^ :) My generic answer for generic devices would be - if resource is physically required (one need to connect the wire), I would say it should be also required in the bindings. This also forces driver developer to think about these resources and might result on portable/better code. However your point is correct that it might create many "fake" regulators, because pretty often these are fixed on the board and not controllable. Therefore I am fine with not requiring them - to adjust the bindings to real life cases. Best regards, Krzysztof
Hi Krzysztof On Tue, Jul 12, 2022 at 05:32:45PM +0200, Krzysztof Kozlowski wrote: > On 12/07/2022 17:25, Jacopo Mondi wrote: > > Hi Krzysztof > > could you have a look at the below question ? > > Sorry, there was a bunch of quoted text without end. When you reply > under quote, please remove the rest of the quote. None of us have a lot > of time to waste on scrolling emails... > I should have kept a counter of the times I've been told "please do not remove context, I'm so busy I do not have time to read the whole thread" and "please remove context, I'm so busy I cannot read the whole email". After 5 years of kernel development I would now know what to do. > > > > If no need to resend from Tommaso I think the series could be > > collected for v5.20. > > > > On Mon, Jul 11, 2022 at 11:37:05AM +0200, Jacopo Mondi wrote: > >> Hi Tommaso, Krzysztof, > >> > >> This has been reviewed by Krzysztof already, so I guess it's fine, > >> but let me ask anyway > >> > >> On Thu, Jun 30, 2022 at 03:48:34PM +0200, Tommaso Merciai wrote: > >>> Add documentation of device tree in YAML schema for the OV5693 > >>> CMOS image sensor from Omnivision > >>> > >>> Signed-off-by: Tommaso Merciai <tommaso.merciai@amarulasolutions.com> > >>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > >>> --- > >>> Changes since v1: > >>> - Fix allOf position as suggested by Krzysztof > >>> - Remove port description as suggested by Krzysztof > >>> - Fix EOF as suggested by Krzysztof > >>> > >>> Changes since v2: > >>> - Fix commit body as suggested by Krzysztof > >>> > >>> Changes since v3: > >>> - Add reviewed-by tags, suggested by Jacopo, Krzysztof > >>> > >>> Changes since v4: > >>> - Remove wrong Sakari reviewed-by tag, suggested by Krzysztof, Sakari > >>> > >>> .../bindings/media/i2c/ovti,ov5693.yaml | 106 ++++++++++++++++++ > >>> MAINTAINERS | 1 + > >>> 2 files changed, 107 insertions(+) > >>> create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml > >>> > >>> diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml > >>> new file mode 100644 > >>> index 000000000000..b83c9fc04023 > >>> --- /dev/null > >>> +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml > >>> @@ -0,0 +1,106 @@ > >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > >>> +# Copyright (c) 2022 Amarulasolutions > >>> +%YAML 1.2 > >>> +--- > >>> +$id: http://devicetree.org/schemas/media/i2c/ovti,ov5693.yaml# > >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# > >>> + > >>> +title: Omnivision OV5693 CMOS Sensor > >>> + > >>> +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, > >>> + 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). > >>> + > >>> +allOf: > >>> + - $ref: /schemas/media/video-interface-devices.yaml# > >>> + > >>> +properties: > >>> + compatible: > >>> + const: ovti,ov5693 > >>> + > >>> + reg: > >>> + maxItems: 1 > >>> + > >>> + clocks: > >>> + description: > >>> + System input clock (aka XVCLK). From 6 to 27 MHz. > >>> + maxItems: 1 > >>> + > >>> + dovdd-supply: > >>> + description: > >>> + Digital I/O voltage supply, 1.8V. > >>> + > >>> + avdd-supply: > >>> + description: > >>> + Analog voltage supply, 2.8V. > >>> + > >>> + dvdd-supply: > >>> + description: > >>> + Digital core voltage supply, 1.2V. > >>> + > >>> + reset-gpios: > >>> + description: > >>> + The phandle and specifier for the GPIO that controls sensor reset. > >>> + This corresponds to the hardware pin XSHUTDN which is physically > >>> + active low. > >>> + maxItems: 1 > >>> + > >>> +required: > >>> + - compatible > >>> + - reg > >>> + - clocks > >>> + - dovdd-supply > >>> + - avdd-supply > >>> + - dvdd-supply > >> > >> Should supplies be made mandatory ? Sensors are often powered by fixed > >> rails. Do we want DTS writers to create "fixed-regulators" for all of > >> them ? The fact the regulator framework creates dummies if there's no > >> entry in .dts for a regulator makes me think it's fine to have them > >> optional, but I understand how Linux works should not be an indication > >> of how a bindings should look like. > >> > > > > This question ^ :) > > My generic answer for generic devices would be - if resource is > physically required (one need to connect the wire), I would say it > should be also required in the bindings. This also forces driver > developer to think about these resources and might result on > portable/better code. > > However your point is correct that it might create many "fake" > regulators, because pretty often these are fixed on the board and not > controllable. Therefore I am fine with not requiring them - to adjust > the bindings to real life cases. Tommaso if you can re-send this one with the supplies dropped I think the series is still in time for being collected for this merge window (Sakari to confirm this). Thanks j > > Best regards, > Krzysztof
On 12/07/2022 18:12, Jacopo Mondi wrote: > Hi Krzysztof > > On Tue, Jul 12, 2022 at 05:32:45PM +0200, Krzysztof Kozlowski wrote: >> On 12/07/2022 17:25, Jacopo Mondi wrote: >>> Hi Krzysztof >>> could you have a look at the below question ? >> >> Sorry, there was a bunch of quoted text without end. When you reply >> under quote, please remove the rest of the quote. None of us have a lot >> of time to waste on scrolling emails... >> > > I should have kept a counter of the times I've been told "please do > not remove context, I'm so busy I do not have time to read the whole > thread" and "please remove context, I'm so busy I cannot read the > whole email". > > After 5 years of kernel development I would now know what to do. I never got the first one, only the second nags. :) (...) >>>> Should supplies be made mandatory ? Sensors are often powered by fixed >>>> rails. Do we want DTS writers to create "fixed-regulators" for all of >>>> them ? The fact the regulator framework creates dummies if there's no >>>> entry in .dts for a regulator makes me think it's fine to have them >>>> optional, but I understand how Linux works should not be an indication >>>> of how a bindings should look like. >>>> >>> >>> This question ^ :) >> >> My generic answer for generic devices would be - if resource is >> physically required (one need to connect the wire), I would say it >> should be also required in the bindings. This also forces driver >> developer to think about these resources and might result on >> portable/better code. >> >> However your point is correct that it might create many "fake" >> regulators, because pretty often these are fixed on the board and not >> controllable. Therefore I am fine with not requiring them - to adjust >> the bindings to real life cases. > > Tommaso if you can re-send this one with the supplies dropped I think > the series is still in time for being collected for this merge window > (Sakari to confirm this). Sure. In either case please keep my review-by tag. Best regards, Krzysztof
Hi Jacopo, On Tue, Jul 12, 2022 at 06:12:36PM +0200, Jacopo Mondi wrote: > Hi Krzysztof > > On Tue, Jul 12, 2022 at 05:32:45PM +0200, Krzysztof Kozlowski wrote: > > On 12/07/2022 17:25, Jacopo Mondi wrote: > > > Hi Krzysztof > > > could you have a look at the below question ? > > > > Sorry, there was a bunch of quoted text without end. When you reply > > under quote, please remove the rest of the quote. None of us have a lot > > of time to waste on scrolling emails... > > > > I should have kept a counter of the times I've been told "please do > not remove context, I'm so busy I do not have time to read the whole > thread" and "please remove context, I'm so busy I cannot read the > whole email". > > After 5 years of kernel development I would now know what to do. > > > > > > > If no need to resend from Tommaso I think the series could be > > > collected for v5.20. > > > > > > On Mon, Jul 11, 2022 at 11:37:05AM +0200, Jacopo Mondi wrote: > > >> Hi Tommaso, Krzysztof, > > >> > > >> This has been reviewed by Krzysztof already, so I guess it's fine, > > >> but let me ask anyway > > >> > > >> On Thu, Jun 30, 2022 at 03:48:34PM +0200, Tommaso Merciai wrote: > > >>> Add documentation of device tree in YAML schema for the OV5693 > > >>> CMOS image sensor from Omnivision > > >>> > > >>> Signed-off-by: Tommaso Merciai <tommaso.merciai@amarulasolutions.com> > > >>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > > >>> --- > > >>> Changes since v1: > > >>> - Fix allOf position as suggested by Krzysztof > > >>> - Remove port description as suggested by Krzysztof > > >>> - Fix EOF as suggested by Krzysztof > > >>> > > >>> Changes since v2: > > >>> - Fix commit body as suggested by Krzysztof > > >>> > > >>> Changes since v3: > > >>> - Add reviewed-by tags, suggested by Jacopo, Krzysztof > > >>> > > >>> Changes since v4: > > >>> - Remove wrong Sakari reviewed-by tag, suggested by Krzysztof, Sakari > > >>> > > >>> .../bindings/media/i2c/ovti,ov5693.yaml | 106 ++++++++++++++++++ > > >>> MAINTAINERS | 1 + > > >>> 2 files changed, 107 insertions(+) > > >>> create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml > > >>> > > >>> diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml > > >>> new file mode 100644 > > >>> index 000000000000..b83c9fc04023 > > >>> --- /dev/null > > >>> +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml > > >>> @@ -0,0 +1,106 @@ > > >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > >>> +# Copyright (c) 2022 Amarulasolutions > > >>> +%YAML 1.2 > > >>> +--- > > >>> +$id: http://devicetree.org/schemas/media/i2c/ovti,ov5693.yaml# > > >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# > > >>> + > > >>> +title: Omnivision OV5693 CMOS Sensor > > >>> + > > >>> +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, > > >>> + 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). > > >>> + > > >>> +allOf: > > >>> + - $ref: /schemas/media/video-interface-devices.yaml# > > >>> + > > >>> +properties: > > >>> + compatible: > > >>> + const: ovti,ov5693 > > >>> + > > >>> + reg: > > >>> + maxItems: 1 > > >>> + > > >>> + clocks: > > >>> + description: > > >>> + System input clock (aka XVCLK). From 6 to 27 MHz. > > >>> + maxItems: 1 > > >>> + > > >>> + dovdd-supply: > > >>> + description: > > >>> + Digital I/O voltage supply, 1.8V. > > >>> + > > >>> + avdd-supply: > > >>> + description: > > >>> + Analog voltage supply, 2.8V. > > >>> + > > >>> + dvdd-supply: > > >>> + description: > > >>> + Digital core voltage supply, 1.2V. > > >>> + > > >>> + reset-gpios: > > >>> + description: > > >>> + The phandle and specifier for the GPIO that controls sensor reset. > > >>> + This corresponds to the hardware pin XSHUTDN which is physically > > >>> + active low. > > >>> + maxItems: 1 > > >>> + > > >>> +required: > > >>> + - compatible > > >>> + - reg > > >>> + - clocks > > >>> + - dovdd-supply > > >>> + - avdd-supply > > >>> + - dvdd-supply > > >> > > >> Should supplies be made mandatory ? Sensors are often powered by fixed > > >> rails. Do we want DTS writers to create "fixed-regulators" for all of > > >> them ? The fact the regulator framework creates dummies if there's no > > >> entry in .dts for a regulator makes me think it's fine to have them > > >> optional, but I understand how Linux works should not be an indication > > >> of how a bindings should look like. > > >> > > > > > > This question ^ :) > > > > My generic answer for generic devices would be - if resource is > > physically required (one need to connect the wire), I would say it > > should be also required in the bindings. This also forces driver > > developer to think about these resources and might result on > > portable/better code. > > > > However your point is correct that it might create many "fake" > > regulators, because pretty often these are fixed on the board and not > > controllable. Therefore I am fine with not requiring them - to adjust > > the bindings to real life cases. > > Tommaso if you can re-send this one with the supplies dropped I think > the series is still in time for being collected for this merge window > (Sakari to confirm this). Perfect, I'll send v6 with your suggestion. Thanks, Tommaso > > Thanks > j > > > > > Best regards, > > Krzysztof
diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml new file mode 100644 index 000000000000..b83c9fc04023 --- /dev/null +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml @@ -0,0 +1,106 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +# Copyright (c) 2022 Amarulasolutions +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/media/i2c/ovti,ov5693.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Omnivision OV5693 CMOS Sensor + +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, + 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). + +allOf: + - $ref: /schemas/media/video-interface-devices.yaml# + +properties: + compatible: + const: ovti,ov5693 + + reg: + maxItems: 1 + + clocks: + description: + System input clock (aka XVCLK). From 6 to 27 MHz. + maxItems: 1 + + dovdd-supply: + description: + Digital I/O voltage supply, 1.8V. + + avdd-supply: + description: + Analog voltage supply, 2.8V. + + dvdd-supply: + description: + Digital core voltage supply, 1.2V. + + reset-gpios: + description: + The phandle and specifier for the GPIO that controls sensor reset. + This corresponds to the hardware pin XSHUTDN which is physically + active low. + maxItems: 1 + +required: + - compatible + - reg + - clocks + - dovdd-supply + - avdd-supply + - dvdd-supply + - port + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/clock/px30-cru.h> + #include <dt-bindings/gpio/gpio.h> + #include <dt-bindings/pinctrl/rockchip.h> + + i2c { + #address-cells = <1>; + #size-cells = <0>; + + ov5693: camera@36 { + compatible = "ovti,ov5693"; + reg = <0x36>; + + reset-gpios = <&gpio2 RK_PB1 GPIO_ACTIVE_LOW>; + pinctrl-names = "default"; + pinctrl-0 = <&cif_clkout_m0>; + + clocks = <&cru SCLK_CIF_OUT>; + assigned-clocks = <&cru SCLK_CIF_OUT>; + assigned-clock-rates = <19200000>; + + avdd-supply = <&vcc_1v8>; + dvdd-supply = <&vcc_1v2>; + dovdd-supply = <&vcc_2v8>; + + rotation = <90>; + orientation = <0>; + + port { + ucam_out: endpoint { + remote-endpoint = <&mipi_in_ucam>; + data-lanes = <1 2>; + link-frequencies = /bits/ 64 <450000000>; + }; + }; + }; + }; + +... diff --git a/MAINTAINERS b/MAINTAINERS index 1fc9ead83d2a..844307cb20c4 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -14719,6 +14719,7 @@ M: Daniel Scally <djrscally@gmail.com> L: linux-media@vger.kernel.org S: Maintained T: git git://linuxtv.org/media_tree.git +F: Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml F: drivers/media/i2c/ov5693.c OMNIVISION OV5695 SENSOR DRIVER