Hi Laurent,
On 22/08/2017, 02:01, "Laurent Pinchart" <laurent.pinchart@xxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxx>
> >> ---
> >>
> >> .../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
��.n��������+%������w��{.n����z�{��ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
