[PATCH v2 0/11] LVDS Display Bridge support for i.MX

[PATCH v2 0/11] LVDS Display Bridge support for i.MX

[Philipp Zabel]

Hi,

the following patches add support for LVDS displays on
i.MX53 and i.MX6q boards.

The clock patches are needed because the LVDS serial clock
needs to be in lockstep with the IPU display interface clock
providing the pixel data. A fixed factor of 7:1 (or 3.5:1 in
dual link mode) needs to be maintained. This is achieved on
i.MX by clocking the LDB DI clock directly from a PLL, and
manually setting the 3.5/7:1 divider depending on dual/single
link mode. The IPU display interface clock is then sourced
from the divided LDB clock.

Changes since v1:
 - Added device-tree bindings documentation
 - Dropped remains of GPIO handling from LDB driver, sorry about that

regards
Philipp

---
 .../devicetree/bindings/staging/imx-drm/ldb.txt    |  99 ++++
 arch/arm/boot/dts/imx51.dtsi                       |   2 +
 arch/arm/boot/dts/imx53.dtsi                       |  34 ++
 arch/arm/boot/dts/imx6q.dtsi                       |  17 +
 arch/arm/boot/dts/imx6qdl.dtsi                     |  26 +
 arch/arm/mach-imx/clk-imx51-imx53.c                |  19 +-
 arch/arm/mach-imx/clk-imx6q.c                      |  43 +-
 arch/arm/mach-imx/clk.h                            |  17 +
 arch/arm/mach-imx/common.h                         |   1 +
 arch/arm/mach-imx/mach-imx6q.c                     |   2 +-
 drivers/staging/imx-drm/Kconfig                    |   7 +
 drivers/staging/imx-drm/Makefile                   |   1 +
 drivers/staging/imx-drm/imx-ldb.c                  | 611 +++++++++++++++++++++
 13 files changed, 854 insertions(+), 25 deletions(-)

[PATCH v2 01/11] staging: drm/imx: Add LDB support

[Philipp Zabel]

From: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

This adds support for the LVDS Display Bridge contained
in i.MX5 and i.MX6 SoCs.

Bit mapping, data width, and video timings are configurable
via device tree. Dual-channel mode is supported for a single
high-resolution source.

Signed-off-by: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
Changes since v1:
 - Added device-tree bindings documentation
 - Removed remains of GPIO handling
---
 .../devicetree/bindings/staging/imx-drm/ldb.txt    |  99 ++++
 drivers/staging/imx-drm/Kconfig                    |   7 +
 drivers/staging/imx-drm/Makefile                   |   1 +
 drivers/staging/imx-drm/imx-ldb.c                  | 611 +++++++++++++++++++++
 4 files changed, 718 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/staging/imx-drm/ldb.txt
 create mode 100644 drivers/staging/imx-drm/imx-ldb.c

diff --git a/Documentation/devicetree/bindings/staging/imx-drm/ldb.txt b/Documentation/devicetree/bindings/staging/imx-drm/ldb.txt
new file mode 100644
index 0000000..ed93778
--- /dev/null
+++ b/Documentation/devicetree/bindings/staging/imx-drm/ldb.txt
@@ -0,0 +1,99 @@
+Device-Tree bindings for LVDS Display Bridge (ldb)
+
+LVDS Display Bridge
+===================
+
+The LVDS Display Bridge device tree node contains up to two lvds-channel
+nodes describing each of the two LVDS encoder channels of the bridge.
+
+Required properties:
+ - #address-cells : should be <1>
+ - #size-cells : should be <0>
+ - compatible : should be "fsl,imx53-ldb" or "fsl,imx6q-ldb".
+                Both LDB versions are similar, but i.MX6 has an additional
+                multiplexer in the front to select any of the four IPU display
+                interfaces as input for each LVDS channel.
+ - gpr : should be <&gpr> on i.MX53 and i.MX6q.
+         The phandle points to the iomuxc-gpr region containing the LVDS
+         control register.
+- clocks, clock-names : phandles to the LDB divider and selector clocks and to
+                        the display interface selector clocks, as described in
+                        Documentation/devicetree/bindings/clock/clock-bindings.txt
+        The following clocks are expected on i.MX53:
+                "di0_pll" - LDB LVDS channel 0 mux
+                "di1_pll" - LDB LVDS channel 1 mux
+                "di0" - LDB LVDS channel 0 gate
+                "di1" - LDB LVDS channel 1 gate
+                "di0_sel" - IPU1 DI0 mux
+                "di1_sel" - IPU1 DI1 mux
+        On i.MX6q the following additional clocks are needed:
+                "di2_sel" - IPU2 DI0 mux
+                "di3_sel" - IPU2 DI1 mux
+        The needed clock numbers for each are documented in
+        Documentation/devicetree/bindings/clock/imx5-clock.txt, and in
+        Documentation/devicetree/bindings/clock/imx6q-clock.txt.
+
+Optional properties:
+ - pinctrl-names : should be "default" on i.MX53, not used on i.MX6q
+ - pinctrl-0 : a phandle pointing to LVDS pin settings on i.MX53,
+               not used on i.MX6q
+ - fsl,dual-channel : boolean. if it exists, only LVDS channel 0 should
+   be configured - one input will be distributed on both outputs in dual
+   channel mode
+
+LVDS Channel
+============
+
+Each LVDS Channel has to contain a display-timings node that describes the
+video timings for the connected LVDS display. For detailed information, also
+have a look at Documentation/devicetree/bindings/video/display-timing.txt.
+
+Required properties:
+ - reg : should be <0> or <1>
+ - crtcs : a list of phandles with index pointing to the IPU display interfaces
+           that can be used as video source for this channel.
+ - fsl,data-mapping : should be "spwg" or "jeida"
+                      This describes how the color bits are laid out in the
+                      serialized LVDS signal.
+ - fsl,data-width : should be <18> or <24>
+
+example:
+
+gpr: iomuxc-gpr@53fa8000 {
+	/* ... */
+};
+
+ldb: ldb@53fa8008 {
+	#address-cells = <1>;
+	#size-cells = <0>;
+	compatible = "fsl,imx53-ldb";
+	gpr = <&gpr>;
+	clocks = <&clks 122>, <&clks 120>,
+		 <&clks 115>, <&clks 116>,
+		 <&clks 123>, <&clks 85>;
+	clock-names = "di0_pll", "di1_pll",
+		      "di0_sel", "di1_sel",
+		      "di0", "di1";
+
+	lvds-channel@0 {
+		reg = <0>;
+		crtcs = <&ipu 0>;
+		fsl,data-mapping = "spwg";
+		fsl,data-width = <24>;
+
+		display-timings {
+			/* ... */
+		};
+	};
+
+	lvds-channel@1 {
+		reg = <1>;
+		crtcs = <&ipu 1>;
+		fsl,data-mapping = "spwg";
+		fsl,data-width = <24>;
+
+		display-timings {
+			/* ... */
+		};
+	};
+};
diff --git a/drivers/staging/imx-drm/Kconfig b/drivers/staging/imx-drm/Kconfig
index be7e2e3..3ec352c 100644
--- a/drivers/staging/imx-drm/Kconfig
+++ b/drivers/staging/imx-drm/Kconfig
@@ -20,6 +20,13 @@ config DRM_IMX_PARALLEL_DISPLAY
 	tristate "Support for parallel displays"
 	depends on DRM_IMX
 
+config DRM_IMX_LDB
+	tristate "Support for LVDS displays"
+	help
+	  Choose this to enable the internal LVDS Display Bridge (LDB)
+	  found on i.MX53 and i.MX6 processors.
+	depends on DRM_IMX
+
 config DRM_IMX_IPUV3_CORE
 	tristate "IPUv3 core support"
 	depends on DRM_IMX
diff --git a/drivers/staging/imx-drm/Makefile b/drivers/staging/imx-drm/Makefile
index 83a9056..9bfac13 100644
--- a/drivers/staging/imx-drm/Makefile
+++ b/drivers/staging/imx-drm/Makefile
@@ -4,6 +4,7 @@ imxdrm-objs := imx-drm-core.o imx-fb.o
 obj-$(CONFIG_DRM_IMX) += imxdrm.o
 
 obj-$(CONFIG_DRM_IMX_PARALLEL_DISPLAY) += parallel-display.o
+obj-$(CONFIG_DRM_IMX_LDB) += imx-ldb.o
 obj-$(CONFIG_DRM_IMX_FB_HELPER) += imx-fbdev.o
 obj-$(CONFIG_DRM_IMX_IPUV3_CORE) += ipu-v3/
 obj-$(CONFIG_DRM_IMX_IPUV3)	+= ipuv3-crtc.o
