Re: [PATCH v4 06/12] media: dt-bindings: add bindings for i.MX7 media driver

[Sakari Ailus]

On Tue, May 22, 2018 at 02:19:21PM +0100, Rui Miguel Silva wrote:
> Hi Sakari,
> On Fri 18 May 2018 at 22:13, Sakari Ailus wrote:
> > On Fri, May 18, 2018 at 09:27:58AM +0100, Rui Miguel Silva wrote:
> > > > > +endpoint node
> > > > > +-
> > > > > +
> > > > > +- data-lanes: (required) an array specifying active > >
> > > physical
> > > > > MIPI-CSI2
> > > > > + data input lanes and their mapping to > > logical 
> > > > > lanes;
> > > the
> > > > > + array's content is unused, only its > > length is
> > > meaningful;
> > 
> > Btw. do note that you may get a warning due to this from the CSI-2 bus
> > property parsing code if the lane numbers are wrong.
> > 
> > > > > +
> > > > > +- fsl,csis-hs-settle : (optional) differential receiver > >
> > > (HS-RX)
> > > > > settle time;
> > > > > Could you calculate this, as other drivers do? It probably >
> > > changes
> > > > depending on the device runtime configuration.
> > > 
> > > The only reference to possible values to this parameter is given by
> > > table in [0], can you point me out the formula for imx7 in the
> > > documentation?
> > 
> > I don't know imx7 but the other CSI-2 drivers need no such system
> > specific
> > configuration.
> 
> Hum, I think there is at least one more (which this is compliant) that
> also use this configuration parameter. [0]

Ah, that's where it then probably comes from, if the two hardware blocks share 
some bits.

I guess it's fine to keep it if you think it's really necessary.

-- 
Sakari Ailus
sakari.ai...@linux.intel.com

Re: [PATCH v4 06/12] media: dt-bindings: add bindings for i.MX7 media driver

[Rui Miguel Silva]

Hi Sakari,
On Fri 18 May 2018 at 22:13, Sakari Ailus wrote:
On Fri, May 18, 2018 at 09:27:58AM +0100, Rui Miguel Silva 
wrote:

> > +endpoint node
> > +-
> > +
> > +- data-lanes: (required) an array specifying active 
> > physical

> > MIPI-CSI2
> > +		data input lanes and their mapping to 
> > logical lanes; the
> > +		array's content is unused, only its 
> > length is meaningful;


Btw. do note that you may get a warning due to this from the 
CSI-2 bus

property parsing code if the lane numbers are wrong.


> > +
> > +- fsl,csis-hs-settle : (optional) differential receiver 
> > (HS-RX)

> > settle time;
> 
> Could you calculate this, as other drivers do? It probably 
> changes

> depending on the device runtime configuration.

The only reference to possible values to this parameter is 
given by

table in [0], can you point me out the formula for imx7 in the
documentation?


I don't know imx7 but the other CSI-2 drivers need no such 
system specific

configuration.


Hum, I think there is at least one more (which this is compliant) 
that

also use this configuration parameter. [0]

---
Cheers,
Rui

[0]: 
https://github.com/torvalds/linux/blob/a048a07d7f4535baa4cbad6bc024f175317ab938/Documentation/devicetree/bindings/media/samsung-mipi-csis.txt#L46

Re: [PATCH v4 06/12] media: dt-bindings: add bindings for i.MX7 media driver

[Sakari Ailus]

On Fri, May 18, 2018 at 09:27:58AM +0100, Rui Miguel Silva wrote:
> > > +endpoint node
> > > +-
> > > +
> > > +- data-lanes: (required) an array specifying active physical
> > > MIPI-CSI2
> > > + data input lanes and their mapping to logical lanes; the
> > > + array's content is unused, only its length is meaningful;

Btw. do note that you may get a warning due to this from the CSI-2 bus
property parsing code if the lane numbers are wrong.

> > > +
> > > +- fsl,csis-hs-settle : (optional) differential receiver (HS-RX)
> > > settle time;
> > 
> > Could you calculate this, as other drivers do? It probably changes
> > depending on the device runtime configuration.
> 
> The only reference to possible values to this parameter is given by
> table in [0], can you point me out the formula for imx7 in the
> documentation?

I don't know imx7 but the other CSI-2 drivers need no such system specific
configuration.

-- 
Sakari Ailus
sakari.ai...@linux.intel.com

Re: [PATCH v4 06/12] media: dt-bindings: add bindings for i.MX7 media driver

[Rui Miguel Silva]

Hi Sakari,
Thanks for the review.
On Fri 18 May 2018 at 06:58, Sakari Ailus wrote:

Hi Rui,

On Thu, May 17, 2018 at 01:50:27PM +0100, Rui Miguel Silva 
wrote:

Add bindings documentation for i.MX7 media drivers.

Signed-off-by: Rui Miguel Silva 
---
 .../devicetree/bindings/media/imx7.txt| 145 
 ++

 1 file changed, 145 insertions(+)
 create mode 100644 
 Documentation/devicetree/bindings/media/imx7.txt


diff --git a/Documentation/devicetree/bindings/media/imx7.txt 
b/Documentation/devicetree/bindings/media/imx7.txt

new file mode 100644
index ..161cff8e6442
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/imx7.txt
@@ -0,0 +1,145 @@
+Freescale i.MX7 Media Video Device
+==
+
+Video Media Controller node
+---


Note that DT bindings document the hardware, they are as such 
not Linux

dependent.


This was removed in this series, however I removed it in the wrong 
patch,
If you see patch 11/12 you will see this being removed. I will fix 
this

in v5. Thanks for notice it.




+
+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.


Ditto.


+
+Required properties:
+- compatible : "fsl,imx7-capture-subsystem";
+- ports  : Should contain a list of phandles pointing to 
camera

+   sensor interface port of CSI
+
+example:
+
+capture-subsystem {


What's the purpose of this node, if you only refer to another 
device? This

one rather does not look like a real device at all.


+   compatible = "fsl,imx7-capture-subsystem";
+   ports = <>;
+};
+
+
+mipi_csi2 node
+--
+
+This is the device node for the MIPI CSI-2 receiver core in 
i.MX7 SoC. It is

+compatible with previous version of Samsung D-phy.
+
+Required properties:
+
+- compatible: "fsl,imx7-mipi-csi2";
+- reg   : base address and length of the register set 
for the device;

+- interrupts: should contain MIPI CSIS interrupt;
+- clocks: list of clock specifiers, see
+ 
Documentation/devicetree/bindings/clock/clock-bindings.txt for 
details;
+- clock-names   : must contain "pclk", "wrap" and "phy" 
entries, matching

+  entries in the clock property;
+- power-domains : a phandle to the power domain, see
+ 
Documentation/devicetree/bindings/power/power_domain.txt for 
details.

+- reset-names   : should include following entry "mrst";
+- resets: a list of phandle, should contain reset 
entry of

+  reset-names;
+- phy-supply: from the generic phy bindings, a phandle to 
a regulator that

+ provides power to MIPI CSIS core;
+- bus-width : maximum number of data lanes supported (SoC 
specific);

+
+Optional properties:
+
+- clock-frequency : The IP's main (system bus) clock frequency 
in Hz, default
+		value when this property is not specified is 
166 MHz;

+
+port node
+-
+
+- reg		  : (required) can take the values 0 or 1, 
where 0 is the
+ related sink port and port 1 should be 
the source one;

+
+endpoint node
+-
+
+- data-lanes: (required) an array specifying active 
physical MIPI-CSI2
+		data input lanes and their mapping to logical 
lanes; the
+		array's content is unused, only its length is 
meaningful;

+
+- fsl,csis-hs-settle : (optional) differential receiver 
(HS-RX) settle time;


Could you calculate this, as other drivers do? It probably 
changes

depending on the device runtime configuration.


The only reference to possible values to this parameter is given 
by

table in [0], can you point me out the formula for imx7 in the
documentation?

---
Cheers,
Rui

[0] https://community.nxp.com/thread/463777

Re: [PATCH v4 06/12] media: dt-bindings: add bindings for i.MX7 media driver

[Sakari Ailus]

Hi Rui,

On Thu, May 17, 2018 at 01:50:27PM +0100, Rui Miguel Silva wrote:
> Add bindings documentation for i.MX7 media drivers.
> 
> Signed-off-by: Rui Miguel Silva 
> ---
>  .../devicetree/bindings/media/imx7.txt| 145 ++
>  1 file changed, 145 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/imx7.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/imx7.txt 
> b/Documentation/devicetree/bindings/media/imx7.txt
> new file mode 100644
> index ..161cff8e6442
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/imx7.txt
> @@ -0,0 +1,145 @@
> +Freescale i.MX7 Media Video Device
> +==
> +
> +Video Media Controller node
> +---

Note that DT bindings document the hardware, they are as such not Linux
dependent.

> +
> +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.

Ditto.

> +
> +Required properties:
> +- compatible : "fsl,imx7-capture-subsystem";
> +- ports  : Should contain a list of phandles pointing to camera
> + sensor interface port of CSI
> +
> +example:
> +
> +capture-subsystem {

What's the purpose of this node, if you only refer to another device? This
one rather does not look like a real device at all.

> + compatible = "fsl,imx7-capture-subsystem";
> + ports = <>;
> +};
> +
> +
> +mipi_csi2 node
> +--
> +
> +This is the device node for the MIPI CSI-2 receiver core in i.MX7 SoC. It is
> +compatible with previous version of Samsung D-phy.
> +
> +Required properties:
> +
> +- compatible: "fsl,imx7-mipi-csi2";
> +- reg   : base address and length of the register set for the device;
> +- interrupts: should contain MIPI CSIS interrupt;
> +- clocks: list of clock specifiers, see
> +Documentation/devicetree/bindings/clock/clock-bindings.txt for 
> details;
> +- clock-names   : must contain "pclk", "wrap" and "phy" entries, matching
> +  entries in the clock property;
> +- power-domains : a phandle to the power domain, see
> +  Documentation/devicetree/bindings/power/power_domain.txt for 
> details.
> +- reset-names   : should include following entry "mrst";
> +- resets: a list of phandle, should contain reset entry of
> +  reset-names;
> +- phy-supply: from the generic phy bindings, a phandle to a regulator 
> that
> +   provides power to MIPI CSIS core;
> +- bus-width : maximum number of data lanes supported (SoC specific);
> +
> +Optional properties:
> +
> +- clock-frequency : The IP's main (system bus) clock frequency in Hz, default
> + value when this property is not specified is 166 MHz;
> +
> +port node
> +-
> +
> +- reg  : (required) can take the values 0 or 1, where 0 is 
> the
> + related sink port and port 1 should be the source one;
> +
> +endpoint node
> +-
> +
> +- data-lanes: (required) an array specifying active physical MIPI-CSI2
> + data input lanes and their mapping to logical lanes; the
> + array's content is unused, only its length is meaningful;
> +
> +- fsl,csis-hs-settle : (optional) differential receiver (HS-RX) settle time;

Could you calculate this, as other drivers do? It probably changes
depending on the device runtime configuration.

