* [PATCH V2 0/4] Add support for DS90UB954-Q1
@ 2025-12-02 10:22 Yemike Abhilash Chandra
2025-12-02 10:22 ` [PATCH V2 1/4] media: dt-bindings: ti,ds90ub960: Refactor port definitions Yemike Abhilash Chandra
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Yemike Abhilash Chandra @ 2025-12-02 10:22 UTC (permalink / raw)
To: tomi.valkeinen, mchehab, robh, krzk+dt, conor+dt, hverkuil,
sakari.ailus, laurent.pinchart
Cc: hansg, mehdi.djait, ribalda, git, vladimir.zapolskiy,
benjamin.mugnier, dongcheng.yan, u-kumar1, jai.luthra,
linux-media, devicetree, linux-kernel, y-abhilashchandra
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
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH V2 1/4] media: dt-bindings: ti,ds90ub960: Refactor port definitions 2025-12-02 10:22 [PATCH V2 0/4] Add support for DS90UB954-Q1 Yemike Abhilash Chandra @ 2025-12-02 10:22 ` Yemike Abhilash Chandra 2025-12-05 15:11 ` Rob Herring 2025-12-02 10:22 ` [PATCH V2 2/4] media: i2c: ds90ub960: Use enums for chip type and chip family Yemike Abhilash Chandra ` (2 subsequent siblings) 3 siblings, 1 reply; 11+ messages in thread From: Yemike Abhilash Chandra @ 2025-12-02 10:22 UTC (permalink / raw) To: tomi.valkeinen, mchehab, robh, krzk+dt, conor+dt, hverkuil, sakari.ailus, laurent.pinchart Cc: hansg, mehdi.djait, ribalda, git, vladimir.zapolskiy, benjamin.mugnier, dongcheng.yan, u-kumar1, jai.luthra, linux-media, devicetree, linux-kernel, y-abhilashchandra 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH V2 1/4] media: dt-bindings: ti,ds90ub960: Refactor port definitions 2025-12-02 10:22 ` [PATCH V2 1/4] media: dt-bindings: ti,ds90ub960: Refactor port definitions Yemike Abhilash Chandra @ 2025-12-05 15:11 ` Rob Herring 2025-12-10 9:25 ` Yemike Abhilash Chandra 0 siblings, 1 reply; 11+ messages in thread From: Rob Herring @ 2025-12-05 15:11 UTC (permalink / raw) To: Yemike Abhilash Chandra Cc: tomi.valkeinen, mchehab, krzk+dt, conor+dt, hverkuil, sakari.ailus, laurent.pinchart, hansg, mehdi.djait, ribalda, git, vladimir.zapolskiy, benjamin.mugnier, dongcheng.yan, u-kumar1, jai.luthra, linux-media, devicetree, linux-kernel 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 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH V2 1/4] media: dt-bindings: ti,ds90ub960: Refactor port definitions 2025-12-05 15:11 ` Rob Herring @ 2025-12-10 9:25 ` Yemike Abhilash Chandra 0 siblings, 0 replies; 11+ messages in thread From: Yemike Abhilash Chandra @ 2025-12-10 9:25 UTC (permalink / raw) To: Rob Herring Cc: tomi.valkeinen, mchehab, krzk+dt, conor+dt, hverkuil, sakari.ailus, laurent.pinchart, hansg, mehdi.djait, ribalda, git, vladimir.zapolskiy, benjamin.mugnier, dongcheng.yan, u-kumar1, jai.luthra, linux-media, devicetree, linux-kernel 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 >> ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH V2 2/4] media: i2c: ds90ub960: Use enums for chip type and chip family 2025-12-02 10:22 [PATCH V2 0/4] Add support for DS90UB954-Q1 Yemike Abhilash Chandra 2025-12-02 10:22 ` [PATCH V2 1/4] media: dt-bindings: ti,ds90ub960: Refactor port definitions Yemike Abhilash Chandra @ 2025-12-02 10:22 ` Yemike Abhilash Chandra 2025-12-05 10:46 ` Tomi Valkeinen 2025-12-02 10:22 ` [PATCH V2 3/4] media: dt-bindings: ti,ds90ub960: Add support for DS90UB954-Q1 Yemike Abhilash Chandra 2025-12-02 10:22 ` [PATCH V2 4/4] media: i2c: ds90ub960: " Yemike Abhilash Chandra 3 siblings, 1 reply; 11+ messages in thread From: Yemike Abhilash Chandra @ 2025-12-02 10:22 UTC (permalink / raw) To: tomi.valkeinen, mchehab, robh, krzk+dt, conor+dt, hverkuil, sakari.ailus, laurent.pinchart Cc: hansg, mehdi.djait, ribalda, git, vladimir.zapolskiy, benjamin.mugnier, dongcheng.yan, u-kumar1, jai.luthra, linux-media, devicetree, linux-kernel, y-abhilashchandra 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH V2 2/4] media: i2c: ds90ub960: Use enums for chip type and chip family 2025-12-02 10:22 ` [PATCH V2 2/4] media: i2c: ds90ub960: Use enums for chip type and chip family Yemike Abhilash Chandra @ 2025-12-05 10:46 ` Tomi Valkeinen 0 siblings, 0 replies; 11+ messages in thread From: Tomi Valkeinen @ 2025-12-05 10:46 UTC (permalink / raw) To: Yemike Abhilash Chandra Cc: hansg, mehdi.djait, ribalda, git, vladimir.zapolskiy, benjamin.mugnier, dongcheng.yan, u-kumar1, jai.luthra, linux-media, devicetree, linux-kernel, mchehab, robh, krzk+dt, conor+dt, hverkuil, sakari.ailus, laurent.pinchart 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH V2 3/4] media: dt-bindings: ti,ds90ub960: Add support for DS90UB954-Q1 2025-12-02 10:22 [PATCH V2 0/4] Add support for DS90UB954-Q1 Yemike Abhilash Chandra 2025-12-02 10:22 ` [PATCH V2 1/4] media: dt-bindings: ti,ds90ub960: Refactor port definitions Yemike Abhilash Chandra 2025-12-02 10:22 ` [PATCH V2 2/4] media: i2c: ds90ub960: Use enums for chip type and chip family Yemike Abhilash Chandra @ 2025-12-02 10:22 ` Yemike Abhilash Chandra 2025-12-05 15:17 ` Rob Herring 2025-12-02 10:22 ` [PATCH V2 4/4] media: i2c: ds90ub960: " Yemike Abhilash Chandra 3 siblings, 1 reply; 11+ messages in thread From: Yemike Abhilash Chandra @ 2025-12-02 10:22 UTC (permalink / raw) To: tomi.valkeinen, mchehab, robh, krzk+dt, conor+dt, hverkuil, sakari.ailus, laurent.pinchart Cc: hansg, mehdi.djait, ribalda, git, vladimir.zapolskiy, benjamin.mugnier, dongcheng.yan, u-kumar1, jai.luthra, linux-media, devicetree, linux-kernel, y-abhilashchandra 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH V2 3/4] media: dt-bindings: ti,ds90ub960: Add support for DS90UB954-Q1 2025-12-02 10:22 ` [PATCH V2 3/4] media: dt-bindings: ti,ds90ub960: Add support for DS90UB954-Q1 Yemike Abhilash Chandra @ 2025-12-05 15:17 ` Rob Herring 2025-12-10 9:33 ` Yemike Abhilash Chandra 0 siblings, 1 reply; 11+ messages in thread From: Rob Herring @ 2025-12-05 15:17 UTC (permalink / raw) To: Yemike Abhilash Chandra Cc: tomi.valkeinen, mchehab, krzk+dt, conor+dt, hverkuil, sakari.ailus, laurent.pinchart, hansg, mehdi.djait, ribalda, git, vladimir.zapolskiy, benjamin.mugnier, dongcheng.yan, u-kumar1, jai.luthra, linux-media, devicetree, linux-kernel 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH V2 3/4] media: dt-bindings: ti,ds90ub960: Add support for DS90UB954-Q1 2025-12-05 15:17 ` Rob Herring @ 2025-12-10 9:33 ` Yemike Abhilash Chandra 0 siblings, 0 replies; 11+ messages in thread From: Yemike Abhilash Chandra @ 2025-12-10 9:33 UTC (permalink / raw) To: Rob Herring Cc: tomi.valkeinen, mchehab, krzk+dt, conor+dt, hverkuil, sakari.ailus, laurent.pinchart, hansg, mehdi.djait, ribalda, git, vladimir.zapolskiy, benjamin.mugnier, dongcheng.yan, u-kumar1, jai.luthra, linux-media, devicetree, linux-kernel 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 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH V2 4/4] media: i2c: ds90ub960: Add support for DS90UB954-Q1 2025-12-02 10:22 [PATCH V2 0/4] Add support for DS90UB954-Q1 Yemike Abhilash Chandra ` (2 preceding siblings ...) 2025-12-02 10:22 ` [PATCH V2 3/4] media: dt-bindings: ti,ds90ub960: Add support for DS90UB954-Q1 Yemike Abhilash Chandra @ 2025-12-02 10:22 ` Yemike Abhilash Chandra 2025-12-05 11:10 ` Tomi Valkeinen 3 siblings, 1 reply; 11+ messages in thread From: Yemike Abhilash Chandra @ 2025-12-02 10:22 UTC (permalink / raw) To: tomi.valkeinen, mchehab, robh, krzk+dt, conor+dt, hverkuil, sakari.ailus, laurent.pinchart Cc: hansg, mehdi.djait, ribalda, git, vladimir.zapolskiy, benjamin.mugnier, dongcheng.yan, u-kumar1, jai.luthra, linux-media, devicetree, linux-kernel, y-abhilashchandra 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH V2 4/4] media: i2c: ds90ub960: Add support for DS90UB954-Q1 2025-12-02 10:22 ` [PATCH V2 4/4] media: i2c: ds90ub960: " Yemike Abhilash Chandra @ 2025-12-05 11:10 ` Tomi Valkeinen 0 siblings, 0 replies; 11+ messages in thread From: Tomi Valkeinen @ 2025-12-05 11:10 UTC (permalink / raw) To: Yemike Abhilash Chandra Cc: hansg, mehdi.djait, ribalda, git, vladimir.zapolskiy, benjamin.mugnier, dongcheng.yan, u-kumar1, jai.luthra, linux-media, devicetree, linux-kernel, mchehab, robh, krzk+dt, conor+dt, hverkuil, sakari.ailus, laurent.pinchart 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2025-12-10 9:34 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-12-02 10:22 [PATCH V2 0/4] Add support for DS90UB954-Q1 Yemike Abhilash Chandra 2025-12-02 10:22 ` [PATCH V2 1/4] media: dt-bindings: ti,ds90ub960: Refactor port definitions Yemike Abhilash Chandra 2025-12-05 15:11 ` Rob Herring 2025-12-10 9:25 ` Yemike Abhilash Chandra 2025-12-02 10:22 ` [PATCH V2 2/4] media: i2c: ds90ub960: Use enums for chip type and chip family Yemike Abhilash Chandra 2025-12-05 10:46 ` Tomi Valkeinen 2025-12-02 10:22 ` [PATCH V2 3/4] media: dt-bindings: ti,ds90ub960: Add support for DS90UB954-Q1 Yemike Abhilash Chandra 2025-12-05 15:17 ` Rob Herring 2025-12-10 9:33 ` Yemike Abhilash Chandra 2025-12-02 10:22 ` [PATCH V2 4/4] media: i2c: ds90ub960: " Yemike Abhilash Chandra 2025-12-05 11:10 ` Tomi Valkeinen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).