Re: [PATCH v4 3/8] dt-bindings: media: add bindings for TI DS90UB960

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Rob,

On 02/11/2022 19:26, Rob Herring wrote:
On Tue, Nov 01, 2022 at 03:20:27PM +0200, Tomi Valkeinen wrote:
Add DT bindings for TI DS90UB960 FPDLink-3 Deserializer.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@xxxxxxxxxxxxxxxx>
---
  .../bindings/media/i2c/ti,ds90ub960.yaml      | 392 ++++++++++++++++++
  1 file changed, 392 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml

diff --git a/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml b/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml
new file mode 100644
index 000000000000..4456d9b3e2c7
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml
@@ -0,0 +1,392 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/i2c/ti,ds90ub960.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Texas Instruments DS90UB9XX Family FPD-Link Deserializer Hubs
+
+maintainers:
+  - Tomi Valkeinen <tomi.valkeinen@xxxxxxxxxxxxxxxx>
+
+description: |

Don't need '|'

Hmm, ok... But why does that work? I can only find yaml examples for multi-line with either | or >.

+  The TI DS90UB9XX devices are FPD-Link video deserializers with I2C and GPIO
+  forwarding.
+
+properties:
+  compatible:
+    enum:
+      - ti,ds90ub960-q1
+      - ti,ds90ub9702-q1
+
+  reg:
+    maxItems: 1
+    description:
+      i2c addresses for the deserializer and the serializers
+
+  reg-names:
+    items:
+      - const: main

'reg-names' is not all that useful with only 1 entry.

True.

+
+  clocks:
+    maxItems: 1
+    description:
+      Reference clock connected to the REFCLK pin.
+
+  clock-names:
+    items:
+      - const: refclk
+
+  powerdown-gpios:
+    maxItems: 1
+    description:
+      Specifier for the GPIO connected to the PDB pin.
+
+  i2c-alias-pool:

Something common or could be? If not, then needs a vendor prefix.

I'll have to think about this. It is related to the i2c-atr, so I think it might be a common thing.

+    $ref: /schemas/types.yaml#/definitions/uint16-array
+    description:
+      i2c alias pool for remote devices.

Needs a better description. What's an 'alias pool'?

Right.

"i2c alias pool is a pool of i2c addresses on the main i2c bus that can be used to access the remote peripherals. Each remote peripheral is assigned an alias from the pool, and transactions to that address will be forwarded to the remote peripheral, with the address translated to the remote peripheral's real address."

0-0xffff are valid values?

They are i2c addresses, and linux i2c uses u16 for addresses. Then again, the fpdlink devices only support 7-bit addresses, so maybe this could be an uint8 array. I am not sure what's the best way to define this.

+
+  links:
+    type: object
+    additionalProperties: false
+
+    properties:
+      '#address-cells':
+        const: 1
+
+      '#size-cells':
+        const: 0
+
+      manual-strobe:
+        type: boolean
+        description:
+          Enable manual strobe position and EQ level
+
+    patternProperties:
+      '^link@[0-9a-f]+$':
+        type: object
+        additionalProperties: false
+        properties:
+          reg:
+            description: The link number
+            maxItems: 1
+
+          i2c-alias:

Vendor prefix.

+            description: |
+              The i2c address used for the serializer. Transactions to this
+              address on the i2c bus where the deserializer resides are
+              forwarded to the serializer.
+
+          rx-mode:

Vendor prefix. And so on...

Yes, I totally missed these.

+            $ref: /schemas/types.yaml#/definitions/uint32
+            enum:
+              - 0 # RAW10
+              - 1 # RAW12 HF
+              - 2 # RAW12 LF
+              - 3 # CSI2 SYNC
+              - 4 # CSI2 NON-SYNC
+            description: FPD-Link Input Mode
+
+          cdr-mode:
+            $ref: /schemas/types.yaml#/definitions/uint32
+            enum:
+              - 0 # FPD3
+              - 1 # FPD4
+            description: FPD-Link CDR Mode
+
+          strobe-pos:
+            $ref: /schemas/types.yaml#/definitions/int32
+            minimum: -13
+            maximum: 13
+            description: Manual strobe position, from -13 to 13

No need to put constraints in free form text.

Ok.

+
+          eq-level:
+            $ref: /schemas/types.yaml#/definitions/uint32
+            maximum: 14
+            description: Manual EQ level, from 0 to 14
+
+          serializer:
+            type: object
+            description: FPD-Link Serializer node
+
+        required:
+          - reg
+          - i2c-alias
+          - rx-mode
+          - serializer
+
+  ports:
+    $ref: /schemas/graph.yaml#/properties/ports
+
+    properties:
+      port@0:
+        $ref: /schemas/graph.yaml#/$defs/port-base

            additionalProperties: false

This gives me check errors about the port's 'reg' property. Using 'unevaluatedProperties' works fine. Is 'unevaluatedProperties' correct, or am I missing something here?

+        description: FPD-Link input 0
+
+        properties:
+          endpoint:
+            $ref: /schemas/media/video-interfaces.yaml#

                unevaluatedProperties: false

Same for the other port nodes

Yep.

+
+      port@1:
+        $ref: /schemas/graph.yaml#/$defs/port-base
+        description: FPD-Link input 1
+
+        properties:
+          endpoint:
+            $ref: /schemas/media/video-interfaces.yaml#
+
+      port@2:
+        $ref: /schemas/graph.yaml#/$defs/port-base
+        description: FPD-Link input 2
+
+        properties:
+          endpoint:
+            $ref: /schemas/media/video-interfaces.yaml#
+
+      port@3:
+        $ref: /schemas/graph.yaml#/$defs/port-base
+        description: FPD-Link input 3
+
+        properties:
+          endpoint:
+            $ref: /schemas/media/video-interfaces.yaml#
+
+      port@4:
+        $ref: /schemas/graph.yaml#/$defs/port-base
+        description: CSI-2 Output 0
+
+        properties:
+          endpoint:
+            $ref: /schemas/media/video-interfaces.yaml#
+
+            properties:
+              clock-lanes:
+                maxItems: 1
+
+              data-lanes:
+                minItems: 1
+                maxItems: 4

Why the constraints on this endpoint? Are the other ones actually using
properties from video-interfaces.yaml? If not, then just reference
/properties/port and drop 'endpoint' instead.

The ports 0-3 do not use any properties from video-interfaces.yaml, so I'll drop the endpoint.

Ports 4,5 are CSI-2 ports and need the clock-lanes and data-lanes to be defined.

Is there something wrong with the constraints, or were you just wondering about the difference between ports 0-3 and 4,5

+
+      port@5:
+        $ref: /schemas/graph.yaml#/$defs/port-base
+        description: CSI-2 Output 1
+
+        properties:
+          endpoint:
+            $ref: /schemas/media/video-interfaces.yaml#
+
+            properties:
+              clock-lanes:
+                maxItems: 1
+
+              data-lanes:
+                minItems: 1
+                maxItems: 4
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - ports
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    i2c {
+      clock-frequency = <400000>;
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      deser@3d {
+        compatible = "ti,ds90ub960-q1";
+
+        reg-names = "main";
+        reg       = <0x3d>;
+
+        clock-names = "refclk";
+        clocks = <&fixed_clock>;
+
+        powerdown-gpios = <&pca9555 7 GPIO_ACTIVE_LOW>;
+
+        i2c-alias-pool = /bits/ 16 <0x4a 0x4b 0x4c 0x4d 0x4e 0x4f>;
+
+        ports {
+          #address-cells = <1>;
+          #size-cells = <0>;
+
+          /* Port 0, Camera 0 */
+          port@0 {
+            reg = <0>;
+
+            ub960_fpd3_1_in: endpoint {
+              remote-endpoint = <&ub953_1_out>;
+
+              rx-mode = <0>;

Looks like this is not defined under 'endpoint'.

Indeed, and after adding the 'unevaluatedProperties' I do get a warning here.

+            };
+          };
+
+          /* Port 0, Camera 1 */
+          port@1 {
+            reg = <1>;
+
+            ub960_fpd3_2_in: endpoint {
+              remote-endpoint = <&ub913_2_out>;
+
+              rx-mode = <0>;
+            };
+          };
+
+          /* Port 4, CSI-2 TX */
+          port@4 {
+            reg = <4>;
+            ds90ub960_0_csi_out: endpoint {
+              clock-lanes = <0>;
+              data-lanes = <1 2 3 4>;
+              link-frequencies = /bits/ 64 <800000000>;
+              remote-endpoint = <&csi2_phy0>;
+            };
+          };
+        };
+
+        links {
+          #address-cells = <1>;
+          #size-cells = <0>;
+
+          /* Link 0 has DS90UB953 serializer and IMX390 sensor */
+
+          link@0 {
+            reg = <0>;
+            i2c-alias = <68>;
+
+            rx-mode = <3>;
+
+            serializer1: serializer {
+              compatible = "ti,ds90ub953-q1";
+
+              gpio-controller;
+              #gpio-cells = <2>;
+
+              #clock-cells = <0>;
+
+              ports {
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                port@0 {
+                  reg = <0>;
+                  ub953_1_in: endpoint {
+                    clock-lanes = <0>;
+                    data-lanes = <1 2 3 4>;
+                    remote-endpoint = <&sensor_1_out>;
+                  };
+                };
+
+                port@1 {
+                  reg = <1>;
+
+                  ub953_1_out: endpoint {
+                    remote-endpoint = <&ub960_fpd3_1_in>;
+                  };
+                };
+              };
+
+              i2c {
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                sensor@21 {
+                  compatible = "sony,imx390";

DT_CHECKER_FLAGS=-m gives a warning here, as sony,imx390 is not in upstream. The sensor details are not really relevant here, but I used the data for the setup I have.

Should I instead use some sensor here that is in upstream, which I think should work with the fpdlink ICs?

 Tomi




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux