Message-ID: <20150206081318.GA15088@victor>
Date:	Fri, 6 Feb 2015 16:13:20 +0800
From:	Liu Ying <Ying.Liu@...escale.com>
To:	Philipp Zabel <p.zabel@...gutronix.de>
CC:	<dri-devel@...ts.freedesktop.org>, <stefan.wahren@...e.com>,
	<devicetree@...r.kernel.org>, <linux@....linux.org.uk>,
	<kernel@...gutronix.de>, <airlied@...ux.ie>,
	<linux-kernel@...r.kernel.org>, <a.hajda@...sung.com>,
	<thierry.reding@...il.com>, <mturquette@...aro.org>,
	<shawn.guo@...aro.org>, <linux-arm-kernel@...ts.infradead.org>,
	<andyshrk@...il.com>
Subject: Re: [PATCH RFC v8 11/21] Documentation: dt-bindings: Add bindings
 for Synopsys DW MIPI DSI DRM bridge driver

On Thu, Feb 05, 2015 at 11:10:04AM +0100, Philipp Zabel wrote:
> Am Mittwoch, den 31.12.2014, 16:23 +0800 schrieb Liu Ying:
> > This patch adds device tree bindings for Synopsys DesignWare MIPI DSI
> > host controller DRM bridge driver.
> > 
> > Signed-off-by: Liu Ying <Ying.Liu@...escale.com>
> > ---
> > v7->v8:
> >  * None.
> > 
> > v6->v7:
> >  * None.
> > 
> > v5->v6:
> >  * Add the #address-cells and #size-cells properties in the example 'ports'
> >    node.
> >  * Remove the useless input-port properties from the example port@0 and port@1
> >    nodes.
> > 
> > v4->v5:
> >  * None.
> > 
> > v3->v4:
> >  * Newly introduced in v4.  This is separated from the relevant driver patch
> >    in v3 to address Stefan Wahren's comment.
> > 
> >  .../devicetree/bindings/drm/bridge/dw_mipi_dsi.txt | 73 ++++++++++++++++++++++
> >  1 file changed, 73 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/drm/bridge/dw_mipi_dsi.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/drm/bridge/dw_mipi_dsi.txt b/Documentation/devicetree/bindings/drm/bridge/dw_mipi_dsi.txt
> > new file mode 100644
> > index 0000000..f88a8d6
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/drm/bridge/dw_mipi_dsi.txt
> > @@ -0,0 +1,73 @@
> > +Device-Tree bindings for Synopsys DesignWare MIPI DSI host controller
> > +
> > +The controller is a digital core that implements all protocol functions
> > +defined in the MIPI DSI specification, providing an interface between
> > +the system and the MIPI DPHY, and allowing communication with a MIPI DSI
> > +compliant display.
> > +
> > +Required properties:
> > + - #address-cells: Should be <1>.
> > + - #size-cells: Should be <0>.
> > + - compatible: The compatible string should be "fsl,imx6q-mipi-dsi" for
> > +   i.MX6q/sdl SoCs.  For other SoCs, please refer to their specific
> > +   device tree binding documentations.
> 
> I think the compatible property should additionally contain
> "snps,dw-mipi-dsi". Also I think other SoCs using the same IP core
> should eventually list their compatibles here, but that's for later.
> 
> How about:
> + - compatible: The compatible string contain "fsl,imx6q-mipi-dsi" for
> +   i.MX6q/sdl SoCs.  For other SoCs, please refer to their specific
> +   device tree binding documentations. A common compatible string
> +   "snps,dw-mipi-dsi" should be appended for all SoCs.

I'm not sure if "snps,dw-mipi-dsi" should be appended.

Is it a compatible string that SoC specific drivers should actually use to
bind a device?

As Andy mentioned in [1], DW MIPI DSI embedded in RK618 is configured via I2C
bus, while i.MX6Q/DL is configured via ARM bus...

Another concern is that if users only specify "snps,dw-mipi-dsi" in their
device tree sources and use a kernel which supports multiple platforms,
say ARM multi v7 platforms, could several enabled SoC specific drivers be
probed for a single DW MIPI DSI device?

[1] http://lists.freedesktop.org/archives/dri-devel/2014-December/074344.html

> 
> > + - reg: Represent the physical address range of the controller.
> > + - interrupts: Represent the controller's interrupt to the CPU(s).
> > + - clocks, clock-names: Phandles to the controller pll reference and
> > +   core configuration clocks, as described in [1].
> 
> From the MIPI CSI-2 datasheets it looks like the D-PHY has a refclk and
> a cfg_clk input. So I suspect from the name of the "core_cfg" clock,
> that it actually represents two clock inputs, the "cfg_clk" wired to the
> D-PHY and a core clock wired to the MIPI DSI host controller. I am not
> sure if there are designs that control those clocks separately, but I
> think it'd be safer to split this into two clocks in the device tree.

What MIPI CSI-2 datasheets do you refer to?
I don't find the refclk and cfg_clk in the MIPI CSI chapter of the i.MX6DQ
Reference Manual v2[2].

I think we need to know the design philosophy of DW MIPI DSI clock sources
to settle down the documentation here.  I've asked our internal MIPI DSI
SoC owner for ideas.  But, someone from Synopsys might be the right
person, perhaps.

[2] http://cache.freescale.com/files/32bit/doc/ref_manual/IMX6DQRM.pdf?fasp=1&WT_TYPE=Reference%20Manuals&WT_VENDOR=FREESCALE&WT_FILE_FORMAT=pdf&WT_ASSET=Documentation&fileExt=.pdf

> 
> Also I am not sure which input to the MIPI DSI host controller the core
> clock represents. The i.MX6DQ Reference Manual v2 calls the remaining
> clock inputs gated by mipi_core_cfg_clk_enable "ac_clk_125m" and
> "ips_clk" (I think the latter is the ABP clock driving the register
> bank, just called "pclk" in the MIPI CSI-2 documentation).

The same MIPI CSI-2 documentation you mentioned above?

I'm also puzzled about the clocks.

Regards,
Liu Ying

> 
> regards
> Philipp
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists