diff mbox series

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

Message ID 20221101132032.1542416-4-tomi.valkeinen@ideasonboard.com
State Superseded
Headers show
Series i2c-atr and FPDLink | expand

Commit Message

Tomi Valkeinen Nov. 1, 2022, 1:20 p.m. UTC
Add DT bindings for TI DS90UB960 FPDLink-3 Deserializer.

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

Comments

Rob Herring Nov. 1, 2022, 4:47 p.m. UTC | #1
On Tue, 01 Nov 2022 15:20:27 +0200, Tomi Valkeinen wrote:
> Add DT bindings for TI DS90UB960 FPDLink-3 Deserializer.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
>  .../bindings/media/i2c/ti,ds90ub960.yaml      | 392 ++++++++++++++++++
>  1 file changed, 392 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.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/ti,ds90ub960.example.dtb:0:0: /example-0/i2c/deser@3d/links/link@0/serializer: failed to match any schema with compatible: ['ti,ds90ub953-q1']
Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.example.dtb:0:0: /example-0/i2c/deser@3d/links/link@0/serializer/i2c/sensor@21: failed to match any schema with compatible: ['sony,imx390']
Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.example.dtb:0:0: /example-0/i2c/deser@3d/links/link@1/serializer: failed to match any schema with compatible: ['ti,ds90ub913a-q1']
Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.example.dtb:0:0: /example-0/i2c/deser@3d/links/link@1/serializer/i2c/sensor@30: failed to match any schema with compatible: ['ovti,ov10635']

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

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.
Rob Herring Nov. 2, 2022, 5:26 p.m. UTC | #2
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@ideasonboard.com>
> ---
>  .../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@ideasonboard.com>
> +
> +description: |

Don't need '|'

> +  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.

> +
> +  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.

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

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

0-0xffff are valid values?

> +
> +  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...

> +            $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.

> +
> +          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

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

               unevaluatedProperties: false

Same for the other port nodes

> +
> +      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.

> +
> +      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'.

> +            };
> +          };
> +
> +          /* 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";
> +                  reg = <0x21>;
> +
> +                  clocks = <&clk_cam_27M>;
> +                  clock-names = "inck";
> +
> +                  xclr-gpios = <&serializer1 0 GPIO_ACTIVE_LOW>;
> +                  error0-gpios = <&serializer1 1 GPIO_ACTIVE_HIGH>;
> +                  error1-gpios = <&serializer1 2 GPIO_ACTIVE_HIGH>;
> +                  comready-gpios = <&serializer1 3 GPIO_ACTIVE_HIGH>;
> +
> +                  port {
> +                    sensor_1_out: endpoint {
> +                      remote-endpoint = <&ub953_1_in>;
> +                    };
> +                  };
> +                };
> +              };
> +            };
> +          };  /* End of link@0 */
> +
> +          /* Link 1 has DS90UB913 serializer and OV10635 sensor */
> +
> +          link@1 {
> +            reg = <1>;
> +            i2c-alias = <69>;
> +
> +            rx-mode = <0>;
> +
> +            serializer2: serializer {
> +              compatible = "ti,ds90ub913a-q1";
> +
> +              gpio-controller;
> +              #gpio-cells = <2>;
> +
> +              clocks = <&clk_cam_48M>;
> +              clock-names = "clkin";
> +
> +              #clock-cells = <0>;
> +
> +              ports {
> +                #address-cells = <1>;
> +                #size-cells = <0>;
> +
> +                port@0 {
> +                  reg = <0>;
> +                  ub913_2_in: endpoint {
> +                    remote-endpoint = <&sensor_2_out>;
> +                  };
> +                };
> +
> +                port@1 {
> +                  reg = <1>;
> +
> +                  ub913_2_out: endpoint {
> +                    remote-endpoint = <&ub960_fpd3_2_in>;
> +                  };
> +                };
> +              };
> +
> +              i2c {
> +                #address-cells = <1>;
> +                #size-cells = <0>;
> +
> +                sensor@30 {
> +                  compatible = "ovti,ov10635";
> +                  reg = <0x30>;
> +
> +                  clocks = <&serializer2>;
> +                  clock-names = "xvclk";
> +
> +                  powerdown-gpios = <&serializer2 0 GPIO_ACTIVE_HIGH>;
> +
> +                  port {
> +                    sensor_2_out: endpoint {
> +                      remote-endpoint = <&ub913_2_in>;
> +                      hsync-active = <1>;
> +                      vsync-active = <1>;
> +                      pclk-sample = <0>;
> +                      bus-width = <10>;
> +                    };
> +                  };
> +                };
> +              };
> +            };
> +          }; /* End of link@1 */
> +        };
> +      };
> +    };
> +...
> -- 
> 2.34.1
> 
>
Tomi Valkeinen Nov. 3, 2022, 11:50 a.m. UTC | #3
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@ideasonboard.com>
>> ---
>>   .../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@ideasonboard.com>
>> +
>> +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
Vaittinen, Matti Nov. 3, 2022, 12:13 p.m. UTC | #4
On 11/3/22 13:50, Tomi Valkeinen wrote:
> Hi Rob,
> 
> On 02/11/2022 19:26, Rob Herring wrote:
>> On Tue, Nov 01, 2022 at 03:20:27PM +0200, Tomi Valkeinen wrote:
>>> +
>>> +  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.

