Dear all,
Basically this is a resend and rebase due that [2] and [3] are currently
merged so all dependecies are in mainline now.
To remmember:
* 1/4 mfd: cros_ec: Get rid of cros_ec_check_features from cros_ec_dev.
As pointed by Lee Jones in this thread [1] we should not use the MFD API
outside of MFD. For this reason the cros-ec-rtc did not get accepted yet.
The reality is that we are calling mfd_add_devices from cros-ec-dev driver
already, so this patch get rid off the MFD calls inside the chardev driver
and moves to cros-ec MFD. Also I think the chardev device should simply
implement the ioctl calls to access to it from userspace.
The above patch involves MFD, IIO and platform chrome subsystems.
* 2/4 mfd: cros_ec: Introduce RTC commands and events definitions
* 3/4 rtc: cros-ec: add cros-ec-rtc driver
* 4/4 mfd: cros_ec: add RTC as mfd subdevice
These patches are the cros-ec RTC driver, 3 and 4 patches are already
acked by the subsystem maintainers involved and are equal to the last
version I send. Patch 5 registers the rtc cell inside the cros-ec MFD
intead of cros-ec-dev chardev driver.
Changes since v2:
- Rebase on top of mainline.
- Removed patch 'mfd: cros-ec: Fix host command buffer size' from series
as was already picked.
Changes since v1:
- Removed patch 'iio: cros_ec_sensors: Fix return value to get raw and
calibbias data' from series as was already picked.
- Removed patch 'iio: cros_ec_sensors: Fix return value to get raw and
calibbias data' from series as was already picked.
- Patch 2/5: Acked-by: Jonathan Cameron <***@kernel.org>
[1] https://www.spinics.net/lists/kernel/msg2465099.html
[2] https://lkml.org/lkml/2017/3/17/319
[3] https://lkml.org/lkml/2017/3/17/321
Best regards,
Enric Balletbo i Serra (1):
mfd: cros_ec: Get rid of cros_ec_check_features from cros_ec_dev.
Stephen Barber (3):
mfd: cros_ec: Introduce RTC commands and events definitions.
rtc: cros-ec: add cros-ec-rtc driver.
mfd: cros_ec: add RTC as mfd subdevice
.../iio/common/cros_ec_sensors/cros_ec_sensors.c | 8 -
.../common/cros_ec_sensors/cros_ec_sensors_core.c | 8 +-
drivers/iio/light/cros_ec_light_prox.c | 8 -
drivers/iio/pressure/cros_ec_baro.c | 8 -
drivers/mfd/cros_ec.c | 178 +++++++++
drivers/platform/chrome/cros_ec_dev.c | 161 --------
drivers/rtc/Kconfig | 10 +
drivers/rtc/Makefile | 1 +
drivers/rtc/rtc-cros-ec.c | 412 +++++++++++++++++++++
include/linux/mfd/cros_ec.h | 6 +-
include/linux/mfd/cros_ec_commands.h | 8 +
11 files changed, 619 insertions(+), 189 deletions(-)
create mode 100644 drivers/rtc/rtc-cros-ec.c
--
2.9.3
From: Stephen Barber <[email protected]>
If the EC supports RTC host commands, expose an RTC device.
Signed-off-by: Stephen Barber <[email protected]>
Signed-off-by: Enric Balletbo i Serra <[email protected]>
---
drivers/mfd/cros_ec.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
index 75a27a6..ff972fb 100644
--- a/drivers/mfd/cros_ec.c
+++ b/drivers/mfd/cros_ec.c
@@ -51,6 +51,10 @@ static const struct mfd_cell ec_pd_cell = {
.pdata_size = sizeof(pd_p),
};
+static const struct mfd_cell ec_rtc_cell = {
+ .name = "cros-ec-rtc",
+};
+
static irqreturn_t ec_irq_thread(int irq, void *data)
{
struct cros_ec_device *ec_dev = data;
@@ -245,6 +249,16 @@ static void cros_ec_sensors_register(struct cros_ec_device *ec_dev)
kfree(msg);
}
+static void cros_ec_rtc_register(struct cros_ec_device *ec_dev)
+{
+ int ret;
+
+ ret = mfd_add_devices(ec_dev->dev, PLATFORM_DEVID_AUTO, &ec_rtc_cell,
+ 1, NULL, 0, NULL);
+ if (ret)
+ dev_err(ec_dev->dev, "failed to add EC RTC\n");
+}
+
int cros_ec_register(struct cros_ec_device *ec_dev)
{
struct device *dev = ec_dev->dev;
@@ -294,6 +308,10 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
if (cros_ec_check_features(ec_dev, EC_FEATURE_MOTION_SENSE))
cros_ec_sensors_register(ec_dev);
+ /* Check whether this EC has RTC support */
+ if (cros_ec_check_features(ec_dev, EC_FEATURE_RTC))
+ cros_ec_rtc_register(ec_dev);
+
if (ec_dev->max_passthru) {
/*
* Register a PD device as well on top of this device.
--
2.9.3
From: Stephen Barber <[email protected]>
On platforms with a Chrome OS EC, the EC can function as a simple RTC.
Add a basic driver with this functionality.
Signed-off-by: Stephen Barber <[email protected]>
Signed-off-by: Enric Balletbo i Serra <[email protected]>
Acked-by: Alexandre Belloni <[email protected]>
---
drivers/rtc/Kconfig | 10 ++
drivers/rtc/Makefile | 1 +
drivers/rtc/rtc-cros-ec.c | 412 ++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 423 insertions(+)
create mode 100644 drivers/rtc/rtc-cros-ec.c
diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 8d3b957..c255f27 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -1204,6 +1204,16 @@ config RTC_DRV_ZYNQMP
If you say yes here you get support for the RTC controller found on
Xilinx Zynq Ultrascale+ MPSoC.
+config RTC_DRV_CROS_EC
+ tristate "Chrome OS EC RTC driver"
+ depends on MFD_CROS_EC
+ help
+ If you say yes here you will get support for the
+ Chrome OS Embedded Controller's RTC.
+
+ This driver can also be built as a module. If so, the module
+ will be called rtc-cros-ec.
+
comment "on-CPU RTC drivers"
config RTC_DRV_ASM9260
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index 13857d2..8162983 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -41,6 +41,7 @@ obj-$(CONFIG_RTC_DRV_BQ4802) += rtc-bq4802.o
obj-$(CONFIG_RTC_DRV_CMOS) += rtc-cmos.o
obj-$(CONFIG_RTC_DRV_COH901331) += rtc-coh901331.o
obj-$(CONFIG_RTC_DRV_CPCAP) += rtc-cpcap.o
+obj-$(CONFIG_RTC_DRV_CROS_EC) += rtc-cros-ec.o
obj-$(CONFIG_RTC_DRV_DA9052) += rtc-da9052.o
obj-$(CONFIG_RTC_DRV_DA9055) += rtc-da9055.o
obj-$(CONFIG_RTC_DRV_DA9063) += rtc-da9063.o
diff --git a/drivers/rtc/rtc-cros-ec.c b/drivers/rtc/rtc-cros-ec.c
new file mode 100644
index 0000000..a5c2512
--- /dev/null
+++ b/drivers/rtc/rtc-cros-ec.c
@@ -0,0 +1,412 @@
+/*
+ * RTC driver for Chrome OS Embedded Controller
+ *
+ * Copyright (c) 2017, Google, Inc
+ *
+ * Author: Stephen Barber <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope 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.
+ */
+
+#include <linux/kernel.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/rtc.h>
+#include <linux/slab.h>
+
+#define DRV_NAME "cros-ec-rtc"
+
+/**
+ * struct cros_ec_rtc - Driver data for EC RTC
+ *
+ * @cros_ec: Pointer to EC device
+ * @rtc: Pointer to RTC device
+ * @notifier: Notifier info for responding to EC events
+ * @saved_alarm: Alarm to restore when interrupts are reenabled
+ */
+struct cros_ec_rtc {
+ struct cros_ec_device *cros_ec;
+ struct rtc_device *rtc;
+ struct notifier_block notifier;
+ u32 saved_alarm;
+};
+
+static int cros_ec_rtc_get(struct cros_ec_device *cros_ec, u32 command,
+ u32 *response)
+{
+ int ret;
+ struct {
+ struct cros_ec_command msg;
+ struct ec_response_rtc data;
+ } __packed msg;
+
+ memset(&msg, 0, sizeof(msg));
+ msg.msg.command = command;
+ msg.msg.insize = sizeof(msg.data);
+
+ ret = cros_ec_cmd_xfer_status(cros_ec, &msg.msg);
+ if (ret < 0) {
+ dev_err(cros_ec->dev,
+ "error getting %s from EC: %d\n",
+ command == EC_CMD_RTC_GET_VALUE ? "time" : "alarm",
+ ret);
+ return ret;
+ }
+
+ *response = msg.data.time;
+
+ return 0;
+}
+
+static int cros_ec_rtc_set(struct cros_ec_device *cros_ec, u32 command,
+ u32 param)
+{
+ int ret = 0;
+ struct {
+ struct cros_ec_command msg;
+ struct ec_response_rtc data;
+ } __packed msg;
+
+ memset(&msg, 0, sizeof(msg));
+ msg.msg.command = command;
+ msg.msg.outsize = sizeof(msg.data);
+ msg.data.time = param;
+
+ ret = cros_ec_cmd_xfer_status(cros_ec, &msg.msg);
+ if (ret < 0) {
+ dev_err(cros_ec->dev, "error setting %s on EC: %d\n",
+ command == EC_CMD_RTC_SET_VALUE ? "time" : "alarm",
+ ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+/* Read the current time from the EC. */
+static int cros_ec_rtc_read_time(struct device *dev, struct rtc_time *tm)
+{
+ struct cros_ec_rtc *cros_ec_rtc = dev_get_drvdata(dev);
+ struct cros_ec_device *cros_ec = cros_ec_rtc->cros_ec;
+ int ret;
+ u32 time;
+
+ ret = cros_ec_rtc_get(cros_ec, EC_CMD_RTC_GET_VALUE, &time);
+ if (ret) {
+ dev_err(dev, "error getting time: %d\n", ret);
+ return ret;
+ }
+
+ rtc_time64_to_tm(time, tm);
+
+ return 0;
+}
+
+/* Set the current EC time. */
+static int cros_ec_rtc_set_time(struct device *dev, struct rtc_time *tm)
+{
+ struct cros_ec_rtc *cros_ec_rtc = dev_get_drvdata(dev);
+ struct cros_ec_device *cros_ec = cros_ec_rtc->cros_ec;
+ int ret;
+ time64_t time;
+
+ time = rtc_tm_to_time64(tm);
+ if (time < 0 || time > U32_MAX)
+ return -EINVAL;
+
+ ret = cros_ec_rtc_set(cros_ec, EC_CMD_RTC_SET_VALUE, (u32)time);
+ if (ret < 0) {
+ dev_err(dev, "error setting time: %d\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+/* Read alarm time from RTC. */
+static int cros_ec_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
+{
+ struct cros_ec_rtc *cros_ec_rtc = dev_get_drvdata(dev);
+ struct cros_ec_device *cros_ec = cros_ec_rtc->cros_ec;
+ int ret;
+ u32 current_time, alarm_offset;
+
+ /*
+ * The EC host command for getting the alarm is relative (i.e. 5
+ * seconds from now) whereas rtc_wkalrm is absolute. Get the current
+ * RTC time first so we can calculate the relative time.
+ */
+ ret = cros_ec_rtc_get(cros_ec, EC_CMD_RTC_GET_VALUE, ¤t_time);
+ if (ret < 0) {
+ dev_err(dev, "error getting time: %d\n", ret);
+ return ret;
+ }
+
+ ret = cros_ec_rtc_get(cros_ec, EC_CMD_RTC_GET_ALARM, &alarm_offset);
+ if (ret < 0) {
+ dev_err(dev, "error getting alarm: %d\n", ret);
+ return ret;
+ }
+
+ rtc_time64_to_tm(current_time + alarm_offset, &alrm->time);
+
+ return 0;
+}
+
+/* Set the EC's RTC alarm. */
+static int cros_ec_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
+{
+ struct cros_ec_rtc *cros_ec_rtc = dev_get_drvdata(dev);
+ struct cros_ec_device *cros_ec = cros_ec_rtc->cros_ec;
+ int ret;
+ time64_t alarm_time;
+ u32 current_time, alarm_offset;
+
+ /*
+ * The EC host command for setting the alarm is relative
+ * (i.e. 5 seconds from now) whereas rtc_wkalrm is absolute.
+ * Get the current RTC time first so we can calculate the
+ * relative time.
+ */
+ ret = cros_ec_rtc_get(cros_ec, EC_CMD_RTC_GET_VALUE, ¤t_time);
+ if (ret < 0) {
+ dev_err(dev, "error getting time: %d\n", ret);
+ return ret;
+ }
+
+ alarm_time = rtc_tm_to_time64(&alrm->time);
+
+ if (alarm_time < 0 || alarm_time > U32_MAX)
+ return -EINVAL;
+
+ if (!alrm->enabled) {
+ /*
+ * If the alarm is being disabled, send an alarm
+ * clear command.
+ */
+ alarm_offset = EC_RTC_ALARM_CLEAR;
+ cros_ec_rtc->saved_alarm = (u32)alarm_time;
+ } else {
+ /* Don't set an alarm in the past. */
+ if ((u32)alarm_time < current_time)
+ alarm_offset = EC_RTC_ALARM_CLEAR;
+ else
+ alarm_offset = (u32)alarm_time - current_time;
+ }
+
+ ret = cros_ec_rtc_set(cros_ec, EC_CMD_RTC_SET_ALARM, alarm_offset);
+ if (ret < 0) {
+ dev_err(dev, "error setting alarm: %d\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int cros_ec_rtc_alarm_irq_enable(struct device *dev,
+ unsigned int enabled)
+{
+ struct cros_ec_rtc *cros_ec_rtc = dev_get_drvdata(dev);
+ struct cros_ec_device *cros_ec = cros_ec_rtc->cros_ec;
+ int ret;
+ u32 current_time, alarm_offset, alarm_value;
+
+ ret = cros_ec_rtc_get(cros_ec, EC_CMD_RTC_GET_VALUE, ¤t_time);
+ if (ret < 0) {
+ dev_err(dev, "error getting time: %d\n", ret);
+ return ret;
+ }
+
+ if (enabled) {
+ /* Restore saved alarm if it's still in the future. */
+ if (cros_ec_rtc->saved_alarm < current_time)
+ alarm_offset = EC_RTC_ALARM_CLEAR;
+ else
+ alarm_offset = cros_ec_rtc->saved_alarm - current_time;
+
+ ret = cros_ec_rtc_set(cros_ec, EC_CMD_RTC_SET_ALARM,
+ alarm_offset);
+ if (ret < 0) {
+ dev_err(dev, "error restoring alarm: %d\n", ret);
+ return ret;
+ }
+ } else {
+ /* Disable alarm, saving the old alarm value. */
+ ret = cros_ec_rtc_get(cros_ec, EC_CMD_RTC_GET_ALARM,
+ &alarm_offset);
+ if (ret < 0) {
+ dev_err(dev, "error saving alarm: %d\n", ret);
+ return ret;
+ }
+
+ alarm_value = current_time + alarm_offset;
+
+ /*
+ * If the current EC alarm is already past, we don't want
+ * to set an alarm when we go through the alarm irq enable
+ * path.
+ */
+ if (alarm_value < current_time)
+ cros_ec_rtc->saved_alarm = EC_RTC_ALARM_CLEAR;
+ else
+ cros_ec_rtc->saved_alarm = alarm_value;
+
+ alarm_offset = EC_RTC_ALARM_CLEAR;
+ ret = cros_ec_rtc_set(cros_ec, EC_CMD_RTC_SET_ALARM,
+ alarm_offset);
+ if (ret < 0) {
+ dev_err(dev, "error disabling alarm: %d\n", ret);
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
+static int cros_ec_rtc_event(struct notifier_block *nb,
+ unsigned long queued_during_suspend,
+ void *_notify)
+{
+ struct cros_ec_rtc *cros_ec_rtc;
+ struct rtc_device *rtc;
+ struct cros_ec_device *cros_ec;
+ u32 host_event;
+
+ cros_ec_rtc = container_of(nb, struct cros_ec_rtc, notifier);
+ rtc = cros_ec_rtc->rtc;
+ cros_ec = cros_ec_rtc->cros_ec;
+
+ host_event = cros_ec_get_host_event(cros_ec);
+ if (host_event & EC_HOST_EVENT_MASK(EC_HOST_EVENT_RTC)) {
+ rtc_update_irq(rtc, 1, RTC_IRQF | RTC_AF);
+ return NOTIFY_OK;
+ } else {
+ return NOTIFY_DONE;
+ }
+}
+
+static const struct rtc_class_ops cros_ec_rtc_ops = {
+ .read_time = cros_ec_rtc_read_time,
+ .set_time = cros_ec_rtc_set_time,
+ .read_alarm = cros_ec_rtc_read_alarm,
+ .set_alarm = cros_ec_rtc_set_alarm,
+ .alarm_irq_enable = cros_ec_rtc_alarm_irq_enable,
+};
+
+#ifdef CONFIG_PM_SLEEP
+static int cros_ec_rtc_suspend(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct cros_ec_rtc *cros_ec_rtc = dev_get_drvdata(&pdev->dev);
+
+ if (device_may_wakeup(dev))
+ enable_irq_wake(cros_ec_rtc->cros_ec->irq);
+
+ return 0;
+}
+
+static int cros_ec_rtc_resume(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct cros_ec_rtc *cros_ec_rtc = dev_get_drvdata(&pdev->dev);
+
+ if (device_may_wakeup(dev))
+ disable_irq_wake(cros_ec_rtc->cros_ec->irq);
+
+ return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(cros_ec_rtc_pm_ops, cros_ec_rtc_suspend,
+ cros_ec_rtc_resume);
+
+static int cros_ec_rtc_probe(struct platform_device *pdev)
+{
+ struct cros_ec_device *cros_ec_dev = dev_get_drvdata(pdev->dev.parent);
+ struct cros_ec_rtc *cros_ec_rtc;
+ struct rtc_time tm;
+ int ret;
+
+ cros_ec_rtc = devm_kzalloc(&pdev->dev, sizeof(*cros_ec_rtc),
+ GFP_KERNEL);
+ if (!cros_ec_rtc)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, cros_ec_rtc);
+ cros_ec_rtc->cros_ec = cros_ec_dev;
+
+ /* Get initial time */
+ ret = cros_ec_rtc_read_time(&pdev->dev, &tm);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to read RTC time\n");
+ return ret;
+ }
+
+ ret = device_init_wakeup(&pdev->dev, 1);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to initialize wakeup\n");
+ return ret;
+ }
+
+ cros_ec_rtc->rtc = devm_rtc_device_register(&pdev->dev, DRV_NAME,
+ &cros_ec_rtc_ops,
+ THIS_MODULE);
+ if (IS_ERR(cros_ec_rtc->rtc)) {
+ ret = PTR_ERR(cros_ec_rtc->rtc);
+ dev_err(&pdev->dev, "failed to register rtc device\n");
+ return ret;
+ }
+
+ /* Get RTC events from the EC. */
+ cros_ec_rtc->notifier.notifier_call = cros_ec_rtc_event;
+ ret = blocking_notifier_chain_register(&cros_ec_dev->event_notifier,
+ &cros_ec_rtc->notifier);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to register notifier\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+static int cros_ec_rtc_remove(struct platform_device *pdev)
+{
+ struct cros_ec_rtc *cros_ec_rtc = platform_get_drvdata(pdev);
+ struct device *dev = &pdev->dev;
+ int ret;
+
+ ret = blocking_notifier_chain_unregister(
+ &cros_ec_rtc->cros_ec->event_notifier,
+ &cros_ec_rtc->notifier);
+ if (ret) {
+ dev_err(dev, "failed to unregister notifier\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+static struct platform_driver cros_ec_rtc_driver = {
+ .probe = cros_ec_rtc_probe,
+ .remove = cros_ec_rtc_remove,
+ .driver = {
+ .name = DRV_NAME,
+ .pm = &cros_ec_rtc_pm_ops,
+ },
+};
+
+module_platform_driver(cros_ec_rtc_driver);
+
+MODULE_DESCRIPTION("RTC driver for Chrome OS ECs");
+MODULE_AUTHOR("Stephen Barber <[email protected]>");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:" DRV_NAME);
--
2.9.3
From: Stephen Barber <[email protected]>
The EC can function as a simple RT, this patch adds the RTC related
definitions needed by the rtc-cros-ec driver.
Signed-off-by: Stephen Barber <[email protected]>
Signed-off-by: Enric Balletbo i Serra <[email protected]>
Acked-by: Lee Jones <[email protected]>
---
include/linux/mfd/cros_ec_commands.h | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/include/linux/mfd/cros_ec_commands.h b/include/linux/mfd/cros_ec_commands.h
index 190c8f4..26ef492 100644
--- a/include/linux/mfd/cros_ec_commands.h
+++ b/include/linux/mfd/cros_ec_commands.h
@@ -286,6 +286,9 @@ enum host_event_code {
/* Hang detect logic detected a hang and warm rebooted the AP */
EC_HOST_EVENT_HANG_REBOOT = 21,
+ /* EC RTC event occurred */
+ EC_HOST_EVENT_RTC = 26,
+
/*
* The high bit of the event mask is not used as a host event code. If
* it reads back as set, then the entire event mask should be
@@ -794,6 +797,8 @@ enum ec_feature_code {
EC_FEATURE_USB_MUX = 23,
/* Motion Sensor code has an internal software FIFO */
EC_FEATURE_MOTION_SENSE_FIFO = 24,
+ /* EC has RTC feature that can be controlled by host commands */
+ EC_FEATURE_RTC = 27,
};
#define EC_FEATURE_MASK_0(event_code) (1UL << (event_code % 32))
@@ -1704,6 +1709,9 @@ struct ec_response_rtc {
#define EC_CMD_RTC_SET_VALUE 0x46
#define EC_CMD_RTC_SET_ALARM 0x47
+/* Pass as param to SET_ALARM to clear the current alarm */
+#define EC_RTC_ALARM_CLEAR 0
+
/*****************************************************************************/
/* Port80 log access */
--
2.9.3
The cros_ec_dev driver should be used only to expose the Chrome OS Embedded
Controller to user-space and should not be used to add MFD devices by
calling mfd_add_devices. This patch moves this logic to the MFD cros_ec
driver and removes the MFD bits from the character device driver. Also
makes independent the IIO driver from the character device as also has no
sense.
Signed-off-by: Enric Balletbo i Serra <[email protected]>
Acked-by: Jonathan Cameron <[email protected]>
---
.../iio/common/cros_ec_sensors/cros_ec_sensors.c | 8 -
.../common/cros_ec_sensors/cros_ec_sensors_core.c | 8 +-
drivers/iio/light/cros_ec_light_prox.c | 8 -
drivers/iio/pressure/cros_ec_baro.c | 8 -
drivers/mfd/cros_ec.c | 160 ++++++++++++++++++++
drivers/platform/chrome/cros_ec_dev.c | 161 ---------------------
include/linux/mfd/cros_ec.h | 6 +-
7 files changed, 170 insertions(+), 189 deletions(-)
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..9b53a01 100644
--- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
+++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
@@ -191,19 +191,11 @@ static const struct iio_info ec_sensors_info = {
static int cros_ec_sensors_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
- struct cros_ec_dev *ec_dev = dev_get_drvdata(dev->parent);
- struct cros_ec_device *ec_device;
struct iio_dev *indio_dev;
struct cros_ec_sensors_state *state;
struct iio_chan_spec *channel;
int ret, i;
- if (!ec_dev || !ec_dev->ec_dev) {
- dev_warn(&pdev->dev, "No CROS EC device found.\n");
- return -EINVAL;
- }
- ec_device = ec_dev->ec_dev;
-
indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*state));
if (!indio_dev)
return -ENOMEM;
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..0cdb64a 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
@@ -41,12 +41,13 @@ int cros_ec_sensors_core_init(struct platform_device *pdev,
{
struct device *dev = &pdev->dev;
struct cros_ec_sensors_core_state *state = iio_priv(indio_dev);
- struct cros_ec_dev *ec = dev_get_drvdata(pdev->dev.parent);
+ struct cros_ec_device *ec_dev = dev_get_drvdata(pdev->dev.parent);
struct cros_ec_sensor_platform *sensor_platform = dev_get_platdata(dev);
platform_set_drvdata(pdev, indio_dev);
- state->ec = ec->ec_dev;
+ state->ec = ec_dev;
+
state->msg = devm_kzalloc(&pdev->dev,
max((u16)sizeof(struct ec_params_motion_sense),
state->ec->max_response), GFP_KERNEL);
@@ -59,7 +60,8 @@ int cros_ec_sensors_core_init(struct platform_device *pdev,
/* Set up the host command structure. */
state->msg->version = 2;
- state->msg->command = EC_CMD_MOTION_SENSE_CMD + ec->cmd_offset;
+ state->msg->command = EC_CMD_MOTION_SENSE_CMD +
+ sensor_platform->cmd_offset;
state->msg->outsize = sizeof(struct ec_params_motion_sense);
indio_dev->dev.parent = &pdev->dev;
diff --git a/drivers/iio/light/cros_ec_light_prox.c b/drivers/iio/light/cros_ec_light_prox.c
index 7217223..2133ddc 100644
--- a/drivers/iio/light/cros_ec_light_prox.c
+++ b/drivers/iio/light/cros_ec_light_prox.c
@@ -181,19 +181,11 @@ static const struct iio_info cros_ec_light_prox_info = {
static int cros_ec_light_prox_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
- struct cros_ec_dev *ec_dev = dev_get_drvdata(dev->parent);
- struct cros_ec_device *ec_device;
struct iio_dev *indio_dev;
struct cros_ec_light_prox_state *state;
struct iio_chan_spec *channel;
int ret;
- if (!ec_dev || !ec_dev->ec_dev) {
- dev_warn(dev, "No CROS EC device found.\n");
- return -EINVAL;
- }
- ec_device = ec_dev->ec_dev;
-
indio_dev = devm_iio_device_alloc(dev, sizeof(*state));
if (!indio_dev)
return -ENOMEM;
diff --git a/drivers/iio/pressure/cros_ec_baro.c b/drivers/iio/pressure/cros_ec_baro.c
index 48b2a30..dbea18b 100644
--- a/drivers/iio/pressure/cros_ec_baro.c
+++ b/drivers/iio/pressure/cros_ec_baro.c
@@ -126,19 +126,11 @@ static const struct iio_info cros_ec_baro_info = {
static int cros_ec_baro_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
- struct cros_ec_dev *ec_dev = dev_get_drvdata(dev->parent);
- struct cros_ec_device *ec_device;
struct iio_dev *indio_dev;
struct cros_ec_baro_state *state;
struct iio_chan_spec *channel;
int ret;
- if (!ec_dev || !ec_dev->ec_dev) {
- dev_warn(dev, "No CROS EC device found.\n");
- return -EINVAL;
- }
- ec_device = ec_dev->ec_dev;
-
indio_dev = devm_iio_device_alloc(dev, sizeof(*state));
if (!indio_dev)
return -ENOMEM;
diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
index b0ca5a4c..75a27a6 100644
--- a/drivers/mfd/cros_ec.c
+++ b/drivers/mfd/cros_ec.c
@@ -91,6 +91,160 @@ static int cros_ec_sleep_event(struct cros_ec_device *ec_dev, u8 sleep_event)
return cros_ec_cmd_xfer(ec_dev, &buf.msg);
}
+static int cros_ec_check_features(struct cros_ec_device *ec_dev, int feature)
+{
+ struct cros_ec_command *msg;
+ int ret;
+
+ if (ec_dev->features[0] == -1U && ec_dev->features[1] == -1U) {
+ /* features bitmap not read yet */
+
+ msg = kmalloc(sizeof(*msg) + sizeof(ec_dev->features),
+ GFP_KERNEL);
+ if (!msg)
+ return -ENOMEM;
+
+ msg->version = 0;
+ msg->command = EC_CMD_GET_FEATURES + ec_p.cmd_offset;
+ msg->insize = sizeof(ec_dev->features);
+ msg->outsize = 0;
+
+ ret = cros_ec_cmd_xfer(ec_dev, msg);
+ if (ret < 0 || msg->result != EC_RES_SUCCESS) {
+ dev_warn(ec_dev->dev, "cannot get EC features: %d/%d\n",
+ ret, msg->result);
+ memset(ec_dev->features, 0, sizeof(ec_dev->features));
+ }
+
+ memcpy(ec_dev->features, msg->data, sizeof(ec_dev->features));
+
+ dev_dbg(ec_dev->dev, "EC features %08x %08x\n",
+ ec_dev->features[0], ec_dev->features[1]);
+
+ kfree(msg);
+ }
+
+ return ec_dev->features[feature / 32] & EC_FEATURE_MASK_0(feature);
+}
+
+static void cros_ec_sensors_register(struct cros_ec_device *ec_dev)
+{
+ /*
+ * Issue a command to get the number of sensor reported.
+ * Build an array of sensors driver and register them all.
+ */
+ int ret, i, id, sensor_num;
+ struct mfd_cell *sensor_cells;
+ struct cros_ec_sensor_platform *sensor_platforms;
+ int sensor_type[MOTIONSENSE_TYPE_MAX];
+ struct ec_params_motion_sense *params;
+ struct ec_response_motion_sense *resp;
+ struct cros_ec_command *msg;
+
+ msg = kzalloc(sizeof(struct cros_ec_command) +
+ max(sizeof(*params), sizeof(*resp)), GFP_KERNEL);
+ if (msg == NULL)
+ return;
+
+ msg->version = 2;
+ msg->command = EC_CMD_MOTION_SENSE_CMD + ec_p.cmd_offset;
+ msg->outsize = sizeof(*params);
+ msg->insize = sizeof(*resp);
+
+ params = (struct ec_params_motion_sense *)msg->data;
+ params->cmd = MOTIONSENSE_CMD_DUMP;
+
+ ret = cros_ec_cmd_xfer(ec_dev, msg);
+ if (ret < 0 || msg->result != EC_RES_SUCCESS) {
+ dev_warn(ec_dev->dev, "cannot get EC sensor information: %d/%d\n",
+ ret, msg->result);
+ goto error;
+ }
+
+ 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),
+ GFP_KERNEL);
+ if (sensor_cells == NULL)
+ goto error;
+
+ sensor_platforms = kzalloc(sizeof(struct cros_ec_sensor_platform) *
+ (sensor_num + 1), GFP_KERNEL);
+ if (sensor_platforms == NULL)
+ goto error_platforms;
+
+ memset(sensor_type, 0, sizeof(sensor_type));
+ id = 0;
+ for (i = 0; i < sensor_num; i++) {
+ params->cmd = MOTIONSENSE_CMD_INFO;
+ params->info.sensor_num = i;
+ ret = cros_ec_cmd_xfer(ec_dev, msg);
+ if (ret < 0 || msg->result != EC_RES_SUCCESS) {
+ dev_warn(ec_dev->dev, "no info for EC sensor %d : %d/%d\n",
+ i, ret, msg->result);
+ continue;
+ }
+ switch (resp->info.type) {
+ case MOTIONSENSE_TYPE_ACCEL:
+ sensor_cells[id].name = "cros-ec-accel";
+ break;
+ case MOTIONSENSE_TYPE_BARO:
+ sensor_cells[id].name = "cros-ec-baro";
+ break;
+ case MOTIONSENSE_TYPE_GYRO:
+ sensor_cells[id].name = "cros-ec-gyro";
+ break;
+ case MOTIONSENSE_TYPE_MAG:
+ sensor_cells[id].name = "cros-ec-mag";
+ break;
+ case MOTIONSENSE_TYPE_PROX:
+ sensor_cells[id].name = "cros-ec-prox";
+ break;
+ case MOTIONSENSE_TYPE_LIGHT:
+ sensor_cells[id].name = "cros-ec-light";
+ break;
+ case MOTIONSENSE_TYPE_ACTIVITY:
+ sensor_cells[id].name = "cros-ec-activity";
+ break;
+ default:
+ dev_warn(ec_dev->dev, "unknown type %d\n",
+ resp->info.type);
+ continue;
+ }
+ sensor_platforms[id].sensor_num = i;
+ sensor_platforms[id].cmd_offset = ec_p.cmd_offset;
+ sensor_cells[id].id = sensor_type[resp->info.type];
+ sensor_cells[id].platform_data = &sensor_platforms[id];
+ sensor_cells[id].pdata_size =
+ sizeof(struct cros_ec_sensor_platform);
+
+ 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++;
+ }
+
+ ret = mfd_add_devices(ec_dev->dev, PLATFORM_DEVID_AUTO, sensor_cells,
+ id, NULL, 0, NULL);
+ if (ret)
+ dev_err(ec_dev->dev, "failed to add EC sensors\n");
+
+ kfree(sensor_platforms);
+error_platforms:
+ kfree(sensor_cells);
+error:
+ kfree(msg);
+}
+
int cros_ec_register(struct cros_ec_device *ec_dev)
{
struct device *dev = ec_dev->dev;
@@ -101,6 +255,8 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
ec_dev->max_request = sizeof(struct ec_params_hello);
ec_dev->max_response = sizeof(struct ec_response_get_protocol_info);
ec_dev->max_passthru = 0;
+ ec_dev->features[0] = -1U; /* Not cached yet */
+ ec_dev->features[1] = -1U; /* Not cached yet */
ec_dev->din = devm_kzalloc(dev, ec_dev->din_size, GFP_KERNEL);
if (!ec_dev->din)
@@ -134,6 +290,10 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
goto fail_mfd;
}
+ /* Check whether this EC is a sensor hub. */
+ if (cros_ec_check_features(ec_dev, EC_FEATURE_MOTION_SENSE))
+ cros_ec_sensors_register(ec_dev);
+
if (ec_dev->max_passthru) {
/*
* Register a PD device as well on top of this device.
diff --git a/drivers/platform/chrome/cros_ec_dev.c b/drivers/platform/chrome/cros_ec_dev.c
index cf6c4f0..bd07df5 100644
--- a/drivers/platform/chrome/cros_ec_dev.c
+++ b/drivers/platform/chrome/cros_ec_dev.c
@@ -90,41 +90,6 @@ static int ec_get_version(struct cros_ec_dev *ec, char *str, int maxlen)
return ret;
}
-static int cros_ec_check_features(struct cros_ec_dev *ec, int feature)
-{
- struct cros_ec_command *msg;
- int ret;
-
- if (ec->features[0] == -1U && ec->features[1] == -1U) {
- /* features bitmap not read yet */
-
- msg = kmalloc(sizeof(*msg) + sizeof(ec->features), GFP_KERNEL);
- if (!msg)
- return -ENOMEM;
-
- msg->version = 0;
- msg->command = EC_CMD_GET_FEATURES + ec->cmd_offset;
- msg->insize = sizeof(ec->features);
- msg->outsize = 0;
-
- ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
- if (ret < 0 || msg->result != EC_RES_SUCCESS) {
- dev_warn(ec->dev, "cannot get EC features: %d/%d\n",
- ret, msg->result);
- memset(ec->features, 0, sizeof(ec->features));
- }
-
- memcpy(ec->features, msg->data, sizeof(ec->features));
-
- dev_dbg(ec->dev, "EC features %08x %08x\n",
- ec->features[0], ec->features[1]);
-
- kfree(msg);
- }
-
- return ec->features[feature / 32] & EC_FEATURE_MASK_0(feature);
-}
-
/* Device file ops */
static int ec_device_open(struct inode *inode, struct file *filp)
{
@@ -268,126 +233,6 @@ static void __remove(struct device *dev)
kfree(ec);
}
-static void cros_ec_sensors_register(struct cros_ec_dev *ec)
-{
- /*
- * Issue a command to get the number of sensor reported.
- * Build an array of sensors driver and register them all.
- */
- int ret, i, id, sensor_num;
- struct mfd_cell *sensor_cells;
- struct cros_ec_sensor_platform *sensor_platforms;
- int sensor_type[MOTIONSENSE_TYPE_MAX];
- struct ec_params_motion_sense *params;
- struct ec_response_motion_sense *resp;
- struct cros_ec_command *msg;
-
- msg = kzalloc(sizeof(struct cros_ec_command) +
- max(sizeof(*params), sizeof(*resp)), GFP_KERNEL);
- if (msg == NULL)
- return;
-
- msg->version = 2;
- msg->command = EC_CMD_MOTION_SENSE_CMD + ec->cmd_offset;
- msg->outsize = sizeof(*params);
- msg->insize = sizeof(*resp);
-
- params = (struct ec_params_motion_sense *)msg->data;
- params->cmd = MOTIONSENSE_CMD_DUMP;
-
- ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
- if (ret < 0 || msg->result != EC_RES_SUCCESS) {
- dev_warn(ec->dev, "cannot get EC sensor information: %d/%d\n",
- ret, msg->result);
- goto error;
- }
-
- 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),
- GFP_KERNEL);
- if (sensor_cells == NULL)
- goto error;
-
- sensor_platforms = kzalloc(sizeof(struct cros_ec_sensor_platform) *
- (sensor_num + 1), GFP_KERNEL);
- if (sensor_platforms == NULL)
- goto error_platforms;
-
- memset(sensor_type, 0, sizeof(sensor_type));
- id = 0;
- for (i = 0; i < sensor_num; i++) {
- params->cmd = MOTIONSENSE_CMD_INFO;
- params->info.sensor_num = i;
- ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
- if (ret < 0 || msg->result != EC_RES_SUCCESS) {
- dev_warn(ec->dev, "no info for EC sensor %d : %d/%d\n",
- i, ret, msg->result);
- continue;
- }
- switch (resp->info.type) {
- case MOTIONSENSE_TYPE_ACCEL:
- sensor_cells[id].name = "cros-ec-accel";
- break;
- case MOTIONSENSE_TYPE_BARO:
- sensor_cells[id].name = "cros-ec-baro";
- break;
- case MOTIONSENSE_TYPE_GYRO:
- sensor_cells[id].name = "cros-ec-gyro";
- break;
- case MOTIONSENSE_TYPE_MAG:
- sensor_cells[id].name = "cros-ec-mag";
- break;
- case MOTIONSENSE_TYPE_PROX:
- sensor_cells[id].name = "cros-ec-prox";
- break;
- case MOTIONSENSE_TYPE_LIGHT:
- sensor_cells[id].name = "cros-ec-light";
- break;
- case MOTIONSENSE_TYPE_ACTIVITY:
- sensor_cells[id].name = "cros-ec-activity";
- break;
- default:
- dev_warn(ec->dev, "unknown type %d\n", resp->info.type);
- continue;
- }
- sensor_platforms[id].sensor_num = i;
- sensor_cells[id].id = sensor_type[resp->info.type];
- sensor_cells[id].platform_data = &sensor_platforms[id];
- sensor_cells[id].pdata_size =
- sizeof(struct cros_ec_sensor_platform);
-
- 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 (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE_FIFO)) {
- sensor_cells[id].name = "cros-ec-ring";
- id++;
- }
-
- ret = mfd_add_devices(ec->dev, 0, sensor_cells, id,
- NULL, 0, NULL);
- if (ret)
- dev_err(ec->dev, "failed to add EC sensors\n");
-
- kfree(sensor_platforms);
-error_platforms:
- kfree(sensor_cells);
-error:
- kfree(msg);
-}
-
static int ec_device_probe(struct platform_device *pdev)
{
int retval = -ENOMEM;
@@ -402,8 +247,6 @@ static int ec_device_probe(struct platform_device *pdev)
ec->ec_dev = dev_get_drvdata(dev->parent);
ec->dev = dev;
ec->cmd_offset = ec_platform->cmd_offset;
- ec->features[0] = -1U; /* Not cached yet */
- ec->features[1] = -1U; /* Not cached yet */
device_initialize(&ec->class_dev);
cdev_init(&ec->cdev, &fops);
@@ -432,10 +275,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);
diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
index 4e887ba..a4138a5 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -115,6 +115,7 @@ struct cros_ec_command {
* @event_notifier: interrupt event notifier for transport devices.
* @event_data: raw payload transferred with the MKBP event.
* @event_size: size in bytes of the event data.
+ * @features: stores the EC features.
*/
struct cros_ec_device {
@@ -150,15 +151,19 @@ struct cros_ec_device {
struct ec_response_get_next_event event_data;
int event_size;
u32 host_event_wake_mask;
+ u32 features[2];
};
/**
* struct cros_ec_sensor_platform - ChromeOS EC sensor platform information
*
* @sensor_num: Id of the sensor, as reported by the EC.
+ * @cmd_offset: offset to apply for each command. Set when
+ * registering a devicde behind another one.
*/
struct cros_ec_sensor_platform {
u8 sensor_num;
+ u16 cmd_offset;
};
/* struct cros_ec_platform - ChromeOS EC platform information
@@ -192,7 +197,6 @@ struct cros_ec_dev {
struct device *dev;
struct cros_ec_debugfs *debug_info;
u16 cmd_offset;
- u32 features[2];
};
/**
--
2.9.3
Hi Enric,
On 07/12/2017 03:13 AM, Enric Balletbo i Serra wrote:
> From: Stephen Barber <[email protected]>
>
> The EC can function as a simple RT, this patch adds the RTC related
> definitions needed by the rtc-cros-ec driver.
>
> Signed-off-by: Stephen Barber <[email protected]>
> Signed-off-by: Enric Balletbo i Serra <[email protected]>
> Acked-by: Lee Jones <[email protected]>
Acked-by: Benson Leung <[email protected]>
--
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
[email protected]
Chromium OS Project
[email protected]
Hi Enric,
On 07/12/2017 03:13 AM, Enric Balletbo i Serra wrote:
> From: Stephen Barber <[email protected]>
>
> On platforms with a Chrome OS EC, the EC can function as a simple RTC.
> Add a basic driver with this functionality.
>
> Signed-off-by: Stephen Barber <[email protected]>
> Signed-off-by: Enric Balletbo i Serra <[email protected]>
> Acked-by: Alexandre Belloni <[email protected]>
Acked-by: Benson Leung <[email protected]>
Thanks!
Benson
--
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
[email protected]
Chromium OS Project
[email protected]
On Wed, Jul 12, 2017 at 3:13 AM, Enric Balletbo i Serra
<[email protected]> wrote:
> The cros_ec_dev driver should be used only to expose the Chrome OS Embedded
> Controller to user-space and should not be used to add MFD devices by
> calling mfd_add_devices. This patch moves this logic to the MFD cros_ec
> driver and removes the MFD bits from the character device driver. Also
> makes independent the IIO driver from the character device as also has no
> sense.
cros_ec_dev serves another purpose: it allows to represent an EC that
does not have a cros_ec structure. It happens when there are several
EC in a chromebook, and one EC is connected through another EC.
One example is Samus (Pixel 2): where we have:
(main SOC, Application Processor) AP --> (main Embedded Controller) EC
---> (Power Delivery [PD}) EC
We access to the PD EC via pass-through commands through the main EC.
Each EC has a cros_ec_dev structure, but only the main EC as a
cros_ec_device structure (I will forever regret the structure names).
Now form the AP point of view, both ECs use the same protocol. That
why the sensors and other devcies that are registered by looking at
the feature fields are registered with cros_ec_dev as their parent.
Other devices that are registered from the device tree, predating the
feature field support, are registered with cros_ec_device as their
parent.
Gwendal.
>
> Signed-off-by: Enric Balletbo i Serra <[email protected]>
> Acked-by: Jonathan Cameron <[email protected]>
> ---
> .../iio/common/cros_ec_sensors/cros_ec_sensors.c | 8 -
> .../common/cros_ec_sensors/cros_ec_sensors_core.c | 8 +-
> drivers/iio/light/cros_ec_light_prox.c | 8 -
> drivers/iio/pressure/cros_ec_baro.c | 8 -
> drivers/mfd/cros_ec.c | 160 ++++++++++++++++++++
> drivers/platform/chrome/cros_ec_dev.c | 161 ---------------------
> include/linux/mfd/cros_ec.h | 6 +-
> 7 files changed, 170 insertions(+), 189 deletions(-)
>
> 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..9b53a01 100644
> --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
> +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
> @@ -191,19 +191,11 @@ static const struct iio_info ec_sensors_info = {
> static int cros_ec_sensors_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> - struct cros_ec_dev *ec_dev = dev_get_drvdata(dev->parent);
> - struct cros_ec_device *ec_device;
> struct iio_dev *indio_dev;
> struct cros_ec_sensors_state *state;
> struct iio_chan_spec *channel;
> int ret, i;
>
> - if (!ec_dev || !ec_dev->ec_dev) {
> - dev_warn(&pdev->dev, "No CROS EC device found.\n");
> - return -EINVAL;
> - }
> - ec_device = ec_dev->ec_dev;
> -
> indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*state));
> if (!indio_dev)
> return -ENOMEM;
> 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..0cdb64a 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
> @@ -41,12 +41,13 @@ int cros_ec_sensors_core_init(struct platform_device *pdev,
> {
> struct device *dev = &pdev->dev;
> struct cros_ec_sensors_core_state *state = iio_priv(indio_dev);
> - struct cros_ec_dev *ec = dev_get_drvdata(pdev->dev.parent);
> + struct cros_ec_device *ec_dev = dev_get_drvdata(pdev->dev.parent);
> struct cros_ec_sensor_platform *sensor_platform = dev_get_platdata(dev);
>
> platform_set_drvdata(pdev, indio_dev);
>
> - state->ec = ec->ec_dev;
> + state->ec = ec_dev;
> +
> state->msg = devm_kzalloc(&pdev->dev,
> max((u16)sizeof(struct ec_params_motion_sense),
> state->ec->max_response), GFP_KERNEL);
> @@ -59,7 +60,8 @@ int cros_ec_sensors_core_init(struct platform_device *pdev,
>
> /* Set up the host command structure. */
> state->msg->version = 2;
> - state->msg->command = EC_CMD_MOTION_SENSE_CMD + ec->cmd_offset;
> + state->msg->command = EC_CMD_MOTION_SENSE_CMD +
> + sensor_platform->cmd_offset;
> state->msg->outsize = sizeof(struct ec_params_motion_sense);
>
> indio_dev->dev.parent = &pdev->dev;
> diff --git a/drivers/iio/light/cros_ec_light_prox.c b/drivers/iio/light/cros_ec_light_prox.c
> index 7217223..2133ddc 100644
> --- a/drivers/iio/light/cros_ec_light_prox.c
> +++ b/drivers/iio/light/cros_ec_light_prox.c
> @@ -181,19 +181,11 @@ static const struct iio_info cros_ec_light_prox_info = {
> static int cros_ec_light_prox_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> - struct cros_ec_dev *ec_dev = dev_get_drvdata(dev->parent);
> - struct cros_ec_device *ec_device;
> struct iio_dev *indio_dev;
> struct cros_ec_light_prox_state *state;
> struct iio_chan_spec *channel;
> int ret;
>
> - if (!ec_dev || !ec_dev->ec_dev) {
> - dev_warn(dev, "No CROS EC device found.\n");
> - return -EINVAL;
> - }
> - ec_device = ec_dev->ec_dev;
> -
> indio_dev = devm_iio_device_alloc(dev, sizeof(*state));
> if (!indio_dev)
> return -ENOMEM;
> diff --git a/drivers/iio/pressure/cros_ec_baro.c b/drivers/iio/pressure/cros_ec_baro.c
> index 48b2a30..dbea18b 100644
> --- a/drivers/iio/pressure/cros_ec_baro.c
> +++ b/drivers/iio/pressure/cros_ec_baro.c
> @@ -126,19 +126,11 @@ static const struct iio_info cros_ec_baro_info = {
> static int cros_ec_baro_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> - struct cros_ec_dev *ec_dev = dev_get_drvdata(dev->parent);
> - struct cros_ec_device *ec_device;
> struct iio_dev *indio_dev;
> struct cros_ec_baro_state *state;
> struct iio_chan_spec *channel;
> int ret;
>
> - if (!ec_dev || !ec_dev->ec_dev) {
> - dev_warn(dev, "No CROS EC device found.\n");
> - return -EINVAL;
> - }
> - ec_device = ec_dev->ec_dev;
> -
> indio_dev = devm_iio_device_alloc(dev, sizeof(*state));
> if (!indio_dev)
> return -ENOMEM;
> diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
> index b0ca5a4c..75a27a6 100644
> --- a/drivers/mfd/cros_ec.c
> +++ b/drivers/mfd/cros_ec.c
> @@ -91,6 +91,160 @@ static int cros_ec_sleep_event(struct cros_ec_device *ec_dev, u8 sleep_event)
> return cros_ec_cmd_xfer(ec_dev, &buf.msg);
> }
>
> +static int cros_ec_check_features(struct cros_ec_device *ec_dev, int feature)
> +{
> + struct cros_ec_command *msg;
> + int ret;
> +
> + if (ec_dev->features[0] == -1U && ec_dev->features[1] == -1U) {
> + /* features bitmap not read yet */
> +
> + msg = kmalloc(sizeof(*msg) + sizeof(ec_dev->features),
> + GFP_KERNEL);
> + if (!msg)
> + return -ENOMEM;
> +
> + msg->version = 0;
> + msg->command = EC_CMD_GET_FEATURES + ec_p.cmd_offset;
> + msg->insize = sizeof(ec_dev->features);
> + msg->outsize = 0;
> +
> + ret = cros_ec_cmd_xfer(ec_dev, msg);
> + if (ret < 0 || msg->result != EC_RES_SUCCESS) {
> + dev_warn(ec_dev->dev, "cannot get EC features: %d/%d\n",
> + ret, msg->result);
> + memset(ec_dev->features, 0, sizeof(ec_dev->features));
> + }
> +
> + memcpy(ec_dev->features, msg->data, sizeof(ec_dev->features));
> +
> + dev_dbg(ec_dev->dev, "EC features %08x %08x\n",
> + ec_dev->features[0], ec_dev->features[1]);
> +
> + kfree(msg);
> + }
> +
> + return ec_dev->features[feature / 32] & EC_FEATURE_MASK_0(feature);
> +}
> +
> +static void cros_ec_sensors_register(struct cros_ec_device *ec_dev)
> +{
> + /*
> + * Issue a command to get the number of sensor reported.
> + * Build an array of sensors driver and register them all.
> + */
> + int ret, i, id, sensor_num;
> + struct mfd_cell *sensor_cells;
> + struct cros_ec_sensor_platform *sensor_platforms;
> + int sensor_type[MOTIONSENSE_TYPE_MAX];
> + struct ec_params_motion_sense *params;
> + struct ec_response_motion_sense *resp;
> + struct cros_ec_command *msg;
> +
> + msg = kzalloc(sizeof(struct cros_ec_command) +
> + max(sizeof(*params), sizeof(*resp)), GFP_KERNEL);
> + if (msg == NULL)
> + return;
> +
> + msg->version = 2;
> + msg->command = EC_CMD_MOTION_SENSE_CMD + ec_p.cmd_offset;
> + msg->outsize = sizeof(*params);
> + msg->insize = sizeof(*resp);
> +
> + params = (struct ec_params_motion_sense *)msg->data;
> + params->cmd = MOTIONSENSE_CMD_DUMP;
> +
> + ret = cros_ec_cmd_xfer(ec_dev, msg);
> + if (ret < 0 || msg->result != EC_RES_SUCCESS) {
> + dev_warn(ec_dev->dev, "cannot get EC sensor information: %d/%d\n",
> + ret, msg->result);
> + goto error;
> + }
> +
> + 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),
> + GFP_KERNEL);
> + if (sensor_cells == NULL)
> + goto error;
> +
> + sensor_platforms = kzalloc(sizeof(struct cros_ec_sensor_platform) *
> + (sensor_num + 1), GFP_KERNEL);
> + if (sensor_platforms == NULL)
> + goto error_platforms;
> +
> + memset(sensor_type, 0, sizeof(sensor_type));
> + id = 0;
> + for (i = 0; i < sensor_num; i++) {
> + params->cmd = MOTIONSENSE_CMD_INFO;
> + params->info.sensor_num = i;
> + ret = cros_ec_cmd_xfer(ec_dev, msg);
> + if (ret < 0 || msg->result != EC_RES_SUCCESS) {
> + dev_warn(ec_dev->dev, "no info for EC sensor %d : %d/%d\n",
> + i, ret, msg->result);
> + continue;
> + }
> + switch (resp->info.type) {
> + case MOTIONSENSE_TYPE_ACCEL:
> + sensor_cells[id].name = "cros-ec-accel";
> + break;
> + case MOTIONSENSE_TYPE_BARO:
> + sensor_cells[id].name = "cros-ec-baro";
> + break;
> + case MOTIONSENSE_TYPE_GYRO:
> + sensor_cells[id].name = "cros-ec-gyro";
> + break;
> + case MOTIONSENSE_TYPE_MAG:
> + sensor_cells[id].name = "cros-ec-mag";
> + break;
> + case MOTIONSENSE_TYPE_PROX:
> + sensor_cells[id].name = "cros-ec-prox";
> + break;
> + case MOTIONSENSE_TYPE_LIGHT:
> + sensor_cells[id].name = "cros-ec-light";
> + break;
> + case MOTIONSENSE_TYPE_ACTIVITY:
> + sensor_cells[id].name = "cros-ec-activity";
> + break;
> + default:
> + dev_warn(ec_dev->dev, "unknown type %d\n",
> + resp->info.type);
> + continue;
> + }
> + sensor_platforms[id].sensor_num = i;
> + sensor_platforms[id].cmd_offset = ec_p.cmd_offset;
> + sensor_cells[id].id = sensor_type[resp->info.type];
> + sensor_cells[id].platform_data = &sensor_platforms[id];
> + sensor_cells[id].pdata_size =
> + sizeof(struct cros_ec_sensor_platform);
> +
> + 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++;
> + }
> +
> + ret = mfd_add_devices(ec_dev->dev, PLATFORM_DEVID_AUTO, sensor_cells,
> + id, NULL, 0, NULL);
> + if (ret)
> + dev_err(ec_dev->dev, "failed to add EC sensors\n");
> +
> + kfree(sensor_platforms);
> +error_platforms:
> + kfree(sensor_cells);
> +error:
> + kfree(msg);
> +}
> +
> int cros_ec_register(struct cros_ec_device *ec_dev)
> {
> struct device *dev = ec_dev->dev;
> @@ -101,6 +255,8 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
> ec_dev->max_request = sizeof(struct ec_params_hello);
> ec_dev->max_response = sizeof(struct ec_response_get_protocol_info);
> ec_dev->max_passthru = 0;
> + ec_dev->features[0] = -1U; /* Not cached yet */
> + ec_dev->features[1] = -1U; /* Not cached yet */
>
> ec_dev->din = devm_kzalloc(dev, ec_dev->din_size, GFP_KERNEL);
> if (!ec_dev->din)
> @@ -134,6 +290,10 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
> goto fail_mfd;
> }
>
> + /* Check whether this EC is a sensor hub. */
> + if (cros_ec_check_features(ec_dev, EC_FEATURE_MOTION_SENSE))
> + cros_ec_sensors_register(ec_dev);
> +
> if (ec_dev->max_passthru) {
> /*
> * Register a PD device as well on top of this device.
> diff --git a/drivers/platform/chrome/cros_ec_dev.c b/drivers/platform/chrome/cros_ec_dev.c
> index cf6c4f0..bd07df5 100644
> --- a/drivers/platform/chrome/cros_ec_dev.c
> +++ b/drivers/platform/chrome/cros_ec_dev.c
> @@ -90,41 +90,6 @@ static int ec_get_version(struct cros_ec_dev *ec, char *str, int maxlen)
> return ret;
> }
>
> -static int cros_ec_check_features(struct cros_ec_dev *ec, int feature)
> -{
> - struct cros_ec_command *msg;
> - int ret;
> -
> - if (ec->features[0] == -1U && ec->features[1] == -1U) {
> - /* features bitmap not read yet */
> -
> - msg = kmalloc(sizeof(*msg) + sizeof(ec->features), GFP_KERNEL);
> - if (!msg)
> - return -ENOMEM;
> -
> - msg->version = 0;
> - msg->command = EC_CMD_GET_FEATURES + ec->cmd_offset;
> - msg->insize = sizeof(ec->features);
> - msg->outsize = 0;
> -
> - ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
> - if (ret < 0 || msg->result != EC_RES_SUCCESS) {
> - dev_warn(ec->dev, "cannot get EC features: %d/%d\n",
> - ret, msg->result);
> - memset(ec->features, 0, sizeof(ec->features));
> - }
> -
> - memcpy(ec->features, msg->data, sizeof(ec->features));
> -
> - dev_dbg(ec->dev, "EC features %08x %08x\n",
> - ec->features[0], ec->features[1]);
> -
> - kfree(msg);
> - }
> -
> - return ec->features[feature / 32] & EC_FEATURE_MASK_0(feature);
> -}
> -
> /* Device file ops */
> static int ec_device_open(struct inode *inode, struct file *filp)
> {
> @@ -268,126 +233,6 @@ static void __remove(struct device *dev)
> kfree(ec);
> }
>
> -static void cros_ec_sensors_register(struct cros_ec_dev *ec)
> -{
> - /*
> - * Issue a command to get the number of sensor reported.
> - * Build an array of sensors driver and register them all.
> - */
> - int ret, i, id, sensor_num;
> - struct mfd_cell *sensor_cells;
> - struct cros_ec_sensor_platform *sensor_platforms;
> - int sensor_type[MOTIONSENSE_TYPE_MAX];
> - struct ec_params_motion_sense *params;
> - struct ec_response_motion_sense *resp;
> - struct cros_ec_command *msg;
> -
> - msg = kzalloc(sizeof(struct cros_ec_command) +
> - max(sizeof(*params), sizeof(*resp)), GFP_KERNEL);
> - if (msg == NULL)
> - return;
> -
> - msg->version = 2;
> - msg->command = EC_CMD_MOTION_SENSE_CMD + ec->cmd_offset;
> - msg->outsize = sizeof(*params);
> - msg->insize = sizeof(*resp);
> -
> - params = (struct ec_params_motion_sense *)msg->data;
> - params->cmd = MOTIONSENSE_CMD_DUMP;
> -
> - ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
> - if (ret < 0 || msg->result != EC_RES_SUCCESS) {
> - dev_warn(ec->dev, "cannot get EC sensor information: %d/%d\n",
> - ret, msg->result);
> - goto error;
> - }
> -
> - 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),
> - GFP_KERNEL);
> - if (sensor_cells == NULL)
> - goto error;
> -
> - sensor_platforms = kzalloc(sizeof(struct cros_ec_sensor_platform) *
> - (sensor_num + 1), GFP_KERNEL);
> - if (sensor_platforms == NULL)
> - goto error_platforms;
> -
> - memset(sensor_type, 0, sizeof(sensor_type));
> - id = 0;
> - for (i = 0; i < sensor_num; i++) {
> - params->cmd = MOTIONSENSE_CMD_INFO;
> - params->info.sensor_num = i;
> - ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
> - if (ret < 0 || msg->result != EC_RES_SUCCESS) {
> - dev_warn(ec->dev, "no info for EC sensor %d : %d/%d\n",
> - i, ret, msg->result);
> - continue;
> - }
> - switch (resp->info.type) {
> - case MOTIONSENSE_TYPE_ACCEL:
> - sensor_cells[id].name = "cros-ec-accel";
> - break;
> - case MOTIONSENSE_TYPE_BARO:
> - sensor_cells[id].name = "cros-ec-baro";
> - break;
> - case MOTIONSENSE_TYPE_GYRO:
> - sensor_cells[id].name = "cros-ec-gyro";
> - break;
> - case MOTIONSENSE_TYPE_MAG:
> - sensor_cells[id].name = "cros-ec-mag";
> - break;
> - case MOTIONSENSE_TYPE_PROX:
> - sensor_cells[id].name = "cros-ec-prox";
> - break;
> - case MOTIONSENSE_TYPE_LIGHT:
> - sensor_cells[id].name = "cros-ec-light";
> - break;
> - case MOTIONSENSE_TYPE_ACTIVITY:
> - sensor_cells[id].name = "cros-ec-activity";
> - break;
> - default:
> - dev_warn(ec->dev, "unknown type %d\n", resp->info.type);
> - continue;
> - }
> - sensor_platforms[id].sensor_num = i;
> - sensor_cells[id].id = sensor_type[resp->info.type];
> - sensor_cells[id].platform_data = &sensor_platforms[id];
> - sensor_cells[id].pdata_size =
> - sizeof(struct cros_ec_sensor_platform);
> -
> - 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 (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE_FIFO)) {
> - sensor_cells[id].name = "cros-ec-ring";
> - id++;
> - }
> -
> - ret = mfd_add_devices(ec->dev, 0, sensor_cells, id,
> - NULL, 0, NULL);
> - if (ret)
> - dev_err(ec->dev, "failed to add EC sensors\n");
> -
> - kfree(sensor_platforms);
> -error_platforms:
> - kfree(sensor_cells);
> -error:
> - kfree(msg);
> -}
> -
> static int ec_device_probe(struct platform_device *pdev)
> {
> int retval = -ENOMEM;
> @@ -402,8 +247,6 @@ static int ec_device_probe(struct platform_device *pdev)
> ec->ec_dev = dev_get_drvdata(dev->parent);
> ec->dev = dev;
> ec->cmd_offset = ec_platform->cmd_offset;
> - ec->features[0] = -1U; /* Not cached yet */
> - ec->features[1] = -1U; /* Not cached yet */
> device_initialize(&ec->class_dev);
> cdev_init(&ec->cdev, &fops);
>
> @@ -432,10 +275,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);
>
> diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
> index 4e887ba..a4138a5 100644
> --- a/include/linux/mfd/cros_ec.h
> +++ b/include/linux/mfd/cros_ec.h
> @@ -115,6 +115,7 @@ struct cros_ec_command {
> * @event_notifier: interrupt event notifier for transport devices.
> * @event_data: raw payload transferred with the MKBP event.
> * @event_size: size in bytes of the event data.
> + * @features: stores the EC features.
> */
> struct cros_ec_device {
>
> @@ -150,15 +151,19 @@ struct cros_ec_device {
> struct ec_response_get_next_event event_data;
> int event_size;
> u32 host_event_wake_mask;
> + u32 features[2];
> };
>
> /**
> * struct cros_ec_sensor_platform - ChromeOS EC sensor platform information
> *
> * @sensor_num: Id of the sensor, as reported by the EC.
> + * @cmd_offset: offset to apply for each command. Set when
> + * registering a devicde behind another one.
> */
> struct cros_ec_sensor_platform {
> u8 sensor_num;
> + u16 cmd_offset;
> };
>
> /* struct cros_ec_platform - ChromeOS EC platform information
> @@ -192,7 +197,6 @@ struct cros_ec_dev {
> struct device *dev;
> struct cros_ec_debugfs *debug_info;
> u16 cmd_offset;
> - u32 features[2];
> };
>
> /**
> --
> 2.9.3
>
Hi Gwendal,
2017-07-13 22:33 GMT+02:00 Gwendal Grignou <[email protected]>:
> On Wed, Jul 12, 2017 at 3:13 AM, Enric Balletbo i Serra
> <[email protected]> wrote:
>> The cros_ec_dev driver should be used only to expose the Chrome OS Embedded
>> Controller to user-space and should not be used to add MFD devices by
>> calling mfd_add_devices. This patch moves this logic to the MFD cros_ec
>> driver and removes the MFD bits from the character device driver. Also
>> makes independent the IIO driver from the character device as also has no
>> sense.
>
> cros_ec_dev serves another purpose: it allows to represent an EC that
> does not have a cros_ec structure. It happens when there are several
> EC in a chromebook, and one EC is connected through another EC.
> One example is Samus (Pixel 2): where we have:
>
> (main SOC, Application Processor) AP --> (main Embedded Controller) EC
> ---> (Power Delivery [PD}) EC
>
> We access to the PD EC via pass-through commands through the main EC.
> Each EC has a cros_ec_dev structure, but only the main EC as a
> cros_ec_device structure (I will forever regret the structure names).
>
> Now form the AP point of view, both ECs use the same protocol. That
> why the sensors and other devcies that are registered by looking at
> the feature fields are registered with cros_ec_dev as their parent.
> Other devices that are registered from the device tree, predating the
> feature field support, are registered with cros_ec_device as their
> parent.
>
Interesting I didn't know that. So are you saying that this patch will
break support for devices like Pixel 2? I tested the patches on
various devices but not on Pixel 2 so could be.
Thanks,
Enric
> Gwendal.
>
>>
>> Signed-off-by: Enric Balletbo i Serra <[email protected]>
>> Acked-by: Jonathan Cameron <[email protected]>
>> ---
>> .../iio/common/cros_ec_sensors/cros_ec_sensors.c | 8 -
>> .../common/cros_ec_sensors/cros_ec_sensors_core.c | 8 +-
>> drivers/iio/light/cros_ec_light_prox.c | 8 -
>> drivers/iio/pressure/cros_ec_baro.c | 8 -
>> drivers/mfd/cros_ec.c | 160 ++++++++++++++++++++
>> drivers/platform/chrome/cros_ec_dev.c | 161 ---------------------
>> include/linux/mfd/cros_ec.h | 6 +-
>> 7 files changed, 170 insertions(+), 189 deletions(-)
>>
>> 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..9b53a01 100644
>> --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
>> +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
>> @@ -191,19 +191,11 @@ static const struct iio_info ec_sensors_info = {
>> static int cros_ec_sensors_probe(struct platform_device *pdev)
>> {
>> struct device *dev = &pdev->dev;
>> - struct cros_ec_dev *ec_dev = dev_get_drvdata(dev->parent);
>> - struct cros_ec_device *ec_device;
>> struct iio_dev *indio_dev;
>> struct cros_ec_sensors_state *state;
>> struct iio_chan_spec *channel;
>> int ret, i;
>>
>> - if (!ec_dev || !ec_dev->ec_dev) {
>> - dev_warn(&pdev->dev, "No CROS EC device found.\n");
>> - return -EINVAL;
>> - }
>> - ec_device = ec_dev->ec_dev;
>> -
>> indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*state));
>> if (!indio_dev)
>> return -ENOMEM;
>> 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..0cdb64a 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
>> @@ -41,12 +41,13 @@ int cros_ec_sensors_core_init(struct platform_device *pdev,
>> {
>> struct device *dev = &pdev->dev;
>> struct cros_ec_sensors_core_state *state = iio_priv(indio_dev);
>> - struct cros_ec_dev *ec = dev_get_drvdata(pdev->dev.parent);
>> + struct cros_ec_device *ec_dev = dev_get_drvdata(pdev->dev.parent);
>> struct cros_ec_sensor_platform *sensor_platform = dev_get_platdata(dev);
>>
>> platform_set_drvdata(pdev, indio_dev);
>>
>> - state->ec = ec->ec_dev;
>> + state->ec = ec_dev;
>> +
>> state->msg = devm_kzalloc(&pdev->dev,
>> max((u16)sizeof(struct ec_params_motion_sense),
>> state->ec->max_response), GFP_KERNEL);
>> @@ -59,7 +60,8 @@ int cros_ec_sensors_core_init(struct platform_device *pdev,
>>
>> /* Set up the host command structure. */
>> state->msg->version = 2;
>> - state->msg->command = EC_CMD_MOTION_SENSE_CMD + ec->cmd_offset;
>> + state->msg->command = EC_CMD_MOTION_SENSE_CMD +
>> + sensor_platform->cmd_offset;
>> state->msg->outsize = sizeof(struct ec_params_motion_sense);
>>
>> indio_dev->dev.parent = &pdev->dev;
>> diff --git a/drivers/iio/light/cros_ec_light_prox.c b/drivers/iio/light/cros_ec_light_prox.c
>> index 7217223..2133ddc 100644
>> --- a/drivers/iio/light/cros_ec_light_prox.c
>> +++ b/drivers/iio/light/cros_ec_light_prox.c
>> @@ -181,19 +181,11 @@ static const struct iio_info cros_ec_light_prox_info = {
>> static int cros_ec_light_prox_probe(struct platform_device *pdev)
>> {
>> struct device *dev = &pdev->dev;
>> - struct cros_ec_dev *ec_dev = dev_get_drvdata(dev->parent);
>> - struct cros_ec_device *ec_device;
>> struct iio_dev *indio_dev;
>> struct cros_ec_light_prox_state *state;
>> struct iio_chan_spec *channel;
>> int ret;
>>
>> - if (!ec_dev || !ec_dev->ec_dev) {
>> - dev_warn(dev, "No CROS EC device found.\n");
>> - return -EINVAL;
>> - }
>> - ec_device = ec_dev->ec_dev;
>> -
>> indio_dev = devm_iio_device_alloc(dev, sizeof(*state));
>> if (!indio_dev)
>> return -ENOMEM;
>> diff --git a/drivers/iio/pressure/cros_ec_baro.c b/drivers/iio/pressure/cros_ec_baro.c
>> index 48b2a30..dbea18b 100644
>> --- a/drivers/iio/pressure/cros_ec_baro.c
>> +++ b/drivers/iio/pressure/cros_ec_baro.c
>> @@ -126,19 +126,11 @@ static const struct iio_info cros_ec_baro_info = {
>> static int cros_ec_baro_probe(struct platform_device *pdev)
>> {
>> struct device *dev = &pdev->dev;
>> - struct cros_ec_dev *ec_dev = dev_get_drvdata(dev->parent);
>> - struct cros_ec_device *ec_device;
>> struct iio_dev *indio_dev;
>> struct cros_ec_baro_state *state;
>> struct iio_chan_spec *channel;
>> int ret;
>>
>> - if (!ec_dev || !ec_dev->ec_dev) {
>> - dev_warn(dev, "No CROS EC device found.\n");
>> - return -EINVAL;
>> - }
>> - ec_device = ec_dev->ec_dev;
>> -
>> indio_dev = devm_iio_device_alloc(dev, sizeof(*state));
>> if (!indio_dev)
>> return -ENOMEM;
>> diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
>> index b0ca5a4c..75a27a6 100644
>> --- a/drivers/mfd/cros_ec.c
>> +++ b/drivers/mfd/cros_ec.c
>> @@ -91,6 +91,160 @@ static int cros_ec_sleep_event(struct cros_ec_device *ec_dev, u8 sleep_event)
>> return cros_ec_cmd_xfer(ec_dev, &buf.msg);
>> }
>>
>> +static int cros_ec_check_features(struct cros_ec_device *ec_dev, int feature)
>> +{
>> + struct cros_ec_command *msg;
>> + int ret;
>> +
>> + if (ec_dev->features[0] == -1U && ec_dev->features[1] == -1U) {
>> + /* features bitmap not read yet */
>> +
>> + msg = kmalloc(sizeof(*msg) + sizeof(ec_dev->features),
>> + GFP_KERNEL);
>> + if (!msg)
>> + return -ENOMEM;
>> +
>> + msg->version = 0;
>> + msg->command = EC_CMD_GET_FEATURES + ec_p.cmd_offset;
>> + msg->insize = sizeof(ec_dev->features);
>> + msg->outsize = 0;
>> +
>> + ret = cros_ec_cmd_xfer(ec_dev, msg);
>> + if (ret < 0 || msg->result != EC_RES_SUCCESS) {
>> + dev_warn(ec_dev->dev, "cannot get EC features: %d/%d\n",
>> + ret, msg->result);
>> + memset(ec_dev->features, 0, sizeof(ec_dev->features));
>> + }
>> +
>> + memcpy(ec_dev->features, msg->data, sizeof(ec_dev->features));
>> +
>> + dev_dbg(ec_dev->dev, "EC features %08x %08x\n",
>> + ec_dev->features[0], ec_dev->features[1]);
>> +
>> + kfree(msg);
>> + }
>> +
>> + return ec_dev->features[feature / 32] & EC_FEATURE_MASK_0(feature);
>> +}
>> +
>> +static void cros_ec_sensors_register(struct cros_ec_device *ec_dev)
>> +{
>> + /*
>> + * Issue a command to get the number of sensor reported.
>> + * Build an array of sensors driver and register them all.
>> + */
>> + int ret, i, id, sensor_num;
>> + struct mfd_cell *sensor_cells;
>> + struct cros_ec_sensor_platform *sensor_platforms;
>> + int sensor_type[MOTIONSENSE_TYPE_MAX];
>> + struct ec_params_motion_sense *params;
>> + struct ec_response_motion_sense *resp;
>> + struct cros_ec_command *msg;
>> +
>> + msg = kzalloc(sizeof(struct cros_ec_command) +
>> + max(sizeof(*params), sizeof(*resp)), GFP_KERNEL);
>> + if (msg == NULL)
>> + return;
>> +
>> + msg->version = 2;
>> + msg->command = EC_CMD_MOTION_SENSE_CMD + ec_p.cmd_offset;
>> + msg->outsize = sizeof(*params);
>> + msg->insize = sizeof(*resp);
>> +
>> + params = (struct ec_params_motion_sense *)msg->data;
>> + params->cmd = MOTIONSENSE_CMD_DUMP;
>> +
>> + ret = cros_ec_cmd_xfer(ec_dev, msg);
>> + if (ret < 0 || msg->result != EC_RES_SUCCESS) {
>> + dev_warn(ec_dev->dev, "cannot get EC sensor information: %d/%d\n",
>> + ret, msg->result);
>> + goto error;
>> + }
>> +
>> + 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),
>> + GFP_KERNEL);
>> + if (sensor_cells == NULL)
>> + goto error;
>> +
>> + sensor_platforms = kzalloc(sizeof(struct cros_ec_sensor_platform) *
>> + (sensor_num + 1), GFP_KERNEL);
>> + if (sensor_platforms == NULL)
>> + goto error_platforms;
>> +
>> + memset(sensor_type, 0, sizeof(sensor_type));
>> + id = 0;
>> + for (i = 0; i < sensor_num; i++) {
>> + params->cmd = MOTIONSENSE_CMD_INFO;
>> + params->info.sensor_num = i;
>> + ret = cros_ec_cmd_xfer(ec_dev, msg);
>> + if (ret < 0 || msg->result != EC_RES_SUCCESS) {
>> + dev_warn(ec_dev->dev, "no info for EC sensor %d : %d/%d\n",
>> + i, ret, msg->result);
>> + continue;
>> + }
>> + switch (resp->info.type) {
>> + case MOTIONSENSE_TYPE_ACCEL:
>> + sensor_cells[id].name = "cros-ec-accel";
>> + break;
>> + case MOTIONSENSE_TYPE_BARO:
>> + sensor_cells[id].name = "cros-ec-baro";
>> + break;
>> + case MOTIONSENSE_TYPE_GYRO:
>> + sensor_cells[id].name = "cros-ec-gyro";
>> + break;
>> + case MOTIONSENSE_TYPE_MAG:
>> + sensor_cells[id].name = "cros-ec-mag";
>> + break;
>> + case MOTIONSENSE_TYPE_PROX:
>> + sensor_cells[id].name = "cros-ec-prox";
>> + break;
>> + case MOTIONSENSE_TYPE_LIGHT:
>> + sensor_cells[id].name = "cros-ec-light";
>> + break;
>> + case MOTIONSENSE_TYPE_ACTIVITY:
>> + sensor_cells[id].name = "cros-ec-activity";
>> + break;
>> + default:
>> + dev_warn(ec_dev->dev, "unknown type %d\n",
>> + resp->info.type);
>> + continue;
>> + }
>> + sensor_platforms[id].sensor_num = i;
>> + sensor_platforms[id].cmd_offset = ec_p.cmd_offset;
>> + sensor_cells[id].id = sensor_type[resp->info.type];
>> + sensor_cells[id].platform_data = &sensor_platforms[id];
>> + sensor_cells[id].pdata_size =
>> + sizeof(struct cros_ec_sensor_platform);
>> +
>> + 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++;
>> + }
>> +
>> + ret = mfd_add_devices(ec_dev->dev, PLATFORM_DEVID_AUTO, sensor_cells,
>> + id, NULL, 0, NULL);
>> + if (ret)
>> + dev_err(ec_dev->dev, "failed to add EC sensors\n");
>> +
>> + kfree(sensor_platforms);
>> +error_platforms:
>> + kfree(sensor_cells);
>> +error:
>> + kfree(msg);
>> +}
>> +
>> int cros_ec_register(struct cros_ec_device *ec_dev)
>> {
>> struct device *dev = ec_dev->dev;
>> @@ -101,6 +255,8 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
>> ec_dev->max_request = sizeof(struct ec_params_hello);
>> ec_dev->max_response = sizeof(struct ec_response_get_protocol_info);
>> ec_dev->max_passthru = 0;
>> + ec_dev->features[0] = -1U; /* Not cached yet */
>> + ec_dev->features[1] = -1U; /* Not cached yet */
>>
>> ec_dev->din = devm_kzalloc(dev, ec_dev->din_size, GFP_KERNEL);
>> if (!ec_dev->din)
>> @@ -134,6 +290,10 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
>> goto fail_mfd;
>> }
>>
>> + /* Check whether this EC is a sensor hub. */
>> + if (cros_ec_check_features(ec_dev, EC_FEATURE_MOTION_SENSE))
>> + cros_ec_sensors_register(ec_dev);
>> +
>> if (ec_dev->max_passthru) {
>> /*
>> * Register a PD device as well on top of this device.
>> diff --git a/drivers/platform/chrome/cros_ec_dev.c b/drivers/platform/chrome/cros_ec_dev.c
>> index cf6c4f0..bd07df5 100644
>> --- a/drivers/platform/chrome/cros_ec_dev.c
>> +++ b/drivers/platform/chrome/cros_ec_dev.c
>> @@ -90,41 +90,6 @@ static int ec_get_version(struct cros_ec_dev *ec, char *str, int maxlen)
>> return ret;
>> }
>>
>> -static int cros_ec_check_features(struct cros_ec_dev *ec, int feature)
>> -{
>> - struct cros_ec_command *msg;
>> - int ret;
>> -
>> - if (ec->features[0] == -1U && ec->features[1] == -1U) {
>> - /* features bitmap not read yet */
>> -
>> - msg = kmalloc(sizeof(*msg) + sizeof(ec->features), GFP_KERNEL);
>> - if (!msg)
>> - return -ENOMEM;
>> -
>> - msg->version = 0;
>> - msg->command = EC_CMD_GET_FEATURES + ec->cmd_offset;
>> - msg->insize = sizeof(ec->features);
>> - msg->outsize = 0;
>> -
>> - ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
>> - if (ret < 0 || msg->result != EC_RES_SUCCESS) {
>> - dev_warn(ec->dev, "cannot get EC features: %d/%d\n",
>> - ret, msg->result);
>> - memset(ec->features, 0, sizeof(ec->features));
>> - }
>> -
>> - memcpy(ec->features, msg->data, sizeof(ec->features));
>> -
>> - dev_dbg(ec->dev, "EC features %08x %08x\n",
>> - ec->features[0], ec->features[1]);
>> -
>> - kfree(msg);
>> - }
>> -
>> - return ec->features[feature / 32] & EC_FEATURE_MASK_0(feature);
>> -}
>> -
>> /* Device file ops */
>> static int ec_device_open(struct inode *inode, struct file *filp)
>> {
>> @@ -268,126 +233,6 @@ static void __remove(struct device *dev)
>> kfree(ec);
>> }
>>
>> -static void cros_ec_sensors_register(struct cros_ec_dev *ec)
>> -{
>> - /*
>> - * Issue a command to get the number of sensor reported.
>> - * Build an array of sensors driver and register them all.
>> - */
>> - int ret, i, id, sensor_num;
>> - struct mfd_cell *sensor_cells;
>> - struct cros_ec_sensor_platform *sensor_platforms;
>> - int sensor_type[MOTIONSENSE_TYPE_MAX];
>> - struct ec_params_motion_sense *params;
>> - struct ec_response_motion_sense *resp;
>> - struct cros_ec_command *msg;
>> -
>> - msg = kzalloc(sizeof(struct cros_ec_command) +
>> - max(sizeof(*params), sizeof(*resp)), GFP_KERNEL);
>> - if (msg == NULL)
>> - return;
>> -
>> - msg->version = 2;
>> - msg->command = EC_CMD_MOTION_SENSE_CMD + ec->cmd_offset;
>> - msg->outsize = sizeof(*params);
>> - msg->insize = sizeof(*resp);
>> -
>> - params = (struct ec_params_motion_sense *)msg->data;
>> - params->cmd = MOTIONSENSE_CMD_DUMP;
>> -
>> - ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
>> - if (ret < 0 || msg->result != EC_RES_SUCCESS) {
>> - dev_warn(ec->dev, "cannot get EC sensor information: %d/%d\n",
>> - ret, msg->result);
>> - goto error;
>> - }
>> -
>> - 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),
>> - GFP_KERNEL);
>> - if (sensor_cells == NULL)
>> - goto error;
>> -
>> - sensor_platforms = kzalloc(sizeof(struct cros_ec_sensor_platform) *
>> - (sensor_num + 1), GFP_KERNEL);
>> - if (sensor_platforms == NULL)
>> - goto error_platforms;
>> -
>> - memset(sensor_type, 0, sizeof(sensor_type));
>> - id = 0;
>> - for (i = 0; i < sensor_num; i++) {
>> - params->cmd = MOTIONSENSE_CMD_INFO;
>> - params->info.sensor_num = i;
>> - ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
>> - if (ret < 0 || msg->result != EC_RES_SUCCESS) {
>> - dev_warn(ec->dev, "no info for EC sensor %d : %d/%d\n",
>> - i, ret, msg->result);
>> - continue;
>> - }
>> - switch (resp->info.type) {
>> - case MOTIONSENSE_TYPE_ACCEL:
>> - sensor_cells[id].name = "cros-ec-accel";
>> - break;
>> - case MOTIONSENSE_TYPE_BARO:
>> - sensor_cells[id].name = "cros-ec-baro";
>> - break;
>> - case MOTIONSENSE_TYPE_GYRO:
>> - sensor_cells[id].name = "cros-ec-gyro";
>> - break;
>> - case MOTIONSENSE_TYPE_MAG:
>> - sensor_cells[id].name = "cros-ec-mag";
>> - break;
>> - case MOTIONSENSE_TYPE_PROX:
>> - sensor_cells[id].name = "cros-ec-prox";
>> - break;
>> - case MOTIONSENSE_TYPE_LIGHT:
>> - sensor_cells[id].name = "cros-ec-light";
>> - break;
>> - case MOTIONSENSE_TYPE_ACTIVITY:
>> - sensor_cells[id].name = "cros-ec-activity";
>> - break;
>> - default:
>> - dev_warn(ec->dev, "unknown type %d\n", resp->info.type);
>> - continue;
>> - }
>> - sensor_platforms[id].sensor_num = i;
>> - sensor_cells[id].id = sensor_type[resp->info.type];
>> - sensor_cells[id].platform_data = &sensor_platforms[id];
>> - sensor_cells[id].pdata_size =
>> - sizeof(struct cros_ec_sensor_platform);
>> -
>> - 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 (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE_FIFO)) {
>> - sensor_cells[id].name = "cros-ec-ring";
>> - id++;
>> - }
>> -
>> - ret = mfd_add_devices(ec->dev, 0, sensor_cells, id,
>> - NULL, 0, NULL);
>> - if (ret)
>> - dev_err(ec->dev, "failed to add EC sensors\n");
>> -
>> - kfree(sensor_platforms);
>> -error_platforms:
>> - kfree(sensor_cells);
>> -error:
>> - kfree(msg);
>> -}
>> -
>> static int ec_device_probe(struct platform_device *pdev)
>> {
>> int retval = -ENOMEM;
>> @@ -402,8 +247,6 @@ static int ec_device_probe(struct platform_device *pdev)
>> ec->ec_dev = dev_get_drvdata(dev->parent);
>> ec->dev = dev;
>> ec->cmd_offset = ec_platform->cmd_offset;
>> - ec->features[0] = -1U; /* Not cached yet */
>> - ec->features[1] = -1U; /* Not cached yet */
>> device_initialize(&ec->class_dev);
>> cdev_init(&ec->cdev, &fops);
>>
>> @@ -432,10 +275,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);
>>
>> diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
>> index 4e887ba..a4138a5 100644
>> --- a/include/linux/mfd/cros_ec.h
>> +++ b/include/linux/mfd/cros_ec.h
>> @@ -115,6 +115,7 @@ struct cros_ec_command {
>> * @event_notifier: interrupt event notifier for transport devices.
>> * @event_data: raw payload transferred with the MKBP event.
>> * @event_size: size in bytes of the event data.
>> + * @features: stores the EC features.
>> */
>> struct cros_ec_device {
>>
>> @@ -150,15 +151,19 @@ struct cros_ec_device {
>> struct ec_response_get_next_event event_data;
>> int event_size;
>> u32 host_event_wake_mask;
>> + u32 features[2];
>> };
>>
>> /**
>> * struct cros_ec_sensor_platform - ChromeOS EC sensor platform information
>> *
>> * @sensor_num: Id of the sensor, as reported by the EC.
>> + * @cmd_offset: offset to apply for each command. Set when
>> + * registering a devicde behind another one.
>> */
>> struct cros_ec_sensor_platform {
>> u8 sensor_num;
>> + u16 cmd_offset;
>> };
>>
>> /* struct cros_ec_platform - ChromeOS EC platform information
>> @@ -192,7 +197,6 @@ struct cros_ec_dev {
>> struct device *dev;
>> struct cros_ec_debugfs *debug_info;
>> u16 cmd_offset;
>> - u32 features[2];
>> };
>>
>> /**
>> --
>> 2.9.3
>>
Hi Enric,
On Mon, Jul 17, 2017 at 12:30:30PM +0200, Enric Balletbo Serra wrote:
> Hi Gwendal,
>
> Interesting I didn't know that. So are you saying that this patch will
> break support for devices like Pixel 2? I tested the patches on
> various devices but not on Pixel 2 so could be.
This would affect any systems where we have chained ECs. Samus (Chromebook
Pixel 2015) has its PD EC hooked up in this way. Practically speaking,
that would mean that the cros_usbpd-charger power supply driver may not work
correctly.
I don't think we expect any other systems to have this configuration,
but the first one we shipped did, so we will have to support it.
Thanks,
Benson
--
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
[email protected]
Chromium OS Project
[email protected]
On Mon, 17 Jul 2017, Enric Balletbo Serra wrote:
> Hi Gwendal,
>
> 2017-07-13 22:33 GMT+02:00 Gwendal Grignou <[email protected]>:
> > On Wed, Jul 12, 2017 at 3:13 AM, Enric Balletbo i Serra
> > <[email protected]> wrote:
> >> The cros_ec_dev driver should be used only to expose the Chrome OS Embedded
> >> Controller to user-space and should not be used to add MFD devices by
> >> calling mfd_add_devices. This patch moves this logic to the MFD cros_ec
> >> driver and removes the MFD bits from the character device driver. Also
> >> makes independent the IIO driver from the character device as also has no
> >> sense.
> >
> > cros_ec_dev serves another purpose: it allows to represent an EC that
> > does not have a cros_ec structure. It happens when there are several
> > EC in a chromebook, and one EC is connected through another EC.
> > One example is Samus (Pixel 2): where we have:
> >
> > (main SOC, Application Processor) AP --> (main Embedded Controller) EC
> > ---> (Power Delivery [PD}) EC
> >
> > We access to the PD EC via pass-through commands through the main EC.
> > Each EC has a cros_ec_dev structure, but only the main EC as a
> > cros_ec_device structure (I will forever regret the structure names).
> >
> > Now form the AP point of view, both ECs use the same protocol. That
> > why the sensors and other devcies that are registered by looking at
> > the feature fields are registered with cros_ec_dev as their parent.
> > Other devices that are registered from the device tree, predating the
> > feature field support, are registered with cros_ec_device as their
> > parent.
> >
>
> Interesting I didn't know that. So are you saying that this patch will
> break support for devices like Pixel 2? I tested the patches on
> various devices but not on Pixel 2 so could be.
Does this affect the rest of the series?
Or should I review/apply as usual?
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
Lee,
2017-07-18 11:19 GMT+02:00 Lee Jones <[email protected]>:
> On Mon, 17 Jul 2017, Enric Balletbo Serra wrote:
>
>> Hi Gwendal,
>>
>> 2017-07-13 22:33 GMT+02:00 Gwendal Grignou <[email protected]>:
>> > On Wed, Jul 12, 2017 at 3:13 AM, Enric Balletbo i Serra
>> > <[email protected]> wrote:
>> >> The cros_ec_dev driver should be used only to expose the Chrome OS Embedded
>> >> Controller to user-space and should not be used to add MFD devices by
>> >> calling mfd_add_devices. This patch moves this logic to the MFD cros_ec
>> >> driver and removes the MFD bits from the character device driver. Also
>> >> makes independent the IIO driver from the character device as also has no
>> >> sense.
>> >
>> > cros_ec_dev serves another purpose: it allows to represent an EC that
>> > does not have a cros_ec structure. It happens when there are several
>> > EC in a chromebook, and one EC is connected through another EC.
>> > One example is Samus (Pixel 2): where we have:
>> >
>> > (main SOC, Application Processor) AP --> (main Embedded Controller) EC
>> > ---> (Power Delivery [PD}) EC
>> >
>> > We access to the PD EC via pass-through commands through the main EC.
>> > Each EC has a cros_ec_dev structure, but only the main EC as a
>> > cros_ec_device structure (I will forever regret the structure names).
>> >
>> > Now form the AP point of view, both ECs use the same protocol. That
>> > why the sensors and other devcies that are registered by looking at
>> > the feature fields are registered with cros_ec_dev as their parent.
>> > Other devices that are registered from the device tree, predating the
>> > feature field support, are registered with cros_ec_device as their
>> > parent.
>> >
>>
>> Interesting I didn't know that. So are you saying that this patch will
>> break support for devices like Pixel 2? I tested the patches on
>> various devices but not on Pixel 2 so could be.
>
> Does this affect the rest of the series?
>
Yes, because this patch moves cros_ec_check_features from char dev driver...
> Or should I review/apply as usual?
>
Please, hold on while I figure out what's exactly the problem with
Samus. Sorry for the noise.
Cheers,
Enric
> --
> Lee Jones
> Linaro STMicroelectronics Landing Team Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog
On Wed, 12 Jul 2017, Enric Balletbo i Serra wrote:
> From: Stephen Barber <[email protected]>
>
> If the EC supports RTC host commands, expose an RTC device.
>
> Signed-off-by: Stephen Barber <[email protected]>
> Signed-off-by: Enric Balletbo i Serra <[email protected]>
> ---
> drivers/mfd/cros_ec.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
> index 75a27a6..ff972fb 100644
> --- a/drivers/mfd/cros_ec.c
> +++ b/drivers/mfd/cros_ec.c
> @@ -51,6 +51,10 @@ static const struct mfd_cell ec_pd_cell = {
> .pdata_size = sizeof(pd_p),
> };
>
> +static const struct mfd_cell ec_rtc_cell = {
> + .name = "cros-ec-rtc",
> +};
> +
> static irqreturn_t ec_irq_thread(int irq, void *data)
> {
> struct cros_ec_device *ec_dev = data;
> @@ -245,6 +249,16 @@ static void cros_ec_sensors_register(struct cros_ec_device *ec_dev)
> kfree(msg);
> }
>
> +static void cros_ec_rtc_register(struct cros_ec_device *ec_dev)
> +{
> + int ret;
> +
> + ret = mfd_add_devices(ec_dev->dev, PLATFORM_DEVID_AUTO, &ec_rtc_cell,
> + 1, NULL, 0, NULL);
> + if (ret)
> + dev_err(ec_dev->dev, "failed to add EC RTC\n");
> +}
Why do you need a new function just to call a single function?
And by making it void, you're also loosing the return value.
If mfd_add_devices() fails, something has gone wrong!
> int cros_ec_register(struct cros_ec_device *ec_dev)
> {
> struct device *dev = ec_dev->dev;
> @@ -294,6 +308,10 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
> if (cros_ec_check_features(ec_dev, EC_FEATURE_MOTION_SENSE))
> cros_ec_sensors_register(ec_dev);
>
> + /* Check whether this EC has RTC support */
> + if (cros_ec_check_features(ec_dev, EC_FEATURE_RTC))
> + cros_ec_rtc_register(ec_dev);
> +
> if (ec_dev->max_passthru) {
> /*
> * Register a PD device as well on top of this device.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog