[PATCH 0/4] platform/chrome: Support for cros_ec_accel_legacy driver

[PATCH 0/4] platform/chrome: Support for cros_ec_accel_legacy driver

[Thierry Escande]

Hi,

This series enables a driver for a legacy accelerometer driver used on
Chromebook devices with older EC firmware.

In addition to the cros_ec_accel_legacy driver, this series contains a
fix that registers the cros_ec_lpc driver on Chromebook devices that
does not have the GOOG0004 ACPI entry. In such case, the driver register
the device itself. This series also adds support to the cros_ec_lpc
driver for Glimmer based devices (Lenovo Yoga 11e).

The last patch is the code used to register this driver if the usual way
of registering the croc_ec sensors hub fails. In this case, the 2
accelerometers (base and lid) are registered using mfd_add_devices().
As there is work in progress to remove calls to mfd_add_devices() from
cros_ec_dev.c, this code is in a separate patch with the idea to make
that easier.

Regards,
 Thierry

Enric Balletbo i Serra (1):
  platform/chrome: cros_ec_lpc: Register the driver if ACPI entry is
    missing.

Gwendal Grignou (1):
  platform/chrome: Add cros_ec_accel_legacy driver

Thierry Escande (2):
  platform/chrome: cros_ec_lpc: Add support for Google Glimmer
  platform/chrome: Register cros_ec_accel_legacy driver

 drivers/iio/accel/Kconfig                |  11 +
 drivers/iio/accel/Makefile               |   2 +
 drivers/iio/accel/cros_ec_accel_legacy.c | 444 +++++++++++++++++++++++++++++++
 drivers/platform/chrome/cros_ec_dev.c    |  53 ++++
 drivers/platform/chrome/cros_ec_lpc.c    |  41 ++-
 5 files changed, 550 insertions(+), 1 deletion(-)
 create mode 100644 drivers/iio/accel/cros_ec_accel_legacy.c

-- 
2.7.4

[PATCH 1/4] platform/chrome: cros_ec_lpc: Register the driver if ACPI entry is missing.

[Thierry Escande]

From: Enric Balletbo i Serra <enric.balletbo@collabora.com>

Commit 12278dc7c572 ("platform/chrome: cros_ec_lpc: Add support for
GOOG004 ACPI device") added support when the firmware reports the ACPI
device, there are some firmwares though that doesn't report this device
but have it. In such cases we need to instantiate the driver explicitly
if it is not instantiated through ACPI.

Fixes: 12278dc7c572 ("platform/chrome: cros_ec_lpc: Add support for GOOG004 ACPI device")
Signed-off-by: Guenter Roeck <groeck@chromium.org>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
---
 drivers/platform/chrome/cros_ec_lpc.c | 34 +++++++++++++++++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
index 2b6436d..ccdb270 100644
--- a/drivers/platform/chrome/cros_ec_lpc.c
+++ b/drivers/platform/chrome/cros_ec_lpc.c
@@ -35,6 +35,9 @@
 #define DRV_NAME "cros_ec_lpcs"
 #define ACPI_DRV_NAME "GOOG0004"
 
+/* True if ACPI device is present */
+static bool cros_ec_lpc_acpi_device_found;
+
 static int ec_response_timed_out(void)
 {
 	unsigned long one_second = jiffies + HZ;
@@ -396,9 +399,21 @@ static struct platform_driver cros_ec_lpc_driver = {
 	.remove = cros_ec_lpc_remove,
 };
 
+static struct platform_device cros_ec_lpc_device = {
+	.name = DRV_NAME
+};
+
+static acpi_status cros_ec_lpc_parse_device(acpi_handle handle, u32 level,
+					    void *context, void **retval)
+{
+	*(bool *)context = true;
+	return AE_CTRL_TERMINATE;
+}
+
 static int __init cros_ec_lpc_init(void)
 {
 	int ret;
+	acpi_status status;
 
 	if (!dmi_check_system(cros_ec_lpc_dmi_table)) {
 		pr_err(DRV_NAME ": unsupported system.\n");
@@ -415,11 +430,28 @@ static int __init cros_ec_lpc_init(void)
 		return ret;
 	}
 
-	return 0;
+	status = acpi_get_devices(ACPI_DRV_NAME, cros_ec_lpc_parse_device,
+				  &cros_ec_lpc_acpi_device_found, NULL);
+	if (ACPI_FAILURE(status))
+		pr_warn(DRV_NAME ": Looking for %s failed\n", ACPI_DRV_NAME);
+
+	if (!cros_ec_lpc_acpi_device_found) {
+		/* Register the device, and it'll get hooked up automatically */
+		ret = platform_device_register(&cros_ec_lpc_device);
+		if (ret) {
+			pr_err(DRV_NAME ": can't register device: %d\n", ret);
+			platform_driver_unregister(&cros_ec_lpc_driver);
+			cros_ec_lpc_reg_destroy();
+		}
+	}
+
+	return ret;
 }
 
 static void __exit cros_ec_lpc_exit(void)
 {
+	if (!cros_ec_lpc_acpi_device_found)
+		platform_device_unregister(&cros_ec_lpc_device);
 	platform_driver_unregister(&cros_ec_lpc_driver);
 	cros_ec_lpc_reg_destroy();
 }
-- 
2.7.4

[PATCH 2/4] platform/chrome: cros_ec_lpc: Add support for Google Glimmer

[Thierry Escande]

This patch adds device information to the DMI table of the cros_ec_lpc
driver for Google Glimmer devices. Since Google BIOS does not enumerate
devices in the LPC bus, the cros_ec_lpc driver checks for system
compatibility and registers the cros_ec device itself.

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

diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
index ccdb270..8e76ca5 100644
--- a/drivers/platform/chrome/cros_ec_lpc.c
+++ b/drivers/platform/chrome/cros_ec_lpc.c
@@ -365,6 +365,13 @@ static struct dmi_system_id cros_ec_lpc_dmi_table[] __initdata = {
 			DMI_MATCH(DMI_PRODUCT_NAME, "Peppy"),
 		},
 	},
+	{
+		/* x86-glimmer, the Lenovo Thinkpad Yoga 11e. */
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "GOOGLE"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "Glimmer"),
+		},
+	},
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(dmi, cros_ec_lpc_dmi_table);
-- 
2.7.4

[PATCH 3/4] platform/chrome: Add cros_ec_accel_legacy driver

[Thierry Escande]

From: Gwendal Grignou <gwendal@chromium.org>

Add driver to support older EC firmware that only support deprecated
ec command. Rely on ACPI memory map register to access sensor
information.
Present same interface as the regular cros_ec sensor stack:
- one iio device per accelerometer
- use HTML5 axis definition
- use iio abi units
- accept calibration calls, but do nothing
Chrome can use the same code than regular cros_ec sensor stack to
calculate orientation and lid angle.

Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
---
 drivers/iio/accel/Kconfig                |  11 +
 drivers/iio/accel/Makefile               |   2 +
 drivers/iio/accel/cros_ec_accel_legacy.c | 444 +++++++++++++++++++++++++++++++
 3 files changed, 457 insertions(+)
 create mode 100644 drivers/iio/accel/cros_ec_accel_legacy.c

diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
index 15de262..5a1ef29 100644
--- a/drivers/iio/accel/Kconfig
+++ b/drivers/iio/accel/Kconfig
@@ -148,6 +148,17 @@ config HID_SENSOR_ACCEL_3D
 	  To compile this driver as a module, choose M here: the
 	  module will be called hid-sensor-accel-3d.
 
+config IIO_CROS_EC_ACCEL_LEGACY
+	tristate "ChromeOS EC Legacy Accelerometer Sensor"
+	select IIO_BUFFER
+	select IIO_TRIGGERED_BUFFER
+	select CROS_EC_LPC_REGISTER_DEVICE
+	help
+	  Say yes here to get support for accelerometers on Chromebook using
+	  legacy EC firmware.
+	  Sensor data is retrieved through IO memory.
+	  Newer devices should use IIO_CROS_EC_SENSORS.
+
 config IIO_ST_ACCEL_3AXIS
 	tristate "STMicroelectronics accelerometers 3-Axis Driver"
 	depends on (I2C || SPI_MASTER) && SYSFS
diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
index 31fba19..fdd054a 100644
--- a/drivers/iio/accel/Makefile
+++ b/drivers/iio/accel/Makefile
@@ -43,6 +43,8 @@ obj-$(CONFIG_SCA3000)		+= sca3000.o
 obj-$(CONFIG_STK8312)		+= stk8312.o
 obj-$(CONFIG_STK8BA50)		+= stk8ba50.o
 
+obj-$(CONFIG_IIO_CROS_EC_ACCEL_LEGACY) += cros_ec_accel_legacy.o
+
 obj-$(CONFIG_IIO_SSP_SENSORS_COMMONS) += ssp_accel_sensor.o
 
 obj-$(CONFIG_IIO_ST_ACCEL_3AXIS) += st_accel.o
