[PATCH v3 1/3] dt-bindings: vendor-prefixes: Add prefix alliedvision

[PATCH v3 1/3] dt-bindings: vendor-prefixes: Add prefix alliedvision

[Tommaso Merciai]

Add a vendor prefix entry for Allied Vision Inc.
(https://www.alliedvision.com)

Signed-off-by: Tommaso Merciai <tomm.merciai@gmail.com>
---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 82d39ab0231b..7bd678956b3d 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -77,6 +77,8 @@ patternProperties:
     description: ALFA Network Inc.
   "^allegro,.*":
     description: Allegro DVT
+  "^alliedvision,.*":
+    description: Allied Vision Inc.
   "^allo,.*":
     description: Allo.com
   "^allwinner,.*":
-- 
2.34.1

Re: [PATCH v3 1/3] dt-bindings: vendor-prefixes: Add prefix alliedvision

[Laurent Pinchart]

Hi Tommaso,

Thank you for the patch.

On Tue, Jun 06, 2023 at 05:54:02PM +0200, Tommaso Merciai wrote:
> Add a vendor prefix entry for Allied Vision Inc.
> (https://www.alliedvision.com)
> 
> Signed-off-by: Tommaso Merciai <tomm.merciai@gmail.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> index 82d39ab0231b..7bd678956b3d 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> @@ -77,6 +77,8 @@ patternProperties:
>      description: ALFA Network Inc.
>    "^allegro,.*":
>      description: Allegro DVT
> +  "^alliedvision,.*":
> +    description: Allied Vision Inc.
>    "^allo,.*":
>      description: Allo.com
>    "^allwinner,.*":

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 1/3] dt-bindings: vendor-prefixes: Add prefix alliedvision

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 490 bytes --]

On Tue, Jun 06, 2023 at 07:29:11PM +0300, Laurent Pinchart wrote:
> Hi Tommaso,
> 
> Thank you for the patch.
> 
> On Tue, Jun 06, 2023 at 05:54:02PM +0200, Tommaso Merciai wrote:
> > Add a vendor prefix entry for Allied Vision Inc.
> > (https://www.alliedvision.com)
> > 
> > Signed-off-by: Tommaso Merciai <tomm.merciai@gmail.com>
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Acked-by: Conor Dooley <conor.dooley@microchip.com>

Cheers,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v3 1/3] dt-bindings: vendor-prefixes: Add prefix alliedvision

[Krzysztof Kozlowski]

