Hello,
The mainline ChromeOS Embedded Controller (EC) driver is still missing some
features that are present in the downstream ChromiumOS tree. These are:
- User-space device interface
- Low Pin Count (LPC) interface
- Access to vboot context stored on a block device
- Access to vboot context stored on EC's nvram
- Power Delivery Device
- Support for multiple EC in a system
This series adds support for the first of these missing features: the cros
ec dev driver that provides the interface used by user-space to access the
EC. The support patches were taken from the downstream ChromiumOS 3.14
kernel tree with fixes and cleanups squashed to have a minimal patch-set.
The series is composed of the following patches:
Bill Richardson (3):
mfd: cros_ec: Add Chrome OS EC userspace device interface
mfd: cros_ec: Create sysfs attributes for the ChromeOS EC.
mfd: cros_ec: Expose Chrome OS Lightbar to users
drivers/mfd/Kconfig | 12 ++
drivers/mfd/Makefile | 2 +
drivers/mfd/cros_ec.c | 4 +
drivers/mfd/cros_ec_dev.c | 363 ++++++++++++++++++++++++++++++++++++++++
drivers/mfd/cros_ec_dev.h | 29 ++++
drivers/mfd/cros_ec_lightbar.c | 347 ++++++++++++++++++++++++++++++++++++++
drivers/mfd/cros_ec_sysfs.c | 271 ++++++++++++++++++++++++++++++
include/linux/mfd/cros_ec.h | 12 +-
include/linux/mfd/cros_ec_dev.h | 47 ++++++
9 files changed, 1086 insertions(+), 1 deletion(-)
create mode 100644 drivers/mfd/cros_ec_dev.c
create mode 100644 drivers/mfd/cros_ec_dev.h
create mode 100644 drivers/mfd/cros_ec_lightbar.c
create mode 100644 drivers/mfd/cros_ec_sysfs.c
create mode 100644 include/linux/mfd/cros_ec_dev.h
Patch #1 adds the cros_ec_dev driver that provides the dev interface to
user-space. Patch #2 adds sysfs entries that can be used to get information
and to control the EC and Patch #3 adds sysfs entries to control Chromebook
Pixel's four-element LED lightbar.
The patches were tested on a Exynos5420 Peach Pit Chromebook by using the
sysfs entries and the ectool.
Best regards,
Javier
From: Bill Richardson <[email protected]>
This patch adds a device interface to access the
ChromeOS Embedded Controller from user-space.
Signed-off-by: Bill Richardson <[email protected]>
Reviewed-on: https://gerrit-int.chromium.org/38364
Reviewed-by: Simon Glass <[email protected]>
Tested-by: Simon Glass <[email protected]>
Signed-off-by: Javier Martinez Canillas <[email protected]>
---
drivers/mfd/Kconfig | 12 ++
drivers/mfd/Makefile | 1 +
drivers/mfd/cros_ec.c | 4 +
drivers/mfd/cros_ec_dev.c | 355 ++++++++++++++++++++++++++++++++++++++++
include/linux/mfd/cros_ec.h | 12 +-
include/linux/mfd/cros_ec_dev.h | 47 ++++++
6 files changed, 430 insertions(+), 1 deletion(-)
create mode 100644 drivers/mfd/cros_ec_dev.c
create mode 100644 include/linux/mfd/cros_ec_dev.h
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 72d3808..31420a7 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -115,6 +115,18 @@ config MFD_CROS_EC_SPI
response time cannot be guaranteed, we support ignoring
'pre-amble' bytes before the response actually starts.
+config MFD_CROS_EC_DEV
+ tristate "ChromeOS Embedded Controller userspace device interface"
+ depends on MFD_CROS_EC
+
+ help
+ If you say Y here, you get support for talking to the ChromeOS
+ EC from userspace. This is separate from the normal kernel
+ interfaces.
+
+ To compile this driver as a module, choose M here: the module will be
+ called cros_ec_dev.
+
config MFD_ASIC3
bool "Compaq ASIC3"
depends on GPIOLIB && ARM
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 53467e2..5104a25 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_MFD_BCM590XX) += bcm590xx.o
obj-$(CONFIG_MFD_CROS_EC) += cros_ec.o
obj-$(CONFIG_MFD_CROS_EC_I2C) += cros_ec_i2c.o
obj-$(CONFIG_MFD_CROS_EC_SPI) += cros_ec_spi.o
+obj-$(CONFIG_MFD_CROS_EC_DEV) += cros_ec_dev.o
rtsx_pci-objs := rtsx_pcr.o rtsx_gops.o rts5209.o rts5229.o rtl8411.o rts5227.o rts5249.o
obj-$(CONFIG_MFD_RTSX_PCI) += rtsx_pci.o
diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
index fc0c81e..197b273 100644
--- a/drivers/mfd/cros_ec.c
+++ b/drivers/mfd/cros_ec.c
@@ -119,6 +119,10 @@ static const struct mfd_cell cros_devs[] = {
.id = 2,
.of_compatible = "google,cros-ec-i2c-tunnel",
},
+ {
+ .name = "cros-ec-dev",
+ .id = 3,
+ },
};
int cros_ec_register(struct cros_ec_device *ec_dev)
diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
new file mode 100644
index 0000000..c18eb40
--- /dev/null
+++ b/drivers/mfd/cros_ec_dev.c
@@ -0,0 +1,355 @@
+/*
+ * cros_ec_dev - expose the Chrome OS Embedded Controller to userspace
+ *
+ * Copyright (C) 2014 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/>.
+ */
+
+#define pr_fmt(fmt) "cros_ec_dev: " fmt
+
+#include <linux/compat.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/fs.h>
+#include <linux/mfd/cros_ec.h>
+#include <linux/mfd/cros_ec_commands.h>
+#include <linux/mfd/cros_ec_dev.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/printk.h>
+#include <linux/types.h>
+#include <linux/uaccess.h>
+
+/* Device variables */
+#define CROS_CLASS_NAME "chromeos"
+static struct cros_ec_device *ec;
+static struct class *cros_class;
+static int ec_major;
+
+
+/* Basic communication */
+static int ec_get_version(struct cros_ec_device *ec, char *str, int maxlen)
+{
+ struct ec_response_get_version resp;
+ static const char * const current_image_name[] = {
+ "unknown", "read-only", "read-write", "invalid",
+ };
+ struct cros_ec_command msg = {
+ .version = 0,
+ .command = EC_CMD_GET_VERSION,
+ .outdata = NULL,
+ .outsize = 0,
+ .indata = (uint8_t *)&resp,
+ .insize = sizeof(resp),
+ };
+ int ret;
+
+ ret = cros_ec_cmd_xfer(ec, &msg);
+ if (ret < 0)
+ return ret;
+ if (msg.result != EC_RES_SUCCESS) {
+ snprintf(str, maxlen,
+ "%s\nUnknown EC version: EC returned %d\n",
+ CROS_EC_DEV_VERSION, msg.result);
+ return 0;
+ }
+ if (resp.current_image >= ARRAY_SIZE(current_image_name))
+ resp.current_image = 3; /* invalid */
+ snprintf(str, maxlen, "%s\n%s\n%s\n\%s\n", CROS_EC_DEV_VERSION,
+ resp.version_string_ro, resp.version_string_rw,
+ current_image_name[resp.current_image]);
+
+ return 0;
+}
+
+/* Device file ops */
+static int ec_device_open(struct inode *inode, struct file *filp)
+{
+ return 0;
+}
+
+static int ec_device_release(struct inode *inode, struct file *filp)
+{
+ return 0;
+}
+
+static ssize_t ec_device_read(struct file *filp, char __user *buffer,
+ size_t length, loff_t *offset)
+{
+ char msg[sizeof(struct ec_response_get_version) +
+ sizeof(CROS_EC_DEV_VERSION)];
+ size_t count;
+ int ret;
+
+ if (*offset != 0)
+ return 0;
+
+ ret = ec_get_version(ec, msg, sizeof(msg));
+ if (ret)
+ return ret;
+ count = min(length, strlen(msg));
+
+ if (copy_to_user(buffer, msg, count))
+ return -EFAULT;
+
+ *offset += count;
+ return count;
+}
+
+
+/* Ioctls */
+static long ec_device_ioctl_xcmd(void __user *argp)
+{
+ long ret;
+ struct cros_ec_command s_cmd;
+ uint8_t *user_indata;
+ uint8_t buf[EC_PROTO2_MAX_PARAM_SIZE];
+
+ if (copy_from_user(&s_cmd, argp, sizeof(s_cmd)))
+ return -EFAULT;
+ if (s_cmd.outsize &&
+ copy_from_user(&buf, (void __user *)s_cmd.outdata, sizeof(buf)))
+ return -EFAULT;
+
+ user_indata = s_cmd.indata;
+ s_cmd.indata = buf;
+ s_cmd.outdata = buf;
+ ret = cros_ec_cmd_xfer(ec, &s_cmd);
+ s_cmd.indata = user_indata;
+
+ /* Only copy data to userland if data was received. */
+ if (ret > 0 && s_cmd.insize) {
+ unsigned size = ret;
+
+ size = min(size, s_cmd.insize);
+ if (copy_to_user((void __user *)s_cmd.indata, buf, size))
+ return -EFAULT;
+ }
+ if (copy_to_user(argp, &s_cmd, sizeof(s_cmd)))
+ return -EFAULT;
+
+ return ret;
+}
+
+static long ec_device_ioctl_readmem(void __user *argp)
+{
+ struct cros_ec_readmem s_mem;
+ char buf[EC_MEMMAP_SIZE];
+ long num;
+
+ /* Not every platform supports direct reads */
+ if (!ec->cmd_readmem)
+ return -ENOTTY;
+
+ if (copy_from_user(&s_mem, argp, sizeof(s_mem)))
+ return -EFAULT;
+ num = ec->cmd_readmem(ec, s_mem.offset, s_mem.bytes, buf);
+ if (num <= 0)
+ return num;
+ if (copy_to_user((void __user *)s_mem.buffer, buf, num))
+ return -EFAULT;
+ return num;
+}
+
+static long ec_device_ioctl(struct file *filp, unsigned int cmd,
+ unsigned long arg)
+{
+ void __user *argp = (void __user *)arg;
+
+ if (_IOC_TYPE(cmd) != CROS_EC_DEV_IOC)
+ return -ENOTTY;
+
+ switch (cmd) {
+ case CROS_EC_DEV_IOCXCMD:
+ return ec_device_ioctl_xcmd(argp);
+ case CROS_EC_DEV_IOCRDMEM:
+ return ec_device_ioctl_readmem(argp);
+ }
+
+ return -ENOTTY;
+}
+
+#ifdef CONFIG_COMPAT
+struct compat_cros_ec_command {
+ uint32_t version;
+ uint32_t command;
+ compat_uptr_t outdata;
+ uint32_t outsize;
+ compat_uptr_t indata;
+ uint32_t insize;
+ uint32_t result;
+};
+
+struct compat_cros_ec_readmem {
+ uint32_t offset;
+ uint32_t bytes;
+ compat_uptr_t buffer;
+};
+
+#define CROS_EC_DEV_COMPAT_IOCXCMD _IOWR(':', 0, struct compat_cros_ec_command)
+#define CROS_EC_DEV_COMPAT_IOCRDMEM _IOWR(':', 1, struct compat_cros_ec_readmem)
+
+static long ec_device_compat_ioctl_readmem(void __user *argp)
+{
+ struct compat_cros_ec_readmem compat_s_mem;
+ char buf[EC_MEMMAP_SIZE];
+ long num;
+
+ /* Not every platform supports direct reads */
+ if (!ec->cmd_readmem)
+ return -ENOTTY;
+
+ if (copy_from_user(&compat_s_mem, argp, sizeof(compat_s_mem)))
+ return -EFAULT;
+
+ num = ec->cmd_readmem(ec, compat_s_mem.offset, compat_s_mem.bytes, buf);
+ if (num <= 0)
+ return num;
+
+ if (copy_to_user(compat_ptr(compat_s_mem.buffer), buf, num))
+ return -EFAULT;
+ return num;
+}
+
+static long ec_device_compat_ioctl_xcmd(void __user *argp)
+{
+ long ret;
+ struct cros_ec_command s_cmd;
+ struct compat_cros_ec_command compat_s_cmd;
+ uint8_t buf[EC_PROTO2_MAX_PARAM_SIZE];
+
+ if (copy_from_user(&compat_s_cmd, argp, sizeof(compat_s_cmd)))
+ return -EFAULT;
+
+ s_cmd.version = compat_s_cmd.version;
+ s_cmd.command = compat_s_cmd.command;
+ s_cmd.insize = compat_s_cmd.insize;
+ s_cmd.outsize = compat_s_cmd.outsize;
+
+ if (s_cmd.outsize &&
+ copy_from_user(&buf, compat_ptr(compat_s_cmd.outdata), sizeof(buf)))
+ return -EFAULT;
+
+ s_cmd.indata = buf;
+ s_cmd.outdata = buf;
+ ret = cros_ec_cmd_xfer(ec, &s_cmd);
+
+ compat_s_cmd.result = s_cmd.result;
+
+ /* Only copy data to userland if data was received. */
+ if (ret > 0 && s_cmd.insize) {
+ unsigned size = ret;
+
+ size = min(size, s_cmd.insize);
+ if (copy_to_user(compat_ptr(compat_s_cmd.indata), buf, size))
+ return -EFAULT;
+ }
+
+ if (copy_to_user(argp, &compat_s_cmd, sizeof(compat_s_cmd)))
+ return -EFAULT;
+
+ return ret;
+}
+
+static long ec_device_compat_ioctl(struct file *filp, unsigned int cmd,
+ unsigned long arg)
+{
+ void __user
+ *argp = (void __user *)arg;
+ switch (cmd) {
+ case CROS_EC_DEV_COMPAT_IOCXCMD:
+ return ec_device_compat_ioctl_xcmd(argp);
+ case CROS_EC_DEV_COMPAT_IOCRDMEM:
+ return ec_device_compat_ioctl_readmem(argp);
+ }
+ return -ENOTTY;
+}
+#endif /* CONFIG_COMPAT */
+
+
+/* Module initialization */
+static const struct file_operations fops = {
+ .open = ec_device_open,
+ .release = ec_device_release,
+ .read = ec_device_read,
+ .unlocked_ioctl = ec_device_ioctl,
+#ifdef CONFIG_COMPAT
+ .compat_ioctl = ec_device_compat_ioctl,
+#endif
+};
+
+static int ec_device_probe(struct platform_device *pdev)
+{
+ int retval = -ENOTTY;
+
+ ec = dev_get_drvdata(pdev->dev.parent);
+
+ cros_class = class_create(THIS_MODULE, CROS_CLASS_NAME);
+ if (IS_ERR(cros_class)) {
+ pr_err("failed to register device class\n");
+ retval = PTR_ERR(cros_class);
+ goto failed_class;
+ }
+
+ /* Let the kernel pick the major num for us */
+ ec_major = register_chrdev(0, CROS_EC_DEV_NAME, &fops);
+ if (ec_major < 0) {
+ pr_err("failed to register device\n");
+ retval = ec_major;
+ goto failed_chrdevreg;
+ }
+
+ /* Instantiate it */
+ ec->vdev = device_create(cros_class, NULL, MKDEV(ec_major, 0),
+ NULL, CROS_EC_DEV_NAME);
+ if (IS_ERR(ec->vdev)) {
+ pr_err("failed to create device\n");
+ retval = PTR_ERR(ec->vdev);
+ goto failed_devreg;
+ }
+
+ return 0;
+
+failed_devreg:
+ unregister_chrdev(ec_major, CROS_EC_DEV_NAME);
+failed_chrdevreg:
+ class_destroy(cros_class);
+failed_class:
+ return retval;
+}
+
+static int ec_device_remove(struct platform_device *pdev)
+{
+ device_destroy(cros_class, MKDEV(ec_major, 0));
+ unregister_chrdev(ec_major, CROS_EC_DEV_NAME);
+ class_destroy(cros_class);
+ return 0;
+}
+
+static struct platform_driver cros_ec_dev_driver = {
+ .driver = {
+ .name = "cros-ec-dev",
+ .owner = THIS_MODULE,
+ },
+ .probe = ec_device_probe,
+ .remove = ec_device_remove,
+};
+
+module_platform_driver(cros_ec_dev_driver);
+
+MODULE_AUTHOR("Bill Richardson <[email protected]>");
+MODULE_DESCRIPTION("Userspace interface to the Chrome OS Embedded Controller");
+MODULE_VERSION("1.0");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
index 0e166b9..b1cb934 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -59,9 +59,16 @@ struct cros_ec_command {
*
* @ec_name: name of EC device (e.g. 'chromeos-ec')
* @phys_name: name of physical comms layer (e.g. 'i2c-4')
- * @dev: Device pointer
+ * @dev: Device pointer for physical comms device
+ * @vdev: Device pointer for virtual comms device
* @was_wake_device: true if this device was set to wake the system from
* sleep at the last suspend
+ * @cmd_read_mem: direct read of the EC memory-mapped region, if supported
+ * @offset is within EC_LPC_ADDR_MEMMAP region.
+ * @bytes: number of bytes to read. zero means "read a string" (including
+ * the trailing '\0'). At most only EC_MEMMAP_SIZE bytes can be read.
+ * Caller must ensure that the buffer is large enough for the result when
+ * reading a string.
*
* @priv: Private data
* @irq: Interrupt to use
@@ -90,8 +97,11 @@ struct cros_ec_device {
const char *ec_name;
const char *phys_name;
struct device *dev;
+ struct device *vdev;
bool was_wake_device;
struct class *cros_class;
+ int (*cmd_readmem)(struct cros_ec_device *ec, unsigned int offset,
+ unsigned int bytes, void *dest);
/* These are used to implement the platform-specific interface */
void *priv;
diff --git a/include/linux/mfd/cros_ec_dev.h b/include/linux/mfd/cros_ec_dev.h
new file mode 100644
index 0000000..465e926
--- /dev/null
+++ b/include/linux/mfd/cros_ec_dev.h
@@ -0,0 +1,47 @@
+/*
+ * cros_ec_dev - expose the Chrome OS Embedded Controller to userspace
+ *
+ * Copyright (C) 2013 The Chromium OS Authors
+ *
+ * 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 _CROS_EC_DEV_H_
+#define _CROS_EC_DEV_H_
+
+#include <linux/ioctl.h>
+#include <linux/types.h>
+#include <linux/mfd/cros_ec.h>
+
+#define CROS_EC_DEV_NAME "cros_ec"
+#define CROS_EC_DEV_VERSION "1.0.0"
+
+/*
+ * @offset: within EC_LPC_ADDR_MEMMAP region
+ * @bytes: number of bytes to read. zero means "read a string" (including '\0')
+ * (at most only EC_MEMMAP_SIZE bytes can be read)
+ * @buffer: where to store the result
+ * ioctl returns the number of bytes read, negative on error
+ */
+struct cros_ec_readmem {
+ uint32_t offset;
+ uint32_t bytes;
+ char *buffer;
+};
+
+#define CROS_EC_DEV_IOC ':'
+#define CROS_EC_DEV_IOCXCMD _IOWR(':', 0, struct cros_ec_command)
+#define CROS_EC_DEV_IOCRDMEM _IOWR(':', 1, struct cros_ec_readmem)
+
+#endif /* _CROS_EC_DEV_H_ */
--
2.1.0
From: Bill Richardson <[email protected]>
This adds some sysfs entries to provide userspace control of the
four-element LED "lightbar" on the Chromebook Pixel. This only instantiates
the lightbar controls if the device actually exists.
To prevent DoS attacks, this interface is limited to 20 accesses/second,
although that rate can be adjusted by a privileged user.
On Chromebooks without a lightbar, this should have no effect. On the
Chromebook Pixel, you should be able to do things like this:
$ cd /sys/devices/virtual/chromeos/cros_ec/lightbar
$ echo 0x80 > brightness
$ echo 255 > brightness
$
$ cat sequence
S0
$ echo konami > sequence
$ cat sequence
KONAMI
$
$ cat sequence
S0
And
$ cd /sys/devices/virtual/chromeos/cros_ec/lightbar
$ echo stop > sequence
$ echo "4 255 255 255" > led_rgb
$ echo "0 255 0 0 1 0 255 0 2 0 0 255 3 255 255 0" > led_rgb
$ echo run > sequence
Test the DoS prevention with this:
$ cd /sys/devices/virtual/chromeos/cros_ec/lightbar
$ echo 500 > interval_msec
$ time (cat version version version version version version version)
Signed-off-by: Bill Richardson <[email protected]>
Reviewed-by: Olof Johansson <[email protected]>
Tested-by: Doug Anderson <[email protected]>
Reviewed-by: Benson Leung <[email protected]>
Signed-off-by: Javier Martinez Canillas <[email protected]>
---
drivers/mfd/Makefile | 2 +-
drivers/mfd/cros_ec_dev.c | 2 +
drivers/mfd/cros_ec_dev.h | 3 +
drivers/mfd/cros_ec_lightbar.c | 347 +++++++++++++++++++++++++++++++++++++++++
4 files changed, 353 insertions(+), 1 deletion(-)
create mode 100644 drivers/mfd/cros_ec_lightbar.c
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index eb96986..54ef1bf 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -12,7 +12,7 @@ obj-$(CONFIG_MFD_BCM590XX) += bcm590xx.o
obj-$(CONFIG_MFD_CROS_EC) += cros_ec.o
obj-$(CONFIG_MFD_CROS_EC_I2C) += cros_ec_i2c.o
obj-$(CONFIG_MFD_CROS_EC_SPI) += cros_ec_spi.o
-cros_ec_devs-objs := cros_ec_dev.o cros_ec_sysfs.o
+cros_ec_devs-objs := cros_ec_dev.o cros_ec_sysfs.o cros_ec_lightbar.o
obj-$(CONFIG_MFD_CROS_EC_DEV) += cros_ec_devs.o
rtsx_pci-objs := rtsx_pcr.o rtsx_gops.o rts5209.o rts5229.o rtl8411.o rts5227.o rts5249.o
diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
index 1db60dd..dbffac9 100644
--- a/drivers/mfd/cros_ec_dev.c
+++ b/drivers/mfd/cros_ec_dev.c
@@ -324,6 +324,7 @@ static int ec_device_probe(struct platform_device *pdev)
/* Initialize extra interfaces */
ec_dev_sysfs_init(ec);
+ ec_dev_lightbar_init(ec);
return 0;
@@ -337,6 +338,7 @@ failed_class:
static int ec_device_remove(struct platform_device *pdev)
{
+ ec_dev_lightbar_remove(ec);
ec_dev_sysfs_remove(ec);
device_destroy(cros_class, MKDEV(ec_major, 0));
unregister_chrdev(ec_major, CROS_EC_DEV_NAME);
diff --git a/drivers/mfd/cros_ec_dev.h b/drivers/mfd/cros_ec_dev.h
index 0bb833e..cb17f4c 100644
--- a/drivers/mfd/cros_ec_dev.h
+++ b/drivers/mfd/cros_ec_dev.h
@@ -23,4 +23,7 @@ struct cros_ec_device;
void ec_dev_sysfs_init(struct cros_ec_device *);
void ec_dev_sysfs_remove(struct cros_ec_device *);
+void ec_dev_lightbar_init(struct cros_ec_device *);
+void ec_dev_lightbar_remove(struct cros_ec_device *);
+
#endif /* _DRV_CROS_EC_DEV_H_ */
diff --git a/drivers/mfd/cros_ec_lightbar.c b/drivers/mfd/cros_ec_lightbar.c
new file mode 100644
index 0000000..fb1282d
--- /dev/null
+++ b/drivers/mfd/cros_ec_lightbar.c
@@ -0,0 +1,347 @@
+/*
+ * cros_ec_lightbar - expose the Chromebook Pixel lightbar to userspace
+ *
+ * Copyright (C) 2014 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/>.
+ */
+
+#define pr_fmt(fmt) "cros_ec_lightbar: " fmt
+
+#include <linux/ctype.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/fs.h>
+#include <linux/kobject.h>
+#include <linux/mfd/cros_ec.h>
+#include <linux/mfd/cros_ec_commands.h>
+#include <linux/mfd/cros_ec_dev.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/sched.h>
+#include <linux/types.h>
+#include <linux/uaccess.h>
+
+#include "cros_ec_dev.h"
+
+/* Rate-limit the lightbar interface to prevent DoS. */
+static unsigned long lb_interval_jiffies = 50 * HZ / 1000;
+
+static ssize_t interval_msec_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ unsigned long msec = lb_interval_jiffies * 1000 / HZ;
+
+ return scnprintf(buf, PAGE_SIZE, "%lu\n", msec);
+}
+
+static ssize_t interval_msec_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ unsigned long msec;
+
+ if (kstrtoul(buf, 0, &msec))
+ return -EINVAL;
+
+ lb_interval_jiffies = msec * HZ / 1000;
+
+ return count;
+}
+
+static DEFINE_MUTEX(lb_mutex);
+/* Return 0 if able to throttle correctly, error otherwise */
+static int lb_throttle(void)
+{
+ static unsigned long last_access;
+ unsigned long now, next_timeslot;
+ long delay;
+ int ret = 0;
+
+ mutex_lock(&lb_mutex);
+
+ now = jiffies;
+ next_timeslot = last_access + lb_interval_jiffies;
+
+ if (time_before(now, next_timeslot)) {
+ delay = (long)(next_timeslot) - (long)now;
+ set_current_state(TASK_INTERRUPTIBLE);
+ if (schedule_timeout(delay) > 0) {
+ /* interrupted - just abort */
+ ret = -EINTR;
+ goto out;
+ }
+ now = jiffies;
+ }
+
+ last_access = now;
+out:
+ mutex_unlock(&lb_mutex);
+
+ return ret;
+}
+
+#define INIT_MSG(P, R) { \
+ .command = EC_CMD_LIGHTBAR_CMD, \
+ .outdata = (uint8_t *)&P, \
+ .outsize = sizeof(P), \
+ .indata = (uint8_t *)&R, \
+ .insize = sizeof(R), \
+ }
+
+static int get_lightbar_version(struct cros_ec_device *ec,
+ uint32_t *ver_ptr, uint32_t *flg_ptr)
+{
+ struct ec_params_lightbar param;
+ struct ec_response_lightbar resp;
+ struct cros_ec_command msg = INIT_MSG(param, resp);
+ int ret;
+
+ param.cmd = LIGHTBAR_CMD_VERSION;
+ ret = cros_ec_cmd_xfer(ec, &msg);
+ if (ret < 0)
+ return 0;
+
+ switch (msg.result) {
+ case EC_RES_INVALID_PARAM:
+ /* Pixel had no version command. */
+ if (ver_ptr)
+ *ver_ptr = 0;
+ if (flg_ptr)
+ *flg_ptr = 0;
+ return 1;
+
+ case EC_RES_SUCCESS:
+ /* Future devices w/lightbars should implement this command */
+ if (ver_ptr)
+ *ver_ptr = resp.version.num;
+ if (flg_ptr)
+ *flg_ptr = resp.version.flags;
+ return 1;
+ }
+
+ /* Anything else (ie, EC_RES_INVALID_COMMAND) - no lightbar */
+ return 0;
+}
+
+static ssize_t version_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ uint32_t version, flags;
+ struct cros_ec_device *ec = dev_get_drvdata(dev);
+ int ret;
+
+ ret = lb_throttle();
+ if (ret)
+ return ret;
+
+ /* This should always succeed, because we check during init. */
+ if (!get_lightbar_version(ec, &version, &flags))
+ return -EIO;
+
+ return scnprintf(buf, PAGE_SIZE, "%d %d\n", version, flags);
+}
+
+static ssize_t brightness_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct ec_params_lightbar param;
+ struct ec_response_lightbar resp;
+ struct cros_ec_command msg = INIT_MSG(param, resp);
+ int ret;
+ unsigned int val;
+ struct cros_ec_device *ec = dev_get_drvdata(dev);
+
+ if (kstrtouint(buf, 0, &val))
+ return -EINVAL;
+ param.cmd = LIGHTBAR_CMD_BRIGHTNESS;
+ param.brightness.num = val;
+ ret = lb_throttle();
+ if (ret)
+ return ret;
+ ret = cros_ec_cmd_xfer(ec, &msg);
+ if (ret < 0)
+ return ret;
+ if (msg.result != EC_RES_SUCCESS)
+ return -EINVAL;
+
+ return count;
+}
+
+
+/*
+ * We expect numbers, and we'll keep reading until we find them, skipping over
+ * any whitespace (sysfs guarantees that the input is null-terminated). Every
+ * four numbers are sent to the lightbar as <LED,R,G,B>. We fail at the first
+ * parsing error, if we don't parse any numbers, or if we have numbers left
+ * over.
+ */
+static ssize_t led_rgb_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct ec_params_lightbar param;
+ struct ec_response_lightbar resp;
+ struct cros_ec_command msg = INIT_MSG(param, resp);
+ struct cros_ec_device *ec = dev_get_drvdata(dev);
+ unsigned int val[4];
+ int ret, i = 0, j = 0, ok = 0;
+
+ do {
+ /* Skip any whitespace */
+ while (*buf && isspace(*buf))
+ buf++;
+ if (!*buf)
+ break;
+
+ if (kstrtouint(buf, 0, &val[i++]))
+ return -EINVAL;
+
+ if (i == 4) {
+ param.cmd = LIGHTBAR_CMD_RGB;
+ param.rgb.led = val[0];
+ param.rgb.red = val[1];
+ param.rgb.green = val[2];
+ param.rgb.blue = val[3];
+ /*
+ * Throttle only the first of every four transactions,
+ * so that the user can update all four LEDs at once.
+ */
+ if ((j++ % 4) == 0) {
+ ret = lb_throttle();
+ if (ret)
+ return ret;
+ }
+ ret = cros_ec_cmd_xfer(ec, &msg);
+ if (ret < 0)
+ return ret;
+ if (msg.result != EC_RES_SUCCESS)
+ return -EINVAL;
+ i = 0;
+ ok = 1;
+ }
+
+ /* Skip over the number we just read */
+ while (*buf && !isspace(*buf))
+ buf++;
+
+ } while (*buf);
+
+ return (ok && i == 0) ? count : -EINVAL;
+}
+
+static const char const *seqname[] = {
+ "ERROR", "S5", "S3", "S0", "S5S3", "S3S0",
+ "S0S3", "S3S5", "STOP", "RUN", "PULSE", "TEST", "KONAMI",
+};
+
+static ssize_t sequence_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct ec_params_lightbar param;
+ struct ec_response_lightbar resp;
+ struct cros_ec_command msg = INIT_MSG(param, resp);
+ int ret;
+ struct cros_ec_device *ec = dev_get_drvdata(dev);
+
+ param.cmd = LIGHTBAR_CMD_GET_SEQ;
+ ret = lb_throttle();
+ if (ret)
+ return ret;
+ ret = cros_ec_cmd_xfer(ec, &msg);
+ if (ret < 0)
+ return ret;
+ if (msg.result != EC_RES_SUCCESS)
+ return scnprintf(buf, PAGE_SIZE,
+ "ERROR: EC returned %d\n", msg.result);
+
+ if (resp.get_seq.num >= ARRAY_SIZE(seqname))
+ return scnprintf(buf, PAGE_SIZE, "%d\n", resp.get_seq.num);
+ else
+ return scnprintf(buf, PAGE_SIZE, "%s\n",
+ seqname[resp.get_seq.num]);
+}
+
+static ssize_t sequence_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct ec_params_lightbar param;
+ struct ec_response_lightbar resp;
+ struct cros_ec_command msg = INIT_MSG(param, resp);
+ unsigned int num;
+ int ret, len;
+ struct cros_ec_device *ec = dev_get_drvdata(dev);
+
+ for (len = 0; len < count; len++)
+ if (!isalnum(buf[len]))
+ break;
+
+ for (num = 0; num < ARRAY_SIZE(seqname); num++)
+ if (!strncasecmp(seqname[num], buf, len))
+ break;
+
+ if (num >= ARRAY_SIZE(seqname))
+ kstrtouint(buf, 0, &num);
+
+ param.cmd = LIGHTBAR_CMD_SEQ;
+ param.seq.num = num;
+ ret = lb_throttle();
+ if (ret)
+ return ret;
+ ret = cros_ec_cmd_xfer(ec, &msg);
+ if (ret < 0)
+ return ret;
+ if (msg.result != EC_RES_SUCCESS)
+ return -EINVAL;
+
+ return count;
+}
+
+/* Module initialization */
+
+static DEVICE_ATTR_RW(interval_msec);
+static DEVICE_ATTR_RO(version);
+static DEVICE_ATTR_WO(brightness);
+static DEVICE_ATTR_WO(led_rgb);
+static DEVICE_ATTR_RW(sequence);
+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,
+ NULL,
+};
+static struct attribute_group lb_cmds_attr_group = {
+ .name = "lightbar",
+ .attrs = __lb_cmds_attrs,
+};
+
+void ec_dev_lightbar_init(struct cros_ec_device *ec)
+{
+ int ret = 0;
+
+ /* Only instantiate this stuff if the EC has a lightbar */
+ if (!get_lightbar_version(ec, NULL, NULL))
+ return;
+
+ ret = sysfs_create_group(&ec->vdev->kobj, &lb_cmds_attr_group);
+ if (ret)
+ pr_warn("sysfs_create_group() failed: %d\n", ret);
+}
+
+void ec_dev_lightbar_remove(struct cros_ec_device *ec)
+{
+ sysfs_remove_group(&ec->vdev->kobj, &lb_cmds_attr_group);
+}
--
2.1.0
From: Bill Richardson <[email protected]>
This adds the first few sysfs attributes for the Chrome OS EC. These
controls are made available under /sys/devices/virtual/chromeos/cros_ec
flashinfo - display current flash info
reboot - tell the EC to reboot in various ways
version - information about the EC software and hardware
Future changes will build on this to add additional controls.
>From a root shell, you should be able to do things like this:
cd /sys/devices/virtual/chromeos/cros_ec
cat flashinfo
cat version
echo rw > reboot
cat version
echo ro > reboot
cat version
echo rw > reboot
cat version
echo cold > reboot
That last command will reboot the AP too.
Signed-off-by: Bill Richardson <[email protected]>
Reviewed-by: Olof Johansson <[email protected]>
Signed-off-by: Javier Martinez Canillas <[email protected]>
---
drivers/mfd/Makefile | 3 +-
drivers/mfd/cros_ec_dev.c | 10 +-
drivers/mfd/cros_ec_dev.h | 26 +++++
drivers/mfd/cros_ec_sysfs.c | 271 ++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 307 insertions(+), 3 deletions(-)
create mode 100644 drivers/mfd/cros_ec_dev.h
create mode 100644 drivers/mfd/cros_ec_sysfs.c
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 5104a25..eb96986 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -12,7 +12,8 @@ obj-$(CONFIG_MFD_BCM590XX) += bcm590xx.o
obj-$(CONFIG_MFD_CROS_EC) += cros_ec.o
obj-$(CONFIG_MFD_CROS_EC_I2C) += cros_ec_i2c.o
obj-$(CONFIG_MFD_CROS_EC_SPI) += cros_ec_spi.o
-obj-$(CONFIG_MFD_CROS_EC_DEV) += cros_ec_dev.o
+cros_ec_devs-objs := cros_ec_dev.o cros_ec_sysfs.o
+obj-$(CONFIG_MFD_CROS_EC_DEV) += cros_ec_devs.o
rtsx_pci-objs := rtsx_pcr.o rtsx_gops.o rts5209.o rts5229.o rtl8411.o rts5227.o rts5249.o
obj-$(CONFIG_MFD_RTSX_PCI) += rtsx_pci.o
diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
index c18eb40..1db60dd 100644
--- a/drivers/mfd/cros_ec_dev.c
+++ b/drivers/mfd/cros_ec_dev.c
@@ -32,6 +32,8 @@
#include <linux/types.h>
#include <linux/uaccess.h>
+#include "cros_ec_dev.h"
+
/* Device variables */
#define CROS_CLASS_NAME "chromeos"
static struct cros_ec_device *ec;
@@ -311,15 +313,18 @@ static int ec_device_probe(struct platform_device *pdev)
goto failed_chrdevreg;
}
- /* Instantiate it */
+ /* Instantiate it (and remember the EC) */
ec->vdev = device_create(cros_class, NULL, MKDEV(ec_major, 0),
- NULL, CROS_EC_DEV_NAME);
+ ec, CROS_EC_DEV_NAME);
if (IS_ERR(ec->vdev)) {
pr_err("failed to create device\n");
retval = PTR_ERR(ec->vdev);
goto failed_devreg;
}
+ /* Initialize extra interfaces */
+ ec_dev_sysfs_init(ec);
+
return 0;
failed_devreg:
@@ -332,6 +337,7 @@ failed_class:
static int ec_device_remove(struct platform_device *pdev)
{
+ ec_dev_sysfs_remove(ec);
device_destroy(cros_class, MKDEV(ec_major, 0));
unregister_chrdev(ec_major, CROS_EC_DEV_NAME);
class_destroy(cros_class);
diff --git a/drivers/mfd/cros_ec_dev.h b/drivers/mfd/cros_ec_dev.h
new file mode 100644
index 0000000..0bb833e
--- /dev/null
+++ b/drivers/mfd/cros_ec_dev.h
@@ -0,0 +1,26 @@
+/*
+ * Copyright (C) 2014 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_DEV_H_
+#define _DRV_CROS_EC_DEV_H_
+
+struct cros_ec_device;
+
+void ec_dev_sysfs_init(struct cros_ec_device *);
+void ec_dev_sysfs_remove(struct cros_ec_device *);
+
+#endif /* _DRV_CROS_EC_DEV_H_ */
diff --git a/drivers/mfd/cros_ec_sysfs.c b/drivers/mfd/cros_ec_sysfs.c
new file mode 100644
index 0000000..7359c9f
--- /dev/null
+++ b/drivers/mfd/cros_ec_sysfs.c
@@ -0,0 +1,271 @@
+/*
+ * cros_ec_sysfs - expose the Chrome OS EC through sysfs
+ *
+ * Copyright (C) 2014 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/>.
+ */
+
+#define pr_fmt(fmt) "cros_ec_sysfs: " fmt
+
+#include <linux/ctype.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/fs.h>
+#include <linux/kobject.h>
+#include <linux/mfd/cros_ec.h>
+#include <linux/mfd/cros_ec_commands.h>
+#include <linux/mfd/cros_ec_dev.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/printk.h>
+#include <linux/stat.h>
+#include <linux/types.h>
+#include <linux/uaccess.h>
+
+#include "cros_ec_dev.h"
+
+/* Accessor functions */
+
+static ssize_t show_ec_reboot(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ int count = 0;
+
+ count += scnprintf(buf + count, PAGE_SIZE - count,
+ "ro|rw|cancel|cold|disable-jump|hibernate");
+ count += scnprintf(buf + count, PAGE_SIZE - count,
+ " [at-shutdown]\n");
+ return count;
+}
+
+static ssize_t store_ec_reboot(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ static const struct {
+ const char * const str;
+ uint8_t cmd;
+ uint8_t flags;
+ } words[] = {
+ {"cancel", EC_REBOOT_CANCEL, 0},
+ {"ro", EC_REBOOT_JUMP_RO, 0},
+ {"rw", EC_REBOOT_JUMP_RW, 0},
+ {"cold", EC_REBOOT_COLD, 0},
+ {"disable-jump", EC_REBOOT_DISABLE_JUMP, 0},
+ {"hibernate", EC_REBOOT_HIBERNATE, 0},
+ {"at-shutdown", -1, EC_REBOOT_FLAG_ON_AP_SHUTDOWN},
+ };
+ struct ec_params_reboot_ec param;
+ struct cros_ec_command msg = { 0 };
+ int got_cmd = 0, offset = 0;
+ int i;
+ int ret;
+ struct cros_ec_device *ec = dev_get_drvdata(dev);
+
+ param.flags = 0;
+ while (1) {
+ /* Find word to start scanning */
+ while (buf[offset] && isspace(buf[offset]))
+ offset++;
+ if (!buf[offset])
+ break;
+
+ for (i = 0; i < ARRAY_SIZE(words); i++) {
+ if (!strncasecmp(words[i].str, buf+offset,
+ strlen(words[i].str))) {
+ if (words[i].flags) {
+ param.flags |= words[i].flags;
+ } else {
+ param.cmd = words[i].cmd;
+ got_cmd = 1;
+ }
+ break;
+ }
+ }
+
+ /* On to the next word, if any */
+ while (buf[offset] && !isspace(buf[offset]))
+ offset++;
+ }
+
+ if (!got_cmd)
+ return -EINVAL;
+
+ msg.command = EC_CMD_REBOOT_EC;
+ msg.outdata = (uint8_t *)¶m;
+ msg.outsize = sizeof(param);
+ ret = cros_ec_cmd_xfer(ec, &msg);
+ if (ret < 0)
+ return ret;
+ if (msg.result != EC_RES_SUCCESS) {
+ dev_dbg(ec->dev, "EC result %d\n", msg.result);
+ return -EINVAL;
+ }
+
+ return count;
+}
+
+static ssize_t show_ec_version(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ static const char * const image_names[] = {"unknown", "RO", "RW"};
+ struct ec_response_get_version r_ver;
+ struct ec_response_get_chip_info r_chip;
+ struct ec_response_board_version r_board;
+ struct cros_ec_command msg = { 0 };
+ char r_build_string[EC_HOST_PARAM_SIZE];
+ int ret;
+ int count = 0;
+ struct cros_ec_device *ec = dev_get_drvdata(dev);
+
+ /* Get versions. RW may change. */
+ msg.command = EC_CMD_GET_VERSION;
+ msg.indata = (uint8_t *)&r_ver;
+ msg.insize = sizeof(r_ver);
+ ret = cros_ec_cmd_xfer(ec, &msg);
+ if (ret < 0)
+ return ret;
+ if (msg.result != EC_RES_SUCCESS)
+ return scnprintf(buf, PAGE_SIZE,
+ "ERROR: EC returned %d\n", msg.result);
+
+ /* Strings should be null-terminated, but let's be sure. */
+ r_ver.version_string_ro[sizeof(r_ver.version_string_ro) - 1] = '\0';
+ r_ver.version_string_rw[sizeof(r_ver.version_string_rw) - 1] = '\0';
+ count += scnprintf(buf + count, PAGE_SIZE - count,
+ "RO version: %s\n", r_ver.version_string_ro);
+ count += scnprintf(buf + count, PAGE_SIZE - count,
+ "RW version: %s\n", r_ver.version_string_rw);
+ count += scnprintf(buf + count, PAGE_SIZE - count,
+ "Firmware copy: %s\n",
+ (r_ver.current_image < ARRAY_SIZE(image_names) ?
+ image_names[r_ver.current_image] : "?"));
+
+ /* Get build info. */
+ msg.command = EC_CMD_GET_BUILD_INFO;
+ msg.indata = (uint8_t *)r_build_string;
+ msg.insize = sizeof(r_build_string);
+ ret = cros_ec_cmd_xfer(ec, &msg);
+ if (ret < 0)
+ count += scnprintf(buf + count, PAGE_SIZE - count,
+ "Build info: XFER ERROR %d\n", ret);
+ else if (msg.result != EC_RES_SUCCESS)
+ count += scnprintf(buf + count, PAGE_SIZE - count,
+ "Build info: EC error %d\n", msg.result);
+ else {
+ r_build_string[sizeof(r_build_string) - 1] = '\0';
+ count += scnprintf(buf + count, PAGE_SIZE - count,
+ "Build info: %s\n", r_build_string);
+ }
+
+ /* Get chip info. */
+ msg.command = EC_CMD_GET_CHIP_INFO;
+ msg.indata = (uint8_t *)&r_chip;
+ msg.insize = sizeof(r_chip);
+ ret = cros_ec_cmd_xfer(ec, &msg);
+ if (ret < 0)
+ count += scnprintf(buf + count, PAGE_SIZE - count,
+ "Chip info: XFER ERROR %d\n", ret);
+ else if (msg.result != EC_RES_SUCCESS)
+ count += scnprintf(buf + count, PAGE_SIZE - count,
+ "Chip info: EC error %d\n", msg.result);
+ else {
+ r_chip.vendor[sizeof(r_chip.vendor) - 1] = '\0';
+ r_chip.name[sizeof(r_chip.name) - 1] = '\0';
+ r_chip.revision[sizeof(r_chip.revision) - 1] = '\0';
+ count += scnprintf(buf + count, PAGE_SIZE - count,
+ "Chip vendor: %s\n", r_chip.vendor);
+ count += scnprintf(buf + count, PAGE_SIZE - count,
+ "Chip name: %s\n", r_chip.name);
+ count += scnprintf(buf + count, PAGE_SIZE - count,
+ "Chip revision: %s\n", r_chip.revision);
+ }
+
+ /* Get board version */
+ msg.command = EC_CMD_GET_BOARD_VERSION;
+ msg.indata = (uint8_t *)&r_board;
+ msg.insize = sizeof(r_board);
+ ret = cros_ec_cmd_xfer(ec, &msg);
+ if (ret < 0)
+ count += scnprintf(buf + count, PAGE_SIZE - count,
+ "Board version: XFER ERROR %d\n", ret);
+ else if (msg.result != EC_RES_SUCCESS)
+ count += scnprintf(buf + count, PAGE_SIZE - count,
+ "Board version: EC error %d\n", msg.result);
+ else {
+ count += scnprintf(buf + count, PAGE_SIZE - count,
+ "Board version: %d\n",
+ r_board.board_version);
+ }
+
+ return count;
+}
+
+static ssize_t show_ec_flashinfo(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct ec_response_flash_info resp;
+ struct cros_ec_command msg = { 0 };
+ int ret;
+ struct cros_ec_device *ec = dev_get_drvdata(dev);
+
+ /* The flash info shouldn't ever change, but ask each time anyway. */
+ msg.command = EC_CMD_FLASH_INFO;
+ msg.indata = (uint8_t *)&resp;
+ msg.insize = sizeof(resp);
+ ret = cros_ec_cmd_xfer(ec, &msg);
+ if (ret < 0)
+ return ret;
+ if (msg.result != EC_RES_SUCCESS)
+ return scnprintf(buf, PAGE_SIZE,
+ "ERROR: EC returned %d\n", msg.result);
+
+ return scnprintf(buf, PAGE_SIZE,
+ "FlashSize %d\nWriteSize %d\n"
+ "EraseSize %d\nProtectSize %d\n",
+ resp.flash_size, resp.write_block_size,
+ resp.erase_block_size, resp.protect_block_size);
+}
+
+/* Module initialization */
+
+static DEVICE_ATTR(reboot, S_IWUSR | S_IRUGO, show_ec_reboot, store_ec_reboot);
+static DEVICE_ATTR(version, S_IRUGO, show_ec_version, NULL);
+static DEVICE_ATTR(flashinfo, S_IRUGO, show_ec_flashinfo, NULL);
+
+static struct attribute *__ec_attrs[] = {
+ &dev_attr_reboot.attr,
+ &dev_attr_version.attr,
+ &dev_attr_flashinfo.attr,
+ NULL,
+};
+
+static struct attribute_group ec_attr_group = {
+ .attrs = __ec_attrs,
+};
+
+void ec_dev_sysfs_init(struct cros_ec_device *ec)
+{
+ int error;
+
+ error = sysfs_create_group(&ec->vdev->kobj, &ec_attr_group);
+ if (error)
+ pr_warn("failed to create group: %d\n", error);
+}
+
+void ec_dev_sysfs_remove(struct cros_ec_device *ec)
+{
+ sysfs_remove_group(&ec->vdev->kobj, &ec_attr_group);
+}
--
2.1.0
On Mon, 17 Nov 2014, Javier Martinez Canillas wrote:
> From: Bill Richardson <[email protected]>
>
> This patch adds a device interface to access the
> ChromeOS Embedded Controller from user-space.
>
> Signed-off-by: Bill Richardson <[email protected]>
> Reviewed-on: https://gerrit-int.chromium.org/38364
> Reviewed-by: Simon Glass <[email protected]>
> Tested-by: Simon Glass <[email protected]>
> Signed-off-by: Javier Martinez Canillas <[email protected]>
> ---
> drivers/mfd/Kconfig | 12 ++
> drivers/mfd/Makefile | 1 +
> drivers/mfd/cros_ec.c | 4 +
> drivers/mfd/cros_ec_dev.c | 355 ++++++++++++++++++++++++++++++++++++++++
> include/linux/mfd/cros_ec.h | 12 +-
> include/linux/mfd/cros_ec_dev.h | 47 ++++++
> 6 files changed, 430 insertions(+), 1 deletion(-)
> create mode 100644 drivers/mfd/cros_ec_dev.c
> create mode 100644 include/linux/mfd/cros_ec_dev.h
>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 72d3808..31420a7 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -115,6 +115,18 @@ config MFD_CROS_EC_SPI
> response time cannot be guaranteed, we support ignoring
> 'pre-amble' bytes before the response actually starts.
>
> +config MFD_CROS_EC_DEV
_DEV as a post-fix doesn't really describe the driver.
> + tristate "ChromeOS Embedded Controller userspace device interface"
> + depends on MFD_CROS_EC
> +
Superfluous '\n'.
> + help
> + If you say Y here, you get support for talking to the ChromeOS
> + EC from userspace. This is separate from the normal kernel
> + interfaces.
> +
> + To compile this driver as a module, choose M here: the module will be
> + called cros_ec_dev.
> +
> config MFD_ASIC3
> bool "Compaq ASIC3"
> depends on GPIOLIB && ARM
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index 53467e2..5104a25 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -12,6 +12,7 @@ obj-$(CONFIG_MFD_BCM590XX) += bcm590xx.o
> obj-$(CONFIG_MFD_CROS_EC) += cros_ec.o
> obj-$(CONFIG_MFD_CROS_EC_I2C) += cros_ec_i2c.o
> obj-$(CONFIG_MFD_CROS_EC_SPI) += cros_ec_spi.o
> +obj-$(CONFIG_MFD_CROS_EC_DEV) += cros_ec_dev.o
Alphabetical?
> rtsx_pci-objs := rtsx_pcr.o rtsx_gops.o rts5209.o rts5229.o rtl8411.o rts5227.o rts5249.o
> obj-$(CONFIG_MFD_RTSX_PCI) += rtsx_pci.o
> diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
> index fc0c81e..197b273 100644
> --- a/drivers/mfd/cros_ec.c
> +++ b/drivers/mfd/cros_ec.c
> @@ -119,6 +119,10 @@ static const struct mfd_cell cros_devs[] = {
> .id = 2,
> .of_compatible = "google,cros-ec-i2c-tunnel",
> },
> + {
> + .name = "cros-ec-dev",
> + .id = 3,
> + },
> };
>
> int cros_ec_register(struct cros_ec_device *ec_dev)
> diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
> new file mode 100644
> index 0000000..c18eb40
> --- /dev/null
> +++ b/drivers/mfd/cros_ec_dev.c
> @@ -0,0 +1,355 @@
> +/*
> + * cros_ec_dev - expose the Chrome OS Embedded Controller to userspace
> + *
> + * Copyright (C) 2014 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/>.
> + */
> +
> +#define pr_fmt(fmt) "cros_ec_dev: " fmt
This doesn't appear to even be used?
> +#include <linux/compat.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/fs.h>
> +#include <linux/mfd/cros_ec.h>
> +#include <linux/mfd/cros_ec_commands.h>
> +#include <linux/mfd/cros_ec_dev.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/printk.h>
What do you need this for?
> +#include <linux/types.h>
> +#include <linux/uaccess.h>
> +
> +/* Device variables */
> +#define CROS_CLASS_NAME "chromeos"
Any reason why you can't use the name directly?
> +static struct cros_ec_device *ec;
Can't you put this in a struct somewhere?
> +static struct class *cros_class;
Why this this global instead of using the one in 'struct
cros_ec_device'?
> +static int ec_major;
Same here. Only use globals if you're forced to.
> +
> +
Superfluous '\n'.
> +/* Basic communication */
> +static int ec_get_version(struct cros_ec_device *ec, char *str, int maxlen)
> +{
> + struct ec_response_get_version resp;
> + static const char * const current_image_name[] = {
> + "unknown", "read-only", "read-write", "invalid",
> + };
> + struct cros_ec_command msg = {
> + .version = 0,
> + .command = EC_CMD_GET_VERSION,
> + .outdata = NULL,
> + .outsize = 0,
> + .indata = (uint8_t *)&resp,
> + .insize = sizeof(resp),
> + };
> + int ret;
> +
> + ret = cros_ec_cmd_xfer(ec, &msg);
> + if (ret < 0)
> + return ret;
> + if (msg.result != EC_RES_SUCCESS) {
> + snprintf(str, maxlen,
> + "%s\nUnknown EC version: EC returned %d\n",
> + CROS_EC_DEV_VERSION, msg.result);
> + return 0;
> + }
> + if (resp.current_image >= ARRAY_SIZE(current_image_name))
> + resp.current_image = 3; /* invalid */
Add a '\n' here.
> + snprintf(str, maxlen, "%s\n%s\n%s\n\%s\n", CROS_EC_DEV_VERSION,
> + resp.version_string_ro, resp.version_string_rw,
> + current_image_name[resp.current_image]);
> +
> + return 0;
> +}
> +
> +/* Device file ops */
> +static int ec_device_open(struct inode *inode, struct file *filp)
> +{
> + return 0;
> +}
> +
> +static int ec_device_release(struct inode *inode, struct file *filp)
> +{
> + return 0;
> +}
> +
> +static ssize_t ec_device_read(struct file *filp, char __user *buffer,
> + size_t length, loff_t *offset)
> +{
> + char msg[sizeof(struct ec_response_get_version) +
> + sizeof(CROS_EC_DEV_VERSION)];
> + size_t count;
> + int ret;
> +
> + if (*offset != 0)
> + return 0;
> +
> + ret = ec_get_version(ec, msg, sizeof(msg));
> + if (ret)
> + return ret;
New line here. Not sure why you are bunching up these segments.
> + count = min(length, strlen(msg));
> +
> + if (copy_to_user(buffer, msg, count))
> + return -EFAULT;
> +
> + *offset += count;
You know offset is 0 here, so you can just *offset = count.
> + return count;
> +}
> +
> +
Please remove these double line spaces throughout.
> +/* Ioctls */
> +static long ec_device_ioctl_xcmd(void __user *argp)
> +{
> + long ret;
> + struct cros_ec_command s_cmd;
> + uint8_t *user_indata;
> + uint8_t buf[EC_PROTO2_MAX_PARAM_SIZE];
> +
> + if (copy_from_user(&s_cmd, argp, sizeof(s_cmd)))
> + return -EFAULT;
> + if (s_cmd.outsize &&
> + copy_from_user(&buf, (void __user *)s_cmd.outdata, sizeof(buf)))
> + return -EFAULT;
> +
> + user_indata = s_cmd.indata;
> + s_cmd.indata = buf;
> + s_cmd.outdata = buf;
> + ret = cros_ec_cmd_xfer(ec, &s_cmd);
> + s_cmd.indata = user_indata;
For conforming if you swap these round, place a '\n' after this line
and move the check for 'ret' to just after the call to
cros_ec_cmd_xfer().
> + /* Only copy data to userland if data was received. */
> + if (ret > 0 && s_cmd.insize) {
> + unsigned size = ret;
> +
> + size = min(size, s_cmd.insize);
> + if (copy_to_user((void __user *)s_cmd.indata, buf, size))
> + return -EFAULT;
> + }
> + if (copy_to_user(argp, &s_cmd, sizeof(s_cmd)))
> + return -EFAULT;
> +
> + return ret;
> +}
> +
> +static long ec_device_ioctl_readmem(void __user *argp)
> +{
> + struct cros_ec_readmem s_mem;
> + char buf[EC_MEMMAP_SIZE];
> + long num;
> +
> + /* Not every platform supports direct reads */
> + if (!ec->cmd_readmem)
> + return -ENOTTY;
> +
> + if (copy_from_user(&s_mem, argp, sizeof(s_mem)))
> + return -EFAULT;
> + num = ec->cmd_readmem(ec, s_mem.offset, s_mem.bytes, buf);
> + if (num <= 0)
> + return num;
> + if (copy_to_user((void __user *)s_mem.buffer, buf, num))
> + return -EFAULT;
> + return num;
> +}
> +
> +static long ec_device_ioctl(struct file *filp, unsigned int cmd,
> + unsigned long arg)
> +{
> + void __user *argp = (void __user *)arg;
> +
> + if (_IOC_TYPE(cmd) != CROS_EC_DEV_IOC)
> + return -ENOTTY;
> +
> + switch (cmd) {
> + case CROS_EC_DEV_IOCXCMD:
> + return ec_device_ioctl_xcmd(argp);
> + case CROS_EC_DEV_IOCRDMEM:
> + return ec_device_ioctl_readmem(argp);
> + }
> +
> + return -ENOTTY;
> +}
> +
> +#ifdef CONFIG_COMPAT
> +struct compat_cros_ec_command {
> + uint32_t version;
> + uint32_t command;
> + compat_uptr_t outdata;
> + uint32_t outsize;
> + compat_uptr_t indata;
> + uint32_t insize;
> + uint32_t result;
> +};
> +
> +struct compat_cros_ec_readmem {
> + uint32_t offset;
> + uint32_t bytes;
> + compat_uptr_t buffer;
> +};
> +
> +#define CROS_EC_DEV_COMPAT_IOCXCMD _IOWR(':', 0, struct compat_cros_ec_command)
> +#define CROS_EC_DEV_COMPAT_IOCRDMEM _IOWR(':', 1, struct compat_cros_ec_readmem)
> +
> +static long ec_device_compat_ioctl_readmem(void __user *argp)
> +{
> + struct compat_cros_ec_readmem compat_s_mem;
> + char buf[EC_MEMMAP_SIZE];
> + long num;
> +
> + /* Not every platform supports direct reads */
> + if (!ec->cmd_readmem)
> + return -ENOTTY;
> +
> + if (copy_from_user(&compat_s_mem, argp, sizeof(compat_s_mem)))
> + return -EFAULT;
> +
> + num = ec->cmd_readmem(ec, compat_s_mem.offset, compat_s_mem.bytes, buf);
> + if (num <= 0)
> + return num;
> +
> + if (copy_to_user(compat_ptr(compat_s_mem.buffer), buf, num))
> + return -EFAULT;
> + return num;
> +}
> +
> +static long ec_device_compat_ioctl_xcmd(void __user *argp)
> +{
> + long ret;
> + struct cros_ec_command s_cmd;
> + struct compat_cros_ec_command compat_s_cmd;
> + uint8_t buf[EC_PROTO2_MAX_PARAM_SIZE];
> +
> + if (copy_from_user(&compat_s_cmd, argp, sizeof(compat_s_cmd)))
> + return -EFAULT;
> +
> + s_cmd.version = compat_s_cmd.version;
> + s_cmd.command = compat_s_cmd.command;
> + s_cmd.insize = compat_s_cmd.insize;
> + s_cmd.outsize = compat_s_cmd.outsize;
> +
> + if (s_cmd.outsize &&
> + copy_from_user(&buf, compat_ptr(compat_s_cmd.outdata), sizeof(buf)))
> + return -EFAULT;
> +
> + s_cmd.indata = buf;
> + s_cmd.outdata = buf;
> + ret = cros_ec_cmd_xfer(ec, &s_cmd);
> +
> + compat_s_cmd.result = s_cmd.result;
> +
> + /* Only copy data to userland if data was received. */
> + if (ret > 0 && s_cmd.insize) {
> + unsigned size = ret;
> +
> + size = min(size, s_cmd.insize);
> + if (copy_to_user(compat_ptr(compat_s_cmd.indata), buf, size))
> + return -EFAULT;
> + }
> +
> + if (copy_to_user(argp, &compat_s_cmd, sizeof(compat_s_cmd)))
> + return -EFAULT;
> +
> + return ret;
> +}
> +
> +static long ec_device_compat_ioctl(struct file *filp, unsigned int cmd,
> + unsigned long arg)
> +{
> + void __user
> + *argp = (void __user *)arg;
Why is this expressed over multiple lines?
> + switch (cmd) {
> + case CROS_EC_DEV_COMPAT_IOCXCMD:
> + return ec_device_compat_ioctl_xcmd(argp);
> + case CROS_EC_DEV_COMPAT_IOCRDMEM:
> + return ec_device_compat_ioctl_readmem(argp);
> + }
'\n'
> + return -ENOTTY;
> +}
> +#endif /* CONFIG_COMPAT */
> +
> +
Please remove.
> +/* Module initialization */
> +static const struct file_operations fops = {
> + .open = ec_device_open,
> + .release = ec_device_release,
> + .read = ec_device_read,
> + .unlocked_ioctl = ec_device_ioctl,
> +#ifdef CONFIG_COMPAT
> + .compat_ioctl = ec_device_compat_ioctl,
> +#endif
> +};
> +
> +static int ec_device_probe(struct platform_device *pdev)
> +{
> + int retval = -ENOTTY;
> +
> + ec = dev_get_drvdata(pdev->dev.parent);
Stick this as the first line in the function.
> + cros_class = class_create(THIS_MODULE, CROS_CLASS_NAME);
> + if (IS_ERR(cros_class)) {
> + pr_err("failed to register device class\n");
Why are you using pr_err() over dev_err()?
> + retval = PTR_ERR(cros_class);
> + goto failed_class;
Remove this goto and the corresponding label and just return from here.
> + }
> +
> + /* Let the kernel pick the major num for us */
> + ec_major = register_chrdev(0, CROS_EC_DEV_NAME, &fops);
> + if (ec_major < 0) {
> + pr_err("failed to register device\n");
> + retval = ec_major;
> + goto failed_chrdevreg;
> + }
> +
> + /* Instantiate it */
> + ec->vdev = device_create(cros_class, NULL, MKDEV(ec_major, 0),
> + NULL, CROS_EC_DEV_NAME);
> + if (IS_ERR(ec->vdev)) {
> + pr_err("failed to create device\n");
> + retval = PTR_ERR(ec->vdev);
> + goto failed_devreg;
> + }
> +
> + return 0;
> +
> +failed_devreg:
> + unregister_chrdev(ec_major, CROS_EC_DEV_NAME);
> +failed_chrdevreg:
> + class_destroy(cros_class);
> +failed_class:
> + return retval;
> +}
> +
> +static int ec_device_remove(struct platform_device *pdev)
> +{
> + device_destroy(cros_class, MKDEV(ec_major, 0));
> + unregister_chrdev(ec_major, CROS_EC_DEV_NAME);
> + class_destroy(cros_class);
> + return 0;
> +}
> +
> +static struct platform_driver cros_ec_dev_driver = {
> + .driver = {
> + .name = "cros-ec-dev",
> + .owner = THIS_MODULE,
Remove this line.
> + },
> + .probe = ec_device_probe,
> + .remove = ec_device_remove,
> +};
Where is this device registered from?
> +module_platform_driver(cros_ec_dev_driver);
> +
> +MODULE_AUTHOR("Bill Richardson <[email protected]>");
> +MODULE_DESCRIPTION("Userspace interface to the Chrome OS Embedded Controller");
> +MODULE_VERSION("1.0");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
> index 0e166b9..b1cb934 100644
> --- a/include/linux/mfd/cros_ec.h
> +++ b/include/linux/mfd/cros_ec.h
> @@ -59,9 +59,16 @@ struct cros_ec_command {
> *
> * @ec_name: name of EC device (e.g. 'chromeos-ec')
> * @phys_name: name of physical comms layer (e.g. 'i2c-4')
> - * @dev: Device pointer
> + * @dev: Device pointer for physical comms device
> + * @vdev: Device pointer for virtual comms device
> * @was_wake_device: true if this device was set to wake the system from
> * sleep at the last suspend
> + * @cmd_read_mem: direct read of the EC memory-mapped region, if supported
> + * @offset is within EC_LPC_ADDR_MEMMAP region.
> + * @bytes: number of bytes to read. zero means "read a string" (including
> + * the trailing '\0'). At most only EC_MEMMAP_SIZE bytes can be read.
> + * Caller must ensure that the buffer is large enough for the result when
> + * reading a string.
> *
> * @priv: Private data
> * @irq: Interrupt to use
> @@ -90,8 +97,11 @@ struct cros_ec_device {
> const char *ec_name;
> const char *phys_name;
> struct device *dev;
> + struct device *vdev;
> bool was_wake_device;
> struct class *cros_class;
> + int (*cmd_readmem)(struct cros_ec_device *ec, unsigned int offset,
> + unsigned int bytes, void *dest);
>
> /* These are used to implement the platform-specific interface */
> void *priv;
> diff --git a/include/linux/mfd/cros_ec_dev.h b/include/linux/mfd/cros_ec_dev.h
> new file mode 100644
> index 0000000..465e926
> --- /dev/null
> +++ b/include/linux/mfd/cros_ec_dev.h
> @@ -0,0 +1,47 @@
> +/*
> + * cros_ec_dev - expose the Chrome OS Embedded Controller to userspace
> + *
> + * Copyright (C) 2013 The Chromium OS Authors
> + *
> + * 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 _CROS_EC_DEV_H_
> +#define _CROS_EC_DEV_H_
> +
> +#include <linux/ioctl.h>
> +#include <linux/types.h>
> +#include <linux/mfd/cros_ec.h>
> +
> +#define CROS_EC_DEV_NAME "cros_ec"
> +#define CROS_EC_DEV_VERSION "1.0.0"
> +
> +/*
> + * @offset: within EC_LPC_ADDR_MEMMAP region
> + * @bytes: number of bytes to read. zero means "read a string" (including '\0')
> + * (at most only EC_MEMMAP_SIZE bytes can be read)
> + * @buffer: where to store the result
> + * ioctl returns the number of bytes read, negative on error
> + */
> +struct cros_ec_readmem {
> + uint32_t offset;
> + uint32_t bytes;
> + char *buffer;
> +};
> +
> +#define CROS_EC_DEV_IOC ':'
> +#define CROS_EC_DEV_IOCXCMD _IOWR(':', 0, struct cros_ec_command)
> +#define CROS_EC_DEV_IOCRDMEM _IOWR(':', 1, struct cros_ec_readmem)
> +
> +#endif /* _CROS_EC_DEV_H_ */
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
On Mon, 17 Nov 2014, Javier Martinez Canillas wrote:
> From: Bill Richardson <[email protected]>
>
> This adds some sysfs entries to provide userspace control of the
> four-element LED "lightbar" on the Chromebook Pixel. This only instantiates
> the lightbar controls if the device actually exists.
>
> To prevent DoS attacks, this interface is limited to 20 accesses/second,
> although that rate can be adjusted by a privileged user.
>
> On Chromebooks without a lightbar, this should have no effect. On the
> Chromebook Pixel, you should be able to do things like this:
>
> $ cd /sys/devices/virtual/chromeos/cros_ec/lightbar
> $ echo 0x80 > brightness
> $ echo 255 > brightness
> $
> $ cat sequence
> S0
> $ echo konami > sequence
> $ cat sequence
> KONAMI
> $
> $ cat sequence
> S0
>
> And
>
> $ cd /sys/devices/virtual/chromeos/cros_ec/lightbar
> $ echo stop > sequence
> $ echo "4 255 255 255" > led_rgb
> $ echo "0 255 0 0 1 0 255 0 2 0 0 255 3 255 255 0" > led_rgb
> $ echo run > sequence
>
> Test the DoS prevention with this:
>
> $ cd /sys/devices/virtual/chromeos/cros_ec/lightbar
> $ echo 500 > interval_msec
> $ time (cat version version version version version version version)
>
> Signed-off-by: Bill Richardson <[email protected]>
> Reviewed-by: Olof Johansson <[email protected]>
> Tested-by: Doug Anderson <[email protected]>
> Reviewed-by: Benson Leung <[email protected]>
> Signed-off-by: Javier Martinez Canillas <[email protected]>
> ---
> drivers/mfd/Makefile | 2 +-
> drivers/mfd/cros_ec_dev.c | 2 +
> drivers/mfd/cros_ec_dev.h | 3 +
> drivers/mfd/cros_ec_lightbar.c | 347 +++++++++++++++++++++++++++++++++++++++++
By the sounds of the description, it doesn't seem as though this
driver lives in MFD. I suggest another home, such as drivers/led.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
On Mon, 17 Nov 2014, Javier Martinez Canillas wrote:
> From: Bill Richardson <[email protected]>
>
> This adds the first few sysfs attributes for the Chrome OS EC. These
> controls are made available under /sys/devices/virtual/chromeos/cros_ec
>
> flashinfo - display current flash info
drivers/mtd?
> reboot - tell the EC to reboot in various ways
drivers/power?
> version - information about the EC software and hardware
What's the difference between this version and the version you can
read in the new _dev driver?
> Future changes will build on this to add additional controls.
>
> From a root shell, you should be able to do things like this:
>
> cd /sys/devices/virtual/chromeos/cros_ec
> cat flashinfo
> cat version
> echo rw > reboot
> cat version
> echo ro > reboot
> cat version
> echo rw > reboot
> cat version
> echo cold > reboot
>
> That last command will reboot the AP too.
>
> Signed-off-by: Bill Richardson <[email protected]>
> Reviewed-by: Olof Johansson <[email protected]>
> Signed-off-by: Javier Martinez Canillas <[email protected]>
> ---
> drivers/mfd/Makefile | 3 +-
> drivers/mfd/cros_ec_dev.c | 10 +-
> drivers/mfd/cros_ec_dev.h | 26 +++++
> drivers/mfd/cros_ec_sysfs.c | 271 ++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 307 insertions(+), 3 deletions(-)
> create mode 100644 drivers/mfd/cros_ec_dev.h
> create mode 100644 drivers/mfd/cros_ec_sysfs.c
[...]
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
> +#ifdef CONFIG_COMPAT
> +struct compat_cros_ec_command {
> + uint32_t version;
> + uint32_t command;
> + compat_uptr_t outdata;
> + uint32_t outsize;
> + compat_uptr_t indata;
> + uint32_t insize;
> + uint32_t result;
> +};
> +
> +struct compat_cros_ec_readmem {
> + uint32_t offset;
> + uint32_t bytes;
> + compat_uptr_t buffer;
> +};
>
This is a new API - arrange them to be 64bit safe and properly padded,
there is no excuse for needing compat crap except for legacy interfaces
you can't fix.
Alan
Hello Alan,
Thanks a lot for your feedback.
On 11/18/2014 06:00 PM, One Thousand Gnomes wrote:
>> +#ifdef CONFIG_COMPAT
>> +struct compat_cros_ec_command {
>> + uint32_t version;
>> + uint32_t command;
>> + compat_uptr_t outdata;
>> + uint32_t outsize;
>> + compat_uptr_t indata;
>> + uint32_t insize;
>> + uint32_t result;
>> +};
>> +
>> +struct compat_cros_ec_readmem {
>> + uint32_t offset;
>> + uint32_t bytes;
>> + compat_uptr_t buffer;
>> +};
>>
>
> This is a new API - arrange them to be 64bit safe and properly padded,
> there is no excuse for needing compat crap except for legacy interfaces
> you can't fix.
>
Is true that this is a new API for mainline but there is a lot of ChromeOS
installations that depends on this API which means that just replacing the
kernel with a mainline one there, will break existing user-space programs.
But I understand that since those binaries were using a non-ustream kernel
it is expected that the kernel API could be changed.
I think it would be great to keep existing binaries working but if changing
the API is required, then I can certainly do that when doing a re-spin.
Best regards,
Javier
On Wed, Nov 19, 2014 at 10:37 AM, Javier Martinez Canillas
<[email protected]> wrote:
> Hello Alan,
>
> Thanks a lot for your feedback.
>
> On 11/18/2014 06:00 PM, One Thousand Gnomes wrote:
>>> +#ifdef CONFIG_COMPAT
>>> +struct compat_cros_ec_command {
>>> + uint32_t version;
>>> + uint32_t command;
>>> + compat_uptr_t outdata;
>>> + uint32_t outsize;
>>> + compat_uptr_t indata;
>>> + uint32_t insize;
>>> + uint32_t result;
>>> +};
>>> +
>>> +struct compat_cros_ec_readmem {
>>> + uint32_t offset;
>>> + uint32_t bytes;
>>> + compat_uptr_t buffer;
>>> +};
>>>
>>
>> This is a new API - arrange them to be 64bit safe and properly padded,
>> there is no excuse for needing compat crap except for legacy interfaces
>> you can't fix.
>>
>
> Is true that this is a new API for mainline but there is a lot of ChromeOS
> installations that depends on this API which means that just replacing the
> kernel with a mainline one there, will break existing user-space programs.
I think we can deal with that, at least if we pick new ioctl numbers
so we can tell from the userspace tool which interface is in use
during transition.
> But I understand that since those binaries were using a non-ustream kernel
> it is expected that the kernel API could be changed.
>
> I think it would be great to keep existing binaries working but if changing
> the API is required, then I can certainly do that when doing a re-spin.
I think there's some value in that, but i'm also somewhat embarrassed
to have missed this aspect when doing internal review, and do agree
with Alan. :) And we have only a few tools that use this interface so
we should be able to cope with it.
-Olof
Hello Olof,
On 11/19/2014 09:45 PM, Olof Johansson wrote:
>> Is true that this is a new API for mainline but there is a lot of ChromeOS
>> installations that depends on this API which means that just replacing the
>> kernel with a mainline one there, will break existing user-space programs.
>
> I think we can deal with that, at least if we pick new ioctl numbers
> so we can tell from the userspace tool which interface is in use
> during transition.
>
>> But I understand that since those binaries were using a non-ustream kernel
>> it is expected that the kernel API could be changed.
>>
>> I think it would be great to keep existing binaries working but if changing
>> the API is required, then I can certainly do that when doing a re-spin.
>
> I think there's some value in that, but i'm also somewhat embarrassed
> to have missed this aspect when doing internal review, and do agree
> with Alan. :) And we have only a few tools that use this interface so
> we should be able to cope with it.
>
>
Thanks a lot for your feedback. I'll follow Alan suggestion then and make
the structs to be 64-bit safe and properly padded. Also, I'll follow your
suggestion and use a different magic number for the IOCTLs so user-space
programs can be backward compatible if needed.
> -Olof
>
Best regards,
Javier
Hello Lee,
Thanks a lot for your feedback.
On 11/18/2014 03:18 PM, Lee Jones wrote:
>>
>> +config MFD_CROS_EC_DEV
>
> _DEV as a post-fix doesn't really describe the driver.
>
the _DEV is meant to denote (user-space device interface) but is true that
a better name can be used, I'll see what I can came with.
>> + tristate "ChromeOS Embedded Controller userspace device interface"
>> + depends on MFD_CROS_EC
>> +
>
> Superfluous '\n'.
>
Ok
>> --- a/drivers/mfd/Makefile
>> +++ b/drivers/mfd/Makefile
>> @@ -12,6 +12,7 @@ obj-$(CONFIG_MFD_BCM590XX) += bcm590xx.o
>> obj-$(CONFIG_MFD_CROS_EC) += cros_ec.o
>> obj-$(CONFIG_MFD_CROS_EC_I2C) += cros_ec_i2c.o
>> obj-$(CONFIG_MFD_CROS_EC_SPI) += cros_ec_spi.o
>> +obj-$(CONFIG_MFD_CROS_EC_DEV) += cros_ec_dev.o
>
> Alphabetical?
>
Ok
>> +
>> +#define pr_fmt(fmt) "cros_ec_dev: " fmt
>
> This doesn't appear to even be used?
>
It is used by pr_* family of functions, e.g: from include/linux/printk.h
#define pr_err(fmt, ...) \
printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
>> +#include <linux/compat.h>
>> +#include <linux/delay.h>
>> +#include <linux/device.h>
>> +#include <linux/fs.h>
>> +#include <linux/mfd/cros_ec.h>
>> +#include <linux/mfd/cros_ec_commands.h>
>> +#include <linux/mfd/cros_ec_dev.h>
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/printk.h>
>
> What do you need this for?
>
the printk.h header? to use the pr_* functions but I'll make sure that only
the needed headers are included.
>> +#include <linux/types.h>
>> +#include <linux/uaccess.h>
>> +
>> +/* Device variables */
>> +#define CROS_CLASS_NAME "chromeos"
>
> Any reason why you can't use the name directly?
>
I prefer macros if possible since they cost nothing and give you an indirection
level if you want to change it later. Any reason to not use a define directive?
>> +static struct cros_ec_device *ec;
>
> Can't you put this in a struct somewhere?
>
Yes, I'll remove the global variable and put it in struct file .private_data
>> +static struct class *cros_class;
>
> Why this this global instead of using the one in 'struct
> cros_ec_device'?
>
>> +static int ec_major;
>
> Same here. Only use globals if you're forced to.
>
Ok, I'll look how to refactor to remove these globals too.
>> +
>> +
>
> Superfluous '\n'.
>
Ok
>> +
>> + ret = cros_ec_cmd_xfer(ec, &msg);
>> + if (ret < 0)
>> + return ret;
>> + if (msg.result != EC_RES_SUCCESS) {
>> + snprintf(str, maxlen,
>> + "%s\nUnknown EC version: EC returned %d\n",
>> + CROS_EC_DEV_VERSION, msg.result);
>> + return 0;
>> + }
>> + if (resp.current_image >= ARRAY_SIZE(current_image_name))
>> + resp.current_image = 3; /* invalid */
>
> Add a '\n' here.
>
Ok
>> + if (*offset != 0)
>> + return 0;
>> +
>> + ret = ec_get_version(ec, msg, sizeof(msg));
>> + if (ret)
>> + return ret;
>
> New line here. Not sure why you are bunching up these segments.
>
Ok
>> + count = min(length, strlen(msg));
>> +
>> + if (copy_to_user(buffer, msg, count))
>> + return -EFAULT;
>> +
>> + *offset += count;
>
> You know offset is 0 here, so you can just *offset = count.
>
True
>> + return count;
>> +}
>> +
>> +
>
> Please remove these double line spaces throughout.
>
Ok
>> +/* Ioctls */
>> +static long ec_device_ioctl_xcmd(void __user *argp)
>> +{
>> + long ret;
>> + struct cros_ec_command s_cmd;
>> + uint8_t *user_indata;
>> + uint8_t buf[EC_PROTO2_MAX_PARAM_SIZE];
>> +
>> + if (copy_from_user(&s_cmd, argp, sizeof(s_cmd)))
>> + return -EFAULT;
>> + if (s_cmd.outsize &&
>> + copy_from_user(&buf, (void __user *)s_cmd.outdata, sizeof(buf)))
>> + return -EFAULT;
>> +
>> + user_indata = s_cmd.indata;
>> + s_cmd.indata = buf;
>> + s_cmd.outdata = buf;
>> + ret = cros_ec_cmd_xfer(ec, &s_cmd);
>> + s_cmd.indata = user_indata;
>
> For conforming if you swap these round, place a '\n' after this line
> and move the check for 'ret' to just after the call to
> cros_ec_cmd_xfer().
>
Ok
>> +static long ec_device_compat_ioctl(struct file *filp, unsigned int cmd,
>> + unsigned long arg)
>> +{
>> + void __user
>> + *argp = (void __user *)arg;
>
> Why is this expressed over multiple lines?
>
No reason at all, I'll change it.
>> + switch (cmd) {
>> + case CROS_EC_DEV_COMPAT_IOCXCMD:
>> + return ec_device_compat_ioctl_xcmd(argp);
>> + case CROS_EC_DEV_COMPAT_IOCRDMEM:
>> + return ec_device_compat_ioctl_readmem(argp);
>> + }
>
> '\n'
>
Ok
>> + return -ENOTTY;
>> +}
>> +#endif /* CONFIG_COMPAT */
>> +
>> +
>
> Please remove.
>
Ok
>> +static int ec_device_probe(struct platform_device *pdev)
>> +{
>> + int retval = -ENOTTY;
>> +
>> + ec = dev_get_drvdata(pdev->dev.parent);
>
> Stick this as the first line in the function.
>
Ok
>> + cros_class = class_create(THIS_MODULE, CROS_CLASS_NAME);
>> + if (IS_ERR(cros_class)) {
>> + pr_err("failed to register device class\n");
>
> Why are you using pr_err() over dev_err()?
>
You are right, I'll use dev_err() instead.
>> + retval = PTR_ERR(cros_class);
>> + goto failed_class;
>
> Remove this goto and the corresponding label and just return from here.
>
Ok
>> +
>> +static struct platform_driver cros_ec_dev_driver = {
>> + .driver = {
>> + .name = "cros-ec-dev",
>> + .owner = THIS_MODULE,
>
> Remove this line.
>
Right, I've seen some cleanups efforts to remove the owner from drivers
but forgot when reviewing this driver for posting.
>> + },
>> + .probe = ec_device_probe,
>> + .remove = ec_device_remove,
>> +};
>
> Where is this device registered from?
>
>> +module_platform_driver(cros_ec_dev_driver);
This preprocessor macro is expanded to (from include/linux/device.h):
module_platform_driver() -> module_driver()
#define module_driver(__driver, __register, __unregister, ...) \
static int __init __driver##_init(void) \
{ \
return __register(&(__driver) , ##__VA_ARGS__); \
} \
Again, thanks a lot for your detailed review.
Best regards,
Javier
[...]
> >> +#include <linux/compat.h>
> >> +#include <linux/delay.h>
> >> +#include <linux/device.h>
> >> +#include <linux/fs.h>
> >> +#include <linux/mfd/cros_ec.h>
> >> +#include <linux/mfd/cros_ec_commands.h>
> >> +#include <linux/mfd/cros_ec_dev.h>
> >> +#include <linux/module.h>
> >> +#include <linux/platform_device.h>
> >> +#include <linux/printk.h>
> >
> > What do you need this for?
> >
>
> the printk.h header? to use the pr_* functions but I'll make sure that only
> the needed headers are included.
Right, I think don't think you should be using those on a platform device.
> >> +#include <linux/types.h>
> >> +#include <linux/uaccess.h>
> >> +
> >> +/* Device variables */
> >> +#define CROS_CLASS_NAME "chromeos"
> >
> > Any reason why you can't use the name directly?
> >
>
> I prefer macros if possible since they cost nothing and give you an indirection
> level if you want to change it later. Any reason to not use a define directive?
Exactly as you said, they add a layer of (pointless)
indirection/complexity. You only use this name once, just change it
where you use it if you wish to (but probably never will) adapt the
name.
[...]
> >> +static struct platform_driver cros_ec_dev_driver = {
> >> + .driver = {
> >> + .name = "cros-ec-dev",
> >> + .owner = THIS_MODULE,
> >
> > Remove this line.
> >
>
> Right, I've seen some cleanups efforts to remove the owner from drivers
> but forgot when reviewing this driver for posting.
>
> >> + },
> >> + .probe = ec_device_probe,
> >> + .remove = ec_device_remove,
> >> +};
> >
> > Where is this device registered from?
> >
> >> +module_platform_driver(cros_ec_dev_driver);
>
> This preprocessor macro is expanded to (from include/linux/device.h):
>
> module_platform_driver() -> module_driver()
>
> #define module_driver(__driver, __register, __unregister, ...) \
> static int __init __driver##_init(void) \
> { \
> return __register(&(__driver) , ##__VA_ARGS__); \
> } \
I know how the device driver model works. I'm asking where the
'device' is registered from, not the 'driver' i.e. platform data, DT,
ACPI?
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
Hello Lee,
On 11/18/2014 03:26 PM, Lee Jones wrote:
> On Mon, 17 Nov 2014, Javier Martinez Canillas wrote:
>
>> From: Bill Richardson <[email protected]>
>>
>> This adds the first few sysfs attributes for the Chrome OS EC. These
>> controls are made available under /sys/devices/virtual/chromeos/cros_ec
>>
>> flashinfo - display current flash info
>
> drivers/mtd?
>
>> reboot - tell the EC to reboot in various ways
>
> drivers/power?
>
Well this driver is special in the sense that there is a Cortex-M Embedded
Controller that has different peripherals (flash, keyboard, charger, etc).
The kernel communicates with these peripherals by using a tunnel through
the protocol used to communicate with the Embedded Controller (SPI/I2C/LPC).
But you are right that this is not suitable for drivers/mfd, I just added
there because that is the location in the downstream ChromeOS kernel but
like you said, the MFD subsystem should not be a dumping ground for devices
that is not clear where should live.
On v2 I'll add this sysfs interface and the dev to drivers/platform/chrome/
since I think is a better place for the cros_ec dev driver. It will still
be spawns from the cros_ec MFD driver though but that is OK for you AFAIU.
>> version - information about the EC software and hardware
>
> What's the difference between this version and the version you can
> read in the new _dev driver?
>
The version information read from the cros_ec dev interface is a subset
of the information read from the version sysfs, e.g:
$ cat /dev/cros_ec
1.0.0
pit_v1.1.1198-1cd618e
pit_v1.1.1198-1cd618e
read-only
$ cat /sys/class/chromeos/cros_ec/version
RO version: pit_v1.1.1198-1cd618e
RW version: pit_v1.1.1198-1cd618e
Firmware copy: RO
Build info: pit_v1.1.1198-1cd618e 2014-04-30 16:29:52 @build122-m2
Chip vendor: stm
Chip name: stm32l15x
Chip revision:
Board version: 0
I don't really know why there are two interfaces for this but I guess
that different user-space utilities use one or another. Maybe the
ChromiumOS folks can comment on this.
Best regards,
Javier
Hello Lee,
On 11/18/2014 03:22 PM, Lee Jones wrote:
>> ---
>> drivers/mfd/Makefile | 2 +-
>> drivers/mfd/cros_ec_dev.c | 2 +
>> drivers/mfd/cros_ec_dev.h | 3 +
>> drivers/mfd/cros_ec_lightbar.c | 347 +++++++++++++++++++++++++++++++++++++++++
>
> By the sounds of the description, it doesn't seem as though this
> driver lives in MFD. I suggest another home, such as drivers/led.
>
You are right. As I said in the other email, I'll move it to
drivers/platform/chrome/ which seems like a more suitable place.
Thanks a lot for your feedback.
Best regards,
Javier
Hello Lee,
On 11/20/2014 12:58 PM, Lee Jones wrote:
>> the printk.h header? to use the pr_* functions but I'll make sure that only
>> the needed headers are included.
>
> Right, I think don't think you should be using those on a platform device.
>
Yes, I'll use dev_err() instead.
>> I prefer macros if possible since they cost nothing and give you an indirection
>> level if you want to change it later. Any reason to not use a define directive?
>
> Exactly as you said, they add a layer of (pointless)
> indirection/complexity. You only use this name once, just change it
> where you use it if you wish to (but probably never will) adapt the
> name.
>
Fair enough, I'll remove it.
>
> I know how the device driver model works. I'm asking where the
> 'device' is registered from, not the 'driver' i.e. platform data, DT,
> ACPI?
>
Right, sorry for misunderstanding your question and the silly comment then.
$Subject adds a "cros-ec-dev" mfd cell to the cros ec mfd driver.
So the device is registered from DT when the cros ec device node is
matched (e.g: "google,cros-ec-spi" or "google,cros-ec-i2c") and the
cros ec mfd driver probe function calls mfd_add_devices().
Best regards,
Javier
> > I know how the device driver model works. I'm asking where the
> > 'device' is registered from, not the 'driver' i.e. platform data, DT,
> > ACPI?
> >
>
> Right, sorry for misunderstanding your question and the silly comment then.
>
> $Subject adds a "cros-ec-dev" mfd cell to the cros ec mfd driver.
> So the device is registered from DT when the cros ec device node is
> matched (e.g: "google,cros-ec-spi" or "google,cros-ec-i2c") and the
> cros ec mfd driver probe function calls mfd_add_devices().
Ah, so it's registered from another MFD device. That's okay then.
That answers my question. FWIW I was going to ask about the missing
match table, but that's not required in this case.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
On Thu, 20 Nov 2014, Javier Martinez Canillas wrote:
> Hello Lee,
>
> On 11/18/2014 03:22 PM, Lee Jones wrote:
> >> ---
> >> drivers/mfd/Makefile | 2 +-
> >> drivers/mfd/cros_ec_dev.c | 2 +
> >> drivers/mfd/cros_ec_dev.h | 3 +
> >> drivers/mfd/cros_ec_lightbar.c | 347 +++++++++++++++++++++++++++++++++++++++++
> >
> > By the sounds of the description, it doesn't seem as though this
> > driver lives in MFD. I suggest another home, such as drivers/led.
> >
>
> You are right. As I said in the other email, I'll move it to
> drivers/platform/chrome/ which seems like a more suitable place.
Does it? I would have thought 'lightbar' lives in LEDs, but I guess
that's between you and the drivers/platform/chrome maintainer now.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
Hello Lee,
On 11/20/2014 02:27 PM, Lee Jones wrote:
>> > By the sounds of the description, it doesn't seem as though this
>> > driver lives in MFD. I suggest another home, such as drivers/led.
>> >
>>
>> You are right. As I said in the other email, I'll move it to
>> drivers/platform/chrome/ which seems like a more suitable place.
>
> Does it? I would have thought 'lightbar' lives in LEDs, but I guess
> that's between you and the drivers/platform/chrome maintainer now.
>
Ok, I'll move cros_ec_lightbar to drivers/led then and cros_ec_dev
to drivers/platform/chrome.
Thanks a lot for all your suggestions!
Best regards,
Javier
On Thu, Nov 20, 2014 at 3:58 AM, Javier Martinez Canillas
<[email protected]> wrote:
>
> I don't really know why there are two interfaces for this but I guess
> that different user-space utilities use one or another. Maybe the
> ChromiumOS folks can comment on this.
The /dev/cros_ec interface responds to ioctls and is the primary means
by which userspace applications talk to the EC (replacing three really
awkward bus-specific libraries). It emits a simplified version string
when read so that humans can tell if the EC is alive and speaking the
same protocol that the userspace app expects.
The /sys/class/chromeos/cros_ec/* tree was originally envisioned as a
way to provide additional human-readable interfaces to a subset of the
EC commands, but except for the lightbar it's not often used. The
version component here shows all the version-related info that the EC
can provide.
I apologize for the poor kernel conventions used here. In my
ignorance, I apparently chose the worst code possible to use as a
template, and then invented some new stupidness of my own.
Bill
--
Art for Art's Sake
Engineering for Money
Hello Bill,
On 11/20/2014 07:16 PM, Bill Richardson wrote:
>>
>> I don't really know why there are two interfaces for this but I guess
>> that different user-space utilities use one or another. Maybe the
>> ChromiumOS folks can comment on this.
>
> The /dev/cros_ec interface responds to ioctls and is the primary means
> by which userspace applications talk to the EC (replacing three really
> awkward bus-specific libraries). It emits a simplified version string
> when read so that humans can tell if the EC is alive and speaking the
> same protocol that the userspace app expects.
>
> The /sys/class/chromeos/cros_ec/* tree was originally envisioned as a
> way to provide additional human-readable interfaces to a subset of the
> EC commands, but except for the lightbar it's not often used. The
> version component here shows all the version-related info that the EC
> can provide.
>
Got it, thanks a lot for the explanation.
Best regards,
Javier