diff --git a/drivers/iio/accel/cros_ec_accel_legacy.c b/drivers/iio/accel/cros_ec_accel_legacy.c
new file mode 100644
index 0000000..26e71a1
--- /dev/null
+++ b/drivers/iio/accel/cros_ec_accel_legacy.c
@@ -0,0 +1,444 @@
+/*
+ * Driver for older Chrome OS EC accelerometer
+ *
+ * Copyright 2017 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 memory mapper cros-ec interface to communicate
+ * with the Chrome OS EC about accelerometer data.
+ * Accelerometer access is presented through iio sysfs.
+ */
+
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/kfifo_buf.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/kernel.h>
+#include <linux/mfd/cros_ec.h>
+#include <linux/mfd/cros_ec_commands.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/sysfs.h>
+#include <linux/platform_device.h>
+
+#define DRV_NAME	"cros-ec-accel-legacy"
+
+/* Indices for EC sensor values. */
+enum {
+	X,
+	Y,
+	Z,
+	MAX_AXIS,
+};
+
+struct cros_ec_accel_legacy_state;
+static ssize_t cros_ec_accel_legacy_id(struct iio_dev *indio_dev,
+				       uintptr_t private,
+				       const struct iio_chan_spec *chan,
+				       char *buf);
+
+static ssize_t cros_ec_accel_legacy_loc(struct iio_dev *indio_dev,
+					uintptr_t private,
+					const struct iio_chan_spec *chan,
+					char *buf);
+
+static const struct iio_chan_spec_ext_info cros_ec_accel_legacy_ext_info[] = {
+	{
+		.name = "id",
+		.shared = IIO_SHARED_BY_ALL,
+		.read = cros_ec_accel_legacy_id,
+	},
+	{
+		.name = "location",
+		.shared = IIO_SHARED_BY_ALL,
+		.read = cros_ec_accel_legacy_loc,
+	},
+	{ }
+};
+
+#define CROS_EC_ACCEL_LEGACY_CHAN(_axis)				\
+	{								\
+		.type = IIO_ACCEL,					\
+		.channel2 = IIO_MOD_X + (_axis),			\
+		.modified = 1,					        \
+		.info_mask_separate =					\
+			BIT(IIO_CHAN_INFO_RAW) |			\
+			BIT(IIO_CHAN_INFO_CALIBSCALE) |			\
+			BIT(IIO_CHAN_INFO_CALIBBIAS),			\
+		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SCALE),	\
+		.ext_info = cros_ec_accel_legacy_ext_info,		\
+		.scan_type = {						\
+			.sign = 's',					\
+			.realbits = 16,					\
+			.storagebits = 16,				\
+			.shift = 0,					\
+		},							\
+	}								\
+
+static struct iio_chan_spec ec_accel_channels[] = {
+	CROS_EC_ACCEL_LEGACY_CHAN(X),
+	CROS_EC_ACCEL_LEGACY_CHAN(Y),
+	CROS_EC_ACCEL_LEGACY_CHAN(Z),
+	IIO_CHAN_SOFT_TIMESTAMP(MAX_AXIS)
+};
+
+/* State data for cros_ec_accel_legacy iio driver. */
+struct cros_ec_accel_legacy_state {
+	struct cros_ec_device *ec;
+
+	/*
+	 * Static array to hold data from a single capture. For each
+	 * channel we need 2 bytes, except for the timestamp. The timestamp
+	 * is always last and is always 8-byte aligned.
+	 */
+	union {
+		s16 samples[MAX_AXIS];
+		u64 ts[DIV_ROUND_UP(ARRAY_SIZE(ec_accel_channels),
+				    sizeof(u64) / sizeof(u16)) + 1];
+	} capture_data;
+	s8 sign[MAX_AXIS];
+	u8 sensor_num;
+};
+
+static int ec_cmd_read_u8(struct cros_ec_device *ec, unsigned int offset,
+			  u8 *dest)
+{
+	return ec->cmd_readmem(ec, offset, 1, dest);
+}
+
+static int ec_cmd_read_u16(struct cros_ec_device *ec, unsigned int offset,
+			   u16 *dest)
+{
+	u16 tmp;
+	int ret = ec->cmd_readmem(ec, offset, 2, &tmp);
+
+	*dest = le16_to_cpu(tmp);
+
+	return ret;
+}
+
+/**
+ * read_ec_until_not_busy - read from EC status byte until it reads not busy.
+ *
+ * @st Pointer to state information for device.
+ * @return 8-bit status if ok, -ve on error
+ */
+static int read_ec_until_not_busy(struct cros_ec_accel_legacy_state *st)
+{
+	struct cros_ec_device *ec = st->ec;
+	u8 status;
+	int attempts = 0;
+
+	ec_cmd_read_u8(ec, EC_MEMMAP_ACC_STATUS, &status);
+	while (status & EC_MEMMAP_ACC_STATUS_BUSY_BIT) {
+		/* Give up after enough attempts, return error. */
+		if (attempts++ >= 50)
+			return -EIO;
+
+		/* Small delay every so often. */
+		if (attempts % 5 == 0)
+			msleep(25);
+
+		ec_cmd_read_u8(ec, EC_MEMMAP_ACC_STATUS, &status);
+	}
+
+	return status;
+}
+
+/**
+ * read_ec_accel_data_unsafe - read acceleration data from EC shared memory.
+ *
+ * @st Pointer to state information for device.
+ * @scan_mask Bitmap of the sensor indices to scan.
+ * @data Location to store data.
+ *
+ * Note this is the unsafe function for reading the EC data. It does not
+ * guarantee that the EC will not modify the data as it is being read in.
+ */
+static void read_ec_accel_data_unsafe(struct cros_ec_accel_legacy_state *st,
+				      unsigned long scan_mask, s16 *data)
+{
+	int i = 0;
+	int num_enabled = bitmap_weight(&scan_mask, MAX_AXIS);
+
+	/*
+	 * Read all sensors enabled in scan_mask. Each value is 2
+	 * bytes.
+	 */
+	while (num_enabled--) {
+		i = find_next_bit(&scan_mask, MAX_AXIS, i);
+		ec_cmd_read_u16(
+			st->ec,
+			EC_MEMMAP_ACC_DATA +
+				sizeof(s16) *
+				(1 + i + st->sensor_num * MAX_AXIS),
+			data);
+		*data *= st->sign[i];
+		i++;
+		data++;
+	}
+}
+
+/**
+ * read_ec_accel_data - read acceleration data from EC shared memory.
+ *
+ * @st Pointer to state information for device.
+ * @scan_mask Bitmap of the sensor indices to scan.
+ * @data Location to store data.
+ * @return 0 if ok, -ve on error
+ *
+ * Note: this is the safe function for reading the EC data. It guarantees
+ * that the data sampled was not modified by the EC while being read.
+ */
+static int read_ec_accel_data(struct cros_ec_accel_legacy_state *st,
+			      unsigned long scan_mask, s16 *data)
+{
+	u8 samp_id = 0xff;
+	u8 status = 0;
+	int rv;
+	int attempts = 0;
+
+	/*
+	 * Continually read all data from EC until the status byte after
+	 * all reads reflects that the EC is not busy and the sample id
+	 * matches the sample id from before all reads. This guarantees
+	 * that data read in was not modified by the EC while reading.
+	 */
+	while ((status & (EC_MEMMAP_ACC_STATUS_BUSY_BIT |
+			  EC_MEMMAP_ACC_STATUS_SAMPLE_ID_MASK)) != samp_id) {
+		/* If we have tried to read too many times, return error. */
+		if (attempts++ >= 5)
+			return -EIO;
+
+		/* Read status byte until EC is not busy. */
+		rv = read_ec_until_not_busy(st);
+		if (rv < 0)
+			return rv;
+		status = rv;
+
+		/*
+		 * Store the current sample id so that we can compare to the
+		 * sample id after reading the data.
+		 */
+		samp_id = status & EC_MEMMAP_ACC_STATUS_SAMPLE_ID_MASK;
+
+		/* Read all EC data, format it, and store it into data. */
+		read_ec_accel_data_unsafe(st, scan_mask, data);
+
+		/* Read status byte. */
+		ec_cmd_read_u8(st->ec, EC_MEMMAP_ACC_STATUS, &status);
+	}
+
+	return 0;
+}
+
+static int cros_ec_accel_legacy_read(struct iio_dev *indio_dev,
+				     struct iio_chan_spec const *chan,
+				     int *val, int *val2, long mask)
+{
+	struct cros_ec_accel_legacy_state *st = iio_priv(indio_dev);
+	s16 data = 0;
+	int ret = IIO_VAL_INT;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		ret = read_ec_accel_data(st, (1 << chan->scan_index), &data);
+		if (ret)
+			return ret;
+		*val = data;
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		/* Sensor scale hard coded at 10 bits per g. */
+		*val = 0;
+		*val2 = IIO_G_TO_M_S_2(1024);
+		return IIO_VAL_INT_PLUS_MICRO;
+	case IIO_CHAN_INFO_CALIBBIAS:
+		/* Calibration not supported. */
+		*val = 0;
+		return IIO_VAL_INT;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int cros_ec_accel_legacy_write(struct iio_dev *indio_dev,
+				      struct iio_chan_spec const *chan,
+				      int val, int val2, long mask)
+{
+	int ret = 0;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_CALIBBIAS:
+		/* Do nothing, to allow calibration script to work. */
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+	return ret;
+}
+
+static const struct iio_info cros_ec_accel_legacy_info = {
+	.read_raw = &cros_ec_accel_legacy_read,
+	.write_raw = &cros_ec_accel_legacy_write,
+	.driver_module = THIS_MODULE,
+};
+
+/**
+ * accel_capture - the trigger handler function
+ *
+ * @irq: the interrupt number
+ * @p: private data - always a pointer to the poll func.
+ *
+ * On a trigger event occurring, if the pollfunc is attached then this
+ * handler is called as a threaded interrupt (and hence may sleep). It
+ * is responsible for grabbing data from the device and pushing it into
+ * the associated buffer.
+ */
+static irqreturn_t cros_ec_accel_legacy_capture(int irq, void *p)
+{
+	struct iio_poll_func *pf = p;
+	struct iio_dev *indio_dev = pf->indio_dev;
+	struct cros_ec_accel_legacy_state *st = iio_priv(indio_dev);
+
+	/* Clear capture data. */
+	memset(st->capture_data.samples, 0, sizeof(st->capture_data));
+
+	/*
+	 * Read data based on which channels are enabled in scan mask. Note
+	 * that on a capture we are always reading the calibrated data.
+	 */
+	read_ec_accel_data(st, *indio_dev->active_scan_mask,
+			   st->capture_data.samples);
+
+	/* Store the timestamp last 8 bytes of data. */
+	if (indio_dev->scan_timestamp)
+		st->capture_data.ts[(indio_dev->scan_bytes - 1) / sizeof(s64)] =
+			iio_get_time_ns(indio_dev);
+
+	iio_push_to_buffers(indio_dev, (u8 *)st->capture_data.samples);
+
+	/*
+	 * Tell the core we are done with this trigger and ready for the
+	 * next one.
+	 */
+	iio_trigger_notify_done(indio_dev->trig);
+
+	return IRQ_HANDLED;
+}
+
+static char *cros_ec_accel_legacy_loc_strings[] = {
+	[MOTIONSENSE_LOC_BASE] = "base",
+	[MOTIONSENSE_LOC_LID] = "lid",
+	[MOTIONSENSE_LOC_MAX] = "unknown",
+};
+
+static ssize_t cros_ec_accel_legacy_loc(struct iio_dev *indio_dev,
+					uintptr_t private,
+					const struct iio_chan_spec *chan,
+					char *buf)
+{
+	struct cros_ec_accel_legacy_state *st = iio_priv(indio_dev);
+
+	return sprintf(buf, "%s\n",
+		       cros_ec_accel_legacy_loc_strings[st->sensor_num +
+							MOTIONSENSE_LOC_BASE]);
+}
+
+static ssize_t cros_ec_accel_legacy_id(struct iio_dev *indio_dev,
+				       uintptr_t private,
+				       const struct iio_chan_spec *chan,
+				       char *buf)
+{
+	struct cros_ec_accel_legacy_state *st = iio_priv(indio_dev);
+
+	return sprintf(buf, "%d\n", st->sensor_num);
+}
+
+static int cros_ec_accel_legacy_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct cros_ec_dev *ec = dev_get_drvdata(dev->parent);
+	struct cros_ec_sensor_platform *sensor_platform = dev_get_platdata(dev);
+	struct iio_dev *indio_dev;
+	struct cros_ec_accel_legacy_state *state;
+	int ret, i;
+
+	if (!ec || !ec->ec_dev) {
+		dev_warn(&pdev->dev, "No EC device found.\n");
+		return -EINVAL;
+	}
+
+	if (!ec->ec_dev->cmd_readmem) {
+		dev_warn(&pdev->dev, "EC does not support direct reads.\n");
+		return -EINVAL;
+	}
+
+	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*state));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, indio_dev);
+	state = iio_priv(indio_dev);
+	state->ec = ec->ec_dev;
+	state->sensor_num = sensor_platform->sensor_num;
+
+	indio_dev->dev.parent = dev;
+	indio_dev->name = pdev->name;
+	indio_dev->channels = ec_accel_channels;
+	/*
+	 * Present the channel using HTML5 standard:
+	 * need to invert X and Y and invert some lid axis.
+	 */
+	for (i = X ; i < MAX_AXIS; i++) {
+		switch (i) {
+		case X:
+			ec_accel_channels[X].scan_index = Y;
+		case Y:
+			ec_accel_channels[Y].scan_index = X;
+		case Z:
+			ec_accel_channels[Z].scan_index = Z;
+		}
+		if (state->sensor_num == MOTIONSENSE_LOC_LID && i != Y)
+			state->sign[i] = -1;
+		else
+			state->sign[i] = 1;
+	}
+	indio_dev->num_channels = ARRAY_SIZE(ec_accel_channels);
+	indio_dev->dev.parent = &pdev->dev;
+	indio_dev->info = &cros_ec_accel_legacy_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	ret = devm_iio_triggered_buffer_setup(dev, indio_dev, NULL,
+					      cros_ec_accel_legacy_capture,
+					      NULL);
+	if (ret)
+		return ret;
+
+	return devm_iio_device_register(dev, indio_dev);
+}
+
+static struct platform_driver cros_ec_accel_platform_driver = {
+	.driver = {
+		.name	= DRV_NAME,
+	},
+	.probe		= cros_ec_accel_legacy_probe,
+};
+module_platform_driver(cros_ec_accel_platform_driver);
+
+MODULE_DESCRIPTION("ChromeOS EC legacy accelerometer driver");
+MODULE_AUTHOR("Gwendal Grignou <gwendal@chromium.org>");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:" DRV_NAME);
-- 
2.7.4

Re: [PATCH 3/4] platform/chrome: Add cros_ec_accel_legacy driver

[Jonathan Cameron]

On Mon, 18 Sep 2017 16:53:17 +0200
Thierry Escande <thierry.escande@collabora.com> wrote:

> From: Gwendal Grignou <gwendal@chromium.org>
> 
> Add driver to support older EC firmware that only support deprecated
> ec command. Rely on ACPI memory map register to access sensor
> information.
> Present same interface as the regular cros_ec sensor stack:
> - one iio device per accelerometer
> - use HTML5 axis definition
> - use iio abi units
> - accept calibration calls, but do nothing
> Chrome can use the same code than regular cros_ec sensor stack to
> calculate orientation and lid angle.
> 
> Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
> Signed-off-by: Thierry Escande <thierry.escande@collabora.com>

A few minor bits and pieces inline.

Jonathan

> ---
>  drivers/iio/accel/Kconfig                |  11 +
>  drivers/iio/accel/Makefile               |   2 +
>  drivers/iio/accel/cros_ec_accel_legacy.c | 444 +++++++++++++++++++++++++++++++
>  3 files changed, 457 insertions(+)
>  create mode 100644 drivers/iio/accel/cros_ec_accel_legacy.c
> 
> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
> index 15de262..5a1ef29 100644
> --- a/drivers/iio/accel/Kconfig
> +++ b/drivers/iio/accel/Kconfig
> @@ -148,6 +148,17 @@ config HID_SENSOR_ACCEL_3D
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called hid-sensor-accel-3d.
>  
> +config IIO_CROS_EC_ACCEL_LEGACY
> +	tristate "ChromeOS EC Legacy Accelerometer Sensor"
> +	select IIO_BUFFER
> +	select IIO_TRIGGERED_BUFFER
> +	select CROS_EC_LPC_REGISTER_DEVICE
> +	help
> +	  Say yes here to get support for accelerometers on Chromebook using
> +	  legacy EC firmware.
> +	  Sensor data is retrieved through IO memory.
> +	  Newer devices should use IIO_CROS_EC_SENSORS.
> +
>  config IIO_ST_ACCEL_3AXIS
>  	tristate "STMicroelectronics accelerometers 3-Axis Driver"
>  	depends on (I2C || SPI_MASTER) && SYSFS
> diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
> index 31fba19..fdd054a 100644
> --- a/drivers/iio/accel/Makefile
> +++ b/drivers/iio/accel/Makefile
> @@ -43,6 +43,8 @@ obj-$(CONFIG_SCA3000)		+= sca3000.o
>  obj-$(CONFIG_STK8312)		+= stk8312.o
>  obj-$(CONFIG_STK8BA50)		+= stk8ba50.o
>  
> +obj-$(CONFIG_IIO_CROS_EC_ACCEL_LEGACY) += cros_ec_accel_legacy.o
> +
>  obj-$(CONFIG_IIO_SSP_SENSORS_COMMONS) += ssp_accel_sensor.o
>  
>  obj-$(CONFIG_IIO_ST_ACCEL_3AXIS) += st_accel.o
> diff --git a/drivers/iio/accel/cros_ec_accel_legacy.c b/drivers/iio/accel/cros_ec_accel_legacy.c
> new file mode 100644
> index 0000000..26e71a1
> --- /dev/null
> +++ b/drivers/iio/accel/cros_ec_accel_legacy.c
> @@ -0,0 +1,444 @@
> +/*
> + * Driver for older Chrome OS EC accelerometer
> + *
> + * Copyright 2017 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 memory mapper cros-ec interface to communicate
> + * with the Chrome OS EC about accelerometer data.
> + * Accelerometer access is presented through iio sysfs.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/kfifo_buf.h>
> +#include <linux/iio/trigger_consumer.h>
> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/kernel.h>
> +#include <linux/mfd/cros_ec.h>
> +#include <linux/mfd/cros_ec_commands.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/sysfs.h>
> +#include <linux/platform_device.h>
> +
> +#define DRV_NAME	"cros-ec-accel-legacy"
> +
> +/* Indices for EC sensor values. */
> +enum {
> +	X,
> +	Y,
> +	Z,
> +	MAX_AXIS,
> +};
> +
> +struct cros_ec_accel_legacy_state;
> +static ssize_t cros_ec_accel_legacy_id(struct iio_dev *indio_dev,
> +				       uintptr_t private,
> +				       const struct iio_chan_spec *chan,
> +				       char *buf);