On 06/06/2023 17:54, Tommaso Merciai wrote:
> Add a vendor prefix entry for Allied Vision Inc.
> (https://www.alliedvision.com)
> 
> Signed-off-by: Tommaso Merciai <tomm.merciai@gmail.com>


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

[PATCH v3 2/3] media: dt-bindings: alvium: add document YAML binding

[Tommaso Merciai]

Add documentation of device tree in YAML schema for the ALVIUM
Camera from Allied Vision Inc.

References:
 - https://www.alliedvision.com/en/products/embedded-vision-solutions

Signed-off-by: Tommaso Merciai <tomm.merciai@gmail.com>
---
Changes since v1:
 - Fixed build error as suggested by RHerring bot

Changes since v2:
 - Fixed License as suggested by KKozlowski/CDooley
 - Removed rotation property as suggested by CDooley/LPinchart
 - Fixed example node name as suggested by CDooley
 - Fixed title as suggested by LPinchart
 - Fixed compatible name as suggested by LPinchart
 - Removed clock as suggested by LPinchart
 - Removed gpios not as suggested by LPinchart
 - Renamed property name streamon-delay into alliedvision,lp2hs-delay-us
 - Fixed vendor prefix, unit append as suggested by KKozlowski
 - Fixed data-lanes
 - Fixed blank space + example indentation (from 6 -> 4 space) as suggested by KKozlowski
 - Dropped status into example  as suggested by KKozlowski
 - Added vcc-ext-in supply as suggested by LPinchart
 - Dropped pinctrl into example as suggested by LPinchart

 .../media/i2c/alliedvision,alvium-csi2.yaml   | 93 +++++++++++++++++++
 1 file changed, 93 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/alliedvision,alvium-csi2.yaml

diff --git a/Documentation/devicetree/bindings/media/i2c/alliedvision,alvium-csi2.yaml b/Documentation/devicetree/bindings/media/i2c/alliedvision,alvium-csi2.yaml
new file mode 100644
index 000000000000..191534e2f7bd
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/alliedvision,alvium-csi2.yaml
@@ -0,0 +1,93 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/i2c/alliedvision,alvium.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Allied Vision Alvium Camera
+
+maintainers:
+  - Tommaso Merciai <tomm.merciai@gmail.com>
+  - Martin Hecht <martin.hecht@avnet.eu>
+
+allOf:
+  - $ref: /schemas/media/video-interface-devices.yaml#
+
+properties:
+  compatible:
+    const: alliedvision,alvium-csi2
+
+  reg:
+    maxItems: 1
+
+  vcc-ext-in-supply:
+    description:
+      Definition of the regulator used as interface power supply.
+
+  alliedvision,lp2hs-delay-us:
+    maxItems: 1
+    description:
+      Low power to high speed delay time in microseconds.
+      The purpose of this property is force a DPhy reset for the period
+      described by the microseconds on the property, before it starts
+      streaming. To be clear, with that value bigger than 0 the Alvium
+      forces a dphy-reset on all lanes for that period. That means all
+      lanes go up into low power state. This may help a csi2 rx ip to
+      reset if that IP can't deal with a continous clock.
+
+  port:
+    description: Digital Output Port
+    $ref: /schemas/graph.yaml#/$defs/port-base
+    additionalProperties: false
+
+    properties:
+      endpoint:
+        $ref: /schemas/media/video-interfaces.yaml#
+        unevaluatedProperties: false
+
+        properties:
+          link-frequencies: true
+
+          data-lanes:
+            minItems: 1
+            items:
+              - const: 1
+              - const: 2
+              - const: 3
+              - const: 4
+
+        required:
+          - data-lanes
+          - link-frequencies
+
+required:
+  - compatible
+  - reg
+  - vcc-ext-in-supply
+  - port
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        alvium: camera@3c {
+            compatible = "alliedvision,alvium-csi2";
+            reg = <0x3c>;
+            vcc-ext-in-supply = <&reg_vcc_ext_in>;
+            alliedvision,lp2hs-delay-us = <20>;
+
+            port {
+                alvium_out: endpoint {
+                    remote-endpoint = <&mipi_csi_0_in>;
+                    data-lanes = <1 2 3 4>;
+                    link-frequencies = /bits/ 64 <681250000>;
+                };
+            };
+        };
+    };
+
+...
-- 
2.34.1

Re: [PATCH v3 2/3] media: dt-bindings: alvium: add document YAML binding

[Rob Herring]

On Tue, 06 Jun 2023 17:54:03 +0200, Tommaso Merciai wrote:
> Add documentation of device tree in YAML schema for the ALVIUM
> Camera from Allied Vision Inc.
> 
> References:
>  - https://www.alliedvision.com/en/products/embedded-vision-solutions
> 
> Signed-off-by: Tommaso Merciai <tomm.merciai@gmail.com>
> ---
> Changes since v1:
>  - Fixed build error as suggested by RHerring bot
> 
> Changes since v2:
>  - Fixed License as suggested by KKozlowski/CDooley
>  - Removed rotation property as suggested by CDooley/LPinchart
>  - Fixed example node name as suggested by CDooley
>  - Fixed title as suggested by LPinchart
>  - Fixed compatible name as suggested by LPinchart
>  - Removed clock as suggested by LPinchart
>  - Removed gpios not as suggested by LPinchart
>  - Renamed property name streamon-delay into alliedvision,lp2hs-delay-us
>  - Fixed vendor prefix, unit append as suggested by KKozlowski
>  - Fixed data-lanes
>  - Fixed blank space + example indentation (from 6 -> 4 space) as suggested by KKozlowski
>  - Dropped status into example  as suggested by KKozlowski
>  - Added vcc-ext-in supply as suggested by LPinchart
>  - Dropped pinctrl into example as suggested by LPinchart
> 
>  .../media/i2c/alliedvision,alvium-csi2.yaml   | 93 +++++++++++++++++++
>  1 file changed, 93 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/alliedvision,alvium-csi2.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
./Documentation/devicetree/bindings/media/i2c/alliedvision,alvium-csi2.yaml: $id: relative path/filename doesn't match actual path or filename
	expected: http://devicetree.org/schemas/media/i2c/alliedvision,alvium-csi2.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230606155416.260941-3-tomm.merciai@gmail.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH v3 2/3] media: dt-bindings: alvium: add document YAML binding

