diff mbox series

[v10,1/4] dt-bindings: media: i2c: Add bindings for Maxim Integrated MAX9286

Message ID 20200612144713.502006-2-kieran.bingham+renesas@ideasonboard.com
State Changes Requested
Headers show
Series MAX9286 GMSL Support (+RDACM20) | expand

Checks

Context Check Description
robh/dt-meta-schema fail build log
robh/checkpatch success

Commit Message

Kieran Bingham June 12, 2020, 2:47 p.m. UTC
From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

The MAX9286 deserializes video data received on up to 4 Gigabit
Multimedia Serial Links (GMSL) and outputs them on a CSI-2 port using up
to 4 data lanes.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Reviewed-by: Rob Herring <robh@kernel.org>

---

v7:
 - Collect Rob's RB tag
 - Remove redundant maxItems from remote-endpoints
 - Fix SPDX licence tag

v10:
[Jacopo]
 - Fix dt-validation
 - Fix dt-binding examples with 2 reg entries

[Kieran]
 - Correctly match the hex camera node reg
 - Add (required) GPIO controller support

 .../bindings/media/i2c/maxim,max9286.yaml     | 366 ++++++++++++++++++
 1 file changed, 366 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml

Comments

Rob Herring June 12, 2020, 10:10 p.m. UTC | #1
On Fri, 12 Jun 2020 15:47:10 +0100, Kieran Bingham wrote:
> From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> 
> The MAX9286 deserializes video data received on up to 4 Gigabit
> Multimedia Serial Links (GMSL) and outputs them on a CSI-2 port using up
> to 4 data lanes.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> Reviewed-by: Rob Herring <robh@kernel.org>
> 
> ---
> 
> v7:
>  - Collect Rob's RB tag
>  - Remove redundant maxItems from remote-endpoints
>  - Fix SPDX licence tag
> 
> v10:
> [Jacopo]
>  - Fix dt-validation
>  - Fix dt-binding examples with 2 reg entries
> 
> [Kieran]
>  - Correctly match the hex camera node reg
>  - Add (required) GPIO controller support
> 
>  .../bindings/media/i2c/maxim,max9286.yaml     | 366 ++++++++++++++++++
>  1 file changed, 366 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml
> 


My bot found errors running 'make dt_binding_check' on your patch:

/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/media/i2c/maxim,max9286.example.dt.yaml: example-0: i2c@e66d8000:reg:0: [0, 3865935872, 0, 64] is too long


See https://patchwork.ozlabs.org/patch/1308280

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure dt-schema is up to date:

pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade

Please check and re-submit.
Jacopo Mondi June 13, 2020, 12:32 p.m. UTC | #2
Hi Rob,

On Fri, Jun 12, 2020 at 04:10:03PM -0600, Rob Herring wrote:
> On Fri, 12 Jun 2020 15:47:10 +0100, Kieran Bingham wrote:
> > From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> >
> > The MAX9286 deserializes video data received on up to 4 Gigabit
> > Multimedia Serial Links (GMSL) and outputs them on a CSI-2 port using up
> > to 4 data lanes.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> > Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > Reviewed-by: Rob Herring <robh@kernel.org>
> >
> > ---
> >
> > v7:
> >  - Collect Rob's RB tag
> >  - Remove redundant maxItems from remote-endpoints
> >  - Fix SPDX licence tag
> >
> > v10:
> > [Jacopo]
> >  - Fix dt-validation
> >  - Fix dt-binding examples with 2 reg entries
> >
> > [Kieran]
> >  - Correctly match the hex camera node reg
> >  - Add (required) GPIO controller support
> >
> >  .../bindings/media/i2c/maxim,max9286.yaml     | 366 ++++++++++++++++++
> >  1 file changed, 366 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml
> >
>
>
> My bot found errors running 'make dt_binding_check' on your patch:
>
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/media/i2c/maxim,max9286.example.dt.yaml: example-0: i2c@e66d8000:reg:0: [0, 3865935872, 0, 64] is too long
>
>
> See https://patchwork.ozlabs.org/patch/1308280
>
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure dt-schema is up to date:
>
> pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade
>