> +
> +example:
> +
> +mipi_csi: mipi-csi@3075 {
> +#address-cells = <1>;
> +#size-cells = <0>;
> +
> +compatible = "fsl,imx7-mipi-csi2";
> +reg = <0x3075 0x1>;
> +interrupts = ;
> +clocks = < IMX7D_IPG_ROOT_CLK>,
> +< IMX7D_MIPI_CSI_ROOT_CLK>,
> +< IMX7D_MIPI_DPHY_ROOT_CLK>;
> +clock-names = "pclk", "wrap", "phy";
> +clock-names = "mipi", "phy";
> +clock-frequency = <16600>;
> +power-domains = <_mipi_phy>;
> +phy-supply = <_1p0d>;
> +resets = < IMX7_RESET_MIPI_PHY_MRST>;
> +reset-names = "mrst";
> +bus-width = <4>;
> +fsl,csis-hs-settle = <3>;
> +fsl,csis-clk-settle = <0>;
> +
> +port@0 {
> +reg = <0>;
> +
> +mipi_from_sensor: endpoint {
> +remote-endpoint = <_to_mipi>;
> +data-lanes = <1>;
> +};
> +};
> +
> +port@1 {
> +reg = <1>;
> +
> +mipi_vc0_to_csi_mux: endpoint {
> +remote-endpoint = <_mux_from_mipi_vc0>;
> +   

[PATCH v4 06/12] media: dt-bindings: add bindings for i.MX7 media driver

[Rui Miguel Silva]

Add bindings documentation for i.MX7 media drivers.

Signed-off-by: Rui Miguel Silva 
---
 .../devicetree/bindings/media/imx7.txt| 145 ++
 1 file changed, 145 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/imx7.txt

diff --git a/Documentation/devicetree/bindings/media/imx7.txt 
b/Documentation/devicetree/bindings/media/imx7.txt
new file mode 100644
index ..161cff8e6442
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/imx7.txt
@@ -0,0 +1,145 @@
+Freescale i.MX7 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,imx7-capture-subsystem";
+- ports  : Should contain a list of phandles pointing to camera
+   sensor interface port of CSI
+
+example:
+
+capture-subsystem {
+   compatible = "fsl,imx7-capture-subsystem";
+   ports = <>;
+};
+
+
+mipi_csi2 node
+--
+
+This is the device node for the MIPI CSI-2 receiver core in i.MX7 SoC. It is
+compatible with previous version of Samsung D-phy.
+
+Required properties:
+
+- compatible: "fsl,imx7-mipi-csi2";
+- reg   : base address and length of the register set for the device;
+- interrupts: should contain MIPI CSIS interrupt;
+- clocks: list of clock specifiers, see
+Documentation/devicetree/bindings/clock/clock-bindings.txt for details;
+- clock-names   : must contain "pclk", "wrap" and "phy" entries, matching
+  entries in the clock property;
+- power-domains : a phandle to the power domain, see
+  Documentation/devicetree/bindings/power/power_domain.txt for details.
+- reset-names   : should include following entry "mrst";
+- resets: a list of phandle, should contain reset entry of
+  reset-names;
+- phy-supply: from the generic phy bindings, a phandle to a regulator that
+ provides power to MIPI CSIS core;
+- bus-width : maximum number of data lanes supported (SoC specific);
+
+Optional properties:
+
+- clock-frequency : The IP's main (system bus) clock frequency in Hz, default
+   value when this property is not specified is 166 MHz;
+
+port node
+-
+
+- reg: (required) can take the values 0 or 1, where 0 is the
+ related sink port and port 1 should be the source one;
+
+endpoint node
+-
+
+- data-lanes: (required) an array specifying active physical MIPI-CSI2
+   data input lanes and their mapping to logical lanes; the
+   array's content is unused, only its length is meaningful;
+
+- fsl,csis-hs-settle : (optional) differential receiver (HS-RX) settle time;
+
+example:
+
+mipi_csi: mipi-csi@3075 {
+#address-cells = <1>;
+#size-cells = <0>;
+
+compatible = "fsl,imx7-mipi-csi2";
+reg = <0x3075 0x1>;
+interrupts = ;
+clocks = < IMX7D_IPG_ROOT_CLK>,
+< IMX7D_MIPI_CSI_ROOT_CLK>,
+< IMX7D_MIPI_DPHY_ROOT_CLK>;
+clock-names = "pclk", "wrap", "phy";
+clock-names = "mipi", "phy";
+clock-frequency = <16600>;
+power-domains = <_mipi_phy>;
+phy-supply = <_1p0d>;
+resets = < IMX7_RESET_MIPI_PHY_MRST>;
+reset-names = "mrst";
+bus-width = <4>;
+fsl,csis-hs-settle = <3>;
+fsl,csis-clk-settle = <0>;
+
+port@0 {
+reg = <0>;
+
+mipi_from_sensor: endpoint {
+remote-endpoint = <_to_mipi>;
+data-lanes = <1>;
+};
+};
+
+port@1 {
+reg = <1>;
+
+mipi_vc0_to_csi_mux: endpoint {
+remote-endpoint = <_mux_from_mipi_vc0>;
+};
+};
+};
+
+
+csi node
+
+
+This is device node for the CMOS Sensor Interface (CSI) which enables the chip
+to connect directly to external CMOS image sensors.
+
+Required properties:
+
+- compatible: "fsl,imx7-csi";
+- reg   : base address and length of the register set for the device;
+- interrupts: should contain CSI interrupt;
+- clocks: list of clock specifiers, see
+Documentation/devicetree/bindings/clock/clock-bindings.txt for details;
+- clock-names   : must contain "axi", "mclk" and "dcic" entries, matching
+ entries in the clock property;
+
+example:
+
+