[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5b6b7c00-1553-45e3-90b9-ddd0da86263b@ti.com>
Date: Tue, 6 Jan 2026 15:36:10 +0530
From: Yemike Abhilash Chandra <y-abhilashchandra@...com>
To: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
CC: <hansg@...nel.org>, <mehdi.djait@...ux.intel.com>, <ribalda@...omium.org>,
<git@...tzsch.eu>, <vladimir.zapolskiy@...aro.org>,
<benjamin.mugnier@...s.st.com>, <dongcheng.yan@...el.com>, <u-kumar1@...com>,
<jai.luthra@...ux.dev>, <linux-media@...r.kernel.org>,
<devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<mchehab@...nel.org>, <robh@...nel.org>, <krzk+dt@...nel.org>,
<conor+dt@...nel.org>, <hverkuil@...all.nl>, <sakari.ailus@...ux.intel.com>,
<laurent.pinchart@...asonboard.com>
Subject: Re: [PATCH V3 3/4] media: dt-bindings: ti,ds90ub960: Add support for
DS90UB954-Q1
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@...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
>
Powered by blists - more mailing lists