[PATCH V2 0/4] Add support for DS90UB954-Q1

[PATCH V2 0/4] Add support for DS90UB954-Q1

[Yemike Abhilash Chandra]

DS90UB954-Q1 is an FPDLink-III deserializer that is mostly register
compatible with DS90UB960-Q1. The main difference is that it supports
half of the RX and TX ports, i.e. 2x FPDLink RX ports and 1x CSI TX
port. Therefore, add support for DS90UB954 within the existing DS90UB960
bindings and the driver.

Changelog:
Changes in v2:
- Refactor the port definitions in the DT bindings and then add support for the DS90UB954.
- Use enums for the chip type and chip family.
- Some status registers are reserved on the DS90UB954 and always read as zero in log_status.
  Skip these registers.
- Move the link-frequency check for the DS90UB954 into the existing block that validates
  allowed link frequencies for the DS90UB960, and check for DS90UB954 first and then 1200 MHz
  next, as this ordering is more logical to readers.
- The strobe setting registers differ slightly between the DS90UB960 and the DS90UB954. Update
  the code to accommodate these differences.
- Although REFCLK_FREQ measurement is not synchronized on the DS90UB954, a single read is
  practically sufficient. Remove the loop that performs two reads.
- Fix a few minor issues in the Kconfig description and code comments.

Note: I did not collect the ACK from Conor since there is significant change in the bindings in v2.

Test logs: https://gist.github.com/Yemike-Abhilash-Chandra/ca582375fe682221c6597e60f247d92f
DT binding check results: https://gist.github.com/Yemike-Abhilash-Chandra/bd6050d021f72a78ac82b3b342e923f2
Link for v1: https://lore.kernel.org/all/20250523083655.3876005-1-y-abhilashchandra@ti.com/

Yemike Abhilash Chandra (4):
  media: dt-bindings: ti,ds90ub960: Refactor port definitions
  media: i2c: ds90ub960: Use enums for chip type and chip family
  media: dt-bindings: ti,ds90ub960: Add support for DS90UB954-Q1
  media: i2c: ds90ub960: Add support for DS90UB954-Q1

 .../bindings/media/i2c/ti,ds90ub960.yaml      | 400 +++++++++++++-----
 drivers/media/i2c/Kconfig                     |   4 +-
 drivers/media/i2c/ds90ub960.c                 | 221 +++++++---
 3 files changed, 461 insertions(+), 164 deletions(-)

-- 
2.34.1

[PATCH V2 1/4] media: dt-bindings: ti,ds90ub960: Refactor port definitions

[Yemike Abhilash Chandra]

The current bindings duplicate the port definitions for each FPD-Link RX
and CSI-2 TX ports. This results in a large amount of repeated schema
blocks and makes it harder to extend the bindings for new devices.

Refactor the bindings by introducing shared deftinitions for FPD-Link
input ports and CSI-2 output ports. No functional change intended.

Signed-off-by: Yemike Abhilash Chandra <y-abhilashchandra@ti.com>
---
 .../bindings/media/i2c/ti,ds90ub960.yaml      | 120 +++++++-----------
 1 file changed, 44 insertions(+), 76 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml b/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml
index 0539d52de422..6a78288aebaa 100644
--- a/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml
+++ b/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml
@@ -125,102 +125,35 @@ properties:
 
   ports:
     $ref: /schemas/graph.yaml#/properties/ports
+    description: |
+      Ports represent FPD-Link inputs to the deserializer and CSI TX outputs from the deserializer.
+      Their number is model-dependent.
 
     properties:
       port@0:
-        $ref: /schemas/graph.yaml#/$defs/port-base
-        unevaluatedProperties: false
+        $ref: '#/$defs/FPDLink-input-port'
         description: FPD-Link input 0
 
-        properties:
-          endpoint:
-            $ref: /schemas/media/video-interfaces.yaml#
-            unevaluatedProperties: false
-            description:
-              Endpoint for FPD-Link port. If the RX mode for this port is RAW,
-              hsync-active and vsync-active must be defined.
-
       port@1:
-        $ref: /schemas/graph.yaml#/$defs/port-base
-        unevaluatedProperties: false
+        $ref: '#/$defs/FPDLink-input-port'
         description: FPD-Link input 1
 
-        properties:
-          endpoint:
-            $ref: /schemas/media/video-interfaces.yaml#
-            unevaluatedProperties: false
-            description:
-              Endpoint for FPD-Link port. If the RX mode for this port is RAW,
-              hsync-active and vsync-active must be defined.
-
       port@2:
-        $ref: /schemas/graph.yaml#/$defs/port-base
-        unevaluatedProperties: false
+        $ref: '#/$defs/FPDLink-input-port'
         description: FPD-Link input 2
 
-        properties:
-          endpoint:
-            $ref: /schemas/media/video-interfaces.yaml#
-            unevaluatedProperties: false
-            description:
-              Endpoint for FPD-Link port. If the RX mode for this port is RAW,
-              hsync-active and vsync-active must be defined.
-
       port@3:
-        $ref: /schemas/graph.yaml#/$defs/port-base
-        unevaluatedProperties: false
+        $ref: '#/$defs/FPDLink-input-port'
         description: FPD-Link input 3
 
-        properties:
-          endpoint:
-            $ref: /schemas/media/video-interfaces.yaml#
-            unevaluatedProperties: false
-            description:
-              Endpoint for FPD-Link port. If the RX mode for this port is RAW,
-              hsync-active and vsync-active must be defined.
-
       port@4:
-        $ref: /schemas/graph.yaml#/$defs/port-base
-        unevaluatedProperties: false
+        $ref: '#/$defs/CSI2-output-port'
         description: CSI-2 Output 0
 
-        properties:
-          endpoint:
-            $ref: /schemas/media/video-interfaces.yaml#
-            unevaluatedProperties: false
-
-            properties:
-              data-lanes:
-                minItems: 1
-                maxItems: 4
-              link-frequencies:
-                maxItems: 1
-
-            required:
-              - data-lanes
-              - link-frequencies
-
       port@5:
-        $ref: /schemas/graph.yaml#/$defs/port-base
-        unevaluatedProperties: false
+        $ref: '#/$defs/CSI2-output-port'
         description: CSI-2 Output 1
 
-        properties:
-          endpoint:
-            $ref: /schemas/media/video-interfaces.yaml#
-            unevaluatedProperties: false
-
-            properties:
-              data-lanes:
-                minItems: 1
-                maxItems: 4
-              link-frequencies:
-                maxItems: 1
-
-            required:
-              - data-lanes
-              - link-frequencies
-
     required:
       - port@0
       - port@1
@@ -236,6 +169,41 @@ required:
   - clock-names
   - ports
 
+$defs:
+  FPDLink-input-port:
+    $ref: /schemas/graph.yaml#/$defs/port-base
+    unevaluatedProperties: false
+    description: FPD-Link input
+
+    properties:
+      endpoint:
+        $ref: /schemas/media/video-interfaces.yaml#
+        unevaluatedProperties: false
+        description:
+          Endpoint for FPD-Link port. If the RX mode for this port is RAW,
+          hsync-active and vsync-active must be defined.
+
+  CSI2-output-port:
+    $ref: /schemas/graph.yaml#/$defs/port-base
+    unevaluatedProperties: false
+    description: CSI-2 Output
+
+    properties:
+      endpoint:
+        $ref: /schemas/media/video-interfaces.yaml#
+        unevaluatedProperties: false
+
+        properties:
+          data-lanes:
+            minItems: 1
+            maxItems: 4
+          link-frequencies:
+            maxItems: 1
+
+        required:
+          - data-lanes
+          - link-frequencies
+
 unevaluatedProperties: false
 
 examples:
-- 
2.34.1

[PATCH V2 2/4] media: i2c: ds90ub960: Use enums for chip type and chip family

[Yemike Abhilash Chandra]

Replace chip-specific boolean flags with chip_type and chip_family enums.
This simplifies the process of adding support for newer devices and also
improves code readability.

Signed-off-by: Yemike Abhilash Chandra <y-abhilashchandra@ti.com>
---
 drivers/media/i2c/ds90ub960.c | 56 ++++++++++++++++++++++++-----------
 1 file changed, 38 insertions(+), 18 deletions(-)

diff --git a/drivers/media/i2c/ds90ub960.c b/drivers/media/i2c/ds90ub960.c
index 5a83218e64ab..45494fcaf095 100644
--- a/drivers/media/i2c/ds90ub960.c
+++ b/drivers/media/i2c/ds90ub960.c
@@ -454,12 +454,21 @@
 #define UB960_MAX_EQ_LEVEL  14
 #define UB960_NUM_EQ_LEVELS (UB960_MAX_EQ_LEVEL - UB960_MIN_EQ_LEVEL + 1)
 