I have updated my dt-schema installation to the latest github master
-------------------------------------------------------------------------------
Successfully installed dtschema-2020.6.dev8+g4d2d86c

https://github.com/devicetree-org/dt-schema/commit/4d2d86c5cd65cd3944ce0aaa400866bc36727bea

$ /usr/bin/dt-validate -V
2020.6.dev8+g4d2d86c
-------------------------------------------------------------------------------

But I still cannot reproduce the error.

However, I see this commit in your next branch
https://github.com/devicetree-org/dt-schema/commit/b72500282cfd2eba6f9df4d7553f696544b40ee6
"schemas: Add a schema to check 'reg' sizes "

Which sounds very likely related to the above reported error.
Was this intentional ?

I'm not sure how I should handle this. The error reports the i2c node
parents should have both address-cells and size-cells properties set
to 2, but in the example there is not i2c node parent at all :)
Should I add a parent node for the i2c in the example snippet ?

Thanks
  j

> Please check and re-submit.
>
Rob Herring June 15, 2020, 3:02 p.m. UTC | #3
On Sat, Jun 13, 2020 at 6:28 AM Jacopo Mondi <jacopo@jmondi.org> wrote:
>
> Hi Rob,
>
> On Fri, Jun 12, 2020 at 04:10:03PM -0600, Rob Herring wrote:
> > On Fri, 12 Jun 2020 15:47:10 +0100, Kieran Bingham wrote:
> > > From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > >
> > > The MAX9286 deserializes video data received on up to 4 Gigabit
> > > Multimedia Serial Links (GMSL) and outputs them on a CSI-2 port using up
> > > to 4 data lanes.
> > >
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > > Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> > > Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > > Reviewed-by: Rob Herring <robh@kernel.org>
> > >
> > > ---
> > >
> > > v7:
> > >  - Collect Rob's RB tag
> > >  - Remove redundant maxItems from remote-endpoints
> > >  - Fix SPDX licence tag
> > >
> > > v10:
> > > [Jacopo]
> > >  - Fix dt-validation
> > >  - Fix dt-binding examples with 2 reg entries
> > >
> > > [Kieran]
> > >  - Correctly match the hex camera node reg
> > >  - Add (required) GPIO controller support
> > >
> > >  .../bindings/media/i2c/maxim,max9286.yaml     | 366 ++++++++++++++++++
> > >  1 file changed, 366 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml
> > >
> >
> >
> > My bot found errors running 'make dt_binding_check' on your patch:
> >
> > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/media/i2c/maxim,max9286.example.dt.yaml: example-0: i2c@e66d8000:reg:0: [0, 3865935872, 0, 64] is too long
> >
> >
> > See https://patchwork.ozlabs.org/patch/1308280
> >
> > If you already ran 'make dt_binding_check' and didn't see the above
> > error(s), then make sure dt-schema is up to date:
> >
> > pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade
> >
>
> I have updated my dt-schema installation to the latest github master
> -------------------------------------------------------------------------------
> Successfully installed dtschema-2020.6.dev8+g4d2d86c
>
> https://github.com/devicetree-org/dt-schema/commit/4d2d86c5cd65cd3944ce0aaa400866bc36727bea
>
> $ /usr/bin/dt-validate -V
> 2020.6.dev8+g4d2d86c
> -------------------------------------------------------------------------------
>
> But I still cannot reproduce the error.
>
> However, I see this commit in your next branch
> https://github.com/devicetree-org/dt-schema/commit/b72500282cfd2eba6f9df4d7553f696544b40ee6
> "schemas: Add a schema to check 'reg' sizes "
>
> Which sounds very likely related to the above reported error.
> Was this intentional ?

Yes, I can't add the new checks to master until all the in tree schema
are fixed yet I want to check submissions with pending checks, so I
created the 'next' branch.

> I'm not sure how I should handle this. The error reports the i2c node
> parents should have both address-cells and size-cells properties set
> to 2, but in the example there is not i2c node parent at all :)
> Should I add a parent node for the i2c in the example snippet ?

