linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RESEND 00/13] platform/chrome: Add console debugfs, lpc and lightbar programming support
@ 2017-05-16 16:13 Enric Balletbo i Serra
  2017-05-16 16:13 ` [PATCH RESEND 01/13] mfd: cros_ec: Add helper for event notifier Enric Balletbo i Serra
                   ` (12 more replies)
  0 siblings, 13 replies; 31+ messages in thread
From: Enric Balletbo i Serra @ 2017-05-16 16:13 UTC (permalink / raw)
  To: olof, bleung, linux-kernel; +Cc: lee.jones

Dear Olof, Benson,

We have some patches that stayed in the mailing list for long time and already missed some merge window. Please kindly take into consideration to not miss more merge windows.

As some patches depends on the other to apply cleanly I created a patch series with all includes to make easier the merge, so this series include the following patches.

 - https://lkml.org/lkml/2016/8/23/10
 - https://lkml.org/lkml/2016/12/2/360
 - https://lkml.org/lkml/2016/12/16/438
 - https://lkml.org/lkml/2017/1/11/579

This series is a rebase on top of linux-next

If you prefer to pick the patches from a git repository, the following
changes since commit 15efd2015d5e7265273187d37a89aa985565ab2d

  mfd: cros_ec: Add helper for event notifier.

are available in the git repository at:

  https://git.collabora.com/cgit/linux.git upstream/chromeos/for-4.13

for you to fetch changes up to ee7c27a73820c245af2a29cdd3810196c2b02b67

  platform/chrome: cros_ec_lightbar - Avoid I2C xfer to EC during suspend

Many thanks for your help,
 Enric

Archana Patni (1):
  platform/chrome: cros_ec_lpc: Add power management ops

Eric Caruso (4):
  mfd: cros_ec: add debugfs, console log file
  platform/chrome: cros_ec_lightbar - Add lightbar program feature to
    sysfs
  platform/chrome: cros_ec_lightbar - Control of suspend/resume lightbar
    sequence
  platform/chrome: cros_ec_lightbar - Add userspace lightbar control bit
    to EC

Gwendal Grignou (3):
  mfd: cros_ec: Add helper for event notifier.
  platform/chrome: cros_ec_lpc: Add support for GOOG004 ACPI device
  platform/chrome: cros_ec_lpc: Add MKBP events support over ACPI

Jeffery Yu (1):
  platform/chrome: cros_ec_lightbar - Avoid I2C xfer to EC during
    suspend

Nicolas Boichat (2):
  mfd: cros_ec: Add EC console read structures definitions
  mfd: cros_ec: Add support for dumping panic information

Shawn Nematbakhsh (2):
  platform/chrome: cros_ec_lpc: Add R/W helpers to LPC protocol variants
  platform/chrome: cros_ec_lpc: Add support for mec1322 EC

 drivers/platform/chrome/Kconfig            |  14 +-
 drivers/platform/chrome/Makefile           |   7 +-
 drivers/platform/chrome/cros_ec_debugfs.c  | 401 +++++++++++++++++++++++++++++
 drivers/platform/chrome/cros_ec_debugfs.h  |  27 ++
 drivers/platform/chrome/cros_ec_dev.c      |  40 +++
 drivers/platform/chrome/cros_ec_dev.h      |   6 +
 drivers/platform/chrome/cros_ec_lightbar.c | 197 +++++++++++++-
 drivers/platform/chrome/cros_ec_lpc.c      | 167 +++++++-----
 drivers/platform/chrome/cros_ec_lpc_mec.c  | 140 ++++++++++
 drivers/platform/chrome/cros_ec_lpc_reg.c  | 133 ++++++++++
 drivers/platform/chrome/cros_ec_proto.c    |  20 ++
 include/linux/mfd/cros_ec.h                |  14 +
 include/linux/mfd/cros_ec_commands.h       |  42 ++-
 include/linux/mfd/cros_ec_lpc_mec.h        |  90 +++++++
 include/linux/mfd/cros_ec_lpc_reg.h        |  61 +++++
 15 files changed, 1286 insertions(+), 73 deletions(-)
 create mode 100644 drivers/platform/chrome/cros_ec_debugfs.c
 create mode 100644 drivers/platform/chrome/cros_ec_debugfs.h
 create mode 100644 drivers/platform/chrome/cros_ec_lpc_mec.c
 create mode 100644 drivers/platform/chrome/cros_ec_lpc_reg.c
 create mode 100644 include/linux/mfd/cros_ec_lpc_mec.h
 create mode 100644 include/linux/mfd/cros_ec_lpc_reg.h

-- 
2.9.3

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

* [PATCH RESEND 01/13] mfd: cros_ec: Add helper for event notifier.
  2017-05-16 16:13 [PATCH RESEND 00/13] platform/chrome: Add console debugfs, lpc and lightbar programming support Enric Balletbo i Serra
@ 2017-05-16 16:13 ` Enric Balletbo i Serra
  2017-06-16 20:45   ` Benson Leung
  2017-05-16 16:13 ` [PATCH RESEND 02/13] mfd: cros_ec: Add EC console read structures definitions Enric Balletbo i Serra
                   ` (11 subsequent siblings)
  12 siblings, 1 reply; 31+ messages in thread
From: Enric Balletbo i Serra @ 2017-05-16 16:13 UTC (permalink / raw)
  To: olof, bleung, linux-kernel; +Cc: lee.jones, Gwendal Grignou

From: Gwendal Grignou <gwendal@chromium.org>

Add cros_ec_get_event() entry point to retrieve event within functions
called by the notifier.

Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/platform/chrome/cros_ec_proto.c | 20 ++++++++++++++++++++
 include/linux/mfd/cros_ec.h             | 10 ++++++++++
 2 files changed, 30 insertions(+)

diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
index ed5dee7..7428c2b 100644
--- a/drivers/platform/chrome/cros_ec_proto.c
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -494,3 +494,23 @@ int cros_ec_get_next_event(struct cros_ec_device *ec_dev)
 		return get_keyboard_state_event(ec_dev);
 }
 EXPORT_SYMBOL(cros_ec_get_next_event);
+
+u32 cros_ec_get_host_event(struct cros_ec_device *ec_dev)
+{
+	u32 host_event;
+
+	BUG_ON(!ec_dev->mkbp_event_supported);
+
+	if (ec_dev->event_data.event_type != EC_MKBP_EVENT_HOST_EVENT)
+		return 0;
+
+	if (ec_dev->event_size != sizeof(host_event)) {
+		dev_warn(ec_dev->dev, "Invalid host event size\n");
+		return 0;
+	}
+
+	host_event = get_unaligned_le32(&ec_dev->event_data.data.host_event);
+
+	return host_event;
+}
+EXPORT_SYMBOL(cros_ec_get_host_event);
diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
index 28baee6..b61b2e0 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -300,6 +300,16 @@ int cros_ec_query_all(struct cros_ec_device *ec_dev);
  */
 int cros_ec_get_next_event(struct cros_ec_device *ec_dev);
 
+/**
+ * cros_ec_get_host_event - Return a mask of event set by the EC.
+ *
+ * When MKBP is supported, when the EC raises an interrupt,
+ * We collect the events raised and call the functions in the ec notifier.
+ *
+ * This function is a helper to know which events are raised.
+ */
+u32 cros_ec_get_host_event(struct cros_ec_device *ec_dev);
+
 /* sysfs stuff */
 extern struct attribute_group cros_ec_attr_group;
 extern struct attribute_group cros_ec_lightbar_attr_group;
-- 
2.9.3

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

* [PATCH RESEND 02/13] mfd: cros_ec: Add EC console read structures definitions
  2017-05-16 16:13 [PATCH RESEND 00/13] platform/chrome: Add console debugfs, lpc and lightbar programming support Enric Balletbo i Serra
  2017-05-16 16:13 ` [PATCH RESEND 01/13] mfd: cros_ec: Add helper for event notifier Enric Balletbo i Serra
@ 2017-05-16 16:13 ` Enric Balletbo i Serra
  2017-06-16 20:47   ` Benson Leung
  2017-05-16 16:13 ` [PATCH RESEND 03/13] mfd: cros_ec: add debugfs, console log file Enric Balletbo i Serra
                   ` (10 subsequent siblings)
  12 siblings, 1 reply; 31+ messages in thread
From: Enric Balletbo i Serra @ 2017-05-16 16:13 UTC (permalink / raw)
  To: olof, bleung, linux-kernel; +Cc: lee.jones, Nicolas Boichat

From: Nicolas Boichat <drinkcat@chromium.org>

ec_params_console_read_v1 is used to capture EC logs from kernel,
and ec_params_get_cmd_versions_v1 is used to probe whether EC
supports that command.

Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>
Reviewed-by: Guenter Roeck <groeck@chromium.org>
Acked-by: Lee Jones <lee.jones@linaro.org>
Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
 include/linux/mfd/cros_ec_commands.h | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/include/linux/mfd/cros_ec_commands.h b/include/linux/mfd/cros_ec_commands.h
index c93e7e0..1b19e424 100644
--- a/include/linux/mfd/cros_ec_commands.h
+++ b/include/linux/mfd/cros_ec_commands.h
@@ -625,6 +625,10 @@ struct ec_params_get_cmd_versions {
 	uint8_t cmd;      /* Command to check */
 } __packed;
 
+struct ec_params_get_cmd_versions_v1 {
+	uint16_t cmd;     /* Command to check */
+} __packed;
+
 struct ec_response_get_cmd_versions {
 	/*
 	 * Mask of supported versions; use EC_VER_MASK() to compare with a
@@ -2285,13 +2289,28 @@ struct ec_params_charge_control {
 #define EC_CMD_CONSOLE_SNAPSHOT 0x97
 
 /*
- * Read next chunk of data from saved snapshot.
+ * Read data from the saved snapshot. If the subcmd parameter is
+ * CONSOLE_READ_NEXT, this will return data starting from the beginning of
+ * the latest snapshot. If it is CONSOLE_READ_RECENT, it will start from the
+ * end of the previous snapshot.
+ *
+ * The params are only looked at in version >= 1 of this command. Prior
+ * versions will just default to CONSOLE_READ_NEXT behavior.
  *
  * Response is null-terminated string.  Empty string, if there is no more
  * remaining output.
  */
 #define EC_CMD_CONSOLE_READ 0x98
 
+enum ec_console_read_subcmd {
+	CONSOLE_READ_NEXT = 0,
+	CONSOLE_READ_RECENT
+};
+
+struct ec_params_console_read_v1 {
+	uint8_t subcmd; /* enum ec_console_read_subcmd */
+} __packed;
+
 /*****************************************************************************/
 
 /*
-- 
2.9.3

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

* [PATCH RESEND 03/13] mfd: cros_ec: add debugfs, console log file
  2017-05-16 16:13 [PATCH RESEND 00/13] platform/chrome: Add console debugfs, lpc and lightbar programming support Enric Balletbo i Serra
  2017-05-16 16:13 ` [PATCH RESEND 01/13] mfd: cros_ec: Add helper for event notifier Enric Balletbo i Serra
  2017-05-16 16:13 ` [PATCH RESEND 02/13] mfd: cros_ec: Add EC console read structures definitions Enric Balletbo i Serra
@ 2017-05-16 16:13 ` Enric Balletbo i Serra
  2017-06-16 19:42   ` Benson Leung
  2017-05-16 16:13 ` [PATCH RESEND 04/13] mfd: cros_ec: Add support for dumping panic information Enric Balletbo i Serra
                   ` (9 subsequent siblings)
  12 siblings, 1 reply; 31+ messages in thread
From: Enric Balletbo i Serra @ 2017-05-16 16:13 UTC (permalink / raw)
  To: olof, bleung, linux-kernel; +Cc: lee.jones, Eric Caruso, Nicolas Boichat

From: Eric Caruso <ejcaruso@chromium.org>

If the EC supports the new CONSOLE_READ command type, then we
place a console_log file in debugfs for that EC device which allows
us to grab EC logs. The kernel will poll every 10 seconds for the
log and keep its own buffer, but userspace should grab this and
write it out to some logs which actually get rotated.

Signed-off-by: Eric Caruso <ejcaruso@chromium.org>
Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>
Acked-by: Lee Jones <lee.jones@linaro.org>
Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
 drivers/platform/chrome/Makefile          |   3 +-
 drivers/platform/chrome/cros_ec_debugfs.c | 347 ++++++++++++++++++++++++++++++
 drivers/platform/chrome/cros_ec_debugfs.h |  27 +++
 drivers/platform/chrome/cros_ec_dev.c     |   7 +
 include/linux/mfd/cros_ec.h               |   4 +
 5 files changed, 387 insertions(+), 1 deletion(-)
 create mode 100644 drivers/platform/chrome/cros_ec_debugfs.c
 create mode 100644 drivers/platform/chrome/cros_ec_debugfs.h

diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
index 4f34627..3870afe 100644
--- a/drivers/platform/chrome/Makefile
+++ b/drivers/platform/chrome/Makefile
@@ -2,7 +2,8 @@
 obj-$(CONFIG_CHROMEOS_LAPTOP)		+= chromeos_laptop.o
 obj-$(CONFIG_CHROMEOS_PSTORE)		+= chromeos_pstore.o
 cros_ec_devs-objs			:= cros_ec_dev.o cros_ec_sysfs.o \
-					   cros_ec_lightbar.o cros_ec_vbc.o
+					   cros_ec_lightbar.o cros_ec_vbc.o \
+					   cros_ec_debugfs.o
 obj-$(CONFIG_CROS_EC_CHARDEV)		+= cros_ec_devs.o
 obj-$(CONFIG_CROS_EC_LPC)		+= cros_ec_lpc.o
 obj-$(CONFIG_CROS_EC_PROTO)		+= cros_ec_proto.o
diff --git a/drivers/platform/chrome/cros_ec_debugfs.c b/drivers/platform/chrome/cros_ec_debugfs.c
new file mode 100644
index 0000000..f8195bb
--- /dev/null
+++ b/drivers/platform/chrome/cros_ec_debugfs.c
@@ -0,0 +1,347 @@
+/*
+ * cros_ec_debugfs - debug logs for Chrome OS EC
+ *
+ * Copyright 2015 Google, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/circ_buf.h>
+#include <linux/debugfs.h>
+#include <linux/delay.h>
+#include <linux/fs.h>
+#include <linux/mfd/cros_ec.h>
+#include <linux/mfd/cros_ec_commands.h>
+#include <linux/mutex.h>
+#include <linux/poll.h>
+#include <linux/sched.h>
+#include <linux/slab.h>
+#include <linux/wait.h>
+
+#include "cros_ec_dev.h"
+#include "cros_ec_debugfs.h"
+
+#define LOG_SHIFT		14
+#define LOG_SIZE		(1 << LOG_SHIFT)
+#define LOG_POLL_SEC		10
+
+#define CIRC_ADD(idx, size, value)	(((idx) + (value)) & ((size) - 1))
+
+/* struct cros_ec_debugfs - ChromeOS EC debugging information
+ *
+ * @ec: EC device this debugfs information belongs to
+ * @dir: dentry for debugfs files
+ * @log_buffer: circular buffer for console log information
+ * @read_msg: preallocated EC command and buffer to read console log
+ * @log_mutex: mutex to protect circular buffer
+ * @log_wq: waitqueue for log readers
+ * @log_poll_work: recurring task to poll EC for new console log data
+ */
+struct cros_ec_debugfs {
+	struct cros_ec_dev *ec;
+	struct dentry *dir;
+	struct circ_buf log_buffer;
+	struct cros_ec_command *read_msg;
+	struct mutex log_mutex;
+	wait_queue_head_t log_wq;
+	struct delayed_work log_poll_work;
+};
+
+/*
+ * We need to make sure that the EC log buffer on the UART is large enough,
+ * so that it is unlikely enough to overlow within LOG_POLL_SEC.
+ */
+static void cros_ec_console_log_work(struct work_struct *__work)
+{
+	struct cros_ec_debugfs *debug_info =
+		container_of(to_delayed_work(__work),
+			     struct cros_ec_debugfs,
+			     log_poll_work);
+	struct cros_ec_dev *ec = debug_info->ec;
+	struct circ_buf *cb = &debug_info->log_buffer;
+	struct cros_ec_command snapshot_msg = {
+		.command = EC_CMD_CONSOLE_SNAPSHOT + ec->cmd_offset,
+	};
+
+	struct ec_params_console_read_v1 *read_params =
+		(struct ec_params_console_read_v1 *)debug_info->read_msg->data;
+	uint8_t *ec_buffer = (uint8_t *)debug_info->read_msg->data;
+	int idx;
+	int buf_space;
+	int ret;
+
+	ret = cros_ec_cmd_xfer(ec->ec_dev, &snapshot_msg);
+	if (ret < 0) {
+		dev_err(ec->dev, "EC communication failed\n");
+		goto resched;
+	}
+	if (snapshot_msg.result != EC_RES_SUCCESS) {
+		dev_err(ec->dev, "EC failed to snapshot the console log\n");
+		goto resched;
+	}
+
+	/* Loop until we have read everything, or there's an error. */
+	mutex_lock(&debug_info->log_mutex);
+	buf_space = CIRC_SPACE(cb->head, cb->tail, LOG_SIZE);
+
+	while (1) {
+		if (!buf_space) {
+			dev_info_once(ec->dev,
+				      "Some logs may have been dropped...\n");
+			break;
+		}
+
+		memset(read_params, '\0', sizeof(*read_params));
+		read_params->subcmd = CONSOLE_READ_RECENT;
+		ret = cros_ec_cmd_xfer(ec->ec_dev, debug_info->read_msg);
+		if (ret < 0) {
+			dev_err(ec->dev, "EC communication failed\n");
+			break;
+		}
+		if (debug_info->read_msg->result != EC_RES_SUCCESS) {
+			dev_err(ec->dev,
+				"EC failed to read the console log\n");
+			break;
+		}
+
+		/* If the buffer is empty, we're done here. */
+		if (ret == 0 || ec_buffer[0] == '\0')
+			break;
+
+		idx = 0;
+		while (idx < ret && ec_buffer[idx] != '\0' && buf_space > 0) {
+			cb->buf[cb->head] = ec_buffer[idx];
+			cb->head = CIRC_ADD(cb->head, LOG_SIZE, 1);
+			idx++;
+			buf_space--;
+		}
+
+		wake_up(&debug_info->log_wq);
+	}
+
+	mutex_unlock(&debug_info->log_mutex);
+
+resched:
+	schedule_delayed_work(&debug_info->log_poll_work,
+			      msecs_to_jiffies(LOG_POLL_SEC * 1000));
+}
+
+static int cros_ec_console_log_open(struct inode *inode, struct file *file)
+{
+	file->private_data = inode->i_private;
+
+	return nonseekable_open(inode, file);
+}
+
+static ssize_t cros_ec_console_log_read(struct file *file, char __user *buf,
+					size_t count, loff_t *ppos)
+{
+	struct cros_ec_debugfs *debug_info = file->private_data;
+	struct circ_buf *cb = &debug_info->log_buffer;
+	ssize_t ret;
+
+	mutex_lock(&debug_info->log_mutex);
+
+	while (!CIRC_CNT(cb->head, cb->tail, LOG_SIZE)) {
+		if (file->f_flags & O_NONBLOCK) {
+			ret = -EAGAIN;
+			goto error;
+		}
+
+		mutex_unlock(&debug_info->log_mutex);
+
+		ret = wait_event_interruptible(debug_info->log_wq,
+					CIRC_CNT(cb->head, cb->tail, LOG_SIZE));
+		if (ret < 0)
+			return ret;
+
+		mutex_lock(&debug_info->log_mutex);
+	}
+
+	/* Only copy until the end of the circular buffer, and let userspace
+	 * retry to get the rest of the data.
+	 */
+	ret = min_t(size_t, CIRC_CNT_TO_END(cb->head, cb->tail, LOG_SIZE),
+		    count);
+
+	if (copy_to_user(buf, cb->buf + cb->tail, ret)) {
+		ret = -EFAULT;
+		goto error;
+	}
+
+	cb->tail = CIRC_ADD(cb->tail, LOG_SIZE, ret);
+
+error:
+	mutex_unlock(&debug_info->log_mutex);
+	return ret;
+}
+
+static unsigned int cros_ec_console_log_poll(struct file *file,
+					     poll_table *wait)
+{
+	struct cros_ec_debugfs *debug_info = file->private_data;
+	unsigned int mask = 0;
+
+	poll_wait(file, &debug_info->log_wq, wait);
+
+	mutex_lock(&debug_info->log_mutex);
+	if (CIRC_CNT(debug_info->log_buffer.head,
+		     debug_info->log_buffer.tail,
+		     LOG_SIZE))
+		mask |= POLLIN | POLLRDNORM;
+	mutex_unlock(&debug_info->log_mutex);
+
+	return mask;
+}
+
+static int cros_ec_console_log_release(struct inode *inode, struct file *file)
+{
+	return 0;
+}
+
+const struct file_operations cros_ec_console_log_fops = {
+	.owner = THIS_MODULE,
+	.open = cros_ec_console_log_open,
+	.read = cros_ec_console_log_read,
+	.llseek = no_llseek,
+	.poll = cros_ec_console_log_poll,
+	.release = cros_ec_console_log_release,
+};
+
+static int ec_read_version_supported(struct cros_ec_dev *ec)
+{
+	struct ec_params_get_cmd_versions_v1 *params;
+	struct ec_response_get_cmd_versions *response;
+	int ret;
+
+	struct cros_ec_command *msg;
+
+	msg = kzalloc(sizeof(*msg) + max(sizeof(params), sizeof(response)),
+		GFP_KERNEL);
+	if (!msg)
+		return 0;
+
+	msg->command = EC_CMD_GET_CMD_VERSIONS + ec->cmd_offset;
+	msg->outsize = sizeof(*params);
+	msg->insize = sizeof(*response);
+
+	params = (struct ec_params_get_cmd_versions_v1 *)msg->data;
+	params->cmd = EC_CMD_CONSOLE_READ;
+	response = (struct ec_response_get_cmd_versions *)msg->data;
+
+	ret = cros_ec_cmd_xfer(ec->ec_dev, msg) >= 0 &&
+		msg->result == EC_RES_SUCCESS &&
+		(response->version_mask & EC_VER_MASK(1));
+
+	kfree(msg);
+
+	return ret;
+}
+
+static int cros_ec_create_console_log(struct cros_ec_debugfs *debug_info)
+{
+	struct cros_ec_dev *ec = debug_info->ec;
+	char *buf;
+	int read_params_size;
+	int read_response_size;
+
+	if (!ec_read_version_supported(ec)) {
+		dev_warn(ec->dev,
+			"device does not support reading the console log\n");
+		return 0;
+	}
+
+	buf = devm_kzalloc(ec->dev, LOG_SIZE, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	read_params_size = sizeof(struct ec_params_console_read_v1);
+	read_response_size = ec->ec_dev->max_response;
+	debug_info->read_msg = devm_kzalloc(ec->dev,
+		sizeof(*debug_info->read_msg) +
+			max(read_params_size, read_response_size), GFP_KERNEL);
+	if (!debug_info->read_msg)
+		return -ENOMEM;
+
+	debug_info->read_msg->version = 1;
+	debug_info->read_msg->command = EC_CMD_CONSOLE_READ + ec->cmd_offset;
+	debug_info->read_msg->outsize = read_params_size;
+	debug_info->read_msg->insize = read_response_size;
+
+	debug_info->log_buffer.buf = buf;
+	debug_info->log_buffer.head = 0;
+	debug_info->log_buffer.tail = 0;
+
+	mutex_init(&debug_info->log_mutex);
+	init_waitqueue_head(&debug_info->log_wq);
+
+	if (!debugfs_create_file("console_log",
+				 S_IFREG | S_IRUGO,
+				 debug_info->dir,
+				 debug_info,
+				 &cros_ec_console_log_fops))
+		return -ENOMEM;
+
+	INIT_DELAYED_WORK(&debug_info->log_poll_work,
+			  cros_ec_console_log_work);
+	schedule_delayed_work(&debug_info->log_poll_work, 0);
+
+	return 0;
+}
+
+static void cros_ec_cleanup_console_log(struct cros_ec_debugfs *debug_info)
+{
+	if (debug_info->log_buffer.buf) {
+		cancel_delayed_work_sync(&debug_info->log_poll_work);
+		mutex_destroy(&debug_info->log_mutex);
+	}
+}
+
+int cros_ec_debugfs_init(struct cros_ec_dev *ec)
+{
+	struct cros_ec_platform *ec_platform = dev_get_platdata(ec->dev);
+	const char *name = ec_platform->ec_name;
+	struct cros_ec_debugfs *debug_info;
+	int ret;
+
+	debug_info = devm_kzalloc(ec->dev, sizeof(*debug_info), GFP_KERNEL);
+	if (!debug_info)
+		return -ENOMEM;
+
+	debug_info->ec = ec;
+	debug_info->dir = debugfs_create_dir(name, NULL);
+	if (!debug_info->dir)
+		return -ENOMEM;
+
+	ret = cros_ec_create_console_log(debug_info);
+	if (ret)
+		goto remove_debugfs;
+
+	ec->debug_info = debug_info;
+
+	return 0;
+
+remove_debugfs:
+	debugfs_remove_recursive(debug_info->dir);
+	return ret;
+}
+
+void cros_ec_debugfs_remove(struct cros_ec_dev *ec)
+{
+	if (!ec->debug_info)
+		return;
+
+	debugfs_remove_recursive(ec->debug_info->dir);
+	cros_ec_cleanup_console_log(ec->debug_info);
+}
diff --git a/drivers/platform/chrome/cros_ec_debugfs.h b/drivers/platform/chrome/cros_ec_debugfs.h
new file mode 100644
index 0000000..1ff3a50
--- /dev/null
+++ b/drivers/platform/chrome/cros_ec_debugfs.h
@@ -0,0 +1,27 @@
+/*
+ * Copyright 2015 Google, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef _DRV_CROS_EC_DEBUGFS_H_
+#define _DRV_CROS_EC_DEBUGFS_H_
+
+#include "cros_ec_dev.h"
+
+/* debugfs stuff */
+int cros_ec_debugfs_init(struct cros_ec_dev *ec);
+void cros_ec_debugfs_remove(struct cros_ec_dev *ec);
+
+#endif  /* _DRV_CROS_EC_DEBUGFS_H_ */
diff --git a/drivers/platform/chrome/cros_ec_dev.c b/drivers/platform/chrome/cros_ec_dev.c
index 6aa120c..20ce1c2 100644
--- a/drivers/platform/chrome/cros_ec_dev.c
+++ b/drivers/platform/chrome/cros_ec_dev.c
@@ -24,6 +24,7 @@
 #include <linux/slab.h>
 #include <linux/uaccess.h>
 
+#include "cros_ec_debugfs.h"
 #include "cros_ec_dev.h"
 
 /* Device variables */
@@ -427,6 +428,9 @@ static int ec_device_probe(struct platform_device *pdev)
 		goto failed;
 	}
 
+	if (cros_ec_debugfs_init(ec))
+		dev_warn(dev, "failed to create debugfs directory\n");
+
 	/* check whether this EC is a sensor hub. */
 	if (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE))
 		cros_ec_sensors_register(ec);
@@ -441,6 +445,9 @@ static int ec_device_probe(struct platform_device *pdev)
 static int ec_device_remove(struct platform_device *pdev)
 {
 	struct cros_ec_dev *ec = dev_get_drvdata(&pdev->dev);
+
+	cros_ec_debugfs_remove(ec);
+
 	cdev_del(&ec->cdev);
 	device_unregister(&ec->class_dev);
 	return 0;
diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
index b61b2e0..3b16c90 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -172,6 +172,8 @@ struct cros_ec_platform {
 	u16 cmd_offset;
 };
 
+struct cros_ec_debugfs;
+
 /*
  * struct cros_ec_dev - ChromeOS EC device entry point
  *
@@ -179,6 +181,7 @@ struct cros_ec_platform {
  * @cdev: Character device structure in /dev
  * @ec_dev: cros_ec_device structure to talk to the physical device
  * @dev: pointer to the platform device
+ * @debug_info: cros_ec_debugfs structure for debugging information
  * @cmd_offset: offset to apply for each command.
  */
 struct cros_ec_dev {
@@ -186,6 +189,7 @@ struct cros_ec_dev {
 	struct cdev cdev;
 	struct cros_ec_device *ec_dev;
 	struct device *dev;
+	struct cros_ec_debugfs *debug_info;
 	u16 cmd_offset;
 	u32 features[2];
 };
-- 
2.9.3

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

* [PATCH RESEND 04/13] mfd: cros_ec: Add support for dumping panic information
  2017-05-16 16:13 [PATCH RESEND 00/13] platform/chrome: Add console debugfs, lpc and lightbar programming support Enric Balletbo i Serra
                   ` (2 preceding siblings ...)
  2017-05-16 16:13 ` [PATCH RESEND 03/13] mfd: cros_ec: add debugfs, console log file Enric Balletbo i Serra
@ 2017-05-16 16:13 ` Enric Balletbo i Serra
  2017-06-22 23:38   ` Benson Leung
  2017-05-16 16:13 ` [PATCH RESEND 05/13] platform/chrome: cros_ec_lpc: Add R/W helpers to LPC protocol variants Enric Balletbo i Serra
                   ` (8 subsequent siblings)
  12 siblings, 1 reply; 31+ messages in thread
From: Enric Balletbo i Serra @ 2017-05-16 16:13 UTC (permalink / raw)
  To: olof, bleung, linux-kernel; +Cc: lee.jones, Nicolas Boichat

From: Nicolas Boichat <drinkcat@chromium.org>

This dumps the EC panic information from the previous reboot.

Similar to the information presented by ectool panicinfo, except
that we do not bother doing any parsing (we should write a small
offline tool for that).

Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>
Reviewed-by: Guenter Roeck <groeck@chromium.org>
Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
 drivers/platform/chrome/cros_ec_debugfs.c | 54 +++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)

diff --git a/drivers/platform/chrome/cros_ec_debugfs.c b/drivers/platform/chrome/cros_ec_debugfs.c
index f8195bb..8133c33 100644
--- a/drivers/platform/chrome/cros_ec_debugfs.c
+++ b/drivers/platform/chrome/cros_ec_debugfs.c
@@ -47,15 +47,19 @@
  * @log_mutex: mutex to protect circular buffer
  * @log_wq: waitqueue for log readers
  * @log_poll_work: recurring task to poll EC for new console log data
+ * @panicinfo_blob: panicinfo debugfs blob
  */
 struct cros_ec_debugfs {
 	struct cros_ec_dev *ec;
 	struct dentry *dir;
+	/* EC log */
 	struct circ_buf log_buffer;
 	struct cros_ec_command *read_msg;
 	struct mutex log_mutex;
 	wait_queue_head_t log_wq;
 	struct delayed_work log_poll_work;
+	/* EC panicinfo */
+	struct debugfs_blob_wrapper panicinfo_blob;
 };
 
 /*
@@ -308,6 +312,52 @@ static void cros_ec_cleanup_console_log(struct cros_ec_debugfs *debug_info)
 	}
 }
 
+static int cros_ec_create_panicinfo(struct cros_ec_debugfs *debug_info)
+{
+	struct cros_ec_device *ec_dev = debug_info->ec->ec_dev;
+	int ret;
+	struct cros_ec_command *msg;
+	int insize;
+
+	insize = ec_dev->max_response;
+
+	msg = devm_kzalloc(debug_info->ec->dev,
+			sizeof(*msg) + insize, GFP_KERNEL);
+	if (!msg)
+		return -ENOMEM;
+
+	msg->command = EC_CMD_GET_PANIC_INFO;
+	msg->insize = insize;
+
+	ret = cros_ec_cmd_xfer(ec_dev, msg);
+	if (ret < 0) {
+		dev_warn(debug_info->ec->dev, "Cannot read panicinfo.\n");
+		ret = 0;
+		goto free;
+	}
+
+	/* No panic data */
+	if (ret == 0)
+		goto free;
+
+	debug_info->panicinfo_blob.data = msg->data;
+	debug_info->panicinfo_blob.size = ret;
+
+	if (!debugfs_create_blob("panicinfo",
+				 S_IFREG | S_IRUGO,
+				 debug_info->dir,
+				 &debug_info->panicinfo_blob)) {
+		ret = -ENOMEM;
+		goto free;
+	}
+
+	return 0;
+
+free:
+	devm_kfree(debug_info->ec->dev, msg);
+	return ret;
+}
+
 int cros_ec_debugfs_init(struct cros_ec_dev *ec)
 {
 	struct cros_ec_platform *ec_platform = dev_get_platdata(ec->dev);
@@ -324,6 +374,10 @@ int cros_ec_debugfs_init(struct cros_ec_dev *ec)
 	if (!debug_info->dir)
 		return -ENOMEM;
 
+	ret = cros_ec_create_panicinfo(debug_info);
+	if (ret)
+		goto remove_debugfs;
+
 	ret = cros_ec_create_console_log(debug_info);
 	if (ret)
 		goto remove_debugfs;
-- 
2.9.3

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

* [PATCH RESEND 05/13] platform/chrome: cros_ec_lpc: Add R/W helpers to LPC protocol variants
  2017-05-16 16:13 [PATCH RESEND 00/13] platform/chrome: Add console debugfs, lpc and lightbar programming support Enric Balletbo i Serra
                   ` (3 preceding siblings ...)
  2017-05-16 16:13 ` [PATCH RESEND 04/13] mfd: cros_ec: Add support for dumping panic information Enric Balletbo i Serra
@ 2017-05-16 16:13 ` Enric Balletbo i Serra
  2017-05-22 11:09   ` Lee Jones
  2017-06-22 19:02   ` Benson Leung
  2017-05-16 16:13 ` [PATCH RESEND 06/13] platform/chrome: cros_ec_lpc: Add support for mec1322 EC Enric Balletbo i Serra
                   ` (7 subsequent siblings)
  12 siblings, 2 replies; 31+ messages in thread
