On 3/25/19 6:35 PM, Neil Armstrong wrote: > The Amlogic G12A SoC embeds a second CEC controller with a totally > different design. > > The two controller can work in the same time since the CEC line can > be set to two different pins on the two controllers. > > This second CEC controller is documented as "AO-CEC-B", thus the > registers will be named "CECB_" to differenciate with the other > AO-CEC driver. > > Unlike the other AO-CEC controller, this one takes the Oscillator > clock as input and embeds a dual-divider to provide a precise > 32768Hz clock for communication. This is handled by registering > a clock in the driver. > > Unlike the other AO-CEC controller, this controller supports setting > up to 15 logical adresses and supports the signal_free_time settings > in the transmit function. > > Unfortunately, this controller does not support "monitor" mode. > > Signed-off-by: Neil Armstrong <narmstrong@xxxxxxxxxxxx> > --- > drivers/media/platform/Kconfig | 13 + > drivers/media/platform/meson/Makefile | 1 + > drivers/media/platform/meson/ao-cec-g12a.c | 783 +++++++++++++++++++++ > 3 files changed, 797 insertions(+) > create mode 100644 drivers/media/platform/meson/ao-cec-g12a.c > > diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig > index 4acbed189644..92ea07ddc609 100644 > --- a/drivers/media/platform/Kconfig > +++ b/drivers/media/platform/Kconfig > @@ -578,6 +578,19 @@ config VIDEO_MESON_AO_CEC > generic CEC framework interface. > CEC bus is present in the HDMI connector and enables communication > > +config VIDEO_MESON_G12A_AO_CEC > + tristate "Amlogic Meson G12A AO CEC driver" > + depends on ARCH_MESON || COMPILE_TEST > + select CEC_CORE > + select CEC_NOTIFIER > + ---help--- > + This is a driver for Amlogic Meson G12A SoCs AO CEC interface. > + This driver if for the new AO-CEC module found in G12A SoCs, > + usually named AO_CEC_B in documentation. > + It uses the generic CEC framework interface. > + CEC bus is present in the HDMI connector and enables communication > + between compatible devices. > + > config CEC_GPIO > tristate "Generic GPIO-based CEC driver" > depends on PREEMPT || COMPILE_TEST > diff --git a/drivers/media/platform/meson/Makefile b/drivers/media/platform/meson/Makefile > index 597beb8f34d1..f611c23c3718 100644 > --- a/drivers/media/platform/meson/Makefile > +++ b/drivers/media/platform/meson/Makefile > @@ -1 +1,2 @@ > obj-$(CONFIG_VIDEO_MESON_AO_CEC) += ao-cec.o > +obj-$(CONFIG_VIDEO_MESON_G12A_AO_CEC) += ao-cec-g12a.o > diff --git a/drivers/media/platform/meson/ao-cec-g12a.c b/drivers/media/platform/meson/ao-cec-g12a.c > new file mode 100644 > index 000000000000..8977ae994164 > --- /dev/null > +++ b/drivers/media/platform/meson/ao-cec-g12a.c > @@ -0,0 +1,783 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Driver for Amlogic Meson AO CEC G12A Controller > + * > + * Copyright (C) 2017 Amlogic, Inc. All rights reserved > + * Copyright (C) 2019 BayLibre, SAS > + * Author: Neil Armstrong <narmstrong@xxxxxxxxxxxx> > + */ > + > +#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 <linux/slab.h> > +#include <linux/regmap.h> > +#include <media/cec.h> > +#include <media/cec-notifier.h> > +#include <linux/clk-provider.h> > + > +/* CEC Registers */ > + > +#define CECB_CLK_CNTL_REG0 0x00 > + > +#define CECB_CLK_CNTL_N1 GENMASK(11, 0) > +#define CECB_CLK_CNTL_N2 GENMASK(23, 12) > +#define CECB_CLK_CNTL_DUAL_EN BIT(28) > +#define CECB_CLK_CNTL_OUTPUT_EN BIT(30) > +#define CECB_CLK_CNTL_INPUT_EN BIT(31) > + > +#define CECB_CLK_CNTL_REG1 0x04 > + > +#define CECB_CLK_CNTL_M1 GENMASK(11, 0) > +#define CECB_CLK_CNTL_M2 GENMASK(23, 12) > +#define CECB_CLK_CNTL_BYPASS_EN BIT(24) > + > +/* > + * [14:12] Filter_del. For glitch-filtering CEC line, ignore signal > + * change pulse width < filter_del * T(filter_tick) * 3. > + * [9:8] Filter_tick_sel: Select which periodical pulse for > + * glitch-filtering CEC line signal. > + * - 0=Use T(xtal)*3 = 125ns; > + * - 1=Use once-per-1us pulse; > + * - 2=Use once-per-10us pulse; > + * - 3=Use once-per-100us pulse. > + * [3] Sysclk_en. 0=Disable system clock; 1=Enable system clock. > + * [2:1] cntl_clk > + * - 0 = Disable clk (Power-off mode) > + * - 1 = Enable gated clock (Normal mode) > + * - 2 = Enable free-run clk (Debug mode) > + * [0] SW_RESET 1=Apply reset; 0=No reset. > + */ > +#define CECB_GEN_CNTL_REG 0x08 > + > +#define CECB_GEN_CNTL_RESET BIT(0) > +#define CECB_GEN_CNTL_CLK_DISABLE 0 > +#define CECB_GEN_CNTL_CLK_ENABLE 1 > +#define CECB_GEN_CNTL_CLK_ENABLE_DBG 2 > +#define CECB_GEN_CNTL_CLK_CTRL_MASK GENMASK(2, 1) > +#define CECB_GEN_CNTL_SYS_CLK_EN BIT(3) > +#define CECB_GEN_CNTL_FILTER_TICK_125NS 0 > +#define CECB_GEN_CNTL_FILTER_TICK_1US 1 > +#define CECB_GEN_CNTL_FILTER_TICK_10US 2 > +#define CECB_GEN_CNTL_FILTER_TICK_100US 3 > +#define CECB_GEN_CNTL_FILTER_TICK_SEL GENMASK(9, 8) > +#define CECB_GEN_CNTL_FILTER_DEL GENMASK(14, 12) > + > +/* > + * [7:0] cec_reg_addr > + * [15:8] cec_reg_wrdata > + * [16] cec_reg_wr > + * - 0 = Read > + * - 1 = Write > + * [31:24] cec_reg_rddata > + */ > +#define CECB_RW_REG 0x0c > + > +#define CECB_RW_ADDR GENMASK(7, 0) > +#define CECB_RW_WR_DATA GENMASK(15, 8) > +#define CECB_RW_WRITE_EN BIT(16) > +#define CECB_RW_BUS_BUSY BIT(23) > +#define CECB_RW_RD_DATA GENMASK(31, 24) > + > +/* > + * [0] DONE Interrupt > + * [1] End Of Message Interrupt > + * [2] Not Acknowlegde Interrupt > + * [3] Arbitration Loss Interrupt > + * [4] Initiator Error Interrupt > + * [5] Follower Error Interrupt > + * [6] Wake-Up Interrupt > + */ > +#define CECB_INTR_MASKN_REG 0x10 > +#define CECB_INTR_CLR_REG 0x14 > +#define CECB_INTR_STAT_REG 0x18 > + > +#define CECB_INTR_DONE BIT(0) > +#define CECB_INTR_EOM BIT(1) > +#define CECB_INTR_NACK BIT(2) > +#define CECB_INTR_ARB_LOSS BIT(3) > +#define CECB_INTR_INITIATOR_ERR BIT(4) > +#define CECB_INTR_FOLLOWER_ERR BIT(5) > +#define CECB_INTR_WAKE_UP BIT(6) > + > +/* CEC Commands */ > + > +#define CECB_CTRL 0x00 > + > +#define CECB_CTRL_SEND BIT(0) > +#define CECB_CTRL_TYPE GENMASK(2, 1) > +#define CECB_CTRL_TYPE_RETRY 0 > +#define CECB_CTRL_TYPE_NEW 1 > +#define CECB_CTRL_TYPE_NEXT 2 > + > +#define CECB_CTRL2 0x01 > +#define CECB_INTR_MASK 0x02 > +#define CECB_LADD_LOW 0x05 > +#define CECB_LADD_HIGH 0x06 > +#define CECB_TX_CNT 0x07 > +#define CECB_RX_CNT 0x08 > +#define CECB_STAT0 0x09 > +#define CECB_TX_DATA00 0x10 > +#define CECB_TX_DATA01 0x11 > +#define CECB_TX_DATA02 0x12 > +#define CECB_TX_DATA03 0x13 > +#define CECB_TX_DATA04 0x14 > +#define CECB_TX_DATA05 0x15 > +#define CECB_TX_DATA06 0x16 > +#define CECB_TX_DATA07 0x17 > +#define CECB_TX_DATA08 0x18 > +#define CECB_TX_DATA09 0x19 > +#define CECB_TX_DATA10 0x1A > +#define CECB_TX_DATA11 0x1B > +#define CECB_TX_DATA12 0x1C > +#define CECB_TX_DATA13 0x1D > +#define CECB_TX_DATA14 0x1E > +#define CECB_TX_DATA15 0x1F > +#define CECB_RX_DATA00 0x20 > +#define CECB_RX_DATA01 0x21 > +#define CECB_RX_DATA02 0x22 > +#define CECB_RX_DATA03 0x23 > +#define CECB_RX_DATA04 0x24 > +#define CECB_RX_DATA05 0x25 > +#define CECB_RX_DATA06 0x26 > +#define CECB_RX_DATA07 0x27 > +#define CECB_RX_DATA08 0x28 > +#define CECB_RX_DATA09 0x29 > +#define CECB_RX_DATA10 0x2A > +#define CECB_RX_DATA11 0x2B > +#define CECB_RX_DATA12 0x2C > +#define CECB_RX_DATA13 0x2D > +#define CECB_RX_DATA14 0x2E > +#define CECB_RX_DATA15 0x2F > +#define CECB_LOCK_BUF 0x30 > + > +#define CECB_LOCK_BUF_EN BIT(0) > + > +#define CECB_WAKEUPCTRL 0x31 > + > +struct meson_ao_cec_g12a_device { > + struct platform_device *pdev; > + struct regmap *regmap; > + struct regmap *regmap_cec; > + spinlock_t cec_reg_lock; > + struct cec_notifier *notify; > + struct cec_adapter *adap; > + struct cec_msg rx_msg; > + struct clk *oscin; > + struct clk *core; > +}; > + > +static const struct regmap_config meson_ao_cec_g12a_regmap_conf = { > + .reg_bits = 8, > + .val_bits = 32, > + .reg_stride = 4, > + .max_register = CECB_INTR_STAT_REG, > +}; > + > +/* > + * The AO-CECB embeds a dual/divider to generate a more precise > + * 32,768KHz clock for CEC core clock. > + * ______ ______ > + * | | | | > + * ______ | Div1 |-| Cnt1 | ______ > + * | | /|______| |______|\ | | > + * Xtal-->| Gate |---| ______ ______ X-X--| Gate |--> > + * |______| | \| | | |/ | |______| > + * | | Div2 |-| Cnt2 | | > + * | |______| |______| | > + * |_______________________| > + * > + * The dividing can be switched to single or dual, with a counter > + * for each divider to set when the switching is done. > + * The entire dividing mechanism can be also bypassed. > + */ > + > +struct meson_ao_cec_g12a_dualdiv_clk { > + struct clk_hw hw; > + struct regmap *regmap; > +}; > + > +#define hw_to_meson_ao_cec_g12a_dualdiv_clk(_hw) \ > + container_of(_hw, struct meson_ao_cec_g12a_dualdiv_clk, hw) \ > + > +static unsigned long > +meson_ao_cec_g12a_dualdiv_clk_recalc_rate(struct clk_hw *hw, > + unsigned long parent_rate) > +{ > + struct meson_ao_cec_g12a_dualdiv_clk *dualdiv_clk = > + hw_to_meson_ao_cec_g12a_dualdiv_clk(hw); > + unsigned long n1; > + u32 reg0, reg1; > + > + regmap_read(dualdiv_clk->regmap, CECB_CLK_CNTL_REG0, ®0); > + regmap_read(dualdiv_clk->regmap, CECB_CLK_CNTL_REG0, ®1); > + > + if (reg1 & CECB_CLK_CNTL_BYPASS_EN) > + return parent_rate; > + > + if (reg0 & CECB_CLK_CNTL_DUAL_EN) { > + unsigned long n2, m1, m2, f1, f2, p1, p2; > + > + n1 = FIELD_GET(CECB_CLK_CNTL_N1, reg0) + 1; > + n2 = FIELD_GET(CECB_CLK_CNTL_N2, reg0) + 1; > + > + m1 = FIELD_GET(CECB_CLK_CNTL_M1, reg1) + 1; > + m2 = FIELD_GET(CECB_CLK_CNTL_M1, reg1) + 1; > + > + f1 = DIV_ROUND_CLOSEST(parent_rate, n1); > + f2 = DIV_ROUND_CLOSEST(parent_rate, n2); > + > + p1 = DIV_ROUND_CLOSEST(100000000 * m1, f1 * (m1 + m2)); > + p2 = DIV_ROUND_CLOSEST(100000000 * m2, f2 * (m1 + m2)); > + > + return DIV_ROUND_UP(100000000, p1 + p2); > + } > + > + n1 = FIELD_GET(CECB_CLK_CNTL_N1, reg0) + 1; > + > + return DIV_ROUND_CLOSEST(parent_rate, n1); > +} > + > +static int meson_ao_cec_g12a_dualdiv_clk_enable(struct clk_hw *hw) > +{ > + struct meson_ao_cec_g12a_dualdiv_clk *dualdiv_clk = > + hw_to_meson_ao_cec_g12a_dualdiv_clk(hw); > + > + > + /* Disable Input & Output */ > + regmap_update_bits(dualdiv_clk->regmap, CECB_CLK_CNTL_REG0, > + CECB_CLK_CNTL_INPUT_EN | CECB_CLK_CNTL_OUTPUT_EN, > + 0); > + > + /* Set N1 & N2 */ > + regmap_update_bits(dualdiv_clk->regmap, CECB_CLK_CNTL_REG0, > + CECB_CLK_CNTL_N1, > + FIELD_PREP(CECB_CLK_CNTL_N1, 733 - 1)); > + > + regmap_update_bits(dualdiv_clk->regmap, CECB_CLK_CNTL_REG0, > + CECB_CLK_CNTL_N2, > + FIELD_PREP(CECB_CLK_CNTL_N2, 732 - 1)); > + > + /* Set M1 & M2 */ > + regmap_update_bits(dualdiv_clk->regmap, CECB_CLK_CNTL_REG1, > + CECB_CLK_CNTL_M1, > + FIELD_PREP(CECB_CLK_CNTL_M1, 8 - 1)); > + > + regmap_update_bits(dualdiv_clk->regmap, CECB_CLK_CNTL_REG1, > + CECB_CLK_CNTL_M2, > + FIELD_PREP(CECB_CLK_CNTL_M2, 11 - 1)); > + > + /* Enable Dual divisor */ > + regmap_update_bits(dualdiv_clk->regmap, CECB_CLK_CNTL_REG0, > + CECB_CLK_CNTL_DUAL_EN, CECB_CLK_CNTL_DUAL_EN); > + > + /* Disable divisor bypass */ > + regmap_update_bits(dualdiv_clk->regmap, CECB_CLK_CNTL_REG1, > + CECB_CLK_CNTL_BYPASS_EN, 0); > + > + /* Enable Input & Output */ > + regmap_update_bits(dualdiv_clk->regmap, CECB_CLK_CNTL_REG0, > + CECB_CLK_CNTL_INPUT_EN | CECB_CLK_CNTL_OUTPUT_EN, > + CECB_CLK_CNTL_INPUT_EN | CECB_CLK_CNTL_OUTPUT_EN); > + > + return 0; > +} > + > +static void meson_ao_cec_g12a_dualdiv_clk_disable(struct clk_hw *hw) > +{ > + struct meson_ao_cec_g12a_dualdiv_clk *dualdiv_clk = > + hw_to_meson_ao_cec_g12a_dualdiv_clk(hw); > + > + regmap_update_bits(dualdiv_clk->regmap, CECB_CLK_CNTL_REG0, > + CECB_CLK_CNTL_INPUT_EN | CECB_CLK_CNTL_OUTPUT_EN, > + 0); > +} > + > +static int meson_ao_cec_g12a_dualdiv_clk_is_enabled(struct clk_hw *hw) > +{ > + struct meson_ao_cec_g12a_dualdiv_clk *dualdiv_clk = > + hw_to_meson_ao_cec_g12a_dualdiv_clk(hw); > + int val; > + > + regmap_read(dualdiv_clk->regmap, CECB_CLK_CNTL_REG0, &val); > + > + return !!(val & (CECB_CLK_CNTL_INPUT_EN | CECB_CLK_CNTL_OUTPUT_EN)); > +} > + > +static const struct clk_ops meson_ao_cec_g12a_dualdiv_clk_ops = { > + .recalc_rate = meson_ao_cec_g12a_dualdiv_clk_recalc_rate, > + .is_enabled = meson_ao_cec_g12a_dualdiv_clk_is_enabled, > + .enable = meson_ao_cec_g12a_dualdiv_clk_enable, > + .disable = meson_ao_cec_g12a_dualdiv_clk_disable, > +}; > + > +static int meson_ao_cec_g12a_setup_clk(struct meson_ao_cec_g12a_device *ao_cec) > +{ > + struct meson_ao_cec_g12a_dualdiv_clk *dualdiv_clk; > + struct device *dev = &ao_cec->pdev->dev; > + struct clk_init_data init; > + const char *parent_name; > + struct clk *clk; > + char *name; > + > + dualdiv_clk = devm_kzalloc(dev, sizeof(*dualdiv_clk), GFP_KERNEL); > + if (!dualdiv_clk) > + return -ENOMEM; > + > + name = kasprintf(GFP_KERNEL, "%s#dualdiv_clk", dev_name(dev)); > + if (!name) > + return -ENOMEM; > + > + parent_name = __clk_get_name(ao_cec->oscin); > + > + init.name = name; > + init.ops = &meson_ao_cec_g12a_dualdiv_clk_ops; > + init.flags = 0; > + init.parent_names = &parent_name; > + init.num_parents = 1; > + dualdiv_clk->regmap = ao_cec->regmap; > + dualdiv_clk->hw.init = &init; > + > + clk = devm_clk_register(dev, &dualdiv_clk->hw); > + kfree(name); > + if (IS_ERR(clk)) { > + dev_err(dev, "failed to register clock\n"); > + return PTR_ERR(clk); > + } > + > + ao_cec->core = clk; > + > + return 0; > +} > + > +static int meson_ao_cec_g12a_read(void *context, unsigned int addr, > + unsigned int *data) > +{ > + struct meson_ao_cec_g12a_device *ao_cec = context; > + u32 reg = FIELD_PREP(CECB_RW_ADDR, addr); > + unsigned long flags; > + int ret = 0; > + > + spin_lock_irqsave(&ao_cec->cec_reg_lock, flags); > + > + ret = regmap_write(ao_cec->regmap, CECB_RW_REG, reg); > + > + ret = regmap_read_poll_timeout(ao_cec->regmap, CECB_RW_REG, reg, > + !(reg & CECB_RW_BUS_BUSY), > + 5, 1000); > + if (ret) > + goto read_out; > + > + ret = regmap_read(ao_cec->regmap, CECB_RW_REG, ®); > + > + *data = FIELD_GET(CECB_RW_RD_DATA, reg); > + > +read_out: > + spin_unlock_irqrestore(&ao_cec->cec_reg_lock, flags); > + > + return ret; > +} > + > +static int meson_ao_cec_g12a_write(void *context, unsigned int addr, > + unsigned int data) > +{ > + struct meson_ao_cec_g12a_device *ao_cec = context; > + unsigned long flags; > + u32 reg = FIELD_PREP(CECB_RW_ADDR, addr) | > + FIELD_PREP(CECB_RW_WR_DATA, data) | > + CECB_RW_WRITE_EN; > + int ret = 0; > + > + spin_lock_irqsave(&ao_cec->cec_reg_lock, flags); > + > + ret = regmap_write(ao_cec->regmap, CECB_RW_REG, reg); > + > + spin_unlock_irqrestore(&ao_cec->cec_reg_lock, flags); > + > + return ret; > +} > + > +static const struct regmap_config meson_ao_cec_g12a_cec_regmap_conf = { > + .reg_bits = 8, > + .val_bits = 8, > + .reg_read = meson_ao_cec_g12a_read, > + .reg_write = meson_ao_cec_g12a_write, > + .max_register = 0xffff, > + .fast_io = true, > +}; > + > +static inline void > +meson_ao_cec_g12a_irq_setup(struct meson_ao_cec_g12a_device *ao_cec, > + bool enable) > +{ > + u32 cfg = CECB_INTR_DONE | CECB_INTR_EOM | CECB_INTR_NACK | > + CECB_INTR_ARB_LOSS | CECB_INTR_INITIATOR_ERR | > + CECB_INTR_FOLLOWER_ERR; > + > + regmap_write(ao_cec->regmap, CECB_INTR_MASKN_REG, > + enable ? cfg : 0); > +} > + > +static void meson_ao_cec_g12a_irq_rx(struct meson_ao_cec_g12a_device *ao_cec) > +{ > + int i, ret = 0; > + u32 val; > + > + ret = regmap_read(ao_cec->regmap_cec, CECB_RX_CNT, &val); > + > + ao_cec->rx_msg.len = val; > + 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++) { > + ret |= regmap_read(ao_cec->regmap_cec, > + CECB_RX_DATA00 + i, &val); > + > + ao_cec->rx_msg.msg[i] = val & 0xff; > + } > + > + ret |= regmap_write(ao_cec->regmap_cec, CECB_LOCK_BUF, 0); > + if (ret) > + return; > + > + cec_received_msg(ao_cec->adap, &ao_cec->rx_msg); > +} > + > +static irqreturn_t meson_ao_cec_g12a_irq(int irq, void *data) > +{ > + struct meson_ao_cec_g12a_device *ao_cec = data; > + u32 stat; > + > + regmap_read(ao_cec->regmap, CECB_INTR_STAT_REG, &stat); > + if (stat) > + return IRQ_WAKE_THREAD; > + > + return IRQ_NONE; > +} > + > +static irqreturn_t meson_ao_cec_g12a_irq_thread(int irq, void *data) > +{ > + struct meson_ao_cec_g12a_device *ao_cec = data; > + u32 stat; > + > + regmap_read(ao_cec->regmap, CECB_INTR_STAT_REG, &stat); > + regmap_write(ao_cec->regmap, CECB_INTR_CLR_REG, stat); > + > + if (stat & CECB_INTR_DONE) > + cec_transmit_attempt_done(ao_cec->adap, CEC_TX_STATUS_OK); > + > + if (stat & CECB_INTR_EOM) > + meson_ao_cec_g12a_irq_rx(ao_cec); > + > + if (stat & CECB_INTR_NACK) > + cec_transmit_attempt_done(ao_cec->adap, CEC_TX_STATUS_NACK); > + > + if (stat & CECB_INTR_ARB_LOSS) { > + regmap_write(ao_cec->regmap_cec, CECB_TX_CNT, 0); > + regmap_update_bits(ao_cec->regmap_cec, CECB_CTRL, > + CECB_CTRL_SEND | CECB_CTRL_TYPE, 0); > + cec_transmit_attempt_done(ao_cec->adap, CEC_TX_STATUS_ARB_LOST); > + } > + > + if (stat & CECB_INTR_INITIATOR_ERR) > + cec_transmit_attempt_done(ao_cec->adap, CEC_TX_STATUS_NACK); > + > + if (stat & CECB_INTR_FOLLOWER_ERR) { > + regmap_write(ao_cec->regmap_cec, CECB_LOCK_BUF, 0); > + cec_transmit_attempt_done(ao_cec->adap, CEC_TX_STATUS_NACK); Any idea what CECB_INTR_INITIATOR_ERR and CECB_INTR_FOLLOWER_ERR actually mean? I.e. is returning NACK right here, or would TX_STATUS_ERROR be a better choice? I invented that status precisely for cases where there is an error, but we don't know what it means. Are you sure that CECB_INTR_FOLLOWER_ERR applies to a transmit and not a receive? 'Follower' suggests that some error occurred while receiving a message. If I am right, then just ignore it. Regarding CECB_INTR_INITIATOR_ERR: I suspect that this indicates a LOW DRIVE error situation, in which case you'd return that transmit status. > + } > + > + return IRQ_HANDLED; > +} > + > +static int > +meson_ao_cec_g12a_set_log_addr(struct cec_adapter *adap, u8 logical_addr) > +{ > + struct meson_ao_cec_g12a_device *ao_cec = adap->priv; > + int ret = 0; > + > + if (logical_addr == CEC_LOG_ADDR_INVALID) { > + ret = regmap_write(ao_cec->regmap_cec, CECB_LADD_LOW, 0); > + ret = regmap_write(ao_cec->regmap_cec, CECB_LADD_HIGH, 0); Just ignore the error codes and return 0 here. The CEC core will WARN if this function returns anything other than 0 when invalidating the logical addresses. It assumes this will always succeed. > + } else if (logical_addr < 8) { > + ret = regmap_update_bits(ao_cec->regmap_cec, CECB_LADD_LOW, > + BIT(logical_addr), > + BIT(logical_addr)); > + } else { > + ret = regmap_update_bits(ao_cec->regmap_cec, CECB_LADD_HIGH, > + BIT(logical_addr - 8), > + BIT(logical_addr - 8)); > + } > + > + /* Always set Broadcast/Unregistered 15 address */ > + ret |= regmap_update_bits(ao_cec->regmap_cec, CECB_LADD_HIGH, I'd just do: if (!ret) ret = regmap_... Error codes are not a bitmask after all. I see that elsewhere as well. It's OK to use |=, but then when you return from the function you would have to do something like: return ret ? -EIO : 0; Regards, Hans > + BIT(CEC_LOG_ADDR_UNREGISTERED - 8), > + BIT(CEC_LOG_ADDR_UNREGISTERED - 8)); > + > + return ret; > +} > + > +static int meson_ao_cec_g12a_transmit(struct cec_adapter *adap, u8 attempts, > + u32 signal_free_time, struct cec_msg *msg) > +{ > + struct meson_ao_cec_g12a_device *ao_cec = adap->priv; > + unsigned int type; > + int ret = 0; > + u32 val; > + int i; > + > + /* Check if RX is in progress */ > + ret = regmap_read(ao_cec->regmap_cec, CECB_LOCK_BUF, &val); > + if (ret) > + return ret; > + if (val & CECB_LOCK_BUF_EN) > + return -EBUSY; > + > + /* Check if TX Busy */ > + ret = regmap_read(ao_cec->regmap_cec, CECB_CTRL, &val); > + if (ret) > + return ret; > + if (val & CECB_CTRL_SEND) > + return -EBUSY; > + > + switch (signal_free_time) { > + case CEC_SIGNAL_FREE_TIME_RETRY: > + type = CECB_CTRL_TYPE_RETRY; > + break; > + case CEC_SIGNAL_FREE_TIME_NEXT_XFER: > + type = CECB_CTRL_TYPE_NEXT; > + break; > + case CEC_SIGNAL_FREE_TIME_NEW_INITIATOR: > + default: > + type = CECB_CTRL_TYPE_NEW; > + break; > + } > + > + for (i = 0; i < msg->len; i++) > + ret |= regmap_write(ao_cec->regmap_cec, CECB_TX_DATA00 + i, > + msg->msg[i]); > + > + ret |= regmap_write(ao_cec->regmap_cec, CECB_TX_CNT, msg->len); > + ret = regmap_update_bits(ao_cec->regmap_cec, CECB_CTRL, > + CECB_CTRL_SEND | > + CECB_CTRL_TYPE, > + CECB_CTRL_SEND | > + FIELD_PREP(CECB_CTRL_TYPE, type)); > + > + return ret; > +} > + > +static int meson_ao_cec_g12a_adap_enable(struct cec_adapter *adap, bool enable) > +{ > + struct meson_ao_cec_g12a_device *ao_cec = adap->priv; > + > + meson_ao_cec_g12a_irq_setup(ao_cec, false); > + > + regmap_update_bits(ao_cec->regmap, CECB_GEN_CNTL_REG, > + CECB_GEN_CNTL_RESET, CECB_GEN_CNTL_RESET); > + > + if (!enable) > + return 0; > + > + /* Setup Filter */ > + regmap_update_bits(ao_cec->regmap, CECB_GEN_CNTL_REG, > + CECB_GEN_CNTL_FILTER_TICK_SEL | > + CECB_GEN_CNTL_FILTER_DEL, > + FIELD_PREP(CECB_GEN_CNTL_FILTER_TICK_SEL, > + CECB_GEN_CNTL_FILTER_TICK_1US) | > + FIELD_PREP(CECB_GEN_CNTL_FILTER_DEL, 7)); > + > + /* Enable System Clock */ > + regmap_update_bits(ao_cec->regmap, CECB_GEN_CNTL_REG, > + CECB_GEN_CNTL_SYS_CLK_EN, > + CECB_GEN_CNTL_SYS_CLK_EN); > + > + /* Enable gated clock (Normal mode). */ > + regmap_update_bits(ao_cec->regmap, CECB_GEN_CNTL_REG, > + CECB_GEN_CNTL_CLK_CTRL_MASK, > + FIELD_PREP(CECB_GEN_CNTL_CLK_CTRL_MASK, > + CECB_GEN_CNTL_CLK_ENABLE)); > + > + /* Release Reset */ > + regmap_update_bits(ao_cec->regmap, CECB_GEN_CNTL_REG, > + CECB_GEN_CNTL_RESET, 0); > + > + meson_ao_cec_g12a_irq_setup(ao_cec, true); > + > + return 0; > +} > + > +static const struct cec_adap_ops meson_ao_cec_g12a_ops = { > + .adap_enable = meson_ao_cec_g12a_adap_enable, > + .adap_log_addr = meson_ao_cec_g12a_set_log_addr, > + .adap_transmit = meson_ao_cec_g12a_transmit, > +}; > + > +static int meson_ao_cec_g12a_probe(struct platform_device *pdev) > +{ > + struct meson_ao_cec_g12a_device *ao_cec; > + struct platform_device *hdmi_dev; > + struct device_node *np; > + struct resource *res; > + void __iomem *base; > + 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); > + of_node_put(np); > + if (hdmi_dev == NULL) > + return -EPROBE_DEFER; > + > + put_device(&hdmi_dev->dev); > + 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->pdev = pdev; > + > + ao_cec->notify = cec_notifier_get(&hdmi_dev->dev); > + if (!ao_cec->notify) > + return -ENOMEM; > + > + ao_cec->adap = cec_allocate_adapter(&meson_ao_cec_g12a_ops, ao_cec, > + "meson_g12a_ao_cec", > + CEC_CAP_DEFAULTS, > + CEC_MAX_LOG_ADDRS); > + 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); > + base = devm_ioremap_resource(&pdev->dev, res); > + if (IS_ERR(base)) { > + ret = PTR_ERR(base); > + goto out_probe_adapter; > + } > + > + ao_cec->regmap = devm_regmap_init_mmio(&pdev->dev, base, > + &meson_ao_cec_g12a_regmap_conf); > + if (IS_ERR(ao_cec->regmap)) { > + ret = PTR_ERR(ao_cec->regmap); > + goto out_probe_adapter; > + } > + > + ao_cec->regmap_cec = devm_regmap_init(&pdev->dev, NULL, ao_cec, > + &meson_ao_cec_g12a_cec_regmap_conf); > + if (IS_ERR(ao_cec->regmap_cec)) { > + ret = PTR_ERR(ao_cec->regmap_cec); > + goto out_probe_adapter; > + } > + > + irq = platform_get_irq(pdev, 0); > + ret = devm_request_threaded_irq(&pdev->dev, irq, > + meson_ao_cec_g12a_irq, > + meson_ao_cec_g12a_irq_thread, > + 0, NULL, ao_cec); > + if (ret) { > + dev_err(&pdev->dev, "irq request failed\n"); > + goto out_probe_adapter; > + } > + > + ao_cec->oscin = devm_clk_get(&pdev->dev, "oscin"); > + if (IS_ERR(ao_cec->oscin)) { > + dev_err(&pdev->dev, "oscin clock request failed\n"); > + ret = PTR_ERR(ao_cec->oscin); > + goto out_probe_adapter; > + } > + > + ret = meson_ao_cec_g12a_setup_clk(ao_cec); > + if (ret) > + goto out_probe_clk; > + > + ret = clk_prepare_enable(ao_cec->core); > + if (ret) { > + dev_err(&pdev->dev, "core clock enable failed\n"); > + goto out_probe_clk; > + } > + > + device_reset_optional(&pdev->dev); > + > + 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_core_clk; > + } > + > + /* Setup Hardware */ > + regmap_write(ao_cec->regmap, CECB_GEN_CNTL_REG, CECB_GEN_CNTL_RESET); > + > + cec_register_cec_notifier(ao_cec->adap, ao_cec->notify); > + > + return 0; > + > +out_probe_core_clk: > + clk_disable_unprepare(ao_cec->core); > + > +out_probe_clk: > + clk_disable_unprepare(ao_cec->oscin); > + > +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_g12a_remove(struct platform_device *pdev) > +{ > + struct meson_ao_cec_g12a_device *ao_cec = platform_get_drvdata(pdev); > + > + clk_disable_unprepare(ao_cec->oscin); > + > + cec_unregister_adapter(ao_cec->adap); > + > + cec_notifier_put(ao_cec->notify); > + > + return 0; > +} > + > +static const struct of_device_id meson_ao_cec_g12a_of_match[] = { > + { .compatible = "amlogic,meson-g12a-ao-cec-b", }, > + { /* sentinel */ } > +}; > +MODULE_DEVICE_TABLE(of, meson_ao_cec_g12a_of_match); > + > +static struct platform_driver meson_ao_cec_g12a_driver = { > + .probe = meson_ao_cec_g12a_probe, > + .remove = meson_ao_cec_g12a_remove, > + .driver = { > + .name = "meson-ao-cec-g12a", > + .of_match_table = of_match_ptr(meson_ao_cec_g12a_of_match), > + }, > +}; > + > +module_platform_driver(meson_ao_cec_g12a_driver); > + > +MODULE_DESCRIPTION("Meson AO CEC G12A Controller driver"); > +MODULE_AUTHOR("Neil Armstrong <narmstrong@xxxxxxxxxxxx>"); > +MODULE_LICENSE("GPL"); > Regards, Hans