The examples have default sizes of 1 cell. If you need something
different, the example has to define a parent node to specify it. In
your case, I'd just change 'reg' to use 1 cell each.

Rob
Jacopo Mondi June 15, 2020, 3:19 p.m. UTC | #4
Hi Rob,

On Mon, Jun 15, 2020 at 09:02:28AM -0600, Rob Herring wrote:
> On Sat, Jun 13, 2020 at 6:28 AM Jacopo Mondi <jacopo@jmondi.org> wrote:
> >
> > Hi Rob,
> >
> > On Fri, Jun 12, 2020 at 04:10:03PM -0600, Rob Herring wrote:
> > > On Fri, 12 Jun 2020 15:47:10 +0100, Kieran Bingham wrote:
> > > > From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > > >
> > > > The MAX9286 deserializes video data received on up to 4 Gigabit
> > > > Multimedia Serial Links (GMSL) and outputs them on a CSI-2 port using up
> > > > to 4 data lanes.
> > > >
> > > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > > > Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> > > > Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > > > Reviewed-by: Rob Herring <robh@kernel.org>
> > > >
> > > > ---
> > > >
> > > > v7:
> > > >  - Collect Rob's RB tag
> > > >  - Remove redundant maxItems from remote-endpoints
> > > >  - Fix SPDX licence tag
> > > >
> > > > v10:
> > > > [Jacopo]
> > > >  - Fix dt-validation
> > > >  - Fix dt-binding examples with 2 reg entries
> > > >
> > > > [Kieran]
> > > >  - Correctly match the hex camera node reg
> > > >  - Add (required) GPIO controller support
> > > >
> > > >  .../bindings/media/i2c/maxim,max9286.yaml     | 366 ++++++++++++++++++
> > > >  1 file changed, 366 insertions(+)
> > > >  create mode 100644 Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml
> > > >
> > >
> > >
> > > My bot found errors running 'make dt_binding_check' on your patch:
> > >
> > > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/media/i2c/maxim,max9286.example.dt.yaml: example-0: i2c@e66d8000:reg:0: [0, 3865935872, 0, 64] is too long
> > >
> > >
> > > See https://patchwork.ozlabs.org/patch/1308280
> > >
> > > If you already ran 'make dt_binding_check' and didn't see the above
> > > error(s), then make sure dt-schema is up to date:
> > >
> > > pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade
> > >
> >
> > I have updated my dt-schema installation to the latest github master
> > -------------------------------------------------------------------------------
> > Successfully installed dtschema-2020.6.dev8+g4d2d86c
> >
> > https://github.com/devicetree-org/dt-schema/commit/4d2d86c5cd65cd3944ce0aaa400866bc36727bea
> >
> > $ /usr/bin/dt-validate -V
> > 2020.6.dev8+g4d2d86c
> > -------------------------------------------------------------------------------
> >
> > But I still cannot reproduce the error.
> >
> > However, I see this commit in your next branch
> > https://github.com/devicetree-org/dt-schema/commit/b72500282cfd2eba6f9df4d7553f696544b40ee6
> > "schemas: Add a schema to check 'reg' sizes "
> >
> > Which sounds very likely related to the above reported error.
> > Was this intentional ?
>
> Yes, I can't add the new checks to master until all the in tree schema
> are fixed yet I want to check submissions with pending checks, so I
> created the 'next' branch.

I see, makes sense. Can I just suggest to add a few words about this
new branch in the automated reply ? Otherwise the ones who are not aware
of this (like I was) will keep wondering why they don't see the error your
bot reported even if they have updated their dt-schema version to the
latest available master.

>
> > I'm not sure how I should handle this. The error reports the i2c node
> > parents should have both address-cells and size-cells properties set
> > to 2, but in the example there is not i2c node parent at all :)
> > Should I add a parent node for the i2c in the example snippet ?
>
> The examples have default sizes of 1 cell. If you need something
> different, the example has to define a parent node to specify it. In
> your case, I'd just change 'reg' to use 1 cell each.
>

Thanks, will fix.


