[v10,5/8] dt-bindings: media: add TI DS90UB960 FPD-Link III Deserializer
Commit Message
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>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
.../bindings/media/i2c/ti,ds90ub960.yaml | 431 ++++++++++++++++++
1 file changed, 431 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml
Comments
> + 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.
After some initial discussion with Tomi on IRC, this question is
probably more for Luca:
Why is "i2c-alias-pool" in the drivers binding and not a regular i2c
binding? Same question for the implementation of the alias-pool
handling. Shouldn't this be in the i2c-atr library? I'd think managing
the list of aliases would look all the same in the drivers otherwise?
Hi Wolfram, Tomi,
On Tue, 18 Apr 2023 15:06:10 +0200
Wolfram Sang <wsa@kernel.org> wrote:
> > + 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.
>
> After some initial discussion with Tomi on IRC, this question is
> probably more for Luca:
>
> Why is "i2c-alias-pool" in the drivers binding and not a regular i2c
> binding? Same question for the implementation of the alias-pool
> handling. Shouldn't this be in the i2c-atr library? I'd think managing
> the list of aliases would look all the same in the drivers otherwise?
I think that this _was_ the plan, as it looks obviously cleaner, but
then we agreed that we should remove the pool entirely, so I didn't
bother moving it.
Best regards,
Luca
> > Why is "i2c-alias-pool" in the drivers binding and not a regular i2c
> > binding? Same question for the implementation of the alias-pool
> > handling. Shouldn't this be in the i2c-atr library? I'd think managing
> > the list of aliases would look all the same in the drivers otherwise?
>
> I think that this _was_ the plan, as it looks obviously cleaner, but
> then we agreed that we should remove the pool entirely, so I didn't
> bother moving it.
Ah, you mean we agreed on that at the Plumbers BoF? I think we can
conclude this is obsolete meanwhile. GMSL encodes the target addresses
in DT. Rob is also fine with the binding here to encode the pool in DT.
Let's follow that road, I'd say.
Hi Wolfram,
On Wed, 19 Apr 2023 10:05:54 +0200
Wolfram Sang <wsa@kernel.org> wrote:
> > > Why is "i2c-alias-pool" in the drivers binding and not a regular i2c
> > > binding? Same question for the implementation of the alias-pool
> > > handling. Shouldn't this be in the i2c-atr library? I'd think managing
> > > the list of aliases would look all the same in the drivers otherwise?
> >
> > I think that this _was_ the plan, as it looks obviously cleaner, but
> > then we agreed that we should remove the pool entirely, so I didn't
> > bother moving it.
>
> Ah, you mean we agreed on that at the Plumbers BoF? I think we can
> conclude this is obsolete meanwhile. GMSL encodes the target addresses
> in DT. Rob is also fine with the binding here to encode the pool in DT.
> Let's follow that road, I'd say.
Sure, I'm not questioning that. Apologies if it did look like. I was
just trying to explain (to myself as well) why this hadn't been done
previously.
Best regards,
Luca
Hi Luca,
> > Ah, you mean we agreed on that at the Plumbers BoF? I think we can
> > conclude this is obsolete meanwhile. GMSL encodes the target addresses
> > in DT. Rob is also fine with the binding here to encode the pool in DT.
> > Let's follow that road, I'd say.
>
> Sure, I'm not questioning that. Apologies if it did look like. I was
> just trying to explain (to myself as well) why this hadn't been done
> previously.
All fine. I didn't understand it this way and mainly wanted to summarize
what happened since the BoF. To myself as well ;) And also for Tomi so
he knows what happened.
On 18/04/2023 16:06, Wolfram Sang wrote:
>
>> + 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.
>
> After some initial discussion with Tomi on IRC, this question is
> probably more for Luca:
>
> Why is "i2c-alias-pool" in the drivers binding and not a regular i2c
Where should be the binding documented? A new
Documentation/devicetree/bindings/i2c/i2c-atr.yaml file that only
contains the i2c-alias-pool?
> binding? Same question for the implementation of the alias-pool
> handling. Shouldn't this be in the i2c-atr library? I'd think managing
> the list of aliases would look all the same in the drivers otherwise?
I think this is fine, but I also think that we need to keep the door
open to other kinds of alias management. We only have a single user for
this for now. A driver/device might have other requirements for its
i2c-atr. Say, a pool per link, or perhaps runtime events may affect the
pool.
If we dictate the use of i2c-alias-pool property and the i2c-atr will
automatically get an alias from that pool, i2c-atr won't be usable for
the hypothetical drivers that have other needs.
With that in mind the current binding and i2c-atr.c is safe, as the
i2c-atr.c isn't even aware of the pool.
We can easily re-arrange the code later if and when we get more users
and understand their needs. But the bindings are important to get
right(-ish) now. So:
- Is the "i2c-alias-pool" property a driver property or a common
property for all drivers using i2c-atr?
- It the property mandatory or optional? It must be optional, as a setup
(meaning, e.g., what cameras you happen to connect) might not have any
i2c addressable remote devices, in which case the driver doesn't even
need i2c-atr (even if it supports i2c-atr). But is it optional even in
the case where the driver needs i2c-atr? In other words, do we allow
some other way to manage the aliases?
How does this sound:
- If "i2c-alias-pool" is present in the DT data of the device passed to
i2c_atr_new(), i2c_atr_new() will parse the property. i2c-atr.c will
export functions to get a new alias and to release a previously reserved
alias. The driver can use those functions in attach/detach_client()
callbacks. In other words, the alias pool management wouldn't be fully
automatic inside the i2c-atr, but it would provide helpers for the
driver to do the common work.
- If "i2c-alias-pool" is not present, i2c-atr.c will behave as it does
now, and expects the driver to manage the aliases.
Also, looking at the ub960 code... I don't think this will simplify the
attach/detach_client callbacks much. Most of the code in those functions
is about managing the UB960's registers related to ATR, not managing the
address pool itself. However, it will remove the probe time
"i2c-alias-pool" parsing from the driver, which is nice.
Tomi
Hi Tomi,
> How does this sound:
>
> - If "i2c-alias-pool" is present in the DT data of the device passed to
> i2c_atr_new(), i2c_atr_new() will parse the property. i2c-atr.c will export
> functions to get a new alias and to release a previously reserved alias. The
> driver can use those functions in attach/detach_client() callbacks. In other
> words, the alias pool management wouldn't be fully automatic inside the
> i2c-atr, but it would provide helpers for the driver to do the common work.
>
> - If "i2c-alias-pool" is not present, i2c-atr.c will behave as it does now,
> and expects the driver to manage the aliases.
So, how does a driver manage the aliases without a pool of available
addresses? I can't imagine another way right now.
In general, your above proposal sounds good to me. With my lack of
imagination regarding a different alias handling, I could also see that
i2c-atr already provides the alias to the attach callback. But if you
teach me another way of alias handling, then I could agree that your
proposal makes sense as is.
And yes, the "i2c-alias-pool" is definately optional.
Happy hacking,
Wolfram
On 20/04/2023 21:47, Wolfram Sang wrote:
> Hi Tomi,
>
>> How does this sound:
>>
>> - If "i2c-alias-pool" is present in the DT data of the device passed to
>> i2c_atr_new(), i2c_atr_new() will parse the property. i2c-atr.c will export
>> functions to get a new alias and to release a previously reserved alias. The
>> driver can use those functions in attach/detach_client() callbacks. In other
>> words, the alias pool management wouldn't be fully automatic inside the
>> i2c-atr, but it would provide helpers for the driver to do the common work.
>>
>> - If "i2c-alias-pool" is not present, i2c-atr.c will behave as it does now,
>> and expects the driver to manage the aliases.
>
> So, how does a driver manage the aliases without a pool of available
> addresses? I can't imagine another way right now.
>
> In general, your above proposal sounds good to me. With my lack of
> imagination regarding a different alias handling, I could also see that
> i2c-atr already provides the alias to the attach callback. But if you
> teach me another way of alias handling, then I could agree that your
> proposal makes sense as is.
Oh, my imagination doesn't go so far to give you concrete examples. If
the driver has to do runtime decisions on the pool, a fixed pool handled
by the i2c-atr won't work. But why exactly would there be runtime events
affecting the pool... I don't know.
Maybe that doesn't matter here. We can start with the i2c-atr providing
the alias to the attach callback, and if we ever need something else,
the (kernel internal) API can be changed accordingly. The DT bindings
should be fine in either case.
Tomi
new file mode 100644
@@ -0,0 +1,431 @@
+# 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 device.
+
+ 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 device.
+
+ 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
+ link-frequencies:
+ maxItems: 1
+
+ required:
+ - data-lanes
+ - link-frequencies
+
+ 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
+ link-frequencies:
+ maxItems: 1
+
+ required:
+ - data-lanes
+ - link-frequencies
+
+ required:
+ - port@0
+ - port@1
+ - port@2
+ - port@3
+ - port@4
+ - port@5
+
+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 = <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>;
+ };
+ };
+
+ /* Port 1, Camera 1 */
+ port@1 {
+ reg = <1>;
+
+ ub960_fpd3_2_in: endpoint {
+ remote-endpoint = <&ub913_2_out>;
+ hsync-active = <0>;
+ vsync-active = <1>;
+ };
+ };
+
+ /* Port 2, unconnected */
+ port@2 {
+ reg = <2>;
+ };
+
+ /* Port 3, unconnected */
+ port@3 {
+ reg = <3>;
+ };
+
+ /* Port 4, CSI-2 TX */
+ port@4 {
+ reg = <4>;
+ ds90ub960_0_csi_out: endpoint {
+ data-lanes = <1 2 3 4>;
+ link-frequencies = /bits/ 64 <800000000>;
+ remote-endpoint = <&csi2_phy0>;
+ };
+ };
+
+ /* Port 5, unconnected */
+ port@5 {
+ reg = <5>;
+ };
+ };
+
+ links {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ /* Link 0 has DS90UB953 serializer and IMX274 sensor */
+
+ link@0 {
+ reg = <0>;
+ i2c-alias = <0x44>;
+
+ ti,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 {
+ 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@1a {
+ compatible = "sony,imx274";
+ reg = <0x1a>;
+
+ reset-gpios = <&serializer1 0 GPIO_ACTIVE_LOW>;
+
+ port {
+ sensor_1_out: endpoint {
+ remote-endpoint = <&ub953_1_in>;
+ };
+ };
+ };
+ };
+ };
+ }; /* End of link@0 */
+
+ /* Link 1 has DS90UB913 serializer and MT9V111 sensor */
+
+ link@1 {
+ reg = <1>;
+ i2c-alias = <0x45>;
+
+ ti,rx-mode = <0>;
+
+ serializer2: 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_2_in: endpoint {
+ remote-endpoint = <&sensor_2_out>;
+ pclk-sample = <1>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+
+ ub913_2_out: endpoint {
+ remote-endpoint = <&ub960_fpd3_2_in>;
+ };
+ };
+ };
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ sensor@48 {
+ compatible = "aptina,mt9v111";
+ reg = <0x48>;
+
+ clocks = <&serializer2>;
+
+ port {
+ sensor_2_out: endpoint {
+ remote-endpoint = <&ub913_2_in>;
+ };
+ };
+ };
+ };
+ };
+ }; /* End of link@1 */
+ };
+ };
+ };
+...