2015-02-02 11:26:57

by Javier Martinez Canillas

[permalink] [raw]
Subject: [PATCH v5 0/7] platform/chrome: Add user-space dev inferface support

Hello,

The mainline ChromeOS Embedded Controller (EC) driver is still missing some
features that are present in the downstream ChromiumOS tree. These are:

- Low Pin Count (LPC) interface
- User-space device 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 is a fifth version of a series that adds support for the first two of
the missing features: the EC LPC and EC character device interfaces that
are used by user-space to access the ChromeOS EC. The support patches were
taken from the downstream ChromiumOS 3.14 tree with the fixes and cleanups
squashed to have a minimal patch-set.

The version of the ChromeOS EC chardev driver in this series still does not
reflect the latest one that is in the downstream ChromiumOS 3.14 kernel but
makes the delta shorter. Following patches will add the remaining missing
features until both trees are in sync. I preferred to first add the initial
support and then adding the other features to both maintain the original
patch history in the downstream kernel and so preserve the patch authorship
and also make the diff to have a working cros user-space interface smaller.

This version solves the issues pointed out in the earlier revision.

A big difference between this series and the downstream ChromiumOS kernel is
that the ioctl API is modified to make it 64-bit safe and compatible with both
64 and 32 bit user-space binaries. The data structures passed as arguments in
the ChromiumOS ioctl interface commands has pointers fields and since these
have different byte boundaries alignment requirement, the ChromiumOS driver
has a compat ioctl interface. The feedback was that this had to be avoided
since this was a new ioctl API so the pointers fields were replaced with a set
of fixed-size arrays to be used instead. This has the drawback that more data
could be used and copied between user and kernel space so feedback is welcomed
if there is a better approach to solve this kind of issues.

The patches were tested on an Exynos5420 Peach Pit Chromebook and (thanks to
Bill Richardson) on an x86 Pixel Chromebook using a modified ectool [0] to use
the new ioctl API. The LPC interface driver and the lightbar sysfs driver were
also tested on the Pixel Chromebook.

The series is composed of the following patches:

Bill Richardson (4):
platform/chrome: Add cros_ec_lpc driver for x86 devices
platform/chrome: Add Chrome OS EC userspace device interface
platform/chrome: Create sysfs attributes for the ChromeOS EC
platform/chrome: Expose Chrome OS Lightbar to users

Javier Martinez Canillas (3):
mfd: cros_ec: Use fixed size arrays to transfer data with the EC
mfd: cros_ec: Add char dev and virtual dev pointers
mfd: cros_ec: Instantiate ChromeOS EC character device

Documentation/ioctl/ioctl-number.txt | 1 +
drivers/i2c/busses/i2c-cros-ec-tunnel.c | 51 +---
drivers/input/keyboard/cros_ec_keyb.c | 13 +-
drivers/mfd/cros_ec.c | 19 +-
drivers/platform/chrome/Kconfig | 26 +-
drivers/platform/chrome/Makefile | 3 +
drivers/platform/chrome/cros_ec_dev.c | 274 +++++++++++++++++++++
drivers/platform/chrome/cros_ec_dev.h | 53 +++++
drivers/platform/chrome/cros_ec_lightbar.c | 367 +++++++++++++++++++++++++++++
drivers/platform/chrome/cros_ec_lpc.c | 319 +++++++++++++++++++++++++
drivers/platform/chrome/cros_ec_sysfs.c | 271 +++++++++++++++++++++
include/linux/mfd/cros_ec.h | 23 +-
12 files changed, 1358 insertions(+), 62 deletions(-)
create mode 100644 drivers/platform/chrome/cros_ec_dev.c
create mode 100644 drivers/platform/chrome/cros_ec_dev.h
create mode 100644 drivers/platform/chrome/cros_ec_lightbar.c
create mode 100644 drivers/platform/chrome/cros_ec_lpc.c
create mode 100644 drivers/platform/chrome/cros_ec_sysfs.c

Patch #1 modified the struct cros_ec_command structure so it can be used
as an ioctl argument and be 64 and 32 bit safe and patch #2 adds fields
to the struct cros_ec_device that will be needed by the EC chardev driver.

Patch #3 adds support for the EC LPC interface used on x86 Chromebooks.

Patch #4 adds the ChromeOS chardev driver and patch #5 instantiates it
from the mfd cros_ec driver.

Patch #6 exposes sysfs attributes that can be used by user space programs
to get information and control the ChromeOS EC.

Patch #7 exposes sysfs attributes that are used to control the lightbar
RGB LEDs found on the Pixel Chromebook.

The patches must be applied together and in order due dependencies.
Lee Jones has already acked the MFD changes so I think this could go
through Olof's chrome-platform tree.

Best regards,
Javier

[0]: git://git.collabora.co.uk/git/user/javier/ec.git mainline-ioctl


2015-02-02 11:26:59

by Javier Martinez Canillas

[permalink] [raw]
Subject: [PATCH v5 1/7] mfd: cros_ec: Use fixed size arrays to transfer data with the EC

The struct cros_ec_command will be used as an ioctl() argument for the
API to control the ChromeOS EC from user-space. So the data structure
has to be 64-bit safe to make it compatible between 32 and 64 avoiding
the need for a compat ioctl interface. Since pointers are self-aligned
to different byte boundaries, use fixed size arrays instead of pointers
for transferring ingoing and outgoing data with the Embedded Controller.

Also, re-arrange struct members by decreasing alignment requirements to
reduce the needing padding size.

Signed-off-by: Javier Martinez Canillas <[email protected]>
Acked-by: Lee Jones <[email protected]>
---

Hello,

I choose EC_PROTO2_MAX_PARAM_SIZE as the maximum length for the input and
output buffers since I see that is what is assumed in the cros_ec driver
that is the maximum lengths. But the downstream kernel has also suppport
for the EC host command protocol v3 even though there is currently no bus
specific code to handle v3 packets. So I wonder if this is a good max len
or if a different size should be used instead.

Best regards,
Javier

Changes since v4: None.

Changes since v3: None.

Changes since v2: None, added Lee Jones Acked-by tag.

Changes since v1: None, new patch
---
drivers/i2c/busses/i2c-cros-ec-tunnel.c | 51 +++++++--------------------------
drivers/input/keyboard/cros_ec_keyb.c | 13 +++++----
drivers/mfd/cros_ec.c | 15 +++++-----
include/linux/mfd/cros_ec.h | 8 +++---
4 files changed, 29 insertions(+), 58 deletions(-)

diff --git a/drivers/i2c/busses/i2c-cros-ec-tunnel.c b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
index 875c22ae5400..fa8dedd8c3a2 100644
--- a/drivers/i2c/busses/i2c-cros-ec-tunnel.c
+++ b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
@@ -182,72 +182,41 @@ static int ec_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg i2c_msgs[],
const u16 bus_num = bus->remote_bus;
int request_len;
int response_len;
- u8 *request = NULL;
- u8 *response = NULL;
int result;
- struct cros_ec_command msg;
+ struct cros_ec_command msg = { };

request_len = ec_i2c_count_message(i2c_msgs, num);
if (request_len < 0) {
dev_warn(dev, "Error constructing message %d\n", request_len);
- result = request_len;
- goto exit;
+ return request_len;
}
+
response_len = ec_i2c_count_response(i2c_msgs, num);
if (response_len < 0) {
/* Unexpected; no errors should come when NULL response */
dev_warn(dev, "Error preparing response %d\n", response_len);
- result = response_len;
- goto exit;
- }
-
- if (request_len <= ARRAY_SIZE(bus->request_buf)) {
- request = bus->request_buf;
- } else {
- request = kzalloc(request_len, GFP_KERNEL);
- if (request == NULL) {
- result = -ENOMEM;
- goto exit;
- }
- }
- if (response_len <= ARRAY_SIZE(bus->response_buf)) {
- response = bus->response_buf;
- } else {
- response = kzalloc(response_len, GFP_KERNEL);
- if (response == NULL) {
- result = -ENOMEM;
- goto exit;
- }
+ return response_len;
}

- result = ec_i2c_construct_message(request, i2c_msgs, num, bus_num);
+ result = ec_i2c_construct_message(msg.outdata, i2c_msgs, num, bus_num);
if (result)
- goto exit;
+ return result;

msg.version = 0;
msg.command = EC_CMD_I2C_PASSTHRU;
- msg.outdata = request;
msg.outsize = request_len;
- msg.indata = response;
msg.insize = response_len;

result = cros_ec_cmd_xfer(bus->ec, &msg);
if (result < 0)
- goto exit;
+ return result;

- result = ec_i2c_parse_response(response, i2c_msgs, &num);
+ result = ec_i2c_parse_response(msg.indata, i2c_msgs, &num);
if (result < 0)
- goto exit;
+ return result;

/* Indicate success by saying how many messages were sent */
- result = num;
-exit:
- if (request != bus->request_buf)
- kfree(request);
- if (response != bus->response_buf)
- kfree(response);
-
- return result;
+ return num;
}

static u32 ec_i2c_functionality(struct i2c_adapter *adap)
diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
index ffa989f2c785..769f8f7f62b7 100644
--- a/drivers/input/keyboard/cros_ec_keyb.c
+++ b/drivers/input/keyboard/cros_ec_keyb.c
@@ -148,16 +148,19 @@ static void cros_ec_keyb_process(struct cros_ec_keyb *ckdev,

static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev, uint8_t *kb_state)
{
+ int ret;
struct cros_ec_command msg = {
- .version = 0,
.command = EC_CMD_MKBP_STATE,
- .outdata = NULL,
- .outsize = 0,
- .indata = kb_state,
.insize = ckdev->cols,
};

- return cros_ec_cmd_xfer(ckdev->ec, &msg);
+ ret = cros_ec_cmd_xfer(ckdev->ec, &msg);
+ if (ret < 0)
+ return ret;
+
+ memcpy(kb_state, msg.indata, ckdev->cols);
+
+ return 0;
}

static irqreturn_t cros_ec_keyb_irq(int irq, void *data)
diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
index fc0c81ef04ff..c872e1b0eaf8 100644
--- a/drivers/mfd/cros_ec.c
+++ b/drivers/mfd/cros_ec.c
@@ -74,15 +74,11 @@ int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
ret = ec_dev->cmd_xfer(ec_dev, msg);
if (msg->result == EC_RES_IN_PROGRESS) {
int i;
- struct cros_ec_command status_msg;
- struct ec_response_get_comms_status status;
+ struct cros_ec_command status_msg = { };
+ struct ec_response_get_comms_status *status;

- status_msg.version = 0;
status_msg.command = EC_CMD_GET_COMMS_STATUS;
- status_msg.outdata = NULL;
- status_msg.outsize = 0;
- status_msg.indata = (uint8_t *)&status;
- status_msg.insize = sizeof(status);
+ status_msg.insize = sizeof(*status);

/*
* Query the EC's status until it's no longer busy or
@@ -98,7 +94,10 @@ int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
msg->result = status_msg.result;
if (status_msg.result != EC_RES_SUCCESS)
break;
- if (!(status.flags & EC_COMMS_STATUS_PROCESSING))
+
+ status = (struct ec_response_get_comms_status *)
+ status_msg.indata;
+ if (!(status->flags & EC_COMMS_STATUS_PROCESSING))
break;
}
}
diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
index 0e166b92f5b4..71675b14b5ca 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -38,20 +38,20 @@ enum {
/*
* @version: Command version number (often 0)
* @command: Command to send (EC_CMD_...)
- * @outdata: Outgoing data to EC
* @outsize: Outgoing length in bytes
- * @indata: Where to put the incoming data from EC
* @insize: Max number of bytes to accept from EC
* @result: EC's response to the command (separate from communication failure)
+ * @outdata: Outgoing data to EC
+ * @indata: Where to put the incoming data from EC
*/
struct cros_ec_command {
uint32_t version;
uint32_t command;
- uint8_t *outdata;
uint32_t outsize;
- uint8_t *indata;
uint32_t insize;
uint32_t result;
+ uint8_t outdata[EC_PROTO2_MAX_PARAM_SIZE];
+ uint8_t indata[EC_PROTO2_MAX_PARAM_SIZE];
};

/**
--
2.1.3

2015-02-02 11:29:16

by Javier Martinez Canillas

[permalink] [raw]
Subject: [PATCH v5 2/7] mfd: cros_ec: Add char dev and virtual dev pointers

The ChromeOS Embedded Controller has to be accessed by applications.
A virtual character device is used as an interface with user-space.

Extend the struct cros_ec_device with the fields needed by the driver
of this virtual character device.

Signed-off-by: Javier Martinez Canillas <[email protected]>
Acked-by: Lee Jones <[email protected]>
---

Changes since v4: None.

Changes since v3: None.

Changes since v2: None, added Lee Jones Acked-by tag.

Changes since v1: None, new patch.
---
include/linux/mfd/cros_ec.h | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
index 71675b14b5ca..324a34683971 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -16,6 +16,7 @@
#ifndef __LINUX_MFD_CROS_EC_H
#define __LINUX_MFD_CROS_EC_H

+#include <linux/cdev.h>
#include <linux/notifier.h>
#include <linux/mfd/cros_ec_commands.h>
#include <linux/mutex.h>
@@ -59,9 +60,17 @@ 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
+ * @cdev: Character device structure for virtual comms device
* @was_wake_device: true if this device was set to wake the system from
* sleep at the last suspend
+ * @cmd_readmem: 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 +99,12 @@ struct cros_ec_device {
const char *ec_name;
const char *phys_name;
struct device *dev;
+ struct device *vdev;
+ struct cdev cdev;
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;
--
2.1.3

2015-02-02 11:28:21

by Javier Martinez Canillas

[permalink] [raw]
Subject: [PATCH v5 3/7] platform/chrome: Add cros_ec_lpc driver for x86 devices

From: Bill Richardson <[email protected]>

Chromebooks have an Embedded Controller (EC) that is used to
implement various functions such as keyboard, power and battery.

The AP can communicate with the EC through different bus types
such as I2C, SPI or LPC.

The cros_ec mfd driver is then composed of a core driver that
register the sub-devices as mfd cells and provide a high level
communication interface that is used by the rest of the kernel
and bus specific interfaces modules.

Each connection method then has its own driver, which register
with the EC driver interface-agnostic interface.

Currently, there are drivers to communicate with the EC over
I2C and SPI and this driver adds support for LPC.

Signed-off-by: Bill Richardson <[email protected]>
Signed-off-by: Javier Martinez Canillas <[email protected]>
---

Changes since v4:
- Move from drivers/misc to drivers/platform/chrome/ since is a better fit.
- Remove space in Kconfig text. Suggested by Paul Bolle.
- Expand Kconfig help to explain that the driver can be
built as a module.
- Add an dmi MODULE_DEVICE_TABLE() to have modinfo aliasses for auto-loading.
- Use dmi_check_system() on module_init() to check the system DMI info.

Changes since v3:
- Rename MYNAME to DRV_NAME
- Use devm_request_region() instead of request_region.
Suggested by Varka Bhadram.
- Remove release_region from cleanup handle logic by using devres API.
Suggested by Varka Bhadram.
- Use {dev,pr}_err() instead of {dev,pr}_warn() to log errors.

Changes since v2:
- Move out from drivers/mfd to drivers/misc. Suggested by Lee Jones.

Changes since v1: None, new patch.
---
drivers/platform/chrome/Kconfig | 12 ++
drivers/platform/chrome/Makefile | 1 +
drivers/platform/chrome/cros_ec_lpc.c | 319 ++++++++++++++++++++++++++++++++++
3 files changed, 332 insertions(+)
create mode 100644 drivers/platform/chrome/cros_ec_lpc.c

diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
index 440ed776efd4..6c5f5a1e1175 100644
--- a/drivers/platform/chrome/Kconfig
+++ b/drivers/platform/chrome/Kconfig
@@ -39,4 +39,16 @@ config CHROMEOS_PSTORE
The module will be called chromeos_pstore.


+config CROS_EC_LPC
+ tristate "ChromeOS Embedded Controller (LPC)"
+ depends on MFD_CROS_EC
+ 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
+ checksum. This is used for userspace access only. The kernel
+ typically has its own communication methods.
+
+ To compile this driver as a module, choose M here: the
+ module will be called cros_ec_lpc.
+
endif # CHROMEOS_PLATFORMS
diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
index 2b860ca7450f..03c0260369d9 100644
--- a/drivers/platform/chrome/Makefile
+++ b/drivers/platform/chrome/Makefile
@@ -1,3 +1,4 @@

obj-$(CONFIG_CHROMEOS_LAPTOP) += chromeos_laptop.o
obj-$(CONFIG_CHROMEOS_PSTORE) += chromeos_pstore.o
+obj-$(CONFIG_CROS_EC_LPC) += cros_ec_lpc.o
diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
new file mode 100644
index 000000000000..822fdb36ded9
--- /dev/null
+++ b/drivers/platform/chrome/cros_ec_lpc.c
@@ -0,0 +1,319 @@
+/*
+ * cros_ec_lpc - LPC access to the Chrome OS Embedded Controller
+ *
+ * Copyright (C) 2012-2015 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/dmi.h>
+#include <linux/delay.h>
+#include <linux/mfd/cros_ec.h>
+#include <linux/mfd/cros_ec_commands.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/printk.h>
+
+#define DRV_NAME "cros_ec_lpc"
+
+static int ec_response_timed_out(void)
+{
+ unsigned long one_second = jiffies + HZ;
+
+ usleep_range(200, 300);
+ do {
+ if (!(inb(EC_LPC_ADDR_HOST_CMD) & EC_LPC_STATUS_BUSY_MASK))
+ return 0;
+ usleep_range(100, 200);
+ } while (time_before(jiffies, one_second));
+
+ return 1;
+}
+
+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;
+ int ret = 0;
+
+ if (msg->outsize > EC_PROTO2_MAX_PARAM_SIZE ||
+ msg->insize > EC_PROTO2_MAX_PARAM_SIZE) {
+ dev_err(ec->dev,
+ "invalid buffer sizes (out %d, in %d)\n",
+ msg->outsize, msg->insize);
+ return -EINVAL;
+ }
+
+ /* Now actually send the command to the EC and get the result */
+ args.flags = EC_HOST_ARGS_FLAG_FROM_HOST;
+ args.command_version = msg->version;
+ args.data_size = msg->outsize;
+
+ /* Initialize checksum */
+ csum = msg->command + args.flags +
+ args.command_version + args.data_size;
+
+ /* Copy data and update checksum */
+ for (i = 0; i < msg->outsize; i++) {
+ outb(msg->outdata[i], EC_LPC_ADDR_HOST_PARAM + i);
+ csum += msg->outdata[i];
+ }
+
+ /* 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);
+
+ /* Here we go */
+ outb(msg->command, EC_LPC_ADDR_HOST_CMD);
+
+ if (ec_response_timed_out()) {
+ dev_warn(ec->dev, "EC responsed timed out\n");
+ ret = -EIO;
+ goto done;
+ }
+
+ /* Check result */
+ msg->result = inb(EC_LPC_ADDR_HOST_DATA);
+
+ switch (msg->result) {
+ case EC_RES_SUCCESS:
+ break;
+ case EC_RES_IN_PROGRESS:
+ ret = -EAGAIN;
+ dev_dbg(ec->dev, "command 0x%02x in progress\n",
+ msg->command);
+ goto done;
+ default:
+ dev_dbg(ec->dev, "command 0x%02x returned %d\n",
+ msg->command, msg->result);
+ }
+
+ /* 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);
+
+ if (args.data_size > msg->insize) {
+ dev_err(ec->dev,
+ "packet too long (%d bytes, expected %d)",
+ args.data_size, msg->insize);
+ ret = -ENOSPC;
+ goto done;
+ }
+
+ /* Start calculating response checksum */
+ csum = msg->command + args.flags +
+ args.command_version + args.data_size;
+
+ /* Read response and update checksum */
+ for (i = 0; i < args.data_size; i++) {
+ msg->indata[i] = inb(EC_LPC_ADDR_HOST_PARAM + i);
+ csum += msg->indata[i];
+ }
+
+ /* Verify checksum */
+ if (args.checksum != (csum & 0xFF)) {
+ dev_err(ec->dev,
+ "bad packet checksum, expected %02x, got %02x\n",
+ args.checksum, csum & 0xFF);
+ ret = -EBADMSG;
+ goto done;
+ }
+
+ /* Return actual amount of data received */
+ ret = args.data_size;
+done:
+ return ret;
+}
+
+/* Returns num bytes read, or negative on error. Doesn't need locking. */
+static int cros_ec_lpc_readmem(struct cros_ec_device *ec, unsigned int offset,
+ unsigned int bytes, void *dest)
+{
+ int i = offset;
+ char *s = dest;
+ int cnt = 0;
+
+ if (offset >= EC_MEMMAP_SIZE - bytes)
+ return -EINVAL;
+
+ /* fixed length */
+ if (bytes) {
+ for (; cnt < bytes; i++, s++, cnt++)
+ *s = inb(EC_LPC_ADDR_MEMMAP + i);
+ return cnt;
+ }
+
+ /* string */
+ for (; i < EC_MEMMAP_SIZE; i++, s++) {
+ *s = inb(EC_LPC_ADDR_MEMMAP + i);
+ cnt++;
+ if (!*s)
+ break;
+ }
+
+ return cnt;
+}
+
+static int cros_ec_lpc_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct cros_ec_device *ec_dev;
+ int ret;
+
+ if (!devm_request_region(dev, EC_LPC_ADDR_MEMMAP, EC_MEMMAP_SIZE,
+ dev_name(dev))) {
+ dev_err(dev, "couldn't reserve memmap region\n");
+ return -EBUSY;
+ }
+
+ if ((inb(EC_LPC_ADDR_MEMMAP + EC_MEMMAP_ID) != 'E') ||
+ (inb(EC_LPC_ADDR_MEMMAP + EC_MEMMAP_ID + 1) != 'C')) {
+ dev_err(dev, "EC ID not detected\n");
+ return -ENODEV;
+ }
+
+ if (!devm_request_region(dev, EC_HOST_CMD_REGION0,
+ EC_HOST_CMD_REGION_SIZE, dev_name(dev))) {
+ dev_err(dev, "couldn't reserve region0\n");
+ return -EBUSY;
+ }
+ if (!devm_request_region(dev, EC_HOST_CMD_REGION1,
+ EC_HOST_CMD_REGION_SIZE, dev_name(dev))) {
+ dev_err(dev, "couldn't reserve region1\n");
+ return -EBUSY;
+ }
+
+ ec_dev = devm_kzalloc(dev, sizeof(*ec_dev), GFP_KERNEL);
+ if (!ec_dev)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, ec_dev);
+ ec_dev->dev = dev;
+ ec_dev->ec_name = pdev->name;
+ ec_dev->phys_name = dev_name(dev);
+ ec_dev->parent = dev;
+ ec_dev->cmd_xfer = cros_ec_cmd_xfer_lpc;
+ ec_dev->cmd_readmem = cros_ec_lpc_readmem;
+
+ ret = cros_ec_register(ec_dev);
+ if (ret) {
+ dev_err(dev, "couldn't register ec_dev (%d)\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int cros_ec_lpc_remove(struct platform_device *pdev)
+{
+ struct cros_ec_device *ec_dev;
+
+ ec_dev = platform_get_drvdata(pdev);
+ cros_ec_remove(ec_dev);
+
+ return 0;
+}
+
+static struct dmi_system_id cros_ec_lpc_dmi_table[] __initdata = {
+ {
+ /*
+ * Today all Chromebooks/boxes ship with Google_* as version and
+ * coreboot as bios vendor. No other systems with this
+ * combination are known to date.
+ */
+ .matches = {
+ DMI_MATCH(DMI_BIOS_VENDOR, "coreboot"),
+ DMI_MATCH(DMI_BIOS_VERSION, "Google_"),
+ },
+ },
+ {
+ /* x86-link, the Chromebook Pixel. */
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "GOOGLE"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "Link"),
+ },
+ },
+ {
+ /* x86-peppy, the Acer C720 Chromebook. */
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Acer"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "Peppy"),
+ },
+ },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(dmi, cros_ec_lpc_dmi_table);
+
+static struct platform_driver cros_ec_lpc_driver = {
+ .driver = {
+ .name = DRV_NAME,
+ .owner = THIS_MODULE,
+ },
+ .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;
+
+ if (!dmi_check_system(cros_ec_lpc_dmi_table)) {
+ pr_err(DRV_NAME ": unsupported system.\n");
+ return -ENODEV;
+ }
+
+ /* Register the driver */
+ ret = platform_driver_register(&cros_ec_lpc_driver);
+ if (ret) {
+ pr_err(DRV_NAME ": can't register driver: %d\n", ret);
+ 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);
+ 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);
+}
+
+module_init(cros_ec_lpc_init);
+module_exit(cros_ec_lpc_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("ChromeOS EC LPC driver");
--
2.1.3

2015-02-02 11:28:19

by Javier Martinez Canillas

[permalink] [raw]
Subject: [PATCH v5 4/7] platform/chrome: Add Chrome OS EC userspace device interface

From: Bill Richardson <[email protected]>

This patch adds a device interface to access the
Chrome OS Embedded Controller from user-space.

Signed-off-by: Bill Richardson <[email protected]>
Reviewed-by: Simon Glass <[email protected]>
Signed-off-by: Javier Martinez Canillas <[email protected]>
Reviewed-by: Gwendal Grignou <[email protected]>
---

Changes since v4: None.

Changes since v3: None.

Changes since v2:
- Rename the devname to "cros-ec-ctl". Suggested by Lee Jones.
- Added Gwendal Grignou Reviewed-by tag.

Changes since v1: None, new patch.
---
Documentation/ioctl/ioctl-number.txt | 1 +
drivers/platform/chrome/Kconfig | 14 +-
drivers/platform/chrome/Makefile | 1 +
drivers/platform/chrome/cros_ec_dev.c | 268 ++++++++++++++++++++++++++++++++++
drivers/platform/chrome/cros_ec_dev.h | 47 ++++++
5 files changed, 328 insertions(+), 3 deletions(-)
create mode 100644 drivers/platform/chrome/cros_ec_dev.c
create mode 100644 drivers/platform/chrome/cros_ec_dev.h

diff --git a/Documentation/ioctl/ioctl-number.txt b/Documentation/ioctl/ioctl-number.txt
index 8136e1fd30fd..51f4221657bf 100644
--- a/Documentation/ioctl/ioctl-number.txt
+++ b/Documentation/ioctl/ioctl-number.txt
@@ -321,6 +321,7 @@ Code Seq#(hex) Include File Comments
0xDB 00-0F drivers/char/mwave/mwavepub.h
0xDD 00-3F ZFCP device driver see drivers/s390/scsi/
<mailto:[email protected]>
+0xEC 00-01 drivers/platform/chrome/cros_ec_dev.h ChromeOS EC driver
0xF3 00-3F drivers/usb/misc/sisusbvga/sisusb.h sisfb (in development)
<mailto:[email protected]>
0xF4 00-1F video/mbxfb.h mbxfb
diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
index 6c5f5a1e1175..d4befbffae85 100644
--- a/drivers/platform/chrome/Kconfig
+++ b/drivers/platform/chrome/Kconfig
@@ -4,7 +4,7 @@

menuconfig CHROME_PLATFORMS
bool "Platform support for Chrome hardware"
- depends on X86
+ depends on X86 || ARM
---help---
Say Y here to get to see options for platform support for
various Chromebooks and Chromeboxes. This option alone does
@@ -16,8 +16,7 @@ if CHROME_PLATFORMS

config CHROMEOS_LAPTOP
tristate "Chrome OS Laptop"
- depends on I2C
- depends on DMI
+ depends on I2C && DMI && X86
---help---
This driver instantiates i2c and smbus devices such as
light sensors and touchpads.
@@ -27,6 +26,7 @@ config CHROMEOS_LAPTOP

config CHROMEOS_PSTORE
tristate "Chrome OS pstore support"
+ depends on X86
---help---
This module instantiates the persistent storage on x86 ChromeOS
devices. It can be used to store away console logs and crash
@@ -38,6 +38,14 @@ config CHROMEOS_PSTORE
If you have a supported Chromebook, choose Y or M here.
The module will be called chromeos_pstore.

+config CROS_EC_CHARDEV
+ tristate "Chrome OS Embedded Controller userspace device interface"
+ depends on MFD_CROS_EC
+ ---help---
+ This driver adds support to talk with the ChromeOS EC from userspace.
+
+ If you have a supported Chromebook, choose Y or M here.
+ The module will be called cros_ec_dev.

config CROS_EC_LPC
tristate "ChromeOS Embedded Controller (LPC)"
diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
index 03c0260369d9..ec682900f889 100644
--- a/drivers/platform/chrome/Makefile
+++ b/drivers/platform/chrome/Makefile
@@ -1,4 +1,5 @@

obj-$(CONFIG_CHROMEOS_LAPTOP) += chromeos_laptop.o
obj-$(CONFIG_CHROMEOS_PSTORE) += chromeos_pstore.o
+obj-$(CONFIG_CROS_EC_CHARDEV) += cros_ec_dev.o
obj-$(CONFIG_CROS_EC_LPC) += cros_ec_lpc.o
diff --git a/drivers/platform/chrome/cros_ec_dev.c b/drivers/platform/chrome/cros_ec_dev.c
new file mode 100644
index 000000000000..94c1442d5104
--- /dev/null
+++ b/drivers/platform/chrome/cros_ec_dev.c
@@ -0,0 +1,268 @@
+/*
+ * cros_ec_dev - expose the Chrome OS Embedded Controller to user-space
+ *
+ * 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/>.
+ */
+
+#include <linux/fs.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/uaccess.h>
+
+#include "cros_ec_dev.h"
+
+/* Device variables */
+#define CROS_MAX_DEV 128
+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 = { 0 },
+ .outsize = 0,
+ .indata = { 0 },
+ .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;
+ }
+
+ resp = (struct ec_response_get_version *)msg.indata;
+ 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)
+{
+ filp->private_data = container_of(inode->i_cdev,
+ struct cros_ec_device, cdev);
+ 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)
+{
+ struct cros_ec_device *ec = filp->private_data;
+ 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(struct cros_ec_device *ec, void __user *arg)
+{
+ long ret;
+ struct cros_ec_command s_cmd = { };
+
+ if (copy_from_user(&s_cmd, arg, sizeof(s_cmd)))
+ return -EFAULT;
+
+ ret = cros_ec_cmd_xfer(ec, &s_cmd);
+ /* Only copy data to userland if data was received. */
+ if (ret < 0)
+ return ret;
+
+ if (copy_to_user(arg, &s_cmd, sizeof(s_cmd)))
+ return -EFAULT;
+
+ return 0;
+}
+
+static long ec_device_ioctl_readmem(struct cros_ec_device *ec, void __user *arg)
+{
+ struct cros_ec_readmem s_mem = { };
+ long num;
+
+ /* Not every platform supports direct reads */
+ if (!ec->cmd_readmem)
+ return -ENOTTY;
+
+ if (copy_from_user(&s_mem, arg, sizeof(s_mem)))
+ return -EFAULT;
+
+ num = ec->cmd_readmem(ec, s_mem.offset, s_mem.bytes, s_mem.buffer);
+ if (num <= 0)
+ return num;
+
+ if (copy_to_user((void __user *)arg, &s_mem, sizeof(s_mem)))
+ return -EFAULT;
+
+ return 0;
+}
+
+static long ec_device_ioctl(struct file *filp, unsigned int cmd,
+ unsigned long arg)
+{
+ struct cros_ec_device *ec = filp->private_data;
+
+ if (_IOC_TYPE(cmd) != CROS_EC_DEV_IOC)
+ return -ENOTTY;
+
+ switch (cmd) {
+ case CROS_EC_DEV_IOCXCMD:
+ return ec_device_ioctl_xcmd(ec, (void __user *)arg);
+ case CROS_EC_DEV_IOCRDMEM:
+ return ec_device_ioctl_readmem(ec, (void __user *)arg);
+ }
+
+ return -ENOTTY;
+}
+
+/* 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,
+};
+
+static int ec_device_probe(struct platform_device *pdev)
+{
+ struct cros_ec_device *ec = dev_get_drvdata(pdev->dev.parent);
+ int retval = -ENOTTY;
+ dev_t devno = MKDEV(ec_major, 0);
+
+ /* Instantiate it (and remember the EC) */
+ cdev_init(&ec->cdev, &fops);
+
+ retval = cdev_add(&ec->cdev, devno, 1);
+ if (retval) {
+ dev_err(&pdev->dev, ": failed to add character device\n");
+ return retval;
+ }
+
+ ec->vdev = device_create(cros_class, NULL, devno, ec,
+ CROS_EC_DEV_NAME);
+ if (IS_ERR(ec->vdev)) {
+ retval = PTR_ERR(ec->vdev);
+ dev_err(&pdev->dev, ": failed to create device\n");
+ cdev_del(&ec->cdev);
+ return retval;
+ }
+
+ return 0;
+}
+
+static int ec_device_remove(struct platform_device *pdev)
+{
+ struct cros_ec_device *ec = dev_get_drvdata(pdev->dev.parent);
+
+ device_destroy(cros_class, MKDEV(ec_major, 0));
+ cdev_del(&ec->cdev);
+ return 0;
+}
+
+static struct platform_driver cros_ec_dev_driver = {
+ .driver = {
+ .name = "cros-ec-ctl",
+ },
+ .probe = ec_device_probe,
+ .remove = ec_device_remove,
+};
+
+static int __init cros_ec_dev_init(void)
+{
+ int ret;
+ dev_t dev = 0;
+
+ cros_class = class_create(THIS_MODULE, "chromeos");
+ if (IS_ERR(cros_class)) {
+ pr_err(CROS_EC_DEV_NAME ": failed to register device class\n");
+ return PTR_ERR(cros_class);
+ }
+
+ /* Get a range of minor numbers (starting with 0) to work with */
+ ret = alloc_chrdev_region(&dev, 0, CROS_MAX_DEV, CROS_EC_DEV_NAME);
+ if (ret < 0) {
+ pr_err(CROS_EC_DEV_NAME ": alloc_chrdev_region() failed\n");
+ goto failed_chrdevreg;
+ }
+ ec_major = MAJOR(dev);
+
+ /* Register the driver */
+ ret = platform_driver_register(&cros_ec_dev_driver);
+ if (ret < 0) {
+ pr_warn(CROS_EC_DEV_NAME ": can't register driver: %d\n", ret);
+ goto failed_devreg;
+ }
+ return 0;
+
+failed_devreg:
+ unregister_chrdev_region(MKDEV(ec_major, 0), CROS_MAX_DEV);
+failed_chrdevreg:
+ class_destroy(cros_class);
+ return ret;
+}
+
+static void __exit cros_ec_dev_exit(void)
+{
+ platform_driver_unregister(&cros_ec_dev_driver);
+ unregister_chrdev(ec_major, CROS_EC_DEV_NAME);
+ class_destroy(cros_class);
+}
+
+module_init(cros_ec_dev_init);
+module_exit(cros_ec_dev_exit);
+
+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/drivers/platform/chrome/cros_ec_dev.h b/drivers/platform/chrome/cros_ec_dev.h
new file mode 100644
index 000000000000..15c54c4c5531
--- /dev/null
+++ b/drivers/platform/chrome/cros_ec_dev.h
@@ -0,0 +1,47 @@
+/*
+ * 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/>.
+ */
+
+#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;
+ uint8_t buffer[EC_MEMMAP_SIZE];
+};
+
+#define CROS_EC_DEV_IOC 0xEC
+#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)
+
+#endif /* _CROS_EC_DEV_H_ */
--
2.1.3

