From e43c426a777eb2554d8a29d31311debe897badd9 Mon Sep 17 00:00:00 2001 From: Enric Balletbo i Serra Date: Wed, 3 Apr 2019 15:47:18 +0200 Subject: platform/chrome: cros_ec_debugfs: Remove dev_warn when console log is not supported If the debugfs interface is enabled, every time a CrOS device is instantiated a warning like this can appear for every probed device. "device does not support reading the console log" The warning message adds nothing, rather it is source of confusion as this is expected on some cases. For example, on Samus, that has a cros-ec and a cros-pd instance the message appears twice, and I suspect this will happen also on those devices that has a non-standard EC. If the command is not supported just return silently and don't print the warning, otherwise the code will already print an error. Signed-off-by: Enric Balletbo i Serra Reviewed-by: Guenter Roeck --- drivers/platform/chrome/cros_ec_debugfs.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'drivers/platform') diff --git a/drivers/platform/chrome/cros_ec_debugfs.c b/drivers/platform/chrome/cros_ec_debugfs.c index 71308766e891..f0de1991b688 100644 --- a/drivers/platform/chrome/cros_ec_debugfs.c +++ b/drivers/platform/chrome/cros_ec_debugfs.c @@ -306,11 +306,12 @@ static int cros_ec_create_console_log(struct cros_ec_debugfs *debug_info) int read_params_size; int read_response_size; - if (!ec_read_version_supported(ec)) { - dev_warn(ec->dev, - "device does not support reading the console log\n"); + /* + * If the console log feature is not supported return silently and + * don't create the console_log entry. + */ + if (!ec_read_version_supported(ec)) return 0; - } buf = devm_kzalloc(ec->dev, LOG_SIZE, GFP_KERNEL); if (!buf) -- cgit v1.2.3 From b18e606ff313e20e72b4a53cdddc16f476f2a740 Mon Sep 17 00:00:00 2001 From: Enric Balletbo i Serra Date: Mon, 8 Apr 2019 17:05:38 +0200 Subject: platform/chrome: cros_ec_debugfs: no need to check return value of debugfs_create functions When calling debugfs functions, there is no need to ever check the return value. The function can work or not, but the code logic should never do something different based on this. Cc: Greg Kroah-Hartman Signed-off-by: Enric Balletbo i Serra Reviewed-by: Guenter Roeck --- drivers/platform/chrome/cros_ec_debugfs.c | 35 ++++++------------------------- 1 file changed, 6 insertions(+), 29 deletions(-) (limited to 'drivers/platform') diff --git a/drivers/platform/chrome/cros_ec_debugfs.c b/drivers/platform/chrome/cros_ec_debugfs.c index f0de1991b688..a712508e4646 100644 --- a/drivers/platform/chrome/cros_ec_debugfs.c +++ b/drivers/platform/chrome/cros_ec_debugfs.c @@ -337,12 +337,8 @@ static int cros_ec_create_console_log(struct cros_ec_debugfs *debug_info) mutex_init(&debug_info->log_mutex); init_waitqueue_head(&debug_info->log_wq); - if (!debugfs_create_file("console_log", - S_IFREG | 0444, - debug_info->dir, - debug_info, - &cros_ec_console_log_fops)) - return -ENOMEM; + debugfs_create_file("console_log", S_IFREG | 0444, debug_info->dir, + debug_info, &cros_ec_console_log_fops); INIT_DELAYED_WORK(&debug_info->log_poll_work, cros_ec_console_log_work); @@ -390,13 +386,8 @@ static int cros_ec_create_panicinfo(struct cros_ec_debugfs *debug_info) debug_info->panicinfo_blob.data = msg->data; debug_info->panicinfo_blob.size = ret; - if (!debugfs_create_blob("panicinfo", - S_IFREG | 0444, - debug_info->dir, - &debug_info->panicinfo_blob)) { - ret = -ENOMEM; - goto free; - } + debugfs_create_blob("panicinfo", S_IFREG | 0444, debug_info->dir, + &debug_info->panicinfo_blob); return 0; @@ -405,15 +396,6 @@ free: return ret; } -static int cros_ec_create_pdinfo(struct cros_ec_debugfs *debug_info) -{ - if (!debugfs_create_file("pdinfo", 0444, debug_info->dir, debug_info, - &cros_ec_pdinfo_fops)) - return -ENOMEM; - - return 0; -} - static int cros_ec_debugfs_probe(struct platform_device *pd) { struct cros_ec_dev *ec = dev_get_drvdata(pd->dev.parent); @@ -428,8 +410,6 @@ static int cros_ec_debugfs_probe(struct platform_device *pd) debug_info->ec = ec; debug_info->dir = debugfs_create_dir(name, NULL); - if (!debug_info->dir) - return -ENOMEM; ret = cros_ec_create_panicinfo(debug_info); if (ret) @@ -439,9 +419,8 @@ static int cros_ec_debugfs_probe(struct platform_device *pd) if (ret) goto remove_debugfs; - ret = cros_ec_create_pdinfo(debug_info); - if (ret) - goto remove_log; + debugfs_create_file("pdinfo", 0444, debug_info->dir, debug_info, + &cros_ec_pdinfo_fops); ec->debug_info = debug_info; @@ -449,8 +428,6 @@ static int cros_ec_debugfs_probe(struct platform_device *pd) return 0; -remove_log: - cros_ec_cleanup_console_log(debug_info); remove_debugfs: debugfs_remove_recursive(debug_info->dir); return ret; -- cgit v1.2.3 From 37a186225a0c020516bafad2727fdcdfc039a1e4 Mon Sep 17 00:00:00 2001 From: Douglas Anderson Date: Wed, 3 Apr 2019 13:31:37 -0700 Subject: platform/chrome: cros_ec_spi: Transfer messages at high priority The software running on the Chrome OS Embedded Controller (cros_ec) handles SPI transfers in a bit of a wonky way. Specifically if the EC sees too long of a delay in a SPI transfer it will give up and the transfer will be counted as failed. Unfortunately the timeout is fairly short, though the actual number may be different for different EC codebases. We can end up tripping the timeout pretty easily if we happen to preempt the task running the SPI transfer and don't get back to it for a little while. Historically this hasn't been a _huge_ deal because: 1. On old devices Chrome OS used to run PREEMPT_VOLUNTARY. That meant we were pretty unlikely to take a big break from the transfer. 2. On recent devices we had faster / more processors. 3. Recent devices didn't use "cros-ec-spi-pre-delay". Using that delay makes us more likely to trip this use case. 4. For whatever reasons (I didn't dig) old kernels seem to be less likely to trip this. 5. For the most part it's kinda OK if a few transfers to the EC fail. Mostly we're just polling the battery or doing some other task where we'll try again. Even with the above things, this issue has reared its ugly head periodically. We could solve this in a nice way by adding reliable retries to the EC protocol [1] or by re-designing the code in the EC codebase to allow it to wait longer, but that code doesn't ever seem to get changed. ...and even if it did, it wouldn't help old devices. It's now time to finally take a crack at making this a little better. This patch isn't guaranteed to make every cros_ec SPI transfer perfect, but it should improve things by a few orders of magnitude. Specifically you can try this on a rk3288-veyron Chromebook (which is slower and also _does_ need "cros-ec-spi-pre-delay"): md5sum /dev/zero & md5sum /dev/zero & md5sum /dev/zero & md5sum /dev/zero & while true; do cat /sys/class/power_supply/sbs-20-000b/charge_now > /dev/null; done ...before this patch you'll see boatloads of errors. After this patch I don't see any in the testing I did. The way this patch works is by effectively boosting the priority of the cros_ec transfers. As far as I know there is no simple way to just boost the priority of the current process temporarily so the way we accomplish this is by queuing the work on the system_highpri_wq. NOTE: this patch relies on the fact that the SPI framework attempts to push the messages out on the calling context (which is the one that is boosted to high priority). As I understand from earlier (long ago) discussions with Mark Brown this should be a fine assumption. Even if it isn't true sometimes this patch will still not make things worse. [1] https://crbug.com/678675 Signed-off-by: Douglas Anderson Reviewed-by: Matthias Kaehlcke Reviewed-by: Brian Norris Signed-off-by: Enric Balletbo i Serra --- drivers/platform/chrome/cros_ec_spi.c | 80 ++++++++++++++++++++++++++++++++--- 1 file changed, 74 insertions(+), 6 deletions(-) (limited to 'drivers/platform') diff --git a/drivers/platform/chrome/cros_ec_spi.c b/drivers/platform/chrome/cros_ec_spi.c index ffc38f9d4829..8e9451720e73 100644 --- a/drivers/platform/chrome/cros_ec_spi.c +++ b/drivers/platform/chrome/cros_ec_spi.c @@ -75,6 +75,27 @@ struct cros_ec_spi { unsigned int end_of_msg_delay; }; +typedef int (*cros_ec_xfer_fn_t) (struct cros_ec_device *ec_dev, + struct cros_ec_command *ec_msg); + +/** + * struct cros_ec_xfer_work_params - params for our high priority workers + * + * @work: The work_struct needed to queue work + * @fn: The function to use to transfer + * @ec_dev: ChromeOS EC device + * @ec_msg: Message to transfer + * @ret: The return value of the function + */ + +struct cros_ec_xfer_work_params { + struct work_struct work; + cros_ec_xfer_fn_t fn; + struct cros_ec_device *ec_dev; + struct cros_ec_command *ec_msg; + int ret; +}; + static void debug_packet(struct device *dev, const char *name, u8 *ptr, int len) { @@ -350,13 +371,13 @@ static int cros_ec_spi_receive_response(struct cros_ec_device *ec_dev, } /** - * cros_ec_pkt_xfer_spi - Transfer a packet over SPI and receive the reply + * do_cros_ec_pkt_xfer_spi - Transfer a packet over SPI and receive the reply * * @ec_dev: ChromeOS EC device * @ec_msg: Message to transfer */ -static int cros_ec_pkt_xfer_spi(struct cros_ec_device *ec_dev, - struct cros_ec_command *ec_msg) +static int do_cros_ec_pkt_xfer_spi(struct cros_ec_device *ec_dev, + struct cros_ec_command *ec_msg) { struct ec_host_response *response; struct cros_ec_spi *ec_spi = ec_dev->priv; @@ -493,13 +514,13 @@ exit: } /** - * cros_ec_cmd_xfer_spi - Transfer a message over SPI and receive the reply + * do_cros_ec_cmd_xfer_spi - Transfer a message over SPI and receive the reply * * @ec_dev: ChromeOS EC device * @ec_msg: Message to transfer */ -static int cros_ec_cmd_xfer_spi(struct cros_ec_device *ec_dev, - struct cros_ec_command *ec_msg) +static int do_cros_ec_cmd_xfer_spi(struct cros_ec_device *ec_dev, + struct cros_ec_command *ec_msg) { struct cros_ec_spi *ec_spi = ec_dev->priv; struct spi_transfer trans; @@ -611,6 +632,53 @@ exit: return ret; } +static void cros_ec_xfer_high_pri_work(struct work_struct *work) +{ + struct cros_ec_xfer_work_params *params; + + params = container_of(work, struct cros_ec_xfer_work_params, work); + params->ret = params->fn(params->ec_dev, params->ec_msg); +} + +static int cros_ec_xfer_high_pri(struct cros_ec_device *ec_dev, + struct cros_ec_command *ec_msg, + cros_ec_xfer_fn_t fn) +{ + struct cros_ec_xfer_work_params params; + + INIT_WORK_ONSTACK(¶ms.work, cros_ec_xfer_high_pri_work); + params.ec_dev = ec_dev; + params.ec_msg = ec_msg; + params.fn = fn; + + /* + * This looks a bit ridiculous. Why do the work on a + * different thread if we're just going to block waiting for + * the thread to finish? The key here is that the thread is + * running at high priority but the calling context might not + * be. We need to be at high priority to avoid getting + * context switched out for too long and the EC giving up on + * the transfer. + */ + queue_work(system_highpri_wq, ¶ms.work); + flush_work(¶ms.work); + destroy_work_on_stack(¶ms.work); + + return params.ret; +} + +static int cros_ec_pkt_xfer_spi(struct cros_ec_device *ec_dev, + struct cros_ec_command *ec_msg) +{ + return cros_ec_xfer_high_pri(ec_dev, ec_msg, do_cros_ec_pkt_xfer_spi); +} + +static int cros_ec_cmd_xfer_spi(struct cros_ec_device *ec_dev, + struct cros_ec_command *ec_msg) +{ + return cros_ec_xfer_high_pri(ec_dev, ec_msg, do_cros_ec_cmd_xfer_spi); +} + static void cros_ec_spi_dt_probe(struct cros_ec_spi *ec_spi, struct device *dev) { struct device_node *np = dev->of_node; -- cgit v1.2.3 From a2679b64719085196a8e1762a40e90e92b1f3cf5 Mon Sep 17 00:00:00 2001 From: Guenter Roeck Date: Wed, 3 Apr 2019 16:05:28 +0200 Subject: platform/chrome: Add CrOS USB PD logging driver The CrOS USB PD logging feature is logically separate functionality of the charge manager, hence has its own driver. The driver logs the event data for the USB PD charger available in some ChromeOS Embedded Controllers. Signed-off-by: Guenter Roeck [remove macro to APPEND_STRING and minor cleanups] Signed-off-by: Enric Balletbo i Serra --- drivers/platform/chrome/Kconfig | 12 ++ drivers/platform/chrome/Makefile | 1 + drivers/platform/chrome/cros_usbpd_logger.c | 262 ++++++++++++++++++++++++++++ 3 files changed, 275 insertions(+) create mode 100644 drivers/platform/chrome/cros_usbpd_logger.c (limited to 'drivers/platform') diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig index 9186d81a51cc..4313a8bcf0ab 100644 --- a/drivers/platform/chrome/Kconfig +++ b/drivers/platform/chrome/Kconfig @@ -152,6 +152,18 @@ config CROS_EC_SYSFS To compile this driver as a module, choose M here: the module will be called cros_ec_sysfs. +config CROS_USBPD_LOGGER + tristate "Logging driver for USB PD charger" + depends on CHARGER_CROS_USBPD + default y + select RTC_LIB + help + This option enables support for logging event data for the USB PD charger + available in the Embedded Controller on ChromeOS systems. + + To compile this driver as a module, choose M here: the + module will be called cros_usbpd_logger. + source "drivers/platform/chrome/wilco_ec/Kconfig" endif # CHROMEOS_PLATFORMS diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile index 1e2f0029b597..0bee2e44748e 100644 --- a/drivers/platform/chrome/Makefile +++ b/drivers/platform/chrome/Makefile @@ -14,5 +14,6 @@ obj-$(CONFIG_CROS_EC_LIGHTBAR) += cros_ec_lightbar.o obj-$(CONFIG_CROS_EC_VBC) += cros_ec_vbc.o obj-$(CONFIG_CROS_EC_DEBUGFS) += cros_ec_debugfs.o obj-$(CONFIG_CROS_EC_SYSFS) += cros_ec_sysfs.o +obj-$(CONFIG_CROS_USBPD_LOGGER) += cros_usbpd_logger.o obj-$(CONFIG_WILCO_EC) += wilco_ec/ diff --git a/drivers/platform/chrome/cros_usbpd_logger.c b/drivers/platform/chrome/cros_usbpd_logger.c new file mode 100644 index 000000000000..7c7b267626a0 --- /dev/null +++ b/drivers/platform/chrome/cros_usbpd_logger.c @@ -0,0 +1,262 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Logging driver for ChromeOS EC based USBPD Charger. + * + * Copyright 2018 Google LLC. + */ + +#include +#include +#include +#include +#include +#include +#include + +#define DRV_NAME "cros-usbpd-logger" + +#define CROS_USBPD_MAX_LOG_ENTRIES 30 +#define CROS_USBPD_LOG_UPDATE_DELAY msecs_to_jiffies(60000) +#define CROS_USBPD_DATA_SIZE 16 +#define CROS_USBPD_LOG_RESP_SIZE (sizeof(struct ec_response_pd_log) + \ + CROS_USBPD_DATA_SIZE) +#define CROS_USBPD_BUFFER_SIZE (sizeof(struct cros_ec_command) + \ + CROS_USBPD_LOG_RESP_SIZE) +/* Buffer for building the PDLOG string */ +#define BUF_SIZE 80 + +struct logger_data { + struct device *dev; + struct cros_ec_dev *ec_dev; + u8 ec_buffer[CROS_USBPD_BUFFER_SIZE]; + struct delayed_work log_work; + struct workqueue_struct *log_workqueue; +}; + +static const char * const chg_type_names[] = { + "None", "PD", "Type-C", "Proprietary", "DCP", "CDP", "SDP", + "Other", "VBUS" +}; + +static const char * const role_names[] = { + "Disconnected", "SRC", "SNK", "SNK (not charging)" +}; + +static const char * const fault_names[] = { + "---", "OCP", "fast OCP", "OVP", "Discharge" +}; + +static int append_str(char *buf, int pos, const char *fmt, ...) +{ + va_list args; + int i; + + va_start(args, fmt); + i = vsnprintf(buf + pos, BUF_SIZE - pos, fmt, args); + va_end(args); + + return i; +} + +static struct ec_response_pd_log *ec_get_log_entry(struct logger_data *logger) +{ + struct cros_ec_dev *ec_dev = logger->ec_dev; + struct cros_ec_command *msg; + int ret; + + msg = (struct cros_ec_command *)logger->ec_buffer; + + msg->command = ec_dev->cmd_offset + EC_CMD_PD_GET_LOG_ENTRY; + msg->insize = CROS_USBPD_LOG_RESP_SIZE; + + ret = cros_ec_cmd_xfer_status(ec_dev->ec_dev, msg); + if (ret < 0) + return ERR_PTR(ret); + + return (struct ec_response_pd_log *)msg->data; +} + +static void cros_usbpd_print_log_entry(struct ec_response_pd_log *r, + ktime_t tstamp) +{ + const char *fault, *role, *chg_type; + struct usb_chg_measures *meas; + struct mcdp_info *minfo; + int role_idx, type_idx; + char buf[BUF_SIZE + 1]; + struct rtc_time rt; + int len = 0; + s32 rem; + int i; + + /* The timestamp is the number of 1024th of seconds in the past */ + tstamp = ktime_sub_us(tstamp, r->timestamp << PD_LOG_TIMESTAMP_SHIFT); + rt = rtc_ktime_to_tm(tstamp); + + switch (r->type) { + case PD_EVENT_MCU_CHARGE: + if (r->data & CHARGE_FLAGS_OVERRIDE) + len += append_str(buf, len, "override "); + + if (r->data & CHARGE_FLAGS_DELAYED_OVERRIDE) + len += append_str(buf, len, "pending_override "); + + role_idx = r->data & CHARGE_FLAGS_ROLE_MASK; + role = role_idx < ARRAY_SIZE(role_names) ? + role_names[role_idx] : "Unknown"; + + type_idx = (r->data & CHARGE_FLAGS_TYPE_MASK) + >> CHARGE_FLAGS_TYPE_SHIFT; + + chg_type = type_idx < ARRAY_SIZE(chg_type_names) ? + chg_type_names[type_idx] : "???"; + + if (role_idx == USB_PD_PORT_POWER_DISCONNECTED || + role_idx == USB_PD_PORT_POWER_SOURCE) { + len += append_str(buf, len, "%s", role); + break; + } + + meas = (struct usb_chg_measures *)r->payload; + len += append_str(buf, len, "%s %s %s %dmV max %dmV / %dmA", + role, r->data & CHARGE_FLAGS_DUAL_ROLE ? + "DRP" : "Charger", + chg_type, meas->voltage_now, + meas->voltage_max, meas->current_max); + break; + case PD_EVENT_ACC_RW_FAIL: + len += append_str(buf, len, "RW signature check failed"); + break; + case PD_EVENT_PS_FAULT: + fault = r->data < ARRAY_SIZE(fault_names) ? fault_names[r->data] + : "???"; + len += append_str(buf, len, "Power supply fault: %s", fault); + break; + case PD_EVENT_VIDEO_DP_MODE: + len += append_str(buf, len, "DP mode %sabled", r->data == 1 ? + "en" : "dis"); + break; + case PD_EVENT_VIDEO_CODEC: + minfo = (struct mcdp_info *)r->payload; + len += append_str(buf, len, "HDMI info: family:%04x chipid:%04x ", + MCDP_FAMILY(minfo->family), + MCDP_CHIPID(minfo->chipid)); + len += append_str(buf, len, "irom:%d.%d.%d fw:%d.%d.%d", + minfo->irom.major, minfo->irom.minor, + minfo->irom.build, minfo->fw.major, + minfo->fw.minor, minfo->fw.build); + break; + default: + len += append_str(buf, len, "Event %02x (%04x) [", r->type, + r->data); + + for (i = 0; i < PD_LOG_SIZE(r->size_port); i++) + len += append_str(buf, len, "%02x ", r->payload[i]); + + len += append_str(buf, len, "]"); + break; + } + + div_s64_rem(ktime_to_ms(tstamp), MSEC_PER_SEC, &rem); + pr_info("PDLOG %d/%02d/%02d %02d:%02d:%02d.%03d P%d %s\n", + rt.tm_year + 1900, rt.tm_mon + 1, rt.tm_mday, + rt.tm_hour, rt.tm_min, rt.tm_sec, rem, + PD_LOG_PORT(r->size_port), buf); +} + +static void cros_usbpd_log_check(struct work_struct *work) +{ + struct logger_data *logger = container_of(to_delayed_work(work), + struct logger_data, + log_work); + struct device *dev = logger->dev; + struct ec_response_pd_log *r; + int entries = 0; + ktime_t now; + + while (entries++ < CROS_USBPD_MAX_LOG_ENTRIES) { + r = ec_get_log_entry(logger); + now = ktime_get_real(); + if (IS_ERR(r)) { + dev_dbg(dev, "Cannot get PD log %ld\n", PTR_ERR(r)); + break; + } + if (r->type == PD_EVENT_NO_ENTRY) + break; + + cros_usbpd_print_log_entry(r, now); + } + + queue_delayed_work(logger->log_workqueue, &logger->log_work, + CROS_USBPD_LOG_UPDATE_DELAY); +} + +static int cros_usbpd_logger_probe(struct platform_device *pd) +{ + struct cros_ec_dev *ec_dev = dev_get_drvdata(pd->dev.parent); + struct device *dev = &pd->dev; + struct logger_data *logger; + + logger = devm_kzalloc(dev, sizeof(*logger), GFP_KERNEL); + if (!logger) + return -ENOMEM; + + logger->dev = dev; + logger->ec_dev = ec_dev; + + platform_set_drvdata(pd, logger); + + /* Retrieve PD event logs periodically */ + INIT_DELAYED_WORK(&logger->log_work, cros_usbpd_log_check); + logger->log_workqueue = create_singlethread_workqueue("cros_usbpd_log"); + queue_delayed_work(logger->log_workqueue, &logger->log_work, + CROS_USBPD_LOG_UPDATE_DELAY); + + return 0; +} + +static int cros_usbpd_logger_remove(struct platform_device *pd) +{ + struct logger_data *logger = platform_get_drvdata(pd); + + cancel_delayed_work_sync(&logger->log_work); + + return 0; +} + +static int __maybe_unused cros_usbpd_logger_resume(struct device *dev) +{ + struct logger_data *logger = dev_get_drvdata(dev); + + queue_delayed_work(logger->log_workqueue, &logger->log_work, + CROS_USBPD_LOG_UPDATE_DELAY); + + return 0; +} + +static int __maybe_unused cros_usbpd_logger_suspend(struct device *dev) +{ + struct logger_data *logger = dev_get_drvdata(dev); + + cancel_delayed_work_sync(&logger->log_work); + + return 0; +} + +static SIMPLE_DEV_PM_OPS(cros_usbpd_logger_pm_ops, cros_usbpd_logger_suspend, + cros_usbpd_logger_resume); + +static struct platform_driver cros_usbpd_logger_driver = { + .driver = { + .name = DRV_NAME, + .pm = &cros_usbpd_logger_pm_ops, + }, + .probe = cros_usbpd_logger_probe, + .remove = cros_usbpd_logger_remove, +}; + +module_platform_driver(cros_usbpd_logger_driver); + +MODULE_LICENSE("GPL v2"); +MODULE_DESCRIPTION("Logging driver for ChromeOS EC USBPD Charger."); +MODULE_ALIAS("platform:" DRV_NAME); -- cgit v1.2.3 From 94d4e7af14a1170e34cf082d92e4c02de9e9fb88 Mon Sep 17 00:00:00 2001 From: Enrico Granata Date: Wed, 3 Apr 2019 15:40:36 -0700 Subject: platform/chrome: cros_ec_proto: check for NULL transfer function As new transfer mechanisms are added to the EC codebase, they may not support v2 of the EC protocol. If the v3 initial handshake transfer fails, the kernel will try and call cmd_xfer as a fallback. If v2 is not supported, cmd_xfer will be NULL, and the code will end up causing a kernel panic. Add a check for NULL before calling the transfer function, along with a helpful comment explaining how one might end up in this situation. Signed-off-by: Enrico Granata Reviewed-by: Jett Rink Signed-off-by: Enric Balletbo i Serra --- drivers/platform/chrome/cros_ec_proto.c | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'drivers/platform') diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c index 97a068dff192..3bb954997ebc 100644 --- a/drivers/platform/chrome/cros_ec_proto.c +++ b/drivers/platform/chrome/cros_ec_proto.c @@ -56,6 +56,17 @@ static int send_command(struct cros_ec_device *ec_dev, else xfer_fxn = ec_dev->cmd_xfer; + if (!xfer_fxn) { + /* + * This error can happen if a communication error happened and + * the EC is trying to use protocol v2, on an underlying + * communication mechanism that does not support v2. + */ + dev_err_once(ec_dev->dev, + "missing EC transfer API, cannot send command\n"); + return -EIO; + } + ret = (*xfer_fxn)(ec_dev, msg); if (msg->result == EC_RES_IN_PROGRESS) { int i; -- cgit v1.2.3 From 14e14aaf61321ba30d0bbdf4c4668f260ca1141c Mon Sep 17 00:00:00 2001 From: Nick Crews Date: Thu, 4 Apr 2019 16:54:15 -0600 Subject: platform/chrome: wilco_ec: Standardize mailbox interface The current API for the wilco EC mailbox interface is bad. It assumes that most messages sent to the EC follow a similar structure, with a command byte in MBOX[0], followed by a junk byte, followed by actual data. This doesn't happen in several cases, such as setting the RTC time, using the raw debugfs interface, and reading or writing properties such as the Peak Shift policy (this last to be submitted soon). Similarly for the response message from the EC, the current interface assumes that the first byte of data is always 0, and the second byte is unused. However, in both setting and getting the RTC time, in the debugfs interface, and for reading and writing properties, this isn't true. The current way to resolve this is to use WILCO_EC_FLAG_RAW* flags to specify when and when not to skip these initial bytes in the sent and received message. They are confusing and used so much that they are normal, and not exceptions. In addition, the first byte of response in the debugfs interface is still always skipped, which is weird, since this raw interface should be giving the entire result. Additionally, sent messages assume the first byte is a command, and so struct wilco_ec_message contains the "command" field. In setting or getting properties however, the first byte is not a command, and so this field has to be filled with a byte that isn't actually a command. This is again inconsistent. wilco_ec_message contains a result field as well, copied from wilco_ec_response->result. The message result field should be removed: if the message fails, the cause is already logged, and the callers are alerted. They will never care about the actual state of the result flag. These flags and different cases make the wilco_ec_transfer() function, used in wilco_ec_mailbox(), really gross, dealing with a bunch of different cases. It's difficult to figure out what it is doing. Finally, making these assumptions about the structure of a message make it so that the messages do not correspond well with the specification for the EC's mailbox interface. For instance, this interface specification may say that MBOX[9] in the received message contains some information, but the calling code needs to remember that the first byte of response is always skipped, and because it didn't set the RESPONSE_RAW flag, the next byte is also skipped, so this information is actually contained within wilco_ec_message->response_data[7]. This makes it difficult to maintain this code in the future. To fix these problems this patch standardizes the mailbox interface by: - Removing the WILCO_EC_FLAG_RAW* flags - Removing the command and reserved_raw bytes from wilco_ec_request - Removing the mbox0 byte from wilco_ec_response - Simplifying wilco_ec_transfer() because of these changes - Gives the callers of wilco_ec_mailbox() the responsibility of exactly and consistently defining the structure of the mailbox request and response - Removing command and result from wilco_ec_message. This results in the reduction of total code, and makes it much more maintainable and understandable. Signed-off-by: Nick Crews Acked-by: Alexandre Belloni Signed-off-by: Enric Balletbo i Serra --- drivers/platform/chrome/wilco_ec/debugfs.c | 42 +++++------------------ drivers/platform/chrome/wilco_ec/mailbox.c | 53 ++++++++++++------------------ 2 files changed, 29 insertions(+), 66 deletions(-) (limited to 'drivers/platform') diff --git a/drivers/platform/chrome/wilco_ec/debugfs.c b/drivers/platform/chrome/wilco_ec/debugfs.c index c090db2cd5be..8d307378c1cb 100644 --- a/drivers/platform/chrome/wilco_ec/debugfs.c +++ b/drivers/platform/chrome/wilco_ec/debugfs.c @@ -4,31 +4,7 @@ * * Copyright 2019 Google LLC * - * There is only one attribute used for debugging, called raw. - * You can write a hexadecimal sentence to raw, and that series of bytes - * will be sent to the EC. Then, you can read the bytes of response - * by reading from raw. - * - * For writing: - * Bytes 0-1 indicate the message type: - * 00 F0 = Execute Legacy Command - * 00 F2 = Read/Write NVRAM Property - * Byte 2 provides the command code - * Bytes 3+ consist of the data passed in the request - * - * When referencing the EC interface spec, byte 2 corresponds to MBOX[0], - * byte 3 corresponds to MBOX[1], etc. - * - * At least three bytes are required, for the msg type and command, - * with additional bytes optional for additional data. - * - * Example: - * // Request EC info type 3 (EC firmware build date) - * $ echo 00 f0 38 00 03 00 > raw - * // View the result. The decoded ASCII result "12/21/18" is - * // included after the raw hex. - * $ cat raw - * 00 31 32 2f 32 31 2f 31 38 00 38 00 01 00 2f 00 .12/21/18.8... + * See Documentation/ABI/testing/debugfs-wilco-ec for usage. */ #include @@ -136,18 +112,15 @@ static ssize_t raw_write(struct file *file, const char __user *user_buf, ret = parse_hex_sentence(buf, kcount, request_data, TYPE_AND_DATA_SIZE); if (ret < 0) return ret; - /* Need at least two bytes for message type and one for command */ + /* Need at least two bytes for message type and one byte of data */ if (ret < 3) return -EINVAL; - /* Clear response data buffer */ - memset(debug_info->raw_data, '\0', EC_MAILBOX_DATA_SIZE_EXTENDED); - msg.type = request_data[0] << 8 | request_data[1]; - msg.flags = WILCO_EC_FLAG_RAW; - msg.command = request_data[2]; - msg.request_data = ret > 3 ? request_data + 3 : 0; - msg.request_size = ret - 3; + msg.flags = 0; + msg.request_data = request_data + 2; + msg.request_size = ret - 2; + memset(debug_info->raw_data, 0, sizeof(debug_info->raw_data)); msg.response_data = debug_info->raw_data; msg.response_size = EC_MAILBOX_DATA_SIZE; @@ -174,7 +147,8 @@ static ssize_t raw_read(struct file *file, char __user *user_buf, size_t count, fmt_len = hex_dump_to_buffer(debug_info->raw_data, debug_info->response_size, 16, 1, debug_info->formatted_data, - FORMATTED_BUFFER_SIZE, true); + sizeof(debug_info->formatted_data), + true); /* Only return response the first time it is read */ debug_info->response_size = 0; } diff --git a/drivers/platform/chrome/wilco_ec/mailbox.c b/drivers/platform/chrome/wilco_ec/mailbox.c index 14355668ddfa..7fb58b487963 100644 --- a/drivers/platform/chrome/wilco_ec/mailbox.c +++ b/drivers/platform/chrome/wilco_ec/mailbox.c @@ -92,21 +92,10 @@ static void wilco_ec_prepare(struct wilco_ec_message *msg, struct wilco_ec_request *rq) { memset(rq, 0, sizeof(*rq)); - - /* Handle messages without trimming bytes from the request */ - if (msg->request_size && msg->flags & WILCO_EC_FLAG_RAW_REQUEST) { - rq->reserved_raw = *(u8 *)msg->request_data; - msg->request_size--; - memmove(msg->request_data, msg->request_data + 1, - msg->request_size); - } - - /* Fill in request packet */ rq->struct_version = EC_MAILBOX_PROTO_VERSION; rq->mailbox_id = msg->type; rq->mailbox_version = EC_MAILBOX_VERSION; - rq->data_size = msg->request_size + EC_MAILBOX_DATA_EXTRA; - rq->command = msg->command; + rq->data_size = msg->request_size; /* Checksum header and data */ rq->checksum = wilco_ec_checksum(rq, sizeof(*rq)); @@ -159,6 +148,12 @@ static int wilco_ec_transfer(struct wilco_ec_device *ec, return -EIO; } + /* + * The EC always returns either EC_MAILBOX_DATA_SIZE or + * EC_MAILBOX_DATA_SIZE_EXTENDED bytes of data, so we need to + * calculate the checksum on **all** of this data, even if we + * won't use all of it. + */ if (msg->flags & WILCO_EC_FLAG_EXTENDED_DATA) size = EC_MAILBOX_DATA_SIZE_EXTENDED; else @@ -173,33 +168,26 @@ static int wilco_ec_transfer(struct wilco_ec_device *ec, return -EBADMSG; } - /* Check that the EC reported success */ - msg->result = rs->result; - if (msg->result) { - dev_dbg(ec->dev, "bad response: 0x%02x\n", msg->result); + if (rs->result) { + dev_dbg(ec->dev, "EC reported failure: 0x%02x\n", rs->result); return -EBADMSG; } - /* Check the returned data size, skipping the header */ if (rs->data_size != size) { dev_dbg(ec->dev, "unexpected packet size (%u != %zu)", rs->data_size, size); return -EMSGSIZE; } - /* Skip 1 response data byte unless specified */ - size = (msg->flags & WILCO_EC_FLAG_RAW_RESPONSE) ? 0 : 1; - if ((ssize_t) rs->data_size - size < msg->response_size) { - dev_dbg(ec->dev, "response data too short (%zd < %zu)", - (ssize_t) rs->data_size - size, msg->response_size); + if (rs->data_size < msg->response_size) { + dev_dbg(ec->dev, "EC didn't return enough data (%u < %zu)", + rs->data_size, msg->response_size); return -EMSGSIZE; } - /* Ignore response data bytes as requested */ - memcpy(msg->response_data, rs->data + size, msg->response_size); + memcpy(msg->response_data, rs->data, msg->response_size); - /* Return actual amount of data received */ - return msg->response_size; + return rs->data_size; } /** @@ -207,10 +195,12 @@ static int wilco_ec_transfer(struct wilco_ec_device *ec, * @ec: EC device. * @msg: EC message data for request and response. * - * On entry msg->type, msg->flags, msg->command, msg->request_size, - * msg->response_size, and msg->request_data should all be filled in. + * On entry msg->type, msg->request_size, and msg->request_data should all be + * filled in. If desired, msg->flags can be set. * - * On exit msg->result and msg->response_data will be filled. + * If a response is expected, msg->response_size should be set, and + * msg->response_data should point to a buffer with enough space. On exit + * msg->response_data will be filled. * * Return: number of bytes received or negative error code on failure. */ @@ -219,9 +209,8 @@ int wilco_ec_mailbox(struct wilco_ec_device *ec, struct wilco_ec_message *msg) struct wilco_ec_request *rq; int ret; - dev_dbg(ec->dev, "cmd=%02x type=%04x flags=%02x rslen=%zu rqlen=%zu\n", - msg->command, msg->type, msg->flags, msg->response_size, - msg->request_size); + dev_dbg(ec->dev, "type=%04x flags=%02x rslen=%zu rqlen=%zu\n", + msg->type, msg->flags, msg->response_size, msg->request_size); mutex_lock(&ec->mailbox_lock); /* Prepare request packet */ -- cgit v1.2.3 From 9e2b0e0be64227ba8f09008d32d75280595a5464 Mon Sep 17 00:00:00 2001 From: Nick Crews Date: Fri, 12 Apr 2019 12:14:43 -0600 Subject: platform/chrome: wilco_ec: Add h1_gpio status to debugfs As part of Chrome OS's FAFT (Fully Automated Firmware Testing) tests, we need to ensure that the H1 chip is properly setting some GPIO lines. The h1_gpio attribute exposes the state of the lines: - ENTRY_TO_FACT_MODE in BIT(0) - SPI_CHROME_SEL in BIT(1) There are two reasons that I am exposing this in debugfs, and not as a GPIO: 1. This is only useful for testing, so end users shouldn't ever care about this. In fact, if it passes the tests, then the value of h1_gpio will always be 2, so it would be really uninteresting for users. 2. This GPIO is not connected to, controlled by, or really even related to the AP. The GPIO runs between the EC and the H1 security chip. Changes in v4: - Use "0x02x\n" instead of "02x\n" for format string - Use DEFINE_DEBUGFS_ATTRIBUTE() - Add documentation Changes in v3: - Fix documentation to correspond with formatting change in v2. Changes in v2: - Zero out the unused fields in the request. - Format result as "%02x\n" instead of as a decimal. Signed-off-by: Nick Crews Signed-off-by: Enric Balletbo i Serra --- drivers/platform/chrome/wilco_ec/debugfs.c | 47 ++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) (limited to 'drivers/platform') diff --git a/drivers/platform/chrome/wilco_ec/debugfs.c b/drivers/platform/chrome/wilco_ec/debugfs.c index 8d307378c1cb..f163476d080d 100644 --- a/drivers/platform/chrome/wilco_ec/debugfs.c +++ b/drivers/platform/chrome/wilco_ec/debugfs.c @@ -164,6 +164,51 @@ static const struct file_operations fops_raw = { .llseek = no_llseek, }; +#define CMD_KB_CHROME 0x88 +#define SUB_CMD_H1_GPIO 0x0A + +struct h1_gpio_status_request { + u8 cmd; /* Always CMD_KB_CHROME */ + u8 reserved; + u8 sub_cmd; /* Always SUB_CMD_H1_GPIO */ +} __packed; + +struct hi_gpio_status_response { + u8 status; /* 0 if allowed */ + u8 val; /* BIT(0)=ENTRY_TO_FACT_MODE, BIT(1)=SPI_CHROME_SEL */ +} __packed; + +static int h1_gpio_get(void *arg, u64 *val) +{ + struct wilco_ec_device *ec = arg; + struct h1_gpio_status_request rq; + struct hi_gpio_status_response rs; + struct wilco_ec_message msg; + int ret; + + memset(&rq, 0, sizeof(rq)); + rq.cmd = CMD_KB_CHROME; + rq.sub_cmd = SUB_CMD_H1_GPIO; + + memset(&msg, 0, sizeof(msg)); + msg.type = WILCO_EC_MSG_LEGACY; + msg.request_data = &rq; + msg.request_size = sizeof(rq); + msg.response_data = &rs; + msg.response_size = sizeof(rs); + ret = wilco_ec_mailbox(ec, &msg); + if (ret < 0) + return ret; + if (rs.status) + return -EIO; + + *val = rs.val; + + return 0; +} + +DEFINE_DEBUGFS_ATTRIBUTE(fops_h1_gpio, h1_gpio_get, NULL, "0x%02llx\n"); + /** * wilco_ec_debugfs_probe() - Create the debugfs node * @pdev: The platform device, probably created in core.c @@ -185,6 +230,8 @@ static int wilco_ec_debugfs_probe(struct platform_device *pdev) if (!debug_info->dir) return 0; debugfs_create_file("raw", 0644, debug_info->dir, NULL, &fops_raw); + debugfs_create_file("h1_gpio", 0444, debug_info->dir, ec, + &fops_h1_gpio); return 0; } -- cgit v1.2.3 From 2de89fd989584c54f5f3fcea808504c0149ba837 Mon Sep 17 00:00:00 2001 From: Pi-Hsun Shih Date: Fri, 12 Apr 2019 15:18:50 +0800 Subject: platform/chrome: cros_ec: Add EC host command support using rpmsg Add EC host command support through rpmsg. Signed-off-by: Pi-Hsun Shih Signed-off-by: Enric Balletbo i Serra --- drivers/platform/chrome/Kconfig | 12 ++ drivers/platform/chrome/Makefile | 1 + drivers/platform/chrome/cros_ec_rpmsg.c | 258 ++++++++++++++++++++++++++++++++ 3 files changed, 271 insertions(+) create mode 100644 drivers/platform/chrome/cros_ec_rpmsg.c (limited to 'drivers/platform') diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig index 4313a8bcf0ab..997317d2f2b9 100644 --- a/drivers/platform/chrome/Kconfig +++ b/drivers/platform/chrome/Kconfig @@ -59,6 +59,18 @@ config CROS_EC_I2C a checksum. Failing accesses will be retried three times to improve reliability. +config CROS_EC_RPMSG + tristate "ChromeOS Embedded Controller (rpmsg)" + depends on MFD_CROS_EC && RPMSG && OF + help + If you say Y here, you get support for talking to the ChromeOS EC + through rpmsg. This uses a simple byte-level protocol with a + checksum. Also since there's no addition EC-to-host interrupt, this + use a byte in message to distinguish host event from host command. + + To compile this driver as a module, choose M here: the + module will be called cros_ec_rpmsg. + config CROS_EC_SPI tristate "ChromeOS Embedded Controller (SPI)" depends on MFD_CROS_EC && SPI diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile index 0bee2e44748e..2cdee395c9f1 100644 --- a/drivers/platform/chrome/Makefile +++ b/drivers/platform/chrome/Makefile @@ -4,6 +4,7 @@ obj-$(CONFIG_CHROMEOS_LAPTOP) += chromeos_laptop.o obj-$(CONFIG_CHROMEOS_PSTORE) += chromeos_pstore.o obj-$(CONFIG_CHROMEOS_TBMC) += chromeos_tbmc.o obj-$(CONFIG_CROS_EC_I2C) += cros_ec_i2c.o +obj-$(CONFIG_CROS_EC_RPMSG) += cros_ec_rpmsg.o obj-$(CONFIG_CROS_EC_SPI) += cros_ec_spi.o cros_ec_lpcs-objs := cros_ec_lpc.o cros_ec_lpc_reg.o cros_ec_lpcs-$(CONFIG_CROS_EC_LPC_MEC) += cros_ec_lpc_mec.o diff --git a/drivers/platform/chrome/cros_ec_rpmsg.c b/drivers/platform/chrome/cros_ec_rpmsg.c new file mode 100644 index 000000000000..5d3fb2abad1d --- /dev/null +++ b/drivers/platform/chrome/cros_ec_rpmsg.c @@ -0,0 +1,258 @@ +// SPDX-License-Identifier: GPL-2.0 +// +// Copyright 2018 Google LLC. + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define EC_MSG_TIMEOUT_MS 200 +#define HOST_COMMAND_MARK 1 +#define HOST_EVENT_MARK 2 + +/** + * struct cros_ec_rpmsg_response - rpmsg message format from from EC. + * + * @type: The type of message, should be either HOST_COMMAND_MARK or + * HOST_EVENT_MARK, representing that the message is a response to + * host command, or a host event. + * @data: ec_host_response for host command. + */ +struct cros_ec_rpmsg_response { + u8 type; + u8 data[] __aligned(4); +}; + +/** + * struct cros_ec_rpmsg - information about a EC over rpmsg. + * + * @rpdev: rpmsg device we are connected to + * @xfer_ack: completion for host command transfer. + * @host_event_work: Work struct for pending host event. + */ +struct cros_ec_rpmsg { + struct rpmsg_device *rpdev; + struct completion xfer_ack; + struct work_struct host_event_work; +}; + +/** + * cros_ec_cmd_xfer_rpmsg - Transfer a message over rpmsg and receive the reply + * + * @ec_dev: ChromeOS EC device + * @ec_msg: Message to transfer + * + * This is only used for old EC proto version, and is not supported for this + * driver. + * + * Return: -EINVAL + */ +static int cros_ec_cmd_xfer_rpmsg(struct cros_ec_device *ec_dev, + struct cros_ec_command *ec_msg) +{ + return -EINVAL; +} + +/** + * cros_ec_pkt_xfer_rpmsg - Transfer a packet over rpmsg and receive the reply + * + * @ec_dev: ChromeOS EC device + * @ec_msg: Message to transfer + * + * Return: number of bytes of the reply on success or negative error code. + */ +static int cros_ec_pkt_xfer_rpmsg(struct cros_ec_device *ec_dev, + struct cros_ec_command *ec_msg) +{ + struct cros_ec_rpmsg *ec_rpmsg = ec_dev->priv; + struct rpmsg_device *rpdev = ec_rpmsg->rpdev; + struct ec_host_response *response; + unsigned long timeout; + int len; + int ret; + u8 sum; + int i; + + ec_msg->result = 0; + len = cros_ec_prepare_tx(ec_dev, ec_msg); + dev_dbg(ec_dev->dev, "prepared, len=%d\n", len); + + reinit_completion(&ec_rpmsg->xfer_ack); + ret = rpmsg_send(rpdev->ept, ec_dev->dout, len); + if (ret) { + dev_err(ec_dev->dev, "rpmsg send failed\n"); + return ret; + } + + timeout = msecs_to_jiffies(EC_MSG_TIMEOUT_MS); + ret = wait_for_completion_timeout(&ec_rpmsg->xfer_ack, timeout); + if (!ret) { + dev_err(ec_dev->dev, "rpmsg send timeout\n"); + return -EIO; + } + + /* check response error code */ + response = (struct ec_host_response *)ec_dev->din; + ec_msg->result = response->result; + + ret = cros_ec_check_result(ec_dev, ec_msg); + if (ret) + goto exit; + + if (response->data_len > ec_msg->insize) { + dev_err(ec_dev->dev, "packet too long (%d bytes, expected %d)", + response->data_len, ec_msg->insize); + ret = -EMSGSIZE; + goto exit; + } + + /* copy response packet payload and compute checksum */ + memcpy(ec_msg->data, ec_dev->din + sizeof(*response), + response->data_len); + + sum = 0; + for (i = 0; i < sizeof(*response) + response->data_len; i++) + sum += ec_dev->din[i]; + + if (sum) { + dev_err(ec_dev->dev, "bad packet checksum, calculated %x\n", + sum); + ret = -EBADMSG; + goto exit; + } + + ret = response->data_len; +exit: + if (ec_msg->command == EC_CMD_REBOOT_EC) + msleep(EC_REBOOT_DELAY_MS); + + return ret; +} + +static void +cros_ec_rpmsg_host_event_function(struct work_struct *host_event_work) +{ + struct cros_ec_rpmsg *ec_rpmsg = container_of(host_event_work, + struct cros_ec_rpmsg, + host_event_work); + struct cros_ec_device *ec_dev = dev_get_drvdata(&ec_rpmsg->rpdev->dev); + bool wake_event = true; + int ret; + + ret = cros_ec_get_next_event(ec_dev, &wake_event); + + /* + * Signal only if wake host events or any interrupt if + * cros_ec_get_next_event() returned an error (default value for + * wake_event is true) + */ + if (wake_event && device_may_wakeup(ec_dev->dev)) + pm_wakeup_event(ec_dev->dev, 0); + + if (ret > 0) + blocking_notifier_call_chain(&ec_dev->event_notifier, + 0, ec_dev); +} + +static int cros_ec_rpmsg_callback(struct rpmsg_device *rpdev, void *data, + int len, void *priv, u32 src) +{ + struct cros_ec_device *ec_dev = dev_get_drvdata(&rpdev->dev); + struct cros_ec_rpmsg *ec_rpmsg = ec_dev->priv; + struct cros_ec_rpmsg_response *resp; + + if (!len) { + dev_warn(ec_dev->dev, "rpmsg received empty response"); + return -EINVAL; + } + + resp = data; + len -= offsetof(struct cros_ec_rpmsg_response, data); + if (resp->type == HOST_COMMAND_MARK) { + if (len > ec_dev->din_size) { + dev_warn(ec_dev->dev, + "received length %d > din_size %d, truncating", + len, ec_dev->din_size); + len = ec_dev->din_size; + } + + memcpy(ec_dev->din, resp->data, len); + complete(&ec_rpmsg->xfer_ack); + } else if (resp->type == HOST_EVENT_MARK) { + schedule_work(&ec_rpmsg->host_event_work); + } else { + dev_warn(ec_dev->dev, "rpmsg received invalid type = %d", + resp->type); + return -EINVAL; + } + + return 0; +} + +static int cros_ec_rpmsg_probe(struct rpmsg_device *rpdev) +{ + struct device *dev = &rpdev->dev; + struct cros_ec_rpmsg *ec_rpmsg; + struct cros_ec_device *ec_dev; + + ec_dev = devm_kzalloc(dev, sizeof(*ec_dev), GFP_KERNEL); + if (!ec_dev) + return -ENOMEM; + + ec_rpmsg = devm_kzalloc(dev, sizeof(*ec_rpmsg), GFP_KERNEL); + if (!ec_rpmsg) + return -ENOMEM; + + ec_dev->dev = dev; + ec_dev->priv = ec_rpmsg; + ec_dev->cmd_xfer = cros_ec_cmd_xfer_rpmsg; + ec_dev->pkt_xfer = cros_ec_pkt_xfer_rpmsg; + ec_dev->phys_name = dev_name(&rpdev->dev); + ec_dev->din_size = sizeof(struct ec_host_response) + + sizeof(struct ec_response_get_protocol_info); + ec_dev->dout_size = sizeof(struct ec_host_request); + dev_set_drvdata(dev, ec_dev); + + ec_rpmsg->rpdev = rpdev; + init_completion(&ec_rpmsg->xfer_ack); + INIT_WORK(&ec_rpmsg->host_event_work, + cros_ec_rpmsg_host_event_function); + + return cros_ec_register(ec_dev); +} + +static void cros_ec_rpmsg_remove(struct rpmsg_device *rpdev) +{ + struct cros_ec_device *ec_dev = dev_get_drvdata(&rpdev->dev); + struct cros_ec_rpmsg *ec_rpmsg = ec_dev->priv; + + cancel_work_sync(&ec_rpmsg->host_event_work); +} + +static const struct of_device_id cros_ec_rpmsg_of_match[] = { + { .compatible = "google,cros-ec-rpmsg", }, + { } +}; +MODULE_DEVICE_TABLE(of, cros_ec_rpmsg_of_match); + +static struct rpmsg_driver cros_ec_driver_rpmsg = { + .drv = { + .name = "cros-ec-rpmsg", + .of_match_table = cros_ec_rpmsg_of_match, + }, + .probe = cros_ec_rpmsg_probe, + .remove = cros_ec_rpmsg_remove, + .callback = cros_ec_rpmsg_callback, +}; + +module_rpmsg_driver(cros_ec_driver_rpmsg); + +MODULE_LICENSE("GPL v2"); +MODULE_DESCRIPTION("ChromeOS EC multi function device (rpmsg)"); -- cgit v1.2.3 From 81f6ec233355be6898d99add553d27e31b4b7cd7 Mon Sep 17 00:00:00 2001 From: Enric Balletbo i Serra Date: Mon, 15 Apr 2019 23:35:06 +0200 Subject: platform/chrome: cros_ec_debugfs: Use cros_ec_cmd_xfer_status helper This patch makes use of cros_ec_cmd_xfer_status() instead of cros_ec_cmd_xfer() so we can remove some redundant code. Signed-off-by: Enric Balletbo i Serra Reviewed-by: Guenter Roeck --- drivers/platform/chrome/cros_ec_debugfs.c | 30 ++++++++---------------------- 1 file changed, 8 insertions(+), 22 deletions(-) (limited to 'drivers/platform') diff --git a/drivers/platform/chrome/cros_ec_debugfs.c b/drivers/platform/chrome/cros_ec_debugfs.c index a712508e4646..0bd163723548 100644 --- a/drivers/platform/chrome/cros_ec_debugfs.c +++ b/drivers/platform/chrome/cros_ec_debugfs.c @@ -72,15 +72,9 @@ static void cros_ec_console_log_work(struct work_struct *__work) int buf_space; int ret; - ret = cros_ec_cmd_xfer(ec->ec_dev, &snapshot_msg); - if (ret < 0) { - dev_err(ec->dev, "EC communication failed\n"); - goto resched; - } - if (snapshot_msg.result != EC_RES_SUCCESS) { - dev_err(ec->dev, "EC failed to snapshot the console log\n"); + ret = cros_ec_cmd_xfer_status(ec->ec_dev, &snapshot_msg); + if (ret < 0) goto resched; - } /* Loop until we have read everything, or there's an error. */ mutex_lock(&debug_info->log_mutex); @@ -95,16 +89,10 @@ static void cros_ec_console_log_work(struct work_struct *__work) memset(read_params, '\0', sizeof(*read_params)); read_params->subcmd = CONSOLE_READ_RECENT; - ret = cros_ec_cmd_xfer(ec->ec_dev, debug_info->read_msg); - if (ret < 0) { - dev_err(ec->dev, "EC communication failed\n"); - break; - } - if (debug_info->read_msg->result != EC_RES_SUCCESS) { - dev_err(ec->dev, - "EC failed to read the console log\n"); + ret = cros_ec_cmd_xfer_status(ec->ec_dev, + debug_info->read_msg); + if (ret < 0) break; - } /* If the buffer is empty, we're done here. */ if (ret == 0 || ec_buffer[0] == '\0') @@ -290,9 +278,8 @@ static int ec_read_version_supported(struct cros_ec_dev *ec) params->cmd = EC_CMD_CONSOLE_READ; response = (struct ec_response_get_cmd_versions *)msg->data; - ret = cros_ec_cmd_xfer(ec->ec_dev, msg) >= 0 && - msg->result == EC_RES_SUCCESS && - (response->version_mask & EC_VER_MASK(1)); + ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg) >= 0 && + response->version_mask & EC_VER_MASK(1); kfree(msg); @@ -372,9 +359,8 @@ static int cros_ec_create_panicinfo(struct cros_ec_debugfs *debug_info) msg->command = EC_CMD_GET_PANIC_INFO; msg->insize = insize; - ret = cros_ec_cmd_xfer(ec_dev, msg); + ret = cros_ec_cmd_xfer_status(ec_dev, msg); if (ret < 0) { - dev_warn(debug_info->ec->dev, "Cannot read panicinfo.\n"); ret = 0; goto free; } -- cgit v1.2.3 From 58a2109f6eb46b2952e2ce3fe776ce02c0c540dd Mon Sep 17 00:00:00 2001 From: Raul E Rangel Date: Tue, 16 Apr 2019 15:03:28 -0600 Subject: platform/chrome: cros_ec_proto: Add trace event to trace EC commands This is useful to see which EC commands are being executed and when. To enable: echo 1 > /sys/kernel/debug/tracing/events/cros_ec/enable Example: cros_ec_cmd: version: 0, command: EC_CMD_GET_VERSION cros_ec_cmd: version: 0, command: EC_CMD_GET_PROTOCOL_INFO cros_ec_cmd: version: 1, command: EC_CMD_GET_CMD_VERSIONS cros_ec_cmd: version: 1, command: EC_CMD_USB_PD_CONTROL The list of current commands is generated using the following script: sed -n 's/^#define \(EC_CMD_[[:alnum:]_]*\)\s.*/\tTRACE_SYMBOL(\1),\\/p' include/linux/mfd/cros_ec_commands.h Signed-off-by: Raul E Rangel Reviewed-by: Ross Zwisler Reviewed-by: Steven Rostedt (VMware) Signed-off-by: Enric Balletbo i Serra --- drivers/platform/chrome/Makefile | 5 +- drivers/platform/chrome/cros_ec_proto.c | 4 ++ drivers/platform/chrome/cros_ec_trace.c | 124 ++++++++++++++++++++++++++++++++ drivers/platform/chrome/cros_ec_trace.h | 51 +++++++++++++ 4 files changed, 183 insertions(+), 1 deletion(-) create mode 100644 drivers/platform/chrome/cros_ec_trace.c create mode 100644 drivers/platform/chrome/cros_ec_trace.h (limited to 'drivers/platform') diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile index 2cdee395c9f1..1b2f1dcfcd5c 100644 --- a/drivers/platform/chrome/Makefile +++ b/drivers/platform/chrome/Makefile @@ -1,5 +1,8 @@ # SPDX-License-Identifier: GPL-2.0 +# tell define_trace.h where to find the cros ec trace header +CFLAGS_cros_ec_trace.o:= -I$(src) + obj-$(CONFIG_CHROMEOS_LAPTOP) += chromeos_laptop.o obj-$(CONFIG_CHROMEOS_PSTORE) += chromeos_pstore.o obj-$(CONFIG_CHROMEOS_TBMC) += chromeos_tbmc.o @@ -9,7 +12,7 @@ obj-$(CONFIG_CROS_EC_SPI) += cros_ec_spi.o cros_ec_lpcs-objs := cros_ec_lpc.o cros_ec_lpc_reg.o cros_ec_lpcs-$(CONFIG_CROS_EC_LPC_MEC) += cros_ec_lpc_mec.o obj-$(CONFIG_CROS_EC_LPC) += cros_ec_lpcs.o -obj-$(CONFIG_CROS_EC_PROTO) += cros_ec_proto.o +obj-$(CONFIG_CROS_EC_PROTO) += cros_ec_proto.o cros_ec_trace.o obj-$(CONFIG_CROS_KBD_LED_BACKLIGHT) += cros_kbd_led_backlight.o obj-$(CONFIG_CROS_EC_LIGHTBAR) += cros_ec_lightbar.o obj-$(CONFIG_CROS_EC_VBC) += cros_ec_vbc.o diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c index 3bb954997ebc..171475862ede 100644 --- a/drivers/platform/chrome/cros_ec_proto.c +++ b/drivers/platform/chrome/cros_ec_proto.c @@ -10,6 +10,8 @@ #include #include +#include "cros_ec_trace.h" + #define EC_COMMAND_RETRIES 50 static int prepare_packet(struct cros_ec_device *ec_dev, @@ -51,6 +53,8 @@ static int send_command(struct cros_ec_device *ec_dev, int ret; int (*xfer_fxn)(struct cros_ec_device *ec, struct cros_ec_command *msg); + trace_cros_ec_cmd(msg); + if (ec_dev->proto_version > 2) xfer_fxn = ec_dev->pkt_xfer; else diff --git a/drivers/platform/chrome/cros_ec_trace.c b/drivers/platform/chrome/cros_ec_trace.c new file mode 100644 index 000000000000..0a76412095a9 --- /dev/null +++ b/drivers/platform/chrome/cros_ec_trace.c @@ -0,0 +1,124 @@ +// SPDX-License-Identifier: GPL-2.0 +// Trace events for the ChromeOS Embedded Controller +// +// Copyright 2019 Google LLC. + +#define TRACE_SYMBOL(a) {a, #a} + +// Generate the list using the following script: +// sed -n 's/^#define \(EC_CMD_[[:alnum:]_]*\)\s.*/\tTRACE_SYMBOL(\1), \\/p' include/linux/mfd/cros_ec_commands.h +#define EC_CMDS \ + TRACE_SYMBOL(EC_CMD_PROTO_VERSION), \ + TRACE_SYMBOL(EC_CMD_HELLO), \ + TRACE_SYMBOL(EC_CMD_GET_VERSION), \ + TRACE_SYMBOL(EC_CMD_READ_TEST), \ + TRACE_SYMBOL(EC_CMD_GET_BUILD_INFO), \ + TRACE_SYMBOL(EC_CMD_GET_CHIP_INFO), \ + TRACE_SYMBOL(EC_CMD_GET_BOARD_VERSION), \ + TRACE_SYMBOL(EC_CMD_READ_MEMMAP), \ + TRACE_SYMBOL(EC_CMD_GET_CMD_VERSIONS), \ + TRACE_SYMBOL(EC_CMD_GET_COMMS_STATUS), \ + TRACE_SYMBOL(EC_CMD_TEST_PROTOCOL), \ + TRACE_SYMBOL(EC_CMD_GET_PROTOCOL_INFO), \ + TRACE_SYMBOL(EC_CMD_GSV_PAUSE_IN_S5), \ + TRACE_SYMBOL(EC_CMD_GET_FEATURES), \ + TRACE_SYMBOL(EC_CMD_FLASH_INFO), \ + TRACE_SYMBOL(EC_CMD_FLASH_READ), \ + TRACE_SYMBOL(EC_CMD_FLASH_WRITE), \ + TRACE_SYMBOL(EC_CMD_FLASH_ERASE), \ + TRACE_SYMBOL(EC_CMD_FLASH_PROTECT), \ + TRACE_SYMBOL(EC_CMD_FLASH_REGION_INFO), \ + TRACE_SYMBOL(EC_CMD_VBNV_CONTEXT), \ + TRACE_SYMBOL(EC_CMD_PWM_GET_FAN_TARGET_RPM), \ + TRACE_SYMBOL(EC_CMD_PWM_SET_FAN_TARGET_RPM), \ + TRACE_SYMBOL(EC_CMD_PWM_GET_KEYBOARD_BACKLIGHT), \ + TRACE_SYMBOL(EC_CMD_PWM_SET_KEYBOARD_BACKLIGHT), \ + TRACE_SYMBOL(EC_CMD_PWM_SET_FAN_DUTY), \ + TRACE_SYMBOL(EC_CMD_PWM_SET_DUTY), \ + TRACE_SYMBOL(EC_CMD_PWM_GET_DUTY), \ + TRACE_SYMBOL(EC_CMD_LIGHTBAR_CMD), \ + TRACE_SYMBOL(EC_CMD_LED_CONTROL), \ + TRACE_SYMBOL(EC_CMD_VBOOT_HASH), \ + TRACE_SYMBOL(EC_CMD_MOTION_SENSE_CMD), \ + TRACE_SYMBOL(EC_CMD_USB_CHARGE_SET_MODE), \ + TRACE_SYMBOL(EC_CMD_PSTORE_INFO), \ + TRACE_SYMBOL(EC_CMD_PSTORE_READ), \ + TRACE_SYMBOL(EC_CMD_PSTORE_WRITE), \ + TRACE_SYMBOL(EC_CMD_RTC_GET_VALUE), \ + TRACE_SYMBOL(EC_CMD_RTC_GET_ALARM), \ + TRACE_SYMBOL(EC_CMD_RTC_SET_VALUE), \ + TRACE_SYMBOL(EC_CMD_RTC_SET_ALARM), \ + TRACE_SYMBOL(EC_CMD_PORT80_LAST_BOOT), \ + TRACE_SYMBOL(EC_CMD_PORT80_READ), \ + TRACE_SYMBOL(EC_CMD_THERMAL_SET_THRESHOLD), \ + TRACE_SYMBOL(EC_CMD_THERMAL_GET_THRESHOLD), \ + TRACE_SYMBOL(EC_CMD_THERMAL_AUTO_FAN_CTRL), \ + TRACE_SYMBOL(EC_CMD_TMP006_GET_CALIBRATION), \ + TRACE_SYMBOL(EC_CMD_TMP006_SET_CALIBRATION), \ + TRACE_SYMBOL(EC_CMD_TMP006_GET_RAW), \ + TRACE_SYMBOL(EC_CMD_MKBP_STATE), \ + TRACE_SYMBOL(EC_CMD_MKBP_INFO), \ + TRACE_SYMBOL(EC_CMD_MKBP_SIMULATE_KEY), \ + TRACE_SYMBOL(EC_CMD_MKBP_SET_CONFIG), \ + TRACE_SYMBOL(EC_CMD_MKBP_GET_CONFIG), \ + TRACE_SYMBOL(EC_CMD_KEYSCAN_SEQ_CTRL), \ + TRACE_SYMBOL(EC_CMD_GET_NEXT_EVENT), \ + TRACE_SYMBOL(EC_CMD_TEMP_SENSOR_GET_INFO), \ + TRACE_SYMBOL(EC_CMD_HOST_EVENT_GET_B), \ + TRACE_SYMBOL(EC_CMD_HOST_EVENT_GET_SMI_MASK), \ + TRACE_SYMBOL(EC_CMD_HOST_EVENT_GET_SCI_MASK), \ + TRACE_SYMBOL(EC_CMD_HOST_EVENT_GET_WAKE_MASK), \ + TRACE_SYMBOL(EC_CMD_HOST_EVENT_SET_SMI_MASK), \ + TRACE_SYMBOL(EC_CMD_HOST_EVENT_SET_SCI_MASK), \ + TRACE_SYMBOL(EC_CMD_HOST_EVENT_CLEAR), \ + TRACE_SYMBOL(EC_CMD_HOST_EVENT_SET_WAKE_MASK), \ + TRACE_SYMBOL(EC_CMD_HOST_EVENT_CLEAR_B), \ + TRACE_SYMBOL(EC_CMD_SWITCH_ENABLE_BKLIGHT), \ + TRACE_SYMBOL(EC_CMD_SWITCH_ENABLE_WIRELESS), \ + TRACE_SYMBOL(EC_CMD_GPIO_SET), \ + TRACE_SYMBOL(EC_CMD_GPIO_GET), \ + TRACE_SYMBOL(EC_CMD_I2C_READ), \ + TRACE_SYMBOL(EC_CMD_I2C_WRITE), \ + TRACE_SYMBOL(EC_CMD_CHARGE_CONTROL), \ + TRACE_SYMBOL(EC_CMD_CONSOLE_SNAPSHOT), \ + TRACE_SYMBOL(EC_CMD_CONSOLE_READ), \ + TRACE_SYMBOL(EC_CMD_BATTERY_CUT_OFF), \ + TRACE_SYMBOL(EC_CMD_USB_MUX), \ + TRACE_SYMBOL(EC_CMD_LDO_SET), \ + TRACE_SYMBOL(EC_CMD_LDO_GET), \ + TRACE_SYMBOL(EC_CMD_POWER_INFO), \ + TRACE_SYMBOL(EC_CMD_I2C_PASSTHRU), \ + TRACE_SYMBOL(EC_CMD_HANG_DETECT), \ + TRACE_SYMBOL(EC_CMD_CHARGE_STATE), \ + TRACE_SYMBOL(EC_CMD_CHARGE_CURRENT_LIMIT), \ + TRACE_SYMBOL(EC_CMD_EXTERNAL_POWER_LIMIT), \ + TRACE_SYMBOL(EC_CMD_HOST_SLEEP_EVENT), \ + TRACE_SYMBOL(EC_CMD_SB_READ_WORD), \ + TRACE_SYMBOL(EC_CMD_SB_WRITE_WORD), \ + TRACE_SYMBOL(EC_CMD_SB_READ_BLOCK), \ + TRACE_SYMBOL(EC_CMD_SB_WRITE_BLOCK), \ + TRACE_SYMBOL(EC_CMD_BATTERY_VENDOR_PARAM), \ + TRACE_SYMBOL(EC_CMD_CODEC_I2S), \ + TRACE_SYMBOL(EC_CMD_REBOOT_EC), \ + TRACE_SYMBOL(EC_CMD_GET_PANIC_INFO), \ + TRACE_SYMBOL(EC_CMD_ACPI_READ), \ + TRACE_SYMBOL(EC_CMD_ACPI_WRITE), \ + TRACE_SYMBOL(EC_CMD_ACPI_QUERY_EVENT), \ + TRACE_SYMBOL(EC_CMD_CEC_WRITE_MSG), \ + TRACE_SYMBOL(EC_CMD_CEC_SET), \ + TRACE_SYMBOL(EC_CMD_CEC_GET), \ + TRACE_SYMBOL(EC_CMD_REBOOT), \ + TRACE_SYMBOL(EC_CMD_RESEND_RESPONSE), \ + TRACE_SYMBOL(EC_CMD_VERSION0), \ + TRACE_SYMBOL(EC_CMD_PD_EXCHANGE_STATUS), \ + TRACE_SYMBOL(EC_CMD_USB_PD_CONTROL), \ + TRACE_SYMBOL(EC_CMD_USB_PD_PORTS), \ + TRACE_SYMBOL(EC_CMD_USB_PD_POWER_INFO), \ + TRACE_SYMBOL(EC_CMD_CHARGE_PORT_COUNT), \ + TRACE_SYMBOL(EC_CMD_USB_PD_DISCOVERY), \ + TRACE_SYMBOL(EC_CMD_PD_CHARGE_PORT_OVERRIDE), \ + TRACE_SYMBOL(EC_CMD_PD_GET_LOG_ENTRY), \ + TRACE_SYMBOL(EC_CMD_USB_PD_MUX_INFO) + +#define CREATE_TRACE_POINTS +#include "cros_ec_trace.h" diff --git a/drivers/platform/chrome/cros_ec_trace.h b/drivers/platform/chrome/cros_ec_trace.h new file mode 100644 index 000000000000..7ae3b89c78b9 --- /dev/null +++ b/drivers/platform/chrome/cros_ec_trace.h @@ -0,0 +1,51 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Trace events for the ChromeOS Embedded Controller + * + * Copyright 2019 Google LLC. + */ + +#undef TRACE_SYSTEM +#define TRACE_SYSTEM cros_ec + +#if !defined(_CROS_EC_TRACE_H_) || defined(TRACE_HEADER_MULTI_READ) +#define _CROS_EC_TRACE_H_ + +#include +#include + +#include + +DECLARE_EVENT_CLASS(cros_ec_cmd_class, + TP_PROTO(struct cros_ec_command *cmd), + TP_ARGS(cmd), + TP_STRUCT__entry( + __field(uint32_t, version) + __field(uint32_t, command) + ), + TP_fast_assign( + __entry->version = cmd->version; + __entry->command = cmd->command; + ), + TP_printk("version: %u, command: %s", __entry->version, + __print_symbolic(__entry->command, EC_CMDS)) +); + + +DEFINE_EVENT(cros_ec_cmd_class, cros_ec_cmd, + TP_PROTO(struct cros_ec_command *cmd), + TP_ARGS(cmd) +); + + +#endif /* _CROS_EC_TRACE_H_ */ + +/* this part must be outside header guard */ + +#undef TRACE_INCLUDE_PATH +#define TRACE_INCLUDE_PATH . + +#undef TRACE_INCLUDE_FILE +#define TRACE_INCLUDE_FILE cros_ec_trace + +#include -- cgit v1.2.3