Wouldn't a bit of reorganizing of the code remove the necessity to have
these forward declarations?

> +
> +static ssize_t cros_ec_accel_legacy_loc(struct iio_dev *indio_dev,
> +					uintptr_t private,
> +					const struct iio_chan_spec *chan,
> +					char *buf);
> +
> +static const struct iio_chan_spec_ext_info cros_ec_accel_legacy_ext_info[] = {
> +	{
> +		.name = "id",

Sysfs attributes all need documenting...
Documentation/ABI/testing/sysfs_bus_iio_cross_ec

location is there, but this legacy id isn't.

> +		.shared = IIO_SHARED_BY_ALL,
> +		.read = cros_ec_accel_legacy_id,
> +	},
> +	{
> +		.name = "location",
> +		.shared = IIO_SHARED_BY_ALL,
> +		.read = cros_ec_accel_legacy_loc,
> +	},
> +	{ }
> +};
> +
> +#define CROS_EC_ACCEL_LEGACY_CHAN(_axis)				\
> +	{								\
> +		.type = IIO_ACCEL,					\
> +		.channel2 = IIO_MOD_X + (_axis),			\
> +		.modified = 1,					        \
> +		.info_mask_separate =					\
> +			BIT(IIO_CHAN_INFO_RAW) |			\
> +			BIT(IIO_CHAN_INFO_CALIBSCALE) |			\
> +			BIT(IIO_CHAN_INFO_CALIBBIAS),			\
> +		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SCALE),	\
> +		.ext_info = cros_ec_accel_legacy_ext_info,		\
> +		.scan_type = {						\
> +			.sign = 's',					\
> +			.realbits = 16,					\
> +			.storagebits = 16,				\
> +			.shift = 0,					\

Shift of 0 is a fairly obvious default so don't specify it.

> +		},							\
> +	}								\
> +
> +static struct iio_chan_spec ec_accel_channels[] = {
> +	CROS_EC_ACCEL_LEGACY_CHAN(X),
> +	CROS_EC_ACCEL_LEGACY_CHAN(Y),
> +	CROS_EC_ACCEL_LEGACY_CHAN(Z),
> +	IIO_CHAN_SOFT_TIMESTAMP(MAX_AXIS)
> +};
> +
> +/* State data for cros_ec_accel_legacy iio driver. */
> +struct cros_ec_accel_legacy_state {
> +	struct cros_ec_device *ec;
> +
> +	/*
> +	 * Static array to hold data from a single capture. For each
> +	 * channel we need 2 bytes, except for the timestamp. The timestamp
> +	 * is always last and is always 8-byte aligned.
> +	 */
> +	union {
> +		s16 samples[MAX_AXIS];
> +		u64 ts[DIV_ROUND_UP(ARRAY_SIZE(ec_accel_channels),
> +				    sizeof(u64) / sizeof(u16)) + 1];
> +	} capture_data;
> +	s8 sign[MAX_AXIS];
> +	u8 sensor_num;
> +};
> +
> +static int ec_cmd_read_u8(struct cros_ec_device *ec, unsigned int offset,
> +			  u8 *dest)
> +{
> +	return ec->cmd_readmem(ec, offset, 1, dest);
> +}
> +
> +static int ec_cmd_read_u16(struct cros_ec_device *ec, unsigned int offset,
> +			   u16 *dest)
> +{
> +	u16 tmp;
> +	int ret = ec->cmd_readmem(ec, offset, 2, &tmp);
> +
> +	*dest = le16_to_cpu(tmp);
> +
> +	return ret;
> +}
> +
> +/**
> + * read_ec_until_not_busy - read from EC status byte until it reads not busy.
> + *
> + * @st Pointer to state information for device.
> + * @return 8-bit status if ok, -ve on error
> + */
> +static int read_ec_until_not_busy(struct cros_ec_accel_legacy_state *st)
> +{
> +	struct cros_ec_device *ec = st->ec;
> +	u8 status;
> +	int attempts = 0;
> +
> +	ec_cmd_read_u8(ec, EC_MEMMAP_ACC_STATUS, &status);
> +	while (status & EC_MEMMAP_ACC_STATUS_BUSY_BIT) {
> +		/* Give up after enough attempts, return error. */
> +		if (attempts++ >= 50)
> +			return -EIO;
> +
> +		/* Small delay every so often. */
> +		if (attempts % 5 == 0)
> +			msleep(25);
> +
> +		ec_cmd_read_u8(ec, EC_MEMMAP_ACC_STATUS, &status);
> +	}
> +
> +	return status;
> +}
> +
> +/**
> + * read_ec_accel_data_unsafe - read acceleration data from EC shared memory.
> + *
> + * @st Pointer to state information for device.
> + * @scan_mask Bitmap of the sensor indices to scan.
> + * @data Location to store data.
> + *
> + * Note this is the unsafe function for reading the EC data. It does not
> + * guarantee that the EC will not modify the data as it is being read in.
> + */
> +static void read_ec_accel_data_unsafe(struct cros_ec_accel_legacy_state *st,
> +				      unsigned long scan_mask, s16 *data)
> +{
> +	int i = 0;
> +	int num_enabled = bitmap_weight(&scan_mask, MAX_AXIS);
> +
> +	/*
> +	 * Read all sensors enabled in scan_mask. Each value is 2
> +	 * bytes.
> +	 */
> +	while (num_enabled--) {
> +		i = find_next_bit(&scan_mask, MAX_AXIS, i);
> +		ec_cmd_read_u16(
> +			st->ec,
> +			EC_MEMMAP_ACC_DATA +
> +				sizeof(s16) *
> +				(1 + i + st->sensor_num * MAX_AXIS),
> +			data);
> +		*data *= st->sign[i];
> +		i++;
> +		data++;
> +	}
> +}
> +
> +/**
> + * read_ec_accel_data - read acceleration data from EC shared memory.
> + *
> + * @st Pointer to state information for device.
> + * @scan_mask Bitmap of the sensor indices to scan.
> + * @data Location to store data.
> + * @return 0 if ok, -ve on error
> + *
> + * Note: this is the safe function for reading the EC data. It guarantees
> + * that the data sampled was not modified by the EC while being read.
> + */
> +static int read_ec_accel_data(struct cros_ec_accel_legacy_state *st,
> +			      unsigned long scan_mask, s16 *data)
> +{
> +	u8 samp_id = 0xff;
> +	u8 status = 0;
> +	int rv;
Given rest of the driver uses ret for this purpose, perhaps here as well?
> +	int attempts = 0;
> +
> +	/*
> +	 * Continually read all data from EC until the status byte after
> +	 * all reads reflects that the EC is not busy and the sample id
> +	 * matches the sample id from before all reads. This guarantees
> +	 * that data read in was not modified by the EC while reading.
> +	 */

Nasty ;)  You have my sympathies.