2015-02-02 11:27:07

by Javier Martinez Canillas

[permalink] [raw]
Subject: [PATCH v5 5/7] mfd: cros_ec: Instantiate ChromeOS EC character device

The ChromeOS EC character device is an user-space interface to
allow applications to access the Embedded Controller.

Add a cell for this device so it's spawned from the mfd driver.

Signed-off-by: Javier Martinez Canillas <[email protected]>
Acked-by: Lee Jones <[email protected]>
---

Changes since v4: None.

Changes since v3: None, added Acked-by tag from Lee Jones.

Changes since v2:
- Rename the name to "cros-ec-ctl". Suggested by Lee Jones.

Changes since v1: None, new patch.
---
drivers/mfd/cros_ec.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
index c872e1b0eaf8..c4aecc6f8373 100644
--- a/drivers/mfd/cros_ec.c
+++ b/drivers/mfd/cros_ec.c
@@ -118,6 +118,10 @@ static const struct mfd_cell cros_devs[] = {
.id = 2,
.of_compatible = "google,cros-ec-i2c-tunnel",
},
+ {
+ .name = "cros-ec-ctl",
+ .id = 3,
+ },
};

int cros_ec_register(struct cros_ec_device *ec_dev)
--
2.1.3

2015-02-02 11:27:10

