Re: [PATCH v6 02/39] [media] dt-bindings: Add bindings for i.MX media driver

2017-04-03 Thread Russell King - ARM Linux 
On Mon, Apr 03, 2017 at 09:07:43AM -0500, Rob Herring wrote:
> On Tue, Mar 28, 2017 at 05:35:52PM -0700, Steve Longerbeam wrote:
> > I assume if there's another binding doc in progress, it means
> > someone is working on another Synopsys DW CSI-2 subdevice driver.
> 
> Yes. see http://patchwork.ozlabs.org/patch/736177/
> 
> > Unfortunately I don't have the time to contribute and switch to
> > this other subdevice, and do test/debug.
> 
> >From a DT perspective, I'm not asking that you share the subdevice 
> driver, only the binding. Simply put, there's 1 h/w block here, so there 
> should only be 1 binding. The binding is an ABI, so you can't just merge 
> it and change it later.

I think it would be nice to have some kind of standard base binding
for CSI2 interfaces, but beyond the standard compatible/reg/interrupts
and graph properties, I'm not sure what it would look like.

As far as those properties go, the iMX6 version does better than the
DW version, because we specify the full graph, whereas the DW version
only specifies the downstream link.  Once that's done, there's some
properties (like those specifying the output configuration) which
probably ought to be moved to the graph links instead, once they exist.

So, if anything, I think it's the DW version needs to be augmented with
fuller information, and some of the properties moved.

Also, as I've mentioned in my other reply, while they may both appear
to be called "Synopsys DW CSI-2" devices, they appear to be quite
different from the hardware perspective.

The rest 

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH v6 02/39] [media] dt-bindings: Add bindings for i.MX media driver

2017-04-03 Thread Russell King - ARM Linux 
On Mon, Apr 03, 2017 at 09:11:35AM -0500, Rob Herring wrote:
> On Wed, Mar 29, 2017 at 09:39:05AM +0100, Russell King - ARM Linux wrote:
> > On Tue, Mar 28, 2017 at 07:21:34PM -0500, Rob Herring wrote:
> > > On Mon, Mar 27, 2017 at 7:40 PM, Steve Longerbeam  
> > > wrote:
> > > > Add bindings documentation for the i.MX media driver.
> > > >
> > > > Signed-off-by: Steve Longerbeam 
> > > > ---
> > > >  Documentation/devicetree/bindings/media/imx.txt | 74 
> > > > +
> > > >  1 file changed, 74 insertions(+)
> > > >  create mode 100644 Documentation/devicetree/bindings/media/imx.txt
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/media/imx.txt 
> > > > b/Documentation/devicetree/bindings/media/imx.txt
> > > > new file mode 100644
> > > > index 000..3059c06
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/media/imx.txt
> > > > @@ -0,0 +1,74 @@
> > > > +Freescale i.MX Media Video Device
> > > > +=
> > > > +
> > > > +Video Media Controller node
> > > > +---
> > > > +
> > > > +This is the media controller node for video capture support. It is a
> > > > +virtual device that lists the camera serial interface nodes that the
> > > > +media device will control.
> > > > +
> > > > +Required properties:
> > > > +- compatible : "fsl,imx-capture-subsystem";
> > > > +- ports  : Should contain a list of phandles pointing to camera
> > > > +   sensor interface ports of IPU devices
> > > > +
> > > > +example:
> > > > +
> > > > +capture-subsystem {
> > > > +   compatible = "fsl,imx-capture-subsystem";
> > > > +   ports = <_csi0>, <_csi1>;
> > > > +};
> > > > +
> > > > +fim child node
> > > > +--
> > > > +
> > > > +This is an optional child node of the ipu_csi port nodes. If present 
> > > > and
> > > > +available, it enables the Frame Interval Monitor. Its properties can be
> > > > +used to modify the method in which the FIM measures frame intervals.
> > > > +Refer to Documentation/media/v4l-drivers/imx.rst for more info on the
> > > > +Frame Interval Monitor.
> > > > +
> > > > +Optional properties:
> > > > +- fsl,input-capture-channel: an input capture channel and channel 
> > > > flags,
> > > > +specified as . The channel 
> > > > number
> > > > +must be 0 or 1. The flags can be
> > > > +IRQ_TYPE_EDGE_RISING, 
> > > > IRQ_TYPE_EDGE_FALLING, or
> > > > +IRQ_TYPE_EDGE_BOTH, and specify which input
> > > > +capture signal edge will trigger the input
> > > > +capture event. If an input capture channel 
> > > > is
> > > > +specified, the FIM will use this method to
> > > > +measure frame intervals instead of via the 
> > > > EOF
> > > > +interrupt. The input capture method is much
> > > > +preferred over EOF as it is not subject to
> > > > +interrupt latency errors. However it 
> > > > requires
> > > > +routing the VSYNC or FIELD output signals 
> > > > of
> > > > +the camera sensor to one of the i.MX input
> > > > +capture pads (SD1_DAT0, SD1_DAT1), which 
> > > > also
> > > > +gives up support for SD1.
> > > > +
> > > > +
> > > > +mipi_csi2 node
> > > > +--
> > > > +
> > > > +This is the device node for the MIPI CSI-2 Receiver, required for MIPI
> > > > +CSI-2 sensors.
> > > > +
> > > > +Required properties:
> > > > +- compatible   : "fsl,imx6-mipi-csi2", "snps,dw-mipi-csi2";
> > > 
> > > As I mentioned in v5, there's a DW CSI2 binding in progress. This
> > > needs to be based on that.
> > 
> > Maybe someone can provide some kind of reference to it, and it's
> > associated driver?
> 
> Let me Google that for you (TM). The reference is in my comments on v5. 
> Here's a reference to it [1].

