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

[PATCH v4 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>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
Changes since v3:
 - Collected tags from LPinchart, CDooley, KKozlowski

 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

[PATCH v4 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>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.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

Changes since v3:
 - Fixed vcc-ext-in-supply description as suggested by LPinchart
 - Fixed alliedvision,lp2hs-delay-us description as suggested by LPinchart
 - Added maximum to alliedvision,lp2hs-delay-us as suggested by LPinchart
 - Collected Reviewed-by tag from LPinchart

 .../media/i2c/alliedvision,alvium-csi2.yaml   | 96 +++++++++++++++++++
 1 file changed, 96 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..4726d0068229
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/alliedvision,alvium-csi2.yaml
@@ -0,0 +1,96 @@
+# 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: |
+      The regulator that supplies power to the VCC_EXT_IN pins.
+
+  alliedvision,lp2hs-delay-us:
+    maximum: 150000
+    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.
+
+  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 v4 2/3] media: dt-bindings: alvium: add document YAML binding

[Laurent Pinchart]

On Wed, Jun 07, 2023 at 03:19:24PM +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>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.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
> 
> Changes since v3:
>  - Fixed vcc-ext-in-supply description as suggested by LPinchart
>  - Fixed alliedvision,lp2hs-delay-us description as suggested by LPinchart
>  - Added maximum to alliedvision,lp2hs-delay-us as suggested by LPinchart
>  - Collected Reviewed-by tag from LPinchart
> 
>  .../media/i2c/alliedvision,alvium-csi2.yaml   | 96 +++++++++++++++++++
>  1 file changed, 96 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..4726d0068229
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/alliedvision,alvium-csi2.yaml
> @@ -0,0 +1,96 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/i2c/alliedvision,alvium.yaml#

This is still wrong, see Rob's bot reply in the previous version.

> +$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: |
> +      The regulator that supplies power to the VCC_EXT_IN pins.
> +
> +  alliedvision,lp2hs-delay-us:
> +    maximum: 150000
> +    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.
> +
> +  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 v4 2/3] media: dt-bindings: alvium: add document YAML binding

[Krzysztof Kozlowski]

On 07/06/2023 15:19, 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>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.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
> 
> Changes since v3:
>  - Fixed vcc-ext-in-supply description as suggested by LPinchart
>  - Fixed alliedvision,lp2hs-delay-us description as suggested by LPinchart
>  - Added maximum to alliedvision,lp2hs-delay-us as suggested by LPinchart
>  - Collected Reviewed-by tag from LPinchart

You still did not test it before sending. Four versions of which none
were tested :(

Best regards,
Krzysztof

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

[Tommaso Merciai]

On Wed, Jun 07, 2023 at 05:06:38PM +0300, Laurent Pinchart wrote:
> On Wed, Jun 07, 2023 at 03:19:24PM +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>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.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
> > 
> > Changes since v3:
> >  - Fixed vcc-ext-in-supply description as suggested by LPinchart
> >  - Fixed alliedvision,lp2hs-delay-us description as suggested by LPinchart
> >  - Added maximum to alliedvision,lp2hs-delay-us as suggested by LPinchart
> >  - Collected Reviewed-by tag from LPinchart
> > 
> >  .../media/i2c/alliedvision,alvium-csi2.yaml   | 96 +++++++++++++++++++
> >  1 file changed, 96 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..4726d0068229
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/i2c/alliedvision,alvium-csi2.yaml
> > @@ -0,0 +1,96 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/media/i2c/alliedvision,alvium.yaml#
> 
> This is still wrong, see Rob's bot reply in the previous version.

Arg... :'(

Thanks Laurent.

Regards,
Tommaso

> 
> > +$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: |
> > +      The regulator that supplies power to the VCC_EXT_IN pins.
> > +
> > +  alliedvision,lp2hs-delay-us:
> > +    maximum: 150000
> > +    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.
> > +
> > +  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 v4 2/3] media: dt-bindings: alvium: add document YAML binding

[Rob Herring]

On Wed, 07 Jun 2023 15:19:24 +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>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.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
> 
> Changes since v3:
>  - Fixed vcc-ext-in-supply description as suggested by LPinchart
>  - Fixed alliedvision,lp2hs-delay-us description as suggested by LPinchart
>  - Added maximum to alliedvision,lp2hs-delay-us as suggested by LPinchart
>  - Collected Reviewed-by tag from LPinchart
> 
>  .../media/i2c/alliedvision,alvium-csi2.yaml   | 96 +++++++++++++++++++
>  1 file changed, 96 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/20230607131936.382406-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 v4 2/3] media: dt-bindings: alvium: add document YAML binding

