[PATCH v3 0/7] TDA998x CEC support

[PATCH v3 0/7] TDA998x CEC support

[Russell King - ARM Linux]

Hi,

This patch series adds CEC support to the DRM TDA998x driver.  The
TDA998x family of devices integrate a TDA9950 CEC at a separate I2C
address from the HDMI encoder.

Implementation of the CEC part is separate to allow independent CEC
implementations, or independent HDMI implementations (since the
TDA9950 may be a separate device.)

 .../devicetree/bindings/display/bridge/tda998x.txt |   3 +
 drivers/gpu/drm/i2c/Kconfig                        |   6 +
 drivers/gpu/drm/i2c/Makefile                       |   1 +
 drivers/gpu/drm/i2c/tda9950.c                      | 509 +++++++++++++++++++++
 drivers/gpu/drm/i2c/tda998x_drv.c                  | 242 ++++++++--
 include/linux/platform_data/tda9950.h              |  16 +
 6 files changed, 750 insertions(+), 27 deletions(-)
 create mode 100644 drivers/gpu/drm/i2c/tda9950.c
 create mode 100644 include/linux/platform_data/tda9950.h

v3: addressed most of Hans comments in v2
v2: updated DT property.
 
-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

[PATCH v3 1/7] drm/i2c: tda998x: move mutex/waitqueue/timer/work init early

[Russell King]

Move the mutex, waitqueue, timer and detect work initialisation early
in the driver's initialisation, rather than being after we've registered
the CEC device.

Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/gpu/drm/i2c/tda998x_drv.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index cd3f0873bbdd..83407159e957 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -1475,7 +1475,11 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv)
 	u32 video;
 	int rev_lo, rev_hi, ret;
 
-	mutex_init(&priv->audio_mutex); /* Protect access from audio thread */
+	mutex_init(&priv->mutex);	/* protect the page access */
+	mutex_init(&priv->audio_mutex); /* protect access from audio thread */
+	init_waitqueue_head(&priv->edid_delay_waitq);
+	timer_setup(&priv->edid_delay_timer, tda998x_edid_delay_done, 0);
+	INIT_WORK(&priv->detect_work, tda998x_detect_work);
 
 	priv->vip_cntrl_0 = VIP_CNTRL_0_SWAP_A(2) | VIP_CNTRL_0_SWAP_B(3);
 	priv->vip_cntrl_1 = VIP_CNTRL_1_SWAP_C(0) | VIP_CNTRL_1_SWAP_D(1);
@@ -1489,11 +1493,6 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv)
 	if (!priv->cec)
 		return -ENODEV;
 
-	mutex_init(&priv->mutex);	/* protect the page access */
-	init_waitqueue_head(&priv->edid_delay_waitq);
-	timer_setup(&priv->edid_delay_timer, tda998x_edid_delay_done, 0);
-	INIT_WORK(&priv->detect_work, tda998x_detect_work);
-
 	/* wake up the device: */
 	cec_write(priv, REG_CEC_ENAMODS,
 			CEC_ENAMODS_EN_RXSENS | CEC_ENAMODS_EN_HDMI);
-- 
2.7.4

[PATCH v3 2/7] drm/i2c: tda998x: fix error cleanup paths

[Russell King]

If tda998x_get_audio_ports() fails, and we requested the interrupt, we
fail to free the interrupt before returning failure.  Rework the failure
cleanup code and exit paths so that we always clean up properly after an
error, and always propagate the error code.

Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/gpu/drm/i2c/tda998x_drv.c | 31 ++++++++++++++++++-------------
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 83407159e957..2a99930f1bda 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -1501,10 +1501,15 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv)
 
 	/* read version: */
 	rev_lo = reg_read(priv, REG_VERSION_LSB);
+	if (rev_lo < 0) {
+		dev_err(&client->dev, "failed to read version: %d\n", rev_lo);
+		return rev_lo;
+	}
+
 	rev_hi = reg_read(priv, REG_VERSION_MSB);
-	if (rev_lo < 0 || rev_hi < 0) {
-		ret = rev_lo < 0 ? rev_lo : rev_hi;
-		goto fail;
+	if (rev_hi < 0) {
+		dev_err(&client->dev, "failed to read version: %d\n", rev_hi);
+		return rev_hi;
 	}
 
 	priv->rev = rev_lo | rev_hi << 8;
@@ -1528,7 +1533,7 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv)
 	default:
 		dev_err(&client->dev, "found unsupported device: %04x\n",
 			priv->rev);
-		goto fail;
+		return -ENXIO;
 	}
 
 	/* after reset, enable DDC: */
@@ -1566,7 +1571,7 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv)
 			dev_err(&client->dev,
 				"failed to request IRQ#%u: %d\n",
 				client->irq, ret);
-			goto fail;
+			goto err_irq;
 		}
 
 		/* enable HPD irq */
@@ -1589,19 +1594,19 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv)
 
 	ret = tda998x_get_audio_ports(priv, np);
 	if (ret)
-		goto fail;
+		goto err_audio;
 
 	if (priv->audio_port[0].format != AFMT_UNUSED)
 		tda998x_audio_codec_init(priv, &client->dev);
 
 	return 0;
-fail:
-	/* if encoder_init fails, the encoder slave is never registered,
-	 * so cleanup here:
-	 */
-	if (priv->cec)
-		i2c_unregister_device(priv->cec);
-	return -ENXIO;
+
+err_audio:
+	if (client->irq)
+		free_irq(client->irq, priv);
+err_irq:
+	i2c_unregister_device(priv->cec);
+	return ret;
 }
 
 static void tda998x_encoder_prepare(struct drm_encoder *encoder)
-- 
2.7.4

[PATCH v3 3/7] drm/i2c: tda998x: move CEC device initialisation later

[Russell King]

We no longer use the CEC client to access the CEC part itself, so we can
move this later in the initialisation sequence.

Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/gpu/drm/i2c/tda998x_drv.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 2a99930f1bda..7f2762fab5c9 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -1489,9 +1489,6 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv)
 	priv->cec_addr = 0x34 + (client->addr & 0x03);
 	priv->current_page = 0xff;
 	priv->hdmi = client;
-	priv->cec = i2c_new_dummy(client->adapter, priv->cec_addr);
-	if (!priv->cec)
-		return -ENODEV;
 
 	/* wake up the device: */
 	cec_write(priv, REG_CEC_ENAMODS,
@@ -1578,6 +1575,12 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv)
 		cec_write(priv, REG_CEC_RXSHPDINTENA, CEC_RXSHPDLEV_HPD);
 	}
 
+	priv->cec = i2c_new_dummy(client->adapter, priv->cec_addr);
+	if (!priv->cec) {
+		ret = -ENODEV;
+		goto fail;
+	}
+
 	/* enable EDID read irq: */
 	reg_set(priv, REG_INT_FLAGS_2, INT_FLAGS_2_EDID_BLK_RD);
 
@@ -1594,14 +1597,14 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv)
 
 	ret = tda998x_get_audio_ports(priv, np);
 	if (ret)
-		goto err_audio;
+		goto fail;
 
 	if (priv->audio_port[0].format != AFMT_UNUSED)
 		tda998x_audio_codec_init(priv, &client->dev);
 
 	return 0;
 
-err_audio:
+fail:
 	if (client->irq)
 		free_irq(client->irq, priv);
 err_irq:
-- 
2.7.4

[PATCH v3 4/7] drm/i2c: tda998x: always disable and clear interrupts at probe

[Russell King]

Always disable and clear interrupts at probe time to ensure that the
TDA998x is in a sane state.  This ensures that the interrupt line,
which is also the CEC clock calibration signal, is always deasserted.

Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/gpu/drm/i2c/tda998x_drv.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 7f2762fab5c9..16e0439cad44 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -1546,6 +1546,15 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv)
 	cec_write(priv, REG_CEC_FRO_IM_CLK_CTRL,
 			CEC_FRO_IM_CLK_CTRL_GHOST_DIS | CEC_FRO_IM_CLK_CTRL_IMCLK_SEL);
 
+	/* ensure interrupts are disabled */
+	cec_write(priv, REG_CEC_RXSHPDINTENA, 0);
+
+	/* clear pending interrupts */
+	cec_read(priv, REG_CEC_RXSHPDINT);
+	reg_read(priv, REG_INT_FLAGS_0);
+	reg_read(priv, REG_INT_FLAGS_1);
+	reg_read(priv, REG_INT_FLAGS_2);
+
 	/* initialize the optional IRQ */
 	if (client->irq) {
 		unsigned long irq_flags;
@@ -1553,11 +1562,6 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv)
 		/* init read EDID waitqueue and HDP work */
 		init_waitqueue_head(&priv->wq_edid);
 
-		/* clear pending interrupts */
-		reg_read(priv, REG_INT_FLAGS_0);
-		reg_read(priv, REG_INT_FLAGS_1);
-		reg_read(priv, REG_INT_FLAGS_2);
-
 		irq_flags =
 			irqd_get_trigger_type(irq_get_irq_data(client->irq));
 		irq_flags |= IRQF_SHARED | IRQF_ONESHOT;
-- 
2.7.4

[PATCH v3 5/7] drm/i2c: tda9950: add CEC driver

[Russell King]

Add a CEC driver for the TDA9950, which is a stand-alone I2C CEC device,
but is also integrated into HDMI transceivers such as the TDA9989 and
TDA19989.

The TDA9950 contains a command processor which handles retransmissions
and the low level bus protocol.  The driver just has to read and write
the messages, and handle error conditions.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/gpu/drm/i2c/Kconfig           |   5 +
 drivers/gpu/drm/i2c/Makefile          |   1 +
 drivers/gpu/drm/i2c/tda9950.c         | 509 ++++++++++++++++++++++++++++++++++
 include/linux/platform_data/tda9950.h |  16 ++
 4 files changed, 531 insertions(+)
 create mode 100644 drivers/gpu/drm/i2c/tda9950.c
 create mode 100644 include/linux/platform_data/tda9950.h

diff --git a/drivers/gpu/drm/i2c/Kconfig b/drivers/gpu/drm/i2c/Kconfig
index a6c92beb410a..3a232f5ff0a1 100644
--- a/drivers/gpu/drm/i2c/Kconfig
+++ b/drivers/gpu/drm/i2c/Kconfig
@@ -26,4 +26,9 @@ config DRM_I2C_NXP_TDA998X
 	help
 	  Support for NXP Semiconductors TDA998X HDMI encoders.
 
+config DRM_I2C_NXP_TDA9950
+	tristate "NXP Semiconductors TDA9950/TDA998X HDMI CEC"
+	select CEC_NOTIFIER
+	select CEC_CORE
+
 endmenu
diff --git a/drivers/gpu/drm/i2c/Makefile b/drivers/gpu/drm/i2c/Makefile
index b20100c18ffb..a962f6f08568 100644
--- a/drivers/gpu/drm/i2c/Makefile
+++ b/drivers/gpu/drm/i2c/Makefile
@@ -7,3 +7,4 @@ obj-$(CONFIG_DRM_I2C_SIL164) += sil164.o
 
 tda998x-y := tda998x_drv.o
 obj-$(CONFIG_DRM_I2C_NXP_TDA998X) += tda998x.o
