Re: [PATCH v5 1/2] media: dt-bindings: Add ST VD55G1 camera sensor

[Benjamin Mugnier <benjamin.mugnier@foss.st.com>]

Hi Laurent,

Thank you for your review.

On 4/22/25 15:11, Laurent Pinchart wrote:
> Hi Benjamin,
> 
> Thank you for the patch.
> 
> On Fri, Apr 04, 2025 at 04:50:51PM +0200, Benjamin Mugnier wrote:
>> Also update MAINTAINERS file accordingly.
>>
>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> Signed-off-by: Benjamin Mugnier <benjamin.mugnier@foss.st.com>
>> ---
>>  .../devicetree/bindings/media/i2c/st,vd55g1.yaml   | 132 +++++++++++++++++++++
>>  MAINTAINERS                                        |   7 ++
>>  2 files changed, 139 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/media/i2c/st,vd55g1.yaml b/Documentation/devicetree/bindings/media/i2c/st,vd55g1.yaml
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..6b777f86790da4e5941ac1cad86dc1a5021f9f5b
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/i2c/st,vd55g1.yaml
>> @@ -0,0 +1,132 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +# Copyright (c) 2025 STMicroelectronics SA.
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/media/i2c/st,vd55g1.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: STMicroelectronics VD55G1 Global Shutter Image Sensor
>> +
>> +maintainers:
>> +  - Benjamin Mugnier <benjamin.mugnier@foss.st.com>
>> +  - Sylvain Petinot <sylvain.petinot@foss.st.com>
>> +
>> +description: |-
>> + The STMicroelectronics VD55G1 is a global shutter image sensor with an active
>> + array size of 804H x 704V. It is programmable through I2C interface. The I2C
>> + address is fixed to 0x10.
> 
> If you intend for this block of text to be split in two paragraphs, it's
> missing a blank line here. Otherwise, it should be reflowed as a single
> paragraph.

Thanks, I'll add a blank line.

> 
>> + Image data is sent through MIPI CSI-2, which is configured as only 1 data
>> + lane. The sensor provides 4 GPIOS that can be used for external LED signal
>> + (synchronized with sensor integration periods).
>> +
>> +allOf:
>> +  - $ref: /schemas/media/video-interface-devices.yaml#
>> +
>> +properties:
>> +  compatible:
>> +    const: st,vd55g1
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  clocks:
>> +    maxItems: 1
>> +
>> +  vcore-supply:
>> +    description: Digital core power supply (1.15V)
>> +
>> +  vddio-supply:
>> +    description: Digital IO power supply (1.8V)
>> +
>> +  vana-supply:
>> +    description: Analog power supply (2.8V)
>> +
>> +  reset-gpios:
>> +    description: Sensor reset active low GPIO (XSHUTDOWN)
>> +    maxItems: 1
>> +
>> +  st,leds:
>> +    description:
>> +      List sensor's GPIOs used to control strobe light sources during exposure
>> +      time. The numbers identify the sensor pin on which the illumination
>> +      system is connected. GPIOs are active-high.
> 
> If multiple GPIOs are specified, do they all serve the exact same
> purpose, or is there a need to differentiate them ?

The same purpose. All GPIOs mentioned in this list will be used for
illumination and will strobe the same way regardless of their id.

> 
>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
>> +    minItems: 1
>> +    maxItems: 4
>> +    items:
>> +      minimum: 0
>> +      maximum: 3
>> +
>> +  port:
>> +    $ref: /schemas/graph.yaml#/$defs/port-base
>> +    additionalProperties: false
>> +
>> +    properties:
>> +      endpoint:
>> +        $ref: /schemas/media/video-interfaces.yaml#
>> +        unevaluatedProperties: false
>> +
>> +        properties:
>> +          data-lanes:
>> +            items:
>> +              - const: 1
>> +
>> +          link-frequencies:
>> +            maxItems: 1
>> +            items:
>> +              minimum: 125000000
>> +              maximum: 600000000
>> +
>> +          lane-polarities:
>> +            minItems: 1
>> +            maxItems: 2
> 
> Does the sensor support non-continuous D-PHY clock ?

No it doesn't, continuous clock only.

> 
>> +
>> +        required:
>> +          - data-lanes
>> +          - link-frequencies
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - clocks
>> +  - vcore-supply
>> +  - vddio-supply
>> +  - vana-supply
>> +  - reset-gpios
>> +  - port
>> +
>> +unevaluatedProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/gpio/gpio.h>
>> +
>> +    i2c {
>> +        #address-cells = <1>;
>> +        #size-cells = <0>;
>> +
>> +        camera-sensor@10 {
>> +            compatible = "st,vd55g1";
>> +            reg = <0x10>;
>> +
>> +            clocks = <&camera_clk_12M>;
>> +
>> +            vcore-supply = <&camera_vcore_v1v15>;
>> +            vddio-supply = <&camera_vddio_v1v8>;
>> +            vana-supply = <&camera_vana_v2v8>;
>> +
>> +            reset-gpios = <&gpio 5 GPIO_ACTIVE_LOW>;
>> +            st,leds = <2>;
>> +
>> +            orientation = <2>;
>> +            rotation = <0>;
>> +
>> +            port {
>> +                endpoint {
>> +                    data-lanes = <1>;
>> +                    link-frequencies = /bits/ 64 <600000000>;
>> +                    remote-endpoint = <&csiphy0_ep>;
>> +                };
>> +            };
>> +        };
>> +    };
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 2286200b355bde3604607be916ef09aa88feed0e..4f5e9005063a157de69e81b10f8def9da9e6c04c 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -22410,6 +22410,13 @@ S:	Maintained
>>  F:	Documentation/hwmon/stpddc60.rst
>>  F:	drivers/hwmon/pmbus/stpddc60.c
>>  
>> +ST VD55G1 DRIVER
>> +M:	Benjamin Mugnier <benjamin.mugnier@foss.st.com>
>> +M:	Sylvain Petinot <sylvain.petinot@foss.st.com>
>> +L:	linux-media@vger.kernel.org
>> +S:	Maintained
>> +F:	Documentation/devicetree/bindings/media/i2c/st,vd55g1.yaml
>> +
>>  ST VGXY61 DRIVER
>>  M:	Benjamin Mugnier <benjamin.mugnier@foss.st.com>
>>  M:	Sylvain Petinot <sylvain.petinot@foss.st.com>
> 

-- 
Regards,
Benjamin