Hello, On Fri, Mar 11, 2022 at 03:13:11PM +0200, Sakari Ailus wrote: > On Fri, Mar 11, 2022 at 12:25:38PM +0100, Benjamin Mugnier wrote: > > On 10/03/2022 16:38, Dave Stevenson wrote: > > > On Thu, 10 Mar 2022 at 13:37, Benjamin Mugnier wrote: > > >> > > >> Add device tree binding for the ST VGXY61 camera sensor, and update > > >> MAINTAINERS file. > > >> > > >> Signed-off-by: Benjamin Mugnier <benjamin.mugnier@xxxxxxxxxxx> > > >> --- > > >> .../bindings/media/i2c/st,st-vgxy61.yaml | 134 ++++++++++++++++++ > > >> MAINTAINERS | 10 ++ > > >> 2 files changed, 144 insertions(+) > > >> create mode 100644 Documentation/devicetree/bindings/media/i2c/st,st-vgxy61.yaml > > >> > > >> diff --git a/Documentation/devicetree/bindings/media/i2c/st,st-vgxy61.yaml b/Documentation/devicetree/bindings/media/i2c/st,st-vgxy61.yaml > > >> new file mode 100644 > > >> index 000000000000..8740ed2623e4 > > >> --- /dev/null > > >> +++ b/Documentation/devicetree/bindings/media/i2c/st,st-vgxy61.yaml > > >> @@ -0,0 +1,134 @@ > > >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > > >> +# Copyright (c) 2022 STMicroelectronics SA. > > >> +%YAML 1.2 > > >> +--- > > >> +$id: http://devicetree.org/schemas/media/i2c/st,st-vgxy61.yaml# > > >> +$schema: http://devicetree.org/meta-schemas/core.yaml# > > >> + > > >> +title: STMicroelectronics VGxy61 HDR Global Shutter Sensor Family Device Tree Bindings > > >> + > > >> +maintainers: > > >> + - Benjamin Mugnier <benjamin.mugnier@xxxxxxxxxxx> > > >> + - Sylvain Petinot <sylvain.petinot@xxxxxxxxxxx> > > >> + > > >> +description: |- > > >> + STMicroelectronics VGxy61 family has a CSI-2 output port. CSI-2 output is a > > >> + quad lanes 800Mbps per lane. > > >> + Supported formats are RAW8, RAW10, RAW12, RAW14 and RAW16. > > >> + Following part number are supported > > >> + - VG5661 and VG6661 are 1.6 Mpx (1464 x 1104) monochrome and color sensors. > > >> + Maximum frame rate is 75 fps. > > >> + - VG5761 and VG6761 are 2.3 Mpx (1944 x 1204) monochrome and color sensors. > > >> + Maximum frame rate is 60 fps. > > >> + > > >> +properties: > > >> + compatible: > > >> + const: st,st-vgxy61 > > >> + > > >> + reg: > > >> + maxItems: 1 > > >> + > > >> + clocks: > > >> + maxItems: 1 > > >> + > > >> + clock-names: > > >> + description: > > >> + Input clock for the sensor. > > >> + items: > > >> + - const: xclk > > Do you need this if you have a single clock? > > Also see Documentation/driver-api/media/camera-sensor.rst . > > > >> + > > >> + VCORE-supply: > > >> + description: > > >> + Sensor digital core supply. Must be 1.2 volts. > > >> + > > >> + VDDIO-supply: > > >> + description: > > >> + Sensor digital IO supply. Must be 1.8 volts. > > >> + > > >> + VANA-supply: > > >> + description: > > >> + Sensor analog supply. Must be 2.8 volts. > > >> + > > >> + reset-gpios: > > >> + description: > > >> + Reference to the GPIO connected to the reset pin, if any. > > >> + This is an active low signal to the vgxy61. > > >> + > > >> + invert-gpios-polarity: > > >> + description: > > >> + If gpios polarity should be inversed > > > > > > s/inversed/inverted > > > > > > > Ok. > > > > >> + type: boolean > > >> + > > >> + slave-mode: > > >> + description: > > >> + If the sensor operates in slave mode > > >> + type: boolean > > > > > > This is one I've been meaning to raise for a while. > > > Is DT the correct place to be configuring hardware sync options for > > > image sensors? (There may be the linguistic discussions over master / > > > slave terminology too). > > > We also have IMX477 and a number of other sensors that support > > > external sync control of some form. > > > > > > As I see it, there are nominally 3 settings - disabled (reduces EMC > > > noise), generate syncs, and receive syncs. > > > For test purposes it would be useful to be able to switch between > > > generate and receive modes at runtime, so that would make it a control > > > instead of being fixed in DT. > > > > > > If it should be configured in DT, then how does ACPI need to handle it? > > > > > > If DT is the correct place to define the role, should it be in > > > video-interfaces.yaml as an optional property, instead of being a > > > sensor specific binding? > > > > > > Sorry, more questions rather than answers. > > > > > > Dave > > > > Maybe I can provide additional info on this sensor to help find an > > answer. The "slave mode" has 2 settings: enabled or disabled. If disabled > > you are in master mode ('generate sync' and 'disabled' modes Dave > > mentionned, they are the same here), and if enabled you are in slave mode > > ('receive sync'). As you said he master sends frame sync signals to the > > slave each frame acquired, this allows both sensors to synchronize > > themselves. > > > > I put this in the device tree as we only use it for 3D stereocam boards > > which already have 2 sensors on them, meaning this is hardware specific. > > I don't have any use case where we manually wire 2 sensors on 2 separate > > boards. One good point you mentioned is that I may not always want run > > this board in master/slaver, and both sensors could run on master mode > > without interacting with each other, thus justifying a dedicated v4l2 > > control. > > > > Any ideas on how to name it instead of "slave mode" for coherency between > > sensors? > > How is this wired? The slave-mode property documentation explicitly refers > to synchronisation signals that do not exists in CSI-2. What is slave mode in this case ? Does it only mean that the sensor is externally triggered, or is there something else ? For parallel interfaces with H/V sync there's a possibility of the H/V sync signals being inputs instead of outputs, but that's not applicable to CSI-2. > > >> + #TODO check all this or copy from elsewhere > > > > Just noticed this and will remove it. > > > > >> + port: > > >> + $ref: /schemas/graph.yaml#/$defs/port-base > > >> + additionalProperties: false > > >> + > > >> + properties: > > >> + endpoint: > > >> + $ref: /schemas/media/video-interfaces.yaml# > > >> + unevaluatedProperties: false > > >> + > > >> + properties: > > >> + clock-lane: > > >> + description: > > >> + Clock lane index > > >> + maxItems: 1 > > Does the device support lane reordering? If not, please drop. > > > >> + > > >> + data-lanes: > > >> + description: > > >> + CSI lanes to use > > >> + items: > > >> + - const: 1 > > >> + - const: 2 > > >> + - const: 3 > > >> + - const: 4 > > Which lane configurations does the device support? If it's four lanes only, > then you can drop this property, too. > > > >> + > > >> + remote-endpoint: true > > >> + > > >> + required: > > >> + - clock-lane > > >> + - data-lanes > > >> + - remote-endpoint > > Listing remote-endpoint here isn't needed as this comes from the schema. > > > >> + > > >> +required: > > >> + - compatible > > >> + - clocks > > >> + - clock-names > > >> + - VCORE-supply > > >> + - VDDIO-supply > > >> + - VANA-supply > > >> + > > >> +additionalProperties: false > > >> + > > >> +examples: > > >> + - | > > >> + #include <dt-bindings/gpio/gpio.h> > > >> + i2c { > > >> + #address-cells = <1>; > > >> + #size-cells = <0>; > > >> + vgxy61: csi2tx@10 { > > >> + compatible = "st,st-vgxy61"; > > >> + reg = <0x10>; > > >> + status = "okay"; > > >> + clocks = <&clk_ext_camera>; > > >> + clock-names = "xclk"; > > >> + VCORE-supply = <&v1v2>; > > >> + VDDIO-supply = <&v1v8>; > > >> + VANA-supply = <&v2v8>; > > >> + reset-gpios = <&mfxgpio 18 GPIO_ACTIVE_LOW>; > > >> + port { > > >> + ep0: endpoint { > > >> + clock-lane = <0>; > > >> + data-lanes = <1 2 3 4>; > > >> + remote-endpoint = <&mipi_csi2_out>; > > >> + }; > > >> + }; > > >> + }; > > >> + }; > > >> +... > > >> diff --git a/MAINTAINERS b/MAINTAINERS > > >> index 83d27b57016f..f358d15f68a0 100644 > > >> --- a/MAINTAINERS > > >> +++ b/MAINTAINERS > > >> @@ -18297,6 +18297,16 @@ S: Maintained > > >> F: Documentation/hwmon/stpddc60.rst > > >> F: drivers/hwmon/pmbus/stpddc60.c > > >> > > >> +ST VGXY61 DRIVER > > >> +M: Benjamin Mugnier <benjamin.mugnier@xxxxxxxxxxx> > > >> +M: Sylvain Petinot <sylvain.petinot@xxxxxxxxxxx> > > >> +L: linux-media@xxxxxxxxxxxxxxx > > >> +S: Maintained > > >> +T: git git://linuxtv.org/media_tree.git > > >> +F: Documentation/devicetree/bindings/media/i2c/st,st-vgxy61.txt > > >> +F: drivers/media/i2c/st-vgxy61.c > > >> + > > Extra newline. > > > >> + > > >> ST VL53L0X ToF RANGER(I2C) IIO DRIVER > > >> M: Song Qiang <songqiang1304521@xxxxxxxxx> > > >> L: linux-iio@xxxxxxxxxxxxxxx -- Regards, Laurent Pinchart