[Tommaso Merciai]

Hi Krzysztof,

On Wed, Jun 07, 2023 at 04:18:48PM +0200, Krzysztof Kozlowski wrote:
> On 07/06/2023 15:19, 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>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.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
> > 
> > Changes since v3:
> >  - Fixed vcc-ext-in-supply description as suggested by LPinchart
> >  - Fixed alliedvision,lp2hs-delay-us description as suggested by LPinchart
> >  - Added maximum to alliedvision,lp2hs-delay-us as suggested by LPinchart
> >  - Collected Reviewed-by tag from LPinchart
> 
> You still did not test it before sending. Four versions of which none
> were tested :(

You are right.. my bad. :'(

After fixing id, as suggested by Laurent/bot into:

$id: http://devicetree.org/schemas/media/i2c/alliedvision,alvium-csi2.yaml#

I'm running the following test:

make dt_binding_check DT_SCHEMA_FILES=alliedvision,alvium-csi2.yaml

With the following result:

  LINT    Documentation/devicetree/bindings
  CHKDT   Documentation/devicetree/bindings/processed-schema.json
  SCHEMA  Documentation/devicetree/bindings/processed-schema.json
/home/tom/work/mainline/linux/Documentation/devicetree/bindings/media/i2c/.alliedvision,alvium-csi2.example.dts.pre.yaml: ignoring, error parsing file
  DTEX    Documentation/devicetree/bindings/media/i2c/alliedvision,alvium-csi2.example.dts
  DTC_CHK Documentation/devicetree/bindings/media/i2c/alliedvision,alvium-csi2.example.dtb

Is that correct?
Let me know. Thanks in advance :)

Regards,
Tommaso

> 
> Best regards,
> Krzysztof
> 

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

[Krzysztof Kozlowski]

