[RFC PATCH] i2c: i2c-imx-hdmi: add support of iMX6 HDMI DDC I2C master bus

[RFC PATCH] i2c: i2c-imx-hdmi: add support of iMX6 HDMI DDC I2C master bus

[Vladimir Zapolskiy]

Please review a proposed iMX6 HDMI DDC controller driver written on
top of I2C framework.

The driver intends to support HDMI on-controller I2C master bus with
limited cababilities, however by nature of the sub-device it seems
reasonable to separate it into a stand-alone driver, which also can be
used independently on general HDMI controller driver, for example as
an additional I2C bus on a board, but please be aware that the bus is
not compliant to I2C specification. To support the selected I2C bus
approach, I suppose it should be possible to read/write e.g. a
connected EEPROM device, not just HDMI EDID blobs.

In my practice I've met iMX6Q boards, where HDMI DDC lines are
connected to iMX6 HDMI DDC pins and not to any of 3 I2C busses, so
presence in the kernel of this driver or a driver with similar
functionality seems to be important.

Main issues with the device/driver:
* iMX6 HDMI controller becomes a shared device (both interrupt and
  memory region) between HDMI driver itself and this I2C bus driver,
* Only two patterns of I2C transactions are supported due to hardware
  limitation, these patters are described in driver's code header.
* The bus controller supports one more multi-byte read pattern (so
  called Extended Read Mode), but Freescale documentation is too vague
  to easily add this mode into the driver.

If somebody wants to test the driver, please do additional changes to
the code:

-----8<-----8<-----8<-----8<-----8<-----

I. iMX6QDL SoC DTS:

diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index c701af9..8475e12 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -916,6 +916,19 @@
 				status = "disabled";
 			};
 
