Re: [PATCH v6 5/8] dt-bindings: media: add TI DS90UB960 FPD-Link III Deserializer

From: Tomi Valkeinen
Date: Mon Jan 09 2023 - 04:55:28 EST


On 09/01/2023 11:09, Laurent Pinchart wrote:
On Mon, Jan 09, 2023 at 10:30:13AM +0200, Tomi Valkeinen wrote:
On 08/01/2023 05:23, Laurent Pinchart wrote:
Hi Tomi,

Thank you for the patch.

On Thu, Jan 05, 2023 at 04:03:04PM +0200, Tomi Valkeinen wrote:
Add DT bindings for TI DS90UB960 FPD-Link III Deserializer.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@xxxxxxxxxxxxxxxx>
Reviewed-by: Rob Herring <robh@xxxxxxxxxx>
---
.../bindings/media/i2c/ti,ds90ub960.yaml | 402 ++++++++++++++++++
1 file changed, 402 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml

diff --git a/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml b/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml
new file mode 100644
index 000000000000..664799ae55be
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml
@@ -0,0 +1,402 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/i2c/ti,ds90ub960.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Texas Instruments DS90UB9XX Family FPD-Link Deserializer Hubs
+
+maintainers:
+ - Tomi Valkeinen <tomi.valkeinen@xxxxxxxxxxxxxxxx>
+
+description:
+ The TI DS90UB9XX devices are FPD-Link video deserializers with I2C and GPIO
+ forwarding.
+
+properties:
+ compatible:
+ enum:
+ - ti,ds90ub960-q1
+ - ti,ds90ub9702-q1
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+ description:
+ Reference clock connected to the REFCLK pin.
+
+ clock-names:
+ items:
+ - const: refclk
+
+ powerdown-gpios:
+ maxItems: 1
+ description:
+ Specifier for the GPIO connected to the PDB pin.
+
+ i2c-alias-pool:
+ $ref: /schemas/types.yaml#/definitions/uint16-array
+ description:
+ I2C alias pool is a pool of I2C addresses on the main I2C bus that can be
+ used to access the remote peripherals on the serializer's I2C bus. The
+ addresses must be available, not used by any other peripheral. Each
+ remote peripheral is assigned an alias from the pool, and transactions to
+ that address will be forwarded to the remote peripheral, with the address
+ translated to the remote peripheral's real address. This property is not
+ needed if there are no I2C addressable remote peripherals.
+
+ links:
+ type: object
+ additionalProperties: false
+
+ properties:
+ '#address-cells':
+ const: 1
+
+ '#size-cells':
+ const: 0
+
+ ti,manual-strobe:
+ type: boolean
+ description:
+ Enable manual strobe position and EQ level
+
+ patternProperties:
+ '^link@[0-3]$':
+ type: object
+ additionalProperties: false
+ properties:
+ reg:
+ description: The link number
+ maxItems: 1
+
+ i2c-alias:
+ description:
+ The I2C address used for the serializer. Transactions to this
+ address on the I2C bus where the deserializer resides are
+ forwarded to the serializer.
+
+ ti,rx-mode:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum:
+ - 0 # RAW10
+ - 1 # RAW12 HF
+ - 2 # RAW12 LF
+ - 3 # CSI2 SYNC
+ - 4 # CSI2 NON-SYNC
+ description:
+ FPD-Link Input Mode. This should reflect the hardware and the
+ default mode of the connected camera module.

As the remote device may not be a camera, I'd write "of the connected
device" or "of the connected serializer".

I was trying to include the sensor also in the "camera module", as the
sensor's "normal" pixel cloud would affect RAW modes (HF/LF). Perhaps
"connected device" covers this.

+
+ ti,cdr-mode:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum:
+ - 0 # FPD-Link III
+ - 1 # FPD-Link IV
+ description:
+ FPD-Link CDR Mode. This should reflect the hardware and the
+ default mode of the connected camera module.
+
+ ti,strobe-pos:
+ $ref: /schemas/types.yaml#/definitions/int32
+ minimum: -13
+ maximum: 13
+ description: Manual strobe position
+
+ ti,eq-level:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ maximum: 14
+ description: Manual EQ level
+
+ serializer:
+ type: object
+ description: FPD-Link Serializer node
+
+ required:
+ - reg
+ - i2c-alias
+ - ti,rx-mode
+ - serializer
+
+ ports:
+ $ref: /schemas/graph.yaml#/properties/ports
+
+ properties:
+ port@0:
+ $ref: /schemas/graph.yaml#/$defs/port-base
+ unevaluatedProperties: false
+ 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
+ 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
+ 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
+ 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
+ description: CSI-2 Output 0
+
+ properties:
+ endpoint:
+ $ref: /schemas/media/video-interfaces.yaml#
+ unevaluatedProperties: false
+
+ properties:
+ data-lanes:
+ minItems: 1
+ maxItems: 4
+
+ required:
+ - data-lanes
+
+ port@5:
+ $ref: /schemas/graph.yaml#/$defs/port-base
+ unevaluatedProperties: false
+ description: CSI-2 Output 1
+
+ properties:
+ endpoint:
+ $ref: /schemas/media/video-interfaces.yaml#
+ unevaluatedProperties: false
+
+ properties:
+ data-lanes:
+ minItems: 1
+ maxItems: 4
+
+ required:
+ - data-lanes

I think you need

required:
- port@0
- port@1
- port@2
- port@3
- port@4
- port@5

Is that needed? I think often some of the ports are unused (e.g. the
example in this yaml file). Is it customary to still require empty port
nodes in the DT?

Ports are an intrinsic property of a device, they don't depend on the
device integration in the system. In this case, the UB960 has four
FPD-Link inputs and two CSI-2 outputs, that's a property of the chip.
They don't have to be connected to anything on the board, so endpooints
are optional.

Yes, but why do they have to be required? A missing port node implies that it's not used, doesn't it? I don't mind much, it just feels a bit extra to add multiple almost empty "port@X { reg = <X>; };" style nodes to the dts file.

Tomi