[Laurent Pinchart]

Hi Tommaso,

Thank you for the patch.

On Tue, Jun 06, 2023 at 05:54:03PM +0200, Tommaso Merciai wrote:
> Add documentation of device tree in YAML schema for the ALVIUM
> Camera from Allied Vision Inc.
> 
> References:
>  - https://www.alliedvision.com/en/products/embedded-vision-solutions
> 
> Signed-off-by: Tommaso Merciai <tomm.merciai@gmail.com>
> ---
> Changes since v1:
>  - Fixed build error as suggested by RHerring bot
> 
> Changes since v2:
>  - Fixed License as suggested by KKozlowski/CDooley
>  - Removed rotation property as suggested by CDooley/LPinchart
>  - Fixed example node name as suggested by CDooley
>  - Fixed title as suggested by LPinchart
>  - Fixed compatible name as suggested by LPinchart
>  - Removed clock as suggested by LPinchart
>  - Removed gpios not as suggested by LPinchart
>  - Renamed property name streamon-delay into alliedvision,lp2hs-delay-us
>  - Fixed vendor prefix, unit append as suggested by KKozlowski
>  - Fixed data-lanes
>  - Fixed blank space + example indentation (from 6 -> 4 space) as suggested by KKozlowski
>  - Dropped status into example  as suggested by KKozlowski
>  - Added vcc-ext-in supply as suggested by LPinchart
>  - Dropped pinctrl into example as suggested by LPinchart
> 
>  .../media/i2c/alliedvision,alvium-csi2.yaml   | 93 +++++++++++++++++++
>  1 file changed, 93 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/alliedvision,alvium-csi2.yaml
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/alliedvision,alvium-csi2.yaml b/Documentation/devicetree/bindings/media/i2c/alliedvision,alvium-csi2.yaml
> new file mode 100644
> index 000000000000..191534e2f7bd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/alliedvision,alvium-csi2.yaml
> @@ -0,0 +1,93 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/i2c/alliedvision,alvium.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Allied Vision Alvium Camera
> +
> +maintainers:
> +  - Tommaso Merciai <tomm.merciai@gmail.com>
> +  - Martin Hecht <martin.hecht@avnet.eu>
> +
> +allOf:
> +  - $ref: /schemas/media/video-interface-devices.yaml#
> +
> +properties:
> +  compatible:
> +    const: alliedvision,alvium-csi2
> +
> +  reg:
> +    maxItems: 1
> +
> +  vcc-ext-in-supply:
> +    description:
> +      Definition of the regulator used as interface power supply.

      The regulator that supplies power to the VCC_EXT_IN pins.

> +
> +  alliedvision,lp2hs-delay-us:
> +    maxItems: 1
> +    description:
> +      Low power to high speed delay time in microseconds.

You can drop "in microseconds", that's implied by the suffix.

> +      The purpose of this property is force a DPhy reset for the period
> +      described by the microseconds on the property, before it starts
> +      streaming. To be clear, with that value bigger than 0 the Alvium
> +      forces a dphy-reset on all lanes for that period. That means all
> +      lanes go up into low power state. This may help a csi2 rx ip to
> +      reset if that IP can't deal with a continous clock.

I'd like to propose what I think is a clearer version:

    description: |
      Low power to high speed delay time.

      If the value is larger than 0, the camera forces a reset of all
      D-PHY lanes for the duration specified by this property. All lanes
      will transition to the low-power state and back to the high-speed
      state after the delay. Otherwise the lanes will transition to and
      remain in the high-speed state immediately after power on.

      This is meant to help CSI-2 receivers synchronizing their D-PHY
      RX.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +
> +  port:
> +    description: Digital Output Port
> +    $ref: /schemas/graph.yaml#/$defs/port-base
> +    additionalProperties: false
> +
> +    properties:
> +      endpoint:
> +        $ref: /schemas/media/video-interfaces.yaml#
> +        unevaluatedProperties: false
> +
> +        properties:
> +          link-frequencies: true
> +
> +          data-lanes:
> +            minItems: 1
> +            items:
> +              - const: 1
> +              - const: 2
> +              - const: 3
> +              - const: 4
> +
> +        required:
> +          - data-lanes
> +          - link-frequencies
> +
> +required:
> +  - compatible
> +  - reg
> +  - vcc-ext-in-supply
> +  - port
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        alvium: camera@3c {
> +            compatible = "alliedvision,alvium-csi2";
> +            reg = <0x3c>;
> +            vcc-ext-in-supply = <&reg_vcc_ext_in>;
> +            alliedvision,lp2hs-delay-us = <20>;
> +
> +            port {
> +                alvium_out: endpoint {
> +                    remote-endpoint = <&mipi_csi_0_in>;
> +                    data-lanes = <1 2 3 4>;
> +                    link-frequencies = /bits/ 64 <681250000>;
> +                };
> +            };
> +        };
> +    };
> +
> +...

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 2/3] media: dt-bindings: alvium: add document YAML binding

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1564 bytes --]

Hey Laurent, Tommaso,

On Tue, Jun 06, 2023 at 07:36:56PM +0300, Laurent Pinchart wrote:
> On Tue, Jun 06, 2023 at 05:54:03PM +0200, Tommaso Merciai wrote:

> > +  alliedvision,lp2hs-delay-us:
> > +    maxItems: 1
> > +    description:
> > +      Low power to high speed delay time in microseconds.
> 
> You can drop "in microseconds", that's implied by the suffix.
> 
> > +      The purpose of this property is force a DPhy reset for the period
> > +      described by the microseconds on the property, before it starts
> > +      streaming. To be clear, with that value bigger than 0 the Alvium
> > +      forces a dphy-reset on all lanes for that period. That means all
> > +      lanes go up into low power state. This may help a csi2 rx ip to
> > +      reset if that IP can't deal with a continous clock.
> 
> I'd like to propose what I think is a clearer version:
> 
>     description: |
>       Low power to high speed delay time.
> 
>       If the value is larger than 0, the camera forces a reset of all
>       D-PHY lanes for the duration specified by this property. All lanes
>       will transition to the low-power state and back to the high-speed
>       state after the delay. Otherwise the lanes will transition to and
>       remain in the high-speed state immediately after power on.
> 
>       This is meant to help CSI-2 receivers synchronizing their D-PHY
>       RX.

Question about the property.
Why not make it have a minimum value of 1 and drop the special-case
behaviour for zero?

Cheers,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v3 2/3] media: dt-bindings: alvium: add document YAML binding

[Laurent Pinchart]

On Tue, Jun 06, 2023 at 07:07:42PM +0100, Conor Dooley wrote:
> Hey Laurent, Tommaso,
> 
> On Tue, Jun 06, 2023 at 07:36:56PM +0300, Laurent Pinchart wrote:
> > On Tue, Jun 06, 2023 at 05:54:03PM +0200, Tommaso Merciai wrote:
> 
> > > +  alliedvision,lp2hs-delay-us:
> > > +    maxItems: 1
> > > +    description:
> > > +      Low power to high speed delay time in microseconds.
> > 
> > You can drop "in microseconds", that's implied by the suffix.
> > 
> > > +      The purpose of this property is force a DPhy reset for the period
> > > +      described by the microseconds on the property, before it starts
> > > +      streaming. To be clear, with that value bigger than 0 the Alvium
> > > +      forces a dphy-reset on all lanes for that period. That means all
> > > +      lanes go up into low power state. This may help a csi2 rx ip to
> > > +      reset if that IP can't deal with a continous clock.
> > 
> > I'd like to propose what I think is a clearer version:
> > 
> >     description: |
> >       Low power to high speed delay time.
> > 
> >       If the value is larger than 0, the camera forces a reset of all
> >       D-PHY lanes for the duration specified by this property. All lanes
> >       will transition to the low-power state and back to the high-speed
> >       state after the delay. Otherwise the lanes will transition to and
> >       remain in the high-speed state immediately after power on.
> > 
> >       This is meant to help CSI-2 receivers synchronizing their D-PHY
> >       RX.
> 
> Question about the property.
> Why not make it have a minimum value of 1 and drop the special-case
> behaviour for zero?

The property is optional, so it can indeed be omitted if no delay is
desired. I have no strong preference on whether or not to allow 0 as a
valid value.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 2/3] media: dt-bindings: alvium: add document YAML binding

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2337 bytes --]