+			hdmi_ddc: i2c@00120000 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				compatible = "fsl,imx6q-hdmi-ddc";
+				reg = <0x00120000 0x8000>;
+				interrupts-extended =
+					<&intc 0 115 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&clks IMX6QDL_CLK_HDMI_IAHB>,
+					 <&clks IMX6QDL_CLK_HDMI_ISFR>;
+				clock-names = "iahb", "isfr";
+				status = "disabled";
+			};
+
 			i2c1: i2c@021a0000 {
 				#address-cells = <1>;
 				#size-cells = <0>;

Also you may want to add an I2C alias to assign an I2C bus, iMX6Q example:

diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
index e9f3646..e54d81e 100644
--- a/arch/arm/boot/dts/imx6q.dtsi
+++ b/arch/arm/boot/dts/imx6q.dtsi
@@ -14,6 +14,7 @@
 
 / {
 	aliases {
+		i2c3 = &hdmi_ddc;
 		spi4 = &ecspi5;
 	};
 

II. iMX6 Board DTS (iMX6Q SabreLite example), testing with main HDMI driver:

diff --git a/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi b/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
index 0a36129..161593e 100644
--- a/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
@@ -173,6 +173,18 @@
 	status = "okay";
 };
 
+&hdmi {
+	ddc-i2c-bus = <&hdmi_ddc>;
+	status = "okay";
+};
+
+&hdmi_ddc {
+	clock-frequency = <100000>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_hdmi_ddc>;
+	status = "okay";
+};
+
 &i2c1 {
 	clock-frequency = <100000>;
 	pinctrl-names = "default";
@@ -258,6 +270,13 @@
 			>;
 		};
 
+		pinctrl_hdmi_ddc: hdmi_ddcgrp {
+			fsl,pins = <
+				MX6QDL_PAD_KEY_COL3__HDMI_TX_DDC_SCL    0x4001b8b1
+				MX6QDL_PAD_KEY_ROW3__HDMI_TX_DDC_SDA    0x4001b8b1
+			>;
+		};
+
 		pinctrl_i2c1: i2c1grp {
 			fsl,pins = <
 				MX6QDL_PAD_EIM_D21__I2C1_SCL		0x4001b8b1

III. iMX6 HDMI controller driver (remove I2C bus interrupt mutes):

diff --git a/drivers/staging/imx-drm/imx-hdmi.c b/drivers/staging/imx-drm/imx-hdmi.c
index 18c9ccd..1914b0b 100644
--- a/drivers/staging/imx-drm/imx-hdmi.c
+++ b/drivers/staging/imx-drm/imx-hdmi.c
@@ -1342,8 +1342,6 @@ static void initialize_hdmi_ih_mutes(struct imx_hdmi *hdmi)
 	hdmi_writeb(hdmi, 0xff, HDMI_GP_MASK);
 	hdmi_writeb(hdmi, 0xff, HDMI_A_APIINTMSK);
 	hdmi_writeb(hdmi, 0xff, HDMI_CEC_MASK);
-	hdmi_writeb(hdmi, 0xff, HDMI_I2CM_INT);
-	hdmi_writeb(hdmi, 0xff, HDMI_I2CM_CTLINT);
 
 	/* Disable interrupts in the IH_MUTE_* registers */
 	hdmi_writeb(hdmi, 0xff, HDMI_IH_MUTE_FC_STAT0);
@@ -1351,7 +1349,6 @@ static void initialize_hdmi_ih_mutes(struct imx_hdmi *hdmi)
 	hdmi_writeb(hdmi, 0xff, HDMI_IH_MUTE_FC_STAT2);
 	hdmi_writeb(hdmi, 0xff, HDMI_IH_MUTE_AS_STAT0);
 	hdmi_writeb(hdmi, 0xff, HDMI_IH_MUTE_PHY_STAT0);
-	hdmi_writeb(hdmi, 0xff, HDMI_IH_MUTE_I2CM_STAT0);
 	hdmi_writeb(hdmi, 0xff, HDMI_IH_MUTE_CEC_STAT0);
 	hdmi_writeb(hdmi, 0xff, HDMI_IH_MUTE_VP_STAT0);
 	hdmi_writeb(hdmi, 0xff, HDMI_IH_MUTE_I2CMPHY_STAT0);

-----8<-----8<-----8<-----8<-----8<-----

As I mentioned above the bus driver may be used independently on
presence/absence of iMX6 HDMI driver.

I'd appreciate to get review responses, if the selected approach based
on I2C framework is acceptable, I'll remove RFC tag and add updates
of all the missing parts (iMX6 DTS files, Documentation, HDMI driver
and addressed review comments). Comments how to gracefully align this
driver with iMX HDMI driver are welcome as well.

The patch is based and tested on 3.17, but it is cleanly applicable to
3.18-rc2.

Vladimir Zapolskiy (1):
  i2c: i2c-imx-hdmi: add support of iMX6 HDMI DDC I2C master bus

 drivers/i2c/busses/Kconfig        |   13 +
 drivers/i2c/busses/Makefile       |    1 +
 drivers/i2c/busses/i2c-imx-hdmi.c |  485 +++++++++++++++++++++++++++++++++++++
 3 files changed, 499 insertions(+)
 create mode 100644 drivers/i2c/busses/i2c-imx-hdmi.c

-- 
1.7.10.4

[RFC PATCH] i2c: i2c-imx-hdmi: add support of iMX6 HDMI DDC I2C master bus

[Vladimir Zapolskiy]

The i.MX6 SoC in HDMI controller has one more on-SoC I2C bus dedicated
to read EDID data from a connected monitor. The controller does not
fully support I2C communications, only specific forms of I2C transfer
patters can be programmed, but having those limitations in mind it
seems reasonable to add another I2C bus driver representing iMX6 HDMI
DDC bus. The bus can be used to read monitor's EDID or access EEPROM,
but communication to most of the I2C devices will fail or be invalid.

The iMX6 HDMI DDC I2C bus may be used separately from main iMX6 HDMI
controller code, for instance as an additional I2C bus with limited
capabilities.

Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
Cc: Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
Cc: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
---
 drivers/i2c/busses/Kconfig        |   13 +
 drivers/i2c/busses/Makefile       |    1 +
 drivers/i2c/busses/i2c-imx-hdmi.c |  493 +++++++++++++++++++++++++++++++++++++
 3 files changed, 507 insertions(+)
 create mode 100644 drivers/i2c/busses/i2c-imx-hdmi.c

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 2ac87fa..51f12f3 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -1021,4 +1021,17 @@ config SCx200_ACB
 	  This support is also available as a module.  If so, the module
 	  will be called scx200_acb.
 
+config I2C_IMX_HDMI
+	tristate "IMX6 HDMI DDC I2C master interface"
+	depends on ARCH_MXC
+	default n
+	help
+	  Say Y here if you want to use the HDMI DDC bus controller
+	  found on the Freescale i.MX6 processors. Most probably you
+	  don't need this driver, if HDMI monitor is supposed to be
+	  connected over regular iMX6 I2C bus.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called i2c-imx-hdmi.
+
 endmenu
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 49bf07e..9cf7d5a 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -97,6 +97,7 @@ obj-$(CONFIG_I2C_ACORN)		+= i2c-acorn.o
 obj-$(CONFIG_I2C_BCM_KONA)	+= i2c-bcm-kona.o
 obj-$(CONFIG_I2C_CROS_EC_TUNNEL)	+= i2c-cros-ec-tunnel.o
 obj-$(CONFIG_I2C_ELEKTOR)	+= i2c-elektor.o
+obj-$(CONFIG_I2C_IMX_HDMI)	+= i2c-imx-hdmi.o
 obj-$(CONFIG_I2C_PCA_ISA)	+= i2c-pca-isa.o
 obj-$(CONFIG_I2C_SIBYTE)	+= i2c-sibyte.o
 obj-$(CONFIG_SCx200_ACB)	+= scx200_acb.o
diff --git a/drivers/i2c/busses/i2c-imx-hdmi.c b/drivers/i2c/busses/i2c-imx-hdmi.c
new file mode 100644
index 0000000..ae6c18a
--- /dev/null
+++ b/drivers/i2c/busses/i2c-imx-hdmi.c
@@ -0,0 +1,485 @@
+/*
+ * I2C master driver for iMX6 HDMI DDC bus
+ *
+ * Copyright (C) 2013-2014 Mentor Graphics
+ *
+ * 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.
+ */
+
+/*
+ * According to the iMX6 Reference Manual only two types of transactions
+ * are suported by HDMI I2C Master Interface in Normal Mode:
+ *
+ * A) one byte data write transaction (I2C spec 2 bytes transmission):
+ *    master   S|slave addr[6:0]|0|     |slave reg[7:0]|     |data[7:0]|     |P
+ *    slave                       | Ack |              | Ack |         | Ack |
+ *
+ * B) one byte data read transaction (I2C spec write/read combined format):
+ *    master   S|slave addr[6:0]|0|     |slave reg[7:0]|     | ...
+ *    slave                       | Ack |              | Ack | ...
+ *
+ *    master   ... Sr|slave addr[6:0]|1|     |         | Ack |P
+ *    slave    ...                     | Ack |data[7:0]|
+ *
+ *
+ * The technical limitations of the iMX6 HDMI E-DDC bus does not allow
+ * to call it an I2C compatible bus, however relativery large subset of
+ * I2C transactions can be decomposed into aforementioned data read/write
+ * operations and many I2C devices correctly support those operations (and
+ * of course it is possible to read EDID blob of a connected HDMI monitor),
+ * so practically it makes sense to have an I2C bus driver utilizing
+ * Linux I2C framework potentialities.
+ */
+
+#include <linux/clk.h>
+#include <linux/i2c.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+
+/* iMX6 HDMI shared registers */
+#define HDMI_IH_I2CM_STAT0			0x0105
+#define HDMI_IH_MUTE_I2CM_STAT0			0x0185
+
+/* HDMI_IH_I2CM_STAT0 / HDMI_IH_MUTE_I2CM_STAT0 register bits */
+#define HDMI_IH_I2CM_STAT0_ERROR		BIT(0)
+#define HDMI_IH_I2CM_STAT0_DONE			BIT(1)
+
+/* iMX6 HDMI I2C master (E-DDC) register offsets */
+#define HDMI_I2CM_SLAVE				0x7E00
+#define HDMI_I2CM_ADDRESS			0x7E01
+#define HDMI_I2CM_DATAO				0x7E02
+#define HDMI_I2CM_DATAI				0x7E03
+#define HDMI_I2CM_OPERATION			0x7E04
+#define HDMI_I2CM_INT				0x7E05
+#define HDMI_I2CM_CTLINT			0x7E06
+#define HDMI_I2CM_DIV				0x7E07
+#define HDMI_I2CM_SEGADDR			0x7E08
+#define HDMI_I2CM_SOFTRSTZ			0x7E09
+#define HDMI_I2CM_SEGPTR			0x7E0A
+#define HDMI_I2CM_SS_SCL_HCNT_1_ADDR		0x7E0B
+#define HDMI_I2CM_SS_SCL_HCNT_0_ADDR		0x7E0C
+#define HDMI_I2CM_SS_SCL_LCNT_1_ADDR		0x7E0D
+#define HDMI_I2CM_SS_SCL_LCNT_0_ADDR		0x7E0E
+#define HDMI_I2CM_FS_SCL_HCNT_1_ADDR		0x7E0F
+#define HDMI_I2CM_FS_SCL_HCNT_0_ADDR		0x7E10
+#define HDMI_I2CM_FS_SCL_LCNT_1_ADDR		0x7E11
+#define HDMI_I2CM_FS_SCL_LCNT_0_ADDR		0x7E12
+
+/* HDMI_I2CM_OPERATION register bits */
+#define HDMI_I2CM_OPERATION_READ		BIT(0)
+#define HDMI_I2CM_OPERATION_READ_EXT		BIT(1)
+#define HDMI_I2CM_OPERATION_WRITE		BIT(4)
+
+/* HDMI_I2CM_INT register bits */
+#define HDMI_I2CM_INT_DONE_MASK			BIT(2)
+#define HDMI_I2CM_INT_DONE_POL			BIT(3)
+
+/* HDMI_I2CM_CTLINT register bits */
+#define HDMI_I2CM_CTLINT_ARB_MASK		BIT(2)
+#define HDMI_I2CM_CTLINT_ARB_POL		BIT(3)
+#define HDMI_I2CM_CTLINT_NAC_MASK		BIT(6)
+#define HDMI_I2CM_CTLINT_NAC_POL		BIT(7)
+
+
+struct imx_hdmi_i2c {
+	struct i2c_adapter	adap;
+	struct device		*dev;
+
+	void __iomem		*base;
+	struct clk		*isfr_clk;
+	struct clk		*iahb_clk;
+
+	spinlock_t		lock;
+	u8			stat;
+	struct completion	cmp;
+
+	u8			slave_reg;
+	bool			is_regaddr;
+};
+
+static void hdmi_writeb(struct imx_hdmi_i2c *pd, unsigned int reg, u8 value)
+{
+	dev_dbg(pd->dev, "write: reg 0x%04x, val 0x%02x\n", reg, value);
+	writeb_relaxed(value, pd->base + reg);
+}
+
+static u8 hdmi_readb(struct imx_hdmi_i2c *pd, unsigned int reg)
+{
+	u8 value;
+
+	value = readb_relaxed(pd->base + reg);
+	dev_dbg(pd->dev, "read: reg 0x%04x, val 0x%02x\n", reg, value);
+
+	return value;
+}
+
+/* Initialize iMX6 HDMI DDC I2CM controller */
+static void imx_hdmi_hwinit(struct imx_hdmi_i2c *pd)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&pd->lock, flags);
+
+	/* Set Fast Mode speed */
+	hdmi_writeb(pd, HDMI_I2CM_DIV, 0x0b);
+
+	/* Software reset */
+	hdmi_writeb(pd, HDMI_I2CM_SOFTRSTZ, 0x00);
+
+	/* Set done, not acknowledged and arbitration interrupt polarities */
+	hdmi_writeb(pd, HDMI_I2CM_INT, HDMI_I2CM_INT_DONE_POL);
+	hdmi_writeb(pd, HDMI_I2CM_CTLINT,
+		    HDMI_I2CM_CTLINT_NAC_POL | HDMI_I2CM_CTLINT_ARB_POL);
+
+	/* Clear DONE and ERROR interrupts */
+	hdmi_writeb(pd, HDMI_IH_I2CM_STAT0,
+		    HDMI_IH_I2CM_STAT0_ERROR | HDMI_IH_I2CM_STAT0_DONE);
+
+	/* Mute DONE and ERROR interrupts */
+	hdmi_writeb(pd, HDMI_IH_MUTE_I2CM_STAT0,
+		    HDMI_IH_I2CM_STAT0_ERROR | HDMI_IH_I2CM_STAT0_DONE);
+
+	spin_unlock_irqrestore(&pd->lock, flags);
+}
+
+static irqreturn_t imx_hdmi_i2c_isr(int irq, void *dev)
+{
+	struct imx_hdmi_i2c *pd = dev;
+	unsigned long flags;
+
+	spin_lock_irqsave(&pd->lock, flags);
+
+	pd->stat = hdmi_readb(pd, HDMI_IH_I2CM_STAT0);
+	if (!pd->stat) {
+		spin_unlock_irqrestore(&pd->lock, flags);
+		return IRQ_NONE;
+	}
+
+	dev_dbg(pd->dev, "irq: 0x%02x, addr: 0x%02x, reg: 0x%02x\n",
+		pd->stat, hdmi_readb(pd, HDMI_I2CM_SLAVE),
+		hdmi_readb(pd, HDMI_I2CM_ADDRESS));
+
+	hdmi_writeb(pd, HDMI_IH_I2CM_STAT0, pd->stat);
+	complete(&pd->cmp);
+
+	spin_unlock_irqrestore(&pd->lock, flags);
+
+	return IRQ_HANDLED;
+}
+
+static int xfer_read(struct imx_hdmi_i2c *pd, unsigned char *buf, int length)
+{
+	int stat;
+	unsigned long flags;
+
+	spin_lock_irqsave(&pd->lock, flags);
+
+	if (!pd->is_regaddr) {
+		dev_dbg(pd->dev, "set read register address to 0\n");
+		pd->slave_reg = 0x00;
+		pd->is_regaddr = true;
+	}
+
+	while (length--) {
+		hdmi_writeb(pd, HDMI_I2CM_ADDRESS, pd->slave_reg++);
+		hdmi_writeb(pd, HDMI_I2CM_OPERATION, HDMI_I2CM_OPERATION_READ);
+		pd->stat = 0;
+
+		spin_unlock_irqrestore(&pd->lock, flags);
+
+		stat = wait_for_completion_interruptible_timeout(&pd->cmp,
+								 HZ / 10);
+		if (!stat)
+			return -ETIMEDOUT;
+		if (stat < 0)
+			return stat;
+
+		spin_lock_irqsave(&pd->lock, flags);
+
+		/* Check for error condition on the bus */
+		if (pd->stat & HDMI_IH_I2CM_STAT0_ERROR) {
+			spin_unlock_irqrestore(&pd->lock, flags);
+			return -EIO;
+		}
+
+		*buf++ = hdmi_readb(pd, HDMI_I2CM_DATAI);
+	}
+
+	spin_unlock_irqrestore(&pd->lock, flags);
+
+	return 0;
+}
+
+static int xfer_write(struct imx_hdmi_i2c *pd, unsigned char *buf, int length)
+{
+	int stat;
+	unsigned long flags;
+
+	spin_lock_irqsave(&pd->lock, flags);
+
+	if (!pd->is_regaddr) {
+		if (length) {
+			/* Use the first write byte as register address */
+			pd->slave_reg = buf[0];
+			length--;
+			buf++;
+		} else {
+			dev_dbg(pd->dev, "set write register address to 0\n");
+			pd->slave_reg = 0x00;
+		}
+		pd->is_regaddr = true;
+	}
+
+	while (length--) {
+		hdmi_writeb(pd, HDMI_I2CM_DATAO, *buf++);
+		hdmi_writeb(pd, HDMI_I2CM_ADDRESS, pd->slave_reg++);
+		hdmi_writeb(pd, HDMI_I2CM_OPERATION, HDMI_I2CM_OPERATION_WRITE);
+		pd->stat = 0;
+
+		spin_unlock_irqrestore(&pd->lock, flags);
+
+		stat = wait_for_completion_interruptible_timeout(&pd->cmp,
+								 HZ / 10);
+		if (!stat)
+			return -ETIMEDOUT;
+		if (stat < 0)
+			return stat;
+
+		spin_lock_irqsave(&pd->lock, flags);
+
+		/* Check for error condition on the bus */
+		if (pd->stat & HDMI_IH_I2CM_STAT0_ERROR) {
+			spin_unlock_irqrestore(&pd->lock, flags);
+			return -EIO;
+		}
+	}
+
+	spin_unlock_irqrestore(&pd->lock, flags);
+
+	return 0;
+}
+
+static int i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
+{
+	struct imx_hdmi_i2c *pd = i2c_get_adapdata(adap);
+
+	int i, ret;
+	u8 addr;
+	unsigned long flags;
+
+	dev_dbg(pd->dev, "xfer: num: %d, addr: 0x%x\n", num, msgs[0].addr);
+
+	spin_lock_irqsave(&pd->lock, flags);
+
+	hdmi_writeb(pd, HDMI_IH_MUTE_I2CM_STAT0, 0x00);
+
+	/* Set slave device address from the first transaction */
+	addr = msgs[0].addr;
+	hdmi_writeb(pd, HDMI_I2CM_SLAVE, addr);
+
+	/* Set slave device register address on transfer */
+	pd->is_regaddr = false;
+
+	spin_unlock_irqrestore(&pd->lock, flags);
+
+	for (i = 0; i < num; i++) {
+		dev_dbg(pd->dev, "xfer: num: %d/%d, len: %d, flags: 0x%x\n",
+			i + 1, num, msgs[i].len, msgs[i].flags);
+
+		if (msgs[i].addr != addr) {
+			dev_warn(pd->dev,
+				 "unsupported transaction, changed slave address\n");
+			ret = -EOPNOTSUPP;
+			break;
+		}
+
+		if (msgs[i].len == 0) {
+			dev_dbg(pd->dev,
+				 "unsupported transaction %d/%d, no data\n",
+				 i + 1, num);
+			ret = -EOPNOTSUPP;
+			break;
+		}
+
+		if (msgs[i].flags & I2C_M_RD)
+			ret = xfer_read(pd, msgs[i].buf, msgs[i].len);
+		else
+			ret = xfer_write(pd, msgs[i].buf, msgs[i].len);
+
+		if (ret < 0)
+			break;
+	}
+
+	if (!ret)
+		ret = num;
+
+	spin_lock_irqsave(&pd->lock, flags);
+
+	/* Mute DONE and ERROR interrupts */
+	hdmi_writeb(pd, HDMI_IH_MUTE_I2CM_STAT0,
+		    HDMI_IH_I2CM_STAT0_ERROR | HDMI_IH_I2CM_STAT0_DONE);
+
+	spin_unlock_irqrestore(&pd->lock, flags);
+
+	return ret;
+}
+
+static u32 i2c_func(struct i2c_adapter *adapter)
+{
+	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
+}
+
+static struct i2c_algorithm imx_hdmi_algorithm = {
+	.master_xfer	= i2c_xfer,
+	.functionality	= i2c_func,
+};
+
+static int imx_hdmi_i2c_probe(struct platform_device *pdev)
+{
+	struct imx_hdmi_i2c *pd;
+	struct i2c_adapter *adap;
+	struct resource *res;
+	int ret, irq;
+
+	pd = devm_kzalloc(&pdev->dev, sizeof(struct imx_hdmi_i2c), GFP_KERNEL);
+	if (!pd)
+		return -ENOMEM;
+	pd->dev = &pdev->dev;
+
+	/*
+	 * iMX6 HDMI controller memory region is shared among device drivers,
+	 * therefore do not ioremap_resource() it exclusively.
+	 */
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	pd->base = devm_ioremap(pd->dev, res->start, resource_size(res));
+	if (IS_ERR(pd->base)) {
+		dev_err(pd->dev, "unable to ioremap: %ld\n", PTR_ERR(pd->base));
+		return PTR_ERR(pd->base);
+	}
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
+		dev_err(pd->dev, "no irq resource\n");
+		return irq;
+	}
+
+	ret = devm_request_irq(pd->dev, irq, imx_hdmi_i2c_isr,
+			       IRQF_SHARED, "imx hdmi i2c", pd);
+	if (ret < 0) {
+		dev_err(pd->dev, "unable to request irq %d: %d\n", irq, ret);
+		return ret;
+	}
+
+	pd->isfr_clk = devm_clk_get(pd->dev, "isfr");
+	if (IS_ERR(pd->isfr_clk)) {
+		ret = PTR_ERR(pd->isfr_clk);
+		dev_err(pd->dev, "unable to get HDMI isfr clk: %d\n", ret);
+		return ret;
+	}
+
+	ret = clk_prepare_enable(pd->isfr_clk);
+	if (ret) {
+		dev_err(pd->dev, "cannot enable HDMI isfr clock: %d\n", ret);
+		return ret;
+	}
+
+	pd->iahb_clk = devm_clk_get(pd->dev, "iahb");
+	if (IS_ERR(pd->iahb_clk)) {
+		ret = PTR_ERR(pd->iahb_clk);
+		dev_err(pd->dev, "unable to get HDMI iahb clk: %d\n", ret);
+		goto err_isfr;
+	}
+
+	ret = clk_prepare_enable(pd->iahb_clk);
+	if (ret) {
+		dev_err(pd->dev, "cannot enable HDMI iahb clock: %d\n", ret);
+		goto err_isfr;
+	}
+
+	spin_lock_init(&pd->lock);
+	init_completion(&pd->cmp);
+
+	adap = &pd->adap;
+	adap->algo = &imx_hdmi_algorithm;
+	adap->dev.parent = pd->dev;
+	adap->nr = pdev->id;
+	adap->dev.of_node = pdev->dev.of_node;
+	i2c_set_adapdata(adap, pd);
+	strlcpy(adap->name, pdev->name, sizeof(adap->name));
+
+	ret = i2c_add_numbered_adapter(adap);
+	if (ret) {
+		dev_err(pd->dev, "cannot add numbered adapter\n");
+		goto err_register;
+	}
+
+	platform_set_drvdata(pdev, pd);
+
+	/* reset I2C controller */
+	imx_hdmi_hwinit(pd);
+
+	dev_info(pd->dev, "registered %s bus driver\n", adap->name);
+
+	return 0;
+
+ err_register:
+	clk_disable_unprepare(pd->iahb_clk);
+ err_isfr:
+	clk_disable_unprepare(pd->isfr_clk);
+
+	return ret;
+}
+
+static int imx_hdmi_i2c_remove(struct platform_device *pdev)
+{
+	struct imx_hdmi_i2c *pd = platform_get_drvdata(pdev);
+
+	platform_set_drvdata(pdev, NULL);
+
+	clk_disable_unprepare(pd->iahb_clk);
+	clk_disable_unprepare(pd->isfr_clk);
+
+	i2c_del_adapter(&pd->adap);
+
+	return 0;
+}
+
+static const struct of_device_id imx_hdmi_ddc_dt_ids[] = {
+	{ .compatible = "fsl,imx6q-hdmi-ddc", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, imx_hdmi_ddc_dt_ids);
+
+static struct platform_driver imx_hdmi_i2c_driver = {
+	.driver		= {
+		.name	= "imx_hdmi_i2c",
+		.owner	= THIS_MODULE,
+		.of_match_table = imx_hdmi_ddc_dt_ids,
+	},
+	.probe		= imx_hdmi_i2c_probe,
+	.remove		= imx_hdmi_i2c_remove,
+};
+
+static int __init imx_hdmi_i2c_init(void)
+{
+	return platform_driver_register(&imx_hdmi_i2c_driver);
+}
+
+static void __exit imx_hdmi_i2c_exit(void)
+{
+	platform_driver_unregister(&imx_hdmi_i2c_driver);
+}
+
+subsys_initcall(imx_hdmi_i2c_init);
+module_exit(imx_hdmi_i2c_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("iMX6 HDMI DDC I2C master bus driver");
+MODULE_AUTHOR("Vladimir Zapolskiy <vladimir_zapolskiy-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>");
-- 
1.7.10.4

Re: [RFC PATCH] i2c: i2c-imx-hdmi: add support of iMX6 HDMI DDC I2C master bus

[Vladimir Zapolskiy]

Hi Wolfram, Philipp,

On 30.10.2014 18:54, Vladimir Zapolskiy wrote:
> Please review a proposed iMX6 HDMI DDC controller driver written on
> top of I2C framework.
> 
> The driver intends to support HDMI on-controller I2C master bus with
> limited cababilities, however by nature of the sub-device it seems
> reasonable to separate it into a stand-alone driver, which also can be
> used independently on general HDMI controller driver, for example as
> an additional I2C bus on a board, but please be aware that the bus is
> not compliant to I2C specification. To support the selected I2C bus
> approach, I suppose it should be possible to read/write e.g. a
> connected EEPROM device, not just HDMI EDID blobs.
> 
> In my practice I've met iMX6Q boards, where HDMI DDC lines are
> connected to iMX6 HDMI DDC pins and not to any of 3 I2C busses, so
> presence in the kernel of this driver or a driver with similar
> functionality seems to be important.
> 
> Main issues with the device/driver:
> * iMX6 HDMI controller becomes a shared device (both interrupt and
>   memory region) between HDMI driver itself and this I2C bus driver,
> * Only two patterns of I2C transactions are supported due to hardware
>   limitation, these patters are described in driver's code header.
> * The bus controller supports one more multi-byte read pattern (so
>   called Extended Read Mode), but Freescale documentation is too vague
>   to easily add this mode into the driver.
> 
> If somebody wants to test the driver, please do additional changes to
> the code:
> 
> -----8<-----8<-----8<-----8<-----8<-----
> 
> I. iMX6QDL SoC DTS:
> 
> diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
> index c701af9..8475e12 100644
> --- a/arch/arm/boot/dts/imx6qdl.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl.dtsi
> @@ -916,6 +916,19 @@
>  				status = "disabled";
>  			};
>  
> +			hdmi_ddc: i2c@00120000 {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +				compatible = "fsl,imx6q-hdmi-ddc";
> +				reg = <0x00120000 0x8000>;
> +				interrupts-extended =
> +					<&intc 0 115 IRQ_TYPE_LEVEL_HIGH>;
> +				clocks = <&clks IMX6QDL_CLK_HDMI_IAHB>,
> +					 <&clks IMX6QDL_CLK_HDMI_ISFR>;
> +				clock-names = "iahb", "isfr";
> +				status = "disabled";
> +			};
> +
>  			i2c1: i2c@021a0000 {
>  				#address-cells = <1>;
>  				#size-cells = <0>;
> 
> Also you may want to add an I2C alias to assign an I2C bus, iMX6Q example:
> 
> diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
> index e9f3646..e54d81e 100644
> --- a/arch/arm/boot/dts/imx6q.dtsi
> +++ b/arch/arm/boot/dts/imx6q.dtsi
> @@ -14,6 +14,7 @@
>  
>  / {
>  	aliases {
> +		i2c3 = &hdmi_ddc;
>  		spi4 = &ecspi5;
>  	};
>  
> 
> II. iMX6 Board DTS (iMX6Q SabreLite example), testing with main HDMI driver:
> 
> diff --git a/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi b/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
> index 0a36129..161593e 100644
> --- a/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
> @@ -173,6 +173,18 @@
>  	status = "okay";
>  };
>  
> +&hdmi {
> +	ddc-i2c-bus = <&hdmi_ddc>;
> +	status = "okay";
> +};
> +
> +&hdmi_ddc {
> +	clock-frequency = <100000>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_hdmi_ddc>;
> +	status = "okay";
> +};
> +
>  &i2c1 {
>  	clock-frequency = <100000>;
>  	pinctrl-names = "default";
> @@ -258,6 +270,13 @@
>  			>;
>  		};
>  
> +		pinctrl_hdmi_ddc: hdmi_ddcgrp {
> +			fsl,pins = <
> +				MX6QDL_PAD_KEY_COL3__HDMI_TX_DDC_SCL    0x4001b8b1
> +				MX6QDL_PAD_KEY_ROW3__HDMI_TX_DDC_SDA    0x4001b8b1
> +			>;
> +		};
> +
>  		pinctrl_i2c1: i2c1grp {
>  			fsl,pins = <
>  				MX6QDL_PAD_EIM_D21__I2C1_SCL		0x4001b8b1
> 
> III. iMX6 HDMI controller driver (remove I2C bus interrupt mutes):
> 
> diff --git a/drivers/staging/imx-drm/imx-hdmi.c b/drivers/staging/imx-drm/imx-hdmi.c
> index 18c9ccd..1914b0b 100644
> --- a/drivers/staging/imx-drm/imx-hdmi.c
> +++ b/drivers/staging/imx-drm/imx-hdmi.c
> @@ -1342,8 +1342,6 @@ static void initialize_hdmi_ih_mutes(struct imx_hdmi *hdmi)
>  	hdmi_writeb(hdmi, 0xff, HDMI_GP_MASK);
>  	hdmi_writeb(hdmi, 0xff, HDMI_A_APIINTMSK);
>  	hdmi_writeb(hdmi, 0xff, HDMI_CEC_MASK);
> -	hdmi_writeb(hdmi, 0xff, HDMI_I2CM_INT);
> -	hdmi_writeb(hdmi, 0xff, HDMI_I2CM_CTLINT);
>  
>  	/* Disable interrupts in the IH_MUTE_* registers */
>  	hdmi_writeb(hdmi, 0xff, HDMI_IH_MUTE_FC_STAT0);
> @@ -1351,7 +1349,6 @@ static void initialize_hdmi_ih_mutes(struct imx_hdmi *hdmi)
>  	hdmi_writeb(hdmi, 0xff, HDMI_IH_MUTE_FC_STAT2);
>  	hdmi_writeb(hdmi, 0xff, HDMI_IH_MUTE_AS_STAT0);
>  	hdmi_writeb(hdmi, 0xff, HDMI_IH_MUTE_PHY_STAT0);
> -	hdmi_writeb(hdmi, 0xff, HDMI_IH_MUTE_I2CM_STAT0);
>  	hdmi_writeb(hdmi, 0xff, HDMI_IH_MUTE_CEC_STAT0);
>  	hdmi_writeb(hdmi, 0xff, HDMI_IH_MUTE_VP_STAT0);
>  	hdmi_writeb(hdmi, 0xff, HDMI_IH_MUTE_I2CMPHY_STAT0);
> 
> -----8<-----8<-----8<-----8<-----8<-----
> 
> As I mentioned above the bus driver may be used independently on
> presence/absence of iMX6 HDMI driver.
> 
> I'd appreciate to get review responses, if the selected approach based
> on I2C framework is acceptable, I'll remove RFC tag and add updates
> of all the missing parts (iMX6 DTS files, Documentation, HDMI driver
> and addressed review comments). Comments how to gracefully align this
> driver with iMX HDMI driver are welcome as well.
> 
> The patch is based and tested on 3.17, but it is cleanly applicable to
> 3.18-rc2.
> 
> Vladimir Zapolskiy (1):
>   i2c: i2c-imx-hdmi: add support of iMX6 HDMI DDC I2C master bus
> 
>  drivers/i2c/busses/Kconfig        |   13 +
>  drivers/i2c/busses/Makefile       |    1 +
>  drivers/i2c/busses/i2c-imx-hdmi.c |  485 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 499 insertions(+)
>  create mode 100644 drivers/i2c/busses/i2c-imx-hdmi.c
> 

unfortunately I haven't received any comments about acceptance or
aversion of the selected approach for handling iMX6 HDMI DDC bus,
especially I was interested in DTS layout. So, I plan to resend the
proposed change for review with minor changes only, add DTS
documentation and remove RFC tag, hopefully inclusion into the next may
happen.

Please feel free to recommend any potentially interested or concerned
persons into the list of reviewers.

--
With best wishes,
Vladimir