From: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
To: Rob Herring <robh@kernel.org>
Cc: devicetree@vger.kernel.org, linux-i2c@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-media@vger.kernel.org,
Hans Verkuil <hverkuil-cisco@xs4all.nl>,
Jacopo Mondi <jacopo@jmondi.org>,
Kieran Bingham <kieran.bingham@ideasonboard.com>,
Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
Luca Ceresoli <luca@lucaceresoli.net>,
Mark Rutland <mark.rutland@arm.com>,
Matti Vaittinen <Matti.Vaittinen@fi.rohmeurope.com>,
Mauro Carvalho Chehab <mchehab@kernel.org>,
Peter Rosin <peda@axentia.se>,
Sakari Ailus <sakari.ailus@linux.intel.com>,
Vladimir Zapolskiy <vz@mleia.com>,
Wolfram Sang <wsa@the-dreams.de>,
satish.nagireddy@getcruise.com
Subject: Re: [PATCH v4 3/8] dt-bindings: media: add bindings for TI DS90UB960
Date: Thu, 3 Nov 2022 13:50:43 +0200 [thread overview]
Message-ID: <6c254d5f-9fa1-b06a-4edb-7e58e4b33101@ideasonboard.com> (raw)
In-Reply-To: <20221102172630.GA4140587-robh@kernel.org>
Hi Rob,
On 02/11/2022 19:26, Rob Herring wrote:
> On Tue, Nov 01, 2022 at 03:20:27PM +0200, Tomi Valkeinen wrote:
>> Add DT bindings for TI DS90UB960 FPDLink-3 Deserializer.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>> ---
>> .../bindings/media/i2c/ti,ds90ub960.yaml | 392 ++++++++++++++++++
>> 1 file changed, 392 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..4456d9b3e2c7
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml
>> @@ -0,0 +1,392 @@
>> +# 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: |
>
> Don't need '|'
Hmm, ok... But why does that work? I can only find yaml examples for
multi-line with either | or >.
>> + 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
>> + description:
>> + i2c addresses for the deserializer and the serializers
>> +
>> + reg-names:
>> + items:
>> + - const: main
>
> 'reg-names' is not all that useful with only 1 entry.
True.
>> +
>> + 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:
>
> Something common or could be? If not, then needs a vendor prefix.
I'll have to think about this. It is related to the i2c-atr, so I think
it might be a common thing.
>> + $ref: /schemas/types.yaml#/definitions/uint16-array
>> + description:
>> + i2c alias pool for remote devices.
>
> Needs a better description. What's an 'alias pool'?
Right.
"i2c alias pool is a pool of i2c addresses on the main i2c bus that can
be used to access the remote peripherals. 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."
> 0-0xffff are valid values?
They are i2c addresses, and linux i2c uses u16 for addresses. Then
again, the fpdlink devices only support 7-bit addresses, so maybe this
could be an uint8 array. I am not sure what's the best way to define this.
>> +
>> + links:
>> + type: object
>> + additionalProperties: false
>> +
>> + properties:
>> + '#address-cells':
>> + const: 1
>> +
>> + '#size-cells':
>> + const: 0
>> +
>> + manual-strobe:
>> + type: boolean
>> + description:
>> + Enable manual strobe position and EQ level
>> +
>> + patternProperties:
>> + '^link@[0-9a-f]+$':
>> + type: object
>> + additionalProperties: false
>> + properties:
>> + reg:
>> + description: The link number
>> + maxItems: 1
>> +
>> + i2c-alias:
>
> Vendor prefix.
>
>> + 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.
>> +
>> + rx-mode:
>
> Vendor prefix. And so on...
Yes, I totally missed these.
>> + $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
>> +
>> + cdr-mode:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + enum:
>> + - 0 # FPD3
>> + - 1 # FPD4
>> + description: FPD-Link CDR Mode
>> +
>> + strobe-pos:
>> + $ref: /schemas/types.yaml#/definitions/int32
>> + minimum: -13
>> + maximum: 13
>> + description: Manual strobe position, from -13 to 13
>
> No need to put constraints in free form text.
Ok.
>> +
>> + eq-level:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + maximum: 14
>> + description: Manual EQ level, from 0 to 14
>> +
>> + serializer:
>> + type: object
>> + description: FPD-Link Serializer node
>> +
>> + required:
>> + - reg
>> + - i2c-alias
>> + - rx-mode
>> + - serializer
>> +
>> + ports:
>> + $ref: /schemas/graph.yaml#/properties/ports
>> +
>> + properties:
>> + port@0:
>> + $ref: /schemas/graph.yaml#/$defs/port-base
>
> additionalProperties: false
This gives me check errors about the port's 'reg' property. Using
'unevaluatedProperties' works fine. Is 'unevaluatedProperties' correct,
or am I missing something here?
>> + description: FPD-Link input 0
>> +
>> + properties:
>> + endpoint:
>> + $ref: /schemas/media/video-interfaces.yaml#
>
> unevaluatedProperties: false
>
> Same for the other port nodes
Yep.
>> +
>> + port@1:
>> + $ref: /schemas/graph.yaml#/$defs/port-base
>> + description: FPD-Link input 1
>> +
>> + properties:
>> + endpoint:
>> + $ref: /schemas/media/video-interfaces.yaml#
>> +
>> + port@2:
>> + $ref: /schemas/graph.yaml#/$defs/port-base
>> + description: FPD-Link input 2
>> +
>> + properties:
>> + endpoint:
>> + $ref: /schemas/media/video-interfaces.yaml#
>> +
>> + port@3:
>> + $ref: /schemas/graph.yaml#/$defs/port-base
>> + description: FPD-Link input 3
>> +
>> + properties:
>> + endpoint:
>> + $ref: /schemas/media/video-interfaces.yaml#
>> +
>> + port@4:
>> + $ref: /schemas/graph.yaml#/$defs/port-base
>> + description: CSI-2 Output 0
>> +
>> + properties:
>> + endpoint:
>> + $ref: /schemas/media/video-interfaces.yaml#
>> +
>> + properties:
>> + clock-lanes:
>> + maxItems: 1
>> +
>> + data-lanes:
>> + minItems: 1
>> + maxItems: 4
>
> Why the constraints on this endpoint? Are the other ones actually using
> properties from video-interfaces.yaml? If not, then just reference
> /properties/port and drop 'endpoint' instead.
The ports 0-3 do not use any properties from video-interfaces.yaml, so
I'll drop the endpoint.
Ports 4,5 are CSI-2 ports and need the clock-lanes and data-lanes to be
defined.
Is there something wrong with the constraints, or were you just
wondering about the difference between ports 0-3 and 4,5
>> +
>> + port@5:
>> + $ref: /schemas/graph.yaml#/$defs/port-base
>> + description: CSI-2 Output 1
>> +
>> + properties:
>> + endpoint:
>> + $ref: /schemas/media/video-interfaces.yaml#
>> +
>> + properties:
>> + clock-lanes:
>> + maxItems: 1
>> +
>> + data-lanes:
>> + minItems: 1
>> + maxItems: 4
>> +
>> +required:
>> + - compatible
>> + - reg
>> + - clocks
>> + - clock-names
>> + - ports
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> + - |
>> + #include <dt-bindings/gpio/gpio.h>
>> +
>> + i2c {
>> + clock-frequency = <400000>;
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + deser@3d {
>> + compatible = "ti,ds90ub960-q1";
>> +
>> + reg-names = "main";
>> + reg = <0x3d>;
>> +
>> + clock-names = "refclk";
>> + clocks = <&fixed_clock>;
>> +
>> + powerdown-gpios = <&pca9555 7 GPIO_ACTIVE_LOW>;
>> +
>> + i2c-alias-pool = /bits/ 16 <0x4a 0x4b 0x4c 0x4d 0x4e 0x4f>;
>> +
>> + ports {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + /* Port 0, Camera 0 */
>> + port@0 {
>> + reg = <0>;
>> +
>> + ub960_fpd3_1_in: endpoint {
>> + remote-endpoint = <&ub953_1_out>;
>> +
>> + rx-mode = <0>;
>
> Looks like this is not defined under 'endpoint'.
Indeed, and after adding the 'unevaluatedProperties' I do get a warning
here.
>> + };
>> + };
>> +
>> + /* Port 0, Camera 1 */
>> + port@1 {
>> + reg = <1>;
>> +
>> + ub960_fpd3_2_in: endpoint {
>> + remote-endpoint = <&ub913_2_out>;
>> +
>> + rx-mode = <0>;
>> + };
>> + };
>> +
>> + /* Port 4, CSI-2 TX */
>> + port@4 {
>> + reg = <4>;
>> + ds90ub960_0_csi_out: endpoint {
>> + clock-lanes = <0>;
>> + 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 IMX390 sensor */
>> +
>> + link@0 {
>> + reg = <0>;
>> + i2c-alias = <68>;
>> +
>> + rx-mode = <3>;
>> +
>> + serializer1: serializer {
>> + compatible = "ti,ds90ub953-q1";
>> +
>> + gpio-controller;
>> + #gpio-cells = <2>;
>> +
>> + #clock-cells = <0>;
>> +
>> + ports {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + port@0 {
>> + reg = <0>;
>> + ub953_1_in: endpoint {
>> + clock-lanes = <0>;
>> + data-lanes = <1 2 3 4>;
>> + remote-endpoint = <&sensor_1_out>;
>> + };
>> + };
>> +
>> + port@1 {
>> + reg = <1>;
>> +
>> + ub953_1_out: endpoint {
>> + remote-endpoint = <&ub960_fpd3_1_in>;
>> + };
>> + };
>> + };
>> +
>> + i2c {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + sensor@21 {
>> + compatible = "sony,imx390";
DT_CHECKER_FLAGS=-m gives a warning here, as sony,imx390 is not in
upstream. The sensor details are not really relevant here, but I used
the data for the setup I have.
Should I instead use some sensor here that is in upstream, which I think
should work with the fpdlink ICs?
Tomi
next prev parent reply other threads:[~2022-11-03 11:50 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-01 13:20 [PATCH v4 0/8] i2c-atr and FPDLink Tomi Valkeinen
2022-11-01 13:20 ` [PATCH v4 1/8] i2c: core: let adapters be notified of client attach/detach Tomi Valkeinen
2022-11-01 13:20 ` [PATCH v4 2/8] i2c: add I2C Address Translator (ATR) support Tomi Valkeinen
2022-11-01 14:30 ` Andy Shevchenko
2022-11-04 11:59 ` Tomi Valkeinen
2022-11-04 12:38 ` Andy Shevchenko
2022-11-04 15:26 ` Tomi Valkeinen
2022-11-04 17:27 ` Andy Shevchenko
2022-11-07 11:40 ` Luca Ceresoli
2022-12-08 7:56 ` Tomi Valkeinen
2022-11-01 13:20 ` [PATCH v4 3/8] dt-bindings: media: add bindings for TI DS90UB960 Tomi Valkeinen
2022-11-01 16:47 ` Rob Herring
2022-11-02 17:26 ` Rob Herring
2022-11-03 11:50 ` Tomi Valkeinen [this message]
2022-11-03 12:13 ` Vaittinen, Matti
2022-11-03 12:32 ` Tomi Valkeinen
2022-11-11 16:26 ` Luca Ceresoli
2022-12-08 9:23 ` Tomi Valkeinen
2022-11-11 16:32 ` Luca Ceresoli
2022-11-11 16:35 ` Luca Ceresoli
2022-11-01 13:20 ` [PATCH v4 4/8] dt-bindings: media: add bindings for TI DS90UB913 Tomi Valkeinen
2022-11-01 16:47 ` Rob Herring
2022-11-02 17:27 ` Rob Herring
2022-11-01 13:20 ` [PATCH v4 5/8] dt-bindings: media: add bindings for TI DS90UB953 Tomi Valkeinen
2022-11-01 16:47 ` Rob Herring
2022-11-01 13:20 ` [PATCH v4 6/8] media: i2c: add DS90UB960 driver Tomi Valkeinen
2022-11-02 8:05 ` Tomi Valkeinen
2022-11-01 13:20 ` [PATCH v4 7/8] media: i2c: add DS90UB913 driver Tomi Valkeinen
2022-11-01 13:20 ` [PATCH v4 8/8] media: i2c: add DS90UB953 driver Tomi Valkeinen
2022-11-07 11:48 ` [PATCH v4 0/8] i2c-atr and FPDLink Luca Ceresoli
2022-11-07 12:12 ` Vaittinen, Matti
2022-11-07 12:12 ` Tomi Valkeinen
2022-11-07 14:37 ` Vaittinen, Matti
2022-11-07 15:37 ` Tomi Valkeinen
2022-11-07 17:47 ` Vaittinen, Matti
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=6c254d5f-9fa1-b06a-4edb-7e58e4b33101@ideasonboard.com \
--to=tomi.valkeinen@ideasonboard.com \
--cc=Matti.Vaittinen@fi.rohmeurope.com \
--cc=devicetree@vger.kernel.org \
--cc=hverkuil-cisco@xs4all.nl \
--cc=jacopo@jmondi.org \
--cc=kieran.bingham@ideasonboard.com \
--cc=laurent.pinchart@ideasonboard.com \
--cc=linux-i2c@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=luca@lucaceresoli.net \
--cc=mark.rutland@arm.com \
--cc=mchehab@kernel.org \
--cc=peda@axentia.se \
--cc=robh@kernel.org \
--cc=sakari.ailus@linux.intel.com \
--cc=satish.nagireddy@getcruise.com \
--cc=vz@mleia.com \
--cc=wsa@the-dreams.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).