Re: [PATCH v5 4/8] dt-bindings: media: add bindings for TI DS90UB953

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/12/2022 19:34, Laurent Pinchart wrote:
Hi Tomi,

Thank you for the patch.

On Thu, Dec 08, 2022 at 12:40:02PM +0200, Tomi Valkeinen wrote:
Add DT bindings for TI DS90UB953 FPDLink-3 Serializer.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@xxxxxxxxxxxxxxxx>
---
  .../bindings/media/i2c/ti,ds90ub953.yaml      | 112 ++++++++++++++++++
  1 file changed, 112 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/media/i2c/ti,ds90ub953.yaml

diff --git a/Documentation/devicetree/bindings/media/i2c/ti,ds90ub953.yaml b/Documentation/devicetree/bindings/media/i2c/ti,ds90ub953.yaml
new file mode 100644
index 000000000000..fd7d25d93e2c
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ti,ds90ub953.yaml
@@ -0,0 +1,112 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/i2c/ti,ds90ub953.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Texas Instruments DS90UB953 FPD-Link 3 Serializer
+
+maintainers:
+  - Tomi Valkeinen <tomi.valkeinen@xxxxxxxxxxxxxxxx>
+
+description:
+  The TI DS90UB953 is an FPD-Link 3 video serializer for MIPI CSI-2.
+
+properties:
+  compatible:
+    enum:
+      - ti,ds90ub953-q1
+      - ti,ds90ub971-q1
+
+  '#gpio-cells':
+    const: 2

I would add a description here, to tell what the cells correspond to. In
particular, the first cell selects the GPIO_* pin number, it would be
nice to document that its value should be in the range [0, 3].

Same comment for patch 3/8 (DS90UB913 bindings). There you could also
mention that GPO2 and the output clock are mutually exclusive.

Yep. I have added this for ub913:
First cell is the GPO pin number, second cell is the flags. The GPO pin
      number must be in range of [0, 3]. Note that GPOs 2 and 3 are not
      available in external oscillator mode.

and this for ub953:
First cell is the GPIO pin number, second cell is the flags. The GPIO pin
      number must be in range of [0, 3].

+
+  gpio-controller: true
+

No need for clocks and clock-names for the reference input clock ? Or is
this because you support sync mode only for now ?

Right, I don't have the clock on my hw, but it's probably better to add it to the binding already.

+  '#clock-cells':
+    const: 0
+
+  ports:
+    $ref: /schemas/graph.yaml#/properties/ports
+
+    properties:
+      port@0:
+        $ref: /schemas/graph.yaml#/$defs/port-base
+        unevaluatedProperties: false
+        description: CSI-2 input port
+
+        properties:
+          endpoint:
+            $ref: /schemas/media/video-interfaces.yaml#
+            unevaluatedProperties: false

Should the data-lanes property be required for the CSI-2 input ?

Yes.

+
+      port@1:
+        $ref: /schemas/graph.yaml#/properties/port
+        unevaluatedProperties: false
+        description: FPD-Link 3 output port
+
+  i2c:
+    $ref: /schemas/i2c/i2c-controller.yaml#
+    unevaluatedProperties: false
+
+required:
+  - compatible
+  - '#gpio-cells'
+  - gpio-controller
+  - '#clock-cells'
+  - ports
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    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_in: endpoint {
+            clock-lanes = <0>;
+            data-lanes = <1 2 3 4>;
+            remote-endpoint = <&sensor_out>;
+          };
+        };
+
+        port@1 {
+          reg = <1>;
+          endpoint {
+            remote-endpoint = <&deser_fpd_in>;
+          };
+        };
+      };
+
+      i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        sensor@1a {
+          compatible = "sony,imx274";
+          reg = <0x1a>;
+
+          reset-gpios = <&serializer 0 GPIO_ACTIVE_LOW>;

Maybe add

           clocks = <&serializer>;
	  clock-names = "inck";

to showcase the clock connection ?

Yes, that's a good idea.

 Tomi




[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux