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

[PATCH V3 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 v3:
- Remove the '|' character from the ports description since preserving formatting is not required. (Rob)
- Wrap the ports description at 80 characters. (Rob)
- Remove the example added for DS90UB954, as it is just a subset of the DS90UB960 example. (Rob)
- Change conditional checks to check for applicable chips over negated checks. (Tomi)
- Keep the model name in the ub960_hw_data structure and remove the switch-case from the probe function. (Tomi)
- Remove redundant reads and writes to the same register. (Tomi)
- Correct the bit positions for data delay in set_strobe_pos for UB954. (Tomi)
- Address a few minor nitpicks in code comments. (Tomi)

Test logs: https://gist.github.com/Yemike-Abhilash-Chandra/ba055ea9fd2c559d0bd939c3da724e4d
DT binding check results: https://gist.github.com/Yemike-Abhilash-Chandra/1092d51ca809bfa494449e6d7f06df73
(No errors related to ti,ds90ub960.yaml)

Link for v2: https://lore.kernel.org/all/20251202102208.80713-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      | 213 ++++++++---------
 drivers/media/i2c/Kconfig                     |   4 +-
 drivers/media/i2c/ds90ub960.c                 | 216 ++++++++++++------
 3 files changed, 263 insertions(+), 170 deletions(-)

-- 
2.34.1

[PATCH V3 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>
---
Changelog:
Changes in v3:
- Remove the | character from the ports description since preserving formatting is not required. (Rob)
- Wrap the ports description at 80 characters. (Rob)

 .../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..cc61604eca37 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. The number of ports 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 V3 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>
---
Changelog:
Changes in v3:
- Change conditional checks to check for applicable chips over negated checks. (Tomi)
- Keep the model name in the ub960_hw_data structure and remove the switch-case from the probe function. (Tomi)

 drivers/media/i2c/ds90ub960.c | 38 +++++++++++++++++++++++------------
 1 file changed, 25 insertions(+), 13 deletions(-)

diff --git a/drivers/media/i2c/ds90ub960.c b/drivers/media/i2c/ds90ub960.c
index 5a83218e64ab..bb453d39e0c1 100644
--- a/drivers/media/i2c/ds90ub960.c
+++ b/drivers/media/i2c/ds90ub960.c
@@ -454,12 +454,22 @@
 #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 +1943,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 +2205,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 +2371,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 +3643,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 == 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 +4269,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 == UB960) {
 			ret = ub960_log_status_ub960_sp_eq(priv, nport);
 			if (ret)
 				return ret;
@@ -4417,7 +4427,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;
 	}
@@ -5019,7 +5029,7 @@ static int ub960_enable_core_hw(struct ub960_data *priv)
 	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 +5048,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 +5121,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);
@@ -5180,16 +5190,18 @@ 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 V3 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>
---
Changelog:
Changes in v3:
- Remove the example added for DS90UB954, as it is just a subset of the DS90UB960 example. (Rob)

 .../bindings/media/i2c/ti,ds90ub960.yaml      | 113 ++++++++++++------
 1 file changed, 77 insertions(+), 36 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml b/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml
index cc61604eca37..8e2b82d6dc81 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. The number of ports 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,6 +169,82 @@ $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:
-- 
2.34.1

