Message ID | 20180605233435.18102-2-kieran.bingham+renesas@ideasonboard.com (mailing list archive) |
---|---|
State | Not Applicable |
Delegated to: | Geert Uytterhoeven |
Headers | show |
Hi Kieran, On Wed, Jun 6, 2018 at 1:34 AM, Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> wrote: > Provide device tree binding documentation for the MAX9286 Quad GMSL > deserialiser. > > Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> Thanks for your patch! > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/i2c/max9286.txt > @@ -0,0 +1,75 @@ > +* Maxim Integrated MAX9286 GMSL Quad 1.5Gbps GMSL Deserializer > + > +Required Properties: > + - compatible: Shall be "maxim,max9286" > + > +The following required properties are defined externally in > +Documentation/devicetree/bindings/i2c/i2c-mux.txt: > + - Standard I2C mux properties. > + - I2C child bus nodes. > + > +A maximum of 4 I2C child nodes can be specified on the MAX9286, to > +correspond with a maximum of 4 input devices. > + > +The device node must contain one 'port' child node per device input and output > +port, in accordance with the video interface bindings defined in > +Documentation/devicetree/bindings/media/video-interfaces.txt. The port nodes > +are numbered as follows. > + > + Port Type > + ---------------------- > + 0 sink > + 1 sink > + 2 sink > + 3 sink > + 4 source I assume the source and at least one sink are thus mandatory? Would it make sense to use port 0 for the source? This would simplify extending the binding to devices with more input ports later. Gr{oetje,eeting}s, Geert
Hi Geert On 06/06/18 07:34, Geert Uytterhoeven wrote: > Hi Kieran, > > On Wed, Jun 6, 2018 at 1:34 AM, Kieran Bingham > <kieran.bingham+renesas@ideasonboard.com> wrote: >> Provide device tree binding documentation for the MAX9286 Quad GMSL >> deserialiser. >> >> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > > Thanks for your patch! Thanks for your review, >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/media/i2c/max9286.txt >> @@ -0,0 +1,75 @@ >> +* Maxim Integrated MAX9286 GMSL Quad 1.5Gbps GMSL Deserializer >> + >> +Required Properties: >> + - compatible: Shall be "maxim,max9286" >> + >> +The following required properties are defined externally in >> +Documentation/devicetree/bindings/i2c/i2c-mux.txt: >> + - Standard I2C mux properties. >> + - I2C child bus nodes. >> + >> +A maximum of 4 I2C child nodes can be specified on the MAX9286, to >> +correspond with a maximum of 4 input devices. >> + >> +The device node must contain one 'port' child node per device input and output >> +port, in accordance with the video interface bindings defined in >> +Documentation/devicetree/bindings/media/video-interfaces.txt. The port nodes >> +are numbered as follows. >> + >> + Port Type >> + ---------------------- >> + 0 sink >> + 1 sink >> + 2 sink >> + 3 sink >> + 4 source > > I assume the source and at least one sink are thus mandatory? Yes, that would make some sense :D > Would it make sense to use port 0 for the source? > This would simplify extending the binding to devices with more input > ports later. I think that sounds like a reasonable suggestion too. I'm not sure how much extending this device (family) would get yet, but it doesn't hurt to future proof. -- Regards Kieran > Gr{oetje,eeting}s, > > Geert >
On 6/6/2018 2:34 AM, Kieran Bingham wrote: > Provide device tree binding documentation for the MAX9286 Quad GMSL > deserialiser. > > Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > --- > .../devicetree/bindings/media/i2c/max9286.txt | 75 +++++++++++++++++++ > 1 file changed, 75 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/i2c/max9286.txt > > diff --git a/Documentation/devicetree/bindings/media/i2c/max9286.txt b/Documentation/devicetree/bindings/media/i2c/max9286.txt > new file mode 100644 > index 000000000000..e6e5d2c93245 > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/i2c/max9286.txt > @@ -0,0 +1,75 @@ > +* Maxim Integrated MAX9286 GMSL Quad 1.5Gbps GMSL Deserializer > + > +Required Properties: > + - compatible: Shall be "maxim,max9286" > + > +The following required properties are defined externally in > +Documentation/devicetree/bindings/i2c/i2c-mux.txt: > + - Standard I2C mux properties. > + - I2C child bus nodes. > + > +A maximum of 4 I2C child nodes can be specified on the MAX9286, to > +correspond with a maximum of 4 input devices. > + > +The device node must contain one 'port' child node per device input and output > +port, in accordance with the video interface bindings defined in > +Documentation/devicetree/bindings/media/video-interfaces.txt. The port nodes > +are numbered as follows. > + > + Port Type > + ---------------------- > + 0 sink > + 1 sink > + 2 sink > + 3 sink > + 4 source > + > +Example: > +&i2c4 { > + gmsl-deserializer@0 { Not @4c? > + compatible = "maxim,max9286"; > + reg = <0x4c>; > + poc-supply = <&poc_12v>; > + > + #address-cells = <1>; > + #size-cells = <0>; > + [...] > + i2c@0 { > + #address-cells = <1>; > + #size-cells = <0>; > + reg = <0>; > + > + camera@51 { > + compatible = MAXIM_CAMERA0; What, again? > + reg = <0x51 0x61>; > + > + port { > + rdacm20_out0: endpoint { > + remote-endpoint = <&max9286_in0>; > + }; > + }; > + }; > + }; > + }; > +}; MBR, Sergei
Hi Sergei, On 06/06/18 10:14, Sergei Shtylyov wrote: > On 6/6/2018 2:34 AM, Kieran Bingham wrote: > >> Provide device tree binding documentation for the MAX9286 Quad GMSL >> deserialiser. >> >> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> >> --- >> .../devicetree/bindings/media/i2c/max9286.txt | 75 +++++++++++++++++++ >> 1 file changed, 75 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/media/i2c/max9286.txt >> >> diff --git a/Documentation/devicetree/bindings/media/i2c/max9286.txt >> b/Documentation/devicetree/bindings/media/i2c/max9286.txt >> new file mode 100644 >> index 000000000000..e6e5d2c93245 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/media/i2c/max9286.txt >> @@ -0,0 +1,75 @@ >> +* Maxim Integrated MAX9286 GMSL Quad 1.5Gbps GMSL Deserializer >> + >> +Required Properties: >> + - compatible: Shall be "maxim,max9286" >> + >> +The following required properties are defined externally in >> +Documentation/devicetree/bindings/i2c/i2c-mux.txt: >> + - Standard I2C mux properties. >> + - I2C child bus nodes. >> + >> +A maximum of 4 I2C child nodes can be specified on the MAX9286, to >> +correspond with a maximum of 4 input devices. >> + >> +The device node must contain one 'port' child node per device input and output >> +port, in accordance with the video interface bindings defined in >> +Documentation/devicetree/bindings/media/video-interfaces.txt. The port nodes >> +are numbered as follows. >> + >> + Port Type >> + ---------------------- >> + 0 sink >> + 1 sink >> + 2 sink >> + 3 sink >> + 4 source >> + >> +Example: >> +&i2c4 { >> + gmsl-deserializer@0 { > > Not @4c? Hrm possibly. In our current working DTB (where I obtained this from) we have gmsl-deserializer@0, and gmsl-deserializer@1. I suspect you are probably right though, this should likely be the i2c address. Thanks for the spot. >> + compatible = "maxim,max9286"; >> + reg = <0x4c>; >> + poc-supply = <&poc_12v>; >> + >> + #address-cells = <1>; >> + #size-cells = <0>; >> + > [...] >> + i2c@0 { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + reg = <0>; >> + >> + camera@51 { >> + compatible = MAXIM_CAMERA0; > > What, again? Yup. Same example extract :D - I'll make sure it's fixed. Regards Kieran >> + reg = <0x51 0x61>; >> + >> + port { >> + rdacm20_out0: endpoint { >> + remote-endpoint = <&max9286_in0>; >> + }; >> + }; >> + }; >> + }; >> + }; >> +}; > > MBR, Sergei
Hi Geert, I'm replying here, even if a new version of the bindings for this chip has been posted[1], as they have the same ports layout. [1] https://www.spinics.net/lists/linux-renesas-soc/msg29307.html On Wed, Jun 06, 2018 at 08:34:41AM +0200, Geert Uytterhoeven wrote: > Hi Kieran, > > On Wed, Jun 6, 2018 at 1:34 AM, Kieran Bingham > <kieran.bingham+renesas@ideasonboard.com> wrote: > > Provide device tree binding documentation for the MAX9286 Quad GMSL > > deserialiser. > > > > Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > > Thanks for your patch! > > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/media/i2c/max9286.txt > > @@ -0,0 +1,75 @@ > > +* Maxim Integrated MAX9286 GMSL Quad 1.5Gbps GMSL Deserializer > > + > > +Required Properties: > > + - compatible: Shall be "maxim,max9286" > > + > > +The following required properties are defined externally in > > +Documentation/devicetree/bindings/i2c/i2c-mux.txt: > > + - Standard I2C mux properties. > > + - I2C child bus nodes. > > + > > +A maximum of 4 I2C child nodes can be specified on the MAX9286, to > > +correspond with a maximum of 4 input devices. > > + > > +The device node must contain one 'port' child node per device input and output > > +port, in accordance with the video interface bindings defined in > > +Documentation/devicetree/bindings/media/video-interfaces.txt. The port nodes > > +are numbered as follows. > > + > > + Port Type > > + ---------------------- > > + 0 sink > > + 1 sink > > + 2 sink > > + 3 sink > > + 4 source > > I assume the source and at least one sink are thus mandatory? > > Would it make sense to use port 0 for the source? > This would simplify extending the binding to devices with more input > ports later. I see your point, but as someone that has no idea how future chips could look like, I wonder why having multiple outputs it's more un-likely to happen than having more inputs added. Do you have any suggestion on how we can handle both cases? Thanks j > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds
Hi Jacopo, On Tue, Jul 17, 2018 at 2:13 PM jacopo mondi <jacopo@jmondi.org> wrote: > I'm replying here, even if a new version of the bindings for this > chip has been posted[1], as they have the same ports layout. > > [1] https://www.spinics.net/lists/linux-renesas-soc/msg29307.html > > On Wed, Jun 06, 2018 at 08:34:41AM +0200, Geert Uytterhoeven wrote: > > Hi Kieran, > > > > On Wed, Jun 6, 2018 at 1:34 AM, Kieran Bingham > > <kieran.bingham+renesas@ideasonboard.com> wrote: > > > Provide device tree binding documentation for the MAX9286 Quad GMSL > > > deserialiser. > > > > > > Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > > > > Thanks for your patch! > > > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/media/i2c/max9286.txt > > > @@ -0,0 +1,75 @@ > > > +* Maxim Integrated MAX9286 GMSL Quad 1.5Gbps GMSL Deserializer > > > + > > > +Required Properties: > > > + - compatible: Shall be "maxim,max9286" > > > + > > > +The following required properties are defined externally in > > > +Documentation/devicetree/bindings/i2c/i2c-mux.txt: > > > + - Standard I2C mux properties. > > > + - I2C child bus nodes. > > > + > > > +A maximum of 4 I2C child nodes can be specified on the MAX9286, to > > > +correspond with a maximum of 4 input devices. > > > + > > > +The device node must contain one 'port' child node per device input and output > > > +port, in accordance with the video interface bindings defined in > > > +Documentation/devicetree/bindings/media/video-interfaces.txt. The port nodes > > > +are numbered as follows. > > > + > > > + Port Type > > > + ---------------------- > > > + 0 sink > > > + 1 sink > > > + 2 sink > > > + 3 sink > > > + 4 source > > > > I assume the source and at least one sink are thus mandatory? > > > > Would it make sense to use port 0 for the source? > > This would simplify extending the binding to devices with more input > > ports later. > > I see your point, but as someone that has no idea how future chips could look > like, I wonder why having multiple outputs it's more un-likely to > happen than having more inputs added. I also don't know. I was just thinking "What if another chip has less or more sinks?". > Do you have any suggestion on how we can handle both cases? Instead of having a single "ports" subnode, you could split it in two subnodes, "sinks" and "sources"? I don't know if that's feasible. Gr{oetje,eeting}s, Geert
Hi Geert, On Tue, Jul 17, 2018 at 02:23:16PM +0200, Geert Uytterhoeven wrote: > Hi Jacopo, > > On Tue, Jul 17, 2018 at 2:13 PM jacopo mondi <jacopo@jmondi.org> wrote: > > I'm replying here, even if a new version of the bindings for this > > chip has been posted[1], as they have the same ports layout. > > > > [1] https://www.spinics.net/lists/linux-renesas-soc/msg29307.html > > > > On Wed, Jun 06, 2018 at 08:34:41AM +0200, Geert Uytterhoeven wrote: > > > Hi Kieran, > > > > > > On Wed, Jun 6, 2018 at 1:34 AM, Kieran Bingham > > > <kieran.bingham+renesas@ideasonboard.com> wrote: > > > > Provide device tree binding documentation for the MAX9286 Quad GMSL > > > > deserialiser. > > > > > > > > Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > > > > > > Thanks for your patch! > > > > > > > --- /dev/null > > > > +++ b/Documentation/devicetree/bindings/media/i2c/max9286.txt > > > > @@ -0,0 +1,75 @@ > > > > +* Maxim Integrated MAX9286 GMSL Quad 1.5Gbps GMSL Deserializer > > > > + > > > > +Required Properties: > > > > + - compatible: Shall be "maxim,max9286" > > > > + > > > > +The following required properties are defined externally in > > > > +Documentation/devicetree/bindings/i2c/i2c-mux.txt: > > > > + - Standard I2C mux properties. > > > > + - I2C child bus nodes. > > > > + > > > > +A maximum of 4 I2C child nodes can be specified on the MAX9286, to > > > > +correspond with a maximum of 4 input devices. > > > > + > > > > +The device node must contain one 'port' child node per device input and output > > > > +port, in accordance with the video interface bindings defined in > > > > +Documentation/devicetree/bindings/media/video-interfaces.txt. The port nodes > > > > +are numbered as follows. > > > > + > > > > + Port Type > > > > + ---------------------- > > > > + 0 sink > > > > + 1 sink > > > > + 2 sink > > > > + 3 sink > > > > + 4 source > > > > > > I assume the source and at least one sink are thus mandatory? > > > > > > Would it make sense to use port 0 for the source? > > > This would simplify extending the binding to devices with more input > > > ports later. > > > > I see your point, but as someone that has no idea how future chips could look > > like, I wonder why having multiple outputs it's more un-likely to > > happen than having more inputs added. > > I also don't know. > I was just thinking "What if another chip has less or more sinks?". > > > Do you have any suggestion on how we can handle both cases? > > Instead of having a single "ports" subnode, you could split it in two subnodes, > "sinks" and "sources"? I don't know if that's feasible. > Maybe I didn't get you here, and sorry to repeat something obvious for you but, that would be against basic assumptions both in fwnode/of framework and in media framework too. See the graph.txt documentation and video_interfaces.txt, the layout with a single 'ports' node with multiple 'port' sub-nodes is not avoidable, afaik. What is not -theoretically- forbidden is to have port subnodes with multiple endpoints, which is also used as an example in multiple places in the documentation files I mentioned above. Unfortunately, as we experienced with the ADV748x and the R-Car CSI-2 receiver upstreaming effort, the v4l2-async framework relies on matching on the remote endpoint's parent device node, and Kieran and Niklas had to use a custom endpoint matching logic for the R-Car CSI-2 and adv748x combo, something I'm sure nobody wants to repeat here. We said that multiple times that we would like to tackle this limitation (if that's limitation) in the v4l2_async framework, maybe if GMSL-like devices with multiple input/output ports comes out, we might have enough motivation to do that ;) ? Thanks j > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds
Hi Jacopo, On Tue, Jul 17, 2018 at 2:59 PM jacopo mondi <jacopo@jmondi.org> wrote: > On Tue, Jul 17, 2018 at 02:23:16PM +0200, Geert Uytterhoeven wrote: > > On Tue, Jul 17, 2018 at 2:13 PM jacopo mondi <jacopo@jmondi.org> wrote: > > > I'm replying here, even if a new version of the bindings for this > > > chip has been posted[1], as they have the same ports layout. > > > > > > [1] https://www.spinics.net/lists/linux-renesas-soc/msg29307.html > > > > > > On Wed, Jun 06, 2018 at 08:34:41AM +0200, Geert Uytterhoeven wrote: > > > > Hi Kieran, > > > > > > > > On Wed, Jun 6, 2018 at 1:34 AM, Kieran Bingham > > > > <kieran.bingham+renesas@ideasonboard.com> wrote: > > > > > Provide device tree binding documentation for the MAX9286 Quad GMSL > > > > > deserialiser. > > > > > > > > > > Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > > > > > > > > Thanks for your patch! > > > > > > > > > --- /dev/null > > > > > +++ b/Documentation/devicetree/bindings/media/i2c/max9286.txt > > > > > @@ -0,0 +1,75 @@ > > > > > +* Maxim Integrated MAX9286 GMSL Quad 1.5Gbps GMSL Deserializer > > > > > + > > > > > +Required Properties: > > > > > + - compatible: Shall be "maxim,max9286" > > > > > + > > > > > +The following required properties are defined externally in > > > > > +Documentation/devicetree/bindings/i2c/i2c-mux.txt: > > > > > + - Standard I2C mux properties. > > > > > + - I2C child bus nodes. > > > > > + > > > > > +A maximum of 4 I2C child nodes can be specified on the MAX9286, to > > > > > +correspond with a maximum of 4 input devices. > > > > > + > > > > > +The device node must contain one 'port' child node per device input and output > > > > > +port, in accordance with the video interface bindings defined in > > > > > +Documentation/devicetree/bindings/media/video-interfaces.txt. The port nodes > > > > > +are numbered as follows. > > > > > + > > > > > + Port Type > > > > > + ---------------------- > > > > > + 0 sink > > > > > + 1 sink > > > > > + 2 sink > > > > > + 3 sink > > > > > + 4 source > > > > > > > > I assume the source and at least one sink are thus mandatory? > > > > > > > > Would it make sense to use port 0 for the source? > > > > This would simplify extending the binding to devices with more input > > > > ports later. > > > > > > I see your point, but as someone that has no idea how future chips could look > > > like, I wonder why having multiple outputs it's more un-likely to > > > happen than having more inputs added. > > > > I also don't know. > > I was just thinking "What if another chip has less or more sinks?". > > > > > Do you have any suggestion on how we can handle both cases? > > > > Instead of having a single "ports" subnode, you could split it in two subnodes, > > "sinks" and "sources"? I don't know if that's feasible. > > > Maybe I didn't get you here, and sorry to repeat something obvious for > you but, that would be against basic assumptions both in fwnode/of framework > and in media framework too. See the graph.txt documentation and > video_interfaces.txt, the layout with a single 'ports' node with > multiple 'port' sub-nodes is not avoidable, afaik. > > What is not -theoretically- forbidden is to have port subnodes with > multiple endpoints, which is also used as an example in multiple > places in the documentation files I mentioned above. Unfortunately, as > we experienced with the ADV748x and the R-Car CSI-2 receiver > upstreaming effort, the v4l2-async framework relies on matching on the > remote endpoint's parent device node, and Kieran and Niklas had to use > a custom endpoint matching logic for the R-Car CSI-2 and adv748x > combo, something I'm sure nobody wants to repeat here. > > We said that multiple times that we would like to tackle this > limitation (if that's limitation) in the v4l2_async framework, maybe > if GMSL-like devices with multiple input/output ports comes out, we > might have enough motivation to do that ;) ? I'm totally ignorant w.r.t. multimedia endpoints, hence the "... don't know if that's feasible". Gr{oetje,eeting}s, Geert
diff --git a/Documentation/devicetree/bindings/media/i2c/max9286.txt b/Documentation/devicetree/bindings/media/i2c/max9286.txt new file mode 100644 index 000000000000..e6e5d2c93245 --- /dev/null +++ b/Documentation/devicetree/bindings/media/i2c/max9286.txt @@ -0,0 +1,75 @@ +* Maxim Integrated MAX9286 GMSL Quad 1.5Gbps GMSL Deserializer + +Required Properties: + - compatible: Shall be "maxim,max9286" + +The following required properties are defined externally in +Documentation/devicetree/bindings/i2c/i2c-mux.txt: + - Standard I2C mux properties. + - I2C child bus nodes. + +A maximum of 4 I2C child nodes can be specified on the MAX9286, to +correspond with a maximum of 4 input devices. + +The device node must contain one 'port' child node per device input and output +port, in accordance with the video interface bindings defined in +Documentation/devicetree/bindings/media/video-interfaces.txt. The port nodes +are numbered as follows. + + Port Type + ---------------------- + 0 sink + 1 sink + 2 sink + 3 sink + 4 source + +Example: +&i2c4 { + gmsl-deserializer@0 { + compatible = "maxim,max9286"; + reg = <0x4c>; + poc-supply = <&poc_12v>; + + #address-cells = <1>; + #size-cells = <0>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + max9286_in0: endpoint { + remote-endpoint = <&rdacm20_out0>; + }; + }; + + port@4 { + reg = <4>; + max9286_out0: endpoint { + clock-lanes = <0>; + data-lanes = <1 2 3 4>; + remote-endpoint = <&csi40_in>; + }; + }; + }; + + i2c@0 { + #address-cells = <1>; + #size-cells = <0>; + reg = <0>; + + camera@51 { + compatible = MAXIM_CAMERA0; + reg = <0x51 0x61>; + + port { + rdacm20_out0: endpoint { + remote-endpoint = <&max9286_in0>; + }; + }; + }; + }; + }; +};
Provide device tree binding documentation for the MAX9286 Quad GMSL deserialiser. Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> --- .../devicetree/bindings/media/i2c/max9286.txt | 75 +++++++++++++++++++ 1 file changed, 75 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/i2c/max9286.txt