Looking at the actual driver, it seems to at least have a different
register layout:

registerimx6dw
version 0x000   0x000
n_lanes 0x004   0x004
phy_shutdownz   0x008   -
dphy_resetz 0x00c   -
resetn  0x010   0x008
phy_state   0x014   -
data_ids_1  0x018   0x010
data_ids_2  0x01c   0x014
err10x020   -
err20x024   -
msk10x028   -
msk20x02c   -
phy_tst_ctrl0   0x030   -
phy_tst_ctrl1   0x034   -
sft_reset   0xf00   -   (not part of CSI2, but a IMX6 specific
CSI2 to IPU gasket layer, but lives in
CSI2's register region)

The DW version has many more registers than are documented by the iMX6
version.  Only the first two registers appear to be common

Re: [PATCH v6 02/39] [media] dt-bindings: Add bindings for i.MX media driver

2017-04-03 Thread Rob Herring 
On Wed, Mar 29, 2017 at 09:39:05AM +0100, Russell King - ARM Linux wrote:
> On Tue, Mar 28, 2017 at 07:21:34PM -0500, Rob Herring wrote:
> > On Mon, Mar 27, 2017 at 7:40 PM, Steve Longerbeam  
> > wrote:
> > > Add bindings documentation for the i.MX media driver.
> > >
> > > Signed-off-by: Steve Longerbeam 
> > > ---
> > >  Documentation/devicetree/bindings/media/imx.txt | 74 
> > > +
> > >  1 file changed, 74 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/media/imx.txt
> > >
> > > diff --git a/Documentation/devicetree/bindings/media/imx.txt 
> > > b/Documentation/devicetree/bindings/media/imx.txt
> > > new file mode 100644
> > > index 000..3059c06
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/media/imx.txt
> > > @@ -0,0 +1,74 @@
> > > +Freescale i.MX Media Video Device
> > > +=
> > > +
> > > +Video Media Controller node
> > > +---
> > > +
> > > +This is the media controller node for video capture support. It is a
> > > +virtual device that lists the camera serial interface nodes that the
> > > +media device will control.
> > > +
> > > +Required properties:
> > > +- compatible : "fsl,imx-capture-subsystem";
> > > +- ports  : Should contain a list of phandles pointing to camera
> > > +   sensor interface ports of IPU devices
> > > +
> > > +example:
> > > +
> > > +capture-subsystem {
> > > +   compatible = "fsl,imx-capture-subsystem";
> > > +   ports = <_csi0>, <_csi1>;
> > > +};
> > > +
> > > +fim child node
> > > +--
> > > +
> > > +This is an optional child node of the ipu_csi port nodes. If present and
> > > +available, it enables the Frame Interval Monitor. Its properties can be
> > > +used to modify the method in which the FIM measures frame intervals.
> > > +Refer to Documentation/media/v4l-drivers/imx.rst for more info on the
> > > +Frame Interval Monitor.
> > > +
> > > +Optional properties:
> > > +- fsl,input-capture-channel: an input capture channel and channel flags,
> > > +specified as . The channel number
> > > +must be 0 or 1. The flags can be
> > > +IRQ_TYPE_EDGE_RISING, IRQ_TYPE_EDGE_FALLING, 
> > > or
> > > +IRQ_TYPE_EDGE_BOTH, and specify which input
> > > +capture signal edge will trigger the input
> > > +capture event. If an input capture channel is
> > > +specified, the FIM will use this method to
> > > +measure frame intervals instead of via the 
> > > EOF
> > > +interrupt. The input capture method is much
> > > +preferred over EOF as it is not subject to
> > > +interrupt latency errors. However it requires
> > > +routing the VSYNC or FIELD output signals of
> > > +the camera sensor to one of the i.MX input
> > > +capture pads (SD1_DAT0, SD1_DAT1), which also
> > > +gives up support for SD1.
> > > +
> > > +
> > > +mipi_csi2 node
> > > +--
> > > +
> > > +This is the device node for the MIPI CSI-2 Receiver, required for MIPI
> > > +CSI-2 sensors.
> > > +
> > > +Required properties:
> > > +- compatible   : "fsl,imx6-mipi-csi2", "snps,dw-mipi-csi2";
> > 
> > As I mentioned in v5, there's a DW CSI2 binding in progress. This
> > needs to be based on that.
> 
> Maybe someone can provide some kind of reference to it, and it's
> associated driver?

Let me Google that for you (TM). The reference is in my comments on v5. 
Here's a reference to it [1].

[1] https://lkml.org/lkml/2017/3/20/524

> 
> -- 
> RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
> according to speedtest.net.

Re: [PATCH v6 02/39] [media] dt-bindings: Add bindings for i.MX media driver

2017-04-03 Thread Rob Herring 
On Tue, Mar 28, 2017 at 05:35:52PM -0700, Steve Longerbeam wrote:
> 
> 
> On 03/28/2017 05:21 PM, Rob Herring wrote:
> > On Mon, Mar 27, 2017 at 7:40 PM, Steve Longerbeam  
> > wrote:
> > > Add bindings documentation for the i.MX media driver.
> > > 
> > > 
> > > +
> > > +mipi_csi2 node
> > > +--
> > > +
> > > +This is the device node for the MIPI CSI-2 Receiver, required for MIPI
> > > +CSI-2 sensors.
> > > +
> > > +Required properties:
> > > +- compatible   : "fsl,imx6-mipi-csi2", "snps,dw-mipi-csi2";
> > 
> > As I mentioned in v5, there's a DW CSI2 binding in progress. This
> > needs to be based on that.
> 
> Hi Rob, I'm not sure what you are asking me to do.
> 
> I assume if there's another binding doc in progress, it means
> someone is working on another Synopsys DW CSI-2 subdevice driver.

Yes. see http://patchwork.ozlabs.org/patch/736177/

> Unfortunately I don't have the time to contribute and switch to
> this other subdevice, and do test/debug.

>From a DT perspective, I'm not asking that you share the subdevice 
driver, only the binding. Simply put, there's 1 h/w block here, so there 
should only be 1 binding. The binding is an ABI, so you can't just merge 
it and change it later.

The driver side is a decision for the V4L2 maintainers.

> For now I would prefer if this patchset is merged as is, and
> then contribute/switch to another CSI-2 subdev later. It is
> also getting very difficult managing all these patches (39 as
> of this version), and I'd prefer not to spam the lists with
> such large patchsets for too much longer.

Then maybe you should figure out how to split up the series. I've not 
looked at it to provide suggestions.

Rob

Re: [PATCH v6 02/39] [media] dt-bindings: Add bindings for i.MX media driver

2017-03-29 Thread Russell King - ARM Linux 
On Tue, Mar 28, 2017 at 07:21:34PM -0500, Rob Herring wrote:
> On Mon, Mar 27, 2017 at 7:40 PM, Steve Longerbeam  
> wrote:
> > Add bindings documentation for the i.MX media driver.
> >
> > Signed-off-by: Steve Longerbeam 
> > ---
> >  Documentation/devicetree/bindings/media/imx.txt | 74 
> > +
> >  1 file changed, 74 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/media/imx.txt
> >
> > diff --git a/Documentation/devicetree/bindings/media/imx.txt 
> > b/Documentation/devicetree/bindings/media/imx.txt
> > new file mode 100644
> > index 000..3059c06
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/imx.txt
> > @@ -0,0 +1,74 @@
> > +Freescale i.MX Media Video Device
> > +=
> > +
> > +Video Media Controller node
> > +---
> > +
> > +This is the media controller node for video capture support. It is a
> > +virtual device that lists the camera serial interface nodes that the
> > +media device will control.
> > +
> > +Required properties:
> > +- compatible : "fsl,imx-capture-subsystem";
> > +- ports  : Should contain a list of phandles pointing to camera
> > +   sensor interface ports of IPU devices
> > +
> > +example:
> > +
> > +capture-subsystem {
> > +   compatible = "fsl,imx-capture-subsystem";
> > +   ports = <_csi0>, <_csi1>;
> > +};
> > +
> > +fim child node
> > +--
> > +
> > +This is an optional child node of the ipu_csi port nodes. If present and
> > +available, it enables the Frame Interval Monitor. Its properties can be
> > +used to modify the method in which the FIM measures frame intervals.
> > +Refer to Documentation/media/v4l-drivers/imx.rst for more info on the
> > +Frame Interval Monitor.
> > +
> > +Optional properties:
> > +- fsl,input-capture-channel: an input capture channel and channel flags,
> > +specified as . The channel number
> > +must be 0 or 1. The flags can be
> > +IRQ_TYPE_EDGE_RISING, IRQ_TYPE_EDGE_FALLING, or
> > +IRQ_TYPE_EDGE_BOTH, and specify which input
> > +capture signal edge will trigger the input
> > +capture event. If an input capture channel is
> > +specified, the FIM will use this method to
> > +measure frame intervals instead of via the EOF
> > +interrupt. The input capture method is much
> > +preferred over EOF as it is not subject to
> > +interrupt latency errors. However it requires
> > +routing the VSYNC or FIELD output signals of
> > +the camera sensor to one of the i.MX input
> > +capture pads (SD1_DAT0, SD1_DAT1), which also
> > +gives up support for SD1.
> > +
> > +
> > +mipi_csi2 node
> > +--
> > +
> > +This is the device node for the MIPI CSI-2 Receiver, required for MIPI
> > +CSI-2 sensors.
> > +
> > +Required properties:
> > +- compatible   : "fsl,imx6-mipi-csi2", "snps,dw-mipi-csi2";
> 
> As I mentioned in v5, there's a DW CSI2 binding in progress. This
> needs to be based on that.

Maybe someone can provide some kind of reference to it, and it's
associated driver?

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH v6 02/39] [media] dt-bindings: Add bindings for i.MX media driver

2017-03-28 Thread Steve Longerbeam 



On 03/28/2017 05:21 PM, Rob Herring wrote:

On Mon, Mar 27, 2017 at 7:40 PM, Steve Longerbeam  wrote:

Add bindings documentation for the i.MX media driver.


+
+mipi_csi2 node
+--
+
+This is the device node for the MIPI CSI-2 Receiver, required for MIPI
+CSI-2 sensors.
+
+Required properties:
+- compatible   : "fsl,imx6-mipi-csi2", "snps,dw-mipi-csi2";


As I mentioned in v5, there's a DW CSI2 binding in progress. This
needs to be based on that.


Hi Rob, I'm not sure what you are asking me to do.

I assume if there's another binding doc in progress, it means
someone is working on another Synopsys DW CSI-2 subdevice driver.

Unfortunately I don't have the time to contribute and switch to
this other subdevice, and do test/debug.

For now I would prefer if this patchset is merged as is, and
then contribute/switch to another CSI-2 subdev later. It is
also getting very difficult managing all these patches (39 as
of this version), and I'd prefer not to spam the lists with
such large patchsets for too much longer.


Steve

Re: [PATCH v6 02/39] [media] dt-bindings: Add bindings for i.MX media driver

2017-03-28 Thread Rob Herring 
On Mon, Mar 27, 2017 at 7:40 PM, Steve Longerbeam  wrote:
> Add bindings documentation for the i.MX media driver.
>
> Signed-off-by: Steve Longerbeam 
> ---
>  Documentation/devicetree/bindings/media/imx.txt | 74 
> +
>  1 file changed, 74 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/imx.txt
>
> diff --git a/Documentation/devicetree/bindings/media/imx.txt 
> b/Documentation/devicetree/bindings/media/imx.txt
> new file mode 100644
> index 000..3059c06
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/imx.txt
> @@ -0,0 +1,74 @@
> +Freescale i.MX Media Video Device
> +=
> +
> +Video Media Controller node
> +---
> +
> +This is the media controller node for video capture support. It is a
> +virtual device that lists the camera serial interface nodes that the
> +media device will control.
> +
> +Required properties:
> +- compatible : "fsl,imx-capture-subsystem";
> +- ports  : Should contain a list of phandles pointing to camera
> +   sensor interface ports of IPU devices
> +
> +example:
> +
> +capture-subsystem {
> +   compatible = "fsl,imx-capture-subsystem";
> +   ports = <_csi0>, <_csi1>;
> +};
> +
> +fim child node
> +--
> +
> +This is an optional child node of the ipu_csi port nodes. If present and
> +available, it enables the Frame Interval Monitor. Its properties can be
> +used to modify the method in which the FIM measures frame intervals.
> +Refer to Documentation/media/v4l-drivers/imx.rst for more info on the
> +Frame Interval Monitor.
> +
> +Optional properties:
> +- fsl,input-capture-channel: an input capture channel and channel flags,
> +specified as . The channel number
> +must be 0 or 1. The flags can be
> +IRQ_TYPE_EDGE_RISING, IRQ_TYPE_EDGE_FALLING, or
> +IRQ_TYPE_EDGE_BOTH, and specify which input
> +capture signal edge will trigger the input
> +capture event. If an input capture channel is
> +specified, the FIM will use this method to
> +measure frame intervals instead of via the EOF
> +interrupt. The input capture method is much
> +preferred over EOF as it is not subject to
> +interrupt latency errors. However it requires
> +routing the VSYNC or FIELD output signals of
> +the camera sensor to one of the i.MX input
> +capture pads (SD1_DAT0, SD1_DAT1), which also
> +gives up support for SD1.
> +
> +
> +mipi_csi2 node
> +--
> +
> +This is the device node for the MIPI CSI-2 Receiver, required for MIPI
> +CSI-2 sensors.
> +
> +Required properties:
> +- compatible   : "fsl,imx6-mipi-csi2", "snps,dw-mipi-csi2";

As I mentioned in v5, there's a DW CSI2 binding in progress. This
needs to be based on that.

Rob

[PATCH v6 02/39] [media] dt-bindings: Add bindings for i.MX media driver

2017-03-27 Thread Steve Longerbeam 
Add bindings documentation for the i.MX media driver.

Signed-off-by: Steve Longerbeam 
---
 Documentation/devicetree/bindings/media/imx.txt | 74 +
 1 file changed, 74 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/imx.txt

diff --git a/Documentation/devicetree/bindings/media/imx.txt 
b/Documentation/devicetree/bindings/media/imx.txt
new file mode 100644
index 000..3059c06
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/imx.txt
@@ -0,0 +1,74 @@
+Freescale i.MX Media Video Device
+=
+
+Video Media Controller node
+---
+
+This is the media controller node for video capture support. It is a
+virtual device that lists the camera serial interface nodes that the
+media device will control.
+
+Required properties:
+- compatible : "fsl,imx-capture-subsystem";
+- ports  : Should contain a list of phandles pointing to camera
+   sensor interface ports of IPU devices
+
+example:
+
+capture-subsystem {
+   compatible = "fsl,imx-capture-subsystem";
+   ports = <_csi0>, <_csi1>;
+};
+
+fim child node
+--
+
+This is an optional child node of the ipu_csi port nodes. If present and
+available, it enables the Frame Interval Monitor. Its properties can be
+used to modify the method in which the FIM measures frame intervals.
+Refer to Documentation/media/v4l-drivers/imx.rst for more info on the
+Frame Interval Monitor.
+
+Optional properties:
+- fsl,input-capture-channel: an input capture channel and channel flags,
+specified as . The channel number
+must be 0 or 1. The flags can be
+IRQ_TYPE_EDGE_RISING, IRQ_TYPE_EDGE_FALLING, or
+IRQ_TYPE_EDGE_BOTH, and specify which input
+capture signal edge will trigger the input
+capture event. If an input capture channel is
+specified, the FIM will use this method to
+measure frame intervals instead of via the EOF
+interrupt. The input capture method is much
+preferred over EOF as it is not subject to
+interrupt latency errors. However it requires
+routing the VSYNC or FIELD output signals of
+the camera sensor to one of the i.MX input
+capture pads (SD1_DAT0, SD1_DAT1), which also
+gives up support for SD1.
+
+
+mipi_csi2 node
+--
+
+This is the device node for the MIPI CSI-2 Receiver, required for MIPI
+CSI-2 sensors.
+
+Required properties:
+- compatible   : "fsl,imx6-mipi-csi2", "snps,dw-mipi-csi2";
+- reg   : physical base address and length of the register set;
+- clocks   : the MIPI CSI-2 receiver requires three clocks: hsi_tx
+ (the D-PHY clock), video_27m (D-PHY PLL reference
+ clock), and eim_podf;
+- clock-names  : must contain "dphy", "ref", "pix";
+- port@*: five port nodes must exist, containing endpoints
+ connecting to the source and sink devices according to
+ of_graph bindings. The first port is an input port,
+ connecting with a MIPI CSI-2 source, and ports 1
+ through 4 are output ports connecting with parallel
+ bus sink endpoint nodes and correspond to the four
+ MIPI CSI-2 virtual channel outputs.
+
+Optional properties:
+- interrupts   : must contain two level-triggered interrupts,
+ in order: 100 and 101;
-- 
2.7.4