+enum chip_type {
+	UB960,
+	UB9702,
+};
+
+enum chip_family {
+	FAMILY_FPD3,
+	FAMILY_FPD4,
+};
+
 struct ub960_hw_data {
-	const char *model;
+	enum chip_type chip_type;
+	enum chip_family chip_family;
 	u8 num_rxports;
 	u8 num_txports;
-	bool is_ub9702;
-	bool is_fpdlink4;
 };
 
 enum ub960_rxport_mode {
@@ -1933,7 +1942,7 @@ static int ub960_rxport_wait_locks(struct ub960_data *priv,
 		if (ret)
 			return ret;
 
-		if (priv->hw_data->is_ub9702) {
+		if (priv->hw_data->chip_type == UB9702) {
 			dev_dbg(dev, "\trx%u: locked, freq %llu Hz\n",
 				nport, ((u64)v * HZ_PER_MHZ) >> 8);
 		} else {
@@ -2195,7 +2204,7 @@ static int ub960_rxport_add_serializer(struct ub960_data *priv, u8 nport)
 
 	ser_pdata->port = nport;
 	ser_pdata->atr = priv->atr;
-	if (priv->hw_data->is_ub9702)
+	if (priv->hw_data->chip_type == UB9702)
 		ser_pdata->bc_rate = ub960_calc_bc_clk_rate_ub9702(priv, rxport);
 	else
 		ser_pdata->bc_rate = ub960_calc_bc_clk_rate_ub960(priv, rxport);
@@ -2361,7 +2370,7 @@ static int ub960_init_tx_ports(struct ub960_data *priv)
 {
 	int ret;
 
-	if (priv->hw_data->is_ub9702)
+	if (priv->hw_data->chip_type == UB9702)
 		ret = ub960_init_tx_ports_ub9702(priv);
 	else
 		ret = ub960_init_tx_ports_ub960(priv);
@@ -3633,7 +3642,7 @@ static int ub960_configure_ports_for_streaming(struct ub960_data *priv,
 
 		case RXPORT_MODE_CSI2_SYNC:
 		case RXPORT_MODE_CSI2_NONSYNC:
-			if (!priv->hw_data->is_ub9702) {
+			if (priv->hw_data->chip_type != UB9702) {
 				/* Map all VCs from this port to the same VC */
 				ub960_rxport_write(priv, nport, UB960_RR_CSI_VC_MAP,
 						   (vc << UB960_RR_CSI_VC_MAP_SHIFT(3)) |
@@ -4259,7 +4268,7 @@ static int ub960_log_status(struct v4l2_subdev *sd)
 
 		dev_info(dev, "\tcsi_err_counter %u\n", v);
 
-		if (!priv->hw_data->is_ub9702) {
+		if (priv->hw_data->chip_type != UB9702) {
 			ret = ub960_log_status_ub960_sp_eq(priv, nport);
 			if (ret)
 				return ret;
@@ -4417,7 +4426,7 @@ ub960_parse_dt_rxport_link_properties(struct ub960_data *priv,
 		return -EINVAL;
 	}
 
-	if (!priv->hw_data->is_fpdlink4 && cdr_mode == RXPORT_CDR_FPD4) {
+	if (priv->hw_data->chip_family != FAMILY_FPD4 && cdr_mode == RXPORT_CDR_FPD4) {
 		dev_err(dev, "rx%u: FPD-Link 4 CDR not supported\n", nport);
 		return -EINVAL;
 	}
@@ -4976,6 +4985,7 @@ static int ub960_get_hw_resources(struct ub960_data *priv)
 static int ub960_enable_core_hw(struct ub960_data *priv)
 {
 	struct device *dev = &priv->client->dev;
+	const char *model;
 	u8 rev_mask;
 	int ret;
 	u8 dev_sts;
@@ -5012,14 +5022,24 @@ static int ub960_enable_core_hw(struct ub960_data *priv)
 		goto err_pd_gpio;
 	}
 
-	dev_dbg(dev, "Found %s (rev/mask %#04x)\n", priv->hw_data->model,
-		rev_mask);
+	switch (priv->hw_data->chip_type) {
+	case UB960:
+		model = "UB960";
+		break;
+	case UB9702:
+		model = "Ub9702";
+		break;
+	default:
+		model = "Unknown";
+		break;
+	}
+	dev_dbg(dev, "Found %s (rev/mask %#04x)\n", model, rev_mask);
 
 	ret = ub960_read(priv, UB960_SR_DEVICE_STS, &dev_sts, NULL);
 	if (ret)
 		goto err_pd_gpio;
 
-	if (priv->hw_data->is_ub9702)
+	if (priv->hw_data->chip_type == UB9702)
 		ret = ub960_read(priv, UB9702_SR_REFCLK_FREQ, &refclk_freq,
 				 NULL);
 	else
@@ -5038,7 +5058,7 @@ static int ub960_enable_core_hw(struct ub960_data *priv)
 		goto err_pd_gpio;
 
 	/* release GPIO lock */
-	if (priv->hw_data->is_ub9702) {
+	if (priv->hw_data->chip_type == UB9702) {
 		ret = ub960_update_bits(priv, UB960_SR_RESET,
 					UB960_SR_RESET_GPIO_LOCK_RELEASE,
 					UB960_SR_RESET_GPIO_LOCK_RELEASE,
@@ -5111,7 +5131,7 @@ static int ub960_probe(struct i2c_client *client)
 	if (ret)
 		goto err_free_ports;
 
-	if (priv->hw_data->is_ub9702)
+	if (priv->hw_data->chip_type == UB9702)
 		ret = ub960_init_rx_ports_ub9702(priv);
 	else
 		ret = ub960_init_rx_ports_ub960(priv);
@@ -5179,17 +5199,17 @@ static void ub960_remove(struct i2c_client *client)
 }
 
 static const struct ub960_hw_data ds90ub960_hw = {
-	.model = "ub960",
+	.chip_type = UB960,
+	.chip_family = FAMILY_FPD3,
 	.num_rxports = 4,
 	.num_txports = 2,
 };
 
 static const struct ub960_hw_data ds90ub9702_hw = {
-	.model = "ub9702",
+	.chip_type = UB9702,
+	.chip_family = FAMILY_FPD4,
 	.num_rxports = 4,
 	.num_txports = 2,
-	.is_ub9702 = true,
-	.is_fpdlink4 = true,
 };
 
 static const struct i2c_device_id ub960_id[] = {
-- 
2.34.1

[PATCH V2 3/4] media: dt-bindings: ti,ds90ub960: Add support for DS90UB954-Q1

[Yemike Abhilash Chandra]

DS90UB954-Q1 is an FPDLink-III deserializer that is mostly register
compatible with DS90UB960-Q1. The main difference is that it supports
half of the RX and TX ports, i.e. 2x FPDLink RX ports and 1x CSI TX
port. Therefore, add support for DS90UB954 within the existing bindings.

Link: https://www.ti.com/lit/gpn/ds90ub954-q1
Signed-off-by: Yemike Abhilash Chandra <y-abhilashchandra@ti.com>
---
 .../bindings/media/i2c/ti,ds90ub960.yaml      | 300 +++++++++++++++---
 1 file changed, 264 insertions(+), 36 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml b/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml
index 6a78288aebaa..1ef977c2e479 100644
--- a/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml
+++ b/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml
@@ -13,12 +13,10 @@ description:
   The TI DS90UB9XX devices are FPD-Link video deserializers with I2C and GPIO
   forwarding.
 
-allOf:
-  - $ref: /schemas/i2c/i2c-atr.yaml#
-
 properties:
   compatible:
     enum:
+      - ti,ds90ub954-q1
       - ti,ds90ub960-q1
       - ti,ds90ub9702-q1
 
@@ -129,39 +127,6 @@ properties:
       Ports represent FPD-Link inputs to the deserializer and CSI TX outputs from the deserializer.
       Their number is model-dependent.
 
-    properties:
-      port@0:
-        $ref: '#/$defs/FPDLink-input-port'
-        description: FPD-Link input 0
-
-      port@1:
-        $ref: '#/$defs/FPDLink-input-port'
-        description: FPD-Link input 1
-
-      port@2:
-        $ref: '#/$defs/FPDLink-input-port'
-        description: FPD-Link input 2
-
-      port@3:
-        $ref: '#/$defs/FPDLink-input-port'
-        description: FPD-Link input 3
-
-      port@4:
-        $ref: '#/$defs/CSI2-output-port'
-        description: CSI-2 Output 0
-
-      port@5:
-        $ref: '#/$defs/CSI2-output-port'
-        description: CSI-2 Output 1
-
-    required:
-      - port@0
-      - port@1
-      - port@2
-      - port@3
-      - port@4
-      - port@5
-
 required:
   - compatible
   - reg
@@ -204,9 +169,86 @@ $defs:
           - data-lanes
           - link-frequencies
 
+allOf:
+  - $ref: /schemas/i2c/i2c-atr.yaml#
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - ti,ds90ub960-q1
+              - ti,ds90ub9702-q1
+    then:
+      properties:
+        ports:
+          properties:
+            port@0:
+              $ref: '#/$defs/FPDLink-input-port'
+              description: FPD-Link input 0
+
+            port@1:
+              $ref: '#/$defs/FPDLink-input-port'
+              description: FPD-Link input 1
+
+            port@2:
+              $ref: '#/$defs/FPDLink-input-port'
+              description: FPD-Link input 2
+
+            port@3:
+              $ref: '#/$defs/FPDLink-input-port'
+              description: FPD-Link input 3
+
+            port@4:
+              $ref: '#/$defs/CSI2-output-port'
+              description: CSI-2 Output 0
+
+            port@5:
+              $ref: '#/$defs/CSI2-output-port'
+              description: CSI-2 Output 1
+
+          required:
+            - port@0
+            - port@1
+            - port@2
+            - port@3
+            - port@4
+            - port@5
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: ti,ds90ub954-q1
+    then:
+      properties:
+        ports:
+          properties:
+            port@0:
+              $ref: '#/$defs/FPDLink-input-port'
+              description: FPD-Link input 0
+
+            port@1:
+              $ref: '#/$defs/FPDLink-input-port'
+              description: FPD-Link input 1
+
+            port@2:
+              $ref: '#/$defs/CSI2-output-port'
+              description: CSI-2 Output 0
+
+          required:
+            - port@0
+            - port@1
+            - port@2
+
+        links:
+          properties:
+            link@2: false
+            link@3: false
+
 unevaluatedProperties: false
 
 examples:
+  # Example with ds90ub960 Deserializer
   - |
     #include <dt-bindings/gpio/gpio.h>
 
@@ -406,4 +448,190 @@ examples:
         };
       };
     };
+
+  # Example with ds90ub954 Deserializer
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    i2c {
+      clock-frequency = <400000>;
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      deser@3d {
+        compatible = "ti,ds90ub954-q1";
+        reg = <0x3d>;
+
+        clock-names = "refclk";
+        clocks = <&fixed_clock>;
+
+        powerdown-gpios = <&pca9555 7 GPIO_ACTIVE_LOW>;
+
+        i2c-alias-pool = <0x4a 0x4b 0x4c>;
+
+        ports {
+          #address-cells = <1>;
+          #size-cells = <0>;
+
+          /* Port 0, Camera 0 */
+          port@0 {
+            reg = <0>;
+
+            ub954_fpd3_1_in: endpoint {
+              remote-endpoint = <&ub953_2_out>;
+            };
+          };
+
+          /* Port 1, Camera 1 */
+          port@1 {
+            reg = <1>;
+
+            ub954_fpd3_2_in: endpoint {
+              remote-endpoint = <&ub913_3_out>;
+              hsync-active = <0>;
+              vsync-active = <1>;
+            };
+          };
+
+          /* Port 2, CSI-2 TX */
+          port@2 {
+            reg = <2>;
+            ds90ub954_0_csi_out: endpoint {
+              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 IMX274 sensor */
+
+          link@0 {
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            reg = <0>;
+            i2c-alias = <0x44>;
+
+            ti,rx-mode = <3>;
+
+            serializer3: serializer@30 {
+              compatible = "ti,ds90ub953-q1";
+              reg = <0x30>;
+
+              gpio-controller;
+              #gpio-cells = <2>;
+
+              #clock-cells = <0>;
+
+              ports {
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                port@0 {
+                  reg = <0>;
+                  ub953_2_in: endpoint {
+                    data-lanes = <1 2 3 4>;
+                    remote-endpoint = <&sensor_3_out>;
+                  };
+                };
+
+                port@1 {
+                  reg = <1>;
+
+                  ub953_2_out: endpoint {
+                    remote-endpoint = <&ub954_fpd3_1_in>;
+                  };
+                };
+              };
+
+              i2c {
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                sensor@1a {
+                  compatible = "sony,imx274";
+                  reg = <0x1a>;
+
+                  clocks = <&serializer>;
+                  clock-names = "inck";
+
+                  reset-gpios = <&serializer3 0 GPIO_ACTIVE_LOW>;
+
+                  port {
+                    sensor_3_out: endpoint {
+                      remote-endpoint = <&ub953_2_in>;
+                    };
+                  };
+                };
+              };
+            };
+          };  /* End of link@0 */
+
+          /* Link 1 has DS90UB913 serializer and MT9V111 sensor */
+
+          link@1 {
+            reg = <1>;
+            i2c-alias = <0x45>;
+
+            ti,rx-mode = <0>;
+
+            serializer4: 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_3_in: endpoint {
+                    remote-endpoint = <&sensor_4_out>;
+                    pclk-sample = <1>;
+                  };
+                };
+
+                port@1 {
+                  reg = <1>;
+
+                  ub913_3_out: endpoint {
+                    remote-endpoint = <&ub954_fpd3_2_in>;
+                  };
+                };
+              };
+
+              i2c {
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                sensor@48 {
+                  compatible = "aptina,mt9v111";
+                  reg = <0x48>;
+
+                  clocks = <&serializer4>;
+
+                  port {
+                    sensor_4_out: endpoint {
+                      remote-endpoint = <&ub913_3_in>;
+                    };
+                  };
+                };
+              };
+            };
+          }; /* End of link@1 */
+        };
+      };
+    };
 ...
-- 
2.34.1

[PATCH V2 4/4] media: i2c: ds90ub960: Add support for DS90UB954-Q1

[Yemike Abhilash Chandra]

DS90UB954-Q1 is an FPDLink-III deserializer that is mostly register
compatible with DS90UB960-Q1. The main difference is that it supports half
of the RX and TX ports, i.e. 2x FPDLink RX ports and 1x CSI TX port.

A couple of differences are between the status registers and the
strobe setting registers. Hence accommodate these differences in
the UB960 driver so that we can reuse a large part of the existing code.

Link: https://www.ti.com/lit/gpn/ds90ub954-q1
Signed-off-by: Yemike Abhilash Chandra <y-abhilashchandra@ti.com>
---
Refer table 5.2.1 STROBE_SET Register in [1] for DS90UB954 strobe
setting register.

[1]: https://www.ti.com/lit/an/snla301/snla301.pdf

 drivers/media/i2c/Kconfig     |   4 +-
 drivers/media/i2c/ds90ub960.c | 165 +++++++++++++++++++++++++---------
 2 files changed, 125 insertions(+), 44 deletions(-)

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 745819c625d6..52104f76e371 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -1703,8 +1703,8 @@ config VIDEO_DS90UB960
 	select V4L2_FWNODE
 	select VIDEO_V4L2_SUBDEV_API
 	help
-	  Device driver for the Texas Instruments DS90UB960
-	  FPD-Link III Deserializer and DS90UB9702 FPD-Link IV Deserializer.
+	  Device driver for the Texas Instruments DS90UB954, DS90UB960
+	  FPD-Link III Deserializers and DS90UB9702 FPD-Link IV Deserializer.
 
 config VIDEO_MAX96714
 	tristate "Maxim MAX96714 GMSL2 deserializer"
diff --git a/drivers/media/i2c/ds90ub960.c b/drivers/media/i2c/ds90ub960.c
index 45494fcaf095..7d3e5a87bb17 100644
--- a/drivers/media/i2c/ds90ub960.c
+++ b/drivers/media/i2c/ds90ub960.c
@@ -396,6 +396,12 @@
 #define UB960_IR_RX_ANA_STROBE_SET_DATA_NO_EXTRA_DELAY	BIT(3)
 #define UB960_IR_RX_ANA_STROBE_SET_DATA_DELAY_MASK	GENMASK(2, 0)
 
+#define UB954_IR_RX_ANA_STROBE_SET_CLK_DATA		0x08
+#define UB954_IR_RX_ANA_STROBE_SET_CLK_NO_EXTRA_DELAY	BIT(3)
+#define UB954_IR_RX_ANA_STROBE_SET_DATA_NO_EXTRA_DELAY	BIT(7)
+#define UB954_IR_RX_ANA_STROBE_SET_CLK_DELAY_MASK	GENMASK(2, 0)
+#define UB954_IR_RX_ANA_STROBE_SET_DATA_DELAY_MASK	GENMASK(4, 6)
+
 /* UB9702 Registers */
 
 #define UB9702_SR_CSI_EXCLUSIVE_FWD2		0x3c
@@ -455,6 +461,7 @@
 #define UB960_NUM_EQ_LEVELS (UB960_MAX_EQ_LEVEL - UB960_MIN_EQ_LEVEL + 1)
 
 enum chip_type {
+	UB954,
 	UB960,
 	UB9702,
 };
@@ -1000,6 +1007,10 @@ static int ub960_txport_select(struct ub960_data *priv, u8 nport)
 
 	lockdep_assert_held(&priv->reg_lock);
 
+	/* UB954 has only 1 CSI TX. Hence, no need to select */
+	if (priv->hw_data->chip_type == UB954)
+		return 0;
+
 	if (priv->reg_current.txport == nport)
 		return 0;
 
@@ -1424,10 +1435,11 @@ static int ub960_parse_dt_txport(struct ub960_data *priv,
 	priv->tx_link_freq[0] = vep.link_frequencies[0];
 	priv->tx_data_rate = priv->tx_link_freq[0] * 2;
 
-	if (priv->tx_data_rate != MHZ(1600) &&
-	    priv->tx_data_rate != MHZ(1200) &&
-	    priv->tx_data_rate != MHZ(800) &&
-	    priv->tx_data_rate != MHZ(400)) {
+	if ((priv->tx_data_rate != MHZ(1600) &&
+	     priv->tx_data_rate != MHZ(1200) &&
+	     priv->tx_data_rate != MHZ(800) &&
+	     priv->tx_data_rate != MHZ(400)) ||
+	     (priv->hw_data->chip_type == UB954 && priv->tx_data_rate == MHZ(1200))) {
 		dev_err(dev, "tx%u: invalid 'link-frequencies' value\n", nport);
 		ret = -EINVAL;
 		goto err_free_vep;
@@ -1551,22 +1563,44 @@ static int ub960_rxport_get_strobe_pos(struct ub960_data *priv,
 	u8 clk_delay, data_delay;
 	int ret;
 
-	ret = ub960_read_ind(priv, UB960_IND_TARGET_RX_ANA(nport),
-			     UB960_IR_RX_ANA_STROBE_SET_CLK, &v, NULL);
-	if (ret)
-		return ret;
+	/*
+	 * DS90UB960 has two separate registers for clk and data delay whereas
+	 * DS90UB954 has a single combined register. Hence read accordingly
+	 */
+	if (priv->hw_data->chip_type == UB954) {
+		ret = ub960_read_ind(priv, UB960_IND_TARGET_RX_ANA(nport),
+				     UB954_IR_RX_ANA_STROBE_SET_CLK_DATA, &v, NULL);
+		if (ret)
+			return ret;
 
-	clk_delay = (v & UB960_IR_RX_ANA_STROBE_SET_CLK_NO_EXTRA_DELAY) ?
-			    0 : UB960_MANUAL_STROBE_EXTRA_DELAY;
+		clk_delay = (v & UB954_IR_RX_ANA_STROBE_SET_CLK_NO_EXTRA_DELAY) ?
+			     0 : UB960_MANUAL_STROBE_EXTRA_DELAY;
 
-	ret = ub960_read_ind(priv, UB960_IND_TARGET_RX_ANA(nport),
-			     UB960_IR_RX_ANA_STROBE_SET_DATA, &v, NULL);
-	if (ret)
-		return ret;
+		ret = ub960_read_ind(priv, UB960_IND_TARGET_RX_ANA(nport),
+				     UB954_IR_RX_ANA_STROBE_SET_CLK_DATA, &v, NULL);
+		if (ret)
+			return ret;
+
+		data_delay = (v & UB954_IR_RX_ANA_STROBE_SET_DATA_NO_EXTRA_DELAY) ?
+			      0 : UB960_MANUAL_STROBE_EXTRA_DELAY;
+	} else {
+		ret = ub960_read_ind(priv, UB960_IND_TARGET_RX_ANA(nport),
+				     UB960_IR_RX_ANA_STROBE_SET_CLK, &v, NULL);
+		if (ret)
+			return ret;
 
-	data_delay = (v & UB960_IR_RX_ANA_STROBE_SET_DATA_NO_EXTRA_DELAY) ?
+		clk_delay = (v & UB960_IR_RX_ANA_STROBE_SET_CLK_NO_EXTRA_DELAY) ?
 			     0 : UB960_MANUAL_STROBE_EXTRA_DELAY;
 
+		ret = ub960_read_ind(priv, UB960_IND_TARGET_RX_ANA(nport),
+				     UB960_IR_RX_ANA_STROBE_SET_DATA, &v, NULL);
+		if (ret)
+			return ret;
+
+		data_delay = (v & UB960_IR_RX_ANA_STROBE_SET_DATA_NO_EXTRA_DELAY) ?
+			      0 : UB960_MANUAL_STROBE_EXTRA_DELAY;
+	}
+
 	ret = ub960_rxport_read(priv, nport, UB960_RR_SFILTER_STS_0, &v, NULL);
 	if (ret)
 		return ret;
@@ -1590,8 +1624,17 @@ static int ub960_rxport_set_strobe_pos(struct ub960_data *priv,
 	u8 clk_delay, data_delay;
 	int ret = 0;
 
-	clk_delay = UB960_IR_RX_ANA_STROBE_SET_CLK_NO_EXTRA_DELAY;
-	data_delay = UB960_IR_RX_ANA_STROBE_SET_DATA_NO_EXTRA_DELAY;
+	/*
+	 * DS90UB960 has two separate registers for clk and data delay whereas
+	 * DS90UB954 has a single combined register. Hence assign accordingly.
+	 */
+	if (priv->hw_data->chip_type == UB954) {
+		clk_delay = UB954_IR_RX_ANA_STROBE_SET_CLK_NO_EXTRA_DELAY;
+		data_delay = UB954_IR_RX_ANA_STROBE_SET_DATA_NO_EXTRA_DELAY;
+	} else {
+		clk_delay = UB960_IR_RX_ANA_STROBE_SET_CLK_NO_EXTRA_DELAY;
+		data_delay = UB960_IR_RX_ANA_STROBE_SET_DATA_NO_EXTRA_DELAY;
+	}
 
 	if (strobe_pos < UB960_MIN_AEQ_STROBE_POS)
 		clk_delay = abs(strobe_pos) - UB960_MANUAL_STROBE_EXTRA_DELAY;
@@ -1602,11 +1645,25 @@ static int ub960_rxport_set_strobe_pos(struct ub960_data *priv,
 	else if (strobe_pos > 0)
 		data_delay = strobe_pos | UB960_IR_RX_ANA_STROBE_SET_DATA_NO_EXTRA_DELAY;
 
-	ub960_write_ind(priv, UB960_IND_TARGET_RX_ANA(nport),
-			UB960_IR_RX_ANA_STROBE_SET_CLK, clk_delay, &ret);
-
-	ub960_write_ind(priv, UB960_IND_TARGET_RX_ANA(nport),
-			UB960_IR_RX_ANA_STROBE_SET_DATA, data_delay, &ret);
+	/*
+	 * DS90UB960 has two separate registers for clk and data delay whereas
+	 * DS90UB954 has a single combined register. Hence write the registers accordingly.
+	 */
+	if (priv->hw_data->chip_type == UB954) {
+		ub960_ind_update_bits(priv, UB960_IND_TARGET_RX_ANA(nport),
+				      UB954_IR_RX_ANA_STROBE_SET_CLK_DATA,
+				      UB954_IR_RX_ANA_STROBE_SET_CLK_NO_EXTRA_DELAY,
+				      clk_delay, &ret);
+		ub960_ind_update_bits(priv, UB960_IND_TARGET_RX_ANA(nport),
+				      UB954_IR_RX_ANA_STROBE_SET_CLK_DATA,
+				      UB954_IR_RX_ANA_STROBE_SET_DATA_NO_EXTRA_DELAY,
+				      data_delay, &ret);
+	} else {
+		ub960_write_ind(priv, UB960_IND_TARGET_RX_ANA(nport),
+				UB960_IR_RX_ANA_STROBE_SET_CLK, clk_delay, &ret);
+		ub960_write_ind(priv, UB960_IND_TARGET_RX_ANA(nport),
+				UB960_IR_RX_ANA_STROBE_SET_DATA, data_delay, &ret);
+	}
 
 	return ret;
 }
@@ -4176,33 +4233,40 @@ static int ub960_log_status(struct v4l2_subdev *sd)
 		dev_info(dev, "\tsync %u, pass %u\n", v & (u8)BIT(1),
 			 v & (u8)BIT(0));
 
-		ret = ub960_read16(priv, UB960_SR_CSI_FRAME_COUNT_HI(nport),
-				   &v16, NULL);
-		if (ret)
-			return ret;
+		/*
+		 * Frame counter, frame error counter, line counter and line error counter
+		 * registers are marked as reserved in the UB954 datasheet. Hence restrict
+		 * the following register reads only for UB960 and UB9702.
+		 */
+		if (priv->hw_data->chip_type != UB954) {
+			ret = ub960_read16(priv, UB960_SR_CSI_FRAME_COUNT_HI(nport),
+					   &v16, NULL);
+			if (ret)
+				return ret;
 
-		dev_info(dev, "\tframe counter %u\n", v16);
+			dev_info(dev, "\tframe counter %u\n", v16);
 
-		ret = ub960_read16(priv, UB960_SR_CSI_FRAME_ERR_COUNT_HI(nport),
-				   &v16, NULL);
-		if (ret)
-			return ret;
+			ret = ub960_read16(priv, UB960_SR_CSI_FRAME_ERR_COUNT_HI(nport),
+					   &v16, NULL);
+			if (ret)
+				return ret;
 
-		dev_info(dev, "\tframe error counter %u\n", v16);
+			dev_info(dev, "\tframe error counter %u\n", v16);
 
-		ret = ub960_read16(priv, UB960_SR_CSI_LINE_COUNT_HI(nport),
-				   &v16, NULL);
-		if (ret)
-			return ret;
+			ret = ub960_read16(priv, UB960_SR_CSI_LINE_COUNT_HI(nport),
+					   &v16, NULL);
+			if (ret)
+				return ret;
 
-		dev_info(dev, "\tline counter %u\n", v16);
+			dev_info(dev, "\tline counter %u\n", v16);
 
-		ret = ub960_read16(priv, UB960_SR_CSI_LINE_ERR_COUNT_HI(nport),
-				   &v16, NULL);
-		if (ret)
-			return ret;
+			ret = ub960_read16(priv, UB960_SR_CSI_LINE_ERR_COUNT_HI(nport),
+					   &v16, NULL);
+			if (ret)
+				return ret;
 
-		dev_info(dev, "\tline error counter %u\n", v16);
+			dev_info(dev, "\tline error counter %u\n", v16);
+		}
 	}
 
 	for_each_rxport(priv, it) {
@@ -5023,6 +5087,9 @@ static int ub960_enable_core_hw(struct ub960_data *priv)
 	}
 
 	switch (priv->hw_data->chip_type) {
+	case UB954:
+		model = "UB954";
+		break;
 	case UB960:
 		model = "UB960";
 		break;
@@ -5039,6 +5106,11 @@ static int ub960_enable_core_hw(struct ub960_data *priv)
 	if (ret)
 		goto err_pd_gpio;
 
+	/*
+	 * UB954 REFCLK_FREQ is not synchronized, so multiple reads are recommended
+	 * by the datasheet. However, we use the same logic as UB960 (single read),
+	 * as practical testing showed this is sufficient and stable for UB954 as well.
+	 */
 	if (priv->hw_data->chip_type == UB9702)
 		ret = ub960_read(priv, UB9702_SR_REFCLK_FREQ, &refclk_freq,
 				 NULL);
@@ -5198,6 +5270,13 @@ static void ub960_remove(struct i2c_client *client)
 	mutex_destroy(&priv->reg_lock);
 }
 
+static const struct ub960_hw_data ds90ub954_hw = {
+	.chip_type = UB954,
+	.chip_family = FAMILY_FPD3,
+	.num_rxports = 2,
+	.num_txports = 1,
+};
+
 static const struct ub960_hw_data ds90ub960_hw = {
 	.chip_type = UB960,
 	.chip_family = FAMILY_FPD3,
@@ -5213,6 +5292,7 @@ static const struct ub960_hw_data ds90ub9702_hw = {
 };
 
 static const struct i2c_device_id ub960_id[] = {
+	{ "ds90ub954-q1", (kernel_ulong_t)&ds90ub954_hw },
 	{ "ds90ub960-q1", (kernel_ulong_t)&ds90ub960_hw },
 	{ "ds90ub9702-q1", (kernel_ulong_t)&ds90ub9702_hw },
 	{}
@@ -5220,6 +5300,7 @@ static const struct i2c_device_id ub960_id[] = {
 MODULE_DEVICE_TABLE(i2c, ub960_id);
 
 static const struct of_device_id ub960_dt_ids[] = {
+	{ .compatible = "ti,ds90ub954-q1", .data = &ds90ub954_hw },
 	{ .compatible = "ti,ds90ub960-q1", .data = &ds90ub960_hw },
 	{ .compatible = "ti,ds90ub9702-q1", .data = &ds90ub9702_hw },
 	{}
-- 
2.34.1

Re: [PATCH V2 2/4] media: i2c: ds90ub960: Use enums for chip type and chip family

[Tomi Valkeinen]

Hi,

On 02/12/2025 12:22, Yemike Abhilash Chandra wrote:
> Replace chip-specific boolean flags with chip_type and chip_family enums.
> This simplifies the process of adding support for newer devices and also
> improves code readability.
> 
> Signed-off-by: Yemike Abhilash Chandra <y-abhilashchandra@ti.com>
> ---
>  drivers/media/i2c/ds90ub960.c | 56 ++++++++++++++++++++++++-----------
>  1 file changed, 38 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/media/i2c/ds90ub960.c b/drivers/media/i2c/ds90ub960.c
> index 5a83218e64ab..45494fcaf095 100644
> --- a/drivers/media/i2c/ds90ub960.c
> +++ b/drivers/media/i2c/ds90ub960.c
> @@ -454,12 +454,21 @@
>  #define UB960_MAX_EQ_LEVEL  14
>  #define UB960_NUM_EQ_LEVELS (UB960_MAX_EQ_LEVEL - UB960_MIN_EQ_LEVEL + 1)
>  
> +enum chip_type {
> +	UB960,
> +	UB9702,
> +};
> +
> +enum chip_family {
> +	FAMILY_FPD3,
> +	FAMILY_FPD4,
> +};
> +
>  struct ub960_hw_data {
> -	const char *model;
> +	enum chip_type chip_type;
> +	enum chip_family chip_family;
>  	u8 num_rxports;
>  	u8 num_txports;
> -	bool is_ub9702;
> -	bool is_fpdlink4;
>  };
>  
>  enum ub960_rxport_mode {
> @@ -1933,7 +1942,7 @@ static int ub960_rxport_wait_locks(struct ub960_data *priv,
>  		if (ret)
>  			return ret;
>  
> -		if (priv->hw_data->is_ub9702) {
> +		if (priv->hw_data->chip_type == UB9702) {
>  			dev_dbg(dev, "\trx%u: locked, freq %llu Hz\n",
>  				nport, ((u64)v * HZ_PER_MHZ) >> 8);
>  		} else {
> @@ -2195,7 +2204,7 @@ static int ub960_rxport_add_serializer(struct ub960_data *priv, u8 nport)
>  
>  	ser_pdata->port = nport;
>  	ser_pdata->atr = priv->atr;
> -	if (priv->hw_data->is_ub9702)
> +	if (priv->hw_data->chip_type == UB9702)
>  		ser_pdata->bc_rate = ub960_calc_bc_clk_rate_ub9702(priv, rxport);
>  	else
>  		ser_pdata->bc_rate = ub960_calc_bc_clk_rate_ub960(priv, rxport);
> @@ -2361,7 +2370,7 @@ static int ub960_init_tx_ports(struct ub960_data *priv)
>  {
>  	int ret;
>  
> -	if (priv->hw_data->is_ub9702)
> +	if (priv->hw_data->chip_type == UB9702)
>  		ret = ub960_init_tx_ports_ub9702(priv);
>  	else
>  		ret = ub960_init_tx_ports_ub960(priv);
> @@ -3633,7 +3642,7 @@ static int ub960_configure_ports_for_streaming(struct ub960_data *priv,
>  
>  		case RXPORT_MODE_CSI2_SYNC:
>  		case RXPORT_MODE_CSI2_NONSYNC:
> -			if (!priv->hw_data->is_ub9702) {
> +			if (priv->hw_data->chip_type != UB9702) {

While the above is correct, I think it's better to do 'if
(what-we-need-here)'. So rather check for UB960.

>  				/* Map all VCs from this port to the same VC */
>  				ub960_rxport_write(priv, nport, UB960_RR_CSI_VC_MAP,
>  						   (vc << UB960_RR_CSI_VC_MAP_SHIFT(3)) |
> @@ -4259,7 +4268,7 @@ static int ub960_log_status(struct v4l2_subdev *sd)
>  
>  		dev_info(dev, "\tcsi_err_counter %u\n", v);
>  
> -		if (!priv->hw_data->is_ub9702) {
> +		if (priv->hw_data->chip_type != UB9702) {

Same here.

>  			ret = ub960_log_status_ub960_sp_eq(priv, nport);
>  			if (ret)
>  				return ret;
> @@ -4417,7 +4426,7 @@ ub960_parse_dt_rxport_link_properties(struct ub960_data *priv,
>  		return -EINVAL;
>  	}
>  
> -	if (!priv->hw_data->is_fpdlink4 && cdr_mode == RXPORT_CDR_FPD4) {
> +	if (priv->hw_data->chip_family != FAMILY_FPD4 && cdr_mode == RXPORT_CDR_FPD4) {
>  		dev_err(dev, "rx%u: FPD-Link 4 CDR not supported\n", nport);
>  		return -EINVAL;
>  	}
> @@ -4976,6 +4985,7 @@ static int ub960_get_hw_resources(struct ub960_data *priv)
>  static int ub960_enable_core_hw(struct ub960_data *priv)
>  {
>  	struct device *dev = &priv->client->dev;
> +	const char *model;
>  	u8 rev_mask;
>  	int ret;
>  	u8 dev_sts;
> @@ -5012,14 +5022,24 @@ static int ub960_enable_core_hw(struct ub960_data *priv)
>  		goto err_pd_gpio;
>  	}
>  
> -	dev_dbg(dev, "Found %s (rev/mask %#04x)\n", priv->hw_data->model,
> -		rev_mask);
> +	switch (priv->hw_data->chip_type) {
> +	case UB960:
> +		model = "UB960";
> +		break;
> +	case UB9702:
> +		model = "Ub9702";
> +		break;
> +	default:
> +		model = "Unknown";
> +		break;
> +	}
> +	dev_dbg(dev, "Found %s (rev/mask %#04x)\n", model, rev_mask);
>  
>  	ret = ub960_read(priv, UB960_SR_DEVICE_STS, &dev_sts, NULL);
>  	if (ret)
>  		goto err_pd_gpio;
>  
> -	if (priv->hw_data->is_ub9702)
> +	if (priv->hw_data->chip_type == UB9702)
>  		ret = ub960_read(priv, UB9702_SR_REFCLK_FREQ, &refclk_freq,
>  				 NULL);
>  	else
> @@ -5038,7 +5058,7 @@ static int ub960_enable_core_hw(struct ub960_data *priv)
>  		goto err_pd_gpio;
>  
>  	/* release GPIO lock */
> -	if (priv->hw_data->is_ub9702) {
> +	if (priv->hw_data->chip_type == UB9702) {
>  		ret = ub960_update_bits(priv, UB960_SR_RESET,
>  					UB960_SR_RESET_GPIO_LOCK_RELEASE,
>  					UB960_SR_RESET_GPIO_LOCK_RELEASE,
> @@ -5111,7 +5131,7 @@ static int ub960_probe(struct i2c_client *client)
>  	if (ret)
>  		goto err_free_ports;
>  
> -	if (priv->hw_data->is_ub9702)
> +	if (priv->hw_data->chip_type == UB9702)
>  		ret = ub960_init_rx_ports_ub9702(priv);
>  	else
>  		ret = ub960_init_rx_ports_ub960(priv);
> @@ -5179,17 +5199,17 @@ static void ub960_remove(struct i2c_client *client)
>  }
>  
>  static const struct ub960_hw_data ds90ub960_hw = {
> -	.model = "ub960",
> +	.chip_type = UB960,
> +	.chip_family = FAMILY_FPD3,

I think we can keep the model name here. It's a bit duplicate with the
chip_type, but allows us to drop that switch-case from probe.

>  	.num_rxports = 4,
>  	.num_txports = 2,
>  };
>  
>  static const struct ub960_hw_data ds90ub9702_hw = {
> -	.model = "ub9702",
> +	.chip_type = UB9702,
> +	.chip_family = FAMILY_FPD4,
>  	.num_rxports = 4,
>  	.num_txports = 2,
> -	.is_ub9702 = true,
> -	.is_fpdlink4 = true,
>  };
>  
>  static const struct i2c_device_id ub960_id[] = {

 Tomi

Re: [PATCH V2 4/4] media: i2c: ds90ub960: Add support for DS90UB954-Q1

[Tomi Valkeinen]

Hi,

On 02/12/2025 12:22, Yemike Abhilash Chandra wrote:
> DS90UB954-Q1 is an FPDLink-III deserializer that is mostly register
> compatible with DS90UB960-Q1. The main difference is that it supports half
> of the RX and TX ports, i.e. 2x FPDLink RX ports and 1x CSI TX port.
> 
> A couple of differences are between the status registers and the
> strobe setting registers. Hence accommodate these differences in
> the UB960 driver so that we can reuse a large part of the existing code.
> 
> Link: https://www.ti.com/lit/gpn/ds90ub954-q1
> Signed-off-by: Yemike Abhilash Chandra <y-abhilashchandra@ti.com>
> ---
> Refer table 5.2.1 STROBE_SET Register in [1] for DS90UB954 strobe
> setting register.
> 
> [1]: https://www.ti.com/lit/an/snla301/snla301.pdf
> 
>  drivers/media/i2c/Kconfig     |   4 +-
>  drivers/media/i2c/ds90ub960.c | 165 +++++++++++++++++++++++++---------
>  2 files changed, 125 insertions(+), 44 deletions(-)
> 
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index 745819c625d6..52104f76e371 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -1703,8 +1703,8 @@ config VIDEO_DS90UB960
>  	select V4L2_FWNODE
>  	select VIDEO_V4L2_SUBDEV_API
>  	help
> -	  Device driver for the Texas Instruments DS90UB960
> -	  FPD-Link III Deserializer and DS90UB9702 FPD-Link IV Deserializer.
> +	  Device driver for the Texas Instruments DS90UB954, DS90UB960
> +	  FPD-Link III Deserializers and DS90UB9702 FPD-Link IV Deserializer.
>  
>  config VIDEO_MAX96714
>  	tristate "Maxim MAX96714 GMSL2 deserializer"
> diff --git a/drivers/media/i2c/ds90ub960.c b/drivers/media/i2c/ds90ub960.c
> index 45494fcaf095..7d3e5a87bb17 100644
> --- a/drivers/media/i2c/ds90ub960.c
> +++ b/drivers/media/i2c/ds90ub960.c
> @@ -396,6 +396,12 @@
>  #define UB960_IR_RX_ANA_STROBE_SET_DATA_NO_EXTRA_DELAY	BIT(3)
>  #define UB960_IR_RX_ANA_STROBE_SET_DATA_DELAY_MASK	GENMASK(2, 0)
>  
> +#define UB954_IR_RX_ANA_STROBE_SET_CLK_DATA		0x08
> +#define UB954_IR_RX_ANA_STROBE_SET_CLK_NO_EXTRA_DELAY	BIT(3)
> +#define UB954_IR_RX_ANA_STROBE_SET_DATA_NO_EXTRA_DELAY	BIT(7)
> +#define UB954_IR_RX_ANA_STROBE_SET_CLK_DELAY_MASK	GENMASK(2, 0)
> +#define UB954_IR_RX_ANA_STROBE_SET_DATA_DELAY_MASK	GENMASK(4, 6)
> +
>  /* UB9702 Registers */
>  
>  #define UB9702_SR_CSI_EXCLUSIVE_FWD2		0x3c
> @@ -455,6 +461,7 @@
>  #define UB960_NUM_EQ_LEVELS (UB960_MAX_EQ_LEVEL - UB960_MIN_EQ_LEVEL + 1)
>  
>  enum chip_type {
> +	UB954,
>  	UB960,
>  	UB9702,
>  };
> @@ -1000,6 +1007,10 @@ static int ub960_txport_select(struct ub960_data *priv, u8 nport)
>  
>  	lockdep_assert_held(&priv->reg_lock);
>  
> +	/* UB954 has only 1 CSI TX. Hence, no need to select */
> +	if (priv->hw_data->chip_type == UB954)
> +		return 0;
> +
>  	if (priv->reg_current.txport == nport)
>  		return 0;
>  
> @@ -1424,10 +1435,11 @@ static int ub960_parse_dt_txport(struct ub960_data *priv,
>  	priv->tx_link_freq[0] = vep.link_frequencies[0];
>  	priv->tx_data_rate = priv->tx_link_freq[0] * 2;
>  
> -	if (priv->tx_data_rate != MHZ(1600) &&
> -	    priv->tx_data_rate != MHZ(1200) &&
> -	    priv->tx_data_rate != MHZ(800) &&
> -	    priv->tx_data_rate != MHZ(400)) {
> +	if ((priv->tx_data_rate != MHZ(1600) &&
> +	     priv->tx_data_rate != MHZ(1200) &&
> +	     priv->tx_data_rate != MHZ(800) &&
> +	     priv->tx_data_rate != MHZ(400)) ||
> +	     (priv->hw_data->chip_type == UB954 && priv->tx_data_rate == MHZ(1200))) {
>  		dev_err(dev, "tx%u: invalid 'link-frequencies' value\n", nport);
>  		ret = -EINVAL;
>  		goto err_free_vep;
> @@ -1551,22 +1563,44 @@ static int ub960_rxport_get_strobe_pos(struct ub960_data *priv,
>  	u8 clk_delay, data_delay;
>  	int ret;
>  
> -	ret = ub960_read_ind(priv, UB960_IND_TARGET_RX_ANA(nport),
> -			     UB960_IR_RX_ANA_STROBE_SET_CLK, &v, NULL);
> -	if (ret)
> -		return ret;
> +	/*
> +	 * DS90UB960 has two separate registers for clk and data delay whereas
> +	 * DS90UB954 has a single combined register. Hence read accordingly
> +	 */

Why do you read the single register twice? In any case, I don't think
the comment is needed, as it's quite clear from the code. Unless there's
some extra complication with the registers.

> +	if (priv->hw_data->chip_type == UB954) {
> +		ret = ub960_read_ind(priv, UB960_IND_TARGET_RX_ANA(nport),
> +				     UB954_IR_RX_ANA_STROBE_SET_CLK_DATA, &v, NULL);
> +		if (ret)
> +			return ret;
>  
> -	clk_delay = (v & UB960_IR_RX_ANA_STROBE_SET_CLK_NO_EXTRA_DELAY) ?
> -			    0 : UB960_MANUAL_STROBE_EXTRA_DELAY;
> +		clk_delay = (v & UB954_IR_RX_ANA_STROBE_SET_CLK_NO_EXTRA_DELAY) ?
> +			     0 : UB960_MANUAL_STROBE_EXTRA_DELAY;
>  
> -	ret = ub960_read_ind(priv, UB960_IND_TARGET_RX_ANA(nport),
> -			     UB960_IR_RX_ANA_STROBE_SET_DATA, &v, NULL);
> -	if (ret)
> -		return ret;
> +		ret = ub960_read_ind(priv, UB960_IND_TARGET_RX_ANA(nport),
> +				     UB954_IR_RX_ANA_STROBE_SET_CLK_DATA, &v, NULL);
> +		if (ret)
> +			return ret;
> +
> +		data_delay = (v & UB954_IR_RX_ANA_STROBE_SET_DATA_NO_EXTRA_DELAY) ?
> +			      0 : UB960_MANUAL_STROBE_EXTRA_DELAY;
> +	} else {
> +		ret = ub960_read_ind(priv, UB960_IND_TARGET_RX_ANA(nport),
> +				     UB960_IR_RX_ANA_STROBE_SET_CLK, &v, NULL);
> +		if (ret)
> +			return ret;
>  
> -	data_delay = (v & UB960_IR_RX_ANA_STROBE_SET_DATA_NO_EXTRA_DELAY) ?
> +		clk_delay = (v & UB960_IR_RX_ANA_STROBE_SET_CLK_NO_EXTRA_DELAY) ?
>  			     0 : UB960_MANUAL_STROBE_EXTRA_DELAY;
>  
> +		ret = ub960_read_ind(priv, UB960_IND_TARGET_RX_ANA(nport),
> +				     UB960_IR_RX_ANA_STROBE_SET_DATA, &v, NULL);
> +		if (ret)
> +			return ret;
> +
> +		data_delay = (v & UB960_IR_RX_ANA_STROBE_SET_DATA_NO_EXTRA_DELAY) ?
> +			      0 : UB960_MANUAL_STROBE_EXTRA_DELAY;
> +	}
> +
>  	ret = ub960_rxport_read(priv, nport, UB960_RR_SFILTER_STS_0, &v, NULL);
>  	if (ret)
>  		return ret;
> @@ -1590,8 +1624,17 @@ static int ub960_rxport_set_strobe_pos(struct ub960_data *priv,
>  	u8 clk_delay, data_delay;
>  	int ret = 0;
>  
> -	clk_delay = UB960_IR_RX_ANA_STROBE_SET_CLK_NO_EXTRA_DELAY;
> -	data_delay = UB960_IR_RX_ANA_STROBE_SET_DATA_NO_EXTRA_DELAY;
> +	/*
> +	 * DS90UB960 has two separate registers for clk and data delay whereas
> +	 * DS90UB954 has a single combined register. Hence assign accordingly.
> +	 */
> +	if (priv->hw_data->chip_type == UB954) {
> +		clk_delay = UB954_IR_RX_ANA_STROBE_SET_CLK_NO_EXTRA_DELAY;
> +		data_delay = UB954_IR_RX_ANA_STROBE_SET_DATA_NO_EXTRA_DELAY;
> +	} else {
> +		clk_delay = UB960_IR_RX_ANA_STROBE_SET_CLK_NO_EXTRA_DELAY;
> +		data_delay = UB960_IR_RX_ANA_STROBE_SET_DATA_NO_EXTRA_DELAY;
> +	}
>  
>  	if (strobe_pos < UB960_MIN_AEQ_STROBE_POS)
>  		clk_delay = abs(strobe_pos) - UB960_MANUAL_STROBE_EXTRA_DELAY;
> @@ -1602,11 +1645,25 @@ static int ub960_rxport_set_strobe_pos(struct ub960_data *priv,
>  	else if (strobe_pos > 0)
>  		data_delay = strobe_pos | UB960_IR_RX_ANA_STROBE_SET_DATA_NO_EXTRA_DELAY;
>  
> -	ub960_write_ind(priv, UB960_IND_TARGET_RX_ANA(nport),
> -			UB960_IR_RX_ANA_STROBE_SET_CLK, clk_delay, &ret);
> -
> -	ub960_write_ind(priv, UB960_IND_TARGET_RX_ANA(nport),
> -			UB960_IR_RX_ANA_STROBE_SET_DATA, data_delay, &ret);
> +	/*
> +	 * DS90UB960 has two separate registers for clk and data delay whereas
> +	 * DS90UB954 has a single combined register. Hence write the registers accordingly.
> +	 */
> +	if (priv->hw_data->chip_type == UB954) {
> +		ub960_ind_update_bits(priv, UB960_IND_TARGET_RX_ANA(nport),
> +				      UB954_IR_RX_ANA_STROBE_SET_CLK_DATA,
> +				      UB954_IR_RX_ANA_STROBE_SET_CLK_NO_EXTRA_DELAY,
> +				      clk_delay, &ret);
> +		ub960_ind_update_bits(priv, UB960_IND_TARGET_RX_ANA(nport),
> +				      UB954_IR_RX_ANA_STROBE_SET_CLK_DATA,
> +				      UB954_IR_RX_ANA_STROBE_SET_DATA_NO_EXTRA_DELAY,
> +				      data_delay, &ret);

Here, too. It's a single register, why write it twice?

And I don't think this is correct at all... Did you validate this? The
above only sets the EXTRA_DELAY bits, not the values at all. And the
code that sets clk_delay and data_delay use UB960's bit positions, which
are not the same on UB954.

> +	} else {
> +		ub960_write_ind(priv, UB960_IND_TARGET_RX_ANA(nport),
> +				UB960_IR_RX_ANA_STROBE_SET_CLK, clk_delay, &ret);
> +		ub960_write_ind(priv, UB960_IND_TARGET_RX_ANA(nport),
> +				UB960_IR_RX_ANA_STROBE_SET_DATA, data_delay, &ret);
> +	}
>  
>  	return ret;
>  }
> @@ -4176,33 +4233,40 @@ static int ub960_log_status(struct v4l2_subdev *sd)
>  		dev_info(dev, "\tsync %u, pass %u\n", v & (u8)BIT(1),
>  			 v & (u8)BIT(0));
>  
> -		ret = ub960_read16(priv, UB960_SR_CSI_FRAME_COUNT_HI(nport),
> -				   &v16, NULL);
> -		if (ret)
> -			return ret;
> +		/*
> +		 * Frame counter, frame error counter, line counter and line error counter
> +		 * registers are marked as reserved in the UB954 datasheet. Hence restrict
> +		 * the following register reads only for UB960 and UB9702.
> +		 */
> +		if (priv->hw_data->chip_type != UB954) {

It is better to check for the chips that have the registers, unless
we're sure that this particular chip, ub954, is and will be the only
outlier.

> +			ret = ub960_read16(priv, UB960_SR_CSI_FRAME_COUNT_HI(nport),
> +					   &v16, NULL);
> +			if (ret)
> +				return ret;
>  
> -		dev_info(dev, "\tframe counter %u\n", v16);
> +			dev_info(dev, "\tframe counter %u\n", v16);
>  
> -		ret = ub960_read16(priv, UB960_SR_CSI_FRAME_ERR_COUNT_HI(nport),
> -				   &v16, NULL);
> -		if (ret)
> -			return ret;
> +			ret = ub960_read16(priv, UB960_SR_CSI_FRAME_ERR_COUNT_HI(nport),
> +					   &v16, NULL);
> +			if (ret)
> +				return ret;
>  
> -		dev_info(dev, "\tframe error counter %u\n", v16);
> +			dev_info(dev, "\tframe error counter %u\n", v16);
>  
> -		ret = ub960_read16(priv, UB960_SR_CSI_LINE_COUNT_HI(nport),
> -				   &v16, NULL);
> -		if (ret)
> -			return ret;
> +			ret = ub960_read16(priv, UB960_SR_CSI_LINE_COUNT_HI(nport),
> +					   &v16, NULL);
> +			if (ret)
> +				return ret;
>  
> -		dev_info(dev, "\tline counter %u\n", v16);
> +			dev_info(dev, "\tline counter %u\n", v16);
>  
> -		ret = ub960_read16(priv, UB960_SR_CSI_LINE_ERR_COUNT_HI(nport),
> -				   &v16, NULL);
> -		if (ret)
> -			return ret;
> +			ret = ub960_read16(priv, UB960_SR_CSI_LINE_ERR_COUNT_HI(nport),
> +					   &v16, NULL);
> +			if (ret)
> +				return ret;
>  
> -		dev_info(dev, "\tline error counter %u\n", v16);
> +			dev_info(dev, "\tline error counter %u\n", v16);
> +		}
>  	}
>  
>  	for_each_rxport(priv, it) {
> @@ -5023,6 +5087,9 @@ static int ub960_enable_core_hw(struct ub960_data *priv)
>  	}
>  
>  	switch (priv->hw_data->chip_type) {
> +	case UB954:
> +		model = "UB954";
> +		break;
>  	case UB960:
>  		model = "UB960";
>  		break;
> @@ -5039,6 +5106,11 @@ static int ub960_enable_core_hw(struct ub960_data *priv)
>  	if (ret)
>  		goto err_pd_gpio;
>  
> +	/*
> +	 * UB954 REFCLK_FREQ is not synchronized, so multiple reads are recommended
> +	 * by the datasheet. However, we use the same logic as UB960 (single read),
> +	 * as practical testing showed this is sufficient and stable for UB954 as well.
> +	 */

I think the important point is that the clk rate is only used for a
debug print.

>  	if (priv->hw_data->chip_type == UB9702)
>  		ret = ub960_read(priv, UB9702_SR_REFCLK_FREQ, &refclk_freq,
>  				 NULL);
> @@ -5198,6 +5270,13 @@ static void ub960_remove(struct i2c_client *client)
>  	mutex_destroy(&priv->reg_lock);
>  }
>  
> +static const struct ub960_hw_data ds90ub954_hw = {
> +	.chip_type = UB954,
> +	.chip_family = FAMILY_FPD3,
> +	.num_rxports = 2,
> +	.num_txports = 1,
> +};
> +
>  static const struct ub960_hw_data ds90ub960_hw = {
>  	.chip_type = UB960,
>  	.chip_family = FAMILY_FPD3,
> @@ -5213,6 +5292,7 @@ static const struct ub960_hw_data ds90ub9702_hw = {
>  };
>  
>  static const struct i2c_device_id ub960_id[] = {
> +	{ "ds90ub954-q1", (kernel_ulong_t)&ds90ub954_hw },
>  	{ "ds90ub960-q1", (kernel_ulong_t)&ds90ub960_hw },
>  	{ "ds90ub9702-q1", (kernel_ulong_t)&ds90ub9702_hw },
>  	{}
> @@ -5220,6 +5300,7 @@ static const struct i2c_device_id ub960_id[] = {
>  MODULE_DEVICE_TABLE(i2c, ub960_id);
>  
>  static const struct of_device_id ub960_dt_ids[] = {
> +	{ .compatible = "ti,ds90ub954-q1", .data = &ds90ub954_hw },
>  	{ .compatible = "ti,ds90ub960-q1", .data = &ds90ub960_hw },
>  	{ .compatible = "ti,ds90ub9702-q1", .data = &ds90ub9702_hw },
>  	{}

 Tomi

Re: [PATCH V2 1/4] media: dt-bindings: ti,ds90ub960: Refactor port definitions

[Rob Herring]

On Tue, Dec 02, 2025 at 03:52:05PM +0530, Yemike Abhilash Chandra wrote:
> The current bindings duplicate the port definitions for each FPD-Link RX
> and CSI-2 TX ports. This results in a large amount of repeated schema
> blocks and makes it harder to extend the bindings for new devices.
> 
> Refactor the bindings by introducing shared deftinitions for FPD-Link
> input ports and CSI-2 output ports. No functional change intended.
> 
> Signed-off-by: Yemike Abhilash Chandra <y-abhilashchandra@ti.com>
> ---
>  .../bindings/media/i2c/ti,ds90ub960.yaml      | 120 +++++++-----------
>  1 file changed, 44 insertions(+), 76 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml b/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml
> index 0539d52de422..6a78288aebaa 100644
> --- a/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml
> +++ b/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml
> @@ -125,102 +125,35 @@ properties:
>  
>    ports:
>      $ref: /schemas/graph.yaml#/properties/ports
> +    description: |

Don't need '|' unless you have formatting to preserve.

> +      Ports represent FPD-Link inputs to the deserializer and CSI TX outputs from the deserializer.
> +      Their number is model-dependent.

'The number of ports is...'

Wrap lines at 80.

>  
>      properties:
>        port@0:
> -        $ref: /schemas/graph.yaml#/$defs/port-base
> -        unevaluatedProperties: false
> +        $ref: '#/$defs/FPDLink-input-port'
>          description: FPD-Link input 0
>  
> -        properties:
> -          endpoint:
> -            $ref: /schemas/media/video-interfaces.yaml#
> -            unevaluatedProperties: false
> -            description:
> -              Endpoint for FPD-Link port. If the RX mode for this port is RAW,
> -              hsync-active and vsync-active must be defined.
> -
>        port@1:
> -        $ref: /schemas/graph.yaml#/$defs/port-base
> -        unevaluatedProperties: false
> +        $ref: '#/$defs/FPDLink-input-port'
>          description: FPD-Link input 1
>  
> -        properties:
> -          endpoint:
> -            $ref: /schemas/media/video-interfaces.yaml#
> -            unevaluatedProperties: false
> -            description:
> -              Endpoint for FPD-Link port. If the RX mode for this port is RAW,
> -              hsync-active and vsync-active must be defined.
> -
>        port@2:
> -        $ref: /schemas/graph.yaml#/$defs/port-base
> -        unevaluatedProperties: false
> +        $ref: '#/$defs/FPDLink-input-port'
>          description: FPD-Link input 2
>  
> -        properties:
> -          endpoint:
> -            $ref: /schemas/media/video-interfaces.yaml#
> -            unevaluatedProperties: false
> -            description:
> -              Endpoint for FPD-Link port. If the RX mode for this port is RAW,
> -              hsync-active and vsync-active must be defined.
> -
>        port@3:
> -        $ref: /schemas/graph.yaml#/$defs/port-base
> -        unevaluatedProperties: false
> +        $ref: '#/$defs/FPDLink-input-port'
>          description: FPD-Link input 3
>  
> -        properties:
> -          endpoint:
> -            $ref: /schemas/media/video-interfaces.yaml#
> -            unevaluatedProperties: false
> -            description:
> -              Endpoint for FPD-Link port. If the RX mode for this port is RAW,
> -              hsync-active and vsync-active must be defined.
> -
>        port@4:
> -        $ref: /schemas/graph.yaml#/$defs/port-base
> -        unevaluatedProperties: false
> +        $ref: '#/$defs/CSI2-output-port'
>          description: CSI-2 Output 0
>  
> -        properties:
> -          endpoint:
> -            $ref: /schemas/media/video-interfaces.yaml#
> -            unevaluatedProperties: false
> -
> -            properties:
> -              data-lanes:
> -                minItems: 1
> -                maxItems: 4
> -              link-frequencies:
> -                maxItems: 1
> -
> -            required:
> -              - data-lanes
> -              - link-frequencies
> -
>        port@5:
> -        $ref: /schemas/graph.yaml#/$defs/port-base
> -        unevaluatedProperties: false
> +        $ref: '#/$defs/CSI2-output-port'
>          description: CSI-2 Output 1
>  
> -        properties:
> -          endpoint:
> -            $ref: /schemas/media/video-interfaces.yaml#
> -            unevaluatedProperties: false
> -
> -            properties:
> -              data-lanes:
> -                minItems: 1
> -                maxItems: 4
> -              link-frequencies:
> -                maxItems: 1
> -
> -            required:
> -              - data-lanes
> -              - link-frequencies
> -
>      required:
>        - port@0
>        - port@1
> @@ -236,6 +169,41 @@ required:
>    - clock-names
>    - ports
>  
> +$defs:
> +  FPDLink-input-port:
> +    $ref: /schemas/graph.yaml#/$defs/port-base
> +    unevaluatedProperties: false
> +    description: FPD-Link input
> +
> +    properties:
> +      endpoint:
> +        $ref: /schemas/media/video-interfaces.yaml#
> +        unevaluatedProperties: false
> +        description:
> +          Endpoint for FPD-Link port. If the RX mode for this port is RAW,
> +          hsync-active and vsync-active must be defined.
> +
> +  CSI2-output-port:
> +    $ref: /schemas/graph.yaml#/$defs/port-base
> +    unevaluatedProperties: false
> +    description: CSI-2 Output
> +
> +    properties:
> +      endpoint:
> +        $ref: /schemas/media/video-interfaces.yaml#
> +        unevaluatedProperties: false
> +
> +        properties:
> +          data-lanes:
> +            minItems: 1
> +            maxItems: 4
> +          link-frequencies:
> +            maxItems: 1
> +
> +        required:
> +          - data-lanes
> +          - link-frequencies
> +
>  unevaluatedProperties: false
>  
>  examples:
> -- 
> 2.34.1
> 

Re: [PATCH V2 3/4] media: dt-bindings: ti,ds90ub960: Add support for DS90UB954-Q1

[Rob Herring]

On Tue, Dec 02, 2025 at 03:52:07PM +0530, Yemike Abhilash Chandra wrote:
> DS90UB954-Q1 is an FPDLink-III deserializer that is mostly register
> compatible with DS90UB960-Q1. The main difference is that it supports
> half of the RX and TX ports, i.e. 2x FPDLink RX ports and 1x CSI TX
> port. Therefore, add support for DS90UB954 within the existing bindings.
> 
> Link: https://www.ti.com/lit/gpn/ds90ub954-q1
> Signed-off-by: Yemike Abhilash Chandra <y-abhilashchandra@ti.com>
> ---
>  .../bindings/media/i2c/ti,ds90ub960.yaml      | 300 +++++++++++++++---
>  1 file changed, 264 insertions(+), 36 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml b/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml
> index 6a78288aebaa..1ef977c2e479 100644
> --- a/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml
> +++ b/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml
> @@ -13,12 +13,10 @@ description:
>    The TI DS90UB9XX devices are FPD-Link video deserializers with I2C and GPIO
>    forwarding.
>  
> -allOf:
> -  - $ref: /schemas/i2c/i2c-atr.yaml#
> -
>  properties:
>    compatible:
>      enum:
> +      - ti,ds90ub954-q1
>        - ti,ds90ub960-q1
>        - ti,ds90ub9702-q1
>  
> @@ -129,39 +127,6 @@ properties:
>        Ports represent FPD-Link inputs to the deserializer and CSI TX outputs from the deserializer.
>        Their number is model-dependent.
>  
> -    properties:
> -      port@0:
> -        $ref: '#/$defs/FPDLink-input-port'
> -        description: FPD-Link input 0
> -
> -      port@1:
> -        $ref: '#/$defs/FPDLink-input-port'
> -        description: FPD-Link input 1
> -
> -      port@2:
> -        $ref: '#/$defs/FPDLink-input-port'
> -        description: FPD-Link input 2
> -
> -      port@3:
> -        $ref: '#/$defs/FPDLink-input-port'
> -        description: FPD-Link input 3
> -
> -      port@4:
> -        $ref: '#/$defs/CSI2-output-port'
> -        description: CSI-2 Output 0
> -
> -      port@5:
> -        $ref: '#/$defs/CSI2-output-port'
> -        description: CSI-2 Output 1
> -
> -    required:
> -      - port@0
> -      - port@1
> -      - port@2
> -      - port@3
> -      - port@4
> -      - port@5
> -
>  required:
>    - compatible
>    - reg
> @@ -204,9 +169,86 @@ $defs:
>            - data-lanes
>            - link-frequencies
>  
> +allOf:
> +  - $ref: /schemas/i2c/i2c-atr.yaml#
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - ti,ds90ub960-q1
> +              - ti,ds90ub9702-q1
> +    then:
> +      properties:
> +        ports:
> +          properties:
> +            port@0:
> +              $ref: '#/$defs/FPDLink-input-port'
> +              description: FPD-Link input 0
> +
> +            port@1:
> +              $ref: '#/$defs/FPDLink-input-port'
> +              description: FPD-Link input 1
> +
> +            port@2:
> +              $ref: '#/$defs/FPDLink-input-port'
> +              description: FPD-Link input 2
> +
> +            port@3:
> +              $ref: '#/$defs/FPDLink-input-port'
> +              description: FPD-Link input 3
> +
> +            port@4:
> +              $ref: '#/$defs/CSI2-output-port'
> +              description: CSI-2 Output 0
> +
> +            port@5:
> +              $ref: '#/$defs/CSI2-output-port'
> +              description: CSI-2 Output 1
> +
> +          required:
> +            - port@0
> +            - port@1
> +            - port@2
> +            - port@3
> +            - port@4
> +            - port@5
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: ti,ds90ub954-q1
> +    then:
> +      properties:
> +        ports:
> +          properties:
> +            port@0:
> +              $ref: '#/$defs/FPDLink-input-port'
> +              description: FPD-Link input 0
> +
> +            port@1:
> +              $ref: '#/$defs/FPDLink-input-port'
> +              description: FPD-Link input 1
> +
> +            port@2:
> +              $ref: '#/$defs/CSI2-output-port'
> +              description: CSI-2 Output 0

Wouldn't making this port@4 be more compatible?

> +
> +          required:
> +            - port@0
> +            - port@1
> +            - port@2
> +
> +        links:
> +          properties:
> +            link@2: false
> +            link@3: false
> +
>  unevaluatedProperties: false
>  
>  examples:
> +  # Example with ds90ub960 Deserializer
>    - |
>      #include <dt-bindings/gpio/gpio.h>
>  
> @@ -406,4 +448,190 @@ examples:
>          };
>        };
>      };
> +
> +  # Example with ds90ub954 Deserializer
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>

I don't think we really need a whole other example for something 
that's a subset.

Rob

Re: [PATCH V2 1/4] media: dt-bindings: ti,ds90ub960: Refactor port definitions

[Yemike Abhilash Chandra]

Hi Rob,
Thanks for the review.

On 05/12/25 20:41, Rob Herring wrote:
> On Tue, Dec 02, 2025 at 03:52:05PM +0530, Yemike Abhilash Chandra wrote:
>> The current bindings duplicate the port definitions for each FPD-Link RX
>> and CSI-2 TX ports. This results in a large amount of repeated schema
>> blocks and makes it harder to extend the bindings for new devices.
>>
>> Refactor the bindings by introducing shared deftinitions for FPD-Link
>> input ports and CSI-2 output ports. No functional change intended.
>>
>> Signed-off-by: Yemike Abhilash Chandra <y-abhilashchandra@ti.com>
>> ---
>>   .../bindings/media/i2c/ti,ds90ub960.yaml      | 120 +++++++-----------
>>   1 file changed, 44 insertions(+), 76 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml b/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml
>> index 0539d52de422..6a78288aebaa 100644
>> --- a/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml
>> +++ b/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml
>> @@ -125,102 +125,35 @@ properties:
>>   
>>     ports:
>>       $ref: /schemas/graph.yaml#/properties/ports
>> +    description: |
> 
> Don't need '|' unless you have formatting to preserve.
> 

Understood. I will fix this in v3.

>> +      Ports represent FPD-Link inputs to the deserializer and CSI TX outputs from the deserializer.
>> +      Their number is model-dependent.
> 
> 'The number of ports is...'
> 
> Wrap lines at 80.
> 

I will fix this in v3.

Thanks and Regards,
Yemike Abhilash Chandra

>>   
>>       properties:
>>         port@0:
>> -        $ref: /schemas/graph.yaml#/$defs/port-base
>> -        unevaluatedProperties: false
>> +        $ref: '#/$defs/FPDLink-input-port'
>>           description: FPD-Link input 0
>>   
>> -        properties:
>> -          endpoint:
>> -            $ref: /schemas/media/video-interfaces.yaml#
>> -            unevaluatedProperties: false
>> -            description:
>> -              Endpoint for FPD-Link port. If the RX mode for this port is RAW,
>> -              hsync-active and vsync-active must be defined.
>> -
>>         port@1:
>> -        $ref: /schemas/graph.yaml#/$defs/port-base
>> -        unevaluatedProperties: false
>> +        $ref: '#/$defs/FPDLink-input-port'
>>           description: FPD-Link input 1
>>   
>> -        properties:
>> -          endpoint:
>> -            $ref: /schemas/media/video-interfaces.yaml#
>> -            unevaluatedProperties: false
>> -            description:
>> -              Endpoint for FPD-Link port. If the RX mode for this port is RAW,
>> -              hsync-active and vsync-active must be defined.
>> -
>>         port@2:
>> -        $ref: /schemas/graph.yaml#/$defs/port-base
>> -        unevaluatedProperties: false
>> +        $ref: '#/$defs/FPDLink-input-port'
>>           description: FPD-Link input 2
>>   
>> -        properties:
>> -          endpoint:
>> -            $ref: /schemas/media/video-interfaces.yaml#
>> -            unevaluatedProperties: false
>> -            description:
>> -              Endpoint for FPD-Link port. If the RX mode for this port is RAW,
>> -              hsync-active and vsync-active must be defined.
>> -
>>         port@3:
>> -        $ref: /schemas/graph.yaml#/$defs/port-base
>> -        unevaluatedProperties: false
>> +        $ref: '#/$defs/FPDLink-input-port'
>>           description: FPD-Link input 3
>>   
>> -        properties:
>> -          endpoint:
>> -            $ref: /schemas/media/video-interfaces.yaml#
>> -            unevaluatedProperties: false
>> -            description:
>> -              Endpoint for FPD-Link port. If the RX mode for this port is RAW,
>> -              hsync-active and vsync-active must be defined.
>> -
>>         port@4:
>> -        $ref: /schemas/graph.yaml#/$defs/port-base
>> -        unevaluatedProperties: false
>> +        $ref: '#/$defs/CSI2-output-port'
>>           description: CSI-2 Output 0
>>   
>> -        properties:
>> -          endpoint:
>> -            $ref: /schemas/media/video-interfaces.yaml#
>> -            unevaluatedProperties: false
>> -
>> -            properties:
>> -              data-lanes:
>> -                minItems: 1
>> -                maxItems: 4
>> -              link-frequencies:
>> -                maxItems: 1
>> -
>> -            required:
>> -              - data-lanes
>> -              - link-frequencies
>> -
>>         port@5:
>> -        $ref: /schemas/graph.yaml#/$defs/port-base
>> -        unevaluatedProperties: false
>> +        $ref: '#/$defs/CSI2-output-port'
>>           description: CSI-2 Output 1
>>   
>> -        properties:
>> -          endpoint:
>> -            $ref: /schemas/media/video-interfaces.yaml#
>> -            unevaluatedProperties: false
>> -
>> -            properties:
>> -              data-lanes:
>> -                minItems: 1
>> -                maxItems: 4
>> -              link-frequencies:
>> -                maxItems: 1
>> -
>> -            required:
>> -              - data-lanes
>> -              - link-frequencies
>> -
>>       required:
>>         - port@0
>>         - port@1
>> @@ -236,6 +169,41 @@ required:
>>     - clock-names
>>     - ports
>>   
>> +$defs:
>> +  FPDLink-input-port:
>> +    $ref: /schemas/graph.yaml#/$defs/port-base
>> +    unevaluatedProperties: false
>> +    description: FPD-Link input
>> +
>> +    properties:
>> +      endpoint:
>> +        $ref: /schemas/media/video-interfaces.yaml#
>> +        unevaluatedProperties: false
>> +        description:
>> +          Endpoint for FPD-Link port. If the RX mode for this port is RAW,
>> +          hsync-active and vsync-active must be defined.
>> +
>> +  CSI2-output-port:
>> +    $ref: /schemas/graph.yaml#/$defs/port-base
>> +    unevaluatedProperties: false
>> +    description: CSI-2 Output
>> +
>> +    properties:
>> +      endpoint:
>> +        $ref: /schemas/media/video-interfaces.yaml#
>> +        unevaluatedProperties: false
>> +
>> +        properties:
>> +          data-lanes:
>> +            minItems: 1
>> +            maxItems: 4
>> +          link-frequencies:
>> +            maxItems: 1
>> +
>> +        required:
>> +          - data-lanes
>> +          - link-frequencies
>> +
>>   unevaluatedProperties: false
>>   
>>   examples:
>> -- 
>> 2.34.1
>>

Re: [PATCH V2 3/4] media: dt-bindings: ti,ds90ub960: Add support for DS90UB954-Q1

[Yemike Abhilash Chandra]

On 05/12/25 20:47, Rob Herring wrote:
> On Tue, Dec 02, 2025 at 03:52:07PM +0530, Yemike Abhilash Chandra wrote:
>> DS90UB954-Q1 is an FPDLink-III deserializer that is mostly register
>> compatible with DS90UB960-Q1. The main difference is that it supports
>> half of the RX and TX ports, i.e. 2x FPDLink RX ports and 1x CSI TX
>> port. Therefore, add support for DS90UB954 within the existing bindings.
>>
>> Link: https://www.ti.com/lit/gpn/ds90ub954-q1
>> Signed-off-by: Yemike Abhilash Chandra <y-abhilashchandra@ti.com>
>> ---
>>   .../bindings/media/i2c/ti,ds90ub960.yaml      | 300 +++++++++++++++---
>>   1 file changed, 264 insertions(+), 36 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml b/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml
>> index 6a78288aebaa..1ef977c2e479 100644
>> --- a/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml
>> +++ b/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml
>> @@ -13,12 +13,10 @@ description:
>>     The TI DS90UB9XX devices are FPD-Link video deserializers with I2C and GPIO
>>     forwarding.
>>   
>> -allOf:
>> -  - $ref: /schemas/i2c/i2c-atr.yaml#
>> -
>>   properties:
>>     compatible:
>>       enum:
>> +      - ti,ds90ub954-q1
>>         - ti,ds90ub960-q1
>>         - ti,ds90ub9702-q1
>>   
>> @@ -129,39 +127,6 @@ properties:
>>         Ports represent FPD-Link inputs to the deserializer and CSI TX outputs from the deserializer.
>>         Their number is model-dependent.
>>   
>> -    properties:
>> -      port@0:
>> -        $ref: '#/$defs/FPDLink-input-port'
>> -        description: FPD-Link input 0
>> -
>> -      port@1:
>> -        $ref: '#/$defs/FPDLink-input-port'
>> -        description: FPD-Link input 1
>> -
>> -      port@2:
>> -        $ref: '#/$defs/FPDLink-input-port'
>> -        description: FPD-Link input 2
>> -
>> -      port@3:
>> -        $ref: '#/$defs/FPDLink-input-port'
>> -        description: FPD-Link input 3
>> -
>> -      port@4:
>> -        $ref: '#/$defs/CSI2-output-port'
>> -        description: CSI-2 Output 0
>> -
>> -      port@5:
>> -        $ref: '#/$defs/CSI2-output-port'
>> -        description: CSI-2 Output 1
>> -
>> -    required:
>> -      - port@0
>> -      - port@1
>> -      - port@2
>> -      - port@3
>> -      - port@4
>> -      - port@5
>> -
>>   required:
>>     - compatible
>>     - reg
>> @@ -204,9 +169,86 @@ $defs:
>>             - data-lanes
>>             - link-frequencies
>>   
>> +allOf:
>> +  - $ref: /schemas/i2c/i2c-atr.yaml#
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - ti,ds90ub960-q1
>> +              - ti,ds90ub9702-q1
>> +    then:
>> +      properties:
>> +        ports:
>> +          properties:
>> +            port@0:
>> +              $ref: '#/$defs/FPDLink-input-port'
>> +              description: FPD-Link input 0
>> +
>> +            port@1:
>> +              $ref: '#/$defs/FPDLink-input-port'
>> +              description: FPD-Link input 1
>> +
>> +            port@2:
>> +              $ref: '#/$defs/FPDLink-input-port'
>> +              description: FPD-Link input 2
>> +
>> +            port@3:
>> +              $ref: '#/$defs/FPDLink-input-port'
>> +              description: FPD-Link input 3
>> +
>> +            port@4:
>> +              $ref: '#/$defs/CSI2-output-port'
>> +              description: CSI-2 Output 0
>> +
>> +            port@5:
>> +              $ref: '#/$defs/CSI2-output-port'
>> +              description: CSI-2 Output 1
>> +
>> +          required:
>> +            - port@0
>> +            - port@1
>> +            - port@2
>> +            - port@3
>> +            - port@4
>> +            - port@5
>> +
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            const: ti,ds90ub954-q1
>> +    then:
>> +      properties:
>> +        ports:
>> +          properties:
>> +            port@0:
>> +              $ref: '#/$defs/FPDLink-input-port'
>> +              description: FPD-Link input 0
>> +
>> +            port@1:
>> +              $ref: '#/$defs/FPDLink-input-port'
>> +              description: FPD-Link input 1
>> +
>> +            port@2:
>> +              $ref: '#/$defs/CSI2-output-port'
>> +              description: CSI-2 Output 0
> 
> Wouldn't making this port@4 be more compatible?
> 

The current driver expects the TX port numbers to start immediately 
after the RX ports.
If I change this to port@4, I would also need to update the driver.

Can I keep the port numbering as it is?

>> +
>> +          required:
>> +            - port@0
>> +            - port@1
>> +            - port@2
>> +
>> +        links:
>> +          properties:
>> +            link@2: false
>> +            link@3: false
>> +
>>   unevaluatedProperties: false
>>   
>>   examples:
>> +  # Example with ds90ub960 Deserializer
>>     - |
>>       #include <dt-bindings/gpio/gpio.h>
>>   
>> @@ -406,4 +448,190 @@ examples:
>>           };
>>         };
>>       };
>> +
>> +  # Example with ds90ub954 Deserializer
>> +  - |
>> +    #include <dt-bindings/gpio/gpio.h>
> 
> I don't think we really need a whole other example for something
> that's a subset.
> 

Make sense. I will remove the example in v3.

Thanks and Regards,
Yemike Abhilash Chandra

> Rob
>