* [PATCH v5 0/2] CH7322 CEC controller driver
@ 2020-06-15 19:38 Jeff Chase
2020-06-15 19:38 ` [PATCH v5 1/2] dt-bindings: Add ch7322 media i2c device Jeff Chase
2020-06-15 19:38 ` [PATCH v5 2/2] media: cec: i2c: ch7322: Add ch7322 CEC controller driver Jeff Chase
0 siblings, 2 replies; 13+ messages in thread
From: Jeff Chase @ 2020-06-15 19:38 UTC (permalink / raw)
To: linux-media; +Cc: mchehab, hverkuil-cisco, robh+dt, devicetree, Jeff Chase
Add device driver and device tree bindings for a Chrontel CEC
conroller. This is an I2C device that can send and receive CEC
messages.
Changes from v4:
- rebase onto master (vendor-prefixes.yaml already updated)
Changes from v3:
- address devicetree comments
- revert interrupt mask/unmask in cec adapter enable
(the driver relies on the interrupt to get the physical address)
Changes from v2:
- fix formatting errors
- mask and unmask interrupt in cec adapter enable
Changes from v1:
- fix formatpatch.pl --strict errors
- additional comments
- enable and program logical address register
- add flags to aid interpreting transmit done status
- move ch7322 out of devicetree trivial devices
Jeff Chase (2):
dt-bindings: Add ch7322 media i2c device
media: cec: i2c: ch7322: Add ch7322 CEC controller driver
.../bindings/media/i2c/chrontel,ch7322.yaml | 67 +++
MAINTAINERS | 8 +
drivers/media/cec/Kconfig | 1 +
drivers/media/cec/Makefile | 2 +-
drivers/media/cec/i2c/Kconfig | 14 +
drivers/media/cec/i2c/Makefile | 5 +
drivers/media/cec/i2c/ch7322.c | 503 ++++++++++++++++++
7 files changed, 599 insertions(+), 1 deletion(-)
create mode 100644 Documentation/devicetree/bindings/media/i2c/chrontel,ch7322.yaml
create mode 100644 drivers/media/cec/i2c/Kconfig
create mode 100644 drivers/media/cec/i2c/Makefile
create mode 100644 drivers/media/cec/i2c/ch7322.c
--
2.27.0.290.gba653c62da-goog
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v5 1/2] dt-bindings: Add ch7322 media i2c device
2020-06-15 19:38 [PATCH v5 0/2] CH7322 CEC controller driver Jeff Chase
@ 2020-06-15 19:38 ` Jeff Chase
2020-06-17 20:26 ` Rob Herring
2020-06-15 19:38 ` [PATCH v5 2/2] media: cec: i2c: ch7322: Add ch7322 CEC controller driver Jeff Chase
1 sibling, 1 reply; 13+ messages in thread
From: Jeff Chase @ 2020-06-15 19:38 UTC (permalink / raw)
To: linux-media; +Cc: mchehab, hverkuil-cisco, robh+dt, devicetree, Jeff Chase
The ch7322 is a Chrontel CEC controller.
Signed-off-by: Jeff Chase <jnchase@google.com>
---
.../bindings/media/i2c/chrontel,ch7322.yaml | 67 +++++++++++++++++++
MAINTAINERS | 7 ++
2 files changed, 74 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/i2c/chrontel,ch7322.yaml
diff --git a/Documentation/devicetree/bindings/media/i2c/chrontel,ch7322.yaml b/Documentation/devicetree/bindings/media/i2c/chrontel,ch7322.yaml
new file mode 100644
index 000000000000..daa2869377c5
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/chrontel,ch7322.yaml
@@ -0,0 +1,67 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/media/i2c/chrontel,ch7322.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Chrontel HDMI-CEC Controller
+
+maintainers:
+ - Jeff Chase <jnchase@google.com>
+
+description:
+ The Chrontel CH7322 is a discrete HDMI-CEC controller. It is
+ programmable through I2C and drives a single CEC line.
+
+properties:
+ compatible:
+ const: chrontel,ch7322
+
+ reg:
+ description: I2C device address
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ reset-gpios:
+ description:
+ Reference to the GPIO connected to the RESET pin, if any. This
+ pin is active-low.
+ maxItems: 1
+
+ standby-gpios:
+ description:
+ Reference to the GPIO connected to the OE pin, if any. When low
+ the device will respond to power status requests with "standby"
+ if in auto mode.
+ maxItems: 1
+
+ # see ../cec.txt
+ hdmi-phandle:
+ description: phandle to the HDMI controller
+
+required:
+ - compatible
+ - reg
+ - interrupts
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ ch7322@75 {
+ compatible = "chrontel,ch7322";
+ reg = <0x75>;
+ interrupts = <47 IRQ_TYPE_EDGE_RISING>;
+ standby-gpios = <&gpio 16 GPIO_ACTIVE_LOW>;
+ reset-gpios = <&gpio 15 GPIO_ACTIVE_LOW>;
+ hdmi-phandle = <&hdmi>;
+ };
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index 68f21d46614c..c1c17ca17830 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4101,6 +4101,13 @@ F: drivers/power/supply/cros_usbpd-charger.c
N: cros_ec
N: cros-ec
+CHRONTEL CH7322 CEC DRIVER
+M: Jeff Chase <jnchase@google.com>
+L: linux-media@vger.kernel.org
+S: Maintained
+T: git git://linuxtv.org/media_tree.git
+F: Documentation/devicetree/bindings/media/i2c/chontel,ch7322.yaml
+
CIRRUS LOGIC AUDIO CODEC DRIVERS
M: James Schulman <james.schulman@cirrus.com>
M: David Rhodes <david.rhodes@cirrus.com>
--
2.27.0.290.gba653c62da-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v5 2/2] media: cec: i2c: ch7322: Add ch7322 CEC controller driver
2020-06-15 19:38 [PATCH v5 0/2] CH7322 CEC controller driver Jeff Chase
2020-06-15 19:38 ` [PATCH v5 1/2] dt-bindings: Add ch7322 media i2c device Jeff Chase
@ 2020-06-15 19:38 ` Jeff Chase
2020-06-18 7:51 ` Hans Verkuil
1 sibling, 1 reply; 13+ messages in thread
From: Jeff Chase @ 2020-06-15 19:38 UTC (permalink / raw)
To: linux-media; +Cc: mchehab, hverkuil-cisco, robh+dt, devicetree, Jeff Chase
Add a CEC device driver for the Chrontel ch7322 CEC conroller.
This is an I2C device capable of sending and receiving CEC messages.
Signed-off-by: Jeff Chase <jnchase@google.com>
---
MAINTAINERS | 1 +
drivers/media/cec/Kconfig | 1 +
drivers/media/cec/Makefile | 2 +-
drivers/media/cec/i2c/Kconfig | 14 +
drivers/media/cec/i2c/Makefile | 5 +
drivers/media/cec/i2c/ch7322.c | 503 +++++++++++++++++++++++++++++++++
6 files changed, 525 insertions(+), 1 deletion(-)
create mode 100644 drivers/media/cec/i2c/Kconfig
create mode 100644 drivers/media/cec/i2c/Makefile
create mode 100644 drivers/media/cec/i2c/ch7322.c
diff --git a/MAINTAINERS b/MAINTAINERS
index c1c17ca17830..e2341eaaaf5e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4107,6 +4107,7 @@ L: linux-media@vger.kernel.org
S: Maintained
T: git git://linuxtv.org/media_tree.git
F: Documentation/devicetree/bindings/media/i2c/chontel,ch7322.yaml
+F: drivers/media/cec/i2c/ch7322.c
CIRRUS LOGIC AUDIO CODEC DRIVERS
M: James Schulman <james.schulman@cirrus.com>
diff --git a/drivers/media/cec/Kconfig b/drivers/media/cec/Kconfig
index 7e830444bdbb..9ba3a00dce31 100644
--- a/drivers/media/cec/Kconfig
+++ b/drivers/media/cec/Kconfig
@@ -33,6 +33,7 @@ menuconfig MEDIA_CEC_SUPPORT
adapter that supports HDMI CEC.
if MEDIA_CEC_SUPPORT
+source "drivers/media/cec/i2c/Kconfig"
source "drivers/media/cec/platform/Kconfig"
source "drivers/media/cec/usb/Kconfig"
endif
diff --git a/drivers/media/cec/Makefile b/drivers/media/cec/Makefile
index 74e80e1b3571..23539339bc81 100644
--- a/drivers/media/cec/Makefile
+++ b/drivers/media/cec/Makefile
@@ -1,2 +1,2 @@
# SPDX-License-Identifier: GPL-2.0
-obj-y += core/ platform/ usb/
+obj-y += core/ i2c/ platform/ usb/
diff --git a/drivers/media/cec/i2c/Kconfig b/drivers/media/cec/i2c/Kconfig
new file mode 100644
index 000000000000..e445ca2110b3
--- /dev/null
+++ b/drivers/media/cec/i2c/Kconfig
@@ -0,0 +1,14 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# I2C drivers
+
+config CEC_CH7322
+ tristate "Chrontel CH7322 CEC controller"
+ select I2C
+ select REGMAP_I2C
+ select CEC_CORE
+ help
+ This is a driver for the Chrontel CH7322 CEC controller. It uses the
+ generic CEC framework interface.
+ CEC bus is present in the HDMI connector and enables communication
+ between compatible devices.
diff --git a/drivers/media/cec/i2c/Makefile b/drivers/media/cec/i2c/Makefile
new file mode 100644
index 000000000000..d7496dfd0fa4
--- /dev/null
+++ b/drivers/media/cec/i2c/Makefile
@@ -0,0 +1,5 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Makefile for the CEC I2C device drivers.
+#
+obj-$(CONFIG_CEC_CH7322) += ch7322.o
diff --git a/drivers/media/cec/i2c/ch7322.c b/drivers/media/cec/i2c/ch7322.c
new file mode 100644
index 000000000000..4e6976a51774
--- /dev/null
+++ b/drivers/media/cec/i2c/ch7322.c
@@ -0,0 +1,503 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for the Chrontel CH7322 CEC Controller
+ *
+ * Copyright 2020 Google LLC.
+ */
+
+/*
+ * Notes
+ *
+ * - This device powers on in Auto Mode which has limited functionality. This
+ * driver disables Auto Mode when it attaches.
+ *
+ */
+
+#include <linux/cec.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/regmap.h>
+#include <media/cec.h>
+
+#define CH7322_WRITE 0x00
+#define CH7322_WRITE_MSENT 0x80
+#define CH7322_WRITE_BOK 0x40
+#define CH7322_WRITE_NMASK 0x0f
+
+/* Write buffer is 0x01-0x10 */
+#define CH7322_WRBUF 0x01
+#define CH7322_WRBUF_LEN 0x10
+
+#define CH7322_READ 0x40
+#define CH7322_READ_NRDT 0x80
+#define CH7322_READ_MSENT 0x20
+#define CH7322_READ_NMASK 0x0f
+
+/* Read buffer is 0x41-0x50 */
+#define CH7322_RDBUF 0x41
+#define CH7322_RDBUF_LEN 0x10
+
+#define CH7322_MODE 0x11
+#define CH7322_MODE_AUTO 0x78
+#define CH7322_MODE_SW 0xb5
+
+#define CH7322_RESET 0x12
+#define CH7322_RESET_RST 0x00
+
+#define CH7322_POWER 0x13
+#define CH7322_POWER_FPD 0x04
+
+#define CH7322_CFG0 0x17
+#define CH7322_CFG0_EOBEN 0x40
+#define CH7322_CFG0_PEOB 0x20
+#define CH7322_CFG0_CLRSPP 0x10
+#define CH7322_CFG0_FLOW 0x08
+
+#define CH7322_CFG1 0x1a
+#define CH7322_CFG1_STDBYO 0x04
+#define CH7322_CFG1_HPBP 0x02
+#define CH7322_CFG1_PIO 0x01
+
+#define CH7322_INTCTL 0x1b
+#define CH7322_INTCTL_INTPB 0x80
+#define CH7322_INTCTL_STDBY 0x40
+#define CH7322_INTCTL_HPDFALL 0x20
+#define CH7322_INTCTL_HPDRISE 0x10
+#define CH7322_INTCTL_RXMSG 0x08
+#define CH7322_INTCTL_TXMSG 0x04
+#define CH7322_INTCTL_NEWPHA 0x02
+#define CH7322_INTCTL_ERROR 0x01
+
+#define CH7322_DVCLKFNH 0x1d
+#define CH7322_DVCLKFNL 0x1e
+
+#define CH7322_CTL 0x31
+#define CH7322_CTL_FSTDBY 0x80
+#define CH7322_CTL_PLSEN 0x40
+#define CH7322_CTL_PLSPB 0x20
+#define CH7322_CTL_SPADL 0x10
+#define CH7322_CTL_HINIT 0x08
+#define CH7322_CTL_WPHYA 0x04
+#define CH7322_CTL_H1T 0x02
+#define CH7322_CTL_S1T 0x01
+
+#define CH7322_PAWH 0x32
+#define CH7322_PAWL 0x33
+
+#define CH7322_ADDLW 0x34
+#define CH7322_ADDLW_MASK 0xf0
+
+#define CH7322_ADDLR 0x3d
+#define CH7322_ADDLR_HPD 0x80
+#define CH7322_ADDLR_MASK 0x0f
+
+#define CH7322_INTDATA 0x3e
+#define CH7322_INTDATA_MODE 0x80
+#define CH7322_INTDATA_STDBY 0x40
+#define CH7322_INTDATA_HPDFALL 0x20
+#define CH7322_INTDATA_HPDRISE 0x10
+#define CH7322_INTDATA_RXMSG 0x08
+#define CH7322_INTDATA_TXMSG 0x04
+#define CH7322_INTDATA_NEWPHA 0x02
+#define CH7322_INTDATA_ERROR 0x01
+
+#define CH7322_EVENT 0x3F
+#define CH7322_EVENT_TXERR 0x80
+#define CH7322_EVENT_HRST 0x40
+#define CH7322_EVENT_HFST 0x20
+#define CH7322_EVENT_PHACHG 0x10
+#define CH7322_EVENT_ACTST 0x08
+#define CH7322_EVENT_PHARDY 0x04
+#define CH7322_EVENT_BSOK 0x02
+#define CH7322_EVENT_ERRADCF 0x01
+
+#define CH7322_DID 0x51
+#define CH7322_DID_CH7322 0x5b
+#define CH7322_DID_CH7323 0x5f
+
+#define CH7322_REVISIONID 0x52
+
+#define CH7322_PARH 0x53
+#define CH7322_PARL 0x54
+
+#define CH7322_IOCFG2 0x75
+#define CH7322_IOCFG_CIO 0x80
+#define CH7322_IOCFG_IOCFGMASK 0x78
+#define CH7322_IOCFG_AUDIO 0x04
+#define CH7322_IOCFG_SPAMST 0x02
+#define CH7322_IOCFG_SPAMSP 0x01
+
+#define CH7322_CTL3 0x7b
+#define CH7322_CTL3_SWENA 0x80
+#define CH7322_CTL3_FC_INIT 0x40
+#define CH7322_CTL3_SML_FL 0x20
+#define CH7322_CTL3_SM_RDST 0x10
+#define CH7322_CTL3_SPP_CIAH 0x08
+#define CH7322_CTL3_SPP_CIAL 0x04
+#define CH7322_CTL3_SPP_ACTH 0x02
+#define CH7322_CTL3_SPP_ACTL 0x01
+
+/* BOK status means NACK */
+#define CH7322_TX_FLAG_NACK BIT(0)
+/* Device will retry automatically */
+#define CH7322_TX_FLAG_RETRY BIT(1)
+
+struct ch7322 {
+ struct i2c_client *i2c;
+ struct regmap *regmap;
+ struct cec_adapter *cec;
+ struct mutex mutex; /* device access mutex */
+ u8 tx_flags;
+};
+
+static const struct regmap_config ch7322_regmap = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .max_register = 0x7f,
+ .disable_locking = true,
+
+};
+
+static int ch7322_send_message(struct ch7322 *ch7322, const struct cec_msg *msg)
+{
+ unsigned int val;
+ unsigned int len = msg->len;
+ int ret;
+ int i;
+
+ WARN_ON(!mutex_is_locked(&ch7322->mutex));
+
+ if (len > CH7322_WRBUF_LEN || len < 1)
+ return -EINVAL;
+
+ ret = regmap_read(ch7322->regmap, CH7322_WRITE, &val);
+ if (ret)
+ return ret;
+
+ /* Buffer not ready */
+ if (!(val & CH7322_WRITE_MSENT))
+ return -EBUSY;
+
+ if (cec_msg_opcode(msg) == -1 &&
+ cec_msg_initiator(msg) == cec_msg_destination(msg)) {
+ ch7322->tx_flags = CH7322_TX_FLAG_NACK | CH7322_TX_FLAG_RETRY;
+ } else if (cec_msg_is_broadcast(msg)) {
+ ch7322->tx_flags = CH7322_TX_FLAG_NACK;
+ } else {
+ ch7322->tx_flags = CH7322_TX_FLAG_RETRY;
+ }
+
+ ret = regmap_write(ch7322->regmap, CH7322_WRITE, len - 1);
+ if (ret)
+ return ret;
+
+ for (i = 0; i < len; i++) {
+ ret = regmap_write(ch7322->regmap,
+ CH7322_WRBUF + i, msg->msg[i]);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+static int ch7322_receive_message(struct ch7322 *ch7322, struct cec_msg *msg)
+{
+ unsigned int val;
+ int ret = 0;
+ int i;
+
+ WARN_ON(!mutex_is_locked(&ch7322->mutex));
+
+ ret = regmap_read(ch7322->regmap, CH7322_READ, &val);
+ if (ret)
+ return ret;
+
+ /* Message not ready */
+ if (!(val & CH7322_READ_NRDT))
+ return -EIO;
+
+ msg->len = (val & CH7322_READ_NMASK) + 1;
+
+ /* Read entire RDBUF to clear state */
+ for (i = 0; i < CH7322_RDBUF_LEN; i++) {
+ ret = regmap_read(ch7322->regmap, CH7322_RDBUF + i, &val);
+ if (ret)
+ return ret;
+ msg->msg[i] = (u8)val;
+ }
+
+ return 0;
+}
+
+static void ch7322_tx_done(struct ch7322 *ch7322)
+{
+ int ret;
+ unsigned int val;
+ u8 status, flags;
+
+ mutex_lock(&ch7322->mutex);
+ ret = regmap_read(ch7322->regmap, CH7322_WRITE, &val);
+ flags = ch7322->tx_flags;
+ mutex_unlock(&ch7322->mutex);
+
+ /*
+ * The device returns a one-bit OK status which usually means ACK but
+ * actually means NACK when sending a logical address query or a
+ * broadcast.
+ */
+ if (ret)
+ status = CEC_TX_STATUS_ERROR;
+ else if ((val & CH7322_WRITE_BOK) && (flags & CH7322_TX_FLAG_NACK))
+ status = CEC_TX_STATUS_NACK;
+ else if (val & CH7322_WRITE_BOK)
+ status = CEC_TX_STATUS_OK;
+ else if (flags & CH7322_TX_FLAG_NACK)
+ status = CEC_TX_STATUS_OK;
+ else
+ status = CEC_TX_STATUS_NACK;
+
+ if (status == CEC_TX_STATUS_NACK && (flags & CH7322_TX_FLAG_RETRY))
+ status |= CEC_TX_STATUS_MAX_RETRIES;
+
+ cec_transmit_attempt_done(ch7322->cec, status);
+}
+
+static void ch7322_rx_done(struct ch7322 *ch7322)
+{
+ struct cec_msg msg;
+ int ret;
+
+ mutex_lock(&ch7322->mutex);
+ ret = ch7322_receive_message(ch7322, &msg);
+ mutex_unlock(&ch7322->mutex);
+
+ if (ret)
+ dev_err(&ch7322->i2c->dev, "cec receive error: %d\n", ret);
+ else
+ cec_received_msg(ch7322->cec, &msg);
+}
+
+/*
+ * This device can either monitor the DDC lines to obtain the physical address
+ * or it can allow the host to program it. This driver lets the device obtain
+ * it.
+ */
+static void ch7322_phys_addr(struct ch7322 *ch7322)
+{
+ unsigned int pah, pal;
+ int ret = 0;
+
+ mutex_lock(&ch7322->mutex);
+ ret |= regmap_read(ch7322->regmap, CH7322_PARH, &pah);
+ ret |= regmap_read(ch7322->regmap, CH7322_PARL, &pal);
+ mutex_unlock(&ch7322->mutex);
+
+ if (ret)
+ dev_err(&ch7322->i2c->dev, "phys addr error\n");
+ else
+ cec_s_phys_addr(ch7322->cec, pal | (pah << 8), false);
+}
+
+static void ch7322_handle_events(struct ch7322 *ch7322)
+{
+ unsigned int data = 0;
+
+ mutex_lock(&ch7322->mutex);
+ regmap_read(ch7322->regmap, CH7322_INTDATA, &data);
+ regmap_write(ch7322->regmap, CH7322_INTDATA, data);
+ mutex_unlock(&ch7322->mutex);
+
+ if (data & CH7322_INTDATA_HPDFALL)
+ cec_phys_addr_invalidate(ch7322->cec);
+
+ if (data & CH7322_INTDATA_TXMSG)
+ ch7322_tx_done(ch7322);
+
+ if (data & CH7322_INTDATA_RXMSG)
+ ch7322_rx_done(ch7322);
+
+ if (data & CH7322_INTDATA_NEWPHA)
+ ch7322_phys_addr(ch7322);
+
+ if (data & CH7322_INTDATA_ERROR)
+ dev_dbg(&ch7322->i2c->dev, "unknown error\n");
+}
+
+static irqreturn_t ch7322_irq(int irq, void *dev)
+{
+ struct ch7322 *ch7322 = dev;
+
+ ch7322_handle_events(ch7322);
+
+ return IRQ_HANDLED;
+}
+
+/* This device is always enabled */
+static int ch7322_cec_adap_enable(struct cec_adapter *adap, bool enable)
+{
+ return 0;
+}
+
+static int ch7322_cec_adap_log_addr(struct cec_adapter *adap, u8 log_addr)
+{
+ struct ch7322 *ch7322 = cec_get_drvdata(adap);
+ int ret;
+
+ mutex_lock(&ch7322->mutex);
+ ret = regmap_update_bits(ch7322->regmap, CH7322_ADDLW,
+ CH7322_ADDLW_MASK, log_addr << 4);
+ mutex_unlock(&ch7322->mutex);
+
+ return ret;
+}
+
+static int ch7322_cec_adap_transmit(struct cec_adapter *adap, u8 attempts,
+ u32 signal_free_time, struct cec_msg *msg)
+{
+ struct ch7322 *ch7322 = cec_get_drvdata(adap);
+ int ret;
+
+ mutex_lock(&ch7322->mutex);
+ ret = ch7322_send_message(ch7322, msg);
+ mutex_unlock(&ch7322->mutex);
+
+ return ret;
+}
+
+static const struct cec_adap_ops ch7322_cec_adap_ops = {
+ .adap_enable = ch7322_cec_adap_enable,
+ .adap_log_addr = ch7322_cec_adap_log_addr,
+ .adap_transmit = ch7322_cec_adap_transmit,
+};
+
+static int ch7322_probe(struct i2c_client *client)
+{
+ struct ch7322 *ch7322;
+ int ret;
+ unsigned int val;
+
+ ch7322 = devm_kzalloc(&client->dev, sizeof(*ch7322), GFP_KERNEL);
+ if (!ch7322)
+ return -ENOMEM;
+
+ ch7322->regmap = devm_regmap_init_i2c(client, &ch7322_regmap);
+ if (IS_ERR(ch7322->regmap))
+ return PTR_ERR(ch7322->regmap);
+
+ ret = regmap_read(ch7322->regmap, CH7322_DID, &val);
+ if (ret)
+ return ret;
+
+ if (val != CH7322_DID_CH7322)
+ return -ENOTSUPP;
+
+ mutex_init(&ch7322->mutex);
+ ch7322->i2c = client;
+ ch7322->tx_flags = 0;
+
+ i2c_set_clientdata(client, ch7322);
+
+ /* Disable auto mode */
+ ret = regmap_write(ch7322->regmap, CH7322_MODE, CH7322_MODE_SW);
+ if (ret)
+ goto err_mutex;
+
+ /* Enable logical address register */
+ ret = regmap_update_bits(ch7322->regmap, CH7322_CTL,
+ CH7322_CTL_SPADL, CH7322_CTL_SPADL);
+ if (ret)
+ goto err_mutex;
+
+ ch7322->cec = cec_allocate_adapter(&ch7322_cec_adap_ops, ch7322,
+ dev_name(&client->dev),
+ CEC_CAP_DEFAULTS, 1);
+
+ if (IS_ERR(ch7322->cec)) {
+ ret = PTR_ERR(ch7322->cec);
+ goto err_mutex;
+ }
+
+ ret = cec_register_adapter(ch7322->cec, &client->dev);
+ if (ret) {
+ cec_delete_adapter(ch7322->cec);
+ goto err_mutex;
+ }
+
+ /* Configure and mask interrupt */
+ ret = regmap_write(ch7322->regmap, CH7322_CFG1, 0);
+ if (ret)
+ goto err_cec;
+
+ ret = regmap_write(ch7322->regmap, CH7322_INTCTL, CH7322_INTCTL_INTPB);
+ if (ret)
+ goto err_cec;
+
+ ret = devm_request_threaded_irq(&client->dev, client->irq, NULL,
+ ch7322_irq,
+ IRQF_ONESHOT | IRQF_TRIGGER_RISING,
+ client->name, ch7322);
+ if (ret)
+ goto err_cec;
+
+ ch7322_handle_events(ch7322);
+
+ /* Unmask interrupt */
+ mutex_lock(&ch7322->mutex);
+ ret = regmap_write(ch7322->regmap, CH7322_INTCTL, 0xff);
+ mutex_unlock(&ch7322->mutex);
+
+ if (ret)
+ goto err_cec;
+
+ dev_info(&client->dev, "device registered\n");
+
+ return 0;
+
+err_cec:
+ cec_unregister_adapter(ch7322->cec);
+
+err_mutex:
+ mutex_destroy(&ch7322->mutex);
+ return ret;
+}
+
+static int ch7322_remove(struct i2c_client *client)
+{
+ struct ch7322 *ch7322 = i2c_get_clientdata(client);
+
+ /* Mask interrupt */
+ mutex_lock(&ch7322->mutex);
+ regmap_write(ch7322->regmap, CH7322_INTCTL, CH7322_INTCTL_INTPB);
+ mutex_unlock(&ch7322->mutex);
+
+ cec_unregister_adapter(ch7322->cec);
+ mutex_destroy(&ch7322->mutex);
+
+ dev_info(&client->dev, "device unregistered\n");
+
+ return 0;
+}
+
+static const struct of_device_id ch7322_of_match[] = {
+ { .compatible = "chrontel,ch7322", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, ch7322_of_match);
+
+static struct i2c_driver ch7322_i2c_driver = {
+ .driver = {
+ .name = "ch7322",
+ .of_match_table = of_match_ptr(ch7322_of_match),
+ },
+ .probe_new = ch7322_probe,
+ .remove = ch7322_remove,
+};
+
+module_i2c_driver(ch7322_i2c_driver);
+
+MODULE_DESCRIPTION("Chrontel CH7322 CEC Controller Driver");
+MODULE_AUTHOR("Jeff Chase <jnchase@google.com>");
+MODULE_LICENSE("GPL");
--
2.27.0.290.gba653c62da-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v5 1/2] dt-bindings: Add ch7322 media i2c device
2020-06-15 19:38 ` [PATCH v5 1/2] dt-bindings: Add ch7322 media i2c device Jeff Chase
@ 2020-06-17 20:26 ` Rob Herring
2020-06-17 22:07 ` Jeff Chase
0 siblings, 1 reply; 13+ messages in thread
From: Rob Herring @ 2020-06-17 20:26 UTC (permalink / raw)
To: Jeff Chase; +Cc: mchehab, robh+dt, hverkuil-cisco, linux-media, devicetree
On Mon, 15 Jun 2020 15:38:10 -0400, Jeff Chase wrote:
> The ch7322 is a Chrontel CEC controller.
>
> Signed-off-by: Jeff Chase <jnchase@google.com>
> ---
> .../bindings/media/i2c/chrontel,ch7322.yaml | 67 +++++++++++++++++++
> MAINTAINERS | 7 ++
> 2 files changed, 74 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/i2c/chrontel,ch7322.yaml
>
Please add Acked-by/Reviewed-by tags when posting new versions. However,
there's no need to repost patches *only* to add the tags. The upstream
maintainer will do that for acks received on the version they apply.
If a tag was not added on purpose, please state why and what changed.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 1/2] dt-bindings: Add ch7322 media i2c device
2020-06-17 20:26 ` Rob Herring
@ 2020-06-17 22:07 ` Jeff Chase
0 siblings, 0 replies; 13+ messages in thread
From: Jeff Chase @ 2020-06-17 22:07 UTC (permalink / raw)
To: Rob Herring; +Cc: mchehab, robh+dt, Hans Verkuil, linux-media, devicetree
On Wed, Jun 17, 2020 at 1:26 PM Rob Herring <robh@kernel.org> wrote:
>
> On Mon, 15 Jun 2020 15:38:10 -0400, Jeff Chase wrote:
> > The ch7322 is a Chrontel CEC controller.
> >
> > Signed-off-by: Jeff Chase <jnchase@google.com>
> > ---
> > .../bindings/media/i2c/chrontel,ch7322.yaml | 67 +++++++++++++++++++
> > MAINTAINERS | 7 ++
> > 2 files changed, 74 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/media/i2c/chrontel,ch7322.yaml
> >
>
>
> Please add Acked-by/Reviewed-by tags when posting new versions. However,
> there's no need to repost patches *only* to add the tags. The upstream
> maintainer will do that for acks received on the version they apply.
>
> If a tag was not added on purpose, please state why and what changed.
Sorry, still learning this process.
Hans, should I resubmit or would you add the Reviewed-by tag from v4?
Thanks,
Jeff
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 2/2] media: cec: i2c: ch7322: Add ch7322 CEC controller driver
2020-06-15 19:38 ` [PATCH v5 2/2] media: cec: i2c: ch7322: Add ch7322 CEC controller driver Jeff Chase
@ 2020-06-18 7:51 ` Hans Verkuil
2020-06-18 8:59 ` Jeff Chase
0 siblings, 1 reply; 13+ messages in thread
From: Hans Verkuil @ 2020-06-18 7:51 UTC (permalink / raw)
To: Jeff Chase, linux-media; +Cc: mchehab, robh+dt, devicetree
Hi Jeff,
I realized that there is one thing missing here: there is no way to
associate this CEC device with the corresponding HDMI interface.
Typically this is done either via the device tree (hdmi-phandle property,
see cec_notifier_parse_hdmi_phandle()) or ACPI (see cros-ec or seco drivers).
How is this ch7322 used? Chromebooks? Intel or ARM based?
The link with the HDMI interface is needed for the CEC_ADAP_G_CONNECTOR_INFO
ioctl implementation. See:
https://hverkuil.home.xs4all.nl/spec/userspace-api/cec/cec-ioc-adap-g-conn-info.html
Regards,
Hans
On 15/06/2020 21:38, Jeff Chase wrote:
> Add a CEC device driver for the Chrontel ch7322 CEC conroller.
> This is an I2C device capable of sending and receiving CEC messages.
>
> Signed-off-by: Jeff Chase <jnchase@google.com>
> ---
> MAINTAINERS | 1 +
> drivers/media/cec/Kconfig | 1 +
> drivers/media/cec/Makefile | 2 +-
> drivers/media/cec/i2c/Kconfig | 14 +
> drivers/media/cec/i2c/Makefile | 5 +
> drivers/media/cec/i2c/ch7322.c | 503 +++++++++++++++++++++++++++++++++
> 6 files changed, 525 insertions(+), 1 deletion(-)
> create mode 100644 drivers/media/cec/i2c/Kconfig
> create mode 100644 drivers/media/cec/i2c/Makefile
> create mode 100644 drivers/media/cec/i2c/ch7322.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index c1c17ca17830..e2341eaaaf5e 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4107,6 +4107,7 @@ L: linux-media@vger.kernel.org
> S: Maintained
> T: git git://linuxtv.org/media_tree.git
> F: Documentation/devicetree/bindings/media/i2c/chontel,ch7322.yaml
> +F: drivers/media/cec/i2c/ch7322.c
>
> CIRRUS LOGIC AUDIO CODEC DRIVERS
> M: James Schulman <james.schulman@cirrus.com>
> diff --git a/drivers/media/cec/Kconfig b/drivers/media/cec/Kconfig
> index 7e830444bdbb..9ba3a00dce31 100644
> --- a/drivers/media/cec/Kconfig
> +++ b/drivers/media/cec/Kconfig
> @@ -33,6 +33,7 @@ menuconfig MEDIA_CEC_SUPPORT
> adapter that supports HDMI CEC.
>
> if MEDIA_CEC_SUPPORT
> +source "drivers/media/cec/i2c/Kconfig"
> source "drivers/media/cec/platform/Kconfig"
> source "drivers/media/cec/usb/Kconfig"
> endif
> diff --git a/drivers/media/cec/Makefile b/drivers/media/cec/Makefile
> index 74e80e1b3571..23539339bc81 100644
> --- a/drivers/media/cec/Makefile
> +++ b/drivers/media/cec/Makefile
> @@ -1,2 +1,2 @@
> # SPDX-License-Identifier: GPL-2.0
> -obj-y += core/ platform/ usb/
> +obj-y += core/ i2c/ platform/ usb/
> diff --git a/drivers/media/cec/i2c/Kconfig b/drivers/media/cec/i2c/Kconfig
> new file mode 100644
> index 000000000000..e445ca2110b3
> --- /dev/null
> +++ b/drivers/media/cec/i2c/Kconfig
> @@ -0,0 +1,14 @@
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# I2C drivers
> +
> +config CEC_CH7322
> + tristate "Chrontel CH7322 CEC controller"
> + select I2C
> + select REGMAP_I2C
> + select CEC_CORE
> + help
> + This is a driver for the Chrontel CH7322 CEC controller. It uses the
> + generic CEC framework interface.
> + CEC bus is present in the HDMI connector and enables communication
> + between compatible devices.
> diff --git a/drivers/media/cec/i2c/Makefile b/drivers/media/cec/i2c/Makefile
> new file mode 100644
> index 000000000000..d7496dfd0fa4
> --- /dev/null
> +++ b/drivers/media/cec/i2c/Makefile
> @@ -0,0 +1,5 @@
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Makefile for the CEC I2C device drivers.
> +#
> +obj-$(CONFIG_CEC_CH7322) += ch7322.o
> diff --git a/drivers/media/cec/i2c/ch7322.c b/drivers/media/cec/i2c/ch7322.c
> new file mode 100644
> index 000000000000..4e6976a51774
> --- /dev/null
> +++ b/drivers/media/cec/i2c/ch7322.c
> @@ -0,0 +1,503 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Driver for the Chrontel CH7322 CEC Controller
> + *
> + * Copyright 2020 Google LLC.
> + */
> +
> +/*
> + * Notes
> + *
> + * - This device powers on in Auto Mode which has limited functionality. This
> + * driver disables Auto Mode when it attaches.
> + *
> + */
> +
> +#include <linux/cec.h>
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/regmap.h>
> +#include <media/cec.h>
> +
> +#define CH7322_WRITE 0x00
> +#define CH7322_WRITE_MSENT 0x80
> +#define CH7322_WRITE_BOK 0x40
> +#define CH7322_WRITE_NMASK 0x0f
> +
> +/* Write buffer is 0x01-0x10 */
> +#define CH7322_WRBUF 0x01
> +#define CH7322_WRBUF_LEN 0x10
> +
> +#define CH7322_READ 0x40
> +#define CH7322_READ_NRDT 0x80
> +#define CH7322_READ_MSENT 0x20
> +#define CH7322_READ_NMASK 0x0f
> +
> +/* Read buffer is 0x41-0x50 */
> +#define CH7322_RDBUF 0x41
> +#define CH7322_RDBUF_LEN 0x10
> +
> +#define CH7322_MODE 0x11
> +#define CH7322_MODE_AUTO 0x78
> +#define CH7322_MODE_SW 0xb5
> +
> +#define CH7322_RESET 0x12
> +#define CH7322_RESET_RST 0x00
> +
> +#define CH7322_POWER 0x13
> +#define CH7322_POWER_FPD 0x04
> +
> +#define CH7322_CFG0 0x17
> +#define CH7322_CFG0_EOBEN 0x40
> +#define CH7322_CFG0_PEOB 0x20
> +#define CH7322_CFG0_CLRSPP 0x10
> +#define CH7322_CFG0_FLOW 0x08
> +
> +#define CH7322_CFG1 0x1a
> +#define CH7322_CFG1_STDBYO 0x04
> +#define CH7322_CFG1_HPBP 0x02
> +#define CH7322_CFG1_PIO 0x01
> +
> +#define CH7322_INTCTL 0x1b
> +#define CH7322_INTCTL_INTPB 0x80
> +#define CH7322_INTCTL_STDBY 0x40
> +#define CH7322_INTCTL_HPDFALL 0x20
> +#define CH7322_INTCTL_HPDRISE 0x10
> +#define CH7322_INTCTL_RXMSG 0x08
> +#define CH7322_INTCTL_TXMSG 0x04
> +#define CH7322_INTCTL_NEWPHA 0x02
> +#define CH7322_INTCTL_ERROR 0x01
> +
> +#define CH7322_DVCLKFNH 0x1d
> +#define CH7322_DVCLKFNL 0x1e
> +
> +#define CH7322_CTL 0x31
> +#define CH7322_CTL_FSTDBY 0x80
> +#define CH7322_CTL_PLSEN 0x40
> +#define CH7322_CTL_PLSPB 0x20
> +#define CH7322_CTL_SPADL 0x10
> +#define CH7322_CTL_HINIT 0x08
> +#define CH7322_CTL_WPHYA 0x04
> +#define CH7322_CTL_H1T 0x02
> +#define CH7322_CTL_S1T 0x01
> +
> +#define CH7322_PAWH 0x32
> +#define CH7322_PAWL 0x33
> +
> +#define CH7322_ADDLW 0x34
> +#define CH7322_ADDLW_MASK 0xf0
> +
> +#define CH7322_ADDLR 0x3d
> +#define CH7322_ADDLR_HPD 0x80
> +#define CH7322_ADDLR_MASK 0x0f
> +
> +#define CH7322_INTDATA 0x3e
> +#define CH7322_INTDATA_MODE 0x80
> +#define CH7322_INTDATA_STDBY 0x40
> +#define CH7322_INTDATA_HPDFALL 0x20
> +#define CH7322_INTDATA_HPDRISE 0x10
> +#define CH7322_INTDATA_RXMSG 0x08
> +#define CH7322_INTDATA_TXMSG 0x04
> +#define CH7322_INTDATA_NEWPHA 0x02
> +#define CH7322_INTDATA_ERROR 0x01
> +
> +#define CH7322_EVENT 0x3F
> +#define CH7322_EVENT_TXERR 0x80
> +#define CH7322_EVENT_HRST 0x40
> +#define CH7322_EVENT_HFST 0x20
> +#define CH7322_EVENT_PHACHG 0x10
> +#define CH7322_EVENT_ACTST 0x08
> +#define CH7322_EVENT_PHARDY 0x04
> +#define CH7322_EVENT_BSOK 0x02
> +#define CH7322_EVENT_ERRADCF 0x01
> +
> +#define CH7322_DID 0x51
> +#define CH7322_DID_CH7322 0x5b
> +#define CH7322_DID_CH7323 0x5f
> +
> +#define CH7322_REVISIONID 0x52
> +
> +#define CH7322_PARH 0x53
> +#define CH7322_PARL 0x54
> +
> +#define CH7322_IOCFG2 0x75
> +#define CH7322_IOCFG_CIO 0x80
> +#define CH7322_IOCFG_IOCFGMASK 0x78
> +#define CH7322_IOCFG_AUDIO 0x04
> +#define CH7322_IOCFG_SPAMST 0x02
> +#define CH7322_IOCFG_SPAMSP 0x01
> +
> +#define CH7322_CTL3 0x7b
> +#define CH7322_CTL3_SWENA 0x80
> +#define CH7322_CTL3_FC_INIT 0x40
> +#define CH7322_CTL3_SML_FL 0x20
> +#define CH7322_CTL3_SM_RDST 0x10
> +#define CH7322_CTL3_SPP_CIAH 0x08
> +#define CH7322_CTL3_SPP_CIAL 0x04
> +#define CH7322_CTL3_SPP_ACTH 0x02
> +#define CH7322_CTL3_SPP_ACTL 0x01
> +
> +/* BOK status means NACK */
> +#define CH7322_TX_FLAG_NACK BIT(0)
> +/* Device will retry automatically */
> +#define CH7322_TX_FLAG_RETRY BIT(1)
> +
> +struct ch7322 {
> + struct i2c_client *i2c;
> + struct regmap *regmap;
> + struct cec_adapter *cec;
> + struct mutex mutex; /* device access mutex */
> + u8 tx_flags;
> +};
> +
> +static const struct regmap_config ch7322_regmap = {
> + .reg_bits = 8,
> + .val_bits = 8,
> + .max_register = 0x7f,
> + .disable_locking = true,
> +
> +};
> +
> +static int ch7322_send_message(struct ch7322 *ch7322, const struct cec_msg *msg)
> +{
> + unsigned int val;
> + unsigned int len = msg->len;
> + int ret;
> + int i;
> +
> + WARN_ON(!mutex_is_locked(&ch7322->mutex));
> +
> + if (len > CH7322_WRBUF_LEN || len < 1)
> + return -EINVAL;
> +
> + ret = regmap_read(ch7322->regmap, CH7322_WRITE, &val);
> + if (ret)
> + return ret;
> +
> + /* Buffer not ready */
> + if (!(val & CH7322_WRITE_MSENT))
> + return -EBUSY;
> +
> + if (cec_msg_opcode(msg) == -1 &&
> + cec_msg_initiator(msg) == cec_msg_destination(msg)) {
> + ch7322->tx_flags = CH7322_TX_FLAG_NACK | CH7322_TX_FLAG_RETRY;
> + } else if (cec_msg_is_broadcast(msg)) {
> + ch7322->tx_flags = CH7322_TX_FLAG_NACK;
> + } else {
> + ch7322->tx_flags = CH7322_TX_FLAG_RETRY;
> + }
> +
> + ret = regmap_write(ch7322->regmap, CH7322_WRITE, len - 1);
> + if (ret)
> + return ret;
> +
> + for (i = 0; i < len; i++) {
> + ret = regmap_write(ch7322->regmap,
> + CH7322_WRBUF + i, msg->msg[i]);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int ch7322_receive_message(struct ch7322 *ch7322, struct cec_msg *msg)
> +{
> + unsigned int val;
> + int ret = 0;
> + int i;
> +
> + WARN_ON(!mutex_is_locked(&ch7322->mutex));
> +
> + ret = regmap_read(ch7322->regmap, CH7322_READ, &val);
> + if (ret)
> + return ret;
> +
> + /* Message not ready */
> + if (!(val & CH7322_READ_NRDT))
> + return -EIO;
> +
> + msg->len = (val & CH7322_READ_NMASK) + 1;
> +
> + /* Read entire RDBUF to clear state */
> + for (i = 0; i < CH7322_RDBUF_LEN; i++) {
> + ret = regmap_read(ch7322->regmap, CH7322_RDBUF + i, &val);
> + if (ret)
> + return ret;
> + msg->msg[i] = (u8)val;
> + }
> +
> + return 0;
> +}
> +
> +static void ch7322_tx_done(struct ch7322 *ch7322)
> +{
> + int ret;
> + unsigned int val;
> + u8 status, flags;
> +
> + mutex_lock(&ch7322->mutex);
> + ret = regmap_read(ch7322->regmap, CH7322_WRITE, &val);
> + flags = ch7322->tx_flags;
> + mutex_unlock(&ch7322->mutex);
> +
> + /*
> + * The device returns a one-bit OK status which usually means ACK but
> + * actually means NACK when sending a logical address query or a
> + * broadcast.
> + */
> + if (ret)
> + status = CEC_TX_STATUS_ERROR;
> + else if ((val & CH7322_WRITE_BOK) && (flags & CH7322_TX_FLAG_NACK))
> + status = CEC_TX_STATUS_NACK;
> + else if (val & CH7322_WRITE_BOK)
> + status = CEC_TX_STATUS_OK;
> + else if (flags & CH7322_TX_FLAG_NACK)
> + status = CEC_TX_STATUS_OK;
> + else
> + status = CEC_TX_STATUS_NACK;
> +
> + if (status == CEC_TX_STATUS_NACK && (flags & CH7322_TX_FLAG_RETRY))
> + status |= CEC_TX_STATUS_MAX_RETRIES;
> +
> + cec_transmit_attempt_done(ch7322->cec, status);
> +}
> +
> +static void ch7322_rx_done(struct ch7322 *ch7322)
> +{
> + struct cec_msg msg;
> + int ret;
> +
> + mutex_lock(&ch7322->mutex);
> + ret = ch7322_receive_message(ch7322, &msg);
> + mutex_unlock(&ch7322->mutex);
> +
> + if (ret)
> + dev_err(&ch7322->i2c->dev, "cec receive error: %d\n", ret);
> + else
> + cec_received_msg(ch7322->cec, &msg);
> +}
> +
> +/*
> + * This device can either monitor the DDC lines to obtain the physical address
> + * or it can allow the host to program it. This driver lets the device obtain
> + * it.
> + */
> +static void ch7322_phys_addr(struct ch7322 *ch7322)
> +{
> + unsigned int pah, pal;
> + int ret = 0;
> +
> + mutex_lock(&ch7322->mutex);
> + ret |= regmap_read(ch7322->regmap, CH7322_PARH, &pah);
> + ret |= regmap_read(ch7322->regmap, CH7322_PARL, &pal);
> + mutex_unlock(&ch7322->mutex);
> +
> + if (ret)
> + dev_err(&ch7322->i2c->dev, "phys addr error\n");
> + else
> + cec_s_phys_addr(ch7322->cec, pal | (pah << 8), false);
> +}
> +
> +static void ch7322_handle_events(struct ch7322 *ch7322)
> +{
> + unsigned int data = 0;
> +
> + mutex_lock(&ch7322->mutex);
> + regmap_read(ch7322->regmap, CH7322_INTDATA, &data);
> + regmap_write(ch7322->regmap, CH7322_INTDATA, data);
> + mutex_unlock(&ch7322->mutex);
> +
> + if (data & CH7322_INTDATA_HPDFALL)
> + cec_phys_addr_invalidate(ch7322->cec);
> +
> + if (data & CH7322_INTDATA_TXMSG)
> + ch7322_tx_done(ch7322);
> +
> + if (data & CH7322_INTDATA_RXMSG)
> + ch7322_rx_done(ch7322);
> +
> + if (data & CH7322_INTDATA_NEWPHA)
> + ch7322_phys_addr(ch7322);
> +
> + if (data & CH7322_INTDATA_ERROR)
> + dev_dbg(&ch7322->i2c->dev, "unknown error\n");
> +}
> +
> +static irqreturn_t ch7322_irq(int irq, void *dev)
> +{
> + struct ch7322 *ch7322 = dev;
> +
> + ch7322_handle_events(ch7322);
> +
> + return IRQ_HANDLED;
> +}
> +
> +/* This device is always enabled */
> +static int ch7322_cec_adap_enable(struct cec_adapter *adap, bool enable)
> +{
> + return 0;
> +}
> +
> +static int ch7322_cec_adap_log_addr(struct cec_adapter *adap, u8 log_addr)
> +{
> + struct ch7322 *ch7322 = cec_get_drvdata(adap);
> + int ret;
> +
> + mutex_lock(&ch7322->mutex);
> + ret = regmap_update_bits(ch7322->regmap, CH7322_ADDLW,
> + CH7322_ADDLW_MASK, log_addr << 4);
> + mutex_unlock(&ch7322->mutex);
> +
> + return ret;
> +}
> +
> +static int ch7322_cec_adap_transmit(struct cec_adapter *adap, u8 attempts,
> + u32 signal_free_time, struct cec_msg *msg)
> +{
> + struct ch7322 *ch7322 = cec_get_drvdata(adap);
> + int ret;
> +
> + mutex_lock(&ch7322->mutex);
> + ret = ch7322_send_message(ch7322, msg);
> + mutex_unlock(&ch7322->mutex);
> +
> + return ret;
> +}
> +
> +static const struct cec_adap_ops ch7322_cec_adap_ops = {
> + .adap_enable = ch7322_cec_adap_enable,
> + .adap_log_addr = ch7322_cec_adap_log_addr,
> + .adap_transmit = ch7322_cec_adap_transmit,
> +};
> +
> +static int ch7322_probe(struct i2c_client *client)
> +{
> + struct ch7322 *ch7322;
> + int ret;
> + unsigned int val;
> +
> + ch7322 = devm_kzalloc(&client->dev, sizeof(*ch7322), GFP_KERNEL);
> + if (!ch7322)
> + return -ENOMEM;
> +
> + ch7322->regmap = devm_regmap_init_i2c(client, &ch7322_regmap);
> + if (IS_ERR(ch7322->regmap))
> + return PTR_ERR(ch7322->regmap);
> +
> + ret = regmap_read(ch7322->regmap, CH7322_DID, &val);
> + if (ret)
> + return ret;
> +
> + if (val != CH7322_DID_CH7322)
> + return -ENOTSUPP;
> +
> + mutex_init(&ch7322->mutex);
> + ch7322->i2c = client;
> + ch7322->tx_flags = 0;
> +
> + i2c_set_clientdata(client, ch7322);
> +
> + /* Disable auto mode */
> + ret = regmap_write(ch7322->regmap, CH7322_MODE, CH7322_MODE_SW);
> + if (ret)
> + goto err_mutex;
> +
> + /* Enable logical address register */
> + ret = regmap_update_bits(ch7322->regmap, CH7322_CTL,
> + CH7322_CTL_SPADL, CH7322_CTL_SPADL);
> + if (ret)
> + goto err_mutex;
> +
> + ch7322->cec = cec_allocate_adapter(&ch7322_cec_adap_ops, ch7322,
> + dev_name(&client->dev),
> + CEC_CAP_DEFAULTS, 1);
> +
> + if (IS_ERR(ch7322->cec)) {
> + ret = PTR_ERR(ch7322->cec);
> + goto err_mutex;
> + }
> +
> + ret = cec_register_adapter(ch7322->cec, &client->dev);
> + if (ret) {
> + cec_delete_adapter(ch7322->cec);
> + goto err_mutex;
> + }
> +
> + /* Configure and mask interrupt */
> + ret = regmap_write(ch7322->regmap, CH7322_CFG1, 0);
> + if (ret)
> + goto err_cec;
> +
> + ret = regmap_write(ch7322->regmap, CH7322_INTCTL, CH7322_INTCTL_INTPB);
> + if (ret)
> + goto err_cec;
> +
> + ret = devm_request_threaded_irq(&client->dev, client->irq, NULL,
> + ch7322_irq,
> + IRQF_ONESHOT | IRQF_TRIGGER_RISING,
> + client->name, ch7322);
> + if (ret)
> + goto err_cec;
> +
> + ch7322_handle_events(ch7322);
> +
> + /* Unmask interrupt */
> + mutex_lock(&ch7322->mutex);
> + ret = regmap_write(ch7322->regmap, CH7322_INTCTL, 0xff);
> + mutex_unlock(&ch7322->mutex);
> +
> + if (ret)
> + goto err_cec;
> +
> + dev_info(&client->dev, "device registered\n");
> +
> + return 0;
> +
> +err_cec:
> + cec_unregister_adapter(ch7322->cec);
> +
> +err_mutex:
> + mutex_destroy(&ch7322->mutex);
> + return ret;
> +}
> +
> +static int ch7322_remove(struct i2c_client *client)
> +{
> + struct ch7322 *ch7322 = i2c_get_clientdata(client);
> +
> + /* Mask interrupt */
> + mutex_lock(&ch7322->mutex);
> + regmap_write(ch7322->regmap, CH7322_INTCTL, CH7322_INTCTL_INTPB);
> + mutex_unlock(&ch7322->mutex);
> +
> + cec_unregister_adapter(ch7322->cec);
> + mutex_destroy(&ch7322->mutex);
> +
> + dev_info(&client->dev, "device unregistered\n");
> +
> + return 0;
> +}
> +
> +static const struct of_device_id ch7322_of_match[] = {
> + { .compatible = "chrontel,ch7322", },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, ch7322_of_match);
> +
> +static struct i2c_driver ch7322_i2c_driver = {
> + .driver = {
> + .name = "ch7322",
> + .of_match_table = of_match_ptr(ch7322_of_match),
> + },
> + .probe_new = ch7322_probe,
> + .remove = ch7322_remove,
> +};
> +
> +module_i2c_driver(ch7322_i2c_driver);
> +
> +MODULE_DESCRIPTION("Chrontel CH7322 CEC Controller Driver");
> +MODULE_AUTHOR("Jeff Chase <jnchase@google.com>");
> +MODULE_LICENSE("GPL");
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 2/2] media: cec: i2c: ch7322: Add ch7322 CEC controller driver
2020-06-18 7:51 ` Hans Verkuil
@ 2020-06-18 8:59 ` Jeff Chase
2020-06-18 10:05 ` Hans Verkuil
0 siblings, 1 reply; 13+ messages in thread
From: Jeff Chase @ 2020-06-18 8:59 UTC (permalink / raw)
To: Hans Verkuil; +Cc: linux-media, mchehab, robh+dt, devicetree
Hi Hans,
We are using two of these in an Intel-based Chromebox. I see that the
cros-ec and seco drivers just statically define the PCI BDF of the
Intel graphics device for their boards. I don't see an example of ACPI
passing this information. I can copy cros-ec and seco by adding a
board table and then use the UID of each device to select the correct
port. Adding board-specific configuration to the driver doesn't seem
ideal but I'm not sure what the proper way to pass this using ACPI is.
Thanks,
Jeff
On Thu, Jun 18, 2020 at 12:51 AM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
>
> Hi Jeff,
>
> I realized that there is one thing missing here: there is no way to
> associate this CEC device with the corresponding HDMI interface.
>
> Typically this is done either via the device tree (hdmi-phandle property,
> see cec_notifier_parse_hdmi_phandle()) or ACPI (see cros-ec or seco drivers).
>
> How is this ch7322 used? Chromebooks? Intel or ARM based?
>
> The link with the HDMI interface is needed for the CEC_ADAP_G_CONNECTOR_INFO
> ioctl implementation. See:
>
> https://hverkuil.home.xs4all.nl/spec/userspace-api/cec/cec-ioc-adap-g-conn-info.html
>
> Regards,
>
> Hans
>
> On 15/06/2020 21:38, Jeff Chase wrote:
> > Add a CEC device driver for the Chrontel ch7322 CEC conroller.
> > This is an I2C device capable of sending and receiving CEC messages.
> >
> > Signed-off-by: Jeff Chase <jnchase@google.com>
> > ---
> > MAINTAINERS | 1 +
> > drivers/media/cec/Kconfig | 1 +
> > drivers/media/cec/Makefile | 2 +-
> > drivers/media/cec/i2c/Kconfig | 14 +
> > drivers/media/cec/i2c/Makefile | 5 +
> > drivers/media/cec/i2c/ch7322.c | 503 +++++++++++++++++++++++++++++++++
> > 6 files changed, 525 insertions(+), 1 deletion(-)
> > create mode 100644 drivers/media/cec/i2c/Kconfig
> > create mode 100644 drivers/media/cec/i2c/Makefile
> > create mode 100644 drivers/media/cec/i2c/ch7322.c
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index c1c17ca17830..e2341eaaaf5e 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -4107,6 +4107,7 @@ L: linux-media@vger.kernel.org
> > S: Maintained
> > T: git git://linuxtv.org/media_tree.git
> > F: Documentation/devicetree/bindings/media/i2c/chontel,ch7322.yaml
> > +F: drivers/media/cec/i2c/ch7322.c
> >
> > CIRRUS LOGIC AUDIO CODEC DRIVERS
> > M: James Schulman <james.schulman@cirrus.com>
> > diff --git a/drivers/media/cec/Kconfig b/drivers/media/cec/Kconfig
> > index 7e830444bdbb..9ba3a00dce31 100644
> > --- a/drivers/media/cec/Kconfig
> > +++ b/drivers/media/cec/Kconfig
> > @@ -33,6 +33,7 @@ menuconfig MEDIA_CEC_SUPPORT
> > adapter that supports HDMI CEC.
> >
> > if MEDIA_CEC_SUPPORT
> > +source "drivers/media/cec/i2c/Kconfig"
> > source "drivers/media/cec/platform/Kconfig"
> > source "drivers/media/cec/usb/Kconfig"
> > endif
> > diff --git a/drivers/media/cec/Makefile b/drivers/media/cec/Makefile
> > index 74e80e1b3571..23539339bc81 100644
> > --- a/drivers/media/cec/Makefile
> > +++ b/drivers/media/cec/Makefile
> > @@ -1,2 +1,2 @@
> > # SPDX-License-Identifier: GPL-2.0
> > -obj-y += core/ platform/ usb/
> > +obj-y += core/ i2c/ platform/ usb/
> > diff --git a/drivers/media/cec/i2c/Kconfig b/drivers/media/cec/i2c/Kconfig
> > new file mode 100644
> > index 000000000000..e445ca2110b3
> > --- /dev/null
> > +++ b/drivers/media/cec/i2c/Kconfig
> > @@ -0,0 +1,14 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +#
> > +# I2C drivers
> > +
> > +config CEC_CH7322
> > + tristate "Chrontel CH7322 CEC controller"
> > + select I2C
> > + select REGMAP_I2C
> > + select CEC_CORE
> > + help
> > + This is a driver for the Chrontel CH7322 CEC controller. It uses the
> > + generic CEC framework interface.
> > + CEC bus is present in the HDMI connector and enables communication
> > + between compatible devices.
> > diff --git a/drivers/media/cec/i2c/Makefile b/drivers/media/cec/i2c/Makefile
> > new file mode 100644
> > index 000000000000..d7496dfd0fa4
> > --- /dev/null
> > +++ b/drivers/media/cec/i2c/Makefile
> > @@ -0,0 +1,5 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +#
> > +# Makefile for the CEC I2C device drivers.
> > +#
> > +obj-$(CONFIG_CEC_CH7322) += ch7322.o
> > diff --git a/drivers/media/cec/i2c/ch7322.c b/drivers/media/cec/i2c/ch7322.c
> > new file mode 100644
> > index 000000000000..4e6976a51774
> > --- /dev/null
> > +++ b/drivers/media/cec/i2c/ch7322.c
> > @@ -0,0 +1,503 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Driver for the Chrontel CH7322 CEC Controller
> > + *
> > + * Copyright 2020 Google LLC.
> > + */
> > +
> > +/*
> > + * Notes
> > + *
> > + * - This device powers on in Auto Mode which has limited functionality. This
> > + * driver disables Auto Mode when it attaches.
> > + *
> > + */
> > +
> > +#include <linux/cec.h>
> > +#include <linux/i2c.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/module.h>
> > +#include <linux/mutex.h>
> > +#include <linux/regmap.h>
> > +#include <media/cec.h>
> > +
> > +#define CH7322_WRITE 0x00
> > +#define CH7322_WRITE_MSENT 0x80
> > +#define CH7322_WRITE_BOK 0x40
> > +#define CH7322_WRITE_NMASK 0x0f
> > +
> > +/* Write buffer is 0x01-0x10 */
> > +#define CH7322_WRBUF 0x01
> > +#define CH7322_WRBUF_LEN 0x10
> > +
> > +#define CH7322_READ 0x40
> > +#define CH7322_READ_NRDT 0x80
> > +#define CH7322_READ_MSENT 0x20
> > +#define CH7322_READ_NMASK 0x0f
> > +
> > +/* Read buffer is 0x41-0x50 */
> > +#define CH7322_RDBUF 0x41
> > +#define CH7322_RDBUF_LEN 0x10
> > +
> > +#define CH7322_MODE 0x11
> > +#define CH7322_MODE_AUTO 0x78
> > +#define CH7322_MODE_SW 0xb5
> > +
> > +#define CH7322_RESET 0x12
> > +#define CH7322_RESET_RST 0x00
> > +
> > +#define CH7322_POWER 0x13
> > +#define CH7322_POWER_FPD 0x04
> > +
> > +#define CH7322_CFG0 0x17
> > +#define CH7322_CFG0_EOBEN 0x40
> > +#define CH7322_CFG0_PEOB 0x20
> > +#define CH7322_CFG0_CLRSPP 0x10
> > +#define CH7322_CFG0_FLOW 0x08
> > +
> > +#define CH7322_CFG1 0x1a
> > +#define CH7322_CFG1_STDBYO 0x04
> > +#define CH7322_CFG1_HPBP 0x02
> > +#define CH7322_CFG1_PIO 0x01
> > +
> > +#define CH7322_INTCTL 0x1b
> > +#define CH7322_INTCTL_INTPB 0x80
> > +#define CH7322_INTCTL_STDBY 0x40
> > +#define CH7322_INTCTL_HPDFALL 0x20
> > +#define CH7322_INTCTL_HPDRISE 0x10
> > +#define CH7322_INTCTL_RXMSG 0x08
> > +#define CH7322_INTCTL_TXMSG 0x04
> > +#define CH7322_INTCTL_NEWPHA 0x02
> > +#define CH7322_INTCTL_ERROR 0x01
> > +
> > +#define CH7322_DVCLKFNH 0x1d
> > +#define CH7322_DVCLKFNL 0x1e
> > +
> > +#define CH7322_CTL 0x31
> > +#define CH7322_CTL_FSTDBY 0x80
> > +#define CH7322_CTL_PLSEN 0x40
> > +#define CH7322_CTL_PLSPB 0x20
> > +#define CH7322_CTL_SPADL 0x10
> > +#define CH7322_CTL_HINIT 0x08
> > +#define CH7322_CTL_WPHYA 0x04
> > +#define CH7322_CTL_H1T 0x02
> > +#define CH7322_CTL_S1T 0x01
> > +
> > +#define CH7322_PAWH 0x32
> > +#define CH7322_PAWL 0x33
> > +
> > +#define CH7322_ADDLW 0x34
> > +#define CH7322_ADDLW_MASK 0xf0
> > +
> > +#define CH7322_ADDLR 0x3d
> > +#define CH7322_ADDLR_HPD 0x80
> > +#define CH7322_ADDLR_MASK 0x0f
> > +
> > +#define CH7322_INTDATA 0x3e
> > +#define CH7322_INTDATA_MODE 0x80
> > +#define CH7322_INTDATA_STDBY 0x40
> > +#define CH7322_INTDATA_HPDFALL 0x20
> > +#define CH7322_INTDATA_HPDRISE 0x10
> > +#define CH7322_INTDATA_RXMSG 0x08
> > +#define CH7322_INTDATA_TXMSG 0x04
> > +#define CH7322_INTDATA_NEWPHA 0x02
> > +#define CH7322_INTDATA_ERROR 0x01
> > +
> > +#define CH7322_EVENT 0x3F
> > +#define CH7322_EVENT_TXERR 0x80
> > +#define CH7322_EVENT_HRST 0x40
> > +#define CH7322_EVENT_HFST 0x20
> > +#define CH7322_EVENT_PHACHG 0x10
> > +#define CH7322_EVENT_ACTST 0x08
> > +#define CH7322_EVENT_PHARDY 0x04
> > +#define CH7322_EVENT_BSOK 0x02
> > +#define CH7322_EVENT_ERRADCF 0x01
> > +
> > +#define CH7322_DID 0x51
> > +#define CH7322_DID_CH7322 0x5b
> > +#define CH7322_DID_CH7323 0x5f
> > +
> > +#define CH7322_REVISIONID 0x52
> > +
> > +#define CH7322_PARH 0x53
> > +#define CH7322_PARL 0x54
> > +
> > +#define CH7322_IOCFG2 0x75
> > +#define CH7322_IOCFG_CIO 0x80
> > +#define CH7322_IOCFG_IOCFGMASK 0x78
> > +#define CH7322_IOCFG_AUDIO 0x04
> > +#define CH7322_IOCFG_SPAMST 0x02
> > +#define CH7322_IOCFG_SPAMSP 0x01
> > +
> > +#define CH7322_CTL3 0x7b
> > +#define CH7322_CTL3_SWENA 0x80
> > +#define CH7322_CTL3_FC_INIT 0x40
> > +#define CH7322_CTL3_SML_FL 0x20
> > +#define CH7322_CTL3_SM_RDST 0x10
> > +#define CH7322_CTL3_SPP_CIAH 0x08
> > +#define CH7322_CTL3_SPP_CIAL 0x04
> > +#define CH7322_CTL3_SPP_ACTH 0x02
> > +#define CH7322_CTL3_SPP_ACTL 0x01
> > +
> > +/* BOK status means NACK */
> > +#define CH7322_TX_FLAG_NACK BIT(0)
> > +/* Device will retry automatically */
> > +#define CH7322_TX_FLAG_RETRY BIT(1)
> > +
> > +struct ch7322 {
> > + struct i2c_client *i2c;
> > + struct regmap *regmap;
> > + struct cec_adapter *cec;
> > + struct mutex mutex; /* device access mutex */
> > + u8 tx_flags;
> > +};
> > +
> > +static const struct regmap_config ch7322_regmap = {
> > + .reg_bits = 8,
> > + .val_bits = 8,
> > + .max_register = 0x7f,
> > + .disable_locking = true,
> > +
> > +};
> > +
> > +static int ch7322_send_message(struct ch7322 *ch7322, const struct cec_msg *msg)
> > +{
> > + unsigned int val;
> > + unsigned int len = msg->len;
> > + int ret;
> > + int i;
> > +
> > + WARN_ON(!mutex_is_locked(&ch7322->mutex));
> > +
> > + if (len > CH7322_WRBUF_LEN || len < 1)
> > + return -EINVAL;
> > +
> > + ret = regmap_read(ch7322->regmap, CH7322_WRITE, &val);
> > + if (ret)
> > + return ret;
> > +
> > + /* Buffer not ready */
> > + if (!(val & CH7322_WRITE_MSENT))
> > + return -EBUSY;
> > +
> > + if (cec_msg_opcode(msg) == -1 &&
> > + cec_msg_initiator(msg) == cec_msg_destination(msg)) {
> > + ch7322->tx_flags = CH7322_TX_FLAG_NACK | CH7322_TX_FLAG_RETRY;
> > + } else if (cec_msg_is_broadcast(msg)) {
> > + ch7322->tx_flags = CH7322_TX_FLAG_NACK;
> > + } else {
> > + ch7322->tx_flags = CH7322_TX_FLAG_RETRY;
> > + }
> > +
> > + ret = regmap_write(ch7322->regmap, CH7322_WRITE, len - 1);
> > + if (ret)
> > + return ret;
> > +
> > + for (i = 0; i < len; i++) {
> > + ret = regmap_write(ch7322->regmap,
> > + CH7322_WRBUF + i, msg->msg[i]);
> > + if (ret)
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int ch7322_receive_message(struct ch7322 *ch7322, struct cec_msg *msg)
> > +{
> > + unsigned int val;
> > + int ret = 0;
> > + int i;
> > +
> > + WARN_ON(!mutex_is_locked(&ch7322->mutex));
> > +
> > + ret = regmap_read(ch7322->regmap, CH7322_READ, &val);
> > + if (ret)
> > + return ret;
> > +
> > + /* Message not ready */
> > + if (!(val & CH7322_READ_NRDT))
> > + return -EIO;
> > +
> > + msg->len = (val & CH7322_READ_NMASK) + 1;
> > +
> > + /* Read entire RDBUF to clear state */
> > + for (i = 0; i < CH7322_RDBUF_LEN; i++) {
> > + ret = regmap_read(ch7322->regmap, CH7322_RDBUF + i, &val);
> > + if (ret)
> > + return ret;
> > + msg->msg[i] = (u8)val;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static void ch7322_tx_done(struct ch7322 *ch7322)
> > +{
> > + int ret;
> > + unsigned int val;
> > + u8 status, flags;
> > +
> > + mutex_lock(&ch7322->mutex);
> > + ret = regmap_read(ch7322->regmap, CH7322_WRITE, &val);
> > + flags = ch7322->tx_flags;
> > + mutex_unlock(&ch7322->mutex);
> > +
> > + /*
> > + * The device returns a one-bit OK status which usually means ACK but
> > + * actually means NACK when sending a logical address query or a
> > + * broadcast.
> > + */
> > + if (ret)
> > + status = CEC_TX_STATUS_ERROR;
> > + else if ((val & CH7322_WRITE_BOK) && (flags & CH7322_TX_FLAG_NACK))
> > + status = CEC_TX_STATUS_NACK;
> > + else if (val & CH7322_WRITE_BOK)
> > + status = CEC_TX_STATUS_OK;
> > + else if (flags & CH7322_TX_FLAG_NACK)
> > + status = CEC_TX_STATUS_OK;
> > + else
> > + status = CEC_TX_STATUS_NACK;
> > +
> > + if (status == CEC_TX_STATUS_NACK && (flags & CH7322_TX_FLAG_RETRY))
> > + status |= CEC_TX_STATUS_MAX_RETRIES;
> > +
> > + cec_transmit_attempt_done(ch7322->cec, status);
> > +}
> > +
> > +static void ch7322_rx_done(struct ch7322 *ch7322)
> > +{
> > + struct cec_msg msg;
> > + int ret;
> > +
> > + mutex_lock(&ch7322->mutex);
> > + ret = ch7322_receive_message(ch7322, &msg);
> > + mutex_unlock(&ch7322->mutex);
> > +
> > + if (ret)
> > + dev_err(&ch7322->i2c->dev, "cec receive error: %d\n", ret);
> > + else
> > + cec_received_msg(ch7322->cec, &msg);
> > +}
> > +
> > +/*
> > + * This device can either monitor the DDC lines to obtain the physical address
> > + * or it can allow the host to program it. This driver lets the device obtain
> > + * it.
> > + */
> > +static void ch7322_phys_addr(struct ch7322 *ch7322)
> > +{
> > + unsigned int pah, pal;
> > + int ret = 0;
> > +
> > + mutex_lock(&ch7322->mutex);
> > + ret |= regmap_read(ch7322->regmap, CH7322_PARH, &pah);
> > + ret |= regmap_read(ch7322->regmap, CH7322_PARL, &pal);
> > + mutex_unlock(&ch7322->mutex);
> > +
> > + if (ret)
> > + dev_err(&ch7322->i2c->dev, "phys addr error\n");
> > + else
> > + cec_s_phys_addr(ch7322->cec, pal | (pah << 8), false);
> > +}
> > +
> > +static void ch7322_handle_events(struct ch7322 *ch7322)
> > +{
> > + unsigned int data = 0;
> > +
> > + mutex_lock(&ch7322->mutex);
> > + regmap_read(ch7322->regmap, CH7322_INTDATA, &data);
> > + regmap_write(ch7322->regmap, CH7322_INTDATA, data);
> > + mutex_unlock(&ch7322->mutex);
> > +
> > + if (data & CH7322_INTDATA_HPDFALL)
> > + cec_phys_addr_invalidate(ch7322->cec);
> > +
> > + if (data & CH7322_INTDATA_TXMSG)
> > + ch7322_tx_done(ch7322);
> > +
> > + if (data & CH7322_INTDATA_RXMSG)
> > + ch7322_rx_done(ch7322);
> > +
> > + if (data & CH7322_INTDATA_NEWPHA)
> > + ch7322_phys_addr(ch7322);
> > +
> > + if (data & CH7322_INTDATA_ERROR)
> > + dev_dbg(&ch7322->i2c->dev, "unknown error\n");
> > +}
> > +
> > +static irqreturn_t ch7322_irq(int irq, void *dev)
> > +{
> > + struct ch7322 *ch7322 = dev;
> > +
> > + ch7322_handle_events(ch7322);
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
> > +/* This device is always enabled */
> > +static int ch7322_cec_adap_enable(struct cec_adapter *adap, bool enable)
> > +{
> > + return 0;
> > +}
> > +
> > +static int ch7322_cec_adap_log_addr(struct cec_adapter *adap, u8 log_addr)
> > +{
> > + struct ch7322 *ch7322 = cec_get_drvdata(adap);
> > + int ret;
> > +
> > + mutex_lock(&ch7322->mutex);
> > + ret = regmap_update_bits(ch7322->regmap, CH7322_ADDLW,
> > + CH7322_ADDLW_MASK, log_addr << 4);
> > + mutex_unlock(&ch7322->mutex);
> > +
> > + return ret;
> > +}
> > +
> > +static int ch7322_cec_adap_transmit(struct cec_adapter *adap, u8 attempts,
> > + u32 signal_free_time, struct cec_msg *msg)
> > +{
> > + struct ch7322 *ch7322 = cec_get_drvdata(adap);
> > + int ret;
> > +
> > + mutex_lock(&ch7322->mutex);
> > + ret = ch7322_send_message(ch7322, msg);
> > + mutex_unlock(&ch7322->mutex);
> > +
> > + return ret;
> > +}
> > +
> > +static const struct cec_adap_ops ch7322_cec_adap_ops = {
> > + .adap_enable = ch7322_cec_adap_enable,
> > + .adap_log_addr = ch7322_cec_adap_log_addr,
> > + .adap_transmit = ch7322_cec_adap_transmit,
> > +};
> > +
> > +static int ch7322_probe(struct i2c_client *client)
> > +{
> > + struct ch7322 *ch7322;
> > + int ret;
> > + unsigned int val;
> > +
> > + ch7322 = devm_kzalloc(&client->dev, sizeof(*ch7322), GFP_KERNEL);
> > + if (!ch7322)
> > + return -ENOMEM;
> > +
> > + ch7322->regmap = devm_regmap_init_i2c(client, &ch7322_regmap);
> > + if (IS_ERR(ch7322->regmap))
> > + return PTR_ERR(ch7322->regmap);
> > +
> > + ret = regmap_read(ch7322->regmap, CH7322_DID, &val);
> > + if (ret)
> > + return ret;
> > +
> > + if (val != CH7322_DID_CH7322)
> > + return -ENOTSUPP;
> > +
> > + mutex_init(&ch7322->mutex);
> > + ch7322->i2c = client;
> > + ch7322->tx_flags = 0;
> > +
> > + i2c_set_clientdata(client, ch7322);
> > +
> > + /* Disable auto mode */
> > + ret = regmap_write(ch7322->regmap, CH7322_MODE, CH7322_MODE_SW);
> > + if (ret)
> > + goto err_mutex;
> > +
> > + /* Enable logical address register */
> > + ret = regmap_update_bits(ch7322->regmap, CH7322_CTL,
> > + CH7322_CTL_SPADL, CH7322_CTL_SPADL);
> > + if (ret)
> > + goto err_mutex;
> > +
> > + ch7322->cec = cec_allocate_adapter(&ch7322_cec_adap_ops, ch7322,
> > + dev_name(&client->dev),
> > + CEC_CAP_DEFAULTS, 1);
> > +
> > + if (IS_ERR(ch7322->cec)) {
> > + ret = PTR_ERR(ch7322->cec);
> > + goto err_mutex;
> > + }
> > +
> > + ret = cec_register_adapter(ch7322->cec, &client->dev);
> > + if (ret) {
> > + cec_delete_adapter(ch7322->cec);
> > + goto err_mutex;
> > + }
> > +
> > + /* Configure and mask interrupt */
> > + ret = regmap_write(ch7322->regmap, CH7322_CFG1, 0);
> > + if (ret)
> > + goto err_cec;
> > +
> > + ret = regmap_write(ch7322->regmap, CH7322_INTCTL, CH7322_INTCTL_INTPB);
> > + if (ret)
> > + goto err_cec;
> > +
> > + ret = devm_request_threaded_irq(&client->dev, client->irq, NULL,
> > + ch7322_irq,
> > + IRQF_ONESHOT | IRQF_TRIGGER_RISING,
> > + client->name, ch7322);
> > + if (ret)
> > + goto err_cec;
> > +
> > + ch7322_handle_events(ch7322);
> > +
> > + /* Unmask interrupt */
> > + mutex_lock(&ch7322->mutex);
> > + ret = regmap_write(ch7322->regmap, CH7322_INTCTL, 0xff);
> > + mutex_unlock(&ch7322->mutex);
> > +
> > + if (ret)
> > + goto err_cec;
> > +
> > + dev_info(&client->dev, "device registered\n");
> > +
> > + return 0;
> > +
> > +err_cec:
> > + cec_unregister_adapter(ch7322->cec);
> > +
> > +err_mutex:
> > + mutex_destroy(&ch7322->mutex);
> > + return ret;
> > +}
> > +
> > +static int ch7322_remove(struct i2c_client *client)
> > +{
> > + struct ch7322 *ch7322 = i2c_get_clientdata(client);
> > +
> > + /* Mask interrupt */
> > + mutex_lock(&ch7322->mutex);
> > + regmap_write(ch7322->regmap, CH7322_INTCTL, CH7322_INTCTL_INTPB);
> > + mutex_unlock(&ch7322->mutex);
> > +
> > + cec_unregister_adapter(ch7322->cec);
> > + mutex_destroy(&ch7322->mutex);
> > +
> > + dev_info(&client->dev, "device unregistered\n");
> > +
> > + return 0;
> > +}
> > +
> > +static const struct of_device_id ch7322_of_match[] = {
> > + { .compatible = "chrontel,ch7322", },
> > + {},
> > +};
> > +MODULE_DEVICE_TABLE(of, ch7322_of_match);
> > +
> > +static struct i2c_driver ch7322_i2c_driver = {
> > + .driver = {
> > + .name = "ch7322",
> > + .of_match_table = of_match_ptr(ch7322_of_match),
> > + },
> > + .probe_new = ch7322_probe,
> > + .remove = ch7322_remove,
> > +};
> > +
> > +module_i2c_driver(ch7322_i2c_driver);
> > +
> > +MODULE_DESCRIPTION("Chrontel CH7322 CEC Controller Driver");
> > +MODULE_AUTHOR("Jeff Chase <jnchase@google.com>");
> > +MODULE_LICENSE("GPL");
> >
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 2/2] media: cec: i2c: ch7322: Add ch7322 CEC controller driver
2020-06-18 8:59 ` Jeff Chase
@ 2020-06-18 10:05 ` Hans Verkuil
2020-06-18 16:25 ` Jeff Chase
0 siblings, 1 reply; 13+ messages in thread
From: Hans Verkuil @ 2020-06-18 10:05 UTC (permalink / raw)
To: Jeff Chase; +Cc: linux-media, mchehab, robh+dt, devicetree
On 18/06/2020 10:59, Jeff Chase wrote:
> Hi Hans,
>
> We are using two of these in an Intel-based Chromebox. I see that the
> cros-ec and seco drivers just statically define the PCI BDF of the
> Intel graphics device for their boards. I don't see an example of ACPI
> passing this information. I can copy cros-ec and seco by adding a
> board table and then use the UID of each device to select the correct
> port. Adding board-specific configuration to the driver doesn't seem
> ideal but I'm not sure what the proper way to pass this using ACPI is.
You are right, it's not ACPI, it's using DMI matching.
I have zero knowledge about ACPI, so I have no idea if there is some standard
method of retrieving this association via ACPI.
This particular chip can actually be used both with DMI matching but also
on an ARM with device tree, but since you can't test this on an ARM board,
there is no point in adding support for that.
However, compared to the cros-ec and seco drivers you can do something a bit
different here: those drivers just return -ENODEV if there is no match, but
since this driver reads the EDID it can just continue as long as it does not
set the CEC_CAP_CONNECTOR_INFO capability.
Regards,
Hans
>
> Thanks,
> Jeff
>
> On Thu, Jun 18, 2020 at 12:51 AM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
>>
>> Hi Jeff,
>>
>> I realized that there is one thing missing here: there is no way to
>> associate this CEC device with the corresponding HDMI interface.
>>
>> Typically this is done either via the device tree (hdmi-phandle property,
>> see cec_notifier_parse_hdmi_phandle()) or ACPI (see cros-ec or seco drivers).
>>
>> How is this ch7322 used? Chromebooks? Intel or ARM based?
>>
>> The link with the HDMI interface is needed for the CEC_ADAP_G_CONNECTOR_INFO
>> ioctl implementation. See:
>>
>> https://hverkuil.home.xs4all.nl/spec/userspace-api/cec/cec-ioc-adap-g-conn-info.html
>>
>> Regards,
>>
>> Hans
>>
>> On 15/06/2020 21:38, Jeff Chase wrote:
>>> Add a CEC device driver for the Chrontel ch7322 CEC conroller.
>>> This is an I2C device capable of sending and receiving CEC messages.
>>>
>>> Signed-off-by: Jeff Chase <jnchase@google.com>
>>> ---
>>> MAINTAINERS | 1 +
>>> drivers/media/cec/Kconfig | 1 +
>>> drivers/media/cec/Makefile | 2 +-
>>> drivers/media/cec/i2c/Kconfig | 14 +
>>> drivers/media/cec/i2c/Makefile | 5 +
>>> drivers/media/cec/i2c/ch7322.c | 503 +++++++++++++++++++++++++++++++++
>>> 6 files changed, 525 insertions(+), 1 deletion(-)
>>> create mode 100644 drivers/media/cec/i2c/Kconfig
>>> create mode 100644 drivers/media/cec/i2c/Makefile
>>> create mode 100644 drivers/media/cec/i2c/ch7322.c
>>>
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index c1c17ca17830..e2341eaaaf5e 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -4107,6 +4107,7 @@ L: linux-media@vger.kernel.org
>>> S: Maintained
>>> T: git git://linuxtv.org/media_tree.git
>>> F: Documentation/devicetree/bindings/media/i2c/chontel,ch7322.yaml
>>> +F: drivers/media/cec/i2c/ch7322.c
>>>
>>> CIRRUS LOGIC AUDIO CODEC DRIVERS
>>> M: James Schulman <james.schulman@cirrus.com>
>>> diff --git a/drivers/media/cec/Kconfig b/drivers/media/cec/Kconfig
>>> index 7e830444bdbb..9ba3a00dce31 100644
>>> --- a/drivers/media/cec/Kconfig
>>> +++ b/drivers/media/cec/Kconfig
>>> @@ -33,6 +33,7 @@ menuconfig MEDIA_CEC_SUPPORT
>>> adapter that supports HDMI CEC.
>>>
>>> if MEDIA_CEC_SUPPORT
>>> +source "drivers/media/cec/i2c/Kconfig"
>>> source "drivers/media/cec/platform/Kconfig"
>>> source "drivers/media/cec/usb/Kconfig"
>>> endif
>>> diff --git a/drivers/media/cec/Makefile b/drivers/media/cec/Makefile
>>> index 74e80e1b3571..23539339bc81 100644
>>> --- a/drivers/media/cec/Makefile
>>> +++ b/drivers/media/cec/Makefile
>>> @@ -1,2 +1,2 @@
>>> # SPDX-License-Identifier: GPL-2.0
>>> -obj-y += core/ platform/ usb/
>>> +obj-y += core/ i2c/ platform/ usb/
>>> diff --git a/drivers/media/cec/i2c/Kconfig b/drivers/media/cec/i2c/Kconfig
>>> new file mode 100644
>>> index 000000000000..e445ca2110b3
>>> --- /dev/null
>>> +++ b/drivers/media/cec/i2c/Kconfig
>>> @@ -0,0 +1,14 @@
>>> +# SPDX-License-Identifier: GPL-2.0
>>> +#
>>> +# I2C drivers
>>> +
>>> +config CEC_CH7322
>>> + tristate "Chrontel CH7322 CEC controller"
>>> + select I2C
>>> + select REGMAP_I2C
>>> + select CEC_CORE
>>> + help
>>> + This is a driver for the Chrontel CH7322 CEC controller. It uses the
>>> + generic CEC framework interface.
>>> + CEC bus is present in the HDMI connector and enables communication
>>> + between compatible devices.
>>> diff --git a/drivers/media/cec/i2c/Makefile b/drivers/media/cec/i2c/Makefile
>>> new file mode 100644
>>> index 000000000000..d7496dfd0fa4
>>> --- /dev/null
>>> +++ b/drivers/media/cec/i2c/Makefile
>>> @@ -0,0 +1,5 @@
>>> +# SPDX-License-Identifier: GPL-2.0
>>> +#
>>> +# Makefile for the CEC I2C device drivers.
>>> +#
>>> +obj-$(CONFIG_CEC_CH7322) += ch7322.o
>>> diff --git a/drivers/media/cec/i2c/ch7322.c b/drivers/media/cec/i2c/ch7322.c
>>> new file mode 100644
>>> index 000000000000..4e6976a51774
>>> --- /dev/null
>>> +++ b/drivers/media/cec/i2c/ch7322.c
>>> @@ -0,0 +1,503 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * Driver for the Chrontel CH7322 CEC Controller
>>> + *
>>> + * Copyright 2020 Google LLC.
>>> + */
>>> +
>>> +/*
>>> + * Notes
>>> + *
>>> + * - This device powers on in Auto Mode which has limited functionality. This
>>> + * driver disables Auto Mode when it attaches.
>>> + *
>>> + */
>>> +
>>> +#include <linux/cec.h>
>>> +#include <linux/i2c.h>
>>> +#include <linux/interrupt.h>
>>> +#include <linux/module.h>
>>> +#include <linux/mutex.h>
>>> +#include <linux/regmap.h>
>>> +#include <media/cec.h>
>>> +
>>> +#define CH7322_WRITE 0x00
>>> +#define CH7322_WRITE_MSENT 0x80
>>> +#define CH7322_WRITE_BOK 0x40
>>> +#define CH7322_WRITE_NMASK 0x0f
>>> +
>>> +/* Write buffer is 0x01-0x10 */
>>> +#define CH7322_WRBUF 0x01
>>> +#define CH7322_WRBUF_LEN 0x10
>>> +
>>> +#define CH7322_READ 0x40
>>> +#define CH7322_READ_NRDT 0x80
>>> +#define CH7322_READ_MSENT 0x20
>>> +#define CH7322_READ_NMASK 0x0f
>>> +
>>> +/* Read buffer is 0x41-0x50 */
>>> +#define CH7322_RDBUF 0x41
>>> +#define CH7322_RDBUF_LEN 0x10
>>> +
>>> +#define CH7322_MODE 0x11
>>> +#define CH7322_MODE_AUTO 0x78
>>> +#define CH7322_MODE_SW 0xb5
>>> +
>>> +#define CH7322_RESET 0x12
>>> +#define CH7322_RESET_RST 0x00
>>> +
>>> +#define CH7322_POWER 0x13
>>> +#define CH7322_POWER_FPD 0x04
>>> +
>>> +#define CH7322_CFG0 0x17
>>> +#define CH7322_CFG0_EOBEN 0x40
>>> +#define CH7322_CFG0_PEOB 0x20
>>> +#define CH7322_CFG0_CLRSPP 0x10
>>> +#define CH7322_CFG0_FLOW 0x08
>>> +
>>> +#define CH7322_CFG1 0x1a
>>> +#define CH7322_CFG1_STDBYO 0x04
>>> +#define CH7322_CFG1_HPBP 0x02
>>> +#define CH7322_CFG1_PIO 0x01
>>> +
>>> +#define CH7322_INTCTL 0x1b
>>> +#define CH7322_INTCTL_INTPB 0x80
>>> +#define CH7322_INTCTL_STDBY 0x40
>>> +#define CH7322_INTCTL_HPDFALL 0x20
>>> +#define CH7322_INTCTL_HPDRISE 0x10
>>> +#define CH7322_INTCTL_RXMSG 0x08
>>> +#define CH7322_INTCTL_TXMSG 0x04
>>> +#define CH7322_INTCTL_NEWPHA 0x02
>>> +#define CH7322_INTCTL_ERROR 0x01
>>> +
>>> +#define CH7322_DVCLKFNH 0x1d
>>> +#define CH7322_DVCLKFNL 0x1e
>>> +
>>> +#define CH7322_CTL 0x31
>>> +#define CH7322_CTL_FSTDBY 0x80
>>> +#define CH7322_CTL_PLSEN 0x40
>>> +#define CH7322_CTL_PLSPB 0x20
>>> +#define CH7322_CTL_SPADL 0x10
>>> +#define CH7322_CTL_HINIT 0x08
>>> +#define CH7322_CTL_WPHYA 0x04
>>> +#define CH7322_CTL_H1T 0x02
>>> +#define CH7322_CTL_S1T 0x01
>>> +
>>> +#define CH7322_PAWH 0x32
>>> +#define CH7322_PAWL 0x33
>>> +
>>> +#define CH7322_ADDLW 0x34
>>> +#define CH7322_ADDLW_MASK 0xf0
>>> +
>>> +#define CH7322_ADDLR 0x3d
>>> +#define CH7322_ADDLR_HPD 0x80
>>> +#define CH7322_ADDLR_MASK 0x0f
>>> +
>>> +#define CH7322_INTDATA 0x3e
>>> +#define CH7322_INTDATA_MODE 0x80
>>> +#define CH7322_INTDATA_STDBY 0x40
>>> +#define CH7322_INTDATA_HPDFALL 0x20
>>> +#define CH7322_INTDATA_HPDRISE 0x10
>>> +#define CH7322_INTDATA_RXMSG 0x08
>>> +#define CH7322_INTDATA_TXMSG 0x04
>>> +#define CH7322_INTDATA_NEWPHA 0x02
>>> +#define CH7322_INTDATA_ERROR 0x01
>>> +
>>> +#define CH7322_EVENT 0x3F
>>> +#define CH7322_EVENT_TXERR 0x80
>>> +#define CH7322_EVENT_HRST 0x40
>>> +#define CH7322_EVENT_HFST 0x20
>>> +#define CH7322_EVENT_PHACHG 0x10
>>> +#define CH7322_EVENT_ACTST 0x08
>>> +#define CH7322_EVENT_PHARDY 0x04
>>> +#define CH7322_EVENT_BSOK 0x02
>>> +#define CH7322_EVENT_ERRADCF 0x01
>>> +
>>> +#define CH7322_DID 0x51
>>> +#define CH7322_DID_CH7322 0x5b
>>> +#define CH7322_DID_CH7323 0x5f
>>> +
>>> +#define CH7322_REVISIONID 0x52
>>> +
>>> +#define CH7322_PARH 0x53
>>> +#define CH7322_PARL 0x54
>>> +
>>> +#define CH7322_IOCFG2 0x75
>>> +#define CH7322_IOCFG_CIO 0x80
>>> +#define CH7322_IOCFG_IOCFGMASK 0x78
>>> +#define CH7322_IOCFG_AUDIO 0x04
>>> +#define CH7322_IOCFG_SPAMST 0x02
>>> +#define CH7322_IOCFG_SPAMSP 0x01
>>> +
>>> +#define CH7322_CTL3 0x7b
>>> +#define CH7322_CTL3_SWENA 0x80
>>> +#define CH7322_CTL3_FC_INIT 0x40
>>> +#define CH7322_CTL3_SML_FL 0x20
>>> +#define CH7322_CTL3_SM_RDST 0x10
>>> +#define CH7322_CTL3_SPP_CIAH 0x08
>>> +#define CH7322_CTL3_SPP_CIAL 0x04
>>> +#define CH7322_CTL3_SPP_ACTH 0x02
>>> +#define CH7322_CTL3_SPP_ACTL 0x01
>>> +
>>> +/* BOK status means NACK */
>>> +#define CH7322_TX_FLAG_NACK BIT(0)
>>> +/* Device will retry automatically */
>>> +#define CH7322_TX_FLAG_RETRY BIT(1)
>>> +
>>> +struct ch7322 {
>>> + struct i2c_client *i2c;
>>> + struct regmap *regmap;
>>> + struct cec_adapter *cec;
>>> + struct mutex mutex; /* device access mutex */
>>> + u8 tx_flags;
>>> +};
>>> +
>>> +static const struct regmap_config ch7322_regmap = {
>>> + .reg_bits = 8,
>>> + .val_bits = 8,
>>> + .max_register = 0x7f,
>>> + .disable_locking = true,
>>> +
>>> +};
>>> +
>>> +static int ch7322_send_message(struct ch7322 *ch7322, const struct cec_msg *msg)
>>> +{
>>> + unsigned int val;
>>> + unsigned int len = msg->len;
>>> + int ret;
>>> + int i;
>>> +
>>> + WARN_ON(!mutex_is_locked(&ch7322->mutex));
>>> +
>>> + if (len > CH7322_WRBUF_LEN || len < 1)
>>> + return -EINVAL;
>>> +
>>> + ret = regmap_read(ch7322->regmap, CH7322_WRITE, &val);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + /* Buffer not ready */
>>> + if (!(val & CH7322_WRITE_MSENT))
>>> + return -EBUSY;
>>> +
>>> + if (cec_msg_opcode(msg) == -1 &&
>>> + cec_msg_initiator(msg) == cec_msg_destination(msg)) {
>>> + ch7322->tx_flags = CH7322_TX_FLAG_NACK | CH7322_TX_FLAG_RETRY;
>>> + } else if (cec_msg_is_broadcast(msg)) {
>>> + ch7322->tx_flags = CH7322_TX_FLAG_NACK;
>>> + } else {
>>> + ch7322->tx_flags = CH7322_TX_FLAG_RETRY;
>>> + }
>>> +
>>> + ret = regmap_write(ch7322->regmap, CH7322_WRITE, len - 1);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + for (i = 0; i < len; i++) {
>>> + ret = regmap_write(ch7322->regmap,
>>> + CH7322_WRBUF + i, msg->msg[i]);
>>> + if (ret)
>>> + return ret;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int ch7322_receive_message(struct ch7322 *ch7322, struct cec_msg *msg)
>>> +{
>>> + unsigned int val;
>>> + int ret = 0;
>>> + int i;
>>> +
>>> + WARN_ON(!mutex_is_locked(&ch7322->mutex));
>>> +
>>> + ret = regmap_read(ch7322->regmap, CH7322_READ, &val);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + /* Message not ready */
>>> + if (!(val & CH7322_READ_NRDT))
>>> + return -EIO;
>>> +
>>> + msg->len = (val & CH7322_READ_NMASK) + 1;
>>> +
>>> + /* Read entire RDBUF to clear state */
>>> + for (i = 0; i < CH7322_RDBUF_LEN; i++) {
>>> + ret = regmap_read(ch7322->regmap, CH7322_RDBUF + i, &val);
>>> + if (ret)
>>> + return ret;
>>> + msg->msg[i] = (u8)val;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static void ch7322_tx_done(struct ch7322 *ch7322)
>>> +{
>>> + int ret;
>>> + unsigned int val;
>>> + u8 status, flags;
>>> +
>>> + mutex_lock(&ch7322->mutex);
>>> + ret = regmap_read(ch7322->regmap, CH7322_WRITE, &val);
>>> + flags = ch7322->tx_flags;
>>> + mutex_unlock(&ch7322->mutex);
>>> +
>>> + /*
>>> + * The device returns a one-bit OK status which usually means ACK but
>>> + * actually means NACK when sending a logical address query or a
>>> + * broadcast.
>>> + */
>>> + if (ret)
>>> + status = CEC_TX_STATUS_ERROR;
>>> + else if ((val & CH7322_WRITE_BOK) && (flags & CH7322_TX_FLAG_NACK))
>>> + status = CEC_TX_STATUS_NACK;
>>> + else if (val & CH7322_WRITE_BOK)
>>> + status = CEC_TX_STATUS_OK;
>>> + else if (flags & CH7322_TX_FLAG_NACK)
>>> + status = CEC_TX_STATUS_OK;
>>> + else
>>> + status = CEC_TX_STATUS_NACK;
>>> +
>>> + if (status == CEC_TX_STATUS_NACK && (flags & CH7322_TX_FLAG_RETRY))
>>> + status |= CEC_TX_STATUS_MAX_RETRIES;
>>> +
>>> + cec_transmit_attempt_done(ch7322->cec, status);
>>> +}
>>> +
>>> +static void ch7322_rx_done(struct ch7322 *ch7322)
>>> +{
>>> + struct cec_msg msg;
>>> + int ret;
>>> +
>>> + mutex_lock(&ch7322->mutex);
>>> + ret = ch7322_receive_message(ch7322, &msg);
>>> + mutex_unlock(&ch7322->mutex);
>>> +
>>> + if (ret)
>>> + dev_err(&ch7322->i2c->dev, "cec receive error: %d\n", ret);
>>> + else
>>> + cec_received_msg(ch7322->cec, &msg);
>>> +}
>>> +
>>> +/*
>>> + * This device can either monitor the DDC lines to obtain the physical address
>>> + * or it can allow the host to program it. This driver lets the device obtain
>>> + * it.
>>> + */
>>> +static void ch7322_phys_addr(struct ch7322 *ch7322)
>>> +{
>>> + unsigned int pah, pal;
>>> + int ret = 0;
>>> +
>>> + mutex_lock(&ch7322->mutex);
>>> + ret |= regmap_read(ch7322->regmap, CH7322_PARH, &pah);
>>> + ret |= regmap_read(ch7322->regmap, CH7322_PARL, &pal);
>>> + mutex_unlock(&ch7322->mutex);
>>> +
>>> + if (ret)
>>> + dev_err(&ch7322->i2c->dev, "phys addr error\n");
>>> + else
>>> + cec_s_phys_addr(ch7322->cec, pal | (pah << 8), false);
>>> +}
>>> +
>>> +static void ch7322_handle_events(struct ch7322 *ch7322)
>>> +{
>>> + unsigned int data = 0;
>>> +
>>> + mutex_lock(&ch7322->mutex);
>>> + regmap_read(ch7322->regmap, CH7322_INTDATA, &data);
>>> + regmap_write(ch7322->regmap, CH7322_INTDATA, data);
>>> + mutex_unlock(&ch7322->mutex);
>>> +
>>> + if (data & CH7322_INTDATA_HPDFALL)
>>> + cec_phys_addr_invalidate(ch7322->cec);
>>> +
>>> + if (data & CH7322_INTDATA_TXMSG)
>>> + ch7322_tx_done(ch7322);
>>> +
>>> + if (data & CH7322_INTDATA_RXMSG)
>>> + ch7322_rx_done(ch7322);
>>> +
>>> + if (data & CH7322_INTDATA_NEWPHA)
>>> + ch7322_phys_addr(ch7322);
>>> +
>>> + if (data & CH7322_INTDATA_ERROR)
>>> + dev_dbg(&ch7322->i2c->dev, "unknown error\n");
>>> +}
>>> +
>>> +static irqreturn_t ch7322_irq(int irq, void *dev)
>>> +{
>>> + struct ch7322 *ch7322 = dev;
>>> +
>>> + ch7322_handle_events(ch7322);
>>> +
>>> + return IRQ_HANDLED;
>>> +}
>>> +
>>> +/* This device is always enabled */
>>> +static int ch7322_cec_adap_enable(struct cec_adapter *adap, bool enable)
>>> +{
>>> + return 0;
>>> +}
>>> +
>>> +static int ch7322_cec_adap_log_addr(struct cec_adapter *adap, u8 log_addr)
>>> +{
>>> + struct ch7322 *ch7322 = cec_get_drvdata(adap);
>>> + int ret;
>>> +
>>> + mutex_lock(&ch7322->mutex);
>>> + ret = regmap_update_bits(ch7322->regmap, CH7322_ADDLW,
>>> + CH7322_ADDLW_MASK, log_addr << 4);
>>> + mutex_unlock(&ch7322->mutex);
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static int ch7322_cec_adap_transmit(struct cec_adapter *adap, u8 attempts,
>>> + u32 signal_free_time, struct cec_msg *msg)
>>> +{
>>> + struct ch7322 *ch7322 = cec_get_drvdata(adap);
>>> + int ret;
>>> +
>>> + mutex_lock(&ch7322->mutex);
>>> + ret = ch7322_send_message(ch7322, msg);
>>> + mutex_unlock(&ch7322->mutex);
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static const struct cec_adap_ops ch7322_cec_adap_ops = {
>>> + .adap_enable = ch7322_cec_adap_enable,
>>> + .adap_log_addr = ch7322_cec_adap_log_addr,
>>> + .adap_transmit = ch7322_cec_adap_transmit,
>>> +};
>>> +
>>> +static int ch7322_probe(struct i2c_client *client)
>>> +{
>>> + struct ch7322 *ch7322;
>>> + int ret;
>>> + unsigned int val;
>>> +
>>> + ch7322 = devm_kzalloc(&client->dev, sizeof(*ch7322), GFP_KERNEL);
>>> + if (!ch7322)
>>> + return -ENOMEM;
>>> +
>>> + ch7322->regmap = devm_regmap_init_i2c(client, &ch7322_regmap);
>>> + if (IS_ERR(ch7322->regmap))
>>> + return PTR_ERR(ch7322->regmap);
>>> +
>>> + ret = regmap_read(ch7322->regmap, CH7322_DID, &val);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + if (val != CH7322_DID_CH7322)
>>> + return -ENOTSUPP;
>>> +
>>> + mutex_init(&ch7322->mutex);
>>> + ch7322->i2c = client;
>>> + ch7322->tx_flags = 0;
>>> +
>>> + i2c_set_clientdata(client, ch7322);
>>> +
>>> + /* Disable auto mode */
>>> + ret = regmap_write(ch7322->regmap, CH7322_MODE, CH7322_MODE_SW);
>>> + if (ret)
>>> + goto err_mutex;
>>> +
>>> + /* Enable logical address register */
>>> + ret = regmap_update_bits(ch7322->regmap, CH7322_CTL,
>>> + CH7322_CTL_SPADL, CH7322_CTL_SPADL);
>>> + if (ret)
>>> + goto err_mutex;
>>> +
>>> + ch7322->cec = cec_allocate_adapter(&ch7322_cec_adap_ops, ch7322,
>>> + dev_name(&client->dev),
>>> + CEC_CAP_DEFAULTS, 1);
>>> +
>>> + if (IS_ERR(ch7322->cec)) {
>>> + ret = PTR_ERR(ch7322->cec);
>>> + goto err_mutex;
>>> + }
>>> +
>>> + ret = cec_register_adapter(ch7322->cec, &client->dev);
>>> + if (ret) {
>>> + cec_delete_adapter(ch7322->cec);
>>> + goto err_mutex;
>>> + }
>>> +
>>> + /* Configure and mask interrupt */
>>> + ret = regmap_write(ch7322->regmap, CH7322_CFG1, 0);
>>> + if (ret)
>>> + goto err_cec;
>>> +
>>> + ret = regmap_write(ch7322->regmap, CH7322_INTCTL, CH7322_INTCTL_INTPB);
>>> + if (ret)
>>> + goto err_cec;
>>> +
>>> + ret = devm_request_threaded_irq(&client->dev, client->irq, NULL,
>>> + ch7322_irq,
>>> + IRQF_ONESHOT | IRQF_TRIGGER_RISING,
>>> + client->name, ch7322);
>>> + if (ret)
>>> + goto err_cec;
>>> +
>>> + ch7322_handle_events(ch7322);
>>> +
>>> + /* Unmask interrupt */
>>> + mutex_lock(&ch7322->mutex);
>>> + ret = regmap_write(ch7322->regmap, CH7322_INTCTL, 0xff);
>>> + mutex_unlock(&ch7322->mutex);
>>> +
>>> + if (ret)
>>> + goto err_cec;
>>> +
>>> + dev_info(&client->dev, "device registered\n");
>>> +
>>> + return 0;
>>> +
>>> +err_cec:
>>> + cec_unregister_adapter(ch7322->cec);
>>> +
>>> +err_mutex:
>>> + mutex_destroy(&ch7322->mutex);
>>> + return ret;
>>> +}
>>> +
>>> +static int ch7322_remove(struct i2c_client *client)
>>> +{
>>> + struct ch7322 *ch7322 = i2c_get_clientdata(client);
>>> +
>>> + /* Mask interrupt */
>>> + mutex_lock(&ch7322->mutex);
>>> + regmap_write(ch7322->regmap, CH7322_INTCTL, CH7322_INTCTL_INTPB);
>>> + mutex_unlock(&ch7322->mutex);
>>> +
>>> + cec_unregister_adapter(ch7322->cec);
>>> + mutex_destroy(&ch7322->mutex);
>>> +
>>> + dev_info(&client->dev, "device unregistered\n");
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static const struct of_device_id ch7322_of_match[] = {
>>> + { .compatible = "chrontel,ch7322", },
>>> + {},
>>> +};
>>> +MODULE_DEVICE_TABLE(of, ch7322_of_match);
>>> +
>>> +static struct i2c_driver ch7322_i2c_driver = {
>>> + .driver = {
>>> + .name = "ch7322",
>>> + .of_match_table = of_match_ptr(ch7322_of_match),
>>> + },
>>> + .probe_new = ch7322_probe,
>>> + .remove = ch7322_remove,
>>> +};
>>> +
>>> +module_i2c_driver(ch7322_i2c_driver);
>>> +
>>> +MODULE_DESCRIPTION("Chrontel CH7322 CEC Controller Driver");
>>> +MODULE_AUTHOR("Jeff Chase <jnchase@google.com>");
>>> +MODULE_LICENSE("GPL");
>>>
>>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 2/2] media: cec: i2c: ch7322: Add ch7322 CEC controller driver
2020-06-18 10:05 ` Hans Verkuil
@ 2020-06-18 16:25 ` Jeff Chase
2020-06-18 16:33 ` Hans Verkuil
0 siblings, 1 reply; 13+ messages in thread
From: Jeff Chase @ 2020-06-18 16:25 UTC (permalink / raw)
To: Hans Verkuil; +Cc: linux-media, mchehab, robh+dt, devicetree
On Thu, Jun 18, 2020 at 3:05 AM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
>
> On 18/06/2020 10:59, Jeff Chase wrote:
> > Hi Hans,
> >
> > We are using two of these in an Intel-based Chromebox. I see that the
> > cros-ec and seco drivers just statically define the PCI BDF of the
> > Intel graphics device for their boards. I don't see an example of ACPI
> > passing this information. I can copy cros-ec and seco by adding a
> > board table and then use the UID of each device to select the correct
> > port. Adding board-specific configuration to the driver doesn't seem
> > ideal but I'm not sure what the proper way to pass this using ACPI is.
>
> You are right, it's not ACPI, it's using DMI matching.
>
> I have zero knowledge about ACPI, so I have no idea if there is some standard
> method of retrieving this association via ACPI.
I'm not very familiar with ACPI either. I looked for but did not find
an ACPI equivalent of_get_mac_address().
I believe it's possible to reference the PCI node but it would take a
bit of work on both the coreboot and linux side.
>
> This particular chip can actually be used both with DMI matching but also
> on an ARM with device tree, but since you can't test this on an ARM board,
> there is no point in adding support for that.
>
> However, compared to the cros-ec and seco drivers you can do something a bit
> different here: those drivers just return -ENODEV if there is no match, but
> since this driver reads the EDID it can just continue as long as it does not
> set the CEC_CAP_CONNECTOR_INFO capability.
Is it necessary to add support if we don't set CEC_CAP_CONNECTOR_INFO?
Thanks,
Jeff
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 2/2] media: cec: i2c: ch7322: Add ch7322 CEC controller driver
2020-06-18 16:25 ` Jeff Chase
@ 2020-06-18 16:33 ` Hans Verkuil
2020-06-18 16:40 ` Jeff Chase
0 siblings, 1 reply; 13+ messages in thread
From: Hans Verkuil @ 2020-06-18 16:33 UTC (permalink / raw)
To: Jeff Chase; +Cc: linux-media, mchehab, robh+dt, devicetree
On 18/06/2020 18:25, Jeff Chase wrote:
> On Thu, Jun 18, 2020 at 3:05 AM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
>>
>> On 18/06/2020 10:59, Jeff Chase wrote:
>>> Hi Hans,
>>>
>>> We are using two of these in an Intel-based Chromebox. I see that the
>>> cros-ec and seco drivers just statically define the PCI BDF of the
>>> Intel graphics device for their boards. I don't see an example of ACPI
>>> passing this information. I can copy cros-ec and seco by adding a
>>> board table and then use the UID of each device to select the correct
>>> port. Adding board-specific configuration to the driver doesn't seem
>>> ideal but I'm not sure what the proper way to pass this using ACPI is.
>>
>> You are right, it's not ACPI, it's using DMI matching.
>>
>> I have zero knowledge about ACPI, so I have no idea if there is some standard
>> method of retrieving this association via ACPI.
>
> I'm not very familiar with ACPI either. I looked for but did not find
> an ACPI equivalent of_get_mac_address().
>
> I believe it's possible to reference the PCI node but it would take a
> bit of work on both the coreboot and linux side.
>
>>
>> This particular chip can actually be used both with DMI matching but also
>> on an ARM with device tree, but since you can't test this on an ARM board,
>> there is no point in adding support for that.
>>
>> However, compared to the cros-ec and seco drivers you can do something a bit
>> different here: those drivers just return -ENODEV if there is no match, but
>> since this driver reads the EDID it can just continue as long as it does not
>> set the CEC_CAP_CONNECTOR_INFO capability.
>
> Is it necessary to add support if we don't set CEC_CAP_CONNECTOR_INFO?
It is very desirable. Otherwise userspace will not know which CEC device is associated
with which HDMI device. Since you are using two of these chips for a Chromebox, I
suspect that you actually need to support this.
Also, I am (slowly) working on wiring support for this in all CEC transmitter drivers,
so I prefer not to add CEC drivers without support for this.
I really should have noticed much earlier that support for this was missing. My
apologies for that.
Regards,
Hans
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 2/2] media: cec: i2c: ch7322: Add ch7322 CEC controller driver
2020-06-18 16:33 ` Hans Verkuil
@ 2020-06-18 16:40 ` Jeff Chase
2020-06-20 4:56 ` Jeff Chase
0 siblings, 1 reply; 13+ messages in thread
From: Jeff Chase @ 2020-06-18 16:40 UTC (permalink / raw)
To: Hans Verkuil; +Cc: linux-media, mchehab, robh+dt, devicetree
On Thu, Jun 18, 2020 at 9:33 AM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
>
> On 18/06/2020 18:25, Jeff Chase wrote:
> > On Thu, Jun 18, 2020 at 3:05 AM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
> >>
> >> On 18/06/2020 10:59, Jeff Chase wrote:
> >>> Hi Hans,
> >>>
> >>> We are using two of these in an Intel-based Chromebox. I see that the
> >>> cros-ec and seco drivers just statically define the PCI BDF of the
> >>> Intel graphics device for their boards. I don't see an example of ACPI
> >>> passing this information. I can copy cros-ec and seco by adding a
> >>> board table and then use the UID of each device to select the correct
> >>> port. Adding board-specific configuration to the driver doesn't seem
> >>> ideal but I'm not sure what the proper way to pass this using ACPI is.
> >>
> >> You are right, it's not ACPI, it's using DMI matching.
> >>
> >> I have zero knowledge about ACPI, so I have no idea if there is some standard
> >> method of retrieving this association via ACPI.
> >
> > I'm not very familiar with ACPI either. I looked for but did not find
> > an ACPI equivalent of_get_mac_address().
> >
> > I believe it's possible to reference the PCI node but it would take a
> > bit of work on both the coreboot and linux side.
> >
> >>
> >> This particular chip can actually be used both with DMI matching but also
> >> on an ARM with device tree, but since you can't test this on an ARM board,
> >> there is no point in adding support for that.
> >>
> >> However, compared to the cros-ec and seco drivers you can do something a bit
> >> different here: those drivers just return -ENODEV if there is no match, but
> >> since this driver reads the EDID it can just continue as long as it does not
> >> set the CEC_CAP_CONNECTOR_INFO capability.
> >
> > Is it necessary to add support if we don't set CEC_CAP_CONNECTOR_INFO?
>
> It is very desirable. Otherwise userspace will not know which CEC device is associated
> with which HDMI device. Since you are using two of these chips for a Chromebox, I
> suspect that you actually need to support this.
>
> Also, I am (slowly) working on wiring support for this in all CEC transmitter drivers,
> so I prefer not to add CEC drivers without support for this.
Okay, understood. I'll look into using DMI matching.
Thanks,
Jeff
>
> I really should have noticed much earlier that support for this was missing. My
> apologies for that.
>
> Regards,
>
> Hans
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 2/2] media: cec: i2c: ch7322: Add ch7322 CEC controller driver
2020-06-18 16:40 ` Jeff Chase
@ 2020-06-20 4:56 ` Jeff Chase
2020-06-20 8:57 ` Hans Verkuil
0 siblings, 1 reply; 13+ messages in thread
From: Jeff Chase @ 2020-06-20 4:56 UTC (permalink / raw)
To: Hans Verkuil; +Cc: linux-media, mchehab, robh+dt, devicetree
> > >> This particular chip can actually be used both with DMI matching but also
> > >> on an ARM with device tree, but since you can't test this on an ARM board,
> > >> there is no point in adding support for that.
> > >>
> > >> However, compared to the cros-ec and seco drivers you can do something a bit
> > >> different here: those drivers just return -ENODEV if there is no match, but
> > >> since this driver reads the EDID it can just continue as long as it does not
> > >> set the CEC_CAP_CONNECTOR_INFO capability.
Hi Hans,
After looking into this I don't think I want to actually register the
cec adapter with the hdmi port's cec notifier because then it will
start receiving physical address updates from the port. Since the
driver already updates the physical address itself it seems like that
would cause some issues.
I'm looking at just adding the connector to the adapter directly using
`cec_s_conn_info()`. But it doesn't look like I can get the conn_info
without registering with the notifier. Do you have any thoughts here
before I try tackling changes to the cec framework?
Thanks,
Jeff
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 2/2] media: cec: i2c: ch7322: Add ch7322 CEC controller driver
2020-06-20 4:56 ` Jeff Chase
@ 2020-06-20 8:57 ` Hans Verkuil
0 siblings, 0 replies; 13+ messages in thread
From: Hans Verkuil @ 2020-06-20 8:57 UTC (permalink / raw)
To: Jeff Chase; +Cc: linux-media, mchehab, robh+dt, devicetree
On 20/06/2020 06:56, Jeff Chase wrote:
>>>>> This particular chip can actually be used both with DMI matching but also
>>>>> on an ARM with device tree, but since you can't test this on an ARM board,
>>>>> there is no point in adding support for that.
>>>>>
>>>>> However, compared to the cros-ec and seco drivers you can do something a bit
>>>>> different here: those drivers just return -ENODEV if there is no match, but
>>>>> since this driver reads the EDID it can just continue as long as it does not
>>>>> set the CEC_CAP_CONNECTOR_INFO capability.
>
> Hi Hans,
>
> After looking into this I don't think I want to actually register the
> cec adapter with the hdmi port's cec notifier because then it will
> start receiving physical address updates from the port. Since the
> driver already updates the physical address itself it seems like that
> would cause some issues.
>
> I'm looking at just adding the connector to the adapter directly using
> `cec_s_conn_info()`. But it doesn't look like I can get the conn_info
> without registering with the notifier. Do you have any thoughts here
> before I try tackling changes to the cec framework?
It's a very good point, I hadn't realized this complication.
The notifier support is definitely required for this, but there should be
a way to prevent the notifier from updating the physical address since it
is controlled by the CEC adapter instead.
Suggestion:
Add a 'bool adap_controls_phys_addr;' field to struct cec_adapter. If set to
true, then cec-notifier will skip the cec_s_phys_addr calls.
This driver then just has to set this field to true and it should work.
Regards,
Hans
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2020-06-20 8:57 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-15 19:38 [PATCH v5 0/2] CH7322 CEC controller driver Jeff Chase
2020-06-15 19:38 ` [PATCH v5 1/2] dt-bindings: Add ch7322 media i2c device Jeff Chase
2020-06-17 20:26 ` Rob Herring
2020-06-17 22:07 ` Jeff Chase
2020-06-15 19:38 ` [PATCH v5 2/2] media: cec: i2c: ch7322: Add ch7322 CEC controller driver Jeff Chase
2020-06-18 7:51 ` Hans Verkuil
2020-06-18 8:59 ` Jeff Chase
2020-06-18 10:05 ` Hans Verkuil
2020-06-18 16:25 ` Jeff Chase
2020-06-18 16:33 ` Hans Verkuil
2020-06-18 16:40 ` Jeff Chase
2020-06-20 4:56 ` Jeff Chase
2020-06-20 8:57 ` Hans Verkuil
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).