On Tue, Jun 06, 2023 at 09:17:52PM +0300, Laurent Pinchart wrote:
> On Tue, Jun 06, 2023 at 07:07:42PM +0100, Conor Dooley wrote:
> > Hey Laurent, Tommaso,
> > 
> > On Tue, Jun 06, 2023 at 07:36:56PM +0300, Laurent Pinchart wrote:
> > > On Tue, Jun 06, 2023 at 05:54:03PM +0200, Tommaso Merciai wrote:
> > 
> > > > +  alliedvision,lp2hs-delay-us:
> > > > +    maxItems: 1
> > > > +    description:
> > > > +      Low power to high speed delay time in microseconds.
> > > 
> > > You can drop "in microseconds", that's implied by the suffix.
> > > 
> > > > +      The purpose of this property is force a DPhy reset for the period
> > > > +      described by the microseconds on the property, before it starts
> > > > +      streaming. To be clear, with that value bigger than 0 the Alvium
> > > > +      forces a dphy-reset on all lanes for that period. That means all
> > > > +      lanes go up into low power state. This may help a csi2 rx ip to
> > > > +      reset if that IP can't deal with a continous clock.
> > > 
> > > I'd like to propose what I think is a clearer version:
> > > 
> > >     description: |
> > >       Low power to high speed delay time.
> > > 
> > >       If the value is larger than 0, the camera forces a reset of all
> > >       D-PHY lanes for the duration specified by this property. All lanes
> > >       will transition to the low-power state and back to the high-speed
> > >       state after the delay. Otherwise the lanes will transition to and
> > >       remain in the high-speed state immediately after power on.
> > > 
> > >       This is meant to help CSI-2 receivers synchronizing their D-PHY
> > >       RX.
> > 
> > Question about the property.
> > Why not make it have a minimum value of 1 and drop the special-case
> > behaviour for zero?
> 
> The property is optional, so it can indeed be omitted if no delay is
> desired. I have no strong preference on whether or not to allow 0 as a
> valid value.

FWIW, I prefer the semantics of the property if it doesn't have the
limbo state of being present but doing nothing.

Cheers,
Conor.

BTW, I seem to get bounces from shawnx.tu@intel.com, who is listed in
MAINTAINERS for several drivers. Do you know if they have a non-intel
address to replace those entries with, or should they be dropped?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v3 2/3] media: dt-bindings: alvium: add document YAML binding

[Tommaso Merciai]

Hi Conor,

On Tue, Jun 06, 2023 at 07:23:32PM +0100, Conor Dooley wrote:
> On Tue, Jun 06, 2023 at 09:17:52PM +0300, Laurent Pinchart wrote:
> > On Tue, Jun 06, 2023 at 07:07:42PM +0100, Conor Dooley wrote:
> > > Hey Laurent, Tommaso,
> > > 
> > > On Tue, Jun 06, 2023 at 07:36:56PM +0300, Laurent Pinchart wrote:
> > > > On Tue, Jun 06, 2023 at 05:54:03PM +0200, Tommaso Merciai wrote:
> > > 
> > > > > +  alliedvision,lp2hs-delay-us:
> > > > > +    maxItems: 1
> > > > > +    description:
> > > > > +      Low power to high speed delay time in microseconds.
> > > > 
> > > > You can drop "in microseconds", that's implied by the suffix.
> > > > 
> > > > > +      The purpose of this property is force a DPhy reset for the period
> > > > > +      described by the microseconds on the property, before it starts
> > > > > +      streaming. To be clear, with that value bigger than 0 the Alvium
> > > > > +      forces a dphy-reset on all lanes for that period. That means all
> > > > > +      lanes go up into low power state. This may help a csi2 rx ip to
> > > > > +      reset if that IP can't deal with a continous clock.
> > > > 
> > > > I'd like to propose what I think is a clearer version:
> > > > 
> > > >     description: |
> > > >       Low power to high speed delay time.
> > > > 
> > > >       If the value is larger than 0, the camera forces a reset of all
> > > >       D-PHY lanes for the duration specified by this property. All lanes
> > > >       will transition to the low-power state and back to the high-speed
> > > >       state after the delay. Otherwise the lanes will transition to and
> > > >       remain in the high-speed state immediately after power on.
> > > > 
> > > >       This is meant to help CSI-2 receivers synchronizing their D-PHY
> > > >       RX.
> > > 
> > > Question about the property.
> > > Why not make it have a minimum value of 1 and drop the special-case
> > > behaviour for zero?
> > 
> > The property is optional, so it can indeed be omitted if no delay is
> > desired. I have no strong preference on whether or not to allow 0 as a
> > valid value.
> 
> FWIW, I prefer the semantics of the property if it doesn't have the
> limbo state of being present but doing nothing.
> 
> Cheers,
> Conor.
> 
> BTW, I seem to get bounces from shawnx.tu@intel.com, who is listed in
> MAINTAINERS for several drivers. Do you know if they have a non-intel
> address to replace those entries with, or should they be dropped?

