[PATCH v2 0/2] media: Add Amlogic Meson AO CEC Controller support

[PATCH v2 0/2] media: Add Amlogic Meson AO CEC Controller support

[Neil Armstrong]

The Amlogic SoC embeds a standalone CEC controller, this patch adds a driver
for such controller.
The controller does not need HPD to be active, and could support up to max
5 logical addresses, but only 1 is handled since the Suspend firmware can
make use of this unique logical address to wake up the device.

The Suspend firmware configuration will be added in an other patchset.

Changes since v1 at [1] :
 - add timeout to wait busy, with error return
 - handle busy error in all read/write operations
 - add CEC_CAP_PASSTHROUGH
 - add bindings ack

[1] https://lkml.kernel.org/r/1499336870-24118-1-git-send-email-narmstrong@baylibre.com

Neil Armstrong (2):
  platform: Add Amlogic Meson AO CEC Controller driver
  dt-bindings: media: Add Amlogic Meson AO-CEC bindings

 .../devicetree/bindings/media/meson-ao-cec.txt     |  28 +
 drivers/media/platform/Kconfig                     |  11 +
 drivers/media/platform/Makefile                    |   2 +
 drivers/media/platform/meson/Makefile              |   1 +
 drivers/media/platform/meson/ao-cec.c              | 781 +++++++++++++++++++++
 5 files changed, 823 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/meson-ao-cec.txt
 create mode 100644 drivers/media/platform/meson/Makefile
 create mode 100644 drivers/media/platform/meson/ao-cec.c

-- 
1.9.1

[PATCH v2 1/2] platform: Add Amlogic Meson AO CEC Controller driver

[Neil Armstrong]

The Amlogic SoC embeds a standalone CEC controller, this patch adds a driver
for such controller.
The controller does not need HPD to be active, and could support up to max
5 logical addresses, but only 1 is handled since the Suspend firmware can
make use of this unique logical address to wake up the device.

The Suspend firmware configuration will be added in an other patchset.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/media/platform/Kconfig        |  11 +
 drivers/media/platform/Makefile       |   2 +
 drivers/media/platform/meson/Makefile |   1 +
 drivers/media/platform/meson/ao-cec.c | 781 ++++++++++++++++++++++++++++++++++
 4 files changed, 795 insertions(+)
 create mode 100644 drivers/media/platform/meson/Makefile
 create mode 100644 drivers/media/platform/meson/ao-cec.c

diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index 1313cd5..1e67381 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -536,6 +536,17 @@ menuconfig CEC_PLATFORM_DRIVERS
 
 if CEC_PLATFORM_DRIVERS
 
+config VIDEO_MESON_AO_CEC
+	tristate "Amlogic Meson AO CEC driver"
+	depends on ARCH_MESON || COMPILE_TEST
+	select CEC_CORE
+	select CEC_NOTIFIER
+	---help---
+	  This is a driver for Amlogic Meson SoCs AO CEC interface. It uses the
+	  generic CEC framework interface.
+	  CEC bus is present in the HDMI connector and enables communication
+	  between compatible devices.
+
 config VIDEO_SAMSUNG_S5P_CEC
        tristate "Samsung S5P CEC driver"
        depends on PLAT_S5P || ARCH_EXYNOS || COMPILE_TEST
diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index 9beadc7..a52d7b6 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -86,3 +86,5 @@ obj-$(CONFIG_VIDEO_MEDIATEK_MDP)	+= mtk-mdp/
 obj-$(CONFIG_VIDEO_MEDIATEK_JPEG)	+= mtk-jpeg/
 
 obj-$(CONFIG_VIDEO_QCOM_VENUS)		+= qcom/venus/
+
+obj-y					+= meson/
diff --git a/drivers/media/platform/meson/Makefile b/drivers/media/platform/meson/Makefile
new file mode 100644
index 0000000..597beb8
--- /dev/null
+++ b/drivers/media/platform/meson/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_VIDEO_MESON_AO_CEC)	+= ao-cec.o
diff --git a/drivers/media/platform/meson/ao-cec.c b/drivers/media/platform/meson/ao-cec.c
new file mode 100644
index 0000000..cdc1f61
--- /dev/null
+++ b/drivers/media/platform/meson/ao-cec.c
@@ -0,0 +1,781 @@
+/*
+ * Driver for Amlogic Meson AO CEC Controller
+ *
+ * Copyright (C) 2015 Amlogic, Inc. All rights reserved
+ * Copyright (C) 2017 BayLibre, SAS
+ * Author: Neil Armstrong <narmstrong@baylibre.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0+
+ */
+
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/device.h>
+#include <linux/io.h>
+#include <linux/delay.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/types.h>
+#include <linux/interrupt.h>
+#include <linux/reset.h>
+#include <media/cec.h>
+#include <media/cec-notifier.h>
+
+/* CEC Registers */
+
+/*
+ * [2:1] cntl_clk
+ *  - 0 = Disable clk (Power-off mode)
+ *  - 1 = Enable gated clock (Normal mode)
+ *  - 2 = Enable free-run clk (Debug mode)
+ */
+#define CEC_GEN_CNTL_REG		0x00
+
+#define CEC_GEN_CNTL_RESET		BIT(0)
+#define CEC_GEN_CNTL_CLK_DISABLE	0
+#define CEC_GEN_CNTL_CLK_ENABLE		1
+#define CEC_GEN_CNTL_CLK_ENABLE_DBG	2
+#define CEC_GEN_CNTL_CLK_CTRL_MASK	GENMASK(2, 1)
+
+/*
+ * [7:0] cec_reg_addr
+ * [15:8] cec_reg_wrdata
+ * [16] cec_reg_wr
+ *  - 0 = Read
+ *  - 1 = Write
+ * [23] bus free
+ * [31:24] cec_reg_rddata
+ */
+#define CEC_RW_REG			0x04
+
+#define CEC_RW_ADDR			GENMASK(7, 0)
+#define CEC_RW_WR_DATA			GENMASK(15, 8)
+#define CEC_RW_WRITE_EN			BIT(16)
+#define CEC_RW_BUS_BUSY			BIT(23)
+#define CEC_RW_RD_DATA			GENMASK(31, 24)
+
+/*
+ * [1] tx intr
+ * [2] rx intr
+ */
+#define CEC_INTR_MASKN_REG		0x08
+#define CEC_INTR_CLR_REG		0x0c
+#define CEC_INTR_STAT_REG		0x10
+
+#define CEC_INTR_TX			BIT(1)
+#define CEC_INTR_RX			BIT(2)
+
+/* CEC Commands */
+
+#define CEC_TX_MSG_0_HEADER		0x00
+#define CEC_TX_MSG_1_OPCODE		0x01
+#define CEC_TX_MSG_2_OP1		0x02
+#define CEC_TX_MSG_3_OP2		0x03
+#define CEC_TX_MSG_4_OP3		0x04
+#define CEC_TX_MSG_5_OP4		0x05
+#define CEC_TX_MSG_6_OP5		0x06
+#define CEC_TX_MSG_7_OP6		0x07
+#define CEC_TX_MSG_8_OP7		0x08
+#define CEC_TX_MSG_9_OP8		0x09
+#define CEC_TX_MSG_A_OP9		0x0A
+#define CEC_TX_MSG_B_OP10		0x0B
+#define CEC_TX_MSG_C_OP11		0x0C
+#define CEC_TX_MSG_D_OP12		0x0D
+#define CEC_TX_MSG_E_OP13		0x0E
+#define CEC_TX_MSG_F_OP14		0x0F
+#define CEC_TX_MSG_LENGTH		0x10
+#define CEC_TX_MSG_CMD			0x11
+#define CEC_TX_WRITE_BUF		0x12
+#define CEC_TX_CLEAR_BUF		0x13
+#define CEC_RX_MSG_CMD			0x14
+#define CEC_RX_CLEAR_BUF		0x15
+#define CEC_LOGICAL_ADDR0		0x16
+#define CEC_LOGICAL_ADDR1		0x17
+#define CEC_LOGICAL_ADDR2		0x18
+#define CEC_LOGICAL_ADDR3		0x19
+#define CEC_LOGICAL_ADDR4		0x1A
+#define CEC_CLOCK_DIV_H			0x1B
+#define CEC_CLOCK_DIV_L			0x1C
+#define CEC_QUIESCENT_25MS_BIT7_0	0x20
+#define CEC_QUIESCENT_25MS_BIT11_8	0x21
+#define CEC_STARTBITMINL2H_3MS5_BIT7_0	0x22
+#define CEC_STARTBITMINL2H_3MS5_BIT8	0x23
+#define CEC_STARTBITMAXL2H_3MS9_BIT7_0	0x24
+#define CEC_STARTBITMAXL2H_3MS9_BIT8	0x25
+#define CEC_STARTBITMINH_0MS6_BIT7_0	0x26
+#define CEC_STARTBITMINH_0MS6_BIT8	0x27
+#define CEC_STARTBITMAXH_1MS0_BIT7_0	0x28
+#define CEC_STARTBITMAXH_1MS0_BIT8	0x29
+#define CEC_STARTBITMINTOT_4MS3_BIT7_0	0x2A
+#define CEC_STARTBITMINTOT_4MS3_BIT9_8	0x2B
+#define CEC_STARTBITMAXTOT_4MS7_BIT7_0	0x2C
+#define CEC_STARTBITMAXTOT_4MS7_BIT9_8	0x2D
+#define CEC_LOGIC1MINL2H_0MS4_BIT7_0	0x2E
+#define CEC_LOGIC1MINL2H_0MS4_BIT8	0x2F
+#define CEC_LOGIC1MAXL2H_0MS8_BIT7_0	0x30
+#define CEC_LOGIC1MAXL2H_0MS8_BIT8	0x31
+#define CEC_LOGIC0MINL2H_1MS3_BIT7_0	0x32
+#define CEC_LOGIC0MINL2H_1MS3_BIT8	0x33
+#define CEC_LOGIC0MAXL2H_1MS7_BIT7_0	0x34
+#define CEC_LOGIC0MAXL2H_1MS7_BIT8	0x35
+#define CEC_LOGICMINTOTAL_2MS05_BIT7_0	0x36
+#define CEC_LOGICMINTOTAL_2MS05_BIT9_8	0x37
+#define CEC_LOGICMAXHIGH_2MS8_BIT7_0	0x38
+#define CEC_LOGICMAXHIGH_2MS8_BIT8	0x39
+#define CEC_LOGICERRLOW_3MS4_BIT7_0	0x3A
+#define CEC_LOGICERRLOW_3MS4_BIT8	0x3B
+#define CEC_NOMSMPPOINT_1MS05		0x3C
+#define CEC_DELCNTR_LOGICERR		0x3E
+#define CEC_TXTIME_17MS_BIT7_0		0x40
+#define CEC_TXTIME_17MS_BIT10_8		0x41
+#define CEC_TXTIME_2BIT_BIT7_0		0x42
+#define CEC_TXTIME_2BIT_BIT10_8		0x43
+#define CEC_TXTIME_4BIT_BIT7_0		0x44
+#define CEC_TXTIME_4BIT_BIT10_8		0x45
+#define CEC_STARTBITNOML2H_3MS7_BIT7_0	0x46
+#define CEC_STARTBITNOML2H_3MS7_BIT8	0x47
+#define CEC_STARTBITNOMH_0MS8_BIT7_0	0x48
+#define CEC_STARTBITNOMH_0MS8_BIT8	0x49
+#define CEC_LOGIC1NOML2H_0MS6_BIT7_0	0x4A
+#define CEC_LOGIC1NOML2H_0MS6_BIT8	0x4B
+#define CEC_LOGIC0NOML2H_1MS5_BIT7_0	0x4C
+#define CEC_LOGIC0NOML2H_1MS5_BIT8	0x4D
+#define CEC_LOGIC1NOMH_1MS8_BIT7_0	0x4E
+#define CEC_LOGIC1NOMH_1MS8_BIT8	0x4F
+#define CEC_LOGIC0NOMH_0MS9_BIT7_0	0x50
+#define CEC_LOGIC0NOMH_0MS9_BIT8	0x51
+#define CEC_LOGICERRLOW_3MS6_BIT7_0	0x52
+#define CEC_LOGICERRLOW_3MS6_BIT8	0x53
+#define CEC_CHKCONTENTION_0MS1		0x54
+#define CEC_PREPARENXTBIT_0MS05_BIT7_0	0x56
+#define CEC_PREPARENXTBIT_0MS05_BIT8	0x57
+#define CEC_NOMSMPACKPOINT_0MS45	0x58
+#define CEC_ACK0NOML2H_1MS5_BIT7_0	0x5A
+#define CEC_ACK0NOML2H_1MS5_BIT8	0x5B
+#define CEC_BUGFIX_DISABLE_0		0x60
+#define CEC_BUGFIX_DISABLE_1		0x61
+#define CEC_RX_MSG_0_HEADER		0x80
+#define CEC_RX_MSG_1_OPCODE		0x81
+#define CEC_RX_MSG_2_OP1		0x82
+#define CEC_RX_MSG_3_OP2		0x83
+#define CEC_RX_MSG_4_OP3		0x84
+#define CEC_RX_MSG_5_OP4		0x85
+#define CEC_RX_MSG_6_OP5		0x86
+#define CEC_RX_MSG_7_OP6		0x87
+#define CEC_RX_MSG_8_OP7		0x88
+#define CEC_RX_MSG_9_OP8		0x89
+#define CEC_RX_MSG_A_OP9		0x8A
+#define CEC_RX_MSG_B_OP10		0x8B
+#define CEC_RX_MSG_C_OP11		0x8C
+#define CEC_RX_MSG_D_OP12		0x8D
+#define CEC_RX_MSG_E_OP13		0x8E
+#define CEC_RX_MSG_F_OP14		0x8F
+#define CEC_RX_MSG_LENGTH		0x90
+#define CEC_RX_MSG_STATUS		0x91
+#define CEC_RX_NUM_MSG			0x92
+#define CEC_TX_MSG_STATUS		0x93
+#define CEC_TX_NUM_MSG			0x94
+
+
+/* CEC_TX_MSG_CMD definition */
+#define TX_NO_OP	0  /* No transaction */
+#define TX_REQ_CURRENT	1  /* Transmit earliest message in buffer */
+#define TX_ABORT	2  /* Abort transmitting earliest message */
+#define TX_REQ_NEXT	3  /* Overwrite earliest msg, transmit next */
+
+/* tx_msg_status definition */
+#define TX_IDLE		0  /* No transaction */
+#define TX_BUSY		1  /* Transmitter is busy */
+#define TX_DONE		2  /* Message successfully transmitted */
+#define TX_ERROR	3  /* Message transmitted with error */
+
+/* rx_msg_cmd */
+#define RX_NO_OP	0  /* No transaction */
+#define RX_ACK_CURRENT	1  /* Read earliest message in buffer */
+#define RX_DISABLE	2  /* Disable receiving latest message */
+#define RX_ACK_NEXT	3  /* Clear earliest msg, read next */
+
+/* rx_msg_status */
+#define RX_IDLE		0  /* No transaction */
+#define RX_BUSY		1  /* Receiver is busy */
+#define RX_DONE		2  /* Message has been received successfully */
+#define RX_ERROR	3  /* Message has been received with error */
+
+/* RX_CLEAR_BUF options */
+#define CLEAR_START	1
+#define CLEAR_STOP	0
+
+/* CEC_LOGICAL_ADDRx options */
+#define LOGICAL_ADDR_MASK	0xf
+#define LOGICAL_ADDR_VALID	BIT(4)
+#define LOGICAL_ADDR_DISABLE	0
+
+#define CEC_CLK_RATE		32768
+
+struct meson_ao_cec_device {
+	struct platform_device		*pdev;
+	void __iomem			*base;
+	struct clk			*core;
+	spinlock_t			cec_reg_lock;
+	struct cec_notifier		*notify;
+	struct cec_adapter		*adap;
+	struct cec_msg			rx_msg;
+};
+
+#define writel_bits_relaxed(mask, val, addr) \
+	writel_relaxed((readl_relaxed(addr) & ~(mask)) | (val), addr)
+
+static inline int meson_ao_cec_wait_busy(struct meson_ao_cec_device *ao_cec)
+{
+	ktime_t timeout = ktime_add_us(ktime_get(), 5000);
+
+	while (readl_relaxed(ao_cec->base + CEC_RW_REG) & CEC_RW_BUS_BUSY) {
+		if (ktime_compare(ktime_get(), timeout) > 0)
+			return -ETIMEDOUT;
+	}
+
+	return 0;
+}
+
+static int meson_ao_cec_read(struct meson_ao_cec_device *ao_cec,
+			     unsigned long address, u8 *data)
+{
+	unsigned long flags;
+	u32 reg = FIELD_PREP(CEC_RW_ADDR, address);
+	int ret = 0;
+
+	spin_lock_irqsave(&ao_cec->cec_reg_lock, flags);
+
+	ret = meson_ao_cec_wait_busy(ao_cec);
+	if (ret)
+		goto read_out;
+
+	writel_relaxed(reg, ao_cec->base + CEC_RW_REG);
+
+	ret = meson_ao_cec_wait_busy(ao_cec);
+	if (ret)
+		goto read_out;
+
+	*data = FIELD_GET(CEC_RW_RD_DATA,
+			  readl_relaxed(ao_cec->base + CEC_RW_REG));
+
+read_out:
+	spin_unlock_irqrestore(&ao_cec->cec_reg_lock, flags);
+
+	return ret;
+}
+
+static int meson_ao_cec_write(struct meson_ao_cec_device *ao_cec,
+			      unsigned long address, u8 data)
+{
+	unsigned long flags;
+	u32 reg = FIELD_PREP(CEC_RW_ADDR, address) |
+		  FIELD_PREP(CEC_RW_WR_DATA, data) |
+		  CEC_RW_WRITE_EN;
+	int ret = 0;
+
+	spin_lock_irqsave(&ao_cec->cec_reg_lock, flags);
+
+	ret = meson_ao_cec_wait_busy(ao_cec);
+	if (ret)
+		goto write_out;
+
+	writel_relaxed(reg, ao_cec->base + CEC_RW_REG);
+
+write_out:
+	spin_unlock_irqrestore(&ao_cec->cec_reg_lock, flags);
+
+	return ret;
+}
+
+static inline void meson_ao_cec_irq_setup(struct meson_ao_cec_device *ao_cec,
+				      bool enable)
+{
+	u32 cfg = CEC_INTR_TX | CEC_INTR_RX;
+
+	writel_bits_relaxed(cfg, enable ? cfg : 0,
+			    ao_cec->base + CEC_INTR_MASKN_REG);
+}
+
+static inline int meson_ao_cec_clear(struct meson_ao_cec_device *ao_cec)
+{
+	int ret;
+
+	ret = meson_ao_cec_write(ao_cec, CEC_RX_MSG_CMD, RX_DISABLE);
+	if (ret)
+		return ret;
+	ret = meson_ao_cec_write(ao_cec, CEC_TX_MSG_CMD, TX_ABORT);
+	if (ret)
+		return ret;
+	ret = meson_ao_cec_write(ao_cec, CEC_RX_CLEAR_BUF, 1);
+	if (ret)
+		return ret;
+	ret = meson_ao_cec_write(ao_cec, CEC_TX_CLEAR_BUF, 1);
+	if (ret)
+		return ret;
+
+	udelay(100);
+
+	ret = meson_ao_cec_write(ao_cec, CEC_RX_CLEAR_BUF, 0);
+	if (ret)
+		return ret;
+	ret = meson_ao_cec_write(ao_cec, CEC_TX_CLEAR_BUF, 0);
+	if (ret)
+		return ret;
+
+	udelay(100);
+
+	ret = meson_ao_cec_write(ao_cec, CEC_RX_MSG_CMD, RX_NO_OP);
+	if (ret)
+		return ret;
+	ret = meson_ao_cec_write(ao_cec, CEC_TX_MSG_CMD, TX_NO_OP);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int meson_ao_cec_arbit_bit_time_set(struct meson_ao_cec_device *ao_cec,
+					   unsigned int bit_set,
+					   unsigned int time_set)
+{
+	int ret;
+
+	switch (bit_set) {
+	case CEC_SIGNAL_FREE_TIME_RETRY:
+		ret = meson_ao_cec_write(ao_cec, CEC_TXTIME_4BIT_BIT7_0,
+				   time_set & 0xff);
+		if (ret)
+			return ret;
+		ret = meson_ao_cec_write(ao_cec, CEC_TXTIME_4BIT_BIT10_8,
+				   (time_set >> 8) & 0x7);
+		if (ret)
+			return ret;
+		break;
+
+	case CEC_SIGNAL_FREE_TIME_NEW_INITIATOR:
+		ret = meson_ao_cec_write(ao_cec, CEC_TXTIME_2BIT_BIT7_0,
+				   time_set & 0xff);
+		if (ret)
+			return ret;
+		ret = meson_ao_cec_write(ao_cec, CEC_TXTIME_2BIT_BIT10_8,
+				   (time_set >> 8) & 0x7);
+		if (ret)
+			return ret;
+		break;
+
+	case CEC_SIGNAL_FREE_TIME_NEXT_XFER:
+		ret = meson_ao_cec_write(ao_cec, CEC_TXTIME_17MS_BIT7_0,
+				   time_set & 0xff);
+		if (ret)
+			return ret;
+		ret = meson_ao_cec_write(ao_cec, CEC_TXTIME_17MS_BIT10_8,
+				   (time_set >> 8) & 0x7);
+		if (ret)
+			return ret;
+		break;
+	}
+
+	return 0;
+}
+
+static irqreturn_t meson_ao_cec_irq(int irq, void *data)
+{
+	struct meson_ao_cec_device *ao_cec = data;
+	u32 stat = readl_relaxed(ao_cec->base + CEC_INTR_STAT_REG);
+
+	if (stat)
+		return IRQ_WAKE_THREAD;
+
+	return IRQ_NONE;
+}
+
+static void meson_ao_cec_irq_tx(struct meson_ao_cec_device *ao_cec)
+{
+	unsigned long tx_status = 0;
+	u8 stat;
+	int ret;
+
+	ret = meson_ao_cec_read(ao_cec, CEC_TX_MSG_STATUS, &stat);
+	if (ret)
+		goto tx_reg_err;
+
+	switch (stat) {
+	case TX_DONE:
+		tx_status = CEC_TX_STATUS_OK;
+		break;
+
+	case TX_BUSY:
+		tx_status = CEC_TX_STATUS_ARB_LOST;
+		break;
+
+	case TX_IDLE:
+		tx_status = CEC_TX_STATUS_LOW_DRIVE;
+		break;
+
+	case TX_ERROR:
+	default:
+		tx_status = CEC_TX_STATUS_NACK;
+		break;
+	}
+
+	/* Clear Interruption */
+	writel_relaxed(CEC_INTR_TX, ao_cec->base + CEC_INTR_CLR_REG);
+
+	/* Stop TX */
+	ret = meson_ao_cec_write(ao_cec, CEC_TX_MSG_CMD, TX_NO_OP);
+	if (ret)
+		goto tx_reg_err;
+
+	cec_transmit_attempt_done(ao_cec->adap, tx_status);
+	return;
+
+tx_reg_err:
+	cec_transmit_attempt_done(ao_cec->adap, CEC_TX_STATUS_ERROR);
+}
+
+static void meson_ao_cec_irq_rx(struct meson_ao_cec_device *ao_cec)
+{
+	int i, ret;
+	u8 reg;
+
+	ret = meson_ao_cec_read(ao_cec, CEC_RX_MSG_STATUS, &reg);
+	if (ret)
+		goto rx_out;
+
+	/* RX Error */
+	if (reg != RX_DONE)
+		goto rx_out;
+
+	ret = meson_ao_cec_read(ao_cec, CEC_RX_NUM_MSG, &reg);
+	if (ret || reg != 1)
+		goto rx_out;
+
+	ret = meson_ao_cec_read(ao_cec, CEC_RX_MSG_LENGTH, &reg);
+	if (ret)
+		goto rx_out;
+
+	ao_cec->rx_msg.len = reg + 1;
+	if (ao_cec->rx_msg.len > CEC_MAX_MSG_SIZE)
+		ao_cec->rx_msg.len = CEC_MAX_MSG_SIZE;
+
+	for (i = 0; i < ao_cec->rx_msg.len; i++) {
+		u8 byte;
+
+		ret = meson_ao_cec_read(ao_cec, CEC_RX_MSG_0_HEADER + i, &byte);
+		if (ret)
+			goto rx_out;
+
+		ao_cec->rx_msg.msg[i] = byte;
+	}
+
+	cec_received_msg(ao_cec->adap, &ao_cec->rx_msg);
+
+rx_out:
+	/* Clear Interruption */
+	writel_relaxed(CEC_INTR_RX, ao_cec->base + CEC_INTR_CLR_REG);
+
+	/* Return if previous read errors */
+	if (ret)
+		return;
+
+	/* Ack RX message */
+	ret = meson_ao_cec_write(ao_cec, CEC_RX_MSG_CMD, RX_ACK_CURRENT);
+	if (ret)
+		return;
+	ret = meson_ao_cec_write(ao_cec, CEC_RX_MSG_CMD, RX_NO_OP);
+	if (ret)
+		return;
+
+	/* Clear RX buffer */
+	ret = meson_ao_cec_write(ao_cec, CEC_RX_CLEAR_BUF, CLEAR_START);
+	if (ret)
+		return;
+	ret = meson_ao_cec_write(ao_cec, CEC_RX_CLEAR_BUF, CLEAR_STOP);
+	if (ret)
+		return;
+}
+
+static irqreturn_t meson_ao_cec_irq_thread(int irq, void *data)
+{
+	struct meson_ao_cec_device *ao_cec = data;
+	u32 stat = readl_relaxed(ao_cec->base + CEC_INTR_STAT_REG);
+
+	if (stat & CEC_INTR_TX)
+		meson_ao_cec_irq_tx(ao_cec);
+
+	meson_ao_cec_irq_rx(ao_cec);
+
+	return IRQ_HANDLED;
+}
+
+static int meson_ao_cec_set_log_addr(struct cec_adapter *adap, u8 logical_addr)
+{
+	struct meson_ao_cec_device *ao_cec = adap->priv;
+	int ret;
+
+	ret = meson_ao_cec_write(ao_cec, CEC_LOGICAL_ADDR0,
+				 LOGICAL_ADDR_DISABLE);
+	if (ret)
+		return ret;
+
+	ret = meson_ao_cec_clear(ao_cec);
+	if (ret)
+		return ret;
+
+	if (logical_addr == CEC_LOG_ADDR_INVALID)
+		return 0;
+
+	ret = meson_ao_cec_write(ao_cec, CEC_LOGICAL_ADDR0,
+			   logical_addr & LOGICAL_ADDR_MASK);
+	if (ret)
+		return ret;
+
+	udelay(100);
+
+	ret = meson_ao_cec_write(ao_cec, CEC_LOGICAL_ADDR0,
+			   (logical_addr & LOGICAL_ADDR_MASK) |
+			   LOGICAL_ADDR_VALID);
+
+	return ret;
+}
+
+static int meson_ao_cec_transmit(struct cec_adapter *adap, u8 attempts,
+				 u32 signal_free_time, struct cec_msg *msg)
+{
+	struct meson_ao_cec_device *ao_cec = adap->priv;
+	int i, ret;
+	u8 reg;
+
+	ret = meson_ao_cec_read(ao_cec, CEC_TX_MSG_STATUS, &reg);
+	if (ret)
+		return ret;
+
+	if (reg == TX_BUSY) {
+		dev_err(&ao_cec->pdev->dev, "%s: busy TX\n", __func__);
+		ret = meson_ao_cec_write(ao_cec, CEC_TX_MSG_CMD, TX_ABORT);
+		if (ret)
+			return ret;
+	}
+
+	for (i = 0; i < msg->len; i++) {
+		ret = meson_ao_cec_write(ao_cec, CEC_TX_MSG_0_HEADER + i,
+					 msg->msg[i]);
+		if (ret)
+			return ret;
+	}
+
+	ret = meson_ao_cec_write(ao_cec, CEC_TX_MSG_LENGTH, msg->len - 1);
+	if (ret)
+		return ret;
+
+	ret = meson_ao_cec_write(ao_cec, CEC_TX_MSG_CMD, TX_REQ_CURRENT);
+
+	return ret;
+}
+
+static int meson_ao_cec_adap_enable(struct cec_adapter *adap, bool enable)
+{
+	struct meson_ao_cec_device *ao_cec = adap->priv;
+	int ret;
+
+	meson_ao_cec_irq_setup(ao_cec, false);
+
+	writel_bits_relaxed(CEC_GEN_CNTL_RESET, CEC_GEN_CNTL_RESET,
+			    ao_cec->base + CEC_GEN_CNTL_REG);
+
+	if (!enable)
+		return 0;
+
+	/* Enable gated clock (Normal mode). */
+	writel_bits_relaxed(CEC_GEN_CNTL_CLK_CTRL_MASK,
+			    FIELD_PREP(CEC_GEN_CNTL_CLK_CTRL_MASK,
+				       CEC_GEN_CNTL_CLK_ENABLE),
+			    ao_cec->base + CEC_GEN_CNTL_REG);
+
+	udelay(100);
+
+	/* Release Reset */
+	writel_bits_relaxed(CEC_GEN_CNTL_RESET, 0,
+			    ao_cec->base + CEC_GEN_CNTL_REG);
+
+	/* Clear buffers */
+	ret = meson_ao_cec_clear(ao_cec);
+	if (ret)
+		return ret;
+
+	/* CEC arbitration 3/5/7 bit time set. */
+	ret = meson_ao_cec_arbit_bit_time_set(ao_cec,
+					CEC_SIGNAL_FREE_TIME_RETRY,
+					0x118);
+	if (ret)
+		return ret;
+	ret = meson_ao_cec_arbit_bit_time_set(ao_cec,
+					CEC_SIGNAL_FREE_TIME_NEW_INITIATOR,
+					0x000);
+	if (ret)
+		return ret;
+	ret = meson_ao_cec_arbit_bit_time_set(ao_cec,
+					CEC_SIGNAL_FREE_TIME_NEXT_XFER,
+					0x2aa);
+	if (ret)
+		return ret;
+
+	meson_ao_cec_irq_setup(ao_cec, true);
+
+	return 0;
+}
+
+static const struct cec_adap_ops meson_ao_cec_ops = {
+	.adap_enable = meson_ao_cec_adap_enable,
+	.adap_log_addr = meson_ao_cec_set_log_addr,
+	.adap_transmit = meson_ao_cec_transmit,
+};
+
+static int meson_ao_cec_probe(struct platform_device *pdev)
+{
+	struct meson_ao_cec_device *ao_cec;
+	struct platform_device *hdmi_dev;
+	struct device_node *np;
+	struct resource *res;
+	int ret, irq;
+
+	np = of_parse_phandle(pdev->dev.of_node, "hdmi-phandle", 0);
+	if (!np) {
+		dev_err(&pdev->dev, "Failed to find hdmi node\n");
+		return -ENODEV;
+	}
+
+	hdmi_dev = of_find_device_by_node(np);
+	if (hdmi_dev == NULL)
+		return -EPROBE_DEFER;
+
+	ao_cec = devm_kzalloc(&pdev->dev, sizeof(*ao_cec), GFP_KERNEL);
+	if (!ao_cec)
+		return -ENOMEM;
+
+	spin_lock_init(&ao_cec->cec_reg_lock);
+
+	ao_cec->notify = cec_notifier_get(&hdmi_dev->dev);
+	if (!ao_cec->notify)
+		return -ENOMEM;
+
+	ao_cec->adap = cec_allocate_adapter(&meson_ao_cec_ops, ao_cec,
+					    "meson_ao_cec",
+					    CEC_CAP_LOG_ADDRS |
+					    CEC_CAP_TRANSMIT |
+					    CEC_CAP_RC |
+					    CEC_CAP_PASSTHROUGH,
+					    1); /* Use 1 for now */
+	if (IS_ERR(ao_cec->adap)) {
+		ret = PTR_ERR(ao_cec->adap);
+		goto out_probe_notify;
+	}
+
+	ao_cec->adap->owner = THIS_MODULE;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	ao_cec->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(ao_cec->base)) {
+		ret = PTR_ERR(ao_cec->base);
+		goto out_probe_adapter;
+	}
+
+	irq = platform_get_irq(pdev, 0);
+	ret = devm_request_threaded_irq(&pdev->dev, irq,
+					meson_ao_cec_irq,
+					meson_ao_cec_irq_thread,
+					0, NULL, ao_cec);
+	if (ret) {
+		dev_err(&pdev->dev, "irq request failed\n");
+		goto out_probe_adapter;
+	}
+
+	ao_cec->core = devm_clk_get(&pdev->dev, "core");
+	if (IS_ERR(ao_cec->core)) {
+		dev_err(&pdev->dev, "core clock request failed\n");
+		ret = PTR_ERR(ao_cec->core);
+		goto out_probe_adapter;
+	}
+
+	ret = clk_prepare_enable(ao_cec->core);
+	if (ret) {
+		dev_err(&pdev->dev, "core clock enable failed\n");
+		goto out_probe_adapter;
+	}
+
+	ret = clk_set_rate(ao_cec->core, CEC_CLK_RATE);
+	if (ret) {
+		dev_err(&pdev->dev, "core clock set rate failed\n");
+		goto out_probe_clk;
+	}
+
+	device_reset_optional(&pdev->dev);
+
+	ao_cec->pdev = pdev;
+	platform_set_drvdata(pdev, ao_cec);
+
+	ret = cec_register_adapter(ao_cec->adap, &pdev->dev);
+	if (ret < 0) {
+		cec_notifier_put(ao_cec->notify);
+		goto out_probe_clk;
+	}
+
+	/* Setup Hardware */
+	writel_relaxed(CEC_GEN_CNTL_RESET,
+		       ao_cec->base + CEC_GEN_CNTL_REG);
+
+	cec_register_cec_notifier(ao_cec->adap, ao_cec->notify);
+
+	return 0;
+
+out_probe_clk:
+	clk_disable_unprepare(ao_cec->core);
+
+out_probe_adapter:
+	cec_delete_adapter(ao_cec->adap);
+
+out_probe_notify:
+	cec_notifier_put(ao_cec->notify);
+
+	dev_err(&pdev->dev, "CEC controller registration failed\n");
+
+	return ret;
+}
+
+static int meson_ao_cec_remove(struct platform_device *pdev)
+{
+	struct meson_ao_cec_device *ao_cec = platform_get_drvdata(pdev);
+
+	clk_disable_unprepare(ao_cec->core);
+
+	cec_unregister_adapter(ao_cec->adap);
+
+	cec_notifier_put(ao_cec->notify);
+
+	return 0;
+}
+
+static const struct of_device_id meson_ao_cec_of_match[] = {
+	{ .compatible = "amlogic,meson-gx-ao-cec", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, meson_ao_cec_of_match);
+
+static struct platform_driver meson_ao_cec_driver = {
+	.probe   = meson_ao_cec_probe,
+	.remove  = meson_ao_cec_remove,
+	.driver  = {
+		.name = "meson-ao-cec",
+		.of_match_table = of_match_ptr(meson_ao_cec_of_match),
+	},
+};
+
+module_platform_driver(meson_ao_cec_driver);
+
+MODULE_DESCRIPTION("Meson AO CEC Controller driver");
+MODULE_AUTHOR("Neil Armstrong <narmstrong@baylibre.com>");
+MODULE_LICENSE("GPL");
-- 
1.9.1

[PATCH v2 2/2] dt-bindings: media: Add Amlogic Meson AO-CEC bindings

[Neil Armstrong]

The Amlogic SoCs embeds a standalone CEC Controller, this patch adds this
device bindings.

Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 .../devicetree/bindings/media/meson-ao-cec.txt     | 28 ++++++++++++++++++++++
 1 file changed, 28 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/meson-ao-cec.txt

diff --git a/Documentation/devicetree/bindings/media/meson-ao-cec.txt b/Documentation/devicetree/bindings/media/meson-ao-cec.txt
new file mode 100644
index 0000000..8671bdb
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/meson-ao-cec.txt
@@ -0,0 +1,28 @@
+* Amlogic Meson AO-CEC driver
+
+The Amlogic Meson AO-CEC module is present is Amlogic SoCs and its purpose is
+to handle communication between HDMI connected devices over the CEC bus.
+
+Required properties:
+  - compatible : value should be following
+	"amlogic,meson-gx-ao-cec"
+
+  - reg : Physical base address of the IP registers and length of memory
+	  mapped region.
+
+  - interrupts : AO-CEC interrupt number to the CPU.
+  - clocks : from common clock binding: handle to AO-CEC clock.
+  - clock-names : from common clock binding: must contain "core",
+		  corresponding to entry in the clocks property.
+  - hdmi-phandle: phandle to the HDMI controller
+
+Example:
+
+cec_AO: cec@100 {
+	compatible = "amlogic,meson-gx-ao-cec";
+	reg = <0x0 0x00100 0x0 0x14>;
+	interrupts = <GIC_SPI 199 IRQ_TYPE_EDGE_RISING>;
+	clocks = <&clkc_AO CLKID_AO_CEC_32K>;
+	clock-names = "core";
+	hdmi-phandle = <&hdmi_tx>;
+};
-- 
1.9.1

Re: [PATCH v2 1/2] platform: Add Amlogic Meson AO CEC Controller driver

[Hans Verkuil]

On 10/07/17 10:01, Neil Armstrong wrote:
> The Amlogic SoC embeds a standalone CEC controller, this patch adds a driver
> for such controller.
> The controller does not need HPD to be active, and could support up to max
> 5 logical addresses, but only 1 is handled since the Suspend firmware can
> make use of this unique logical address to wake up the device.
> 
> The Suspend firmware configuration will be added in an other patchset.
> 
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  drivers/media/platform/Kconfig        |  11 +
>  drivers/media/platform/Makefile       |   2 +
>  drivers/media/platform/meson/Makefile |   1 +
>  drivers/media/platform/meson/ao-cec.c | 781 ++++++++++++++++++++++++++++++++++
>  4 files changed, 795 insertions(+)
>  create mode 100644 drivers/media/platform/meson/Makefile
>  create mode 100644 drivers/media/platform/meson/ao-cec.c
> 
> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> index 1313cd5..1e67381 100644
> --- a/drivers/media/platform/Kconfig
> +++ b/drivers/media/platform/Kconfig
> @@ -536,6 +536,17 @@ menuconfig CEC_PLATFORM_DRIVERS
>  
>  if CEC_PLATFORM_DRIVERS
>  
> +config VIDEO_MESON_AO_CEC
> +	tristate "Amlogic Meson AO CEC driver"
> +	depends on ARCH_MESON || COMPILE_TEST
> +	select CEC_CORE
> +	select CEC_NOTIFIER
> +	---help---
> +	  This is a driver for Amlogic Meson SoCs AO CEC interface. It uses the
> +	  generic CEC framework interface.
> +	  CEC bus is present in the HDMI connector and enables communication
> +	  between compatible devices.
> +
>  config VIDEO_SAMSUNG_S5P_CEC
>         tristate "Samsung S5P CEC driver"
>         depends on PLAT_S5P || ARCH_EXYNOS || COMPILE_TEST
> diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
> index 9beadc7..a52d7b6 100644
> --- a/drivers/media/platform/Makefile
> +++ b/drivers/media/platform/Makefile
> @@ -86,3 +86,5 @@ obj-$(CONFIG_VIDEO_MEDIATEK_MDP)	+= mtk-mdp/
>  obj-$(CONFIG_VIDEO_MEDIATEK_JPEG)	+= mtk-jpeg/
>  
>  obj-$(CONFIG_VIDEO_QCOM_VENUS)		+= qcom/venus/
> +
> +obj-y					+= meson/
> diff --git a/drivers/media/platform/meson/Makefile b/drivers/media/platform/meson/Makefile
> new file mode 100644
> index 0000000..597beb8
> --- /dev/null
> +++ b/drivers/media/platform/meson/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_VIDEO_MESON_AO_CEC)	+= ao-cec.o
> diff --git a/drivers/media/platform/meson/ao-cec.c b/drivers/media/platform/meson/ao-cec.c
> new file mode 100644
> index 0000000..cdc1f61
> --- /dev/null
> +++ b/drivers/media/platform/meson/ao-cec.c
> @@ -0,0 +1,781 @@
> +/*
> + * Driver for Amlogic Meson AO CEC Controller
> + *
> + * Copyright (C) 2015 Amlogic, Inc. All rights reserved
> + * Copyright (C) 2017 BayLibre, SAS
> + * Author: Neil Armstrong <narmstrong@baylibre.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/clk.h>
> +#include <linux/device.h>
> +#include <linux/io.h>
> +#include <linux/delay.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/types.h>
> +#include <linux/interrupt.h>
> +#include <linux/reset.h>
> +#include <media/cec.h>
> +#include <media/cec-notifier.h>
> +
> +/* CEC Registers */
> +
> +/*
> + * [2:1] cntl_clk
> + *  - 0 = Disable clk (Power-off mode)
> + *  - 1 = Enable gated clock (Normal mode)
> + *  - 2 = Enable free-run clk (Debug mode)
> + */
> +#define CEC_GEN_CNTL_REG		0x00
> +
> +#define CEC_GEN_CNTL_RESET		BIT(0)
> +#define CEC_GEN_CNTL_CLK_DISABLE	0
> +#define CEC_GEN_CNTL_CLK_ENABLE		1
> +#define CEC_GEN_CNTL_CLK_ENABLE_DBG	2
> +#define CEC_GEN_CNTL_CLK_CTRL_MASK	GENMASK(2, 1)
> +
> +/*
> + * [7:0] cec_reg_addr
> + * [15:8] cec_reg_wrdata
> + * [16] cec_reg_wr
> + *  - 0 = Read
> + *  - 1 = Write
> + * [23] bus free
> + * [31:24] cec_reg_rddata
> + */
> +#define CEC_RW_REG			0x04
> +
> +#define CEC_RW_ADDR			GENMASK(7, 0)
> +#define CEC_RW_WR_DATA			GENMASK(15, 8)
> +#define CEC_RW_WRITE_EN			BIT(16)
> +#define CEC_RW_BUS_BUSY			BIT(23)
> +#define CEC_RW_RD_DATA			GENMASK(31, 24)
> +
> +/*
> + * [1] tx intr
> + * [2] rx intr
> + */
> +#define CEC_INTR_MASKN_REG		0x08
> +#define CEC_INTR_CLR_REG		0x0c
> +#define CEC_INTR_STAT_REG		0x10
> +
> +#define CEC_INTR_TX			BIT(1)
> +#define CEC_INTR_RX			BIT(2)
> +
> +/* CEC Commands */
> +
> +#define CEC_TX_MSG_0_HEADER		0x00
> +#define CEC_TX_MSG_1_OPCODE		0x01
> +#define CEC_TX_MSG_2_OP1		0x02
> +#define CEC_TX_MSG_3_OP2		0x03
> +#define CEC_TX_MSG_4_OP3		0x04
> +#define CEC_TX_MSG_5_OP4		0x05
> +#define CEC_TX_MSG_6_OP5		0x06
> +#define CEC_TX_MSG_7_OP6		0x07
> +#define CEC_TX_MSG_8_OP7		0x08
> +#define CEC_TX_MSG_9_OP8		0x09
> +#define CEC_TX_MSG_A_OP9		0x0A
> +#define CEC_TX_MSG_B_OP10		0x0B
> +#define CEC_TX_MSG_C_OP11		0x0C
> +#define CEC_TX_MSG_D_OP12		0x0D
> +#define CEC_TX_MSG_E_OP13		0x0E
> +#define CEC_TX_MSG_F_OP14		0x0F
> +#define CEC_TX_MSG_LENGTH		0x10
> +#define CEC_TX_MSG_CMD			0x11
> +#define CEC_TX_WRITE_BUF		0x12
> +#define CEC_TX_CLEAR_BUF		0x13
> +#define CEC_RX_MSG_CMD			0x14
> +#define CEC_RX_CLEAR_BUF		0x15
> +#define CEC_LOGICAL_ADDR0		0x16
> +#define CEC_LOGICAL_ADDR1		0x17
> +#define CEC_LOGICAL_ADDR2		0x18
> +#define CEC_LOGICAL_ADDR3		0x19
> +#define CEC_LOGICAL_ADDR4		0x1A
> +#define CEC_CLOCK_DIV_H			0x1B
> +#define CEC_CLOCK_DIV_L			0x1C
> +#define CEC_QUIESCENT_25MS_BIT7_0	0x20
> +#define CEC_QUIESCENT_25MS_BIT11_8	0x21
> +#define CEC_STARTBITMINL2H_3MS5_BIT7_0	0x22
> +#define CEC_STARTBITMINL2H_3MS5_BIT8	0x23
> +#define CEC_STARTBITMAXL2H_3MS9_BIT7_0	0x24
> +#define CEC_STARTBITMAXL2H_3MS9_BIT8	0x25
> +#define CEC_STARTBITMINH_0MS6_BIT7_0	0x26
> +#define CEC_STARTBITMINH_0MS6_BIT8	0x27
> +#define CEC_STARTBITMAXH_1MS0_BIT7_0	0x28
> +#define CEC_STARTBITMAXH_1MS0_BIT8	0x29
> +#define CEC_STARTBITMINTOT_4MS3_BIT7_0	0x2A
> +#define CEC_STARTBITMINTOT_4MS3_BIT9_8	0x2B
> +#define CEC_STARTBITMAXTOT_4MS7_BIT7_0	0x2C
> +#define CEC_STARTBITMAXTOT_4MS7_BIT9_8	0x2D
> +#define CEC_LOGIC1MINL2H_0MS4_BIT7_0	0x2E
> +#define CEC_LOGIC1MINL2H_0MS4_BIT8	0x2F
> +#define CEC_LOGIC1MAXL2H_0MS8_BIT7_0	0x30
> +#define CEC_LOGIC1MAXL2H_0MS8_BIT8	0x31
> +#define CEC_LOGIC0MINL2H_1MS3_BIT7_0	0x32
> +#define CEC_LOGIC0MINL2H_1MS3_BIT8	0x33
> +#define CEC_LOGIC0MAXL2H_1MS7_BIT7_0	0x34
> +#define CEC_LOGIC0MAXL2H_1MS7_BIT8	0x35
> +#define CEC_LOGICMINTOTAL_2MS05_BIT7_0	0x36
> +#define CEC_LOGICMINTOTAL_2MS05_BIT9_8	0x37
> +#define CEC_LOGICMAXHIGH_2MS8_BIT7_0	0x38
> +#define CEC_LOGICMAXHIGH_2MS8_BIT8	0x39
> +#define CEC_LOGICERRLOW_3MS4_BIT7_0	0x3A
> +#define CEC_LOGICERRLOW_3MS4_BIT8	0x3B
> +#define CEC_NOMSMPPOINT_1MS05		0x3C
> +#define CEC_DELCNTR_LOGICERR		0x3E
> +#define CEC_TXTIME_17MS_BIT7_0		0x40
> +#define CEC_TXTIME_17MS_BIT10_8		0x41
> +#define CEC_TXTIME_2BIT_BIT7_0		0x42
> +#define CEC_TXTIME_2BIT_BIT10_8		0x43
> +#define CEC_TXTIME_4BIT_BIT7_0		0x44
> +#define CEC_TXTIME_4BIT_BIT10_8		0x45
> +#define CEC_STARTBITNOML2H_3MS7_BIT7_0	0x46
> +#define CEC_STARTBITNOML2H_3MS7_BIT8	0x47
> +#define CEC_STARTBITNOMH_0MS8_BIT7_0	0x48
> +#define CEC_STARTBITNOMH_0MS8_BIT8	0x49
> +#define CEC_LOGIC1NOML2H_0MS6_BIT7_0	0x4A
> +#define CEC_LOGIC1NOML2H_0MS6_BIT8	0x4B
> +#define CEC_LOGIC0NOML2H_1MS5_BIT7_0	0x4C
> +#define CEC_LOGIC0NOML2H_1MS5_BIT8	0x4D
> +#define CEC_LOGIC1NOMH_1MS8_BIT7_0	0x4E
> +#define CEC_LOGIC1NOMH_1MS8_BIT8	0x4F
> +#define CEC_LOGIC0NOMH_0MS9_BIT7_0	0x50
> +#define CEC_LOGIC0NOMH_0MS9_BIT8	0x51
> +#define CEC_LOGICERRLOW_3MS6_BIT7_0	0x52
> +#define CEC_LOGICERRLOW_3MS6_BIT8	0x53
> +#define CEC_CHKCONTENTION_0MS1		0x54
> +#define CEC_PREPARENXTBIT_0MS05_BIT7_0	0x56
> +#define CEC_PREPARENXTBIT_0MS05_BIT8	0x57
> +#define CEC_NOMSMPACKPOINT_0MS45	0x58
> +#define CEC_ACK0NOML2H_1MS5_BIT7_0	0x5A
> +#define CEC_ACK0NOML2H_1MS5_BIT8	0x5B
> +#define CEC_BUGFIX_DISABLE_0		0x60
> +#define CEC_BUGFIX_DISABLE_1		0x61
> +#define CEC_RX_MSG_0_HEADER		0x80
> +#define CEC_RX_MSG_1_OPCODE		0x81
> +#define CEC_RX_MSG_2_OP1		0x82
> +#define CEC_RX_MSG_3_OP2		0x83
> +#define CEC_RX_MSG_4_OP3		0x84
> +#define CEC_RX_MSG_5_OP4		0x85
> +#define CEC_RX_MSG_6_OP5		0x86
> +#define CEC_RX_MSG_7_OP6		0x87
> +#define CEC_RX_MSG_8_OP7		0x88
> +#define CEC_RX_MSG_9_OP8		0x89
> +#define CEC_RX_MSG_A_OP9		0x8A
> +#define CEC_RX_MSG_B_OP10		0x8B
> +#define CEC_RX_MSG_C_OP11		0x8C
> +#define CEC_RX_MSG_D_OP12		0x8D
> +#define CEC_RX_MSG_E_OP13		0x8E
> +#define CEC_RX_MSG_F_OP14		0x8F
> +#define CEC_RX_MSG_LENGTH		0x90
> +#define CEC_RX_MSG_STATUS		0x91
> +#define CEC_RX_NUM_MSG			0x92
> +#define CEC_TX_MSG_STATUS		0x93
> +#define CEC_TX_NUM_MSG			0x94
> +
> +
> +/* CEC_TX_MSG_CMD definition */
> +#define TX_NO_OP	0  /* No transaction */
> +#define TX_REQ_CURRENT	1  /* Transmit earliest message in buffer */
> +#define TX_ABORT	2  /* Abort transmitting earliest message */
> +#define TX_REQ_NEXT	3  /* Overwrite earliest msg, transmit next */
> +
> +/* tx_msg_status definition */
> +#define TX_IDLE		0  /* No transaction */
> +#define TX_BUSY		1  /* Transmitter is busy */
> +#define TX_DONE		2  /* Message successfully transmitted */
> +#define TX_ERROR	3  /* Message transmitted with error */
> +
> +/* rx_msg_cmd */
> +#define RX_NO_OP	0  /* No transaction */
> +#define RX_ACK_CURRENT	1  /* Read earliest message in buffer */
> +#define RX_DISABLE	2  /* Disable receiving latest message */
> +#define RX_ACK_NEXT	3  /* Clear earliest msg, read next */
> +
> +/* rx_msg_status */
> +#define RX_IDLE		0  /* No transaction */
> +#define RX_BUSY		1  /* Receiver is busy */
> +#define RX_DONE		2  /* Message has been received successfully */
> +#define RX_ERROR	3  /* Message has been received with error */
> +
> +/* RX_CLEAR_BUF options */
> +#define CLEAR_START	1
> +#define CLEAR_STOP	0
> +
> +/* CEC_LOGICAL_ADDRx options */
> +#define LOGICAL_ADDR_MASK	0xf
> +#define LOGICAL_ADDR_VALID	BIT(4)
> +#define LOGICAL_ADDR_DISABLE	0
> +
> +#define CEC_CLK_RATE		32768
> +
> +struct meson_ao_cec_device {
> +	struct platform_device		*pdev;
> +	void __iomem			*base;
> +	struct clk			*core;
> +	spinlock_t			cec_reg_lock;
> +	struct cec_notifier		*notify;
> +	struct cec_adapter		*adap;
> +	struct cec_msg			rx_msg;
> +};
> +
> +#define writel_bits_relaxed(mask, val, addr) \
> +	writel_relaxed((readl_relaxed(addr) & ~(mask)) | (val), addr)
> +
> +static inline int meson_ao_cec_wait_busy(struct meson_ao_cec_device *ao_cec)
> +{
> +	ktime_t timeout = ktime_add_us(ktime_get(), 5000);
> +
> +	while (readl_relaxed(ao_cec->base + CEC_RW_REG) & CEC_RW_BUS_BUSY) {
> +		if (ktime_compare(ktime_get(), timeout) > 0)
> +			return -ETIMEDOUT;
> +	}
> +
> +	return 0;
> +}
> +
> +static int meson_ao_cec_read(struct meson_ao_cec_device *ao_cec,
> +			     unsigned long address, u8 *data)
> +{
> +	unsigned long flags;
> +	u32 reg = FIELD_PREP(CEC_RW_ADDR, address);
> +	int ret = 0;
> +
> +	spin_lock_irqsave(&ao_cec->cec_reg_lock, flags);
> +
> +	ret = meson_ao_cec_wait_busy(ao_cec);
> +	if (ret)
> +		goto read_out;
> +
> +	writel_relaxed(reg, ao_cec->base + CEC_RW_REG);
> +
> +	ret = meson_ao_cec_wait_busy(ao_cec);
> +	if (ret)
> +		goto read_out;
> +
> +	*data = FIELD_GET(CEC_RW_RD_DATA,
> +			  readl_relaxed(ao_cec->base + CEC_RW_REG));
> +
> +read_out:
> +	spin_unlock_irqrestore(&ao_cec->cec_reg_lock, flags);
> +
> +	return ret;
> +}
> +
> +static int meson_ao_cec_write(struct meson_ao_cec_device *ao_cec,
> +			      unsigned long address, u8 data)