[PATCH V3 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>
---
Changelog:
Changes in v3:
- Remove redundant reads and writes to the same register. (Tomi)
- Correct the bit positions for data delay in set_strobe_pos for UB954. (Tomi)
- Address a few minor nitpicks in code comments. (Tomi)

 drivers/media/i2c/Kconfig     |   4 +-
 drivers/media/i2c/ds90ub960.c | 182 ++++++++++++++++++++++++----------
 2 files changed, 129 insertions(+), 57 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 bb453d39e0c1..0b8280e88bd8 100644
--- a/drivers/media/i2c/ds90ub960.c
+++ b/drivers/media/i2c/ds90ub960.c
@@ -396,6 +396,13 @@
 #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)
+#define UB954_IR_RX_ANA_STROBE_SET_DATA_DELAY_SHIFT	4
+
 /* UB9702 Registers */
 
 #define UB9702_SR_CSI_EXCLUSIVE_FWD2		0x3c
@@ -455,6 +462,7 @@
 #define UB960_NUM_EQ_LEVELS (UB960_MAX_EQ_LEVEL - UB960_MIN_EQ_LEVEL + 1)
 
 enum chip_type {
+	UB954,
 	UB960,
 	UB9702,
 };
@@ -1001,6 +1009,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;
 
@@ -1425,10 +1437,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;
@@ -1552,22 +1565,35 @@ 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;
+	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;
+		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;
@@ -1588,26 +1614,49 @@ static int ub960_rxport_get_strobe_pos(struct ub960_data *priv,
 static int ub960_rxport_set_strobe_pos(struct ub960_data *priv,
 				       unsigned int nport, s8 strobe_pos)
 {
-	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;
-
-	if (strobe_pos < UB960_MIN_AEQ_STROBE_POS)
-		clk_delay = abs(strobe_pos) - UB960_MANUAL_STROBE_EXTRA_DELAY;
-	else if (strobe_pos > UB960_MAX_AEQ_STROBE_POS)
-		data_delay = strobe_pos - UB960_MANUAL_STROBE_EXTRA_DELAY;
-	else if (strobe_pos < 0)
-		clk_delay = abs(strobe_pos) | UB960_IR_RX_ANA_STROBE_SET_CLK_NO_EXTRA_DELAY;
-	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);
+	if (priv->hw_data->chip_type == UB954) {
+		u8 clk_data_delay;
+
+		clk_data_delay = UB954_IR_RX_ANA_STROBE_SET_CLK_NO_EXTRA_DELAY |
+				 UB954_IR_RX_ANA_STROBE_SET_DATA_NO_EXTRA_DELAY;
+
+		if (strobe_pos < UB960_MIN_AEQ_STROBE_POS)
+			clk_data_delay = abs(strobe_pos) - UB960_MANUAL_STROBE_EXTRA_DELAY;
+		else if (strobe_pos > UB960_MAX_AEQ_STROBE_POS)
+			clk_data_delay = (strobe_pos - UB960_MANUAL_STROBE_EXTRA_DELAY) <<
+					  UB954_IR_RX_ANA_STROBE_SET_DATA_DELAY_SHIFT;
+		else if (strobe_pos < 0)
+			clk_data_delay = abs(strobe_pos) |
+					 UB954_IR_RX_ANA_STROBE_SET_CLK_NO_EXTRA_DELAY;
+		else if (strobe_pos > 0)
+			clk_data_delay = (strobe_pos |
+					  UB954_IR_RX_ANA_STROBE_SET_DATA_NO_EXTRA_DELAY) <<
+					  UB954_IR_RX_ANA_STROBE_SET_DATA_DELAY_SHIFT;
+
+		ub960_write_ind(priv, UB960_IND_TARGET_RX_ANA(nport),
+				UB954_IR_RX_ANA_STROBE_SET_CLK_DATA, clk_data_delay, &ret);
+	} else {
+		u8 clk_delay, data_delay;
+
+		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;
+		else if (strobe_pos > UB960_MAX_AEQ_STROBE_POS)
+			data_delay = strobe_pos - UB960_MANUAL_STROBE_EXTRA_DELAY;
+		else if (strobe_pos < 0)
+			clk_delay = abs(strobe_pos) | UB960_IR_RX_ANA_STROBE_SET_CLK_NO_EXTRA_DELAY;
+		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);
+	}
 
 	return ret;
 }
