linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCHv2 0/7] cec: add error injection support
@ 2018-03-05 13:51 Hans Verkuil
  2018-03-05 13:51 ` [PATCHv2 1/7] cec: add core " Hans Verkuil
                   ` (7 more replies)
  0 siblings, 8 replies; 14+ messages in thread
From: Hans Verkuil @ 2018-03-05 13:51 UTC (permalink / raw)
  To: linux-media; +Cc: Wolfram Sang, Maxime Ripard, dri-devel

From: Hans Verkuil <hans.verkuil@cisco.com>

This patch series adds support for CEC error injection for drivers
using the CEC Pin Framework (cec-pin.c). There are two CEC drivers
currently using this framework: the sun4i Allwinner A10/A20 driver
and the cec-gpio driver. This patch series was developed with the
cec-gpio driver and a Raspberry Pi.

The CEC Pin Framework is meant for hardware that has no high-level
support but only direct low-level control of the bus (i.e. pull the
CEC line down or read the CEC line). Low-level bus access like that
is ideal to implement error injection since you have full control of
the bus and you can do anything you want.

This new error injection framework can create most if not all error
conditions that I could think of. We (Cisco) used it to verify our
own CEC implementation and in fact this error injection framework
was developed together with the low-level CEC analysis code in the
cec-ctl userspace utility to analyze what is happening on the bus.

I have been working on creating scripts that can test a remote CEC
adapter for low-level compliance with the CEC standard:

https://git.linuxtv.org/hverkuil/v4l-utils.git/log/?h=cec-pin-tests

(note: these scripts are for the v1 version of this patch series,
they need to be updated for this v2)

These scripts are not complete yet since it isn't smart enough to
tell the difference between different (but valid) interpretations
of the CEC specification and actual violations of the specification.
I plan to continue working on that since I would like to have a
test-suite that can check a CEC implementation automatically.

Special thanks go to Wolfram Sang since his i2c error injection
presentation at the Embedded Linux Conference Europe 2017 inspired
me to switch to debugfs for this instead of using ioctls.

Changes since v1:

- added 'mode' support (off/once/always/toggle).
- simplified the error injection data structures and logic.
- added patch 7 to log various errors in the 'status' debugfs file.

Regards,

	Hans


Hans Verkuil (7):
  cec: add core error injection support
  cec-core.rst: document the error injection ops
  cec-pin: create cec_pin_start_timer() function
  cec-pin-error-inj: parse/show error injection
  cec-pin: add error injection support
  cec-pin-error-inj.rst: document CEC Pin Error Injection
  cec-pin: improve status log

 .../media/cec-drivers/cec-pin-error-inj.rst        | 322 +++++++++++
 Documentation/media/cec-drivers/index.rst          |   1 +
 Documentation/media/kapi/cec-core.rst              |  72 ++-
 MAINTAINERS                                        |   1 +
 drivers/media/cec/Kconfig                          |   6 +
 drivers/media/cec/Makefile                         |   4 +
 drivers/media/cec/cec-core.c                       |  58 ++
 drivers/media/cec/cec-pin-error-inj.c              | 341 +++++++++++
 drivers/media/cec/cec-pin-priv.h                   | 124 +++-
 drivers/media/cec/cec-pin.c                        | 627 ++++++++++++++++++---
 include/media/cec.h                                |   5 +
 11 files changed, 1490 insertions(+), 71 deletions(-)
 create mode 100644 Documentation/media/cec-drivers/cec-pin-error-inj.rst
 create mode 100644 drivers/media/cec/cec-pin-error-inj.c

-- 
2.16.1

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCHv2 1/7] cec: add core error injection support
  2018-03-05 13:51 [PATCHv2 0/7] cec: add error injection support Hans Verkuil
@ 2018-03-05 13:51 ` Hans Verkuil
  2018-03-05 13:51 ` [PATCHv2 2/7] cec-core.rst: document the error injection ops Hans Verkuil
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Hans Verkuil @ 2018-03-05 13:51 UTC (permalink / raw)
  To: linux-media; +Cc: Wolfram Sang, Maxime Ripard, dri-devel, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

Add two new ops (error_inj_show and error_inj_parse_line) to support
error injection functionality for CEC adapters. If both are present,
then the core will add a new error-inj debugfs file that can be used
to see the current error injection commands and to set error injection
commands.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/cec/cec-core.c | 58 ++++++++++++++++++++++++++++++++++++++++++++
 include/media/cec.h          |  5 ++++
 2 files changed, 63 insertions(+)

diff --git a/drivers/media/cec/cec-core.c b/drivers/media/cec/cec-core.c
index e47ea22b3c23..ea3eccfdba15 100644
--- a/drivers/media/cec/cec-core.c
+++ b/drivers/media/cec/cec-core.c
@@ -195,6 +195,55 @@ void cec_register_cec_notifier(struct cec_adapter *adap,
 EXPORT_SYMBOL_GPL(cec_register_cec_notifier);
 #endif
 
+#ifdef CONFIG_DEBUG_FS
+static ssize_t cec_error_inj_write(struct file *file,
+	const char __user *ubuf, size_t count, loff_t *ppos)
+{
+	struct seq_file *sf = file->private_data;
+	struct cec_adapter *adap = sf->private;
+	char *buf;
+	char *line;
+	char *p;
+
+	buf = memdup_user_nul(ubuf, min_t(size_t, PAGE_SIZE, count));
+	if (IS_ERR(buf))
+		return PTR_ERR(buf);
+	p = buf;
+	while (p && *p && count >= 0) {
+		p = skip_spaces(p);
+		line = strsep(&p, "\n");
+		if (!*line || *line == '#')
+			continue;
+		if (!adap->ops->error_inj_parse_line(adap, line)) {
+			count = -EINVAL;
+			break;
+		}
+	}
+	kfree(buf);
+	return count;
+}
+
+static int cec_error_inj_show(struct seq_file *sf, void *unused)
+{
+	struct cec_adapter *adap = sf->private;
+
+	return adap->ops->error_inj_show(adap, sf);
+}
+
+static int cec_error_inj_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, cec_error_inj_show, inode->i_private);
+}
+
+static const struct file_operations cec_error_inj_fops = {
+	.open = cec_error_inj_open,
+	.write = cec_error_inj_write,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = single_release,
+};
+#endif
+
 struct cec_adapter *cec_allocate_adapter(const struct cec_adap_ops *ops,
 					 void *priv, const char *name, u32 caps,
 					 u8 available_las)
@@ -334,7 +383,16 @@ int cec_register_adapter(struct cec_adapter *adap,
 		pr_warn("cec-%s: Failed to create status file\n", adap->name);
 		debugfs_remove_recursive(adap->cec_dir);
 		adap->cec_dir = NULL;
+		return 0;
 	}
+	if (!adap->ops->error_inj_show || !adap->ops->error_inj_parse_line)
+		return 0;
+	adap->error_inj_file = debugfs_create_file("error-inj", 0644,
+						   adap->cec_dir, adap,
+						   &cec_error_inj_fops);
+	if (IS_ERR_OR_NULL(adap->error_inj_file))
+		pr_warn("cec-%s: Failed to create error-inj file\n",
+			adap->name);
 #endif
 	return 0;
 }
diff --git a/include/media/cec.h b/include/media/cec.h
index 9afba9b558df..41df048efc55 100644
--- a/include/media/cec.h
+++ b/include/media/cec.h
@@ -117,6 +117,10 @@ struct cec_adap_ops {
 	void (*adap_status)(struct cec_adapter *adap, struct seq_file *file);
 	void (*adap_free)(struct cec_adapter *adap);
 
+	/* Error injection callbacks */
+	int (*error_inj_show)(struct cec_adapter *adap, struct seq_file *sf);
+	bool (*error_inj_parse_line)(struct cec_adapter *adap, char *line);
+
 	/* High-level CEC message callback */
 	int (*received)(struct cec_adapter *adap, struct cec_msg *msg);
 };
