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

On 10/07/17 10:01, Neil Armstrong wrote:

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 [off-list ref]");
+MODULE_LICENSE("GPL");

Regards,

	Hans