by Javier Martinez Canillas

[permalink] [raw]
Subject: [PATCH v5 6/7] platform/chrome: Create sysfs attributes for the ChromeOS EC

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]>
---

Changes since v4: None.

Changes since v3: None.

Changes since v2: None.

Changes since v1:
- Moved from drivers/mfd to drivers/platform/chrome. Suggested by Lee Jones.
- Modify to use the fixed-size arrays for cros_ec_command in and out buffers.
---
drivers/platform/chrome/Makefile | 3 +-
drivers/platform/chrome/cros_ec_dev.c | 4 +
drivers/platform/chrome/cros_ec_dev.h | 3 +
drivers/platform/chrome/cros_ec_sysfs.c | 271 ++++++++++++++++++++++++++++++++
4 files changed, 280 insertions(+), 1 deletion(-)
create mode 100644 drivers/platform/chrome/cros_ec_sysfs.c

diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
index ec682900f889..34b631ceff67 100644
--- a/drivers/platform/chrome/Makefile
+++ b/drivers/platform/chrome/Makefile
@@ -1,5 +1,6 @@

obj-$(CONFIG_CHROMEOS_LAPTOP) += chromeos_laptop.o
obj-$(CONFIG_CHROMEOS_PSTORE) += chromeos_pstore.o
-obj-$(CONFIG_CROS_EC_CHARDEV) += cros_ec_dev.o
+cros_ec_devs-objs := cros_ec_dev.o cros_ec_sysfs.o
+obj-$(CONFIG_CROS_EC_CHARDEV) += cros_ec_devs.o
obj-$(CONFIG_CROS_EC_LPC) += cros_ec_lpc.o
diff --git a/drivers/platform/chrome/cros_ec_dev.c b/drivers/platform/chrome/cros_ec_dev.c
index 94c1442d5104..33f37ad36892 100644
--- a/drivers/platform/chrome/cros_ec_dev.c
+++ b/drivers/platform/chrome/cros_ec_dev.c
@@ -198,6 +198,9 @@ static int ec_device_probe(struct platform_device *pdev)
return retval;
}