Same here! :)

Thanks,
Tommaso

Re: [PATCH v3 2/3] media: dt-bindings: alvium: add document YAML binding

[Tommaso Merciai]

Hi Conor,

Thanks for your feedback.

On Tue, Jun 06, 2023 at 07:07:42PM +0100, Conor Dooley wrote:
> Hey Laurent, Tommaso,
> 
> On Tue, Jun 06, 2023 at 07:36:56PM +0300, Laurent Pinchart wrote:
> > On Tue, Jun 06, 2023 at 05:54:03PM +0200, Tommaso Merciai wrote:
> 
> > > +  alliedvision,lp2hs-delay-us:
> > > +    maxItems: 1
> > > +    description:
> > > +      Low power to high speed delay time in microseconds.
> > 
> > You can drop "in microseconds", that's implied by the suffix.
> > 
> > > +      The purpose of this property is force a DPhy reset for the period
> > > +      described by the microseconds on the property, before it starts
> > > +      streaming. To be clear, with that value bigger than 0 the Alvium
> > > +      forces a dphy-reset on all lanes for that period. That means all
> > > +      lanes go up into low power state. This may help a csi2 rx ip to
> > > +      reset if that IP can't deal with a continous clock.
> > 
> > I'd like to propose what I think is a clearer version:
> > 
> >     description: |
> >       Low power to high speed delay time.
> > 
> >       If the value is larger than 0, the camera forces a reset of all
> >       D-PHY lanes for the duration specified by this property. All lanes
> >       will transition to the low-power state and back to the high-speed
> >       state after the delay. Otherwise the lanes will transition to and
> >       remain in the high-speed state immediately after power on.
> > 
> >       This is meant to help CSI-2 receivers synchronizing their D-PHY
> >       RX.
> 
> Question about the property.
> Why not make it have a minimum value of 1 and drop the special-case
> behaviour for zero?

Personally I prefer to stay with zero case.
This reflect better the real camera register behaviour.

(also is optional)

Thanks! :)
Tommaso

> 
> Cheers,
> Conor.

Re: [PATCH v3 2/3] media: dt-bindings: alvium: add document YAML binding

[Laurent Pinchart]

On Wed, Jun 07, 2023 at 09:14:48AM +0200, Tommaso Merciai wrote:
> On Tue, Jun 06, 2023 at 07:07:42PM +0100, Conor Dooley wrote:
> > On Tue, Jun 06, 2023 at 07:36:56PM +0300, Laurent Pinchart wrote:
> > > On Tue, Jun 06, 2023 at 05:54:03PM +0200, Tommaso Merciai wrote:
> > 
> > > > +  alliedvision,lp2hs-delay-us:
> > > > +    maxItems: 1
> > > > +    description:
> > > > +      Low power to high speed delay time in microseconds.
> > > 
> > > You can drop "in microseconds", that's implied by the suffix.
> > > 
> > > > +      The purpose of this property is force a DPhy reset for the period
> > > > +      described by the microseconds on the property, before it starts
> > > > +      streaming. To be clear, with that value bigger than 0 the Alvium
> > > > +      forces a dphy-reset on all lanes for that period. That means all
> > > > +      lanes go up into low power state. This may help a csi2 rx ip to
> > > > +      reset if that IP can't deal with a continous clock.
> > > 
> > > I'd like to propose what I think is a clearer version:
> > > 
> > >     description: |
> > >       Low power to high speed delay time.
> > > 
> > >       If the value is larger than 0, the camera forces a reset of all
> > >       D-PHY lanes for the duration specified by this property. All lanes
> > >       will transition to the low-power state and back to the high-speed
> > >       state after the delay. Otherwise the lanes will transition to and
> > >       remain in the high-speed state immediately after power on.
> > > 
> > >       This is meant to help CSI-2 receivers synchronizing their D-PHY
> > >       RX.
> > 
> > Question about the property.
> > Why not make it have a minimum value of 1 and drop the special-case
> > behaviour for zero?
> 
> Personally I prefer to stay with zero case.
> This reflect better the real camera register behaviour.