On 07/06/2023 17:20, Tommaso Merciai wrote:
> Hi Krzysztof,
> 
> On Wed, Jun 07, 2023 at 04:18:48PM +0200, Krzysztof Kozlowski wrote:
>> On 07/06/2023 15:19, 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>
>>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.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
>>>
>>> Changes since v3:
>>>  - Fixed vcc-ext-in-supply description as suggested by LPinchart
>>>  - Fixed alliedvision,lp2hs-delay-us description as suggested by LPinchart
>>>  - Added maximum to alliedvision,lp2hs-delay-us as suggested by LPinchart
>>>  - Collected Reviewed-by tag from LPinchart
>>
>> You still did not test it before sending. Four versions of which none
>> were tested :(
> 
> You are right.. my bad. :'(
> 
> After fixing id, as suggested by Laurent/bot into:
> 
> $id: http://devicetree.org/schemas/media/i2c/alliedvision,alvium-csi2.yaml#
> 
> I'm running the following test:
> 
> make dt_binding_check DT_SCHEMA_FILES=alliedvision,alvium-csi2.yaml
> 
> With the following result:
> 
>   LINT    Documentation/devicetree/bindings
>   CHKDT   Documentation/devicetree/bindings/processed-schema.json
>   SCHEMA  Documentation/devicetree/bindings/processed-schema.json
> /home/tom/work/mainline/linux/Documentation/devicetree/bindings/media/i2c/.alliedvision,alvium-csi2.example.dts.pre.yaml: ignoring, error parsing file
>   DTEX    Documentation/devicetree/bindings/media/i2c/alliedvision,alvium-csi2.example.dts
>   DTC_CHK Documentation/devicetree/bindings/media/i2c/alliedvision,alvium-csi2.example.dtb
> 
> Is that correct?

No, it doesn't look correct. You have error parsing your file. Check
your yaml file and its example DTSI.

Be sure you have also yamlling installed.

Best regards,
Krzysztof

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

[Conor Dooley]

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

Hey Tommaso,

On Wed, Jun 07, 2023 at 03:19:24PM +0200, Tommaso Merciai wrote:

> +  alliedvision,lp2hs-delay-us:
> +    maximum: 150000
> +    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.

Since this new version was posted before I got a chance to reply, I
still don't think it makes sense to allow 0 & then special case it,
when testing for the presence of a property is trivial.
The property should describe some behaviour/property of the hardware,
not be a mechanism to convey what you want to write into registers.

I don't really get why you'd not do:
	If present, 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.

> +static int alvium_get_dt_data(struct alvium_dev *alvium)
> +{
> +	struct device *dev = &alvium->i2c_client->dev;
> +	struct device_node *node = dev->of_node;
> +	struct fwnode_handle *endpoint;
> +	int ret = 0;
> +
> +	if (!node)
> +		return -EINVAL;
> +
> +	ret = fwnode_property_read_u32(dev_fwnode(dev),
> +			  "alliedvision,lp2hs-delay-us",
> +			  &alvium->lp2hs_delay);
> +	if (ret)
> +		dev_info(dev, "lp2hs-delay-us not found\n");

And this print, which I also don't understand the presence of as
well behaving driver should be quiet, goes away.

Cheers,
Conor.

> +
> +	endpoint = fwnode_graph_get_next_endpoint(dev_fwnode(dev), NULL);
> +	if (!endpoint) {
> +		dev_err(dev, "endpoint node not found\n");
> +		return -EINVAL;
> +	}
> +
> +	if (v4l2_fwnode_endpoint_alloc_parse(endpoint, &alvium->ep)) {
> +		dev_err(dev, "could not parse endpoint\n");
> +		return 0;
> +	}
> +
> +	if (alvium->ep.bus_type != V4L2_MBUS_CSI2_DPHY) {
> +		dev_err(dev, "unsupported bus type\n");
> +		return -EINVAL;
> +	}
> +
> +	if (!alvium->ep.nr_of_link_frequencies) {
> +		dev_err(dev, "no link frequencies defined");
> +		return -EINVAL;
> +	}
> +
> +	dev_info(dev, "freq: %llu\n",
> +				    alvium->ep.link_frequencies[0]);
> +	dev_info(dev, "lanes: %d\n",
> +				    alvium->ep.bus.mipi_csi2.num_data_lanes);
> +
> +	return 0;
> +}

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

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

[Tommaso Merciai]

On Wed, Jun 07, 2023 at 06:11:54PM +0200, Krzysztof Kozlowski wrote:
> On 07/06/2023 17:20, Tommaso Merciai wrote:
> > Hi Krzysztof,
> > 
> > On Wed, Jun 07, 2023 at 04:18:48PM +0200, Krzysztof Kozlowski wrote:
> >> On 07/06/2023 15:19, 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>
> >>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.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
> >>>
> >>> Changes since v3:
> >>>  - Fixed vcc-ext-in-supply description as suggested by LPinchart
> >>>  - Fixed alliedvision,lp2hs-delay-us description as suggested by LPinchart
> >>>  - Added maximum to alliedvision,lp2hs-delay-us as suggested by LPinchart
> >>>  - Collected Reviewed-by tag from LPinchart
> >>
> >> You still did not test it before sending. Four versions of which none
> >> were tested :(
> > 
> > You are right.. my bad. :'(
> > 
> > After fixing id, as suggested by Laurent/bot into:
> > 
> > $id: http://devicetree.org/schemas/media/i2c/alliedvision,alvium-csi2.yaml#
> > 
> > I'm running the following test:
> > 
> > make dt_binding_check DT_SCHEMA_FILES=alliedvision,alvium-csi2.yaml
> > 
> > With the following result:
> > 
> >   LINT    Documentation/devicetree/bindings
> >   CHKDT   Documentation/devicetree/bindings/processed-schema.json
> >   SCHEMA  Documentation/devicetree/bindings/processed-schema.json
> > /home/tom/work/mainline/linux/Documentation/devicetree/bindings/media/i2c/.alliedvision,alvium-csi2.example.dts.pre.yaml: ignoring, error parsing file
> >   DTEX    Documentation/devicetree/bindings/media/i2c/alliedvision,alvium-csi2.example.dts
> >   DTC_CHK Documentation/devicetree/bindings/media/i2c/alliedvision,alvium-csi2.example.dtb
> > 
> > Is that correct?
> 
> No, it doesn't look correct. You have error parsing your file. Check
> your yaml file and its example DTSI.
> 
> Be sure you have also yamlling installed.

Thanks for the feedback!

Regards,
Tommaso

> 
> Best regards,
> Krzysztof
> 

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

[Tommaso Merciai]

Hi Conor,

On Wed, Jun 07, 2023 at 06:16:19PM +0100, Conor Dooley wrote:
> Hey Tommaso,
> 
> On Wed, Jun 07, 2023 at 03:19:24PM +0200, Tommaso Merciai wrote:
> 
> > +  alliedvision,lp2hs-delay-us:
> > +    maximum: 150000
> > +    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.
> 
> Since this new version was posted before I got a chance to reply, I
> still don't think it makes sense to allow 0 & then special case it,
> when testing for the presence of a property is trivial.

My bad, sry.
My keyboard is too quick :P

> The property should describe some behaviour/property of the hardware,
> not be a mechanism to convey what you want to write into registers.
> 
> I don't really get why you'd not do:
> 	If present, 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.
> 

You are suggesting the following solution:

minimum: 1
maximum: 150000

Right?

Personally I prefer to keep also 0 but never mind
is ok also this solution. :)

Let me know if I have understood correctly pls.
Thanks in advance :)

