On Fri, Apr 02, 2021 at 01:01:22PM +0300, Laurent Pinchart wrote:

On Thu, Apr 01, 2021 at 10:52:01AM -0500, Rob Herring wrote:

quoted

On Tue, Mar 30, 2021 at 11:03:44PM +0530, Pratyush Yadav wrote:

quoted

TI's J721E uses the Cadence CSI2RX and DPHY peripherals to facilitate
capture over a CSI-2 bus. The TI CSI2RX platform driver glues all the
parts together.

Signed-off-by: Pratyush Yadav <redacted>
---
 .../devicetree/bindings/media/ti,csi2rx.yaml  | 70 +++++++++++++++++++
 1 file changed, 70 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/ti,csi2rx.yaml

diff --git a/Documentation/devicetree/bindings/media/ti,csi2rx.yaml b/Documentation/devicetree/bindings/media/ti,csi2rx.yaml
new file mode 100644
index 000000000000..ebd894364391
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/ti,csi2rx.yaml

@@ -0,0 +1,70 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/ti,csi2rx.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TI CSI2RX Wrapper Device Tree Bindings
+

A description would be useful, especially given that the TRM doesn't
mention "CSI2RX".

quoted

quoted

+maintainers:
+  - Pratyush Yadav [off-list ref]
+
+properties:
+  compatible:
+    items:
+      - const: ti,csi2rx
+
+  dmas:
+    description: RX DMA Channel 0

items:
  - description: RX DMA Channel 0

Or just 'maxItems: 1'

quoted

+
+  dma-names:
+    items:
+      - const: rx0
+
+  reg:
+    maxItems: 1
+    description: Base address and size of the TI wrapper registers.

That's all 'reg' properties, drop 'description'.

According to SPRUIL1B, there are four register banks for the CSI_RX_IF,
and two register banks for the DPHY_RX. What's your plan to support
these ? Not everything need to be implemented at once, but backward
compatibility need to be taken into account in the design.

quoted

quoted

+
+  power-domains:
+    maxItems: 1
+    description:
+      PM domain provider node and an args specifier containing
+      the device id value.

Drop.

quoted

+
+  ranges: true
+
+  "#address-cells":
+    const: 2
+
+  "#size-cells":
+    const: 2
+
+patternProperties:
+  "csi-bridge@":

"^csi-bridge@"

quoted

+    type: object
+    description: CSI2 bridge node.

Just an empty node?

Even if the node is optional, it would be useful to include it in the
example below, to show how it's supposed to be used.

quoted

quoted

+
+required:
+  - compatible
+  - reg
+  - dmas
+  - dma-names
+  - power-domains
+  - "#address-cells"
+  - "#size-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/soc/ti,sci_pm_domain.h>
+
+    ti_csi2rx0: ticsi2rx {
+        compatible = "ti,csi2rx";
+        dmas = <&main_udmap 0x4940>;
+        dma-names = "rx0";
+        reg = <0x0 0x4500000 0x0 0x1000>;
+        power-domains = <&k3_pds 26 TI_SCI_PD_EXCLUSIVE>;
+        #address-cells = <2>;
+        #size-cells = <2>;
+    };

It would also be useful to expand this to a full example that includes
integration with the PHY.

-- 
Regards,

Laurent Pinchart