> Rob
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml b/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml
new file mode 100644
index 000000000000..c632a10a753a
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml
@@ -0,0 +1,366 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2019 Renesas Electronics Corp.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/i2c/maxim,max9286.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Maxim Integrated Quad GMSL Deserializer
+
+maintainers:
+  - Jacopo Mondi <jacopo+renesas@jmondi.org>
+  - Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
+  - Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
+  - Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
+
+description: |
+  The MAX9286 deserializer receives video data on up to 4 Gigabit Multimedia
+  Serial Links (GMSL) and outputs them on a CSI-2 D-PHY port using up to 4 data
+  lanes.
+
+  In addition to video data, the GMSL links carry a bidirectional control
+  channel that encapsulates I2C messages. The MAX9286 forwards all I2C traffic
+  not addressed to itself to the other side of the links, where a GMSL
+  serializer will output it on a local I2C bus. In the other direction all I2C
+  traffic received over GMSL by the MAX9286 is output on the local I2C bus.
+
+properties:
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 0
+
+  compatible:
+    const: maxim,max9286
+
+  reg:
+    description: I2C device address
+    maxItems: 1
+
+  poc-supply:
+    description: Regulator providing Power over Coax to the cameras
+    maxItems: 1
+
+  enable-gpios:
+    description: GPIO connected to the \#PWDN pin with inverted polarity
+    maxItems: 1
+
+  gpio-controller: true
+
+  '#gpio-cells':
+      const: 2
+
+  ports:
+    type: object
+    description: |
+      The connections to the MAX9286 GMSL and its endpoint nodes are modelled
+      using the OF graph bindings in accordance with the video interface
+      bindings defined in
+      Documentation/devicetree/bindings/media/video-interfaces.txt.
+
+      The following table lists the port number corresponding to each device
+      port.
+
+        Port            Description
+        ----------------------------------------
+        Port 0          GMSL Input 0
+        Port 1          GMSL Input 1
+        Port 2          GMSL Input 2
+        Port 3          GMSL Input 3
+        Port 4          CSI-2 Output
+
+    properties:
+      '#address-cells':
+        const: 1
+
+      '#size-cells':
+        const: 0
+
+      port@[0-3]:
+        type: object
+        properties:
+          reg:
+            enum: [ 0, 1, 2, 3 ]
+
+          endpoint:
+            type: object
+
+            properties:
+              remote-endpoint:
+                description: |
+                 phandle to the remote GMSL source endpoint subnode in the
+                 remote node port.
+
+            required:
+              - remote-endpoint
+
+        required:
+          - reg
+          - endpoint
+
+        additionalProperties: false
+
+      port@4:
+        type: object
+        properties:
+          reg:
+            const: 4
+
+          endpoint:
+            type: object
+
+            properties:
+              remote-endpoint:
+                description: phandle to the remote CSI-2 sink endpoint.
+
+              data-lanes:
+                description: array of physical CSI-2 data lane indexes.
+
+            required:
+              - remote-endpoint
+              - data-lanes
+
+        required:
+          - reg
+          - endpoint
+
+        additionalProperties: false
+
+    required:
+      - port@4
+
+  i2c-mux:
+    type: object
+    description: |
+      Each GMSL link is modelled as a child bus of an i2c bus
+      multiplexer/switch, in accordance with bindings described in
+      Documentation/devicetree/bindings/i2c/i2c-mux.txt.
+
+    properties:
+      '#address-cells':
+        const: 1
+
+      '#size-cells':
+        const: 0
+
+    patternProperties:
+      "^i2c@[0-3]$":
+        type: object
+        description: |
+          Child node of the i2c bus multiplexer which represents a GMSL link.
+          Each serializer device on the GMSL link remote end is represented with
+          an i2c-mux child node. The MAX9286 chip supports up to 4 GMSL
+          channels.
+
+        properties:
+          '#address-cells':
+            const: 1
+
+          '#size-cells':
+            const: 0
+
+          reg:
+            description: The index of the GMSL channel.
+            maxItems: 1
+
+        patternProperties:
+          "^camera@[a-f0-9]+$":
+            type: object
+            description: |
+              The remote camera device, composed by a GMSL serializer and a
+              connected video source.
+
+            properties:
+              compatible:
+                description: The remote device compatible string.
+
+              reg:
+                minItems: 2
+                maxItems: 3
+                description: |
+                  The I2C addresses to be assigned to the remote devices through
+                  address reprogramming. The number of entries depends on the
+                  requirements of the currently connected remote device.
+
+              port:
+                type: object
+
+                properties:
+                  endpoint:
+                    type: object
+
+                    properties:
+                      remote-endpoint:
+                        description: phandle to the MAX9286 sink endpoint.
+
+                    required:
+                      - remote-endpoint
+
+                    additionalProperties: false
+
+                required:
+                  - endpoint
+
+                additionalProperties: false
+
+            required:
+              - compatible
+              - reg
+              - port
+
+            additionalProperties: false
+
+        additionalProperties: false
+
+    additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - ports
+  - i2c-mux
+  - gpio-controller
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    i2c@e66d8000 {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      reg = <0 0xe66d8000 0 0x40>;
+
+      gmsl-deserializer@2c {
+        compatible = "maxim,max9286";
+        reg = <0x2c>;
+        poc-supply = <&camera_poc_12v>;
+        enable-gpios = <&gpio 13 GPIO_ACTIVE_HIGH>;
+
+        gpio-controller;
+        #gpio-cells = <2>;
+
+        ports {
+          #address-cells = <1>;
+          #size-cells = <0>;
+
+          port@0 {
+            reg = <0>;
+
+            max9286_in0: endpoint {
+              remote-endpoint = <&rdacm20_out0>;
+            };
+          };
+
+          port@1 {
+            reg = <1>;
+
+            max9286_in1: endpoint {
+              remote-endpoint = <&rdacm20_out1>;
+            };
+          };
+
+          port@2 {
+            reg = <2>;
+
+            max9286_in2: endpoint {
+              remote-endpoint = <&rdacm20_out2>;
+            };
+          };
+
+          port@3 {
+            reg = <3>;
+
+            max9286_in3: endpoint {
+              remote-endpoint = <&rdacm20_out3>;
+            };
+          };
+
+          port@4 {
+            reg = <4>;
+
+            max9286_out: endpoint {
+              data-lanes = <1 2 3 4>;
+              remote-endpoint = <&csi40_in>;
+            };
+          };
+        };
+
+        i2c-mux {
+          #address-cells = <1>;
+          #size-cells = <0>;
+
+          i2c@0 {
+            #address-cells = <1>;
+            #size-cells = <0>;
+            reg = <0>;
+
+            camera@51 {
+              compatible = "imi,rdacm20";
+              reg = <0x51>, <0x61>;
+
+              port {
+                rdacm20_out0: endpoint {
+                  remote-endpoint = <&max9286_in0>;
+                };
+              };
+
+            };
+          };
+
+          i2c@1 {
+            #address-cells = <1>;
+            #size-cells = <0>;
+            reg = <1>;
+
+            camera@52 {
+              compatible = "imi,rdacm20";
+              reg = <0x52>, <0x62>;
+
+              port {
+                rdacm20_out1: endpoint {
+                  remote-endpoint = <&max9286_in1>;
+                };
+              };
+            };
+          };
+
+          i2c@2 {
+            #address-cells = <1>;
+            #size-cells = <0>;
+            reg = <2>;
+
+            camera@53 {
+              compatible = "imi,rdacm20";
+              reg = <0x53>, <0x63>;
+
+              port {
+                rdacm20_out2: endpoint {
+                  remote-endpoint = <&max9286_in2>;
+                };
+              };
+            };
+          };
+
+          i2c@3 {
+            #address-cells = <1>;
+            #size-cells = <0>;
+            reg = <3>;
+
+            camera@54 {
+              compatible = "imi,rdacm20";
+              reg = <0x54>, <0x64>;
+
+              port {
+                rdacm20_out3: endpoint {
+                  remote-endpoint = <&max9286_in3>;
+                };
+              };
+            };
+          };
+        };
+      };
+    };