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@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). 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. -- Regards, Laurent Pinchart