[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8f28e551-eb97-0e1f-6902-cdae13a8ed96@synopsys.com>
Date: Mon, 14 Nov 2016 15:31:21 +0000
From: Ramiro Oliveira <Ramiro.Oliveira@...opsys.com>
To: Laurent Pinchart <laurent.pinchart@...asonboard.com>,
Ramiro Oliveira <Ramiro.Oliveira@...opsys.com>
CC: <robh+dt@...nel.org>, <mark.rutland@....com>, <mchehab@...nel.org>,
<devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<linux-media@...r.kernel.org>, <davem@...emloft.net>,
<gregkh@...uxfoundation.org>, <geert+renesas@...der.be>,
<akpm@...ux-foundation.org>, <linux@...ck-us.net>,
<hverkuil@...all.nl>, <laurent.pinchart+renesas@...asonboard.com>,
<arnd@...db.de>, <sudipm.mukherjee@...il.com>,
<tiffany.lin@...iatek.com>, <minghsiu.tsai@...iatek.com>,
<jean-christophe.trotin@...com>, <andrew-ct.chen@...iatek.com>,
<simon.horman@...ronome.com>, <songjun.wu@...rochip.com>,
<bparrot@...com>, <CARLOS.PALMINHA@...opsys.com>
Subject: Re: [PATCH 1/2] Add Documentation for Media Device, Video Device, and
Synopsys DW MIPI CSI-2 Host
Hi Laurent,
Thanks for the feedback.
On 11/14/2016 2:49 PM, Laurent Pinchart wrote:
> Hi Ramiro,
>
> Thank you for the patch.
>
> On Monday 14 Nov 2016 14:20:22 Ramiro Oliveira wrote:
>> Add documentation for Media and Video Device, as well as the DW MIPI CSI-2
>> Host.
>>
>> Signed-off-by: Ramiro Oliveira <roliveir@...opsys.com>
>> ---
>> .../devicetree/bindings/media/snps,dw-mipi-csi.txt | 27 +++++++++++++++++++
>> .../devicetree/bindings/media/snps,plat-ipk.txt | 9 ++++++++
>> .../bindings/media/snps,video-device.txt | 12 ++++++++++
>> 3 files changed, 48 insertions(+)
>> create mode 100644
>> Documentation/devicetree/bindings/media/snps,dw-mipi-csi.txt create mode
>> 100644 Documentation/devicetree/bindings/media/snps,plat-ipk.txt create
>> mode 100644 Documentation/devicetree/bindings/media/snps,video-device.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 0000000..bec7441
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/snps,dw-mipi-csi.txt
>> @@ -0,0 +1,27 @@
>> +Synopsys DesignWare CSI-2 Host controller
>> +
>> +Description
>> +-----------
>> +
>> +This HW block is used to receive image coming from an MIPI CSI-2 compatible
>> +camera.
>
> And what does it do after receiving the stream ? A more detailed description
> would be useful. Is there any public documentation for this IP core ?
>
I can add a more detailed description. Also, here is a link to the
documentation, but I'm afraid you might have to register yourself to access it.
CSI-2 Host
https://www.synopsys.com/dw/doc.php/ds/c/dwc_csi2_controller.pdf
CSI-2 Host IPK
https://www.synopsys.com/dw/doc.php/ds/o/ip_prototyping_kit_mipi_csi2_host_arc.pdf
>> +Required properties:
>> +- compatible: shall be "snps,dw-mipi-csi"
>> +- reg : physical base address and size of the device memory
> mapped
>> + registers;
>> +- interrupts : CSI-2 Host interrupt
>> +- data-lanes : Number of lanes to be used
>
> Is that fixed at synthesis time or configurable at runtime ?
>
The max number is fixed at synthesis time, but you can configure it for lower
values. I added this option here because, although configurable, it's usually a
fixed value.
>> +- output-type : Core output to be used (IPI-> 0 or IDI->1 or BOTH->2)
>
> What are IPI and IDI ?
>
IPI is Image Pixel Interface and IDI Image Data Interface, these are the two
types of data output support by our CSI-2 Host controller
>> +- phys, phy-names: List of one PHY specifier and identifier string (as
>> defined
>> + in Documentation/devicetree/bindings/phy/phy-bindings.txt).
>
> A PHY for what ?
>
Our controller needs a PHY, in this case a MIPI DPHY, to interact with a CSI-2
receiver (usually a sensor).
>> +Optional properties(if in IPI mode):
>> +- ipi-mode : Mode to be used when in IPI(Camera -> 0 or Automatic -> 1)
>> +- ipi-color-mode: Color depth to be used in IPI (48 bits -> 0 or 16 bits ->
>> 1)
>> +- ipi-auto-flush: Data auto-flush (1 -> Yes or 0 -> No)
>> +- virtual-channel: Virtual channel where data is present when in IPI
>
> We need more details than that, this is impossible to review, sorry.
>
Sure, I'll add more details to the descripton
>> +The per-board settings:
>> + - port sub-node describing a single endpoint connected to the dw-mipi-csi
>> + as described in video-interfaces.txt[1].
>
> An example would be nice.
>
I'll add an example of how we're using it.
>> diff --git a/Documentation/devicetree/bindings/media/snps,plat-ipk.txt
>> b/Documentation/devicetree/bindings/media/snps,plat-ipk.txt new file mode
>> 100644
>> index 0000000..2d51541
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/snps,plat-ipk.txt
>> @@ -0,0 +1,9 @@
>> +Synopsys DesignWare CSI-2 Host IPK Media Device
>> +
>> +This Media Device at the moment is not totally functional, however it is a
>> base
>> +for the future.
>
> Then let's add it later :-) We don't want to design incomplete transient DT
> bindings.
>
I'm afraid I wasn't completely clear. This setup is fully functional. Actually
this sentence made sense in the past, but no longer does now.
>> +Required properties:
>> +
>> +- compatible: Must be "snps,plat-ipk".
>> +
>> diff --git a/Documentation/devicetree/bindings/media/snps,video-device.txt
>> b/Documentation/devicetree/bindings/media/snps,video-device.txt new file
>> mode 100644
>> index 0000000..d467092
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/snps,video-device.txt
>> @@ -0,0 +1,12 @@
>> +Synopsys DesignWare CSI-2 Host video device
>> +
>> +This driver handles all the video handling part of this platform.
>
> This is a DT binding documentation, drivers are irrelevant. You should
> describe the hardware only.
>
> More information is needed, based on this document I can't tell what the
> "CSI-2 host video device" is.
>
You're right, I'll add a more detailed description.
>> +Required properties:
>> +
>> +- compatible: Must be "snps,video-device".
>> +
>> +- dmas, dma-names: List of one DMA specifier and identifier string (as
>> defined
>> + in Documentation/devicetree/bindings/dma/dma.txt) per port. Each port
>> + requires a DMA channel with the identifier string set to "port" followed
>> by
>> + the port index.
>
Thanks once again,
Ramiro Oliveira
Powered by blists - more mailing lists