@@ -189,6 +193,7 @@ struct cec_adapter {
 
 	struct dentry *cec_dir;
 	struct dentry *status_file;
+	struct dentry *error_inj_file;
 
 	u16 phys_addrs[15];
 	u32 sequence;
-- 
2.16.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCHv2 2/7] cec-core.rst: document the error injection ops
  2018-03-05 13:51 [PATCHv2 0/7] cec: add error injection support Hans Verkuil
  2018-03-05 13:51 ` [PATCHv2 1/7] cec: add core " Hans Verkuil
@ 2018-03-05 13:51 ` Hans Verkuil
  2018-03-05 13:51 ` [PATCHv2 3/7] cec-pin: create cec_pin_start_timer() function Hans Verkuil
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Hans Verkuil @ 2018-03-05 13:51 UTC (permalink / raw)
  To: linux-media; +Cc: Wolfram Sang, Maxime Ripard, dri-devel, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

Document the new core error injection callbacks.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 Documentation/media/kapi/cec-core.rst | 72 ++++++++++++++++++++++++++++++++++-
 1 file changed, 71 insertions(+), 1 deletion(-)

diff --git a/Documentation/media/kapi/cec-core.rst b/Documentation/media/kapi/cec-core.rst
index 62b9a1448177..a9f53f069a2d 100644
--- a/Documentation/media/kapi/cec-core.rst
+++ b/Documentation/media/kapi/cec-core.rst
@@ -110,11 +110,14 @@ your driver:
 		void (*adap_status)(struct cec_adapter *adap, struct seq_file *file);
 		void (*adap_free)(struct cec_adapter *adap);
 
+		/* Error injection callbacks */
+		...
+
 		/* High-level callbacks */
 		...
 	};
 
-The five low-level ops deal with various aspects of controlling the CEC adapter
+The seven low-level ops deal with various aspects of controlling the CEC adapter
 hardware:
 
 
@@ -286,6 +289,70 @@ handling the receive interrupt. The framework expects to see the cec_transmit_do
 call before the cec_received_msg call, otherwise it can get confused if the
 received message was in reply to the transmitted message.
 
+Optional: Implementing Error Injection Support
+----------------------------------------------
+
+If the CEC adapter supports Error Injection functionality, then that can
+be exposed through the Error Injection callbacks:
+
+.. code-block:: none
+
+	struct cec_adap_ops {
+		/* Low-level callbacks */
+		...
+
+		/* Error injection callbacks */
+		int (*error_inj_show)(struct cec_adapter *adap, struct seq_file *sf);
+		bool (*error_inj_parse_line)(struct cec_adapter *adap, char *line);
+
+		/* High-level CEC message callback */
+		...
+	};
+
+If both callbacks are set, then an ``error-inj`` file will appear in debugfs.
+The basic syntax is as follows:
+
+Leading spaces/tabs are ignored. If the next character is a ``#`` or the end of the
+line was reached, then the whole line is ignored. Otherwise a command is expected.
+
+This basic parsing is done in the CEC Framework. It is up to the driver to decide
+what commands to implement. The only requirement is that the command ``clear`` without
+any arguments must be implemented and that it will remove all current error injection
+commands.
+
+This ensures that you can always do ``echo clear >error-inj`` to clear any error
+injections without having to know the details of the driver-specific commands.
+
+Note that the output of ``error-inj`` shall be valid as input to ``error-inj``.
+So this must work:
+
+.. code-block:: none
+
+	$ cat error-inj >einj.txt
+	$ cat einj.txt >error-inj
+
+The first callback is called when this file is read and it should show the
+the current error injection state:
+
+.. c:function::
+	int (*error_inj_show)(struct cec_adapter *adap, struct seq_file *sf);
+
+It is recommended that it starts with a comment block with basic usage
+information. It returns 0 for success and an error otherwise.
+
+The second callback will parse commands written to the ``error-inj`` file:
+
+.. c:function::
+	bool (*error_inj_parse_line)(struct cec_adapter *adap, char *line);
+
+The ``line`` argument points to the start of the command. Any leading
+spaces or tabs have already been skipped. It is a single line only (so there
+are no embedded newlines) and it is 0-terminated. The callback is free to
+modify the contents of the buffer. It is only called for lines containing a
+command, so this callback is never called for empty lines or comment lines.
+
+Return true if the command was valid or false if there were syntax errors.
+
 Implementing the High-Level CEC Adapter
 ---------------------------------------
 
@@ -298,6 +365,9 @@ CEC protocol driven. The following high-level callbacks are available:
 		/* Low-level callbacks */
 		...
 
+		/* Error injection callbacks */
+		...
+
 		/* High-level CEC message callback */
 		int (*received)(struct cec_adapter *adap, struct cec_msg *msg);
 	};
-- 
2.16.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCHv2 3/7] cec-pin: create cec_pin_start_timer() function
  2018-03-05 13:51 [PATCHv2 0/7] cec: add error injection support Hans Verkuil
  2018-03-05 13:51 ` [PATCHv2 1/7] cec: add core " Hans Verkuil
  2018-03-05 13:51 ` [PATCHv2 2/7] cec-core.rst: document the error injection ops Hans Verkuil
@ 2018-03-05 13:51 ` Hans Verkuil
  2018-03-05 13:51 ` [PATCHv2 4/7] cec-pin-error-inj: parse/show error injection Hans Verkuil
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Hans Verkuil @ 2018-03-05 13:51 UTC (permalink / raw)
  To: linux-media; +Cc: Wolfram Sang, Maxime Ripard, dri-devel, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

This function will be needed for injecting a custom pulse.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/cec/cec-pin-priv.h |  2 ++
 drivers/media/cec/cec-pin.c      | 21 +++++++++++++--------
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/media/cec/cec-pin-priv.h b/drivers/media/cec/cec-pin-priv.h
index cf41c4236efd..4571a0001a9d 100644
--- a/drivers/media/cec/cec-pin-priv.h
+++ b/drivers/media/cec/cec-pin-priv.h
@@ -118,4 +118,6 @@ struct cec_pin {
 	u32				timer_sum_overrun;
 };
 
+void cec_pin_start_timer(struct cec_pin *pin);
+
 #endif
diff --git a/drivers/media/cec/cec-pin.c b/drivers/media/cec/cec-pin.c
index 8e834b9f72c6..67d6ea9f56b6 100644
--- a/drivers/media/cec/cec-pin.c
+++ b/drivers/media/cec/cec-pin.c
@@ -680,6 +680,18 @@ static int cec_pin_adap_log_addr(struct cec_adapter *adap, u8 log_addr)
 	return 0;
 }
 
+void cec_pin_start_timer(struct cec_pin *pin)
+{
+	if (pin->state != CEC_ST_RX_IRQ)
+		return;
+
+	atomic_set(&pin->work_irq_change, CEC_PIN_IRQ_UNCHANGED);
+	pin->ops->disable_irq(pin->adap);
+	cec_pin_high(pin);
+	cec_pin_to_idle(pin);
+	hrtimer_start(&pin->timer, ns_to_ktime(0), HRTIMER_MODE_REL);
+}
+
 static int cec_pin_adap_transmit(struct cec_adapter *adap, u8 attempts,
 				      u32 signal_free_time, struct cec_msg *msg)
 {
@@ -689,14 +701,7 @@ static int cec_pin_adap_transmit(struct cec_adapter *adap, u8 attempts,
 	pin->tx_msg = *msg;
 	pin->work_tx_status = 0;
 	pin->tx_bit = 0;
-	if (pin->state == CEC_ST_RX_IRQ) {
-		atomic_set(&pin->work_irq_change, CEC_PIN_IRQ_UNCHANGED);
-		pin->ops->disable_irq(adap);
-		cec_pin_high(pin);
-		cec_pin_to_idle(pin);
-		hrtimer_start(&pin->timer, ns_to_ktime(0),
-			      HRTIMER_MODE_REL);
-	}
+	cec_pin_start_timer(pin);
 	return 0;
 }
 
-- 
2.16.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCHv2 4/7] cec-pin-error-inj: parse/show error injection
  2018-03-05 13:51 [PATCHv2 0/7] cec: add error injection support Hans Verkuil
                   ` (2 preceding siblings ...)
  2018-03-05 13:51 ` [PATCHv2 3/7] cec-pin: create cec_pin_start_timer() function Hans Verkuil
@ 2018-03-05 13:51 ` Hans Verkuil
  2018-03-05 13:51 ` [PATCHv2 5/7] cec-pin: add error injection support Hans Verkuil
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Hans Verkuil @ 2018-03-05 13:51 UTC (permalink / raw)
  To: linux-media; +Cc: Wolfram Sang, Maxime Ripard, dri-devel, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

Add support to the CEC Pin framework to parse error injection commands
and to show them.

The next patch will do the actual implementation of this.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/cec/Kconfig             |   6 +
 drivers/media/cec/Makefile            |   4 +
 drivers/media/cec/cec-pin-error-inj.c | 341 ++++++++++++++++++++++++++++++++++
 drivers/media/cec/cec-pin-priv.h      |  71 +++++++
 drivers/media/cec/cec-pin.c           |   6 +
 5 files changed, 428 insertions(+)
 create mode 100644 drivers/media/cec/cec-pin-error-inj.c

diff --git a/drivers/media/cec/Kconfig b/drivers/media/cec/Kconfig
index 43428cec3a01..9c2b108c613a 100644
--- a/drivers/media/cec/Kconfig
+++ b/drivers/media/cec/Kconfig
@@ -4,3 +4,9 @@ config MEDIA_CEC_RC
 	depends on CEC_CORE=m || RC_CORE=y
 	---help---
 	  Pass on CEC remote control messages to the RC framework.
+
+config CEC_PIN_ERROR_INJ
+	bool "Enable CEC error injection support"
+	depends on CEC_PIN && DEBUG_FS
+	---help---
+	  This option enables CEC error injection using debugfs.
diff --git a/drivers/media/cec/Makefile b/drivers/media/cec/Makefile
index 41ee3325e1ea..29a2ab9e77c5 100644
--- a/drivers/media/cec/Makefile
+++ b/drivers/media/cec/Makefile
@@ -9,4 +9,8 @@ ifeq ($(CONFIG_CEC_PIN),y)
   cec-objs += cec-pin.o
 endif
 
+ifeq ($(CONFIG_CEC_PIN_ERROR_INJ),y)
+  cec-objs += cec-pin-error-inj.o
+endif
+
 obj-$(CONFIG_CEC_CORE) += cec.o
diff --git a/drivers/media/cec/cec-pin-error-inj.c b/drivers/media/cec/cec-pin-error-inj.c
new file mode 100644
index 000000000000..10f73def9df5
--- /dev/null
+++ b/drivers/media/cec/cec-pin-error-inj.c
@@ -0,0 +1,341 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2017 Cisco Systems, Inc. and/or its affiliates. All rights reserved.
+ */
+
+#include <linux/delay.h>
+#include <linux/slab.h>
+#include <linux/sched/types.h>
+
+#include <media/cec-pin.h>
+#include "cec-pin-priv.h"
+
+struct cec_error_inj_cmd {
+	unsigned int mode_offset;
+	int arg_idx;
+	const char *cmd;
+};
+
+static const struct cec_error_inj_cmd cec_error_inj_cmds[] = {
+	{ CEC_ERROR_INJ_RX_NACK_OFFSET, -1, "rx-nack" },
+	{ CEC_ERROR_INJ_RX_LOW_DRIVE_OFFSET,
+	  CEC_ERROR_INJ_RX_LOW_DRIVE_ARG_IDX, "rx-low-drive" },
+	{ CEC_ERROR_INJ_RX_ADD_BYTE_OFFSET, -1, "rx-add-byte" },
+	{ CEC_ERROR_INJ_RX_REMOVE_BYTE_OFFSET, -1, "rx-remove-byte" },
+	{ CEC_ERROR_INJ_RX_ARB_LOST_OFFSET,
+	  CEC_ERROR_INJ_RX_ARB_LOST_ARG_IDX, "rx-arb-lost" },
+
+	{ CEC_ERROR_INJ_TX_NO_EOM_OFFSET, -1, "tx-no-eom" },
+	{ CEC_ERROR_INJ_TX_EARLY_EOM_OFFSET, -1, "tx-early-eom" },
+	{ CEC_ERROR_INJ_TX_ADD_BYTES_OFFSET,
+	  CEC_ERROR_INJ_TX_ADD_BYTES_ARG_IDX, "tx-add-bytes" },
+	{ CEC_ERROR_INJ_TX_REMOVE_BYTE_OFFSET, -1, "tx-remove-byte" },
+	{ CEC_ERROR_INJ_TX_SHORT_BIT_OFFSET,
+	  CEC_ERROR_INJ_TX_SHORT_BIT_ARG_IDX, "tx-short-bit" },
+	{ CEC_ERROR_INJ_TX_LONG_BIT_OFFSET,
+	  CEC_ERROR_INJ_TX_LONG_BIT_ARG_IDX, "tx-long-bit" },
+	{ CEC_ERROR_INJ_TX_CUSTOM_BIT_OFFSET,
+	  CEC_ERROR_INJ_TX_CUSTOM_BIT_ARG_IDX, "tx-custom-bit" },
+	{ CEC_ERROR_INJ_TX_SHORT_START_OFFSET, -1, "tx-short-start" },
+	{ CEC_ERROR_INJ_TX_LONG_START_OFFSET, -1, "tx-long-start" },
+	{ CEC_ERROR_INJ_TX_CUSTOM_START_OFFSET, -1, "tx-custom-start" },
+	{ CEC_ERROR_INJ_TX_LAST_BIT_OFFSET,
+	  CEC_ERROR_INJ_TX_LAST_BIT_ARG_IDX, "tx-last-bit" },
+	{ CEC_ERROR_INJ_TX_LOW_DRIVE_OFFSET,
+	  CEC_ERROR_INJ_TX_LOW_DRIVE_ARG_IDX, "tx-low-drive" },
+	{ 0, -1, NULL }
+};
+
+u16 cec_pin_rx_error_inj(struct cec_pin *pin)
+{
+	u16 cmd = CEC_ERROR_INJ_OP_ANY;
+
+	/* Only when 18 bits have been received do we have a valid cmd */
+	if (!(pin->error_inj[cmd] & CEC_ERROR_INJ_RX_MASK) &&
+	    pin->rx_bit >= 18)
+		cmd = pin->rx_msg.msg[1];
+	return (pin->error_inj[cmd] & CEC_ERROR_INJ_RX_MASK) ? cmd :
+		CEC_ERROR_INJ_OP_ANY;
+}
+
+u16 cec_pin_tx_error_inj(struct cec_pin *pin)
+{
+	u16 cmd = CEC_ERROR_INJ_OP_ANY;
+
+	if (!(pin->error_inj[cmd] & CEC_ERROR_INJ_TX_MASK) &&
+	    pin->tx_msg.len > 1)
+		cmd = pin->tx_msg.msg[1];
+	return (pin->error_inj[cmd] & CEC_ERROR_INJ_TX_MASK) ? cmd :
+		CEC_ERROR_INJ_OP_ANY;
+}
+
+bool cec_pin_error_inj_parse_line(struct cec_adapter *adap, char *line)
+{
+	static const char *delims = " \t\r";
+	struct cec_pin *pin = adap->pin;
+	unsigned int i;
+	bool has_pos = false;
+	char *p = line;
+	char *token;
+	char *comma;
+	u64 *error;
+	u8 *args;
+	bool has_op;
+	u32 op;
+	u8 mode;
+	u8 pos;
+	u8 v;
+
+	p = skip_spaces(p);
+	token = strsep(&p, delims);
+	if (!strcmp(token, "clear")) {
+		memset(pin->error_inj, 0, sizeof(pin->error_inj));
+		pin->rx_toggle = pin->tx_toggle = false;
+		pin->tx_ignore_nack_until_eom = false;
+		pin->tx_custom_pulse = false;
+		pin->tx_custom_low_usecs = CEC_TIM_CUSTOM_DEFAULT;
+		pin->tx_custom_high_usecs = CEC_TIM_CUSTOM_DEFAULT;
+		return true;
+	}
+	if (!strcmp(token, "rx-clear")) {
+		for (i = 0; i <= CEC_ERROR_INJ_OP_ANY; i++)
+			pin->error_inj[i] &= ~CEC_ERROR_INJ_RX_MASK;
+		pin->rx_toggle = false;
+		return true;
+	}
+	if (!strcmp(token, "tx-clear")) {
+		for (i = 0; i <= CEC_ERROR_INJ_OP_ANY; i++)
+			pin->error_inj[i] &= ~CEC_ERROR_INJ_TX_MASK;
+		pin->tx_toggle = false;
+		pin->tx_ignore_nack_until_eom = false;
+		pin->tx_custom_pulse = false;
+		pin->tx_custom_low_usecs = CEC_TIM_CUSTOM_DEFAULT;
+		pin->tx_custom_high_usecs = CEC_TIM_CUSTOM_DEFAULT;
+		return true;
+	}
+	if (!strcmp(token, "tx-ignore-nack-until-eom")) {
+		pin->tx_ignore_nack_until_eom = true;
+		return true;
+	}
+	if (!strcmp(token, "tx-custom-pulse")) {
+		pin->tx_custom_pulse = true;
+		cec_pin_start_timer(pin);
+		return true;
+	}
+	if (!p)
+		return false;
+
+	p = skip_spaces(p);
+	if (!strcmp(token, "tx-custom-low-usecs")) {
+		u32 usecs;
+
+		if (kstrtou32(p, 0, &usecs) || usecs > 10000000)
+			return false;
+		pin->tx_custom_low_usecs = usecs;
+		return true;
+	}
+	if (!strcmp(token, "tx-custom-high-usecs")) {
+		u32 usecs;
+
+		if (kstrtou32(p, 0, &usecs) || usecs > 10000000)
+			return false;
+		pin->tx_custom_high_usecs = usecs;
+		return true;
+	}
+
+	comma = strchr(token, ',');
+	if (comma)
+		*comma++ = '\0';
+	if (!strcmp(token, "any"))
+		op = CEC_ERROR_INJ_OP_ANY;
+	else if (!kstrtou8(token, 0, &v))
+		op = v;
+	else
+		return false;
+	mode = CEC_ERROR_INJ_MODE_ONCE;
+	if (comma) {
+		if (!strcmp(comma, "off"))
+			mode = CEC_ERROR_INJ_MODE_OFF;
+		else if (!strcmp(comma, "once"))
+			mode = CEC_ERROR_INJ_MODE_ONCE;
+		else if (!strcmp(comma, "always"))
+			mode = CEC_ERROR_INJ_MODE_ALWAYS;
+		else if (!strcmp(comma, "toggle"))
+			mode = CEC_ERROR_INJ_MODE_TOGGLE;
+		else
+			return false;
+	}
+
+	error = pin->error_inj + op;
+	args = pin->error_inj_args[op];
+	has_op = op <= 0xff;
+
+	token = strsep(&p, delims);
+	if (p) {
+		p = skip_spaces(p);
+		has_pos = !kstrtou8(p, 0, &pos);
+	}
+
+	if (!strcmp(token, "clear")) {
+		*error = 0;
+		return true;
+	}
+	if (!strcmp(token, "rx-clear")) {
+		*error &= ~CEC_ERROR_INJ_RX_MASK;
+		return true;
+	}
+	if (!strcmp(token, "tx-clear")) {
+		*error &= ~CEC_ERROR_INJ_TX_MASK;
+		return true;
+	}
+
+	for (i = 0; cec_error_inj_cmds[i].cmd; i++) {
+		const char *cmd = cec_error_inj_cmds[i].cmd;
+		unsigned int mode_offset;
+		u64 mode_mask;
+		int arg_idx;
+		bool is_bit_pos = true;
+
+		if (strcmp(token, cmd))
+			continue;
+
+		mode_offset = cec_error_inj_cmds[i].mode_offset;
+		mode_mask = CEC_ERROR_INJ_MODE_MASK << mode_offset;
+		arg_idx = cec_error_inj_cmds[i].arg_idx;
+
+		if (mode_offset == CEC_ERROR_INJ_RX_ARB_LOST_OFFSET ||
+		    mode_offset == CEC_ERROR_INJ_TX_ADD_BYTES_OFFSET)
+			is_bit_pos = false;
+
+		if (mode_offset == CEC_ERROR_INJ_RX_ARB_LOST_OFFSET) {
+			if (has_op)
+				return false;
+			if (!has_pos)
+				pos = 0x0f;
+		}
+		if (arg_idx >= 0 && is_bit_pos) {
+			if (!has_pos || pos >= 160)
+				return false;
+			if (has_op && pos < 10 + 8)
+				return false;
+			/* Invalid bit position may not be the Ack bit */
+			if ((mode_offset == CEC_ERROR_INJ_TX_SHORT_BIT_OFFSET ||
+			     mode_offset == CEC_ERROR_INJ_TX_LONG_BIT_OFFSET ||
+			     mode_offset == CEC_ERROR_INJ_TX_CUSTOM_BIT_OFFSET) &&
+			    (pos % 10) == 9)
+				return false;
+		}
+		*error &= ~mode_mask;
+		*error |= (u64)mode << mode_offset;
+		if (arg_idx >= 0)
+			args[arg_idx] = pos;
+		return true;
+	}
+	return false;
+}
+
+static void cec_pin_show_cmd(struct seq_file *sf, u32 cmd, u8 mode)
+{
+	if (cmd == CEC_ERROR_INJ_OP_ANY)
+		seq_puts(sf, "any,");
+	else
+		seq_printf(sf, "0x%02x,", cmd);
+	switch (mode) {
+	case CEC_ERROR_INJ_MODE_ONCE:
+		seq_puts(sf, "once ");
+		break;
+	case CEC_ERROR_INJ_MODE_ALWAYS:
+		seq_puts(sf, "always ");
+		break;
+	case CEC_ERROR_INJ_MODE_TOGGLE:
+		seq_puts(sf, "toggle ");
+		break;
+	default:
+		seq_puts(sf, "off ");
+		break;
+	}
+}
+
+int cec_pin_error_inj_show(struct cec_adapter *adap, struct seq_file *sf)
+{
+	struct cec_pin *pin = adap->pin;
+	unsigned int i, j;
+
+	seq_puts(sf, "# Clear error injections:\n");
+	seq_puts(sf, "#   clear          clear all rx and tx error injections\n");
+	seq_puts(sf, "#   rx-clear       clear all rx error injections\n");
+	seq_puts(sf, "#   tx-clear       clear all tx error injections\n");
+	seq_puts(sf, "#   <op> clear     clear all rx and tx error injections for <op>\n");
+	seq_puts(sf, "#   <op> rx-clear  clear all rx error injections for <op>\n");
+	seq_puts(sf, "#   <op> tx-clear  clear all tx error injections for <op>\n");
+	seq_puts(sf, "#\n");
+	seq_puts(sf, "# RX error injection:\n");
+	seq_puts(sf, "#   <op>[,<mode>] rx-nack              NACK the message instead of sending an ACK\n");
+	seq_puts(sf, "#   <op>[,<mode>] rx-low-drive <bit>   force a low-drive condition at this bit position\n");
+	seq_puts(sf, "#   <op>[,<mode>] rx-add-byte          add a spurious byte to the received CEC message\n");
+	seq_puts(sf, "#   <op>[,<mode>] rx-remove-byte       remove the last byte from the received CEC message\n");
+	seq_puts(sf, "#   <op>[,<mode>] rx-arb-lost <poll>   generate a POLL message to trigger an arbitration lost\n");
+	seq_puts(sf, "#\n");
+	seq_puts(sf, "# TX error injection settings:\n");
+	seq_puts(sf, "#   tx-ignore-nack-until-eom           ignore early NACKs until EOM\n");
+	seq_puts(sf, "#   tx-custom-low-usecs <usecs>        define the 'low' time for the custom pulse\n");
+	seq_puts(sf, "#   tx-custom-high-usecs <usecs>       define the 'high' time for the custom pulse\n");
+	seq_puts(sf, "#   tx-custom-pulse                    transmit the custom pulse once the bus is idle\n");
+	seq_puts(sf, "#\n");
+	seq_puts(sf, "# TX error injection:\n");
+	seq_puts(sf, "#   <op>[,<mode>] tx-no-eom            don't set the EOM bit\n");
+	seq_puts(sf, "#   <op>[,<mode>] tx-early-eom         set the EOM bit one byte too soon\n");
+	seq_puts(sf, "#   <op>[,<mode>] tx-add-bytes <num>   append <num> (1-255) spurious bytes to the message\n");
+	seq_puts(sf, "#   <op>[,<mode>] tx-remove-byte       drop the last byte from the message\n");
+	seq_puts(sf, "#   <op>[,<mode>] tx-short-bit <bit>   make this bit shorter than allowed\n");
+	seq_puts(sf, "#   <op>[,<mode>] tx-long-bit <bit>    make this bit longer than allowed\n");
+	seq_puts(sf, "#   <op>[,<mode>] tx-custom-bit <bit>  send the custom pulse instead of this bit\n");
+	seq_puts(sf, "#   <op>[,<mode>] tx-short-start       send a start pulse that's too short\n");
+	seq_puts(sf, "#   <op>[,<mode>] tx-long-start        send a start pulse that's too long\n");
+	seq_puts(sf, "#   <op>[,<mode>] tx-custom-start      send the custom pulse instead of the start pulse\n");
+	seq_puts(sf, "#   <op>[,<mode>] tx-last-bit <bit>    stop sending after this bit\n");
+	seq_puts(sf, "#   <op>[,<mode>] tx-low-drive <bit>   force a low-drive condition at this bit position\n");
+	seq_puts(sf, "#\n");
+	seq_puts(sf, "# <op>       CEC message opcode (0-255) or 'any'\n");
+	seq_puts(sf, "# <mode>     'once' (default), 'always', 'toggle' or 'off'\n");
+	seq_puts(sf, "# <bit>      CEC message bit (0-159)\n");
+	seq_puts(sf, "#            10 bits per 'byte': bits 0-7: data, bit 8: EOM, bit 9: ACK\n");
+	seq_puts(sf, "# <poll>     CEC poll message used to test arbitration lost (0x00-0xff, default 0x0f)\n");
+	seq_puts(sf, "# <usecs>    microseconds (0-10000000, default 1000)\n");
+
+	seq_puts(sf, "\nclear\n");
+
+	for (i = 0; i < ARRAY_SIZE(pin->error_inj); i++) {
+		u64 e = pin->error_inj[i];
+
+		for (j = 0; cec_error_inj_cmds[j].cmd; j++) {
+			const char *cmd = cec_error_inj_cmds[j].cmd;
+			unsigned int mode;
+			unsigned int mode_offset;
+			int arg_idx;
+
+			mode_offset = cec_error_inj_cmds[j].mode_offset;
+			arg_idx = cec_error_inj_cmds[j].arg_idx;
+			mode = (e >> mode_offset) & CEC_ERROR_INJ_MODE_MASK;
+			if (!mode)
+				continue;
+			cec_pin_show_cmd(sf, i, mode);
+			seq_puts(sf, cmd);
+			if (arg_idx >= 0)
+				seq_printf(sf, " %u", pin->error_inj_args[i][arg_idx]);
+			seq_puts(sf, "\n");
+		}
+	}
+
+	if (pin->tx_ignore_nack_until_eom)
+		seq_puts(sf, "tx-ignore-nack-until-eom\n");
+	if (pin->tx_custom_pulse)
+		seq_puts(sf, "tx-custom-pulse\n");
+	if (pin->tx_custom_low_usecs != CEC_TIM_CUSTOM_DEFAULT)
+		seq_printf(sf, "tx-custom-low-usecs %u\n",
+			   pin->tx_custom_low_usecs);
+	if (pin->tx_custom_high_usecs != CEC_TIM_CUSTOM_DEFAULT)
+		seq_printf(sf, "tx-custom-high-usecs %u\n",
+			   pin->tx_custom_high_usecs);
+	return 0;
+}
diff --git a/drivers/media/cec/cec-pin-priv.h b/drivers/media/cec/cec-pin-priv.h
index 4571a0001a9d..779384f18689 100644
--- a/drivers/media/cec/cec-pin-priv.h
+++ b/drivers/media/cec/cec-pin-priv.h
@@ -74,6 +74,55 @@ enum cec_pin_state {
 	CEC_PIN_STATES
 };
 
+/* Error Injection */
+
+/* Error injection modes */
+#define CEC_ERROR_INJ_MODE_OFF				0
+#define CEC_ERROR_INJ_MODE_ONCE				1
+#define CEC_ERROR_INJ_MODE_ALWAYS			2
+#define CEC_ERROR_INJ_MODE_TOGGLE			3
+#define CEC_ERROR_INJ_MODE_MASK				3ULL
+
+/* Receive error injection options */
+#define CEC_ERROR_INJ_RX_NACK_OFFSET			0
+#define CEC_ERROR_INJ_RX_LOW_DRIVE_OFFSET		2
+#define CEC_ERROR_INJ_RX_ADD_BYTE_OFFSET		4
+#define CEC_ERROR_INJ_RX_REMOVE_BYTE_OFFSET		6
+#define CEC_ERROR_INJ_RX_ARB_LOST_OFFSET		8
+#define CEC_ERROR_INJ_RX_MASK				0xffffULL
+
+/* Transmit error injection options */
+#define CEC_ERROR_INJ_TX_NO_EOM_OFFSET			16
+#define CEC_ERROR_INJ_TX_EARLY_EOM_OFFSET		18
+#define CEC_ERROR_INJ_TX_SHORT_BIT_OFFSET		20
+#define CEC_ERROR_INJ_TX_LONG_BIT_OFFSET		22
+#define CEC_ERROR_INJ_TX_CUSTOM_BIT_OFFSET		24
+#define CEC_ERROR_INJ_TX_SHORT_START_OFFSET		26
+#define CEC_ERROR_INJ_TX_LONG_START_OFFSET		28
+#define CEC_ERROR_INJ_TX_CUSTOM_START_OFFSET		30
+#define CEC_ERROR_INJ_TX_LAST_BIT_OFFSET		32
+#define CEC_ERROR_INJ_TX_ADD_BYTES_OFFSET		34
+#define CEC_ERROR_INJ_TX_REMOVE_BYTE_OFFSET		36
+#define CEC_ERROR_INJ_TX_LOW_DRIVE_OFFSET		38
+#define CEC_ERROR_INJ_TX_MASK				0xffffffffffff0000ULL
+
+#define CEC_ERROR_INJ_RX_LOW_DRIVE_ARG_IDX		0
+#define CEC_ERROR_INJ_RX_ARB_LOST_ARG_IDX		1
+
+#define CEC_ERROR_INJ_TX_ADD_BYTES_ARG_IDX		2
+#define CEC_ERROR_INJ_TX_SHORT_BIT_ARG_IDX		3
+#define CEC_ERROR_INJ_TX_LONG_BIT_ARG_IDX		4
+#define CEC_ERROR_INJ_TX_CUSTOM_BIT_ARG_IDX		5
+#define CEC_ERROR_INJ_TX_LAST_BIT_ARG_IDX		6
+#define CEC_ERROR_INJ_TX_LOW_DRIVE_ARG_IDX		7
+#define CEC_ERROR_INJ_NUM_ARGS				8
+
+/* Special CEC op values */
+#define CEC_ERROR_INJ_OP_ANY				0x00000100
+
+/* The default for the low/high time of the custom pulse */
+#define CEC_TIM_CUSTOM_DEFAULT				1000
+
 #define CEC_NUM_PIN_EVENTS 128
 
 #define CEC_PIN_IRQ_UNCHANGED	0
@@ -98,8 +147,10 @@ struct cec_pin {
 	u32				tx_bit;
 	bool				tx_nacked;
 	u32				tx_signal_free_time;
+	bool				tx_toggle;
 	struct cec_msg			rx_msg;
 	u32				rx_bit;
+	bool				rx_toggle;
 
 	struct cec_msg			work_rx_msg;
 	u8				work_tx_status;
@@ -116,8 +167,28 @@ struct cec_pin {
 	u32				timer_300ms_overruns;
 	u32				timer_max_overrun;
 	u32				timer_sum_overrun;
+
+	u32				tx_custom_low_usecs;
+	u32				tx_custom_high_usecs;
+	bool				tx_ignore_nack_until_eom;
+	bool				tx_custom_pulse;
+	bool				tx_generated_poll;
+	bool				tx_post_eom;
+	u8				tx_extra_bytes;
+#ifdef CONFIG_CEC_PIN_ERROR_INJ
+	u64				error_inj[CEC_ERROR_INJ_OP_ANY + 1];
+	u8				error_inj_args[CEC_ERROR_INJ_OP_ANY + 1][CEC_ERROR_INJ_NUM_ARGS];
+#endif
 };
 
 void cec_pin_start_timer(struct cec_pin *pin);
 
+#ifdef CONFIG_CEC_PIN_ERROR_INJ
+bool cec_pin_error_inj_parse_line(struct cec_adapter *adap, char *line);
+int cec_pin_error_inj_show(struct cec_adapter *adap, struct seq_file *sf);
+
+u16 cec_pin_rx_error_inj(struct cec_pin *pin);
+u16 cec_pin_tx_error_inj(struct cec_pin *pin);
+#endif
+
 #endif
diff --git a/drivers/media/cec/cec-pin.c b/drivers/media/cec/cec-pin.c
index 67d6ea9f56b6..7920ea1c940b 100644
--- a/drivers/media/cec/cec-pin.c
+++ b/drivers/media/cec/cec-pin.c
@@ -771,6 +771,10 @@ static const struct cec_adap_ops cec_pin_adap_ops = {
 	.adap_transmit = cec_pin_adap_transmit,
 	.adap_status = cec_pin_adap_status,
 	.adap_free = cec_pin_adap_free,
+#ifdef CONFIG_CEC_PIN_ERROR_INJ
+	.error_inj_parse_line = cec_pin_error_inj_parse_line,
+	.error_inj_show = cec_pin_error_inj_show,
+#endif
 };
 
 struct cec_adapter *cec_pin_allocate_adapter(const struct cec_pin_ops *pin_ops,
@@ -785,6 +789,8 @@ struct cec_adapter *cec_pin_allocate_adapter(const struct cec_pin_ops *pin_ops,
 	hrtimer_init(&pin->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
 	pin->timer.function = cec_pin_timer;
 	init_waitqueue_head(&pin->kthread_waitq);
+	pin->tx_custom_low_usecs = CEC_TIM_CUSTOM_DEFAULT;
+	pin->tx_custom_high_usecs = CEC_TIM_CUSTOM_DEFAULT;
 
 	adap = cec_allocate_adapter(&cec_pin_adap_ops, priv, name,
 			    caps | CEC_CAP_MONITOR_ALL | CEC_CAP_MONITOR_PIN,
-- 
2.16.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCHv2 5/7] cec-pin: add error injection support
  2018-03-05 13:51 [PATCHv2 0/7] cec: add error injection support Hans Verkuil
                   ` (3 preceding siblings ...)
  2018-03-05 13:51 ` [PATCHv2 4/7] cec-pin-error-inj: parse/show error injection Hans Verkuil
@ 2018-03-05 13:51 ` Hans Verkuil
  2018-03-06 22:48   ` Hans Verkuil
  2018-03-05 13:51 ` [PATCHv2 6/7] cec-pin-error-inj.rst: document CEC Pin Error Injection Hans Verkuil
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 14+ messages in thread
From: Hans Verkuil @ 2018-03-05 13:51 UTC (permalink / raw)
  To: linux-media; +Cc: Wolfram Sang, Maxime Ripard, dri-devel, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

Implement all the error injection commands.

The state machine gets new states for the various error situations,
helper functions are added to detect whether an error injection is
active and the actual error injections are implemented.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/cec/cec-pin-priv.h |  38 ++-
 drivers/media/cec/cec-pin.c      | 542 +++++++++++++++++++++++++++++++++++----
 2 files changed, 521 insertions(+), 59 deletions(-)