From: Enric Balletbo i Serra @ 2017-05-16 16:13 UTC (permalink / raw)
  To: olof, bleung, linux-kernel; +Cc: lee.jones, Shawn Nematbakhsh, Thierry Escande

From: Shawn Nematbakhsh <shawnn@chromium.org>

Call common functions for read / write to prepare support for future
LPC protocol variants which use different I/O ops than inb / outb.

Signed-off-by: Shawn Nematbakhsh <shawnn@chromium.org>
Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
---
 drivers/platform/chrome/Makefile          |  3 +-
 drivers/platform/chrome/cros_ec_lpc.c     | 88 +++++++++++++------------------
 drivers/platform/chrome/cros_ec_lpc_reg.c | 64 ++++++++++++++++++++++
 include/linux/mfd/cros_ec_lpc_reg.h       | 47 +++++++++++++++++
 4 files changed, 151 insertions(+), 51 deletions(-)
 create mode 100644 drivers/platform/chrome/cros_ec_lpc_reg.c
 create mode 100644 include/linux/mfd/cros_ec_lpc_reg.h

diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
index 3870afe..61182fd 100644
--- a/drivers/platform/chrome/Makefile
+++ b/drivers/platform/chrome/Makefile
@@ -5,6 +5,7 @@ cros_ec_devs-objs			:= cros_ec_dev.o cros_ec_sysfs.o \
 					   cros_ec_lightbar.o cros_ec_vbc.o \
 					   cros_ec_debugfs.o
 obj-$(CONFIG_CROS_EC_CHARDEV)		+= cros_ec_devs.o
-obj-$(CONFIG_CROS_EC_LPC)		+= cros_ec_lpc.o
+cros_ec_lpcs-objs			:= cros_ec_lpc.o cros_ec_lpc_reg.o
+obj-$(CONFIG_CROS_EC_LPC)		+= cros_ec_lpcs.o
 obj-$(CONFIG_CROS_EC_PROTO)		+= cros_ec_proto.o
 obj-$(CONFIG_CROS_KBD_LED_BACKLIGHT)	+= cros_kbd_led_backlight.o
diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
index f9a2454..6a782a6 100644
--- a/drivers/platform/chrome/cros_ec_lpc.c
+++ b/drivers/platform/chrome/cros_ec_lpc.c
@@ -26,19 +26,22 @@
 #include <linux/io.h>
 #include <linux/mfd/cros_ec.h>
 #include <linux/mfd/cros_ec_commands.h>
+#include <linux/mfd/cros_ec_lpc_reg.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/printk.h>
 