> > +static int alvium_get_dt_data(struct alvium_dev *alvium)
> > +{
> > +	struct device *dev = &alvium->i2c_client->dev;
> > +	struct device_node *node = dev->of_node;
> > +	struct fwnode_handle *endpoint;
> > +	int ret = 0;
> > +
> > +	if (!node)
> > +		return -EINVAL;
> > +
> > +	ret = fwnode_property_read_u32(dev_fwnode(dev),
> > +			  "alliedvision,lp2hs-delay-us",
> > +			  &alvium->lp2hs_delay);
> > +	if (ret)
> > +		dev_info(dev, "lp2hs-delay-us not found\n");
> 
> And this print, which I also don't understand the presence of as
> well behaving driver should be quiet, goes away.

Then you are suggesting to drop this print right?

Thanks for your review!

Regards,
Tommaso

> 
> Cheers,
> Conor.
> 
> > +
> > +	endpoint = fwnode_graph_get_next_endpoint(dev_fwnode(dev), NULL);
> > +	if (!endpoint) {
> > +		dev_err(dev, "endpoint node not found\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (v4l2_fwnode_endpoint_alloc_parse(endpoint, &alvium->ep)) {
> > +		dev_err(dev, "could not parse endpoint\n");
> > +		return 0;
> > +	}
> > +
> > +	if (alvium->ep.bus_type != V4L2_MBUS_CSI2_DPHY) {
> > +		dev_err(dev, "unsupported bus type\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (!alvium->ep.nr_of_link_frequencies) {
> > +		dev_err(dev, "no link frequencies defined");
> > +		return -EINVAL;
> > +	}
> > +
> > +	dev_info(dev, "freq: %llu\n",
> > +				    alvium->ep.link_frequencies[0]);
> > +	dev_info(dev, "lanes: %d\n",
> > +				    alvium->ep.bus.mipi_csi2.num_data_lanes);
> > +
> > +	return 0;
> > +}

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

[Conor Dooley]

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

On Wed, Jun 07, 2023 at 09:49:18PM +0200, Tommaso Merciai wrote:
> On Wed, Jun 07, 2023 at 06:16:19PM +0100, Conor Dooley wrote:
> > On Wed, Jun 07, 2023 at 03:19:24PM +0200, Tommaso Merciai wrote:

> You are suggesting the following solution:
> 
> minimum: 1
> maximum: 150000
> 
> Right?
> 
> Personally I prefer to keep also 0 but never mind
> is ok also this solution. :)
> 
> Let me know if I have understood correctly pls.
> Thanks in advance :)

Yup, you got it.

> > > +static int alvium_get_dt_data(struct alvium_dev *alvium)
> > > +{
> > > +	struct device *dev = &alvium->i2c_client->dev;
> > > +	struct device_node *node = dev->of_node;
> > > +	struct fwnode_handle *endpoint;
> > > +	int ret = 0;
> > > +
> > > +	if (!node)
> > > +		return -EINVAL;
> > > +
> > > +	ret = fwnode_property_read_u32(dev_fwnode(dev),
> > > +			  "alliedvision,lp2hs-delay-us",
> > > +			  &alvium->lp2hs_delay);
> > > +	if (ret)
> > > +		dev_info(dev, "lp2hs-delay-us not found\n");
> > 
> > And this print, which I also don't understand the presence of as
> > well behaving driver should be quiet, goes away.
> 
> Then you are suggesting to drop this print right?

Yeah, in general we do not want drivers to be printing things while they
are behaving correctly & not having the property isn't an error ;)

Cheers,
Conor.

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