| Message ID | m37dhkdrat.fsf@t19.piap.pl (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC,v3] dt-binding: media: document ON Semi AR0521 sensor bindings | expand |
On Wed, Jul 21, 2021 at 10:06:34AM +0200, Krzysztof Hałasa wrote: > This file documents DT bindings for the AR0521 camera sensor driver. > > Signed-off-by: Krzysztof Hałasa <khalasa@piap.pl> > --- > Changes from v2: > - changed "xclk" to "extclk" > - power regulator names etc. > - video output port properties > - cosmetics > - UTF-8 experiments :-) > > diff --git a/Documentation/devicetree/bindings/media/i2c/onnn,ar0521.yaml b/Documentation/devicetree/bindings/media/i2c/onnn,ar0521.yaml > new file mode 100644 > index 000000000000..785bae61bb5e > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/i2c/onnn,ar0521.yaml > @@ -0,0 +1,108 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/media/i2c/onnn,ar0521.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: ON Semiconductor AR0521 MIPI CSI-2 sensor > + > +maintainers: > + - Krzysztof Hałasa <khalasa@piap.pl> > + > +description: |- > + The AR0521 is a raw CMOS image sensor with MIPI CSI-2 and > + I2C-compatible control interface. > + > +properties: > + compatible: > + const: onnn,ar0521 > + > + reg: > + maxItems: 1 > + > + clocks: > + maxItems: 1 > + > + clock-names: > + const: extclk > + > + vaa-supply: > + description: > + Definition of the regulator used as analog (2.7 V) voltage supply. > + > + vdd-supply: > + description: > + Definition of the regulator used as digital core (1.2 V) voltage supply. > + > + vdd_io-supply: > + description: > + Definition of the regulator used as digital I/O (1.8 V) voltage supply. > + > + reset-gpios: > + description: reset GPIO, usually active low > + maxItems: 1 > + > + port: > + $ref: /schemas/graph.yaml#/properties/port $ref: /schemas/graph.yaml#/$defs/port-base unevaluatedProperties: false > + description: | > + Video output port. > + > + properties: > + endpoint: > + $ref: /schemas/media/video-interfaces.yaml# unevaluatedProperties: false > + > + properties: > + data-lanes: > + anyOf: > + - items: > + - const: 1 > + - items: > + - const: 1 > + - const: 2 > + - items: > + - const: 1 > + - const: 2 > + - const: 3 > + - const: 4 > + > +required: > + - compatible > + - reg > + - clocks > + - clock-names > + - vaa-supply > + - vdd-supply > + - vdd_io-supply > + - port > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/gpio/gpio.h> > + #include <dt-bindings/clock/imx6qdl-clock.h> > + > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + > + ar0521: camera-sensor@36 { > + compatible = "onnn,ar0521"; > + reg = <0x36>; > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_mipi_camera>; > + clocks = <&clks IMX6QDL_CLK_CKO>; > + clock-names = "extclk"; > + reset-gpios = <&gpio1 7 GPIO_ACTIVE_LOW>; > + vaa-supply = <®_2p7v>; > + vdd-supply = <®_1p2v>; > + vdd_io-supply = <®_1p8v>; > + > + port { > + mipi_camera_to_mipi_csi2: endpoint { > + remote-endpoint = <&mipi_csi2_in>; > + data-lanes = <1 2 3 4>; > + }; > + }; > + }; > + }; > > -- > Krzysztof "Chris" Hałasa > > Sieć Badawcza Łukasiewicz > Przemysłowy Instytut Automatyki i Pomiarów PIAP > Al. Jerozolimskie 202, 02-486 Warszawa >
Hi Krzysztof, (CC'ing Sakari Ailus) Thank you for the patch. On Wed, Jul 21, 2021 at 10:06:34AM +0200, Krzysztof Hałasa wrote: > This file documents DT bindings for the AR0521 camera sensor driver. > > Signed-off-by: Krzysztof Hałasa <khalasa@piap.pl> > --- > Changes from v2: > - changed "xclk" to "extclk" > - power regulator names etc. > - video output port properties > - cosmetics > - UTF-8 experiments :-) > > diff --git a/Documentation/devicetree/bindings/media/i2c/onnn,ar0521.yaml b/Documentation/devicetree/bindings/media/i2c/onnn,ar0521.yaml > new file mode 100644 > index 000000000000..785bae61bb5e > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/i2c/onnn,ar0521.yaml > @@ -0,0 +1,108 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/media/i2c/onnn,ar0521.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: ON Semiconductor AR0521 MIPI CSI-2 sensor > + > +maintainers: > + - Krzysztof Hałasa <khalasa@piap.pl> > + > +description: |- > + The AR0521 is a raw CMOS image sensor with MIPI CSI-2 and > + I2C-compatible control interface. > + > +properties: > + compatible: > + const: onnn,ar0521 > + > + reg: > + maxItems: 1 > + > + clocks: > + maxItems: 1 > + > + clock-names: > + const: extclk > + > + vaa-supply: > + description: > + Definition of the regulator used as analog (2.7 V) voltage supply. > + > + vdd-supply: > + description: > + Definition of the regulator used as digital core (1.2 V) voltage supply. > + > + vdd_io-supply: > + description: > + Definition of the regulator used as digital I/O (1.8 V) voltage supply. > + > + reset-gpios: > + description: reset GPIO, usually active low > + maxItems: 1 > + > + port: > + $ref: /schemas/graph.yaml#/properties/port > + description: | > + Video output port. > + > + properties: > + endpoint: > + $ref: /schemas/media/video-interfaces.yaml# > + > + properties: > + data-lanes: > + anyOf: > + - items: > + - const: 1 > + - items: > + - const: 1 > + - const: 2 > + - items: > + - const: 1 > + - const: 2 > + - const: 3 > + - const: 4 As the sensor also supports an HiSPi output, I would add the bus-type property: data-lanes: const: 4 Sakari, what do you think ? This way we won't have to rely on an implicit default when (and if) the kernel gets support for HiSPi. With or without this change, and with Rob's comments addressed, Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Thank you for not giving up :-) > + > +required: > + - compatible > + - reg > + - clocks > + - clock-names > + - vaa-supply > + - vdd-supply > + - vdd_io-supply > + - port > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/gpio/gpio.h> > + #include <dt-bindings/clock/imx6qdl-clock.h> > + > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + > + ar0521: camera-sensor@36 { > + compatible = "onnn,ar0521"; > + reg = <0x36>; > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_mipi_camera>; > + clocks = <&clks IMX6QDL_CLK_CKO>; > + clock-names = "extclk"; > + reset-gpios = <&gpio1 7 GPIO_ACTIVE_LOW>; > + vaa-supply = <®_2p7v>; > + vdd-supply = <®_1p2v>; > + vdd_io-supply = <®_1p8v>; > + > + port { > + mipi_camera_to_mipi_csi2: endpoint { > + remote-endpoint = <&mipi_csi2_in>; > + data-lanes = <1 2 3 4>; > + }; > + }; > + }; > + }; >
Hi Laurent, Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes: >> + properties: >> + data-lanes: >> + anyOf: >> + - items: >> + - const: 1 >> + - items: >> + - const: 1 >> + - const: 2 >> + - items: >> + - const: 1 >> + - const: 2 >> + - const: 3 >> + - const: 4 > > As the sensor also supports an HiSPi output, I would add the bus-type > property: > > data-lanes: > const: 4 Is there any example of this? I'm not sure how should it it look like. Something like the following? properties: data-lanes: anyOf: - items: - const: 1 - items: - const: 1 - const: 2 - items: - const: 1 - const: 2 - const: 3 - const: 4 bus-type: data-lanes: const: 4 And... HiSPi would need additional code in the driver. And preferably some testing. I think I'd prefer to have DT and the driver staying in some sort of sync. Also, I'm uncertain about the syntax and the meaning of such, apparently redundant, construct. Nor about its relation to HiSPi. An example would be welcome.
Hi Krzysztof, On Tue, Jul 27, 2021 at 12:36:20PM +0200, Krzysztof Hałasa wrote: > Hi Laurent, > > Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes: > > >> + properties: > >> + data-lanes: > >> + anyOf: > >> + - items: > >> + - const: 1 > >> + - items: > >> + - const: 1 > >> + - const: 2 > >> + - items: > >> + - const: 1 > >> + - const: 2 > >> + - const: 3 > >> + - const: 4 > > > > As the sensor also supports an HiSPi output, I would add the bus-type > > property: > > > > data-lanes: > > const: 4 > > Is there any example of this? I'm not sure how should it it look like. > Something like the following? > > properties: > data-lanes: > anyOf: > - items: > - const: 1 > - items: > - const: 1 > - const: 2 > - items: > - const: 1 > - const: 2 > - const: 3 > - const: 4 > bus-type: > data-lanes: > const: 4 I think Laurent meant: bus-type: const: 4 This way the bindings can be later amended with HiSPi support without relying on defaults. Albeit the other busses in practice almost never end up being used even if supported, apart from the standard BT.601, BT.656 and CSI-2. Either way is fine IMO. > > And... HiSPi would need additional code in the driver. And preferably > some testing. I think I'd prefer to have DT and the driver staying in > some sort of sync. Also, I'm uncertain about the syntax and the meaning > of such, apparently redundant, construct. Nor about its relation to > HiSPi. An example would be welcome. No need to add support for the driver.
Hi Sakari, Sakari Ailus <sakari.ailus@iki.fi> writes: > I think Laurent meant: > > bus-type: > const: 4 > > This way the bindings can be later amended with HiSPi support without > relying on defaults. Albeit the other busses in practice almost never end > up being used even if supported, apart from the standard BT.601, BT.656 and > CSI-2. > > Either way is fine IMO. Ok, so I'll leave it as is (apart from Rob's additions/changes). > No need to add support for the driver. Ok. Thanks.
Sakari, Laurent, Sakari Ailus <sakari.ailus@iki.fi> writes: > I think Laurent meant: > > bus-type: > const: 4 Ok I found what it means :-)
diff --git a/Documentation/devicetree/bindings/media/i2c/onnn,ar0521.yaml b/Documentation/devicetree/bindings/media/i2c/onnn,ar0521.yaml new file mode 100644 index 000000000000..785bae61bb5e --- /dev/null +++ b/Documentation/devicetree/bindings/media/i2c/onnn,ar0521.yaml @@ -0,0 +1,108 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/media/i2c/onnn,ar0521.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: ON Semiconductor AR0521 MIPI CSI-2 sensor + +maintainers: + - Krzysztof Hałasa <khalasa@piap.pl> + +description: |- + The AR0521 is a raw CMOS image sensor with MIPI CSI-2 and + I2C-compatible control interface. + +properties: + compatible: + const: onnn,ar0521 + + reg: + maxItems: 1 + + clocks: + maxItems: 1 + + clock-names: + const: extclk + + vaa-supply: + description: + Definition of the regulator used as analog (2.7 V) voltage supply. + + vdd-supply: + description: + Definition of the regulator used as digital core (1.2 V) voltage supply. + + vdd_io-supply: + description: + Definition of the regulator used as digital I/O (1.8 V) voltage supply. + + reset-gpios: + description: reset GPIO, usually active low + maxItems: 1 + + port: + $ref: /schemas/graph.yaml#/properties/port + description: | + Video output port. + + properties: + endpoint: + $ref: /schemas/media/video-interfaces.yaml# + + properties: + data-lanes: + anyOf: + - items: + - const: 1 + - items: + - const: 1 + - const: 2 + - items: + - const: 1 + - const: 2 + - const: 3 + - const: 4 + +required: + - compatible + - reg + - clocks + - clock-names + - vaa-supply + - vdd-supply + - vdd_io-supply + - port + +additionalProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + #include <dt-bindings/clock/imx6qdl-clock.h> + + i2c { + #address-cells = <1>; + #size-cells = <0>; + + ar0521: camera-sensor@36 { + compatible = "onnn,ar0521"; + reg = <0x36>; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_mipi_camera>; + clocks = <&clks IMX6QDL_CLK_CKO>; + clock-names = "extclk"; + reset-gpios = <&gpio1 7 GPIO_ACTIVE_LOW>; + vaa-supply = <®_2p7v>; + vdd-supply = <®_1p2v>; + vdd_io-supply = <®_1p8v>; + + port { + mipi_camera_to_mipi_csi2: endpoint { + remote-endpoint = <&mipi_csi2_in>; + data-lanes = <1 2 3 4>; + }; + }; + }; + };
This file documents DT bindings for the AR0521 camera sensor driver. Signed-off-by: Krzysztof Hałasa <khalasa@piap.pl> --- Changes from v2: - changed "xclk" to "extclk" - power regulator names etc. - video output port properties - cosmetics - UTF-8 experiments :-)