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

Tomi Valkeinen
2023-01-09T08:31:29Z

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@ideasonboard.com>
>> Reviewed-by: Rob Herring <robh@kernel.org>
>> ---
>> .../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@ideasonboard.com>
>> +
>> +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?

Tomi