diff --git a/drivers/media/cec/cec-pin-priv.h b/drivers/media/cec/cec-pin-priv.h
index 779384f18689..c9349f68e554 100644
--- a/drivers/media/cec/cec-pin-priv.h
+++ b/drivers/media/cec/cec-pin-priv.h
@@ -28,14 +28,30 @@ enum cec_pin_state {
 	CEC_ST_TX_START_BIT_LOW,
 	/* Drive CEC high for the start bit */
 	CEC_ST_TX_START_BIT_HIGH,
+	/* Generate a start bit period that is too short */
+	CEC_ST_TX_START_BIT_HIGH_SHORT,
+	/* Generate a start bit period that is too long */
+	CEC_ST_TX_START_BIT_HIGH_LONG,
+	/* Drive CEC low for the start bit using the custom timing */
+	CEC_ST_TX_START_BIT_LOW_CUSTOM,
+	/* Drive CEC high for the start bit using the custom timing */
+	CEC_ST_TX_START_BIT_HIGH_CUSTOM,
 	/* Drive CEC low for the 0 bit */
 	CEC_ST_TX_DATA_BIT_0_LOW,
 	/* Drive CEC high for the 0 bit */
 	CEC_ST_TX_DATA_BIT_0_HIGH,
+	/* Generate a bit period that is too short */
+	CEC_ST_TX_DATA_BIT_0_HIGH_SHORT,
+	/* Generate a bit period that is too long */
+	CEC_ST_TX_DATA_BIT_0_HIGH_LONG,
 	/* Drive CEC low for the 1 bit */
 	CEC_ST_TX_DATA_BIT_1_LOW,
 	/* Drive CEC high for the 1 bit */
 	CEC_ST_TX_DATA_BIT_1_HIGH,
+	/* Generate a bit period that is too short */
+	CEC_ST_TX_DATA_BIT_1_HIGH_SHORT,
+	/* Generate a bit period that is too long */
+	CEC_ST_TX_DATA_BIT_1_HIGH_LONG,
 	/*
 	 * Wait for start of sample time to check for Ack bit or first
 	 * four initiator bits to check for Arbitration Lost.
@@ -43,6 +59,20 @@ enum cec_pin_state {
 	CEC_ST_TX_DATA_BIT_1_HIGH_PRE_SAMPLE,
 	/* Wait for end of bit period after sampling */
 	CEC_ST_TX_DATA_BIT_1_HIGH_POST_SAMPLE,
+	/* Generate a bit period that is too short */
+	CEC_ST_TX_DATA_BIT_1_HIGH_POST_SAMPLE_SHORT,
+	/* Generate a bit period that is too long */
+	CEC_ST_TX_DATA_BIT_1_HIGH_POST_SAMPLE_LONG,
+	/* Drive CEC low for a data bit using the custom timing */
+	CEC_ST_TX_DATA_BIT_LOW_CUSTOM,
+	/* Drive CEC high for a data bit using the custom timing */
+	CEC_ST_TX_DATA_BIT_HIGH_CUSTOM,
+	/* Drive CEC low for a standalone pulse using the custom timing */
+	CEC_ST_TX_PULSE_LOW_CUSTOM,
+	/* Drive CEC high for a standalone pulse using the custom timing */
+	CEC_ST_TX_PULSE_HIGH_CUSTOM,
+	/* Start low drive */
+	CEC_ST_TX_LOW_DRIVE,
 
 	/* Rx states */
 
@@ -54,8 +84,8 @@ enum cec_pin_state {
 	CEC_ST_RX_DATA_SAMPLE,
 	/* Wait for earliest end of bit period after sampling */
 	CEC_ST_RX_DATA_POST_SAMPLE,
-	/* Wait for CEC to go high (i.e. end of bit period */
-	CEC_ST_RX_DATA_HIGH,
+	/* Wait for CEC to go low (i.e. end of bit period) */
+	CEC_ST_RX_DATA_WAIT_FOR_LOW,
 	/* Drive CEC low to send 0 Ack bit */
 	CEC_ST_RX_ACK_LOW,
 	/* End of 0 Ack time, wait for earliest end of bit period */
@@ -64,9 +94,9 @@ enum cec_pin_state {
 	CEC_ST_RX_ACK_HIGH_POST,
 	/* Wait for earliest end of bit period and end of message */
 	CEC_ST_RX_ACK_FINISH,
-
 	/* Start low drive */
-	CEC_ST_LOW_DRIVE,
+	CEC_ST_RX_LOW_DRIVE,
+
 	/* Monitor pin using interrupts */
 	CEC_ST_RX_IRQ,
 
diff --git a/drivers/media/cec/cec-pin.c b/drivers/media/cec/cec-pin.c
index 7920ea1c940b..430a23392299 100644
--- a/drivers/media/cec/cec-pin.c
+++ b/drivers/media/cec/cec-pin.c
@@ -39,11 +39,29 @@
 #define CEC_TIM_IDLE_SAMPLE		1000
 /* when processing the start bit, sample twice per millisecond */
 #define CEC_TIM_START_BIT_SAMPLE	500
-/* when polling for a state change, sample once every 50 micoseconds */
+/* when polling for a state change, sample once every 50 microseconds */
 #define CEC_TIM_SAMPLE			50
 
 #define CEC_TIM_LOW_DRIVE_ERROR		(1.5 * CEC_TIM_DATA_BIT_TOTAL)
 
+/*
+ * Total data bit time that is too short/long for a valid bit,
+ * used for error injection.
+ */
+#define CEC_TIM_DATA_BIT_TOTAL_SHORT	1800
+#define CEC_TIM_DATA_BIT_TOTAL_LONG	2900
+
+/*
+ * Total start bit time that is too short/long for a valid bit,
+ * used for error injection.
+ */
+#define CEC_TIM_START_BIT_TOTAL_SHORT	4100
+#define CEC_TIM_START_BIT_TOTAL_LONG	5000
+
+/* Data bits are 0-7, EOM is bit 8 and ACK is bit 9 */
+#define EOM_BIT				8
+#define ACK_BIT				9
+
 struct cec_state {
 	const char * const name;
 	unsigned int usecs;
@@ -56,17 +74,32 @@ static const struct cec_state states[CEC_PIN_STATES] = {
 	{ "Tx Wait for High",	   CEC_TIM_IDLE_SAMPLE },
 	{ "Tx Start Bit Low",	   CEC_TIM_START_BIT_LOW },
 	{ "Tx Start Bit High",	   CEC_TIM_START_BIT_TOTAL - CEC_TIM_START_BIT_LOW },
+	{ "Tx Start Bit High Short", CEC_TIM_START_BIT_TOTAL_SHORT - CEC_TIM_START_BIT_LOW },
+	{ "Tx Start Bit High Long", CEC_TIM_START_BIT_TOTAL_LONG - CEC_TIM_START_BIT_LOW },
+	{ "Tx Start Bit Low Custom", 0 },
+	{ "Tx Start Bit High Custom", 0 },
 	{ "Tx Data 0 Low",	   CEC_TIM_DATA_BIT_0_LOW },
 	{ "Tx Data 0 High",	   CEC_TIM_DATA_BIT_TOTAL - CEC_TIM_DATA_BIT_0_LOW },
+	{ "Tx Data 0 High Short",  CEC_TIM_DATA_BIT_TOTAL_SHORT - CEC_TIM_DATA_BIT_0_LOW },
+	{ "Tx Data 0 High Long",   CEC_TIM_DATA_BIT_TOTAL_LONG - CEC_TIM_DATA_BIT_0_LOW },
 	{ "Tx Data 1 Low",	   CEC_TIM_DATA_BIT_1_LOW },
 	{ "Tx Data 1 High",	   CEC_TIM_DATA_BIT_TOTAL - CEC_TIM_DATA_BIT_1_LOW },
-	{ "Tx Data 1 Pre Sample",  CEC_TIM_DATA_BIT_SAMPLE - CEC_TIM_DATA_BIT_1_LOW },
-	{ "Tx Data 1 Post Sample", CEC_TIM_DATA_BIT_TOTAL - CEC_TIM_DATA_BIT_SAMPLE },
+	{ "Tx Data 1 High Short",  CEC_TIM_DATA_BIT_TOTAL_SHORT - CEC_TIM_DATA_BIT_1_LOW },
+	{ "Tx Data 1 High Long",   CEC_TIM_DATA_BIT_TOTAL_LONG - CEC_TIM_DATA_BIT_1_LOW },
+	{ "Tx Data 1 High Pre Sample", CEC_TIM_DATA_BIT_SAMPLE - CEC_TIM_DATA_BIT_1_LOW },
+	{ "Tx Data 1 High Post Sample", CEC_TIM_DATA_BIT_TOTAL - CEC_TIM_DATA_BIT_SAMPLE },
+	{ "Tx Data 1 High Post Sample Short", CEC_TIM_DATA_BIT_TOTAL_SHORT - CEC_TIM_DATA_BIT_SAMPLE },
+	{ "Tx Data 1 High Post Sample Long", CEC_TIM_DATA_BIT_TOTAL_LONG - CEC_TIM_DATA_BIT_SAMPLE },
+	{ "Tx Data Bit Low Custom", 0 },
+	{ "Tx Data Bit High Custom", 0 },
+	{ "Tx Pulse Low Custom",   0 },
+	{ "Tx Pulse High Custom",  0 },
+	{ "Tx Low Drive",	   CEC_TIM_LOW_DRIVE_ERROR },
 	{ "Rx Start Bit Low",	   CEC_TIM_SAMPLE },
 	{ "Rx Start Bit High",	   CEC_TIM_SAMPLE },
 	{ "Rx Data Sample",	   CEC_TIM_DATA_BIT_SAMPLE },
 	{ "Rx Data Post Sample",   CEC_TIM_DATA_BIT_HIGH - CEC_TIM_DATA_BIT_SAMPLE },
-	{ "Rx Data High",	   CEC_TIM_SAMPLE },
+	{ "Rx Data Wait for Low",  CEC_TIM_SAMPLE },
 	{ "Rx Ack Low",		   CEC_TIM_DATA_BIT_0_LOW },
 	{ "Rx Ack Low Post",	   CEC_TIM_DATA_BIT_HIGH - CEC_TIM_DATA_BIT_0_LOW },
 	{ "Rx Ack High Post",	   CEC_TIM_DATA_BIT_HIGH },
@@ -111,6 +144,170 @@ static bool cec_pin_high(struct cec_pin *pin)
 	return cec_pin_read(pin);
 }
 
+static bool rx_error_inj(struct cec_pin *pin, unsigned int mode_offset,
+			 int arg_idx, u8 *arg)
+{
+#ifdef CONFIG_CEC_PIN_ERROR_INJ
+	u16 cmd = cec_pin_rx_error_inj(pin);
+	u64 e = pin->error_inj[cmd];
+	unsigned int mode = (e >> mode_offset) & CEC_ERROR_INJ_MODE_MASK;
+
+	if (arg_idx >= 0) {
+		u8 pos = pin->error_inj_args[cmd][arg_idx];
+
+		if (arg)
+			*arg = pos;
+		else if (pos != pin->rx_bit)
+			return false;
+	}
+
+	switch (mode) {
+	case CEC_ERROR_INJ_MODE_ONCE:
+		pin->error_inj[cmd] &= ~(CEC_ERROR_INJ_MODE_MASK << mode_offset);
+		return true;
+	case CEC_ERROR_INJ_MODE_ALWAYS:
+		return true;
+	case CEC_ERROR_INJ_MODE_TOGGLE:
+		return pin->rx_toggle;
+	default:
+		return false;
+	}
+#else
+	return false;
+#endif
+}
+
+static bool rx_nack(struct cec_pin *pin)
+{
+	return rx_error_inj(pin, CEC_ERROR_INJ_RX_NACK_OFFSET, -1, NULL);
+}
+
+static bool rx_low_drive(struct cec_pin *pin)
+{
+	return rx_error_inj(pin, CEC_ERROR_INJ_RX_LOW_DRIVE_OFFSET,
+			    CEC_ERROR_INJ_RX_LOW_DRIVE_ARG_IDX, NULL);
+}
+
+static bool rx_add_byte(struct cec_pin *pin)
+{
+	return rx_error_inj(pin, CEC_ERROR_INJ_RX_ADD_BYTE_OFFSET, -1, NULL);
+}
+
+static bool rx_remove_byte(struct cec_pin *pin)
+{
+	return rx_error_inj(pin, CEC_ERROR_INJ_RX_REMOVE_BYTE_OFFSET, -1, NULL);
+}
+
+static bool rx_arb_lost(struct cec_pin *pin, u8 *poll)
+{
+	return pin->tx_msg.len == 0 &&
+		rx_error_inj(pin, CEC_ERROR_INJ_RX_ARB_LOST_OFFSET,
+			     CEC_ERROR_INJ_RX_ARB_LOST_ARG_IDX, poll);
+}
+
+static bool tx_error_inj(struct cec_pin *pin, unsigned int mode_offset,
+			 int arg_idx, u8 *arg)
+{
+#ifdef CONFIG_CEC_PIN_ERROR_INJ
+	u16 cmd = cec_pin_tx_error_inj(pin);
+	u64 e = pin->error_inj[cmd];
+	unsigned int mode = (e >> mode_offset) & CEC_ERROR_INJ_MODE_MASK;
+
+	if (arg_idx) {
+		u8 pos = pin->error_inj_args[cmd][arg_idx];
+
+		if (arg)
+			*arg = pos;
+		else if (pos != pin->tx_bit)
+			return false;
+	}
+
+	switch (mode) {
+	case CEC_ERROR_INJ_MODE_ONCE:
+		pin->error_inj[cmd] &= ~(CEC_ERROR_INJ_MODE_MASK << mode_offset);
+		return true;
+	case CEC_ERROR_INJ_MODE_ALWAYS:
+		return true;
+	case CEC_ERROR_INJ_MODE_TOGGLE:
+		return pin->tx_toggle;
+	default:
+		return false;
+	}
+#else
+	return false;
+#endif
+}
+
+static bool tx_no_eom(struct cec_pin *pin)
+{
+	return tx_error_inj(pin, CEC_ERROR_INJ_TX_NO_EOM_OFFSET, -1, NULL);
+}
+
+static bool tx_early_eom(struct cec_pin *pin)
+{
+	return tx_error_inj(pin, CEC_ERROR_INJ_TX_EARLY_EOM_OFFSET, -1, NULL);
+}
+
+static bool tx_short_bit(struct cec_pin *pin)
+{
+	return tx_error_inj(pin, CEC_ERROR_INJ_TX_SHORT_BIT_OFFSET,
+			    CEC_ERROR_INJ_TX_SHORT_BIT_ARG_IDX, NULL);
+}
+
+static bool tx_long_bit(struct cec_pin *pin)
+{
+	return tx_error_inj(pin, CEC_ERROR_INJ_TX_LONG_BIT_OFFSET,
+			    CEC_ERROR_INJ_TX_LONG_BIT_ARG_IDX, NULL);
+}
+
+static bool tx_custom_bit(struct cec_pin *pin)
+{
+	return tx_error_inj(pin, CEC_ERROR_INJ_TX_CUSTOM_BIT_OFFSET,
+			    CEC_ERROR_INJ_TX_CUSTOM_BIT_ARG_IDX, NULL);
+}
+
+static bool tx_short_start(struct cec_pin *pin)
+{
+	return tx_error_inj(pin, CEC_ERROR_INJ_TX_SHORT_START_OFFSET, -1, NULL);
+}
+
+static bool tx_long_start(struct cec_pin *pin)
+{
+	return tx_error_inj(pin, CEC_ERROR_INJ_TX_LONG_START_OFFSET, -1, NULL);
+}
+
+static bool tx_custom_start(struct cec_pin *pin)
+{
+	return tx_error_inj(pin, CEC_ERROR_INJ_TX_CUSTOM_START_OFFSET, -1, NULL);
+}
+
+static bool tx_last_bit(struct cec_pin *pin)
+{
+	return tx_error_inj(pin, CEC_ERROR_INJ_TX_LAST_BIT_OFFSET,
+			    CEC_ERROR_INJ_TX_LAST_BIT_ARG_IDX, NULL);
+}
+
+static u8 tx_add_bytes(struct cec_pin *pin)
+{
+	u8 bytes;
+
+	if (tx_error_inj(pin, CEC_ERROR_INJ_TX_ADD_BYTES_OFFSET,
+			 CEC_ERROR_INJ_TX_ADD_BYTES_ARG_IDX, &bytes))
+		return bytes;
+	return 0;
+}
+
+static bool tx_remove_byte(struct cec_pin *pin)
+{
+	return tx_error_inj(pin, CEC_ERROR_INJ_TX_REMOVE_BYTE_OFFSET, -1, NULL);
+}
+
+static bool tx_low_drive(struct cec_pin *pin)
+{
+	return tx_error_inj(pin, CEC_ERROR_INJ_TX_LOW_DRIVE_OFFSET,
+			    CEC_ERROR_INJ_TX_LOW_DRIVE_ARG_IDX, NULL);
+}
+
 static void cec_pin_to_idle(struct cec_pin *pin)
 {
 	/*
@@ -120,8 +317,16 @@ static void cec_pin_to_idle(struct cec_pin *pin)
 	pin->rx_bit = pin->tx_bit = 0;
 	pin->rx_msg.len = 0;
 	memset(pin->rx_msg.msg, 0, sizeof(pin->rx_msg.msg));
-	pin->state = CEC_ST_IDLE;
 	pin->ts = ns_to_ktime(0);
+	pin->tx_generated_poll = false;
+	pin->tx_post_eom = false;
+	if (pin->state >= CEC_ST_TX_WAIT &&
+	    pin->state <= CEC_ST_TX_LOW_DRIVE)
+		pin->tx_toggle ^= 1;
+	if (pin->state >= CEC_ST_RX_START_BIT_LOW &&
+	    pin->state <= CEC_ST_RX_LOW_DRIVE)
+		pin->rx_toggle ^= 1;
+	pin->state = CEC_ST_IDLE;
 }
 
 /*
@@ -162,42 +367,107 @@ static void cec_pin_tx_states(struct cec_pin *pin, ktime_t ts)
 		break;
 
 	case CEC_ST_TX_START_BIT_LOW:
-		pin->state = CEC_ST_TX_START_BIT_HIGH;
+		if (tx_short_start(pin)) {
+			/*
+			 * Error Injection: send an invalid (too short)
+			 * start pulse.
+			 */
+			pin->state = CEC_ST_TX_START_BIT_HIGH_SHORT;
+		} else if (tx_long_start(pin)) {
+			/*
+			 * Error Injection: send an invalid (too long)
+			 * start pulse.
+			 */
+			pin->state = CEC_ST_TX_START_BIT_HIGH_LONG;
+		} else {
+			pin->state = CEC_ST_TX_START_BIT_HIGH;
+		}
+		/* Generate start bit */
+		cec_pin_high(pin);
+		break;
+
+	case CEC_ST_TX_START_BIT_LOW_CUSTOM:
+		pin->state = CEC_ST_TX_START_BIT_HIGH_CUSTOM;
 		/* Generate start bit */
 		cec_pin_high(pin);
 		break;
 
 	case CEC_ST_TX_DATA_BIT_1_HIGH_POST_SAMPLE:
-		/* If the read value is 1, then all is OK */
-		if (!cec_pin_read(pin)) {
+	case CEC_ST_TX_DATA_BIT_1_HIGH_POST_SAMPLE_SHORT:
+	case CEC_ST_TX_DATA_BIT_1_HIGH_POST_SAMPLE_LONG:
+		if (pin->tx_nacked) {
+			cec_pin_to_idle(pin);
+			pin->tx_msg.len = 0;
+			if (pin->tx_generated_poll)
+				break;
+			pin->work_tx_ts = ts;
+			pin->work_tx_status = CEC_TX_STATUS_NACK;
+			wake_up_interruptible(&pin->kthread_waitq);
+			break;
+		}
+		/* fall through */
+	case CEC_ST_TX_DATA_BIT_0_HIGH:
+	case CEC_ST_TX_DATA_BIT_0_HIGH_SHORT:
+	case CEC_ST_TX_DATA_BIT_0_HIGH_LONG:
+	case CEC_ST_TX_DATA_BIT_1_HIGH:
+	case CEC_ST_TX_DATA_BIT_1_HIGH_SHORT:
+	case CEC_ST_TX_DATA_BIT_1_HIGH_LONG:
+		/*
+		 * If the read value is 1, then all is OK, otherwise we have a
+		 * low drive condition.
+		 *
+		 * Special case: when we generate a poll message due to an
+		 * Arbitration Lost error injection, then ignore this since
+		 * the pin can actually be low in that case.
+		 */
+		if (!cec_pin_read(pin) && !pin->tx_generated_poll) {
 			/*
 			 * It's 0, so someone detected an error and pulled the
 			 * line low for 1.5 times the nominal bit period.
 			 */
 			pin->tx_msg.len = 0;
+			pin->state = CEC_ST_TX_WAIT_FOR_HIGH;
 			pin->work_tx_ts = ts;
 			pin->work_tx_status = CEC_TX_STATUS_LOW_DRIVE;
-			pin->state = CEC_ST_TX_WAIT_FOR_HIGH;
 			wake_up_interruptible(&pin->kthread_waitq);
 			break;
 		}
-		if (pin->tx_nacked) {
+		/* fall through */
+	case CEC_ST_TX_DATA_BIT_HIGH_CUSTOM:
+		if (tx_last_bit(pin)) {
+			/* Error Injection: just stop sending after this bit */
 			cec_pin_to_idle(pin);
 			pin->tx_msg.len = 0;
+			if (pin->tx_generated_poll)
+				break;
 			pin->work_tx_ts = ts;
-			pin->work_tx_status = CEC_TX_STATUS_NACK;
+			pin->work_tx_status = CEC_TX_STATUS_OK;
 			wake_up_interruptible(&pin->kthread_waitq);
 			break;
 		}
-		/* fall through */
-	case CEC_ST_TX_DATA_BIT_0_HIGH:
-	case CEC_ST_TX_DATA_BIT_1_HIGH:
 		pin->tx_bit++;
 		/* fall through */
 	case CEC_ST_TX_START_BIT_HIGH:
-		if (pin->tx_bit / 10 >= pin->tx_msg.len) {
+	case CEC_ST_TX_START_BIT_HIGH_SHORT:
+	case CEC_ST_TX_START_BIT_HIGH_LONG:
+	case CEC_ST_TX_START_BIT_HIGH_CUSTOM:
+		if (tx_low_drive(pin)) {
+			/* Error injection: go to low drive */
+			cec_pin_low(pin);
+			pin->state = CEC_ST_TX_LOW_DRIVE;
+			pin->tx_msg.len = 0;
+			if (pin->tx_generated_poll)
+				break;
+			pin->work_tx_ts = ts;
+			pin->work_tx_status = CEC_TX_STATUS_LOW_DRIVE;
+			wake_up_interruptible(&pin->kthread_waitq);
+			break;
+		}
+		if (pin->tx_bit / 10 >= pin->tx_msg.len + pin->tx_extra_bytes) {
 			cec_pin_to_idle(pin);
 			pin->tx_msg.len = 0;
+			if (pin->tx_generated_poll)
+				break;
 			pin->work_tx_ts = ts;
 			pin->work_tx_status = CEC_TX_STATUS_OK;
 			wake_up_interruptible(&pin->kthread_waitq);
@@ -205,39 +475,79 @@ static void cec_pin_tx_states(struct cec_pin *pin, ktime_t ts)
 		}
 
 		switch (pin->tx_bit % 10) {
-		default:
-			v = pin->tx_msg.msg[pin->tx_bit / 10] &
-				(1 << (7 - (pin->tx_bit % 10)));
+		default: {
+			/*
+			 * In the CEC_ERROR_INJ_TX_ADD_BYTES case we transmit
+			 * extra bytes, so pin->tx_bit / 10 can become >= 16.
+			 * Generate bit values for those extra bytes instead
+			 * of reading them from the transmit buffer.
+			 */
+			unsigned int idx = (pin->tx_bit / 10);
+			u8 val = idx;
+
+			if (idx < pin->tx_msg.len)
+				val = pin->tx_msg.msg[idx];
+			v = val & (1 << (7 - (pin->tx_bit % 10)));
+
 			pin->state = v ? CEC_ST_TX_DATA_BIT_1_LOW :
-				CEC_ST_TX_DATA_BIT_0_LOW;
+					 CEC_ST_TX_DATA_BIT_0_LOW;
 			break;
-		case 8:
-			v = pin->tx_bit / 10 == pin->tx_msg.len - 1;
+		}
+		case EOM_BIT:
+			v = pin->tx_bit / 10 ==
+				pin->tx_msg.len + pin->tx_extra_bytes - 1;
+			if (pin->tx_msg.len > 1 && tx_early_eom(pin)) {
+				/* Error injection: set EOM one byte early */
+				v = pin->tx_bit / 10 ==
+					pin->tx_msg.len + pin->tx_extra_bytes - 2;
+				pin->tx_post_eom = true;
+			}
+			if (tx_no_eom(pin)) {
+				/* Error injection: no EOM */
+				v = false;
+			}
 			pin->state = v ? CEC_ST_TX_DATA_BIT_1_LOW :
-				CEC_ST_TX_DATA_BIT_0_LOW;
+					 CEC_ST_TX_DATA_BIT_0_LOW;
 			break;
-		case 9:
+		case ACK_BIT:
 			pin->state = CEC_ST_TX_DATA_BIT_1_LOW;
 			break;
 		}
+		if (tx_custom_bit(pin))
+			pin->state = CEC_ST_TX_DATA_BIT_LOW_CUSTOM;
 		cec_pin_low(pin);
 		break;
 
 	case CEC_ST_TX_DATA_BIT_0_LOW:
 	case CEC_ST_TX_DATA_BIT_1_LOW:
 		v = pin->state == CEC_ST_TX_DATA_BIT_1_LOW;
-		pin->state = v ? CEC_ST_TX_DATA_BIT_1_HIGH :
-			CEC_ST_TX_DATA_BIT_0_HIGH;
-		is_ack_bit = pin->tx_bit % 10 == 9;
-		if (v && (pin->tx_bit < 4 || is_ack_bit))
+		is_ack_bit = pin->tx_bit % 10 == ACK_BIT;
+		if (v && (pin->tx_bit < 4 || is_ack_bit)) {
 			pin->state = CEC_ST_TX_DATA_BIT_1_HIGH_PRE_SAMPLE;
+		} else if (!is_ack_bit && tx_short_bit(pin)) {
+			/* Error Injection: send an invalid (too short) bit */
+			pin->state = v ? CEC_ST_TX_DATA_BIT_1_HIGH_SHORT :
+					 CEC_ST_TX_DATA_BIT_0_HIGH_SHORT;
+		} else if (!is_ack_bit && tx_long_bit(pin)) {
+			/* Error Injection: send an invalid (too long) bit */
+			pin->state = v ? CEC_ST_TX_DATA_BIT_1_HIGH_LONG :
+					 CEC_ST_TX_DATA_BIT_0_HIGH_LONG;
+		} else {
+			pin->state = v ? CEC_ST_TX_DATA_BIT_1_HIGH :
+					 CEC_ST_TX_DATA_BIT_0_HIGH;
+		}
+		cec_pin_high(pin);
+		break;
+
+	case CEC_ST_TX_DATA_BIT_LOW_CUSTOM:
+		pin->state = CEC_ST_TX_DATA_BIT_HIGH_CUSTOM;
 		cec_pin_high(pin);
 		break;
 
 	case CEC_ST_TX_DATA_BIT_1_HIGH_PRE_SAMPLE:
 		/* Read the CEC value at the sample time */
 		v = cec_pin_read(pin);
-		is_ack_bit = pin->tx_bit % 10 == 9;
+		is_ack_bit = pin->tx_bit % 10 == ACK_BIT;
 		/*
 		 * If v == 0 and we're within the first 4 bits
 		 * of the initiator, then someone else started
@@ -246,7 +556,7 @@ static void cec_pin_tx_states(struct cec_pin *pin, ktime_t ts)
 		 * transmitter has more leading 0 bits in the
 		 * initiator).
 		 */
-		if (!v && !is_ack_bit) {
+		if (!v && !is_ack_bit && !pin->tx_generated_poll) {
 			pin->tx_msg.len = 0;
 			pin->work_tx_ts = ts;
 			pin->work_tx_status = CEC_TX_STATUS_ARB_LOST;
@@ -255,18 +565,27 @@ static void cec_pin_tx_states(struct cec_pin *pin, ktime_t ts)
 			pin->tx_bit = 0;
 			memset(pin->rx_msg.msg, 0, sizeof(pin->rx_msg.msg));
 			pin->rx_msg.msg[0] = pin->tx_msg.msg[0];
-			pin->rx_msg.msg[0] &= ~(1 << (7 - pin->rx_bit));
+			pin->rx_msg.msg[0] &= (0xff << (8 - pin->rx_bit));
 			pin->rx_msg.len = 0;
+			pin->ts = ktime_sub_us(ts, CEC_TIM_DATA_BIT_SAMPLE);
 			pin->state = CEC_ST_RX_DATA_POST_SAMPLE;
 			pin->rx_bit++;
 			break;
 		}
 		pin->state = CEC_ST_TX_DATA_BIT_1_HIGH_POST_SAMPLE;
+		if (!is_ack_bit && tx_short_bit(pin)) {
+			/* Error Injection: send an invalid (too short) bit */
+			pin->state = CEC_ST_TX_DATA_BIT_1_HIGH_POST_SAMPLE_SHORT;
+		} else if (!is_ack_bit && tx_long_bit(pin)) {
+			/* Error Injection: send an invalid (too long) bit */
+			pin->state = CEC_ST_TX_DATA_BIT_1_HIGH_POST_SAMPLE_LONG;
+		}
 		if (!is_ack_bit)
 			break;
 		/* Was the message ACKed? */
 		ack = cec_msg_is_broadcast(&pin->tx_msg) ? v : !v;
-		if (!ack) {
+		if (!ack && !pin->tx_ignore_nack_until_eom &&
+		    pin->tx_bit / 10 < pin->tx_msg.len && !pin->tx_post_eom) {
 			/*
 			 * Note: the CEC spec is ambiguous regarding
 			 * what action to take when a NACK appears
@@ -283,6 +602,15 @@ static void cec_pin_tx_states(struct cec_pin *pin, ktime_t ts)
 		}
 		break;
 
+	case CEC_ST_TX_PULSE_LOW_CUSTOM:
+		cec_pin_high(pin);
+		pin->state = CEC_ST_TX_PULSE_HIGH_CUSTOM;
+		break;
+
+	case CEC_ST_TX_PULSE_HIGH_CUSTOM:
+		cec_pin_to_idle(pin);
+		break;
+
 	default:
 		break;
 	}
@@ -310,6 +638,7 @@ static void cec_pin_rx_states(struct cec_pin *pin, ktime_t ts)
 	bool ack;
 	bool bcast, for_us;
 	u8 dest;
+	u8 poll;
 
 	switch (pin->state) {
 	/* Receive states */
@@ -319,24 +648,44 @@ static void cec_pin_rx_states(struct cec_pin *pin, ktime_t ts)
 			break;
 		pin->state = CEC_ST_RX_START_BIT_HIGH;
 		delta = ktime_us_delta(ts, pin->ts);
-		pin->ts = ts;
 		/* Start bit low is too short, go back to idle */
-		if (delta < CEC_TIM_START_BIT_LOW_MIN -
-			    CEC_TIM_IDLE_SAMPLE) {
+		if (delta < CEC_TIM_START_BIT_LOW_MIN - CEC_TIM_IDLE_SAMPLE) {
 			cec_pin_to_idle(pin);
+			break;
+		}
+		if (rx_arb_lost(pin, &poll)) {
+			cec_msg_init(&pin->tx_msg, poll >> 4, poll & 0xf);
+			pin->tx_generated_poll = true;
+			pin->tx_extra_bytes = 0;
+			pin->state = CEC_ST_TX_START_BIT_HIGH;
+			pin->ts = ts;
 		}
 		break;
 
 	case CEC_ST_RX_START_BIT_HIGH:
 		v = cec_pin_read(pin);
 		delta = ktime_us_delta(ts, pin->ts);
-		if (v && delta > CEC_TIM_START_BIT_TOTAL_MAX -
-				 CEC_TIM_START_BIT_LOW_MIN) {
+		/*
+		 * Unfortunately the spec does not specify when to give up
+		 * and go to idle. We just pick TOTAL_LONG.
+		 */
+		if (v && delta > CEC_TIM_START_BIT_TOTAL_LONG) {
 			cec_pin_to_idle(pin);
 			break;
 		}
 		if (v)
 			break;
+		/* Start bit is too short, go back to idle */
+		if (delta < CEC_TIM_START_BIT_TOTAL_MIN - CEC_TIM_IDLE_SAMPLE) {
+			cec_pin_to_idle(pin);
+			break;
+		}
+		if (rx_low_drive(pin)) {
+			/* Error injection: go to low drive */
+			cec_pin_low(pin);
+			pin->state = CEC_ST_RX_LOW_DRIVE;
+			break;
+		}
 		pin->state = CEC_ST_RX_DATA_SAMPLE;
 		pin->ts = ts;
 		pin->rx_eom = false;
@@ -351,36 +700,48 @@ static void cec_pin_rx_states(struct cec_pin *pin, ktime_t ts)
 				pin->rx_msg.msg[pin->rx_bit / 10] |=
 					v << (7 - (pin->rx_bit % 10));
 			break;
-		case 8:
+		case EOM_BIT:
 			pin->rx_eom = v;
 			pin->rx_msg.len = pin->rx_bit / 10 + 1;
 			break;
-		case 9:
+		case ACK_BIT:
 			break;
 		}
 		pin->rx_bit++;
 		break;
 
 	case CEC_ST_RX_DATA_POST_SAMPLE:
-		pin->state = CEC_ST_RX_DATA_HIGH;
+		pin->state = CEC_ST_RX_DATA_WAIT_FOR_LOW;
 		break;
 
-	case CEC_ST_RX_DATA_HIGH:
+	case CEC_ST_RX_DATA_WAIT_FOR_LOW:
 		v = cec_pin_read(pin);
 		delta = ktime_us_delta(ts, pin->ts);
-		if (v && delta > CEC_TIM_DATA_BIT_TOTAL_MAX) {
+		/*
+		 * Unfortunately the spec does not specify when to give up
+		 * and go to idle. We just pick TOTAL_LONG.
+		 */
+		if (v && delta > CEC_TIM_DATA_BIT_TOTAL_LONG) {
 			cec_pin_to_idle(pin);
 			break;
 		}
 		if (v)
 			break;
+
+		if (rx_low_drive(pin)) {
+			/* Error injection: go to low drive */
+			cec_pin_low(pin);
+			pin->state = CEC_ST_RX_LOW_DRIVE;
+			break;
+		}
+
 		/*
 		 * Go to low drive state when the total bit time is
 		 * too short.
 		 */
 		if (delta < CEC_TIM_DATA_BIT_TOTAL_MIN) {
 			cec_pin_low(pin);
-			pin->state = CEC_ST_LOW_DRIVE;
+			pin->state = CEC_ST_RX_LOW_DRIVE;
 			break;
 		}
 		pin->ts = ts;
@@ -396,6 +757,11 @@ static void cec_pin_rx_states(struct cec_pin *pin, ktime_t ts)
 		/* ACK bit value */
 		ack = bcast ? 1 : !for_us;
 
+		if (for_us && rx_nack(pin)) {
+			/* Error injection: toggle the ACK bit */
+			ack = !ack;
+		}
+
 		if (ack) {
 			/* No need to write to the bus, just wait */
 			pin->state = CEC_ST_RX_ACK_HIGH_POST;
@@ -422,7 +788,7 @@ static void cec_pin_rx_states(struct cec_pin *pin, ktime_t ts)
 			break;
 		}
 		pin->rx_bit++;
-		pin->state = CEC_ST_RX_DATA_HIGH;
+		pin->state = CEC_ST_RX_DATA_WAIT_FOR_LOW;
 		break;
 
 	case CEC_ST_RX_ACK_FINISH:
@@ -444,6 +810,7 @@ static enum hrtimer_restart cec_pin_timer(struct hrtimer *timer)
 	struct cec_adapter *adap = pin->adap;
 	ktime_t ts;
 	s32 delta;
+	u32 usecs;
 
 	ts = ktime_get();
 	if (ktime_to_ns(pin->timer_ts)) {
@@ -491,13 +858,27 @@ static enum hrtimer_restart cec_pin_timer(struct hrtimer *timer)
 	/* Transmit states */
 	case CEC_ST_TX_WAIT_FOR_HIGH:
 	case CEC_ST_TX_START_BIT_LOW:
-	case CEC_ST_TX_DATA_BIT_1_HIGH_POST_SAMPLE:
-	case CEC_ST_TX_DATA_BIT_0_HIGH:
-	case CEC_ST_TX_DATA_BIT_1_HIGH:
 	case CEC_ST_TX_START_BIT_HIGH:
+	case CEC_ST_TX_START_BIT_HIGH_SHORT:
+	case CEC_ST_TX_START_BIT_HIGH_LONG:
+	case CEC_ST_TX_START_BIT_LOW_CUSTOM:
+	case CEC_ST_TX_START_BIT_HIGH_CUSTOM:
 	case CEC_ST_TX_DATA_BIT_0_LOW:
+	case CEC_ST_TX_DATA_BIT_0_HIGH:
+	case CEC_ST_TX_DATA_BIT_0_HIGH_SHORT:
+	case CEC_ST_TX_DATA_BIT_0_HIGH_LONG:
 	case CEC_ST_TX_DATA_BIT_1_LOW:
+	case CEC_ST_TX_DATA_BIT_1_HIGH:
+	case CEC_ST_TX_DATA_BIT_1_HIGH_SHORT:
+	case CEC_ST_TX_DATA_BIT_1_HIGH_LONG:
 	case CEC_ST_TX_DATA_BIT_1_HIGH_PRE_SAMPLE:
+	case CEC_ST_TX_DATA_BIT_1_HIGH_POST_SAMPLE:
+	case CEC_ST_TX_DATA_BIT_1_HIGH_POST_SAMPLE_SHORT:
+	case CEC_ST_TX_DATA_BIT_1_HIGH_POST_SAMPLE_LONG:
+	case CEC_ST_TX_DATA_BIT_LOW_CUSTOM:
+	case CEC_ST_TX_DATA_BIT_HIGH_CUSTOM:
+	case CEC_ST_TX_PULSE_LOW_CUSTOM:
+	case CEC_ST_TX_PULSE_HIGH_CUSTOM:
 		cec_pin_tx_states(pin, ts);
 		break;
 
@@ -506,7 +887,7 @@ static enum hrtimer_restart cec_pin_timer(struct hrtimer *timer)
 	case CEC_ST_RX_START_BIT_HIGH:
 	case CEC_ST_RX_DATA_SAMPLE:
 	case CEC_ST_RX_DATA_POST_SAMPLE:
-	case CEC_ST_RX_DATA_HIGH:
+	case CEC_ST_RX_DATA_WAIT_FOR_LOW:
 	case CEC_ST_RX_ACK_LOW:
 	case CEC_ST_RX_ACK_LOW_POST:
 	case CEC_ST_RX_ACK_HIGH_POST:
@@ -533,7 +914,10 @@ static enum hrtimer_restart cec_pin_timer(struct hrtimer *timer)
 			if (delta / CEC_TIM_DATA_BIT_TOTAL >
 			    pin->tx_signal_free_time) {
 				pin->tx_nacked = false;
-				pin->state = CEC_ST_TX_START_BIT_LOW;
+				if (tx_custom_start(pin))
+					pin->state = CEC_ST_TX_START_BIT_LOW_CUSTOM;
+				else
+					pin->state = CEC_ST_TX_START_BIT_LOW;
 				/* Generate start bit */
 				cec_pin_low(pin);
 				break;
@@ -543,6 +927,13 @@ static enum hrtimer_restart cec_pin_timer(struct hrtimer *timer)
 				pin->state = CEC_ST_TX_WAIT;
 			break;
 		}
+		if (pin->tx_custom_pulse && pin->state == CEC_ST_IDLE) {
+			pin->tx_custom_pulse = false;
+			/* Generate custom pulse */
+			cec_pin_low(pin);
+			pin->state = CEC_ST_TX_PULSE_LOW_CUSTOM;
+			break;
+		}
 		if (pin->state != CEC_ST_IDLE || pin->ops->enable_irq == NULL ||
 		    pin->enable_irq_failed || adap->is_configuring ||
 		    adap->is_configured || adap->monitor_all_cnt)
@@ -553,21 +944,40 @@ static enum hrtimer_restart cec_pin_timer(struct hrtimer *timer)
 		wake_up_interruptible(&pin->kthread_waitq);
 		return HRTIMER_NORESTART;
 
-	case CEC_ST_LOW_DRIVE:
+	case CEC_ST_TX_LOW_DRIVE:
+	case CEC_ST_RX_LOW_DRIVE:
+		cec_pin_high(pin);
 		cec_pin_to_idle(pin);
 		break;
 
 	default:
 		break;
 	}
-	if (!adap->monitor_pin_cnt || states[pin->state].usecs <= 150) {
+
+	switch (pin->state) {
+	case CEC_ST_TX_START_BIT_LOW_CUSTOM:
+	case CEC_ST_TX_DATA_BIT_LOW_CUSTOM:
+	case CEC_ST_TX_PULSE_LOW_CUSTOM:
+		usecs = pin->tx_custom_low_usecs;
+		break;
+	case CEC_ST_TX_START_BIT_HIGH_CUSTOM:
+	case CEC_ST_TX_DATA_BIT_HIGH_CUSTOM:
+	case CEC_ST_TX_PULSE_HIGH_CUSTOM:
+		usecs = pin->tx_custom_high_usecs;
+		break;
+	default:
+		usecs = states[pin->state].usecs;
+		break;
+	}
+
+	if (!adap->monitor_pin_cnt || usecs <= 150) {
 		pin->wait_usecs = 0;
-		pin->timer_ts = ktime_add_us(ts, states[pin->state].usecs);
+		pin->timer_ts = ktime_add_us(ts, usecs);
 		hrtimer_forward_now(timer,
-				ns_to_ktime(states[pin->state].usecs * 1000));
+				ns_to_ktime(usecs * 1000));
 		return HRTIMER_RESTART;
 	}
-	pin->wait_usecs = states[pin->state].usecs - 100;
+	pin->wait_usecs = usecs - 100;
 	pin->timer_ts = ktime_add_us(ts, 100);
 	hrtimer_forward_now(timer, ns_to_ktime(100000));
 	return HRTIMER_RESTART;
@@ -587,9 +997,22 @@ static int cec_pin_thread_func(void *_adap)
 			atomic_read(&pin->work_pin_events));
 
 		if (pin->work_rx_msg.len) {
-			cec_received_msg_ts(adap, &pin->work_rx_msg,
+			struct cec_msg *msg = &pin->work_rx_msg;
+
+			if (msg->len > 1 && msg->len < CEC_MAX_MSG_SIZE &&
+			    rx_add_byte(pin)) {
+				/* Error injection: add byte to the message */
+				msg->msg[msg->len++] = 0x55;
+			}
+			if (msg->len > 2 && rx_remove_byte(pin)) {
+				/* Error injection: remove byte from message */
+				msg->len--;
+			}
+			if (msg->len > CEC_MAX_MSG_SIZE)
+				msg->len = CEC_MAX_MSG_SIZE;
+			cec_received_msg_ts(adap, msg,
 				ns_to_ktime(pin->work_rx_msg.rx_ts));
-			pin->work_rx_msg.len = 0;
+			msg->len = 0;
 		}
 		if (pin->work_tx_status) {
 			unsigned int tx_status = pin->work_tx_status;
@@ -698,7 +1121,16 @@ static int cec_pin_adap_transmit(struct cec_adapter *adap, u8 attempts,
 	struct cec_pin *pin = adap->pin;
 
 	pin->tx_signal_free_time = signal_free_time;
+	pin->tx_extra_bytes = 0;
 	pin->tx_msg = *msg;
+	if (msg->len > 1) {
+		/* Error injection: add byte to the message */
+		pin->tx_extra_bytes = tx_add_bytes(pin);
+	}
+	if (msg->len > 2 && tx_remove_byte(pin)) {
+		/* Error injection: remove byte from the message */
+		pin->tx_msg.len--;
+	}
 	pin->work_tx_status = 0;
 	pin->tx_bit = 0;
 	cec_pin_start_timer(pin);
-- 
2.16.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCHv2 6/7] cec-pin-error-inj.rst: document CEC Pin Error Injection
  2018-03-05 13:51 [PATCHv2 0/7] cec: add error injection support Hans Verkuil
                   ` (4 preceding siblings ...)
  2018-03-05 13:51 ` [PATCHv2 5/7] cec-pin: add error injection support Hans Verkuil
@ 2018-03-05 13:51 ` Hans Verkuil
  2018-03-21 15:45   ` Mauro Carvalho Chehab
  2018-03-05 13:51 ` [PATCHv2 7/7] cec-pin: improve status log Hans Verkuil
  2018-03-05 17:40 ` [PATCHv2 0/7] cec: add error injection support Wolfram Sang
  7 siblings, 1 reply; 14+ messages in thread
From: Hans Verkuil @ 2018-03-05 13:51 UTC (permalink / raw)
  To: linux-media; +Cc: Wolfram Sang, Maxime Ripard, dri-devel, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

The CEC Pin framework adds support for Error Injection.

Document all the error injections commands and how to use it.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 .../media/cec-drivers/cec-pin-error-inj.rst        | 322 +++++++++++++++++++++
 Documentation/media/cec-drivers/index.rst          |   1 +
 MAINTAINERS                                        |   1 +
 3 files changed, 324 insertions(+)
 create mode 100644 Documentation/media/cec-drivers/cec-pin-error-inj.rst

diff --git a/Documentation/media/cec-drivers/cec-pin-error-inj.rst b/Documentation/media/cec-drivers/cec-pin-error-inj.rst
new file mode 100644
index 000000000000..21bda831d3fb
--- /dev/null
+++ b/Documentation/media/cec-drivers/cec-pin-error-inj.rst
@@ -0,0 +1,322 @@
+CEC Pin Framework Error Injection
+=================================
+
+The CEC Pin Framework is a core CEC framework for CEC hardware that only
+has low-level support for the CEC bus. Most hardware today will have
+high-level CEC support where the hardware deals with driving the CEC bus,
+but some older devices aren't that fancy. However, this framework also
+allows you to connect the CEC pin to a GPIO on e.g. a Raspberry Pi and
+you can become an instant CEC adapter.
+
+What makes doing this so interesting is that since we have full control
+over the bus it is easy to support error injection. This is ideal to
+test how well CEC adapters can handle error conditions.
+
+Currently only the cec-gpio driver (when the CEC line is directly
+connected to a pull-up GPIO line) and the AllWinner A10/A20 drm driver
+support this framework.
+
+If ``CONFIG_CEC_PIN_ERROR_INJ`` is enabled, then error injection is available
+through debugfs. Specifically, in ``/sys/kernel/debug/cec/cecX/`` there is
+now an ``error-inj`` file.
+
+With ``cat error-inj`` you can see both the possible commands and the current
+error injection status:
+
+.. code-block:: none
+
+	$ cat /sys/kernel/debug/cec/cec0/error-inj
+	# Clear error injections:
+	#   clear          clear all rx and tx error injections
+	#   rx-clear       clear all rx error injections
+	#   tx-clear       clear all tx error injections
+	#   <op> clear     clear all rx and tx error injections for <op>
+	#   <op> rx-clear  clear all rx error injections for <op>
+	#   <op> tx-clear  clear all tx error injections for <op>
+	#
+	# RX error injection:
+	#   <op>[,<mode>] rx-nack              NACK the message instead of sending an ACK
+	#   <op>[,<mode>] rx-low-drive <bit>   force a low-drive condition at this bit position
+	#   <op>[,<mode>] rx-add-byte          add a spurious byte to the received CEC message
+	#   <op>[,<mode>] rx-remove-byte       remove the last byte from the received CEC message
+	#   <op>[,<mode>] rx-arb-lost <poll>   generate a POLL message to trigger an arbitration lost
+	#
+	# TX error injection settings:
+	#   tx-ignore-nack-until-eom           ignore early NACKs until EOM
+	#   tx-custom-low-usecs <usecs>        define the 'low' time for the custom pulse
+	#   tx-custom-high-usecs <usecs>       define the 'high' time for the custom pulse
+	#   tx-custom-pulse                    transmit the custom pulse once the bus is idle
+	#
+	# TX error injection:
+	#   <op>[,<mode>] tx-no-eom            don't set the EOM bit
+	#   <op>[,<mode>] tx-early-eom         set the EOM bit one byte too soon
+	#   <op>[,<mode>] tx-add-bytes <num>   append <num> (1-255) spurious bytes to the message
+	#   <op>[,<mode>] tx-remove-byte       drop the last byte from the message
+	#   <op>[,<mode>] tx-short-bit <bit>   make this bit shorter than allowed
+	#   <op>[,<mode>] tx-long-bit <bit>    make this bit longer than allowed
+	#   <op>[,<mode>] tx-custom-bit <bit>  send the custom pulse instead of this bit
+	#   <op>[,<mode>] tx-short-start       send a start pulse that's too short
+	#   <op>[,<mode>] tx-long-start        send a start pulse that's too long
+	#   <op>[,<mode>] tx-custom-start      send the custom pulse instead of the start pulse
+	#   <op>[,<mode>] tx-last-bit <bit>    stop sending after this bit
+	#   <op>[,<mode>] tx-low-drive <bit>   force a low-drive condition at this bit position
+	#
+	# <op>       CEC message opcode (0-255) or 'any'
+	# <mode>     'once' (default), 'always', 'toggle' or 'off'
+	# <bit>      CEC message bit (0-159)
+	#            10 bits per 'byte': bits 0-7: data, bit 8: EOM, bit 9: ACK
+	# <poll>     CEC poll message used to test arbitration lost (0x00-0xff, default 0x0f)
+	# <usecs>    microseconds (0-10000000, default 1000)
+
+	clear
+
+You can write error injection commands to ``error-inj`` using ``echo 'cmd' >error-inj``
+or ``cat cmd.txt >error-inj``. The ``cat error-inj`` output contains the current
+error commands. You can save the output to a file and use it as an input to
+``error-inj`` later.
+
+Basic Syntax
+------------
+
+Leading spaces/tabs are ignored. If the next character is a ``#`` or the end of the
+line was reached, then the whole line is ignored. Otherwise a command is expected.
+
+The error injection commands fall in two main groups: those relating to receiving
+CEC messages and those relating to transmitting CEC messages. In addition, there
+are commands to clear existing error injection commands and to create custom
+pulses on the CEC bus.
+
+Most error injection commands can be executed for specific CEC opcodes or for all
+opcodes (``any``). Each command also has a 'mode' which can be ``off`` (can be used
+to turn off an existing error injection command), ``once`` (the default) which will
+trigger the error injection only once for the next received or transmitted message,
+``always`` to always trigger the error injection and ``toggle`` to toggle the error
+injection on or off for every transmit or receive.
+
+So '``any rx-nack``' will NACK the next received CEC message, '``any,always rx-nack``' will
+NACK all received CEC messages and '``0x82,toggle rx-nack``' will only NACK if an Active
+Source message was received and do that only for every other received message.
+
+After an error was injected with mode ``once`` the error injection command is cleared
+automatically, so ``once`` is a one-time deal.
+
+All combinations of ``<op>`` and error injection commands can co-exist. So
+this is fine:
+
+.. code-block:: none
+
+	0x9e tx-add-bytes 1
+	0x9e tx-early-eom
+	0x9f tx-add-bytes 2
+	any rx-nack
+
+All four error injection commands will be active simultaneously.
+
+However, if the same ``<op>`` and command combination is specified,
+but with different arguments:
+
+.. code-block:: none
+
+	0x9e tx-add-bytes 1
+	0x9e tx-add-bytes 2
+
+Then the second will overwrite the first.
+
+Clear Error Injections
+----------------------
+
+``clear``
+    Clear all error injections.
+
+``rx-clear``
+    Clear all receive error injections
+
+``tx-clear``
+    Clear all transmit error injections
+
+``<op> clear``
+    Clear all error injections for the given opcode.
+
+``<op> rx-clear``
+    Clear all receive error injections for the given opcode.
+
+``<op> tx-clear``
+    Clear all transmit error injections for the given opcode.
+
+Receive Messages
+----------------
+
+``<op>[,<mode>] rx-nack``
+    NACK broadcast messages and messages directed to this CEC adapter.
+    Every byte of the message will be NACKed in case the transmitter
+    keeps transmitting after the first byte was NACKed.
+
+``<op>[,<mode>] rx-low-drive <bit>``
+    Force a Low Drive condition at this bit position. If <op> specifies
+    a specific CEC opcode then the bit position must be at least 18,
+    otherwise the opcode hasn't been received yet. This tests if the transmitter
+    can handle the Low Drive condition correctly and reports the error
+    correctly. Note that a Low Drive in the first 4 bits can also be
+    interpreted as an Arbitration Lost condition by the transmitter.
+    This is implementation dependent.
+
+``<op>[,<mode>] rx-add-byte``
+    Add a spurious 0x55 byte to the received CEC message, provided
+    the message was 15 bytes long or less. This is useful to test
+    the high-level protocol since spurious bytes should be ignored.
+
+``<op>[,<mode>] rx-remove-byte``
+    Remove the last byte from the received CEC message, provided it
+    was at least 2 bytes long. This is useful to test the high-level
+    protocol since messages that are too short should be ignored.
+
+``<op>[,<mode>] rx-arb-lost <poll>``
+    Generate a POLL message to trigger an Arbitration Lost condition.
+    This command is only allowed for ``<op>`` values of ``next`` or ``all``.
+    As soon as a start bit has been received the CEC adapter will switch
+    to transmit mode and it will transmit a POLL message. By default this is
+    0x0f, but it can also be specified explicitly via the ``<poll>`` argument.
+
+    This command can be used to test the Arbitration Lost condition in
+    the remote CEC transmitter. Arbitration happens when two CEC adapters
+    start sending a message at the same time. In that case the initiator
+    with the most leading zeroes wins and the other transmitter has to
+    stop transmitting ('Arbitration Lost'). This is very hard to test,
+    except by using this error injection command.
+
+    This does not work if the remote CEC transmitter has logical address
+    0 ('TV') since that will always win.
+
+Transmit Messages
+-----------------
+
+``tx-ignore-nack-until-eom``
+    This setting changes the behavior of transmitting CEC messages. Normally
+    as soon as the receiver NACKs a byte the transmit will stop, but the
+    specification also allows that the full message is transmitted and only
+    at the end will the transmitter look at the ACK bit. This is not recommended
+    behavior since there is no point in keeping the CEC bus busy for longer than
+    is strictly needed. Especially given how slow the bus is.
+
+    This setting can be used to test how well a receiver deals with transmitters
+    that ignore NACKs until the very end of the message.
+
+``<op>[,<mode>] tx-no-eom``
+    Don't set the EOM bit. Normally the last byte of the message has the EOM
+    (End-Of-Message) bit set. With this command the transmit will just stop
+    without ever sending an EOM. This can be used to test how a receiver
+    handles this case. Normally receivers have a time-out after which
+    they will go back to the Idle state.
+
+``<op>[,<mode>] tx-early-eom``
+    Set the EOM bit one byte too soon. This obviously only works for messages
+    of two bytes or more. The EOM bit will be set for the second-to-last byte
+    and not for the final byte. The receiver should ignore the last byte in this
+    case. Since the resulting message is likely to be too short for this same
+    reason the whole message is typically ignored. The receiver should be in
+    Idle state after the last byte was transmitted.
+
+``<op>[,<mode>] tx-add-bytes <num>``
+    Append ``<num>`` (1-255) spurious bytes to the message. The extra bytes
+    have the value of the byte position in the message. So if you transmit a
+    two byte message (e.g. a Get CEC Version message) and add 2 bytes, then
+    the full message received by the remote CEC adapter is ``0x40 0x9f 0x02 0x03``.
+
+    This command can be used to test buffer overflows in the receiver. E.g.
+    what does it do when it receives more than the maximum message size of 16
+    bytes.
+
+``<op>[,<mode>] tx-remove-byte``
+    Drop the last byte from the message, provided the message is at least
+    two bytes long. The receiver should ignore messages that are too short.
+
+``<op>[,<mode>] tx-short-bit <bit>``
+    Make this bit period shorter than allowed. The bit position cannot be
+    an Ack bit.  If <op> specifies a specific CEC opcode then the bit position
+    must be at least 18, otherwise the opcode hasn't been received yet. Normally the
+    period of a data bit is between 2.05 and 2.75 milliseconds. With this
+    command the period of this bit is 1.8 milliseconds, this is done by
+    reducing the time the CEC bus is high. This bit period is less than
+    is allowed and the receiver should respond with a Low Drive
+    condition.
+
+    This command is ignored for 0 bits in bit positions 0 to 3. This is
+    because the receiver also looks for an Arbitration Lost condition in
+    those first four bits and it is undefined what will happen if it
+    sees a too-short 0 bit.
+
+``<op>[,<mode>] tx-long-bit <bit>``
+    Make this bit period longer than is valid. The bit position cannot be
+    an Ack bit.  If <op> specifies a specific CEC opcode then the bit position
+    must be at least 18, otherwise the opcode hasn't been received yet. Normally the
+    period of a data bit is between 2.05 and 2.75 milliseconds. With this
+    command the period of this bit is 2.9 milliseconds, this is done by
+    increasing the time the CEC bus is high.
+
+    Even though this bit period is longer than is valid it is undefined what
+    a receiver will do. It might just accept it, or it might time out and
+    return to Idle state. Unfortunately the CEC specification is silent about
+    this.
+
+    This command is ignored for 0 bits in bit positions 0 to 3. This is
+    because the receiver also looks for an Arbitration Lost condition in
+    those first four bits and it is undefined what will happen if it
+    sees a too-long 0 bit.
+
+``<op>[,<mode>] tx-short-start``
+    Make this start bit period shorter than allowed. Normally the period of a start bit is
+    between 4.3 and 4.7 milliseconds. With this command the period of the
+    start bit is 4.1 milliseconds, this is done by reducing the time the
+    CEC bus is high. This start bit period is less than is allowed
+    and the receiver should return to Idle state when this is detected.
+
+``<op>[,<mode>] tx-long-start``
+    Make this start bit period longer than is valid. Normally the period of a start bit is
+    between 4.3 and 4.7 milliseconds. With this command the period of the
+    start bit is 5 milliseconds, this is done by increasing the time the
+    CEC bus is high. This start bit period is more than is valid
+    and the receiver should return to Idle state when this is detected.
+
+    Even though this start bit period is longer than is valid it is undefined what
+    a receiver will do. It might just accept it, or it might time out and
+    return to Idle state. Unfortunately the CEC specification is silent about
+    this.
+
+``<op>[,<mode>] tx-last-bit <bit>``
+    Just stop transmitting after this bit.  If <op> specifies a specific CEC
+    opcode then the bit position must be at least 18, otherwise the opcode
+    hasn't been received yet. This command can be used to test how the receiver reacts
+    when a message just suddenly stops. It should time out and go back to Idle
+    state.
+
+``<op>[,<mode>] tx-low-drive <bit>``
+    Force a Low Drive condition at this bit position. If <op> specifies a specific
+    CEC opcode then the bit position must be at least 18, otherwise the opcode
+    hasn't been received yet. This can be used to test how the receiver handles Low Drive
+    conditions. Note that if this happens at bit positions 0-3 the receiver
+    can interpret this as an Arbitration Lost condition. This is implementation
+    dependent.
+
+Custom Pulses
+-------------
+
+``tx-custom-low-usecs <usecs>``
+    This defines the duration in microseconds that the custom pulse pulls the CEC line low.
+    The default is 1000 microseconds.
+
+``tx-custom-high-usecs <usecs>``
+    This defines the duration in microseconds that the custom pulse keeps the
+    CEC line high (unless another CEC adapter pulls it low in that time).
+    The default is 1000 microseconds. The total period of the custom pulse is
+    ``tx-custom-low-usecs + tx-custom-high-usecs``.
+
+``<op>[,<mode>] tx-custom-bit <bit>``
+    Send the custom bit instead of a regular data bit. The bit position cannot be
+    an Ack bit.  If <op> specifies a specific CEC opcode then the bit position
+    must be at least 18, otherwise the opcode hasn't been received yet.
+
+``<op>[,<mode>] tx-custom-start``
+    Send the custom bit instead of a regular start bit.
+
+``tx-custom-pulse``
+    Transmit a single custom pulse as soon as the CEC bus is idle.
diff --git a/Documentation/media/cec-drivers/index.rst b/Documentation/media/cec-drivers/index.rst
index 7ef204823422..3580bdb790bc 100644
--- a/Documentation/media/cec-drivers/index.rst
+++ b/Documentation/media/cec-drivers/index.rst
@@ -31,4 +31,5 @@ For more details see the file COPYING in the source distribution of Linux.
 	:maxdepth: 5
 	:numbered:
 
+	cec-pin-error-inj
 	pulse8-cec
diff --git a/MAINTAINERS b/MAINTAINERS
index 91ed6adfa4a6..085b40adbe4d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3305,6 +3305,7 @@ F:	include/media/cec-notifier.h
 F:	include/uapi/linux/cec.h
 F:	include/uapi/linux/cec-funcs.h
 F:	Documentation/devicetree/bindings/media/cec.txt
+F:	Documentation/media/cec-drivers/cec-pin-error-inj.rst
 
 CEC GPIO DRIVER
 M:	Hans Verkuil <hans.verkuil@cisco.com>
-- 
2.16.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCHv2 7/7] cec-pin: improve status log
  2018-03-05 13:51 [PATCHv2 0/7] cec: add error injection support Hans Verkuil
                   ` (5 preceding siblings ...)
  2018-03-05 13:51 ` [PATCHv2 6/7] cec-pin-error-inj.rst: document CEC Pin Error Injection Hans Verkuil
@ 2018-03-05 13:51 ` Hans Verkuil
  2018-03-05 17:40 ` [PATCHv2 0/7] cec: add error injection support Wolfram Sang
  7 siblings, 0 replies; 14+ messages in thread
From: Hans Verkuil @ 2018-03-05 13:51 UTC (permalink / raw)
  To: linux-media; +Cc: Wolfram Sang, Maxime Ripard, dri-devel, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

Keep track of the number of short or long start bits, the number
of short or long data bits and the number of initiated or detected
low drive conditions.

Show this information in the status debugfs log.

Helpful when debugging, particularly when doing error injection
as well.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/cec/cec-pin-priv.h | 13 +++++++++
 drivers/media/cec/cec-pin.c      | 58 +++++++++++++++++++++++++++++++++++++---
 2 files changed, 68 insertions(+), 3 deletions(-)

diff --git a/drivers/media/cec/cec-pin-priv.h b/drivers/media/cec/cec-pin-priv.h
index c9349f68e554..dae8ba6f1037 100644
--- a/drivers/media/cec/cec-pin-priv.h
+++ b/drivers/media/cec/cec-pin-priv.h
@@ -181,6 +181,18 @@ struct cec_pin {
 	struct cec_msg			rx_msg;
 	u32				rx_bit;
 	bool				rx_toggle;
+	u32				rx_start_bit_low_too_short_cnt;
+	u64				rx_start_bit_low_too_short_ts;
+	u32				rx_start_bit_low_too_short_delta;
+	u32				rx_start_bit_too_short_cnt;
+	u64				rx_start_bit_too_short_ts;
+	u32				rx_start_bit_too_short_delta;
+	u32				rx_start_bit_too_long_cnt;
+	u32				rx_data_bit_too_short_cnt;
+	u64				rx_data_bit_too_short_ts;
+	u32				rx_data_bit_too_short_delta;
+	u32				rx_data_bit_too_long_cnt;
+	u32				rx_low_drive_cnt;
 
 	struct cec_msg			work_rx_msg;
 	u8				work_tx_status;
@@ -205,6 +217,7 @@ struct cec_pin {
 	bool				tx_generated_poll;
 	bool				tx_post_eom;
 	u8				tx_extra_bytes;
+	u32				tx_low_drive_cnt;
 #ifdef CONFIG_CEC_PIN_ERROR_INJ
 	u64				error_inj[CEC_ERROR_INJ_OP_ANY + 1];
 	u8				error_inj_args[CEC_ERROR_INJ_OP_ANY + 1][CEC_ERROR_INJ_NUM_ARGS];
diff --git a/drivers/media/cec/cec-pin.c b/drivers/media/cec/cec-pin.c
index 430a23392299..b509df154ca1 100644
--- a/drivers/media/cec/cec-pin.c
+++ b/drivers/media/cec/cec-pin.c
@@ -429,6 +429,7 @@ static void cec_pin_tx_states(struct cec_pin *pin, ktime_t ts)
 			pin->state = CEC_ST_TX_WAIT_FOR_HIGH;
 			pin->work_tx_ts = ts;
 			pin->work_tx_status = CEC_TX_STATUS_LOW_DRIVE;
+			pin->tx_low_drive_cnt++;
 			wake_up_interruptible(&pin->kthread_waitq);
 			break;
 		}
@@ -460,6 +461,7 @@ static void cec_pin_tx_states(struct cec_pin *pin, ktime_t ts)
 				break;
 			pin->work_tx_ts = ts;
 			pin->work_tx_status = CEC_TX_STATUS_LOW_DRIVE;
+			pin->tx_low_drive_cnt++;
 			wake_up_interruptible(&pin->kthread_waitq);
 			break;
 		}
@@ -650,6 +652,10 @@ static void cec_pin_rx_states(struct cec_pin *pin, ktime_t ts)
 		delta = ktime_us_delta(ts, pin->ts);
 		/* Start bit low is too short, go back to idle */
 		if (delta < CEC_TIM_START_BIT_LOW_MIN - CEC_TIM_IDLE_SAMPLE) {
+			if (!pin->rx_start_bit_low_too_short_cnt++) {
+				pin->rx_start_bit_low_too_short_ts = pin->ts;
+				pin->rx_start_bit_low_too_short_delta = delta;
+			}
 			cec_pin_to_idle(pin);
 			break;
 		}
@@ -670,6 +676,7 @@ static void cec_pin_rx_states(struct cec_pin *pin, ktime_t ts)
 		 * and go to idle. We just pick TOTAL_LONG.
 		 */
 		if (v && delta > CEC_TIM_START_BIT_TOTAL_LONG) {
+			pin->rx_start_bit_too_long_cnt++;
 			cec_pin_to_idle(pin);
 			break;
 		}
@@ -677,6 +684,10 @@ static void cec_pin_rx_states(struct cec_pin *pin, ktime_t ts)
 			break;
 		/* Start bit is too short, go back to idle */
 		if (delta < CEC_TIM_START_BIT_TOTAL_MIN - CEC_TIM_IDLE_SAMPLE) {
+			if (!pin->rx_start_bit_too_short_cnt++) {
+				pin->rx_start_bit_too_short_ts = pin->ts;
+				pin->rx_start_bit_too_short_delta = delta;
+			}
 			cec_pin_to_idle(pin);
 			break;
 		}
@@ -684,6 +695,7 @@ static void cec_pin_rx_states(struct cec_pin *pin, ktime_t ts)
 			/* Error injection: go to low drive */
 			cec_pin_low(pin);
 			pin->state = CEC_ST_RX_LOW_DRIVE;
+			pin->rx_low_drive_cnt++;
 			break;
 		}
 		pin->state = CEC_ST_RX_DATA_SAMPLE;
@@ -722,6 +734,7 @@ static void cec_pin_rx_states(struct cec_pin *pin, ktime_t ts)
 		 * and go to idle. We just pick TOTAL_LONG.
 		 */
 		if (v && delta > CEC_TIM_DATA_BIT_TOTAL_LONG) {
+			pin->rx_data_bit_too_long_cnt++;
 			cec_pin_to_idle(pin);
 			break;
 		}
@@ -732,6 +745,7 @@ static void cec_pin_rx_states(struct cec_pin *pin, ktime_t ts)
 			/* Error injection: go to low drive */
 			cec_pin_low(pin);
 			pin->state = CEC_ST_RX_LOW_DRIVE;
+			pin->rx_low_drive_cnt++;
 			break;
 		}
 
@@ -740,8 +754,13 @@ static void cec_pin_rx_states(struct cec_pin *pin, ktime_t ts)
 		 * too short.
 		 */
 		if (delta < CEC_TIM_DATA_BIT_TOTAL_MIN) {
+			if (!pin->rx_data_bit_too_short_cnt++) {
+				pin->rx_data_bit_too_short_ts = pin->ts;
+				pin->rx_data_bit_too_short_delta = delta;
+			}
 			cec_pin_low(pin);
 			pin->state = CEC_ST_RX_LOW_DRIVE;
+			pin->rx_low_drive_cnt++;
 			break;
 		}
 		pin->ts = ts;
@@ -1142,9 +1161,9 @@ static void cec_pin_adap_status(struct cec_adapter *adap,
 {
 	struct cec_pin *pin = adap->pin;
 
-	seq_printf(file, "state:   %s\n", states[pin->state].name);
-	seq_printf(file, "tx_bit:  %d\n", pin->tx_bit);
-	seq_printf(file, "rx_bit:  %d\n", pin->rx_bit);
+	seq_printf(file, "state: %s\n", states[pin->state].name);
+	seq_printf(file, "tx_bit: %d\n", pin->tx_bit);
+	seq_printf(file, "rx_bit: %d\n", pin->rx_bit);
 	seq_printf(file, "cec pin: %d\n", pin->ops->read(adap));
 	seq_printf(file, "irq failed: %d\n", pin->enable_irq_failed);
 	if (pin->timer_100ms_overruns) {
@@ -1157,11 +1176,44 @@ static void cec_pin_adap_status(struct cec_adapter *adap,
 		seq_printf(file, "avg timer overrun: %u usecs\n",
 			   pin->timer_sum_overrun / pin->timer_100ms_overruns);
 	}
+	if (pin->rx_start_bit_low_too_short_cnt)
+		seq_printf(file,
+			   "rx start bit low too short: %u (delta %u, ts %llu)\n",
+			   pin->rx_start_bit_low_too_short_cnt,
+			   pin->rx_start_bit_low_too_short_delta,
+			   pin->rx_start_bit_low_too_short_ts);
+	if (pin->rx_start_bit_too_short_cnt)
+		seq_printf(file,
+			   "rx start bit too short: %u (delta %u, ts %llu)\n",
+			   pin->rx_start_bit_too_short_cnt,
+			   pin->rx_start_bit_too_short_delta,
+			   pin->rx_start_bit_too_short_ts);
+	if (pin->rx_start_bit_too_long_cnt)
+		seq_printf(file, "rx start bit too long: %u\n",
+			   pin->rx_start_bit_too_long_cnt);
+	if (pin->rx_data_bit_too_short_cnt)
+		seq_printf(file,
+			   "rx data bit too short: %u (delta %u, ts %llu)\n",
+			   pin->rx_data_bit_too_short_cnt,
+			   pin->rx_data_bit_too_short_delta,
+			   pin->rx_data_bit_too_short_ts);
+	if (pin->rx_data_bit_too_long_cnt)
+		seq_printf(file, "rx data bit too long: %u\n",
+			   pin->rx_data_bit_too_long_cnt);
+	seq_printf(file, "rx initiated low drive: %u\n", pin->rx_low_drive_cnt);
+	seq_printf(file, "tx detected low drive: %u\n", pin->tx_low_drive_cnt);
 	pin->timer_cnt = 0;
 	pin->timer_100ms_overruns = 0;
 	pin->timer_300ms_overruns = 0;
 	pin->timer_max_overrun = 0;
 	pin->timer_sum_overrun = 0;
+	pin->rx_start_bit_low_too_short_cnt = 0;
+	pin->rx_start_bit_too_short_cnt = 0;
+	pin->rx_start_bit_too_long_cnt = 0;
+	pin->rx_data_bit_too_short_cnt = 0;
+	pin->rx_data_bit_too_long_cnt = 0;
+	pin->rx_low_drive_cnt = 0;
+	pin->tx_low_drive_cnt = 0;
 	if (pin->ops->status)
 		pin->ops->status(adap, file);
 }
-- 
2.16.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCHv2 0/7] cec: add error injection support
  2018-03-05 13:51 [PATCHv2 0/7] cec: add error injection support Hans Verkuil
                   ` (6 preceding siblings ...)
  2018-03-05 13:51 ` [PATCHv2 7/7] cec-pin: improve status log Hans Verkuil
@ 2018-03-05 17:40 ` Wolfram Sang
  7 siblings, 0 replies; 14+ messages in thread
From: Wolfram Sang @ 2018-03-05 17:40 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: linux-media, Maxime Ripard, dri-devel

[-- Attachment #1: Type: text/plain, Size: 298 bytes --]

Hans,

> Special thanks go to Wolfram Sang since his i2c error injection
> presentation at the Embedded Linux Conference Europe 2017 inspired
> me to switch to debugfs for this instead of using ioctls.

You are very welcome! I am glad the presentation did inspire you.

Happy hacking,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCHv2 5/7] cec-pin: add error injection support
  2018-03-05 13:51 ` [PATCHv2 5/7] cec-pin: add error injection support Hans Verkuil
@ 2018-03-06 22:48   ` Hans Verkuil
  2018-03-07 14:17     ` [PATCHv2.1 " Hans Verkuil
  0 siblings, 1 reply; 14+ messages in thread
From: Hans Verkuil @ 2018-03-06 22:48 UTC (permalink / raw)
  To: linux-media; +Cc: Wolfram Sang, Maxime Ripard, dri-devel, Hans Verkuil

On 05/03/18 14:51, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> Implement all the error injection commands.
> 
> The state machine gets new states for the various error situations,
> helper functions are added to detect whether an error injection is
> active and the actual error injections are implemented.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---
>  drivers/media/cec/cec-pin-priv.h |  38 ++-
>  drivers/media/cec/cec-pin.c      | 542 +++++++++++++++++++++++++++++++++++----
>  2 files changed, 521 insertions(+), 59 deletions(-)
> 
> diff --git a/drivers/media/cec/cec-pin-priv.h b/drivers/media/cec/cec-pin-priv.h
> index 779384f18689..c9349f68e554 100644
> --- a/drivers/media/cec/cec-pin-priv.h
> +++ b/drivers/media/cec/cec-pin-priv.h

<snip>

> +static bool tx_error_inj(struct cec_pin *pin, unsigned int mode_offset,
> +			 int arg_idx, u8 *arg)
> +{
> +#ifdef CONFIG_CEC_PIN_ERROR_INJ
> +	u16 cmd = cec_pin_tx_error_inj(pin);
> +	u64 e = pin->error_inj[cmd];
> +	unsigned int mode = (e >> mode_offset) & CEC_ERROR_INJ_MODE_MASK;
> +
> +	if (arg_idx) {

Oops, this should be arg_idx >= 0. It's -1 if there is no argument associated
with the error injection command.

> +		u8 pos = pin->error_inj_args[cmd][arg_idx];
> +
> +		if (arg)
> +			*arg = pos;
> +		else if (pos != pin->tx_bit)
> +			return false;
> +	}
> +
> +	switch (mode) {
> +	case CEC_ERROR_INJ_MODE_ONCE:
> +		pin->error_inj[cmd] &= ~(CEC_ERROR_INJ_MODE_MASK << mode_offset);
> +		return true;
> +	case CEC_ERROR_INJ_MODE_ALWAYS:
> +		return true;
> +	case CEC_ERROR_INJ_MODE_TOGGLE:
> +		return pin->tx_toggle;
> +	default:
> +		return false;
> +	}
> +#else
> +	return false;
> +#endif
> +}

<snip>

> +		case EOM_BIT:
> +			v = pin->tx_bit / 10 ==
> +				pin->tx_msg.len + pin->tx_extra_bytes - 1;
> +			if (pin->tx_msg.len > 1 && tx_early_eom(pin)) {

tx_early_eom should only be called for the second-to-last byte.

> +				/* Error injection: set EOM one byte early */
> +				v = pin->tx_bit / 10 ==
> +					pin->tx_msg.len + pin->tx_extra_bytes - 2;
> +				pin->tx_post_eom = true;
> +			}
> +			if (tx_no_eom(pin)) {

Ditto for tx_no_eom: only call for the last byte.

Otherwise for mode 'once' this error injection command will be cleared
before the end of the message is reached.

> +				/* Error injection: no EOM */
> +				v = false;
> +			}
>  			pin->state = v ? CEC_ST_TX_DATA_BIT_1_LOW :
> -				CEC_ST_TX_DATA_BIT_0_LOW;
> +					 CEC_ST_TX_DATA_BIT_0_LOW;
>  			break;

Regards,

	Hans

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCHv2.1 5/7] cec-pin: add error injection support
  2018-03-06 22:48   ` Hans Verkuil
@ 2018-03-07 14:17     ` Hans Verkuil
  0 siblings, 0 replies; 14+ messages in thread
From: Hans Verkuil @ 2018-03-07 14:17 UTC (permalink / raw)
  To: linux-media; +Cc: Wolfram Sang, Maxime Ripard, dri-devel, Hans Verkuil

Implement all the error injection commands.

The state machine gets new states for the various error situations,
helper functions are added to detect whether an error injection is
active and the actual error injections are implemented.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
Changes since v2:
- Fix wrong arg_idx condition (must be >= 0)
- Fix tx_no_eom and tx_early_eom: don't call this for every byte, only
  for the byte it applies to. Otherwise command will be cleared in the 'once'
  mode.
---
 drivers/media/cec/cec-pin-priv.h |  38 ++-
 drivers/media/cec/cec-pin.c      | 545 +++++++++++++++++++++++++++++++++++----
 2 files changed, 524 insertions(+), 59 deletions(-)

diff --git a/drivers/media/cec/cec-pin-priv.h b/drivers/media/cec/cec-pin-priv.h
index 779384f18689..c9349f68e554 100644
--- a/drivers/media/cec/cec-pin-priv.h
+++ b/drivers/media/cec/cec-pin-priv.h
@@ -28,14 +28,30 @@ enum cec_pin_state {
 	CEC_ST_TX_START_BIT_LOW,
 	/* Drive CEC high for the start bit */
 	CEC_ST_TX_START_BIT_HIGH,
+	/* Generate a start bit period that is too short */
+	CEC_ST_TX_START_BIT_HIGH_SHORT,
+	/* Generate a start bit period that is too long */
+	CEC_ST_TX_START_BIT_HIGH_LONG,
+	/* Drive CEC low for the start bit using the custom timing */
+	CEC_ST_TX_START_BIT_LOW_CUSTOM,
+	/* Drive CEC high for the start bit using the custom timing */
+	CEC_ST_TX_START_BIT_HIGH_CUSTOM,
 	/* Drive CEC low for the 0 bit */
 	CEC_ST_TX_DATA_BIT_0_LOW,
 	/* Drive CEC high for the 0 bit */
 	CEC_ST_TX_DATA_BIT_0_HIGH,
+	/* Generate a bit period that is too short */
+	CEC_ST_TX_DATA_BIT_0_HIGH_SHORT,
+	/* Generate a bit period that is too long */
+	CEC_ST_TX_DATA_BIT_0_HIGH_LONG,
 	/* Drive CEC low for the 1 bit */
 	CEC_ST_TX_DATA_BIT_1_LOW,
 	/* Drive CEC high for the 1 bit */
 	CEC_ST_TX_DATA_BIT_1_HIGH,
+	/* Generate a bit period that is too short */
+	CEC_ST_TX_DATA_BIT_1_HIGH_SHORT,
+	/* Generate a bit period that is too long */
+	CEC_ST_TX_DATA_BIT_1_HIGH_LONG,
 	/*
 	 * Wait for start of sample time to check for Ack bit or first
 	 * four initiator bits to check for Arbitration Lost.
@@ -43,6 +59,20 @@ enum cec_pin_state {
 	CEC_ST_TX_DATA_BIT_1_HIGH_PRE_SAMPLE,
 	/* Wait for end of bit period after sampling */
 	CEC_ST_TX_DATA_BIT_1_HIGH_POST_SAMPLE,
+	/* Generate a bit period that is too short */
+	CEC_ST_TX_DATA_BIT_1_HIGH_POST_SAMPLE_SHORT,
+	/* Generate a bit period that is too long */
+	CEC_ST_TX_DATA_BIT_1_HIGH_POST_SAMPLE_LONG,
+	/* Drive CEC low for a data bit using the custom timing */
+	CEC_ST_TX_DATA_BIT_LOW_CUSTOM,
+	/* Drive CEC high for a data bit using the custom timing */
+	CEC_ST_TX_DATA_BIT_HIGH_CUSTOM,
+	/* Drive CEC low for a standalone pulse using the custom timing */
+	CEC_ST_TX_PULSE_LOW_CUSTOM,
+	/* Drive CEC high for a standalone pulse using the custom timing */
+	CEC_ST_TX_PULSE_HIGH_CUSTOM,
+	/* Start low drive */
+	CEC_ST_TX_LOW_DRIVE,

 	/* Rx states */

@@ -54,8 +84,8 @@ enum cec_pin_state {
 	CEC_ST_RX_DATA_SAMPLE,
 	/* Wait for earliest end of bit period after sampling */
 	CEC_ST_RX_DATA_POST_SAMPLE,
-	/* Wait for CEC to go high (i.e. end of bit period */
-	CEC_ST_RX_DATA_HIGH,
+	/* Wait for CEC to go low (i.e. end of bit period) */
+	CEC_ST_RX_DATA_WAIT_FOR_LOW,
 	/* Drive CEC low to send 0 Ack bit */
 	CEC_ST_RX_ACK_LOW,
 	/* End of 0 Ack time, wait for earliest end of bit period */
@@ -64,9 +94,9 @@ enum cec_pin_state {
 	CEC_ST_RX_ACK_HIGH_POST,
 	/* Wait for earliest end of bit period and end of message */
 	CEC_ST_RX_ACK_FINISH,
-
 	/* Start low drive */
-	CEC_ST_LOW_DRIVE,
+	CEC_ST_RX_LOW_DRIVE,
+
 	/* Monitor pin using interrupts */
 	CEC_ST_RX_IRQ,

diff --git a/drivers/media/cec/cec-pin.c b/drivers/media/cec/cec-pin.c
index 7920ea1c940b..c450145ef67a 100644
--- a/drivers/media/cec/cec-pin.c
+++ b/drivers/media/cec/cec-pin.c
@@ -39,11 +39,29 @@
 #define CEC_TIM_IDLE_SAMPLE		1000
 /* when processing the start bit, sample twice per millisecond */
 #define CEC_TIM_START_BIT_SAMPLE	500
-/* when polling for a state change, sample once every 50 micoseconds */
+/* when polling for a state change, sample once every 50 microseconds */
 #define CEC_TIM_SAMPLE			50

 #define CEC_TIM_LOW_DRIVE_ERROR		(1.5 * CEC_TIM_DATA_BIT_TOTAL)

+/*
+ * Total data bit time that is too short/long for a valid bit,
+ * used for error injection.
+ */
+#define CEC_TIM_DATA_BIT_TOTAL_SHORT	1800
+#define CEC_TIM_DATA_BIT_TOTAL_LONG	2900
+
+/*
+ * Total start bit time that is too short/long for a valid bit,
+ * used for error injection.
+ */
+#define CEC_TIM_START_BIT_TOTAL_SHORT	4100
+#define CEC_TIM_START_BIT_TOTAL_LONG	5000
+
+/* Data bits are 0-7, EOM is bit 8 and ACK is bit 9 */
+#define EOM_BIT				8
+#define ACK_BIT				9
+
 struct cec_state {
 	const char * const name;
 	unsigned int usecs;
@@ -56,17 +74,32 @@ static const struct cec_state states[CEC_PIN_STATES] = {
 	{ "Tx Wait for High",	   CEC_TIM_IDLE_SAMPLE },
 	{ "Tx Start Bit Low",	   CEC_TIM_START_BIT_LOW },
 	{ "Tx Start Bit High",	   CEC_TIM_START_BIT_TOTAL - CEC_TIM_START_BIT_LOW },
+	{ "Tx Start Bit High Short", CEC_TIM_START_BIT_TOTAL_SHORT - CEC_TIM_START_BIT_LOW },
+	{ "Tx Start Bit High Long", CEC_TIM_START_BIT_TOTAL_LONG - CEC_TIM_START_BIT_LOW },
+	{ "Tx Start Bit Low Custom", 0 },
+	{ "Tx Start Bit High Custom", 0 },
 	{ "Tx Data 0 Low",	   CEC_TIM_DATA_BIT_0_LOW },
 	{ "Tx Data 0 High",	   CEC_TIM_DATA_BIT_TOTAL - CEC_TIM_DATA_BIT_0_LOW },
+	{ "Tx Data 0 High Short",  CEC_TIM_DATA_BIT_TOTAL_SHORT - CEC_TIM_DATA_BIT_0_LOW },
+	{ "Tx Data 0 High Long",   CEC_TIM_DATA_BIT_TOTAL_LONG - CEC_TIM_DATA_BIT_0_LOW },
 	{ "Tx Data 1 Low",	   CEC_TIM_DATA_BIT_1_LOW },
 	{ "Tx Data 1 High",	   CEC_TIM_DATA_BIT_TOTAL - CEC_TIM_DATA_BIT_1_LOW },
-	{ "Tx Data 1 Pre Sample",  CEC_TIM_DATA_BIT_SAMPLE - CEC_TIM_DATA_BIT_1_LOW },
-	{ "Tx Data 1 Post Sample", CEC_TIM_DATA_BIT_TOTAL - CEC_TIM_DATA_BIT_SAMPLE },
+	{ "Tx Data 1 High Short",  CEC_TIM_DATA_BIT_TOTAL_SHORT - CEC_TIM_DATA_BIT_1_LOW },
+	{ "Tx Data 1 High Long",   CEC_TIM_DATA_BIT_TOTAL_LONG - CEC_TIM_DATA_BIT_1_LOW },
+	{ "Tx Data 1 High Pre Sample", CEC_TIM_DATA_BIT_SAMPLE - CEC_TIM_DATA_BIT_1_LOW },
+	{ "Tx Data 1 High Post Sample", CEC_TIM_DATA_BIT_TOTAL - CEC_TIM_DATA_BIT_SAMPLE },
+	{ "Tx Data 1 High Post Sample Short", CEC_TIM_DATA_BIT_TOTAL_SHORT - CEC_TIM_DATA_BIT_SAMPLE },
+	{ "Tx Data 1 High Post Sample Long", CEC_TIM_DATA_BIT_TOTAL_LONG - CEC_TIM_DATA_BIT_SAMPLE },
+	{ "Tx Data Bit Low Custom", 0 },
+	{ "Tx Data Bit High Custom", 0 },
+	{ "Tx Pulse Low Custom",   0 },
+	{ "Tx Pulse High Custom",  0 },
+	{ "Tx Low Drive",	   CEC_TIM_LOW_DRIVE_ERROR },
 	{ "Rx Start Bit Low",	   CEC_TIM_SAMPLE },
 	{ "Rx Start Bit High",	   CEC_TIM_SAMPLE },
 	{ "Rx Data Sample",	   CEC_TIM_DATA_BIT_SAMPLE },
 	{ "Rx Data Post Sample",   CEC_TIM_DATA_BIT_HIGH - CEC_TIM_DATA_BIT_SAMPLE },
-	{ "Rx Data High",	   CEC_TIM_SAMPLE },
+	{ "Rx Data Wait for Low",  CEC_TIM_SAMPLE },
 	{ "Rx Ack Low",		   CEC_TIM_DATA_BIT_0_LOW },
 	{ "Rx Ack Low Post",	   CEC_TIM_DATA_BIT_HIGH - CEC_TIM_DATA_BIT_0_LOW },
 	{ "Rx Ack High Post",	   CEC_TIM_DATA_BIT_HIGH },
@@ -111,6 +144,170 @@ static bool cec_pin_high(struct cec_pin *pin)
 	return cec_pin_read(pin);
 }

+static bool rx_error_inj(struct cec_pin *pin, unsigned int mode_offset,
+			 int arg_idx, u8 *arg)
+{
+#ifdef CONFIG_CEC_PIN_ERROR_INJ
+	u16 cmd = cec_pin_rx_error_inj(pin);
+	u64 e = pin->error_inj[cmd];
+	unsigned int mode = (e >> mode_offset) & CEC_ERROR_INJ_MODE_MASK;
+
+	if (arg_idx >= 0) {
+		u8 pos = pin->error_inj_args[cmd][arg_idx];
+
+		if (arg)
+			*arg = pos;
+		else if (pos != pin->rx_bit)
+			return false;
+	}
+
+	switch (mode) {
+	case CEC_ERROR_INJ_MODE_ONCE:
+		pin->error_inj[cmd] &= ~(CEC_ERROR_INJ_MODE_MASK << mode_offset);
+		return true;
+	case CEC_ERROR_INJ_MODE_ALWAYS:
+		return true;
+	case CEC_ERROR_INJ_MODE_TOGGLE:
+		return pin->rx_toggle;
+	default:
+		return false;
+	}
+#else
+	return false;
+#endif
+}
+
+static bool rx_nack(struct cec_pin *pin)
+{
+	return rx_error_inj(pin, CEC_ERROR_INJ_RX_NACK_OFFSET, -1, NULL);
+}
+
+static bool rx_low_drive(struct cec_pin *pin)
+{
+	return rx_error_inj(pin, CEC_ERROR_INJ_RX_LOW_DRIVE_OFFSET,
+			    CEC_ERROR_INJ_RX_LOW_DRIVE_ARG_IDX, NULL);
+}
+
+static bool rx_add_byte(struct cec_pin *pin)
+{
+	return rx_error_inj(pin, CEC_ERROR_INJ_RX_ADD_BYTE_OFFSET, -1, NULL);
+}
+
+static bool rx_remove_byte(struct cec_pin *pin)
+{
+	return rx_error_inj(pin, CEC_ERROR_INJ_RX_REMOVE_BYTE_OFFSET, -1, NULL);
+}
+
+static bool rx_arb_lost(struct cec_pin *pin, u8 *poll)
+{
+	return pin->tx_msg.len == 0 &&
+		rx_error_inj(pin, CEC_ERROR_INJ_RX_ARB_LOST_OFFSET,
+			     CEC_ERROR_INJ_RX_ARB_LOST_ARG_IDX, poll);
+}
+
+static bool tx_error_inj(struct cec_pin *pin, unsigned int mode_offset,
+			 int arg_idx, u8 *arg)
+{
+#ifdef CONFIG_CEC_PIN_ERROR_INJ
+	u16 cmd = cec_pin_tx_error_inj(pin);
+	u64 e = pin->error_inj[cmd];
+	unsigned int mode = (e >> mode_offset) & CEC_ERROR_INJ_MODE_MASK;
+
+	if (arg_idx >= 0) {
+		u8 pos = pin->error_inj_args[cmd][arg_idx];
+
+		if (arg)
+			*arg = pos;
+		else if (pos != pin->tx_bit)
+			return false;
+	}
+
+	switch (mode) {
+	case CEC_ERROR_INJ_MODE_ONCE:
+		pin->error_inj[cmd] &= ~(CEC_ERROR_INJ_MODE_MASK << mode_offset);
+		return true;
+	case CEC_ERROR_INJ_MODE_ALWAYS:
+		return true;
+	case CEC_ERROR_INJ_MODE_TOGGLE:
+		return pin->tx_toggle;
+	default:
+		return false;
+	}
+#else
+	return false;
+#endif
+}
+
+static bool tx_no_eom(struct cec_pin *pin)
+{
+	return tx_error_inj(pin, CEC_ERROR_INJ_TX_NO_EOM_OFFSET, -1, NULL);
+}
+
+static bool tx_early_eom(struct cec_pin *pin)
+{
+	return tx_error_inj(pin, CEC_ERROR_INJ_TX_EARLY_EOM_OFFSET, -1, NULL);
+}
+
+static bool tx_short_bit(struct cec_pin *pin)
+{
+	return tx_error_inj(pin, CEC_ERROR_INJ_TX_SHORT_BIT_OFFSET,
+			    CEC_ERROR_INJ_TX_SHORT_BIT_ARG_IDX, NULL);
+}
+
+static bool tx_long_bit(struct cec_pin *pin)
+{
+	return tx_error_inj(pin, CEC_ERROR_INJ_TX_LONG_BIT_OFFSET,
+			    CEC_ERROR_INJ_TX_LONG_BIT_ARG_IDX, NULL);
+}
+
+static bool tx_custom_bit(struct cec_pin *pin)
+{
+	return tx_error_inj(pin, CEC_ERROR_INJ_TX_CUSTOM_BIT_OFFSET,
+			    CEC_ERROR_INJ_TX_CUSTOM_BIT_ARG_IDX, NULL);
+}
+
+static bool tx_short_start(struct cec_pin *pin)
+{
+	return tx_error_inj(pin, CEC_ERROR_INJ_TX_SHORT_START_OFFSET, -1, NULL);
+}
+
+static bool tx_long_start(struct cec_pin *pin)
+{
+	return tx_error_inj(pin, CEC_ERROR_INJ_TX_LONG_START_OFFSET, -1, NULL);
+}
+
+static bool tx_custom_start(struct cec_pin *pin)
+{
+	return tx_error_inj(pin, CEC_ERROR_INJ_TX_CUSTOM_START_OFFSET, -1, NULL);
+}
+
+static bool tx_last_bit(struct cec_pin *pin)
+{
+	return tx_error_inj(pin, CEC_ERROR_INJ_TX_LAST_BIT_OFFSET,
+			    CEC_ERROR_INJ_TX_LAST_BIT_ARG_IDX, NULL);
+}
+
+static u8 tx_add_bytes(struct cec_pin *pin)
+{
+	u8 bytes;
+
+	if (tx_error_inj(pin, CEC_ERROR_INJ_TX_ADD_BYTES_OFFSET,
+			 CEC_ERROR_INJ_TX_ADD_BYTES_ARG_IDX, &bytes))
+		return bytes;
+	return 0;
+}
+
+static bool tx_remove_byte(struct cec_pin *pin)
+{
+	return tx_error_inj(pin, CEC_ERROR_INJ_TX_REMOVE_BYTE_OFFSET, -1, NULL);
+}
+
+static bool tx_low_drive(struct cec_pin *pin)
+{
+	return tx_error_inj(pin, CEC_ERROR_INJ_TX_LOW_DRIVE_OFFSET,
+			    CEC_ERROR_INJ_TX_LOW_DRIVE_ARG_IDX, NULL);
+}
+
 static void cec_pin_to_idle(struct cec_pin *pin)
 {
 	/*
@@ -120,8 +317,16 @@ static void cec_pin_to_idle(struct cec_pin *pin)
 	pin->rx_bit = pin->tx_bit = 0;
 	pin->rx_msg.len = 0;
 	memset(pin->rx_msg.msg, 0, sizeof(pin->rx_msg.msg));
-	pin->state = CEC_ST_IDLE;
 	pin->ts = ns_to_ktime(0);
+	pin->tx_generated_poll = false;
+	pin->tx_post_eom = false;
+	if (pin->state >= CEC_ST_TX_WAIT &&
+	    pin->state <= CEC_ST_TX_LOW_DRIVE)
+		pin->tx_toggle ^= 1;
+	if (pin->state >= CEC_ST_RX_START_BIT_LOW &&
+	    pin->state <= CEC_ST_RX_LOW_DRIVE)
+		pin->rx_toggle ^= 1;
+	pin->state = CEC_ST_IDLE;
 }

 /*
@@ -162,42 +367,107 @@ static void cec_pin_tx_states(struct cec_pin *pin, ktime_t ts)
 		break;

 	case CEC_ST_TX_START_BIT_LOW:
-		pin->state = CEC_ST_TX_START_BIT_HIGH;
+		if (tx_short_start(pin)) {
+			/*
+			 * Error Injection: send an invalid (too short)
+			 * start pulse.
+			 */
+			pin->state = CEC_ST_TX_START_BIT_HIGH_SHORT;
+		} else if (tx_long_start(pin)) {
+			/*
+			 * Error Injection: send an invalid (too long)
+			 * start pulse.
+			 */
+			pin->state = CEC_ST_TX_START_BIT_HIGH_LONG;
+		} else {
+			pin->state = CEC_ST_TX_START_BIT_HIGH;
+		}
+		/* Generate start bit */
+		cec_pin_high(pin);
+		break;
+
+	case CEC_ST_TX_START_BIT_LOW_CUSTOM:
+		pin->state = CEC_ST_TX_START_BIT_HIGH_CUSTOM;
 		/* Generate start bit */
 		cec_pin_high(pin);
 		break;

 	case CEC_ST_TX_DATA_BIT_1_HIGH_POST_SAMPLE:
-		/* If the read value is 1, then all is OK */
-		if (!cec_pin_read(pin)) {
+	case CEC_ST_TX_DATA_BIT_1_HIGH_POST_SAMPLE_SHORT:
+	case CEC_ST_TX_DATA_BIT_1_HIGH_POST_SAMPLE_LONG:
+		if (pin->tx_nacked) {
+			cec_pin_to_idle(pin);
+			pin->tx_msg.len = 0;
+			if (pin->tx_generated_poll)
+				break;
+			pin->work_tx_ts = ts;
+			pin->work_tx_status = CEC_TX_STATUS_NACK;
+			wake_up_interruptible(&pin->kthread_waitq);
+			break;
+		}
+		/* fall through */
+	case CEC_ST_TX_DATA_BIT_0_HIGH:
+	case CEC_ST_TX_DATA_BIT_0_HIGH_SHORT:
+	case CEC_ST_TX_DATA_BIT_0_HIGH_LONG:
+	case CEC_ST_TX_DATA_BIT_1_HIGH:
+	case CEC_ST_TX_DATA_BIT_1_HIGH_SHORT:
+	case CEC_ST_TX_DATA_BIT_1_HIGH_LONG:
+		/*
+		 * If the read value is 1, then all is OK, otherwise we have a
+		 * low drive condition.
+		 *
+		 * Special case: when we generate a poll message due to an
+		 * Arbitration Lost error injection, then ignore this since
+		 * the pin can actually be low in that case.
+		 */
+		if (!cec_pin_read(pin) && !pin->tx_generated_poll) {
 			/*
 			 * It's 0, so someone detected an error and pulled the
 			 * line low for 1.5 times the nominal bit period.
 			 */
 			pin->tx_msg.len = 0;
+			pin->state = CEC_ST_TX_WAIT_FOR_HIGH;
 			pin->work_tx_ts = ts;
 			pin->work_tx_status = CEC_TX_STATUS_LOW_DRIVE;
-			pin->state = CEC_ST_TX_WAIT_FOR_HIGH;
 			wake_up_interruptible(&pin->kthread_waitq);
 			break;
 		}
-		if (pin->tx_nacked) {
+		/* fall through */
+	case CEC_ST_TX_DATA_BIT_HIGH_CUSTOM:
+		if (tx_last_bit(pin)) {
+			/* Error Injection: just stop sending after this bit */
 			cec_pin_to_idle(pin);
 			pin->tx_msg.len = 0;
+			if (pin->tx_generated_poll)
+				break;
 			pin->work_tx_ts = ts;
-			pin->work_tx_status = CEC_TX_STATUS_NACK;
+			pin->work_tx_status = CEC_TX_STATUS_OK;
 			wake_up_interruptible(&pin->kthread_waitq);
 			break;
 		}
-		/* fall through */
-	case CEC_ST_TX_DATA_BIT_0_HIGH:
-	case CEC_ST_TX_DATA_BIT_1_HIGH:
 		pin->tx_bit++;
 		/* fall through */
 	case CEC_ST_TX_START_BIT_HIGH:
-		if (pin->tx_bit / 10 >= pin->tx_msg.len) {
+	case CEC_ST_TX_START_BIT_HIGH_SHORT:
+	case CEC_ST_TX_START_BIT_HIGH_LONG:
+	case CEC_ST_TX_START_BIT_HIGH_CUSTOM:
+		if (tx_low_drive(pin)) {
+			/* Error injection: go to low drive */
+			cec_pin_low(pin);
+			pin->state = CEC_ST_TX_LOW_DRIVE;
+			pin->tx_msg.len = 0;
+			if (pin->tx_generated_poll)
+				break;
+			pin->work_tx_ts = ts;
+			pin->work_tx_status = CEC_TX_STATUS_LOW_DRIVE;
+			wake_up_interruptible(&pin->kthread_waitq);
+			break;
+		}
+		if (pin->tx_bit / 10 >= pin->tx_msg.len + pin->tx_extra_bytes) {
 			cec_pin_to_idle(pin);
 			pin->tx_msg.len = 0;
+			if (pin->tx_generated_poll)
+				break;
 			pin->work_tx_ts = ts;
 			pin->work_tx_status = CEC_TX_STATUS_OK;
 			wake_up_interruptible(&pin->kthread_waitq);
@@ -205,39 +475,82 @@ static void cec_pin_tx_states(struct cec_pin *pin, ktime_t ts)
 		}

 		switch (pin->tx_bit % 10) {
-		default:
-			v = pin->tx_msg.msg[pin->tx_bit / 10] &
-				(1 << (7 - (pin->tx_bit % 10)));
+		default: {
+			/*
+			 * In the CEC_ERROR_INJ_TX_ADD_BYTES case we transmit
+			 * extra bytes, so pin->tx_bit / 10 can become >= 16.
+			 * Generate bit values for those extra bytes instead
+			 * of reading them from the transmit buffer.
+			 */
+			unsigned int idx = (pin->tx_bit / 10);
+			u8 val = idx;
+
+			if (idx < pin->tx_msg.len)
+				val = pin->tx_msg.msg[idx];
+			v = val & (1 << (7 - (pin->tx_bit % 10)));
+
 			pin->state = v ? CEC_ST_TX_DATA_BIT_1_LOW :
-				CEC_ST_TX_DATA_BIT_0_LOW;
+					 CEC_ST_TX_DATA_BIT_0_LOW;
 			break;
-		case 8:
-			v = pin->tx_bit / 10 == pin->tx_msg.len - 1;
+		}
+		case EOM_BIT: {
+			unsigned int tot_len = pin->tx_msg.len +
+					       pin->tx_extra_bytes;
+			unsigned int tx_byte_idx = pin->tx_bit / 10;
+
+			v = !pin->tx_post_eom && tx_byte_idx == tot_len - 1;
+			if (tot_len > 1 && tx_byte_idx == tot_len - 2 &&
+			    tx_early_eom(pin)) {
+				/* Error injection: set EOM one byte early */
+				v = true;
+				pin->tx_post_eom = true;
+			} else if (v && tx_no_eom(pin)) {
+				/* Error injection: no EOM */
+				v = false;
+			}
 			pin->state = v ? CEC_ST_TX_DATA_BIT_1_LOW :
-				CEC_ST_TX_DATA_BIT_0_LOW;
+					 CEC_ST_TX_DATA_BIT_0_LOW;
 			break;
-		case 9:
+		}
+		case ACK_BIT:
 			pin->state = CEC_ST_TX_DATA_BIT_1_LOW;
 			break;
 		}
+		if (tx_custom_bit(pin))
+			pin->state = CEC_ST_TX_DATA_BIT_LOW_CUSTOM;
 		cec_pin_low(pin);
 		break;

 	case CEC_ST_TX_DATA_BIT_0_LOW:
 	case CEC_ST_TX_DATA_BIT_1_LOW:
 		v = pin->state == CEC_ST_TX_DATA_BIT_1_LOW;
-		pin->state = v ? CEC_ST_TX_DATA_BIT_1_HIGH :
-			CEC_ST_TX_DATA_BIT_0_HIGH;
-		is_ack_bit = pin->tx_bit % 10 == 9;
-		if (v && (pin->tx_bit < 4 || is_ack_bit))
+		is_ack_bit = pin->tx_bit % 10 == ACK_BIT;
+		if (v && (pin->tx_bit < 4 || is_ack_bit)) {
 			pin->state = CEC_ST_TX_DATA_BIT_1_HIGH_PRE_SAMPLE;
+		} else if (!is_ack_bit && tx_short_bit(pin)) {
+			/* Error Injection: send an invalid (too short) bit */
+			pin->state = v ? CEC_ST_TX_DATA_BIT_1_HIGH_SHORT :
+					 CEC_ST_TX_DATA_BIT_0_HIGH_SHORT;
+		} else if (!is_ack_bit && tx_long_bit(pin)) {
+			/* Error Injection: send an invalid (too long) bit */
+			pin->state = v ? CEC_ST_TX_DATA_BIT_1_HIGH_LONG :
+					 CEC_ST_TX_DATA_BIT_0_HIGH_LONG;
+		} else {
+			pin->state = v ? CEC_ST_TX_DATA_BIT_1_HIGH :
+					 CEC_ST_TX_DATA_BIT_0_HIGH;
+		}
+		cec_pin_high(pin);
+		break;
+
+	case CEC_ST_TX_DATA_BIT_LOW_CUSTOM:
+		pin->state = CEC_ST_TX_DATA_BIT_HIGH_CUSTOM;
 		cec_pin_high(pin);
 		break;

 	case CEC_ST_TX_DATA_BIT_1_HIGH_PRE_SAMPLE:
 		/* Read the CEC value at the sample time */
 		v = cec_pin_read(pin);
-		is_ack_bit = pin->tx_bit % 10 == 9;
+		is_ack_bit = pin->tx_bit % 10 == ACK_BIT;
 		/*
 		 * If v == 0 and we're within the first 4 bits
 		 * of the initiator, then someone else started
@@ -246,7 +559,7 @@ static void cec_pin_tx_states(struct cec_pin *pin, ktime_t ts)
 		 * transmitter has more leading 0 bits in the
 		 * initiator).
 		 */
-		if (!v && !is_ack_bit) {
+		if (!v && !is_ack_bit && !pin->tx_generated_poll) {
 			pin->tx_msg.len = 0;
 			pin->work_tx_ts = ts;
 			pin->work_tx_status = CEC_TX_STATUS_ARB_LOST;
@@ -255,18 +568,27 @@ static void cec_pin_tx_states(struct cec_pin *pin, ktime_t ts)
 			pin->tx_bit = 0;
 			memset(pin->rx_msg.msg, 0, sizeof(pin->rx_msg.msg));
 			pin->rx_msg.msg[0] = pin->tx_msg.msg[0];
-			pin->rx_msg.msg[0] &= ~(1 << (7 - pin->rx_bit));
+			pin->rx_msg.msg[0] &= (0xff << (8 - pin->rx_bit));
 			pin->rx_msg.len = 0;
+			pin->ts = ktime_sub_us(ts, CEC_TIM_DATA_BIT_SAMPLE);
 			pin->state = CEC_ST_RX_DATA_POST_SAMPLE;
 			pin->rx_bit++;
 			break;
 		}
 		pin->state = CEC_ST_TX_DATA_BIT_1_HIGH_POST_SAMPLE;
+		if (!is_ack_bit && tx_short_bit(pin)) {
+			/* Error Injection: send an invalid (too short) bit */
+			pin->state = CEC_ST_TX_DATA_BIT_1_HIGH_POST_SAMPLE_SHORT;
+		} else if (!is_ack_bit && tx_long_bit(pin)) {
+			/* Error Injection: send an invalid (too long) bit */
+			pin->state = CEC_ST_TX_DATA_BIT_1_HIGH_POST_SAMPLE_LONG;
+		}
 		if (!is_ack_bit)
 			break;
 		/* Was the message ACKed? */
 		ack = cec_msg_is_broadcast(&pin->tx_msg) ? v : !v;
-		if (!ack) {
+		if (!ack && !pin->tx_ignore_nack_until_eom &&
+		    pin->tx_bit / 10 < pin->tx_msg.len && !pin->tx_post_eom) {
 			/*
 			 * Note: the CEC spec is ambiguous regarding
 			 * what action to take when a NACK appears
@@ -283,6 +605,15 @@ static void cec_pin_tx_states(struct cec_pin *pin, ktime_t ts)
 		}
 		break;

+	case CEC_ST_TX_PULSE_LOW_CUSTOM:
+		cec_pin_high(pin);
+		pin->state = CEC_ST_TX_PULSE_HIGH_CUSTOM;
+		break;
+
+	case CEC_ST_TX_PULSE_HIGH_CUSTOM:
+		cec_pin_to_idle(pin);
+		break;
+
 	default:
 		break;
 	}
@@ -310,6 +641,7 @@ static void cec_pin_rx_states(struct cec_pin *pin, ktime_t ts)
 	bool ack;
 	bool bcast, for_us;
 	u8 dest;
+	u8 poll;

 	switch (pin->state) {
 	/* Receive states */
@@ -319,24 +651,44 @@ static void cec_pin_rx_states(struct cec_pin *pin, ktime_t ts)
 			break;
 		pin->state = CEC_ST_RX_START_BIT_HIGH;
 		delta = ktime_us_delta(ts, pin->ts);
-		pin->ts = ts;
 		/* Start bit low is too short, go back to idle */
-		if (delta < CEC_TIM_START_BIT_LOW_MIN -
-			    CEC_TIM_IDLE_SAMPLE) {
+		if (delta < CEC_TIM_START_BIT_LOW_MIN - CEC_TIM_IDLE_SAMPLE) {
 			cec_pin_to_idle(pin);
+			break;
+		}
+		if (rx_arb_lost(pin, &poll)) {
+			cec_msg_init(&pin->tx_msg, poll >> 4, poll & 0xf);
+			pin->tx_generated_poll = true;
+			pin->tx_extra_bytes = 0;
+			pin->state = CEC_ST_TX_START_BIT_HIGH;
+			pin->ts = ts;
 		}
 		break;

 	case CEC_ST_RX_START_BIT_HIGH:
 		v = cec_pin_read(pin);
 		delta = ktime_us_delta(ts, pin->ts);
-		if (v && delta > CEC_TIM_START_BIT_TOTAL_MAX -
-				 CEC_TIM_START_BIT_LOW_MIN) {
+		/*
+		 * Unfortunately the spec does not specify when to give up
+		 * and go to idle. We just pick TOTAL_LONG.
+		 */
+		if (v && delta > CEC_TIM_START_BIT_TOTAL_LONG) {
 			cec_pin_to_idle(pin);
 			break;
 		}
 		if (v)
 			break;
+		/* Start bit is too short, go back to idle */
+		if (delta < CEC_TIM_START_BIT_TOTAL_MIN - CEC_TIM_IDLE_SAMPLE) {
+			cec_pin_to_idle(pin);
+			break;
+		}
+		if (rx_low_drive(pin)) {
+			/* Error injection: go to low drive */
+			cec_pin_low(pin);
+			pin->state = CEC_ST_RX_LOW_DRIVE;
+			break;
+		}
 		pin->state = CEC_ST_RX_DATA_SAMPLE;
 		pin->ts = ts;
 		pin->rx_eom = false;
@@ -351,36 +703,48 @@ static void cec_pin_rx_states(struct cec_pin *pin, ktime_t ts)
 				pin->rx_msg.msg[pin->rx_bit / 10] |=
 					v << (7 - (pin->rx_bit % 10));
 			break;
-		case 8:
+		case EOM_BIT:
 			pin->rx_eom = v;
 			pin->rx_msg.len = pin->rx_bit / 10 + 1;
 			break;
-		case 9:
+		case ACK_BIT:
 			break;
 		}
 		pin->rx_bit++;
 		break;

 	case CEC_ST_RX_DATA_POST_SAMPLE:
-		pin->state = CEC_ST_RX_DATA_HIGH;
+		pin->state = CEC_ST_RX_DATA_WAIT_FOR_LOW;
 		break;

-	case CEC_ST_RX_DATA_HIGH:
+	case CEC_ST_RX_DATA_WAIT_FOR_LOW:
 		v = cec_pin_read(pin);
 		delta = ktime_us_delta(ts, pin->ts);
-		if (v && delta > CEC_TIM_DATA_BIT_TOTAL_MAX) {
+		/*
+		 * Unfortunately the spec does not specify when to give up
+		 * and go to idle. We just pick TOTAL_LONG.
+		 */
+		if (v && delta > CEC_TIM_DATA_BIT_TOTAL_LONG) {
 			cec_pin_to_idle(pin);
 			break;
 		}
 		if (v)
 			break;
+
+		if (rx_low_drive(pin)) {
+			/* Error injection: go to low drive */
+			cec_pin_low(pin);
+			pin->state = CEC_ST_RX_LOW_DRIVE;
+			break;
+		}
+
 		/*
 		 * Go to low drive state when the total bit time is
 		 * too short.
 		 */
 		if (delta < CEC_TIM_DATA_BIT_TOTAL_MIN) {
 			cec_pin_low(pin);
-			pin->state = CEC_ST_LOW_DRIVE;
+			pin->state = CEC_ST_RX_LOW_DRIVE;
 			break;
 		}
 		pin->ts = ts;
@@ -396,6 +760,11 @@ static void cec_pin_rx_states(struct cec_pin *pin, ktime_t ts)
 		/* ACK bit value */
 		ack = bcast ? 1 : !for_us;

+		if (for_us && rx_nack(pin)) {
+			/* Error injection: toggle the ACK bit */
+			ack = !ack;
+		}
+
 		if (ack) {
 			/* No need to write to the bus, just wait */
 			pin->state = CEC_ST_RX_ACK_HIGH_POST;
@@ -422,7 +791,7 @@ static void cec_pin_rx_states(struct cec_pin *pin, ktime_t ts)
 			break;
 		}
 		pin->rx_bit++;
-		pin->state = CEC_ST_RX_DATA_HIGH;
+		pin->state = CEC_ST_RX_DATA_WAIT_FOR_LOW;
 		break;

 	case CEC_ST_RX_ACK_FINISH:
@@ -444,6 +813,7 @@ static enum hrtimer_restart cec_pin_timer(struct hrtimer *timer)
 	struct cec_adapter *adap = pin->adap;
 	ktime_t ts;
 	s32 delta;
+	u32 usecs;

 	ts = ktime_get();
 	if (ktime_to_ns(pin->timer_ts)) {
@@ -491,13 +861,27 @@ static enum hrtimer_restart cec_pin_timer(struct hrtimer *timer)
 	/* Transmit states */
 	case CEC_ST_TX_WAIT_FOR_HIGH:
 	case CEC_ST_TX_START_BIT_LOW:
-	case CEC_ST_TX_DATA_BIT_1_HIGH_POST_SAMPLE:
-	case CEC_ST_TX_DATA_BIT_0_HIGH:
-	case CEC_ST_TX_DATA_BIT_1_HIGH:
 	case CEC_ST_TX_START_BIT_HIGH:
+	case CEC_ST_TX_START_BIT_HIGH_SHORT:
+	case CEC_ST_TX_START_BIT_HIGH_LONG:
+	case CEC_ST_TX_START_BIT_LOW_CUSTOM:
+	case CEC_ST_TX_START_BIT_HIGH_CUSTOM:
 	case CEC_ST_TX_DATA_BIT_0_LOW:
+	case CEC_ST_TX_DATA_BIT_0_HIGH:
+	case CEC_ST_TX_DATA_BIT_0_HIGH_SHORT:
+	case CEC_ST_TX_DATA_BIT_0_HIGH_LONG:
 	case CEC_ST_TX_DATA_BIT_1_LOW:
+	case CEC_ST_TX_DATA_BIT_1_HIGH:
+	case CEC_ST_TX_DATA_BIT_1_HIGH_SHORT:
+	case CEC_ST_TX_DATA_BIT_1_HIGH_LONG:
 	case CEC_ST_TX_DATA_BIT_1_HIGH_PRE_SAMPLE:
+	case CEC_ST_TX_DATA_BIT_1_HIGH_POST_SAMPLE:
+	case CEC_ST_TX_DATA_BIT_1_HIGH_POST_SAMPLE_SHORT:
+	case CEC_ST_TX_DATA_BIT_1_HIGH_POST_SAMPLE_LONG:
+	case CEC_ST_TX_DATA_BIT_LOW_CUSTOM:
+	case CEC_ST_TX_DATA_BIT_HIGH_CUSTOM:
+	case CEC_ST_TX_PULSE_LOW_CUSTOM:
+	case CEC_ST_TX_PULSE_HIGH_CUSTOM:
 		cec_pin_tx_states(pin, ts);
 		break;

@@ -506,7 +890,7 @@ static enum hrtimer_restart cec_pin_timer(struct hrtimer *timer)
 	case CEC_ST_RX_START_BIT_HIGH:
 	case CEC_ST_RX_DATA_SAMPLE:
 	case CEC_ST_RX_DATA_POST_SAMPLE:
-	case CEC_ST_RX_DATA_HIGH:
+	case CEC_ST_RX_DATA_WAIT_FOR_LOW:
 	case CEC_ST_RX_ACK_LOW:
 	case CEC_ST_RX_ACK_LOW_POST:
 	case CEC_ST_RX_ACK_HIGH_POST:
@@ -533,7 +917,10 @@ static enum hrtimer_restart cec_pin_timer(struct hrtimer *timer)
 			if (delta / CEC_TIM_DATA_BIT_TOTAL >
 			    pin->tx_signal_free_time) {
 				pin->tx_nacked = false;
-				pin->state = CEC_ST_TX_START_BIT_LOW;
+				if (tx_custom_start(pin))
+					pin->state = CEC_ST_TX_START_BIT_LOW_CUSTOM;
+				else
+					pin->state = CEC_ST_TX_START_BIT_LOW;
 				/* Generate start bit */
 				cec_pin_low(pin);
 				break;
@@ -543,6 +930,13 @@ static enum hrtimer_restart cec_pin_timer(struct hrtimer *timer)
 				pin->state = CEC_ST_TX_WAIT;
 			break;
 		}
+		if (pin->tx_custom_pulse && pin->state == CEC_ST_IDLE) {
+			pin->tx_custom_pulse = false;
+			/* Generate custom pulse */
+			cec_pin_low(pin);
+			pin->state = CEC_ST_TX_PULSE_LOW_CUSTOM;
+			break;
+		}
 		if (pin->state != CEC_ST_IDLE || pin->ops->enable_irq == NULL ||
 		    pin->enable_irq_failed || adap->is_configuring ||
 		    adap->is_configured || adap->monitor_all_cnt)
@@ -553,21 +947,40 @@ static enum hrtimer_restart cec_pin_timer(struct hrtimer *timer)
 		wake_up_interruptible(&pin->kthread_waitq);
 		return HRTIMER_NORESTART;

-	case CEC_ST_LOW_DRIVE:
+	case CEC_ST_TX_LOW_DRIVE:
+	case CEC_ST_RX_LOW_DRIVE:
+		cec_pin_high(pin);
 		cec_pin_to_idle(pin);
 		break;

 	default:
 		break;
 	}
-	if (!adap->monitor_pin_cnt || states[pin->state].usecs <= 150) {
+
+	switch (pin->state) {
+	case CEC_ST_TX_START_BIT_LOW_CUSTOM:
+	case CEC_ST_TX_DATA_BIT_LOW_CUSTOM:
+	case CEC_ST_TX_PULSE_LOW_CUSTOM:
+		usecs = pin->tx_custom_low_usecs;
+		break;
+	case CEC_ST_TX_START_BIT_HIGH_CUSTOM:
+	case CEC_ST_TX_DATA_BIT_HIGH_CUSTOM:
+	case CEC_ST_TX_PULSE_HIGH_CUSTOM:
+		usecs = pin->tx_custom_high_usecs;
+		break;
+	default:
+		usecs = states[pin->state].usecs;
+		break;
+	}
+
+	if (!adap->monitor_pin_cnt || usecs <= 150) {
 		pin->wait_usecs = 0;
-		pin->timer_ts = ktime_add_us(ts, states[pin->state].usecs);
+		pin->timer_ts = ktime_add_us(ts, usecs);
 		hrtimer_forward_now(timer,
-				ns_to_ktime(states[pin->state].usecs * 1000));
+				ns_to_ktime(usecs * 1000));
 		return HRTIMER_RESTART;
 	}
-	pin->wait_usecs = states[pin->state].usecs - 100;
+	pin->wait_usecs = usecs - 100;
 	pin->timer_ts = ktime_add_us(ts, 100);
 	hrtimer_forward_now(timer, ns_to_ktime(100000));
 	return HRTIMER_RESTART;
@@ -587,9 +1000,22 @@ static int cec_pin_thread_func(void *_adap)
 			atomic_read(&pin->work_pin_events));

 		if (pin->work_rx_msg.len) {
-			cec_received_msg_ts(adap, &pin->work_rx_msg,
+			struct cec_msg *msg = &pin->work_rx_msg;
+
+			if (msg->len > 1 && msg->len < CEC_MAX_MSG_SIZE &&
+			    rx_add_byte(pin)) {
+				/* Error injection: add byte to the message */
+				msg->msg[msg->len++] = 0x55;
+			}
+			if (msg->len > 2 && rx_remove_byte(pin)) {
+				/* Error injection: remove byte from message */
+				msg->len--;
+			}
+			if (msg->len > CEC_MAX_MSG_SIZE)
+				msg->len = CEC_MAX_MSG_SIZE;
+			cec_received_msg_ts(adap, msg,
 				ns_to_ktime(pin->work_rx_msg.rx_ts));
-			pin->work_rx_msg.len = 0;
+			msg->len = 0;
 		}
 		if (pin->work_tx_status) {
 			unsigned int tx_status = pin->work_tx_status;
@@ -698,7 +1124,16 @@ static int cec_pin_adap_transmit(struct cec_adapter *adap, u8 attempts,
 	struct cec_pin *pin = adap->pin;

 	pin->tx_signal_free_time = signal_free_time;
+	pin->tx_extra_bytes = 0;
 	pin->tx_msg = *msg;
+	if (msg->len > 1) {
+		/* Error injection: add byte to the message */
+		pin->tx_extra_bytes = tx_add_bytes(pin);
+	}
+	if (msg->len > 2 && tx_remove_byte(pin)) {
+		/* Error injection: remove byte from the message */
+		pin->tx_msg.len--;
+	}
 	pin->work_tx_status = 0;
 	pin->tx_bit = 0;
 	cec_pin_start_timer(pin);
-- 
2.16.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCHv2 6/7] cec-pin-error-inj.rst: document CEC Pin Error Injection
  2018-03-05 13:51 ` [PATCHv2 6/7] cec-pin-error-inj.rst: document CEC Pin Error Injection Hans Verkuil
@ 2018-03-21 15:45   ` Mauro Carvalho Chehab
  2018-03-27  7:59     ` Jani Nikula
  0 siblings, 1 reply; 14+ messages in thread
From: Mauro Carvalho Chehab @ 2018-03-21 15:45 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: linux-media, Wolfram Sang, Maxime Ripard, dri-devel, Hans Verkuil

Em Mon,  5 Mar 2018 14:51:38 +0100
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> The CEC Pin framework adds support for Error Injection.
> 
> Document all the error injections commands and how to use it.

Please notice that all debugfs/sysfs entries should *also* be
documented at the standard way, e. g. by adding the corresponding
documentation at Documentation/ABI.

Please see Documentation/ABI/README.

Additionally, there are a few minor nitpicks on this patch.
See below.

The remaining patches looked ok on my eyes.

I'll wait for a v3 with the debugfs ABI documentation in order to merge
it. Feel free to put it on a separate patch.

Regards,
Mauro

> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---
>  .../media/cec-drivers/cec-pin-error-inj.rst        | 322 +++++++++++++++++++++
>  Documentation/media/cec-drivers/index.rst          |   1 +
>  MAINTAINERS                                        |   1 +
>  3 files changed, 324 insertions(+)
>  create mode 100644 Documentation/media/cec-drivers/cec-pin-error-inj.rst
> 
> diff --git a/Documentation/media/cec-drivers/cec-pin-error-inj.rst b/Documentation/media/cec-drivers/cec-pin-error-inj.rst
> new file mode 100644
> index 000000000000..21bda831d3fb
> --- /dev/null
> +++ b/Documentation/media/cec-drivers/cec-pin-error-inj.rst
> @@ -0,0 +1,322 @@
> +CEC Pin Framework Error Injection
> +=================================
> +
> +The CEC Pin Framework is a core CEC framework for CEC hardware that only
> +has low-level support for the CEC bus. Most hardware today will have
> +high-level CEC support where the hardware deals with driving the CEC bus,
> +but some older devices aren't that fancy. However, this framework also
> +allows you to connect the CEC pin to a GPIO on e.g. a Raspberry Pi and
> +you can become an instant CEC adapter.
> +
> +What makes doing this so interesting is that since we have full control
> +over the bus it is easy to support error injection. This is ideal to
> +test how well CEC adapters can handle error conditions.
> +
> +Currently only the cec-gpio driver (when the CEC line is directly
> +connected to a pull-up GPIO line) and the AllWinner A10/A20 drm driver
> +support this framework.
> +
> +If ``CONFIG_CEC_PIN_ERROR_INJ`` is enabled, then error injection is available
> +through debugfs. Specifically, in ``/sys/kernel/debug/cec/cecX/`` there is
> +now an ``error-inj`` file.
> +
> +With ``cat error-inj`` you can see both the possible commands and the current
> +error injection status:
> +
> +.. code-block:: none

It is usually better to use "::" instead of ".. code-block".

> +
> +	$ cat /sys/kernel/debug/cec/cec0/error-inj
> +	# Clear error injections:
> +	#   clear          clear all rx and tx error injections
> +	#   rx-clear       clear all rx error injections
> +	#   tx-clear       clear all tx error injections
> +	#   <op> clear     clear all rx and tx error injections for <op>
> +	#   <op> rx-clear  clear all rx error injections for <op>
> +	#   <op> tx-clear  clear all tx error injections for <op>
> +	#
> +	# RX error injection:
> +	#   <op>[,<mode>] rx-nack              NACK the message instead of sending an ACK
> +	#   <op>[,<mode>] rx-low-drive <bit>   force a low-drive condition at this bit position
> +	#   <op>[,<mode>] rx-add-byte          add a spurious byte to the received CEC message
> +	#   <op>[,<mode>] rx-remove-byte       remove the last byte from the received CEC message
> +	#   <op>[,<mode>] rx-arb-lost <poll>   generate a POLL message to trigger an arbitration lost
> +	#
> +	# TX error injection settings:
> +	#   tx-ignore-nack-until-eom           ignore early NACKs until EOM
> +	#   tx-custom-low-usecs <usecs>        define the 'low' time for the custom pulse
> +	#   tx-custom-high-usecs <usecs>       define the 'high' time for the custom pulse
> +	#   tx-custom-pulse                    transmit the custom pulse once the bus is idle
> +	#
> +	# TX error injection:
> +	#   <op>[,<mode>] tx-no-eom            don't set the EOM bit
> +	#   <op>[,<mode>] tx-early-eom         set the EOM bit one byte too soon
> +	#   <op>[,<mode>] tx-add-bytes <num>   append <num> (1-255) spurious bytes to the message
> +	#   <op>[,<mode>] tx-remove-byte       drop the last byte from the message
> +	#   <op>[,<mode>] tx-short-bit <bit>   make this bit shorter than allowed
> +	#   <op>[,<mode>] tx-long-bit <bit>    make this bit longer than allowed
> +	#   <op>[,<mode>] tx-custom-bit <bit>  send the custom pulse instead of this bit
> +	#   <op>[,<mode>] tx-short-start       send a start pulse that's too short
> +	#   <op>[,<mode>] tx-long-start        send a start pulse that's too long
> +	#   <op>[,<mode>] tx-custom-start      send the custom pulse instead of the start pulse
> +	#   <op>[,<mode>] tx-last-bit <bit>    stop sending after this bit
> +	#   <op>[,<mode>] tx-low-drive <bit>   force a low-drive condition at this bit position
> +	#
> +	# <op>       CEC message opcode (0-255) or 'any'
> +	# <mode>     'once' (default), 'always', 'toggle' or 'off'
> +	# <bit>      CEC message bit (0-159)
> +	#            10 bits per 'byte': bits 0-7: data, bit 8: EOM, bit 9: ACK
> +	# <poll>     CEC poll message used to test arbitration lost (0x00-0xff, default 0x0f)
> +	# <usecs>    microseconds (0-10000000, default 1000)
> +
> +	clear
> +
> +You can write error injection commands to ``error-inj`` using ``echo 'cmd' >error-inj``
> +or ``cat cmd.txt >error-inj``. The ``cat error-inj`` output contains the current
> +error commands. You can save the output to a file and use it as an input to
> +``error-inj`` later.

Please word-wrap it to fit into 80 columns (actually, it is better to use
something lower than that, like 72, as it makes easier to do small
adjustments without needing to change an entire text block.

(Same applies to other parts of this patch)



Thanks,
Mauro

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCHv2 6/7] cec-pin-error-inj.rst: document CEC Pin Error Injection
  2018-03-21 15:45   ` Mauro Carvalho Chehab
@ 2018-03-27  7:59     ` Jani Nikula
  2018-03-27  8:09       ` Wolfram Sang
  0 siblings, 1 reply; 14+ messages in thread
From: Jani Nikula @ 2018-03-27  7:59 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Hans Verkuil
  Cc: Maxime Ripard, Wolfram Sang, Hans Verkuil, dri-devel, linux-media

On Wed, 21 Mar 2018, Mauro Carvalho Chehab <mchehab@s-opensource.com> wrote:
> Please notice that all debugfs/sysfs entries should *also* be
> documented at the standard way, e. g. by adding the corresponding
> documentation at Documentation/ABI.
>
> Please see Documentation/ABI/README.
>
> Additionally, there are a few minor nitpicks on this patch.
> See below.
>
> The remaining patches looked ok on my eyes.
>
> I'll wait for a v3 with the debugfs ABI documentation in order to merge
> it. Feel free to put it on a separate patch.

debugfs ABI? Sounds like an oxymoron to me.

BR,
Jani.

-- 
Jani Nikula, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCHv2 6/7] cec-pin-error-inj.rst: document CEC Pin Error Injection
  2018-03-27  7:59     ` Jani Nikula
@ 2018-03-27  8:09       ` Wolfram Sang
  0 siblings, 0 replies; 14+ messages in thread
From: Wolfram Sang @ 2018-03-27  8:09 UTC (permalink / raw)
  To: Jani Nikula
  Cc: Mauro Carvalho Chehab, Hans Verkuil, Maxime Ripard, Hans Verkuil,
	dri-devel, linux-media

[-- Attachment #1: Type: text/plain, Size: 210 bytes --]


> > I'll wait for a v3 with the debugfs ABI documentation in order to merge
> > it. Feel free to put it on a separate patch.
> 
> debugfs ABI? Sounds like an oxymoron to me.

Heh, thought the same :)


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2018-03-27  8:09 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-05 13:51 [PATCHv2 0/7] cec: add error injection support Hans Verkuil
2018-03-05 13:51 ` [PATCHv2 1/7] cec: add core " Hans Verkuil
2018-03-05 13:51 ` [PATCHv2 2/7] cec-core.rst: document the error injection ops Hans Verkuil
2018-03-05 13:51 ` [PATCHv2 3/7] cec-pin: create cec_pin_start_timer() function Hans Verkuil
2018-03-05 13:51 ` [PATCHv2 4/7] cec-pin-error-inj: parse/show error injection Hans Verkuil
2018-03-05 13:51 ` [PATCHv2 5/7] cec-pin: add error injection support Hans Verkuil
2018-03-06 22:48   ` Hans Verkuil
2018-03-07 14:17     ` [PATCHv2.1 " Hans Verkuil
2018-03-05 13:51 ` [PATCHv2 6/7] cec-pin-error-inj.rst: document CEC Pin Error Injection Hans Verkuil
2018-03-21 15:45   ` Mauro Carvalho Chehab
2018-03-27  7:59     ` Jani Nikula
2018-03-27  8:09       ` Wolfram Sang
2018-03-05 13:51 ` [PATCHv2 7/7] cec-pin: improve status log Hans Verkuil
2018-03-05 17:40 ` [PATCHv2 0/7] cec: add error injection support Wolfram Sang

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).