Hi Dave, Thank you for your review. On 10/03/2022 16:38, Dave Stevenson wrote: > Hi Benjamin > > cc Laurent and Sakari as maintainers of video-interfaces.yaml > > On Thu, 10 Mar 2022 at 13:37, Benjamin Mugnier > <benjamin.mugnier@xxxxxxxxxxx> 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 >> + >> + 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? Regard, Benjamin > >> + #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 >> + >> + data-lanes: >> + description: >> + CSI lanes to use >> + items: >> + - const: 1 >> + - const: 2 >> + - const: 3 >> + - const: 4 >> + >> + remote-endpoint: true >> + >> + required: >> + - clock-lane >> + - data-lanes >> + - remote-endpoint >> + >> +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 >> + >> + >> ST VL53L0X ToF RANGER(I2C) IIO DRIVER >> M: Song Qiang <songqiang1304521@xxxxxxxxx> >> L: linux-iio@xxxxxxxxxxxxxxx >> -- >> 2.25.1 >>