+ /* Initialize extra interfaces */
+ ec_dev_sysfs_init(ec);
+
return 0;
}

@@ -205,6 +208,7 @@ static int ec_device_remove(struct platform_device *pdev)
{
struct cros_ec_device *ec = dev_get_drvdata(pdev->dev.parent);

+ ec_dev_sysfs_remove(ec);
device_destroy(cros_class, MKDEV(ec_major, 0));
cdev_del(&ec->cdev);
return 0;
diff --git a/drivers/platform/chrome/cros_ec_dev.h b/drivers/platform/chrome/cros_ec_dev.h
index 15c54c4c5531..f03613290ecf 100644
--- a/drivers/platform/chrome/cros_ec_dev.h
+++ b/drivers/platform/chrome/cros_ec_dev.h
@@ -44,4 +44,7 @@ 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)

+void ec_dev_sysfs_init(struct cros_ec_device *);
+void ec_dev_sysfs_remove(struct cros_ec_device *);
+
#endif /* _CROS_EC_DEV_H_ */
diff --git a/drivers/platform/chrome/cros_ec_sysfs.c b/drivers/platform/chrome/cros_ec_sysfs.c
new file mode 100644
index 000000000000..fb62ab6cc659
--- /dev/null
+++ b/drivers/platform/chrome/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/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 cros_ec_command msg = { 0 };
+ struct ec_params_reboot_ec *param =
+ (struct ec_params_reboot_ec *)msg.outdata;
+ 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.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 };
+ 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.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);
+
+ r_ver = (struct ec_response_get_version *)msg.indata;
+ /* 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.insize = sizeof(msg.indata);
+ 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 {
+ msg.indata[sizeof(msg.indata) - 1] = '\0';
+ count += scnprintf(buf + count, PAGE_SIZE - count,
+ "Build info: %s\n", msg.indata);
+ }
+
+ /* Get chip info. */
+ msg.command = EC_CMD_GET_CHIP_INFO;
+ 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 = (struct ec_response_get_chip_info *)msg.indata;
+
+ 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.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 {
+ r_board = (struct ec_response_board_version *)msg.indata;
+
+ 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.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);
+
+ resp = (struct ec_response_flash_info *)msg.indata;
+
+ 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.3

2015-02-02 11:27:41

by Javier Martinez Canillas

[permalink] [raw]
Subject: [PATCH v5 7/7] platform/chrome: Expose Chrome OS Lightbar to users

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]>
---

Changes since v4: None.

Changes since v3: None.

Changes since v2: None.

Changes since v1:
- Moved from drivers/mfd to drivers/platform/chrome.
- Modify to use the fixed-size arrays for cros_ec_command in and out buffers.
---
drivers/platform/chrome/Makefile | 2 +-
drivers/platform/chrome/cros_ec_dev.c | 2 +
drivers/platform/chrome/cros_ec_dev.h | 3 +
drivers/platform/chrome/cros_ec_lightbar.c | 367 +++++++++++++++++++++++++++++
4 files changed, 373 insertions(+), 1 deletion(-)
create mode 100644 drivers/platform/chrome/cros_ec_lightbar.c

diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
index 34b631ceff67..bd8d8601e875 100644
--- a/drivers/platform/chrome/Makefile
+++ b/drivers/platform/chrome/Makefile
@@ -1,6 +1,6 @@

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_devs-objs := cros_ec_dev.o cros_ec_sysfs.o cros_ec_lightbar.o
obj-$(CONFIG_CROS_EC_CHARDEV) += cros_ec_devs.o
obj-$(CONFIG_CROS_EC_LPC) += cros_ec_lpc.o
diff --git a/drivers/platform/chrome/cros_ec_dev.c b/drivers/platform/chrome/cros_ec_dev.c
index 33f37ad36892..ce714317a59e 100644
--- a/drivers/platform/chrome/cros_ec_dev.c
+++ b/drivers/platform/chrome/cros_ec_dev.c
@@ -200,6 +200,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;
}
@@ -208,6 +209,7 @@ static int ec_device_remove(struct platform_device *pdev)
{
struct cros_ec_device *ec = dev_get_drvdata(pdev->dev.parent);

+ ec_dev_lightbar_remove(ec);
ec_dev_sysfs_remove(ec);
device_destroy(cros_class, MKDEV(ec_major, 0));
cdev_del(&ec->cdev);
diff --git a/drivers/platform/chrome/cros_ec_dev.h b/drivers/platform/chrome/cros_ec_dev.h
index f03613290ecf..45d67f7e518c 100644
--- a/drivers/platform/chrome/cros_ec_dev.h
+++ b/drivers/platform/chrome/cros_ec_dev.h
@@ -47,4 +47,7 @@ struct cros_ec_readmem {
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 /* _CROS_EC_DEV_H_ */
diff --git a/drivers/platform/chrome/cros_ec_lightbar.c b/drivers/platform/chrome/cros_ec_lightbar.c
new file mode 100644
index 000000000000..35fc892e4c95
--- /dev/null
+++ b/drivers/platform/chrome/cros_ec_lightbar.c
@@ -0,0 +1,367 @@
+/*
+ * 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/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, \
+ .outsize = sizeof(*P), \
+ .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 = (struct ec_params_lightbar *)msg.outdata;
+ 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:
+ resp = (struct ec_response_lightbar *)msg.indata;
+
+ /* 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 = (struct ec_params_lightbar *)msg.outdata;
+ 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;
+
+ ret = sscanf(buf, "%i", &val[i++]);
+ if (ret == 0)
+ return -EINVAL;
+
+ if (i == 4) {
+ param = (struct ec_params_lightbar *)msg.outdata;
+ 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 = (struct ec_params_lightbar *)msg.outdata;
+ 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);
+
+ resp = (struct ec_response_lightbar *)msg.indata;
+ 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)) {
+ ret = kstrtouint(buf, 0, &num);
+ if (ret)
+ return ret;
+ }
+
+ param = (struct ec_params_lightbar *)msg.outdata;
+ 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.3

2015-02-16 08:19:56

by Javier Martinez Canillas

[permalink] [raw]
Subject: Re: [PATCH v5 0/7] platform/chrome: Add user-space dev inferface support

Hello Olof,

On 02/02/2015 12:26 PM, Javier Martinez Canillas wrote:
> Hello,
>
> The mainline ChromeOS Embedded Controller (EC) driver is still missing some
> features that are present in the downstream ChromiumOS tree. These are:
>
> - Low Pin Count (LPC) interface
> - User-space device 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 is a fifth version of a series that adds support for the first two of
> the missing features: the EC LPC and EC character device interfaces that
> are used by user-space to access the ChromeOS EC. The support patches were
> taken from the downstream ChromiumOS 3.14 tree with the fixes and cleanups
> squashed to have a minimal patch-set.
>

Any comments on this series? The last version was posted a couple of weeks
ago but the series have been in the list for months. Lee has already acked
the mfd changes so you can merge all through your chrome-platform tree if
you want.

It wold be great if this series get in to have the EC user-space interface
supported and to minimize the delta with the Chromemium OS kernel since it
still has other features that needs to be upstreamed like multiple EC in a
system and access to vboot context stored in block device or EC's nvram.

Best regards,
Javier

2015-02-18 02:26:53

by Simon Glass

[permalink] [raw]
Subject: Re: [PATCH v5 0/7] platform/chrome: Add user-space dev inferface support

Hi,

On 16 February 2015 at 01:19, Javier Martinez Canillas
<[email protected]> wrote:
> Hello Olof,
>
> On 02/02/2015 12:26 PM, Javier Martinez Canillas wrote:
>> Hello,
>>
>> The mainline ChromeOS Embedded Controller (EC) driver is still missing some
>> features that are present in the downstream ChromiumOS tree. These are:
>>
>> - Low Pin Count (LPC) interface
>> - User-space device 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 is a fifth version of a series that adds support for the first two of
>> the missing features: the EC LPC and EC character device interfaces that
>> are used by user-space to access the ChromeOS EC. The support patches were
>> taken from the downstream ChromiumOS 3.14 tree with the fixes and cleanups
>> squashed to have a minimal patch-set.
>>
>
> Any comments on this series? The last version was posted a couple of weeks
> ago but the series have been in the list for months. Lee has already acked
> the mfd changes so you can merge all through your chrome-platform tree if
> you want.
>
> It wold be great if this series get in to have the EC user-space interface
> supported and to minimize the delta with the Chromemium OS kernel since it
> still has other features that needs to be upstreamed like multiple EC in a
> system and access to vboot context stored in block device or EC's nvram.

Are you sure Olof is the right maintainer for this going to mainline?

I do feel for you trying to get all this in and have seen your many
attempts. It has been in U-Boot for 18 months...I hope you get there
in the end.

Regards,
Simon

2015-02-18 09:27:23

by Javier Martinez Canillas

[permalink] [raw]
Subject: Re: [PATCH v5 0/7] platform/chrome: Add user-space dev inferface support

Hello Simon,

On 02/18/2015 03:26 AM, Simon Glass wrote:
>
> On 16 February 2015 at 01:19, Javier Martinez Canillas
> <[email protected]> wrote:
>> Hello Olof,
>>
>> On 02/02/2015 12:26 PM, Javier Martinez Canillas wrote:
>>> Hello,
>>>
>>> The mainline ChromeOS Embedded Controller (EC) driver is still missing some
>>> features that are present in the downstream ChromiumOS tree. These are:
>>>
>>> - Low Pin Count (LPC) interface
>>> - User-space device 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 is a fifth version of a series that adds support for the first two of
>>> the missing features: the EC LPC and EC character device interfaces that
>>> are used by user-space to access the ChromeOS EC. The support patches were
>>> taken from the downstream ChromiumOS 3.14 tree with the fixes and cleanups
>>> squashed to have a minimal patch-set.
>>>
>>
>> Any comments on this series? The last version was posted a couple of weeks
>> ago but the series have been in the list for months. Lee has already acked
>> the mfd changes so you can merge all through your chrome-platform tree if
>> you want.
>>
>> It wold be great if this series get in to have the EC user-space interface
>> supported and to minimize the delta with the Chromemium OS kernel since it
>> still has other features that needs to be upstreamed like multiple EC in a
>> system and access to vboot context stored in block device or EC's nvram.
>
> Are you sure Olof is the right maintainer for this going to mainline?
>

The downstream ChromiumOS kernel has this stuff in drivers/mfd but Lee was
patient enough to explain to me why it does not fit there. So I think the
best place is drivers/platform/chrome/ which is maintained by Olof.

> I do feel for you trying to get all this in and have seen your many
> attempts. It has been in U-Boot for 18 months...I hope you get there
> in the end.
>

Thanks a lot for your empathy. I do hope to get there too since there are
still more features to match what is in the downstream tree but needs to
be based on this series.

> Regards,
> Simon
>

Best regards,
Javier

2015-02-26 00:54:23

by Olof Johansson

[permalink] [raw]
Subject: Re: [PATCH v5 4/7] platform/chrome: Add Chrome OS EC userspace device interface

On Mon, Feb 02, 2015 at 12:26:25PM +0100, Javier Martinez Canillas wrote:
> From: Bill Richardson <[email protected]>
>
> This patch adds a device interface to access the
> Chrome OS Embedded Controller from user-space.
>
> Signed-off-by: Bill Richardson <[email protected]>
> Reviewed-by: Simon Glass <[email protected]>
> Signed-off-by: Javier Martinez Canillas <[email protected]>
> Reviewed-by: Gwendal Grignou <[email protected]>
> ---
>
> Changes since v4: None.
>
> Changes since v3: None.
>
> Changes since v2:
> - Rename the devname to "cros-ec-ctl". Suggested by Lee Jones.
> - Added Gwendal Grignou Reviewed-by tag.
>
> Changes since v1: None, new patch.

Hi,

> ---
> Documentation/ioctl/ioctl-number.txt | 1 +
> drivers/platform/chrome/Kconfig | 14 +-
> drivers/platform/chrome/Makefile | 1 +
> drivers/platform/chrome/cros_ec_dev.c | 268 ++++++++++++++++++++++++++++++++++
> drivers/platform/chrome/cros_ec_dev.h | 47 ++++++
> 5 files changed, 328 insertions(+), 3 deletions(-)
> create mode 100644 drivers/platform/chrome/cros_ec_dev.c
> create mode 100644 drivers/platform/chrome/cros_ec_dev.h
>
> diff --git a/Documentation/ioctl/ioctl-number.txt b/Documentation/ioctl/ioctl-number.txt
> index 8136e1fd30fd..51f4221657bf 100644
> --- a/Documentation/ioctl/ioctl-number.txt
> +++ b/Documentation/ioctl/ioctl-number.txt
> @@ -321,6 +321,7 @@ Code Seq#(hex) Include File Comments
> 0xDB 00-0F drivers/char/mwave/mwavepub.h
> 0xDD 00-3F ZFCP device driver see drivers/s390/scsi/
> <mailto:[email protected]>
> +0xEC 00-01 drivers/platform/chrome/cros_ec_dev.h ChromeOS EC driver

It seems like a bad idea to reuse the same ioctl numbers as the out-of-tree
driver but changing the arguments. So please allocate a few more and use 2/3
for this calling interface.

[...]

> new file mode 100644
> index 000000000000..15c54c4c5531
> --- /dev/null
> +++ b/drivers/platform/chrome/cros_ec_dev.h
> @@ -0,0 +1,47 @@
> +/*
> + * 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/>.
> + */
> +
> +#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;
> + uint8_t buffer[EC_MEMMAP_SIZE];
> +};
> +
> +#define CROS_EC_DEV_IOC 0xEC
> +#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)

I.e. go with 2/3 here.

(I can just do that change when I apply this patch, let me know if you prefer
that).


-Olof
>

2015-02-26 00:57:39

by Olof Johansson

[permalink] [raw]
Subject: Re: [PATCH v5 0/7] platform/chrome: Add user-space dev inferface support

On Mon, Feb 16, 2015 at 09:19:49AM +0100, Javier Martinez Canillas wrote:
> Hello Olof,
>
> On 02/02/2015 12:26 PM, Javier Martinez Canillas wrote:
> > Hello,
> >
> > The mainline ChromeOS Embedded Controller (EC) driver is still missing some
> > features that are present in the downstream ChromiumOS tree. These are:
> >
> > - Low Pin Count (LPC) interface
> > - User-space device 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 is a fifth version of a series that adds support for the first two of
> > the missing features: the EC LPC and EC character device interfaces that
> > are used by user-space to access the ChromeOS EC. The support patches were
> > taken from the downstream ChromiumOS 3.14 tree with the fixes and cleanups
> > squashed to have a minimal patch-set.
> >
>
> Any comments on this series? The last version was posted a couple of weeks
> ago but the series have been in the list for months. Lee has already acked
> the mfd changes so you can merge all through your chrome-platform tree if
> you want.
>
> It wold be great if this series get in to have the EC user-space interface
> supported and to minimize the delta with the Chromemium OS kernel since it
> still has other features that needs to be upstreamed like multiple EC in a
> system and access to vboot context stored in block device or EC's nvram.

Yeah, sorry for dragging my feet on this. The only thing I found that should
should be revisited is the reuse of the ioctl numbers to make it easier to
transition on the Chrome OS side -- otherwise it'll be hard to know from
userspace to use old or new structs during transition without a flag day.


-Olof

2015-02-26 00:59:14

by Olof Johansson

[permalink] [raw]
Subject: Re: [PATCH v5 0/7] platform/chrome: Add user-space dev inferface support

On Tue, Feb 17, 2015 at 07:26:50PM -0700, Simon Glass wrote:
> Hi,
>
> On 16 February 2015 at 01:19, Javier Martinez Canillas
> <[email protected]> wrote:
> > Hello Olof,
> >
> > On 02/02/2015 12:26 PM, Javier Martinez Canillas wrote:
> >> Hello,
> >>
> >> The mainline ChromeOS Embedded Controller (EC) driver is still missing some
> >> features that are present in the downstream ChromiumOS tree. These are:
> >>
> >> - Low Pin Count (LPC) interface
> >> - User-space device 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 is a fifth version of a series that adds support for the first two of
> >> the missing features: the EC LPC and EC character device interfaces that
> >> are used by user-space to access the ChromeOS EC. The support patches were
> >> taken from the downstream ChromiumOS 3.14 tree with the fixes and cleanups
> >> squashed to have a minimal patch-set.
> >>
> >
> > Any comments on this series? The last version was posted a couple of weeks
> > ago but the series have been in the list for months. Lee has already acked
> > the mfd changes so you can merge all through your chrome-platform tree if
> > you want.
> >
> > It wold be great if this series get in to have the EC user-space interface
> > supported and to minimize the delta with the Chromemium OS kernel since it
> > still has other features that needs to be upstreamed like multiple EC in a
> > system and access to vboot context stored in block device or EC's nvram.
>
> Are you sure Olof is the right maintainer for this going to mainline?

Yes.

> I do feel for you trying to get all this in and have seen your many
> attempts. It has been in U-Boot for 18 months...I hope you get there
> in the end.

Cool, u-boot has userspace interfaces now? I didn't know that.


-Olof

2015-02-26 01:14:28

by Gwendal Grignou

[permalink] [raw]
Subject: Re: [PATCH v5 4/7] platform/chrome: Add Chrome OS EC userspace device interface

Olof,

I think the way Javier did it is fine, the 'major' of the ioctl is
0xEC, from ':'.

Gwendal.

On Wed, Feb 25, 2015 at 4:54 PM, Olof Johansson <[email protected]> wrote:
> On Mon, Feb 02, 2015 at 12:26:25PM +0100, Javier Martinez Canillas wrote:
>> From: Bill Richardson <[email protected]>
>>
>> This patch adds a device interface to access the
>> Chrome OS Embedded Controller from user-space.
>>
>> Signed-off-by: Bill Richardson <[email protected]>
>> Reviewed-by: Simon Glass <[email protected]>
>> Signed-off-by: Javier Martinez Canillas <[email protected]>
>> Reviewed-by: Gwendal Grignou <[email protected]>
>> ---
>>
>> Changes since v4: None.
>>
>> Changes since v3: None.
>>
>> Changes since v2:
>> - Rename the devname to "cros-ec-ctl". Suggested by Lee Jones.
>> - Added Gwendal Grignou Reviewed-by tag.
>>
>> Changes since v1: None, new patch.
>
> Hi,
>
>> ---
>> Documentation/ioctl/ioctl-number.txt | 1 +
>> drivers/platform/chrome/Kconfig | 14 +-
>> drivers/platform/chrome/Makefile | 1 +
>> drivers/platform/chrome/cros_ec_dev.c | 268 ++++++++++++++++++++++++++++++++++
>> drivers/platform/chrome/cros_ec_dev.h | 47 ++++++
>> 5 files changed, 328 insertions(+), 3 deletions(-)
>> create mode 100644 drivers/platform/chrome/cros_ec_dev.c
>> create mode 100644 drivers/platform/chrome/cros_ec_dev.h
>>
>> diff --git a/Documentation/ioctl/ioctl-number.txt b/Documentation/ioctl/ioctl-number.txt
>> index 8136e1fd30fd..51f4221657bf 100644
>> --- a/Documentation/ioctl/ioctl-number.txt
>> +++ b/Documentation/ioctl/ioctl-number.txt
>> @@ -321,6 +321,7 @@ Code Seq#(hex) Include File Comments
>> 0xDB 00-0F drivers/char/mwave/mwavepub.h
>> 0xDD 00-3F ZFCP device driver see drivers/s390/scsi/
>> <mailto:[email protected]>
>> +0xEC 00-01 drivers/platform/chrome/cros_ec_dev.h ChromeOS EC driver
>
> It seems like a bad idea to reuse the same ioctl numbers as the out-of-tree
> driver but changing the arguments. So please allocate a few more and use 2/3
> for this calling interface.
>
> [...]
>
>> new file mode 100644
>> index 000000000000..15c54c4c5531
>> --- /dev/null
>> +++ b/drivers/platform/chrome/cros_ec_dev.h
>> @@ -0,0 +1,47 @@
>> +/*
>> + * 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/>.
>> + */
>> +
>> +#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;
>> + uint8_t buffer[EC_MEMMAP_SIZE];
>> +};
>> +
>> +#define CROS_EC_DEV_IOC 0xEC
>> +#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)
>
> I.e. go with 2/3 here.
>
> (I can just do that change when I apply this patch, let me know if you prefer
> that).
>
>
> -Olof
>>

