| Message ID | 20200612144713.502006-2-kieran.bingham+renesas@ideasonboard.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Kieran Bingham |
| Headers | show |
| Series | MAX9286 GMSL Support (+RDACM20) | expand |
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.
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. >
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
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 --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>; + }; + }; + }; + }; + }; + }; + };