* [PATCH 1/3] staging: imx-drm: document internal HDMI I2C master controller DT binding
[not found] ` <1416073759-19939-1-git-send-email-vladimir_zapolskiy-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
@ 2014-11-15 17:49 ` Vladimir Zapolskiy
[not found] ` <1416073759-19939-2-git-send-email-vladimir_zapolskiy-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
2014-11-15 17:49 ` [PATCH 2/3] i2c: i2c-imx-hdmi: add support of iMX6 HDMI DDC I2C master bus Vladimir Zapolskiy
` (2 subsequent siblings)
3 siblings, 1 reply; 12+ messages in thread
From: Vladimir Zapolskiy @ 2014-11-15 17:49 UTC (permalink / raw)
To: Wolfram Sang, Philipp Zabel
Cc: Shawn Guo, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-media-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-i2c-u79uwXL29TY76Z2rM5mHXA
Provide information about how to bind internal iMX6Q/DL HDMI DDC I2C
master controller. The property is set as optional one, because iMX6
HDMI DDC bus may be represented by one of general purpose I2C busses
found on SoC.
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: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: linux-media-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
---
Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt b/Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt
index 1b756cf..43c8924 100644
--- a/Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt
+++ b/Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt
@@ -10,6 +10,8 @@ Required properties:
- #address-cells : should be <1>
- #size-cells : should be <0>
- compatible : should be "fsl,imx6q-hdmi" or "fsl,imx6dl-hdmi".
+ If internal HDMI DDC I2C master controller is supposed to be used,
+ then "simple-bus" should be added to compatible value.
- gpr : should be <&gpr>.
The phandle points to the iomuxc-gpr region containing the HDMI
multiplexer control register.
@@ -22,6 +24,7 @@ Required properties:
Optional properties:
- ddc-i2c-bus: phandle of an I2C controller used for DDC EDID probing
+ - ddc: internal HDMI DDC I2C master controller
example:
@@ -32,7 +35,7 @@ example:
hdmi: hdmi@0120000 {
#address-cells = <1>;
#size-cells = <0>;
- compatible = "fsl,imx6q-hdmi";
+ compatible = "fsl,imx6q-hdmi", "simple-bus";
reg = <0x00120000 0x9000>;
interrupts = <0 115 0x04>;
gpr = <&gpr>;
@@ -40,6 +43,11 @@ example:
clock-names = "iahb", "isfr";
ddc-i2c-bus = <&i2c2>;
+ hdmi_ddc: ddc {
+ compatible = "fsl,imx6q-hdmi-ddc";
+ status = "disabled";
+ };
+
port@0 {
reg = <0>;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/3] i2c: i2c-imx-hdmi: add support of iMX6 HDMI DDC I2C master bus
[not found] ` <1416073759-19939-1-git-send-email-vladimir_zapolskiy-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
2014-11-15 17:49 ` [PATCH 1/3] staging: imx-drm: document internal HDMI I2C master controller DT binding Vladimir Zapolskiy
@ 2014-11-15 17:49 ` Vladimir Zapolskiy
2014-11-15 17:49 ` [PATCH 3/3] i2c: i2c-imx-hdmi: add documentation file of the bus Vladimir Zapolskiy
2014-11-24 12:20 ` [PATCH 0/3] i2c: i2c-imx-hdmi: add support of iMX6 HDMI DDC I2C bus Wolfram Sang
3 siblings, 0 replies; 12+ messages in thread
From: Vladimir Zapolskiy @ 2014-11-15 17:49 UTC (permalink / raw)
To: Wolfram Sang, Philipp Zabel; +Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA
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 | 510 +++++++++++++++++++++++++++++++++++++
3 files changed, 524 insertions(+)
create mode 100644 drivers/i2c/busses/i2c-imx-hdmi.c
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 06e99eb..82848a9 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -1045,4 +1045,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 78d56c5..49bebb0 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -99,6 +99,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..f9b0118
--- /dev/null
+++ b/drivers/i2c/busses/i2c-imx-hdmi.c
@@ -0,0 +1,510 @@
+/*
+ * 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 supported 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.
+ */
+
+#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;
+ bool initialized;
+
+ void __iomem *base;
+ int irq;
+ 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);
+
+ pd->initialized = true;
+
+ 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);
+
+ /* irq is shared, make sure driver probe is completed */
+ if (!pd->initialized) {
+ spin_unlock_irqrestore(&pd->lock, flags);
+ return IRQ_NONE;
+ }
+
+ 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;
+ struct platform_device *hdmi_pdev;
+ int ret;
+
+ pd = devm_kzalloc(&pdev->dev, sizeof(*pd), GFP_KERNEL);
+ if (!pd)
+ return -ENOMEM;
+ pd->dev = &pdev->dev;
+
+ /* get platform resources from HDMI controller device */
+ hdmi_pdev = to_platform_device(pdev->dev.parent);
+ if (!hdmi_pdev)
+ return -ENODEV;
+
+ dev_dbg(pd->dev, "platform device: %s\n", hdmi_pdev->name);
+
+ /*
+ * iMX6 HDMI controller memory region is shared among device drivers,
+ * therefore do not ioremap_resource() it exclusively.
+ */
+ res = platform_get_resource(hdmi_pdev, IORESOURCE_MEM, 0);
+ if (!res)
+ return -ENODEV;
+
+ 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);
+ }
+
+ pd->irq = platform_get_irq(hdmi_pdev, 0);
+ if (pd->irq < 0) {
+ dev_err(pd->dev, "no irq resource\n");
+ return pd->irq;
+ }
+
+ ret = devm_request_irq(pd->dev, pd->irq, imx_hdmi_i2c_isr,
+ IRQF_SHARED, "imx hdmi i2c", pd);
+ if (ret < 0) {
+ dev_err(pd->dev, "unable to request irq: %d\n", ret);
+ return ret;
+ }
+
+ pd->isfr_clk = devm_clk_get(&hdmi_pdev->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(&hdmi_pdev->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->owner = THIS_MODULE;
+ adap->algo = &imx_hdmi_algorithm;
+ adap->dev.parent = pd->dev;
+ adap->nr = pdev->id;
+ adap->dev.of_node = pdev->dev.of_node;
+ strlcpy(adap->name, pdev->name, sizeof(adap->name));
+ i2c_set_adapdata(adap, pd);
+
+ 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 HDMI DDC I2C master controller */
+ imx_hdmi_hwinit(pd);
+
+ dev_info(pd->dev, "registered %s I2C 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);
+
+ dev_dbg(pd->dev, "deregistering %s I2C bus driver\n", pd->adap.name);
+
+ /* deregister irq handler early */
+ devm_free_irq(pd->dev, pd->irq, pd);
+
+ pd->initialized = false;
+ 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 driver");
+MODULE_AUTHOR("Vladimir Zapolskiy <vladimir_zapolskiy-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>");
--
1.7.10.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 3/3] i2c: i2c-imx-hdmi: add documentation file of the bus
[not found] ` <1416073759-19939-1-git-send-email-vladimir_zapolskiy-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
2014-11-15 17:49 ` [PATCH 1/3] staging: imx-drm: document internal HDMI I2C master controller DT binding Vladimir Zapolskiy
2014-11-15 17:49 ` [PATCH 2/3] i2c: i2c-imx-hdmi: add support of iMX6 HDMI DDC I2C master bus Vladimir Zapolskiy
@ 2014-11-15 17:49 ` Vladimir Zapolskiy
2014-11-24 12:20 ` [PATCH 0/3] i2c: i2c-imx-hdmi: add support of iMX6 HDMI DDC I2C bus Wolfram Sang
3 siblings, 0 replies; 12+ messages in thread
From: Vladimir Zapolskiy @ 2014-11-15 17:49 UTC (permalink / raw)
To: Wolfram Sang, Philipp Zabel; +Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA
Add simple description of iMX6 HDMI DDC controller used as a I2C
master.
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
---
Documentation/i2c/busses/i2c-imx-hdmi | 62 +++++++++++++++++++++++++++++++++
1 file changed, 62 insertions(+)
create mode 100644 Documentation/i2c/busses/i2c-imx-hdmi
diff --git a/Documentation/i2c/busses/i2c-imx-hdmi b/Documentation/i2c/busses/i2c-imx-hdmi
new file mode 100644
index 0000000..20c7101
--- /dev/null
+++ b/Documentation/i2c/busses/i2c-imx-hdmi
@@ -0,0 +1,62 @@
+Kernel driver i2c-imx-hdmi
+
+Supported adapters:
+ * Freescale iMX6Quad HDMI controller
+ * Freescale iMX6DualLite HDMI controller
+ * Freescale iMX6Solo HDMI controller
+
+Datasheets:
+ Publicly available at the Freescale website
+
+Authors:
+ Vladimir Zapolskiy <vladimir_zapolskiy-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
+
+
+Description
+-----------
+
+According to the iMX6 Reference Manual only two types of transactions
+are supported by HDMI I2C Master Interface in Normal Mode:
+
+A) one byte data write transaction (I2C spec write/write 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]|
+
+HDMI I2C Master Interface Extended Read Mode allows to execute one
+more transaction pattern so called segment pointer-based read:
+
+C) segment pointer-based read (I2C spec write/write/read combined format):
+
+ master S|seg addr[6:0]|0| |seg pointer[7:0]| | ...
+ slave | xxx | | xxx | ...
+
+ master ... Sr|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]|
+
+At the moment Extended Read Mode is not supported by the driver.
+
+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,
+but the primary goad of the device is to read EDID blob of a connected
+HDMI monitor.
+
+
+Notes
+-----
+
+The bus driver may be used independently on HDMI controller, but due
+to device limitations it can not support arbitrary slave devices.
--
1.7.10.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 0/3] i2c: i2c-imx-hdmi: add support of iMX6 HDMI DDC I2C bus
[not found] ` <1416073759-19939-1-git-send-email-vladimir_zapolskiy-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
` (2 preceding siblings ...)
2014-11-15 17:49 ` [PATCH 3/3] i2c: i2c-imx-hdmi: add documentation file of the bus Vladimir Zapolskiy
@ 2014-11-24 12:20 ` Wolfram Sang
2014-11-24 19:15 ` Vladimir Zapolskiy
3 siblings, 1 reply; 12+ messages in thread
From: Wolfram Sang @ 2014-11-24 12:20 UTC (permalink / raw)
To: Vladimir Zapolskiy; +Cc: Philipp Zabel, linux-i2c-u79uwXL29TY76Z2rM5mHXA
[-- Attachment #1: Type: text/plain, Size: 2069 bytes --]
On Sat, Nov 15, 2014 at 07:49:16PM +0200, Vladimir Zapolskiy wrote:
> Please review a proposed iMX6 HDMI DDC controller driver written on
> top of I2C framework.
>
> Changes from RFC to v1 version:
> * added I2C bus ducumentation,
> * updated iMX6 HDMI device tree bindings documentation,
> * device resource information is collected from parent HDMI phandle,
> * minor clean-ups.
>
> 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.
>
> 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
> support of iMX6 HDMI I2C bus is important to have.
>
> 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.
>
> The bus driver may be used independently on presence/absence of iMX6
> HDMI controller driver, so build dependeny is not set.
I think there are now 3 drivers in my queue which are not fully I2C
compatible but more supporting the very minimum to, say, read an eeprom.
I am not feeling well to allow them to use I2C_FUNC_I2C. So, I want to
think about ways how to communicate deficiencies like "only 255 byte" or
"only WRRD messages" to users of that I2C controller. This is most
likely not happening before 3.19. But assistance is very welcome.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/3] i2c: i2c-imx-hdmi: add support of iMX6 HDMI DDC I2C bus
2014-11-24 12:20 ` [PATCH 0/3] i2c: i2c-imx-hdmi: add support of iMX6 HDMI DDC I2C bus Wolfram Sang
@ 2014-11-24 19:15 ` Vladimir Zapolskiy
0 siblings, 0 replies; 12+ messages in thread
From: Vladimir Zapolskiy @ 2014-11-24 19:15 UTC (permalink / raw)
To: Wolfram Sang; +Cc: Philipp Zabel, linux-i2c-u79uwXL29TY76Z2rM5mHXA
Hi Wolfram,
On 24.11.2014 14:20, Wolfram Sang wrote:
> On Sat, Nov 15, 2014 at 07:49:16PM +0200, Vladimir Zapolskiy wrote:
>> Please review a proposed iMX6 HDMI DDC controller driver written on
>> top of I2C framework.
>>
>> Changes from RFC to v1 version:
>> * added I2C bus ducumentation,
>> * updated iMX6 HDMI device tree bindings documentation,
>> * device resource information is collected from parent HDMI phandle,
>> * minor clean-ups.
>>
>> 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.
>>
>> 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
>> support of iMX6 HDMI I2C bus is important to have.
>>
>> 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.
>>
>> The bus driver may be used independently on presence/absence of iMX6
>> HDMI controller driver, so build dependeny is not set.
thank you for review.
> I think there are now 3 drivers in my queue which are not fully I2C
> compatible but more supporting the very minimum to, say, read an eeprom.
> I am not feeling well to allow them to use I2C_FUNC_I2C. So, I want to
> think about ways how to communicate deficiencies like "only 255 byte" or
> "only WRRD messages" to users of that I2C controller. This is most
> likely not happening before 3.19. But assistance is very welcome.
>
Yeah, it sounds absolutely correct *not* to allow such drivers (and this
one in particular) to provide naked I2C_FUNC_I2C.
Is there any chance to extend I2C_FUNC_* list? It still has some free
bits, not so many though.
Probably it might be possible to create another list of quirks or
exceptions (in opposite to functionality) and export quirks of a
particular I2C bus to userspace over some userspace to kernel channel
(e.g. adding one more ioctl).
--
With best wishes,
Vladimir
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] staging: imx-drm: document internal HDMI I2C master controller DT binding
[not found] ` <1416073759-19939-2-git-send-email-vladimir_zapolskiy-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
@ 2014-12-01 14:54 ` Vladimir Zapolskiy
2014-12-01 15:11 ` Philipp Zabel
0 siblings, 1 reply; 12+ messages in thread
From: Vladimir Zapolskiy @ 2014-12-01 14:54 UTC (permalink / raw)
To: Philipp Zabel, Shawn Guo
Cc: Wolfram Sang, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-media-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-i2c-u79uwXL29TY76Z2rM5mHXA
Hi Philipp and Shawn,
On 15.11.2014 19:49, Vladimir Zapolskiy wrote:
> Provide information about how to bind internal iMX6Q/DL HDMI DDC I2C
> master controller. The property is set as optional one, because iMX6
> HDMI DDC bus may be represented by one of general purpose I2C busses
> found on SoC.
>
> 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: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: linux-media-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> ---
> Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt b/Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt
> index 1b756cf..43c8924 100644
> --- a/Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt
> +++ b/Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt
> @@ -10,6 +10,8 @@ Required properties:
> - #address-cells : should be <1>
> - #size-cells : should be <0>
> - compatible : should be "fsl,imx6q-hdmi" or "fsl,imx6dl-hdmi".
> + If internal HDMI DDC I2C master controller is supposed to be used,
> + then "simple-bus" should be added to compatible value.
> - gpr : should be <&gpr>.
> The phandle points to the iomuxc-gpr region containing the HDMI
> multiplexer control register.
> @@ -22,6 +24,7 @@ Required properties:
>
> Optional properties:
> - ddc-i2c-bus: phandle of an I2C controller used for DDC EDID probing
> + - ddc: internal HDMI DDC I2C master controller
>
> example:
>
> @@ -32,7 +35,7 @@ example:
> hdmi: hdmi@0120000 {
> #address-cells = <1>;
> #size-cells = <0>;
> - compatible = "fsl,imx6q-hdmi";
> + compatible = "fsl,imx6q-hdmi", "simple-bus";
> reg = <0x00120000 0x9000>;
> interrupts = <0 115 0x04>;
> gpr = <&gpr>;
> @@ -40,6 +43,11 @@ example:
> clock-names = "iahb", "isfr";
> ddc-i2c-bus = <&i2c2>;
>
> + hdmi_ddc: ddc {
> + compatible = "fsl,imx6q-hdmi-ddc";
> + status = "disabled";
> + };
> +
> port@0 {
> reg = <0>;
>
>
knowing in advance that I2C framework lacks a graceful support of non
fully compliant I2C devices, do you have any objections to the proposed
iMX HDMI DTS change?
--
With best wishes,
Vladimir
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] staging: imx-drm: document internal HDMI I2C master controller DT binding
2014-12-01 14:54 ` Vladimir Zapolskiy
@ 2014-12-01 15:11 ` Philipp Zabel
[not found] ` <1417446703.4624.18.camel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
0 siblings, 1 reply; 12+ messages in thread
From: Philipp Zabel @ 2014-12-01 15:11 UTC (permalink / raw)
To: Vladimir Zapolskiy
Cc: Shawn Guo, Wolfram Sang, devicetree, linux-media,
linux-arm-kernel, linux-i2c
Am Montag, den 01.12.2014, 16:54 +0200 schrieb Vladimir Zapolskiy:
> Hi Philipp and Shawn,
>
> On 15.11.2014 19:49, Vladimir Zapolskiy wrote:
> > Provide information about how to bind internal iMX6Q/DL HDMI DDC I2C
> > master controller. The property is set as optional one, because iMX6
> > HDMI DDC bus may be represented by one of general purpose I2C busses
> > found on SoC.
> >
> > Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
> > Cc: Wolfram Sang <wsa@the-dreams.de>
> > Cc: Philipp Zabel <p.zabel@pengutronix.de>
> > Cc: Shawn Guo <shawn.guo@linaro.org>
> > Cc: devicetree@vger.kernel.org
> > Cc: linux-media@vger.kernel.org
> > Cc: linux-arm-kernel@lists.infradead.org
> > Cc: linux-i2c@vger.kernel.org
> > ---
> > Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt | 10 +++++++++-
> > 1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt b/Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt
> > index 1b756cf..43c8924 100644
> > --- a/Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt
> > +++ b/Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt
> > @@ -10,6 +10,8 @@ Required properties:
> > - #address-cells : should be <1>
> > - #size-cells : should be <0>
> > - compatible : should be "fsl,imx6q-hdmi" or "fsl,imx6dl-hdmi".
> > + If internal HDMI DDC I2C master controller is supposed to be used,
> > + then "simple-bus" should be added to compatible value.
> > - gpr : should be <&gpr>.
> > The phandle points to the iomuxc-gpr region containing the HDMI
> > multiplexer control register.
> > @@ -22,6 +24,7 @@ Required properties:
> >
> > Optional properties:
> > - ddc-i2c-bus: phandle of an I2C controller used for DDC EDID probing
> > + - ddc: internal HDMI DDC I2C master controller
> >
> > example:
> >
> > @@ -32,7 +35,7 @@ example:
> > hdmi: hdmi@0120000 {
> > #address-cells = <1>;
> > #size-cells = <0>;
> > - compatible = "fsl,imx6q-hdmi";
> > + compatible = "fsl,imx6q-hdmi", "simple-bus";
> > reg = <0x00120000 0x9000>;
> > interrupts = <0 115 0x04>;
> > gpr = <&gpr>;
> > @@ -40,6 +43,11 @@ example:
> > clock-names = "iahb", "isfr";
> > ddc-i2c-bus = <&i2c2>;
> >
> > + hdmi_ddc: ddc {
> > + compatible = "fsl,imx6q-hdmi-ddc";
> > + status = "disabled";
> > + };
> > +
> > port@0 {
> > reg = <0>;
> >
> >
>
> knowing in advance that I2C framework lacks a graceful support of non
> fully compliant I2C devices, do you have any objections to the proposed
> iMX HDMI DTS change?
I'm not sure about this. Have you seen "drm: Decouple EDID parsing from
I2C adapter"? I feel like in the absence of a ddc-i2c-bus property the
imx-hdmi/dw-hdmi driver should try to use the internal HDMI DDC I2C
master controller, bypassing the I2C framework altogether.
regards
Philipp
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] staging: imx-drm: document internal HDMI I2C master controller DT binding
[not found] ` <1417446703.4624.18.camel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
@ 2014-12-01 15:39 ` Vladimir Zapolskiy
[not found] ` <547C8B9E.8050605-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
0 siblings, 1 reply; 12+ messages in thread
From: Vladimir Zapolskiy @ 2014-12-01 15:39 UTC (permalink / raw)
To: Philipp Zabel
Cc: Shawn Guo, Wolfram Sang, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-media-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-i2c-u79uwXL29TY76Z2rM5mHXA
On 01.12.2014 17:11, Philipp Zabel wrote:
> Am Montag, den 01.12.2014, 16:54 +0200 schrieb Vladimir Zapolskiy:
>> Hi Philipp and Shawn,
>>
>> On 15.11.2014 19:49, Vladimir Zapolskiy wrote:
>>> Provide information about how to bind internal iMX6Q/DL HDMI DDC I2C
>>> master controller. The property is set as optional one, because iMX6
>>> HDMI DDC bus may be represented by one of general purpose I2C busses
>>> found on SoC.
>>>
>>> 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: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>> Cc: linux-media-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>> Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
>>> Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>> ---
>>> Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt | 10 +++++++++-
>>> 1 file changed, 9 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt b/Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt
>>> index 1b756cf..43c8924 100644
>>> --- a/Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt
>>> +++ b/Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt
>>> @@ -10,6 +10,8 @@ Required properties:
>>> - #address-cells : should be <1>
>>> - #size-cells : should be <0>
>>> - compatible : should be "fsl,imx6q-hdmi" or "fsl,imx6dl-hdmi".
>>> + If internal HDMI DDC I2C master controller is supposed to be used,
>>> + then "simple-bus" should be added to compatible value.
>>> - gpr : should be <&gpr>.
>>> The phandle points to the iomuxc-gpr region containing the HDMI
>>> multiplexer control register.
>>> @@ -22,6 +24,7 @@ Required properties:
>>>
>>> Optional properties:
>>> - ddc-i2c-bus: phandle of an I2C controller used for DDC EDID probing
>>> + - ddc: internal HDMI DDC I2C master controller
>>>
>>> example:
>>>
>>> @@ -32,7 +35,7 @@ example:
>>> hdmi: hdmi@0120000 {
>>> #address-cells = <1>;
>>> #size-cells = <0>;
>>> - compatible = "fsl,imx6q-hdmi";
>>> + compatible = "fsl,imx6q-hdmi", "simple-bus";
>>> reg = <0x00120000 0x9000>;
>>> interrupts = <0 115 0x04>;
>>> gpr = <&gpr>;
>>> @@ -40,6 +43,11 @@ example:
>>> clock-names = "iahb", "isfr";
>>> ddc-i2c-bus = <&i2c2>;
>>>
>>> + hdmi_ddc: ddc {
>>> + compatible = "fsl,imx6q-hdmi-ddc";
>>> + status = "disabled";
>>> + };
>>> +
>>> port@0 {
>>> reg = <0>;
>>>
>>>
>>
>> knowing in advance that I2C framework lacks a graceful support of non
>> fully compliant I2C devices, do you have any objections to the proposed
>> iMX HDMI DTS change?
>
> I'm not sure about this. Have you seen "drm: Decouple EDID parsing from
> I2C adapter"? I feel like in the absence of a ddc-i2c-bus property the
> imx-hdmi/dw-hdmi driver should try to use the internal HDMI DDC I2C
> master controller, bypassing the I2C framework altogether.
>
My idea is exactly not to bypass the I2C framework, briefly the
rationale is that
* it allows to reuse I2C UAPI/tools naturally applied to the internal
iMX HDMI DDC bus,
* it allows to use iMX HDMI DDC bus as an additional feature-limited I2C
bus on SoC (who knows, I absolutely won't be surprised, if anyone needs
it on practice),
* if an HDMI controller supports an external I2C bus, the integration
with HDMI DDC bus driver based on I2C framework is seamless.
However I agree that the selected approach may look odd, the question is
if the oddness comes from the technical side or from the fact that
nobody has done it before this way.
I'm open to any critique, if the proposal of creating an I2C bus from
HDMI DDC bus is lame, then I suppose the shared iMX HDMI DDC bus driver
should be converted to something formless and internally used by
imx-hdmi. The negative side-effects of such a change from my point of
view are
* more or less natural modularity is lost,
* a number of I2C framework API/functions should be copy-pasted to the
updated HDMI DDC bus driver to support a subset of I2C read/write
transactions.
--
With best wishes,
Vladimir
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] staging: imx-drm: document internal HDMI I2C master controller DT binding
[not found] ` <547C8B9E.8050605-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
@ 2014-12-01 16:22 ` Philipp Zabel
[not found] ` <1417450979.4624.23.camel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
0 siblings, 1 reply; 12+ messages in thread
From: Philipp Zabel @ 2014-12-01 16:22 UTC (permalink / raw)
To: Vladimir Zapolskiy
Cc: Shawn Guo, Wolfram Sang, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-media-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-i2c-u79uwXL29TY76Z2rM5mHXA, Andy Yan
Hi Vladimir,
[Added Andy Yan to Cc:, because imx-hdmi->dw-hdmi]
Am Montag, den 01.12.2014, 17:39 +0200 schrieb Vladimir Zapolskiy:
> On 01.12.2014 17:11, Philipp Zabel wrote:
> > Am Montag, den 01.12.2014, 16:54 +0200 schrieb Vladimir Zapolskiy:
> >> Hi Philipp and Shawn,
> >>
> >> On 15.11.2014 19:49, Vladimir Zapolskiy wrote:
> >>> Provide information about how to bind internal iMX6Q/DL HDMI DDC I2C
> >>> master controller. The property is set as optional one, because iMX6
> >>> HDMI DDC bus may be represented by one of general purpose I2C busses
> >>> found on SoC.
> >>>
> >>> 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: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> >>> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> >>> Cc: linux-media-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> >>> Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> >>> Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> >>> ---
> >>> Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt | 10 +++++++++-
> >>> 1 file changed, 9 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt b/Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt
> >>> index 1b756cf..43c8924 100644
> >>> --- a/Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt
> >>> +++ b/Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt
> >>> @@ -10,6 +10,8 @@ Required properties:
> >>> - #address-cells : should be <1>
> >>> - #size-cells : should be <0>
> >>> - compatible : should be "fsl,imx6q-hdmi" or "fsl,imx6dl-hdmi".
> >>> + If internal HDMI DDC I2C master controller is supposed to be used,
> >>> + then "simple-bus" should be added to compatible value.
> >>> - gpr : should be <&gpr>.
> >>> The phandle points to the iomuxc-gpr region containing the HDMI
> >>> multiplexer control register.
> >>> @@ -22,6 +24,7 @@ Required properties:
> >>>
> >>> Optional properties:
> >>> - ddc-i2c-bus: phandle of an I2C controller used for DDC EDID probing
> >>> + - ddc: internal HDMI DDC I2C master controller
> >>>
> >>> example:
> >>>
> >>> @@ -32,7 +35,7 @@ example:
> >>> hdmi: hdmi@0120000 {
> >>> #address-cells = <1>;
> >>> #size-cells = <0>;
> >>> - compatible = "fsl,imx6q-hdmi";
> >>> + compatible = "fsl,imx6q-hdmi", "simple-bus";
> >>> reg = <0x00120000 0x9000>;
> >>> interrupts = <0 115 0x04>;
> >>> gpr = <&gpr>;
> >>> @@ -40,6 +43,11 @@ example:
> >>> clock-names = "iahb", "isfr";
> >>> ddc-i2c-bus = <&i2c2>;
> >>>
> >>> + hdmi_ddc: ddc {
> >>> + compatible = "fsl,imx6q-hdmi-ddc";
> >>> + status = "disabled";
> >>> + };
> >>> +
> >>> port@0 {
> >>> reg = <0>;
> >>>
> >>>
> >>
> >> knowing in advance that I2C framework lacks a graceful support of non
> >> fully compliant I2C devices, do you have any objections to the proposed
> >> iMX HDMI DTS change?
> >
> > I'm not sure about this. Have you seen "drm: Decouple EDID parsing from
> > I2C adapter"? I feel like in the absence of a ddc-i2c-bus property the
> > imx-hdmi/dw-hdmi driver should try to use the internal HDMI DDC I2C
> > master controller, bypassing the I2C framework altogether.
> >
> My idea is exactly not to bypass the I2C framework, briefly the
> rationale is that
> * it allows to reuse I2C UAPI/tools naturally applied to the internal
> iMX HDMI DDC bus,
> * it allows to use iMX HDMI DDC bus as an additional feature-limited I2C
> bus on SoC (who knows, I absolutely won't be surprised, if anyone needs
> it on practice),
> * if an HDMI controller supports an external I2C bus, the integration
> with HDMI DDC bus driver based on I2C framework is seamless.
>
> However I agree that the selected approach may look odd, the question is
> if the oddness comes from the technical side or from the fact that
> nobody has done it before this way.
>
> I'm open to any critique, if the proposal of creating an I2C bus from
> HDMI DDC bus is lame, then I suppose the shared iMX HDMI DDC bus driver
> should be converted to something formless and internally used by
> imx-hdmi. The negative side-effects of such a change from my point of
> view are
> * more or less natural modularity is lost,
> * a number of I2C framework API/functions should be copy-pasted to the
> updated HDMI DDC bus driver to support a subset of I2C read/write
> transactions.
If Wolfram is happy to accomodate such feature limited, 'I2C master'
devices in i2c/drivers/busses in principle, I won't disagree.
But then it should be abstracted properly. The dw-hdmi-tx core on i.MX6
has the DDC I2C master register space at 0x7e00 - 0x7e12. What are the
offsets on the Rockchip version? If the "simple-bus" compatible is to be
set on the hdmi driver, the ddc driver should do its own register
access, and therefore needs a reg property. I suspect for the ddc-i2cm
we should get away with a common compatible like "snps,dw-hdmi-i2c".
hdmi: hdmi@120000 {
/* ... */
compatible = "fsl,imx6q-hdmi", "snps,dw-hdmi";
ddc-i2c-bus = <&hdmi_ddc>;
hdmi_ddc: i2c@127e00 {
compatible = "snps,dw-hdmi-i2c";
reg = <0x1207e00 0x13>
};
/* could add phy-i2cm, cec, ... here */
};
Also there's an i2c bus for communication with the phy at 0x3020 -
0x3032, should that be handled in a similar way, then?
Do we need to make the hdmi driver a proper interrupt controller? At
least the two i2c masters have two interrupts each, "done" and "error".
regards
Philipp
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] staging: imx-drm: document internal HDMI I2C master controller DT binding
[not found] ` <1417450979.4624.23.camel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
@ 2014-12-02 6:36 ` Andy Yan
[not found] ` <547D5DE2.2040704-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
0 siblings, 1 reply; 12+ messages in thread
From: Andy Yan @ 2014-12-02 6:36 UTC (permalink / raw)
To: Philipp Zabel, Vladimir Zapolskiy
Cc: Shawn Guo, Wolfram Sang, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-media-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-i2c-u79uwXL29TY76Z2rM5mHXA
Hi Vladimir:
I am working on convert imx-hdmi to dw_hdmi now:
https://lkml.org/lkml/2014/12/1/190
I also have a plan to use the internal HDMI I2C master under the I2c
framework,
and I also have a patch to do this work. So glad to see your work.
Please also Cc me<and.yan-TNX95d0MmH7DzftRWevZcw@public.gmane.org> and Zubair.Kakakhel@imgtec.com,
maybe Zubair also have interests on your future patch.
On 2014年12月02日 00:22, Philipp Zabel wrote:
> Hi Vladimir,
>
> [Added Andy Yan to Cc:, because imx-hdmi->dw-hdmi]
>
> Am Montag, den 01.12.2014, 17:39 +0200 schrieb Vladimir Zapolskiy:
>> On 01.12.2014 17:11, Philipp Zabel wrote:
>>> Am Montag, den 01.12.2014, 16:54 +0200 schrieb Vladimir Zapolskiy:
>>>> Hi Philipp and Shawn,
>>>>
>>>> On 15.11.2014 19:49, Vladimir Zapolskiy wrote:
>>>>> Provide information about how to bind internal iMX6Q/DL HDMI DDC I2C
>>>>> master controller. The property is set as optional one, because iMX6
>>>>> HDMI DDC bus may be represented by one of general purpose I2C busses
>>>>> found on SoC.
>>>>>
>>>>> 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: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>>>> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>>>> Cc: linux-media-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>>>> Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
>>>>> Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>>>> ---
>>>>> Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt | 10 +++++++++-
>>>>> 1 file changed, 9 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt b/Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt
>>>>> index 1b756cf..43c8924 100644
>>>>> --- a/Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt
>>>>> +++ b/Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt
>>>>> @@ -10,6 +10,8 @@ Required properties:
>>>>> - #address-cells : should be <1>
>>>>> - #size-cells : should be <0>
>>>>> - compatible : should be "fsl,imx6q-hdmi" or "fsl,imx6dl-hdmi".
>>>>> + If internal HDMI DDC I2C master controller is supposed to be used,
>>>>> + then "simple-bus" should be added to compatible value.
>>>>> - gpr : should be <&gpr>.
>>>>> The phandle points to the iomuxc-gpr region containing the HDMI
>>>>> multiplexer control register.
>>>>> @@ -22,6 +24,7 @@ Required properties:
>>>>>
>>>>> Optional properties:
>>>>> - ddc-i2c-bus: phandle of an I2C controller used for DDC EDID probing
>>>>> + - ddc: internal HDMI DDC I2C master controller
>>>>>
>>>>> example:
>>>>>
>>>>> @@ -32,7 +35,7 @@ example:
>>>>> hdmi: hdmi@0120000 {
>>>>> #address-cells = <1>;
>>>>> #size-cells = <0>;
>>>>> - compatible = "fsl,imx6q-hdmi";
>>>>> + compatible = "fsl,imx6q-hdmi", "simple-bus";
>>>>> reg = <0x00120000 0x9000>;
>>>>> interrupts = <0 115 0x04>;
>>>>> gpr = <&gpr>;
>>>>> @@ -40,6 +43,11 @@ example:
>>>>> clock-names = "iahb", "isfr";
>>>>> ddc-i2c-bus = <&i2c2>;
>>>>>
>>>>> + hdmi_ddc: ddc {
>>>>> + compatible = "fsl,imx6q-hdmi-ddc";
>>>>> + status = "disabled";
>>>>> + };
>>>>> +
>>>>> port@0 {
>>>>> reg = <0>;
>>>>>
>>>>>
>>>> knowing in advance that I2C framework lacks a graceful support of non
>>>> fully compliant I2C devices, do you have any objections to the proposed
>>>> iMX HDMI DTS change?
>>> I'm not sure about this. Have you seen "drm: Decouple EDID parsing from
>>> I2C adapter"? I feel like in the absence of a ddc-i2c-bus property the
>>> imx-hdmi/dw-hdmi driver should try to use the internal HDMI DDC I2C
>>> master controller, bypassing the I2C framework altogether.
>>>
>> My idea is exactly not to bypass the I2C framework, briefly the
>> rationale is that
>> * it allows to reuse I2C UAPI/tools naturally applied to the internal
>> iMX HDMI DDC bus,
>> * it allows to use iMX HDMI DDC bus as an additional feature-limited I2C
>> bus on SoC (who knows, I absolutely won't be surprised, if anyone needs
>> it on practice),
>> * if an HDMI controller supports an external I2C bus, the integration
>> with HDMI DDC bus driver based on I2C framework is seamless.
>>
>> However I agree that the selected approach may look odd, the question is
>> if the oddness comes from the technical side or from the fact that
>> nobody has done it before this way.
>>
>> I'm open to any critique, if the proposal of creating an I2C bus from
>> HDMI DDC bus is lame, then I suppose the shared iMX HDMI DDC bus driver
>> should be converted to something formless and internally used by
>> imx-hdmi. The negative side-effects of such a change from my point of
>> view are
>> * more or less natural modularity is lost,
>> * a number of I2C framework API/functions should be copy-pasted to the
>> updated HDMI DDC bus driver to support a subset of I2C read/write
>> transactions.
> If Wolfram is happy to accomodate such feature limited, 'I2C master'
> devices in i2c/drivers/busses in principle, I won't disagree.
>
> But then it should be abstracted properly. The dw-hdmi-tx core on i.MX6
> has the DDC I2C master register space at 0x7e00 - 0x7e12. What are the
> offsets on the Rockchip version? If the "simple-bus" compatible is to be
> set on the hdmi driver, the ddc driver should do its own register
> access, and therefore needs a reg property. I suspect for the ddc-i2cm
> we should get away with a common compatible like "snps,dw-hdmi-i2c".
>
> hdmi: hdmi@120000 {
> /* ... */
> compatible = "fsl,imx6q-hdmi", "snps,dw-hdmi";
> ddc-i2c-bus = <&hdmi_ddc>;
>
> hdmi_ddc: i2c@127e00 {
> compatible = "snps,dw-hdmi-i2c";
> reg = <0x1207e00 0x13>
> };
>
> /* could add phy-i2cm, cec, ... here */
> };
>
> Also there's an i2c bus for communication with the phy at 0x3020 -
> 0x3032, should that be handled in a similar way, then?
Rockchip RK3288 has the same DDC I2C master and phy i2c master
register offset as imx hdmi.
what my suggestion is: make the ddc-i2c-bus property optional, if
the dw_hdmi driver
can't found the ddc-i2c-bus node, then use the internal i2cm:
ddc_node = of_parse_phandle(np, "ddc-i2c-bus", 0);
if (ddc_node) {
hdmi->ddc = of_find_i2c_adapter_by_node(ddc_node);
of_node_put(ddc_node);
if (!hdmi->ddc) {
dev_dbg(hdmi->dev, "failed to read ddc node\n");
return -EPROBE_DEFER;
}
} else {
hdmi->ddc = dw_hdmi_ddc_adapter_register(hdmi).
}
and we can have a file put with dw_hdmi.c in a same dir, which
implement standard i2c adapter interface .
I found that other hdmi platform such as msm(/msm/hdmi/hdmi_i2c.c) ,
radeon(radeon_i2c.c) also do like this
>
> Do we need to make the hdmi driver a proper interrupt controller? At
> least the two i2c masters have two interrupts each, "done" and "error".
>
> regards
> Philipp
>
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] staging: imx-drm: document internal HDMI I2C master controller DT binding
[not found] ` <547D5DE2.2040704-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
@ 2014-12-02 13:11 ` Vladimir Zapolskiy
[not found] ` <547DBA99.1010703-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
0 siblings, 1 reply; 12+ messages in thread
From: Vladimir Zapolskiy @ 2014-12-02 13:11 UTC (permalink / raw)
To: Andy Yan, Philipp Zabel, Wolfram Sang
Cc: Shawn Guo, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-media-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-i2c-u79uwXL29TY76Z2rM5mHXA
Hi Andy,
thank you for joining the discussion.
On 02.12.2014 08:36, Andy Yan wrote:
> Hi Vladimir:
>
> I am working on convert imx-hdmi to dw_hdmi now:
> https://lkml.org/lkml/2014/12/1/190
> I also have a plan to use the internal HDMI I2C master under the I2c
> framework,
> and I also have a patch to do this work. So glad to see your work.
> Please also Cc me<and.yan-TNX95d0MmH7DzftRWevZcw@public.gmane.org> and Zubair.Kakakhel@imgtec.com,
> maybe Zubair also have interests on your future patch.
> On 2014年12月02日 00:22, Philipp Zabel wrote:
>> Hi Vladimir,
>>
>> [Added Andy Yan to Cc:, because imx-hdmi->dw-hdmi]
>>
>> Am Montag, den 01.12.2014, 17:39 +0200 schrieb Vladimir Zapolskiy:
>>> On 01.12.2014 17:11, Philipp Zabel wrote:
>>>> Am Montag, den 01.12.2014, 16:54 +0200 schrieb Vladimir Zapolskiy:
>>>>> Hi Philipp and Shawn,
>>>>>
>>>>> On 15.11.2014 19:49, Vladimir Zapolskiy wrote:
>>>>>> Provide information about how to bind internal iMX6Q/DL HDMI DDC I2C
>>>>>> master controller. The property is set as optional one, because iMX6
>>>>>> HDMI DDC bus may be represented by one of general purpose I2C busses
>>>>>> found on SoC.
>>>>>>
>>>>>> 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: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>>>>> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>>>>> Cc: linux-media-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>>>>> Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
>>>>>> Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>>>>> ---
>>>>>> Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt | 10 +++++++++-
>>>>>> 1 file changed, 9 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt b/Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt
>>>>>> index 1b756cf..43c8924 100644
>>>>>> --- a/Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt
>>>>>> +++ b/Documentation/devicetree/bindings/staging/imx-drm/hdmi.txt
>>>>>> @@ -10,6 +10,8 @@ Required properties:
>>>>>> - #address-cells : should be <1>
>>>>>> - #size-cells : should be <0>
>>>>>> - compatible : should be "fsl,imx6q-hdmi" or "fsl,imx6dl-hdmi".
>>>>>> + If internal HDMI DDC I2C master controller is supposed to be used,
>>>>>> + then "simple-bus" should be added to compatible value.
>>>>>> - gpr : should be <&gpr>.
>>>>>> The phandle points to the iomuxc-gpr region containing the HDMI
>>>>>> multiplexer control register.
>>>>>> @@ -22,6 +24,7 @@ Required properties:
>>>>>>
>>>>>> Optional properties:
>>>>>> - ddc-i2c-bus: phandle of an I2C controller used for DDC EDID probing
>>>>>> + - ddc: internal HDMI DDC I2C master controller
>>>>>>
>>>>>> example:
>>>>>>
>>>>>> @@ -32,7 +35,7 @@ example:
>>>>>> hdmi: hdmi@0120000 {
>>>>>> #address-cells = <1>;
>>>>>> #size-cells = <0>;
>>>>>> - compatible = "fsl,imx6q-hdmi";
>>>>>> + compatible = "fsl,imx6q-hdmi", "simple-bus";
>>>>>> reg = <0x00120000 0x9000>;
>>>>>> interrupts = <0 115 0x04>;
>>>>>> gpr = <&gpr>;
>>>>>> @@ -40,6 +43,11 @@ example:
>>>>>> clock-names = "iahb", "isfr";
>>>>>> ddc-i2c-bus = <&i2c2>;
>>>>>>
>>>>>> + hdmi_ddc: ddc {
>>>>>> + compatible = "fsl,imx6q-hdmi-ddc";
>>>>>> + status = "disabled";
>>>>>> + };
>>>>>> +
>>>>>> port@0 {
>>>>>> reg = <0>;
>>>>>>
>>>>>>
>>>>> knowing in advance that I2C framework lacks a graceful support of non
>>>>> fully compliant I2C devices, do you have any objections to the proposed
>>>>> iMX HDMI DTS change?
>>>> I'm not sure about this. Have you seen "drm: Decouple EDID parsing from
>>>> I2C adapter"? I feel like in the absence of a ddc-i2c-bus property the
>>>> imx-hdmi/dw-hdmi driver should try to use the internal HDMI DDC I2C
>>>> master controller, bypassing the I2C framework altogether.
>>>>
>>> My idea is exactly not to bypass the I2C framework, briefly the
>>> rationale is that
>>> * it allows to reuse I2C UAPI/tools naturally applied to the internal
>>> iMX HDMI DDC bus,
>>> * it allows to use iMX HDMI DDC bus as an additional feature-limited I2C
>>> bus on SoC (who knows, I absolutely won't be surprised, if anyone needs
>>> it on practice),
>>> * if an HDMI controller supports an external I2C bus, the integration
>>> with HDMI DDC bus driver based on I2C framework is seamless.
>>>
>>> However I agree that the selected approach may look odd, the question is
>>> if the oddness comes from the technical side or from the fact that
>>> nobody has done it before this way.
>>>
>>> I'm open to any critique, if the proposal of creating an I2C bus from
>>> HDMI DDC bus is lame, then I suppose the shared iMX HDMI DDC bus driver
>>> should be converted to something formless and internally used by
>>> imx-hdmi. The negative side-effects of such a change from my point of
>>> view are
>>> * more or less natural modularity is lost,
>>> * a number of I2C framework API/functions should be copy-pasted to the
>>> updated HDMI DDC bus driver to support a subset of I2C read/write
>>> transactions.
>> If Wolfram is happy to accomodate such feature limited, 'I2C master'
>> devices in i2c/drivers/busses in principle, I won't disagree.
>>
>> But then it should be abstracted properly. The dw-hdmi-tx core on i.MX6
>> has the DDC I2C master register space at 0x7e00 - 0x7e12. What are the
>> offsets on the Rockchip version? If the "simple-bus" compatible is to be
>> set on the hdmi driver, the ddc driver should do its own register
>> access, and therefore needs a reg property. I suspect for the ddc-i2cm
>> we should get away with a common compatible like "snps,dw-hdmi-i2c".
>>
>> hdmi: hdmi@120000 {
>> /* ... */
>> compatible = "fsl,imx6q-hdmi", "snps,dw-hdmi";
>> ddc-i2c-bus = <&hdmi_ddc>;
>>
>> hdmi_ddc: i2c@127e00 {
>> compatible = "snps,dw-hdmi-i2c";
>> reg = <0x1207e00 0x13>
>> };
>>
>> /* could add phy-i2cm, cec, ... here */
>> };
>>
>> Also there's an i2c bus for communication with the phy at 0x3020 -
>> 0x3032, should that be handled in a similar way, then?
>
> Rockchip RK3288 has the same DDC I2C master and phy i2c master
> register offset as imx hdmi.
Good to know, I was not aware of it. Definitely it should be supported
as well.
> what my suggestion is: make the ddc-i2c-bus property optional, if
> the dw_hdmi driver
> can't found the ddc-i2c-bus node, then use the internal i2cm:
> ddc_node = of_parse_phandle(np, "ddc-i2c-bus", 0);
> if (ddc_node) {
> hdmi->ddc = of_find_i2c_adapter_by_node(ddc_node);
> of_node_put(ddc_node);
> if (!hdmi->ddc) {
> dev_dbg(hdmi->dev, "failed to read ddc node\n");
> return -EPROBE_DEFER;
> }
>
> } else {
> hdmi->ddc = dw_hdmi_ddc_adapter_register(hdmi).
> }
>
> and we can have a file put with dw_hdmi.c in a same dir, which
> implement standard i2c adapter interface .
> I found that other hdmi platform such as msm(/msm/hdmi/hdmi_i2c.c) ,
> radeon(radeon_i2c.c) also do like this
Thank you for references.
I see that msm and a number of other DRM device drivers (radeon,
nouveau, i915, gma500) quite resemble the situation, now I'm convinced
that the same should be done for iMX6/RK3288 HDMI driver. In background
I'll try to review all out of i2c/busses/* registered i2c adapters, may
be there is something in common between all of them.
I'll prepare the change of the HDMI DDC support for review (will be able
to test it only on iMX6), Wolfram, please skip from your consideration
the published version of the i2c bus driver.
Wolfram, by the way is I2C_CLASS_DDC adapter class in operational use or
deprecated?
>
>>
>> Do we need to make the hdmi driver a proper interrupt controller? At
>> least the two i2c masters have two interrupts each, "done" and "error".
>>
>> regards
>> Philipp
>>
--
With best wishes,
Vladimir
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] staging: imx-drm: document internal HDMI I2C master controller DT binding
[not found] ` <547DBA99.1010703-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
@ 2014-12-02 13:21 ` Wolfram Sang
0 siblings, 0 replies; 12+ messages in thread
From: Wolfram Sang @ 2014-12-02 13:21 UTC (permalink / raw)
To: Vladimir Zapolskiy
Cc: Andy Yan, Philipp Zabel, Shawn Guo,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-media-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-i2c-u79uwXL29TY76Z2rM5mHXA
[-- Attachment #1: Type: text/plain, Size: 803 bytes --]
> Thank you for references.
Yes, thank you all. Really great to see this consolidation effort going
on!
> I'll try to review all out of i2c/busses/* registered i2c adapters, may
> be there is something in common between all of them.
Yay, cool! Thanks!
> I'll prepare the change of the HDMI DDC support for review (will be able
> to test it only on iMX6), Wolfram, please skip from your consideration
> the published version of the i2c bus driver.
Okay, will do.
> Wolfram, by the way is I2C_CLASS_DDC adapter class in operational use or
> deprecated?
Class based instantiation is NOT deprecated and will stay as far as I
can see. Many platform bus drivers did set the class flag needlessly, so
we had a deprecation mechanism to get rid of those flags in drivers in
order to speed up boot time.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2014-12-02 13:21 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1416073759-19939-1-git-send-email-vladimir_zapolskiy@mentor.com>
[not found] ` <1416073759-19939-1-git-send-email-vladimir_zapolskiy-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
2014-11-15 17:49 ` [PATCH 1/3] staging: imx-drm: document internal HDMI I2C master controller DT binding Vladimir Zapolskiy
[not found] ` <1416073759-19939-2-git-send-email-vladimir_zapolskiy-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
2014-12-01 14:54 ` Vladimir Zapolskiy
2014-12-01 15:11 ` Philipp Zabel
[not found] ` <1417446703.4624.18.camel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2014-12-01 15:39 ` Vladimir Zapolskiy
[not found] ` <547C8B9E.8050605-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
2014-12-01 16:22 ` Philipp Zabel
[not found] ` <1417450979.4624.23.camel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2014-12-02 6:36 ` Andy Yan
[not found] ` <547D5DE2.2040704-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
2014-12-02 13:11 ` Vladimir Zapolskiy
[not found] ` <547DBA99.1010703-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
2014-12-02 13:21 ` Wolfram Sang
2014-11-15 17:49 ` [PATCH 2/3] i2c: i2c-imx-hdmi: add support of iMX6 HDMI DDC I2C master bus Vladimir Zapolskiy
2014-11-15 17:49 ` [PATCH 3/3] i2c: i2c-imx-hdmi: add documentation file of the bus Vladimir Zapolskiy
2014-11-24 12:20 ` [PATCH 0/3] i2c: i2c-imx-hdmi: add support of iMX6 HDMI DDC I2C bus Wolfram Sang
2014-11-24 19:15 ` Vladimir Zapolskiy
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).