2015-02-26 09:08:43

by Javier Martinez Canillas

[permalink] [raw]
Subject: Re: [PATCH v5 4/7] platform/chrome: Add Chrome OS EC userspace device interface

Hello Olof,

Thanks a lot for your feedback.

On 02/26/2015 02:13 AM, Gwendal Grignou wrote:
> Olof,
>
> I think the way Javier did it is fine, the 'major' of the ioctl is
> 0xEC, from ':'.
>
> Gwendal.
>

As Gwendal said, I deliberately changed the IOCTL mayor number to
make it different in both kernels.

downstream:

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

mainline:

#define CROS_EC_DEV_IOC 0xEC
#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)

I can also do what you suggested and keep ':' as the major and use 2/3
as command numbers but I just think 0xEC is a much nicer major for the
interface to talk with the Embedded Controller and it was available ;)

Best regards,
Javier

2015-02-26 16:00:44

by Simon Glass

[permalink] [raw]
Subject: Re: [PATCH v5 0/7] platform/chrome: Add user-space dev inferface support

Hi Olof,

On 25 February 2015 at 17:59, Olof Johansson <[email protected]> wrote:
> On Tue, Feb 17, 2015 at 07:26:50PM -0700, Simon Glass wrote:
>> Hi,
>>
>> On 16 February 2015 at 01:19, Javier Martinez Canillas
>> <[email protected]> wrote:
>> > Hello Olof,
>> >
>> > On 02/02/2015 12:26 PM, Javier Martinez Canillas wrote:
>> >> Hello,
>> >>
>> >> The mainline ChromeOS Embedded Controller (EC) driver is still missing some
>> >> features that are present in the downstream ChromiumOS tree. These are:
>> >>
>> >> - Low Pin Count (LPC) interface
>> >> - User-space device 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 is a fifth version of a series that adds support for the first two of
>> >> the missing features: the EC LPC and EC character device interfaces that
>> >> are used by user-space to access the ChromeOS EC. The support patches were
>> >> taken from the downstream ChromiumOS 3.14 tree with the fixes and cleanups
>> >> squashed to have a minimal patch-set.
>> >>
>> >
>> > Any comments on this series? The last version was posted a couple of weeks
>> > ago but the series have been in the list for months. Lee has already acked
>> > the mfd changes so you can merge all through your chrome-platform tree if
>> > you want.
>> >
>> > It wold be great if this series get in to have the EC user-space interface
>> > supported and to minimize the delta with the Chromemium OS kernel since it
>> > still has other features that needs to be upstreamed like multiple EC in a
>> > system and access to vboot context stored in block device or EC's nvram.
>>
>> Are you sure Olof is the right maintainer for this going to mainline?
>
> Yes.
>
>> I do feel for you trying to get all this in and have seen your many
>> attempts. It has been in U-Boot for 18 months...I hope you get there
>> in the end.
>
> Cool, u-boot has userspace interfaces now? I didn't know that.

Not the 'User-space device interface', the other stuff :-) Anyway it
will be good to see this series in.

Regards,
Simon

2015-02-26 17:38:45

by Olof Johansson

[permalink] [raw]
Subject: Re: [PATCH v5 4/7] platform/chrome: Add Chrome OS EC userspace device interface

On Thu, Feb 26, 2015 at 1:08 AM, Javier Martinez Canillas
<[email protected]> wrote:
> Hello Olof,
>
> Thanks a lot for your feedback.
>
> On 02/26/2015 02:13 AM, Gwendal Grignou wrote:
>> Olof,
>>
>> I think the way Javier did it is fine, the 'major' of the ioctl is
>> 0xEC, from ':'.
>>
>> Gwendal.
>>
>
> As Gwendal said, I deliberately changed the IOCTL mayor number to
> make it different in both kernels.
>
> downstream:
>
> #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)
>
> mainline:
>
> #define CROS_EC_DEV_IOC 0xEC
> #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)
>
> I can also do what you suggested and keep ':' as the major and use 2/3
> as command numbers but I just think 0xEC is a much nicer major for the
> interface to talk with the Embedded Controller and it was available ;)

No, changing major is definitely sufficient and an acceptable way to
do it -- I had missed that you did so.

Thanks, Javier, Gwendal, I'll apply this today and push out. Gwendal
has been giving it a go on a machine here too so I'll check with him
before I push.


-Olof

2015-02-26 23:35:30

by Gwendal Grignou

[permalink] [raw]
Subject: Re: [PATCH v5 0/7] platform/chrome: Add user-space dev inferface support

Tested-by: Gwendal Grignou <[email protected]>
Reviewed-by: Gwendal Grignou <[email protected]>

Tested on a chromebook pixel with kernel 4.0.0-rc1 and ectool using
the enclosed patch in chromiumos platform/ec tree.
I checked the lightbar is working, check the calls with "strace ectool
...", check the sysfs interface calls.