I'd say this should be common. Where the i2c-atr properties should live 
is another question though. If the I2C-atr stays as a genericly usable 
component - then these bindings should be in a file that can be 
referenced by other I2C-atr users (like the UB960 here).

// snip

>>> +
>>> +          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.


I think the i2c-alias might need to be common as well?

Best Regards
	-- Matti Vaittinen
Tomi Valkeinen Nov. 3, 2022, 12:32 p.m. UTC | #5
On 03/11/2022 14:13, Vaittinen, Matti wrote:
> On 11/3/22 13:50, Tomi Valkeinen wrote:
>> Hi Rob,
>>
>> On 02/11/2022 19:26, Rob Herring wrote:
>>> On Tue, Nov 01, 2022 at 03:20:27PM +0200, Tomi Valkeinen wrote:
>>>> +
>>>> +  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.
> 
> I'd say this should be common. Where the i2c-atr properties should live
> is another question though. If the I2C-atr stays as a genericly usable
> component - then these bindings should be in a file that can be
> referenced by other I2C-atr users (like the UB960 here).

Yep. All the links, link, serializer and alias nodes/properties are new 
things here, and I guess these could be used by other deser-ser systems. 
That said, I don't have any experience with other systems.

> // snip
> 
>>>> +
>>>> +          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.
> 
> 
> I think the i2c-alias might need to be common as well?

Perhaps...

I was also thinking that the serializer addresses could be taken from 
the i2c-alias-pool. But maybe that's not a good idea, as the 
serializer-access and remote-peripheral-access are a bit different (e.g. 
no ATR when accessing the serializer).

And I was thinking that, at least here, the alias addresses can be 
"anything", so they could be reserved dynamically at runtime, without 
any predefined aliases. But that might be a bit confusing to the user.

  Tomi
Luca Ceresoli Nov. 11, 2022, 4:26 p.m. UTC | #6
Hello Tomi, Matti, Wolfram,

On Thu, 3 Nov 2022 14:32:02 +0200
Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> wrote:

> On 03/11/2022 14:13, Vaittinen, Matti wrote:
> > On 11/3/22 13:50, Tomi Valkeinen wrote:  
> >> Hi Rob,
> >>
> >> On 02/11/2022 19:26, Rob Herring wrote:  
> >>> On Tue, Nov 01, 2022 at 03:20:27PM +0200, Tomi Valkeinen wrote:  
> >>>> +
> >>>> +  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.  
> > 
> > I'd say this should be common. Where the i2c-atr properties should live
> > is another question though. If the I2C-atr stays as a genericly usable
> > component - then these bindings should be in a file that can be
> > referenced by other I2C-atr users (like the UB960 here).  
> 
> Yep. All the links, link, serializer and alias nodes/properties are new 
> things here, and I guess these could be used by other deser-ser systems. 
> That said, I don't have any experience with other systems.