Speaking of which, could you document the maximum value in the bindings
?

> (also is optional)

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 2/3] media: dt-bindings: alvium: add document YAML binding

[Tommaso Merciai]

Hi Laurent,

On Wed, Jun 07, 2023 at 10:21:34AM +0300, Laurent Pinchart wrote:
> On Wed, Jun 07, 2023 at 09:14:48AM +0200, Tommaso Merciai wrote:
> > On Tue, Jun 06, 2023 at 07:07:42PM +0100, Conor Dooley wrote:
> > > On Tue, Jun 06, 2023 at 07:36:56PM +0300, Laurent Pinchart wrote:
> > > > On Tue, Jun 06, 2023 at 05:54:03PM +0200, Tommaso Merciai wrote:
> > > 
> > > > > +  alliedvision,lp2hs-delay-us:
> > > > > +    maxItems: 1
> > > > > +    description:
> > > > > +      Low power to high speed delay time in microseconds.
> > > > 
> > > > You can drop "in microseconds", that's implied by the suffix.
> > > > 
> > > > > +      The purpose of this property is force a DPhy reset for the period
> > > > > +      described by the microseconds on the property, before it starts
> > > > > +      streaming. To be clear, with that value bigger than 0 the Alvium
> > > > > +      forces a dphy-reset on all lanes for that period. That means all
> > > > > +      lanes go up into low power state. This may help a csi2 rx ip to
> > > > > +      reset if that IP can't deal with a continous clock.
> > > > 
> > > > I'd like to propose what I think is a clearer version:
> > > > 
> > > >     description: |
> > > >       Low power to high speed delay time.
> > > > 
> > > >       If the value is larger than 0, the camera forces a reset of all
> > > >       D-PHY lanes for the duration specified by this property. All lanes
> > > >       will transition to the low-power state and back to the high-speed
> > > >       state after the delay. Otherwise the lanes will transition to and
> > > >       remain in the high-speed state immediately after power on.
> > > > 
> > > >       This is meant to help CSI-2 receivers synchronizing their D-PHY
> > > >       RX.
> > > 
> > > Question about the property.
> > > Why not make it have a minimum value of 1 and drop the special-case
> > > behaviour for zero?
> > 
> > Personally I prefer to stay with zero case.
> > This reflect better the real camera register behaviour.
> 
> Speaking of which, could you document the maximum value in the bindings
> ?

150ms is the max value I'll document this in v4.

Regards,
Tommaso

> 
> > (also is optional)
> 
> -- 
> Regards,
> 
> Laurent Pinchart

Re: [PATCH v3 2/3] media: dt-bindings: alvium: add document YAML binding

[Tommaso Merciai]

Hi Laurent,