> +	while ((status & (EC_MEMMAP_ACC_STATUS_BUSY_BIT |
> +			  EC_MEMMAP_ACC_STATUS_SAMPLE_ID_MASK)) != samp_id) {
> +		/* If we have tried to read too many times, return error. */
> +		if (attempts++ >= 5)
> +			return -EIO;
> +
> +		/* Read status byte until EC is not busy. */
> +		rv = read_ec_until_not_busy(st);
> +		if (rv < 0)
> +			return rv;
> +		status = rv;
> +
> +		/*
> +		 * Store the current sample id so that we can compare to the
> +		 * sample id after reading the data.
> +		 */
> +		samp_id = status & EC_MEMMAP_ACC_STATUS_SAMPLE_ID_MASK;
> +
> +		/* Read all EC data, format it, and store it into data. */
> +		read_ec_accel_data_unsafe(st, scan_mask, data);
> +
> +		/* Read status byte. */
> +		ec_cmd_read_u8(st->ec, EC_MEMMAP_ACC_STATUS, &status);
> +	}
> +
> +	return 0;
> +}
> +
> +static int cros_ec_accel_legacy_read(struct iio_dev *indio_dev,
> +				     struct iio_chan_spec const *chan,
> +				     int *val, int *val2, long mask)
> +{
> +	struct cros_ec_accel_legacy_state *st = iio_priv(indio_dev);
> +	s16 data = 0;
> +	int ret = IIO_VAL_INT;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		ret = read_ec_accel_data(st, (1 << chan->scan_index), &data);
> +		if (ret)
> +			return ret;
> +		*val = data;
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_SCALE:
> +		/* Sensor scale hard coded at 10 bits per g. */
> +		*val = 0;
> +		*val2 = IIO_G_TO_M_S_2(1024);
> +		return IIO_VAL_INT_PLUS_MICRO;
> +	case IIO_CHAN_INFO_CALIBBIAS:
> +		/* Calibration not supported. */
> +		*val = 0;
> +		return IIO_VAL_INT;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int cros_ec_accel_legacy_write(struct iio_dev *indio_dev,
> +				      struct iio_chan_spec const *chan,
> +				      int val, int val2, long mask)
> +{
> +	int ret = 0;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_CALIBBIAS:
> +		/* Do nothing, to allow calibration script to work. */
> +		break;
return 0;
> +	default:
return -EINVAL;

Would be cleaner perhaps?

> +		ret = -EINVAL;
> +		break;
> +	}
> +
> +	return ret;
> +}
> +
> +static const struct iio_info cros_ec_accel_legacy_info = {
> +	.read_raw = &cros_ec_accel_legacy_read,
> +	.write_raw = &cros_ec_accel_legacy_write,
> +	.driver_module = THIS_MODULE,

.driver_module field recently removed in favour or registration time
macro magic.  I'll clean this up when applying.

> +};
> +
> +/**
> + * accel_capture - the trigger handler function
> + *
> + * @irq: the interrupt number
> + * @p: private data - always a pointer to the poll func.
> + *
> + * On a trigger event occurring, if the pollfunc is attached then this
> + * handler is called as a threaded interrupt (and hence may sleep). It
> + * is responsible for grabbing data from the device and pushing it into
> + * the associated buffer.
> + */
> +static irqreturn_t cros_ec_accel_legacy_capture(int irq, void *p)
> +{
> +	struct iio_poll_func *pf = p;
> +	struct iio_dev *indio_dev = pf->indio_dev;
> +	struct cros_ec_accel_legacy_state *st = iio_priv(indio_dev);
> +
> +	/* Clear capture data. */
> +	memset(st->capture_data.samples, 0, sizeof(st->capture_data));
> +
> +	/*
> +	 * Read data based on which channels are enabled in scan mask. Note
> +	 * that on a capture we are always reading the calibrated data.
> +	 */
> +	read_ec_accel_data(st, *indio_dev->active_scan_mask,
> +			   st->capture_data.samples);
> +
> +	/* Store the timestamp last 8 bytes of data. */
> +	if (indio_dev->scan_timestamp)
> +		st->capture_data.ts[(indio_dev->scan_bytes - 1) / sizeof(s64)] =
> +			iio_get_time_ns(indio_dev);
> +
> +	iio_push_to_buffers(indio_dev, (u8 *)st->capture_data.samples);

Why not use iio_push_to_buffers_with_timestamp which will handle the
alignment etc for you.
> +
> +	/*
> +	 * Tell the core we are done with this trigger and ready for the
> +	 * next one.
> +	 */
> +	iio_trigger_notify_done(indio_dev->trig);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static char *cros_ec_accel_legacy_loc_strings[] = {
> +	[MOTIONSENSE_LOC_BASE] = "base",
> +	[MOTIONSENSE_LOC_LID] = "lid",
> +	[MOTIONSENSE_LOC_MAX] = "unknown",
> +};
> +
> +static ssize_t cros_ec_accel_legacy_loc(struct iio_dev *indio_dev,
> +					uintptr_t private,
> +					const struct iio_chan_spec *chan,
> +					char *buf)
> +{
> +	struct cros_ec_accel_legacy_state *st = iio_priv(indio_dev);
> +
> +	return sprintf(buf, "%s\n",
> +		       cros_ec_accel_legacy_loc_strings[st->sensor_num +
> +							MOTIONSENSE_LOC_BASE]);
> +}
> +
> +static ssize_t cros_ec_accel_legacy_id(struct iio_dev *indio_dev,
> +				       uintptr_t private,
> +				       const struct iio_chan_spec *chan,
> +				       char *buf)
> +{
> +	struct cros_ec_accel_legacy_state *st = iio_priv(indio_dev);
> +
> +	return sprintf(buf, "%d\n", st->sensor_num);
> +}
> +
> +static int cros_ec_accel_legacy_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct cros_ec_dev *ec = dev_get_drvdata(dev->parent);
> +	struct cros_ec_sensor_platform *sensor_platform = dev_get_platdata(dev);
> +	struct iio_dev *indio_dev;
> +	struct cros_ec_accel_legacy_state *state;
> +	int ret, i;
> +
> +	if (!ec || !ec->ec_dev) {
> +		dev_warn(&pdev->dev, "No EC device found.\n");
> +		return -EINVAL;
> +	}
> +
> +	if (!ec->ec_dev->cmd_readmem) {
> +		dev_warn(&pdev->dev, "EC does not support direct reads.\n");
> +		return -EINVAL;
> +	}
> +
> +	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*state));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, indio_dev);
> +	state = iio_priv(indio_dev);
> +	state->ec = ec->ec_dev;
> +	state->sensor_num = sensor_platform->sensor_num;
> +
> +	indio_dev->dev.parent = dev;
> +	indio_dev->name = pdev->name;
> +	indio_dev->channels = ec_accel_channels;
> +	/*
> +	 * Present the channel using HTML5 standard:
> +	 * need to invert X and Y and invert some lid axis.
> +	 */
> +	for (i = X ; i < MAX_AXIS; i++) {
> +		switch (i) {
> +		case X:
> +			ec_accel_channels[X].scan_index = Y;
> +		case Y:
> +			ec_accel_channels[Y].scan_index = X;
> +		case Z:
> +			ec_accel_channels[Z].scan_index = Z;
> +		}
> +		if (state->sensor_num == MOTIONSENSE_LOC_LID && i != Y)
> +			state->sign[i] = -1;
> +		else
> +			state->sign[i] = 1;
> +	}
> +	indio_dev->num_channels = ARRAY_SIZE(ec_accel_channels);
> +	indio_dev->dev.parent = &pdev->dev;
> +	indio_dev->info = &cros_ec_accel_legacy_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +	ret = devm_iio_triggered_buffer_setup(dev, indio_dev, NULL,
> +					      cros_ec_accel_legacy_capture,
> +					      NULL);
> +	if (ret)
> +		return ret;
> +
> +	return devm_iio_device_register(dev, indio_dev);
> +}
> +
> +static struct platform_driver cros_ec_accel_platform_driver = {
> +	.driver = {
> +		.name	= DRV_NAME,
> +	},
> +	.probe		= cros_ec_accel_legacy_probe,
> +};
> +module_platform_driver(cros_ec_accel_platform_driver);
> +
> +MODULE_DESCRIPTION("ChromeOS EC legacy accelerometer driver");
> +MODULE_AUTHOR("Gwendal Grignou <gwendal@chromium.org>");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:" DRV_NAME);