-#define DRV_NAME "cros_ec_lpc"
+#define DRV_NAME "cros_ec_lpcs"
 
 static int ec_response_timed_out(void)
 {
 	unsigned long one_second = jiffies + HZ;
+	u8 data;
 
 	usleep_range(200, 300);
 	do {
-		if (!(inb(EC_LPC_ADDR_HOST_CMD) & EC_LPC_STATUS_BUSY_MASK))
+		if (!(cros_ec_lpc_read_bytes(EC_LPC_ADDR_HOST_CMD, 1, &data) &
+		    EC_LPC_STATUS_BUSY_MASK))
 			return 0;
 		usleep_range(100, 200);
 	} while (time_before(jiffies, one_second));
@@ -51,21 +54,20 @@ static int cros_ec_pkt_xfer_lpc(struct cros_ec_device *ec,
 {
 	struct ec_host_request *request;
 	struct ec_host_response response;
-	u8 sum = 0;
-	int i;
+	u8 sum;
 	int ret = 0;
 	u8 *dout;
 
 	ret = cros_ec_prepare_tx(ec, msg);
 
 	/* Write buffer */
-	for (i = 0; i < ret; i++)
-		outb(ec->dout[i], EC_LPC_ADDR_HOST_PACKET + i);
+	cros_ec_lpc_write_bytes(EC_LPC_ADDR_HOST_PACKET, ret, ec->dout);
 
 	request = (struct ec_host_request *)ec->dout;
 
 	/* Here we go */
-	outb(EC_COMMAND_PROTOCOL_3, EC_LPC_ADDR_HOST_CMD);
+	sum = EC_COMMAND_PROTOCOL_3;
+	cros_ec_lpc_write_bytes(EC_LPC_ADDR_HOST_CMD, 1, &sum);
 
 	if (ec_response_timed_out()) {
 		dev_warn(ec->dev, "EC responsed timed out\n");
@@ -74,17 +76,15 @@ static int cros_ec_pkt_xfer_lpc(struct cros_ec_device *ec,
 	}
 
 	/* Check result */
-	msg->result = inb(EC_LPC_ADDR_HOST_DATA);
+	msg->result = cros_ec_lpc_read_bytes(EC_LPC_ADDR_HOST_DATA, 1, &sum);
 	ret = cros_ec_check_result(ec, msg);
 	if (ret)
 		goto done;
 
 	/* Read back response */
 	dout = (u8 *)&response;
-	for (i = 0; i < sizeof(response); i++) {
-		dout[i] = inb(EC_LPC_ADDR_HOST_PACKET + i);
-		sum += dout[i];
-	}
+	sum = cros_ec_lpc_read_bytes(EC_LPC_ADDR_HOST_PACKET, sizeof(response),
+				     dout);
 
 	msg->result = response.result;
 
@@ -97,11 +97,9 @@ static int cros_ec_pkt_xfer_lpc(struct cros_ec_device *ec,
 	}
 
 	/* Read response and process checksum */
-	for (i = 0; i < response.data_len; i++) {
-		msg->data[i] =
-			inb(EC_LPC_ADDR_HOST_PACKET + sizeof(response) + i);
-		sum += msg->data[i];
-	}
+	sum += cros_ec_lpc_read_bytes(EC_LPC_ADDR_HOST_PACKET +
+				      sizeof(response), response.data_len,
+				      msg->data);
 
 	if (sum) {
 		dev_err(ec->dev,
@@ -121,8 +119,7 @@ static int cros_ec_cmd_xfer_lpc(struct cros_ec_device *ec,
 				struct cros_ec_command *msg)
 {
 	struct ec_lpc_host_args args;
-	int csum;
-	int i;
+	u8 sum;
 	int ret = 0;
 
 	if (msg->outsize > EC_PROTO2_MAX_PARAM_SIZE ||
@@ -139,24 +136,20 @@ static int cros_ec_cmd_xfer_lpc(struct cros_ec_device *ec,
 	args.data_size = msg->outsize;
 
 	/* Initialize checksum */
-	csum = msg->command + args.flags +
-		args.command_version + args.data_size;
+	sum = msg->command + args.flags + args.command_version + args.data_size;
 
 	/* Copy data and update checksum */
-	for (i = 0; i < msg->outsize; i++) {
-		outb(msg->data[i], EC_LPC_ADDR_HOST_PARAM + i);
-		csum += msg->data[i];
-	}
+	sum += cros_ec_lpc_write_bytes(EC_LPC_ADDR_HOST_PARAM, msg->outsize,
+				       msg->data);
 
 	/* Finalize checksum and write args */
-	args.checksum = csum & 0xFF;
-	outb(args.flags, EC_LPC_ADDR_HOST_ARGS);
-	outb(args.command_version, EC_LPC_ADDR_HOST_ARGS + 1);
-	outb(args.data_size, EC_LPC_ADDR_HOST_ARGS + 2);
-	outb(args.checksum, EC_LPC_ADDR_HOST_ARGS + 3);
+	args.checksum = sum;
+	cros_ec_lpc_write_bytes(EC_LPC_ADDR_HOST_ARGS, sizeof(args),
+				(u8 *)&args);
 
 	/* Here we go */
-	outb(msg->command, EC_LPC_ADDR_HOST_CMD);
+	sum = msg->command;
+	cros_ec_lpc_write_bytes(EC_LPC_ADDR_HOST_CMD, 1, &sum);
 
 	if (ec_response_timed_out()) {
 		dev_warn(ec->dev, "EC responsed timed out\n");
@@ -165,16 +158,14 @@ static int cros_ec_cmd_xfer_lpc(struct cros_ec_device *ec,
 	}
 
 	/* Check result */
-	msg->result = inb(EC_LPC_ADDR_HOST_DATA);
+	msg->result = cros_ec_lpc_read_bytes(EC_LPC_ADDR_HOST_DATA, 1, &sum);
 	ret = cros_ec_check_result(ec, msg);
 	if (ret)
 		goto done;
 
 	/* Read back args */
-	args.flags = inb(EC_LPC_ADDR_HOST_ARGS);
-	args.command_version = inb(EC_LPC_ADDR_HOST_ARGS + 1);
-	args.data_size = inb(EC_LPC_ADDR_HOST_ARGS + 2);
-	args.checksum = inb(EC_LPC_ADDR_HOST_ARGS + 3);
+	cros_ec_lpc_read_bytes(EC_LPC_ADDR_HOST_ARGS, sizeof(args),
+			       (u8 *)&args);
 
 	if (args.data_size > msg->insize) {
 		dev_err(ec->dev,
@@ -185,20 +176,17 @@ static int cros_ec_cmd_xfer_lpc(struct cros_ec_device *ec,
 	}
 
 	/* Start calculating response checksum */
-	csum = msg->command + args.flags +
-		args.command_version + args.data_size;
+	sum = msg->command + args.flags + args.command_version + args.data_size;
 
 	/* Read response and update checksum */
-	for (i = 0; i < args.data_size; i++) {
-		msg->data[i] = inb(EC_LPC_ADDR_HOST_PARAM + i);
-		csum += msg->data[i];
-	}
+	sum += cros_ec_lpc_read_bytes(EC_LPC_ADDR_HOST_PARAM, args.data_size,
+				      msg->data);
 
 	/* Verify checksum */
-	if (args.checksum != (csum & 0xFF)) {
+	if (args.checksum != sum) {
 		dev_err(ec->dev,
 			"bad packet checksum, expected %02x, got %02x\n",
-			args.checksum, csum & 0xFF);
+			args.checksum, sum);
 		ret = -EBADMSG;
 		goto done;
 	}
@@ -222,14 +210,13 @@ static int cros_ec_lpc_readmem(struct cros_ec_device *ec, unsigned int offset,
 
 	/* fixed length */
 	if (bytes) {
-		for (; cnt < bytes; i++, s++, cnt++)
-			*s = inb(EC_LPC_ADDR_MEMMAP + i);
-		return cnt;
+		cros_ec_lpc_read_bytes(EC_LPC_ADDR_MEMMAP + offset, bytes, s);
+		return bytes;
 	}
 
 	/* string */
 	for (; i < EC_MEMMAP_SIZE; i++, s++) {
-		*s = inb(EC_LPC_ADDR_MEMMAP + i);
+		cros_ec_lpc_read_bytes(EC_LPC_ADDR_MEMMAP + i, 1, s);
 		cnt++;
 		if (!*s)
 			break;
@@ -242,6 +229,7 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct cros_ec_device *ec_dev;
+	u8 buf[2];
 	int ret;
 
 	if (!devm_request_region(dev, EC_LPC_ADDR_MEMMAP, EC_MEMMAP_SIZE,
@@ -250,8 +238,8 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
 		return -EBUSY;
 	}
 
-	if ((inb(EC_LPC_ADDR_MEMMAP + EC_MEMMAP_ID) != 'E') ||
-	    (inb(EC_LPC_ADDR_MEMMAP + EC_MEMMAP_ID + 1) != 'C')) {
+	cros_ec_lpc_read_bytes(EC_LPC_ADDR_MEMMAP + EC_MEMMAP_ID, 2, buf);
+	if (buf[0] != 'E' || buf[1] != 'C') {
 		dev_err(dev, "EC ID not detected\n");
 		return -ENODEV;
 	}
diff --git a/drivers/platform/chrome/cros_ec_lpc_reg.c b/drivers/platform/chrome/cros_ec_lpc_reg.c
new file mode 100644
index 0000000..03c9781
--- /dev/null
+++ b/drivers/platform/chrome/cros_ec_lpc_reg.c
@@ -0,0 +1,64 @@
+/*
+ * cros_ec_lpc_reg - LPC access to the Chrome OS Embedded Controller
+ *
+ * Copyright (C) 2016 Google, Inc
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * This driver uses the Chrome OS EC byte-level message-based protocol for
+ * communicating the keyboard state (which keys are pressed) from a keyboard EC
+ * to the AP over some bus (such as i2c, lpc, spi).  The EC does debouncing,
+ * but everything else (including deghosting) is done here.  The main
+ * motivation for this is to keep the EC firmware as simple as possible, since
+ * it cannot be easily upgraded and EC flash/IRAM space is relatively
+ * expensive.
+ */
+
+#include <linux/io.h>
+#include <linux/mfd/cros_ec.h>
+#include <linux/mfd/cros_ec_commands.h>
+
+static u8 lpc_read_bytes(unsigned int offset, unsigned int length, u8 *dest)
+{
+	int i;
+	int sum = 0;
+
+	for (i = 0; i < length; ++i) {
+		dest[i] = inb(offset + i);
+		sum += dest[i];
+	}
+
+	/* Return checksum of all bytes read */
+	return sum;
+}
+
+static u8 lpc_write_bytes(unsigned int offset, unsigned int length, u8 *msg)
+{
+	int i;
+	int sum = 0;
+
+	for (i = 0; i < length; ++i) {
+		outb(msg[i], offset + i);
+		sum += msg[i];
+	}
+
+	/* Return checksum of all bytes written */
+	return sum;
+}
+
+u8 cros_ec_lpc_read_bytes(unsigned int offset, unsigned int length, u8 *dest)
+{
+	return lpc_read_bytes(offset, length, dest);
+}
+
+u8 cros_ec_lpc_write_bytes(unsigned int offset, unsigned int length, u8 *msg)
+{
+	return lpc_write_bytes(offset, length, msg);
+}
diff --git a/include/linux/mfd/cros_ec_lpc_reg.h b/include/linux/mfd/cros_ec_lpc_reg.h
new file mode 100644
index 0000000..4089bd5
--- /dev/null
+++ b/include/linux/mfd/cros_ec_lpc_reg.h
@@ -0,0 +1,47 @@
+/*
+ * cros_ec_lpc_reg - LPC access to the Chrome OS Embedded Controller
+ *
+ * Copyright (C) 2016 Google, Inc
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * This driver uses the Chrome OS EC byte-level message-based protocol for
+ * communicating the keyboard state (which keys are pressed) from a keyboard EC
+ * to the AP over some bus (such as i2c, lpc, spi).  The EC does debouncing,
+ * but everything else (including deghosting) is done here.  The main
+ * motivation for this is to keep the EC firmware as simple as possible, since
+ * it cannot be easily upgraded and EC flash/IRAM space is relatively
+ * expensive.
+ */
+
+#ifndef __LINUX_MFD_CROS_EC_REG_H
+#define __LINUX_MFD_CROS_EC_REG_H
+
+/**
+ * cros_ec_lpc_read_bytes - Read bytes from a given LPC-mapped address.
+ * Returns 8-bit checksum of all bytes read.
+ *
+ * @offset: Base read address
+ * @length: Number of bytes to read
+ * @dest: Destination buffer
+ */
+u8 cros_ec_lpc_read_bytes(unsigned int offset, unsigned int length, u8 *dest);
+
+/**
+ * cros_ec_lpc_write_bytes - Write bytes to a given LPC-mapped address.
+ * Returns 8-bit checksum of all bytes written.
+ *
+ * @offset: Base write address
+ * @length: Number of bytes to write
+ * @msg: Write data buffer
+ */
+u8 cros_ec_lpc_write_bytes(unsigned int offset, unsigned int length, u8 *msg);
+
+#endif /* __LINUX_MFD_CROS_EC_REG_H */
-- 
2.9.3

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

* [PATCH RESEND 06/13] platform/chrome: cros_ec_lpc: Add support for mec1322 EC
  2017-05-16 16:13 [PATCH RESEND 00/13] platform/chrome: Add console debugfs, lpc and lightbar programming support Enric Balletbo i Serra
                   ` (4 preceding siblings ...)
  2017-05-16 16:13 ` [PATCH RESEND 05/13] platform/chrome: cros_ec_lpc: Add R/W helpers to LPC protocol variants Enric Balletbo i Serra
@ 2017-05-16 16:13 ` Enric Balletbo i Serra
  2017-05-22 11:09   ` Lee Jones
  2017-06-22 19:01   ` Benson Leung
  2017-05-16 16:13 ` [PATCH RESEND 07/13] platform/chrome: cros_ec_lpc: Add support for GOOG004 ACPI device Enric Balletbo i Serra
                   ` (6 subsequent siblings)
  12 siblings, 2 replies; 31+ messages in thread
From: Enric Balletbo i Serra @ 2017-05-16 16:13 UTC (permalink / raw)
  To: olof, bleung, linux-kernel; +Cc: lee.jones, Shawn Nematbakhsh, Thierry Escande

From: Shawn Nematbakhsh <shawnn@chromium.org>

This adds support for the ChromeOS LPC Microchip Embedded Controller
(mec1322) variant.

mec1322 accesses I/O region [800h, 9ffh] through embedded memory
interface (EMI) rather than LPC.

Signed-off-by: Shawn Nematbakhsh <shawnn@chromium.org>
Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
---
 drivers/platform/chrome/Kconfig           |  12 +++
 drivers/platform/chrome/Makefile          |   1 +
 drivers/platform/chrome/cros_ec_lpc.c     |   5 ++
 drivers/platform/chrome/cros_ec_lpc_mec.c | 140 ++++++++++++++++++++++++++++++
 drivers/platform/chrome/cros_ec_lpc_reg.c |  69 +++++++++++++++
 include/linux/mfd/cros_ec_lpc_mec.h       |  90 +++++++++++++++++++
 include/linux/mfd/cros_ec_lpc_reg.h       |  14 +++
 7 files changed, 331 insertions(+)
 create mode 100644 drivers/platform/chrome/cros_ec_lpc_mec.c
 create mode 100644 include/linux/mfd/cros_ec_lpc_mec.h

diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
index 76bdae1..6d80fb5 100644
--- a/drivers/platform/chrome/Kconfig
+++ b/drivers/platform/chrome/Kconfig
@@ -59,6 +59,18 @@ config CROS_EC_LPC
           To compile this driver as a module, choose M here: the
           module will be called cros_ec_lpc.
 
+config CROS_EC_LPC_MEC
+	bool "ChromeOS Embedded Controller LPC Microchip EC (MEC) variant"
+	depends on CROS_EC_LPC
+	default n
+	help
+	  If you say Y here, a variant LPC protocol for the Microchip EC
+	  will be used. Note that this variant is not backward compatible
+	  with non-Microchip ECs.
+
+	  If you have a ChromeOS Embedded Controller Microchip EC variant
+	  choose Y here.
+
 config CROS_EC_PROTO
         bool
         help
diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
index 61182fd..66c345c 100644
--- a/drivers/platform/chrome/Makefile
+++ b/drivers/platform/chrome/Makefile
@@ -6,6 +6,7 @@ cros_ec_devs-objs			:= cros_ec_dev.o cros_ec_sysfs.o \
 					   cros_ec_debugfs.o
 obj-$(CONFIG_CROS_EC_CHARDEV)		+= cros_ec_devs.o
 cros_ec_lpcs-objs			:= cros_ec_lpc.o cros_ec_lpc_reg.o
+cros_ec_lpcs-$(CONFIG_CROS_EC_LPC_MEC)	+= cros_ec_lpc_mec.o
 obj-$(CONFIG_CROS_EC_LPC)		+= cros_ec_lpcs.o
 obj-$(CONFIG_CROS_EC_PROTO)		+= cros_ec_proto.o
 obj-$(CONFIG_CROS_KBD_LED_BACKLIGHT)	+= cros_kbd_led_backlight.o
diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
index 6a782a6..bc2dc62 100644
--- a/drivers/platform/chrome/cros_ec_lpc.c
+++ b/drivers/platform/chrome/cros_ec_lpc.c
@@ -346,10 +346,13 @@ static int __init cros_ec_lpc_init(void)
 		return -ENODEV;
 	}
 
+	cros_ec_lpc_reg_init();
+
 	/* Register the driver */
 	ret = platform_driver_register(&cros_ec_lpc_driver);
 	if (ret) {
 		pr_err(DRV_NAME ": can't register driver: %d\n", ret);
+		cros_ec_lpc_reg_destroy();
 		return ret;
 	}
 
@@ -358,6 +361,7 @@ static int __init cros_ec_lpc_init(void)
 	if (ret) {
 		pr_err(DRV_NAME ": can't register device: %d\n", ret);
 		platform_driver_unregister(&cros_ec_lpc_driver);
+		cros_ec_lpc_reg_destroy();
 		return ret;
 	}
 
@@ -368,6 +372,7 @@ static void __exit cros_ec_lpc_exit(void)
 {
 	platform_device_unregister(&cros_ec_lpc_device);
 	platform_driver_unregister(&cros_ec_lpc_driver);
+	cros_ec_lpc_reg_destroy();
 }
 
 module_init(cros_ec_lpc_init);
diff --git a/drivers/platform/chrome/cros_ec_lpc_mec.c b/drivers/platform/chrome/cros_ec_lpc_mec.c
new file mode 100644
index 0000000..2eda2c2
--- /dev/null
+++ b/drivers/platform/chrome/cros_ec_lpc_mec.c
@@ -0,0 +1,140 @@
+/*
+ * cros_ec_lpc_mec - LPC variant I/O for Microchip EC
+ *
+ * Copyright (C) 2016 Google, Inc
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * This driver uses the Chrome OS EC byte-level message-based protocol for
+ * communicating the keyboard state (which keys are pressed) from a keyboard EC
+ * to the AP over some bus (such as i2c, lpc, spi).  The EC does debouncing,
+ * but everything else (including deghosting) is done here.  The main
+ * motivation for this is to keep the EC firmware as simple as possible, since
+ * it cannot be easily upgraded and EC flash/IRAM space is relatively
+ * expensive.
+ */
+
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/mfd/cros_ec_commands.h>
+#include <linux/mfd/cros_ec_lpc_mec.h>
+#include <linux/mutex.h>
+#include <linux/types.h>
+
+/*
+ * This mutex must be held while accessing the EMI unit. We can't rely on the
+ * EC mutex because memmap data may be accessed without it being held.
+ */
+static struct mutex io_mutex;
+
+/*
+ * cros_ec_lpc_mec_emi_write_address
+ *
+ * Initialize EMI read / write at a given address.
+ *
+ * @addr:        Starting read / write address
+ * @access_type: Type of access, typically 32-bit auto-increment
+ */
+static void cros_ec_lpc_mec_emi_write_address(u16 addr,
+			enum cros_ec_lpc_mec_emi_access_mode access_type)
+{
+	/* Address relative to start of EMI range */
+	addr -= MEC_EMI_RANGE_START;
+	outb((addr & 0xfc) | access_type, MEC_EMI_EC_ADDRESS_B0);
+	outb((addr >> 8) & 0x7f, MEC_EMI_EC_ADDRESS_B1);
+}
+
+/*
+ * cros_ec_lpc_io_bytes_mec - Read / write bytes to MEC EMI port
+ *
+ * @io_type: MEC_IO_READ or MEC_IO_WRITE, depending on request
+ * @offset:  Base read / write address
+ * @length:  Number of bytes to read / write
+ * @buf:     Destination / source buffer
+ *
+ * @return 8-bit checksum of all bytes read / written
+ */
+u8 cros_ec_lpc_io_bytes_mec(enum cros_ec_lpc_mec_io_type io_type,
+			    unsigned int offset, unsigned int length,
+			    u8 *buf)
+{
+	int i = 0;
+	int io_addr;
+	u8 sum = 0;
+	enum cros_ec_lpc_mec_emi_access_mode access, new_access;
+
+	/*
+	 * Long access cannot be used on misaligned data since reading B0 loads
+	 * the data register and writing B3 flushes.
+	 */
+	if (offset & 0x3 || length < 4)
+		access = ACCESS_TYPE_BYTE;
+	else
+		access = ACCESS_TYPE_LONG_AUTO_INCREMENT;
+
+	mutex_lock(&io_mutex);
+
+	/* Initialize I/O at desired address */
+	cros_ec_lpc_mec_emi_write_address(offset, access);
+
+	/* Skip bytes in case of misaligned offset */
+	io_addr = MEC_EMI_EC_DATA_B0 + (offset & 0x3);
+	while (i < length) {
+		while (io_addr <= MEC_EMI_EC_DATA_B3) {
+			if (io_type == MEC_IO_READ)
+				buf[i] = inb(io_addr++);
+			else
+				outb(buf[i], io_addr++);
+
+			sum += buf[i++];
+			offset++;
+
+			/* Extra bounds check in case of misaligned length */
+			if (i == length)
+				goto done;
+		}
+
+		/*
+		 * Use long auto-increment access except for misaligned write,
+		 * since writing B3 triggers the flush.
+		 */
+		if (length - i < 4 && io_type == MEC_IO_WRITE)
+			new_access = ACCESS_TYPE_BYTE;
+		else
+			new_access = ACCESS_TYPE_LONG_AUTO_INCREMENT;
+
+		if (new_access != access ||
+		    access != ACCESS_TYPE_LONG_AUTO_INCREMENT) {
+			access = new_access;
+			cros_ec_lpc_mec_emi_write_address(offset, access);
+		}
+
+		/* Access [B0, B3] on each loop pass */
+		io_addr = MEC_EMI_EC_DATA_B0;
+	}
+
+done:
+	mutex_unlock(&io_mutex);
+
+	return sum;
+}
+EXPORT_SYMBOL(cros_ec_lpc_io_bytes_mec);
+
+void cros_ec_lpc_mec_init(void)
+{
+	mutex_init(&io_mutex);
+}
+EXPORT_SYMBOL(cros_ec_lpc_mec_init);
+
+void cros_ec_lpc_mec_destroy(void)
+{
+	mutex_destroy(&io_mutex);
+}
+EXPORT_SYMBOL(cros_ec_lpc_mec_destroy);
diff --git a/drivers/platform/chrome/cros_ec_lpc_reg.c b/drivers/platform/chrome/cros_ec_lpc_reg.c
index 03c9781..dcc7a3e 100644
--- a/drivers/platform/chrome/cros_ec_lpc_reg.c
+++ b/drivers/platform/chrome/cros_ec_lpc_reg.c
@@ -24,6 +24,7 @@
 #include <linux/io.h>
 #include <linux/mfd/cros_ec.h>
 #include <linux/mfd/cros_ec_commands.h>
+#include <linux/mfd/cros_ec_lpc_mec.h>
 
 static u8 lpc_read_bytes(unsigned int offset, unsigned int length, u8 *dest)
 {
@@ -53,12 +54,80 @@ static u8 lpc_write_bytes(unsigned int offset, unsigned int length, u8 *msg)
 	return sum;
 }
 
+#ifdef CONFIG_CROS_EC_LPC_MEC
+
 u8 cros_ec_lpc_read_bytes(unsigned int offset, unsigned int length, u8 *dest)
 {
+	if (length == 0)
+		return 0;
+
+	/* Access desired range through EMI interface */
+	if (offset >= MEC_EMI_RANGE_START && offset <= MEC_EMI_RANGE_END) {
+		/* Ensure we don't straddle EMI region */
+		if (WARN_ON(offset + length - 1 > MEC_EMI_RANGE_END))
+			return 0;
+
+		return cros_ec_lpc_io_bytes_mec(MEC_IO_READ, offset, length,
+						dest);
+	}
+
+	if (WARN_ON(offset + length > MEC_EMI_RANGE_START &&
+		    offset < MEC_EMI_RANGE_START))
+		return 0;
+
 	return lpc_read_bytes(offset, length, dest);
 }
 
 u8 cros_ec_lpc_write_bytes(unsigned int offset, unsigned int length, u8 *msg)
 {
+	if (length == 0)
+		return 0;
+
+	/* Access desired range through EMI interface */
+	if (offset >= MEC_EMI_RANGE_START && offset <= MEC_EMI_RANGE_END) {
+		/* Ensure we don't straddle EMI region */
+		if (WARN_ON(offset + length - 1 > MEC_EMI_RANGE_END))
+			return 0;
+
+		return cros_ec_lpc_io_bytes_mec(MEC_IO_WRITE, offset, length,
+						msg);
+	}
+
+	if (WARN_ON(offset + length > MEC_EMI_RANGE_START &&
+		    offset < MEC_EMI_RANGE_START))
+		return 0;
+
 	return lpc_write_bytes(offset, length, msg);
 }
+
+void cros_ec_lpc_reg_init(void)
+{
+	cros_ec_lpc_mec_init();
+}
+
+void cros_ec_lpc_reg_destroy(void)
+{
+	cros_ec_lpc_mec_destroy();
+}
+
+#else /* CONFIG_CROS_EC_LPC_MEC */
+
+u8 cros_ec_lpc_read_bytes(unsigned int offset, unsigned int length, u8 *dest)
+{
+	return lpc_read_bytes(offset, length, dest);
+}
+
+u8 cros_ec_lpc_write_bytes(unsigned int offset, unsigned int length, u8 *msg)
+{
+	return lpc_write_bytes(offset, length, msg);
+}
+
+void cros_ec_lpc_reg_init(void)
+{
+}
+
+void cros_ec_lpc_reg_destroy(void)
+{
+}
+
+#endif /* CONFIG_CROS_EC_LPC_MEC */
diff --git a/include/linux/mfd/cros_ec_lpc_mec.h b/include/linux/mfd/cros_ec_lpc_mec.h
new file mode 100644
index 0000000..176496d
--- /dev/null
+++ b/include/linux/mfd/cros_ec_lpc_mec.h
@@ -0,0 +1,90 @@
+/*
+ * cros_ec_lpc_mec - LPC variant I/O for Microchip EC
+ *
+ * Copyright (C) 2016 Google, Inc
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * This driver uses the Chrome OS EC byte-level message-based protocol for
+ * communicating the keyboard state (which keys are pressed) from a keyboard EC
+ * to the AP over some bus (such as i2c, lpc, spi).  The EC does debouncing,
+ * but everything else (including deghosting) is done here.  The main
+ * motivation for this is to keep the EC firmware as simple as possible, since
+ * it cannot be easily upgraded and EC flash/IRAM space is relatively
+ * expensive.
+ */
+
+#ifndef __LINUX_MFD_CROS_EC_MEC_H
+#define __LINUX_MFD_CROS_EC_MEC_H
+
+#include <linux/mfd/cros_ec_commands.h>
+
+enum cros_ec_lpc_mec_emi_access_mode {
+	/* 8-bit access */
+	ACCESS_TYPE_BYTE = 0x0,
+	/* 16-bit access */
+	ACCESS_TYPE_WORD = 0x1,
+	/* 32-bit access */
+	ACCESS_TYPE_LONG = 0x2,
+	/*
+	 * 32-bit access, read or write of MEC_EMI_EC_DATA_B3 causes the
+	 * EC data register to be incremented.
+	 */
+	ACCESS_TYPE_LONG_AUTO_INCREMENT = 0x3,
+};
+
+enum cros_ec_lpc_mec_io_type {
+	MEC_IO_READ,
+	MEC_IO_WRITE,
+};
+
+/* Access IO ranges 0x800 thru 0x9ff using EMI interface instead of LPC */
+#define MEC_EMI_RANGE_START EC_HOST_CMD_REGION0
+#define MEC_EMI_RANGE_END   (EC_LPC_ADDR_MEMMAP + EC_MEMMAP_SIZE)
+
+/* EMI registers are relative to base */
+#define MEC_EMI_BASE 0x800
+#define MEC_EMI_HOST_TO_EC (MEC_EMI_BASE + 0)
+#define MEC_EMI_EC_TO_HOST (MEC_EMI_BASE + 1)
+#define MEC_EMI_EC_ADDRESS_B0 (MEC_EMI_BASE + 2)
+#define MEC_EMI_EC_ADDRESS_B1 (MEC_EMI_BASE + 3)
+#define MEC_EMI_EC_DATA_B0 (MEC_EMI_BASE + 4)
+#define MEC_EMI_EC_DATA_B1 (MEC_EMI_BASE + 5)
+#define MEC_EMI_EC_DATA_B2 (MEC_EMI_BASE + 6)
+#define MEC_EMI_EC_DATA_B3 (MEC_EMI_BASE + 7)
+
+/*
+ * cros_ec_lpc_mec_init
+ *
+ * Initialize MEC I/O.
+ */
+void cros_ec_lpc_mec_init(void);
+
+/*
+ * cros_ec_lpc_mec_destroy
+ *
+ * Cleanup MEC I/O.
+ */
+void cros_ec_lpc_mec_destroy(void);
+
+/**
+ * cros_ec_lpc_io_bytes_mec - Read / write bytes to MEC EMI port
+ *
+ * @io_type: MEC_IO_READ or MEC_IO_WRITE, depending on request
+ * @offset:  Base read / write address
+ * @length:  Number of bytes to read / write
+ * @buf:     Destination / source buffer
+ *
+ * @return 8-bit checksum of all bytes read / written
+ */
+u8 cros_ec_lpc_io_bytes_mec(enum cros_ec_lpc_mec_io_type io_type,
+			    unsigned int offset, unsigned int length, u8 *buf);
+
+#endif /* __LINUX_MFD_CROS_EC_MEC_H */
diff --git a/include/linux/mfd/cros_ec_lpc_reg.h b/include/linux/mfd/cros_ec_lpc_reg.h
index 4089bd5..5560bef 100644
--- a/include/linux/mfd/cros_ec_lpc_reg.h
+++ b/include/linux/mfd/cros_ec_lpc_reg.h
@@ -44,4 +44,18 @@ u8 cros_ec_lpc_read_bytes(unsigned int offset, unsigned int length, u8 *dest);
  */
 u8 cros_ec_lpc_write_bytes(unsigned int offset, unsigned int length, u8 *msg);
 
+/**
+ * cros_ec_lpc_reg_init
+ *
+ * Initialize register I/O.
+ */
+void cros_ec_lpc_reg_init(void);
+
+/**
+ * cros_ec_lpc_reg_destroy
+ *
+ * Cleanup reg I/O.
+ */
+void cros_ec_lpc_reg_destroy(void);
+
 #endif /* __LINUX_MFD_CROS_EC_REG_H */
-- 
2.9.3

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

* [PATCH RESEND 07/13] platform/chrome: cros_ec_lpc: Add support for GOOG004 ACPI device
  2017-05-16 16:13 [PATCH RESEND 00/13] platform/chrome: Add console debugfs, lpc and lightbar programming support Enric Balletbo i Serra
                   ` (5 preceding siblings ...)
  2017-05-16 16:13 ` [PATCH RESEND 06/13] platform/chrome: cros_ec_lpc: Add support for mec1322 EC Enric Balletbo i Serra
@ 2017-05-16 16:13 ` Enric Balletbo i Serra
  2017-06-22 19:39   ` Benson Leung
  2017-05-16 16:13 ` [PATCH RESEND 08/13] platform/chrome: cros_ec_lpc: Add power management ops Enric Balletbo i Serra
                   ` (5 subsequent siblings)
  12 siblings, 1 reply; 31+ messages in thread
From: Enric Balletbo i Serra @ 2017-05-16 16:13 UTC (permalink / raw)
  To: olof, bleung, linux-kernel; +Cc: lee.jones, Gwendal Grignou, Thierry Escande

From: Gwendal Grignou <gwendal@chromium.org>

This patch removes platform_device_register() call and adds an ACPI
device id structure. The driver is now automatically probed for devices
with a GOOG0004 ACPI entry.

Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
---
 drivers/platform/chrome/Kconfig       |  2 +-
 drivers/platform/chrome/cros_ec_lpc.c | 23 +++++++++--------------
 2 files changed, 10 insertions(+), 15 deletions(-)

diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
index 6d80fb5..0ad6e29 100644
--- a/drivers/platform/chrome/Kconfig
+++ b/drivers/platform/chrome/Kconfig
@@ -49,7 +49,7 @@ config CROS_EC_CHARDEV
 
 config CROS_EC_LPC
         tristate "ChromeOS Embedded Controller (LPC)"
-        depends on MFD_CROS_EC && (X86 || COMPILE_TEST)
+        depends on MFD_CROS_EC && ACPI && (X86 || COMPILE_TEST)
         help
           If you say Y here, you get support for talking to the ChromeOS EC
           over an LPC bus. This uses a simple byte-level protocol with a
diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
index bc2dc62..90703521 100644
--- a/drivers/platform/chrome/cros_ec_lpc.c
+++ b/drivers/platform/chrome/cros_ec_lpc.c
@@ -21,6 +21,7 @@
  * expensive.
  */
 
+#include <linux/acpi.h>
 #include <linux/dmi.h>
 #include <linux/delay.h>
 #include <linux/io.h>
@@ -32,6 +33,7 @@
 #include <linux/printk.h>
 
 #define DRV_NAME "cros_ec_lpcs"
+#define ACPI_DRV_NAME "GOOG0004"
 
 static int ec_response_timed_out(void)
 {
@@ -288,6 +290,12 @@ static int cros_ec_lpc_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct acpi_device_id cros_ec_lpc_acpi_device_ids[] = {
+	{ ACPI_DRV_NAME, 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(acpi, cros_ec_lpc_acpi_device_ids);
+
 static struct dmi_system_id cros_ec_lpc_dmi_table[] __initdata = {
 	{
 		/*
@@ -328,15 +336,12 @@ MODULE_DEVICE_TABLE(dmi, cros_ec_lpc_dmi_table);
 static struct platform_driver cros_ec_lpc_driver = {
 	.driver = {
 		.name = DRV_NAME,
+		.acpi_match_table = cros_ec_lpc_acpi_device_ids,
 	},
 	.probe = cros_ec_lpc_probe,
 	.remove = cros_ec_lpc_remove,
 };
 
-static struct platform_device cros_ec_lpc_device = {
-	.name = DRV_NAME
-};
-
 static int __init cros_ec_lpc_init(void)
 {
 	int ret;
@@ -356,21 +361,11 @@ static int __init cros_ec_lpc_init(void)
 		return ret;
 	}
 
-	/* Register the device, and it'll get hooked up automatically */
-	ret = platform_device_register(&cros_ec_lpc_device);
-	if (ret) {
-		pr_err(DRV_NAME ": can't register device: %d\n", ret);
-		platform_driver_unregister(&cros_ec_lpc_driver);
-		cros_ec_lpc_reg_destroy();
-		return ret;
-	}
-
 	return 0;
 }
 
 static void __exit cros_ec_lpc_exit(void)
 {
-	platform_device_unregister(&cros_ec_lpc_device);
 	platform_driver_unregister(&cros_ec_lpc_driver);
 	cros_ec_lpc_reg_destroy();
 }
-- 
2.9.3

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

* [PATCH RESEND 08/13] platform/chrome: cros_ec_lpc: Add power management ops
  2017-05-16 16:13 [PATCH RESEND 00/13] platform/chrome: Add console debugfs, lpc and lightbar programming support Enric Balletbo i Serra
                   ` (6 preceding siblings ...)
  2017-05-16 16:13 ` [PATCH RESEND 07/13] platform/chrome: cros_ec_lpc: Add support for GOOG004 ACPI device Enric Balletbo i Serra
@ 2017-05-16 16:13 ` Enric Balletbo i Serra
  2017-06-22 19:46   ` Benson Leung
  2017-05-16 16:13 ` [PATCH RESEND 09/13] platform/chrome: cros_ec_lpc: Add MKBP events support over ACPI Enric Balletbo i Serra
                   ` (4 subsequent siblings)
  12 siblings, 1 reply; 31+ messages in thread
From: Enric Balletbo i Serra @ 2017-05-16 16:13 UTC (permalink / raw)
  To: olof, bleung, linux-kernel; +Cc: lee.jones, Archana Patni, Thierry Escande

From: Archana Patni <archana.patni@intel.com>

This patch adds suspend and resume pm ops to the LPC ChromeOS EC driver.
These LPC handlers call the croc_ec generic handlers.

Signed-off-by: Archana Patni <archana.patni@intel.com>
Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
---
 drivers/platform/chrome/cros_ec_lpc.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
index 90703521..89afad7 100644
--- a/drivers/platform/chrome/cros_ec_lpc.c
+++ b/drivers/platform/chrome/cros_ec_lpc.c
@@ -333,10 +333,31 @@ static struct dmi_system_id cros_ec_lpc_dmi_table[] __initdata = {
 };
 MODULE_DEVICE_TABLE(dmi, cros_ec_lpc_dmi_table);
 
+#ifdef CONFIG_PM_SLEEP
+static int cros_ec_lpc_suspend(struct device *dev)
+{
+	struct cros_ec_device *ec_dev = dev_get_drvdata(dev);
+
+	return cros_ec_suspend(ec_dev);
+}
+
+static int cros_ec_lpc_resume(struct device *dev)
+{
+	struct cros_ec_device *ec_dev = dev_get_drvdata(dev);
+
+	return cros_ec_resume(ec_dev);
+}
+#endif
+
+const struct dev_pm_ops cros_ec_lpc_pm_ops = {
+	SET_LATE_SYSTEM_SLEEP_PM_OPS(cros_ec_lpc_suspend, cros_ec_lpc_resume)
+};
+
 static struct platform_driver cros_ec_lpc_driver = {
 	.driver = {
 		.name = DRV_NAME,
 		.acpi_match_table = cros_ec_lpc_acpi_device_ids,
+		.pm = &cros_ec_lpc_pm_ops,
 	},
 	.probe = cros_ec_lpc_probe,
 	.remove = cros_ec_lpc_remove,
-- 
2.9.3

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

* [PATCH RESEND 09/13] platform/chrome: cros_ec_lpc: Add MKBP events support over ACPI
  2017-05-16 16:13 [PATCH RESEND 00/13] platform/chrome: Add console debugfs, lpc and lightbar programming support Enric Balletbo i Serra
                   ` (7 preceding siblings ...)
  2017-05-16 16:13 ` [PATCH RESEND 08/13] platform/chrome: cros_ec_lpc: Add power management ops Enric Balletbo i Serra
@ 2017-05-16 16:13 ` Enric Balletbo i Serra
  2017-06-22 19:35   ` Benson Leung
  2017-05-16 16:13 ` [PATCH RESEND 10/13] platform/chrome: cros_ec_lightbar - Add lightbar program feature to sysfs Enric Balletbo i Serra
                   ` (3 subsequent siblings)
  12 siblings, 1 reply; 31+ messages in thread
From: Enric Balletbo i Serra @ 2017-05-16 16:13 UTC (permalink / raw)
  To: olof, bleung, linux-kernel; +Cc: lee.jones, Gwendal Grignou, Thierry Escande

From: Gwendal Grignou <gwendal@chromium.org>

This patch installs a notify handler to process MKBP events for EC
firmware directing them over ACPI.

Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
---
 drivers/platform/chrome/cros_ec_lpc.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
index 89afad7..eeb187e 100644
--- a/drivers/platform/chrome/cros_ec_lpc.c
+++ b/drivers/platform/chrome/cros_ec_lpc.c
@@ -227,9 +227,20 @@ static int cros_ec_lpc_readmem(struct cros_ec_device *ec, unsigned int offset,
 	return cnt;
 }
 
+static void cros_ec_lpc_acpi_notify(acpi_handle device, u32 value, void *data)
+{
+	struct cros_ec_device *ec_dev = data;
+
+	if (ec_dev->mkbp_event_supported && cros_ec_get_next_event(ec_dev) > 0)
+		blocking_notifier_call_chain(&ec_dev->event_notifier, 0,
+					     ec_dev);
+}
+
 static int cros_ec_lpc_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
+	struct acpi_device *adev;
+	acpi_status status;
 	struct cros_ec_device *ec_dev;
 	u8 buf[2];
 	int ret;
@@ -277,12 +288,33 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	/*
+	 * Connect a notify handler to process MKBP messages if we have a
+	 * companion ACPI device.
+	 */
+	adev = ACPI_COMPANION(dev);
+	if (adev) {
+		status = acpi_install_notify_handler(adev->handle,
+						     ACPI_ALL_NOTIFY,
+						     cros_ec_lpc_acpi_notify,
+						     ec_dev);
+		if (ACPI_FAILURE(status))
+			dev_warn(dev, "Failed to register notifier %08x\n",
+				 status);
+	}
+
 	return 0;
 }
 
 static int cros_ec_lpc_remove(struct platform_device *pdev)
 {
 	struct cros_ec_device *ec_dev;
+	struct acpi_device *adev;
+
+	adev = ACPI_COMPANION(&pdev->dev);
+	if (adev)
+		acpi_remove_notify_handler(adev->handle, ACPI_ALL_NOTIFY,
+					   cros_ec_lpc_acpi_notify);
 
 	ec_dev = platform_get_drvdata(pdev);
 	cros_ec_remove(ec_dev);
-- 
2.9.3

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

* [PATCH RESEND 10/13] platform/chrome: cros_ec_lightbar - Add lightbar program feature to sysfs
  2017-05-16 16:13 [PATCH RESEND 00/13] platform/chrome: Add console debugfs, lpc and lightbar programming support Enric Balletbo i Serra
                   ` (8 preceding siblings ...)
  2017-05-16 16:13 ` [PATCH RESEND 09/13] platform/chrome: cros_ec_lpc: Add MKBP events support over ACPI Enric Balletbo i Serra
@ 2017-05-16 16:13 ` Enric Balletbo i Serra
  2017-06-22 23:43   ` Benson Leung
  2017-05-16 16:13 ` [PATCH RESEND 11/13] platform/chrome: cros_ec_lightbar - Control of suspend/resume lightbar sequence Enric Balletbo i Serra
                   ` (2 subsequent siblings)
  12 siblings, 1 reply; 31+ messages in thread
From: Enric Balletbo i Serra @ 2017-05-16 16:13 UTC (permalink / raw)
  To: olof, bleung, linux-kernel; +Cc: lee.jones, Eric Caruso, Guenter Roeck

From: Eric Caruso <ejcaruso@chromium.org>

Add a program feature so we can upload and run programs for lightbar
sequences. We should be able to use this to shift sequences out of the
EC and save space there.

  $ cat <suitable program bin> > /sys/devices/.../cros_ec/program
  $ echo program > /sys/devices/.../cros_ec/sequence

Signed-off-by: Eric Caruso <ejcaruso@chromium.org>
Signed-off-by: Guenter Roeck <groeck@chromium.org>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/platform/chrome/cros_ec_lightbar.c | 69 +++++++++++++++++++++++++++++-
 include/linux/mfd/cros_ec_commands.h       | 12 +++++-
 2 files changed, 79 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_lightbar.c b/drivers/platform/chrome/cros_ec_lightbar.c
index 8df3d44..2667505 100644
--- a/drivers/platform/chrome/cros_ec_lightbar.c
+++ b/drivers/platform/chrome/cros_ec_lightbar.c
@@ -295,7 +295,8 @@ static ssize_t led_rgb_store(struct device *dev, struct device_attribute *attr,
 
 static char const *seqname[] = {
 	"ERROR", "S5", "S3", "S0", "S5S3", "S3S0",
-	"S0S3", "S3S5", "STOP", "RUN", "PULSE", "TEST", "KONAMI",
+	"S0S3", "S3S5", "STOP", "RUN", "KONAMI",
+	"TAP", "PROGRAM",
 };
 
 static ssize_t sequence_show(struct device *dev,
@@ -390,6 +391,69 @@ static ssize_t sequence_store(struct device *dev, struct device_attribute *attr,
 	return ret;
 }
 
+static ssize_t program_store(struct device *dev, struct device_attribute *attr,
+			     const char *buf, size_t count)
+{
+	int extra_bytes, max_size, ret;
+	struct ec_params_lightbar *param;
+	struct cros_ec_command *msg;
+	struct cros_ec_dev *ec = container_of(dev, struct cros_ec_dev,
+					      class_dev);
+
+	/*
+	 * We might need to reject the program for size reasons. The EC
+	 * enforces a maximum program size, but we also don't want to try
+	 * and send a program that is too big for the protocol. In order
+	 * to ensure the latter, we also need to ensure we have extra bytes
+	 * to represent the rest of the packet.
+	 */
+	extra_bytes = sizeof(*param) - sizeof(param->set_program.data);
+	max_size = min(EC_LB_PROG_LEN, ec->ec_dev->max_request - extra_bytes);
+	if (count > max_size) {
+		dev_err(dev, "Program is %u bytes, too long to send (max: %u)",
+			(unsigned int)count, max_size);
+
+		return -EINVAL;
+	}
+
+	msg = alloc_lightbar_cmd_msg(ec);
+	if (!msg)
+		return -ENOMEM;
+
+	ret = lb_throttle();
+	if (ret)
+		goto exit;
+
+	dev_info(dev, "Copying %zu byte program to EC", count);
+
+	param = (struct ec_params_lightbar *)msg->data;
+	param->cmd = LIGHTBAR_CMD_SET_PROGRAM;
+
+	param->set_program.size = count;
+	memcpy(param->set_program.data, buf, count);
+
+	/*
+	 * We need to set the message size manually or else it will use
+	 * EC_LB_PROG_LEN. This might be too long, and the program
+	 * is unlikely to use all of the space.
+	 */
+	msg->outsize = count + extra_bytes;
+
+	ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
+	if (ret < 0)
+		goto exit;
+	if (msg->result != EC_RES_SUCCESS) {
+		ret = -EINVAL;
+		goto exit;
+	}
+
+	ret = count;
+exit:
+	kfree(msg);
+
+	return ret;
+}
+
 /* Module initialization */
 
 static DEVICE_ATTR_RW(interval_msec);
@@ -397,12 +461,15 @@ static DEVICE_ATTR_RO(version);
 static DEVICE_ATTR_WO(brightness);
 static DEVICE_ATTR_WO(led_rgb);
 static DEVICE_ATTR_RW(sequence);
+static DEVICE_ATTR_WO(program);
+
 static struct attribute *__lb_cmds_attrs[] = {
 	&dev_attr_interval_msec.attr,
 	&dev_attr_version.attr,
 	&dev_attr_brightness.attr,
 	&dev_attr_led_rgb.attr,
 	&dev_attr_sequence.attr,
+	&dev_attr_program.attr,
 	NULL,
 };
 
diff --git a/include/linux/mfd/cros_ec_commands.h b/include/linux/mfd/cros_ec_commands.h
index 1b19e424..dbea580 100644
--- a/include/linux/mfd/cros_ec_commands.h
+++ b/include/linux/mfd/cros_ec_commands.h
@@ -1162,6 +1162,13 @@ struct lightbar_params_v1 {
 	struct rgb_s color[8];			/* 0-3 are Google colors */
 } __packed;
 
+/* Lightbar program */
+#define EC_LB_PROG_LEN 192
+struct lightbar_program {
+	uint8_t size;
+	uint8_t data[EC_LB_PROG_LEN];
+};
+
 struct ec_params_lightbar {
 	uint8_t cmd;		      /* Command (see enum lightbar_command) */
 	union {
@@ -1188,6 +1195,7 @@ struct ec_params_lightbar {
 
 		struct lightbar_params_v0 set_params_v0;
 		struct lightbar_params_v1 set_params_v1;
+		struct lightbar_program set_program;
 	};
 } __packed;
 
@@ -1220,7 +1228,8 @@ struct ec_response_lightbar {
 		struct {
 			/* no return params */
 		} off, on, init, set_brightness, seq, reg, set_rgb,
-			demo, set_params_v0, set_params_v1;
+			demo, set_params_v0, set_params_v1,
+			set_program;
 	};
 } __packed;
 
@@ -1244,6 +1253,7 @@ enum lightbar_command {
 	LIGHTBAR_CMD_GET_DEMO = 15,
 	LIGHTBAR_CMD_GET_PARAMS_V1 = 16,
 	LIGHTBAR_CMD_SET_PARAMS_V1 = 17,
+	LIGHTBAR_CMD_SET_PROGRAM = 18,
 	LIGHTBAR_NUM_CMDS
 };
 
-- 
2.9.3

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

* [PATCH RESEND 11/13] platform/chrome: cros_ec_lightbar - Control of suspend/resume lightbar sequence
  2017-05-16 16:13 [PATCH RESEND 00/13] platform/chrome: Add console debugfs, lpc and lightbar programming support Enric Balletbo i Serra
                   ` (9 preceding siblings ...)
  2017-05-16 16:13 ` [PATCH RESEND 10/13] platform/chrome: cros_ec_lightbar - Add lightbar program feature to sysfs Enric Balletbo i Serra
@ 2017-05-16 16:13 ` Enric Balletbo i Serra
  2017-06-23 22:08   ` Benson Leung
  2017-05-16 16:13 ` [PATCH RESEND 12/13] platform/chrome: cros_ec_lightbar - Add userspace lightbar control bit to EC Enric Balletbo i Serra
  2017-05-16 16:13 ` [PATCH RESEND 13/13] platform/chrome: cros_ec_lightbar - Avoid I2C xfer to EC during suspend Enric Balletbo i Serra
  12 siblings, 1 reply; 31+ messages in thread
From: Enric Balletbo i Serra @ 2017-05-16 16:13 UTC (permalink / raw)
  To: olof, bleung, linux-kernel; +Cc: lee.jones, Eric Caruso, Guenter Roeck

From: Eric Caruso <ejcaruso@chromium.org>

Don't let EC control suspend/resume sequence. If the EC controls the
lightbar and sets the sequence when it notices the chipset transitioning
between states, we can't make exceptions for cases where we don't want
to activate the lightbar. Instead, let's move the suspend/resume
notifications into the kernel so we can selectively play the sequences.

Signed-off-by: Eric Caruso <ejcaruso@chromium.org>
Signed-off-by: Guenter Roeck <groeck@chromium.org>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/platform/chrome/cros_ec_dev.c      | 37 +++++++++++++
 drivers/platform/chrome/cros_ec_dev.h      |  6 +++
 drivers/platform/chrome/cros_ec_lightbar.c | 85 ++++++++++++++++++++++++++++--
 include/linux/mfd/cros_ec_commands.h       | 11 +++-
 4 files changed, 134 insertions(+), 5 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_dev.c b/drivers/platform/chrome/cros_ec_dev.c
index 20ce1c2..7c26223 100644
--- a/drivers/platform/chrome/cros_ec_dev.c
+++ b/drivers/platform/chrome/cros_ec_dev.c
@@ -21,6 +21,7 @@
 #include <linux/mfd/core.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
+#include <linux/pm.h>
 #include <linux/slab.h>
 #include <linux/uaccess.h>
 
@@ -435,6 +436,10 @@ static int ec_device_probe(struct platform_device *pdev)
 	if (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE))
 		cros_ec_sensors_register(ec);
 
+	/* Take control of the lightbar from the EC. */
+	if (ec_has_lightbar(ec))
+		lb_manual_suspend_ctrl(ec, 1);
+
 	return 0;
 
 failed:
@@ -446,6 +451,10 @@ static int ec_device_remove(struct platform_device *pdev)
 {
 	struct cros_ec_dev *ec = dev_get_drvdata(&pdev->dev);
 
+	/* Let the EC take over the lightbar again. */
+	if (ec_has_lightbar(ec))
+		lb_manual_suspend_ctrl(ec, 0);
+
 	cros_ec_debugfs_remove(ec);
 
 	cdev_del(&ec->cdev);
@@ -459,9 +468,37 @@ static const struct platform_device_id cros_ec_id[] = {
 };
 MODULE_DEVICE_TABLE(platform, cros_ec_id);
 
+static int ec_device_suspend(struct device *dev)
+{
+	struct cros_ec_dev *ec = dev_get_drvdata(dev);
+
+	if (ec_has_lightbar(ec))
+		lb_suspend(ec);
+
+	return 0;
+}
+
+static int ec_device_resume(struct device *dev)
+{
+	struct cros_ec_dev *ec = dev_get_drvdata(dev);
+
+	if (ec_has_lightbar(ec))
+		lb_resume(ec);
+
+	return 0;
+}
+
+static const struct dev_pm_ops cros_ec_dev_pm_ops = {
+#ifdef CONFIG_PM_SLEEP
+	.suspend = ec_device_suspend,
+	.resume = ec_device_resume,
+#endif
+};
+
 static struct platform_driver cros_ec_dev_driver = {
 	.driver = {
 		.name = "cros-ec-ctl",
+		.pm = &cros_ec_dev_pm_ops,
 	},
 	.probe = ec_device_probe,
 	.remove = ec_device_remove,
diff --git a/drivers/platform/chrome/cros_ec_dev.h b/drivers/platform/chrome/cros_ec_dev.h
index bfd2c84..45e9453 100644
--- a/drivers/platform/chrome/cros_ec_dev.h
+++ b/drivers/platform/chrome/cros_ec_dev.h
@@ -43,4 +43,10 @@ struct cros_ec_readmem {
 #define CROS_EC_DEV_IOCXCMD   _IOWR(CROS_EC_DEV_IOC, 0, struct cros_ec_command)
 #define CROS_EC_DEV_IOCRDMEM  _IOWR(CROS_EC_DEV_IOC, 1, struct cros_ec_readmem)
 
+/* Lightbar utilities */
+extern bool ec_has_lightbar(struct cros_ec_dev *ec);
+extern int lb_manual_suspend_ctrl(struct cros_ec_dev *ec, uint8_t enable);
+extern int lb_suspend(struct cros_ec_dev *ec);
+extern int lb_resume(struct cros_ec_dev *ec);
+
 #endif /* _CROS_EC_DEV_H_ */
diff --git a/drivers/platform/chrome/cros_ec_lightbar.c b/drivers/platform/chrome/cros_ec_lightbar.c
index 2667505..4df379d 100644
--- a/drivers/platform/chrome/cros_ec_lightbar.c
+++ b/drivers/platform/chrome/cros_ec_lightbar.c
@@ -341,6 +341,80 @@ static ssize_t sequence_show(struct device *dev,
 	return ret;
 }
 
+static int lb_send_empty_cmd(struct cros_ec_dev *ec, uint8_t cmd)
+{
+	struct ec_params_lightbar *param;
+	struct cros_ec_command *msg;
+	int ret;
+
+	msg = alloc_lightbar_cmd_msg(ec);
+	if (!msg)
+		return -ENOMEM;
+
+	param = (struct ec_params_lightbar *)msg->data;
+	param->cmd = cmd;
+
+	ret = lb_throttle();
+	if (ret)
+		goto error;
+
+	ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
+	if (ret < 0)
+		goto error;
+	if (msg->result != EC_RES_SUCCESS) {
+		ret = -EINVAL;
+		goto error;
+	}
+	ret = 0;
+error:
+	kfree(msg);
+
+	return ret;
+}
+
+int lb_manual_suspend_ctrl(struct cros_ec_dev *ec, uint8_t enable)
+{
+	struct ec_params_lightbar *param;
+	struct cros_ec_command *msg;
+	int ret;
+
+	msg = alloc_lightbar_cmd_msg(ec);
+	if (!msg)
+		return -ENOMEM;
+
+	param = (struct ec_params_lightbar *)msg->data;
+
+	param->cmd = LIGHTBAR_CMD_MANUAL_SUSPEND_CTRL;
+	param->manual_suspend_ctrl.enable = enable;
+
+	ret = lb_throttle();
+	if (ret)
+		goto error;
+
+	ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
+	if (ret < 0)
+		goto error;
+	if (msg->result != EC_RES_SUCCESS) {
+		ret = -EINVAL;
+		goto error;
+	}
+	ret = 0;
+error:
+	kfree(msg);
+
+	return ret;
+}
+
+int lb_suspend(struct cros_ec_dev *ec)
+{
+	return lb_send_empty_cmd(ec, LIGHTBAR_CMD_SUSPEND);
+}
+
+int lb_resume(struct cros_ec_dev *ec)
+{
+	return lb_send_empty_cmd(ec, LIGHTBAR_CMD_RESUME);
+}
+
 static ssize_t sequence_store(struct device *dev, struct device_attribute *attr,
 			      const char *buf, size_t count)
 {
@@ -473,6 +547,11 @@ static struct attribute *__lb_cmds_attrs[] = {
 	NULL,
 };
 
+bool ec_has_lightbar(struct cros_ec_dev *ec)
+{
+	return !!get_lightbar_version(ec, NULL, NULL);
+}
+
 static umode_t cros_ec_lightbar_attrs_are_visible(struct kobject *kobj,
 						  struct attribute *a, int n)
 {
@@ -489,10 +568,10 @@ static umode_t cros_ec_lightbar_attrs_are_visible(struct kobject *kobj,
 		return 0;
 
 	/* Only instantiate this stuff if the EC has a lightbar */
-	if (get_lightbar_version(ec, NULL, NULL))
+	if (ec_has_lightbar(ec))
 		return a->mode;
-	else
-		return 0;
+
+	return 0;
 }
 
 struct attribute_group cros_ec_lightbar_attr_group = {
diff --git a/include/linux/mfd/cros_ec_commands.h b/include/linux/mfd/cros_ec_commands.h
index dbea580..190c8f4 100644
--- a/include/linux/mfd/cros_ec_commands.h
+++ b/include/linux/mfd/cros_ec_commands.h
@@ -1175,7 +1175,7 @@ struct ec_params_lightbar {
 		struct {
 			/* no args */
 		} dump, off, on, init, get_seq, get_params_v0, get_params_v1,
-			version, get_brightness, get_demo;
+			version, get_brightness, get_demo, suspend, resume;
 
 		struct {
 			uint8_t num;
@@ -1193,6 +1193,10 @@ struct ec_params_lightbar {
 			uint8_t led;
 		} get_rgb;
 
+		struct {
+			uint8_t enable;
+		} manual_suspend_ctrl;
+
 		struct lightbar_params_v0 set_params_v0;
 		struct lightbar_params_v1 set_params_v1;
 		struct lightbar_program set_program;
@@ -1229,7 +1233,7 @@ struct ec_response_lightbar {
 			/* no return params */
 		} off, on, init, set_brightness, seq, reg, set_rgb,
 			demo, set_params_v0, set_params_v1,
-			set_program;
+			set_program, manual_suspend_ctrl, suspend, resume;
 	};
 } __packed;
 
@@ -1254,6 +1258,9 @@ enum lightbar_command {
 	LIGHTBAR_CMD_GET_PARAMS_V1 = 16,
 	LIGHTBAR_CMD_SET_PARAMS_V1 = 17,
 	LIGHTBAR_CMD_SET_PROGRAM = 18,
+	LIGHTBAR_CMD_MANUAL_SUSPEND_CTRL = 19,
+	LIGHTBAR_CMD_SUSPEND = 20,
+	LIGHTBAR_CMD_RESUME = 21,
 	LIGHTBAR_NUM_CMDS
 };
 
-- 
2.9.3

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

* [PATCH RESEND 12/13] platform/chrome: cros_ec_lightbar - Add userspace lightbar control bit to EC
  2017-05-16 16:13 [PATCH RESEND 00/13] platform/chrome: Add console debugfs, lpc and lightbar programming support Enric Balletbo i Serra
                   ` (10 preceding siblings ...)
  2017-05-16 16:13 ` [PATCH RESEND 11/13] platform/chrome: cros_ec_lightbar - Control of suspend/resume lightbar sequence Enric Balletbo i Serra
@ 2017-05-16 16:13 ` Enric Balletbo i Serra
  2017-06-23 22:12   ` Benson Leung
  2017-05-16 16:13 ` [PATCH RESEND 13/13] platform/chrome: cros_ec_lightbar - Avoid I2C xfer to EC during suspend Enric Balletbo i Serra
  12 siblings, 1 reply; 31+ messages in thread
From: Enric Balletbo i Serra @ 2017-05-16 16:13 UTC (permalink / raw)
  To: olof, bleung, linux-kernel; +Cc: lee.jones, Eric Caruso, Guenter Roeck

From: Eric Caruso <ejcaruso@chromium.org>

Some devices might want to turn off the lightbar if e.g. the
system turns the screen off due to idleness. This prevents the
kernel from going through its normal suspend/resume pathways.

Signed-off-by: Eric Caruso <ejcaruso@chromium.org>
Signed-off-by: Guenter Roeck <groeck@chromium.org>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
 drivers/platform/chrome/cros_ec_lightbar.c | 38 ++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/drivers/platform/chrome/cros_ec_lightbar.c b/drivers/platform/chrome/cros_ec_lightbar.c
index 4df379d..e570c1e 100644
--- a/drivers/platform/chrome/cros_ec_lightbar.c
+++ b/drivers/platform/chrome/cros_ec_lightbar.c
@@ -38,6 +38,12 @@
 /* Rate-limit the lightbar interface to prevent DoS. */
 static unsigned long lb_interval_jiffies = 50 * HZ / 1000;
 
+/*
+ * Whether or not we have given userspace control of the lightbar.
+ * If this is true, we won't do anything during suspend/resume.
+ */
+static bool userspace_control;
+
 static ssize_t interval_msec_show(struct device *dev,
 				  struct device_attribute *attr, char *buf)
 {
@@ -407,11 +413,17 @@ int lb_manual_suspend_ctrl(struct cros_ec_dev *ec, uint8_t enable)
 
 int lb_suspend(struct cros_ec_dev *ec)
 {
+	if (userspace_control)
+		return 0;
+
 	return lb_send_empty_cmd(ec, LIGHTBAR_CMD_SUSPEND);
 }
 
 int lb_resume(struct cros_ec_dev *ec)
 {
+	if (userspace_control)
+		return 0;
+
 	return lb_send_empty_cmd(ec, LIGHTBAR_CMD_RESUME);
 }
 
@@ -528,6 +540,30 @@ static ssize_t program_store(struct device *dev, struct device_attribute *attr,
 	return ret;
 }
 
+static ssize_t userspace_control_show(struct device *dev,
+				      struct device_attribute *attr,
+				      char *buf)
+{
+	return scnprintf(buf, PAGE_SIZE, "%d\n", userspace_control);
+}
+
+static ssize_t userspace_control_store(struct device *dev,
+				       struct device_attribute *attr,
+				       const char *buf,
+				       size_t count)
+{
+	bool enable;
+	int ret;
+
+	ret = strtobool(buf, &enable);
+	if (ret < 0)
+		return ret;
+
+	userspace_control = enable;
+
+	return count;
+}
+
 /* Module initialization */
 
 static DEVICE_ATTR_RW(interval_msec);
@@ -536,6 +572,7 @@ static DEVICE_ATTR_WO(brightness);
 static DEVICE_ATTR_WO(led_rgb);
 static DEVICE_ATTR_RW(sequence);
 static DEVICE_ATTR_WO(program);
+static DEVICE_ATTR_RW(userspace_control);
 
 static struct attribute *__lb_cmds_attrs[] = {
 	&dev_attr_interval_msec.attr,
@@ -544,6 +581,7 @@ static struct attribute *__lb_cmds_attrs[] = {
 	&dev_attr_led_rgb.attr,
 	&dev_attr_sequence.attr,
 	&dev_attr_program.attr,
+	&dev_attr_userspace_control.attr,
 	NULL,
 };
 
-- 
2.9.3

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

* [PATCH RESEND 13/13] platform/chrome: cros_ec_lightbar - Avoid I2C xfer to EC during suspend
  2017-05-16 16:13 [PATCH RESEND 00/13] platform/chrome: Add console debugfs, lpc and lightbar programming support Enric Balletbo i Serra
                   ` (11 preceding siblings ...)
  2017-05-16 16:13 ` [PATCH RESEND 12/13] platform/chrome: cros_ec_lightbar - Add userspace lightbar control bit to EC Enric Balletbo i Serra
@ 2017-05-16 16:13 ` Enric Balletbo i Serra
  2017-06-23 22:22   ` Benson Leung
  12 siblings, 1 reply; 31+ messages in thread
From: Enric Balletbo i Serra @ 2017-05-16 16:13 UTC (permalink / raw)
  To: olof, bleung, linux-kernel; +Cc: lee.jones, Jeffery Yu, Guenter Roeck

From: Jeffery Yu <jefferyy@nvidia.com>

A Mutex lock in cros_ec_cmd_xfer which may be held by frozen
Userspace thread during system suspending. So should not
call this routine in suspend thread.

Signed-off-by: Jeffery Yu <jefferyy@nvidia.com>
Signed-off-by: Guenter Roeck <groeck@chromium.org>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
 drivers/platform/chrome/cros_ec_dev.c      | 12 ++++--------
 drivers/platform/chrome/cros_ec_lightbar.c | 13 +++++++++----
 2 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_dev.c b/drivers/platform/chrome/cros_ec_dev.c
index 7c26223..b9bf086 100644
--- a/drivers/platform/chrome/cros_ec_dev.c
+++ b/drivers/platform/chrome/cros_ec_dev.c
@@ -437,8 +437,7 @@ static int ec_device_probe(struct platform_device *pdev)
 		cros_ec_sensors_register(ec);
 
 	/* Take control of the lightbar from the EC. */
-	if (ec_has_lightbar(ec))
-		lb_manual_suspend_ctrl(ec, 1);
+	lb_manual_suspend_ctrl(ec, 1);
 
 	return 0;
 
@@ -452,8 +451,7 @@ static int ec_device_remove(struct platform_device *pdev)
 	struct cros_ec_dev *ec = dev_get_drvdata(&pdev->dev);
 
 	/* Let the EC take over the lightbar again. */
-	if (ec_has_lightbar(ec))
-		lb_manual_suspend_ctrl(ec, 0);
+	lb_manual_suspend_ctrl(ec, 0);
 
 	cros_ec_debugfs_remove(ec);
 
@@ -472,8 +470,7 @@ static int ec_device_suspend(struct device *dev)
 {
 	struct cros_ec_dev *ec = dev_get_drvdata(dev);
 
-	if (ec_has_lightbar(ec))
-		lb_suspend(ec);
+	lb_suspend(ec);
 
 	return 0;
 }
@@ -482,8 +479,7 @@ static int ec_device_resume(struct device *dev)
 {
 	struct cros_ec_dev *ec = dev_get_drvdata(dev);
 
-	if (ec_has_lightbar(ec))
-		lb_resume(ec);
+	lb_resume(ec);
 
 	return 0;
 }
diff --git a/drivers/platform/chrome/cros_ec_lightbar.c b/drivers/platform/chrome/cros_ec_lightbar.c
index e570c1e..fd2b047 100644
--- a/drivers/platform/chrome/cros_ec_lightbar.c
+++ b/drivers/platform/chrome/cros_ec_lightbar.c
@@ -43,6 +43,7 @@ static unsigned long lb_interval_jiffies = 50 * HZ / 1000;
  * If this is true, we won't do anything during suspend/resume.
  */
 static bool userspace_control;
+static struct cros_ec_dev *ec_with_lightbar;
 
 static ssize_t interval_msec_show(struct device *dev,
 				  struct device_attribute *attr, char *buf)
@@ -384,6 +385,9 @@ int lb_manual_suspend_ctrl(struct cros_ec_dev *ec, uint8_t enable)
 	struct cros_ec_command *msg;
 	int ret;
 
+	if (ec != ec_with_lightbar)
+		return 0;
+
 	msg = alloc_lightbar_cmd_msg(ec);
 	if (!msg)
 		return -ENOMEM;
@@ -413,7 +417,7 @@ int lb_manual_suspend_ctrl(struct cros_ec_dev *ec, uint8_t enable)
 
 int lb_suspend(struct cros_ec_dev *ec)
 {
-	if (userspace_control)
+	if (userspace_control || ec != ec_with_lightbar)
 		return 0;
 
 	return lb_send_empty_cmd(ec, LIGHTBAR_CMD_SUSPEND);
@@ -421,7 +425,7 @@ int lb_suspend(struct cros_ec_dev *ec)
 
 int lb_resume(struct cros_ec_dev *ec)
 {
-	if (userspace_control)
+	if (userspace_control || ec != ec_with_lightbar)
 		return 0;
 
 	return lb_send_empty_cmd(ec, LIGHTBAR_CMD_RESUME);
@@ -606,9 +610,10 @@ static umode_t cros_ec_lightbar_attrs_are_visible(struct kobject *kobj,
 		return 0;
 
 	/* Only instantiate this stuff if the EC has a lightbar */
-	if (ec_has_lightbar(ec))
+	if (ec_has_lightbar(ec)) {
+		ec_with_lightbar = ec;
 		return a->mode;
-
+	}
 	return 0;
 }
 
-- 
2.9.3

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

* Re: [PATCH RESEND 05/13] platform/chrome: cros_ec_lpc: Add R/W helpers to LPC protocol variants
  2017-05-16 16:13 ` [PATCH RESEND 05/13] platform/chrome: cros_ec_lpc: Add R/W helpers to LPC protocol variants Enric Balletbo i Serra
@ 2017-05-22 11:09   ` Lee Jones
  2017-06-22 19:02   ` Benson Leung
  1 sibling, 0 replies; 31+ messages in thread
From: Lee Jones @ 2017-05-22 11:09 UTC (permalink / raw)
  To: Enric Balletbo i Serra
  Cc: olof, bleung, linux-kernel, Shawn Nematbakhsh, Thierry Escande

On Tue, 16 May 2017, Enric Balletbo i Serra wrote:

> From: Shawn Nematbakhsh <shawnn@chromium.org>
> 
> Call common functions for read / write to prepare support for future
> LPC protocol variants which use different I/O ops than inb / outb.
> 
> Signed-off-by: Shawn Nematbakhsh <shawnn@chromium.org>
> Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
> ---
>  drivers/platform/chrome/Makefile          |  3 +-
>  drivers/platform/chrome/cros_ec_lpc.c     | 88 +++++++++++++------------------
>  drivers/platform/chrome/cros_ec_lpc_reg.c | 64 ++++++++++++++++++++++
>  include/linux/mfd/cros_ec_lpc_reg.h       | 47 +++++++++++++++++

Acked-by: Lee Jones <lee.jones@linaro.org>

>  4 files changed, 151 insertions(+), 51 deletions(-)
>  create mode 100644 drivers/platform/chrome/cros_ec_lpc_reg.c
>  create mode 100644 include/linux/mfd/cros_ec_lpc_reg.h
> 
> diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
> index 3870afe..61182fd 100644
> --- a/drivers/platform/chrome/Makefile
> +++ b/drivers/platform/chrome/Makefile
> @@ -5,6 +5,7 @@ cros_ec_devs-objs			:= cros_ec_dev.o cros_ec_sysfs.o \
>  					   cros_ec_lightbar.o cros_ec_vbc.o \
>  					   cros_ec_debugfs.o
>  obj-$(CONFIG_CROS_EC_CHARDEV)		+= cros_ec_devs.o
> -obj-$(CONFIG_CROS_EC_LPC)		+= cros_ec_lpc.o
> +cros_ec_lpcs-objs			:= cros_ec_lpc.o cros_ec_lpc_reg.o
> +obj-$(CONFIG_CROS_EC_LPC)		+= cros_ec_lpcs.o
>  obj-$(CONFIG_CROS_EC_PROTO)		+= cros_ec_proto.o
>  obj-$(CONFIG_CROS_KBD_LED_BACKLIGHT)	+= cros_kbd_led_backlight.o
> diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
> index f9a2454..6a782a6 100644
> --- a/drivers/platform/chrome/cros_ec_lpc.c
> +++ b/drivers/platform/chrome/cros_ec_lpc.c
> @@ -26,19 +26,22 @@
>  #include <linux/io.h>
>  #include <linux/mfd/cros_ec.h>
>  #include <linux/mfd/cros_ec_commands.h>
> +#include <linux/mfd/cros_ec_lpc_reg.h>
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
>  #include <linux/printk.h>
>  
> -#define DRV_NAME "cros_ec_lpc"
> +#define DRV_NAME "cros_ec_lpcs"
>  
>  static int ec_response_timed_out(void)
>  {
>  	unsigned long one_second = jiffies + HZ;
> +	u8 data;
>  
>  	usleep_range(200, 300);
>  	do {
> -		if (!(inb(EC_LPC_ADDR_HOST_CMD) & EC_LPC_STATUS_BUSY_MASK))
> +		if (!(cros_ec_lpc_read_bytes(EC_LPC_ADDR_HOST_CMD, 1, &data) &
> +		    EC_LPC_STATUS_BUSY_MASK))
>  			return 0;
>  		usleep_range(100, 200);
>  	} while (time_before(jiffies, one_second));
> @@ -51,21 +54,20 @@ static int cros_ec_pkt_xfer_lpc(struct cros_ec_device *ec,
>  {
>  	struct ec_host_request *request;
>  	struct ec_host_response response;
> -	u8 sum = 0;
> -	int i;
> +	u8 sum;
>  	int ret = 0;
>  	u8 *dout;
>  
>  	ret = cros_ec_prepare_tx(ec, msg);
>  
>  	/* Write buffer */
> -	for (i = 0; i < ret; i++)
> -		outb(ec->dout[i], EC_LPC_ADDR_HOST_PACKET + i);
> +	cros_ec_lpc_write_bytes(EC_LPC_ADDR_HOST_PACKET, ret, ec->dout);
>  
>  	request = (struct ec_host_request *)ec->dout;
>  
>  	/* Here we go */
> -	outb(EC_COMMAND_PROTOCOL_3, EC_LPC_ADDR_HOST_CMD);
> +	sum = EC_COMMAND_PROTOCOL_3;
> +	cros_ec_lpc_write_bytes(EC_LPC_ADDR_HOST_CMD, 1, &sum);
>  
>  	if (ec_response_timed_out()) {
>  		dev_warn(ec->dev, "EC responsed timed out\n");
> @@ -74,17 +76,15 @@ static int cros_ec_pkt_xfer_lpc(struct cros_ec_device *ec,
>  	}
>  
>  	/* Check result */
> -	msg->result = inb(EC_LPC_ADDR_HOST_DATA);
> +	msg->result = cros_ec_lpc_read_bytes(EC_LPC_ADDR_HOST_DATA, 1, &sum);
>  	ret = cros_ec_check_result(ec, msg);
>  	if (ret)
>  		goto done;
>  
>  	/* Read back response */
>  	dout = (u8 *)&response;
> -	for (i = 0; i < sizeof(response); i++) {
> -		dout[i] = inb(EC_LPC_ADDR_HOST_PACKET + i);
> -		sum += dout[i];
> -	}
> +	sum = cros_ec_lpc_read_bytes(EC_LPC_ADDR_HOST_PACKET, sizeof(response),
> +				     dout);
>  
>  	msg->result = response.result;
>  
> @@ -97,11 +97,9 @@ static int cros_ec_pkt_xfer_lpc(struct cros_ec_device *ec,
>  	}
>  
>  	/* Read response and process checksum */
> -	for (i = 0; i < response.data_len; i++) {
> -		msg->data[i] =
> -			inb(EC_LPC_ADDR_HOST_PACKET + sizeof(response) + i);
> -		sum += msg->data[i];
> -	}
> +	sum += cros_ec_lpc_read_bytes(EC_LPC_ADDR_HOST_PACKET +
> +				      sizeof(response), response.data_len,
> +				      msg->data);
>  
>  	if (sum) {
>  		dev_err(ec->dev,
> @@ -121,8 +119,7 @@ static int cros_ec_cmd_xfer_lpc(struct cros_ec_device *ec,
>  				struct cros_ec_command *msg)
>  {
>  	struct ec_lpc_host_args args;
> -	int csum;
> -	int i;
> +	u8 sum;
>  	int ret = 0;
>  
>  	if (msg->outsize > EC_PROTO2_MAX_PARAM_SIZE ||
> @@ -139,24 +136,20 @@ static int cros_ec_cmd_xfer_lpc(struct cros_ec_device *ec,
>  	args.data_size = msg->outsize;
>  
>  	/* Initialize checksum */
> -	csum = msg->command + args.flags +
> -		args.command_version + args.data_size;
> +	sum = msg->command + args.flags + args.command_version + args.data_size;
>  
>  	/* Copy data and update checksum */
> -	for (i = 0; i < msg->outsize; i++) {
> -		outb(msg->data[i], EC_LPC_ADDR_HOST_PARAM + i);
> -		csum += msg->data[i];
> -	}
> +	sum += cros_ec_lpc_write_bytes(EC_LPC_ADDR_HOST_PARAM, msg->outsize,
> +				       msg->data);
>  
>  	/* Finalize checksum and write args */
> -	args.checksum = csum & 0xFF;
> -	outb(args.flags, EC_LPC_ADDR_HOST_ARGS);
> -	outb(args.command_version, EC_LPC_ADDR_HOST_ARGS + 1);
> -	outb(args.data_size, EC_LPC_ADDR_HOST_ARGS + 2);
> -	outb(args.checksum, EC_LPC_ADDR_HOST_ARGS + 3);
> +	args.checksum = sum;
> +	cros_ec_lpc_write_bytes(EC_LPC_ADDR_HOST_ARGS, sizeof(args),
> +				(u8 *)&args);
>  
>  	/* Here we go */
> -	outb(msg->command, EC_LPC_ADDR_HOST_CMD);
> +	sum = msg->command;
> +	cros_ec_lpc_write_bytes(EC_LPC_ADDR_HOST_CMD, 1, &sum);
>  
>  	if (ec_response_timed_out()) {
>  		dev_warn(ec->dev, "EC responsed timed out\n");
> @@ -165,16 +158,14 @@ static int cros_ec_cmd_xfer_lpc(struct cros_ec_device *ec,
>  	}
>  
>  	/* Check result */
> -	msg->result = inb(EC_LPC_ADDR_HOST_DATA);
> +	msg->result = cros_ec_lpc_read_bytes(EC_LPC_ADDR_HOST_DATA, 1, &sum);
>  	ret = cros_ec_check_result(ec, msg);
>  	if (ret)
>  		goto done;
>  
>  	/* Read back args */
> -	args.flags = inb(EC_LPC_ADDR_HOST_ARGS);
> -	args.command_version = inb(EC_LPC_ADDR_HOST_ARGS + 1);
> -	args.data_size = inb(EC_LPC_ADDR_HOST_ARGS + 2);
> -	args.checksum = inb(EC_LPC_ADDR_HOST_ARGS + 3);
> +	cros_ec_lpc_read_bytes(EC_LPC_ADDR_HOST_ARGS, sizeof(args),
> +			       (u8 *)&args);
>  
>  	if (args.data_size > msg->insize) {
>  		dev_err(ec->dev,
> @@ -185,20 +176,17 @@ static int cros_ec_cmd_xfer_lpc(struct cros_ec_device *ec,
>  	}
>  
>  	/* Start calculating response checksum */
> -	csum = msg->command + args.flags +
> -		args.command_version + args.data_size;
> +	sum = msg->command + args.flags + args.command_version + args.data_size;
>  
>  	/* Read response and update checksum */
> -	for (i = 0; i < args.data_size; i++) {
> -		msg->data[i] = inb(EC_LPC_ADDR_HOST_PARAM + i);
> -		csum += msg->data[i];
> -	}
> +	sum += cros_ec_lpc_read_bytes(EC_LPC_ADDR_HOST_PARAM, args.data_size,
> +				      msg->data);
>  
>  	/* Verify checksum */
> -	if (args.checksum != (csum & 0xFF)) {
> +	if (args.checksum != sum) {
>  		dev_err(ec->dev,
>  			"bad packet checksum, expected %02x, got %02x\n",
> -			args.checksum, csum & 0xFF);
> +			args.checksum, sum);
>  		ret = -EBADMSG;
>  		goto done;
>  	}
> @@ -222,14 +210,13 @@ static int cros_ec_lpc_readmem(struct cros_ec_device *ec, unsigned int offset,
>  
>  	/* fixed length */
>  	if (bytes) {
> -		for (; cnt < bytes; i++, s++, cnt++)
> -			*s = inb(EC_LPC_ADDR_MEMMAP + i);
> -		return cnt;
> +		cros_ec_lpc_read_bytes(EC_LPC_ADDR_MEMMAP + offset, bytes, s);
> +		return bytes;
>  	}
>  
>  	/* string */
>  	for (; i < EC_MEMMAP_SIZE; i++, s++) {
> -		*s = inb(EC_LPC_ADDR_MEMMAP + i);
> +		cros_ec_lpc_read_bytes(EC_LPC_ADDR_MEMMAP + i, 1, s);
>  		cnt++;
>  		if (!*s)
>  			break;
> @@ -242,6 +229,7 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
>  	struct cros_ec_device *ec_dev;
> +	u8 buf[2];
>  	int ret;
>  
>  	if (!devm_request_region(dev, EC_LPC_ADDR_MEMMAP, EC_MEMMAP_SIZE,
> @@ -250,8 +238,8 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
>  		return -EBUSY;
>  	}
>  
> -	if ((inb(EC_LPC_ADDR_MEMMAP + EC_MEMMAP_ID) != 'E') ||
> -	    (inb(EC_LPC_ADDR_MEMMAP + EC_MEMMAP_ID + 1) != 'C')) {
> +	cros_ec_lpc_read_bytes(EC_LPC_ADDR_MEMMAP + EC_MEMMAP_ID, 2, buf);
> +	if (buf[0] != 'E' || buf[1] != 'C') {
>  		dev_err(dev, "EC ID not detected\n");
>  		return -ENODEV;
>  	}
> diff --git a/drivers/platform/chrome/cros_ec_lpc_reg.c b/drivers/platform/chrome/cros_ec_lpc_reg.c
> new file mode 100644
> index 0000000..03c9781
> --- /dev/null
> +++ b/drivers/platform/chrome/cros_ec_lpc_reg.c
> @@ -0,0 +1,64 @@
> +/*
> + * cros_ec_lpc_reg - LPC access to the Chrome OS Embedded Controller
> + *
> + * Copyright (C) 2016 Google, Inc
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * This driver uses the Chrome OS EC byte-level message-based protocol for
> + * communicating the keyboard state (which keys are pressed) from a keyboard EC
> + * to the AP over some bus (such as i2c, lpc, spi).  The EC does debouncing,
> + * but everything else (including deghosting) is done here.  The main
> + * motivation for this is to keep the EC firmware as simple as possible, since
> + * it cannot be easily upgraded and EC flash/IRAM space is relatively
> + * expensive.
> + */
> +
> +#include <linux/io.h>
> +#include <linux/mfd/cros_ec.h>
> +#include <linux/mfd/cros_ec_commands.h>
> +
> +static u8 lpc_read_bytes(unsigned int offset, unsigned int length, u8 *dest)
> +{
> +	int i;
> +	int sum = 0;
> +
> +	for (i = 0; i < length; ++i) {
> +		dest[i] = inb(offset + i);
> +		sum += dest[i];
> +	}
> +
> +	/* Return checksum of all bytes read */
> +	return sum;
> +}
> +
> +static u8 lpc_write_bytes(unsigned int offset, unsigned int length, u8 *msg)
> +{
> +	int i;
> +	int sum = 0;
> +
> +	for (i = 0; i < length; ++i) {
> +		outb(msg[i], offset + i);
> +		sum += msg[i];
> +	}
> +
> +	/* Return checksum of all bytes written */
> +	return sum;
> +}
> +
> +u8 cros_ec_lpc_read_bytes(unsigned int offset, unsigned int length, u8 *dest)
> +{
> +	return lpc_read_bytes(offset, length, dest);
> +}
> +
> +u8 cros_ec_lpc_write_bytes(unsigned int offset, unsigned int length, u8 *msg)
> +{
> +	return lpc_write_bytes(offset, length, msg);
> +}
> diff --git a/include/linux/mfd/cros_ec_lpc_reg.h b/include/linux/mfd/cros_ec_lpc_reg.h
> new file mode 100644
> index 0000000..4089bd5
> --- /dev/null
> +++ b/include/linux/mfd/cros_ec_lpc_reg.h
> @@ -0,0 +1,47 @@
> +/*
> + * cros_ec_lpc_reg - LPC access to the Chrome OS Embedded Controller
> + *
> + * Copyright (C) 2016 Google, Inc
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * This driver uses the Chrome OS EC byte-level message-based protocol for
> + * communicating the keyboard state (which keys are pressed) from a keyboard EC
> + * to the AP over some bus (such as i2c, lpc, spi).  The EC does debouncing,
> + * but everything else (including deghosting) is done here.  The main
> + * motivation for this is to keep the EC firmware as simple as possible, since
> + * it cannot be easily upgraded and EC flash/IRAM space is relatively
> + * expensive.
> + */
> +
> +#ifndef __LINUX_MFD_CROS_EC_REG_H
> +#define __LINUX_MFD_CROS_EC_REG_H
> +
> +/**
> + * cros_ec_lpc_read_bytes - Read bytes from a given LPC-mapped address.
> + * Returns 8-bit checksum of all bytes read.
> + *
> + * @offset: Base read address
> + * @length: Number of bytes to read
> + * @dest: Destination buffer
> + */
> +u8 cros_ec_lpc_read_bytes(unsigned int offset, unsigned int length, u8 *dest);
> +
> +/**
> + * cros_ec_lpc_write_bytes - Write bytes to a given LPC-mapped address.
> + * Returns 8-bit checksum of all bytes written.
> + *
> + * @offset: Base write address
> + * @length: Number of bytes to write
> + * @msg: Write data buffer
> + */
> +u8 cros_ec_lpc_write_bytes(unsigned int offset, unsigned int length, u8 *msg);
> +
> +#endif /* __LINUX_MFD_CROS_EC_REG_H */

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

* Re: [PATCH RESEND 06/13] platform/chrome: cros_ec_lpc: Add support for mec1322 EC
  2017-05-16 16:13 ` [PATCH RESEND 06/13] platform/chrome: cros_ec_lpc: Add support for mec1322 EC Enric Balletbo i Serra
@ 2017-05-22 11:09   ` Lee Jones
  2017-06-22 19:01   ` Benson Leung
  1 sibling, 0 replies; 31+ messages in thread
From: Lee Jones @ 2017-05-22 11:09 UTC (permalink / raw)
  To: Enric Balletbo i Serra
  Cc: olof, bleung, linux-kernel, Shawn Nematbakhsh, Thierry Escande

On Tue, 16 May 2017, Enric Balletbo i Serra wrote:

> From: Shawn Nematbakhsh <shawnn@chromium.org>
> 
> This adds support for the ChromeOS LPC Microchip Embedded Controller
> (mec1322) variant.
> 
> mec1322 accesses I/O region [800h, 9ffh] through embedded memory
> interface (EMI) rather than LPC.
> 
> Signed-off-by: Shawn Nematbakhsh <shawnn@chromium.org>
> Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
> ---
>  drivers/platform/chrome/Kconfig           |  12 +++
>  drivers/platform/chrome/Makefile          |   1 +
>  drivers/platform/chrome/cros_ec_lpc.c     |   5 ++
>  drivers/platform/chrome/cros_ec_lpc_mec.c | 140 ++++++++++++++++++++++++++++++
>  drivers/platform/chrome/cros_ec_lpc_reg.c |  69 +++++++++++++++

>  include/linux/mfd/cros_ec_lpc_mec.h       |  90 +++++++++++++++++++
>  include/linux/mfd/cros_ec_lpc_reg.h       |  14 +++

Acked-by: Lee Jones <lee.jones@linaro.org>

>  7 files changed, 331 insertions(+)
>  create mode 100644 drivers/platform/chrome/cros_ec_lpc_mec.c
>  create mode 100644 include/linux/mfd/cros_ec_lpc_mec.h
> 
> diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
> index 76bdae1..6d80fb5 100644
> --- a/drivers/platform/chrome/Kconfig
> +++ b/drivers/platform/chrome/Kconfig
> @@ -59,6 +59,18 @@ config CROS_EC_LPC
>            To compile this driver as a module, choose M here: the
>            module will be called cros_ec_lpc.
>  
> +config CROS_EC_LPC_MEC
> +	bool "ChromeOS Embedded Controller LPC Microchip EC (MEC) variant"
> +	depends on CROS_EC_LPC
> +	default n
> +	help
> +	  If you say Y here, a variant LPC protocol for the Microchip EC
> +	  will be used. Note that this variant is not backward compatible
> +	  with non-Microchip ECs.
> +
> +	  If you have a ChromeOS Embedded Controller Microchip EC variant
> +	  choose Y here.
> +
>  config CROS_EC_PROTO
>          bool
>          help
> diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
> index 61182fd..66c345c 100644
> --- a/drivers/platform/chrome/Makefile
> +++ b/drivers/platform/chrome/Makefile
> @@ -6,6 +6,7 @@ cros_ec_devs-objs			:= cros_ec_dev.o cros_ec_sysfs.o \
>  					   cros_ec_debugfs.o
>  obj-$(CONFIG_CROS_EC_CHARDEV)		+= cros_ec_devs.o
>  cros_ec_lpcs-objs			:= cros_ec_lpc.o cros_ec_lpc_reg.o
> +cros_ec_lpcs-$(CONFIG_CROS_EC_LPC_MEC)	+= cros_ec_lpc_mec.o
>  obj-$(CONFIG_CROS_EC_LPC)		+= cros_ec_lpcs.o
>  obj-$(CONFIG_CROS_EC_PROTO)		+= cros_ec_proto.o
>  obj-$(CONFIG_CROS_KBD_LED_BACKLIGHT)	+= cros_kbd_led_backlight.o
> diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
> index 6a782a6..bc2dc62 100644
> --- a/drivers/platform/chrome/cros_ec_lpc.c
> +++ b/drivers/platform/chrome/cros_ec_lpc.c
> @@ -346,10 +346,13 @@ static int __init cros_ec_lpc_init(void)
>  		return -ENODEV;
>  	}
>  
> +	cros_ec_lpc_reg_init();
> +
>  	/* Register the driver */
>  	ret = platform_driver_register(&cros_ec_lpc_driver);
>  	if (ret) {
>  		pr_err(DRV_NAME ": can't register driver: %d\n", ret);
> +		cros_ec_lpc_reg_destroy();
>  		return ret;
>  	}
>  
> @@ -358,6 +361,7 @@ static int __init cros_ec_lpc_init(void)
>  	if (ret) {
>  		pr_err(DRV_NAME ": can't register device: %d\n", ret);
>  		platform_driver_unregister(&cros_ec_lpc_driver);
> +		cros_ec_lpc_reg_destroy();
>  		return ret;
>  	}
>  
> @@ -368,6 +372,7 @@ static void __exit cros_ec_lpc_exit(void)
>  {
>  	platform_device_unregister(&cros_ec_lpc_device);
>  	platform_driver_unregister(&cros_ec_lpc_driver);
> +	cros_ec_lpc_reg_destroy();
>  }
>  
>  module_init(cros_ec_lpc_init);
> diff --git a/drivers/platform/chrome/cros_ec_lpc_mec.c b/drivers/platform/chrome/cros_ec_lpc_mec.c
> new file mode 100644
> index 0000000..2eda2c2
> --- /dev/null
> +++ b/drivers/platform/chrome/cros_ec_lpc_mec.c
> @@ -0,0 +1,140 @@
> +/*
> + * cros_ec_lpc_mec - LPC variant I/O for Microchip EC
> + *
> + * Copyright (C) 2016 Google, Inc
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * This driver uses the Chrome OS EC byte-level message-based protocol for
> + * communicating the keyboard state (which keys are pressed) from a keyboard EC
> + * to the AP over some bus (such as i2c, lpc, spi).  The EC does debouncing,
> + * but everything else (including deghosting) is done here.  The main
> + * motivation for this is to keep the EC firmware as simple as possible, since
> + * it cannot be easily upgraded and EC flash/IRAM space is relatively
> + * expensive.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/mfd/cros_ec_commands.h>
> +#include <linux/mfd/cros_ec_lpc_mec.h>
> +#include <linux/mutex.h>
> +#include <linux/types.h>
> +
> +/*
> + * This mutex must be held while accessing the EMI unit. We can't rely on the
> + * EC mutex because memmap data may be accessed without it being held.
> + */
> +static struct mutex io_mutex;
> +
> +/*
> + * cros_ec_lpc_mec_emi_write_address
> + *
> + * Initialize EMI read / write at a given address.
> + *
> + * @addr:        Starting read / write address
> + * @access_type: Type of access, typically 32-bit auto-increment
> + */
> +static void cros_ec_lpc_mec_emi_write_address(u16 addr,
> +			enum cros_ec_lpc_mec_emi_access_mode access_type)
> +{
> +	/* Address relative to start of EMI range */
> +	addr -= MEC_EMI_RANGE_START;
> +	outb((addr & 0xfc) | access_type, MEC_EMI_EC_ADDRESS_B0);
> +	outb((addr >> 8) & 0x7f, MEC_EMI_EC_ADDRESS_B1);
> +}
> +
> +/*
> + * cros_ec_lpc_io_bytes_mec - Read / write bytes to MEC EMI port
> + *
> + * @io_type: MEC_IO_READ or MEC_IO_WRITE, depending on request
> + * @offset:  Base read / write address
> + * @length:  Number of bytes to read / write
> + * @buf:     Destination / source buffer
> + *
> + * @return 8-bit checksum of all bytes read / written
> + */
> +u8 cros_ec_lpc_io_bytes_mec(enum cros_ec_lpc_mec_io_type io_type,
> +			    unsigned int offset, unsigned int length,
> +			    u8 *buf)
> +{
> +	int i = 0;
> +	int io_addr;
> +	u8 sum = 0;
> +	enum cros_ec_lpc_mec_emi_access_mode access, new_access;
> +
> +	/*
> +	 * Long access cannot be used on misaligned data since reading B0 loads
> +	 * the data register and writing B3 flushes.
> +	 */
> +	if (offset & 0x3 || length < 4)
> +		access = ACCESS_TYPE_BYTE;
> +	else
> +		access = ACCESS_TYPE_LONG_AUTO_INCREMENT;
> +
> +	mutex_lock(&io_mutex);
> +
> +	/* Initialize I/O at desired address */
> +	cros_ec_lpc_mec_emi_write_address(offset, access);
> +
> +	/* Skip bytes in case of misaligned offset */
> +	io_addr = MEC_EMI_EC_DATA_B0 + (offset & 0x3);
> +	while (i < length) {
> +		while (io_addr <= MEC_EMI_EC_DATA_B3) {
> +			if (io_type == MEC_IO_READ)
> +				buf[i] = inb(io_addr++);
> +			else
> +				outb(buf[i], io_addr++);
> +
> +			sum += buf[i++];
> +			offset++;
> +
> +			/* Extra bounds check in case of misaligned length */
> +			if (i == length)
> +				goto done;
> +		}
> +
> +		/*
> +		 * Use long auto-increment access except for misaligned write,
> +		 * since writing B3 triggers the flush.
> +		 */
> +		if (length - i < 4 && io_type == MEC_IO_WRITE)
> +			new_access = ACCESS_TYPE_BYTE;
> +		else
> +			new_access = ACCESS_TYPE_LONG_AUTO_INCREMENT;
> +
> +		if (new_access != access ||
> +		    access != ACCESS_TYPE_LONG_AUTO_INCREMENT) {
> +			access = new_access;
> +			cros_ec_lpc_mec_emi_write_address(offset, access);
> +		}
> +
> +		/* Access [B0, B3] on each loop pass */
> +		io_addr = MEC_EMI_EC_DATA_B0;
> +	}
> +
> +done:
> +	mutex_unlock(&io_mutex);
> +
> +	return sum;
> +}
> +EXPORT_SYMBOL(cros_ec_lpc_io_bytes_mec);
> +
> +void cros_ec_lpc_mec_init(void)
> +{
> +	mutex_init(&io_mutex);
> +}
> +EXPORT_SYMBOL(cros_ec_lpc_mec_init);
> +
> +void cros_ec_lpc_mec_destroy(void)
> +{
> +	mutex_destroy(&io_mutex);
> +}
> +EXPORT_SYMBOL(cros_ec_lpc_mec_destroy);
> diff --git a/drivers/platform/chrome/cros_ec_lpc_reg.c b/drivers/platform/chrome/cros_ec_lpc_reg.c
> index 03c9781..dcc7a3e 100644
> --- a/drivers/platform/chrome/cros_ec_lpc_reg.c
> +++ b/drivers/platform/chrome/cros_ec_lpc_reg.c
> @@ -24,6 +24,7 @@
>  #include <linux/io.h>
>  #include <linux/mfd/cros_ec.h>
>  #include <linux/mfd/cros_ec_commands.h>
> +#include <linux/mfd/cros_ec_lpc_mec.h>
>  
>  static u8 lpc_read_bytes(unsigned int offset, unsigned int length, u8 *dest)
>  {
> @@ -53,12 +54,80 @@ static u8 lpc_write_bytes(unsigned int offset, unsigned int length, u8 *msg)
>  	return sum;
>  }
>  
> +#ifdef CONFIG_CROS_EC_LPC_MEC
> +
>  u8 cros_ec_lpc_read_bytes(unsigned int offset, unsigned int length, u8 *dest)
>  {
> +	if (length == 0)
> +		return 0;
> +
> +	/* Access desired range through EMI interface */
> +	if (offset >= MEC_EMI_RANGE_START && offset <= MEC_EMI_RANGE_END) {
> +		/* Ensure we don't straddle EMI region */
> +		if (WARN_ON(offset + length - 1 > MEC_EMI_RANGE_END))
> +			return 0;
> +
> +		return cros_ec_lpc_io_bytes_mec(MEC_IO_READ, offset, length,
> +						dest);
> +	}
> +
> +	if (WARN_ON(offset + length > MEC_EMI_RANGE_START &&
> +		    offset < MEC_EMI_RANGE_START))
> +		return 0;
> +
>  	return lpc_read_bytes(offset, length, dest);
>  }
>  
>  u8 cros_ec_lpc_write_bytes(unsigned int offset, unsigned int length, u8 *msg)
>  {
> +	if (length == 0)
> +		return 0;
> +
> +	/* Access desired range through EMI interface */
> +	if (offset >= MEC_EMI_RANGE_START && offset <= MEC_EMI_RANGE_END) {
> +		/* Ensure we don't straddle EMI region */
> +		if (WARN_ON(offset + length - 1 > MEC_EMI_RANGE_END))
> +			return 0;
> +
> +		return cros_ec_lpc_io_bytes_mec(MEC_IO_WRITE, offset, length,
> +						msg);
> +	}
> +
> +	if (WARN_ON(offset + length > MEC_EMI_RANGE_START &&
> +		    offset < MEC_EMI_RANGE_START))
> +		return 0;
> +
>  	return lpc_write_bytes(offset, length, msg);
>  }
> +
> +void cros_ec_lpc_reg_init(void)
> +{
> +	cros_ec_lpc_mec_init();
> +}
> +
> +void cros_ec_lpc_reg_destroy(void)
> +{
> +	cros_ec_lpc_mec_destroy();
> +}
> +
> +#else /* CONFIG_CROS_EC_LPC_MEC */
> +
> +u8 cros_ec_lpc_read_bytes(unsigned int offset, unsigned int length, u8 *dest)
> +{
> +	return lpc_read_bytes(offset, length, dest);
> +}
> +
> +u8 cros_ec_lpc_write_bytes(unsigned int offset, unsigned int length, u8 *msg)
> +{
> +	return lpc_write_bytes(offset, length, msg);
> +}
> +
> +void cros_ec_lpc_reg_init(void)
> +{
> +}
> +
> +void cros_ec_lpc_reg_destroy(void)
> +{
> +}
> +
> +#endif /* CONFIG_CROS_EC_LPC_MEC */
> diff --git a/include/linux/mfd/cros_ec_lpc_mec.h b/include/linux/mfd/cros_ec_lpc_mec.h
> new file mode 100644
> index 0000000..176496d
> --- /dev/null
> +++ b/include/linux/mfd/cros_ec_lpc_mec.h
> @@ -0,0 +1,90 @@
> +/*
> + * cros_ec_lpc_mec - LPC variant I/O for Microchip EC
> + *
> + * Copyright (C) 2016 Google, Inc
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * This driver uses the Chrome OS EC byte-level message-based protocol for
> + * communicating the keyboard state (which keys are pressed) from a keyboard EC
> + * to the AP over some bus (such as i2c, lpc, spi).  The EC does debouncing,
> + * but everything else (including deghosting) is done here.  The main
> + * motivation for this is to keep the EC firmware as simple as possible, since
> + * it cannot be easily upgraded and EC flash/IRAM space is relatively
> + * expensive.
> + */
> +
> +#ifndef __LINUX_MFD_CROS_EC_MEC_H
> +#define __LINUX_MFD_CROS_EC_MEC_H
> +
> +#include <linux/mfd/cros_ec_commands.h>
> +
> +enum cros_ec_lpc_mec_emi_access_mode {
> +	/* 8-bit access */
> +	ACCESS_TYPE_BYTE = 0x0,
> +	/* 16-bit access */
> +	ACCESS_TYPE_WORD = 0x1,
> +	/* 32-bit access */
> +	ACCESS_TYPE_LONG = 0x2,
> +	/*
> +	 * 32-bit access, read or write of MEC_EMI_EC_DATA_B3 causes the
> +	 * EC data register to be incremented.
> +	 */
> +	ACCESS_TYPE_LONG_AUTO_INCREMENT = 0x3,
> +};
> +
> +enum cros_ec_lpc_mec_io_type {
> +	MEC_IO_READ,
> +	MEC_IO_WRITE,
> +};
> +
> +/* Access IO ranges 0x800 thru 0x9ff using EMI interface instead of LPC */
> +#define MEC_EMI_RANGE_START EC_HOST_CMD_REGION0
> +#define MEC_EMI_RANGE_END   (EC_LPC_ADDR_MEMMAP + EC_MEMMAP_SIZE)
> +
> +/* EMI registers are relative to base */
> +#define MEC_EMI_BASE 0x800
> +#define MEC_EMI_HOST_TO_EC (MEC_EMI_BASE + 0)
> +#define MEC_EMI_EC_TO_HOST (MEC_EMI_BASE + 1)
> +#define MEC_EMI_EC_ADDRESS_B0 (MEC_EMI_BASE + 2)
> +#define MEC_EMI_EC_ADDRESS_B1 (MEC_EMI_BASE + 3)
> +#define MEC_EMI_EC_DATA_B0 (MEC_EMI_BASE + 4)
> +#define MEC_EMI_EC_DATA_B1 (MEC_EMI_BASE + 5)
> +#define MEC_EMI_EC_DATA_B2 (MEC_EMI_BASE + 6)
> +#define MEC_EMI_EC_DATA_B3 (MEC_EMI_BASE + 7)
> +
> +/*
> + * cros_ec_lpc_mec_init
> + *
> + * Initialize MEC I/O.
> + */
> +void cros_ec_lpc_mec_init(void);
> +
> +/*
> + * cros_ec_lpc_mec_destroy
> + *
> + * Cleanup MEC I/O.
> + */
> +void cros_ec_lpc_mec_destroy(void);
> +
> +/**
> + * cros_ec_lpc_io_bytes_mec - Read / write bytes to MEC EMI port
> + *
> + * @io_type: MEC_IO_READ or MEC_IO_WRITE, depending on request
> + * @offset:  Base read / write address
> + * @length:  Number of bytes to read / write
> + * @buf:     Destination / source buffer
> + *
> + * @return 8-bit checksum of all bytes read / written
> + */
> +u8 cros_ec_lpc_io_bytes_mec(enum cros_ec_lpc_mec_io_type io_type,
> +			    unsigned int offset, unsigned int length, u8 *buf);
> +
> +#endif /* __LINUX_MFD_CROS_EC_MEC_H */
> diff --git a/include/linux/mfd/cros_ec_lpc_reg.h b/include/linux/mfd/cros_ec_lpc_reg.h
> index 4089bd5..5560bef 100644
> --- a/include/linux/mfd/cros_ec_lpc_reg.h
> +++ b/include/linux/mfd/cros_ec_lpc_reg.h
> @@ -44,4 +44,18 @@ u8 cros_ec_lpc_read_bytes(unsigned int offset, unsigned int length, u8 *dest);
>   */
>  u8 cros_ec_lpc_write_bytes(unsigned int offset, unsigned int length, u8 *msg);
>  
> +/**
> + * cros_ec_lpc_reg_init
> + *
> + * Initialize register I/O.
> + */
> +void cros_ec_lpc_reg_init(void);
> +
> +/**
> + * cros_ec_lpc_reg_destroy
> + *
> + * Cleanup reg I/O.
> + */
> +void cros_ec_lpc_reg_destroy(void);
> +
>  #endif /* __LINUX_MFD_CROS_EC_REG_H */

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

* Re: [PATCH RESEND 03/13] mfd: cros_ec: add debugfs, console log file
  2017-05-16 16:13 ` [PATCH RESEND 03/13] mfd: cros_ec: add debugfs, console log file Enric Balletbo i Serra
@ 2017-06-16 19:42   ` Benson Leung
  0 siblings, 0 replies; 31+ messages in thread
From: Benson Leung @ 2017-06-16 19:42 UTC (permalink / raw)
  To: Enric Balletbo i Serra
  Cc: olof, bleung, linux-kernel, lee.jones, Eric Caruso, Nicolas Boichat

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

Hi Enric,

I have gotten around to reviewing this series, and hope to get
this in ASAP.

I found an issue with this commit, but I'll go ahead and fix it
myself as I'm creating the immutable branch. No need to respin the series.

On Tue, May 16, 2017 at 06:13:09PM +0200, Enric Balletbo i Serra wrote:
> +static int ec_read_version_supported(struct cros_ec_dev *ec)
> +{
> +	struct ec_params_get_cmd_versions_v1 *params;
> +	struct ec_response_get_cmd_versions *response;
> +	int ret;
> +
> +	struct cros_ec_command *msg;
> +
> +	msg = kzalloc(sizeof(*msg) + max(sizeof(params), sizeof(response)),
> +		GFP_KERNEL);
> +	if (!msg)
> +		return 0;
> +
> +	msg->command = EC_CMD_GET_CMD_VERSIONS + ec->cmd_offset;
> +	msg->outsize = sizeof(*params);
> +	msg->insize = sizeof(*response);

By my diff, the above two lines were changed from the original CHROMIUM
commit, based on Doug's comment here: https://lkml.org/lkml/2017/2/22/630

However, this is an incomplete fix. Instead, we should pick this:
https://chromium-review.googlesource.com/#/c/444085/

I'll go ahead and do that. Thanks!

Benson

-- 
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@google.com
Chromium OS Project
bleung@chromium.org

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

* Re: [PATCH RESEND 01/13] mfd: cros_ec: Add helper for event notifier.
  2017-05-16 16:13 ` [PATCH RESEND 01/13] mfd: cros_ec: Add helper for event notifier Enric Balletbo i Serra
@ 2017-06-16 20:45   ` Benson Leung
  0 siblings, 0 replies; 31+ messages in thread
From: Benson Leung @ 2017-06-16 20:45 UTC (permalink / raw)
  To: Enric Balletbo i Serra, olof, bleung, linux-kernel
  Cc: lee.jones, Gwendal Grignou


[-- Attachment #1.1: Type: text/plain, Size: 614 bytes --]

Hi Enric,

On 05/16/2017 09:13 AM, Enric Balletbo i Serra wrote:
> From: Gwendal Grignou <gwendal@chromium.org>
> 
> Add cros_ec_get_event() entry point to retrieve event within functions
> called by the notifier.
> 
> Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Acked-by: Lee Jones <lee.jones@linaro.org>

Applied to my branch. I'll let you know when the whole thing is ready.

-- 
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@google.com
Chromium OS Project
bleung@chromium.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

* Re: [PATCH RESEND 02/13] mfd: cros_ec: Add EC console read structures definitions
  2017-05-16 16:13 ` [PATCH RESEND 02/13] mfd: cros_ec: Add EC console read structures definitions Enric Balletbo i Serra
@ 2017-06-16 20:47   ` Benson Leung
  0 siblings, 0 replies; 31+ messages in thread
From: Benson Leung @ 2017-06-16 20:47 UTC (permalink / raw)
  To: Enric Balletbo i Serra, olof, bleung, linux-kernel
  Cc: lee.jones, Nicolas Boichat


[-- Attachment #1.1: Type: text/plain, Size: 671 bytes --]

Hi Enric,

On 05/16/2017 09:13 AM, Enric Balletbo i Serra wrote:
> From: Nicolas Boichat <drinkcat@chromium.org>
> 
> ec_params_console_read_v1 is used to capture EC logs from kernel,
> and ec_params_get_cmd_versions_v1 is used to probe whether EC
> supports that command.
> 
> Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>
> Reviewed-by: Guenter Roeck <groeck@chromium.org>
> Acked-by: Lee Jones <lee.jones@linaro.org>
> Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>

Thanks. Applied.


-- 
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@google.com
Chromium OS Project
bleung@chromium.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

* Re: [PATCH RESEND 06/13] platform/chrome: cros_ec_lpc: Add support for mec1322 EC
  2017-05-16 16:13 ` [PATCH RESEND 06/13] platform/chrome: cros_ec_lpc: Add support for mec1322 EC Enric Balletbo i Serra
  2017-05-22 11:09   ` Lee Jones
@ 2017-06-22 19:01   ` Benson Leung
  1 sibling, 0 replies; 31+ messages in thread
From: Benson Leung @ 2017-06-22 19:01 UTC (permalink / raw)
  To: Enric Balletbo i Serra
  Cc: olof, bleung, linux-kernel, lee.jones, Shawn Nematbakhsh,
	Thierry Escande

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

Hi Enric,

This patch plus the mec patch lgtm. Thanks!

On Tue, May 16, 2017 at 06:13:12PM +0200, Enric Balletbo i Serra wrote:
> From: Shawn Nematbakhsh <shawnn@chromium.org>
> 
> This adds support for the ChromeOS LPC Microchip Embedded Controller
> (mec1322) variant.
> 
> mec1322 accesses I/O region [800h, 9ffh] through embedded memory
> interface (EMI) rather than LPC.
> 
> Signed-off-by: Shawn Nematbakhsh <shawnn@chromium.org>
> Signed-off-by: Thierry Escande <thierry.escande@collabora.com>

Signed-off-by: Benson Leung <bleung@chromium.org>

Applied to my upcoming immutable branch.

> ---
>  drivers/platform/chrome/Kconfig           |  12 +++
>  drivers/platform/chrome/Makefile          |   1 +
>  drivers/platform/chrome/cros_ec_lpc.c     |   5 ++
>  drivers/platform/chrome/cros_ec_lpc_mec.c | 140 ++++++++++++++++++++++++++++++
>  drivers/platform/chrome/cros_ec_lpc_reg.c |  69 +++++++++++++++
>  include/linux/mfd/cros_ec_lpc_mec.h       |  90 +++++++++++++++++++
>  include/linux/mfd/cros_ec_lpc_reg.h       |  14 +++
>  7 files changed, 331 insertions(+)
>  create mode 100644 drivers/platform/chrome/cros_ec_lpc_mec.c
>  create mode 100644 include/linux/mfd/cros_ec_lpc_mec.h
> 
> diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
> index 76bdae1..6d80fb5 100644
> --- a/drivers/platform/chrome/Kconfig
> +++ b/drivers/platform/chrome/Kconfig
> @@ -59,6 +59,18 @@ config CROS_EC_LPC
>            To compile this driver as a module, choose M here: the
>            module will be called cros_ec_lpc.
>  
> +config CROS_EC_LPC_MEC
> +	bool "ChromeOS Embedded Controller LPC Microchip EC (MEC) variant"
> +	depends on CROS_EC_LPC
> +	default n
> +	help
> +	  If you say Y here, a variant LPC protocol for the Microchip EC
> +	  will be used. Note that this variant is not backward compatible
> +	  with non-Microchip ECs.
> +
> +	  If you have a ChromeOS Embedded Controller Microchip EC variant
> +	  choose Y here.
> +
>  config CROS_EC_PROTO
>          bool
>          help
> diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
> index 61182fd..66c345c 100644
> --- a/drivers/platform/chrome/Makefile
> +++ b/drivers/platform/chrome/Makefile
> @@ -6,6 +6,7 @@ cros_ec_devs-objs			:= cros_ec_dev.o cros_ec_sysfs.o \
>  					   cros_ec_debugfs.o
>  obj-$(CONFIG_CROS_EC_CHARDEV)		+= cros_ec_devs.o
>  cros_ec_lpcs-objs			:= cros_ec_lpc.o cros_ec_lpc_reg.o
> +cros_ec_lpcs-$(CONFIG_CROS_EC_LPC_MEC)	+= cros_ec_lpc_mec.o
>  obj-$(CONFIG_CROS_EC_LPC)		+= cros_ec_lpcs.o
>  obj-$(CONFIG_CROS_EC_PROTO)		+= cros_ec_proto.o
>  obj-$(CONFIG_CROS_KBD_LED_BACKLIGHT)	+= cros_kbd_led_backlight.o
> diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
> index 6a782a6..bc2dc62 100644
> --- a/drivers/platform/chrome/cros_ec_lpc.c
> +++ b/drivers/platform/chrome/cros_ec_lpc.c
> @@ -346,10 +346,13 @@ static int __init cros_ec_lpc_init(void)
>  		return -ENODEV;
>  	}
>  
> +	cros_ec_lpc_reg_init();
> +
>  	/* Register the driver */
>  	ret = platform_driver_register(&cros_ec_lpc_driver);
>  	if (ret) {
>  		pr_err(DRV_NAME ": can't register driver: %d\n", ret);
> +		cros_ec_lpc_reg_destroy();
>  		return ret;
>  	}
>  
> @@ -358,6 +361,7 @@ static int __init cros_ec_lpc_init(void)
>  	if (ret) {
>  		pr_err(DRV_NAME ": can't register device: %d\n", ret);
>  		platform_driver_unregister(&cros_ec_lpc_driver);
> +		cros_ec_lpc_reg_destroy();
>  		return ret;
>  	}
>  
> @@ -368,6 +372,7 @@ static void __exit cros_ec_lpc_exit(void)
>  {
>  	platform_device_unregister(&cros_ec_lpc_device);
>  	platform_driver_unregister(&cros_ec_lpc_driver);
> +	cros_ec_lpc_reg_destroy();
>  }
>  
>  module_init(cros_ec_lpc_init);
> diff --git a/drivers/platform/chrome/cros_ec_lpc_mec.c b/drivers/platform/chrome/cros_ec_lpc_mec.c
> new file mode 100644
> index 0000000..2eda2c2
> --- /dev/null
> +++ b/drivers/platform/chrome/cros_ec_lpc_mec.c
> @@ -0,0 +1,140 @@
> +/*
> + * cros_ec_lpc_mec - LPC variant I/O for Microchip EC
> + *
> + * Copyright (C) 2016 Google, Inc
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * This driver uses the Chrome OS EC byte-level message-based protocol for
> + * communicating the keyboard state (which keys are pressed) from a keyboard EC
> + * to the AP over some bus (such as i2c, lpc, spi).  The EC does debouncing,
> + * but everything else (including deghosting) is done here.  The main
> + * motivation for this is to keep the EC firmware as simple as possible, since
> + * it cannot be easily upgraded and EC flash/IRAM space is relatively
> + * expensive.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/mfd/cros_ec_commands.h>
> +#include <linux/mfd/cros_ec_lpc_mec.h>
> +#include <linux/mutex.h>
> +#include <linux/types.h>
> +
> +/*
> + * This mutex must be held while accessing the EMI unit. We can't rely on the
> + * EC mutex because memmap data may be accessed without it being held.
> + */
> +static struct mutex io_mutex;
> +
> +/*
> + * cros_ec_lpc_mec_emi_write_address
> + *
> + * Initialize EMI read / write at a given address.
> + *
> + * @addr:        Starting read / write address
> + * @access_type: Type of access, typically 32-bit auto-increment
> + */
> +static void cros_ec_lpc_mec_emi_write_address(u16 addr,
> +			enum cros_ec_lpc_mec_emi_access_mode access_type)
> +{
> +	/* Address relative to start of EMI range */
> +	addr -= MEC_EMI_RANGE_START;
> +	outb((addr & 0xfc) | access_type, MEC_EMI_EC_ADDRESS_B0);
> +	outb((addr >> 8) & 0x7f, MEC_EMI_EC_ADDRESS_B1);
> +}
> +
> +/*
> + * cros_ec_lpc_io_bytes_mec - Read / write bytes to MEC EMI port
> + *
> + * @io_type: MEC_IO_READ or MEC_IO_WRITE, depending on request
> + * @offset:  Base read / write address
> + * @length:  Number of bytes to read / write
> + * @buf:     Destination / source buffer
> + *
> + * @return 8-bit checksum of all bytes read / written
> + */
> +u8 cros_ec_lpc_io_bytes_mec(enum cros_ec_lpc_mec_io_type io_type,
> +			    unsigned int offset, unsigned int length,
> +			    u8 *buf)
> +{
> +	int i = 0;
> +	int io_addr;
> +	u8 sum = 0;
> +	enum cros_ec_lpc_mec_emi_access_mode access, new_access;
> +
> +	/*
> +	 * Long access cannot be used on misaligned data since reading B0 loads
> +	 * the data register and writing B3 flushes.
> +	 */
> +	if (offset & 0x3 || length < 4)
> +		access = ACCESS_TYPE_BYTE;
> +	else
> +		access = ACCESS_TYPE_LONG_AUTO_INCREMENT;
> +
> +	mutex_lock(&io_mutex);
> +
> +	/* Initialize I/O at desired address */
> +	cros_ec_lpc_mec_emi_write_address(offset, access);
> +
> +	/* Skip bytes in case of misaligned offset */
> +	io_addr = MEC_EMI_EC_DATA_B0 + (offset & 0x3);
> +	while (i < length) {
> +		while (io_addr <= MEC_EMI_EC_DATA_B3) {
> +			if (io_type == MEC_IO_READ)
> +				buf[i] = inb(io_addr++);
> +			else
> +				outb(buf[i], io_addr++);
> +
> +			sum += buf[i++];
> +			offset++;
> +
> +			/* Extra bounds check in case of misaligned length */
> +			if (i == length)
> +				goto done;
> +		}
> +
> +		/*
> +		 * Use long auto-increment access except for misaligned write,
> +		 * since writing B3 triggers the flush.
> +		 */
> +		if (length - i < 4 && io_type == MEC_IO_WRITE)
> +			new_access = ACCESS_TYPE_BYTE;
> +		else
> +			new_access = ACCESS_TYPE_LONG_AUTO_INCREMENT;
> +
> +		if (new_access != access ||
> +		    access != ACCESS_TYPE_LONG_AUTO_INCREMENT) {
> +			access = new_access;
> +			cros_ec_lpc_mec_emi_write_address(offset, access);
> +		}
> +
> +		/* Access [B0, B3] on each loop pass */
> +		io_addr = MEC_EMI_EC_DATA_B0;
> +	}
> +
> +done:
> +	mutex_unlock(&io_mutex);
> +
> +	return sum;
> +}
> +EXPORT_SYMBOL(cros_ec_lpc_io_bytes_mec);
> +
> +void cros_ec_lpc_mec_init(void)
> +{
> +	mutex_init(&io_mutex);
> +}
> +EXPORT_SYMBOL(cros_ec_lpc_mec_init);
> +
> +void cros_ec_lpc_mec_destroy(void)
> +{
> +	mutex_destroy(&io_mutex);
> +}
> +EXPORT_SYMBOL(cros_ec_lpc_mec_destroy);
> diff --git a/drivers/platform/chrome/cros_ec_lpc_reg.c b/drivers/platform/chrome/cros_ec_lpc_reg.c
> index 03c9781..dcc7a3e 100644
> --- a/drivers/platform/chrome/cros_ec_lpc_reg.c
> +++ b/drivers/platform/chrome/cros_ec_lpc_reg.c
> @@ -24,6 +24,7 @@
>  #include <linux/io.h>
>  #include <linux/mfd/cros_ec.h>
>  #include <linux/mfd/cros_ec_commands.h>
> +#include <linux/mfd/cros_ec_lpc_mec.h>
>  
>  static u8 lpc_read_bytes(unsigned int offset, unsigned int length, u8 *dest)
>  {
> @@ -53,12 +54,80 @@ static u8 lpc_write_bytes(unsigned int offset, unsigned int length, u8 *msg)
>  	return sum;
>  }
>  
> +#ifdef CONFIG_CROS_EC_LPC_MEC
> +
>  u8 cros_ec_lpc_read_bytes(unsigned int offset, unsigned int length, u8 *dest)
>  {
> +	if (length == 0)
> +		return 0;
> +
> +	/* Access desired range through EMI interface */
> +	if (offset >= MEC_EMI_RANGE_START && offset <= MEC_EMI_RANGE_END) {
> +		/* Ensure we don't straddle EMI region */
> +		if (WARN_ON(offset + length - 1 > MEC_EMI_RANGE_END))
> +			return 0;
> +
> +		return cros_ec_lpc_io_bytes_mec(MEC_IO_READ, offset, length,
> +						dest);
> +	}
> +
> +	if (WARN_ON(offset + length > MEC_EMI_RANGE_START &&
> +		    offset < MEC_EMI_RANGE_START))
> +		return 0;
> +
>  	return lpc_read_bytes(offset, length, dest);
>  }
>  
>  u8 cros_ec_lpc_write_bytes(unsigned int offset, unsigned int length, u8 *msg)
>  {
> +	if (length == 0)
> +		return 0;
> +
> +	/* Access desired range through EMI interface */
> +	if (offset >= MEC_EMI_RANGE_START && offset <= MEC_EMI_RANGE_END) {
> +		/* Ensure we don't straddle EMI region */
> +		if (WARN_ON(offset + length - 1 > MEC_EMI_RANGE_END))
> +			return 0;
> +
> +		return cros_ec_lpc_io_bytes_mec(MEC_IO_WRITE, offset, length,
> +						msg);
> +	}
> +
> +	if (WARN_ON(offset + length > MEC_EMI_RANGE_START &&
> +		    offset < MEC_EMI_RANGE_START))
> +		return 0;
> +
>  	return lpc_write_bytes(offset, length, msg);
>  }
> +
> +void cros_ec_lpc_reg_init(void)
> +{
> +	cros_ec_lpc_mec_init();
> +}
> +
> +void cros_ec_lpc_reg_destroy(void)
> +{
> +	cros_ec_lpc_mec_destroy();
> +}
> +
> +#else /* CONFIG_CROS_EC_LPC_MEC */
> +
> +u8 cros_ec_lpc_read_bytes(unsigned int offset, unsigned int length, u8 *dest)
> +{
> +	return lpc_read_bytes(offset, length, dest);
> +}
> +
> +u8 cros_ec_lpc_write_bytes(unsigned int offset, unsigned int length, u8 *msg)
> +{
> +	return lpc_write_bytes(offset, length, msg);
> +}
> +
> +void cros_ec_lpc_reg_init(void)
> +{
> +}
> +
> +void cros_ec_lpc_reg_destroy(void)
> +{
> +}
> +
> +#endif /* CONFIG_CROS_EC_LPC_MEC */
> diff --git a/include/linux/mfd/cros_ec_lpc_mec.h b/include/linux/mfd/cros_ec_lpc_mec.h
> new file mode 100644
> index 0000000..176496d
> --- /dev/null
> +++ b/include/linux/mfd/cros_ec_lpc_mec.h
> @@ -0,0 +1,90 @@
> +/*
> + * cros_ec_lpc_mec - LPC variant I/O for Microchip EC
> + *
> + * Copyright (C) 2016 Google, Inc
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * This driver uses the Chrome OS EC byte-level message-based protocol for
> + * communicating the keyboard state (which keys are pressed) from a keyboard EC
> + * to the AP over some bus (such as i2c, lpc, spi).  The EC does debouncing,
> + * but everything else (including deghosting) is done here.  The main
> + * motivation for this is to keep the EC firmware as simple as possible, since
> + * it cannot be easily upgraded and EC flash/IRAM space is relatively
> + * expensive.
> + */
> +
> +#ifndef __LINUX_MFD_CROS_EC_MEC_H
> +#define __LINUX_MFD_CROS_EC_MEC_H
> +
> +#include <linux/mfd/cros_ec_commands.h>
> +
> +enum cros_ec_lpc_mec_emi_access_mode {
> +	/* 8-bit access */
> +	ACCESS_TYPE_BYTE = 0x0,
> +	/* 16-bit access */
> +	ACCESS_TYPE_WORD = 0x1,
> +	/* 32-bit access */
> +	ACCESS_TYPE_LONG = 0x2,
> +	/*
> +	 * 32-bit access, read or write of MEC_EMI_EC_DATA_B3 causes the
> +	 * EC data register to be incremented.
> +	 */
> +	ACCESS_TYPE_LONG_AUTO_INCREMENT = 0x3,
> +};
> +
> +enum cros_ec_lpc_mec_io_type {
> +	MEC_IO_READ,
> +	MEC_IO_WRITE,
> +};
> +
> +/* Access IO ranges 0x800 thru 0x9ff using EMI interface instead of LPC */
> +#define MEC_EMI_RANGE_START EC_HOST_CMD_REGION0
> +#define MEC_EMI_RANGE_END   (EC_LPC_ADDR_MEMMAP + EC_MEMMAP_SIZE)
> +
> +/* EMI registers are relative to base */
> +#define MEC_EMI_BASE 0x800
> +#define MEC_EMI_HOST_TO_EC (MEC_EMI_BASE + 0)
> +#define MEC_EMI_EC_TO_HOST (MEC_EMI_BASE + 1)
> +#define MEC_EMI_EC_ADDRESS_B0 (MEC_EMI_BASE + 2)
> +#define MEC_EMI_EC_ADDRESS_B1 (MEC_EMI_BASE + 3)
> +#define MEC_EMI_EC_DATA_B0 (MEC_EMI_BASE + 4)
> +#define MEC_EMI_EC_DATA_B1 (MEC_EMI_BASE + 5)
> +#define MEC_EMI_EC_DATA_B2 (MEC_EMI_BASE + 6)
> +#define MEC_EMI_EC_DATA_B3 (MEC_EMI_BASE + 7)
> +
> +/*
> + * cros_ec_lpc_mec_init
> + *
> + * Initialize MEC I/O.
> + */
> +void cros_ec_lpc_mec_init(void);
> +
> +/*
> + * cros_ec_lpc_mec_destroy
> + *
> + * Cleanup MEC I/O.
> + */
> +void cros_ec_lpc_mec_destroy(void);
> +
> +/**
> + * cros_ec_lpc_io_bytes_mec - Read / write bytes to MEC EMI port
> + *
> + * @io_type: MEC_IO_READ or MEC_IO_WRITE, depending on request
> + * @offset:  Base read / write address
> + * @length:  Number of bytes to read / write
> + * @buf:     Destination / source buffer
> + *
> + * @return 8-bit checksum of all bytes read / written
> + */
> +u8 cros_ec_lpc_io_bytes_mec(enum cros_ec_lpc_mec_io_type io_type,
> +			    unsigned int offset, unsigned int length, u8 *buf);
> +
> +#endif /* __LINUX_MFD_CROS_EC_MEC_H */
> diff --git a/include/linux/mfd/cros_ec_lpc_reg.h b/include/linux/mfd/cros_ec_lpc_reg.h
> index 4089bd5..5560bef 100644
> --- a/include/linux/mfd/cros_ec_lpc_reg.h
> +++ b/include/linux/mfd/cros_ec_lpc_reg.h
> @@ -44,4 +44,18 @@ u8 cros_ec_lpc_read_bytes(unsigned int offset, unsigned int length, u8 *dest);
>   */
>  u8 cros_ec_lpc_write_bytes(unsigned int offset, unsigned int length, u8 *msg);
>  
> +/**
> + * cros_ec_lpc_reg_init
> + *
> + * Initialize register I/O.
> + */
> +void cros_ec_lpc_reg_init(void);
> +
> +/**
> + * cros_ec_lpc_reg_destroy
> + *
> + * Cleanup reg I/O.
> + */
> +void cros_ec_lpc_reg_destroy(void);
> +
>  #endif /* __LINUX_MFD_CROS_EC_REG_H */
> -- 
> 2.9.3
> 

-- 
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@google.com
Chromium OS Project
bleung@chromium.org

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

* Re: [PATCH RESEND 05/13] platform/chrome: cros_ec_lpc: Add R/W helpers to LPC protocol variants
  2017-05-16 16:13 ` [PATCH RESEND 05/13] platform/chrome: cros_ec_lpc: Add R/W helpers to LPC protocol variants Enric Balletbo i Serra
  2017-05-22 11:09   ` Lee Jones
@ 2017-06-22 19:02   ` Benson Leung
  1 sibling, 0 replies; 31+ messages in thread
From: Benson Leung @ 2017-06-22 19:02 UTC (permalink / raw)
  To: Enric Balletbo i Serra
  Cc: olof, bleung, linux-kernel, lee.jones, Shawn Nematbakhsh,
	Thierry Escande

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

On Tue, May 16, 2017 at 06:13:11PM +0200, Enric Balletbo i Serra wrote:
> From: Shawn Nematbakhsh <shawnn@chromium.org>
> 
> Call common functions for read / write to prepare support for future
> LPC protocol variants which use different I/O ops than inb / outb.
> 
> Signed-off-by: Shawn Nematbakhsh <shawnn@chromium.org>
> Signed-off-by: Thierry Escande <thierry.escande@collabora.com>

Signed-off-by: Benson Leung <bleung@chromium.org>

Applied to my upcoming immutable branch.

> ---
>  drivers/platform/chrome/Makefile          |  3 +-
>  drivers/platform/chrome/cros_ec_lpc.c     | 88 +++++++++++++------------------
>  drivers/platform/chrome/cros_ec_lpc_reg.c | 64 ++++++++++++++++++++++
>  include/linux/mfd/cros_ec_lpc_reg.h       | 47 +++++++++++++++++
>  4 files changed, 151 insertions(+), 51 deletions(-)
>  create mode 100644 drivers/platform/chrome/cros_ec_lpc_reg.c
>  create mode 100644 include/linux/mfd/cros_ec_lpc_reg.h
> 
> diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
> index 3870afe..61182fd 100644
> --- a/drivers/platform/chrome/Makefile
> +++ b/drivers/platform/chrome/Makefile
> @@ -5,6 +5,7 @@ cros_ec_devs-objs			:= cros_ec_dev.o cros_ec_sysfs.o \
>  					   cros_ec_lightbar.o cros_ec_vbc.o \
>  					   cros_ec_debugfs.o
>  obj-$(CONFIG_CROS_EC_CHARDEV)		+= cros_ec_devs.o
> -obj-$(CONFIG_CROS_EC_LPC)		+= cros_ec_lpc.o
> +cros_ec_lpcs-objs			:= cros_ec_lpc.o cros_ec_lpc_reg.o
> +obj-$(CONFIG_CROS_EC_LPC)		+= cros_ec_lpcs.o
>  obj-$(CONFIG_CROS_EC_PROTO)		+= cros_ec_proto.o
>  obj-$(CONFIG_CROS_KBD_LED_BACKLIGHT)	+= cros_kbd_led_backlight.o
> diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
> index f9a2454..6a782a6 100644
> --- a/drivers/platform/chrome/cros_ec_lpc.c
> +++ b/drivers/platform/chrome/cros_ec_lpc.c
> @@ -26,19 +26,22 @@
>  #include <linux/io.h>
>  #include <linux/mfd/cros_ec.h>
>  #include <linux/mfd/cros_ec_commands.h>
> +#include <linux/mfd/cros_ec_lpc_reg.h>
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
>  #include <linux/printk.h>
>  
> -#define DRV_NAME "cros_ec_lpc"
> +#define DRV_NAME "cros_ec_lpcs"
>  
>  static int ec_response_timed_out(void)
>  {
>  	unsigned long one_second = jiffies + HZ;
> +	u8 data;
>  
>  	usleep_range(200, 300);
>  	do {
> -		if (!(inb(EC_LPC_ADDR_HOST_CMD) & EC_LPC_STATUS_BUSY_MASK))
> +		if (!(cros_ec_lpc_read_bytes(EC_LPC_ADDR_HOST_CMD, 1, &data) &
> +		    EC_LPC_STATUS_BUSY_MASK))
>  			return 0;
>  		usleep_range(100, 200);
>  	} while (time_before(jiffies, one_second));
> @@ -51,21 +54,20 @@ static int cros_ec_pkt_xfer_lpc(struct cros_ec_device *ec,
>  {
>  	struct ec_host_request *request;
>  	struct ec_host_response response;
> -	u8 sum = 0;
> -	int i;
> +	u8 sum;
>  	int ret = 0;
>  	u8 *dout;
>  
>  	ret = cros_ec_prepare_tx(ec, msg);
>  
>  	/* Write buffer */
> -	for (i = 0; i < ret; i++)
> -		outb(ec->dout[i], EC_LPC_ADDR_HOST_PACKET + i);
> +	cros_ec_lpc_write_bytes(EC_LPC_ADDR_HOST_PACKET, ret, ec->dout);
>  
>  	request = (struct ec_host_request *)ec->dout;
>  
>  	/* Here we go */
> -	outb(EC_COMMAND_PROTOCOL_3, EC_LPC_ADDR_HOST_CMD);
> +	sum = EC_COMMAND_PROTOCOL_3;
> +	cros_ec_lpc_write_bytes(EC_LPC_ADDR_HOST_CMD, 1, &sum);
>  
>  	if (ec_response_timed_out()) {
>  		dev_warn(ec->dev, "EC responsed timed out\n");
> @@ -74,17 +76,15 @@ static int cros_ec_pkt_xfer_lpc(struct cros_ec_device *ec,
>  	}
>  
>  	/* Check result */
> -	msg->result = inb(EC_LPC_ADDR_HOST_DATA);
> +	msg->result = cros_ec_lpc_read_bytes(EC_LPC_ADDR_HOST_DATA, 1, &sum);
>  	ret = cros_ec_check_result(ec, msg);
>  	if (ret)
>  		goto done;
>  
>  	/* Read back response */
>  	dout = (u8 *)&response;
> -	for (i = 0; i < sizeof(response); i++) {
> -		dout[i] = inb(EC_LPC_ADDR_HOST_PACKET + i);
> -		sum += dout[i];
> -	}
> +	sum = cros_ec_lpc_read_bytes(EC_LPC_ADDR_HOST_PACKET, sizeof(response),
> +				     dout);
>  
>  	msg->result = response.result;
>  
> @@ -97,11 +97,9 @@ static int cros_ec_pkt_xfer_lpc(struct cros_ec_device *ec,
>  	}
>  
>  	/* Read response and process checksum */
> -	for (i = 0; i < response.data_len; i++) {
> -		msg->data[i] =
> -			inb(EC_LPC_ADDR_HOST_PACKET + sizeof(response) + i);
> -		sum += msg->data[i];
> -	}
> +	sum += cros_ec_lpc_read_bytes(EC_LPC_ADDR_HOST_PACKET +
> +				      sizeof(response), response.data_len,
> +				      msg->data);
>  
>  	if (sum) {
>  		dev_err(ec->dev,
> @@ -121,8 +119,7 @@ static int cros_ec_cmd_xfer_lpc(struct cros_ec_device *ec,
>  				struct cros_ec_command *msg)
>  {
>  	struct ec_lpc_host_args args;
> -	int csum;
> -	int i;
> +	u8 sum;
>  	int ret = 0;
>  
>  	if (msg->outsize > EC_PROTO2_MAX_PARAM_SIZE ||
> @@ -139,24 +136,20 @@ static int cros_ec_cmd_xfer_lpc(struct cros_ec_device *ec,
>  	args.data_size = msg->outsize;
>  
>  	/* Initialize checksum */
> -	csum = msg->command + args.flags +
> -		args.command_version + args.data_size;
> +	sum = msg->command + args.flags + args.command_version + args.data_size;
>  
>  	/* Copy data and update checksum */
> -	for (i = 0; i < msg->outsize; i++) {
> -		outb(msg->data[i], EC_LPC_ADDR_HOST_PARAM + i);
> -		csum += msg->data[i];
> -	}
> +	sum += cros_ec_lpc_write_bytes(EC_LPC_ADDR_HOST_PARAM, msg->outsize,
> +				       msg->data);
>  
>  	/* Finalize checksum and write args */
> -	args.checksum = csum & 0xFF;
> -	outb(args.flags, EC_LPC_ADDR_HOST_ARGS);
> -	outb(args.command_version, EC_LPC_ADDR_HOST_ARGS + 1);
> -	outb(args.data_size, EC_LPC_ADDR_HOST_ARGS + 2);
> -	outb(args.checksum, EC_LPC_ADDR_HOST_ARGS + 3);
> +	args.checksum = sum;
> +	cros_ec_lpc_write_bytes(EC_LPC_ADDR_HOST_ARGS, sizeof(args),
> +				(u8 *)&args);
>  
>  	/* Here we go */
> -	outb(msg->command, EC_LPC_ADDR_HOST_CMD);
> +	sum = msg->command;
> +	cros_ec_lpc_write_bytes(EC_LPC_ADDR_HOST_CMD, 1, &sum);
>  
>  	if (ec_response_timed_out()) {
>  		dev_warn(ec->dev, "EC responsed timed out\n");
> @@ -165,16 +158,14 @@ static int cros_ec_cmd_xfer_lpc(struct cros_ec_device *ec,
>  	}
>  
>  	/* Check result */
> -	msg->result = inb(EC_LPC_ADDR_HOST_DATA);
> +	msg->result = cros_ec_lpc_read_bytes(EC_LPC_ADDR_HOST_DATA, 1, &sum);
>  	ret = cros_ec_check_result(ec, msg);
>  	if (ret)
>  		goto done;
>  
>  	/* Read back args */
> -	args.flags = inb(EC_LPC_ADDR_HOST_ARGS);
> -	args.command_version = inb(EC_LPC_ADDR_HOST_ARGS + 1);
> -	args.data_size = inb(EC_LPC_ADDR_HOST_ARGS + 2);
> -	args.checksum = inb(EC_LPC_ADDR_HOST_ARGS + 3);
> +	cros_ec_lpc_read_bytes(EC_LPC_ADDR_HOST_ARGS, sizeof(args),
> +			       (u8 *)&args);
>  
>  	if (args.data_size > msg->insize) {
>  		dev_err(ec->dev,
> @@ -185,20 +176,17 @@ static int cros_ec_cmd_xfer_lpc(struct cros_ec_device *ec,
>  	}
>  
>  	/* Start calculating response checksum */
> -	csum = msg->command + args.flags +
> -		args.command_version + args.data_size;
> +	sum = msg->command + args.flags + args.command_version + args.data_size;
>  
>  	/* Read response and update checksum */
> -	for (i = 0; i < args.data_size; i++) {
> -		msg->data[i] = inb(EC_LPC_ADDR_HOST_PARAM + i);
> -		csum += msg->data[i];
> -	}
> +	sum += cros_ec_lpc_read_bytes(EC_LPC_ADDR_HOST_PARAM, args.data_size,
> +				      msg->data);
>  
>  	/* Verify checksum */
> -	if (args.checksum != (csum & 0xFF)) {
> +	if (args.checksum != sum) {
>  		dev_err(ec->dev,
>  			"bad packet checksum, expected %02x, got %02x\n",
> -			args.checksum, csum & 0xFF);
> +			args.checksum, sum);
>  		ret = -EBADMSG;
>  		goto done;
>  	}
> @@ -222,14 +210,13 @@ static int cros_ec_lpc_readmem(struct cros_ec_device *ec, unsigned int offset,
>  
>  	/* fixed length */
>  	if (bytes) {
> -		for (; cnt < bytes; i++, s++, cnt++)
> -			*s = inb(EC_LPC_ADDR_MEMMAP + i);
> -		return cnt;
> +		cros_ec_lpc_read_bytes(EC_LPC_ADDR_MEMMAP + offset, bytes, s);
> +		return bytes;
>  	}
>  
>  	/* string */
>  	for (; i < EC_MEMMAP_SIZE; i++, s++) {
> -		*s = inb(EC_LPC_ADDR_MEMMAP + i);
> +		cros_ec_lpc_read_bytes(EC_LPC_ADDR_MEMMAP + i, 1, s);
>  		cnt++;
>  		if (!*s)
>  			break;
> @@ -242,6 +229,7 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
>  	struct cros_ec_device *ec_dev;
> +	u8 buf[2];
>  	int ret;
>  
>  	if (!devm_request_region(dev, EC_LPC_ADDR_MEMMAP, EC_MEMMAP_SIZE,
> @@ -250,8 +238,8 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
>  		return -EBUSY;
>  	}
>  
> -	if ((inb(EC_LPC_ADDR_MEMMAP + EC_MEMMAP_ID) != 'E') ||
> -	    (inb(EC_LPC_ADDR_MEMMAP + EC_MEMMAP_ID + 1) != 'C')) {
> +	cros_ec_lpc_read_bytes(EC_LPC_ADDR_MEMMAP + EC_MEMMAP_ID, 2, buf);
> +	if (buf[0] != 'E' || buf[1] != 'C') {
>  		dev_err(dev, "EC ID not detected\n");
>  		return -ENODEV;
>  	}
> diff --git a/drivers/platform/chrome/cros_ec_lpc_reg.c b/drivers/platform/chrome/cros_ec_lpc_reg.c
> new file mode 100644
> index 0000000..03c9781
> --- /dev/null
> +++ b/drivers/platform/chrome/cros_ec_lpc_reg.c
> @@ -0,0 +1,64 @@
> +/*
> + * cros_ec_lpc_reg - LPC access to the Chrome OS Embedded Controller
> + *
> + * Copyright (C) 2016 Google, Inc
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * This driver uses the Chrome OS EC byte-level message-based protocol for
> + * communicating the keyboard state (which keys are pressed) from a keyboard EC
> + * to the AP over some bus (such as i2c, lpc, spi).  The EC does debouncing,
> + * but everything else (including deghosting) is done here.  The main
> + * motivation for this is to keep the EC firmware as simple as possible, since
> + * it cannot be easily upgraded and EC flash/IRAM space is relatively
> + * expensive.
> + */
> +
> +#include <linux/io.h>
> +#include <linux/mfd/cros_ec.h>
> +#include <linux/mfd/cros_ec_commands.h>
> +
> +static u8 lpc_read_bytes(unsigned int offset, unsigned int length, u8 *dest)
> +{
> +	int i;
> +	int sum = 0;
> +
> +	for (i = 0; i < length; ++i) {
> +		dest[i] = inb(offset + i);
> +		sum += dest[i];
> +	}
> +
> +	/* Return checksum of all bytes read */
> +	return sum;
> +}
> +
> +static u8 lpc_write_bytes(unsigned int offset, unsigned int length, u8 *msg)
> +{
> +	int i;
> +	int sum = 0;
> +
> +	for (i = 0; i < length; ++i) {
> +		outb(msg[i], offset + i);
> +		sum += msg[i];
> +	}
> +
> +	/* Return checksum of all bytes written */
> +	return sum;
> +}
> +
> +u8 cros_ec_lpc_read_bytes(unsigned int offset, unsigned int length, u8 *dest)
> +{
> +	return lpc_read_bytes(offset, length, dest);
> +}
> +
> +u8 cros_ec_lpc_write_bytes(unsigned int offset, unsigned int length, u8 *msg)
> +{
> +	return lpc_write_bytes(offset, length, msg);
> +}
> diff --git a/include/linux/mfd/cros_ec_lpc_reg.h b/include/linux/mfd/cros_ec_lpc_reg.h
> new file mode 100644
> index 0000000..4089bd5
> --- /dev/null
> +++ b/include/linux/mfd/cros_ec_lpc_reg.h
> @@ -0,0 +1,47 @@
> +/*
> + * cros_ec_lpc_reg - LPC access to the Chrome OS Embedded Controller
> + *
> + * Copyright (C) 2016 Google, Inc
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * This driver uses the Chrome OS EC byte-level message-based protocol for
> + * communicating the keyboard state (which keys are pressed) from a keyboard EC
> + * to the AP over some bus (such as i2c, lpc, spi).  The EC does debouncing,
> + * but everything else (including deghosting) is done here.  The main
> + * motivation for this is to keep the EC firmware as simple as possible, since
> + * it cannot be easily upgraded and EC flash/IRAM space is relatively
> + * expensive.
> + */
> +
> +#ifndef __LINUX_MFD_CROS_EC_REG_H
> +#define __LINUX_MFD_CROS_EC_REG_H
> +
> +/**
> + * cros_ec_lpc_read_bytes - Read bytes from a given LPC-mapped address.
> + * Returns 8-bit checksum of all bytes read.
> + *
> + * @offset: Base read address
> + * @length: Number of bytes to read
> + * @dest: Destination buffer
> + */
> +u8 cros_ec_lpc_read_bytes(unsigned int offset, unsigned int length, u8 *dest);
> +
> +/**
> + * cros_ec_lpc_write_bytes - Write bytes to a given LPC-mapped address.
> + * Returns 8-bit checksum of all bytes written.
> + *
> + * @offset: Base write address
> + * @length: Number of bytes to write
> + * @msg: Write data buffer
> + */
> +u8 cros_ec_lpc_write_bytes(unsigned int offset, unsigned int length, u8 *msg);
> +
> +#endif /* __LINUX_MFD_CROS_EC_REG_H */
> -- 
> 2.9.3
> 

-- 
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@google.com
Chromium OS Project
bleung@chromium.org

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

* Re: [PATCH RESEND 09/13] platform/chrome: cros_ec_lpc: Add MKBP events support over ACPI
  2017-05-16 16:13 ` [PATCH RESEND 09/13] platform/chrome: cros_ec_lpc: Add MKBP events support over ACPI Enric Balletbo i Serra
@ 2017-06-22 19:35   ` Benson Leung
  2017-06-23  7:35     ` Thierry Escande
  0 siblings, 1 reply; 31+ messages in thread
From: Benson Leung @ 2017-06-22 19:35 UTC (permalink / raw)
  To: Enric Balletbo i Serra
  Cc: olof, bleung, linux-kernel, lee.jones, Gwendal Grignou,
	Thierry Escande, bleung

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

Hi Enric and Thierry,

Just one minor question about this commit.

On Tue, May 16, 2017 at 06:13:15PM +0200, Enric Balletbo i Serra wrote:
> From: Gwendal Grignou <gwendal@chromium.org>
> 
> This patch installs a notify handler to process MKBP events for EC
> firmware directing them over ACPI.
> 
> Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
> Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
> ---
>  drivers/platform/chrome/cros_ec_lpc.c | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
> index 89afad7..eeb187e 100644
> --- a/drivers/platform/chrome/cros_ec_lpc.c
> +++ b/drivers/platform/chrome/cros_ec_lpc.c
> @@ -227,9 +227,20 @@ static int cros_ec_lpc_readmem(struct cros_ec_device *ec, unsigned int offset,
>  	return cnt;
>  }
>  
> +static void cros_ec_lpc_acpi_notify(acpi_handle device, u32 value, void *data)
> +{
> +	struct cros_ec_device *ec_dev = data;
> +
> +	if (ec_dev->mkbp_event_supported && cros_ec_get_next_event(ec_dev) > 0)
> +		blocking_notifier_call_chain(&ec_dev->event_notifier, 0,
> +					     ec_dev);
> +}
> +
>  static int cros_ec_lpc_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> +	struct acpi_device *adev;
> +	acpi_status status;
>  	struct cros_ec_device *ec_dev;
>  	u8 buf[2];
>  	int ret;
> @@ -277,12 +288,33 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> +	/*
> +	 * Connect a notify handler to process MKBP messages if we have a
> +	 * companion ACPI device.
> +	 */
> +	adev = ACPI_COMPANION(dev);
> +	if (adev) {
> +		status = acpi_install_notify_handler(adev->handle,
> +						     ACPI_ALL_NOTIFY,

Is there a reason you're using ACPI_ALL_NOTIFY here instead of
ACPI_SYSTEM_NOTIFY that is done in the CHROMIUM version of this?

> +						     cros_ec_lpc_acpi_notify,
> +						     ec_dev);
> +		if (ACPI_FAILURE(status))
> +			dev_warn(dev, "Failed to register notifier %08x\n",
> +				 status);
> +	}
> +
>  	return 0;
>  }
>  
>  static int cros_ec_lpc_remove(struct platform_device *pdev)
>  {
>  	struct cros_ec_device *ec_dev;
> +	struct acpi_device *adev;
> +
> +	adev = ACPI_COMPANION(&pdev->dev);
> +	if (adev)
> +		acpi_remove_notify_handler(adev->handle, ACPI_ALL_NOTIFY,
> +					   cros_ec_lpc_acpi_notify);
>  
>  	ec_dev = platform_get_drvdata(pdev);
>  	cros_ec_remove(ec_dev);
> -- 
> 2.9.3
> 

-- 
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@google.com
Chromium OS Project
bleung@chromium.org

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

* Re: [PATCH RESEND 07/13] platform/chrome: cros_ec_lpc: Add support for GOOG004 ACPI device
  2017-05-16 16:13 ` [PATCH RESEND 07/13] platform/chrome: cros_ec_lpc: Add support for GOOG004 ACPI device Enric Balletbo i Serra
@ 2017-06-22 19:39   ` Benson Leung
  0 siblings, 0 replies; 31+ messages in thread
From: Benson Leung @ 2017-06-22 19:39 UTC (permalink / raw)
  To: Enric Balletbo i Serra
  Cc: olof, bleung, linux-kernel, lee.jones, Gwendal Grignou, Thierry Escande

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

Hi Enric and Thierry,

On Tue, May 16, 2017 at 06:13:13PM +0200, Enric Balletbo i Serra wrote:
> From: Gwendal Grignou <gwendal@chromium.org>
> 
> This patch removes platform_device_register() call and adds an ACPI
> device id structure. The driver is now automatically probed for devices
> with a GOOG0004 ACPI entry.
> 
> Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
> Signed-off-by: Thierry Escande <thierry.escande@collabora.com>

Signed-off-by: Benson Leung <bleung@chromium.org>

Applied to what will become the immutable branch.

> ---
>  drivers/platform/chrome/Kconfig       |  2 +-
>  drivers/platform/chrome/cros_ec_lpc.c | 23 +++++++++--------------
>  2 files changed, 10 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
> index 6d80fb5..0ad6e29 100644
> --- a/drivers/platform/chrome/Kconfig
> +++ b/drivers/platform/chrome/Kconfig
> @@ -49,7 +49,7 @@ config CROS_EC_CHARDEV
>  
>  config CROS_EC_LPC
>          tristate "ChromeOS Embedded Controller (LPC)"
> -        depends on MFD_CROS_EC && (X86 || COMPILE_TEST)
> +        depends on MFD_CROS_EC && ACPI && (X86 || COMPILE_TEST)
>          help
>            If you say Y here, you get support for talking to the ChromeOS EC
>            over an LPC bus. This uses a simple byte-level protocol with a
> diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
> index bc2dc62..90703521 100644
> --- a/drivers/platform/chrome/cros_ec_lpc.c
> +++ b/drivers/platform/chrome/cros_ec_lpc.c
> @@ -21,6 +21,7 @@
>   * expensive.
>   */
>  
> +#include <linux/acpi.h>
>  #include <linux/dmi.h>
>  #include <linux/delay.h>
>  #include <linux/io.h>
> @@ -32,6 +33,7 @@
>  #include <linux/printk.h>
>  
>  #define DRV_NAME "cros_ec_lpcs"
> +#define ACPI_DRV_NAME "GOOG0004"
>  
>  static int ec_response_timed_out(void)
>  {
> @@ -288,6 +290,12 @@ static int cros_ec_lpc_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +static const struct acpi_device_id cros_ec_lpc_acpi_device_ids[] = {
> +	{ ACPI_DRV_NAME, 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(acpi, cros_ec_lpc_acpi_device_ids);
> +
>  static struct dmi_system_id cros_ec_lpc_dmi_table[] __initdata = {
>  	{
>  		/*
> @@ -328,15 +336,12 @@ MODULE_DEVICE_TABLE(dmi, cros_ec_lpc_dmi_table);
>  static struct platform_driver cros_ec_lpc_driver = {
>  	.driver = {
>  		.name = DRV_NAME,
> +		.acpi_match_table = cros_ec_lpc_acpi_device_ids,
>  	},
>  	.probe = cros_ec_lpc_probe,
>  	.remove = cros_ec_lpc_remove,
>  };
>  
> -static struct platform_device cros_ec_lpc_device = {
> -	.name = DRV_NAME
> -};
> -
>  static int __init cros_ec_lpc_init(void)
>  {
>  	int ret;
> @@ -356,21 +361,11 @@ static int __init cros_ec_lpc_init(void)
>  		return ret;
>  	}
>  
> -	/* Register the device, and it'll get hooked up automatically */
> -	ret = platform_device_register(&cros_ec_lpc_device);
> -	if (ret) {
> -		pr_err(DRV_NAME ": can't register device: %d\n", ret);
> -		platform_driver_unregister(&cros_ec_lpc_driver);
> -		cros_ec_lpc_reg_destroy();
> -		return ret;
> -	}
> -
>  	return 0;
>  }
>  
>  static void __exit cros_ec_lpc_exit(void)
>  {
> -	platform_device_unregister(&cros_ec_lpc_device);
>  	platform_driver_unregister(&cros_ec_lpc_driver);
>  	cros_ec_lpc_reg_destroy();
>  }
> -- 
> 2.9.3
> 

-- 
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@google.com
Chromium OS Project
bleung@chromium.org

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

* Re: [PATCH RESEND 08/13] platform/chrome: cros_ec_lpc: Add power management ops
  2017-05-16 16:13 ` [PATCH RESEND 08/13] platform/chrome: cros_ec_lpc: Add power management ops Enric Balletbo i Serra
@ 2017-06-22 19:46   ` Benson Leung
  0 siblings, 0 replies; 31+ messages in thread
From: Benson Leung @ 2017-06-22 19:46 UTC (permalink / raw)
  To: Enric Balletbo i Serra
  Cc: olof, bleung, linux-kernel, lee.jones, Archana Patni,
	Thierry Escande, bleung

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

Hi Enric and Thierry,

On Tue, May 16, 2017 at 06:13:14PM +0200, Enric Balletbo i Serra wrote:
> From: Archana Patni <archana.patni@intel.com>
> 
> This patch adds suspend and resume pm ops to the LPC ChromeOS EC driver.
> These LPC handlers call the croc_ec generic handlers.
> 
> Signed-off-by: Archana Patni <archana.patni@intel.com>
> Signed-off-by: Thierry Escande <thierry.escande@collabora.com>

Signed-off-by: Benson Leung <bleung@chromium.org>

Applied. Thanks.

> ---
>  drivers/platform/chrome/cros_ec_lpc.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
> index 90703521..89afad7 100644
> --- a/drivers/platform/chrome/cros_ec_lpc.c
> +++ b/drivers/platform/chrome/cros_ec_lpc.c
> @@ -333,10 +333,31 @@ static struct dmi_system_id cros_ec_lpc_dmi_table[] __initdata = {
>  };
>  MODULE_DEVICE_TABLE(dmi, cros_ec_lpc_dmi_table);
>  
> +#ifdef CONFIG_PM_SLEEP
> +static int cros_ec_lpc_suspend(struct device *dev)
> +{
> +	struct cros_ec_device *ec_dev = dev_get_drvdata(dev);
> +
> +	return cros_ec_suspend(ec_dev);
> +}
> +
> +static int cros_ec_lpc_resume(struct device *dev)
> +{
> +	struct cros_ec_device *ec_dev = dev_get_drvdata(dev);
> +
> +	return cros_ec_resume(ec_dev);
> +}
> +#endif
> +
> +const struct dev_pm_ops cros_ec_lpc_pm_ops = {
> +	SET_LATE_SYSTEM_SLEEP_PM_OPS(cros_ec_lpc_suspend, cros_ec_lpc_resume)
> +};
> +
>  static struct platform_driver cros_ec_lpc_driver = {
>  	.driver = {
>  		.name = DRV_NAME,
>  		.acpi_match_table = cros_ec_lpc_acpi_device_ids,
> +		.pm = &cros_ec_lpc_pm_ops,
>  	},
>  	.probe = cros_ec_lpc_probe,
>  	.remove = cros_ec_lpc_remove,
> -- 
> 2.9.3
> 

-- 
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@google.com
Chromium OS Project
bleung@chromium.org

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

* Re: [PATCH RESEND 04/13] mfd: cros_ec: Add support for dumping panic information
  2017-05-16 16:13 ` [PATCH RESEND 04/13] mfd: cros_ec: Add support for dumping panic information Enric Balletbo i Serra
@ 2017-06-22 23:38   ` Benson Leung
  0 siblings, 0 replies; 31+ messages in thread
From: Benson Leung @ 2017-06-22 23:38 UTC (permalink / raw)
  To: Enric Balletbo i Serra
  Cc: olof, bleung, linux-kernel, lee.jones, Nicolas Boichat, bleung

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

Hi Enric,

On Tue, May 16, 2017 at 06:13:10PM +0200, Enric Balletbo i Serra wrote:
> From: Nicolas Boichat <drinkcat@chromium.org>
> 
> This dumps the EC panic information from the previous reboot.
> 
> Similar to the information presented by ectool panicinfo, except
> that we do not bother doing any parsing (we should write a small
> offline tool for that).
> 
> Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>
> Reviewed-by: Guenter Roeck <groeck@chromium.org>
> Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>

Signed-off-by: Benson Leung <bleung@chromium.org>
Applied. Thanks.

> ---
>  drivers/platform/chrome/cros_ec_debugfs.c | 54 +++++++++++++++++++++++++++++++
>  1 file changed, 54 insertions(+)
> 
> diff --git a/drivers/platform/chrome/cros_ec_debugfs.c b/drivers/platform/chrome/cros_ec_debugfs.c
> index f8195bb..8133c33 100644
> --- a/drivers/platform/chrome/cros_ec_debugfs.c
> +++ b/drivers/platform/chrome/cros_ec_debugfs.c
> @@ -47,15 +47,19 @@
>   * @log_mutex: mutex to protect circular buffer
>   * @log_wq: waitqueue for log readers
>   * @log_poll_work: recurring task to poll EC for new console log data
> + * @panicinfo_blob: panicinfo debugfs blob
>   */
>  struct cros_ec_debugfs {
>  	struct cros_ec_dev *ec;
>  	struct dentry *dir;
> +	/* EC log */
>  	struct circ_buf log_buffer;
>  	struct cros_ec_command *read_msg;
>  	struct mutex log_mutex;
>  	wait_queue_head_t log_wq;
>  	struct delayed_work log_poll_work;
> +	/* EC panicinfo */
> +	struct debugfs_blob_wrapper panicinfo_blob;
>  };
>  
>  /*
> @@ -308,6 +312,52 @@ static void cros_ec_cleanup_console_log(struct cros_ec_debugfs *debug_info)
>  	}
>  }
>  
> +static int cros_ec_create_panicinfo(struct cros_ec_debugfs *debug_info)
> +{
> +	struct cros_ec_device *ec_dev = debug_info->ec->ec_dev;
> +	int ret;
> +	struct cros_ec_command *msg;
> +	int insize;
> +
> +	insize = ec_dev->max_response;
> +
> +	msg = devm_kzalloc(debug_info->ec->dev,
> +			sizeof(*msg) + insize, GFP_KERNEL);
> +	if (!msg)
> +		return -ENOMEM;
> +
> +	msg->command = EC_CMD_GET_PANIC_INFO;
> +	msg->insize = insize;
> +
> +	ret = cros_ec_cmd_xfer(ec_dev, msg);
> +	if (ret < 0) {
> +		dev_warn(debug_info->ec->dev, "Cannot read panicinfo.\n");
> +		ret = 0;
> +		goto free;
> +	}
> +
> +	/* No panic data */
> +	if (ret == 0)
> +		goto free;
> +
> +	debug_info->panicinfo_blob.data = msg->data;
> +	debug_info->panicinfo_blob.size = ret;
> +
> +	if (!debugfs_create_blob("panicinfo",
> +				 S_IFREG | S_IRUGO,
> +				 debug_info->dir,
> +				 &debug_info->panicinfo_blob)) {
> +		ret = -ENOMEM;
> +		goto free;
> +	}
> +
> +	return 0;
> +
> +free:
> +	devm_kfree(debug_info->ec->dev, msg);
> +	return ret;
> +}
> +
>  int cros_ec_debugfs_init(struct cros_ec_dev *ec)
>  {
>  	struct cros_ec_platform *ec_platform = dev_get_platdata(ec->dev);
> @@ -324,6 +374,10 @@ int cros_ec_debugfs_init(struct cros_ec_dev *ec)
>  	if (!debug_info->dir)
>  		return -ENOMEM;
>  
> +	ret = cros_ec_create_panicinfo(debug_info);
> +	if (ret)
> +		goto remove_debugfs;
> +
>  	ret = cros_ec_create_console_log(debug_info);
>  	if (ret)
>  		goto remove_debugfs;
> -- 
> 2.9.3
> 

-- 
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@google.com
Chromium OS Project
bleung@chromium.org

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

* Re: [PATCH RESEND 10/13] platform/chrome: cros_ec_lightbar - Add lightbar program feature to sysfs
  2017-05-16 16:13 ` [PATCH RESEND 10/13] platform/chrome: cros_ec_lightbar - Add lightbar program feature to sysfs Enric Balletbo i Serra
@ 2017-06-22 23:43   ` Benson Leung
  0 siblings, 0 replies; 31+ messages in thread
From: Benson Leung @ 2017-06-22 23:43 UTC (permalink / raw)
  To: Enric Balletbo i Serra
  Cc: olof, bleung, linux-kernel, lee.jones, Eric Caruso, Guenter Roeck

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

Hi Enric,

On Tue, May 16, 2017 at 06:13:16PM +0200, Enric Balletbo i Serra wrote:
> From: Eric Caruso <ejcaruso@chromium.org>
> 
> Add a program feature so we can upload and run programs for lightbar
> sequences. We should be able to use this to shift sequences out of the
> EC and save space there.
> 
>   $ cat <suitable program bin> > /sys/devices/.../cros_ec/program
>   $ echo program > /sys/devices/.../cros_ec/sequence
> 
> Signed-off-by: Eric Caruso <ejcaruso@chromium.org>
> Signed-off-by: Guenter Roeck <groeck@chromium.org>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Acked-by: Lee Jones <lee.jones@linaro.org>

Signed-off-by: Benson Leung <bleung@chromium.org>
Applied.

> ---
>  drivers/platform/chrome/cros_ec_lightbar.c | 69 +++++++++++++++++++++++++++++-
>  include/linux/mfd/cros_ec_commands.h       | 12 +++++-
>  2 files changed, 79 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/platform/chrome/cros_ec_lightbar.c b/drivers/platform/chrome/cros_ec_lightbar.c
> index 8df3d44..2667505 100644
> --- a/drivers/platform/chrome/cros_ec_lightbar.c
> +++ b/drivers/platform/chrome/cros_ec_lightbar.c
> @@ -295,7 +295,8 @@ static ssize_t led_rgb_store(struct device *dev, struct device_attribute *attr,
>  
>  static char const *seqname[] = {
>  	"ERROR", "S5", "S3", "S0", "S5S3", "S3S0",
> -	"S0S3", "S3S5", "STOP", "RUN", "PULSE", "TEST", "KONAMI",
> +	"S0S3", "S3S5", "STOP", "RUN", "KONAMI",
> +	"TAP", "PROGRAM",
>  };
>  
>  static ssize_t sequence_show(struct device *dev,
> @@ -390,6 +391,69 @@ static ssize_t sequence_store(struct device *dev, struct device_attribute *attr,
>  	return ret;
>  }
>  
> +static ssize_t program_store(struct device *dev, struct device_attribute *attr,
> +			     const char *buf, size_t count)
> +{
> +	int extra_bytes, max_size, ret;
> +	struct ec_params_lightbar *param;
> +	struct cros_ec_command *msg;
> +	struct cros_ec_dev *ec = container_of(dev, struct cros_ec_dev,
> +					      class_dev);
> +
> +	/*
> +	 * We might need to reject the program for size reasons. The EC
> +	 * enforces a maximum program size, but we also don't want to try
> +	 * and send a program that is too big for the protocol. In order
> +	 * to ensure the latter, we also need to ensure we have extra bytes
> +	 * to represent the rest of the packet.
> +	 */
> +	extra_bytes = sizeof(*param) - sizeof(param->set_program.data);
> +	max_size = min(EC_LB_PROG_LEN, ec->ec_dev->max_request - extra_bytes);
> +	if (count > max_size) {
> +		dev_err(dev, "Program is %u bytes, too long to send (max: %u)",
> +			(unsigned int)count, max_size);
> +
> +		return -EINVAL;
> +	}
> +
> +	msg = alloc_lightbar_cmd_msg(ec);
> +	if (!msg)
> +		return -ENOMEM;
> +
> +	ret = lb_throttle();
> +	if (ret)
> +		goto exit;
> +
> +	dev_info(dev, "Copying %zu byte program to EC", count);
> +
> +	param = (struct ec_params_lightbar *)msg->data;
> +	param->cmd = LIGHTBAR_CMD_SET_PROGRAM;
> +
> +	param->set_program.size = count;
> +	memcpy(param->set_program.data, buf, count);
> +
> +	/*
> +	 * We need to set the message size manually or else it will use
> +	 * EC_LB_PROG_LEN. This might be too long, and the program
> +	 * is unlikely to use all of the space.
> +	 */
> +	msg->outsize = count + extra_bytes;
> +
> +	ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
> +	if (ret < 0)
> +		goto exit;
> +	if (msg->result != EC_RES_SUCCESS) {
> +		ret = -EINVAL;
> +		goto exit;
> +	}
> +
> +	ret = count;
> +exit:
> +	kfree(msg);
> +
> +	return ret;
> +}
> +
>  /* Module initialization */
>  
>  static DEVICE_ATTR_RW(interval_msec);
> @@ -397,12 +461,15 @@ static DEVICE_ATTR_RO(version);
>  static DEVICE_ATTR_WO(brightness);
>  static DEVICE_ATTR_WO(led_rgb);
>  static DEVICE_ATTR_RW(sequence);
> +static DEVICE_ATTR_WO(program);
> +
>  static struct attribute *__lb_cmds_attrs[] = {
>  	&dev_attr_interval_msec.attr,
>  	&dev_attr_version.attr,
>  	&dev_attr_brightness.attr,
>  	&dev_attr_led_rgb.attr,
>  	&dev_attr_sequence.attr,
> +	&dev_attr_program.attr,
>  	NULL,
>  };
>  
> diff --git a/include/linux/mfd/cros_ec_commands.h b/include/linux/mfd/cros_ec_commands.h
> index 1b19e424..dbea580 100644
> --- a/include/linux/mfd/cros_ec_commands.h
> +++ b/include/linux/mfd/cros_ec_commands.h
> @@ -1162,6 +1162,13 @@ struct lightbar_params_v1 {
>  	struct rgb_s color[8];			/* 0-3 are Google colors */
>  } __packed;
>  
> +/* Lightbar program */
> +#define EC_LB_PROG_LEN 192
> +struct lightbar_program {
> +	uint8_t size;
> +	uint8_t data[EC_LB_PROG_LEN];
> +};
> +
>  struct ec_params_lightbar {
>  	uint8_t cmd;		      /* Command (see enum lightbar_command) */
>  	union {
> @@ -1188,6 +1195,7 @@ struct ec_params_lightbar {
>  
>  		struct lightbar_params_v0 set_params_v0;
>  		struct lightbar_params_v1 set_params_v1;
> +		struct lightbar_program set_program;
>  	};
>  } __packed;
>  
> @@ -1220,7 +1228,8 @@ struct ec_response_lightbar {
>  		struct {
>  			/* no return params */
>  		} off, on, init, set_brightness, seq, reg, set_rgb,
> -			demo, set_params_v0, set_params_v1;
> +			demo, set_params_v0, set_params_v1,
> +			set_program;
>  	};
>  } __packed;
>  
> @@ -1244,6 +1253,7 @@ enum lightbar_command {
>  	LIGHTBAR_CMD_GET_DEMO = 15,
>  	LIGHTBAR_CMD_GET_PARAMS_V1 = 16,
>  	LIGHTBAR_CMD_SET_PARAMS_V1 = 17,
> +	LIGHTBAR_CMD_SET_PROGRAM = 18,
>  	LIGHTBAR_NUM_CMDS
>  };
>  
> -- 
> 2.9.3
> 

-- 
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@google.com
Chromium OS Project
bleung@chromium.org

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

* Re: [PATCH RESEND 09/13] platform/chrome: cros_ec_lpc: Add MKBP events support over ACPI
  2017-06-22 19:35   ` Benson Leung
@ 2017-06-23  7:35     ` Thierry Escande
  2017-06-23 17:54       ` Benson Leung
  0 siblings, 1 reply; 31+ messages in thread
From: Thierry Escande @ 2017-06-23  7:35 UTC (permalink / raw)
  To: Benson Leung, Enric Balletbo i Serra
  Cc: olof, bleung, linux-kernel, lee.jones, Gwendal Grignou

Hi Benson,

On 22/06/2017 21:35, Benson Leung wrote:

<snip>

>> +	adev = ACPI_COMPANION(dev);
>> +	if (adev) {
>> +		status = acpi_install_notify_handler(adev->handle,
>> +						     ACPI_ALL_NOTIFY,
> 
> Is there a reason you're using ACPI_ALL_NOTIFY here instead of
> ACPI_SYSTEM_NOTIFY that is done in the CHROMIUM version of this?
> 
In the original patch 
(https://chromium-review.googlesource.com/c/358155/) ACPI_ALL_NOTIFY is 
passed to acpi_install_notify_handler() and ACPI_SYSTEM_NOTIFY to 
acpi_remove_notify_handler. I changed it for remove_notify call to 
unsure all handler references were removed.

Regards,
  Thierry

>> +						     cros_ec_lpc_acpi_notify,
>> +						     ec_dev);
>> +		if (ACPI_FAILURE(status))
>> +			dev_warn(dev, "Failed to register notifier %08x\n",
>> +				 status);
>> +	}
>> +
>>   	return 0;
>>   }
>>   
>>   static int cros_ec_lpc_remove(struct platform_device *pdev)
>>   {
>>   	struct cros_ec_device *ec_dev;
>> +	struct acpi_device *adev;
>> +
>> +	adev = ACPI_COMPANION(&pdev->dev);
>> +	if (adev)
>> +		acpi_remove_notify_handler(adev->handle, ACPI_ALL_NOTIFY,
>> +					   cros_ec_lpc_acpi_notify);
>>   
>>   	ec_dev = platform_get_drvdata(pdev);
>>   	cros_ec_remove(ec_dev);
>> -- 
>> 2.9.3
>>
> 

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

* Re: [PATCH RESEND 09/13] platform/chrome: cros_ec_lpc: Add MKBP events support over ACPI
  2017-06-23  7:35     ` Thierry Escande
@ 2017-06-23 17:54       ` Benson Leung
  0 siblings, 0 replies; 31+ messages in thread
From: Benson Leung @ 2017-06-23 17:54 UTC (permalink / raw)
  To: Thierry Escande
  Cc: Enric Balletbo i Serra, olof, bleung, linux-kernel, lee.jones,
	Gwendal Grignou

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

Hi Thierry,

On Fri, Jun 23, 2017 at 09:35:06AM +0200, Thierry Escande wrote:
> Hi Benson,
> 
> On 22/06/2017 21:35, Benson Leung wrote:
> 
> <snip>
> 
> >>+	adev = ACPI_COMPANION(dev);
> >>+	if (adev) {
> >>+		status = acpi_install_notify_handler(adev->handle,
> >>+						     ACPI_ALL_NOTIFY,
> >
> >Is there a reason you're using ACPI_ALL_NOTIFY here instead of
> >ACPI_SYSTEM_NOTIFY that is done in the CHROMIUM version of this?
> >
> In the original patch
> (https://chromium-review.googlesource.com/c/358155/) ACPI_ALL_NOTIFY
> is passed to acpi_install_notify_handler() and ACPI_SYSTEM_NOTIFY to
> acpi_remove_notify_handler. I changed it for remove_notify call to
> unsure all handler references were removed.
> 

Oh, I see. Looks good. I'll go ahead and make the same change in the
chromeos-4.4 kernel too.

Signed-off-by: Benson Leung <bleung@chromium.org>

Applied.

-- 
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@google.com
Chromium OS Project
bleung@chromium.org

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

* Re: [PATCH RESEND 11/13] platform/chrome: cros_ec_lightbar - Control of suspend/resume lightbar sequence
  2017-05-16 16:13 ` [PATCH RESEND 11/13] platform/chrome: cros_ec_lightbar - Control of suspend/resume lightbar sequence Enric Balletbo i Serra
@ 2017-06-23 22:08   ` Benson Leung
  0 siblings, 0 replies; 31+ messages in thread
From: Benson Leung @ 2017-06-23 22:08 UTC (permalink / raw)
  To: Enric Balletbo i Serra, olof, bleung, linux-kernel
  Cc: lee.jones, Eric Caruso, Guenter Roeck


[-- Attachment #1.1: Type: text/plain, Size: 8253 bytes --]

Hi Enric,

On 05/16/2017 09:13 AM, Enric Balletbo i Serra wrote:
> From: Eric Caruso <ejcaruso@chromium.org>
> 
> Don't let EC control suspend/resume sequence. If the EC controls the
> lightbar and sets the sequence when it notices the chipset transitioning
> between states, we can't make exceptions for cases where we don't want
> to activate the lightbar. Instead, let's move the suspend/resume
> notifications into the kernel so we can selectively play the sequences.
> 
> Signed-off-by: Eric Caruso <ejcaruso@chromium.org>
> Signed-off-by: Guenter Roeck <groeck@chromium.org>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Acked-by: Lee Jones <lee.jones@linaro.org>

Signed-off-by: Benson Leung <bleung@chromium.org>

Applied. Thanks.

> ---
>  drivers/platform/chrome/cros_ec_dev.c      | 37 +++++++++++++
>  drivers/platform/chrome/cros_ec_dev.h      |  6 +++
>  drivers/platform/chrome/cros_ec_lightbar.c | 85 ++++++++++++++++++++++++++++--
>  include/linux/mfd/cros_ec_commands.h       | 11 +++-
>  4 files changed, 134 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/platform/chrome/cros_ec_dev.c b/drivers/platform/chrome/cros_ec_dev.c
> index 20ce1c2..7c26223 100644
> --- a/drivers/platform/chrome/cros_ec_dev.c
> +++ b/drivers/platform/chrome/cros_ec_dev.c
> @@ -21,6 +21,7 @@
>  #include <linux/mfd/core.h>
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
> +#include <linux/pm.h>
>  #include <linux/slab.h>
>  #include <linux/uaccess.h>
>  
> @@ -435,6 +436,10 @@ static int ec_device_probe(struct platform_device *pdev)
>  	if (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE))
>  		cros_ec_sensors_register(ec);
>  
> +	/* Take control of the lightbar from the EC. */
> +	if (ec_has_lightbar(ec))
> +		lb_manual_suspend_ctrl(ec, 1);
> +
>  	return 0;
>  
>  failed:
> @@ -446,6 +451,10 @@ static int ec_device_remove(struct platform_device *pdev)
>  {
>  	struct cros_ec_dev *ec = dev_get_drvdata(&pdev->dev);
>  
> +	/* Let the EC take over the lightbar again. */
> +	if (ec_has_lightbar(ec))
> +		lb_manual_suspend_ctrl(ec, 0);
> +
>  	cros_ec_debugfs_remove(ec);
>  
>  	cdev_del(&ec->cdev);
> @@ -459,9 +468,37 @@ static const struct platform_device_id cros_ec_id[] = {
>  };
>  MODULE_DEVICE_TABLE(platform, cros_ec_id);
>  
> +static int ec_device_suspend(struct device *dev)
> +{
> +	struct cros_ec_dev *ec = dev_get_drvdata(dev);
> +
> +	if (ec_has_lightbar(ec))
> +		lb_suspend(ec);
> +
> +	return 0;
> +}
> +
> +static int ec_device_resume(struct device *dev)
> +{
> +	struct cros_ec_dev *ec = dev_get_drvdata(dev);
> +
> +	if (ec_has_lightbar(ec))
> +		lb_resume(ec);
> +
> +	return 0;
> +}
> +
> +static const struct dev_pm_ops cros_ec_dev_pm_ops = {
> +#ifdef CONFIG_PM_SLEEP
> +	.suspend = ec_device_suspend,
> +	.resume = ec_device_resume,
> +#endif
> +};
> +
>  static struct platform_driver cros_ec_dev_driver = {
>  	.driver = {
>  		.name = "cros-ec-ctl",
> +		.pm = &cros_ec_dev_pm_ops,
>  	},
>  	.probe = ec_device_probe,
>  	.remove = ec_device_remove,
> diff --git a/drivers/platform/chrome/cros_ec_dev.h b/drivers/platform/chrome/cros_ec_dev.h
> index bfd2c84..45e9453 100644
> --- a/drivers/platform/chrome/cros_ec_dev.h
> +++ b/drivers/platform/chrome/cros_ec_dev.h
> @@ -43,4 +43,10 @@ struct cros_ec_readmem {
>  #define CROS_EC_DEV_IOCXCMD   _IOWR(CROS_EC_DEV_IOC, 0, struct cros_ec_command)
>  #define CROS_EC_DEV_IOCRDMEM  _IOWR(CROS_EC_DEV_IOC, 1, struct cros_ec_readmem)
>  
> +/* Lightbar utilities */
> +extern bool ec_has_lightbar(struct cros_ec_dev *ec);
> +extern int lb_manual_suspend_ctrl(struct cros_ec_dev *ec, uint8_t enable);
> +extern int lb_suspend(struct cros_ec_dev *ec);
> +extern int lb_resume(struct cros_ec_dev *ec);
> +
>  #endif /* _CROS_EC_DEV_H_ */
> diff --git a/drivers/platform/chrome/cros_ec_lightbar.c b/drivers/platform/chrome/cros_ec_lightbar.c
> index 2667505..4df379d 100644
> --- a/drivers/platform/chrome/cros_ec_lightbar.c
> +++ b/drivers/platform/chrome/cros_ec_lightbar.c
> @@ -341,6 +341,80 @@ static ssize_t sequence_show(struct device *dev,
>  	return ret;
>  }
>  
> +static int lb_send_empty_cmd(struct cros_ec_dev *ec, uint8_t cmd)
> +{
> +	struct ec_params_lightbar *param;
> +	struct cros_ec_command *msg;
> +	int ret;
> +
> +	msg = alloc_lightbar_cmd_msg(ec);
> +	if (!msg)
> +		return -ENOMEM;
> +
> +	param = (struct ec_params_lightbar *)msg->data;
> +	param->cmd = cmd;
> +
> +	ret = lb_throttle();
> +	if (ret)
> +		goto error;
> +
> +	ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
> +	if (ret < 0)
> +		goto error;
> +	if (msg->result != EC_RES_SUCCESS) {
> +		ret = -EINVAL;
> +		goto error;
> +	}
> +	ret = 0;
> +error:
> +	kfree(msg);
> +
> +	return ret;
> +}
> +
> +int lb_manual_suspend_ctrl(struct cros_ec_dev *ec, uint8_t enable)
> +{
> +	struct ec_params_lightbar *param;
> +	struct cros_ec_command *msg;
> +	int ret;
> +
> +	msg = alloc_lightbar_cmd_msg(ec);
> +	if (!msg)
> +		return -ENOMEM;
> +
> +	param = (struct ec_params_lightbar *)msg->data;
> +
> +	param->cmd = LIGHTBAR_CMD_MANUAL_SUSPEND_CTRL;
> +	param->manual_suspend_ctrl.enable = enable;
> +
> +	ret = lb_throttle();
> +	if (ret)
> +		goto error;
> +
> +	ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
> +	if (ret < 0)
> +		goto error;
> +	if (msg->result != EC_RES_SUCCESS) {
> +		ret = -EINVAL;
> +		goto error;
> +	}
> +	ret = 0;
> +error:
> +	kfree(msg);
> +
> +	return ret;
> +}
> +
> +int lb_suspend(struct cros_ec_dev *ec)
> +{
> +	return lb_send_empty_cmd(ec, LIGHTBAR_CMD_SUSPEND);
> +}
> +
> +int lb_resume(struct cros_ec_dev *ec)
> +{
> +	return lb_send_empty_cmd(ec, LIGHTBAR_CMD_RESUME);
> +}
> +
>  static ssize_t sequence_store(struct device *dev, struct device_attribute *attr,
>  			      const char *buf, size_t count)
>  {
> @@ -473,6 +547,11 @@ static struct attribute *__lb_cmds_attrs[] = {
>  	NULL,
>  };
>  
> +bool ec_has_lightbar(struct cros_ec_dev *ec)
> +{
> +	return !!get_lightbar_version(ec, NULL, NULL);
> +}
> +
>  static umode_t cros_ec_lightbar_attrs_are_visible(struct kobject *kobj,
>  						  struct attribute *a, int n)
>  {
> @@ -489,10 +568,10 @@ static umode_t cros_ec_lightbar_attrs_are_visible(struct kobject *kobj,
>  		return 0;
>  
>  	/* Only instantiate this stuff if the EC has a lightbar */
> -	if (get_lightbar_version(ec, NULL, NULL))
> +	if (ec_has_lightbar(ec))
>  		return a->mode;
> -	else
> -		return 0;
> +
> +	return 0;
>  }
>  
>  struct attribute_group cros_ec_lightbar_attr_group = {
> diff --git a/include/linux/mfd/cros_ec_commands.h b/include/linux/mfd/cros_ec_commands.h
> index dbea580..190c8f4 100644
> --- a/include/linux/mfd/cros_ec_commands.h
> +++ b/include/linux/mfd/cros_ec_commands.h
> @@ -1175,7 +1175,7 @@ struct ec_params_lightbar {
>  		struct {
>  			/* no args */
>  		} dump, off, on, init, get_seq, get_params_v0, get_params_v1,
> -			version, get_brightness, get_demo;
> +			version, get_brightness, get_demo, suspend, resume;
>  
>  		struct {
>  			uint8_t num;
> @@ -1193,6 +1193,10 @@ struct ec_params_lightbar {
>  			uint8_t led;
>  		} get_rgb;
>  
> +		struct {
> +			uint8_t enable;
> +		} manual_suspend_ctrl;
> +
>  		struct lightbar_params_v0 set_params_v0;
>  		struct lightbar_params_v1 set_params_v1;
>  		struct lightbar_program set_program;
> @@ -1229,7 +1233,7 @@ struct ec_response_lightbar {
>  			/* no return params */
>  		} off, on, init, set_brightness, seq, reg, set_rgb,
>  			demo, set_params_v0, set_params_v1,
> -			set_program;
> +			set_program, manual_suspend_ctrl, suspend, resume;
>  	};
>  } __packed;
>  
> @@ -1254,6 +1258,9 @@ enum lightbar_command {
>  	LIGHTBAR_CMD_GET_PARAMS_V1 = 16,
>  	LIGHTBAR_CMD_SET_PARAMS_V1 = 17,
>  	LIGHTBAR_CMD_SET_PROGRAM = 18,
> +	LIGHTBAR_CMD_MANUAL_SUSPEND_CTRL = 19,
> +	LIGHTBAR_CMD_SUSPEND = 20,
> +	LIGHTBAR_CMD_RESUME = 21,
>  	LIGHTBAR_NUM_CMDS
>  };
>  
> 

-- 
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@google.com
Chromium OS Project
bleung@chromium.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

* Re: [PATCH RESEND 12/13] platform/chrome: cros_ec_lightbar - Add userspace lightbar control bit to EC
  2017-05-16 16:13 ` [PATCH RESEND 12/13] platform/chrome: cros_ec_lightbar - Add userspace lightbar control bit to EC Enric Balletbo i Serra
@ 2017-06-23 22:12   ` Benson Leung
  0 siblings, 0 replies; 31+ messages in thread
From: Benson Leung @ 2017-06-23 22:12 UTC (permalink / raw)
  To: Enric Balletbo i Serra, olof, bleung, linux-kernel
  Cc: lee.jones, Eric Caruso, Guenter Roeck


[-- Attachment #1.1: Type: text/plain, Size: 3289 bytes --]

Hi Enric,

Looks good.

On 05/16/2017 09:13 AM, Enric Balletbo i Serra wrote:
> From: Eric Caruso <ejcaruso@chromium.org>
> 
> Some devices might want to turn off the lightbar if e.g. the
> system turns the screen off due to idleness. This prevents the
> kernel from going through its normal suspend/resume pathways.
> 
> Signed-off-by: Eric Caruso <ejcaruso@chromium.org>
> Signed-off-by: Guenter Roeck <groeck@chromium.org>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>

Signed-off-by: Benson Leung <bleung@chromium.org>

Applied.

> ---
>  drivers/platform/chrome/cros_ec_lightbar.c | 38 ++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
> 
> diff --git a/drivers/platform/chrome/cros_ec_lightbar.c b/drivers/platform/chrome/cros_ec_lightbar.c
> index 4df379d..e570c1e 100644
> --- a/drivers/platform/chrome/cros_ec_lightbar.c
> +++ b/drivers/platform/chrome/cros_ec_lightbar.c
> @@ -38,6 +38,12 @@
>  /* Rate-limit the lightbar interface to prevent DoS. */
>  static unsigned long lb_interval_jiffies = 50 * HZ / 1000;
>  
> +/*
> + * Whether or not we have given userspace control of the lightbar.
> + * If this is true, we won't do anything during suspend/resume.
> + */
> +static bool userspace_control;
> +
>  static ssize_t interval_msec_show(struct device *dev,
>  				  struct device_attribute *attr, char *buf)
>  {
> @@ -407,11 +413,17 @@ int lb_manual_suspend_ctrl(struct cros_ec_dev *ec, uint8_t enable)
>  
>  int lb_suspend(struct cros_ec_dev *ec)
>  {
> +	if (userspace_control)
> +		return 0;
> +
>  	return lb_send_empty_cmd(ec, LIGHTBAR_CMD_SUSPEND);
>  }
>  
>  int lb_resume(struct cros_ec_dev *ec)
>  {
> +	if (userspace_control)
> +		return 0;
> +
>  	return lb_send_empty_cmd(ec, LIGHTBAR_CMD_RESUME);
>  }
>  
> @@ -528,6 +540,30 @@ static ssize_t program_store(struct device *dev, struct device_attribute *attr,
>  	return ret;
>  }
>  
> +static ssize_t userspace_control_show(struct device *dev,
> +				      struct device_attribute *attr,
> +				      char *buf)
> +{
> +	return scnprintf(buf, PAGE_SIZE, "%d\n", userspace_control);
> +}
> +
> +static ssize_t userspace_control_store(struct device *dev,
> +				       struct device_attribute *attr,
> +				       const char *buf,
> +				       size_t count)
> +{
> +	bool enable;
> +	int ret;
> +
> +	ret = strtobool(buf, &enable);
> +	if (ret < 0)
> +		return ret;
> +
> +	userspace_control = enable;
> +
> +	return count;
> +}
> +
>  /* Module initialization */
>  
>  static DEVICE_ATTR_RW(interval_msec);
> @@ -536,6 +572,7 @@ static DEVICE_ATTR_WO(brightness);
>  static DEVICE_ATTR_WO(led_rgb);
>  static DEVICE_ATTR_RW(sequence);
>  static DEVICE_ATTR_WO(program);
> +static DEVICE_ATTR_RW(userspace_control);
>  
>  static struct attribute *__lb_cmds_attrs[] = {
>  	&dev_attr_interval_msec.attr,
> @@ -544,6 +581,7 @@ static struct attribute *__lb_cmds_attrs[] = {
>  	&dev_attr_led_rgb.attr,
>  	&dev_attr_sequence.attr,
>  	&dev_attr_program.attr,
> +	&dev_attr_userspace_control.attr,
>  	NULL,
>  };
>  
> 

-- 
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@google.com
Chromium OS Project
bleung@chromium.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

* Re: [PATCH RESEND 13/13] platform/chrome: cros_ec_lightbar - Avoid I2C xfer to EC during suspend
  2017-05-16 16:13 ` [PATCH RESEND 13/13] platform/chrome: cros_ec_lightbar - Avoid I2C xfer to EC during suspend Enric Balletbo i Serra
@ 2017-06-23 22:22   ` Benson Leung
  0 siblings, 0 replies; 31+ messages in thread
From: Benson Leung @ 2017-06-23 22:22 UTC (permalink / raw)
  To: Enric Balletbo i Serra, olof, bleung, linux-kernel
  Cc: lee.jones, Jeffery Yu, Guenter Roeck


[-- Attachment #1.1: Type: text/plain, Size: 3975 bytes --]

Hi Enric,

On 05/16/2017 09:13 AM, Enric Balletbo i Serra wrote:
> From: Jeffery Yu <jefferyy@nvidia.com>
> 
> A Mutex lock in cros_ec_cmd_xfer which may be held by frozen
> Userspace thread during system suspending. So should not
> call this routine in suspend thread.
> 
> Signed-off-by: Jeffery Yu <jefferyy@nvidia.com>
> Signed-off-by: Guenter Roeck <groeck@chromium.org>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>

Signed-off-by: Benson Leung <bleung@chromium.org>

Applied.
> ---
>  drivers/platform/chrome/cros_ec_dev.c      | 12 ++++--------
>  drivers/platform/chrome/cros_ec_lightbar.c | 13 +++++++++----
>  2 files changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/platform/chrome/cros_ec_dev.c b/drivers/platform/chrome/cros_ec_dev.c
> index 7c26223..b9bf086 100644
> --- a/drivers/platform/chrome/cros_ec_dev.c
> +++ b/drivers/platform/chrome/cros_ec_dev.c
> @@ -437,8 +437,7 @@ static int ec_device_probe(struct platform_device *pdev)
>  		cros_ec_sensors_register(ec);
>  
>  	/* Take control of the lightbar from the EC. */
> -	if (ec_has_lightbar(ec))
> -		lb_manual_suspend_ctrl(ec, 1);
> +	lb_manual_suspend_ctrl(ec, 1);
>  
>  	return 0;
>  
> @@ -452,8 +451,7 @@ static int ec_device_remove(struct platform_device *pdev)
>  	struct cros_ec_dev *ec = dev_get_drvdata(&pdev->dev);
>  
>  	/* Let the EC take over the lightbar again. */
> -	if (ec_has_lightbar(ec))
> -		lb_manual_suspend_ctrl(ec, 0);
> +	lb_manual_suspend_ctrl(ec, 0);
>  
>  	cros_ec_debugfs_remove(ec);
>  
> @@ -472,8 +470,7 @@ static int ec_device_suspend(struct device *dev)
>  {
>  	struct cros_ec_dev *ec = dev_get_drvdata(dev);
>  
> -	if (ec_has_lightbar(ec))
> -		lb_suspend(ec);
> +	lb_suspend(ec);
>  
>  	return 0;
>  }
> @@ -482,8 +479,7 @@ static int ec_device_resume(struct device *dev)
>  {
>  	struct cros_ec_dev *ec = dev_get_drvdata(dev);
>  
> -	if (ec_has_lightbar(ec))
> -		lb_resume(ec);
> +	lb_resume(ec);
>  
>  	return 0;
>  }
> diff --git a/drivers/platform/chrome/cros_ec_lightbar.c b/drivers/platform/chrome/cros_ec_lightbar.c
> index e570c1e..fd2b047 100644
> --- a/drivers/platform/chrome/cros_ec_lightbar.c
> +++ b/drivers/platform/chrome/cros_ec_lightbar.c
> @@ -43,6 +43,7 @@ static unsigned long lb_interval_jiffies = 50 * HZ / 1000;
>   * If this is true, we won't do anything during suspend/resume.
>   */
>  static bool userspace_control;
> +static struct cros_ec_dev *ec_with_lightbar;
>  
>  static ssize_t interval_msec_show(struct device *dev,
>  				  struct device_attribute *attr, char *buf)
> @@ -384,6 +385,9 @@ int lb_manual_suspend_ctrl(struct cros_ec_dev *ec, uint8_t enable)
>  	struct cros_ec_command *msg;
>  	int ret;
>  
> +	if (ec != ec_with_lightbar)
> +		return 0;
> +
>  	msg = alloc_lightbar_cmd_msg(ec);
>  	if (!msg)
>  		return -ENOMEM;
> @@ -413,7 +417,7 @@ int lb_manual_suspend_ctrl(struct cros_ec_dev *ec, uint8_t enable)
>  
>  int lb_suspend(struct cros_ec_dev *ec)
>  {
> -	if (userspace_control)
> +	if (userspace_control || ec != ec_with_lightbar)
>  		return 0;
>  
>  	return lb_send_empty_cmd(ec, LIGHTBAR_CMD_SUSPEND);
> @@ -421,7 +425,7 @@ int lb_suspend(struct cros_ec_dev *ec)
>  
>  int lb_resume(struct cros_ec_dev *ec)
>  {
> -	if (userspace_control)
> +	if (userspace_control || ec != ec_with_lightbar)
>  		return 0;
>  
>  	return lb_send_empty_cmd(ec, LIGHTBAR_CMD_RESUME);
> @@ -606,9 +610,10 @@ static umode_t cros_ec_lightbar_attrs_are_visible(struct kobject *kobj,
>  		return 0;
>  
>  	/* Only instantiate this stuff if the EC has a lightbar */
> -	if (ec_has_lightbar(ec))
> +	if (ec_has_lightbar(ec)) {
> +		ec_with_lightbar = ec;
>  		return a->mode;
> -
> +	}
>  	return 0;
>  }
>  
> 

-- 
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@google.com
Chromium OS Project
bleung@chromium.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

end of thread, other threads:[~2017-06-23 22:22 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-16 16:13 [PATCH RESEND 00/13] platform/chrome: Add console debugfs, lpc and lightbar programming support Enric Balletbo i Serra
2017-05-16 16:13 ` [PATCH RESEND 01/13] mfd: cros_ec: Add helper for event notifier Enric Balletbo i Serra
2017-06-16 20:45   ` Benson Leung
2017-05-16 16:13 ` [PATCH RESEND 02/13] mfd: cros_ec: Add EC console read structures definitions Enric Balletbo i Serra
2017-06-16 20:47   ` Benson Leung
2017-05-16 16:13 ` [PATCH RESEND 03/13] mfd: cros_ec: add debugfs, console log file Enric Balletbo i Serra
2017-06-16 19:42   ` Benson Leung
2017-05-16 16:13 ` [PATCH RESEND 04/13] mfd: cros_ec: Add support for dumping panic information Enric Balletbo i Serra
2017-06-22 23:38   ` Benson Leung
2017-05-16 16:13 ` [PATCH RESEND 05/13] platform/chrome: cros_ec_lpc: Add R/W helpers to LPC protocol variants Enric Balletbo i Serra
2017-05-22 11:09   ` Lee Jones
2017-06-22 19:02   ` Benson Leung
2017-05-16 16:13 ` [PATCH RESEND 06/13] platform/chrome: cros_ec_lpc: Add support for mec1322 EC Enric Balletbo i Serra
2017-05-22 11:09   ` Lee Jones
2017-06-22 19:01   ` Benson Leung
2017-05-16 16:13 ` [PATCH RESEND 07/13] platform/chrome: cros_ec_lpc: Add support for GOOG004 ACPI device Enric Balletbo i Serra
2017-06-22 19:39   ` Benson Leung
2017-05-16 16:13 ` [PATCH RESEND 08/13] platform/chrome: cros_ec_lpc: Add power management ops Enric Balletbo i Serra
2017-06-22 19:46   ` Benson Leung
2017-05-16 16:13 ` [PATCH RESEND 09/13] platform/chrome: cros_ec_lpc: Add MKBP events support over ACPI Enric Balletbo i Serra
2017-06-22 19:35   ` Benson Leung
2017-06-23  7:35     ` Thierry Escande
2017-06-23 17:54       ` Benson Leung
2017-05-16 16:13 ` [PATCH RESEND 10/13] platform/chrome: cros_ec_lightbar - Add lightbar program feature to sysfs Enric Balletbo i Serra
2017-06-22 23:43   ` Benson Leung
2017-05-16 16:13 ` [PATCH RESEND 11/13] platform/chrome: cros_ec_lightbar - Control of suspend/resume lightbar sequence Enric Balletbo i Serra
2017-06-23 22:08   ` Benson Leung
2017-05-16 16:13 ` [PATCH RESEND 12/13] platform/chrome: cros_ec_lightbar - Add userspace lightbar control bit to EC Enric Balletbo i Serra
2017-06-23 22:12   ` Benson Leung
2017-05-16 16:13 ` [PATCH RESEND 13/13] platform/chrome: cros_ec_lightbar - Avoid I2C xfer to EC during suspend Enric Balletbo i Serra
2017-06-23 22:22   ` Benson Leung

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