Re: [PATCH v12 1/8] dt-bindings: media: platform: visconti: Add Toshiba Visconti MIPI CSI-2 Receiver

Hello Ishikawa-san,

Thank you for the patch.

On Mon, Nov 25, 2024 at 06:21:39PM +0900, Yuji Ishikawa wrote:

As Krzysztof mentioned, '|-' isn't needed. See
https://yaml-multiline.info/ for more information. The literal block
style indicator ('|') is only needed when line breaks need to be
preserved, e.g. when the description contains ASCII art.

+  Toshiba Visconti5 SoC MIPI CSI-2 receiver device receives MIPI CSI-2 video
+  stream. Use with VIIF device. T.B.D

T.B.D ?

+
+properties:
+  compatible:
+    const: toshiba,visconti5-csi2rx
+
+  reg:
+    items:
+      - description: Registers for CSI2 receiver control
+
+  interrupts:
+    items:
+      - description: CSI2 Receiver Interrupt
+
+  ports:
+    $ref: /schemas/graph.yaml#/properties/ports
+
+    properties:
+      port@0:
+        $ref: /schemas/graph.yaml#/$defs/port-base
+        unevaluatedProperties: false
+        description:
+          Input port node, single endpoint describing the CSI-2 transmitter.
+
+        properties:
+          endpoint:
+            $ref: video-interfaces.yaml#

Please use a full path for the ref:

            $ref: /schemas/media/video-interfaces.yaml#

+            unevaluatedProperties: false
+
+            properties:
+              data-lanes:
+                description: CSI2 receiver supports 1, 2, 3 or 4 data lanes

You can drop the description. The video-interfaces.yaml schema has a
more complete description, and the fact that the receiver supports
between 1 and 4 lanes is conveyed by minItems and items below.

+                minItems: 1
+                items:
+                  - const: 1
+                  - const: 2
+                  - const: 3
+                  - const: 4
+            required:
+              - data-lanes
+
+      port@1:
+        $ref: /schemas/graph.yaml#/properties/port
+        description:
+          Output port node, single endpoint describing the Visconti VIIF.
+
+    required:
+      - port@0
+      - port@1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - ports
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        csi2rx@1c008000 {

Node names should describe the function of the node, not the precise
model of the device. "csi2" would be a more appropriate name.

+            compatible = "toshiba,visconti5-csi2rx";
+            reg = <0 0x1c008000 0 0x400>;
+            interrupts = <GIC_SPI 73 IRQ_TYPE_LEVEL_HIGH>;
+
+            ports {
+                #address-cells = <1>;
+                #size-cells = <0>;
+                port@0 {
+                    reg = <0>;
+                    csi2rx_in0: endpoint {
+                        data-lanes = <1 2>;
+                        remote-endpoint = <&imx219_out0>;
+                    };
+                };
+                port@1 {
+                    reg = <1>;
+                    csi2rx_out0: endpoint {
+                        remote-endpoint = <&csi_in0>;
+                    };
+                };
+            };
+        };
+    };

-- 
Regards,

Laurent Pinchart