[PATCH 4/4] platform/chrome: Register cros_ec_accel_legacy driver

[Thierry Escande]

With this patch, the cros_ec_ctl driver will register the legacy
accelerometer driver (named cros_ec_accel_legacy) if it fails to
register sensors through the usual path cros_ec_sensors_register().
This legacy device is present on Chromebook devices with older EC
firmware only supporting deprecated EC commands (Glimmer based devices).

Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
---
 drivers/platform/chrome/cros_ec_dev.c | 53 +++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/drivers/platform/chrome/cros_ec_dev.c b/drivers/platform/chrome/cros_ec_dev.c
index cf6c4f0..d64bf05 100644
--- a/drivers/platform/chrome/cros_ec_dev.c
+++ b/drivers/platform/chrome/cros_ec_dev.c
@@ -388,6 +388,56 @@ static void cros_ec_sensors_register(struct cros_ec_dev *ec)
 	kfree(msg);
 }
 
+#define CROS_EC_SENSOR_LEGACY_NUM 2
+static struct mfd_cell cros_ec_accel_legacy_cells[CROS_EC_SENSOR_LEGACY_NUM];
+
+static void cros_ec_accel_legacy_register(struct cros_ec_dev *ec)
+{
+	struct cros_ec_device *ec_dev = ec->ec_dev;
+	u8 status;
+	int i, ret;
+	struct cros_ec_sensor_platform
+		sensor_platforms[CROS_EC_SENSOR_LEGACY_NUM];
+
+	/*
+	 * Check if EC supports direct memory reads and if EC has
+	 * accelerometers.
+	 */
+	if (!ec_dev->cmd_readmem)
+		return;
+
+	ret = ec_dev->cmd_readmem(ec_dev, EC_MEMMAP_ACC_STATUS, 1, &status);
+	if (ret < 0) {
+		dev_warn(ec->dev, "EC does not support direct reads.\n");
+		return;
+	}
+
+	/* Check if EC has accelerometers. */
+	if (!(status & EC_MEMMAP_ACC_STATUS_PRESENCE_BIT)) {
+		dev_info(ec->dev, "EC does not have accelerometers.\n");
+		return;
+	}
+
+	/*
+	 * Register 2 accelerometers
+	 */
+	for (i = 0; i < CROS_EC_SENSOR_LEGACY_NUM; i++) {
+		cros_ec_accel_legacy_cells[i].name = "cros-ec-accel-legacy";
+		sensor_platforms[i].sensor_num = i;
+		cros_ec_accel_legacy_cells[i].id = i;
+		cros_ec_accel_legacy_cells[i].platform_data =
+			&sensor_platforms[i];
+		cros_ec_accel_legacy_cells[i].pdata_size =
+			sizeof(struct cros_ec_sensor_platform);
+	}
+	ret = mfd_add_devices(ec->dev, PLATFORM_DEVID_AUTO,
+			      cros_ec_accel_legacy_cells,
+			      CROS_EC_SENSOR_LEGACY_NUM,
+			      NULL, 0, NULL);
+	if (ret)
+		dev_err(ec_dev->dev, "failed to add EC sensors\n");
+}
+
 static int ec_device_probe(struct platform_device *pdev)
 {
 	int retval = -ENOMEM;
@@ -435,6 +485,9 @@ static int ec_device_probe(struct platform_device *pdev)
 	/* check whether this EC is a sensor hub. */
 	if (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE))
 		cros_ec_sensors_register(ec);
+	else
+		/* Workaroud for older EC firmware */
+		cros_ec_accel_legacy_register(ec);
 
 	/* Take control of the lightbar from the EC. */
 	lb_manual_suspend_ctrl(ec, 1);
-- 
2.7.4