diff --git a/drivers/staging/imx-drm/imx-ldb.c b/drivers/staging/imx-drm/imx-ldb.c
new file mode 100644
index 0000000..226f48f
--- /dev/null
+++ b/drivers/staging/imx-drm/imx-ldb.c
@@ -0,0 +1,611 @@
+/*
+ * i.MX drm driver - LVDS display bridge
+ *
+ * Copyright (C) 2012 Sascha Hauer, Pengutronix
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
+ * MA 02110-1301, USA.
+ */
+
+#include <linux/module.h>
+#include <linux/clk.h>
+#include <drm/drmP.h>
+#include <drm/drm_fb_helper.h>
+#include <drm/drm_crtc_helper.h>
+#include <linux/mfd/syscon.h>
+#include <linux/mfd/syscon/imx6q-iomuxc-gpr.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <video/of_videomode.h>
+#include <linux/regmap.h>
+#include <linux/videodev2.h>
+
+#include "imx-drm.h"
+
+#define DRIVER_NAME "imx-ldb"
+
+#define LDB_CH0_MODE_EN_TO_DI0		(1 << 0)
+#define LDB_CH0_MODE_EN_TO_DI1		(3 << 0)
+#define LDB_CH1_MODE_EN_TO_DI0		(1 << 2)
+#define LDB_CH1_MODE_EN_TO_DI1		(3 << 2)
+#define LDB_SPLIT_MODE_EN		(1 << 4)
+#define LDB_DATA_WIDTH_CH0_24		(1 << 5)
+#define LDB_BIT_MAP_CH0_JEIDA		(1 << 6)
+#define LDB_DATA_WIDTH_CH1_24		(1 << 7)
+#define LDB_BIT_MAP_CH1_JEIDA		(1 << 8)
+#define LDB_DI0_VS_POL_ACT_LOW		(1 << 9)
+#define LDB_DI1_VS_POL_ACT_LOW		(1 << 10)
+#define LDB_BGREF_RMODE_INT		(1 << 15)
+
+#define con_to_imx_ldb_ch(x) container_of(x, struct imx_ldb_channel, connector)
+#define enc_to_imx_ldb_ch(x) container_of(x, struct imx_ldb_channel, encoder)
+
+struct imx_ldb;
+
+struct imx_ldb_channel {
+	struct imx_ldb *ldb;
+	struct drm_connector connector;
+	struct imx_drm_connector *imx_drm_connector;
+	struct drm_encoder encoder;
+	struct imx_drm_encoder *imx_drm_encoder;
+	int chno;
+	void *edid;
+	int edid_len;
+	struct drm_display_mode mode;
+	int mode_valid;
+};
+
+struct bus_mux {
+	int reg;
+	int shift;
+	int mask;
+};
+
+struct imx_ldb {
+	struct regmap *regmap;
+	struct device *dev;
+	struct imx_ldb_channel channel[2];
+	struct clk *clk[2]; /* our own clock */
+	struct clk *clk_sel[4]; /* parent of display clock */
+	struct clk *clk_pll[2]; /* upstream clock we can adjust */
+	u32 ldb_ctrl;
+	const struct bus_mux *lvds_mux;
+};
+
+static enum drm_connector_status imx_ldb_connector_detect(
+		struct drm_connector *connector, bool force)
+{
+	return connector_status_connected;
+}
+
+static void imx_ldb_connector_destroy(struct drm_connector *connector)
+{
+	/* do not free here */
+}
+
+static int imx_ldb_connector_get_modes(struct drm_connector *connector)
+{
+	struct imx_ldb_channel *imx_ldb_ch = con_to_imx_ldb_ch(connector);
+	int num_modes = 0;
+
+	if (imx_ldb_ch->edid) {
+		drm_mode_connector_update_edid_property(connector,
+							imx_ldb_ch->edid);
+		num_modes = drm_add_edid_modes(connector, imx_ldb_ch->edid);
+	}
+
+	if (imx_ldb_ch->mode_valid) {
+		struct drm_display_mode *mode;
+
+		mode = drm_mode_create(connector->dev);
+		drm_mode_copy(mode, &imx_ldb_ch->mode);
+		mode->type |= DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
+		drm_mode_probed_add(connector, mode);
+		num_modes++;
+	}
+
+	return num_modes;
+}
+
+static int imx_ldb_connector_mode_valid(struct drm_connector *connector,
+			  struct drm_display_mode *mode)
+{
+	return 0;
+}
+
+static struct drm_encoder *imx_ldb_connector_best_encoder(
+		struct drm_connector *connector)
+{
+	struct imx_ldb_channel *imx_ldb_ch = con_to_imx_ldb_ch(connector);
+
+	return &imx_ldb_ch->encoder;
+}
+
+static void imx_ldb_encoder_dpms(struct drm_encoder *encoder, int mode)
+{
+}
+
+static bool imx_ldb_encoder_mode_fixup(struct drm_encoder *encoder,
+			   const struct drm_display_mode *mode,
+			   struct drm_display_mode *adjusted_mode)
+{
+/*
+	struct imx_ldb_channel *imx_ldb_ch = enc_to_imx_ldb_ch(encoder);
+
+	adjusted_mode->clock = clk_round_rate(imx_ldb_ch->ldb->clk_pll[imx_ldb_ch->chno],
+			adjusted_mode->clock * 1000) / 1000;
+*/
+	return true;
+}
+
+static void imx_ldb_set_clock(struct imx_ldb *ldb, int mux, int chno,
+		unsigned long serial_clk, unsigned long di_clk)
+{
+	int ret;
+
+	dev_dbg(ldb->dev, "%s: now: %ld want: %ld\n", __func__,
+			clk_get_rate(ldb->clk_pll[chno]), serial_clk);
+	clk_set_rate(ldb->clk_pll[chno], serial_clk);
+
+	dev_dbg(ldb->dev, "%s after: %ld\n", __func__,
+			clk_get_rate(ldb->clk_pll[chno]));
+
+	dev_dbg(ldb->dev, "%s: now: %ld want: %ld\n", __func__,
+			clk_get_rate(ldb->clk[chno]),
+			(long int)di_clk);
+	clk_set_rate(ldb->clk[chno], di_clk);
+
+	dev_dbg(ldb->dev, "%s after: %ld\n", __func__,
+			clk_get_rate(ldb->clk[chno]));
+
+	/* set display clock mux to LDB input clock */
+	ret = clk_set_parent(ldb->clk_sel[mux], ldb->clk[chno]);
+	if (ret) {
+		dev_err(ldb->dev, "unable to set di%d parent clock to ldb_di%d\n", mux, chno);
+	}
+}
+
+static void imx_ldb_encoder_prepare(struct drm_encoder *encoder)
+{
+	struct imx_ldb_channel *imx_ldb_ch = enc_to_imx_ldb_ch(encoder);
+	struct imx_ldb *ldb = imx_ldb_ch->ldb;
+	struct drm_display_mode *mode = &encoder->crtc->mode;
+	unsigned long serial_clk;
+	unsigned long di_clk = mode->clock * 1000;
+	int mux = imx_drm_encoder_get_mux_id(imx_ldb_ch->imx_drm_encoder,
+					     encoder->crtc);
+
+	if (ldb->ldb_ctrl & LDB_SPLIT_MODE_EN) {
+		/* dual channel LVDS mode */
+		serial_clk = 3500UL * mode->clock;
+		imx_ldb_set_clock(ldb, mux, 0, serial_clk, di_clk);
+		imx_ldb_set_clock(ldb, mux, 1, serial_clk, di_clk);
+	} else {
+		serial_clk = 7000UL * mode->clock;
+		imx_ldb_set_clock(ldb, mux, imx_ldb_ch->chno, serial_clk, di_clk);
+	}
+
+	imx_drm_crtc_panel_format(encoder->crtc, DRM_MODE_ENCODER_LVDS,
+			V4L2_PIX_FMT_RGB24);
+}
+
+static void imx_ldb_encoder_commit(struct drm_encoder *encoder)
+{
+	struct imx_ldb_channel *imx_ldb_ch = enc_to_imx_ldb_ch(encoder);
+	struct imx_ldb *ldb = imx_ldb_ch->ldb;
+	int dual = ldb->ldb_ctrl & LDB_SPLIT_MODE_EN;
+	int mux = imx_drm_encoder_get_mux_id(imx_ldb_ch->imx_drm_encoder,
+					     encoder->crtc);
+
+	if (dual) {
+		clk_prepare_enable(ldb->clk[0]);
+		clk_prepare_enable(ldb->clk[1]);
+	}
+
+	if (imx_ldb_ch == &ldb->channel[0] || dual) {
+		ldb->ldb_ctrl &= ~0x3;
+		if (mux == 0 || ldb->lvds_mux)
+			ldb->ldb_ctrl |= LDB_CH0_MODE_EN_TO_DI0;
+		else if (mux == 1)
+			ldb->ldb_ctrl |= LDB_CH0_MODE_EN_TO_DI1;
+	}
+	if (imx_ldb_ch == &ldb->channel[1] || dual) {
+		ldb->ldb_ctrl &= ~0xc;
+		if (mux == 1 || ldb->lvds_mux)
+			ldb->ldb_ctrl |= LDB_CH1_MODE_EN_TO_DI1;
+		else if (mux == 0)
+			ldb->ldb_ctrl |= LDB_CH1_MODE_EN_TO_DI0;
+	}
+
+	if (ldb->lvds_mux) {
+		const struct bus_mux *lvds_mux = NULL;
+
+		if (imx_ldb_ch == &ldb->channel[0])
+			lvds_mux = &ldb->lvds_mux[0];
+		else if (imx_ldb_ch == &ldb->channel[1])
+			lvds_mux = &ldb->lvds_mux[1];
+
+		regmap_update_bits(ldb->regmap, lvds_mux->reg, lvds_mux->mask,
+				   mux << lvds_mux->shift);
+	}
+
+	regmap_write(ldb->regmap, IOMUXC_GPR2, ldb->ldb_ctrl);
+}
+
+static void imx_ldb_encoder_mode_set(struct drm_encoder *encoder,
+			 struct drm_display_mode *mode,
+			 struct drm_display_mode *adjusted_mode)
+{
+	struct imx_ldb_channel *imx_ldb_ch = enc_to_imx_ldb_ch(encoder);
+	struct imx_ldb *ldb = imx_ldb_ch->ldb;
+	int dual = ldb->ldb_ctrl & LDB_SPLIT_MODE_EN;
+
+	if (mode->clock > 170000) {
+		dev_warn(ldb->dev,
+			 "%s: mode exceeds 170 MHz pixel clock\n", __func__);
+	}
+	if (mode->clock > 85000 && !dual) {
+		dev_warn(ldb->dev,
+			 "%s: mode exceeds 85 MHz pixel clock\n", __func__);
+	}
+
+	/* FIXME - assumes straight connections DI0 --> CH0, DI1 --> CH1 */
+	if (imx_ldb_ch == &ldb->channel[0]) {
+		if (mode->flags & DRM_MODE_FLAG_NVSYNC)
+			ldb->ldb_ctrl |= LDB_DI0_VS_POL_ACT_LOW;
+		else if (mode->flags & DRM_MODE_FLAG_PVSYNC)
+			ldb->ldb_ctrl &= ~LDB_DI0_VS_POL_ACT_LOW;
+	}
+	if (imx_ldb_ch == &ldb->channel[1]) {
+		if (mode->flags & DRM_MODE_FLAG_NVSYNC)
+			ldb->ldb_ctrl |= LDB_DI1_VS_POL_ACT_LOW;
+		else if (mode->flags & DRM_MODE_FLAG_PVSYNC)
+			ldb->ldb_ctrl &= ~LDB_DI1_VS_POL_ACT_LOW;
+	}
+}
+
+static void imx_ldb_encoder_disable(struct drm_encoder *encoder)
+{
+	struct imx_ldb_channel *imx_ldb_ch = enc_to_imx_ldb_ch(encoder);
+	struct imx_ldb *ldb = imx_ldb_ch->ldb;
+
+	/*
+	 * imx_ldb_encoder_disable is called by
+	 * drm_helper_disable_unused_functions without
+	 * the encoder being enabled before.
+	 */
+	if (imx_ldb_ch == &ldb->channel[0] && (ldb->ldb_ctrl & 0x3) == 0)
+		return;
+	else if (imx_ldb_ch == &ldb->channel[1] && (ldb->ldb_ctrl & 0xc) == 0)
+		return;
+
+	if (imx_ldb_ch == &ldb->channel[0])
+		ldb->ldb_ctrl &= ~0x3;
+	else if (imx_ldb_ch == &ldb->channel[1])
+		ldb->ldb_ctrl &= ~0xc;
+
+	regmap_write(ldb->regmap, IOMUXC_GPR2, ldb->ldb_ctrl);
+
+	if (ldb->ldb_ctrl & LDB_SPLIT_MODE_EN) {
+		clk_disable_unprepare(ldb->clk[0]);
+		clk_disable_unprepare(ldb->clk[1]);
+	}
+}
+
+static void imx_ldb_encoder_destroy(struct drm_encoder *encoder)
+{
+	/* do not free here */
+}
+
+static struct drm_connector_funcs imx_ldb_connector_funcs = {
+	.dpms = drm_helper_connector_dpms,
+	.fill_modes = drm_helper_probe_single_connector_modes,
+	.detect = imx_ldb_connector_detect,
+	.destroy = imx_ldb_connector_destroy,
+};
+
+static struct drm_connector_helper_funcs imx_ldb_connector_helper_funcs = {
+	.get_modes = imx_ldb_connector_get_modes,
+	.best_encoder = imx_ldb_connector_best_encoder,
+	.mode_valid = imx_ldb_connector_mode_valid,
+};
+
+static struct drm_encoder_funcs imx_ldb_encoder_funcs = {
+	.destroy = imx_ldb_encoder_destroy,
+};
+
+static struct drm_encoder_helper_funcs imx_ldb_encoder_helper_funcs = {
+	.dpms = imx_ldb_encoder_dpms,
+	.mode_fixup = imx_ldb_encoder_mode_fixup,
+	.prepare = imx_ldb_encoder_prepare,
+	.commit = imx_ldb_encoder_commit,
+	.mode_set = imx_ldb_encoder_mode_set,
+	.disable = imx_ldb_encoder_disable,
+};
+
+static int imx_ldb_get_clk(struct imx_ldb *ldb, int chno)
+{
+	char clkname[16];
+
+	sprintf(clkname, "di%d", chno);
+	ldb->clk[chno] = devm_clk_get(ldb->dev, clkname);
+	if (IS_ERR(ldb->clk[chno]))
+		return PTR_ERR(ldb->clk[chno]);
+
+	sprintf(clkname, "di%d_pll", chno);
+	ldb->clk_pll[chno] = devm_clk_get(ldb->dev, clkname);
+	if (IS_ERR(ldb->clk_pll[chno]))
+		return PTR_ERR(ldb->clk_pll[chno]);
+
+	return 0;
+}
+
+static int imx_ldb_register(struct imx_ldb_channel *imx_ldb_ch)
+{
+	int ret;
+	struct imx_ldb *ldb = imx_ldb_ch->ldb;
+
+	ret = imx_ldb_get_clk(ldb, imx_ldb_ch->chno);
+	if (ret)
+		return ret;
+	if (ldb->ldb_ctrl & LDB_SPLIT_MODE_EN) {
+		ret |= imx_ldb_get_clk(ldb, 1);
+		if (ret)
+			return ret;
+	}
+
+	imx_ldb_ch->connector.funcs = &imx_ldb_connector_funcs;
+	imx_ldb_ch->encoder.funcs = &imx_ldb_encoder_funcs;
+
+	imx_ldb_ch->encoder.encoder_type = DRM_MODE_ENCODER_LVDS;
+	imx_ldb_ch->connector.connector_type = DRM_MODE_CONNECTOR_LVDS;
+
+	drm_encoder_helper_add(&imx_ldb_ch->encoder,
+			&imx_ldb_encoder_helper_funcs);
+	ret = imx_drm_add_encoder(&imx_ldb_ch->encoder,
+			&imx_ldb_ch->imx_drm_encoder, THIS_MODULE);
+	if (ret) {
+		dev_err(ldb->dev, "adding encoder failed with %d\n", ret);
+		return ret;
+	}
+
+	drm_connector_helper_add(&imx_ldb_ch->connector,
+			&imx_ldb_connector_helper_funcs);
+
+	ret = imx_drm_add_connector(&imx_ldb_ch->connector,
+			&imx_ldb_ch->imx_drm_connector, THIS_MODULE);
+	if (ret) {
+		imx_drm_remove_encoder(imx_ldb_ch->imx_drm_encoder);
+		dev_err(ldb->dev, "adding connector failed with %d\n", ret);
+		return ret;
+	}
+
+	drm_mode_connector_attach_encoder(&imx_ldb_ch->connector,
+			&imx_ldb_ch->encoder);
+
+	return 0;
+}
+
+enum {
+	LVDS_BIT_MAP_SPWG,
+	LVDS_BIT_MAP_JEIDA
+};
+
+static const char *imx_ldb_bit_mappings[] = {
+	[LVDS_BIT_MAP_SPWG]  = "spwg",
+	[LVDS_BIT_MAP_JEIDA] = "jeida",
+};
+
+const int of_get_data_mapping(struct device_node *np)
+{
+	const char *bm;
+	int ret, i;
+
+	ret = of_property_read_string(np, "fsl,data-mapping", &bm);
+	if (ret < 0)
+		return ret;
+
+	for (i = 0; i < ARRAY_SIZE(imx_ldb_bit_mappings); i++)
+		if (!strcasecmp(bm, imx_ldb_bit_mappings[i]))
+			return i;
+
+	return -EINVAL;
+}
+
+static struct bus_mux imx6q_lvds_mux[2] = {
+	{
+		.reg = IOMUXC_GPR3,
+		.shift = 6,
+		.mask = IMX6Q_GPR3_LVDS0_MUX_CTL_MASK,
+	}, {
+		.reg = IOMUXC_GPR3,
+		.shift = 8,
+		.mask = IMX6Q_GPR3_LVDS1_MUX_CTL_MASK,
+	}
+};
+
+/*
+ * For a device declaring compatible = "fsl,imx6q-ldb", "fsl,imx53-ldb",
+ * of_match_device will walk through this list and take the first entry
+ * matching any of its compatible values. Therefore, the more generic
+ * entries (in this case fsl,imx53-ldb) need to be ordered last.
+ */
+static const struct of_device_id imx_ldb_dt_ids[] = {
+	{ .compatible = "fsl,imx6q-ldb", .data = imx6q_lvds_mux, },
+	{ .compatible = "fsl,imx53-ldb", .data = NULL, },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, imx_ldb_dt_ids);
+
+static int imx_ldb_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	const struct of_device_id *of_id =
+			of_match_device(of_match_ptr(imx_ldb_dt_ids),
+					&pdev->dev);
+	struct device_node *child;
+	const u8 *edidp;
+	struct imx_ldb *imx_ldb;
+	int datawidth;
+	int mapping;
+	int dual;
+	int ret;
+	int i;
+
+	imx_ldb = devm_kzalloc(&pdev->dev, sizeof(*imx_ldb), GFP_KERNEL);
+	if (!imx_ldb)
+		return -ENOMEM;
+
+	imx_ldb->regmap = syscon_regmap_lookup_by_phandle(np, "gpr");
+	if (IS_ERR(imx_ldb->regmap)) {
+		dev_err(&pdev->dev, "failed to get parent regmap\n");
+		return PTR_ERR(imx_ldb->regmap);
+	}
+
+	imx_ldb->dev = &pdev->dev;
+
+	if (of_id)
+		imx_ldb->lvds_mux = of_id->data;
+
+	dual = of_property_read_bool(np, "fsl,dual-channel");
+	if (dual)
+		imx_ldb->ldb_ctrl |= LDB_SPLIT_MODE_EN;
+
+	/*
+	 * There are three diferent possible clock mux configurations:
+	 * i.MX53:  ipu1_di0_sel, ipu1_di1_sel
+	 * i.MX6q:  ipu1_di0_sel, ipu1_di1_sel, ipu2_di0_sel, ipu2_di1_sel
+	 * i.MX6dl: ipu1_di0_sel, ipu1_di1_sel, lcdif_sel
+	 * Map them all to di0_sel...di3_sel.
+	 */
+	for (i = 0; i < 4; i++) {
+		char clkname[16];
+
+		sprintf(clkname, "di%d_sel", i);
+		imx_ldb->clk_sel[i] = devm_clk_get(imx_ldb->dev, clkname);
+		if (IS_ERR(imx_ldb->clk_sel[i])) {
+			ret = PTR_ERR(imx_ldb->clk_sel[i]);
+			imx_ldb->clk_sel[i] = NULL;
+			break;
+		}
+	}
+	if (i == 0)
+		return ret;
+
+	for_each_child_of_node(np, child) {
+		struct imx_ldb_channel *channel;
+
+		ret = of_property_read_u32(child, "reg", &i);
+		if (ret || i < 0 || i > 1)
+			return -EINVAL;
+
+		if (dual && i > 0) {
+			dev_warn(&pdev->dev, "dual-channel mode, ignoring second output\n");
+			continue;
+		}
+
+		if (!of_device_is_available(child))
+			continue;
+
+		channel = &imx_ldb->channel[i];
+		channel->ldb = imx_ldb;
+		channel->chno = i;
+
+		edidp = of_get_property(child, "edid", &channel->edid_len);
+		if (edidp) {
+			channel->edid = kmemdup(edidp, channel->edid_len,
+						GFP_KERNEL);
+		} else {
+			ret = of_get_drm_display_mode(child, &channel->mode, 0);
+			if (!ret)
+				channel->mode_valid = 1;
+		}
+
+		ret = of_property_read_u32(child, "fsl,data-width", &datawidth);
+		if (ret)
+			datawidth = 0;
+		else if (datawidth != 18 && datawidth != 24)
+			return -EINVAL;
+
+		mapping = of_get_data_mapping(child);
+		switch (mapping) {
+		case LVDS_BIT_MAP_SPWG:
+			if (datawidth == 24) {
+				if (i == 0 || dual)
+					imx_ldb->ldb_ctrl |= LDB_DATA_WIDTH_CH0_24;
+				if (i == 1 || dual)
+					imx_ldb->ldb_ctrl |= LDB_DATA_WIDTH_CH1_24;
+			}
+			break;
+		case LVDS_BIT_MAP_JEIDA:
+			if (datawidth == 18) {
+				dev_err(&pdev->dev, "JEIDA standard only supported in 24 bit\n");
+				return -EINVAL;
+			}
+			if (i == 0 || dual)
+				imx_ldb->ldb_ctrl |= LDB_DATA_WIDTH_CH0_24 | LDB_BIT_MAP_CH0_JEIDA;
+			if (i == 1 || dual)
+				imx_ldb->ldb_ctrl |= LDB_DATA_WIDTH_CH1_24 | LDB_BIT_MAP_CH1_JEIDA;
+			break;
+		default:
+			dev_err(&pdev->dev, "data mapping not specified or invalid\n");
+			return -EINVAL;
+		}
+
+		ret = imx_ldb_register(channel);
+		if (ret)
+			return ret;
+
+		imx_drm_encoder_add_possible_crtcs(channel->imx_drm_encoder, child);
+	}
+
+	platform_set_drvdata(pdev, imx_ldb);
+
+	return 0;
+}
+
+static int imx_ldb_remove(struct platform_device *pdev)
+{
+	struct imx_ldb *imx_ldb = platform_get_drvdata(pdev);
+	int i;
+
+	for (i = 0; i < 2; i++) {
+		struct imx_ldb_channel *channel = &imx_ldb->channel[i];
+		struct drm_connector *connector = &channel->connector;
+		struct drm_encoder *encoder = &channel->encoder;
+
+		drm_mode_connector_detach_encoder(connector, encoder);
+
+		imx_drm_remove_connector(channel->imx_drm_connector);
+		imx_drm_remove_encoder(channel->imx_drm_encoder);
+	}
+
+	return 0;
+}
+
+static struct platform_driver imx_ldb_driver = {
+	.probe		= imx_ldb_probe,
+	.remove		= imx_ldb_remove,
+	.driver		= {
+		.of_match_table = imx_ldb_dt_ids,
+		.name	= DRIVER_NAME,
+		.owner	= THIS_MODULE,
+	},
+};
+
+module_platform_driver(imx_ldb_driver);
+
+MODULE_DESCRIPTION("i.MX LVDS driver");
+MODULE_AUTHOR("Sascha Hauer, Pengutronix");
+MODULE_LICENSE("GPL");
-- 
1.8.2.rc2