+obj-$(CONFIG_DRM_I2C_NXP_TDA9950) += tda9950.o
diff --git a/drivers/gpu/drm/i2c/tda9950.c b/drivers/gpu/drm/i2c/tda9950.c
new file mode 100644
index 000000000000..3f7396caad48
--- /dev/null
+++ b/drivers/gpu/drm/i2c/tda9950.c
@@ -0,0 +1,509 @@
+/*
+ *  TDA9950 Consumer Electronics Control driver
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * The NXP TDA9950 implements the HDMI Consumer Electronics Control
+ * interface.  The host interface is similar to a mailbox: the data
+ * registers starting at REG_CDR0 are written to send a command to the
+ * internal CPU, and replies are read from these registers.
+ *
+ * As the data registers represent a mailbox, they must be accessed
+ * as a single I2C transaction.  See the TDA9950 data sheet for details.
+ */
+#include <linux/delay.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/platform_data/tda9950.h>
+#include <linux/slab.h>
+#include <drm/drm_edid.h>
+#include <media/cec.h>
+#include <media/cec-notifier.h>
+
+enum {
+	REG_CSR = 0x00,
+	CSR_BUSY = BIT(7),
+	CSR_INT  = BIT(6),
+	CSR_ERR  = BIT(5),
+
+	REG_CER = 0x01,
+
+	REG_CVR = 0x02,
+
+	REG_CCR = 0x03,
+	CCR_RESET = BIT(7),
+	CCR_ON    = BIT(6),
+
+	REG_ACKH = 0x04,
+	REG_ACKL = 0x05,
+
+	REG_CCONR = 0x06,
+	CCONR_ENABLE_ERROR = BIT(4),
+	CCONR_RETRY_MASK = 7,
+
+	REG_CDR0 = 0x07,
+
+	CDR1_REQ = 0x00,
+	CDR1_CNF = 0x01,
+	CDR1_IND = 0x81,
+	CDR1_ERR = 0x82,
+	CDR1_IER = 0x83,
+
+	CDR2_CNF_SUCCESS    = 0x00,
+	CDR2_CNF_OFF_STATE  = 0x80,
+	CDR2_CNF_BAD_REQ    = 0x81,
+	CDR2_CNF_CEC_ACCESS = 0x82,
+	CDR2_CNF_ARB_ERROR  = 0x83,
+	CDR2_CNF_BAD_TIMING = 0x84,
+	CDR2_CNF_NACK_ADDR  = 0x85,
+	CDR2_CNF_NACK_DATA  = 0x86,
+};
+
+struct tda9950_priv {
+	struct i2c_client *client;
+	struct device *hdmi;
+	struct cec_adapter *adap;
+	struct tda9950_glue *glue;
+	u16 addresses;
+	struct cec_msg rx_msg;
+	struct cec_notifier *notify;
+	bool open;
+};
+
+static int tda9950_write_range(struct i2c_client *client, u8 addr, u8 *p, int cnt)
+{
+	struct i2c_msg msg;
+	u8 buf[cnt + 1];
+	int ret;
+
+	buf[0] = addr;
+	memcpy(buf + 1, p, cnt);
+
+	msg.addr = client->addr;
+	msg.flags = 0;
+	msg.len = cnt + 1;
+	msg.buf = buf;
+
+	dev_dbg(&client->dev, "wr 0x%02x: %*ph\n", addr, cnt, p);
+
+	ret = i2c_transfer(client->adapter, &msg, 1);
+	if (ret < 0)
+		dev_err(&client->dev, "Error %d writing to cec:0x%x\n", ret, addr);
+	return ret < 0 ? ret : 0;
+}
+
+static void tda9950_write(struct i2c_client *client, u8 addr, u8 val)
+{
+	tda9950_write_range(client, addr, &val, 1);
+}
+
+static int tda9950_read_range(struct i2c_client *client, u8 addr, u8 *p, int cnt)
+{
+	struct i2c_msg msg[2];
+	int ret;
+
+	msg[0].addr = client->addr;
+	msg[0].flags = 0;
+	msg[0].len = 1;
+	msg[0].buf = &addr;
+	msg[1].addr = client->addr;
+	msg[1].flags = I2C_M_RD;
+	msg[1].len = cnt;
+	msg[1].buf = p;
+
+	ret = i2c_transfer(client->adapter, msg, 2);
+	if (ret < 0)
+		dev_err(&client->dev, "Error %d reading from cec:0x%x\n", ret, addr);
+
+	dev_dbg(&client->dev, "rd 0x%02x: %*ph\n", addr, cnt, p);
+
+	return ret;
+}
+
+static u8 tda9950_read(struct i2c_client *client, u8 addr)
+{
+	int ret;
+	u8 val;
+
+	ret = tda9950_read_range(client, addr, &val, 1);
+	if (ret < 0)
+		val = 0;
+
+	return val;
+}
+
+static irqreturn_t tda9950_irq(int irq, void *data)
+{
+	struct tda9950_priv *priv = data;
+	unsigned int tx_status;
+	u8 csr, cconr, buf[19];
+	u8 arb_lost_cnt, nack_cnt, err_cnt;
+
+	if (!priv->open)
+		return IRQ_NONE;
+
+	csr = tda9950_read(priv->client, REG_CSR);
+	if (!(csr & CSR_INT))
+		return IRQ_NONE;
+
+	cconr = tda9950_read(priv->client, REG_CCONR) & CCONR_RETRY_MASK;
+
+	tda9950_read_range(priv->client, REG_CDR0, buf, sizeof(buf));
+
+	/*
+	 * This should never happen: the data sheet says that there will
+	 * always be a valid message if the interrupt line is asserted.
+	 */
+	if (buf[0] == 0) {
+		dev_warn(&priv->client->dev, "interrupt pending, but no message?\n");
+		return IRQ_NONE;
+	}
+
+	switch (buf[1]) {
+	case CDR1_CNF: /* transmit result */
+		arb_lost_cnt = nack_cnt = err_cnt = 0;
+		switch (buf[2]) {
+		case CDR2_CNF_SUCCESS:
+			tx_status = CEC_TX_STATUS_OK;
+			break;
+
+		case CDR2_CNF_ARB_ERROR:
+			tx_status = CEC_TX_STATUS_ARB_LOST;
+			arb_lost_cnt = cconr;
+			break;
+
+		case CDR2_CNF_NACK_ADDR:
+			tx_status = CEC_TX_STATUS_NACK;
+			nack_cnt = cconr;
+			break;
+
+		default: /* some other error, refer to TDA9950 docs */
+			dev_err(&priv->client->dev, "CNF reply error 0x%02x\n",
+				buf[2]);
+			tx_status = CEC_TX_STATUS_ERROR;
+			err_cnt = cconr;
+			break;
+		}
+		/* TDA9950 executes all retries for us */
+		tx_status |= CEC_TX_STATUS_MAX_RETRIES;
+		cec_transmit_done(priv->adap, tx_status, arb_lost_cnt,
+				  nack_cnt, 0, err_cnt);
+		break;
+
+	case CDR1_IND:
+		priv->rx_msg.len = buf[0] - 2;
+		if (priv->rx_msg.len > CEC_MAX_MSG_SIZE)
+			priv->rx_msg.len = CEC_MAX_MSG_SIZE;
+
+		memcpy(priv->rx_msg.msg, buf + 2, priv->rx_msg.len);
+		cec_received_msg(priv->adap, &priv->rx_msg);
+		break;
+
+	default: /* unknown */
+		dev_err(&priv->client->dev, "unknown service id 0x%02x\n",
+			buf[1]);
+		break;
+	}
+
+	return IRQ_HANDLED;
+}
+
+static int tda9950_cec_transmit(struct cec_adapter *adap, u8 attempts,
+				u32 signal_free_time, struct cec_msg *msg)
+{
+	struct tda9950_priv *priv = adap->priv;
+	u8 buf[CEC_MAX_MSG_SIZE + 2];
+
+	buf[0] = 2 + msg->len;
+	buf[1] = CDR1_REQ;
+	memcpy(buf + 2, msg->msg, msg->len);
+
+	if (attempts > 5)
+		attempts = 5;
+
+	tda9950_write(priv->client, REG_CCONR, attempts);
+
+	return tda9950_write_range(priv->client, REG_CDR0, buf, 2 + msg->len);
+}
+
+static int tda9950_cec_adap_log_addr(struct cec_adapter *adap, u8 addr)
+{
+	struct tda9950_priv *priv = adap->priv;
+	u16 addresses;
+	u8 buf[2];
+
+	if (addr == CEC_LOG_ADDR_INVALID)
+		addresses = priv->addresses = 0;
+	else
+		addresses = priv->addresses |= BIT(addr);
+
+	/* TDA9950 doesn't want address 15 set */
+	addresses &= 0x7fff;
+	buf[0] = addresses >> 8;
+	buf[1] = addresses;
+
+	return tda9950_write_range(priv->client, REG_ACKH, buf, 2);
+}
+
+/*
+ * When operating as part of the TDA998x, we need additional handling
+ * to initialise and shut down the TDA9950 part of the device.  These
+ * two hooks are provided to allow the TDA998x code to perform those
+ * activities.
+ */
+static int tda9950_glue_open(struct tda9950_priv *priv)
+{
+	int ret = 0;
+
+	if (priv->glue && priv->glue->open)
+		ret = priv->glue->open(priv->glue->data);
+
+	priv->open = true;
+
+	return ret;
+}
+
+static void tda9950_glue_release(struct tda9950_priv *priv)
+{
+	priv->open = false;
+
+	if (priv->glue && priv->glue->release)
+		priv->glue->release(priv->glue->data);
+}
+
+static int tda9950_open(struct tda9950_priv *priv)
+{
+	struct i2c_client *client = priv->client;
+	int ret;
+
+	ret = tda9950_glue_open(priv);
+	if (ret)
+		return ret;
+
+	/* Reset the TDA9950, and wait 250ms for it to recover */
+	tda9950_write(client, REG_CCR, CCR_RESET);
+	msleep(250);
+
+	tda9950_cec_adap_log_addr(priv->adap, CEC_LOG_ADDR_INVALID);
+
+	/* Start the command processor */
+	tda9950_write(client, REG_CCR, CCR_ON);
+
+	return 0;
+}
+
+static void tda9950_release(struct tda9950_priv *priv)
+{
+	struct i2c_client *client = priv->client;
+	int timeout = 50;
+	u8 csr;
+
+	/* Stop the command processor */
+	tda9950_write(client, REG_CCR, 0);
+
+	/* Wait up to .5s for it to signal non-busy */
+	do {
+		csr = tda9950_read(client, REG_CSR);
+		if (!(csr & CSR_BUSY) || --timeout)
+			break;
+		msleep(10);
+	} while (1);
+
+	/* Warn the user that their IRQ may die if it's shared. */
+	if (csr & CSR_BUSY)
+		dev_warn(&client->dev, "command processor failed to stop, irq%d may die (csr=0x%02x)\n",
+			 client->irq, csr);
+
+	tda9950_glue_release(priv);
+}
+
+static int tda9950_cec_adap_enable(struct cec_adapter *adap, bool enable)
+{
+	struct tda9950_priv *priv = adap->priv;
+
+	if (!enable) {
+		tda9950_release(priv);
+		return 0;
+	} else {
+		return tda9950_open(priv);
+	}
+}
+
+static const struct cec_adap_ops tda9950_cec_ops = {
+	.adap_enable = tda9950_cec_adap_enable,
+	.adap_log_addr = tda9950_cec_adap_log_addr,
+	.adap_transmit = tda9950_cec_transmit,
+};
+
+/*
+ * When operating as part of the TDA998x, we need to claim additional
+ * resources.  These two hooks permit the management of those resources.
+ */
+static void tda9950_devm_glue_exit(void *data)
+{
+	struct tda9950_glue *glue = data;
+
+	if (glue && glue->exit)
+		glue->exit(glue->data);
+}
+
+static int tda9950_devm_glue_init(struct device *dev, struct tda9950_glue *glue)
+{
+	int ret;
+
+	if (glue && glue->init) {
+		ret = glue->init(glue->data);
+		if (ret)
+			return ret;
+	}
+
+	ret = devm_add_action(dev, tda9950_devm_glue_exit, glue);
+	if (ret)
+		tda9950_devm_glue_exit(glue);
+
+	return ret;
+}
+
+static void tda9950_cec_del(void *data)
+{
+	struct tda9950_priv *priv = data;
+
+	cec_delete_adapter(priv->adap);
+}
+
+static int tda9950_probe(struct i2c_client *client,
+			 const struct i2c_device_id *id)
+{
+	struct tda9950_glue *glue = client->dev.platform_data;
+	struct device *dev = &client->dev;
+	struct tda9950_priv *priv;
+	unsigned long irqflags;
+	int ret;
+	u8 cvr;
+
+	/*
+	 * We must have I2C functionality: our multi-byte accesses
+	 * must be performed as a single contiguous transaction.
+	 */
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
+		dev_err(&client->dev,
+			"adapter does not support I2C functionality\n");
+		return -ENXIO;
+	}
+
+	/* We must have an interrupt to be functional. */
+	if (client->irq <= 0) {
+		dev_err(&client->dev, "driver requires an interrupt\n");
+		return -ENXIO;
+	}
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->client = client;
+	priv->glue = glue;
+
+	i2c_set_clientdata(client, priv);
+
+	/*
+	 * If we're part of a TDA998x, we want the class devices to be
+	 * associated with the HDMI Tx so we have a tight relationship
+	 * between the HDMI interface and the CEC interface.
+	 */
+	priv->hdmi = dev;
+	if (glue && glue->parent)
+		priv->hdmi = glue->parent;
+
+	priv->adap = cec_allocate_adapter(&tda9950_cec_ops, priv, "tda9950",
+					  CEC_CAP_DEFAULTS,
+					  CEC_MAX_LOG_ADDRS);
+	if (IS_ERR(priv->adap))
+		return PTR_ERR(priv->adap);
+
+	ret = devm_add_action(dev, tda9950_cec_del, priv);
+	if (ret) {
+		cec_delete_adapter(priv->adap);
+		return ret;
+	}
+
+	ret = tda9950_devm_glue_init(dev, glue);
+	if (ret)
+		return ret;
+
+	ret = tda9950_glue_open(priv);
+	if (ret)
+		return ret;
+
+	cvr = tda9950_read(client, REG_CVR);
+
+	dev_info(&client->dev,
+		 "TDA9950 CEC interface, hardware version %u.%u\n",
+		 cvr >> 4, cvr & 15);
+
+	tda9950_glue_release(priv);
+
+	irqflags = IRQF_TRIGGER_FALLING;
+	if (glue)
+		irqflags = glue->irq_flags;
+
+	ret = devm_request_threaded_irq(dev, client->irq, NULL, tda9950_irq,
+					irqflags | IRQF_SHARED | IRQF_ONESHOT,
+					dev_name(&client->dev), priv);
+	if (ret < 0)
+		return ret;
+
+	priv->notify = cec_notifier_get(priv->hdmi);
+	if (!priv->notify)
+		return -ENOMEM;
+
+	ret = cec_register_adapter(priv->adap, priv->hdmi);
+	if (ret < 0) {
+		cec_notifier_put(priv->notify);
+		return ret;
+	}
+
+	/*
+	 * CEC documentation says we must not call cec_delete_adapter
+	 * after a successful call to cec_register_adapter().
+	 */
+	devm_remove_action(dev, tda9950_cec_del, priv);
+
+	cec_register_cec_notifier(priv->adap, priv->notify);
+
+	return 0;
+}
+
+static int tda9950_remove(struct i2c_client *client)
+{
+	struct tda9950_priv *priv = i2c_get_clientdata(client);
+
+	cec_unregister_adapter(priv->adap);
+	cec_notifier_put(priv->notify);
+
+	return 0;
+}
+
+static struct i2c_device_id tda9950_ids[] = {
+	{ "tda9950", 0 },
+	{ },
+};
+MODULE_DEVICE_TABLE(i2c, tda9950_ids);
+
+static struct i2c_driver tda9950_driver = {
+	.probe = tda9950_probe,
+	.remove = tda9950_remove,
+	.driver = {
+		.name = "tda9950",
+	},
+	.id_table = tda9950_ids,
+};
+
+module_i2c_driver(tda9950_driver);
+
+MODULE_AUTHOR("Russell King <rmk+kernel@armlinux.org.uk>");
+MODULE_DESCRIPTION("TDA9950/TDA998x Consumer Electronics Control Driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/platform_data/tda9950.h b/include/linux/platform_data/tda9950.h
new file mode 100644
index 000000000000..c65efd461102
--- /dev/null
+++ b/include/linux/platform_data/tda9950.h
@@ -0,0 +1,16 @@
+#ifndef LINUX_PLATFORM_DATA_TDA9950_H
+#define LINUX_PLATFORM_DATA_TDA9950_H
+
+struct device;
+
+struct tda9950_glue {
+	struct device *parent;
+	unsigned long irq_flags;
+	void *data;
+	int (*init)(void *);
+	void (*exit)(void *);
+	int (*open)(void *);
+	void (*release)(void *);
+};
+
+#endif
-- 
2.7.4

Re: [PATCH v3 5/7] drm/i2c: tda9950: add CEC driver

[Russell King - ARM Linux]

Hi Hans,

Any comments?

Thanks.

On Mon, Apr 09, 2018 at 01:16:32PM +0100, Russell King wrote:
> Add a CEC driver for the TDA9950, which is a stand-alone I2C CEC device,
> but is also integrated into HDMI transceivers such as the TDA9989 and
> TDA19989.
> 
> The TDA9950 contains a command processor which handles retransmissions
> and the low level bus protocol.  The driver just has to read and write
> the messages, and handle error conditions.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
>  drivers/gpu/drm/i2c/Kconfig           |   5 +
>  drivers/gpu/drm/i2c/Makefile          |   1 +
>  drivers/gpu/drm/i2c/tda9950.c         | 509 ++++++++++++++++++++++++++++++++++
>  include/linux/platform_data/tda9950.h |  16 ++
>  4 files changed, 531 insertions(+)
>  create mode 100644 drivers/gpu/drm/i2c/tda9950.c
>  create mode 100644 include/linux/platform_data/tda9950.h
> 
> diff --git a/drivers/gpu/drm/i2c/Kconfig b/drivers/gpu/drm/i2c/Kconfig
> index a6c92beb410a..3a232f5ff0a1 100644
> --- a/drivers/gpu/drm/i2c/Kconfig
> +++ b/drivers/gpu/drm/i2c/Kconfig
> @@ -26,4 +26,9 @@ config DRM_I2C_NXP_TDA998X
>  	help
>  	  Support for NXP Semiconductors TDA998X HDMI encoders.
>  
> +config DRM_I2C_NXP_TDA9950
> +	tristate "NXP Semiconductors TDA9950/TDA998X HDMI CEC"
> +	select CEC_NOTIFIER
> +	select CEC_CORE
> +
>  endmenu
> diff --git a/drivers/gpu/drm/i2c/Makefile b/drivers/gpu/drm/i2c/Makefile
> index b20100c18ffb..a962f6f08568 100644
> --- a/drivers/gpu/drm/i2c/Makefile
> +++ b/drivers/gpu/drm/i2c/Makefile
> @@ -7,3 +7,4 @@ obj-$(CONFIG_DRM_I2C_SIL164) += sil164.o
>  
>  tda998x-y := tda998x_drv.o
>  obj-$(CONFIG_DRM_I2C_NXP_TDA998X) += tda998x.o
> +obj-$(CONFIG_DRM_I2C_NXP_TDA9950) += tda9950.o
> diff --git a/drivers/gpu/drm/i2c/tda9950.c b/drivers/gpu/drm/i2c/tda9950.c
> new file mode 100644
> index 000000000000..3f7396caad48
> --- /dev/null
> +++ b/drivers/gpu/drm/i2c/tda9950.c
> @@ -0,0 +1,509 @@
> +/*
> + *  TDA9950 Consumer Electronics Control driver
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * The NXP TDA9950 implements the HDMI Consumer Electronics Control
> + * interface.  The host interface is similar to a mailbox: the data
> + * registers starting at REG_CDR0 are written to send a command to the
> + * internal CPU, and replies are read from these registers.
> + *
> + * As the data registers represent a mailbox, they must be accessed
> + * as a single I2C transaction.  See the TDA9950 data sheet for details.
> + */
> +#include <linux/delay.h>
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/platform_data/tda9950.h>
> +#include <linux/slab.h>
> +#include <drm/drm_edid.h>
> +#include <media/cec.h>
> +#include <media/cec-notifier.h>
> +
> +enum {
> +	REG_CSR = 0x00,
> +	CSR_BUSY = BIT(7),
> +	CSR_INT  = BIT(6),
> +	CSR_ERR  = BIT(5),
> +
> +	REG_CER = 0x01,
> +
> +	REG_CVR = 0x02,
> +
> +	REG_CCR = 0x03,
> +	CCR_RESET = BIT(7),
> +	CCR_ON    = BIT(6),
> +
> +	REG_ACKH = 0x04,
> +	REG_ACKL = 0x05,
> +
> +	REG_CCONR = 0x06,
> +	CCONR_ENABLE_ERROR = BIT(4),
> +	CCONR_RETRY_MASK = 7,
> +
> +	REG_CDR0 = 0x07,
> +
> +	CDR1_REQ = 0x00,
> +	CDR1_CNF = 0x01,
> +	CDR1_IND = 0x81,
> +	CDR1_ERR = 0x82,
> +	CDR1_IER = 0x83,
> +
> +	CDR2_CNF_SUCCESS    = 0x00,
> +	CDR2_CNF_OFF_STATE  = 0x80,
> +	CDR2_CNF_BAD_REQ    = 0x81,
> +	CDR2_CNF_CEC_ACCESS = 0x82,
> +	CDR2_CNF_ARB_ERROR  = 0x83,
> +	CDR2_CNF_BAD_TIMING = 0x84,
> +	CDR2_CNF_NACK_ADDR  = 0x85,
> +	CDR2_CNF_NACK_DATA  = 0x86,
> +};
> +
> +struct tda9950_priv {
> +	struct i2c_client *client;
> +	struct device *hdmi;
> +	struct cec_adapter *adap;
> +	struct tda9950_glue *glue;
> +	u16 addresses;
> +	struct cec_msg rx_msg;
> +	struct cec_notifier *notify;
> +	bool open;
> +};
> +
> +static int tda9950_write_range(struct i2c_client *client, u8 addr, u8 *p, int cnt)
> +{
> +	struct i2c_msg msg;
> +	u8 buf[cnt + 1];
> +	int ret;
> +
> +	buf[0] = addr;
> +	memcpy(buf + 1, p, cnt);
> +
> +	msg.addr = client->addr;
> +	msg.flags = 0;
> +	msg.len = cnt + 1;
> +	msg.buf = buf;
> +
> +	dev_dbg(&client->dev, "wr 0x%02x: %*ph\n", addr, cnt, p);
> +
> +	ret = i2c_transfer(client->adapter, &msg, 1);
> +	if (ret < 0)
> +		dev_err(&client->dev, "Error %d writing to cec:0x%x\n", ret, addr);
> +	return ret < 0 ? ret : 0;
> +}
> +
> +static void tda9950_write(struct i2c_client *client, u8 addr, u8 val)
> +{
> +	tda9950_write_range(client, addr, &val, 1);
> +}
> +
> +static int tda9950_read_range(struct i2c_client *client, u8 addr, u8 *p, int cnt)
> +{
> +	struct i2c_msg msg[2];
> +	int ret;
> +
> +	msg[0].addr = client->addr;
> +	msg[0].flags = 0;
> +	msg[0].len = 1;
> +	msg[0].buf = &addr;
> +	msg[1].addr = client->addr;
> +	msg[1].flags = I2C_M_RD;
> +	msg[1].len = cnt;
> +	msg[1].buf = p;
> +
> +	ret = i2c_transfer(client->adapter, msg, 2);
> +	if (ret < 0)
> +		dev_err(&client->dev, "Error %d reading from cec:0x%x\n", ret, addr);
> +
> +	dev_dbg(&client->dev, "rd 0x%02x: %*ph\n", addr, cnt, p);
> +
> +	return ret;
> +}
> +
> +static u8 tda9950_read(struct i2c_client *client, u8 addr)
> +{
> +	int ret;
> +	u8 val;
> +
> +	ret = tda9950_read_range(client, addr, &val, 1);
> +	if (ret < 0)
> +		val = 0;
> +
> +	return val;
> +}
> +
> +static irqreturn_t tda9950_irq(int irq, void *data)
> +{
> +	struct tda9950_priv *priv = data;
> +	unsigned int tx_status;
> +	u8 csr, cconr, buf[19];
> +	u8 arb_lost_cnt, nack_cnt, err_cnt;
> +
> +	if (!priv->open)
> +		return IRQ_NONE;
> +
> +	csr = tda9950_read(priv->client, REG_CSR);
> +	if (!(csr & CSR_INT))
> +		return IRQ_NONE;
> +
> +	cconr = tda9950_read(priv->client, REG_CCONR) & CCONR_RETRY_MASK;
> +
> +	tda9950_read_range(priv->client, REG_CDR0, buf, sizeof(buf));
> +
> +	/*
> +	 * This should never happen: the data sheet says that there will
> +	 * always be a valid message if the interrupt line is asserted.
> +	 */
> +	if (buf[0] == 0) {
> +		dev_warn(&priv->client->dev, "interrupt pending, but no message?\n");
> +		return IRQ_NONE;
> +	}
> +
> +	switch (buf[1]) {
> +	case CDR1_CNF: /* transmit result */
> +		arb_lost_cnt = nack_cnt = err_cnt = 0;
> +		switch (buf[2]) {
> +		case CDR2_CNF_SUCCESS:
> +			tx_status = CEC_TX_STATUS_OK;
> +			break;
> +
> +		case CDR2_CNF_ARB_ERROR:
> +			tx_status = CEC_TX_STATUS_ARB_LOST;
> +			arb_lost_cnt = cconr;
> +			break;
> +
> +		case CDR2_CNF_NACK_ADDR:
> +			tx_status = CEC_TX_STATUS_NACK;
> +			nack_cnt = cconr;
> +			break;
> +
> +		default: /* some other error, refer to TDA9950 docs */
> +			dev_err(&priv->client->dev, "CNF reply error 0x%02x\n",
> +				buf[2]);
> +			tx_status = CEC_TX_STATUS_ERROR;
> +			err_cnt = cconr;
> +			break;
> +		}
> +		/* TDA9950 executes all retries for us */
> +		tx_status |= CEC_TX_STATUS_MAX_RETRIES;
> +		cec_transmit_done(priv->adap, tx_status, arb_lost_cnt,
> +				  nack_cnt, 0, err_cnt);
> +		break;
> +
> +	case CDR1_IND:
> +		priv->rx_msg.len = buf[0] - 2;
> +		if (priv->rx_msg.len > CEC_MAX_MSG_SIZE)
> +			priv->rx_msg.len = CEC_MAX_MSG_SIZE;
> +
> +		memcpy(priv->rx_msg.msg, buf + 2, priv->rx_msg.len);
> +		cec_received_msg(priv->adap, &priv->rx_msg);
> +		break;
> +
> +	default: /* unknown */
> +		dev_err(&priv->client->dev, "unknown service id 0x%02x\n",
> +			buf[1]);
> +		break;
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int tda9950_cec_transmit(struct cec_adapter *adap, u8 attempts,
> +				u32 signal_free_time, struct cec_msg *msg)
> +{
> +	struct tda9950_priv *priv = adap->priv;
> +	u8 buf[CEC_MAX_MSG_SIZE + 2];
> +
> +	buf[0] = 2 + msg->len;
> +	buf[1] = CDR1_REQ;
> +	memcpy(buf + 2, msg->msg, msg->len);
> +
> +	if (attempts > 5)
> +		attempts = 5;
> +
> +	tda9950_write(priv->client, REG_CCONR, attempts);
> +
> +	return tda9950_write_range(priv->client, REG_CDR0, buf, 2 + msg->len);
> +}
> +
> +static int tda9950_cec_adap_log_addr(struct cec_adapter *adap, u8 addr)
> +{
> +	struct tda9950_priv *priv = adap->priv;
> +	u16 addresses;
> +	u8 buf[2];
> +
> +	if (addr == CEC_LOG_ADDR_INVALID)
> +		addresses = priv->addresses = 0;
> +	else
> +		addresses = priv->addresses |= BIT(addr);
> +
> +	/* TDA9950 doesn't want address 15 set */
> +	addresses &= 0x7fff;
> +	buf[0] = addresses >> 8;
> +	buf[1] = addresses;
> +
> +	return tda9950_write_range(priv->client, REG_ACKH, buf, 2);
> +}
> +
> +/*
> + * When operating as part of the TDA998x, we need additional handling
> + * to initialise and shut down the TDA9950 part of the device.  These
> + * two hooks are provided to allow the TDA998x code to perform those
> + * activities.
> + */
> +static int tda9950_glue_open(struct tda9950_priv *priv)
> +{
> +	int ret = 0;
> +
> +	if (priv->glue && priv->glue->open)
> +		ret = priv->glue->open(priv->glue->data);
> +
> +	priv->open = true;
> +
> +	return ret;
> +}
> +
> +static void tda9950_glue_release(struct tda9950_priv *priv)
> +{
> +	priv->open = false;
> +
> +	if (priv->glue && priv->glue->release)
> +		priv->glue->release(priv->glue->data);
> +}
> +
> +static int tda9950_open(struct tda9950_priv *priv)
> +{
> +	struct i2c_client *client = priv->client;
> +	int ret;
> +
> +	ret = tda9950_glue_open(priv);
> +	if (ret)
> +		return ret;
> +
> +	/* Reset the TDA9950, and wait 250ms for it to recover */
> +	tda9950_write(client, REG_CCR, CCR_RESET);
> +	msleep(250);
> +
> +	tda9950_cec_adap_log_addr(priv->adap, CEC_LOG_ADDR_INVALID);
> +
> +	/* Start the command processor */
> +	tda9950_write(client, REG_CCR, CCR_ON);
> +
> +	return 0;
> +}
> +
> +static void tda9950_release(struct tda9950_priv *priv)
> +{
> +	struct i2c_client *client = priv->client;
> +	int timeout = 50;
> +	u8 csr;
> +
> +	/* Stop the command processor */
> +	tda9950_write(client, REG_CCR, 0);
> +
> +	/* Wait up to .5s for it to signal non-busy */
> +	do {
> +		csr = tda9950_read(client, REG_CSR);
> +		if (!(csr & CSR_BUSY) || --timeout)
> +			break;
> +		msleep(10);
> +	} while (1);
> +
> +	/* Warn the user that their IRQ may die if it's shared. */
> +	if (csr & CSR_BUSY)
> +		dev_warn(&client->dev, "command processor failed to stop, irq%d may die (csr=0x%02x)\n",
> +			 client->irq, csr);
> +
> +	tda9950_glue_release(priv);
> +}
> +
> +static int tda9950_cec_adap_enable(struct cec_adapter *adap, bool enable)
> +{
> +	struct tda9950_priv *priv = adap->priv;
> +
> +	if (!enable) {
> +		tda9950_release(priv);
> +		return 0;
> +	} else {
> +		return tda9950_open(priv);
> +	}
> +}
> +
> +static const struct cec_adap_ops tda9950_cec_ops = {
> +	.adap_enable = tda9950_cec_adap_enable,
> +	.adap_log_addr = tda9950_cec_adap_log_addr,
> +	.adap_transmit = tda9950_cec_transmit,
> +};
> +
> +/*
> + * When operating as part of the TDA998x, we need to claim additional
> + * resources.  These two hooks permit the management of those resources.
> + */
> +static void tda9950_devm_glue_exit(void *data)
> +{
> +	struct tda9950_glue *glue = data;
> +
> +	if (glue && glue->exit)
> +		glue->exit(glue->data);
> +}
> +
> +static int tda9950_devm_glue_init(struct device *dev, struct tda9950_glue *glue)
> +{
> +	int ret;
> +
> +	if (glue && glue->init) {
> +		ret = glue->init(glue->data);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	ret = devm_add_action(dev, tda9950_devm_glue_exit, glue);
> +	if (ret)
> +		tda9950_devm_glue_exit(glue);
> +
> +	return ret;
> +}
> +
> +static void tda9950_cec_del(void *data)
> +{
> +	struct tda9950_priv *priv = data;
> +
> +	cec_delete_adapter(priv->adap);
> +}
> +
> +static int tda9950_probe(struct i2c_client *client,
> +			 const struct i2c_device_id *id)
> +{
> +	struct tda9950_glue *glue = client->dev.platform_data;
> +	struct device *dev = &client->dev;
> +	struct tda9950_priv *priv;
> +	unsigned long irqflags;
> +	int ret;
> +	u8 cvr;
> +
> +	/*
> +	 * We must have I2C functionality: our multi-byte accesses
> +	 * must be performed as a single contiguous transaction.
> +	 */
> +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
> +		dev_err(&client->dev,
> +			"adapter does not support I2C functionality\n");
> +		return -ENXIO;
> +	}
> +
> +	/* We must have an interrupt to be functional. */
> +	if (client->irq <= 0) {
> +		dev_err(&client->dev, "driver requires an interrupt\n");
> +		return -ENXIO;
> +	}
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->client = client;
> +	priv->glue = glue;
> +
> +	i2c_set_clientdata(client, priv);
> +
> +	/*
> +	 * If we're part of a TDA998x, we want the class devices to be
> +	 * associated with the HDMI Tx so we have a tight relationship
> +	 * between the HDMI interface and the CEC interface.
> +	 */
> +	priv->hdmi = dev;
> +	if (glue && glue->parent)
> +		priv->hdmi = glue->parent;
> +
> +	priv->adap = cec_allocate_adapter(&tda9950_cec_ops, priv, "tda9950",
> +					  CEC_CAP_DEFAULTS,
> +					  CEC_MAX_LOG_ADDRS);
> +	if (IS_ERR(priv->adap))
> +		return PTR_ERR(priv->adap);
> +
> +	ret = devm_add_action(dev, tda9950_cec_del, priv);
> +	if (ret) {
> +		cec_delete_adapter(priv->adap);
> +		return ret;
> +	}
> +
> +	ret = tda9950_devm_glue_init(dev, glue);
> +	if (ret)
> +		return ret;
> +
> +	ret = tda9950_glue_open(priv);
> +	if (ret)
> +		return ret;
> +
> +	cvr = tda9950_read(client, REG_CVR);
> +
> +	dev_info(&client->dev,
> +		 "TDA9950 CEC interface, hardware version %u.%u\n",
> +		 cvr >> 4, cvr & 15);
> +
> +	tda9950_glue_release(priv);
> +
> +	irqflags = IRQF_TRIGGER_FALLING;
> +	if (glue)
> +		irqflags = glue->irq_flags;
> +
> +	ret = devm_request_threaded_irq(dev, client->irq, NULL, tda9950_irq,
> +					irqflags | IRQF_SHARED | IRQF_ONESHOT,
> +					dev_name(&client->dev), priv);
> +	if (ret < 0)
> +		return ret;
> +
> +	priv->notify = cec_notifier_get(priv->hdmi);
> +	if (!priv->notify)
> +		return -ENOMEM;
> +
> +	ret = cec_register_adapter(priv->adap, priv->hdmi);
> +	if (ret < 0) {
> +		cec_notifier_put(priv->notify);
> +		return ret;
> +	}
> +
> +	/*
> +	 * CEC documentation says we must not call cec_delete_adapter
> +	 * after a successful call to cec_register_adapter().
> +	 */
> +	devm_remove_action(dev, tda9950_cec_del, priv);
> +
> +	cec_register_cec_notifier(priv->adap, priv->notify);
> +
> +	return 0;
> +}
> +
> +static int tda9950_remove(struct i2c_client *client)
> +{
> +	struct tda9950_priv *priv = i2c_get_clientdata(client);
> +
> +	cec_unregister_adapter(priv->adap);
> +	cec_notifier_put(priv->notify);
> +
> +	return 0;
> +}
> +
> +static struct i2c_device_id tda9950_ids[] = {
> +	{ "tda9950", 0 },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(i2c, tda9950_ids);
> +
> +static struct i2c_driver tda9950_driver = {
> +	.probe = tda9950_probe,
> +	.remove = tda9950_remove,
> +	.driver = {
> +		.name = "tda9950",
> +	},
> +	.id_table = tda9950_ids,
> +};
> +
> +module_i2c_driver(tda9950_driver);
> +
> +MODULE_AUTHOR("Russell King <rmk+kernel@armlinux.org.uk>");
> +MODULE_DESCRIPTION("TDA9950/TDA998x Consumer Electronics Control Driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/platform_data/tda9950.h b/include/linux/platform_data/tda9950.h
> new file mode 100644
> index 000000000000..c65efd461102
> --- /dev/null
> +++ b/include/linux/platform_data/tda9950.h
> @@ -0,0 +1,16 @@
> +#ifndef LINUX_PLATFORM_DATA_TDA9950_H
> +#define LINUX_PLATFORM_DATA_TDA9950_H
> +
> +struct device;
> +
> +struct tda9950_glue {
> +	struct device *parent;
> +	unsigned long irq_flags;
> +	void *data;
> +	int (*init)(void *);
> +	void (*exit)(void *);
> +	int (*open)(void *);
> +	void (*release)(void *);
> +};
> +
> +#endif
> -- 
> 2.7.4
> 

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

Re: [PATCH v3 5/7] drm/i2c: tda9950: add CEC driver

[Hans Verkuil]

On 04/20/2018 05:31 PM, Russell King - ARM Linux wrote:
> Hi Hans,
> 
> Any comments?

I have been traveling and haven't had time to look at this. Next week will
be busy as well, but I expect to be able to look at it the week after that.

I remember from the previous series that I couldn't test it with my BeagleBone
Black board (the calibration gpio had to switch from in to out but it wasn't allowed
since it had an associated irq). That's still a problem?

I didn't see any changes in that area when I did a quick scan.

Regards,

	Hans

> 
> Thanks.
> 
> On Mon, Apr 09, 2018 at 01:16:32PM +0100, Russell King wrote:
>> Add a CEC driver for the TDA9950, which is a stand-alone I2C CEC device,
>> but is also integrated into HDMI transceivers such as the TDA9989 and
>> TDA19989.
>>
>> The TDA9950 contains a command processor which handles retransmissions
>> and the low level bus protocol.  The driver just has to read and write
>> the messages, and handle error conditions.
>>
>> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
>> ---
>>  drivers/gpu/drm/i2c/Kconfig           |   5 +
>>  drivers/gpu/drm/i2c/Makefile          |   1 +
>>  drivers/gpu/drm/i2c/tda9950.c         | 509 ++++++++++++++++++++++++++++++++++
>>  include/linux/platform_data/tda9950.h |  16 ++
>>  4 files changed, 531 insertions(+)
>>  create mode 100644 drivers/gpu/drm/i2c/tda9950.c
>>  create mode 100644 include/linux/platform_data/tda9950.h
>>
>> diff --git a/drivers/gpu/drm/i2c/Kconfig b/drivers/gpu/drm/i2c/Kconfig
>> index a6c92beb410a..3a232f5ff0a1 100644
>> --- a/drivers/gpu/drm/i2c/Kconfig
>> +++ b/drivers/gpu/drm/i2c/Kconfig
>> @@ -26,4 +26,9 @@ config DRM_I2C_NXP_TDA998X
>>  	help
>>  	  Support for NXP Semiconductors TDA998X HDMI encoders.
>>  
>> +config DRM_I2C_NXP_TDA9950
>> +	tristate "NXP Semiconductors TDA9950/TDA998X HDMI CEC"
>> +	select CEC_NOTIFIER
>> +	select CEC_CORE
>> +
>>  endmenu
>> diff --git a/drivers/gpu/drm/i2c/Makefile b/drivers/gpu/drm/i2c/Makefile
>> index b20100c18ffb..a962f6f08568 100644
>> --- a/drivers/gpu/drm/i2c/Makefile
>> +++ b/drivers/gpu/drm/i2c/Makefile
>> @@ -7,3 +7,4 @@ obj-$(CONFIG_DRM_I2C_SIL164) += sil164.o
>>  
>>  tda998x-y := tda998x_drv.o
>>  obj-$(CONFIG_DRM_I2C_NXP_TDA998X) += tda998x.o
>> +obj-$(CONFIG_DRM_I2C_NXP_TDA9950) += tda9950.o
>> diff --git a/drivers/gpu/drm/i2c/tda9950.c b/drivers/gpu/drm/i2c/tda9950.c
>> new file mode 100644
>> index 000000000000..3f7396caad48
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i2c/tda9950.c
>> @@ -0,0 +1,509 @@
>> +/*
>> + *  TDA9950 Consumer Electronics Control driver
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + * The NXP TDA9950 implements the HDMI Consumer Electronics Control
>> + * interface.  The host interface is similar to a mailbox: the data
>> + * registers starting at REG_CDR0 are written to send a command to the
>> + * internal CPU, and replies are read from these registers.
>> + *
>> + * As the data registers represent a mailbox, they must be accessed
>> + * as a single I2C transaction.  See the TDA9950 data sheet for details.
>> + */
>> +#include <linux/delay.h>
>> +#include <linux/i2c.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/module.h>
>> +#include <linux/platform_data/tda9950.h>
>> +#include <linux/slab.h>
>> +#include <drm/drm_edid.h>
>> +#include <media/cec.h>
>> +#include <media/cec-notifier.h>
>> +
>> +enum {
>> +	REG_CSR = 0x00,
>> +	CSR_BUSY = BIT(7),
>> +	CSR_INT  = BIT(6),
>> +	CSR_ERR  = BIT(5),
>> +
>> +	REG_CER = 0x01,
>> +
>> +	REG_CVR = 0x02,
>> +
>> +	REG_CCR = 0x03,
>> +	CCR_RESET = BIT(7),
>> +	CCR_ON    = BIT(6),
>> +
>> +	REG_ACKH = 0x04,
>> +	REG_ACKL = 0x05,
>> +
>> +	REG_CCONR = 0x06,
>> +	CCONR_ENABLE_ERROR = BIT(4),
>> +	CCONR_RETRY_MASK = 7,
>> +
>> +	REG_CDR0 = 0x07,
>> +
>> +	CDR1_REQ = 0x00,
>> +	CDR1_CNF = 0x01,
>> +	CDR1_IND = 0x81,
>> +	CDR1_ERR = 0x82,
>> +	CDR1_IER = 0x83,
>> +
>> +	CDR2_CNF_SUCCESS    = 0x00,
>> +	CDR2_CNF_OFF_STATE  = 0x80,
>> +	CDR2_CNF_BAD_REQ    = 0x81,
>> +	CDR2_CNF_CEC_ACCESS = 0x82,
>> +	CDR2_CNF_ARB_ERROR  = 0x83,
>> +	CDR2_CNF_BAD_TIMING = 0x84,
>> +	CDR2_CNF_NACK_ADDR  = 0x85,
>> +	CDR2_CNF_NACK_DATA  = 0x86,
>> +};
>> +
>> +struct tda9950_priv {
>> +	struct i2c_client *client;
>> +	struct device *hdmi;
>> +	struct cec_adapter *adap;
>> +	struct tda9950_glue *glue;
>> +	u16 addresses;
>> +	struct cec_msg rx_msg;
>> +	struct cec_notifier *notify;
>> +	bool open;
>> +};
>> +
>> +static int tda9950_write_range(struct i2c_client *client, u8 addr, u8 *p, int cnt)
>> +{
>> +	struct i2c_msg msg;
>> +	u8 buf[cnt + 1];
>> +	int ret;
>> +
>> +	buf[0] = addr;
>> +	memcpy(buf + 1, p, cnt);
>> +
>> +	msg.addr = client->addr;
>> +	msg.flags = 0;
>> +	msg.len = cnt + 1;
>> +	msg.buf = buf;
>> +
>> +	dev_dbg(&client->dev, "wr 0x%02x: %*ph\n", addr, cnt, p);
>> +
>> +	ret = i2c_transfer(client->adapter, &msg, 1);
>> +	if (ret < 0)
>> +		dev_err(&client->dev, "Error %d writing to cec:0x%x\n", ret, addr);
>> +	return ret < 0 ? ret : 0;
>> +}
>> +
>> +static void tda9950_write(struct i2c_client *client, u8 addr, u8 val)
>> +{
>> +	tda9950_write_range(client, addr, &val, 1);
>> +}
>> +
>> +static int tda9950_read_range(struct i2c_client *client, u8 addr, u8 *p, int cnt)
>> +{
>> +	struct i2c_msg msg[2];
>> +	int ret;
>> +
>> +	msg[0].addr = client->addr;
>> +	msg[0].flags = 0;
>> +	msg[0].len = 1;
>> +	msg[0].buf = &addr;
>> +	msg[1].addr = client->addr;
>> +	msg[1].flags = I2C_M_RD;
>> +	msg[1].len = cnt;
>> +	msg[1].buf = p;
>> +
>> +	ret = i2c_transfer(client->adapter, msg, 2);
>> +	if (ret < 0)
>> +		dev_err(&client->dev, "Error %d reading from cec:0x%x\n", ret, addr);
>> +
>> +	dev_dbg(&client->dev, "rd 0x%02x: %*ph\n", addr, cnt, p);
>> +
>> +	return ret;
>> +}
>> +
>> +static u8 tda9950_read(struct i2c_client *client, u8 addr)
>> +{
>> +	int ret;
>> +	u8 val;
>> +
>> +	ret = tda9950_read_range(client, addr, &val, 1);
>> +	if (ret < 0)
>> +		val = 0;
>> +
>> +	return val;
>> +}
>> +
>> +static irqreturn_t tda9950_irq(int irq, void *data)
>> +{
>> +	struct tda9950_priv *priv = data;
>> +	unsigned int tx_status;
>> +	u8 csr, cconr, buf[19];
>> +	u8 arb_lost_cnt, nack_cnt, err_cnt;
>> +
>> +	if (!priv->open)
>> +		return IRQ_NONE;
>> +
>> +	csr = tda9950_read(priv->client, REG_CSR);
>> +	if (!(csr & CSR_INT))
>> +		return IRQ_NONE;
>> +
>> +	cconr = tda9950_read(priv->client, REG_CCONR) & CCONR_RETRY_MASK;
>> +
>> +	tda9950_read_range(priv->client, REG_CDR0, buf, sizeof(buf));
>> +
>> +	/*
>> +	 * This should never happen: the data sheet says that there will
>> +	 * always be a valid message if the interrupt line is asserted.
>> +	 */
>> +	if (buf[0] == 0) {
>> +		dev_warn(&priv->client->dev, "interrupt pending, but no message?\n");
>> +		return IRQ_NONE;
>> +	}
>> +
>> +	switch (buf[1]) {
>> +	case CDR1_CNF: /* transmit result */
>> +		arb_lost_cnt = nack_cnt = err_cnt = 0;
>> +		switch (buf[2]) {
>> +		case CDR2_CNF_SUCCESS:
>> +			tx_status = CEC_TX_STATUS_OK;
>> +			break;
>> +
>> +		case CDR2_CNF_ARB_ERROR:
>> +			tx_status = CEC_TX_STATUS_ARB_LOST;
>> +			arb_lost_cnt = cconr;
>> +			break;
>> +
>> +		case CDR2_CNF_NACK_ADDR:
>> +			tx_status = CEC_TX_STATUS_NACK;
>> +			nack_cnt = cconr;
>> +			break;
>> +
>> +		default: /* some other error, refer to TDA9950 docs */
>> +			dev_err(&priv->client->dev, "CNF reply error 0x%02x\n",
>> +				buf[2]);
>> +			tx_status = CEC_TX_STATUS_ERROR;
>> +			err_cnt = cconr;
>> +			break;
>> +		}
>> +		/* TDA9950 executes all retries for us */
>> +		tx_status |= CEC_TX_STATUS_MAX_RETRIES;
>> +		cec_transmit_done(priv->adap, tx_status, arb_lost_cnt,
>> +				  nack_cnt, 0, err_cnt);
>> +		break;
>> +
>> +	case CDR1_IND:
>> +		priv->rx_msg.len = buf[0] - 2;
>> +		if (priv->rx_msg.len > CEC_MAX_MSG_SIZE)
>> +			priv->rx_msg.len = CEC_MAX_MSG_SIZE;
>> +
>> +		memcpy(priv->rx_msg.msg, buf + 2, priv->rx_msg.len);
>> +		cec_received_msg(priv->adap, &priv->rx_msg);
>> +		break;
>> +
>> +	default: /* unknown */
>> +		dev_err(&priv->client->dev, "unknown service id 0x%02x\n",
>> +			buf[1]);
>> +		break;
>> +	}
>> +
>> +	return IRQ_HANDLED;
>> +}
>> +
>> +static int tda9950_cec_transmit(struct cec_adapter *adap, u8 attempts,
>> +				u32 signal_free_time, struct cec_msg *msg)
>> +{
>> +	struct tda9950_priv *priv = adap->priv;
>> +	u8 buf[CEC_MAX_MSG_SIZE + 2];
>> +
>> +	buf[0] = 2 + msg->len;
>> +	buf[1] = CDR1_REQ;
>> +	memcpy(buf + 2, msg->msg, msg->len);
>> +
>> +	if (attempts > 5)
>> +		attempts = 5;
>> +
>> +	tda9950_write(priv->client, REG_CCONR, attempts);
>> +
>> +	return tda9950_write_range(priv->client, REG_CDR0, buf, 2 + msg->len);
>> +}
>> +
>> +static int tda9950_cec_adap_log_addr(struct cec_adapter *adap, u8 addr)
>> +{
>> +	struct tda9950_priv *priv = adap->priv;
>> +	u16 addresses;
>> +	u8 buf[2];
>> +
>> +	if (addr == CEC_LOG_ADDR_INVALID)
>> +		addresses = priv->addresses = 0;
>> +	else
>> +		addresses = priv->addresses |= BIT(addr);
>> +
>> +	/* TDA9950 doesn't want address 15 set */
>> +	addresses &= 0x7fff;
>> +	buf[0] = addresses >> 8;
>> +	buf[1] = addresses;
>> +
>> +	return tda9950_write_range(priv->client, REG_ACKH, buf, 2);
>> +}
>> +
>> +/*
>> + * When operating as part of the TDA998x, we need additional handling
>> + * to initialise and shut down the TDA9950 part of the device.  These
>> + * two hooks are provided to allow the TDA998x code to perform those
>> + * activities.
>> + */
>> +static int tda9950_glue_open(struct tda9950_priv *priv)
>> +{
>> +	int ret = 0;
>> +
>> +	if (priv->glue && priv->glue->open)
>> +		ret = priv->glue->open(priv->glue->data);
>> +
>> +	priv->open = true;
>> +
>> +	return ret;
>> +}
>> +
>> +static void tda9950_glue_release(struct tda9950_priv *priv)
>> +{
>> +	priv->open = false;
>> +
>> +	if (priv->glue && priv->glue->release)
>> +		priv->glue->release(priv->glue->data);
>> +}
>> +
>> +static int tda9950_open(struct tda9950_priv *priv)
>> +{
>> +	struct i2c_client *client = priv->client;
>> +	int ret;
>> +
>> +	ret = tda9950_glue_open(priv);
>> +	if (ret)
>> +		return ret;
>> +
>> +	/* Reset the TDA9950, and wait 250ms for it to recover */
>> +	tda9950_write(client, REG_CCR, CCR_RESET);
>> +	msleep(250);
>> +
>> +	tda9950_cec_adap_log_addr(priv->adap, CEC_LOG_ADDR_INVALID);
>> +
>> +	/* Start the command processor */
>> +	tda9950_write(client, REG_CCR, CCR_ON);
>> +
>> +	return 0;
>> +}
>> +
>> +static void tda9950_release(struct tda9950_priv *priv)
>> +{
>> +	struct i2c_client *client = priv->client;
>> +	int timeout = 50;
>> +	u8 csr;
>> +
>> +	/* Stop the command processor */
>> +	tda9950_write(client, REG_CCR, 0);
>> +
>> +	/* Wait up to .5s for it to signal non-busy */
>> +	do {
>> +		csr = tda9950_read(client, REG_CSR);
>> +		if (!(csr & CSR_BUSY) || --timeout)
>> +			break;
>> +		msleep(10);
>> +	} while (1);
>> +
>> +	/* Warn the user that their IRQ may die if it's shared. */
>> +	if (csr & CSR_BUSY)
>> +		dev_warn(&client->dev, "command processor failed to stop, irq%d may die (csr=0x%02x)\n",
>> +			 client->irq, csr);
>> +
>> +	tda9950_glue_release(priv);
>> +}
>> +
>> +static int tda9950_cec_adap_enable(struct cec_adapter *adap, bool enable)
>> +{
>> +	struct tda9950_priv *priv = adap->priv;
>> +
>> +	if (!enable) {
>> +		tda9950_release(priv);
>> +		return 0;
>> +	} else {
>> +		return tda9950_open(priv);
>> +	}
>> +}
>> +
>> +static const struct cec_adap_ops tda9950_cec_ops = {
>> +	.adap_enable = tda9950_cec_adap_enable,
>> +	.adap_log_addr = tda9950_cec_adap_log_addr,
>> +	.adap_transmit = tda9950_cec_transmit,
>> +};
>> +
>> +/*
>> + * When operating as part of the TDA998x, we need to claim additional
>> + * resources.  These two hooks permit the management of those resources.
>> + */
>> +static void tda9950_devm_glue_exit(void *data)
>> +{
>> +	struct tda9950_glue *glue = data;
>> +
>> +	if (glue && glue->exit)
>> +		glue->exit(glue->data);
>> +}
>> +
>> +static int tda9950_devm_glue_init(struct device *dev, struct tda9950_glue *glue)
>> +{
>> +	int ret;
>> +
>> +	if (glue && glue->init) {
>> +		ret = glue->init(glue->data);
>> +		if (ret)
>> +			return ret;
>> +	}
>> +
>> +	ret = devm_add_action(dev, tda9950_devm_glue_exit, glue);
>> +	if (ret)
>> +		tda9950_devm_glue_exit(glue);
>> +
>> +	return ret;
>> +}
>> +
>> +static void tda9950_cec_del(void *data)
>> +{
>> +	struct tda9950_priv *priv = data;
>> +
>> +	cec_delete_adapter(priv->adap);
>> +}
>> +
>> +static int tda9950_probe(struct i2c_client *client,
>> +			 const struct i2c_device_id *id)
>> +{
>> +	struct tda9950_glue *glue = client->dev.platform_data;
>> +	struct device *dev = &client->dev;
>> +	struct tda9950_priv *priv;
>> +	unsigned long irqflags;
>> +	int ret;
>> +	u8 cvr;
>> +
>> +	/*
>> +	 * We must have I2C functionality: our multi-byte accesses
>> +	 * must be performed as a single contiguous transaction.
>> +	 */
>> +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
>> +		dev_err(&client->dev,
>> +			"adapter does not support I2C functionality\n");
>> +		return -ENXIO;
>> +	}
>> +
>> +	/* We must have an interrupt to be functional. */
>> +	if (client->irq <= 0) {
>> +		dev_err(&client->dev, "driver requires an interrupt\n");
>> +		return -ENXIO;
>> +	}
>> +
>> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>> +	if (!priv)
>> +		return -ENOMEM;
>> +
>> +	priv->client = client;
>> +	priv->glue = glue;
>> +
>> +	i2c_set_clientdata(client, priv);
>> +
>> +	/*
>> +	 * If we're part of a TDA998x, we want the class devices to be
>> +	 * associated with the HDMI Tx so we have a tight relationship
>> +	 * between the HDMI interface and the CEC interface.
>> +	 */
>> +	priv->hdmi = dev;
>> +	if (glue && glue->parent)
>> +		priv->hdmi = glue->parent;
>> +
>> +	priv->adap = cec_allocate_adapter(&tda9950_cec_ops, priv, "tda9950",
>> +					  CEC_CAP_DEFAULTS,
>> +					  CEC_MAX_LOG_ADDRS);
>> +	if (IS_ERR(priv->adap))
>> +		return PTR_ERR(priv->adap);
>> +
>> +	ret = devm_add_action(dev, tda9950_cec_del, priv);
>> +	if (ret) {
>> +		cec_delete_adapter(priv->adap);
>> +		return ret;
>> +	}
>> +
>> +	ret = tda9950_devm_glue_init(dev, glue);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = tda9950_glue_open(priv);
>> +	if (ret)
>> +		return ret;
>> +
>> +	cvr = tda9950_read(client, REG_CVR);
>> +
>> +	dev_info(&client->dev,
>> +		 "TDA9950 CEC interface, hardware version %u.%u\n",
>> +		 cvr >> 4, cvr & 15);
>> +
>> +	tda9950_glue_release(priv);
>> +
>> +	irqflags = IRQF_TRIGGER_FALLING;
>> +	if (glue)
>> +		irqflags = glue->irq_flags;
>> +
>> +	ret = devm_request_threaded_irq(dev, client->irq, NULL, tda9950_irq,
>> +					irqflags | IRQF_SHARED | IRQF_ONESHOT,
>> +					dev_name(&client->dev), priv);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	priv->notify = cec_notifier_get(priv->hdmi);
>> +	if (!priv->notify)
>> +		return -ENOMEM;
>> +
>> +	ret = cec_register_adapter(priv->adap, priv->hdmi);
>> +	if (ret < 0) {
>> +		cec_notifier_put(priv->notify);
>> +		return ret;
>> +	}
>> +
>> +	/*
>> +	 * CEC documentation says we must not call cec_delete_adapter
>> +	 * after a successful call to cec_register_adapter().
>> +	 */
>> +	devm_remove_action(dev, tda9950_cec_del, priv);
>> +
>> +	cec_register_cec_notifier(priv->adap, priv->notify);
>> +
>> +	return 0;
>> +}
>> +
>> +static int tda9950_remove(struct i2c_client *client)
>> +{
>> +	struct tda9950_priv *priv = i2c_get_clientdata(client);
>> +
>> +	cec_unregister_adapter(priv->adap);
>> +	cec_notifier_put(priv->notify);
>> +
>> +	return 0;
>> +}
>> +
>> +static struct i2c_device_id tda9950_ids[] = {
>> +	{ "tda9950", 0 },
>> +	{ },
>> +};
>> +MODULE_DEVICE_TABLE(i2c, tda9950_ids);
>> +
>> +static struct i2c_driver tda9950_driver = {
>> +	.probe = tda9950_probe,
>> +	.remove = tda9950_remove,
>> +	.driver = {
>> +		.name = "tda9950",
>> +	},
>> +	.id_table = tda9950_ids,
>> +};
>> +
>> +module_i2c_driver(tda9950_driver);
>> +
>> +MODULE_AUTHOR("Russell King <rmk+kernel@armlinux.org.uk>");
>> +MODULE_DESCRIPTION("TDA9950/TDA998x Consumer Electronics Control Driver");
>> +MODULE_LICENSE("GPL v2");
>> diff --git a/include/linux/platform_data/tda9950.h b/include/linux/platform_data/tda9950.h
>> new file mode 100644
>> index 000000000000..c65efd461102
>> --- /dev/null
>> +++ b/include/linux/platform_data/tda9950.h
>> @@ -0,0 +1,16 @@
>> +#ifndef LINUX_PLATFORM_DATA_TDA9950_H
>> +#define LINUX_PLATFORM_DATA_TDA9950_H
>> +
>> +struct device;
>> +
>> +struct tda9950_glue {
>> +	struct device *parent;
>> +	unsigned long irq_flags;
>> +	void *data;
>> +	int (*init)(void *);
>> +	void (*exit)(void *);
>> +	int (*open)(void *);
>> +	void (*release)(void *);
>> +};
>> +
>> +#endif
>> -- 
>> 2.7.4
>>
> 

Re: [PATCH v3 5/7] drm/i2c: tda9950: add CEC driver

[Russell King - ARM Linux]

On Fri, Apr 20, 2018 at 05:48:12PM +0200, Hans Verkuil wrote:
> On 04/20/2018 05:31 PM, Russell King - ARM Linux wrote:
> > Hi Hans,
> > 
> > Any comments?
> 
> I have been traveling and haven't had time to look at this. Next week will
> be busy as well, but I expect to be able to look at it the week after that.

Well, that doesn't work because I won't be reading mail that week,
and I'll probably simply ignore the excessive backlog when I do
start reading mail again.

> I remember from the previous series that I couldn't test it with my BeagleBone
> Black board (the calibration gpio had to switch from in to out but it wasn't allowed
> since it had an associated irq). That's still a problem?
> 
> I didn't see any changes in that area when I did a quick scan.

Correct, and unless you wish me to do the work for you (in which case
you can pay me) nothing is going to change on that front!  Seriously,
please do not expect me to add support for platforms I don't have
access to.  I'm just a volunteer for this, probably the same as you.

I don't think we ended up with an answer for that problem.  I don't
see that dropping the requested interrupt, using the GPIO, and then
re-requesting the interrupt is an option - how do we handle a failure
to re-request the interrupt?  Do we just ignore the error, or let DRM
stop working properly?

In any case, I don't have a working HDMI CEC-compliant setup anymore,
(no TV, just a HDMI monitor now) so I would rather _not_ change the
driver from its known-to-be-working state.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

Re: [PATCH v3 5/7] drm/i2c: tda9950: add CEC driver

[Hans Verkuil]

On 04/09/18 14:16, Russell King wrote:
> Add a CEC driver for the TDA9950, which is a stand-alone I2C CEC device,
> but is also integrated into HDMI transceivers such as the TDA9989 and
> TDA19989.
> 
> The TDA9950 contains a command processor which handles retransmissions
> and the low level bus protocol.  The driver just has to read and write
> the messages, and handle error conditions.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

Reviewed-by: Hans Verkuil <hans.verkuil@cisco.com>

Regards,

	Hans

> ---
>  drivers/gpu/drm/i2c/Kconfig           |   5 +
>  drivers/gpu/drm/i2c/Makefile          |   1 +
>  drivers/gpu/drm/i2c/tda9950.c         | 509 ++++++++++++++++++++++++++++++++++
>  include/linux/platform_data/tda9950.h |  16 ++
>  4 files changed, 531 insertions(+)
>  create mode 100644 drivers/gpu/drm/i2c/tda9950.c
>  create mode 100644 include/linux/platform_data/tda9950.h
> 
> diff --git a/drivers/gpu/drm/i2c/Kconfig b/drivers/gpu/drm/i2c/Kconfig
> index a6c92beb410a..3a232f5ff0a1 100644
> --- a/drivers/gpu/drm/i2c/Kconfig
> +++ b/drivers/gpu/drm/i2c/Kconfig
> @@ -26,4 +26,9 @@ config DRM_I2C_NXP_TDA998X
>  	help
>  	  Support for NXP Semiconductors TDA998X HDMI encoders.
>  
> +config DRM_I2C_NXP_TDA9950
> +	tristate "NXP Semiconductors TDA9950/TDA998X HDMI CEC"
> +	select CEC_NOTIFIER
> +	select CEC_CORE
> +
>  endmenu
> diff --git a/drivers/gpu/drm/i2c/Makefile b/drivers/gpu/drm/i2c/Makefile
> index b20100c18ffb..a962f6f08568 100644
> --- a/drivers/gpu/drm/i2c/Makefile
> +++ b/drivers/gpu/drm/i2c/Makefile
> @@ -7,3 +7,4 @@ obj-$(CONFIG_DRM_I2C_SIL164) += sil164.o
>  
>  tda998x-y := tda998x_drv.o
>  obj-$(CONFIG_DRM_I2C_NXP_TDA998X) += tda998x.o
> +obj-$(CONFIG_DRM_I2C_NXP_TDA9950) += tda9950.o
> diff --git a/drivers/gpu/drm/i2c/tda9950.c b/drivers/gpu/drm/i2c/tda9950.c
> new file mode 100644
> index 000000000000..3f7396caad48
> --- /dev/null
> +++ b/drivers/gpu/drm/i2c/tda9950.c
> @@ -0,0 +1,509 @@
> +/*
> + *  TDA9950 Consumer Electronics Control driver
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * The NXP TDA9950 implements the HDMI Consumer Electronics Control
> + * interface.  The host interface is similar to a mailbox: the data
> + * registers starting at REG_CDR0 are written to send a command to the
> + * internal CPU, and replies are read from these registers.
> + *
> + * As the data registers represent a mailbox, they must be accessed
> + * as a single I2C transaction.  See the TDA9950 data sheet for details.
> + */
> +#include <linux/delay.h>
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/platform_data/tda9950.h>
> +#include <linux/slab.h>
> +#include <drm/drm_edid.h>
> +#include <media/cec.h>
> +#include <media/cec-notifier.h>
> +
> +enum {
> +	REG_CSR = 0x00,
> +	CSR_BUSY = BIT(7),
> +	CSR_INT  = BIT(6),
> +	CSR_ERR  = BIT(5),
> +
> +	REG_CER = 0x01,
> +
> +	REG_CVR = 0x02,
> +
> +	REG_CCR = 0x03,
> +	CCR_RESET = BIT(7),
> +	CCR_ON    = BIT(6),
> +
> +	REG_ACKH = 0x04,
> +	REG_ACKL = 0x05,
> +
> +	REG_CCONR = 0x06,
> +	CCONR_ENABLE_ERROR = BIT(4),
> +	CCONR_RETRY_MASK = 7,
> +
> +	REG_CDR0 = 0x07,
> +
> +	CDR1_REQ = 0x00,
> +	CDR1_CNF = 0x01,
> +	CDR1_IND = 0x81,
> +	CDR1_ERR = 0x82,
> +	CDR1_IER = 0x83,
> +
> +	CDR2_CNF_SUCCESS    = 0x00,
> +	CDR2_CNF_OFF_STATE  = 0x80,
> +	CDR2_CNF_BAD_REQ    = 0x81,
> +	CDR2_CNF_CEC_ACCESS = 0x82,
> +	CDR2_CNF_ARB_ERROR  = 0x83,
> +	CDR2_CNF_BAD_TIMING = 0x84,
> +	CDR2_CNF_NACK_ADDR  = 0x85,
> +	CDR2_CNF_NACK_DATA  = 0x86,
> +};
> +
> +struct tda9950_priv {
> +	struct i2c_client *client;
> +	struct device *hdmi;
> +	struct cec_adapter *adap;
> +	struct tda9950_glue *glue;
> +	u16 addresses;
> +	struct cec_msg rx_msg;
> +	struct cec_notifier *notify;
> +	bool open;
> +};
> +
> +static int tda9950_write_range(struct i2c_client *client, u8 addr, u8 *p, int cnt)
> +{
> +	struct i2c_msg msg;
> +	u8 buf[cnt + 1];
> +	int ret;
> +
> +	buf[0] = addr;
> +	memcpy(buf + 1, p, cnt);
> +
> +	msg.addr = client->addr;
> +	msg.flags = 0;
> +	msg.len = cnt + 1;
> +	msg.buf = buf;
> +
> +	dev_dbg(&client->dev, "wr 0x%02x: %*ph\n", addr, cnt, p);
> +
> +	ret = i2c_transfer(client->adapter, &msg, 1);
> +	if (ret < 0)
> +		dev_err(&client->dev, "Error %d writing to cec:0x%x\n", ret, addr);
> +	return ret < 0 ? ret : 0;
> +}
> +
> +static void tda9950_write(struct i2c_client *client, u8 addr, u8 val)
> +{
> +	tda9950_write_range(client, addr, &val, 1);
> +}
> +
> +static int tda9950_read_range(struct i2c_client *client, u8 addr, u8 *p, int cnt)
> +{
> +	struct i2c_msg msg[2];
> +	int ret;
> +
> +	msg[0].addr = client->addr;
> +	msg[0].flags = 0;
> +	msg[0].len = 1;
> +	msg[0].buf = &addr;
> +	msg[1].addr = client->addr;
> +	msg[1].flags = I2C_M_RD;
> +	msg[1].len = cnt;
> +	msg[1].buf = p;
> +
> +	ret = i2c_transfer(client->adapter, msg, 2);
> +	if (ret < 0)
> +		dev_err(&client->dev, "Error %d reading from cec:0x%x\n", ret, addr);
> +
> +	dev_dbg(&client->dev, "rd 0x%02x: %*ph\n", addr, cnt, p);
> +
> +	return ret;
> +}
> +
> +static u8 tda9950_read(struct i2c_client *client, u8 addr)
> +{
> +	int ret;
> +	u8 val;
> +
> +	ret = tda9950_read_range(client, addr, &val, 1);
> +	if (ret < 0)
> +		val = 0;
> +
> +	return val;
> +}
> +
> +static irqreturn_t tda9950_irq(int irq, void *data)
> +{
> +	struct tda9950_priv *priv = data;
> +	unsigned int tx_status;
> +	u8 csr, cconr, buf[19];
> +	u8 arb_lost_cnt, nack_cnt, err_cnt;
> +
> +	if (!priv->open)
> +		return IRQ_NONE;
> +
> +	csr = tda9950_read(priv->client, REG_CSR);
> +	if (!(csr & CSR_INT))
> +		return IRQ_NONE;
> +
> +	cconr = tda9950_read(priv->client, REG_CCONR) & CCONR_RETRY_MASK;
> +
> +	tda9950_read_range(priv->client, REG_CDR0, buf, sizeof(buf));
> +
> +	/*
> +	 * This should never happen: the data sheet says that there will
> +	 * always be a valid message if the interrupt line is asserted.
> +	 */
> +	if (buf[0] == 0) {
> +		dev_warn(&priv->client->dev, "interrupt pending, but no message?\n");
> +		return IRQ_NONE;
> +	}
> +
> +	switch (buf[1]) {
> +	case CDR1_CNF: /* transmit result */
> +		arb_lost_cnt = nack_cnt = err_cnt = 0;
> +		switch (buf[2]) {
> +		case CDR2_CNF_SUCCESS:
> +			tx_status = CEC_TX_STATUS_OK;
> +			break;
> +
> +		case CDR2_CNF_ARB_ERROR:
> +			tx_status = CEC_TX_STATUS_ARB_LOST;
> +			arb_lost_cnt = cconr;
> +			break;
> +
> +		case CDR2_CNF_NACK_ADDR:
> +			tx_status = CEC_TX_STATUS_NACK;
> +			nack_cnt = cconr;
> +			break;
> +
> +		default: /* some other error, refer to TDA9950 docs */
> +			dev_err(&priv->client->dev, "CNF reply error 0x%02x\n",
> +				buf[2]);
> +			tx_status = CEC_TX_STATUS_ERROR;
> +			err_cnt = cconr;
> +			break;
> +		}
> +		/* TDA9950 executes all retries for us */
> +		tx_status |= CEC_TX_STATUS_MAX_RETRIES;
> +		cec_transmit_done(priv->adap, tx_status, arb_lost_cnt,
> +				  nack_cnt, 0, err_cnt);
> +		break;
> +
> +	case CDR1_IND:
> +		priv->rx_msg.len = buf[0] - 2;
> +		if (priv->rx_msg.len > CEC_MAX_MSG_SIZE)
> +			priv->rx_msg.len = CEC_MAX_MSG_SIZE;
> +
> +		memcpy(priv->rx_msg.msg, buf + 2, priv->rx_msg.len);
> +		cec_received_msg(priv->adap, &priv->rx_msg);
> +		break;
> +
> +	default: /* unknown */
> +		dev_err(&priv->client->dev, "unknown service id 0x%02x\n",
> +			buf[1]);
> +		break;
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int tda9950_cec_transmit(struct cec_adapter *adap, u8 attempts,
> +				u32 signal_free_time, struct cec_msg *msg)
> +{
> +	struct tda9950_priv *priv = adap->priv;
> +	u8 buf[CEC_MAX_MSG_SIZE + 2];
> +
> +	buf[0] = 2 + msg->len;
> +	buf[1] = CDR1_REQ;
> +	memcpy(buf + 2, msg->msg, msg->len);
> +
> +	if (attempts > 5)
> +		attempts = 5;
> +
> +	tda9950_write(priv->client, REG_CCONR, attempts);
> +
> +	return tda9950_write_range(priv->client, REG_CDR0, buf, 2 + msg->len);
> +}
> +
> +static int tda9950_cec_adap_log_addr(struct cec_adapter *adap, u8 addr)
> +{
> +	struct tda9950_priv *priv = adap->priv;
> +	u16 addresses;
> +	u8 buf[2];
> +
> +	if (addr == CEC_LOG_ADDR_INVALID)
> +		addresses = priv->addresses = 0;
> +	else
> +		addresses = priv->addresses |= BIT(addr);
> +
> +	/* TDA9950 doesn't want address 15 set */
> +	addresses &= 0x7fff;
> +	buf[0] = addresses >> 8;
> +	buf[1] = addresses;
> +
> +	return tda9950_write_range(priv->client, REG_ACKH, buf, 2);
> +}
> +
> +/*
> + * When operating as part of the TDA998x, we need additional handling
> + * to initialise and shut down the TDA9950 part of the device.  These
> + * two hooks are provided to allow the TDA998x code to perform those
> + * activities.
> + */
> +static int tda9950_glue_open(struct tda9950_priv *priv)
> +{
> +	int ret = 0;
> +
> +	if (priv->glue && priv->glue->open)
> +		ret = priv->glue->open(priv->glue->data);
> +
> +	priv->open = true;
> +
> +	return ret;
> +}
> +
> +static void tda9950_glue_release(struct tda9950_priv *priv)
> +{
> +	priv->open = false;
> +
> +	if (priv->glue && priv->glue->release)
> +		priv->glue->release(priv->glue->data);
> +}
> +
> +static int tda9950_open(struct tda9950_priv *priv)
> +{
> +	struct i2c_client *client = priv->client;
> +	int ret;
> +
> +	ret = tda9950_glue_open(priv);
> +	if (ret)
> +		return ret;
> +
> +	/* Reset the TDA9950, and wait 250ms for it to recover */
> +	tda9950_write(client, REG_CCR, CCR_RESET);
> +	msleep(250);
> +
> +	tda9950_cec_adap_log_addr(priv->adap, CEC_LOG_ADDR_INVALID);
> +
> +	/* Start the command processor */
> +	tda9950_write(client, REG_CCR, CCR_ON);
> +
> +	return 0;
> +}
> +
> +static void tda9950_release(struct tda9950_priv *priv)
> +{
> +	struct i2c_client *client = priv->client;
> +	int timeout = 50;
> +	u8 csr;
> +
> +	/* Stop the command processor */
> +	tda9950_write(client, REG_CCR, 0);
> +
> +	/* Wait up to .5s for it to signal non-busy */
> +	do {
> +		csr = tda9950_read(client, REG_CSR);
> +		if (!(csr & CSR_BUSY) || --timeout)
> +			break;
> +		msleep(10);
> +	} while (1);
> +
> +	/* Warn the user that their IRQ may die if it's shared. */
> +	if (csr & CSR_BUSY)
> +		dev_warn(&client->dev, "command processor failed to stop, irq%d may die (csr=0x%02x)\n",
> +			 client->irq, csr);
> +
> +	tda9950_glue_release(priv);
> +}
> +
> +static int tda9950_cec_adap_enable(struct cec_adapter *adap, bool enable)
> +{
> +	struct tda9950_priv *priv = adap->priv;
> +
> +	if (!enable) {
> +		tda9950_release(priv);
> +		return 0;
> +	} else {
> +		return tda9950_open(priv);
> +	}
> +}
> +
> +static const struct cec_adap_ops tda9950_cec_ops = {
> +	.adap_enable = tda9950_cec_adap_enable,
> +	.adap_log_addr = tda9950_cec_adap_log_addr,
> +	.adap_transmit = tda9950_cec_transmit,
> +};
> +
> +/*
> + * When operating as part of the TDA998x, we need to claim additional
> + * resources.  These two hooks permit the management of those resources.
> + */
> +static void tda9950_devm_glue_exit(void *data)
> +{
> +	struct tda9950_glue *glue = data;
> +
> +	if (glue && glue->exit)
> +		glue->exit(glue->data);
> +}
> +
> +static int tda9950_devm_glue_init(struct device *dev, struct tda9950_glue *glue)
> +{
> +	int ret;
> +
> +	if (glue && glue->init) {
> +		ret = glue->init(glue->data);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	ret = devm_add_action(dev, tda9950_devm_glue_exit, glue);
> +	if (ret)
> +		tda9950_devm_glue_exit(glue);
> +
> +	return ret;
> +}
> +
> +static void tda9950_cec_del(void *data)
> +{
> +	struct tda9950_priv *priv = data;
> +
> +	cec_delete_adapter(priv->adap);
> +}
> +
> +static int tda9950_probe(struct i2c_client *client,
> +			 const struct i2c_device_id *id)
> +{
> +	struct tda9950_glue *glue = client->dev.platform_data;
> +	struct device *dev = &client->dev;
> +	struct tda9950_priv *priv;
> +	unsigned long irqflags;
> +	int ret;
> +	u8 cvr;
> +
> +	/*
> +	 * We must have I2C functionality: our multi-byte accesses
> +	 * must be performed as a single contiguous transaction.
> +	 */
> +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
> +		dev_err(&client->dev,
> +			"adapter does not support I2C functionality\n");
> +		return -ENXIO;
> +	}
> +
> +	/* We must have an interrupt to be functional. */
> +	if (client->irq <= 0) {
> +		dev_err(&client->dev, "driver requires an interrupt\n");
> +		return -ENXIO;
> +	}
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->client = client;
> +	priv->glue = glue;
> +
> +	i2c_set_clientdata(client, priv);
> +
> +	/*
> +	 * If we're part of a TDA998x, we want the class devices to be
> +	 * associated with the HDMI Tx so we have a tight relationship
> +	 * between the HDMI interface and the CEC interface.
> +	 */
> +	priv->hdmi = dev;
> +	if (glue && glue->parent)
> +		priv->hdmi = glue->parent;
> +
> +	priv->adap = cec_allocate_adapter(&tda9950_cec_ops, priv, "tda9950",
> +					  CEC_CAP_DEFAULTS,
> +					  CEC_MAX_LOG_ADDRS);
> +	if (IS_ERR(priv->adap))
> +		return PTR_ERR(priv->adap);
> +
> +	ret = devm_add_action(dev, tda9950_cec_del, priv);
> +	if (ret) {
> +		cec_delete_adapter(priv->adap);
> +		return ret;
> +	}
> +
> +	ret = tda9950_devm_glue_init(dev, glue);
> +	if (ret)
> +		return ret;
> +
> +	ret = tda9950_glue_open(priv);
> +	if (ret)
> +		return ret;
> +
> +	cvr = tda9950_read(client, REG_CVR);
> +
> +	dev_info(&client->dev,
> +		 "TDA9950 CEC interface, hardware version %u.%u\n",
> +		 cvr >> 4, cvr & 15);
> +
> +	tda9950_glue_release(priv);
> +
> +	irqflags = IRQF_TRIGGER_FALLING;
> +	if (glue)
> +		irqflags = glue->irq_flags;
> +
> +	ret = devm_request_threaded_irq(dev, client->irq, NULL, tda9950_irq,
> +					irqflags | IRQF_SHARED | IRQF_ONESHOT,
> +					dev_name(&client->dev), priv);
> +	if (ret < 0)
> +		return ret;
> +
> +	priv->notify = cec_notifier_get(priv->hdmi);
> +	if (!priv->notify)
> +		return -ENOMEM;
> +
> +	ret = cec_register_adapter(priv->adap, priv->hdmi);
> +	if (ret < 0) {
> +		cec_notifier_put(priv->notify);
> +		return ret;
> +	}
> +
> +	/*
> +	 * CEC documentation says we must not call cec_delete_adapter
> +	 * after a successful call to cec_register_adapter().
> +	 */
> +	devm_remove_action(dev, tda9950_cec_del, priv);
> +
> +	cec_register_cec_notifier(priv->adap, priv->notify);
> +
> +	return 0;
> +}
> +
> +static int tda9950_remove(struct i2c_client *client)
> +{
> +	struct tda9950_priv *priv = i2c_get_clientdata(client);
> +
> +	cec_unregister_adapter(priv->adap);
> +	cec_notifier_put(priv->notify);
> +
> +	return 0;
> +}
> +
> +static struct i2c_device_id tda9950_ids[] = {
> +	{ "tda9950", 0 },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(i2c, tda9950_ids);
> +
> +static struct i2c_driver tda9950_driver = {
> +	.probe = tda9950_probe,
> +	.remove = tda9950_remove,
> +	.driver = {
> +		.name = "tda9950",
> +	},
> +	.id_table = tda9950_ids,
> +};
> +
> +module_i2c_driver(tda9950_driver);
> +
> +MODULE_AUTHOR("Russell King <rmk+kernel@armlinux.org.uk>");
> +MODULE_DESCRIPTION("TDA9950/TDA998x Consumer Electronics Control Driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/platform_data/tda9950.h b/include/linux/platform_data/tda9950.h
> new file mode 100644
> index 000000000000..c65efd461102
> --- /dev/null
> +++ b/include/linux/platform_data/tda9950.h
> @@ -0,0 +1,16 @@
> +#ifndef LINUX_PLATFORM_DATA_TDA9950_H
> +#define LINUX_PLATFORM_DATA_TDA9950_H
> +
> +struct device;
> +
> +struct tda9950_glue {
> +	struct device *parent;
> +	unsigned long irq_flags;
> +	void *data;
> +	int (*init)(void *);
> +	void (*exit)(void *);
> +	int (*open)(void *);
> +	void (*release)(void *);
> +};
> +
> +#endif
> 

[PATCH v3 6/7] drm/i2c: tda998x: add CEC support

[Russell King]

The TDA998x is a HDMI transmitter with a TDA9950 CEC engine integrated
onto the same die.  Add support for the TDA9950 CEC engine to the
TDA998x driver.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/gpu/drm/i2c/Kconfig       |   1 +
 drivers/gpu/drm/i2c/tda998x_drv.c | 195 ++++++++++++++++++++++++++++++++++++--
 2 files changed, 187 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i2c/Kconfig b/drivers/gpu/drm/i2c/Kconfig
index 3a232f5ff0a1..65d3acb61c03 100644
--- a/drivers/gpu/drm/i2c/Kconfig
+++ b/drivers/gpu/drm/i2c/Kconfig
@@ -22,6 +22,7 @@ config DRM_I2C_SIL164
 config DRM_I2C_NXP_TDA998X
 	tristate "NXP Semiconductors TDA998X HDMI encoder"
 	default m if DRM_TILCDC
+	select CEC_CORE if CEC_NOTIFIER
 	select SND_SOC_HDMI_CODEC if SND_SOC
 	help
 	  Support for NXP Semiconductors TDA998X HDMI encoders.
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 16e0439cad44..eb9916bd84a4 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -16,8 +16,10 @@
  */
 
 #include <linux/component.h>
+#include <linux/gpio/consumer.h>
 #include <linux/hdmi.h>
 #include <linux/module.h>
+#include <linux/platform_data/tda9950.h>
 #include <linux/irq.h>
 #include <sound/asoundef.h>
 #include <sound/hdmi-codec.h>
@@ -29,6 +31,8 @@
 #include <drm/drm_of.h>
 #include <drm/i2c/tda998x.h>
 
+#include <media/cec-notifier.h>
+
 #define DBG(fmt, ...) DRM_DEBUG(fmt"\n", ##__VA_ARGS__)
 
 struct tda998x_audio_port {
@@ -55,6 +59,7 @@ struct tda998x_priv {
 	struct platform_device *audio_pdev;
 	struct mutex audio_mutex;
 
+	struct mutex edid_mutex;
 	wait_queue_head_t wq_edid;
 	volatile int wq_edid_wait;
 
@@ -67,6 +72,9 @@ struct tda998x_priv {
 	struct drm_connector connector;
 
 	struct tda998x_audio_port audio_port[2];
+	struct tda9950_glue cec_glue;
+	struct gpio_desc *calib;
+	struct cec_notifier *cec_notify;
 };
 
 #define conn_to_tda998x_priv(x) \
@@ -345,6 +353,12 @@ struct tda998x_priv {
 #define REG_CEC_INTSTATUS	  0xee		      /* read */
 # define CEC_INTSTATUS_CEC	  (1 << 0)
 # define CEC_INTSTATUS_HDMI	  (1 << 1)
+#define REG_CEC_CAL_XOSC_CTRL1    0xf2
+# define CEC_CAL_XOSC_CTRL1_ENA_CAL	BIT(0)
+#define REG_CEC_DES_FREQ2         0xf5
+# define CEC_DES_FREQ2_DIS_AUTOCAL BIT(7)
+#define REG_CEC_CLK               0xf6
+# define CEC_CLK_FRO              0x11
 #define REG_CEC_FRO_IM_CLK_CTRL   0xfb                /* read/write */
 # define CEC_FRO_IM_CLK_CTRL_GHOST_DIS (1 << 7)
 # define CEC_FRO_IM_CLK_CTRL_ENA_OTP   (1 << 6)
@@ -359,6 +373,7 @@ struct tda998x_priv {
 # define CEC_RXSHPDLEV_HPD        (1 << 1)
 
 #define REG_CEC_ENAMODS           0xff                /* read/write */
+# define CEC_ENAMODS_EN_CEC_CLK   (1 << 7)
 # define CEC_ENAMODS_DIS_FRO      (1 << 6)
 # define CEC_ENAMODS_DIS_CCLK     (1 << 5)
 # define CEC_ENAMODS_EN_RXSENS    (1 << 2)
@@ -417,6 +432,114 @@ cec_read(struct tda998x_priv *priv, u8 addr)
 	return val;
 }
 
+static void cec_enamods(struct tda998x_priv *priv, u8 mods, bool enable)
+{
+	int val = cec_read(priv, REG_CEC_ENAMODS);
+
+	if (val < 0)
+		return;
+
+	if (enable)
+		val |= mods;
+	else
+		val &= ~mods;
+
+	cec_write(priv, REG_CEC_ENAMODS, val);
+}
+
+static void tda998x_cec_set_calibration(struct tda998x_priv *priv, bool enable)
+{
+	if (enable) {
+		u8 val;
+
+		cec_write(priv, 0xf3, 0xc0);
+		cec_write(priv, 0xf4, 0xd4);
+
+		/* Enable automatic calibration mode */
+		val = cec_read(priv, REG_CEC_DES_FREQ2);
+		val &= ~CEC_DES_FREQ2_DIS_AUTOCAL;
+		cec_write(priv, REG_CEC_DES_FREQ2, val);
+
+		/* Enable free running oscillator */
+		cec_write(priv, REG_CEC_CLK, CEC_CLK_FRO);
+		cec_enamods(priv, CEC_ENAMODS_DIS_FRO, false);
+
+		cec_write(priv, REG_CEC_CAL_XOSC_CTRL1,
+			  CEC_CAL_XOSC_CTRL1_ENA_CAL);
+	} else {
+		cec_write(priv, REG_CEC_CAL_XOSC_CTRL1, 0);
+	}
+}
+
+/*
+ * Calibration for the internal oscillator: we need to set calibration mode,
+ * and then pulse the IRQ line low for a 10ms ± 1% period.
+ */
+static void tda998x_cec_calibration(struct tda998x_priv *priv)
+{
+	struct gpio_desc *calib = priv->calib;
+
+	mutex_lock(&priv->edid_mutex);
+	if (priv->hdmi->irq > 0)
+		disable_irq(priv->hdmi->irq);
+	gpiod_direction_output(calib, 1);
+	tda998x_cec_set_calibration(priv, true);
+
+	local_irq_disable();
+	gpiod_set_value(calib, 0);
+	mdelay(10);
+	gpiod_set_value(calib, 1);
+	local_irq_enable();
+
+	tda998x_cec_set_calibration(priv, false);
+	gpiod_direction_input(calib);
+	if (priv->hdmi->irq > 0)
+		enable_irq(priv->hdmi->irq);
+	mutex_unlock(&priv->edid_mutex);
+}
+
+static int tda998x_cec_hook_init(void *data)
+{
+	struct tda998x_priv *priv = data;
+	struct gpio_desc *calib;
+
+	calib = gpiod_get(&priv->hdmi->dev, "nxp,calib", GPIOD_ASIS);
+	if (IS_ERR(calib)) {
+		dev_warn(&priv->hdmi->dev, "failed to get calibration gpio: %ld\n",
+			 PTR_ERR(calib));
+		return PTR_ERR(calib);
+	}
+
+	priv->calib = calib;
+
+	return 0;
+}
+
+static void tda998x_cec_hook_exit(void *data)
+{
+	struct tda998x_priv *priv = data;
+
+	gpiod_put(priv->calib);
+	priv->calib = NULL;
+}
+
+static int tda998x_cec_hook_open(void *data)
+{
+	struct tda998x_priv *priv = data;
+
+	cec_enamods(priv, CEC_ENAMODS_EN_CEC_CLK | CEC_ENAMODS_EN_CEC, true);
+	tda998x_cec_calibration(priv);
+
+	return 0;
+}
+
+static void tda998x_cec_hook_release(void *data)
+{
+	struct tda998x_priv *priv = data;
+
+	cec_enamods(priv, CEC_ENAMODS_EN_CEC_CLK | CEC_ENAMODS_EN_CEC, false);
+}
+
 static int
 set_page(struct tda998x_priv *priv, u16 reg)
 {
@@ -657,10 +780,13 @@ static irqreturn_t tda998x_irq_thread(int irq, void *data)
 			sta, cec, lvl, flag0, flag1, flag2);
 
 		if (cec & CEC_RXSHPDINT_HPD) {
-			if (lvl & CEC_RXSHPDLEV_HPD)
+			if (lvl & CEC_RXSHPDLEV_HPD) {
 				tda998x_edid_delay_start(priv);
-			else
+			} else {
 				schedule_work(&priv->detect_work);
+				cec_notifier_set_phys_addr(priv->cec_notify,
+						   CEC_PHYS_ADDR_INVALID);
+			}
 
 			handled = true;
 		}
@@ -981,6 +1107,8 @@ static int tda998x_connector_fill_modes(struct drm_connector *connector,
 	if (connector->edid_blob_ptr) {
 		struct edid *edid = (void *)connector->edid_blob_ptr->data;
 
+		cec_notifier_set_phys_addr_from_edid(priv->cec_notify, edid);
+
 		priv->sink_has_audio = drm_detect_monitor_audio(edid);
 	} else {
 		priv->sink_has_audio = false;
@@ -1024,6 +1152,8 @@ static int read_edid_block(void *data, u8 *buf, unsigned int blk, size_t length)
 	offset = (blk & 1) ? 128 : 0;
 	segptr = blk / 2;
 
+	mutex_lock(&priv->edid_mutex);
+
 	reg_write(priv, REG_DDC_ADDR, 0xa0);
 	reg_write(priv, REG_DDC_OFFS, offset);
 	reg_write(priv, REG_DDC_SEGM_ADDR, 0x60);
@@ -1043,14 +1173,15 @@ static int read_edid_block(void *data, u8 *buf, unsigned int blk, size_t length)
 					msecs_to_jiffies(100));
 		if (i < 0) {
 			dev_err(&priv->hdmi->dev, "read edid wait err %d\n", i);
-			return i;
+			ret = i;
+			goto failed;
 		}
 	} else {
 		for (i = 100; i > 0; i--) {
 			msleep(1);
 			ret = reg_read(priv, REG_INT_FLAGS_2);
 			if (ret < 0)
-				return ret;
+				goto failed;
 			if (ret & INT_FLAGS_2_EDID_BLK_RD)
 				break;
 		}
@@ -1058,17 +1189,22 @@ static int read_edid_block(void *data, u8 *buf, unsigned int blk, size_t length)
 
 	if (i == 0) {
 		dev_err(&priv->hdmi->dev, "read edid timeout\n");
-		return -ETIMEDOUT;
+		ret = -ETIMEDOUT;
+		goto failed;
 	}
 
 	ret = reg_read_range(priv, REG_EDID_DATA_0, buf, length);
 	if (ret != length) {
 		dev_err(&priv->hdmi->dev, "failed to read edid block %d: %d\n",
 			blk, ret);
-		return ret;
+		goto failed;
 	}
 
-	return 0;
+	ret = 0;
+
+ failed:
+	mutex_unlock(&priv->edid_mutex);
+	return ret;
 }
 
 static int tda998x_connector_get_modes(struct drm_connector *connector)
@@ -1423,6 +1559,9 @@ static void tda998x_destroy(struct tda998x_priv *priv)
 	cancel_work_sync(&priv->detect_work);
 
 	i2c_unregister_device(priv->cec);
+
+	if (priv->cec_notify)
+		cec_notifier_put(priv->cec_notify);
 }
 
 /* I2C driver functions */
@@ -1472,11 +1611,13 @@ static int tda998x_get_audio_ports(struct tda998x_priv *priv,
 static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv)
 {
 	struct device_node *np = client->dev.of_node;
+	struct i2c_board_info cec_info;
 	u32 video;
 	int rev_lo, rev_hi, ret;
 
 	mutex_init(&priv->mutex);	/* protect the page access */
 	mutex_init(&priv->audio_mutex); /* protect access from audio thread */
+	mutex_init(&priv->edid_mutex);
 	init_waitqueue_head(&priv->edid_delay_waitq);
 	timer_setup(&priv->edid_delay_timer, tda998x_edid_delay_done, 0);
 	INIT_WORK(&priv->detect_work, tda998x_detect_work);
@@ -1564,6 +1705,9 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv)
 
 		irq_flags =
 			irqd_get_trigger_type(irq_get_irq_data(client->irq));
+
+		priv->cec_glue.irq_flags = irq_flags;
+
 		irq_flags |= IRQF_SHARED | IRQF_ONESHOT;
 		ret = request_threaded_irq(client->irq, NULL,
 					   tda998x_irq_thread, irq_flags,
@@ -1579,7 +1723,34 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv)
 		cec_write(priv, REG_CEC_RXSHPDINTENA, CEC_RXSHPDLEV_HPD);
 	}
 
-	priv->cec = i2c_new_dummy(client->adapter, priv->cec_addr);
+	priv->cec_notify = cec_notifier_get(&client->dev);
+	if (!priv->cec_notify) {
+		ret = -ENOMEM;
+		goto fail;
+	}
+
+	priv->cec_glue.parent = &client->dev;
+	priv->cec_glue.data = priv;
+	priv->cec_glue.init = tda998x_cec_hook_init;
+	priv->cec_glue.exit = tda998x_cec_hook_exit;
+	priv->cec_glue.open = tda998x_cec_hook_open;
+	priv->cec_glue.release = tda998x_cec_hook_release;
+
+	/*
+	 * Some TDA998x are actually two I2C devices merged onto one piece
+	 * of silicon: TDA9989 and TDA19989 combine the HDMI transmitter
+	 * with a slightly modified TDA9950 CEC device.  The CEC device
+	 * is at the TDA9950 address, with the address pins strapped across
+	 * to the TDA998x address pins.  Hence, it always has the same
+	 * offset.
+	 */
+	memset(&cec_info, 0, sizeof(cec_info));
+	strlcpy(cec_info.type, "tda9950", sizeof(cec_info.type));
+	cec_info.addr = priv->cec_addr;
+	cec_info.platform_data = &priv->cec_glue;
+	cec_info.irq = client->irq;
+
+	priv->cec = i2c_new_device(client->adapter, &cec_info);
 	if (!priv->cec) {
 		ret = -ENODEV;
 		goto fail;
@@ -1609,10 +1780,16 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv)
 	return 0;
 
 fail:
+	/* if encoder_init fails, the encoder slave is never registered,
+	 * so cleanup here:
+	 */
+	if (priv->cec)
+		i2c_unregister_device(priv->cec);
+	if (priv->cec_notify)
+		cec_notifier_put(priv->cec_notify);
 	if (client->irq)
 		free_irq(client->irq, priv);
 err_irq:
-	i2c_unregister_device(priv->cec);
 	return ret;
 }
 
-- 
2.7.4

Re: [PATCH v3 6/7] drm/i2c: tda998x: add CEC support

[Hans Verkuil]

On 04/09/18 14:16, Russell King wrote:
> The TDA998x is a HDMI transmitter with a TDA9950 CEC engine integrated
> onto the same die.  Add support for the TDA9950 CEC engine to the
> TDA998x driver.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

Reviewed-by: Hans Verkuil <hans.verkuil@cisco.com>

Regards,

	Hans

> ---
>  drivers/gpu/drm/i2c/Kconfig       |   1 +
>  drivers/gpu/drm/i2c/tda998x_drv.c | 195 ++++++++++++++++++++++++++++++++++++--
>  2 files changed, 187 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i2c/Kconfig b/drivers/gpu/drm/i2c/Kconfig
> index 3a232f5ff0a1..65d3acb61c03 100644
> --- a/drivers/gpu/drm/i2c/Kconfig
> +++ b/drivers/gpu/drm/i2c/Kconfig
> @@ -22,6 +22,7 @@ config DRM_I2C_SIL164
>  config DRM_I2C_NXP_TDA998X
>  	tristate "NXP Semiconductors TDA998X HDMI encoder"
>  	default m if DRM_TILCDC
> +	select CEC_CORE if CEC_NOTIFIER
>  	select SND_SOC_HDMI_CODEC if SND_SOC
>  	help
>  	  Support for NXP Semiconductors TDA998X HDMI encoders.
> diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
> index 16e0439cad44..eb9916bd84a4 100644
> --- a/drivers/gpu/drm/i2c/tda998x_drv.c
> +++ b/drivers/gpu/drm/i2c/tda998x_drv.c
> @@ -16,8 +16,10 @@
>   */
>  
>  #include <linux/component.h>
> +#include <linux/gpio/consumer.h>
>  #include <linux/hdmi.h>
>  #include <linux/module.h>
> +#include <linux/platform_data/tda9950.h>
>  #include <linux/irq.h>
>  #include <sound/asoundef.h>
>  #include <sound/hdmi-codec.h>
> @@ -29,6 +31,8 @@
>  #include <drm/drm_of.h>
>  #include <drm/i2c/tda998x.h>
>  
> +#include <media/cec-notifier.h>
> +
>  #define DBG(fmt, ...) DRM_DEBUG(fmt"\n", ##__VA_ARGS__)
>  
>  struct tda998x_audio_port {
> @@ -55,6 +59,7 @@ struct tda998x_priv {
>  	struct platform_device *audio_pdev;
>  	struct mutex audio_mutex;
>  
> +	struct mutex edid_mutex;
>  	wait_queue_head_t wq_edid;
>  	volatile int wq_edid_wait;
>  
> @@ -67,6 +72,9 @@ struct tda998x_priv {
>  	struct drm_connector connector;
>  
>  	struct tda998x_audio_port audio_port[2];
> +	struct tda9950_glue cec_glue;
> +	struct gpio_desc *calib;
> +	struct cec_notifier *cec_notify;
>  };
>  
>  #define conn_to_tda998x_priv(x) \
> @@ -345,6 +353,12 @@ struct tda998x_priv {
>  #define REG_CEC_INTSTATUS	  0xee		      /* read */
>  # define CEC_INTSTATUS_CEC	  (1 << 0)
>  # define CEC_INTSTATUS_HDMI	  (1 << 1)
> +#define REG_CEC_CAL_XOSC_CTRL1    0xf2
> +# define CEC_CAL_XOSC_CTRL1_ENA_CAL	BIT(0)
> +#define REG_CEC_DES_FREQ2         0xf5
> +# define CEC_DES_FREQ2_DIS_AUTOCAL BIT(7)
> +#define REG_CEC_CLK               0xf6
> +# define CEC_CLK_FRO              0x11
>  #define REG_CEC_FRO_IM_CLK_CTRL   0xfb                /* read/write */
>  # define CEC_FRO_IM_CLK_CTRL_GHOST_DIS (1 << 7)
>  # define CEC_FRO_IM_CLK_CTRL_ENA_OTP   (1 << 6)
> @@ -359,6 +373,7 @@ struct tda998x_priv {
>  # define CEC_RXSHPDLEV_HPD        (1 << 1)
>  
>  #define REG_CEC_ENAMODS           0xff                /* read/write */
> +# define CEC_ENAMODS_EN_CEC_CLK   (1 << 7)
>  # define CEC_ENAMODS_DIS_FRO      (1 << 6)
>  # define CEC_ENAMODS_DIS_CCLK     (1 << 5)
>  # define CEC_ENAMODS_EN_RXSENS    (1 << 2)
> @@ -417,6 +432,114 @@ cec_read(struct tda998x_priv *priv, u8 addr)
>  	return val;
>  }
>  
> +static void cec_enamods(struct tda998x_priv *priv, u8 mods, bool enable)
> +{
> +	int val = cec_read(priv, REG_CEC_ENAMODS);
> +
> +	if (val < 0)
> +		return;
> +
> +	if (enable)
> +		val |= mods;
> +	else
> +		val &= ~mods;
> +
> +	cec_write(priv, REG_CEC_ENAMODS, val);
> +}
> +
> +static void tda998x_cec_set_calibration(struct tda998x_priv *priv, bool enable)
> +{
> +	if (enable) {
> +		u8 val;
> +
> +		cec_write(priv, 0xf3, 0xc0);
> +		cec_write(priv, 0xf4, 0xd4);
> +
> +		/* Enable automatic calibration mode */
> +		val = cec_read(priv, REG_CEC_DES_FREQ2);
> +		val &= ~CEC_DES_FREQ2_DIS_AUTOCAL;
> +		cec_write(priv, REG_CEC_DES_FREQ2, val);
> +
> +		/* Enable free running oscillator */
> +		cec_write(priv, REG_CEC_CLK, CEC_CLK_FRO);
> +		cec_enamods(priv, CEC_ENAMODS_DIS_FRO, false);
> +
> +		cec_write(priv, REG_CEC_CAL_XOSC_CTRL1,
> +			  CEC_CAL_XOSC_CTRL1_ENA_CAL);
> +	} else {
> +		cec_write(priv, REG_CEC_CAL_XOSC_CTRL1, 0);
> +	}
> +}
> +
> +/*
> + * Calibration for the internal oscillator: we need to set calibration mode,
> + * and then pulse the IRQ line low for a 10ms ± 1% period.
> + */
> +static void tda998x_cec_calibration(struct tda998x_priv *priv)
> +{
> +	struct gpio_desc *calib = priv->calib;
> +
> +	mutex_lock(&priv->edid_mutex);
> +	if (priv->hdmi->irq > 0)
> +		disable_irq(priv->hdmi->irq);
> +	gpiod_direction_output(calib, 1);
> +	tda998x_cec_set_calibration(priv, true);
> +
> +	local_irq_disable();
> +	gpiod_set_value(calib, 0);
> +	mdelay(10);
> +	gpiod_set_value(calib, 1);
> +	local_irq_enable();
> +
> +	tda998x_cec_set_calibration(priv, false);
> +	gpiod_direction_input(calib);
> +	if (priv->hdmi->irq > 0)
> +		enable_irq(priv->hdmi->irq);
> +	mutex_unlock(&priv->edid_mutex);
> +}
> +
> +static int tda998x_cec_hook_init(void *data)
> +{
> +	struct tda998x_priv *priv = data;
> +	struct gpio_desc *calib;
> +
> +	calib = gpiod_get(&priv->hdmi->dev, "nxp,calib", GPIOD_ASIS);
> +	if (IS_ERR(calib)) {
> +		dev_warn(&priv->hdmi->dev, "failed to get calibration gpio: %ld\n",
> +			 PTR_ERR(calib));
> +		return PTR_ERR(calib);
> +	}
> +
> +	priv->calib = calib;
> +
> +	return 0;
> +}
> +
> +static void tda998x_cec_hook_exit(void *data)
> +{
> +	struct tda998x_priv *priv = data;
> +
> +	gpiod_put(priv->calib);
> +	priv->calib = NULL;
> +}
> +
> +static int tda998x_cec_hook_open(void *data)
> +{
> +	struct tda998x_priv *priv = data;
> +
> +	cec_enamods(priv, CEC_ENAMODS_EN_CEC_CLK | CEC_ENAMODS_EN_CEC, true);
> +	tda998x_cec_calibration(priv);
> +
> +	return 0;
> +}
> +
> +static void tda998x_cec_hook_release(void *data)
> +{
> +	struct tda998x_priv *priv = data;
> +
> +	cec_enamods(priv, CEC_ENAMODS_EN_CEC_CLK | CEC_ENAMODS_EN_CEC, false);
> +}
> +
>  static int
>  set_page(struct tda998x_priv *priv, u16 reg)
>  {
> @@ -657,10 +780,13 @@ static irqreturn_t tda998x_irq_thread(int irq, void *data)
>  			sta, cec, lvl, flag0, flag1, flag2);
>  
>  		if (cec & CEC_RXSHPDINT_HPD) {
> -			if (lvl & CEC_RXSHPDLEV_HPD)
> +			if (lvl & CEC_RXSHPDLEV_HPD) {
>  				tda998x_edid_delay_start(priv);
> -			else
> +			} else {
>  				schedule_work(&priv->detect_work);
> +				cec_notifier_set_phys_addr(priv->cec_notify,
> +						   CEC_PHYS_ADDR_INVALID);
> +			}
>  
>  			handled = true;
>  		}
> @@ -981,6 +1107,8 @@ static int tda998x_connector_fill_modes(struct drm_connector *connector,
>  	if (connector->edid_blob_ptr) {
>  		struct edid *edid = (void *)connector->edid_blob_ptr->data;
>  
> +		cec_notifier_set_phys_addr_from_edid(priv->cec_notify, edid);
> +
>  		priv->sink_has_audio = drm_detect_monitor_audio(edid);
>  	} else {
>  		priv->sink_has_audio = false;
> @@ -1024,6 +1152,8 @@ static int read_edid_block(void *data, u8 *buf, unsigned int blk, size_t length)
>  	offset = (blk & 1) ? 128 : 0;
>  	segptr = blk / 2;
>  
> +	mutex_lock(&priv->edid_mutex);
> +
>  	reg_write(priv, REG_DDC_ADDR, 0xa0);
>  	reg_write(priv, REG_DDC_OFFS, offset);
>  	reg_write(priv, REG_DDC_SEGM_ADDR, 0x60);
> @@ -1043,14 +1173,15 @@ static int read_edid_block(void *data, u8 *buf, unsigned int blk, size_t length)
>  					msecs_to_jiffies(100));
>  		if (i < 0) {
>  			dev_err(&priv->hdmi->dev, "read edid wait err %d\n", i);
> -			return i;
> +			ret = i;
> +			goto failed;
>  		}
>  	} else {
>  		for (i = 100; i > 0; i--) {
>  			msleep(1);
>  			ret = reg_read(priv, REG_INT_FLAGS_2);
>  			if (ret < 0)
> -				return ret;
> +				goto failed;
>  			if (ret & INT_FLAGS_2_EDID_BLK_RD)
>  				break;
>  		}
> @@ -1058,17 +1189,22 @@ static int read_edid_block(void *data, u8 *buf, unsigned int blk, size_t length)
>  
>  	if (i == 0) {
>  		dev_err(&priv->hdmi->dev, "read edid timeout\n");
> -		return -ETIMEDOUT;
> +		ret = -ETIMEDOUT;
> +		goto failed;
>  	}
>  
>  	ret = reg_read_range(priv, REG_EDID_DATA_0, buf, length);
>  	if (ret != length) {
>  		dev_err(&priv->hdmi->dev, "failed to read edid block %d: %d\n",
>  			blk, ret);
> -		return ret;
> +		goto failed;
>  	}
>  
> -	return 0;
> +	ret = 0;
> +
> + failed:
> +	mutex_unlock(&priv->edid_mutex);
> +	return ret;
>  }
>  
>  static int tda998x_connector_get_modes(struct drm_connector *connector)
> @@ -1423,6 +1559,9 @@ static void tda998x_destroy(struct tda998x_priv *priv)
>  	cancel_work_sync(&priv->detect_work);
>  
>  	i2c_unregister_device(priv->cec);
> +
> +	if (priv->cec_notify)
> +		cec_notifier_put(priv->cec_notify);
>  }
>  
>  /* I2C driver functions */
> @@ -1472,11 +1611,13 @@ static int tda998x_get_audio_ports(struct tda998x_priv *priv,
>  static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv)
>  {
>  	struct device_node *np = client->dev.of_node;
> +	struct i2c_board_info cec_info;
>  	u32 video;
>  	int rev_lo, rev_hi, ret;
>  
>  	mutex_init(&priv->mutex);	/* protect the page access */
>  	mutex_init(&priv->audio_mutex); /* protect access from audio thread */
> +	mutex_init(&priv->edid_mutex);
>  	init_waitqueue_head(&priv->edid_delay_waitq);
>  	timer_setup(&priv->edid_delay_timer, tda998x_edid_delay_done, 0);
>  	INIT_WORK(&priv->detect_work, tda998x_detect_work);
> @@ -1564,6 +1705,9 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv)
>  
>  		irq_flags =
>  			irqd_get_trigger_type(irq_get_irq_data(client->irq));
> +
> +		priv->cec_glue.irq_flags = irq_flags;
> +
>  		irq_flags |= IRQF_SHARED | IRQF_ONESHOT;
>  		ret = request_threaded_irq(client->irq, NULL,
>  					   tda998x_irq_thread, irq_flags,
> @@ -1579,7 +1723,34 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv)
>  		cec_write(priv, REG_CEC_RXSHPDINTENA, CEC_RXSHPDLEV_HPD);
>  	}
>  
> -	priv->cec = i2c_new_dummy(client->adapter, priv->cec_addr);
> +	priv->cec_notify = cec_notifier_get(&client->dev);
> +	if (!priv->cec_notify) {
> +		ret = -ENOMEM;
> +		goto fail;
> +	}
> +
> +	priv->cec_glue.parent = &client->dev;
> +	priv->cec_glue.data = priv;
> +	priv->cec_glue.init = tda998x_cec_hook_init;
> +	priv->cec_glue.exit = tda998x_cec_hook_exit;
> +	priv->cec_glue.open = tda998x_cec_hook_open;
> +	priv->cec_glue.release = tda998x_cec_hook_release;
> +
> +	/*
> +	 * Some TDA998x are actually two I2C devices merged onto one piece
> +	 * of silicon: TDA9989 and TDA19989 combine the HDMI transmitter
> +	 * with a slightly modified TDA9950 CEC device.  The CEC device
> +	 * is at the TDA9950 address, with the address pins strapped across
> +	 * to the TDA998x address pins.  Hence, it always has the same
> +	 * offset.
> +	 */
> +	memset(&cec_info, 0, sizeof(cec_info));
> +	strlcpy(cec_info.type, "tda9950", sizeof(cec_info.type));
> +	cec_info.addr = priv->cec_addr;
> +	cec_info.platform_data = &priv->cec_glue;
> +	cec_info.irq = client->irq;
> +
> +	priv->cec = i2c_new_device(client->adapter, &cec_info);
>  	if (!priv->cec) {
>  		ret = -ENODEV;
>  		goto fail;
> @@ -1609,10 +1780,16 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv)
>  	return 0;
>  
>  fail:
> +	/* if encoder_init fails, the encoder slave is never registered,
> +	 * so cleanup here:
> +	 */
> +	if (priv->cec)
> +		i2c_unregister_device(priv->cec);
> +	if (priv->cec_notify)
> +		cec_notifier_put(priv->cec_notify);
>  	if (client->irq)
>  		free_irq(client->irq, priv);
>  err_irq:
> -	i2c_unregister_device(priv->cec);
>  	return ret;
>  }
>  
> 

[PATCH v3 7/7] dt-bindings: tda998x: add the calibration gpio

[Russell King]

Add the optional calibration gpio for integrated TDA9950 CEC support.
This GPIO corresponds with the interrupt from the TDA998x, as the
calibration requires driving the interrupt pin low.

Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 Documentation/devicetree/bindings/display/bridge/tda998x.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/bridge/tda998x.txt b/Documentation/devicetree/bindings/display/bridge/tda998x.txt
index 24cc2466185a..1a4eaca40d94 100644
--- a/Documentation/devicetree/bindings/display/bridge/tda998x.txt
+++ b/Documentation/devicetree/bindings/display/bridge/tda998x.txt
@@ -27,6 +27,9 @@ Required properties;
 	in question is used. The implementation allows one or two DAIs. If two
 	DAIs are defined, they must be of different type.
 
+  - nxp,calib-gpios: calibration GPIO, which must correspond with the
+	gpio used for the TDA998x interrupt pin.
+
 [1] Documentation/sound/alsa/soc/DAI.txt
 [2] include/dt-bindings/display/tda998x.h
 
-- 
2.7.4

Re: [PATCH v3 7/7] dt-bindings: tda998x: add the calibration gpio

[Hans Verkuil]

On 04/09/18 14:16, Russell King wrote:
> Add the optional calibration gpio for integrated TDA9950 CEC support.
> This GPIO corresponds with the interrupt from the TDA998x, as the
> calibration requires driving the interrupt pin low.
> 
> Reviewed-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

Acked-by: Hans Verkuil <hans.verkuil@cisco.com>

Regards,

	Hans

> ---
>  Documentation/devicetree/bindings/display/bridge/tda998x.txt | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/tda998x.txt b/Documentation/devicetree/bindings/display/bridge/tda998x.txt
> index 24cc2466185a..1a4eaca40d94 100644
> --- a/Documentation/devicetree/bindings/display/bridge/tda998x.txt
> +++ b/Documentation/devicetree/bindings/display/bridge/tda998x.txt
> @@ -27,6 +27,9 @@ Required properties;
>  	in question is used. The implementation allows one or two DAIs. If two
>  	DAIs are defined, they must be of different type.
>  
> +  - nxp,calib-gpios: calibration GPIO, which must correspond with the
> +	gpio used for the TDA998x interrupt pin.
> +
>  [1] Documentation/sound/alsa/soc/DAI.txt
>  [2] include/dt-bindings/display/tda998x.h
>  
> 

Re: [PATCH v3 0/7] TDA998x CEC support

[Hans Verkuil]

On 04/09/18 14:15, Russell King - ARM Linux wrote:
> Hi,
> 
> This patch series adds CEC support to the DRM TDA998x driver.  The
> TDA998x family of devices integrate a TDA9950 CEC at a separate I2C
> address from the HDMI encoder.
> 
> Implementation of the CEC part is separate to allow independent CEC
> implementations, or independent HDMI implementations (since the
> TDA9950 may be a separate device.)

Reviewed, looks good.

Thanks!

	Hans

> 
>  .../devicetree/bindings/display/bridge/tda998x.txt |   3 +
>  drivers/gpu/drm/i2c/Kconfig                        |   6 +
>  drivers/gpu/drm/i2c/Makefile                       |   1 +
>  drivers/gpu/drm/i2c/tda9950.c                      | 509 +++++++++++++++++++++
>  drivers/gpu/drm/i2c/tda998x_drv.c                  | 242 ++++++++--
>  include/linux/platform_data/tda9950.h              |  16 +
>  6 files changed, 750 insertions(+), 27 deletions(-)
>  create mode 100644 drivers/gpu/drm/i2c/tda9950.c
>  create mode 100644 include/linux/platform_data/tda9950.h
> 
> v3: addressed most of Hans comments in v2
> v2: updated DT property.
>  
> 

Re: [PATCH v3 0/7] TDA998x CEC support

[Russell King - ARM Linux]

On Tue, Apr 24, 2018 at 11:29:42AM +0200, Hans Verkuil wrote:
> On 04/09/18 14:15, Russell King - ARM Linux wrote:
> > Hi,
> > 
> > This patch series adds CEC support to the DRM TDA998x driver.  The
> > TDA998x family of devices integrate a TDA9950 CEC at a separate I2C
> > address from the HDMI encoder.
> > 
> > Implementation of the CEC part is separate to allow independent CEC
> > implementations, or independent HDMI implementations (since the
> > TDA9950 may be a separate device.)
> 
> Reviewed, looks good.

Thanks!  If we need to rework the calibration GPIO stuff for BB, we can
do that in a later patch.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

Re: [PATCH v3 0/7] TDA998x CEC support

[Hans Verkuil]

On 04/24/18 11:42, Russell King - ARM Linux wrote:
> On Tue, Apr 24, 2018 at 11:29:42AM +0200, Hans Verkuil wrote:
>> On 04/09/18 14:15, Russell King - ARM Linux wrote:
>>> Hi,
>>>
>>> This patch series adds CEC support to the DRM TDA998x driver.  The
>>> TDA998x family of devices integrate a TDA9950 CEC at a separate I2C
>>> address from the HDMI encoder.
>>>
>>> Implementation of the CEC part is separate to allow independent CEC
>>> implementations, or independent HDMI implementations (since the
>>> TDA9950 may be a separate device.)
>>
>> Reviewed, looks good.
> 
> Thanks!  If we need to rework the calibration GPIO stuff for BB, we can
> do that in a later patch.
> 

Yes, I'll see if I can spend a bit of time on that next week.

Regards,

	Hans