2017-08-11 14:23:14

by Thierry Escande

[permalink] [raw]
Subject: [PATCH v2 0/8] platform/chrome: cros_ec: Fixes and improvements

Hi,

This series contains various fixes and improvements for the ChromeOS
Embedded Controller drivers. These concern PM suspend/resume fixes,
sysfs interface, and module initialization.

Regards,
Thierry

v2 changes:
- Restore original changes from ChromeOS kernel tree (a function is
added in patch #7 and then modified in patch #8). This keeps the
changes as closed to the originals as possible.
- Replace a uint16_t declaration with u16.

Daniel Hung-yu Wu (1):
platform/chrome: cros_ec: register shutdown function for debugfs

Douglas Anderson (1):
mfd: cros_ec: Stop the debugfs work when suspended

Gwendal Grignou (3):
iio: cros_ec: Relax sampling frequency before suspending
platform/chrome: cros_ec: Add sysfs entry to set keyboard wake lid
angle
platform/chrome: cros_ec: sysfs: Modify error handling

Joseph Lo (1):
mfd: cros_ec_i2c: move the system sleep pm ops to late

Vincent Palatin (1):
mfd: cros_ec: fail early if we cannot identify the EC

Wei-Ning Huang (1):
mfd: cros_ec_i2c: add ACPI module device table

.../iio/common/cros_ec_sensors/cros_ec_sensors.c | 1 +
.../common/cros_ec_sensors/cros_ec_sensors_core.c | 49 ++++++++++
.../common/cros_ec_sensors/cros_ec_sensors_core.h | 2 +
drivers/iio/light/cros_ec_light_prox.c | 1 +
drivers/mfd/cros_ec.c | 6 +-
drivers/mfd/cros_ec_i2c.c | 17 +++-
drivers/platform/chrome/cros_ec_debugfs.c | 18 ++++
drivers/platform/chrome/cros_ec_debugfs.h | 2 +
drivers/platform/chrome/cros_ec_dev.c | 45 +++++----
drivers/platform/chrome/cros_ec_sysfs.c | 105 ++++++++++++++++-----
include/linux/mfd/cros_ec.h | 1 +
11 files changed, 204 insertions(+), 43 deletions(-)

--
2.7.4


2017-08-11 14:23:16

by Thierry Escande

[permalink] [raw]
Subject: [PATCH v2 6/8] mfd: cros_ec_i2c: add ACPI module device table

From: Wei-Ning Huang <[email protected]>

Add ACPI module device table for matching cros-ec devices to load the
cros_ec_i2c driver automatically.

Signed-off-by: Wei-Ning Huang <[email protected]>
Signed-off-by: Thierry Escande <[email protected]>
Acked-by: Benson Leung <[email protected]>
---
drivers/mfd/cros_ec_i2c.c | 12 ++++++++++++
1 file changed, 12 insertions(+)

diff --git a/drivers/mfd/cros_ec_i2c.c b/drivers/mfd/cros_ec_i2c.c
index 576fcc4..56fc667 100644
--- a/drivers/mfd/cros_ec_i2c.c
+++ b/drivers/mfd/cros_ec_i2c.c
@@ -13,6 +13,7 @@
* GNU General Public License for more details.
*/

+#include <linux/acpi.h>
#include <linux/delay.h>
#include <linux/kernel.h>
#include <linux/module.h>
@@ -345,11 +346,13 @@ const struct dev_pm_ops cros_ec_i2c_pm_ops = {
SET_LATE_SYSTEM_SLEEP_PM_OPS(cros_ec_i2c_suspend, cros_ec_i2c_resume)
};

+#ifdef CONFIG_OF
static const struct of_device_id cros_ec_i2c_of_match[] = {
{ .compatible = "google,cros-ec-i2c", },
{ /* sentinel */ },
};
MODULE_DEVICE_TABLE(of, cros_ec_i2c_of_match);
+#endif

static const struct i2c_device_id cros_ec_i2c_id[] = {
{ "cros-ec-i2c", 0 },
@@ -357,9 +360,18 @@ static const struct i2c_device_id cros_ec_i2c_id[] = {
};
MODULE_DEVICE_TABLE(i2c, cros_ec_i2c_id);

+#ifdef CONFIG_ACPI
+static const struct acpi_device_id cros_ec_i2c_acpi_id[] = {
+ { "GOOG0008", 0 },
+ { },
+};
+MODULE_DEVICE_TABLE(acpi, cros_ec_i2c_acpi_id);
+#endif
+
static struct i2c_driver cros_ec_driver = {
.driver = {
.name = "cros-ec-i2c",
+ .acpi_match_table = ACPI_PTR(cros_ec_i2c_acpi_id),
.of_match_table = of_match_ptr(cros_ec_i2c_of_match),
.pm = &cros_ec_i2c_pm_ops,
},
--
2.7.4

2017-08-11 14:23:39

by Thierry Escande

[permalink] [raw]
Subject: [PATCH v2 8/8] platform/chrome: cros_ec: sysfs: Modify error handling

From: Gwendal Grignou <[email protected]>

When accessing a sysfs attribute, if the EC command fails, -EPROTO is
now returned instead of an error message as it is unlikely an app is
parsing the error message to do something meaningful.
Also, this patch makes use of cros_ec_cmd_xfer_status() instead of
cros_ec_cmd_xfer() so an error message is printed in the syslog.

Signed-off-by: Gwendal Grignou <[email protected]>
Signed-off-by: Thierry Escande <[email protected]>
---
drivers/platform/chrome/cros_ec_sysfs.c | 30 +++++-------------------------
1 file changed, 5 insertions(+), 25 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_sysfs.c b/drivers/platform/chrome/cros_ec_sysfs.c
index 77b4ba5..0fd88eb 100644
--- a/drivers/platform/chrome/cros_ec_sysfs.c
+++ b/drivers/platform/chrome/cros_ec_sysfs.c
@@ -116,15 +116,9 @@ static ssize_t store_ec_reboot(struct device *dev,
msg->command = EC_CMD_REBOOT_EC + ec->cmd_offset;
msg->outsize = sizeof(*param);
msg->insize = 0;
- ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
- if (ret < 0) {
+ ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
+ if (ret < 0)
count = ret;
- goto exit;
- }
- if (msg->result != EC_RES_SUCCESS) {
- dev_dbg(ec->dev, "EC result %d\n", msg->result);
- count = -EINVAL;
- }
exit:
kfree(msg);
return count;
@@ -152,17 +146,11 @@ static ssize_t show_ec_version(struct device *dev,
msg->command = EC_CMD_GET_VERSION + ec->cmd_offset;
msg->insize = sizeof(*r_ver);
msg->outsize = 0;
- ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
+ ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
if (ret < 0) {
count = ret;
goto exit;
}
- if (msg->result != EC_RES_SUCCESS) {
- count = scnprintf(buf, PAGE_SIZE,
- "ERROR: EC returned %d\n", msg->result);
- goto exit;
- }
-
r_ver = (struct ec_response_get_version *)msg->data;
/* Strings should be null-terminated, but let's be sure. */
r_ver->version_string_ro[sizeof(r_ver->version_string_ro) - 1] = '\0';
@@ -257,14 +245,9 @@ static ssize_t show_ec_flashinfo(struct device *dev,
msg->command = EC_CMD_FLASH_INFO + ec->cmd_offset;
msg->insize = sizeof(*resp);
msg->outsize = 0;
- ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
+ ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
if (ret < 0)
goto exit;
- if (msg->result != EC_RES_SUCCESS) {
- ret = scnprintf(buf, PAGE_SIZE,
- "ERROR: EC returned %d\n", msg->result);
- goto exit;
- }

resp = (struct ec_response_flash_info *)msg->data;

@@ -301,12 +284,9 @@ static ssize_t show_kb_wake_angle(struct device *dev,
param->kb_wake_angle.data = EC_MOTION_SENSE_NO_VALUE;
msg->outsize = sizeof(*param);
msg->insize = sizeof(*resp);
- ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
+ ret = cros_ec_cmd_xfer_status(ec->ec_dev, 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_motion_sense *)msg->data;
return scnprintf(buf, PAGE_SIZE, "%d\n",
resp->kb_wake_angle.ret);
--
2.7.4

2017-08-11 14:23:54

by Thierry Escande

[permalink] [raw]
Subject: [PATCH v2 5/8] mfd: cros_ec: fail early if we cannot identify the EC

From: Vincent Palatin <[email protected]>

If we cannot communicate with the EC chip to detect the protocol version
and its features, it's very likely useless to continue. Else we will
commit all kind of uninformed mistakes (using the wrong protocol, the
wrong buffer size, mixing the EC with other chips).

Signed-off-by: Vincent Palatin <[email protected]>
Signed-off-by: Thierry Escande <[email protected]>
Acked-by: Benson Leung <[email protected]>
---
drivers/mfd/cros_ec.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
index b0ca5a4c..c5528ae 100644
--- a/drivers/mfd/cros_ec.c
+++ b/drivers/mfd/cros_ec.c
@@ -112,7 +112,11 @@ int cros_ec_register(struct cros_ec_device *ec_dev)

mutex_init(&ec_dev->lock);

- cros_ec_query_all(ec_dev);
+ err = cros_ec_query_all(ec_dev);
+ if (err) {
+ dev_err(dev, "Cannot identify the EC: error %d\n", err);
+ return err;
+ }

if (ec_dev->irq) {
err = request_threaded_irq(ec_dev->irq, NULL, ec_irq_thread,
--
2.7.4

2017-08-11 14:23:53

by Thierry Escande

[permalink] [raw]
Subject: [PATCH v2 7/8] platform/chrome: cros_ec: Add sysfs entry to set keyboard wake lid angle

From: Gwendal Grignou <[email protected]>

This adds a sysfs attribute (/sys/class/chromeos/cros_ec/kb_wake_angle)
used to set and get the keyboard wake lid angle. This attribute is
present only if 2 accelerometers are controlled by the EC.

This patch also moves the cros_ec features check before the device is
added so the features map obtained from the EC is ready on time.

Signed-off-by: Gwendal Grignou <[email protected]>
Signed-off-by: Thierry Escande <[email protected]>
---
drivers/platform/chrome/cros_ec_dev.c | 32 ++++++-------
drivers/platform/chrome/cros_ec_sysfs.c | 83 +++++++++++++++++++++++++++++++++
include/linux/mfd/cros_ec.h | 1 +
3 files changed, 97 insertions(+), 19 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_dev.c b/drivers/platform/chrome/cros_ec_dev.c
index 225d8e9..a7d711c 100644
--- a/drivers/platform/chrome/cros_ec_dev.c
+++ b/drivers/platform/chrome/cros_ec_dev.c
@@ -304,8 +304,8 @@ static void cros_ec_sensors_register(struct cros_ec_dev *ec)

resp = (struct ec_response_motion_sense *)msg->data;
sensor_num = resp->dump.sensor_count;
- /* Allocate 2 extra sensors in case lid angle or FIFO are needed */
- sensor_cells = kzalloc(sizeof(struct mfd_cell) * (sensor_num + 2),
+ /* Allocate one extra sensor for MOTION_SENSE_FIFO if needed */
+ sensor_cells = kzalloc(sizeof(struct mfd_cell) * (sensor_num + 1),
GFP_KERNEL);
if (sensor_cells == NULL)
goto error;
@@ -361,16 +361,8 @@ static void cros_ec_sensors_register(struct cros_ec_dev *ec)
sensor_type[resp->info.type]++;
id++;
}
- if (sensor_type[MOTIONSENSE_TYPE_ACCEL] >= 2) {
- sensor_platforms[id].sensor_num = sensor_num;
-
- sensor_cells[id].name = "cros-ec-angle";
- sensor_cells[id].id = 0;
- sensor_cells[id].platform_data = &sensor_platforms[id];
- sensor_cells[id].pdata_size =
- sizeof(struct cros_ec_sensor_platform);
- id++;
- }
+ if (sensor_type[MOTIONSENSE_TYPE_ACCEL] >= 2)
+ ec->has_kb_wake_angle = true;
if (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE_FIFO)) {
sensor_cells[id].name = "cros-ec-ring";
id++;
@@ -423,6 +415,15 @@ static int ec_device_probe(struct platform_device *pdev)
goto failed;
}

+ /* check whether this EC is a sensor hub. */
+ if (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE)) {
+ pr_err("has EC_FEATURE_MOTION_SENSE\n");
+ cros_ec_sensors_register(ec);
+ }
+
+ /* Take control of the lightbar from the EC. */
+ lb_manual_suspend_ctrl(ec, 1);
+
retval = cdev_device_add(&ec->cdev, &ec->class_dev);
if (retval) {
dev_err(dev, "cdev_device_add failed => %d\n", retval);
@@ -432,13 +433,6 @@ static int ec_device_probe(struct platform_device *pdev)
if (cros_ec_debugfs_init(ec))
dev_warn(dev, "failed to create debugfs directory\n");

- /* check whether this EC is a sensor hub. */
- if (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE))
- cros_ec_sensors_register(ec);
-
- /* Take control of the lightbar from the EC. */
- lb_manual_suspend_ctrl(ec, 1);
-
return 0;

failed:
diff --git a/drivers/platform/chrome/cros_ec_sysfs.c b/drivers/platform/chrome/cros_ec_sysfs.c
index f3baf99..77b4ba5 100644
--- a/drivers/platform/chrome/cros_ec_sysfs.c
+++ b/drivers/platform/chrome/cros_ec_sysfs.c
@@ -278,20 +278,103 @@ static ssize_t show_ec_flashinfo(struct device *dev,
return ret;
}

+/* Keyboard wake angle control */
+
+static ssize_t show_kb_wake_angle(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct ec_response_motion_sense *resp;
+ struct ec_params_motion_sense *param;
+ struct cros_ec_command *msg;
+ int ret;
+ struct cros_ec_dev *ec = container_of(
+ dev, struct cros_ec_dev, class_dev);
+
+ msg = kmalloc(sizeof(*msg) + EC_HOST_PARAM_SIZE, GFP_KERNEL);
+ if (!msg)
+ return -ENOMEM;
+
+ param = (struct ec_params_motion_sense *)msg->data;
+ msg->command = EC_CMD_MOTION_SENSE_CMD + ec->cmd_offset;
+ msg->version = 2;
+ param->cmd = MOTIONSENSE_CMD_KB_WAKE_ANGLE;
+ param->kb_wake_angle.data = EC_MOTION_SENSE_NO_VALUE;
+ msg->outsize = sizeof(*param);
+ msg->insize = sizeof(*resp);
+ ret = cros_ec_cmd_xfer(ec->ec_dev, 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_motion_sense *)msg->data;
+ return scnprintf(buf, PAGE_SIZE, "%d\n",
+ resp->kb_wake_angle.ret);
+}
+
+static ssize_t store_kb_wake_angle(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct ec_params_motion_sense *param;
+ struct cros_ec_command *msg;
+ int ret;
+ struct cros_ec_dev *ec = container_of(
+ dev, struct cros_ec_dev, class_dev);
+ u16 angle;
+
+ ret = kstrtou16(buf, 0, &angle);
+ if (ret)
+ return ret;
+
+ msg = kmalloc(sizeof(*msg) + EC_HOST_PARAM_SIZE, GFP_KERNEL);
+ if (!msg)
+ return -ENOMEM;
+
+ param = (struct ec_params_motion_sense *)msg->data;
+ msg->command = EC_CMD_MOTION_SENSE_CMD + ec->cmd_offset;
+ msg->version = 2;
+ param->cmd = MOTIONSENSE_CMD_KB_WAKE_ANGLE;
+ param->kb_wake_angle.data = angle;
+ msg->outsize = sizeof(*param);
+ msg->insize = sizeof(struct ec_response_motion_sense);
+ ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
+ if (ret < 0)
+ return ret;
+ return count;
+}
+
/* 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 DEVICE_ATTR(kb_wake_angle, S_IWUSR | S_IRUGO, show_kb_wake_angle,
+ store_kb_wake_angle);

static struct attribute *__ec_attrs[] = {
+ &dev_attr_kb_wake_angle.attr,
&dev_attr_reboot.attr,
&dev_attr_version.attr,
&dev_attr_flashinfo.attr,
NULL,
};

+static umode_t cros_ec_ctrl_visible(struct kobject *kobj,
+ struct attribute *a, int n)
+{
+ struct device *dev = container_of(kobj, struct device, kobj);
+ struct cros_ec_dev *ec = container_of(dev, struct cros_ec_dev,
+ class_dev);
+
+ if (a == &dev_attr_kb_wake_angle.attr && !ec->has_kb_wake_angle)
+ return 0;
+
+ return a->mode;
+}
+
struct attribute_group cros_ec_attr_group = {
.attrs = __ec_attrs,
+ .is_visible = cros_ec_ctrl_visible,
};

diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
index 4e887ba..350b8a4 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -191,6 +191,7 @@ struct cros_ec_dev {
struct cros_ec_device *ec_dev;
struct device *dev;
struct cros_ec_debugfs *debug_info;
+ bool has_kb_wake_angle;
u16 cmd_offset;
u32 features[2];
};
--
2.7.4

2017-08-11 14:24:45

by Thierry Escande

[permalink] [raw]
Subject: [PATCH v2 3/8] mfd: cros_ec: Stop the debugfs work when suspended

From: Douglas Anderson <[email protected]>

This patch stops the debugfs worker thread when the device is suspended.
This change avoids messages like:

cros-ec-spi spi5.0: spi transfer failed: -108
cros-ec-spi spi5.0: cs-deassert spi transfer failed: -108
cros-ec-ctl cros-ec-ctl.0.auto: EC communication failed

Signed-off-by: Douglas Anderson <[email protected]>
Signed-off-by: Thierry Escande <[email protected]>
---
drivers/platform/chrome/cros_ec_debugfs.c | 18 ++++++++++++++++++
drivers/platform/chrome/cros_ec_debugfs.h | 2 ++
drivers/platform/chrome/cros_ec_dev.c | 4 ++++
3 files changed, 24 insertions(+)

diff --git a/drivers/platform/chrome/cros_ec_debugfs.c b/drivers/platform/chrome/cros_ec_debugfs.c
index 4cc66f4..515f411 100644
--- a/drivers/platform/chrome/cros_ec_debugfs.c
+++ b/drivers/platform/chrome/cros_ec_debugfs.c
@@ -399,3 +399,21 @@ void cros_ec_debugfs_remove(struct cros_ec_dev *ec)
debugfs_remove_recursive(ec->debug_info->dir);
cros_ec_cleanup_console_log(ec->debug_info);
}
+
+void cros_ec_debugfs_suspend(struct cros_ec_dev *ec)
+{
+ /*
+ * cros_ec_debugfs_init() failures are non-fatal; it's also possible
+ * that we initted things but decided that console log wasn't supported.
+ * We'll use the same set of checks that cros_ec_debugfs_remove() +
+ * cros_ec_cleanup_console_log() end up using to handle those cases.
+ */
+ if (ec->debug_info && ec->debug_info->log_buffer.buf)
+ cancel_delayed_work_sync(&ec->debug_info->log_poll_work);
+}
+
+void cros_ec_debugfs_resume(struct cros_ec_dev *ec)
+{
+ if (ec->debug_info && ec->debug_info->log_buffer.buf)
+ schedule_delayed_work(&ec->debug_info->log_poll_work, 0);
+}
diff --git a/drivers/platform/chrome/cros_ec_debugfs.h b/drivers/platform/chrome/cros_ec_debugfs.h
index 1ff3a50..d29e410 100644
--- a/drivers/platform/chrome/cros_ec_debugfs.h
+++ b/drivers/platform/chrome/cros_ec_debugfs.h
@@ -23,5 +23,7 @@
/* debugfs stuff */
int cros_ec_debugfs_init(struct cros_ec_dev *ec);
void cros_ec_debugfs_remove(struct cros_ec_dev *ec);
+void cros_ec_debugfs_suspend(struct cros_ec_dev *ec);
+void cros_ec_debugfs_resume(struct cros_ec_dev *ec);

#endif /* _DRV_CROS_EC_DEBUGFS_H_ */
diff --git a/drivers/platform/chrome/cros_ec_dev.c b/drivers/platform/chrome/cros_ec_dev.c
index cf6c4f0..2571f5e 100644
--- a/drivers/platform/chrome/cros_ec_dev.c
+++ b/drivers/platform/chrome/cros_ec_dev.c
@@ -470,6 +470,8 @@ static __maybe_unused int ec_device_suspend(struct device *dev)
{
struct cros_ec_dev *ec = dev_get_drvdata(dev);

+ cros_ec_debugfs_suspend(ec);
+
lb_suspend(ec);

return 0;
@@ -481,6 +483,8 @@ static __maybe_unused int ec_device_resume(struct device *dev)

lb_resume(ec);

+ cros_ec_debugfs_resume(ec);
+
return 0;
}

--
2.7.4

2017-08-11 14:24:44

by Thierry Escande

[permalink] [raw]
Subject: [PATCH v2 4/8] platform/chrome: cros_ec: register shutdown function for debugfs

From: Daniel Hung-yu Wu <[email protected]>

Reboot or shutdown during delayed works could corrupt communication with
EC and certain I2C controller may not be able to recover from the error
state.

This patch registers a shutdown callback used to cancel the debugfs log
worker thread.

Signed-off-by: Daniel Hung-yu Wu <[email protected]>
Signed-off-by: Thierry Escande <[email protected]>
---
drivers/platform/chrome/cros_ec_dev.c | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/drivers/platform/chrome/cros_ec_dev.c b/drivers/platform/chrome/cros_ec_dev.c
index 2571f5e..225d8e9 100644
--- a/drivers/platform/chrome/cros_ec_dev.c
+++ b/drivers/platform/chrome/cros_ec_dev.c
@@ -460,6 +460,14 @@ static int ec_device_remove(struct platform_device *pdev)
return 0;
}

+static void ec_device_shutdown(struct platform_device *pdev)
+{
+ struct cros_ec_dev *ec = dev_get_drvdata(&pdev->dev);
+
+ /* Be sure to clear up debugfs delayed works */
+ cros_ec_debugfs_remove(ec);
+}
+
static const struct platform_device_id cros_ec_id[] = {
{ "cros-ec-ctl", 0 },
{ /* sentinel */ },
@@ -502,6 +510,7 @@ static struct platform_driver cros_ec_dev_driver = {
},
.probe = ec_device_probe,
.remove = ec_device_remove,
+ .shutdown = ec_device_shutdown,
};

static int __init cros_ec_dev_init(void)
--
2.7.4

2017-08-11 14:25:31

by Thierry Escande

[permalink] [raw]
Subject: [PATCH v2 1/8] iio: cros_ec: Relax sampling frequency before suspending

From: Gwendal Grignou <[email protected]>

If an application set a tight sampling frequency, given the interrupt
use is a wakup source, suspend will not happen: the kernel will receive
a wake up interrupt and will cancel the suspend process.

Given cros_ec sensors type is non wake up, this patch adds prepare and
complete callbacks to set 1s sampling period just before suspend. This
ensures the sensor hub will not be a source of interrupt during the
suspend process.

Signed-off-by: Gwendal Grignou <[email protected]>
Signed-off-by: Thierry Escande <[email protected]>
---
.../iio/common/cros_ec_sensors/cros_ec_sensors.c | 1 +
.../common/cros_ec_sensors/cros_ec_sensors_core.c | 49 ++++++++++++++++++++++
.../common/cros_ec_sensors/cros_ec_sensors_core.h | 2 +
drivers/iio/light/cros_ec_light_prox.c | 1 +
4 files changed, 53 insertions(+)

diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
index 38e8783..116da2c 100644
--- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
+++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
@@ -292,6 +292,7 @@ MODULE_DEVICE_TABLE(platform, cros_ec_sensors_ids);
static struct platform_driver cros_ec_sensors_platform_driver = {
.driver = {
.name = "cros-ec-sensors",
+ .pm = &cros_ec_sensors_pm_ops,
},
.probe = cros_ec_sensors_probe,
.id_table = cros_ec_sensors_ids,
diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
index 416cae5..a620eb5 100644
--- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
+++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
@@ -446,5 +446,54 @@ int cros_ec_sensors_core_write(struct cros_ec_sensors_core_state *st,
}
EXPORT_SYMBOL_GPL(cros_ec_sensors_core_write);

+static int __maybe_unused cros_ec_sensors_prepare(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct iio_dev *indio_dev = platform_get_drvdata(pdev);
+ struct cros_ec_sensors_core_state *st = iio_priv(indio_dev);
+
+ if (st->curr_sampl_freq == 0)
+ return 0;
+
+ /*
+ * If the sensors are sampled at high frequency, we will not be able to
+ * sleep. Set sampling to a long period if necessary.
+ */
+ if (st->curr_sampl_freq < CROS_EC_MIN_SUSPEND_SAMPLING_FREQUENCY) {
+ mutex_lock(&st->cmd_lock);
+ st->param.cmd = MOTIONSENSE_CMD_EC_RATE;
+ st->param.ec_rate.data = CROS_EC_MIN_SUSPEND_SAMPLING_FREQUENCY;
+ cros_ec_motion_send_host_cmd(st, 0);
+ mutex_unlock(&st->cmd_lock);
+ }
+ return 0;
+}
+
+static void __maybe_unused cros_ec_sensors_complete(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct iio_dev *indio_dev = platform_get_drvdata(pdev);
+ struct cros_ec_sensors_core_state *st = iio_priv(indio_dev);
+
+ if (st->curr_sampl_freq == 0)
+ return;
+
+ if (st->curr_sampl_freq < CROS_EC_MIN_SUSPEND_SAMPLING_FREQUENCY) {
+ mutex_lock(&st->cmd_lock);
+ st->param.cmd = MOTIONSENSE_CMD_EC_RATE;
+ st->param.ec_rate.data = st->curr_sampl_freq;
+ cros_ec_motion_send_host_cmd(st, 0);
+ mutex_unlock(&st->cmd_lock);
+ }
+}
+
+const struct dev_pm_ops cros_ec_sensors_pm_ops = {
+#ifdef CONFIG_PM_SLEEP
+ .prepare = cros_ec_sensors_prepare,
+ .complete = cros_ec_sensors_complete
+#endif
+};
+EXPORT_SYMBOL_GPL(cros_ec_sensors_pm_ops);
+
MODULE_DESCRIPTION("ChromeOS EC sensor hub core functions");
MODULE_LICENSE("GPL v2");
diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.h b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.h
index 8bc2ca3..2edf68d 100644
--- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.h
+++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.h
@@ -169,6 +169,8 @@ int cros_ec_sensors_core_write(struct cros_ec_sensors_core_state *st,
struct iio_chan_spec const *chan,
int val, int val2, long mask);

+extern const struct dev_pm_ops cros_ec_sensors_pm_ops;
+
/* List of extended channel specification for all sensors */
extern const struct iio_chan_spec_ext_info cros_ec_sensors_ext_info[];

diff --git a/drivers/iio/light/cros_ec_light_prox.c b/drivers/iio/light/cros_ec_light_prox.c
index 7217223..f8658d4 100644
--- a/drivers/iio/light/cros_ec_light_prox.c
+++ b/drivers/iio/light/cros_ec_light_prox.c
@@ -279,6 +279,7 @@ MODULE_DEVICE_TABLE(platform, cros_ec_light_prox_ids);
static struct platform_driver cros_ec_light_prox_platform_driver = {
.driver = {
.name = "cros-ec-light-prox",
+ .pm = &cros_ec_sensors_pm_ops,
},
.probe = cros_ec_light_prox_probe,
.id_table = cros_ec_light_prox_ids,
--
2.7.4

2017-08-11 14:25:30

by Thierry Escande

[permalink] [raw]
Subject: [PATCH v2 2/8] mfd: cros_ec_i2c: move the system sleep pm ops to late

From: Joseph Lo <[email protected]>

The cros_ec_i2c driver is still active after it had suspended or before it
resumes. Besides that, it also tried to transfer data even after the I2C
host had been suspended. This leads the system to crash.

During the test, we also observed that the EC needs to be resumed
earlier due to some status polling from the EC firmware (e.g. battery
status). This patch moves the PM ops to late stage to make it work
normally.

Signed-off-by: Joseph Lo <[email protected]>
Signed-off-by: Thierry Escande <[email protected]>
Acked-by: Benson Leung <[email protected]>
---
drivers/mfd/cros_ec_i2c.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/mfd/cros_ec_i2c.c b/drivers/mfd/cros_ec_i2c.c
index 9f70de1..576fcc4 100644
--- a/drivers/mfd/cros_ec_i2c.c
+++ b/drivers/mfd/cros_ec_i2c.c
@@ -341,8 +341,9 @@ static int cros_ec_i2c_resume(struct device *dev)
}
#endif

-static SIMPLE_DEV_PM_OPS(cros_ec_i2c_pm_ops, cros_ec_i2c_suspend,
- cros_ec_i2c_resume);
+const struct dev_pm_ops cros_ec_i2c_pm_ops = {
+ SET_LATE_SYSTEM_SLEEP_PM_OPS(cros_ec_i2c_suspend, cros_ec_i2c_resume)
+};

static const struct of_device_id cros_ec_i2c_of_match[] = {
{ .compatible = "google,cros-ec-i2c", },
--
2.7.4

2017-08-12 12:48:51

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v2 1/8] iio: cros_ec: Relax sampling frequency before suspending

On Fri, 11 Aug 2017 16:22:58 +0200
Thierry Escande <[email protected]> wrote:

> From: Gwendal Grignou <[email protected]>
>
> If an application set a tight sampling frequency, given the interrupt
> use is a wakup source, suspend will not happen: the kernel will receive
> a wake up interrupt and will cancel the suspend process.
>
> Given cros_ec sensors type is non wake up, this patch adds prepare and
> complete callbacks to set 1s sampling period just before suspend. This
> ensures the sensor hub will not be a source of interrupt during the
> suspend process.
>
> Signed-off-by: Gwendal Grignou <[email protected]>
> Signed-off-by: Thierry Escande <[email protected]>
Acked-by: Jonathan Cameron <[email protected]>

Thanks,

Jonathan
> ---
> .../iio/common/cros_ec_sensors/cros_ec_sensors.c | 1 +
> .../common/cros_ec_sensors/cros_ec_sensors_core.c | 49 ++++++++++++++++++++++
> .../common/cros_ec_sensors/cros_ec_sensors_core.h | 2 +
> drivers/iio/light/cros_ec_light_prox.c | 1 +
> 4 files changed, 53 insertions(+)
>
> diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
> index 38e8783..116da2c 100644
> --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
> +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
> @@ -292,6 +292,7 @@ MODULE_DEVICE_TABLE(platform, cros_ec_sensors_ids);
> static struct platform_driver cros_ec_sensors_platform_driver = {
> .driver = {
> .name = "cros-ec-sensors",
> + .pm = &cros_ec_sensors_pm_ops,
> },
> .probe = cros_ec_sensors_probe,
> .id_table = cros_ec_sensors_ids,
> diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> index 416cae5..a620eb5 100644
> --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> @@ -446,5 +446,54 @@ int cros_ec_sensors_core_write(struct cros_ec_sensors_core_state *st,
> }
> EXPORT_SYMBOL_GPL(cros_ec_sensors_core_write);
>
> +static int __maybe_unused cros_ec_sensors_prepare(struct device *dev)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> + struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> + struct cros_ec_sensors_core_state *st = iio_priv(indio_dev);
> +
> + if (st->curr_sampl_freq == 0)
> + return 0;
> +
> + /*
> + * If the sensors are sampled at high frequency, we will not be able to
> + * sleep. Set sampling to a long period if necessary.
> + */
> + if (st->curr_sampl_freq < CROS_EC_MIN_SUSPEND_SAMPLING_FREQUENCY) {
> + mutex_lock(&st->cmd_lock);
> + st->param.cmd = MOTIONSENSE_CMD_EC_RATE;
> + st->param.ec_rate.data = CROS_EC_MIN_SUSPEND_SAMPLING_FREQUENCY;
> + cros_ec_motion_send_host_cmd(st, 0);
> + mutex_unlock(&st->cmd_lock);
> + }
> + return 0;
> +}
> +
> +static void __maybe_unused cros_ec_sensors_complete(struct device *dev)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> + struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> + struct cros_ec_sensors_core_state *st = iio_priv(indio_dev);
> +
> + if (st->curr_sampl_freq == 0)
> + return;
> +
> + if (st->curr_sampl_freq < CROS_EC_MIN_SUSPEND_SAMPLING_FREQUENCY) {
> + mutex_lock(&st->cmd_lock);
> + st->param.cmd = MOTIONSENSE_CMD_EC_RATE;
> + st->param.ec_rate.data = st->curr_sampl_freq;
> + cros_ec_motion_send_host_cmd(st, 0);
> + mutex_unlock(&st->cmd_lock);
> + }
> +}
> +
> +const struct dev_pm_ops cros_ec_sensors_pm_ops = {
> +#ifdef CONFIG_PM_SLEEP
> + .prepare = cros_ec_sensors_prepare,
> + .complete = cros_ec_sensors_complete
> +#endif
> +};
> +EXPORT_SYMBOL_GPL(cros_ec_sensors_pm_ops);
> +
> MODULE_DESCRIPTION("ChromeOS EC sensor hub core functions");
> MODULE_LICENSE("GPL v2");
> diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.h b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.h
> index 8bc2ca3..2edf68d 100644
> --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.h
> +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.h
> @@ -169,6 +169,8 @@ int cros_ec_sensors_core_write(struct cros_ec_sensors_core_state *st,
> struct iio_chan_spec const *chan,
> int val, int val2, long mask);
>
> +extern const struct dev_pm_ops cros_ec_sensors_pm_ops;
> +
> /* List of extended channel specification for all sensors */
> extern const struct iio_chan_spec_ext_info cros_ec_sensors_ext_info[];
>
> diff --git a/drivers/iio/light/cros_ec_light_prox.c b/drivers/iio/light/cros_ec_light_prox.c
> index 7217223..f8658d4 100644
> --- a/drivers/iio/light/cros_ec_light_prox.c
> +++ b/drivers/iio/light/cros_ec_light_prox.c
> @@ -279,6 +279,7 @@ MODULE_DEVICE_TABLE(platform, cros_ec_light_prox_ids);
> static struct platform_driver cros_ec_light_prox_platform_driver = {
> .driver = {
> .name = "cros-ec-light-prox",
> + .pm = &cros_ec_sensors_pm_ops,
> },
> .probe = cros_ec_light_prox_probe,
> .id_table = cros_ec_light_prox_ids,

2017-08-15 06:31:20

by Benson Leung

[permalink] [raw]
Subject: Re: [PATCH v2 3/8] mfd: cros_ec: Stop the debugfs work when suspended

On Fri, Aug 11, 2017 at 04:23:00PM +0200, Thierry Escande wrote:
> From: Douglas Anderson <[email protected]>
>
> This patch stops the debugfs worker thread when the device is suspended.
> This change avoids messages like:
>
> cros-ec-spi spi5.0: spi transfer failed: -108
> cros-ec-spi spi5.0: cs-deassert spi transfer failed: -108
> cros-ec-ctl cros-ec-ctl.0.auto: EC communication failed
>
> Signed-off-by: Douglas Anderson <[email protected]>
> Signed-off-by: Thierry Escande <[email protected]>

Applied.

Thanks,
Benson
--
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
[email protected]
Chromium OS Project
[email protected]


Attachments:
(No filename) (683.00 B)
signature.asc (819.00 B)
Digital signature
Download all attachments

2017-08-15 06:35:36

by Benson Leung

[permalink] [raw]
Subject: Re: [PATCH v2 4/8] platform/chrome: cros_ec: register shutdown function for debugfs

Hi Thierry,

On Fri, Aug 11, 2017 at 04:23:01PM +0200, Thierry Escande wrote:
> From: Daniel Hung-yu Wu <[email protected]>
>
> Reboot or shutdown during delayed works could corrupt communication with
> EC and certain I2C controller may not be able to recover from the error
> state.
>
> This patch registers a shutdown callback used to cancel the debugfs log
> worker thread.
>
> Signed-off-by: Daniel Hung-yu Wu <[email protected]>
> Signed-off-by: Thierry Escande <[email protected]>

Applied.

Thanks,
Benson


--
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
[email protected]
Chromium OS Project
[email protected]


Attachments:
(No filename) (656.00 B)
signature.asc (819.00 B)
Digital signature
Download all attachments

2017-09-11 19:39:43

by Benson Leung

[permalink] [raw]
Subject: Re: [PATCH v2 7/8] platform/chrome: cros_ec: Add sysfs entry to set keyboard wake lid angle

Hi Thierry,

On Fri, Aug 11, 2017 at 04:23:04PM +0200, Thierry Escande wrote:
> From: Gwendal Grignou <[email protected]>
>
> This adds a sysfs attribute (/sys/class/chromeos/cros_ec/kb_wake_angle)
> used to set and get the keyboard wake lid angle. This attribute is
> present only if 2 accelerometers are controlled by the EC.
>
> This patch also moves the cros_ec features check before the device is
> added so the features map obtained from the EC is ready on time.
>
> Signed-off-by: Gwendal Grignou <[email protected]>
> Signed-off-by: Thierry Escande <[email protected]>

V2 Looks good. Applied. Thanks!

--
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
[email protected]
Chromium OS Project
[email protected]


Attachments:
(No filename) (760.00 B)
signature.asc (819.00 B)
Digital signature
Download all attachments

2017-09-11 19:42:07

by Benson Leung

[permalink] [raw]
Subject: Re: [PATCH v2 8/8] platform/chrome: cros_ec: sysfs: Modify error handling

Hi Thierry,

On Fri, Aug 11, 2017 at 04:23:05PM +0200, Thierry Escande wrote:
> From: Gwendal Grignou <[email protected]>
>
> When accessing a sysfs attribute, if the EC command fails, -EPROTO is
> now returned instead of an error message as it is unlikely an app is
> parsing the error message to do something meaningful.
> Also, this patch makes use of cros_ec_cmd_xfer_status() instead of
> cros_ec_cmd_xfer() so an error message is printed in the syslog.
>
> Signed-off-by: Gwendal Grignou <[email protected]>
> Signed-off-by: Thierry Escande <[email protected]>

Looks good. Applied.

--
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
[email protected]
Chromium OS Project
[email protected]


Attachments:
(No filename) (738.00 B)
signature.asc (819.00 B)
Digital signature
Download all attachments