Re: [PATCH v2 01/11] staging: drm/imx: Add LDB support

[Martin Fuzzey]

Hi Philipp,

On 27/03/13 18:30, Philipp Zabel wrote:
> +static bool imx_ldb_encoder_mode_fixup(struct drm_encoder *encoder,
> +			   const struct drm_display_mode *mode,
> +			   struct drm_display_mode *adjusted_mode)
> +{
> +/*
> +	struct imx_ldb_channel *imx_ldb_ch = enc_to_imx_ldb_ch(encoder);
> +
> +	adjusted_mode->clock = clk_round_rate(imx_ldb_ch->ldb->clk_pll[imx_ldb_ch->chno],
> +			adjusted_mode->clock * 1000) / 1000;
> +*/
This should probably be removed :)
> +static void imx_ldb_set_clock(struct imx_ldb *ldb, int mux, int chno,
> +		unsigned long serial_clk, unsigned long di_clk)
> +{
> +	int ret;
> +
> +	dev_dbg(ldb->dev, "%s: now: %ld want: %ld\n", __func__,
> +			clk_get_rate(ldb->clk_pll[chno]), serial_clk);
> +	clk_set_rate(ldb->clk_pll[chno], serial_clk);
> +
> +	dev_dbg(ldb->dev, "%s after: %ld\n", __func__,
> +			clk_get_rate(ldb->clk_pll[chno]));
> +
> +	dev_dbg(ldb->dev, "%s: now: %ld want: %ld\n", __func__,
> +			clk_get_rate(ldb->clk[chno]),
> +			(long int)di_clk);
> +	clk_set_rate(ldb->clk[chno], di_clk);
> +
> +	dev_dbg(ldb->dev, "%s after: %ld\n", __func__,
> +			clk_get_rate(ldb->clk[chno]));
> +
Are all these debug statements still useful?
> +static void imx_ldb_encoder_commit(struct drm_encoder *encoder)
> +{
> +	if (imx_ldb_ch == &ldb->channel[0] || dual) {
> +		ldb->ldb_ctrl &= ~0x3;
> +		if (mux == 0 || ldb->lvds_mux)
> +			ldb->ldb_ctrl |= LDB_CH0_MODE_EN_TO_DI0;
> +		else if (mux == 1)
> +			ldb->ldb_ctrl |= LDB_CH0_MODE_EN_TO_DI1;
> +	}
> +	if (imx_ldb_ch == &ldb->channel[1] || dual) {
> +		ldb->ldb_ctrl &= ~0xc;
> +		if (mux == 1 || ldb->lvds_mux)
> +			ldb->ldb_ctrl |= LDB_CH1_MODE_EN_TO_DI1;
> +		else if (mux == 0)
> +			ldb->ldb_ctrl |= LDB_CH1_MODE_EN_TO_DI0;
> +	}
Maybe avoid the magic 0x3 and 0x0c by building from LDB_CHx_MODE_EN_TO_DIx?
> +
> +static void imx_ldb_encoder_disable(struct drm_encoder *encoder)
> +{
> +	struct imx_ldb_channel *imx_ldb_ch = enc_to_imx_ldb_ch(encoder);
> +	struct imx_ldb *ldb = imx_ldb_ch->ldb;
> +
> +	/*
> +	 * imx_ldb_encoder_disable is called by
> +	 * drm_helper_disable_unused_functions without
> +	 * the encoder being enabled before.
> +	 */
> +	if (imx_ldb_ch == &ldb->channel[0] && (ldb->ldb_ctrl & 0x3) == 0)
> +		return;
> +	else if (imx_ldb_ch == &ldb->channel[1] && (ldb->ldb_ctrl & 0xc) == 0)
> +		return;
> +
> +	if (imx_ldb_ch == &ldb->channel[0])
> +		ldb->ldb_ctrl &= ~0x3;
> +	else if (imx_ldb_ch == &ldb->channel[1])
> +		ldb->ldb_ctrl &= ~0xc;
> +
Idem magic numbers


Regards,

Martin

Re: [PATCH v2 01/11] staging: drm/imx: Add LDB support

[Philipp Zabel]

Hi Martin,

Am Mittwoch, den 27.03.2013, 19:40 +0100 schrieb Martin Fuzzey:
> Hi Philipp,
> 
> On 27/03/13 18:30, Philipp Zabel wrote:
> > +static bool imx_ldb_encoder_mode_fixup(struct drm_encoder *encoder,
> > +			   const struct drm_display_mode *mode,
> > +			   struct drm_display_mode *adjusted_mode)
> > +{
> > +/*
> > +	struct imx_ldb_channel *imx_ldb_ch = enc_to_imx_ldb_ch(encoder);
> > +
> > +	adjusted_mode->clock = clk_round_rate(imx_ldb_ch->ldb->clk_pll[imx_ldb_ch->chno],
> > +			adjusted_mode->clock * 1000) / 1000;
> > +*/
> This should probably be removed :)

Right.

> > +static void imx_ldb_set_clock(struct imx_ldb *ldb, int mux, int chno,
> > +		unsigned long serial_clk, unsigned long di_clk)
> > +{
> > +	int ret;
> > +
> > +	dev_dbg(ldb->dev, "%s: now: %ld want: %ld\n", __func__,
> > +			clk_get_rate(ldb->clk_pll[chno]), serial_clk);
> > +	clk_set_rate(ldb->clk_pll[chno], serial_clk);
> > +
> > +	dev_dbg(ldb->dev, "%s after: %ld\n", __func__,
> > +			clk_get_rate(ldb->clk_pll[chno]));
> > +
> > +	dev_dbg(ldb->dev, "%s: now: %ld want: %ld\n", __func__,
> > +			clk_get_rate(ldb->clk[chno]),
> > +			(long int)di_clk);
> > +	clk_set_rate(ldb->clk[chno], di_clk);
> > +
> > +	dev_dbg(ldb->dev, "%s after: %ld\n", __func__,
> > +			clk_get_rate(ldb->clk[chno]));
> > +
> Are all these debug statements still useful?

The clocking is a hairy, especially if the board code decided to use an
unexpected PLL as source for the LVDS serial clock. But I agree this
could be condensed a bit.

> > +static void imx_ldb_encoder_commit(struct drm_encoder *encoder)
> > +{
> > +	if (imx_ldb_ch == &ldb->channel[0] || dual) {
> > +		ldb->ldb_ctrl &= ~0x3;
> > +		if (mux == 0 || ldb->lvds_mux)
> > +			ldb->ldb_ctrl |= LDB_CH0_MODE_EN_TO_DI0;
> > +		else if (mux == 1)
> > +			ldb->ldb_ctrl |= LDB_CH0_MODE_EN_TO_DI1;
> > +	}
> > +	if (imx_ldb_ch == &ldb->channel[1] || dual) {
> > +		ldb->ldb_ctrl &= ~0xc;
> > +		if (mux == 1 || ldb->lvds_mux)
> > +			ldb->ldb_ctrl |= LDB_CH1_MODE_EN_TO_DI1;
> > +		else if (mux == 0)
> > +			ldb->ldb_ctrl |= LDB_CH1_MODE_EN_TO_DI0;
> > +	}
> Maybe avoid the magic 0x3 and 0x0c by building from LDB_CHx_MODE_EN_TO_DIx?

Ok, I'll prune the magic constants.

> > +
> > +static void imx_ldb_encoder_disable(struct drm_encoder *encoder)
> > +{
> > +	struct imx_ldb_channel *imx_ldb_ch = enc_to_imx_ldb_ch(encoder);
> > +	struct imx_ldb *ldb = imx_ldb_ch->ldb;
> > +
> > +	/*
> > +	 * imx_ldb_encoder_disable is called by
> > +	 * drm_helper_disable_unused_functions without
> > +	 * the encoder being enabled before.
> > +	 */
> > +	if (imx_ldb_ch == &ldb->channel[0] && (ldb->ldb_ctrl & 0x3) == 0)
> > +		return;
> > +	else if (imx_ldb_ch == &ldb->channel[1] && (ldb->ldb_ctrl & 0xc) == 0)
> > +		return;
> > +
> > +	if (imx_ldb_ch == &ldb->channel[0])
> > +		ldb->ldb_ctrl &= ~0x3;
> > +	else if (imx_ldb_ch == &ldb->channel[1])
> > +		ldb->ldb_ctrl &= ~0xc;
> > +
> Idem magic numbers

thanks
Philipp

Re: [PATCH v2 01/11] staging: drm/imx: Add LDB support

[Shawn Guo]

On Wed, Mar 27, 2013 at 06:30:35PM +0100, Philipp Zabel wrote:
> +config DRM_IMX_LDB
> +	tristate "Support for LVDS displays"
> +	help
> +	  Choose this to enable the internal LVDS Display Bridge (LDB)
> +	  found on i.MX53 and i.MX6 processors.
> +	depends on DRM_IMX
> +

Shouldn't we select OF_VIDEOMODE here, since of_get_drm_display_mode()
is called there.

Shawn

Re: [PATCH v2 01/11] staging: drm/imx: Add LDB support

[Philipp Zabel]

Am Donnerstag, den 28.03.2013, 14:08 +0800 schrieb Shawn Guo:
> On Wed, Mar 27, 2013 at 06:30:35PM +0100, Philipp Zabel wrote:
> > +config DRM_IMX_LDB
> > +	tristate "Support for LVDS displays"
> > +	help
> > +	  Choose this to enable the internal LVDS Display Bridge (LDB)
> > +	  found on i.MX53 and i.MX6 processors.
> > +	depends on DRM_IMX
> > +
> 
> Shouldn't we select OF_VIDEOMODE here, since of_get_drm_display_mode()
> is called there.

Yes, will fix.

thank you
Philipp

[PATCH v2 02/11] ARM i.MX5: Move IPU clock lookups into device tree

[Philipp Zabel]

Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 arch/arm/boot/dts/imx51.dtsi        | 2 ++
 arch/arm/boot/dts/imx53.dtsi        | 2 ++
 arch/arm/mach-imx/clk-imx51-imx53.c | 7 -------
 3 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/arch/arm/boot/dts/imx51.dtsi b/arch/arm/boot/dts/imx51.dtsi
index fcf035b..e9480b8 100644
--- a/arch/arm/boot/dts/imx51.dtsi
+++ b/arch/arm/boot/dts/imx51.dtsi
@@ -67,6 +67,8 @@
 			compatible = "fsl,imx51-ipu";
 			reg = <0x40000000 0x20000000>;
 			interrupts = <11 10>;
+			clocks = <&clks 59>, <&clks 110>, <&clks 61>;
+			clock-names = "bus", "di0", "di1";
 		};
 
 		aips@70000000 { /* AIPS1 */
diff --git a/arch/arm/boot/dts/imx53.dtsi b/arch/arm/boot/dts/imx53.dtsi
index d05aa21..7a6f5a8 100644
--- a/arch/arm/boot/dts/imx53.dtsi
+++ b/arch/arm/boot/dts/imx53.dtsi
@@ -72,6 +72,8 @@
 			compatible = "fsl,imx53-ipu";
 			reg = <0x18000000 0x080000000>;
 			interrupts = <11 10>;
+			clocks = <&clks 59>, <&clks 110>, <&clks 61>;
+			clock-names = "bus", "di0", "di1";
 		};
 
 		aips@50000000 { /* AIPS1 */
diff --git a/arch/arm/mach-imx/clk-imx51-imx53.c b/arch/arm/mach-imx/clk-imx51-imx53.c
index 0f39f8c..d22ee6a 100644
--- a/arch/arm/mach-imx/clk-imx51-imx53.c
+++ b/arch/arm/mach-imx/clk-imx51-imx53.c
@@ -362,9 +362,6 @@ int __init mx51_clocks_init(unsigned long rate_ckil, unsigned long rate_osc,
 	clk_register_clkdev(clk[mx51_mipi], "mipi_hsp", NULL);
 	clk_register_clkdev(clk[vpu_gate], NULL, "imx51-vpu.0");
 	clk_register_clkdev(clk[fec_gate], NULL, "imx27-fec.0");
-	clk_register_clkdev(clk[ipu_gate], "bus", "40000000.ipu");
-	clk_register_clkdev(clk[ipu_di0_gate], "di0", "40000000.ipu");
-	clk_register_clkdev(clk[ipu_di1_gate], "di1", "40000000.ipu");
 	clk_register_clkdev(clk[usb_phy_gate], "phy", "mxc-ehci.0");
 	clk_register_clkdev(clk[esdhc1_ipg_gate], "ipg", "sdhci-esdhc-imx51.0");
 	clk_register_clkdev(clk[dummy], "ahb", "sdhci-esdhc-imx51.0");
@@ -471,10 +468,6 @@ int __init mx53_clocks_init(unsigned long rate_ckil, unsigned long rate_osc,
 	clk_register_clkdev(clk[vpu_gate], NULL, "imx53-vpu.0");
 	clk_register_clkdev(clk[i2c3_gate], NULL, "imx21-i2c.2");
 	clk_register_clkdev(clk[fec_gate], NULL, "imx25-fec.0");
-	clk_register_clkdev(clk[ipu_gate], "bus", "18000000.ipu");
-	clk_register_clkdev(clk[ipu_di0_gate], "di0", "18000000.ipu");
-	clk_register_clkdev(clk[ipu_di1_gate], "di1", "18000000.ipu");
-	clk_register_clkdev(clk[ipu_gate], "hsp", "18000000.ipu");
 	clk_register_clkdev(clk[usb_phy1_gate], "usb_phy1", "mxc-ehci.0");
 	clk_register_clkdev(clk[esdhc1_ipg_gate], "ipg", "sdhci-esdhc-imx53.0");
 	clk_register_clkdev(clk[dummy], "ahb", "sdhci-esdhc-imx53.0");
-- 
1.8.2.rc2

Re: [PATCH v2 02/11] ARM i.MX5: Move IPU clock lookups into device tree

[Shawn Guo]

On Wed, Mar 27, 2013 at 06:30:36PM +0100, Philipp Zabel wrote:
> Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

Applied, thanks.

> ---
>  arch/arm/boot/dts/imx51.dtsi        | 2 ++
>  arch/arm/boot/dts/imx53.dtsi        | 2 ++
>  arch/arm/mach-imx/clk-imx51-imx53.c | 7 -------
>  3 files changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/imx51.dtsi b/arch/arm/boot/dts/imx51.dtsi
> index fcf035b..e9480b8 100644
> --- a/arch/arm/boot/dts/imx51.dtsi
> +++ b/arch/arm/boot/dts/imx51.dtsi
> @@ -67,6 +67,8 @@
>  			compatible = "fsl,imx51-ipu";
>  			reg = <0x40000000 0x20000000>;
>  			interrupts = <11 10>;
> +			clocks = <&clks 59>, <&clks 110>, <&clks 61>;
> +			clock-names = "bus", "di0", "di1";
>  		};
>  
>  		aips@70000000 { /* AIPS1 */
> diff --git a/arch/arm/boot/dts/imx53.dtsi b/arch/arm/boot/dts/imx53.dtsi
> index d05aa21..7a6f5a8 100644
> --- a/arch/arm/boot/dts/imx53.dtsi
> +++ b/arch/arm/boot/dts/imx53.dtsi
> @@ -72,6 +72,8 @@
>  			compatible = "fsl,imx53-ipu";
>  			reg = <0x18000000 0x080000000>;
>  			interrupts = <11 10>;
> +			clocks = <&clks 59>, <&clks 110>, <&clks 61>;
> +			clock-names = "bus", "di0", "di1";
>  		};
>  
>  		aips@50000000 { /* AIPS1 */
> diff --git a/arch/arm/mach-imx/clk-imx51-imx53.c b/arch/arm/mach-imx/clk-imx51-imx53.c
> index 0f39f8c..d22ee6a 100644
> --- a/arch/arm/mach-imx/clk-imx51-imx53.c
> +++ b/arch/arm/mach-imx/clk-imx51-imx53.c
> @@ -362,9 +362,6 @@ int __init mx51_clocks_init(unsigned long rate_ckil, unsigned long rate_osc,
>  	clk_register_clkdev(clk[mx51_mipi], "mipi_hsp", NULL);
>  	clk_register_clkdev(clk[vpu_gate], NULL, "imx51-vpu.0");
>  	clk_register_clkdev(clk[fec_gate], NULL, "imx27-fec.0");
> -	clk_register_clkdev(clk[ipu_gate], "bus", "40000000.ipu");
> -	clk_register_clkdev(clk[ipu_di0_gate], "di0", "40000000.ipu");
> -	clk_register_clkdev(clk[ipu_di1_gate], "di1", "40000000.ipu");
>  	clk_register_clkdev(clk[usb_phy_gate], "phy", "mxc-ehci.0");
>  	clk_register_clkdev(clk[esdhc1_ipg_gate], "ipg", "sdhci-esdhc-imx51.0");
>  	clk_register_clkdev(clk[dummy], "ahb", "sdhci-esdhc-imx51.0");
> @@ -471,10 +468,6 @@ int __init mx53_clocks_init(unsigned long rate_ckil, unsigned long rate_osc,
>  	clk_register_clkdev(clk[vpu_gate], NULL, "imx53-vpu.0");
>  	clk_register_clkdev(clk[i2c3_gate], NULL, "imx21-i2c.2");
>  	clk_register_clkdev(clk[fec_gate], NULL, "imx25-fec.0");
> -	clk_register_clkdev(clk[ipu_gate], "bus", "18000000.ipu");
> -	clk_register_clkdev(clk[ipu_di0_gate], "di0", "18000000.ipu");
> -	clk_register_clkdev(clk[ipu_di1_gate], "di1", "18000000.ipu");
> -	clk_register_clkdev(clk[ipu_gate], "hsp", "18000000.ipu");
>  	clk_register_clkdev(clk[usb_phy1_gate], "usb_phy1", "mxc-ehci.0");
>  	clk_register_clkdev(clk[esdhc1_ipg_gate], "ipg", "sdhci-esdhc-imx53.0");
>  	clk_register_clkdev(clk[dummy], "ahb", "sdhci-esdhc-imx53.0");
> -- 
> 1.8.2.rc2
> 

[PATCH v2 04/11] ARM i.MX6q: Add audio/video PLL post dividers for i.MX6q rev 1.1

[Philipp Zabel]

Query silicon revision to determine clock tree and add post
dividers for newer revisions.

Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 arch/arm/mach-imx/clk-imx6q.c | 30 +++++++++++++++++++++++-------
 1 file changed, 23 insertions(+), 7 deletions(-)

diff --git a/arch/arm/mach-imx/clk-imx6q.c b/arch/arm/mach-imx/clk-imx6q.c
index 2f9ff93..fbab4a9 100644
--- a/arch/arm/mach-imx/clk-imx6q.c
+++ b/arch/arm/mach-imx/clk-imx6q.c
@@ -22,6 +22,7 @@
 
 #include "clk.h"
 #include "common.h"
+#include "hardware.h"
 
 #define CCGR0				0x68
 #define CCGR1				0x6c
@@ -109,29 +110,32 @@ static const char *periph_clk2_sels[]	= { "pll3_usb_otg", "osc", };
 static const char *periph_sels[]	= { "periph_pre", "periph_clk2", };
 static const char *periph2_sels[]	= { "periph2_pre", "periph2_clk2", };
 static const char *axi_sels[]		= { "periph", "pll2_pfd2_396m", "pll3_pfd1_540m", };
-static const char *audio_sels[]	= { "pll4_audio", "pll3_pfd2_508m", "pll3_pfd3_454m", "pll3_usb_otg", };
+static const char *audio_sels[]	= { "pll4_test_div", "pll3_pfd2_508m", "pll3_pfd3_454m", "pll3_usb_otg", };
 static const char *gpu_axi_sels[]	= { "axi", "ahb", };
 static const char *gpu2d_core_sels[]	= { "axi", "pll3_usb_otg", "pll2_pfd0_352m", "pll2_pfd2_396m", };
 static const char *gpu3d_core_sels[]	= { "mmdc_ch0_axi", "pll3_usb_otg", "pll2_pfd1_594m", "pll2_pfd2_396m", };
 static const char *gpu3d_shader_sels[] = { "mmdc_ch0_axi", "pll3_usb_otg", "pll2_pfd1_594m", "pll2_pfd9_720m", };
 static const char *ipu_sels[]		= { "mmdc_ch0_axi", "pll2_pfd2_396m", "pll3_120m", "pll3_pfd1_540m", };
-static const char *ldb_di_sels[]	= { "pll5_video", "pll2_pfd0_352m", "pll2_pfd2_396m", "mmdc_ch1_axi", "pll3_pfd1_540m", };
-static const char *ipu_di_pre_sels[]	= { "mmdc_ch0_axi", "pll3_usb_otg", "pll5_video", "pll2_pfd0_352m", "pll2_pfd2_396m", "pll3_pfd1_540m", };
+static const char *ldb_di_sels[]	= { "pll5_control3", "pll2_pfd0_352m", "pll2_pfd2_396m", "mmdc_ch1_axi", "pll3_pfd1_540m", };
+static const char *ipu_di_pre_sels[]	= { "mmdc_ch0_axi", "pll3_usb_otg", "pll5_control3", "pll2_pfd0_352m", "pll2_pfd2_396m", "pll3_pfd1_540m", };
 static const char *ipu1_di0_sels[]	= { "ipu1_di0_pre", "dummy", "dummy", "ldb_di0", "ldb_di1", };
 static const char *ipu1_di1_sels[]	= { "ipu1_di1_pre", "dummy", "dummy", "ldb_di0", "ldb_di1", };
 static const char *ipu2_di0_sels[]	= { "ipu2_di0_pre", "dummy", "dummy", "ldb_di0", "ldb_di1", };
 static const char *ipu2_di1_sels[]	= { "ipu2_di1_pre", "dummy", "dummy", "ldb_di0", "ldb_di1", };
 static const char *hsi_tx_sels[]	= { "pll3_120m", "pll2_pfd2_396m", };
 static const char *pcie_axi_sels[]	= { "axi", "ahb", };
-static const char *ssi_sels[]		= { "pll3_pfd2_508m", "pll3_pfd3_454m", "pll4_audio", };
+static const char *ssi_sels[]		= { "pll3_pfd2_508m", "pll3_pfd3_454m", "pll4_test_div", };
 static const char *usdhc_sels[]	= { "pll2_pfd2_396m", "pll2_pfd0_352m", };
 static const char *enfc_sels[]	= { "pll2_pfd0_352m", "pll2_bus", "pll3_usb_otg", "pll2_pfd2_396m", };
 static const char *emi_sels[]		= { "axi", "pll3_usb_otg", "pll2_pfd2_396m", "pll2_pfd0_352m", };
 static const char *vdo_axi_sels[]	= { "axi", "ahb", };
 static const char *vpu_axi_sels[]	= { "axi", "pll2_pfd2_396m", "pll2_pfd0_352m", };
-static const char *cko1_sels[]	= { "pll3_usb_otg", "pll2_bus", "pll1_sys", "pll5_video",
+static const char *cko1_sels[]	= { "pll3_usb_otg", "pll2_bus", "pll1_sys", "pll5_control3",
 				    "dummy", "axi", "enfc", "ipu1_di0", "ipu1_di1", "ipu2_di0",
-				    "ipu2_di1", "ahb", "ipg", "ipg_per", "ckil", "pll4_audio", };
+				    "ipu2_di1", "ahb", "ipg", "ipg_per", "ckil", "pll4_test_div", };
+
+static struct clk_div_table test_div_table[] = { { 2, 1 }, { 1, 2 }, { 0, 4 }, { 0, 0 }, };
+static struct clk_div_table control3_table[] = { { 0, 1 }, { 1, 2 }, { 3, 4 }, { 0, 0 }, };
 
 enum mx6q_clks {
 	dummy, ckil, ckih, osc, pll2_pfd0_352m, pll2_pfd1_594m, pll2_pfd2_396m,
@@ -165,7 +169,7 @@ enum mx6q_clks {
 	pll4_audio, pll5_video, pll8_mlb, pll7_usb_host, pll6_enet, ssi1_ipg,
 	ssi2_ipg, ssi3_ipg, rom, usbphy1, usbphy2, ldb_di0_div_3_5, ldb_di1_div_3_5,
 	sata_ref, sata_ref_100m, pcie_ref, pcie_ref_125m, enet_ref, usbphy1_gate,
-	usbphy2_gate, clk_max
+	usbphy2_gate, pll4_test_div, pll5_test_div, pll5_control3, clk_max
 };
 
 static struct clk *clk[clk_max];
@@ -208,6 +212,14 @@ int __init mx6q_clocks_init(void)
 	base = of_iomap(np, 0);
 	WARN_ON(!base);
 
+	/* Audio/video PLL post dividers do not work on i.MX6q revision 1.0 */
+	if (imx6q_revision() == IMX_CHIP_REVISION_1_0) {
+		test_div_table[1].div = 1;
+		test_div_table[2].div = 1;
+		control3_table[1].div = 1;
+		control3_table[2].div = 1;
+	};
+
 	/*                   type                               name         parent_name  base     div_mask */
 	clk[pll1_sys]      = imx_clk_pllv3(IMX_PLLV3_SYS,	"pll1_sys",	"osc", base,        0x7f);
 	clk[pll2_bus]      = imx_clk_pllv3(IMX_PLLV3_GENERIC,	"pll2_bus",	"osc", base + 0x30, 0x1);
@@ -260,6 +272,10 @@ int __init mx6q_clocks_init(void)
 	clk[pll3_60m]  = imx_clk_fixed_factor("pll3_60m",  "pll3_usb_otg",   1, 8);
 	clk[twd]       = imx_clk_fixed_factor("twd",       "arm",            1, 2);
 
+	clk[pll4_test_div] = clk_register_divider_table(NULL, "pll4_test_div", "pll4_audio", CLK_SET_RATE_PARENT, base + 0x70, 19, 2, 0, test_div_table, NULL);
+	clk[pll5_test_div] = clk_register_divider_table(NULL, "pll5_test_div", "pll5_video", CLK_SET_RATE_PARENT, base + 0xa0, 19, 2, 0, test_div_table, NULL);
+	clk[pll5_control3] = clk_register_divider_table(NULL, "pll5_control3", "pll5_test_div", CLK_SET_RATE_PARENT, base + 0x170, 30, 2, 0, control3_table, NULL);
+
 	np = of_find_compatible_node(NULL, NULL, "fsl,imx6q-ccm");
 	base = of_iomap(np, 0);
 	WARN_ON(!base);
-- 
1.8.2.rc2

Re: [PATCH v2 04/11] ARM i.MX6q: Add audio/video PLL post dividers for i.MX6q rev 1.1

[Shawn Guo]

On Wed, Mar 27, 2013 at 06:30:38PM +0100, Philipp Zabel wrote:
> Query silicon revision to determine clock tree and add post
> dividers for newer revisions.
> 
> Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> ---
>  arch/arm/mach-imx/clk-imx6q.c | 30 +++++++++++++++++++++++-------
>  1 file changed, 23 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm/mach-imx/clk-imx6q.c b/arch/arm/mach-imx/clk-imx6q.c
> index 2f9ff93..fbab4a9 100644
> --- a/arch/arm/mach-imx/clk-imx6q.c
> +++ b/arch/arm/mach-imx/clk-imx6q.c
> @@ -22,6 +22,7 @@
>  
>  #include "clk.h"
>  #include "common.h"
> +#include "hardware.h"
>  
>  #define CCGR0				0x68
>  #define CCGR1				0x6c
> @@ -109,29 +110,32 @@ static const char *periph_clk2_sels[]	= { "pll3_usb_otg", "osc", };
>  static const char *periph_sels[]	= { "periph_pre", "periph_clk2", };
>  static const char *periph2_sels[]	= { "periph2_pre", "periph2_clk2", };
>  static const char *axi_sels[]		= { "periph", "pll2_pfd2_396m", "pll3_pfd1_540m", };
> -static const char *audio_sels[]	= { "pll4_audio", "pll3_pfd2_508m", "pll3_pfd3_454m", "pll3_usb_otg", };
> +static const char *audio_sels[]	= { "pll4_test_div", "pll3_pfd2_508m", "pll3_pfd3_454m", "pll3_usb_otg", };
>  static const char *gpu_axi_sels[]	= { "axi", "ahb", };
>  static const char *gpu2d_core_sels[]	= { "axi", "pll3_usb_otg", "pll2_pfd0_352m", "pll2_pfd2_396m", };
>  static const char *gpu3d_core_sels[]	= { "mmdc_ch0_axi", "pll3_usb_otg", "pll2_pfd1_594m", "pll2_pfd2_396m", };
>  static const char *gpu3d_shader_sels[] = { "mmdc_ch0_axi", "pll3_usb_otg", "pll2_pfd1_594m", "pll2_pfd9_720m", };
>  static const char *ipu_sels[]		= { "mmdc_ch0_axi", "pll2_pfd2_396m", "pll3_120m", "pll3_pfd1_540m", };
> -static const char *ldb_di_sels[]	= { "pll5_video", "pll2_pfd0_352m", "pll2_pfd2_396m", "mmdc_ch1_axi", "pll3_pfd1_540m", };
> -static const char *ipu_di_pre_sels[]	= { "mmdc_ch0_axi", "pll3_usb_otg", "pll5_video", "pll2_pfd0_352m", "pll2_pfd2_396m", "pll3_pfd1_540m", };
> +static const char *ldb_di_sels[]	= { "pll5_control3", "pll2_pfd0_352m", "pll2_pfd2_396m", "mmdc_ch1_axi", "pll3_pfd1_540m", };
> +static const char *ipu_di_pre_sels[]	= { "mmdc_ch0_axi", "pll3_usb_otg", "pll5_control3", "pll2_pfd0_352m", "pll2_pfd2_396m", "pll3_pfd1_540m", };
>  static const char *ipu1_di0_sels[]	= { "ipu1_di0_pre", "dummy", "dummy", "ldb_di0", "ldb_di1", };
>  static const char *ipu1_di1_sels[]	= { "ipu1_di1_pre", "dummy", "dummy", "ldb_di0", "ldb_di1", };
>  static const char *ipu2_di0_sels[]	= { "ipu2_di0_pre", "dummy", "dummy", "ldb_di0", "ldb_di1", };
>  static const char *ipu2_di1_sels[]	= { "ipu2_di1_pre", "dummy", "dummy", "ldb_di0", "ldb_di1", };
>  static const char *hsi_tx_sels[]	= { "pll3_120m", "pll2_pfd2_396m", };
>  static const char *pcie_axi_sels[]	= { "axi", "ahb", };
> -static const char *ssi_sels[]		= { "pll3_pfd2_508m", "pll3_pfd3_454m", "pll4_audio", };
> +static const char *ssi_sels[]		= { "pll3_pfd2_508m", "pll3_pfd3_454m", "pll4_test_div", };
>  static const char *usdhc_sels[]	= { "pll2_pfd2_396m", "pll2_pfd0_352m", };
>  static const char *enfc_sels[]	= { "pll2_pfd0_352m", "pll2_bus", "pll3_usb_otg", "pll2_pfd2_396m", };
>  static const char *emi_sels[]		= { "axi", "pll3_usb_otg", "pll2_pfd2_396m", "pll2_pfd0_352m", };
>  static const char *vdo_axi_sels[]	= { "axi", "ahb", };
>  static const char *vpu_axi_sels[]	= { "axi", "pll2_pfd2_396m", "pll2_pfd0_352m", };
> -static const char *cko1_sels[]	= { "pll3_usb_otg", "pll2_bus", "pll1_sys", "pll5_video",
> +static const char *cko1_sels[]	= { "pll3_usb_otg", "pll2_bus", "pll1_sys", "pll5_control3",
>  				    "dummy", "axi", "enfc", "ipu1_di0", "ipu1_di1", "ipu2_di0",
> -				    "ipu2_di1", "ahb", "ipg", "ipg_per", "ckil", "pll4_audio", };
> +				    "ipu2_di1", "ahb", "ipg", "ipg_per", "ckil", "pll4_test_div", };
> +
> +static struct clk_div_table test_div_table[] = { { 2, 1 }, { 1, 2 }, { 0, 4 }, { 0, 0 }, };
> +static struct clk_div_table control3_table[] = { { 0, 1 }, { 1, 2 }, { 3, 4 }, { 0, 0 }, };

Please rewrite the tables in the way how clk_enet_ref_table is written.
Also, why { 0, 0 } at the end of both tables?

>  
>  enum mx6q_clks {
>  	dummy, ckil, ckih, osc, pll2_pfd0_352m, pll2_pfd1_594m, pll2_pfd2_396m,
> @@ -165,7 +169,7 @@ enum mx6q_clks {
>  	pll4_audio, pll5_video, pll8_mlb, pll7_usb_host, pll6_enet, ssi1_ipg,
>  	ssi2_ipg, ssi3_ipg, rom, usbphy1, usbphy2, ldb_di0_div_3_5, ldb_di1_div_3_5,
>  	sata_ref, sata_ref_100m, pcie_ref, pcie_ref_125m, enet_ref, usbphy1_gate,
> -	usbphy2_gate, clk_max
> +	usbphy2_gate, pll4_test_div, pll5_test_div, pll5_control3, clk_max
>  };
>  
>  static struct clk *clk[clk_max];
> @@ -208,6 +212,14 @@ int __init mx6q_clocks_init(void)
>  	base = of_iomap(np, 0);
>  	WARN_ON(!base);
>  
> +	/* Audio/video PLL post dividers do not work on i.MX6q revision 1.0 */
> +	if (imx6q_revision() == IMX_CHIP_REVISION_1_0) {
> +		test_div_table[1].div = 1;
> +		test_div_table[2].div = 1;
> +		control3_table[1].div = 1;
> +		control3_table[2].div = 1;
> +	};
> +
>  	/*                   type                               name         parent_name  base     div_mask */
>  	clk[pll1_sys]      = imx_clk_pllv3(IMX_PLLV3_SYS,	"pll1_sys",	"osc", base,        0x7f);
>  	clk[pll2_bus]      = imx_clk_pllv3(IMX_PLLV3_GENERIC,	"pll2_bus",	"osc", base + 0x30, 0x1);
> @@ -260,6 +272,10 @@ int __init mx6q_clocks_init(void)
>  	clk[pll3_60m]  = imx_clk_fixed_factor("pll3_60m",  "pll3_usb_otg",   1, 8);
>  	clk[twd]       = imx_clk_fixed_factor("twd",       "arm",            1, 2);
>  
> +	clk[pll4_test_div] = clk_register_divider_table(NULL, "pll4_test_div", "pll4_audio", CLK_SET_RATE_PARENT, base + 0x70, 19, 2, 0, test_div_table, NULL);
> +	clk[pll5_test_div] = clk_register_divider_table(NULL, "pll5_test_div", "pll5_video", CLK_SET_RATE_PARENT, base + 0xa0, 19, 2, 0, test_div_table, NULL);
> +	clk[pll5_control3] = clk_register_divider_table(NULL, "pll5_control3", "pll5_test_div", CLK_SET_RATE_PARENT, base + 0x170, 30, 2, 0, control3_table, NULL);
> +

I'm wondering how the dividers are named.  Why "test" and "control3"?

Also, shouldn't the last argument be &imx_ccm_lock rather than NULL?

Shawn

>  	np = of_find_compatible_node(NULL, NULL, "fsl,imx6q-ccm");
>  	base = of_iomap(np, 0);
>  	WARN_ON(!base);
> -- 
> 1.8.2.rc2
> 

Re: [PATCH v2 04/11] ARM i.MX6q: Add audio/video PLL post dividers for i.MX6q rev 1.1

[Philipp Zabel]

Am Donnerstag, den 28.03.2013, 15:20 +0800 schrieb Shawn Guo:
> On Wed, Mar 27, 2013 at 06:30:38PM +0100, Philipp Zabel wrote:
> > Query silicon revision to determine clock tree and add post
> > dividers for newer revisions.
> > 
> > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> > ---
> >  arch/arm/mach-imx/clk-imx6q.c | 30 +++++++++++++++++++++++-------
> >  1 file changed, 23 insertions(+), 7 deletions(-)
> > 
> > diff --git a/arch/arm/mach-imx/clk-imx6q.c b/arch/arm/mach-imx/clk-imx6q.c
> > index 2f9ff93..fbab4a9 100644
> > --- a/arch/arm/mach-imx/clk-imx6q.c
> > +++ b/arch/arm/mach-imx/clk-imx6q.c
> > @@ -22,6 +22,7 @@
> >  
> >  #include "clk.h"
> >  #include "common.h"
> > +#include "hardware.h"
> >  
> >  #define CCGR0				0x68
> >  #define CCGR1				0x6c
> > @@ -109,29 +110,32 @@ static const char *periph_clk2_sels[]	= { "pll3_usb_otg", "osc", };
> >  static const char *periph_sels[]	= { "periph_pre", "periph_clk2", };
> >  static const char *periph2_sels[]	= { "periph2_pre", "periph2_clk2", };
> >  static const char *axi_sels[]		= { "periph", "pll2_pfd2_396m", "pll3_pfd1_540m", };
> > -static const char *audio_sels[]	= { "pll4_audio", "pll3_pfd2_508m", "pll3_pfd3_454m", "pll3_usb_otg", };
> > +static const char *audio_sels[]	= { "pll4_test_div", "pll3_pfd2_508m", "pll3_pfd3_454m", "pll3_usb_otg", };
> >  static const char *gpu_axi_sels[]	= { "axi", "ahb", };
> >  static const char *gpu2d_core_sels[]	= { "axi", "pll3_usb_otg", "pll2_pfd0_352m", "pll2_pfd2_396m", };
> >  static const char *gpu3d_core_sels[]	= { "mmdc_ch0_axi", "pll3_usb_otg", "pll2_pfd1_594m", "pll2_pfd2_396m", };
> >  static const char *gpu3d_shader_sels[] = { "mmdc_ch0_axi", "pll3_usb_otg", "pll2_pfd1_594m", "pll2_pfd9_720m", };
> >  static const char *ipu_sels[]		= { "mmdc_ch0_axi", "pll2_pfd2_396m", "pll3_120m", "pll3_pfd1_540m", };
> > -static const char *ldb_di_sels[]	= { "pll5_video", "pll2_pfd0_352m", "pll2_pfd2_396m", "mmdc_ch1_axi", "pll3_pfd1_540m", };
> > -static const char *ipu_di_pre_sels[]	= { "mmdc_ch0_axi", "pll3_usb_otg", "pll5_video", "pll2_pfd0_352m", "pll2_pfd2_396m", "pll3_pfd1_540m", };
> > +static const char *ldb_di_sels[]	= { "pll5_control3", "pll2_pfd0_352m", "pll2_pfd2_396m", "mmdc_ch1_axi", "pll3_pfd1_540m", };
> > +static const char *ipu_di_pre_sels[]	= { "mmdc_ch0_axi", "pll3_usb_otg", "pll5_control3", "pll2_pfd0_352m", "pll2_pfd2_396m", "pll3_pfd1_540m", };
> >  static const char *ipu1_di0_sels[]	= { "ipu1_di0_pre", "dummy", "dummy", "ldb_di0", "ldb_di1", };
> >  static const char *ipu1_di1_sels[]	= { "ipu1_di1_pre", "dummy", "dummy", "ldb_di0", "ldb_di1", };
> >  static const char *ipu2_di0_sels[]	= { "ipu2_di0_pre", "dummy", "dummy", "ldb_di0", "ldb_di1", };
> >  static const char *ipu2_di1_sels[]	= { "ipu2_di1_pre", "dummy", "dummy", "ldb_di0", "ldb_di1", };
> >  static const char *hsi_tx_sels[]	= { "pll3_120m", "pll2_pfd2_396m", };
> >  static const char *pcie_axi_sels[]	= { "axi", "ahb", };
> > -static const char *ssi_sels[]		= { "pll3_pfd2_508m", "pll3_pfd3_454m", "pll4_audio", };
> > +static const char *ssi_sels[]		= { "pll3_pfd2_508m", "pll3_pfd3_454m", "pll4_test_div", };
> >  static const char *usdhc_sels[]	= { "pll2_pfd2_396m", "pll2_pfd0_352m", };
> >  static const char *enfc_sels[]	= { "pll2_pfd0_352m", "pll2_bus", "pll3_usb_otg", "pll2_pfd2_396m", };
> >  static const char *emi_sels[]		= { "axi", "pll3_usb_otg", "pll2_pfd2_396m", "pll2_pfd0_352m", };
> >  static const char *vdo_axi_sels[]	= { "axi", "ahb", };
> >  static const char *vpu_axi_sels[]	= { "axi", "pll2_pfd2_396m", "pll2_pfd0_352m", };
> > -static const char *cko1_sels[]	= { "pll3_usb_otg", "pll2_bus", "pll1_sys", "pll5_video",
> > +static const char *cko1_sels[]	= { "pll3_usb_otg", "pll2_bus", "pll1_sys", "pll5_control3",
> >  				    "dummy", "axi", "enfc", "ipu1_di0", "ipu1_di1", "ipu2_di0",
> > -				    "ipu2_di1", "ahb", "ipg", "ipg_per", "ckil", "pll4_audio", };
> > +				    "ipu2_di1", "ahb", "ipg", "ipg_per", "ckil", "pll4_test_div", };
> > +
> > +static struct clk_div_table test_div_table[] = { { 2, 1 }, { 1, 2 }, { 0, 4 }, { 0, 0 }, };
> > +static struct clk_div_table control3_table[] = { { 0, 1 }, { 1, 2 }, { 3, 4 }, { 0, 0 }, };
> 
> Please rewrite the tables in the way how clk_enet_ref_table is written.
> Also, why { 0, 0 } at the end of both tables?

In the loops in _get_table_maxdiv(), _get_table_div(), and
_get_table_val(), in drivers/clk/clk-divider.c the exit condition
is .div == 0, so there needs to be a sentinel with .div = 0 at the end
of each clk_div_table. It's also documented in the kerneldoc comment for
clk_register_divider_table.
I can write that as "{ }", like this:

static struct clk_div_table test_div_table[] = {
        { .val = 2, .div = 1 },
        { .val = 1, .div = 2 },
        { .val = 0, .div = 4 },
        { }
};

static struct clk_div_table control3_table[] = {
        { .val = 0, .div = 1 },
        { .val = 1, .div = 2 },
        { .val = 3, .div = 4 },
        { }
};

> >  enum mx6q_clks {
> >  	dummy, ckil, ckih, osc, pll2_pfd0_352m, pll2_pfd1_594m, pll2_pfd2_396m,
> > @@ -165,7 +169,7 @@ enum mx6q_clks {
> >  	pll4_audio, pll5_video, pll8_mlb, pll7_usb_host, pll6_enet, ssi1_ipg,
> >  	ssi2_ipg, ssi3_ipg, rom, usbphy1, usbphy2, ldb_di0_div_3_5, ldb_di1_div_3_5,
> >  	sata_ref, sata_ref_100m, pcie_ref, pcie_ref_125m, enet_ref, usbphy1_gate,
> > -	usbphy2_gate, clk_max
> > +	usbphy2_gate, pll4_test_div, pll5_test_div, pll5_control3, clk_max
> >  };
> >  
> >  static struct clk *clk[clk_max];
> > @@ -208,6 +212,14 @@ int __init mx6q_clocks_init(void)
> >  	base = of_iomap(np, 0);
> >  	WARN_ON(!base);
> >  
> > +	/* Audio/video PLL post dividers do not work on i.MX6q revision 1.0 */
> > +	if (imx6q_revision() == IMX_CHIP_REVISION_1_0) {
> > +		test_div_table[1].div = 1;
> > +		test_div_table[2].div = 1;
> > +		control3_table[1].div = 1;
> > +		control3_table[2].div = 1;
> > +	};
> > +
> >  	/*                   type                               name         parent_name  base     div_mask */
> >  	clk[pll1_sys]      = imx_clk_pllv3(IMX_PLLV3_SYS,	"pll1_sys",	"osc", base,        0x7f);
> >  	clk[pll2_bus]      = imx_clk_pllv3(IMX_PLLV3_GENERIC,	"pll2_bus",	"osc", base + 0x30, 0x1);
> > @@ -260,6 +272,10 @@ int __init mx6q_clocks_init(void)
> >  	clk[pll3_60m]  = imx_clk_fixed_factor("pll3_60m",  "pll3_usb_otg",   1, 8);
> >  	clk[twd]       = imx_clk_fixed_factor("twd",       "arm",            1, 2);
> >  
> > +	clk[pll4_test_div] = clk_register_divider_table(NULL, "pll4_test_div", "pll4_audio", CLK_SET_RATE_PARENT, base + 0x70, 19, 2, 0, test_div_table, NULL);
> > +	clk[pll5_test_div] = clk_register_divider_table(NULL, "pll5_test_div", "pll5_video", CLK_SET_RATE_PARENT, base + 0xa0, 19, 2, 0, test_div_table, NULL);
> > +	clk[pll5_control3] = clk_register_divider_table(NULL, "pll5_control3", "pll5_test_div", CLK_SET_RATE_PARENT, base + 0x170, 30, 2, 0, control3_table, NULL);
> > +
> 
> I'm wondering how the dividers are named.  Why "test" and "control3"?

Because I didn't realize that the names were fixed in the final
documentation. I'll rename pll[45]_test_div to pll[45]_post_div.
pll5_control3 should be renamed to pll5_video_div, I guess.

This is documented in chapter 18.7 CCM Analog Memory Map/Register
Definition of the i.MX 6Dual/6Quad Applications Processor Reference
Manual Rev. 0.

> Also, shouldn't the last argument be &imx_ccm_lock rather than NULL?

Yes, I'll fix that.

regards
Philipp

Re: [PATCH v2 04/11] ARM i.MX6q: Add audio/video PLL post dividers for i.MX6q rev 1.1

[Shawn Guo]

On Thu, Mar 28, 2013 at 10:58:00AM +0100, Philipp Zabel wrote:
> In the loops in _get_table_maxdiv(), _get_table_div(), and
> _get_table_val(), in drivers/clk/clk-divider.c the exit condition
> is .div == 0, so there needs to be a sentinel with .div = 0 at the end
> of each clk_div_table. It's also documented in the kerneldoc comment for
> clk_register_divider_table.

Ah, I do not know that.  So clk_enet_ref_table is actually buggy?

> I can write that as "{ }", like this:
> 
> static struct clk_div_table test_div_table[] = {
>         { .val = 2, .div = 1 },
>         { .val = 1, .div = 2 },
>         { .val = 0, .div = 4 },
>         { }
> };
> 
Yes, that's much better for reading.  But to be consistent with
clk_enet_ref_table, please also put a comma after .div.

> static struct clk_div_table control3_table[] = {
>         { .val = 0, .div = 1 },
>         { .val = 1, .div = 2 },
>         { .val = 3, .div = 4 },
>         { }
> };
> 
Per Reference Manual, we should have one more entry below?

	{ .val = 2, .div = 1, }

...

> Because I didn't realize that the names were fixed in the final
> documentation. I'll rename pll[45]_test_div to pll[45]_post_div.
> pll5_control3 should be renamed to pll5_video_div, I guess.

Yea, that's much easier for users to map code and document.

Shawn

Re: [PATCH v2 04/11] ARM i.MX6q: Add audio/video PLL post dividers for i.MX6q rev 1.1

[Philipp Zabel]

Am Donnerstag, den 28.03.2013, 22:43 +0800 schrieb Shawn Guo:
> On Thu, Mar 28, 2013 at 10:58:00AM +0100, Philipp Zabel wrote:
> > In the loops in _get_table_maxdiv(), _get_table_div(), and
> > _get_table_val(), in drivers/clk/clk-divider.c the exit condition
> > is .div == 0, so there needs to be a sentinel with .div = 0 at the end
> > of each clk_div_table. It's also documented in the kerneldoc comment for
> > clk_register_divider_table.
> 
> Ah, I do not know that.  So clk_enet_ref_table is actually buggy?

Yes. For some reason I thought that for 2 bit dividers, the code would
stop after four entries, but that is not true. The clk_enet_ref_table is
missing the final { .div = 0 } entry.

> > I can write that as "{ }", like this:
> > 
> > static struct clk_div_table test_div_table[] = {
> >         { .val = 2, .div = 1 },
> >         { .val = 1, .div = 2 },
> >         { .val = 0, .div = 4 },
> >         { }
> > };
> > 
> Yes, that's much better for reading.  But to be consistent with
> clk_enet_ref_table, please also put a comma after .div.

I'll do that.

> > static struct clk_div_table control3_table[] = {
> >         { .val = 0, .div = 1 },
> >         { .val = 1, .div = 2 },
> >         { .val = 3, .div = 4 },
> >         { }
> > };
> > 
> Per Reference Manual, we should have one more entry below?
> 
> 	{ .val = 2, .div = 1, }

Ok. The clock code will always choose val=0, but in the unlikely case
that some boot code writes val=2 before starting Linux, at least we can
detect it correctly.

> ...
> 
> > Because I didn't realize that the names were fixed in the final
> > documentation. I'll rename pll[45]_test_div to pll[45]_post_div.
> > pll5_control3 should be renamed to pll5_video_div, I guess.
> 
> Yea, that's much easier for users to map code and document.

regards
Philipp

[PATCH v2 06/11] ARM i.MX: Add imx_clk_divider_flags and imx_clk_mux_flags

[Philipp Zabel]

The default is for dividers to set CLK_SET_PARENT_RATE and for muxes to
not set that flag. In the LDB clock tree, we need the opposite, so add
functions to create divider and mux clocks with configurable flags.

Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 arch/arm/mach-imx/clk.h | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/arch/arm/mach-imx/clk.h b/arch/arm/mach-imx/clk.h
index 9d1f3b9..d9d9d9c 100644
--- a/arch/arm/mach-imx/clk.h
+++ b/arch/arm/mach-imx/clk.h
@@ -59,6 +59,14 @@ static inline struct clk *imx_clk_divider(const char *name, const char *parent,
 			reg, shift, width, 0, &imx_ccm_lock);
 }
 
+static inline struct clk *imx_clk_divider_flags(const char *name,
+		const char *parent, void __iomem *reg, u8 shift, u8 width,
+		unsigned long flags)
+{
+	return clk_register_divider(NULL, name, parent, flags,
+			reg, shift, width, 0, &imx_ccm_lock);
+}
+
 static inline struct clk *imx_clk_gate(const char *name, const char *parent,
 		void __iomem *reg, u8 shift)
 {
@@ -73,6 +81,15 @@ static inline struct clk *imx_clk_mux(const char *name, void __iomem *reg,
 			width, 0, &imx_ccm_lock);
 }
 
+static inline struct clk *imx_clk_mux_flags(const char *name,
+		void __iomem *reg, u8 shift, u8 width, const char **parents,
+		int num_parents, unsigned long flags)
+{
+	return clk_register_mux(NULL, name, parents, num_parents,
+			flags, reg, shift, width, 0,
+			&imx_ccm_lock);
+}
+
 static inline struct clk *imx_clk_fixed_factor(const char *name,
 		const char *parent, unsigned int mult, unsigned int div)
 {
-- 
1.8.2.rc2

Re: [PATCH v2 06/11] ARM i.MX: Add imx_clk_divider_flags and imx_clk_mux_flags

[Shawn Guo]

On Wed, Mar 27, 2013 at 06:30:40PM +0100, Philipp Zabel wrote:
> The default is for dividers to set CLK_SET_PARENT_RATE and for muxes to
> not set that flag. In the LDB clock tree, we need the opposite, so add
> functions to create divider and mux clocks with configurable flags.
> 
> Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

Applied, thanks.

[PATCH v2 07/11] ARM i.MX53: fix ldb di divider and selector clocks

[Philipp Zabel]

Use imx_clk_mux_flags and imx_clk_divider_flags to set the appropriate
flags for the LDB display interface divider and selector clocks.

Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 arch/arm/mach-imx/clk-imx51-imx53.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-imx/clk-imx51-imx53.c b/arch/arm/mach-imx/clk-imx51-imx53.c
index d22ee6a..ed08aba 100644
--- a/arch/arm/mach-imx/clk-imx51-imx53.c
+++ b/arch/arm/mach-imx/clk-imx51-imx53.c
@@ -420,15 +420,15 @@ int __init mx53_clocks_init(unsigned long rate_ckil, unsigned long rate_osc,
 	clk[pll3_sw] = imx_clk_pllv2("pll3_sw", "osc", MX53_DPLL3_BASE);
 	clk[pll4_sw] = imx_clk_pllv2("pll4_sw", "osc", MX53_DPLL4_BASE);
 
-	clk[ldb_di1_sel] = imx_clk_mux("ldb_di1_sel", MXC_CCM_CSCMR2, 9, 1,
-				mx53_ldb_di1_sel, ARRAY_SIZE(mx53_ldb_di1_sel));
 	clk[ldb_di1_div_3_5] = imx_clk_fixed_factor("ldb_di1_div_3_5", "ldb_di1_sel", 2, 7);
-	clk[ldb_di1_div] = imx_clk_divider("ldb_di1_div", "ldb_di1_div_3_5", MXC_CCM_CSCMR2, 11, 1);
+	clk[ldb_di1_div] = imx_clk_divider_flags("ldb_di1_div", "ldb_di1_div_3_5", MXC_CCM_CSCMR2, 11, 1, 0);
+	clk[ldb_di1_sel] = imx_clk_mux_flags("ldb_di1_sel", MXC_CCM_CSCMR2, 9, 1,
+				mx53_ldb_di1_sel, ARRAY_SIZE(mx53_ldb_di1_sel), CLK_SET_RATE_PARENT);
 	clk[di_pll4_podf] = imx_clk_divider("di_pll4_podf", "pll4_sw", MXC_CCM_CDCDR, 16, 3);
-	clk[ldb_di0_sel] = imx_clk_mux("ldb_di0_sel", MXC_CCM_CSCMR2, 8, 1,
-				mx53_ldb_di0_sel, ARRAY_SIZE(mx53_ldb_di0_sel));
 	clk[ldb_di0_div_3_5] = imx_clk_fixed_factor("ldb_di0_div_3_5", "ldb_di0_sel", 2, 7);
-	clk[ldb_di0_div] = imx_clk_divider("ldb_di0_div", "ldb_di0_div_3_5", MXC_CCM_CSCMR2, 10, 1);
+	clk[ldb_di0_div] = imx_clk_divider_flags("ldb_di0_div", "ldb_di0_div_3_5", MXC_CCM_CSCMR2, 10, 1, 0);
+	clk[ldb_di0_sel] = imx_clk_mux_flags("ldb_di0_sel", MXC_CCM_CSCMR2, 8, 1,
+				mx53_ldb_di0_sel, ARRAY_SIZE(mx53_ldb_di0_sel), CLK_SET_RATE_PARENT);
 	clk[ldb_di0_gate] = imx_clk_gate2("ldb_di0_gate", "ldb_di0_div", MXC_CCM_CCGR6, 28);
 	clk[ldb_di1_gate] = imx_clk_gate2("ldb_di1_gate", "ldb_di1_div", MXC_CCM_CCGR6, 30);
 	clk[ipu_di0_sel] = imx_clk_mux("ipu_di0_sel", MXC_CCM_CSCMR2, 26, 3,
-- 
1.8.2.rc2

Re: [PATCH v2 07/11] ARM i.MX53: fix ldb di divider and selector clocks

[Shawn Guo]

On Wed, Mar 27, 2013 at 06:30:41PM +0100, Philipp Zabel wrote:
> Use imx_clk_mux_flags and imx_clk_divider_flags to set the appropriate
> flags for the LDB display interface divider and selector clocks.
> 
> Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

Applied, thanks.

[PATCH v2 08/11] ARM i.MX6q: fix ldb di divider and selector clocks

[Philipp Zabel]

Use imx_clk_mux_flags and imx_clk_divider_flags to set the appropriate
flags for the LDB display interface divider and selector clocks.

Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 arch/arm/mach-imx/clk-imx6q.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-imx/clk-imx6q.c b/arch/arm/mach-imx/clk-imx6q.c
index 5528656..516f03e 100644
--- a/arch/arm/mach-imx/clk-imx6q.c
+++ b/arch/arm/mach-imx/clk-imx6q.c
@@ -299,8 +299,8 @@ int __init mx6q_clocks_init(void)
 	clk[gpu3d_shader_sel] = imx_clk_mux("gpu3d_shader_sel", base + 0x18, 8,  2, gpu3d_shader_sels, ARRAY_SIZE(gpu3d_shader_sels));
 	clk[ipu1_sel]         = imx_clk_mux("ipu1_sel",         base + 0x3c, 9,  2, ipu_sels,          ARRAY_SIZE(ipu_sels));
 	clk[ipu2_sel]         = imx_clk_mux("ipu2_sel",         base + 0x3c, 14, 2, ipu_sels,          ARRAY_SIZE(ipu_sels));
-	clk[ldb_di0_sel]      = imx_clk_mux("ldb_di0_sel",      base + 0x2c, 9,  3, ldb_di_sels,       ARRAY_SIZE(ldb_di_sels));
-	clk[ldb_di1_sel]      = imx_clk_mux("ldb_di1_sel",      base + 0x2c, 12, 3, ldb_di_sels,       ARRAY_SIZE(ldb_di_sels));
+	clk[ldb_di0_sel]      = imx_clk_mux_flags("ldb_di0_sel", base + 0x2c, 9,  3, ldb_di_sels,      ARRAY_SIZE(ldb_di_sels), CLK_SET_RATE_PARENT);
+	clk[ldb_di1_sel]      = imx_clk_mux_flags("ldb_di1_sel", base + 0x2c, 12, 3, ldb_di_sels,      ARRAY_SIZE(ldb_di_sels), CLK_SET_RATE_PARENT);
 	clk[ipu1_di0_pre_sel] = imx_clk_mux("ipu1_di0_pre_sel", base + 0x34, 6,  3, ipu_di_pre_sels,   ARRAY_SIZE(ipu_di_pre_sels));
 	clk[ipu1_di1_pre_sel] = imx_clk_mux("ipu1_di1_pre_sel", base + 0x34, 15, 3, ipu_di_pre_sels,   ARRAY_SIZE(ipu_di_pre_sels));
 	clk[ipu2_di0_pre_sel] = imx_clk_mux("ipu2_di0_pre_sel", base + 0x38, 6,  3, ipu_di_pre_sels,   ARRAY_SIZE(ipu_di_pre_sels));
@@ -348,9 +348,9 @@ int __init mx6q_clocks_init(void)
 	clk[ipu1_podf]        = imx_clk_divider("ipu1_podf",        "ipu1_sel",          base + 0x3c, 11, 3);
 	clk[ipu2_podf]        = imx_clk_divider("ipu2_podf",        "ipu2_sel",          base + 0x3c, 16, 3);
 	clk[ldb_di0_div_3_5]  = imx_clk_fixed_factor("ldb_di0_div_3_5", "ldb_di0_sel", 2, 7);
-	clk[ldb_di0_podf]     = imx_clk_divider("ldb_di0_podf",     "ldb_di0_div_3_5",       base + 0x20, 10, 1);
+	clk[ldb_di0_podf]     = imx_clk_divider_flags("ldb_di0_podf", "ldb_di0_div_3_5", base + 0x20, 10, 1, 0);
 	clk[ldb_di1_div_3_5]  = imx_clk_fixed_factor("ldb_di1_div_3_5", "ldb_di1_sel", 2, 7);
-	clk[ldb_di1_podf]     = imx_clk_divider("ldb_di1_podf",     "ldb_di1_div_3_5",   base + 0x20, 11, 1);
+	clk[ldb_di1_podf]     = imx_clk_divider_flags("ldb_di1_podf", "ldb_di1_div_3_5", base + 0x20, 11, 1, 0);
 	clk[ipu1_di0_pre]     = imx_clk_divider("ipu1_di0_pre",     "ipu1_di0_pre_sel",  base + 0x34, 3,  3);
 	clk[ipu1_di1_pre]     = imx_clk_divider("ipu1_di1_pre",     "ipu1_di1_pre_sel",  base + 0x34, 12, 3);
 	clk[ipu2_di0_pre]     = imx_clk_divider("ipu2_di0_pre",     "ipu2_di0_pre_sel",  base + 0x38, 3,  3);
-- 
1.8.2.rc2

Re: [PATCH v2 08/11] ARM i.MX6q: fix ldb di divider and selector clocks

[Shawn Guo]

On Wed, Mar 27, 2013 at 06:30:42PM +0100, Philipp Zabel wrote:
> Use imx_clk_mux_flags and imx_clk_divider_flags to set the appropriate
> flags for the LDB display interface divider and selector clocks.
> 
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>

Applied, thanks.

[PATCH v2 09/11] ARM i.MX53: Add IOMUXC GPR to device tree

[Philipp Zabel]

Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 arch/arm/boot/dts/imx53.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/imx53.dtsi b/arch/arm/boot/dts/imx53.dtsi
index 7a6f5a8..b07bbdcc 100644
--- a/arch/arm/boot/dts/imx53.dtsi
+++ b/arch/arm/boot/dts/imx53.dtsi
@@ -506,6 +506,11 @@
 
 			};
 
+			gpr: iomuxc-gpr@53fa8000 {
+				compatible = "fsl,imx53-iomuxc-gpr", "syscon";
+				reg = <0x53fa8000 0xc>;
+			};
+
 			pwm1: pwm@53fb4000 {
 				#pwm-cells = <2>;
 				compatible = "fsl,imx53-pwm", "fsl,imx27-pwm";
-- 
1.8.2.rc2

[PATCH v2 10/11] ARM i.MX53: Add LDB device to device tree

[Philipp Zabel]

Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 arch/arm/boot/dts/imx53.dtsi | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/arch/arm/boot/dts/imx53.dtsi b/arch/arm/boot/dts/imx53.dtsi
index b07bbdcc..30aed40 100644
--- a/arch/arm/boot/dts/imx53.dtsi
+++ b/arch/arm/boot/dts/imx53.dtsi
@@ -511,6 +511,33 @@
 				reg = <0x53fa8000 0xc>;
 			};
 
+			ldb: ldb@53fa8008 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				compatible = "fsl,imx53-ldb";
+				reg = <0x53fa8008 0x4>;
+				gpr = <&gpr>;
+				clocks = <&clks 122>, <&clks 120>,
+					 <&clks 115>, <&clks 116>,
+					 <&clks 123>, <&clks 85>;
+				clock-names = "di0_pll", "di1_pll",
+					      "di0_sel", "di1_sel",
+					      "di0", "di1";
+				status = "disabled";
+
+				lvds-channel@0 {
+					reg = <0>;
+					crtcs = <&ipu 0>;
+					status = "disabled";
+				};
+
+				lvds-channel@1 {
+					reg = <1>;
+					crtcs = <&ipu 1>;
+					status = "disabled";
+				};
+			};
+
 			pwm1: pwm@53fb4000 {
 				#pwm-cells = <2>;
 				compatible = "fsl,imx53-pwm", "fsl,imx27-pwm";
-- 
1.8.2.rc2

Re: [PATCH v2 10/11] ARM i.MX53: Add LDB device to device tree

[Martin Fuzzey]

On 27/03/13 18:30, Philipp Zabel wrote:
> Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> ---
>   arch/arm/boot/dts/imx53.dtsi | 27 +++++++++++++++++++++++++++
>   1 file changed, 27 insertions(+)
>
> diff --git a/arch/arm/boot/dts/imx53.dtsi b/arch/arm/boot/dts/imx53.dtsi
> index b07bbdcc..30aed40 100644
> --- a/arch/arm/boot/dts/imx53.dtsi
> +++ b/arch/arm/boot/dts/imx53.dtsi
> @@ -511,6 +511,33 @@
>   				reg = <0x53fa8000 0xc>;
>   			};
>   
> +			ldb: ldb@53fa8008 {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +				compatible = "fsl,imx53-ldb";
> +				reg = <0x53fa8008 0x4>;
> +				gpr = <&gpr>;
> +				clocks = <&clks 122>, <&clks 120>,
> +					 <&clks 115>, <&clks 116>,
> +					 <&clks 123>, <&clks 85>;
> +				clock-names = "di0_pll", "di1_pll",
> +					      "di0_sel", "di1_sel",
> +					      "di0", "di1";
> +				status = "disabled";
> +
> +				lvds-channel@0 {
> +					reg = <0>;
> +					crtcs = <&ipu 0>;
> +					status = "disabled";
> +				};
> +
> +				lvds-channel@1 {
> +					reg = <1>;
> +					crtcs = <&ipu 1>;
> +					status = "disabled";
> +				};
> +			};
> +
>   			pwm1: pwm@53fb4000 {
>   				#pwm-cells = <2>;
>   				compatible = "fsl,imx53-pwm", "fsl,imx27-pwm";
Maybe pinctrl entries should also be added?

Regards,

Martin

Re: [PATCH v2 10/11] ARM i.MX53: Add LDB device to device tree

[Shawn Guo]

On Wed, Mar 27, 2013 at 07:43:40PM +0100, Martin Fuzzey wrote:
> On 27/03/13 18:30, Philipp Zabel wrote:
> >Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> >---
> >  arch/arm/boot/dts/imx53.dtsi | 27 +++++++++++++++++++++++++++
> >  1 file changed, 27 insertions(+)
> >
> >diff --git a/arch/arm/boot/dts/imx53.dtsi b/arch/arm/boot/dts/imx53.dtsi
> >index b07bbdcc..30aed40 100644
> >--- a/arch/arm/boot/dts/imx53.dtsi
> >+++ b/arch/arm/boot/dts/imx53.dtsi
> >@@ -511,6 +511,33 @@
> >  				reg = <0x53fa8000 0xc>;
> >  			};
> >+			ldb: ldb@53fa8008 {
> >+				#address-cells = <1>;
> >+				#size-cells = <0>;
> >+				compatible = "fsl,imx53-ldb";
> >+				reg = <0x53fa8008 0x4>;
> >+				gpr = <&gpr>;
> >+				clocks = <&clks 122>, <&clks 120>,
> >+					 <&clks 115>, <&clks 116>,
> >+					 <&clks 123>, <&clks 85>;
> >+				clock-names = "di0_pll", "di1_pll",
> >+					      "di0_sel", "di1_sel",
> >+					      "di0", "di1";
> >+				status = "disabled";
> >+
> >+				lvds-channel@0 {
> >+					reg = <0>;
> >+					crtcs = <&ipu 0>;
> >+					status = "disabled";
> >+				};
> >+
> >+				lvds-channel@1 {
> >+					reg = <1>;
> >+					crtcs = <&ipu 1>;
> >+					status = "disabled";
> >+				};
> >+			};
> >+
> >  			pwm1: pwm@53fb4000 {
> >  				#pwm-cells = <2>;
> >  				compatible = "fsl,imx53-pwm", "fsl,imx27-pwm";
> Maybe pinctrl entries should also be added?

Generally, the pinctrl entries should be added in <board>.dts.

Shawn

Re: [PATCH v2 10/11] ARM i.MX53: Add LDB device to device tree

[Martin Fuzzey]

On 28/03/13 08:32, Shawn Guo wrote:
> On Wed, Mar 27, 2013 at 07:43:40PM +0100, Martin Fuzzey wrote:
>>
>> Maybe pinctrl entries should also be added?
> Generally, the pinctrl entries should be added in <board>.dts.

Yes sorry I meant the pinctrl groups under the iomuxc node.

Regards,
Martin

Re: [PATCH v2 10/11] ARM i.MX53: Add LDB device to device tree

[Philipp Zabel]

Am Donnerstag, den 28.03.2013, 09:41 +0100 schrieb Martin Fuzzey:
> On 28/03/13 08:32, Shawn Guo wrote:
> > On Wed, Mar 27, 2013 at 07:43:40PM +0100, Martin Fuzzey wrote:
> >>
> >> Maybe pinctrl entries should also be added?
> > Generally, the pinctrl entries should be added in <board>.dts.
> 
> Yes sorry I meant the pinctrl groups under the iomuxc node.

I'm not fond of putting each and every possible pin group configuration
into the dtsi. But since there is only one possibility to multiplex each
of the LVDS0 and LVDS1 pads on i.MX53, I wouldn't resist:

ldb {
	pinctrl_lvds0_1: lvds0grp-1 {
		fsl,pins = <
			730 0x80000000	/* MX53_PAD_LVDS0_TX3_P__LDB_LVDS0_TX3 */
			732 0x80000000	/* MX53_PAD_LVDS0_CLK_P__LDB_LVDS0_CLK */
			734 0x80000000	/* MX53_PAD_LVDS0_TX2_P__LDB_LVDS0_TX2 */
			736 0x80000000	/* MX53_PAD_LVDS0_TX1_P__LDB_LVDS0_TX1 */
			738 0x80000000	/* MX53_PAD_LVDS0_TX0_P__LDB_LVDS0_TX0 */
		>;
	};

	pinctrl_lvds1_1: lvds1grp-1 {
		fsl,pins = <
			720 0x80000000	/* MX53_PAD_LVDS1_TX3_P__LDB_LVDS1_TX3 */
			722 0x80000000	/* MX53_PAD_LVDS1_TX2_P__LDB_LVDS1_TX2 */
			724 0x80000000	/* MX53_PAD_LVDS1_CLK_P__LDB_LVDS1_CLK */
			726 0x80000000	/* MX53_PAD_LVDS1_TX1_P__LDB_LVDS1_TX1 */
			728 0x80000000	/* MX53_PAD_LVDS1_TX0_P__LDB_LVDS1_TX0 */
		>;
	};
};

regards
Philipp

[PATCH v2 11/11] ARM i.MX6q: Add LDB device to device tree

[Philipp Zabel]

From: Steffen Trumtrar <s.trumtrar-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

Add ldb device tree node and clock lookups.

Signed-off-by: Steffen Trumtrar <s.trumtrar-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 arch/arm/boot/dts/imx6q.dtsi   | 17 +++++++++++++++++
 arch/arm/boot/dts/imx6qdl.dtsi | 26 ++++++++++++++++++++++++++
 2 files changed, 43 insertions(+)

diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
index cba021e..1a30227 100644
--- a/arch/arm/boot/dts/imx6q.dtsi
+++ b/arch/arm/boot/dts/imx6q.dtsi
@@ -294,3 +294,20 @@
 		};
 	};
 };
+
+&ldb {
+	clocks = <&clks 33>, <&clks 34>,
+		 <&clks 39>, <&clks 40>, <&clks 41>, <&clks 42>,
+		 <&clks 135>, <&clks 136>;
+	clock-names = "di0_pll", "di1_pll",
+		      "di0_sel", "di1_sel", "di2_sel", "di3_sel",
+		      "di0", "di1";
+
+	lvds-channel@0 {
+		crtcs = <&ipu1 0>, <&ipu1 1>, <&ipu2 0>, <&ipu2 1>;
+	};
+
+	lvds-channel@1 {
+		crtcs = <&ipu1 0>, <&ipu1 1>, <&ipu2 0>, <&ipu2 1>;
+	};
+};
diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index 06ec460..dd5ef96 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -529,6 +529,32 @@
 				reg = <0x020e0000 0x38>;
 			};
 
+			ldb: ldb@020e0008 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				compatible = "fsl,imx6q-ldb", "fsl,imx53-ldb";
+				clocks = <&clks 33>, <&clks 34>,
+					 <&clks 39>, <&clks 40>,
+					 <&clks 135>, <&clks 136>;
+				clock-names = "di0_pll", "di1_pll",
+					      "di0_sel", "di1_sel",
+					      "di0", "di1";
+				gpr = <&gpr>;
+				status = "disabled";
+
+				lvds-channel@0 {
+					reg = <0>;
+					crtcs = <&ipu1 0>;
+					status = "disabled";
+				};
+
+				lvds-channel@1 {
+					reg = <1>;
+					crtcs = <&ipu1 1>;
+					status = "disabled";
+				};
+			};
+
 			dcic1: dcic@020e4000 {
 				reg = <0x020e4000 0x4000>;
 				interrupts = <0 124 0x04>;
-- 
1.8.2.rc2

Re: [PATCH v2 11/11] ARM i.MX6q: Add LDB device to device tree

[Shawn Guo]

On Wed, Mar 27, 2013 at 06:30:45PM +0100, Philipp Zabel wrote:
> From: Steffen Trumtrar <s.trumtrar-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> 
> Add ldb device tree node and clock lookups.
> 
> Signed-off-by: Steffen Trumtrar <s.trumtrar-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> ---
>  arch/arm/boot/dts/imx6q.dtsi   | 17 +++++++++++++++++
>  arch/arm/boot/dts/imx6qdl.dtsi | 26 ++++++++++++++++++++++++++
>  2 files changed, 43 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
> index cba021e..1a30227 100644
> --- a/arch/arm/boot/dts/imx6q.dtsi
> +++ b/arch/arm/boot/dts/imx6q.dtsi
> @@ -294,3 +294,20 @@
>  		};
>  	};
>  };
> +
> +&ldb {
> +	clocks = <&clks 33>, <&clks 34>,
> +		 <&clks 39>, <&clks 40>, <&clks 41>, <&clks 42>,
> +		 <&clks 135>, <&clks 136>;
> +	clock-names = "di0_pll", "di1_pll",
> +		      "di0_sel", "di1_sel", "di2_sel", "di3_sel",
> +		      "di0", "di1";

These are identical with the ones in imx6qdl.dtsi, so not needed at all?

> +
> +	lvds-channel@0 {
> +		crtcs = <&ipu1 0>, <&ipu1 1>, <&ipu2 0>, <&ipu2 1>;
> +	};
> +
> +	lvds-channel@1 {
> +		crtcs = <&ipu1 0>, <&ipu1 1>, <&ipu2 0>, <&ipu2 1>;
> +	};
> +};
> diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
> index 06ec460..dd5ef96 100644
> --- a/arch/arm/boot/dts/imx6qdl.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl.dtsi
> @@ -529,6 +529,32 @@
>  				reg = <0x020e0000 0x38>;
>  			};
>  
> +			ldb: ldb@020e0008 {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +				compatible = "fsl,imx6q-ldb", "fsl,imx53-ldb";

Since both compatible strings are in the driver matching table, it's not
necessary to have "fsl,imx53-ldb" listed here.

Shawn

> +				clocks = <&clks 33>, <&clks 34>,
> +					 <&clks 39>, <&clks 40>,
> +					 <&clks 135>, <&clks 136>;
> +				clock-names = "di0_pll", "di1_pll",
> +					      "di0_sel", "di1_sel",
> +					      "di0", "di1";
> +				gpr = <&gpr>;
> +				status = "disabled";
> +
> +				lvds-channel@0 {
> +					reg = <0>;
> +					crtcs = <&ipu1 0>;
> +					status = "disabled";
> +				};
> +
> +				lvds-channel@1 {
> +					reg = <1>;
> +					crtcs = <&ipu1 1>;
> +					status = "disabled";
> +				};
> +			};
> +
>  			dcic1: dcic@020e4000 {
>  				reg = <0x020e4000 0x4000>;
>  				interrupts = <0 124 0x04>;
> -- 
> 1.8.2.rc2
> 

Re: [PATCH v2 11/11] ARM i.MX6q: Add LDB device to device tree

[Philipp Zabel]

Am Donnerstag, den 28.03.2013, 15:51 +0800 schrieb Shawn Guo:
> On Wed, Mar 27, 2013 at 06:30:45PM +0100, Philipp Zabel wrote:
> > From: Steffen Trumtrar <s.trumtrar-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > 
> > Add ldb device tree node and clock lookups.
> > 
> > Signed-off-by: Steffen Trumtrar <s.trumtrar-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > ---
> >  arch/arm/boot/dts/imx6q.dtsi   | 17 +++++++++++++++++
> >  arch/arm/boot/dts/imx6qdl.dtsi | 26 ++++++++++++++++++++++++++
> >  2 files changed, 43 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
> > index cba021e..1a30227 100644
> > --- a/arch/arm/boot/dts/imx6q.dtsi
> > +++ b/arch/arm/boot/dts/imx6q.dtsi
> > @@ -294,3 +294,20 @@
> >  		};
> >  	};
> >  };
> > +
> > +&ldb {
> > +	clocks = <&clks 33>, <&clks 34>,
> > +		 <&clks 39>, <&clks 40>, <&clks 41>, <&clks 42>,
> > +		 <&clks 135>, <&clks 136>;
> > +	clock-names = "di0_pll", "di1_pll",
> > +		      "di0_sel", "di1_sel", "di2_sel", "di3_sel",
> > +		      "di0", "di1";
> 
> These are identical with the ones in imx6qdl.dtsi, so not needed at all?

The ldb node in imx6qdl.dtsi doesn't have the di[23]_sel clocks, because
i.MX6dl doesn't have the second IPU.
On i.MX6q, di[23]_sel should point to the ipu2_di0_sel and ipu2_di1_sel
mux clocks. On i.MX6dl, di2_sel should point to lcdif_sel, eventually,
and di3_sel shouldn't be given.

Should I remove the clocks from imx6qdl.dtsi altogether, to avoid
confusion?

> > +
> > +	lvds-channel@0 {
> > +		crtcs = <&ipu1 0>, <&ipu1 1>, <&ipu2 0>, <&ipu2 1>;
> > +	};
> > +
> > +	lvds-channel@1 {
> > +		crtcs = <&ipu1 0>, <&ipu1 1>, <&ipu2 0>, <&ipu2 1>;
> > +	};
> > +};
> > diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
> > index 06ec460..dd5ef96 100644
> > --- a/arch/arm/boot/dts/imx6qdl.dtsi
> > +++ b/arch/arm/boot/dts/imx6qdl.dtsi
> > @@ -529,6 +529,32 @@
> >  				reg = <0x020e0000 0x38>;
> >  			};
> >  
> > +			ldb: ldb@020e0008 {
> > +				#address-cells = <1>;
> > +				#size-cells = <0>;
> > +				compatible = "fsl,imx6q-ldb", "fsl,imx53-ldb";
> 
> Since both compatible strings are in the driver matching table, it's not
> necessary to have "fsl,imx53-ldb" listed here.

I originally intended to split the input multiplexer from the LDB
driver, as we have the same for HDMI and MIPI on i.MX6 (minus the clock
multiplexing requirements), because apart from that, the LDB blocks are
identical. Shouldn't this be documented in the device tree?

regards
Philipp

Re: [PATCH v2 11/11] ARM i.MX6q: Add LDB device to device tree

[Shawn Guo]

On Thu, Mar 28, 2013 at 10:58:07AM +0100, Philipp Zabel wrote:
> The ldb node in imx6qdl.dtsi doesn't have the di[23]_sel clocks, because
> i.MX6dl doesn't have the second IPU.
> On i.MX6q, di[23]_sel should point to the ipu2_di0_sel and ipu2_di1_sel
> mux clocks. On i.MX6dl, di2_sel should point to lcdif_sel, eventually,
> and di3_sel shouldn't be given.
> 
Ah, sorry, I overlooked the difference.

> Should I remove the clocks from imx6qdl.dtsi altogether, to avoid
> confusion?
> 
Yea, I think it's less confusing to just have clocks and clock-names
defined in imx6q and imx6dl dts respectively.

> I originally intended to split the input multiplexer from the LDB
> driver, as we have the same for HDMI and MIPI on i.MX6 (minus the clock
> multiplexing requirements), because apart from that, the LDB blocks are
> identical. Shouldn't this be documented in the device tree?
> 
Ok, that's fine then.  I had the question because I saw imx6q compatible
already in the matching table.

Shawn

[PATCH v2 03/11] ARM i.MX6q: export imx6q_revision

[Philipp Zabel]

So it can be used in clk-imx6q.c for revision dependent clock tree setup.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 arch/arm/mach-imx/common.h     | 1 +
 arch/arm/mach-imx/mach-imx6q.c | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-imx/common.h b/arch/arm/mach-imx/common.h
index 5a800bf..6c909d1 100644
--- a/arch/arm/mach-imx/common.h
+++ b/arch/arm/mach-imx/common.h
@@ -74,6 +74,7 @@ extern void mxc_set_cpu_type(unsigned int type);
 extern void mxc_restart(char, const char *);
 extern void mxc_arch_reset_init(void __iomem *);
 extern int mx53_revision(void);
+extern int imx6q_revision(void);
 extern int mx53_display_revision(void);
 extern void imx_set_aips(void __iomem *);
 extern int mxc_device_init(void);
diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
index 9ffd103..ca10acc 100644
--- a/arch/arm/mach-imx/mach-imx6q.c
+++ b/arch/arm/mach-imx/mach-imx6q.c
@@ -41,7 +41,7 @@
 
 #define IMX6Q_ANALOG_DIGPROG	0x260
 
-static int imx6q_revision(void)
+int imx6q_revision(void)
 {
 	struct device_node *np;
 	void __iomem *base;
-- 
1.8.2.rc2

Re: [PATCH v2 03/11] ARM i.MX6q: export imx6q_revision

[Shawn Guo]

On Wed, Mar 27, 2013 at 06:30:37PM +0100, Philipp Zabel wrote:
> So it can be used in clk-imx6q.c for revision dependent clock tree setup.
> 
> Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

Applied, thanks.

> ---
>  arch/arm/mach-imx/common.h     | 1 +
>  arch/arm/mach-imx/mach-imx6q.c | 2 +-
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-imx/common.h b/arch/arm/mach-imx/common.h
> index 5a800bf..6c909d1 100644
> --- a/arch/arm/mach-imx/common.h
> +++ b/arch/arm/mach-imx/common.h
> @@ -74,6 +74,7 @@ extern void mxc_set_cpu_type(unsigned int type);
>  extern void mxc_restart(char, const char *);
>  extern void mxc_arch_reset_init(void __iomem *);
>  extern int mx53_revision(void);
> +extern int imx6q_revision(void);
>  extern int mx53_display_revision(void);
>  extern void imx_set_aips(void __iomem *);
>  extern int mxc_device_init(void);
> diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
> index 9ffd103..ca10acc 100644
> --- a/arch/arm/mach-imx/mach-imx6q.c
> +++ b/arch/arm/mach-imx/mach-imx6q.c
> @@ -41,7 +41,7 @@
>  
>  #define IMX6Q_ANALOG_DIGPROG	0x260
>  
> -static int imx6q_revision(void)
> +int imx6q_revision(void)
>  {
>  	struct device_node *np;
>  	void __iomem *base;
> -- 
> 1.8.2.rc2
> 

[PATCH v2 05/11] ARM i.MX6q: set the LDB serial clock parent to the video PLL

[Philipp Zabel]

On i.MX6q revision 1.1 and later, set the video PLL as parent for
the LDB clock branch. On revision 1.0, the video PLL is useless
due to missing dividers, so keep the default parent (mmdc_ch1_axi).

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 arch/arm/mach-imx/clk-imx6q.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/mach-imx/clk-imx6q.c b/arch/arm/mach-imx/clk-imx6q.c
index fbab4a9..5528656 100644
--- a/arch/arm/mach-imx/clk-imx6q.c
+++ b/arch/arm/mach-imx/clk-imx6q.c
@@ -465,6 +465,11 @@ int __init mx6q_clocks_init(void)
 	clk_register_clkdev(clk[cko1], "cko1", NULL);
 	clk_register_clkdev(clk[arm], NULL, "cpu0");
 
+	if (imx6q_revision() != IMX_CHIP_REVISION_1_0) {
+		clk_set_parent(clk[ldb_di0_sel], clk[pll5_control3]);
+		clk_set_parent(clk[ldb_di1_sel], clk[pll5_control3]);
+	}
+
 	/*
 	 * The gpmi needs 100MHz frequency in the EDO/Sync mode,
 	 * We can not get the 100MHz from the pll2_pfd0_352m.
-- 
1.8.2.rc2