I suggest a different prototype for the write functions:

static void meson_ao_cec_write(struct meson_ao_cec_device *ao_cec,
			      unsigned long address, u8 data, int *res);

and that the write function starts with:

	if (*res)
		return;

This makes it possible to replace awkward code like this:

	int ret;

	ret = meson_ao_cec_write(ao_cec, CEC_RX_MSG_CMD, RX_DISABLE);
	if (ret)
		return ret;
	ret = meson_ao_cec_write(ao_cec, CEC_TX_MSG_CMD, TX_ABORT);
	if (ret)
		return ret;

to this:

	int ret = 0;

	meson_ao_cec_write(ao_cec, CEC_RX_MSG_CMD, RX_DISABLE, &ret);
	meson_ao_cec_write(ao_cec, CEC_TX_MSG_CMD, TX_ABORT, &ret);
	...
	return ret;

The read() function isn't called that often, so I leave it up to you to decide
whether or not to change that function to something similar.

> +{
> +	unsigned long flags;
> +	u32 reg = FIELD_PREP(CEC_RW_ADDR, address) |
> +		  FIELD_PREP(CEC_RW_WR_DATA, data) |
> +		  CEC_RW_WRITE_EN;
> +	int ret = 0;
> +
> +	spin_lock_irqsave(&ao_cec->cec_reg_lock, flags);
> +
> +	ret = meson_ao_cec_wait_busy(ao_cec);
> +	if (ret)
> +		goto write_out;
> +
> +	writel_relaxed(reg, ao_cec->base + CEC_RW_REG);
> +
> +write_out:
> +	spin_unlock_irqrestore(&ao_cec->cec_reg_lock, flags);
> +
> +	return ret;
> +}
> +
> +static inline void meson_ao_cec_irq_setup(struct meson_ao_cec_device *ao_cec,
> +				      bool enable)
> +{
> +	u32 cfg = CEC_INTR_TX | CEC_INTR_RX;
> +
> +	writel_bits_relaxed(cfg, enable ? cfg : 0,
> +			    ao_cec->base + CEC_INTR_MASKN_REG);
> +}
> +
> +static inline int meson_ao_cec_clear(struct meson_ao_cec_device *ao_cec)
> +{
> +	int ret;
> +
> +	ret = meson_ao_cec_write(ao_cec, CEC_RX_MSG_CMD, RX_DISABLE);
> +	if (ret)
> +		return ret;
> +	ret = meson_ao_cec_write(ao_cec, CEC_TX_MSG_CMD, TX_ABORT);
> +	if (ret)
> +		return ret;
> +	ret = meson_ao_cec_write(ao_cec, CEC_RX_CLEAR_BUF, 1);
> +	if (ret)
> +		return ret;
> +	ret = meson_ao_cec_write(ao_cec, CEC_TX_CLEAR_BUF, 1);
> +	if (ret)
> +		return ret;
> +
> +	udelay(100);
> +
> +	ret = meson_ao_cec_write(ao_cec, CEC_RX_CLEAR_BUF, 0);
> +	if (ret)
> +		return ret;
> +	ret = meson_ao_cec_write(ao_cec, CEC_TX_CLEAR_BUF, 0);
> +	if (ret)
> +		return ret;
> +
> +	udelay(100);
> +
> +	ret = meson_ao_cec_write(ao_cec, CEC_RX_MSG_CMD, RX_NO_OP);
> +	if (ret)
> +		return ret;
> +	ret = meson_ao_cec_write(ao_cec, CEC_TX_MSG_CMD, TX_NO_OP);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static int meson_ao_cec_arbit_bit_time_set(struct meson_ao_cec_device *ao_cec,
> +					   unsigned int bit_set,
> +					   unsigned int time_set)
> +{
> +	int ret;
> +
> +	switch (bit_set) {
> +	case CEC_SIGNAL_FREE_TIME_RETRY:
> +		ret = meson_ao_cec_write(ao_cec, CEC_TXTIME_4BIT_BIT7_0,
> +				   time_set & 0xff);
> +		if (ret)
> +			return ret;
> +		ret = meson_ao_cec_write(ao_cec, CEC_TXTIME_4BIT_BIT10_8,
> +				   (time_set >> 8) & 0x7);
> +		if (ret)
> +			return ret;
> +		break;
> +
> +	case CEC_SIGNAL_FREE_TIME_NEW_INITIATOR:
> +		ret = meson_ao_cec_write(ao_cec, CEC_TXTIME_2BIT_BIT7_0,
> +				   time_set & 0xff);
> +		if (ret)
> +			return ret;
> +		ret = meson_ao_cec_write(ao_cec, CEC_TXTIME_2BIT_BIT10_8,
> +				   (time_set >> 8) & 0x7);
> +		if (ret)
> +			return ret;
> +		break;
> +
> +	case CEC_SIGNAL_FREE_TIME_NEXT_XFER:
> +		ret = meson_ao_cec_write(ao_cec, CEC_TXTIME_17MS_BIT7_0,
> +				   time_set & 0xff);
> +		if (ret)
> +			return ret;
> +		ret = meson_ao_cec_write(ao_cec, CEC_TXTIME_17MS_BIT10_8,
> +				   (time_set >> 8) & 0x7);
> +		if (ret)
> +			return ret;
> +		break;
> +	}
> +
> +	return 0;
> +}
> +
> +static irqreturn_t meson_ao_cec_irq(int irq, void *data)
> +{
> +	struct meson_ao_cec_device *ao_cec = data;
> +	u32 stat = readl_relaxed(ao_cec->base + CEC_INTR_STAT_REG);
> +
> +	if (stat)
> +		return IRQ_WAKE_THREAD;
> +
> +	return IRQ_NONE;
> +}
> +
> +static void meson_ao_cec_irq_tx(struct meson_ao_cec_device *ao_cec)
> +{
> +	unsigned long tx_status = 0;
> +	u8 stat;
> +	int ret;
> +
> +	ret = meson_ao_cec_read(ao_cec, CEC_TX_MSG_STATUS, &stat);
> +	if (ret)
> +		goto tx_reg_err;
> +
> +	switch (stat) {
> +	case TX_DONE:
> +		tx_status = CEC_TX_STATUS_OK;
> +		break;
> +
> +	case TX_BUSY:
> +		tx_status = CEC_TX_STATUS_ARB_LOST;
> +		break;
> +
> +	case TX_IDLE:
> +		tx_status = CEC_TX_STATUS_LOW_DRIVE;
> +		break;
> +
> +	case TX_ERROR:
> +	default:
> +		tx_status = CEC_TX_STATUS_NACK;
> +		break;
> +	}
> +
> +	/* Clear Interruption */
> +	writel_relaxed(CEC_INTR_TX, ao_cec->base + CEC_INTR_CLR_REG);
> +
> +	/* Stop TX */
> +	ret = meson_ao_cec_write(ao_cec, CEC_TX_MSG_CMD, TX_NO_OP);
> +	if (ret)
> +		goto tx_reg_err;
> +
> +	cec_transmit_attempt_done(ao_cec->adap, tx_status);
> +	return;
> +
> +tx_reg_err:
> +	cec_transmit_attempt_done(ao_cec->adap, CEC_TX_STATUS_ERROR);
> +}
> +
> +static void meson_ao_cec_irq_rx(struct meson_ao_cec_device *ao_cec)
> +{
> +	int i, ret;
> +	u8 reg;
> +
> +	ret = meson_ao_cec_read(ao_cec, CEC_RX_MSG_STATUS, &reg);
> +	if (ret)
> +		goto rx_out;
> +
> +	/* RX Error */
> +	if (reg != RX_DONE)
> +		goto rx_out;
> +
> +	ret = meson_ao_cec_read(ao_cec, CEC_RX_NUM_MSG, &reg);
> +	if (ret || reg != 1)
> +		goto rx_out;
> +
> +	ret = meson_ao_cec_read(ao_cec, CEC_RX_MSG_LENGTH, &reg);
> +	if (ret)
> +		goto rx_out;
> +
> +	ao_cec->rx_msg.len = reg + 1;
> +	if (ao_cec->rx_msg.len > CEC_MAX_MSG_SIZE)
> +		ao_cec->rx_msg.len = CEC_MAX_MSG_SIZE;
> +
> +	for (i = 0; i < ao_cec->rx_msg.len; i++) {
> +		u8 byte;
> +
> +		ret = meson_ao_cec_read(ao_cec, CEC_RX_MSG_0_HEADER + i, &byte);
> +		if (ret)
> +			goto rx_out;
> +
> +		ao_cec->rx_msg.msg[i] = byte;
> +	}
> +
> +	cec_received_msg(ao_cec->adap, &ao_cec->rx_msg);
> +
> +rx_out:
> +	/* Clear Interruption */
> +	writel_relaxed(CEC_INTR_RX, ao_cec->base + CEC_INTR_CLR_REG);
> +
> +	/* Return if previous read errors */
> +	if (ret)
> +		return;
> +
> +	/* Ack RX message */
> +	ret = meson_ao_cec_write(ao_cec, CEC_RX_MSG_CMD, RX_ACK_CURRENT);
> +	if (ret)
> +		return;
> +	ret = meson_ao_cec_write(ao_cec, CEC_RX_MSG_CMD, RX_NO_OP);
> +	if (ret)
> +		return;
> +
> +	/* Clear RX buffer */
> +	ret = meson_ao_cec_write(ao_cec, CEC_RX_CLEAR_BUF, CLEAR_START);
> +	if (ret)
> +		return;
> +	ret = meson_ao_cec_write(ao_cec, CEC_RX_CLEAR_BUF, CLEAR_STOP);
> +	if (ret)
> +		return;
> +}
> +
> +static irqreturn_t meson_ao_cec_irq_thread(int irq, void *data)
> +{
> +	struct meson_ao_cec_device *ao_cec = data;
> +	u32 stat = readl_relaxed(ao_cec->base + CEC_INTR_STAT_REG);
> +
> +	if (stat & CEC_INTR_TX)
> +		meson_ao_cec_irq_tx(ao_cec);
> +
> +	meson_ao_cec_irq_rx(ao_cec);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int meson_ao_cec_set_log_addr(struct cec_adapter *adap, u8 logical_addr)
> +{
> +	struct meson_ao_cec_device *ao_cec = adap->priv;
> +	int ret;
> +
> +	ret = meson_ao_cec_write(ao_cec, CEC_LOGICAL_ADDR0,
> +				 LOGICAL_ADDR_DISABLE);
> +	if (ret)
> +		return ret;
> +
> +	ret = meson_ao_cec_clear(ao_cec);
> +	if (ret)
> +		return ret;
> +
> +	if (logical_addr == CEC_LOG_ADDR_INVALID)
> +		return 0;
> +
> +	ret = meson_ao_cec_write(ao_cec, CEC_LOGICAL_ADDR0,
> +			   logical_addr & LOGICAL_ADDR_MASK);
> +	if (ret)
> +		return ret;
> +
> +	udelay(100);
> +
> +	ret = meson_ao_cec_write(ao_cec, CEC_LOGICAL_ADDR0,
> +			   (logical_addr & LOGICAL_ADDR_MASK) |
> +			   LOGICAL_ADDR_VALID);
> +
> +	return ret;
> +}
> +
> +static int meson_ao_cec_transmit(struct cec_adapter *adap, u8 attempts,
> +				 u32 signal_free_time, struct cec_msg *msg)
> +{
> +	struct meson_ao_cec_device *ao_cec = adap->priv;
> +	int i, ret;
> +	u8 reg;
> +
> +	ret = meson_ao_cec_read(ao_cec, CEC_TX_MSG_STATUS, &reg);
> +	if (ret)
> +		return ret;
> +
> +	if (reg == TX_BUSY) {
> +		dev_err(&ao_cec->pdev->dev, "%s: busy TX\n", __func__);

s/busy TX/busy TX: aborting/

to make it clear that this TX is aborted.

> +		ret = meson_ao_cec_write(ao_cec, CEC_TX_MSG_CMD, TX_ABORT);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	for (i = 0; i < msg->len; i++) {
> +		ret = meson_ao_cec_write(ao_cec, CEC_TX_MSG_0_HEADER + i,
> +					 msg->msg[i]);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	ret = meson_ao_cec_write(ao_cec, CEC_TX_MSG_LENGTH, msg->len - 1);
> +	if (ret)
> +		return ret;
> +
> +	ret = meson_ao_cec_write(ao_cec, CEC_TX_MSG_CMD, TX_REQ_CURRENT);
> +
> +	return ret;
> +}
> +
> +static int meson_ao_cec_adap_enable(struct cec_adapter *adap, bool enable)
> +{
> +	struct meson_ao_cec_device *ao_cec = adap->priv;
> +	int ret;
> +
> +	meson_ao_cec_irq_setup(ao_cec, false);
> +
> +	writel_bits_relaxed(CEC_GEN_CNTL_RESET, CEC_GEN_CNTL_RESET,
> +			    ao_cec->base + CEC_GEN_CNTL_REG);
> +
> +	if (!enable)
> +		return 0;
> +
> +	/* Enable gated clock (Normal mode). */
> +	writel_bits_relaxed(CEC_GEN_CNTL_CLK_CTRL_MASK,
> +			    FIELD_PREP(CEC_GEN_CNTL_CLK_CTRL_MASK,
> +				       CEC_GEN_CNTL_CLK_ENABLE),
> +			    ao_cec->base + CEC_GEN_CNTL_REG);
> +
> +	udelay(100);
> +
> +	/* Release Reset */
> +	writel_bits_relaxed(CEC_GEN_CNTL_RESET, 0,
> +			    ao_cec->base + CEC_GEN_CNTL_REG);
> +
> +	/* Clear buffers */
> +	ret = meson_ao_cec_clear(ao_cec);
> +	if (ret)
> +		return ret;
> +
> +	/* CEC arbitration 3/5/7 bit time set. */
> +	ret = meson_ao_cec_arbit_bit_time_set(ao_cec,
> +					CEC_SIGNAL_FREE_TIME_RETRY,
> +					0x118);
> +	if (ret)
> +		return ret;
> +	ret = meson_ao_cec_arbit_bit_time_set(ao_cec,
> +					CEC_SIGNAL_FREE_TIME_NEW_INITIATOR,
> +					0x000);
> +	if (ret)
> +		return ret;
> +	ret = meson_ao_cec_arbit_bit_time_set(ao_cec,
> +					CEC_SIGNAL_FREE_TIME_NEXT_XFER,
> +					0x2aa);
> +	if (ret)
> +		return ret;
> +
> +	meson_ao_cec_irq_setup(ao_cec, true);
> +
> +	return 0;
> +}
> +
> +static const struct cec_adap_ops meson_ao_cec_ops = {
> +	.adap_enable = meson_ao_cec_adap_enable,
> +	.adap_log_addr = meson_ao_cec_set_log_addr,
> +	.adap_transmit = meson_ao_cec_transmit,
> +};
> +
> +static int meson_ao_cec_probe(struct platform_device *pdev)
> +{
> +	struct meson_ao_cec_device *ao_cec;
> +	struct platform_device *hdmi_dev;
> +	struct device_node *np;
> +	struct resource *res;
> +	int ret, irq;
> +
> +	np = of_parse_phandle(pdev->dev.of_node, "hdmi-phandle", 0);
> +	if (!np) {
> +		dev_err(&pdev->dev, "Failed to find hdmi node\n");
> +		return -ENODEV;
> +	}
> +
> +	hdmi_dev = of_find_device_by_node(np);
> +	if (hdmi_dev == NULL)
> +		return -EPROBE_DEFER;
> +
> +	ao_cec = devm_kzalloc(&pdev->dev, sizeof(*ao_cec), GFP_KERNEL);
> +	if (!ao_cec)
> +		return -ENOMEM;
> +
> +	spin_lock_init(&ao_cec->cec_reg_lock);
> +
> +	ao_cec->notify = cec_notifier_get(&hdmi_dev->dev);
> +	if (!ao_cec->notify)
> +		return -ENOMEM;
> +
> +	ao_cec->adap = cec_allocate_adapter(&meson_ao_cec_ops, ao_cec,
> +					    "meson_ao_cec",
> +					    CEC_CAP_LOG_ADDRS |
> +					    CEC_CAP_TRANSMIT |
> +					    CEC_CAP_RC |
> +					    CEC_CAP_PASSTHROUGH,
> +					    1); /* Use 1 for now */

I recommend that you add support for 2 logical addresses. More isn't allowed
by the CEC 2.0 spec anyway (no such restriction for CEC 1.4, but more than
two really isn't needed).

> +	if (IS_ERR(ao_cec->adap)) {
> +		ret = PTR_ERR(ao_cec->adap);
> +		goto out_probe_notify;
> +	}
> +
> +	ao_cec->adap->owner = THIS_MODULE;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	ao_cec->base = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(ao_cec->base)) {
> +		ret = PTR_ERR(ao_cec->base);
> +		goto out_probe_adapter;
> +	}
> +
> +	irq = platform_get_irq(pdev, 0);
> +	ret = devm_request_threaded_irq(&pdev->dev, irq,
> +					meson_ao_cec_irq,
> +					meson_ao_cec_irq_thread,
> +					0, NULL, ao_cec);
> +	if (ret) {
> +		dev_err(&pdev->dev, "irq request failed\n");
> +		goto out_probe_adapter;
> +	}
> +
> +	ao_cec->core = devm_clk_get(&pdev->dev, "core");
> +	if (IS_ERR(ao_cec->core)) {
> +		dev_err(&pdev->dev, "core clock request failed\n");
> +		ret = PTR_ERR(ao_cec->core);
> +		goto out_probe_adapter;
> +	}
> +
> +	ret = clk_prepare_enable(ao_cec->core);
> +	if (ret) {
> +		dev_err(&pdev->dev, "core clock enable failed\n");
> +		goto out_probe_adapter;
> +	}
> +
> +	ret = clk_set_rate(ao_cec->core, CEC_CLK_RATE);
> +	if (ret) {
> +		dev_err(&pdev->dev, "core clock set rate failed\n");
> +		goto out_probe_clk;
> +	}
> +
> +	device_reset_optional(&pdev->dev);
> +
> +	ao_cec->pdev = pdev;
> +	platform_set_drvdata(pdev, ao_cec);
> +
> +	ret = cec_register_adapter(ao_cec->adap, &pdev->dev);
> +	if (ret < 0) {
> +		cec_notifier_put(ao_cec->notify);
> +		goto out_probe_clk;
> +	}
> +
> +	/* Setup Hardware */
> +	writel_relaxed(CEC_GEN_CNTL_RESET,
> +		       ao_cec->base + CEC_GEN_CNTL_REG);
> +
> +	cec_register_cec_notifier(ao_cec->adap, ao_cec->notify);
> +
> +	return 0;
> +
> +out_probe_clk:
> +	clk_disable_unprepare(ao_cec->core);
> +
> +out_probe_adapter:
> +	cec_delete_adapter(ao_cec->adap);
> +
> +out_probe_notify:
> +	cec_notifier_put(ao_cec->notify);
> +
> +	dev_err(&pdev->dev, "CEC controller registration failed\n");
> +
> +	return ret;
> +}
> +
> +static int meson_ao_cec_remove(struct platform_device *pdev)
> +{
> +	struct meson_ao_cec_device *ao_cec = platform_get_drvdata(pdev);
> +
> +	clk_disable_unprepare(ao_cec->core);
> +
> +	cec_unregister_adapter(ao_cec->adap);
> +
> +	cec_notifier_put(ao_cec->notify);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id meson_ao_cec_of_match[] = {
> +	{ .compatible = "amlogic,meson-gx-ao-cec", },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, meson_ao_cec_of_match);
> +
> +static struct platform_driver meson_ao_cec_driver = {
> +	.probe   = meson_ao_cec_probe,
> +	.remove  = meson_ao_cec_remove,
> +	.driver  = {
> +		.name = "meson-ao-cec",
> +		.of_match_table = of_match_ptr(meson_ao_cec_of_match),
> +	},
> +};
> +
> +module_platform_driver(meson_ao_cec_driver);
> +
> +MODULE_DESCRIPTION("Meson AO CEC Controller driver");
> +MODULE_AUTHOR("Neil Armstrong <narmstrong@baylibre.com>");
> +MODULE_LICENSE("GPL");
> 

Regards,

	Hans

Re: [PATCH v2 1/2] platform: Add Amlogic Meson AO CEC Controller driver

[Neil Armstrong]

Hi Hans,

On 07/17/2017 10:01 AM, Hans Verkuil wrote:
> On 10/07/17 10:01, Neil Armstrong wrote:
>> The Amlogic SoC embeds a standalone CEC controller, this patch adds a driver
>> for such controller.
>> The controller does not need HPD to be active, and could support up to max
>> 5 logical addresses, but only 1 is handled since the Suspend firmware can
>> make use of this unique logical address to wake up the device.
>>
>> The Suspend firmware configuration will be added in an other patchset.
>>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>>  drivers/media/platform/Kconfig        |  11 +
>>  drivers/media/platform/Makefile       |   2 +
>>  drivers/media/platform/meson/Makefile |   1 +
>>  drivers/media/platform/meson/ao-cec.c | 781 ++++++++++++++++++++++++++++++++++
>>  4 files changed, 795 insertions(+)
>>  create mode 100644 drivers/media/platform/meson/Makefile
>>  create mode 100644 drivers/media/platform/meson/ao-cec.c
>>
>> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
>> index 1313cd5..1e67381 100644
>> --- a/drivers/media/platform/Kconfig
>> +++ b/drivers/media/platform/Kconfig
>> @@ -536,6 +536,17 @@ menuconfig CEC_PLATFORM_DRIVERS
>>  
>>  if CEC_PLATFORM_DRIVERS
>>  
>> +config VIDEO_MESON_AO_CEC
>> +	tristate "Amlogic Meson AO CEC driver"
>> +	depends on ARCH_MESON || COMPILE_TEST
>> +	select CEC_CORE
>> +	select CEC_NOTIFIER
>> +	---help---
>> +	  This is a driver for Amlogic Meson SoCs AO CEC interface. It uses the
>> +	  generic CEC framework interface.
>> +	  CEC bus is present in the HDMI connector and enables communication
>> +	  between compatible devices.
>> +
>>  config VIDEO_SAMSUNG_S5P_CEC
>>         tristate "Samsung S5P CEC driver"
>>         depends on PLAT_S5P || ARCH_EXYNOS || COMPILE_TEST
>> diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
>> index 9beadc7..a52d7b6 100644
>> --- a/drivers/media/platform/Makefile
>> +++ b/drivers/media/platform/Makefile
>> @@ -86,3 +86,5 @@ obj-$(CONFIG_VIDEO_MEDIATEK_MDP)	+= mtk-mdp/
>>  obj-$(CONFIG_VIDEO_MEDIATEK_JPEG)	+= mtk-jpeg/
>>  
>>  obj-$(CONFIG_VIDEO_QCOM_VENUS)		+= qcom/venus/
>> +
>> +obj-y					+= meson/
>> diff --git a/drivers/media/platform/meson/Makefile b/drivers/media/platform/meson/Makefile
>> new file mode 100644
>> index 0000000..597beb8
>> --- /dev/null
>> +++ b/drivers/media/platform/meson/Makefile
>> @@ -0,0 +1 @@
>> +obj-$(CONFIG_VIDEO_MESON_AO_CEC)	+= ao-cec.o
>> diff --git a/drivers/media/platform/meson/ao-cec.c b/drivers/media/platform/meson/ao-cec.c
>> new file mode 100644
>> index 0000000..cdc1f61
>> --- /dev/null
>> +++ b/drivers/media/platform/meson/ao-cec.c
>> @@ -0,0 +1,781 @@
>> +/*
>> + * Driver for Amlogic Meson AO CEC Controller
>> + *
>> + * Copyright (C) 2015 Amlogic, Inc. All rights reserved
>> + * Copyright (C) 2017 BayLibre, SAS
>> + * Author: Neil Armstrong <narmstrong@baylibre.com>
>> + *
>> + * SPDX-License-Identifier: GPL-2.0+
>> + */
>> +
>> +#include <linux/bitfield.h>
>> +#include <linux/clk.h>
>> +#include <linux/device.h>
>> +#include <linux/io.h>
>> +#include <linux/delay.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/types.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/reset.h>
>> +#include <media/cec.h>
>> +#include <media/cec-notifier.h>
>> +
>> +/* CEC Registers */
>> +
>> +/*
>> + * [2:1] cntl_clk
>> + *  - 0 = Disable clk (Power-off mode)
>> + *  - 1 = Enable gated clock (Normal mode)
>> + *  - 2 = Enable free-run clk (Debug mode)
>> + */
>> +#define CEC_GEN_CNTL_REG		0x00
>> +
>> +#define CEC_GEN_CNTL_RESET		BIT(0)
>> +#define CEC_GEN_CNTL_CLK_DISABLE	0
>> +#define CEC_GEN_CNTL_CLK_ENABLE		1
>> +#define CEC_GEN_CNTL_CLK_ENABLE_DBG	2
>> +#define CEC_GEN_CNTL_CLK_CTRL_MASK	GENMASK(2, 1)
>> +
>> +/*
>> + * [7:0] cec_reg_addr
>> + * [15:8] cec_reg_wrdata
>> + * [16] cec_reg_wr
>> + *  - 0 = Read
>> + *  - 1 = Write
>> + * [23] bus free
>> + * [31:24] cec_reg_rddata
>> + */
>> +#define CEC_RW_REG			0x04
>> +
>> +#define CEC_RW_ADDR			GENMASK(7, 0)
>> +#define CEC_RW_WR_DATA			GENMASK(15, 8)
>> +#define CEC_RW_WRITE_EN			BIT(16)
>> +#define CEC_RW_BUS_BUSY			BIT(23)
>> +#define CEC_RW_RD_DATA			GENMASK(31, 24)
>> +
>> +/*
>> + * [1] tx intr
>> + * [2] rx intr
>> + */
>> +#define CEC_INTR_MASKN_REG		0x08
>> +#define CEC_INTR_CLR_REG		0x0c
>> +#define CEC_INTR_STAT_REG		0x10
>> +
>> +#define CEC_INTR_TX			BIT(1)
>> +#define CEC_INTR_RX			BIT(2)
>> +
>> +/* CEC Commands */
>> +
>> +#define CEC_TX_MSG_0_HEADER		0x00
>> +#define CEC_TX_MSG_1_OPCODE		0x01
>> +#define CEC_TX_MSG_2_OP1		0x02
>> +#define CEC_TX_MSG_3_OP2		0x03
>> +#define CEC_TX_MSG_4_OP3		0x04
>> +#define CEC_TX_MSG_5_OP4		0x05
>> +#define CEC_TX_MSG_6_OP5		0x06
>> +#define CEC_TX_MSG_7_OP6		0x07
>> +#define CEC_TX_MSG_8_OP7		0x08
>> +#define CEC_TX_MSG_9_OP8		0x09
>> +#define CEC_TX_MSG_A_OP9		0x0A
>> +#define CEC_TX_MSG_B_OP10		0x0B
>> +#define CEC_TX_MSG_C_OP11		0x0C
>> +#define CEC_TX_MSG_D_OP12		0x0D
>> +#define CEC_TX_MSG_E_OP13		0x0E
>> +#define CEC_TX_MSG_F_OP14		0x0F
>> +#define CEC_TX_MSG_LENGTH		0x10
>> +#define CEC_TX_MSG_CMD			0x11
>> +#define CEC_TX_WRITE_BUF		0x12
>> +#define CEC_TX_CLEAR_BUF		0x13
>> +#define CEC_RX_MSG_CMD			0x14
>> +#define CEC_RX_CLEAR_BUF		0x15
>> +#define CEC_LOGICAL_ADDR0		0x16
>> +#define CEC_LOGICAL_ADDR1		0x17
>> +#define CEC_LOGICAL_ADDR2		0x18
>> +#define CEC_LOGICAL_ADDR3		0x19
>> +#define CEC_LOGICAL_ADDR4		0x1A
>> +#define CEC_CLOCK_DIV_H			0x1B
>> +#define CEC_CLOCK_DIV_L			0x1C
>> +#define CEC_QUIESCENT_25MS_BIT7_0	0x20
>> +#define CEC_QUIESCENT_25MS_BIT11_8	0x21
>> +#define CEC_STARTBITMINL2H_3MS5_BIT7_0	0x22
>> +#define CEC_STARTBITMINL2H_3MS5_BIT8	0x23
>> +#define CEC_STARTBITMAXL2H_3MS9_BIT7_0	0x24
>> +#define CEC_STARTBITMAXL2H_3MS9_BIT8	0x25
>> +#define CEC_STARTBITMINH_0MS6_BIT7_0	0x26
>> +#define CEC_STARTBITMINH_0MS6_BIT8	0x27
>> +#define CEC_STARTBITMAXH_1MS0_BIT7_0	0x28
>> +#define CEC_STARTBITMAXH_1MS0_BIT8	0x29
>> +#define CEC_STARTBITMINTOT_4MS3_BIT7_0	0x2A
>> +#define CEC_STARTBITMINTOT_4MS3_BIT9_8	0x2B
>> +#define CEC_STARTBITMAXTOT_4MS7_BIT7_0	0x2C
>> +#define CEC_STARTBITMAXTOT_4MS7_BIT9_8	0x2D
>> +#define CEC_LOGIC1MINL2H_0MS4_BIT7_0	0x2E
>> +#define CEC_LOGIC1MINL2H_0MS4_BIT8	0x2F
>> +#define CEC_LOGIC1MAXL2H_0MS8_BIT7_0	0x30
>> +#define CEC_LOGIC1MAXL2H_0MS8_BIT8	0x31
>> +#define CEC_LOGIC0MINL2H_1MS3_BIT7_0	0x32
>> +#define CEC_LOGIC0MINL2H_1MS3_BIT8	0x33
>> +#define CEC_LOGIC0MAXL2H_1MS7_BIT7_0	0x34
>> +#define CEC_LOGIC0MAXL2H_1MS7_BIT8	0x35
>> +#define CEC_LOGICMINTOTAL_2MS05_BIT7_0	0x36
>> +#define CEC_LOGICMINTOTAL_2MS05_BIT9_8	0x37
>> +#define CEC_LOGICMAXHIGH_2MS8_BIT7_0	0x38
>> +#define CEC_LOGICMAXHIGH_2MS8_BIT8	0x39
>> +#define CEC_LOGICERRLOW_3MS4_BIT7_0	0x3A
>> +#define CEC_LOGICERRLOW_3MS4_BIT8	0x3B
>> +#define CEC_NOMSMPPOINT_1MS05		0x3C
>> +#define CEC_DELCNTR_LOGICERR		0x3E
>> +#define CEC_TXTIME_17MS_BIT7_0		0x40
>> +#define CEC_TXTIME_17MS_BIT10_8		0x41
>> +#define CEC_TXTIME_2BIT_BIT7_0		0x42
>> +#define CEC_TXTIME_2BIT_BIT10_8		0x43
>> +#define CEC_TXTIME_4BIT_BIT7_0		0x44
>> +#define CEC_TXTIME_4BIT_BIT10_8		0x45
>> +#define CEC_STARTBITNOML2H_3MS7_BIT7_0	0x46
>> +#define CEC_STARTBITNOML2H_3MS7_BIT8	0x47
>> +#define CEC_STARTBITNOMH_0MS8_BIT7_0	0x48
>> +#define CEC_STARTBITNOMH_0MS8_BIT8	0x49
>> +#define CEC_LOGIC1NOML2H_0MS6_BIT7_0	0x4A
>> +#define CEC_LOGIC1NOML2H_0MS6_BIT8	0x4B
>> +#define CEC_LOGIC0NOML2H_1MS5_BIT7_0	0x4C
>> +#define CEC_LOGIC0NOML2H_1MS5_BIT8	0x4D
>> +#define CEC_LOGIC1NOMH_1MS8_BIT7_0	0x4E
>> +#define CEC_LOGIC1NOMH_1MS8_BIT8	0x4F
>> +#define CEC_LOGIC0NOMH_0MS9_BIT7_0	0x50
>> +#define CEC_LOGIC0NOMH_0MS9_BIT8	0x51
>> +#define CEC_LOGICERRLOW_3MS6_BIT7_0	0x52
>> +#define CEC_LOGICERRLOW_3MS6_BIT8	0x53
>> +#define CEC_CHKCONTENTION_0MS1		0x54
>> +#define CEC_PREPARENXTBIT_0MS05_BIT7_0	0x56
>> +#define CEC_PREPARENXTBIT_0MS05_BIT8	0x57
>> +#define CEC_NOMSMPACKPOINT_0MS45	0x58
>> +#define CEC_ACK0NOML2H_1MS5_BIT7_0	0x5A
>> +#define CEC_ACK0NOML2H_1MS5_BIT8	0x5B
>> +#define CEC_BUGFIX_DISABLE_0		0x60
>> +#define CEC_BUGFIX_DISABLE_1		0x61
>> +#define CEC_RX_MSG_0_HEADER		0x80
>> +#define CEC_RX_MSG_1_OPCODE		0x81
>> +#define CEC_RX_MSG_2_OP1		0x82
>> +#define CEC_RX_MSG_3_OP2		0x83
>> +#define CEC_RX_MSG_4_OP3		0x84
>> +#define CEC_RX_MSG_5_OP4		0x85
>> +#define CEC_RX_MSG_6_OP5		0x86
>> +#define CEC_RX_MSG_7_OP6		0x87
>> +#define CEC_RX_MSG_8_OP7		0x88
>> +#define CEC_RX_MSG_9_OP8		0x89
>> +#define CEC_RX_MSG_A_OP9		0x8A
>> +#define CEC_RX_MSG_B_OP10		0x8B
>> +#define CEC_RX_MSG_C_OP11		0x8C
>> +#define CEC_RX_MSG_D_OP12		0x8D
>> +#define CEC_RX_MSG_E_OP13		0x8E
>> +#define CEC_RX_MSG_F_OP14		0x8F
>> +#define CEC_RX_MSG_LENGTH		0x90
>> +#define CEC_RX_MSG_STATUS		0x91
>> +#define CEC_RX_NUM_MSG			0x92
>> +#define CEC_TX_MSG_STATUS		0x93
>> +#define CEC_TX_NUM_MSG			0x94
>> +
>> +
>> +/* CEC_TX_MSG_CMD definition */
>> +#define TX_NO_OP	0  /* No transaction */
>> +#define TX_REQ_CURRENT	1  /* Transmit earliest message in buffer */
>> +#define TX_ABORT	2  /* Abort transmitting earliest message */
>> +#define TX_REQ_NEXT	3  /* Overwrite earliest msg, transmit next */
>> +
>> +/* tx_msg_status definition */
>> +#define TX_IDLE		0  /* No transaction */
>> +#define TX_BUSY		1  /* Transmitter is busy */
>> +#define TX_DONE		2  /* Message successfully transmitted */
>> +#define TX_ERROR	3  /* Message transmitted with error */
>> +
>> +/* rx_msg_cmd */
>> +#define RX_NO_OP	0  /* No transaction */
>> +#define RX_ACK_CURRENT	1  /* Read earliest message in buffer */
>> +#define RX_DISABLE	2  /* Disable receiving latest message */
>> +#define RX_ACK_NEXT	3  /* Clear earliest msg, read next */
>> +
>> +/* rx_msg_status */
>> +#define RX_IDLE		0  /* No transaction */
>> +#define RX_BUSY		1  /* Receiver is busy */
>> +#define RX_DONE		2  /* Message has been received successfully */
>> +#define RX_ERROR	3  /* Message has been received with error */
>> +
>> +/* RX_CLEAR_BUF options */
>> +#define CLEAR_START	1
>> +#define CLEAR_STOP	0
>> +
>> +/* CEC_LOGICAL_ADDRx options */
>> +#define LOGICAL_ADDR_MASK	0xf
>> +#define LOGICAL_ADDR_VALID	BIT(4)
>> +#define LOGICAL_ADDR_DISABLE	0
>> +
>> +#define CEC_CLK_RATE		32768
>> +
>> +struct meson_ao_cec_device {
>> +	struct platform_device		*pdev;
>> +	void __iomem			*base;
>> +	struct clk			*core;
>> +	spinlock_t			cec_reg_lock;
>> +	struct cec_notifier		*notify;
>> +	struct cec_adapter		*adap;
>> +	struct cec_msg			rx_msg;
>> +};
>> +
>> +#define writel_bits_relaxed(mask, val, addr) \
>> +	writel_relaxed((readl_relaxed(addr) & ~(mask)) | (val), addr)
>> +
>> +static inline int meson_ao_cec_wait_busy(struct meson_ao_cec_device *ao_cec)
>> +{
>> +	ktime_t timeout = ktime_add_us(ktime_get(), 5000);
>> +
>> +	while (readl_relaxed(ao_cec->base + CEC_RW_REG) & CEC_RW_BUS_BUSY) {
>> +		if (ktime_compare(ktime_get(), timeout) > 0)
>> +			return -ETIMEDOUT;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int meson_ao_cec_read(struct meson_ao_cec_device *ao_cec,
>> +			     unsigned long address, u8 *data)
>> +{
>> +	unsigned long flags;
>> +	u32 reg = FIELD_PREP(CEC_RW_ADDR, address);
>> +	int ret = 0;
>> +
>> +	spin_lock_irqsave(&ao_cec->cec_reg_lock, flags);
>> +
>> +	ret = meson_ao_cec_wait_busy(ao_cec);
>> +	if (ret)
>> +		goto read_out;
>> +
>> +	writel_relaxed(reg, ao_cec->base + CEC_RW_REG);
>> +
>> +	ret = meson_ao_cec_wait_busy(ao_cec);
>> +	if (ret)
>> +		goto read_out;
>> +
>> +	*data = FIELD_GET(CEC_RW_RD_DATA,
>> +			  readl_relaxed(ao_cec->base + CEC_RW_REG));
>> +
>> +read_out:
>> +	spin_unlock_irqrestore(&ao_cec->cec_reg_lock, flags);
>> +
>> +	return ret;
>> +}
>> +
>> +static int meson_ao_cec_write(struct meson_ao_cec_device *ao_cec,
>> +			      unsigned long address, u8 data)
> 
> I suggest a different prototype for the write functions:
> 
> static void meson_ao_cec_write(struct meson_ao_cec_device *ao_cec,
> 			      unsigned long address, u8 data, int *res);
> 
> and that the write function starts with:
> 
> 	if (*res)
> 		return;
> 
> This makes it possible to replace awkward code like this:
> 
> 	int ret;
> 
> 	ret = meson_ao_cec_write(ao_cec, CEC_RX_MSG_CMD, RX_DISABLE);
> 	if (ret)
> 		return ret;
> 	ret = meson_ao_cec_write(ao_cec, CEC_TX_MSG_CMD, TX_ABORT);
> 	if (ret)
> 		return ret;
> 
> to this:
> 
> 	int ret = 0;
> 
> 	meson_ao_cec_write(ao_cec, CEC_RX_MSG_CMD, RX_DISABLE, &ret);
> 	meson_ao_cec_write(ao_cec, CEC_TX_MSG_CMD, TX_ABORT, &ret);
> 	...
> 	return ret;
> 
> The read() function isn't called that often, so I leave it up to you to decide
> whether or not to change that function to something similar.


Sure, it moves the ugliness elsewhere, but I'll try it anyway.

> 
>> +{
>> +	unsigned long flags;
>> +	u32 reg = FIELD_PREP(CEC_RW_ADDR, address) |
>> +		  FIELD_PREP(CEC_RW_WR_DATA, data) |
>> +		  CEC_RW_WRITE_EN;
>> +	int ret = 0;
>> +
>> +	spin_lock_irqsave(&ao_cec->cec_reg_lock, flags);
>> +
>> +	ret = meson_ao_cec_wait_busy(ao_cec);
>> +	if (ret)
>> +		goto write_out;
>> +
>> +	writel_relaxed(reg, ao_cec->base + CEC_RW_REG);
>> +
>> +write_out:
>> +	spin_unlock_irqrestore(&ao_cec->cec_reg_lock, flags);
>> +
>> +	return ret;
>> +}
>> +
>> +static inline void meson_ao_cec_irq_setup(struct meson_ao_cec_device *ao_cec,
>> +				      bool enable)
>> +{
>> +	u32 cfg = CEC_INTR_TX | CEC_INTR_RX;
>> +
>> +	writel_bits_relaxed(cfg, enable ? cfg : 0,
>> +			    ao_cec->base + CEC_INTR_MASKN_REG);
>> +}
>> +
>> +static inline int meson_ao_cec_clear(struct meson_ao_cec_device *ao_cec)
>> +{
>> +	int ret;
>> +
>> +	ret = meson_ao_cec_write(ao_cec, CEC_RX_MSG_CMD, RX_DISABLE);
>> +	if (ret)
>> +		return ret;
>> +	ret = meson_ao_cec_write(ao_cec, CEC_TX_MSG_CMD, TX_ABORT);
>> +	if (ret)
>> +		return ret;
>> +	ret = meson_ao_cec_write(ao_cec, CEC_RX_CLEAR_BUF, 1);
>> +	if (ret)
>> +		return ret;
>> +	ret = meson_ao_cec_write(ao_cec, CEC_TX_CLEAR_BUF, 1);
>> +	if (ret)
>> +		return ret;
>> +
>> +	udelay(100);
>> +
>> +	ret = meson_ao_cec_write(ao_cec, CEC_RX_CLEAR_BUF, 0);
>> +	if (ret)
>> +		return ret;
>> +	ret = meson_ao_cec_write(ao_cec, CEC_TX_CLEAR_BUF, 0);
>> +	if (ret)
>> +		return ret;
>> +
>> +	udelay(100);
>> +
>> +	ret = meson_ao_cec_write(ao_cec, CEC_RX_MSG_CMD, RX_NO_OP);
>> +	if (ret)
>> +		return ret;
>> +	ret = meson_ao_cec_write(ao_cec, CEC_TX_MSG_CMD, TX_NO_OP);
>> +	if (ret)
>> +		return ret;
>> +
>> +	return 0;
>> +}
>> +
>> +static int meson_ao_cec_arbit_bit_time_set(struct meson_ao_cec_device *ao_cec,
>> +					   unsigned int bit_set,
>> +					   unsigned int time_set)
>> +{
>> +	int ret;
>> +
>> +	switch (bit_set) {
>> +	case CEC_SIGNAL_FREE_TIME_RETRY:
>> +		ret = meson_ao_cec_write(ao_cec, CEC_TXTIME_4BIT_BIT7_0,
>> +				   time_set & 0xff);
>> +		if (ret)
>> +			return ret;
>> +		ret = meson_ao_cec_write(ao_cec, CEC_TXTIME_4BIT_BIT10_8,
>> +				   (time_set >> 8) & 0x7);
>> +		if (ret)
>> +			return ret;
>> +		break;
>> +
>> +	case CEC_SIGNAL_FREE_TIME_NEW_INITIATOR:
>> +		ret = meson_ao_cec_write(ao_cec, CEC_TXTIME_2BIT_BIT7_0,
>> +				   time_set & 0xff);
>> +		if (ret)
>> +			return ret;
>> +		ret = meson_ao_cec_write(ao_cec, CEC_TXTIME_2BIT_BIT10_8,
>> +				   (time_set >> 8) & 0x7);
>> +		if (ret)
>> +			return ret;
>> +		break;
>> +
>> +	case CEC_SIGNAL_FREE_TIME_NEXT_XFER:
>> +		ret = meson_ao_cec_write(ao_cec, CEC_TXTIME_17MS_BIT7_0,
>> +				   time_set & 0xff);
>> +		if (ret)
>> +			return ret;
>> +		ret = meson_ao_cec_write(ao_cec, CEC_TXTIME_17MS_BIT10_8,
>> +				   (time_set >> 8) & 0x7);
>> +		if (ret)
>> +			return ret;
>> +		break;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static irqreturn_t meson_ao_cec_irq(int irq, void *data)
>> +{
>> +	struct meson_ao_cec_device *ao_cec = data;
>> +	u32 stat = readl_relaxed(ao_cec->base + CEC_INTR_STAT_REG);
>> +
>> +	if (stat)
>> +		return IRQ_WAKE_THREAD;
>> +
>> +	return IRQ_NONE;
>> +}
>> +
>> +static void meson_ao_cec_irq_tx(struct meson_ao_cec_device *ao_cec)
>> +{
>> +	unsigned long tx_status = 0;
>> +	u8 stat;
>> +	int ret;
>> +
>> +	ret = meson_ao_cec_read(ao_cec, CEC_TX_MSG_STATUS, &stat);
>> +	if (ret)
>> +		goto tx_reg_err;
>> +
>> +	switch (stat) {
>> +	case TX_DONE:
>> +		tx_status = CEC_TX_STATUS_OK;
>> +		break;
>> +
>> +	case TX_BUSY:
>> +		tx_status = CEC_TX_STATUS_ARB_LOST;
>> +		break;
>> +
>> +	case TX_IDLE:
>> +		tx_status = CEC_TX_STATUS_LOW_DRIVE;
>> +		break;
>> +
>> +	case TX_ERROR:
>> +	default:
>> +		tx_status = CEC_TX_STATUS_NACK;
>> +		break;
>> +	}
>> +
>> +	/* Clear Interruption */
>> +	writel_relaxed(CEC_INTR_TX, ao_cec->base + CEC_INTR_CLR_REG);
>> +
>> +	/* Stop TX */
>> +	ret = meson_ao_cec_write(ao_cec, CEC_TX_MSG_CMD, TX_NO_OP);
>> +	if (ret)
>> +		goto tx_reg_err;
>> +
>> +	cec_transmit_attempt_done(ao_cec->adap, tx_status);
>> +	return;
>> +
>> +tx_reg_err:
>> +	cec_transmit_attempt_done(ao_cec->adap, CEC_TX_STATUS_ERROR);
>> +}
>> +
>> +static void meson_ao_cec_irq_rx(struct meson_ao_cec_device *ao_cec)
>> +{
>> +	int i, ret;
>> +	u8 reg;
>> +
>> +	ret = meson_ao_cec_read(ao_cec, CEC_RX_MSG_STATUS, &reg);
>> +	if (ret)
>> +		goto rx_out;
>> +
>> +	/* RX Error */
>> +	if (reg != RX_DONE)
>> +		goto rx_out;
>> +
>> +	ret = meson_ao_cec_read(ao_cec, CEC_RX_NUM_MSG, &reg);
>> +	if (ret || reg != 1)
>> +		goto rx_out;
>> +
>> +	ret = meson_ao_cec_read(ao_cec, CEC_RX_MSG_LENGTH, &reg);
>> +	if (ret)
>> +		goto rx_out;
>> +
>> +	ao_cec->rx_msg.len = reg + 1;
>> +	if (ao_cec->rx_msg.len > CEC_MAX_MSG_SIZE)
>> +		ao_cec->rx_msg.len = CEC_MAX_MSG_SIZE;
>> +
>> +	for (i = 0; i < ao_cec->rx_msg.len; i++) {
>> +		u8 byte;
>> +
>> +		ret = meson_ao_cec_read(ao_cec, CEC_RX_MSG_0_HEADER + i, &byte);
>> +		if (ret)
>> +			goto rx_out;
>> +
>> +		ao_cec->rx_msg.msg[i] = byte;
>> +	}
>> +
>> +	cec_received_msg(ao_cec->adap, &ao_cec->rx_msg);
>> +
>> +rx_out:
>> +	/* Clear Interruption */
>> +	writel_relaxed(CEC_INTR_RX, ao_cec->base + CEC_INTR_CLR_REG);
>> +
>> +	/* Return if previous read errors */
>> +	if (ret)
>> +		return;
>> +
>> +	/* Ack RX message */
>> +	ret = meson_ao_cec_write(ao_cec, CEC_RX_MSG_CMD, RX_ACK_CURRENT);
>> +	if (ret)
>> +		return;
>> +	ret = meson_ao_cec_write(ao_cec, CEC_RX_MSG_CMD, RX_NO_OP);
>> +	if (ret)
>> +		return;
>> +
>> +	/* Clear RX buffer */
>> +	ret = meson_ao_cec_write(ao_cec, CEC_RX_CLEAR_BUF, CLEAR_START);
>> +	if (ret)
>> +		return;
>> +	ret = meson_ao_cec_write(ao_cec, CEC_RX_CLEAR_BUF, CLEAR_STOP);
>> +	if (ret)
>> +		return;
>> +}
>> +
>> +static irqreturn_t meson_ao_cec_irq_thread(int irq, void *data)
>> +{
>> +	struct meson_ao_cec_device *ao_cec = data;
>> +	u32 stat = readl_relaxed(ao_cec->base + CEC_INTR_STAT_REG);
>> +
>> +	if (stat & CEC_INTR_TX)
>> +		meson_ao_cec_irq_tx(ao_cec);
>> +
>> +	meson_ao_cec_irq_rx(ao_cec);
>> +
>> +	return IRQ_HANDLED;
>> +}
>> +
>> +static int meson_ao_cec_set_log_addr(struct cec_adapter *adap, u8 logical_addr)
>> +{
>> +	struct meson_ao_cec_device *ao_cec = adap->priv;
>> +	int ret;
>> +
>> +	ret = meson_ao_cec_write(ao_cec, CEC_LOGICAL_ADDR0,
>> +				 LOGICAL_ADDR_DISABLE);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = meson_ao_cec_clear(ao_cec);
>> +	if (ret)
>> +		return ret;
>> +
>> +	if (logical_addr == CEC_LOG_ADDR_INVALID)
>> +		return 0;
>> +
>> +	ret = meson_ao_cec_write(ao_cec, CEC_LOGICAL_ADDR0,
>> +			   logical_addr & LOGICAL_ADDR_MASK);
>> +	if (ret)
>> +		return ret;
>> +
>> +	udelay(100);
>> +
>> +	ret = meson_ao_cec_write(ao_cec, CEC_LOGICAL_ADDR0,
>> +			   (logical_addr & LOGICAL_ADDR_MASK) |
>> +			   LOGICAL_ADDR_VALID);
>> +
>> +	return ret;
>> +}
>> +
>> +static int meson_ao_cec_transmit(struct cec_adapter *adap, u8 attempts,
>> +				 u32 signal_free_time, struct cec_msg *msg)
>> +{
>> +	struct meson_ao_cec_device *ao_cec = adap->priv;
>> +	int i, ret;
>> +	u8 reg;
>> +
>> +	ret = meson_ao_cec_read(ao_cec, CEC_TX_MSG_STATUS, &reg);
>> +	if (ret)
>> +		return ret;
>> +
>> +	if (reg == TX_BUSY) {
>> +		dev_err(&ao_cec->pdev->dev, "%s: busy TX\n", __func__);
> 
> s/busy TX/busy TX: aborting/
> 
> to make it clear that this TX is aborted.


OK

> 
>> +		ret = meson_ao_cec_write(ao_cec, CEC_TX_MSG_CMD, TX_ABORT);
>> +		if (ret)
>> +			return ret;
>> +	}
>> +
>> +	for (i = 0; i < msg->len; i++) {
>> +		ret = meson_ao_cec_write(ao_cec, CEC_TX_MSG_0_HEADER + i,
>> +					 msg->msg[i]);
>> +		if (ret)
>> +			return ret;
>> +	}
>> +
>> +	ret = meson_ao_cec_write(ao_cec, CEC_TX_MSG_LENGTH, msg->len - 1);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = meson_ao_cec_write(ao_cec, CEC_TX_MSG_CMD, TX_REQ_CURRENT);
>> +
>> +	return ret;
>> +}
>> +
>> +static int meson_ao_cec_adap_enable(struct cec_adapter *adap, bool enable)
>> +{
>> +	struct meson_ao_cec_device *ao_cec = adap->priv;
>> +	int ret;
>> +
>> +	meson_ao_cec_irq_setup(ao_cec, false);
>> +
>> +	writel_bits_relaxed(CEC_GEN_CNTL_RESET, CEC_GEN_CNTL_RESET,
>> +			    ao_cec->base + CEC_GEN_CNTL_REG);
>> +
>> +	if (!enable)
>> +		return 0;
>> +
>> +	/* Enable gated clock (Normal mode). */
>> +	writel_bits_relaxed(CEC_GEN_CNTL_CLK_CTRL_MASK,
>> +			    FIELD_PREP(CEC_GEN_CNTL_CLK_CTRL_MASK,
>> +				       CEC_GEN_CNTL_CLK_ENABLE),
>> +			    ao_cec->base + CEC_GEN_CNTL_REG);
>> +
>> +	udelay(100);
>> +
>> +	/* Release Reset */
>> +	writel_bits_relaxed(CEC_GEN_CNTL_RESET, 0,
>> +			    ao_cec->base + CEC_GEN_CNTL_REG);
>> +
>> +	/* Clear buffers */
>> +	ret = meson_ao_cec_clear(ao_cec);
>> +	if (ret)
>> +		return ret;
>> +
>> +	/* CEC arbitration 3/5/7 bit time set. */
>> +	ret = meson_ao_cec_arbit_bit_time_set(ao_cec,
>> +					CEC_SIGNAL_FREE_TIME_RETRY,
>> +					0x118);
>> +	if (ret)
>> +		return ret;
>> +	ret = meson_ao_cec_arbit_bit_time_set(ao_cec,
>> +					CEC_SIGNAL_FREE_TIME_NEW_INITIATOR,
>> +					0x000);
>> +	if (ret)
>> +		return ret;
>> +	ret = meson_ao_cec_arbit_bit_time_set(ao_cec,
>> +					CEC_SIGNAL_FREE_TIME_NEXT_XFER,
>> +					0x2aa);
>> +	if (ret)
>> +		return ret;
>> +
>> +	meson_ao_cec_irq_setup(ao_cec, true);
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct cec_adap_ops meson_ao_cec_ops = {
>> +	.adap_enable = meson_ao_cec_adap_enable,
>> +	.adap_log_addr = meson_ao_cec_set_log_addr,
>> +	.adap_transmit = meson_ao_cec_transmit,
>> +};
>> +
>> +static int meson_ao_cec_probe(struct platform_device *pdev)
>> +{
>> +	struct meson_ao_cec_device *ao_cec;
>> +	struct platform_device *hdmi_dev;
>> +	struct device_node *np;
>> +	struct resource *res;
>> +	int ret, irq;
>> +
>> +	np = of_parse_phandle(pdev->dev.of_node, "hdmi-phandle", 0);
>> +	if (!np) {
>> +		dev_err(&pdev->dev, "Failed to find hdmi node\n");
>> +		return -ENODEV;
>> +	}
>> +
>> +	hdmi_dev = of_find_device_by_node(np);
>> +	if (hdmi_dev == NULL)
>> +		return -EPROBE_DEFER;
>> +
>> +	ao_cec = devm_kzalloc(&pdev->dev, sizeof(*ao_cec), GFP_KERNEL);
>> +	if (!ao_cec)
>> +		return -ENOMEM;
>> +
>> +	spin_lock_init(&ao_cec->cec_reg_lock);
>> +
>> +	ao_cec->notify = cec_notifier_get(&hdmi_dev->dev);
>> +	if (!ao_cec->notify)
>> +		return -ENOMEM;
>> +
>> +	ao_cec->adap = cec_allocate_adapter(&meson_ao_cec_ops, ao_cec,
>> +					    "meson_ao_cec",
>> +					    CEC_CAP_LOG_ADDRS |
>> +					    CEC_CAP_TRANSMIT |
>> +					    CEC_CAP_RC |
>> +					    CEC_CAP_PASSTHROUGH,
>> +					    1); /* Use 1 for now */
> 
> I recommend that you add support for 2 logical addresses. More isn't allowed
> by the CEC 2.0 spec anyway (no such restriction for CEC 1.4, but more than
> two really isn't needed).

I know, but in the "communication" register with the suspend/poweroff firmware
that  handles the wake up, only a single logical address is supported...

What should I do in this case ? Which logical adress should I pass to the firmware when implementing ir ?

> 
>> +	if (IS_ERR(ao_cec->adap)) {
>> +		ret = PTR_ERR(ao_cec->adap);
>> +		goto out_probe_notify;
>> +	}
>> +
>> +	ao_cec->adap->owner = THIS_MODULE;
>> +
>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +	ao_cec->base = devm_ioremap_resource(&pdev->dev, res);
>> +	if (IS_ERR(ao_cec->base)) {
>> +		ret = PTR_ERR(ao_cec->base);
>> +		goto out_probe_adapter;
>> +	}
>> +
>> +	irq = platform_get_irq(pdev, 0);
>> +	ret = devm_request_threaded_irq(&pdev->dev, irq,
>> +					meson_ao_cec_irq,
>> +					meson_ao_cec_irq_thread,
>> +					0, NULL, ao_cec);
>> +	if (ret) {
>> +		dev_err(&pdev->dev, "irq request failed\n");
>> +		goto out_probe_adapter;
>> +	}
>> +
>> +	ao_cec->core = devm_clk_get(&pdev->dev, "core");
>> +	if (IS_ERR(ao_cec->core)) {
>> +		dev_err(&pdev->dev, "core clock request failed\n");
>> +		ret = PTR_ERR(ao_cec->core);
>> +		goto out_probe_adapter;
>> +	}
>> +
>> +	ret = clk_prepare_enable(ao_cec->core);
>> +	if (ret) {
>> +		dev_err(&pdev->dev, "core clock enable failed\n");
>> +		goto out_probe_adapter;
>> +	}
>> +
>> +	ret = clk_set_rate(ao_cec->core, CEC_CLK_RATE);
>> +	if (ret) {
>> +		dev_err(&pdev->dev, "core clock set rate failed\n");
>> +		goto out_probe_clk;
>> +	}
>> +
>> +	device_reset_optional(&pdev->dev);
>> +
>> +	ao_cec->pdev = pdev;
>> +	platform_set_drvdata(pdev, ao_cec);
>> +
>> +	ret = cec_register_adapter(ao_cec->adap, &pdev->dev);
>> +	if (ret < 0) {
>> +		cec_notifier_put(ao_cec->notify);
>> +		goto out_probe_clk;
>> +	}
>> +
>> +	/* Setup Hardware */
>> +	writel_relaxed(CEC_GEN_CNTL_RESET,
>> +		       ao_cec->base + CEC_GEN_CNTL_REG);
>> +
>> +	cec_register_cec_notifier(ao_cec->adap, ao_cec->notify);
>> +
>> +	return 0;
>> +
>> +out_probe_clk:
>> +	clk_disable_unprepare(ao_cec->core);
>> +
>> +out_probe_adapter:
>> +	cec_delete_adapter(ao_cec->adap);
>> +
>> +out_probe_notify:
>> +	cec_notifier_put(ao_cec->notify);
>> +
>> +	dev_err(&pdev->dev, "CEC controller registration failed\n");
>> +
>> +	return ret;
>> +}
>> +
>> +static int meson_ao_cec_remove(struct platform_device *pdev)
>> +{
>> +	struct meson_ao_cec_device *ao_cec = platform_get_drvdata(pdev);
>> +
>> +	clk_disable_unprepare(ao_cec->core);
>> +
>> +	cec_unregister_adapter(ao_cec->adap);
>> +
>> +	cec_notifier_put(ao_cec->notify);
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct of_device_id meson_ao_cec_of_match[] = {
>> +	{ .compatible = "amlogic,meson-gx-ao-cec", },
>> +	{ /* sentinel */ }
>> +};
>> +MODULE_DEVICE_TABLE(of, meson_ao_cec_of_match);
>> +
>> +static struct platform_driver meson_ao_cec_driver = {
>> +	.probe   = meson_ao_cec_probe,
>> +	.remove  = meson_ao_cec_remove,
>> +	.driver  = {
>> +		.name = "meson-ao-cec",
>> +		.of_match_table = of_match_ptr(meson_ao_cec_of_match),
>> +	},
>> +};
>> +
>> +module_platform_driver(meson_ao_cec_driver);
>> +
>> +MODULE_DESCRIPTION("Meson AO CEC Controller driver");
>> +MODULE_AUTHOR("Neil Armstrong <narmstrong@baylibre.com>");
>> +MODULE_LICENSE("GPL");
>>
> 
> Regards,
> 
> 	Hans
> 

Thanks,
Neil

Re: [PATCH v2 1/2] platform: Add Amlogic Meson AO CEC Controller driver

[Hans Verkuil]

On 07/25/17 14:34, Neil Armstrong wrote:
> Hi Hans,

>>> +static int meson_ao_cec_probe(struct platform_device *pdev)
>>> +{
>>> +	struct meson_ao_cec_device *ao_cec;
>>> +	struct platform_device *hdmi_dev;
>>> +	struct device_node *np;
>>> +	struct resource *res;
>>> +	int ret, irq;
>>> +
>>> +	np = of_parse_phandle(pdev->dev.of_node, "hdmi-phandle", 0);
>>> +	if (!np) {
>>> +		dev_err(&pdev->dev, "Failed to find hdmi node\n");
>>> +		return -ENODEV;
>>> +	}
>>> +
>>> +	hdmi_dev = of_find_device_by_node(np);
>>> +	if (hdmi_dev == NULL)
>>> +		return -EPROBE_DEFER;
>>> +
>>> +	ao_cec = devm_kzalloc(&pdev->dev, sizeof(*ao_cec), GFP_KERNEL);
>>> +	if (!ao_cec)
>>> +		return -ENOMEM;
>>> +
>>> +	spin_lock_init(&ao_cec->cec_reg_lock);
>>> +
>>> +	ao_cec->notify = cec_notifier_get(&hdmi_dev->dev);
>>> +	if (!ao_cec->notify)
>>> +		return -ENOMEM;
>>> +
>>> +	ao_cec->adap = cec_allocate_adapter(&meson_ao_cec_ops, ao_cec,
>>> +					    "meson_ao_cec",
>>> +					    CEC_CAP_LOG_ADDRS |
>>> +					    CEC_CAP_TRANSMIT |
>>> +					    CEC_CAP_RC |
>>> +					    CEC_CAP_PASSTHROUGH,
>>> +					    1); /* Use 1 for now */
>>
>> I recommend that you add support for 2 logical addresses. More isn't allowed
>> by the CEC 2.0 spec anyway (no such restriction for CEC 1.4, but more than
>> two really isn't needed).
> 
> I know, but in the "communication" register with the suspend/poweroff firmware
> that  handles the wake up, only a single logical address is supported...
> 
> What should I do in this case ? Which logical adress should I pass to the firmware when implementing ir ?

Ah, OK. Interesting.

>From cec-adap.c:

                if (log_addrs->num_log_addrs == 2) {
                        if (!(type_mask & ((1 << CEC_LOG_ADDR_TYPE_AUDIOSYSTEM) |
                                           (1 << CEC_LOG_ADDR_TYPE_TV)))) {
                                dprintk(1, "two LAs is only allowed for audiosystem and TV\n");
                                return -EINVAL;
                        }
                        if (!(type_mask & ((1 << CEC_LOG_ADDR_TYPE_PLAYBACK) |
                                           (1 << CEC_LOG_ADDR_TYPE_RECORD)))) {
                                dprintk(1, "an audiosystem/TV can only be combined with record or playback\n");
                                return -EINVAL;
                        }
                }

So you would store the TV or AUDIOSYSTEM logical address in the firmware, since those
describe the system best.

I.e. it is a TV/Audiosystem with recording/playback capabilities.

The problem is that for CEC 1.4 no such restriction is imposed (the test above is
specific to CEC 2.0). But I think it makes sense to just check if TV/Audiosystem
is selected and pick that as the LA to store in the firmware, and otherwise just
pick the first LA (log_addr[0]).

Regards,

	Hans

Re: [PATCH v2 1/2] platform: Add Amlogic Meson AO CEC Controller driver

[Neil Armstrong]

On 07/25/2017 03:45 PM, Hans Verkuil wrote:
> On 07/25/17 14:34, Neil Armstrong wrote:
>> Hi Hans,
> 
>>>> +static int meson_ao_cec_probe(struct platform_device *pdev)
>>>> +{
>>>> +	struct meson_ao_cec_device *ao_cec;
>>>> +	struct platform_device *hdmi_dev;
>>>> +	struct device_node *np;
>>>> +	struct resource *res;
>>>> +	int ret, irq;
>>>> +
>>>> +	np = of_parse_phandle(pdev->dev.of_node, "hdmi-phandle", 0);
>>>> +	if (!np) {
>>>> +		dev_err(&pdev->dev, "Failed to find hdmi node\n");
>>>> +		return -ENODEV;
>>>> +	}
>>>> +
>>>> +	hdmi_dev = of_find_device_by_node(np);
>>>> +	if (hdmi_dev == NULL)
>>>> +		return -EPROBE_DEFER;
>>>> +
>>>> +	ao_cec = devm_kzalloc(&pdev->dev, sizeof(*ao_cec), GFP_KERNEL);
>>>> +	if (!ao_cec)
>>>> +		return -ENOMEM;
>>>> +
>>>> +	spin_lock_init(&ao_cec->cec_reg_lock);
>>>> +
>>>> +	ao_cec->notify = cec_notifier_get(&hdmi_dev->dev);
>>>> +	if (!ao_cec->notify)
>>>> +		return -ENOMEM;
>>>> +
>>>> +	ao_cec->adap = cec_allocate_adapter(&meson_ao_cec_ops, ao_cec,
>>>> +					    "meson_ao_cec",
>>>> +					    CEC_CAP_LOG_ADDRS |
>>>> +					    CEC_CAP_TRANSMIT |
>>>> +					    CEC_CAP_RC |
>>>> +					    CEC_CAP_PASSTHROUGH,
>>>> +					    1); /* Use 1 for now */
>>>
>>> I recommend that you add support for 2 logical addresses. More isn't allowed
>>> by the CEC 2.0 spec anyway (no such restriction for CEC 1.4, but more than
>>> two really isn't needed).
>>
>> I know, but in the "communication" register with the suspend/poweroff firmware
>> that  handles the wake up, only a single logical address is supported...
>>
>> What should I do in this case ? Which logical adress should I pass to the firmware when implementing ir ?
> 
> Ah, OK. Interesting.
> 
> From cec-adap.c:
> 
>                 if (log_addrs->num_log_addrs == 2) {
>                         if (!(type_mask & ((1 << CEC_LOG_ADDR_TYPE_AUDIOSYSTEM) |
>                                            (1 << CEC_LOG_ADDR_TYPE_TV)))) {
>                                 dprintk(1, "two LAs is only allowed for audiosystem and TV\n");
>                                 return -EINVAL;
>                         }
>                         if (!(type_mask & ((1 << CEC_LOG_ADDR_TYPE_PLAYBACK) |
>                                            (1 << CEC_LOG_ADDR_TYPE_RECORD)))) {
>                                 dprintk(1, "an audiosystem/TV can only be combined with record or playback\n");
>                                 return -EINVAL;
>                         }
>                 }
> 
> So you would store the TV or AUDIOSYSTEM logical address in the firmware, since those
> describe the system best.
> 
> I.e. it is a TV/Audiosystem with recording/playback capabilities.
> 
> The problem is that for CEC 1.4 no such restriction is imposed (the test above is
> specific to CEC 2.0). But I think it makes sense to just check if TV/Audiosystem
> is selected and pick that as the LA to store in the firmware, and otherwise just
> pick the first LA (log_addr[0]).


Ok I'll add support for dual LA, and I'll do this LA selection when I'll add firmware support.

Thanks,
Neil


> Regards,
> 
> 	Hans
> 

Re: [PATCH v2 1/2] platform: Add Amlogic Meson AO CEC Controller driver

[Neil Armstrong]

On 07/27/2017 04:43 PM, Neil Armstrong wrote:
> On 07/25/2017 03:45 PM, Hans Verkuil wrote:
>> On 07/25/17 14:34, Neil Armstrong wrote:
>>> Hi Hans,
>>
>>>>> +static int meson_ao_cec_probe(struct platform_device *pdev)
>>>>> +{
>>>>> +	struct meson_ao_cec_device *ao_cec;
>>>>> +	struct platform_device *hdmi_dev;
>>>>> +	struct device_node *np;
>>>>> +	struct resource *res;
>>>>> +	int ret, irq;
>>>>> +
>>>>> +	np = of_parse_phandle(pdev->dev.of_node, "hdmi-phandle", 0);
>>>>> +	if (!np) {
>>>>> +		dev_err(&pdev->dev, "Failed to find hdmi node\n");
>>>>> +		return -ENODEV;
>>>>> +	}
>>>>> +
>>>>> +	hdmi_dev = of_find_device_by_node(np);
>>>>> +	if (hdmi_dev == NULL)
>>>>> +		return -EPROBE_DEFER;
>>>>> +
>>>>> +	ao_cec = devm_kzalloc(&pdev->dev, sizeof(*ao_cec), GFP_KERNEL);
>>>>> +	if (!ao_cec)
>>>>> +		return -ENOMEM;
>>>>> +
>>>>> +	spin_lock_init(&ao_cec->cec_reg_lock);
>>>>> +
>>>>> +	ao_cec->notify = cec_notifier_get(&hdmi_dev->dev);
>>>>> +	if (!ao_cec->notify)
>>>>> +		return -ENOMEM;
>>>>> +
>>>>> +	ao_cec->adap = cec_allocate_adapter(&meson_ao_cec_ops, ao_cec,
>>>>> +					    "meson_ao_cec",
>>>>> +					    CEC_CAP_LOG_ADDRS |
>>>>> +					    CEC_CAP_TRANSMIT |
>>>>> +					    CEC_CAP_RC |
>>>>> +					    CEC_CAP_PASSTHROUGH,
>>>>> +					    1); /* Use 1 for now */
>>>>
>>>> I recommend that you add support for 2 logical addresses. More isn't allowed
>>>> by the CEC 2.0 spec anyway (no such restriction for CEC 1.4, but more than
>>>> two really isn't needed).
>>>
>>> I know, but in the "communication" register with the suspend/poweroff firmware
>>> that  handles the wake up, only a single logical address is supported...
>>>
>>> What should I do in this case ? Which logical adress should I pass to the firmware when implementing ir ?
>>
>> Ah, OK. Interesting.
>>
>> From cec-adap.c:
>>
>>                 if (log_addrs->num_log_addrs == 2) {
>>                         if (!(type_mask & ((1 << CEC_LOG_ADDR_TYPE_AUDIOSYSTEM) |
>>                                            (1 << CEC_LOG_ADDR_TYPE_TV)))) {
>>                                 dprintk(1, "two LAs is only allowed for audiosystem and TV\n");
>>                                 return -EINVAL;
>>                         }
>>                         if (!(type_mask & ((1 << CEC_LOG_ADDR_TYPE_PLAYBACK) |
>>                                            (1 << CEC_LOG_ADDR_TYPE_RECORD)))) {
>>                                 dprintk(1, "an audiosystem/TV can only be combined with record or playback\n");
>>                                 return -EINVAL;
>>                         }
>>                 }
>>
>> So you would store the TV or AUDIOSYSTEM logical address in the firmware, since those
>> describe the system best.
>>
>> I.e. it is a TV/Audiosystem with recording/playback capabilities.
>>
>> The problem is that for CEC 1.4 no such restriction is imposed (the test above is
>> specific to CEC 2.0). But I think it makes sense to just check if TV/Audiosystem
>> is selected and pick that as the LA to store in the firmware, and otherwise just
>> pick the first LA (log_addr[0]).
> 
> 
> Ok I'll add support for dual LA, and I'll do this LA selection when I'll add firmware support.

Sorry, but having more than 1 LA makes the CEC controller very unstable, I'll need more info from amlogic to activate multiple LAs.

Neil

> 
> Thanks,
> Neil
> 
> 
>> Regards,
>>
>> 	Hans
>>
>