@@ -3643,7 +3692,8 @@ 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->chip_type == UB960) {
+			if (priv->hw_data->chip_type == UB960 ||
+			    priv->hw_data->chip_type == UB954) {
 				/* 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)) |
@@ -4177,33 +4227,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 == UB960 || priv->hw_data->chip_type == UB9702) {
+			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) {
@@ -4269,7 +4326,7 @@ static int ub960_log_status(struct v4l2_subdev *sd)
 
 		dev_info(dev, "\tcsi_err_counter %u\n", v);
 
-		if (priv->hw_data->chip_type == UB960) {
+		if (priv->hw_data->chip_type == UB960 || priv->hw_data->chip_type == UB954) {
 			ret = ub960_log_status_ub960_sp_eq(priv, nport);
 			if (ret)
 				return ret;
@@ -5029,6 +5086,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, a single read is practically seen to be
+	 * sufficient and moreover it 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);
@@ -5188,6 +5250,14 @@ static void ub960_remove(struct i2c_client *client)
 	mutex_destroy(&priv->reg_lock);
 }
 
+static const struct ub960_hw_data ds90ub954_hw = {
+	.model = "ub954",
+	.chip_type = UB954,
+	.chip_family = FAMILY_FPD3,
+	.num_rxports = 2,
+	.num_txports = 1,
+};
+
 static const struct ub960_hw_data ds90ub960_hw = {
 	.model = "ub960",
 	.chip_type = UB960,
@@ -5205,6 +5275,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 },
 	{}
@@ -5212,6 +5283,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 V3 2/4] media: i2c: ds90ub960: Use enums for chip type and chip family

[Tomi Valkeinen]

Hi,

On 19/12/2025 14:29, 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>
> ---
> Changelog:
> Changes in v3:
> - Change conditional checks to check for applicable chips over negated checks. (Tomi)
> - Keep the model name in the ub960_hw_data structure and remove the switch-case from the probe function. (Tomi)
> 
>  drivers/media/i2c/ds90ub960.c | 38 +++++++++++++++++++++++------------
>  1 file changed, 25 insertions(+), 13 deletions(-)
Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

 Tomi

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

[Tomi Valkeinen]

Hi,

On 19/12/2025 14:29, 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>
> ---
> Changelog:
> Changes in v3:
> - Remove redundant reads and writes to the same register. (Tomi)
> - Correct the bit positions for data delay in set_strobe_pos for UB954. (Tomi)
> - Address a few minor nitpicks in code comments. (Tomi)
> 
>  drivers/media/i2c/Kconfig     |   4 +-
>  drivers/media/i2c/ds90ub960.c | 182 ++++++++++++++++++++++++----------
>  2 files changed, 129 insertions(+), 57 deletions(-)
Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

 Tomi

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

[Tomi Valkeinen]

Hi,

On 19/12/2025 14:29, 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>
> ---
> Changelog:
> Changes in v3:
> - Remove the | character from the ports description since preserving formatting is not required. (Rob)
> - Wrap the ports description at 80 characters. (Rob)
> 
>  .../bindings/media/i2c/ti,ds90ub960.yaml      | 120 +++++++-----------
>  1 file changed, 44 insertions(+), 76 deletions(-)
My DT skills are not good enough to say if this is 100% correct or not,
but it makes sense and looks good to me. So:

Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

 Tomi

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

[Tomi Valkeinen]

Hi,

On 19/12/2025 14:29, 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>
> ---
> Changelog:
> Changes in v3:
> - Remove the example added for DS90UB954, as it is just a subset of the DS90UB960 example. (Rob)
> 
>  .../bindings/media/i2c/ti,ds90ub960.yaml      | 113 ++++++++++++------
>  1 file changed, 77 insertions(+), 36 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml b/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml
> index cc61604eca37..8e2b82d6dc81 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. The number of ports 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,6 +169,82 @@ $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
I can't help but think if this is good or not. In other words, if we
specifically add ports per compatible, why wouldn't we also add
specifically links per compatible? Or, if we just disable links as
above, why don't we do it the same way for ports?

 Tomi

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

[Rob Herring (Arm)]

On Fri, 19 Dec 2025 17:59:52 +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>
> ---
> Changelog:
> Changes in v3:
> - Remove the | character from the ports description since preserving formatting is not required. (Rob)
> - Wrap the ports description at 80 characters. (Rob)
> 
>  .../bindings/media/i2c/ti,ds90ub960.yaml      | 120 +++++++-----------
>  1 file changed, 44 insertions(+), 76 deletions(-)
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>

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

[Rob Herring (Arm)]

On Fri, 19 Dec 2025 17:59:54 +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>
> ---
> Changelog:
> Changes in v3:
> - Remove the example added for DS90UB954, as it is just a subset of the DS90UB960 example. (Rob)
> 
>  .../bindings/media/i2c/ti,ds90ub960.yaml      | 113 ++++++++++++------
>  1 file changed, 77 insertions(+), 36 deletions(-)
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>

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

[Yemike Abhilash Chandra]

Hi Tomi,

Thank you for the review.

On 22/12/25 16:59, Tomi Valkeinen wrote:
> Hi,
> 
> On 19/12/2025 14:29, 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>
>> ---
>> Changelog:
>> Changes in v3:
>> - Remove the example added for DS90UB954, as it is just a subset of the DS90UB960 example. (Rob)
>>
>>   .../bindings/media/i2c/ti,ds90ub960.yaml      | 113 ++++++++++++------
>>   1 file changed, 77 insertions(+), 36 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml b/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml
>> index cc61604eca37..8e2b82d6dc81 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. The number of ports 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,6 +169,82 @@ $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
> I can't help but think if this is good or not. In other words, if we
> specifically add ports per compatible, why wouldn't we also add
> specifically links per compatible? Or, if we just disable links as
> above, why don't we do it the same way for ports?
> 

Quoting writing schemas:

"When bindings cover multiple similar devices that differ in some 
properties,
those properties should be constrained for each device. This usually means:

  * In top level 'properties' define the property with the broadest 
constraints.
  * In 'if:then:' blocks, further narrow the constraints for those 
properties.
  * Do not define the properties within an 'if:then:' block (note that
    'additionalItems' also won't allow that)."


Since new properties cannot be introduced inside allOf / if:then, it is 
not possible to define
device-specific patternProperties for links directly under each 
condition like below

- if:
     properties:
       compatible:
         contains:
           enum:
             - ti,ds90ub960-q1
             - ti,ds90ub9702-q1
   then:
     properties:
       links:
         patternProperties:
           '^link@[0-3]$':

- if:
     properties:
       compatible:
         contains:
           const: ti,ds90ub954-q1

   then:
     properties:
       links:
	patternProperties:
	  '^link@[0-1]$':

Therefore, a broad top-level definition such as:

patternProperties:
   '^link@[0-3]$':

is required, with device-specific constraints applied later via 
conditional logic

This works for ports since we have a

patternProperties:
   '^port@[0-9a-f]+$':

already defined at /schemas/graph.yaml#/properties/ports which we refer 
in the top level schema

Note that an alternative could also be modeled by explicitly disallowing 
unused ports for ds90ub954,
namely port 2, port3 and port 5 but that approach would require changes 
in the driver, as it currently
assumes that TX ports start immediately after RX ports. Hence I 
preferred having ports 0, 1 and 2 for ds90ub954

The approach that was used in this patch was also hinted at in our 
earlier discussion at
https://lore.kernel.org/all/58b309d9-de03-4818-8d38-a27cc68466db@ideasonboard.com/

Thanks and Regards,
Yemike Abhilash Chandra


>   Tomi
> 

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

[Jai Luthra]

Hi Abhilash,

Thank you for the patch.

Quoting Yemike Abhilash Chandra (2025-12-19 17:59:53)
> 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>
> ---
> Changelog:
> Changes in v3:
> - Change conditional checks to check for applicable chips over negated checks. (Tomi)
> - Keep the model name in the ub960_hw_data structure and remove the switch-case from the probe function. (Tomi)
> 
>  drivers/media/i2c/ds90ub960.c | 38 +++++++++++++++++++++++------------
>  1 file changed, 25 insertions(+), 13 deletions(-)
> 

Reviewed-by: Jai Luthra <jai.luthra@ideasonboard.com>

[...]

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

[Jai Luthra]

Hi Abhilash,

Quoting Yemike Abhilash Chandra (2025-12-19 17:59:55)
> 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>
> ---
> Changelog:
> Changes in v3:
> - Remove redundant reads and writes to the same register. (Tomi)
> - Correct the bit positions for data delay in set_strobe_pos for UB954. (Tomi)
> - Address a few minor nitpicks in code comments. (Tomi)
> 
>  drivers/media/i2c/Kconfig     |   4 +-
>  drivers/media/i2c/ds90ub960.c | 182 ++++++++++++++++++++++++----------
>  2 files changed, 129 insertions(+), 57 deletions(-)
> 

Reviewed-by: Jai Luthra <jai.luthra@ideasonboard.com>

[...]

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

[Tomi Valkeinen]

Hi,

On 06/01/2026 12:06, Yemike Abhilash Chandra wrote:
> Hi Tomi,
> 
> Thank you for the review.
> 
> On 22/12/25 16:59, Tomi Valkeinen wrote:
>> Hi,
>>
>> On 19/12/2025 14:29, 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>
>>> ---
>>> Changelog:
>>> Changes in v3:
>>> - Remove the example added for DS90UB954, as it is just a subset of
>>> the DS90UB960 example. (Rob)
>>>
>>>   .../bindings/media/i2c/ti,ds90ub960.yaml      | 113 ++++++++++++------
>>>   1 file changed, 77 insertions(+), 36 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/media/i2c/
>>> ti,ds90ub960.yaml b/Documentation/devicetree/bindings/media/i2c/
>>> ti,ds90ub960.yaml
>>> index cc61604eca37..8e2b82d6dc81 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. The number of ports 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,6 +169,82 @@ $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
>> I can't help but think if this is good or not. In other words, if we
>> specifically add ports per compatible, why wouldn't we also add
>> specifically links per compatible? Or, if we just disable links as
>> above, why don't we do it the same way for ports?
>>
> 
> Quoting writing schemas:
> 
> "When bindings cover multiple similar devices that differ in some
> properties,
> those properties should be constrained for each device. This usually means:
> 
>  * In top level 'properties' define the property with the broadest
> constraints.
>  * In 'if:then:' blocks, further narrow the constraints for those
> properties.
>  * Do not define the properties within an 'if:then:' block (note that
>    'additionalItems' also won't allow that)."
> 
> 
> Since new properties cannot be introduced inside allOf / if:then, it is
> not possible to define
> device-specific patternProperties for links directly under each
> condition like below
> 
> - if:
>     properties:
>       compatible:
>         contains:
>           enum:
>             - ti,ds90ub960-q1
>             - ti,ds90ub9702-q1
>   then:
>     properties:
>       links:
>         patternProperties:
>           '^link@[0-3]$':
> 
> - if:
>     properties:
>       compatible:
>         contains:
>           const: ti,ds90ub954-q1
> 
>   then:
>     properties:
>       links:
>     patternProperties:
>       '^link@[0-1]$':
> 
> Therefore, a broad top-level definition such as:
> 
> patternProperties:
>   '^link@[0-3]$':
> 
> is required, with device-specific constraints applied later via
> conditional logic
> 
> This works for ports since we have a
> 
> patternProperties:
>   '^port@[0-9a-f]+$':
> 
> already defined at /schemas/graph.yaml#/properties/ports which we refer
> in the top level schema
> 
> Note that an alternative could also be modeled by explicitly disallowing
> unused ports for ds90ub954,
> namely port 2, port3 and port 5 but that approach would require changes
> in the driver, as it currently
> assumes that TX ports start immediately after RX ports. Hence I
> preferred having ports 0, 1 and 2 for ds90ub954

I see. Yes, if we would just disable ports for ub954, we'd have ports 0,
1 and 4.

Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

 Tomi

> The approach that was used in this patch was also hinted at in our
> earlier discussion at
> https://lore.kernel.org/all/58b309d9-de03-4818-8d38-
> a27cc68466db@ideasonboard.com/
> 
> Thanks and Regards,
> Yemike Abhilash Chandra
> 
> 
>>   Tomi
>>
>