| Message ID | 20170720092302.2982-2-maxime.ripard@free-electrons.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Jul 20, 2017 at 11:23:01AM +0200, Maxime Ripard wrote: > The Cadence MIPI-CSI2 RX controller is a CSI2RX bridge that supports up to > 4 CSI-2 lanes, and can route the frames to up to 4 streams, depending on > the hardware implementation. > > It can operate with an external D-PHY, an internal one or no D-PHY at all > in some configurations. > > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> > --- > .../devicetree/bindings/media/cdns-csi2rx.txt | 87 ++++++++++++++++++++++ > 1 file changed, 87 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/cdns-csi2rx.txt Acked-by: Rob Herring <robh@kernel.org>
Maxime Ripard <maxime.ripard@free-electrons.com> wrote on Thu [2017-Jul-20 11:23:01 +0200]: > The Cadence MIPI-CSI2 RX controller is a CSI2RX bridge that supports up to > 4 CSI-2 lanes, and can route the frames to up to 4 streams, depending on > the hardware implementation. > > It can operate with an external D-PHY, an internal one or no D-PHY at all > in some configurations. > > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> > --- > .../devicetree/bindings/media/cdns-csi2rx.txt | 87 ++++++++++++++++++++++ > 1 file changed, 87 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/cdns-csi2rx.txt > Acked-by: Benoit Parrot <bparrot@ti.com>
Hi Maxime, Thank you for the patch. On Thursday 20 Jul 2017 11:23:01 Maxime Ripard wrote: > The Cadence MIPI-CSI2 RX controller is a CSI2RX bridge that supports up to > 4 CSI-2 lanes, and can route the frames to up to 4 streams, depending on > the hardware implementation. > > It can operate with an external D-PHY, an internal one or no D-PHY at all > in some configurations. Without any PHY ? I'm curious, how does that work ? > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> > --- > .../devicetree/bindings/media/cdns-csi2rx.txt | 87 ++++++++++++++++++ > 1 file changed, 87 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/cdns-csi2rx.txt > > diff --git a/Documentation/devicetree/bindings/media/cdns-csi2rx.txt > b/Documentation/devicetree/bindings/media/cdns-csi2rx.txt new file mode > 100644 > index 000000000000..e08547abe885 > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/cdns-csi2rx.txt > @@ -0,0 +1,87 @@ > +Cadence MIPI-CSI2 RX controller > +=============================== > + > +The Cadence MIPI-CSI2 RX controller is a CSI-2 bridge supporting up to 4 > CSI > +lanes in input, and 4 different pixel streams in output. > + > +Required properties: > + - compatible: must be set to "cdns,csi2rx" and an SoC-specific compatible > + - reg: base address and size of the memory mapped region > + - clocks: phandles to the clocks driving the controller > + - clock-names: must contain: > + * sys_clk: main clock > + * p_clk: register bank clock > + * p_free_clk: free running register bank clock > + * pixel_ifX_clk: pixel stream output clock, one for each stream > + implemented in hardware, between 0 and 3 Nitpicking, I would write the name is pixel_if[0-3]_clk, it took me a few seconds to see that the X was a placeholder. > + * dphy_rx_clk: D-PHY byte clock, if implemented in hardware > + - phys: phandle to the external D-PHY > + - phy-names: must contain dphy, if the implementation uses an > + external D-PHY I would move the last two properties in an optional category as they're effectively optional. I think you should also explain a bit more clearly that the phys property must not be present if the phy-names property is not present. > + > +Required subnodes: > + - ports: A ports node with endpoint definitions as defined in > + Documentation/devicetree/bindings/media/video-interfaces.txt. > The > + first port subnode should be the input endpoint, the second one > the > + outputs > + > + The output port should have as many endpoints as stream supported by > + the hardware implementation, between 1 and 4, their ID being the > + stream output number used in the implementation. I don't think that's correct. The IP has four independent outputs, it should have 4 output ports for a total for 5 ports. Multiple endpoints per port would describe multiple connections from the same output to different sinks. > +Example: > + > +csi2rx: csi-bridge@0d060000 { > + compatible = "cdns,csi2rx"; > + reg = <0x0d060000 0x1000>; > + clocks = <&byteclock>, <&byteclock>, <&byteclock>, > + <&coreclock>, <&coreclock>, > + <&coreclock>, <&coreclock>; > + clock-names = "sys_clk", "p_clk", "p_free_clk", > + "pixel_if0_clk", "pixel_if1_clk", > + "pixel_if2_clk", "pixel_if3_clk"; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + #address-cells = <1>; > + #size-cells = <0>; > + reg = <0>; > + > + csi2rx_in_sensor: endpoint@0 { > + reg = <0>; > + remote-endpoint = <&sensor_out_csi2rx>; > + clock-lanes = <0>; > + data-lanes = <1 2>; > + }; > + }; > + > + port@1 { > + #address-cells = <1>; > + #size-cells = <0>; > + reg = <1>; > + > + csi2rx_out_grabber0: endpoint@0 { > + reg = <0>; > + remote-endpoint = <&grabber0_in_csi2rx>; > + }; > + > + csi2rx_out_grabber1: endpoint@1 { > + reg = <1>; > + remote-endpoint = <&grabber1_in_csi2rx>; > + }; > + > + csi2rx_out_grabber2: endpoint@2 { > + reg = <2>; > + remote-endpoint = <&grabber2_in_csi2rx>; > + }; > + > + csi2rx_out_grabber3: endpoint@3 { > + reg = <3>; > + remote-endpoint = <&grabber3_in_csi2rx>; > + }; > + }; > + }; > +};
Hi Laurent, Thanks a lot for reviewing those patches. On Mon, Aug 07, 2017 at 11:18:03PM +0300, Laurent Pinchart wrote: > Hi Maxime, > > Thank you for the patch. > > On Thursday 20 Jul 2017 11:23:01 Maxime Ripard wrote: > > The Cadence MIPI-CSI2 RX controller is a CSI2RX bridge that supports up to > > 4 CSI-2 lanes, and can route the frames to up to 4 streams, depending on > > the hardware implementation. > > > > It can operate with an external D-PHY, an internal one or no D-PHY at all > > in some configurations. > > Without any PHY ? I'm curious, how does that work ? We're currently working on an FPGA exactly with that configuration. So I guess the answer would be "it doesn't on an ASIC" :) > > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> > > --- > > .../devicetree/bindings/media/cdns-csi2rx.txt | 87 ++++++++++++++++++ > > 1 file changed, 87 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/media/cdns-csi2rx.txt > > > > diff --git a/Documentation/devicetree/bindings/media/cdns-csi2rx.txt > > b/Documentation/devicetree/bindings/media/cdns-csi2rx.txt new file mode > > 100644 > > index 000000000000..e08547abe885 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/media/cdns-csi2rx.txt > > @@ -0,0 +1,87 @@ > > +Cadence MIPI-CSI2 RX controller > > +=============================== > > + > > +The Cadence MIPI-CSI2 RX controller is a CSI-2 bridge supporting up to 4 > > CSI > > +lanes in input, and 4 different pixel streams in output. > > + > > +Required properties: > > + - compatible: must be set to "cdns,csi2rx" and an SoC-specific compatible > > + - reg: base address and size of the memory mapped region > > + - clocks: phandles to the clocks driving the controller > > + - clock-names: must contain: > > + * sys_clk: main clock > > + * p_clk: register bank clock > > + * p_free_clk: free running register bank clock > > + * pixel_ifX_clk: pixel stream output clock, one for each stream > > + implemented in hardware, between 0 and 3 > > Nitpicking, I would write the name is pixel_if[0-3]_clk, it took me a few > seconds to see that the X was a placeholder. Ok. > > + * dphy_rx_clk: D-PHY byte clock, if implemented in hardware > > + - phys: phandle to the external D-PHY > > + - phy-names: must contain dphy, if the implementation uses an > > + external D-PHY > > I would move the last two properties in an optional category as they're > effectively optional. I think you should also explain a bit more clearly that > the phys property must not be present if the phy-names property is not > present. It's not really optional. The IP has a configuration register that allows you to see if it's been synthesized with or without a PHY. If the right bit is set, that property will be mandatory, if not, it's useless. Maybe it's just semantics, but to me, optional means that it can operate with or without it under any circumstances. It's not really the case here. > > + > > +Required subnodes: > > + - ports: A ports node with endpoint definitions as defined in > > + Documentation/devicetree/bindings/media/video-interfaces.txt. > > The > > + first port subnode should be the input endpoint, the second one > > the > > + outputs > > + > > + The output port should have as many endpoints as stream supported by > > + the hardware implementation, between 1 and 4, their ID being the > > + stream output number used in the implementation. > > I don't think that's correct. The IP has four independent outputs, it should > have 4 output ports for a total for 5 ports. Multiple endpoints per port would > describe multiple connections from the same output to different sinks. Ok. Thanks! Maxime
Hi Maxime, On Tuesday, 22 August 2017 11:53:20 EEST Maxime Ripard wrote: > On Mon, Aug 07, 2017 at 11:18:03PM +0300, Laurent Pinchart wrote: > > On Thursday 20 Jul 2017 11:23:01 Maxime Ripard wrote: > >> The Cadence MIPI-CSI2 RX controller is a CSI2RX bridge that supports up > >> to 4 CSI-2 lanes, and can route the frames to up to 4 streams, depending > >> on the hardware implementation. > >> > >> It can operate with an external D-PHY, an internal one or no D-PHY at > >> all in some configurations. > > > > Without any PHY ? I'm curious, how does that work ? > > We're currently working on an FPGA exactly with that configuration. So > I guess the answer would be "it doesn't on an ASIC" :) What's connected to the input of the CSI-2 receiver ? > >> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> > >> --- > >> > >> .../devicetree/bindings/media/cdns-csi2rx.txt | 87 ++++++++++++++++ > >> 1 file changed, 87 insertions(+) > >> create mode 100644 > >> Documentation/devicetree/bindings/media/cdns-csi2rx.txt > >> > >> diff --git a/Documentation/devicetree/bindings/media/cdns-csi2rx.txt > >> b/Documentation/devicetree/bindings/media/cdns-csi2rx.txt new file mode > >> 100644 > >> index 000000000000..e08547abe885 > >> --- /dev/null > >> +++ b/Documentation/devicetree/bindings/media/cdns-csi2rx.txt > >> @@ -0,0 +1,87 @@ > >> +Cadence MIPI-CSI2 RX controller > >> +=============================== > >> + > >> +The Cadence MIPI-CSI2 RX controller is a CSI-2 bridge supporting up to > >> 4> CSI > >> +lanes in input, and 4 different pixel streams in output. > >> + > >> +Required properties: > >> + - compatible: must be set to "cdns,csi2rx" and an SoC-specific > >> compatible > >> + - reg: base address and size of the memory mapped region > >> + - clocks: phandles to the clocks driving the controller > >> + - clock-names: must contain: > >> + * sys_clk: main clock > >> + * p_clk: register bank clock > >> + * p_free_clk: free running register bank clock > >> + * pixel_ifX_clk: pixel stream output clock, one for each stream > >> + implemented in hardware, between 0 and 3 > > > > Nitpicking, I would write the name is pixel_if[0-3]_clk, it took me a few > > seconds to see that the X was a placeholder. > > Ok. > > >> + * dphy_rx_clk: D-PHY byte clock, if implemented in hardware > >> + - phys: phandle to the external D-PHY > >> + - phy-names: must contain dphy, if the implementation uses an > >> + external D-PHY > > > > I would move the last two properties in an optional category as they're > > effectively optional. I think you should also explain a bit more clearly > > that the phys property must not be present if the phy-names property is > > not present. > > It's not really optional. The IP has a configuration register that > allows you to see if it's been synthesized with or without a PHY. If > the right bit is set, that property will be mandatory, if not, it's > useless. Just to confirm, the PHY is a separate IP core, right ? Is the CSI-2 receiver input interface different when used with a PHY and when used without one ? Could a third-party PHY be used as well ? If so, would the PHY synthesis bit be set or not ? > Maybe it's just semantics, but to me, optional means that it can > operate with or without it under any circumstances. It's not really > the case here. It'sa semantic issue, but documenting a property as required when it can be ommitted under some circumstances seems even weirder to me :-) I understand the optional category as "can be ommitted in certain circumstances". > >> + > >> +Required subnodes: > >> + - ports: A ports node with endpoint definitions as defined in > >> + > >> Documentation/devicetree/bindings/media/video-interfaces.txt. > >> The > >> + first port subnode should be the input endpoint, the second > >> one the > >> + outputs > >> + > >> + The output port should have as many endpoints as stream supported by > >> + the hardware implementation, between 1 and 4, their ID being the > >> + stream output number used in the implementation. > > > > I don't think that's correct. The IP has four independent outputs, it > > should have 4 output ports for a total for 5 ports. Multiple endpoints > > per port would describe multiple connections from the same output to > > different sinks. > > Ok.
Hi Laurent,
On 22/08/2017, 02:01, "Laurent Pinchart" <laurent.pinchart@ideasonboard.com> wrote:
EXTERNAL MAIL
Hi Maxime,
On Tuesday, 22 August 2017 11:53:20 EEST Maxime Ripard wrote:
> On Mon, Aug 07, 2017 at 11:18:03PM +0300, Laurent Pinchart wrote:
> > On Thursday 20 Jul 2017 11:23:01 Maxime Ripard wrote:
> >> The Cadence MIPI-CSI2 RX controller is a CSI2RX bridge that supports up
> >> to 4 CSI-2 lanes, and can route the frames to up to 4 streams, depending
> >> on the hardware implementation.
> >>
> >> It can operate with an external D-PHY, an internal one or no D-PHY at
> >> all in some configurations.
> >
> > Without any PHY ? I'm curious, how does that work ?
>
> We're currently working on an FPGA exactly with that configuration. So
> I guess the answer would be "it doesn't on an ASIC" :)
What's connected to the input of the CSI-2 receiver ?
It is connected to an instance of a CSI TX digital interface.
> >> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> >> ---
> >>
> >> .../devicetree/bindings/media/cdns-csi2rx.txt | 87 ++++++++++++++++
> >> 1 file changed, 87 insertions(+)
> >> create mode 100644
> >> Documentation/devicetree/bindings/media/cdns-csi2rx.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/media/cdns-csi2rx.txt
> >> b/Documentation/devicetree/bindings/media/cdns-csi2rx.txt new file mode
> >> 100644
> >> index 000000000000..e08547abe885
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/media/cdns-csi2rx.txt
> >> @@ -0,0 +1,87 @@
> >> +Cadence MIPI-CSI2 RX controller
> >> +===============================
> >> +
> >> +The Cadence MIPI-CSI2 RX controller is a CSI-2 bridge supporting up to
> >> 4> CSI
> >> +lanes in input, and 4 different pixel streams in output.
> >> +
> >> +Required properties:
> >> + - compatible: must be set to "cdns,csi2rx" and an SoC-specific
> >> compatible
> >> + - reg: base address and size of the memory mapped region
> >> + - clocks: phandles to the clocks driving the controller
> >> + - clock-names: must contain:
> >> + * sys_clk: main clock
> >> + * p_clk: register bank clock
> >> + * p_free_clk: free running register bank clock
> >> + * pixel_ifX_clk: pixel stream output clock, one for each stream
> >> + implemented in hardware, between 0 and 3
> >
> > Nitpicking, I would write the name is pixel_if[0-3]_clk, it took me a few
> > seconds to see that the X was a placeholder.
>
> Ok.
>
> >> + * dphy_rx_clk: D-PHY byte clock, if implemented in hardware
> >> + - phys: phandle to the external D-PHY
> >> + - phy-names: must contain dphy, if the implementation uses an
> >> + external D-PHY
> >
> > I would move the last two properties in an optional category as they're
> > effectively optional. I think you should also explain a bit more clearly
> > that the phys property must not be present if the phy-names property is
> > not present.
>
> It's not really optional. The IP has a configuration register that
> allows you to see if it's been synthesized with or without a PHY. If
> the right bit is set, that property will be mandatory, if not, it's
> useless.
Just to confirm, the PHY is a separate IP core, right ? Is the CSI-2 receiver
input interface different when used with a PHY and when used without one ?
Could a third-party PHY be used as well ? If so, would the PHY synthesis bit
be set or not ?
The PHY (in our case a D-PHY) is a separate entity, it can be from a 3rd party as the IP interface is standard, the SoC integrator would set the bit accordingly based on whether any PHY is present or not.
There is also an option of routing digital output from a CSI-TX to a CSI-RX and in such case a PHY would not need to be used (as in the case of our current platform).
> Maybe it's just semantics, but to me, optional means that it can
> operate with or without it under any circumstances. It's not really
> the case here.
It'sa semantic issue, but documenting a property as required when it can be
ommitted under some circumstances seems even weirder to me :-) I understand
the optional category as "can be ommitted in certain circumstances".
> >> +
> >> +Required subnodes:
> >> + - ports: A ports node with endpoint definitions as defined in
> >> +
> >> Documentation/devicetree/bindings/media/video-interfaces.txt.
> >> The
> >> + first port subnode should be the input endpoint, the second
> >> one the
> >> + outputs
> >> +
> >> + The output port should have as many endpoints as stream supported by
> >> + the hardware implementation, between 1 and 4, their ID being the
> >> + stream output number used in the implementation.
> >
> > I don't think that's correct. The IP has four independent outputs, it
> > should have 4 output ports for a total for 5 ports. Multiple endpoints
> > per port would describe multiple connections from the same output to
> > different sinks.
>
> Ok.
Regards,
Cyprian
Hi Cyprian, On Tuesday, 22 August 2017 22:25:49 EEST Cyprian Wronka wrote: > On 22/08/2017, 02:01, Laurent Pinchart wrote: >> On Tuesday, 22 August 2017 11:53:20 EEST Maxime Ripard wrote: >>> On Mon, Aug 07, 2017 at 11:18:03PM +0300, Laurent Pinchart wrote: >>>> On Thursday 20 Jul 2017 11:23:01 Maxime Ripard wrote: >>>> The Cadence MIPI-CSI2 RX controller is a CSI2RX bridge that >>>>> supports up to 4 CSI-2 lanes, and can route the frames to up to 4 >>>>> streams, depending on the hardware implementation. >>>>> >>>>> It can operate with an external D-PHY, an internal one or no D-PHY >>>>> at all in some configurations. >>>> >>>> Without any PHY ? I'm curious, how does that work ? >>> >>> We're currently working on an FPGA exactly with that configuration. >>> So I guess the answer would be "it doesn't on an ASIC" :) >>> >> What's connected to the input of the CSI-2 receiver ? >> > It is connected to an instance of a CSI TX digital interface. That makes sense. I suppose it's a test setup >>>>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> >>>>> --- >>>>> >>>>> >>>>> .../devicetree/bindings/media/cdns-csi2rx.txt> | 87 >>>>> ++++++++++++++++ >>>>> 1 file changed, 87 insertions(+) >>>>> create mode 100644 >>>>> Documentation/devicetree/bindings/media/cdns-csi2rx.txt >>>>> >>>>> >>>>> diff --git >>>>> a/Documentation/devicetree/bindings/media/cdns-csi2rx.txt >>>>> b/Documentation/devicetree/bindings/media/cdns-csi2rx.txt new file >>>>> mode 100644 >>>>> index 000000000000..e08547abe885 >>>>> --- /dev/null >>>>> +++ b/Documentation/devicetree/bindings/media/cdns-csi2rx.txt >>>>> @@ -0,0 +1,87 @@ >>>>> +Cadence MIPI-CSI2 RX controller >>>>> +=============================== >>>>> + >>>>> +The Cadence MIPI-CSI2 RX controller is a CSI-2 bridge supporting >>>>> up to 4 CSI >>>>> +lanes in input, and 4 different pixel streams in output. >>>>> + >>>>> +Required properties: >>>>> + - compatible: must be set to "cdns,csi2rx" and an SoC-specific >>>>> compatible >>>>> + - reg: base address and size of the memory mapped region >>>>> + - clocks: phandles to the clocks driving the controller >>>>> + - clock-names: must contain: >>>>> + * sys_clk: main clock >>>>> + * p_clk: register bank clock >>>>> + * p_free_clk: free running register bank clock >>>>> + * pixel_ifX_clk: pixel stream output clock, one for each >>>>> stream >>>>> + implemented in hardware, between 0 and 3 >>>> >>>> Nitpicking, I would write the name is pixel_if[0-3]_clk, it took me >>>> a few seconds to see that the X was a placeholder. >>> >>> Ok. >>> >>>>> + * dphy_rx_clk: D-PHY byte clock, if implemented in hardware >>>>> + - phys: phandle to the external D-PHY >>>>> + - phy-names: must contain dphy, if the implementation uses an >>>>> + external D-PHY >>>> >>>> I would move the last two properties in an optional category as >>>> they're effectively optional. I think you should also explain a bit more >>>> clearly that the phys property must not be present if the phy-names >>>> property is not present. >>> >>> It's not really optional. The IP has a configuration register that >>> allows you to see if it's been synthesized with or without a PHY. If >>> the right bit is set, that property will be mandatory, if not, it's >>> useless. >> >> Just to confirm, the PHY is a separate IP core, right ? Is the CSI-2 >> receiver input interface different when used with a PHY and when used >> without one ? Could a third-party PHY be used as well ? If so, would the >> PHY synthesis bit be set or not ? > > The PHY (in our case a D-PHY) is a separate entity, it can be from a 3rd > party as the IP interface is standard, the SoC integrator would set the bit > accordingly based on whether any PHY is present or not. There is also an > option of routing digital output from a CSI-TX to a CSI-RX and in such case > a PHY would not need to be used (as in the case of our current platform). OK, thank you for the clarification. Maxime mentioned that a bit can be read from a register to notify whether a PHY has been synthesized or not. Does it influence the CSI-2 RX input interface at all, or is the CSI-2 RX IP core synthesized the same way regardless of whether a PHY is present or not ? >>> Maybe it's just semantics, but to me, optional means that it can >>> operate with or without it under any circumstances. It's not really >>> the case here. >> >> It's a semantic issue, but documenting a property as required when it can >> be ommitted under some circumstances seems even weirder to me :-) I >> understand the optional category as "can be ommitted in certain >> circumstances". >> >>>>> + >>>>> +Required subnodes: >>>>> + - ports: A ports node with endpoint definitions as defined in >>>>> + Documentation/devicetree/bindings/media/video-interfaces.txt. The >>>>> + first port subnode should be the input endpoint, the >>>>> second one the >>>>> + outputs >>>>> + >>>>> + The output port should have as many endpoints as stream >>>>> supported by >>>>> + the hardware implementation, between 1 and 4, their ID being >>>>> the >>>>> + stream output number used in the implementation. >>>> >>>> I don't think that's correct. The IP has four independent outputs, >>>> it should have 4 output ports for a total for 5 ports. Multiple >>>> endpoints per port would describe multiple connections from the same >>>> output to different sinks. >>> >>> Ok.
Hi Laurent, On Wed, Aug 23, 2017 at 12:03:32AM +0300, Laurent Pinchart wrote: > >>>>> + - phys: phandle to the external D-PHY > >>>>> + - phy-names: must contain dphy, if the implementation uses an > >>>>> + external D-PHY > >>>> > >>>> I would move the last two properties in an optional category as > >>>> they're effectively optional. I think you should also explain a bit more > >>>> clearly that the phys property must not be present if the phy-names > >>>> property is not present. > >>> > >>> It's not really optional. The IP has a configuration register that > >>> allows you to see if it's been synthesized with or without a PHY. If > >>> the right bit is set, that property will be mandatory, if not, it's > >>> useless. > >> > >> Just to confirm, the PHY is a separate IP core, right ? Is the CSI-2 > >> receiver input interface different when used with a PHY and when used > >> without one ? Could a third-party PHY be used as well ? If so, would the > >> PHY synthesis bit be set or not ? > > > > The PHY (in our case a D-PHY) is a separate entity, it can be from a 3rd > > party as the IP interface is standard, the SoC integrator would set the bit > > accordingly based on whether any PHY is present or not. There is also an > > option of routing digital output from a CSI-TX to a CSI-RX and in such case > > a PHY would not need to be used (as in the case of our current platform). > > OK, thank you for the clarification. > > Maxime mentioned that a bit can be read from a register to notify whether a > PHY has been synthesized or not. Does it influence the CSI-2 RX input > interface at all, or is the CSI-2 RX IP core synthesized the same way > regardless of whether a PHY is present or not ? So we got an answer to this, and the physical interface remains the same. However, the PHY bit is set only when there's an internal D-PHY, which means we have basically three cases: - No D-PHY at all, D-PHY presence bit not set - Internal D-PHY, D-PHY presence bit set - External D-PHY, D-PHY presence bit not set I guess that solves our discussion about whether the phys property should be marked optional or not. It should indeed be optional, and when it's not there, the D-PHY presence bit will tell whether we have to program the internal D-PHY or not. Maxime
Hi Maxime, On Friday, 25 August 2017 17:44:40 EEST Maxime Ripard wrote: > On Wed, Aug 23, 2017 at 12:03:32AM +0300, Laurent Pinchart wrote: > >>>>>> + - phys: phandle to the external D-PHY > >>>>>> + - phy-names: must contain dphy, if the implementation uses an > >>>>>> + external D-PHY > >>>>> > >>>>> I would move the last two properties in an optional category as > >>>>> they're effectively optional. I think you should also explain a bit > >>>>> more clearly that the phys property must not be present if the phy- > >>>>> names property is not present. > >>>> > >>>> It's not really optional. The IP has a configuration register that > >>>> allows you to see if it's been synthesized with or without a PHY. If > >>>> the right bit is set, that property will be mandatory, if not, it's > >>>> useless. > >>> > >>> Just to confirm, the PHY is a separate IP core, right ? Is the CSI-2 > >>> receiver input interface different when used with a PHY and when used > >>> without one ? Could a third-party PHY be used as well ? If so, would > >>> the PHY synthesis bit be set or not ? > >> > >> The PHY (in our case a D-PHY) is a separate entity, it can be from a 3rd > >> party as the IP interface is standard, the SoC integrator would set the > >> bit accordingly based on whether any PHY is present or not. There is also > >> an option of routing digital output from a CSI-TX to a CSI-RX and in such > >> case a PHY would not need to be used (as in the case of our current > >> platform). > > > > OK, thank you for the clarification. > > > > Maxime mentioned that a bit can be read from a register to notify whether > > a PHY has been synthesized or not. Does it influence the CSI-2 RX input > > interface at all, or is the CSI-2 RX IP core synthesized the same way > > regardless of whether a PHY is present or not ? > > So we got an answer to this, and the physical interface remains the > same. > > However, the PHY bit is set only when there's an internal D-PHY, which > means we have basically three cases: > - No D-PHY at all, D-PHY presence bit not set > - Internal D-PHY, D-PHY presence bit set > - External D-PHY, D-PHY presence bit not set > > I guess that solves our discussion about whether the phys property > should be marked optional or not. It should indeed be optional, and > when it's not there, the D-PHY presence bit will tell whether we have > to program the internal D-PHY or not. Is the internal D-PHY programmed through the register space of the CSI2-RX ? If so I agree with you.
diff --git a/Documentation/devicetree/bindings/media/cdns-csi2rx.txt b/Documentation/devicetree/bindings/media/cdns-csi2rx.txt new file mode 100644 index 000000000000..e08547abe885 --- /dev/null +++ b/Documentation/devicetree/bindings/media/cdns-csi2rx.txt @@ -0,0 +1,87 @@ +Cadence MIPI-CSI2 RX controller +=============================== + +The Cadence MIPI-CSI2 RX controller is a CSI-2 bridge supporting up to 4 CSI +lanes in input, and 4 different pixel streams in output. + +Required properties: + - compatible: must be set to "cdns,csi2rx" and an SoC-specific compatible + - reg: base address and size of the memory mapped region + - clocks: phandles to the clocks driving the controller + - clock-names: must contain: + * sys_clk: main clock + * p_clk: register bank clock + * p_free_clk: free running register bank clock + * pixel_ifX_clk: pixel stream output clock, one for each stream + implemented in hardware, between 0 and 3 + * dphy_rx_clk: D-PHY byte clock, if implemented in hardware + - phys: phandle to the external D-PHY + - phy-names: must contain dphy, if the implementation uses an + external D-PHY + +Required subnodes: + - ports: A ports node with endpoint definitions as defined in + Documentation/devicetree/bindings/media/video-interfaces.txt. The + first port subnode should be the input endpoint, the second one the + outputs + + The output port should have as many endpoints as stream supported by + the hardware implementation, between 1 and 4, their ID being the + stream output number used in the implementation. + +Example: + +csi2rx: csi-bridge@0d060000 { + compatible = "cdns,csi2rx"; + reg = <0x0d060000 0x1000>; + clocks = <&byteclock>, <&byteclock>, <&byteclock>, + <&coreclock>, <&coreclock>, + <&coreclock>, <&coreclock>; + clock-names = "sys_clk", "p_clk", "p_free_clk", + "pixel_if0_clk", "pixel_if1_clk", + "pixel_if2_clk", "pixel_if3_clk"; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + #address-cells = <1>; + #size-cells = <0>; + reg = <0>; + + csi2rx_in_sensor: endpoint@0 { + reg = <0>; + remote-endpoint = <&sensor_out_csi2rx>; + clock-lanes = <0>; + data-lanes = <1 2>; + }; + }; + + port@1 { + #address-cells = <1>; + #size-cells = <0>; + reg = <1>; + + csi2rx_out_grabber0: endpoint@0 { + reg = <0>; + remote-endpoint = <&grabber0_in_csi2rx>; + }; + + csi2rx_out_grabber1: endpoint@1 { + reg = <1>; + remote-endpoint = <&grabber1_in_csi2rx>; + }; + + csi2rx_out_grabber2: endpoint@2 { + reg = <2>; + remote-endpoint = <&grabber2_in_csi2rx>; + }; + + csi2rx_out_grabber3: endpoint@3 { + reg = <3>; + remote-endpoint = <&grabber3_in_csi2rx>; + }; + }; + }; +};
The Cadence MIPI-CSI2 RX controller is a CSI2RX bridge that supports up to 4 CSI-2 lanes, and can route the frames to up to 4 streams, depending on the hardware implementation. It can operate with an external D-PHY, an internal one or no D-PHY at all in some configurations. Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> --- .../devicetree/bindings/media/cdns-csi2rx.txt | 87 ++++++++++++++++++++++ 1 file changed, 87 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/cdns-csi2rx.txt