---
util/comm-dev.c | 6 +++---
util/cros_ec_dev.h | 10 +++++-----
2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/util/comm-dev.c b/util/comm-dev.c
index cdbbbdf..1b9958d 100644
--- a/util/comm-dev.c
+++ b/util/comm-dev.c
@@ -13,9 +13,9 @@
#include <sys/types.h>
#include <unistd.h>
.
+#include "ec_commands.h"
#include "cros_ec_dev.h"
#include "comm-host.h"
-#include "ec_commands.h"
.
static int fd = -1;
.
@@ -61,9 +61,8 @@ static int ec_command_dev(int command, int version,
s_cmd.version = version;
s_cmd.result = 0xff;
s_cmd.outsize = outsize;
- s_cmd.outdata = (uint8_t *)outdata;
s_cmd.insize = insize;
- s_cmd.indata = indata;
+ memcpy(s_cmd.outdata, outdata, outsize);
.
r = ioctl(fd, CROS_EC_DEV_IOCXCMD, &s_cmd);
if (r < 0) {
@@ -83,6 +82,7 @@ static int ec_command_dev(int command, int version,
strresult(s_cmd.result));
return -EECRESULT - s_cmd.result;
}
+ memcpy(indata, s_cmd.indata, insize);
.
return r;
}
diff --git a/util/cros_ec_dev.h b/util/cros_ec_dev.h
index f6f5a55..55184ca 100644
--- a/util/cros_ec_dev.h
+++ b/util/cros_ec_dev.h
@@ -26,11 +26,11 @@
struct cros_ec_command {
uint32_t version;
uint32_t command;
- uint8_t *outdata;
uint32_t outsize;
- uint8_t *indata;
uint32_t insize;
uint32_t result;
+ uint8_t outdata[EC_PROTO2_MAX_PARAM_SIZE];
+ uint8_t indata[EC_PROTO2_MAX_PARAM_SIZE];
};
.
/*
@@ -46,8 +46,8 @@ struct cros_ec_readmem {
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)
+#define CROS_EC_DEV_IOC 0xEC
+#define CROS_EC_DEV_IOCXCMD _IOWR(0xEC, 0, struct cros_ec_command)
+#define CROS_EC_DEV_IOCRDMEM _IOWR(0xEC, 1, struct cros_ec_readmem)
.
#endif /* _CROS_EC_DEV_H_ */
--.
2.2.0.rc0.207.ga3a616c

On Mon, Feb 2, 2015 at 3:26 AM, Javier Martinez Canillas
<[email protected]> wrote:
> Hello,
>
> The mainline ChromeOS Embedded Controller (EC) driver is still missing some
> features that are present in the downstream ChromiumOS tree. These are:
>
> - Low Pin Count (LPC) interface
> - User-space device 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 is a fifth version of a series that adds support for the first two of
> the missing features: the EC LPC and EC character device interfaces that
> are used by user-space to access the ChromeOS EC. The support patches were
> taken from the downstream ChromiumOS 3.14 tree with the fixes and cleanups
> squashed to have a minimal patch-set.
>
> The version of the ChromeOS EC chardev driver in this series still does not
> reflect the latest one that is in the downstream ChromiumOS 3.14 kernel but
> makes the delta shorter. Following patches will add the remaining missing
> features until both trees are in sync. I preferred to first add the initial
> support and then adding the other features to both maintain the original
> patch history in the downstream kernel and so preserve the patch authorship
> and also make the diff to have a working cros user-space interface smaller.
>
> This version solves the issues pointed out in the earlier revision.
>
> A big difference between this series and the downstream ChromiumOS kernel is
> that the ioctl API is modified to make it 64-bit safe and compatible with both
> 64 and 32 bit user-space binaries. The data structures passed as arguments in
> the ChromiumOS ioctl interface commands has pointers fields and since these
> have different byte boundaries alignment requirement, the ChromiumOS driver
> has a compat ioctl interface. The feedback was that this had to be avoided
> since this was a new ioctl API so the pointers fields were replaced with a set
> of fixed-size arrays to be used instead. This has the drawback that more data
> could be used and copied between user and kernel space so feedback is welcomed
> if there is a better approach to solve this kind of issues.
>
> The patches were tested on an Exynos5420 Peach Pit Chromebook and (thanks to
> Bill Richardson) on an x86 Pixel Chromebook using a modified ectool [0] to use
> the new ioctl API. The LPC interface driver and the lightbar sysfs driver were
> also tested on the Pixel Chromebook.
>
> The series is composed of the following patches:
>
> Bill Richardson (4):
> platform/chrome: Add cros_ec_lpc driver for x86 devices
> platform/chrome: Add Chrome OS EC userspace device interface
> platform/chrome: Create sysfs attributes for the ChromeOS EC
> platform/chrome: Expose Chrome OS Lightbar to users
>
> Javier Martinez Canillas (3):
> mfd: cros_ec: Use fixed size arrays to transfer data with the EC
> mfd: cros_ec: Add char dev and virtual dev pointers
> mfd: cros_ec: Instantiate ChromeOS EC character device
>
> Documentation/ioctl/ioctl-number.txt | 1 +
> drivers/i2c/busses/i2c-cros-ec-tunnel.c | 51 +---
> drivers/input/keyboard/cros_ec_keyb.c | 13 +-
> drivers/mfd/cros_ec.c | 19 +-
> drivers/platform/chrome/Kconfig | 26 +-
> drivers/platform/chrome/Makefile | 3 +
> drivers/platform/chrome/cros_ec_dev.c | 274 +++++++++++++++++++++
> drivers/platform/chrome/cros_ec_dev.h | 53 +++++
> drivers/platform/chrome/cros_ec_lightbar.c | 367 +++++++++++++++++++++++++++++
> drivers/platform/chrome/cros_ec_lpc.c | 319 +++++++++++++++++++++++++
> drivers/platform/chrome/cros_ec_sysfs.c | 271 +++++++++++++++++++++
> include/linux/mfd/cros_ec.h | 23 +-
> 12 files changed, 1358 insertions(+), 62 deletions(-)
> create mode 100644 drivers/platform/chrome/cros_ec_dev.c
> create mode 100644 drivers/platform/chrome/cros_ec_dev.h
> create mode 100644 drivers/platform/chrome/cros_ec_lightbar.c
> create mode 100644 drivers/platform/chrome/cros_ec_lpc.c
> create mode 100644 drivers/platform/chrome/cros_ec_sysfs.c
>
> Patch #1 modified the struct cros_ec_command structure so it can be used
> as an ioctl argument and be 64 and 32 bit safe and patch #2 adds fields
> to the struct cros_ec_device that will be needed by the EC chardev driver.
>
> Patch #3 adds support for the EC LPC interface used on x86 Chromebooks.
>
> Patch #4 adds the ChromeOS chardev driver and patch #5 instantiates it
> from the mfd cros_ec driver.
>
> Patch #6 exposes sysfs attributes that can be used by user space programs
> to get information and control the ChromeOS EC.
>
> Patch #7 exposes sysfs attributes that are used to control the lightbar
> RGB LEDs found on the Pixel Chromebook.
>
> The patches must be applied together and in order due dependencies.
> Lee Jones has already acked the MFD changes so I think this could go
> through Olof's chrome-platform tree.
>
> Best regards,
> Javier
>
> [0]: git://git.collabora.co.uk/git/user/javier/ec.git mainline-ioctl

2015-02-27 00:11:24

by Olof Johansson

[permalink] [raw]
Subject: Re: [PATCH v5 0/7] platform/chrome: Add user-space dev inferface support

On Thu, Feb 26, 2015 at 3:35 PM, Gwendal Grignou <[email protected]> wrote:
> Tested-by: Gwendal Grignou <[email protected]>
> Reviewed-by: Gwendal Grignou <[email protected]>
>
> Tested on a chromebook pixel with kernel 4.0.0-rc1 and ectool using
> the enclosed patch in chromiumos platform/ec tree.
> I checked the lightbar is working, check the calls with "strace ectool
> ...", check the sysfs interface calls.


Thanks Gwendal.

Series has been applied and pushed out to for-next now.

Thanks all. Bring on the next batch. Javier!


-Olof

2015-02-27 05:18:14

by Javier Martinez Canillas

[permalink] [raw]
Subject: Re: [PATCH v5 0/7] platform/chrome: Add user-space dev inferface support

Hello,

On 02/27/2015 01:11 AM, Olof Johansson wrote:
> On Thu, Feb 26, 2015 at 3:35 PM, Gwendal Grignou <[email protected]> wrote:
>> Tested-by: Gwendal Grignou <[email protected]>
>> Reviewed-by: Gwendal Grignou <[email protected]>
>>
>> Tested on a chromebook pixel with kernel 4.0.0-rc1 and ectool using
>> the enclosed patch in chromiumos platform/ec tree.
>> I checked the lightbar is working, check the calls with "strace ectool
>> ...", check the sysfs interface calls.
>
>
> Thanks Gwendal.
>

Thanks a lot Gwendal for testing

> Series has been applied and pushed out to for-next now.
>
> Thanks all. Bring on the next batch. Javier!
>

Thanks, Fengguang's build bot found a couple of issues
though so I'll post the fixes for those now.

>
> -Olof
>

Best regards,
Javier