Hi Rob, Thank you for your feedback and sorry for the late reply.
On 3/15/2017 6:26 PM, Rob Herring wrote: > On Tue, Mar 07, 2017 at 02:37:48PM +0000, Ramiro Oliveira wrote: >> Create device tree bindings documentation for the Synopsys DW MIPI CSI-2 >> Host. >> >> Signed-off-by: Ramiro Oliveira <roliv...@synopsys.com> >> --- >> .../devicetree/bindings/media/snps,dw-mipi-csi.txt | 37 >> ++++++++++++++++++++++ >> 1 file changed, 37 insertions(+) >> create mode 100644 >> Documentation/devicetree/bindings/media/snps,dw-mipi-csi.txt >> >> diff --git a/Documentation/devicetree/bindings/media/snps,dw-mipi-csi.txt >> b/Documentation/devicetree/bindings/media/snps,dw-mipi-csi.txt >> new file mode 100644 >> index 000000000000..5b24eb43d760 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/media/snps,dw-mipi-csi.txt >> @@ -0,0 +1,37 @@ >> +Synopsys DesignWare CSI-2 Host controller >> + >> +Description >> +----------- >> + >> +This HW block is used to receive image coming from an MIPI CSI-2 compatible >> +camera. >> + >> +Required properties: >> +- compatible: shall be "snps,dw-mipi-csi" > > ... "and an SoC specific compatible string" > I'm not sure what you mean by this. >> +- reg : physical base address and size of the device memory >> mapped >> + registers; >> +- interrupts : CSI-2 Host interrupt >> +- output-type : Core output to be used (IPI-> 0 or IDI->1 or BOTH->2) >> These >> + values choose which of the Core outputs will be used, it can be Image Data >> + Interface or Image Pixel Interface. >> +- phys: List of one PHY specifier (as defined in >> + Documentation/devicetree/bindings/phy/phy-bindings.txt). This PHY is a >> MIPI >> + DPHY working in RX mode. >> +- resets: Reference to a reset controller (optional) >> + >> +Optional properties(if in IPI mode): >> +- ipi-mode : Mode to be used when in IPI(Camera -> 0 or Controller -> 1) >> + This property defines if the controller will use the video timings >> available >> + in the video stream or if it will use pre-defined ones. > > This could be boolean instead. I'd expect the former to be the typical > mode, so name the property for the latter. > Sure. I can do this. >> +- ipi-color-mode: Bus depth to be used in IPI (48 bits -> 0 or 16 bits -> 1) >> + This property defines the width of the IPI bus. > > Are these the only 2 modes that any h/w would ever have (not just your > controller)? Perhaps using the actual bits for the value would be > better. Also, if this is the bus width, then the property should be > named that as bus widths and pixel formats or color modes are not > necessarily the same. > I'm not sure I understand what you mean, but I'll try to explain what this does. Our controller, when in IPI mode, has a data bus that can be either 16 or 48 bits wide. However when in 48 bits mode some data types will not use the entire bus width. >> +- ipi-auto-flush: Data auto-flush (1 -> Yes or 0 -> No). This property >> defines >> + if the data is automatically flushed in each vsync or if this process is >> done >> + manually > > This could be boolean. > I'll change it. >> +- virtual-channel: Virtual channel where data is present when in IPI mode. >> This >> + property chooses the virtual channel which IPI will use to retrieve the >> video >> + stream. > > Again, some of these should probably be common properties (and therefore > documented in a common location). I'm looking for some feedback from > video/camera maintainers on this. I know I've seen some other similar > bindings for camera interfaces recently. > You're right, Virtual channel is MIPI CSI-2 property, not a Synopsys one, so this could be a common property, although I haven't found it anywhere. > Rob > -- Best Regards Ramiro Oliveira ramiro.olive...@synopsys.com