On Tue, Jun 06, 2023 at 07:36:56PM +0300, Laurent Pinchart wrote:
> Hi Tommaso,
> 
> Thank you for the patch.
> 
> On Tue, Jun 06, 2023 at 05:54:03PM +0200, Tommaso Merciai wrote:
> > Add documentation of device tree in YAML schema for the ALVIUM
> > Camera from Allied Vision Inc.
> > 
> > References:
> >  - https://www.alliedvision.com/en/products/embedded-vision-solutions
> > 
> > Signed-off-by: Tommaso Merciai <tomm.merciai@gmail.com>
> > ---
> > Changes since v1:
> >  - Fixed build error as suggested by RHerring bot
> > 
> > Changes since v2:
> >  - Fixed License as suggested by KKozlowski/CDooley
> >  - Removed rotation property as suggested by CDooley/LPinchart
> >  - Fixed example node name as suggested by CDooley
> >  - Fixed title as suggested by LPinchart
> >  - Fixed compatible name as suggested by LPinchart
> >  - Removed clock as suggested by LPinchart
> >  - Removed gpios not as suggested by LPinchart
> >  - Renamed property name streamon-delay into alliedvision,lp2hs-delay-us
> >  - Fixed vendor prefix, unit append as suggested by KKozlowski
> >  - Fixed data-lanes
> >  - Fixed blank space + example indentation (from 6 -> 4 space) as suggested by KKozlowski
> >  - Dropped status into example  as suggested by KKozlowski
> >  - Added vcc-ext-in supply as suggested by LPinchart
> >  - Dropped pinctrl into example as suggested by LPinchart
> > 
> >  .../media/i2c/alliedvision,alvium-csi2.yaml   | 93 +++++++++++++++++++
> >  1 file changed, 93 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/media/i2c/alliedvision,alvium-csi2.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/media/i2c/alliedvision,alvium-csi2.yaml b/Documentation/devicetree/bindings/media/i2c/alliedvision,alvium-csi2.yaml
> > new file mode 100644
> > index 000000000000..191534e2f7bd
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/i2c/alliedvision,alvium-csi2.yaml
> > @@ -0,0 +1,93 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/media/i2c/alliedvision,alvium.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Allied Vision Alvium Camera
> > +
> > +maintainers:
> > +  - Tommaso Merciai <tomm.merciai@gmail.com>
> > +  - Martin Hecht <martin.hecht@avnet.eu>
> > +
> > +allOf:
> > +  - $ref: /schemas/media/video-interface-devices.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    const: alliedvision,alvium-csi2
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  vcc-ext-in-supply:
> > +    description:
> > +      Definition of the regulator used as interface power supply.
> 
>       The regulator that supplies power to the VCC_EXT_IN pins.

Oks. Ty.

> 
> > +
> > +  alliedvision,lp2hs-delay-us:
> > +    maxItems: 1
> > +    description:
> > +      Low power to high speed delay time in microseconds.
> 
> You can drop "in microseconds", that's implied by the suffix.

Oks.

> 
> > +      The purpose of this property is force a DPhy reset for the period
> > +      described by the microseconds on the property, before it starts
> > +      streaming. To be clear, with that value bigger than 0 the Alvium
> > +      forces a dphy-reset on all lanes for that period. That means all
> > +      lanes go up into low power state. This may help a csi2 rx ip to
> > +      reset if that IP can't deal with a continous clock.
> 
> I'd like to propose what I think is a clearer version:
> 
>     description: |
>       Low power to high speed delay time.
> 
>       If the value is larger than 0, the camera forces a reset of all
>       D-PHY lanes for the duration specified by this property. All lanes
>       will transition to the low-power state and back to the high-speed
>       state after the delay. Otherwise the lanes will transition to and
>       remain in the high-speed state immediately after power on.
> 
>       This is meant to help CSI-2 receivers synchronizing their D-PHY
>       RX.

Thanks!
I'll fix this in v4 :)

Regards,
Tommaso

> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> > +
> > +  port:
> > +    description: Digital Output Port
> > +    $ref: /schemas/graph.yaml#/$defs/port-base
> > +    additionalProperties: false
> > +
> > +    properties:
> > +      endpoint:
> > +        $ref: /schemas/media/video-interfaces.yaml#
> > +        unevaluatedProperties: false
> > +
> > +        properties:
> > +          link-frequencies: true
> > +
> > +          data-lanes:
> > +            minItems: 1
> > +            items:
> > +              - const: 1
> > +              - const: 2
> > +              - const: 3
> > +              - const: 4
> > +
> > +        required:
> > +          - data-lanes
> > +          - link-frequencies
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - vcc-ext-in-supply
> > +  - port
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    i2c {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        alvium: camera@3c {
> > +            compatible = "alliedvision,alvium-csi2";
> > +            reg = <0x3c>;
> > +            vcc-ext-in-supply = <&reg_vcc_ext_in>;
> > +            alliedvision,lp2hs-delay-us = <20>;
> > +
> > +            port {
> > +                alvium_out: endpoint {
> > +                    remote-endpoint = <&mipi_csi_0_in>;
> > +                    data-lanes = <1 2 3 4>;
> > +                    link-frequencies = /bits/ 64 <681250000>;
> > +                };
> > +            };
> > +        };
> > +    };
> > +
> > +...
> 
> -- 
> Regards,
> 
> Laurent Pinchart