The i2c-alias-pool was discussed during the RFC,v2 review [1] and it
was agreed that it should be generic. The same principle should apply
to the other ATR properties.

That said, at some point it was also decided that the alias pool should
just be ditched in favor of an automatic selection of an unused address
by the i2c core [2] [3]. Maybe that idea has changed, definitely some
i2c core things needed to be omdified for it to happen, but overall I'm
still convinced automatic assignment without a pool was a good idea.

> >>>> +
> >>>> +          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.  
> > 
> > 
> > I think the i2c-alias might need to be common as well?  
> 
> Perhaps...
> 
> I was also thinking that the serializer addresses could be taken from 
> the i2c-alias-pool. But maybe that's not a good idea, as the 
> serializer-access and remote-peripheral-access are a bit different (e.g. 
> no ATR when accessing the serializer).
> 
> And I was thinking that, at least here, the alias addresses can be 
> "anything", so they could be reserved dynamically at runtime, without 
> any predefined aliases. But that might be a bit confusing to the user.

I think the serialized alias selection is in an intermediate situation
between the deser and the remove chips. For the deser there is a
physical address, nothing strange. For the remote chips, lots of things
could happen, including having several chips. The serializer is in
known quantity (one per connector) and the fact that it is "on the
remote bus" is also somewhat false as it does create the remote bus
itself. Thus for me it would make sense to keep the current structure
(which is quite simple) but mine in not a strong position.

[1]
https://lore.kernel.org/lkml/CAL_JsqKDkixeDJJVxbzWebD6nqMHyk6QqDGSKrQho0THjLdmKQ@mail.gmail.com/

[2] https://lore.kernel.org/lkml/20190903093455.GD1020@kunai/

[3] https://lucaceresoli.net/plumbers-i2c-bof/
Luca Ceresoli Nov. 11, 2022, 4:32 p.m. UTC | #7
Hi Tomi,

On Thu, 3 Nov 2022 13:50:43 +0200
Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> wrote:

> 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@ideasonboard.com>
> >> ---
> >>   .../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@ideasonboard.com>
> >> +
> >> +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."

Good description, but I think re-adding this sentence from my v2 would
be useful:

  list of I2C addresses that are known to be available on the"local"
  (SoC-to-deser) I2C bus

> > 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.

In DT the Linux implementation is irrelevant. Also if we want the ATR
code to be generic we must prepare DT bindings for future devices.
Thus, being these I2C addresses, they could be 7 or 10 bits, thus the
need for at least 16 bits.

All the above will become irrelevant in case the alias pool will
disappear, though.
Luca Ceresoli Nov. 11, 2022, 4:35 p.m. UTC | #8
Hi Tomi,

On Tue,  1 Nov 2022 15:20:27 +0200
Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> wrote:

> Add DT bindings for TI DS90UB960 FPDLink-3 Deserializer.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

...

> +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>;
> +            };
> +          };
> +
> +          /* Port 0, Camera 1 */

I guess you mean "Port 1" here.
Tomi Valkeinen Dec. 8, 2022, 9:23 a.m. UTC | #9
Hi Luca,

On 11/11/2022 18:26, Luca Ceresoli wrote:
> Hello Tomi, Matti, Wolfram,
> 
> On Thu, 3 Nov 2022 14:32:02 +0200
> Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> wrote:
> 
>> On 03/11/2022 14:13, Vaittinen, Matti wrote:
>>> On 11/3/22 13:50, Tomi Valkeinen wrote:
>>>> Hi Rob,
>>>>
>>>> On 02/11/2022 19:26, Rob Herring wrote:
>>>>> On Tue, Nov 01, 2022 at 03:20:27PM +0200, Tomi Valkeinen wrote:
>>>>>> +
>>>>>> +  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.
>>>
>>> I'd say this should be common. Where the i2c-atr properties should live
>>> is another question though. If the I2C-atr stays as a genericly usable
>>> component - then these bindings should be in a file that can be
>>> referenced by other I2C-atr users (like the UB960 here).
>>
>> Yep. All the links, link, serializer and alias nodes/properties are new
>> things here, and I guess these could be used by other deser-ser systems.
>> That said, I don't have any experience with other systems.
> 
> The i2c-alias-pool was discussed during the RFC,v2 review [1] and it
> was agreed that it should be generic. The same principle should apply
> to the other ATR properties.
> 
> That said, at some point it was also decided that the alias pool should
> just be ditched in favor of an automatic selection of an unused address
> by the i2c core [2] [3]. Maybe that idea has changed, definitely some
> i2c core things needed to be omdified for it to happen, but overall I'm
> still convinced automatic assignment without a pool was a good idea.

Yes, the serializer and the remote peripheral i2c aliases can be 
dynamically reserved at runtime, so the i2c-alias-pool and the i2c-alias 
are, in that sense, not needed.

I haven't looked at this in depth yet, but reading the references you 
gave, it sounds like it's not quite clear what addresses are available 
and what are not.

On the other hand, is dynamic i2c address reservation something that the 
users expect to happen? All i2c devices I have used have always had a 
fixed address in the DT, even if at times the devices may support 
choosing between a few different addresses.

Keeping with that tradition, would it be best to just use fixed i2c 
aliases, defined in the DT, for the serializers and the remote 
peripherals? In the current series this is already the case for 
serializers (with i2c-alias property), but we could do something similar 
for the remote peripherals.

  Tomi
diff mbox series

Patch

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@ideasonboard.com>
+
+description: |
+  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
+
+  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:
+    $ref: /schemas/types.yaml#/definitions/uint16-array
+    description:
+      i2c alias pool for remote devices.
+
+  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:
+            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:
+            $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
+
+          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
+        description: FPD-Link input 0
+
+        properties:
+          endpoint:
+            $ref: /schemas/media/video-interfaces.yaml#
+
+      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
+
+      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>;
+            };
+          };
+
+          /* 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";
+                  reg = <0x21>;
+
+                  clocks = <&clk_cam_27M>;
+                  clock-names = "inck";
+
+                  xclr-gpios = <&serializer1 0 GPIO_ACTIVE_LOW>;
+                  error0-gpios = <&serializer1 1 GPIO_ACTIVE_HIGH>;
+                  error1-gpios = <&serializer1 2 GPIO_ACTIVE_HIGH>;
+                  comready-gpios = <&serializer1 3 GPIO_ACTIVE_HIGH>;
+
+                  port {
+                    sensor_1_out: endpoint {
+                      remote-endpoint = <&ub953_1_in>;
+                    };
+                  };
+                };
+              };
+            };
+          };  /* End of link@0 */
+
+          /* Link 1 has DS90UB913 serializer and OV10635 sensor */
+
+          link@1 {
+            reg = <1>;
+            i2c-alias = <69>;
+
+            rx-mode = <0>;
+
+            serializer2: serializer {
+              compatible = "ti,ds90ub913a-q1";
+
+              gpio-controller;
+              #gpio-cells = <2>;
+
+              clocks = <&clk_cam_48M>;
+              clock-names = "clkin";
+
+              #clock-cells = <0>;
+
+              ports {
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                port@0 {
+                  reg = <0>;
+                  ub913_2_in: endpoint {
+                    remote-endpoint = <&sensor_2_out>;
+                  };
+                };
+
+                port@1 {
+                  reg = <1>;
+
+                  ub913_2_out: endpoint {
+                    remote-endpoint = <&ub960_fpd3_2_in>;
+                  };
+                };
+              };
+
+              i2c {
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                sensor@30 {
+                  compatible = "ovti,ov10635";
+                  reg = <0x30>;
+
+                  clocks = <&serializer2>;
+                  clock-names = "xvclk";
+
+                  powerdown-gpios = <&serializer2 0 GPIO_ACTIVE_HIGH>;
+
+                  port {
+                    sensor_2_out: endpoint {
+                      remote-endpoint = <&ub913_2_in>;
+                      hsync-active = <1>;
+                      vsync-active = <1>;
+                      pclk-sample = <0>;
+                      bus-width = <10>;
+                    };
+                  };
+                };
+              };
+            };
+          }; /* End of link@1 */
+        };
+      };
+    };
+...