From 432418bf27ababad360d4c07d2efcd235107a971 Mon Sep 17 00:00:00 2001 From: Tzung-Bi Shih Date: Mon, 21 Jul 2025 04:44:49 +0000 Subject: [PATCH 1/9] platform/chrome: cros_ec_chardev: Remove redundant struct field `ec_dev` in the `struct chardev_data` is unused. Remove the field and the whole struct as well. Link: https://lore.kernel.org/r/20250721044456.2736300-2-tzungbi@kernel.org Signed-off-by: Tzung-Bi Shih --- drivers/platform/chrome/cros_ec_chardev.c | 28 +++++++++-------------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/drivers/platform/chrome/cros_ec_chardev.c b/drivers/platform/chrome/cros_ec_chardev.c index 21a484385fc5..5c858d30dd52 100644 --- a/drivers/platform/chrome/cros_ec_chardev.c +++ b/drivers/platform/chrome/cros_ec_chardev.c @@ -31,11 +31,6 @@ /* Arbitrary bounded size for the event queue */ #define CROS_MAX_EVENT_LEN PAGE_SIZE -struct chardev_data { - struct cros_ec_dev *ec_dev; - struct miscdevice misc; -}; - struct chardev_priv { struct cros_ec_dev *ec_dev; struct notifier_block notifier; @@ -379,29 +374,28 @@ static int cros_ec_chardev_probe(struct platform_device *pdev) { struct cros_ec_dev *ec_dev = dev_get_drvdata(pdev->dev.parent); struct cros_ec_platform *ec_platform = dev_get_platdata(ec_dev->dev); - struct chardev_data *data; + struct miscdevice *misc; /* Create a char device: we want to create it anew */ - data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL); - if (!data) + misc = devm_kzalloc(&pdev->dev, sizeof(*misc), GFP_KERNEL); + if (!misc) return -ENOMEM; - data->ec_dev = ec_dev; - data->misc.minor = MISC_DYNAMIC_MINOR; - data->misc.fops = &chardev_fops; - data->misc.name = ec_platform->ec_name; - data->misc.parent = pdev->dev.parent; + misc->minor = MISC_DYNAMIC_MINOR; + misc->fops = &chardev_fops; + misc->name = ec_platform->ec_name; + misc->parent = pdev->dev.parent; - dev_set_drvdata(&pdev->dev, data); + dev_set_drvdata(&pdev->dev, misc); - return misc_register(&data->misc); + return misc_register(misc); } static void cros_ec_chardev_remove(struct platform_device *pdev) { - struct chardev_data *data = dev_get_drvdata(&pdev->dev); + struct miscdevice *misc = dev_get_drvdata(&pdev->dev); - misc_deregister(&data->misc); + misc_deregister(misc); } static const struct platform_device_id cros_ec_chardev_id[] = { From 032c59c1d3cd456053339fb95f6ee8e68c5ad1e2 Mon Sep 17 00:00:00 2001 From: Tzung-Bi Shih Date: Mon, 21 Jul 2025 04:44:51 +0000 Subject: [PATCH 2/9] platform/chrome: cros_ec_chardev: Decouple fops from struct cros_ec_dev The fops doesn't really need to hold a reference to struct cros_ec_dev. Remove the references from the fops. No functional changes. Link: https://lore.kernel.org/r/20250721044456.2736300-4-tzungbi@kernel.org Signed-off-by: Tzung-Bi Shih --- drivers/platform/chrome/cros_ec_chardev.c | 44 +++++++++++------------ 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/drivers/platform/chrome/cros_ec_chardev.c b/drivers/platform/chrome/cros_ec_chardev.c index 5c858d30dd52..c9d80ad5b57e 100644 --- a/drivers/platform/chrome/cros_ec_chardev.c +++ b/drivers/platform/chrome/cros_ec_chardev.c @@ -32,12 +32,13 @@ #define CROS_MAX_EVENT_LEN PAGE_SIZE struct chardev_priv { - struct cros_ec_dev *ec_dev; + struct cros_ec_device *ec_dev; struct notifier_block notifier; wait_queue_head_t wait_event; unsigned long event_mask; struct list_head events; size_t event_len; + u16 cmd_offset; }; struct ec_event { @@ -47,7 +48,7 @@ struct ec_event { u8 data[]; }; -static int ec_get_version(struct cros_ec_dev *ec, char *str, int maxlen) +static int ec_get_version(struct chardev_priv *priv, char *str, int maxlen) { static const char * const current_image_name[] = { "unknown", "read-only", "read-write", "invalid", @@ -60,10 +61,10 @@ static int ec_get_version(struct cros_ec_dev *ec, char *str, int maxlen) if (!msg) return -ENOMEM; - msg->command = EC_CMD_GET_VERSION + ec->cmd_offset; + msg->command = EC_CMD_GET_VERSION + priv->cmd_offset; msg->insize = sizeof(*resp); - ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg); + ret = cros_ec_cmd_xfer_status(priv->ec_dev, msg); if (ret < 0) { snprintf(str, maxlen, "Unknown EC version, returned error: %d\n", @@ -91,7 +92,7 @@ static int cros_ec_chardev_mkbp_event(struct notifier_block *nb, { struct chardev_priv *priv = container_of(nb, struct chardev_priv, notifier); - struct cros_ec_device *ec_dev = priv->ec_dev->ec_dev; + struct cros_ec_device *ec_dev = priv->ec_dev; struct ec_event *event; unsigned long event_bit = 1 << ec_dev->event_data.event_type; int total_size = sizeof(*event) + ec_dev->event_size; @@ -156,7 +157,8 @@ out: static int cros_ec_chardev_open(struct inode *inode, struct file *filp) { struct miscdevice *mdev = filp->private_data; - struct cros_ec_dev *ec_dev = dev_get_drvdata(mdev->parent); + struct cros_ec_dev *ec = dev_get_drvdata(mdev->parent); + struct cros_ec_device *ec_dev = ec->ec_dev; struct chardev_priv *priv; int ret; @@ -165,13 +167,14 @@ static int cros_ec_chardev_open(struct inode *inode, struct file *filp) return -ENOMEM; priv->ec_dev = ec_dev; + priv->cmd_offset = ec->cmd_offset; filp->private_data = priv; INIT_LIST_HEAD(&priv->events); init_waitqueue_head(&priv->wait_event); nonseekable_open(inode, filp); priv->notifier.notifier_call = cros_ec_chardev_mkbp_event; - ret = blocking_notifier_chain_register(&ec_dev->ec_dev->event_notifier, + ret = blocking_notifier_chain_register(&ec_dev->event_notifier, &priv->notifier); if (ret) { dev_err(ec_dev->dev, "failed to register event notifier\n"); @@ -199,7 +202,6 @@ static ssize_t cros_ec_chardev_read(struct file *filp, char __user *buffer, char msg[sizeof(struct ec_response_get_version) + sizeof(CROS_EC_DEV_VERSION)]; struct chardev_priv *priv = filp->private_data; - struct cros_ec_dev *ec_dev = priv->ec_dev; size_t count; int ret; @@ -233,7 +235,7 @@ static ssize_t cros_ec_chardev_read(struct file *filp, char __user *buffer, if (*offset != 0) return 0; - ret = ec_get_version(ec_dev, msg, sizeof(msg)); + ret = ec_get_version(priv, msg, sizeof(msg)); if (ret) return ret; @@ -249,10 +251,10 @@ static ssize_t cros_ec_chardev_read(struct file *filp, char __user *buffer, static int cros_ec_chardev_release(struct inode *inode, struct file *filp) { struct chardev_priv *priv = filp->private_data; - struct cros_ec_dev *ec_dev = priv->ec_dev; + struct cros_ec_device *ec_dev = priv->ec_dev; struct ec_event *event, *e; - blocking_notifier_chain_unregister(&ec_dev->ec_dev->event_notifier, + blocking_notifier_chain_unregister(&ec_dev->event_notifier, &priv->notifier); list_for_each_entry_safe(event, e, &priv->events, node) { @@ -267,7 +269,7 @@ static int cros_ec_chardev_release(struct inode *inode, struct file *filp) /* * Ioctls */ -static long cros_ec_chardev_ioctl_xcmd(struct cros_ec_dev *ec, void __user *arg) +static long cros_ec_chardev_ioctl_xcmd(struct chardev_priv *priv, void __user *arg) { struct cros_ec_command *s_cmd; struct cros_ec_command u_cmd; @@ -296,8 +298,8 @@ static long cros_ec_chardev_ioctl_xcmd(struct cros_ec_dev *ec, void __user *arg) goto exit; } - s_cmd->command += ec->cmd_offset; - ret = cros_ec_cmd_xfer(ec->ec_dev, s_cmd); + s_cmd->command += priv->cmd_offset; + ret = cros_ec_cmd_xfer(priv->ec_dev, s_cmd); /* Only copy data to userland if data was received. */ if (ret < 0) goto exit; @@ -309,10 +311,9 @@ exit: return ret; } -static long cros_ec_chardev_ioctl_readmem(struct cros_ec_dev *ec, - void __user *arg) +static long cros_ec_chardev_ioctl_readmem(struct chardev_priv *priv, void __user *arg) { - struct cros_ec_device *ec_dev = ec->ec_dev; + struct cros_ec_device *ec_dev = priv->ec_dev; struct cros_ec_readmem s_mem = { }; long num; @@ -341,16 +342,15 @@ static long cros_ec_chardev_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) { struct chardev_priv *priv = filp->private_data; - struct cros_ec_dev *ec = priv->ec_dev; if (_IOC_TYPE(cmd) != CROS_EC_DEV_IOC) return -ENOTTY; switch (cmd) { case CROS_EC_DEV_IOCXCMD: - return cros_ec_chardev_ioctl_xcmd(ec, (void __user *)arg); + return cros_ec_chardev_ioctl_xcmd(priv, (void __user *)arg); case CROS_EC_DEV_IOCRDMEM: - return cros_ec_chardev_ioctl_readmem(ec, (void __user *)arg); + return cros_ec_chardev_ioctl_readmem(priv, (void __user *)arg); case CROS_EC_DEV_IOCEVENTMASK: priv->event_mask = arg; return 0; @@ -372,8 +372,8 @@ static const struct file_operations chardev_fops = { static int cros_ec_chardev_probe(struct platform_device *pdev) { - struct cros_ec_dev *ec_dev = dev_get_drvdata(pdev->dev.parent); - struct cros_ec_platform *ec_platform = dev_get_platdata(ec_dev->dev); + struct cros_ec_dev *ec = dev_get_drvdata(pdev->dev.parent); + struct cros_ec_platform *ec_platform = dev_get_platdata(ec->dev); struct miscdevice *misc; /* Create a char device: we want to create it anew */ From 8abbbbb588f1f1bf95ae56c1531a17520ce487e2 Mon Sep 17 00:00:00 2001 From: "Gustavo A. R. Silva" Date: Mon, 11 Aug 2025 22:27:22 +0900 Subject: [PATCH 3/9] platform/chrome: cros_ec: Avoid -Wflex-array-member-not-at-end warning -Wflex-array-member-not-at-end was introduced in GCC-14, and we are getting ready to enable it, globally. Use the new TRAILING_OVERLAP() helper to fix the following warning: drivers/platform/chrome/cros_ec.c:106:40: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] This helper creates a union between a flexible-array member (FAM) and a set of members that would otherwise follow it. This overlays the trailing members onto the FAM while preserving the original memory layout. Signed-off-by: Gustavo A. R. Silva Link: https://lore.kernel.org/r/aJnvuv334M7TljoB@kspp Signed-off-by: Tzung-Bi Shih --- drivers/platform/chrome/cros_ec.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/platform/chrome/cros_ec.c b/drivers/platform/chrome/cros_ec.c index fd58781a2fb7..b1730362e399 100644 --- a/drivers/platform/chrome/cros_ec.c +++ b/drivers/platform/chrome/cros_ec.c @@ -102,14 +102,13 @@ EXPORT_SYMBOL(cros_ec_irq_thread); static int cros_ec_sleep_event(struct cros_ec_device *ec_dev, u8 sleep_event) { int ret; - struct { - struct cros_ec_command msg; + TRAILING_OVERLAP(struct cros_ec_command, msg, data, union { struct ec_params_host_sleep_event req0; struct ec_params_host_sleep_event_v1 req1; struct ec_response_host_sleep_event_v1 resp1; } u; - } __packed buf; + ) __packed buf; memset(&buf, 0, sizeof(buf)); From f7439a723e5aa5b35c76355e1b9b2cd1108f656e Mon Sep 17 00:00:00 2001 From: Liao Yuanhong Date: Wed, 13 Aug 2025 17:48:55 +0800 Subject: [PATCH 4/9] platform/chrome: wilco_ec: Remove redundant semicolons Remove unnecessary semicolons. Signed-off-by: Liao Yuanhong Link: https://lore.kernel.org/r/20250813094858.557742-1-liaoyuanhong@vivo.com Signed-off-by: Tzung-Bi Shih --- drivers/platform/chrome/wilco_ec/telemetry.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/platform/chrome/wilco_ec/telemetry.c b/drivers/platform/chrome/wilco_ec/telemetry.c index 7d8ae2cbf72f..b18043e31ae4 100644 --- a/drivers/platform/chrome/wilco_ec/telemetry.c +++ b/drivers/platform/chrome/wilco_ec/telemetry.c @@ -388,7 +388,7 @@ static int telem_device_probe(struct platform_device *pdev) dev_set_name(&dev_data->dev, TELEM_DEV_NAME_FMT, minor); device_initialize(&dev_data->dev); - /* Initialize the character device and add it to userspace */; + /* Initialize the character device and add it to userspace */ cdev_init(&dev_data->cdev, &telem_fops); error = cdev_device_add(&dev_data->cdev, &dev_data->dev); if (error) { From 918856986014142271a70a334d300994b9c41720 Mon Sep 17 00:00:00 2001 From: Tzung-Bi Shih Date: Thu, 28 Aug 2025 08:35:57 +0000 Subject: [PATCH 5/9] platform/chrome: Centralize cros_ec_device allocation Introduce a helper function, cros_ec_device_alloc(), to centralize the allocation of the struct cros_ec_device. Convert all protocol device drivers to use this new function. This is a preparatory step for separating common initialization logic out of device drivers' probe() and cros_ec_register(). Link: https://lore.kernel.org/r/20250828083601.856083-2-tzungbi@kernel.org Signed-off-by: Tzung-Bi Shih --- drivers/platform/chrome/cros_ec.c | 12 ++++++++++++ drivers/platform/chrome/cros_ec.h | 3 +++ drivers/platform/chrome/cros_ec_i2c.c | 4 ++-- drivers/platform/chrome/cros_ec_ishtp.c | 2 +- drivers/platform/chrome/cros_ec_lpc.c | 2 +- drivers/platform/chrome/cros_ec_rpmsg.c | 2 +- drivers/platform/chrome/cros_ec_spi.c | 2 +- drivers/platform/chrome/cros_ec_uart.c | 2 +- 8 files changed, 22 insertions(+), 7 deletions(-) diff --git a/drivers/platform/chrome/cros_ec.c b/drivers/platform/chrome/cros_ec.c index b1730362e399..25283a148ab9 100644 --- a/drivers/platform/chrome/cros_ec.c +++ b/drivers/platform/chrome/cros_ec.c @@ -30,6 +30,18 @@ static struct cros_ec_platform pd_p = { .cmd_offset = EC_CMD_PASSTHRU_OFFSET(CROS_EC_DEV_PD_INDEX), }; +struct cros_ec_device *cros_ec_device_alloc(struct device *dev) +{ + struct cros_ec_device *ec_dev; + + ec_dev = devm_kzalloc(dev, sizeof(*ec_dev), GFP_KERNEL); + if (!ec_dev) + return NULL; + + return ec_dev; +} +EXPORT_SYMBOL(cros_ec_device_alloc); + /** * cros_ec_irq_handler() - top half part of the interrupt handler * @irq: IRQ id diff --git a/drivers/platform/chrome/cros_ec.h b/drivers/platform/chrome/cros_ec.h index 6b95f1e0bace..cd4643bc5367 100644 --- a/drivers/platform/chrome/cros_ec.h +++ b/drivers/platform/chrome/cros_ec.h @@ -11,6 +11,9 @@ #include struct cros_ec_device; +struct device; + +struct cros_ec_device *cros_ec_device_alloc(struct device *dev); int cros_ec_register(struct cros_ec_device *ec_dev); void cros_ec_unregister(struct cros_ec_device *ec_dev); diff --git a/drivers/platform/chrome/cros_ec_i2c.c b/drivers/platform/chrome/cros_ec_i2c.c index 38af97cdaab2..ee3c5130ec3f 100644 --- a/drivers/platform/chrome/cros_ec_i2c.c +++ b/drivers/platform/chrome/cros_ec_i2c.c @@ -289,10 +289,10 @@ done: static int cros_ec_i2c_probe(struct i2c_client *client) { struct device *dev = &client->dev; - struct cros_ec_device *ec_dev = NULL; + struct cros_ec_device *ec_dev; int err; - ec_dev = devm_kzalloc(dev, sizeof(*ec_dev), GFP_KERNEL); + ec_dev = cros_ec_device_alloc(dev); if (!ec_dev) return -ENOMEM; diff --git a/drivers/platform/chrome/cros_ec_ishtp.c b/drivers/platform/chrome/cros_ec_ishtp.c index 7e7190b30cbb..c102a796670c 100644 --- a/drivers/platform/chrome/cros_ec_ishtp.c +++ b/drivers/platform/chrome/cros_ec_ishtp.c @@ -543,7 +543,7 @@ static int cros_ec_dev_init(struct ishtp_cl_data *client_data) struct cros_ec_device *ec_dev; struct device *dev = cl_data_to_dev(client_data); - ec_dev = devm_kzalloc(dev, sizeof(*ec_dev), GFP_KERNEL); + ec_dev = cros_ec_device_alloc(dev); if (!ec_dev) return -ENOMEM; diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c index 7d9a78289c96..30fa89b81666 100644 --- a/drivers/platform/chrome/cros_ec_lpc.c +++ b/drivers/platform/chrome/cros_ec_lpc.c @@ -637,7 +637,7 @@ static int cros_ec_lpc_probe(struct platform_device *pdev) } } - ec_dev = devm_kzalloc(dev, sizeof(*ec_dev), GFP_KERNEL); + ec_dev = cros_ec_device_alloc(dev); if (!ec_dev) return -ENOMEM; diff --git a/drivers/platform/chrome/cros_ec_rpmsg.c b/drivers/platform/chrome/cros_ec_rpmsg.c index bc2666491db1..9ac2b923db6d 100644 --- a/drivers/platform/chrome/cros_ec_rpmsg.c +++ b/drivers/platform/chrome/cros_ec_rpmsg.c @@ -216,7 +216,7 @@ static int cros_ec_rpmsg_probe(struct rpmsg_device *rpdev) struct cros_ec_device *ec_dev; int ret; - ec_dev = devm_kzalloc(dev, sizeof(*ec_dev), GFP_KERNEL); + ec_dev = cros_ec_device_alloc(dev); if (!ec_dev) return -ENOMEM; diff --git a/drivers/platform/chrome/cros_ec_spi.c b/drivers/platform/chrome/cros_ec_spi.c index 8ca0f854e7ac..c778300a4145 100644 --- a/drivers/platform/chrome/cros_ec_spi.c +++ b/drivers/platform/chrome/cros_ec_spi.c @@ -749,7 +749,7 @@ static int cros_ec_spi_probe(struct spi_device *spi) if (ec_spi == NULL) return -ENOMEM; ec_spi->spi = spi; - ec_dev = devm_kzalloc(dev, sizeof(*ec_dev), GFP_KERNEL); + ec_dev = cros_ec_device_alloc(dev); if (!ec_dev) return -ENOMEM; diff --git a/drivers/platform/chrome/cros_ec_uart.c b/drivers/platform/chrome/cros_ec_uart.c index 19c179d49c90..1a7511b1bbe3 100644 --- a/drivers/platform/chrome/cros_ec_uart.c +++ b/drivers/platform/chrome/cros_ec_uart.c @@ -259,7 +259,7 @@ static int cros_ec_uart_probe(struct serdev_device *serdev) if (!ec_uart) return -ENOMEM; - ec_dev = devm_kzalloc(dev, sizeof(*ec_dev), GFP_KERNEL); + ec_dev = cros_ec_device_alloc(dev); if (!ec_dev) return -ENOMEM; From e19ceeb1c0f63e3e15b197c5f34797134b51ba0e Mon Sep 17 00:00:00 2001 From: Tzung-Bi Shih Date: Thu, 28 Aug 2025 08:35:58 +0000 Subject: [PATCH 6/9] platform/chrome: Centralize common cros_ec_device initialization Move the common initialization from protocol device drivers into central cros_ec_device_alloc(). This removes duplicated code from each driver's probe function. The buffer sizes are now calculated once, using the maximum possible overhead required by any of the transport protocols, ensuring the allocated buffers are sufficient for all cases. Link: https://lore.kernel.org/r/20250828083601.856083-3-tzungbi@kernel.org Signed-off-by: Tzung-Bi Shih --- drivers/platform/chrome/cros_ec.c | 9 +++++++++ drivers/platform/chrome/cros_ec_i2c.c | 5 ----- drivers/platform/chrome/cros_ec_ishtp.c | 4 ---- drivers/platform/chrome/cros_ec_lpc.c | 4 ---- drivers/platform/chrome/cros_ec_rpmsg.c | 4 ---- drivers/platform/chrome/cros_ec_spi.c | 5 ----- drivers/platform/chrome/cros_ec_uart.c | 4 ---- include/linux/platform_data/cros_ec_proto.h | 14 ++++++++++---- 8 files changed, 19 insertions(+), 30 deletions(-) diff --git a/drivers/platform/chrome/cros_ec.c b/drivers/platform/chrome/cros_ec.c index 25283a148ab9..da049068b6e9 100644 --- a/drivers/platform/chrome/cros_ec.c +++ b/drivers/platform/chrome/cros_ec.c @@ -38,6 +38,15 @@ struct cros_ec_device *cros_ec_device_alloc(struct device *dev) if (!ec_dev) return NULL; + ec_dev->din_size = sizeof(struct ec_host_response) + + sizeof(struct ec_response_get_protocol_info) + + EC_MAX_RESPONSE_OVERHEAD; + ec_dev->dout_size = sizeof(struct ec_host_request) + + sizeof(struct ec_params_rwsig_action) + + EC_MAX_REQUEST_OVERHEAD; + + ec_dev->dev = dev; + return ec_dev; } EXPORT_SYMBOL(cros_ec_device_alloc); diff --git a/drivers/platform/chrome/cros_ec_i2c.c b/drivers/platform/chrome/cros_ec_i2c.c index ee3c5130ec3f..def1144a077e 100644 --- a/drivers/platform/chrome/cros_ec_i2c.c +++ b/drivers/platform/chrome/cros_ec_i2c.c @@ -297,16 +297,11 @@ static int cros_ec_i2c_probe(struct i2c_client *client) return -ENOMEM; i2c_set_clientdata(client, ec_dev); - ec_dev->dev = dev; ec_dev->priv = client; ec_dev->irq = client->irq; ec_dev->cmd_xfer = cros_ec_cmd_xfer_i2c; ec_dev->pkt_xfer = cros_ec_pkt_xfer_i2c; ec_dev->phys_name = client->adapter->name; - ec_dev->din_size = sizeof(struct ec_host_response_i2c) + - sizeof(struct ec_response_get_protocol_info); - ec_dev->dout_size = sizeof(struct ec_host_request_i2c) + - sizeof(struct ec_params_rwsig_action); err = cros_ec_register(ec_dev); if (err) { diff --git a/drivers/platform/chrome/cros_ec_ishtp.c b/drivers/platform/chrome/cros_ec_ishtp.c index c102a796670c..4e74e702c5a2 100644 --- a/drivers/platform/chrome/cros_ec_ishtp.c +++ b/drivers/platform/chrome/cros_ec_ishtp.c @@ -550,14 +550,10 @@ static int cros_ec_dev_init(struct ishtp_cl_data *client_data) client_data->ec_dev = ec_dev; dev->driver_data = ec_dev; - ec_dev->dev = dev; ec_dev->priv = client_data->cros_ish_cl; ec_dev->cmd_xfer = NULL; ec_dev->pkt_xfer = cros_ec_pkt_xfer_ish; ec_dev->phys_name = dev_name(dev); - ec_dev->din_size = sizeof(struct cros_ish_in_msg) + - sizeof(struct ec_response_get_protocol_info); - ec_dev->dout_size = sizeof(struct cros_ish_out_msg) + sizeof(struct ec_params_rwsig_action); return cros_ec_register(ec_dev); } diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c index 30fa89b81666..78cfff80cdea 100644 --- a/drivers/platform/chrome/cros_ec_lpc.c +++ b/drivers/platform/chrome/cros_ec_lpc.c @@ -642,14 +642,10 @@ static int cros_ec_lpc_probe(struct platform_device *pdev) return -ENOMEM; platform_set_drvdata(pdev, ec_dev); - ec_dev->dev = dev; ec_dev->phys_name = dev_name(dev); ec_dev->cmd_xfer = cros_ec_cmd_xfer_lpc; ec_dev->pkt_xfer = cros_ec_pkt_xfer_lpc; ec_dev->cmd_readmem = cros_ec_lpc_readmem; - 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) + sizeof(struct ec_params_rwsig_action); ec_dev->priv = ec_lpc; /* diff --git a/drivers/platform/chrome/cros_ec_rpmsg.c b/drivers/platform/chrome/cros_ec_rpmsg.c index 9ac2b923db6d..09bd9e49464e 100644 --- a/drivers/platform/chrome/cros_ec_rpmsg.c +++ b/drivers/platform/chrome/cros_ec_rpmsg.c @@ -224,14 +224,10 @@ static int cros_ec_rpmsg_probe(struct rpmsg_device *rpdev) 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) + sizeof(struct ec_params_rwsig_action); dev_set_drvdata(dev, ec_dev); ec_rpmsg->rpdev = rpdev; diff --git a/drivers/platform/chrome/cros_ec_spi.c b/drivers/platform/chrome/cros_ec_spi.c index c778300a4145..28fa82f8cb07 100644 --- a/drivers/platform/chrome/cros_ec_spi.c +++ b/drivers/platform/chrome/cros_ec_spi.c @@ -757,16 +757,11 @@ static int cros_ec_spi_probe(struct spi_device *spi) cros_ec_spi_dt_probe(ec_spi, dev); spi_set_drvdata(spi, ec_dev); - ec_dev->dev = dev; ec_dev->priv = ec_spi; ec_dev->irq = spi->irq; ec_dev->cmd_xfer = cros_ec_cmd_xfer_spi; ec_dev->pkt_xfer = cros_ec_pkt_xfer_spi; ec_dev->phys_name = dev_name(&ec_spi->spi->dev); - ec_dev->din_size = EC_MSG_PREAMBLE_COUNT + - sizeof(struct ec_host_response) + - sizeof(struct ec_response_get_protocol_info); - ec_dev->dout_size = sizeof(struct ec_host_request) + sizeof(struct ec_params_rwsig_action); ec_spi->last_transfer_ns = ktime_get_ns(); diff --git a/drivers/platform/chrome/cros_ec_uart.c b/drivers/platform/chrome/cros_ec_uart.c index 1a7511b1bbe3..d5b37414ff12 100644 --- a/drivers/platform/chrome/cros_ec_uart.c +++ b/drivers/platform/chrome/cros_ec_uart.c @@ -276,14 +276,10 @@ static int cros_ec_uart_probe(struct serdev_device *serdev) /* Initialize ec_dev for cros_ec */ ec_dev->phys_name = dev_name(dev); - ec_dev->dev = dev; ec_dev->priv = ec_uart; ec_dev->irq = ec_uart->irq; ec_dev->cmd_xfer = NULL; ec_dev->pkt_xfer = cros_ec_uart_pkt_xfer; - 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) + sizeof(struct ec_params_rwsig_action); serdev_device_set_client_ops(serdev, &cros_ec_uart_client_ops); diff --git a/include/linux/platform_data/cros_ec_proto.h b/include/linux/platform_data/cros_ec_proto.h index 3ec24f445c29..4d96cffbb9e3 100644 --- a/include/linux/platform_data/cros_ec_proto.h +++ b/include/linux/platform_data/cros_ec_proto.h @@ -33,12 +33,18 @@ /* * Max bus-specific overhead incurred by request/responses. - * I2C requires 1 additional byte for requests. - * I2C requires 2 additional bytes for responses. - * SPI requires up to 32 additional bytes for responses. + * + * Request: + * - I2C requires 1 byte (see struct ec_host_request_i2c). + * - ISHTP requires 4 bytes (see struct cros_ish_out_msg). + * + * Response: + * - I2C requires 2 bytes (see struct ec_host_response_i2c). + * - ISHTP requires 4 bytes (see struct cros_ish_in_msg). + * - SPI requires 32 bytes (see EC_MSG_PREAMBLE_COUNT). */ #define EC_PROTO_VERSION_UNKNOWN 0 -#define EC_MAX_REQUEST_OVERHEAD 1 +#define EC_MAX_REQUEST_OVERHEAD 4 #define EC_MAX_RESPONSE_OVERHEAD 32 /* From 7a79b0bfd8b3995a39d25bffcf57273635c0e542 Mon Sep 17 00:00:00 2001 From: Tzung-Bi Shih Date: Thu, 28 Aug 2025 08:35:59 +0000 Subject: [PATCH 7/9] platform/chrome: cros_ec: Separate initialization from cros_ec_register() Move the initialization of the `struct cros_ec_device` from cros_ec_register() into cros_ec_device_alloc(). This decouples device initialization from registration. By doing so, the per-device lock is now available immediately after allocation, allowing it to be used safely even before the device is fully registered. Link: https://lore.kernel.org/r/20250828083601.856083-4-tzungbi@kernel.org Signed-off-by: Tzung-Bi Shih --- drivers/platform/chrome/cros_ec.c | 57 ++++++++++++++++--------------- 1 file changed, 30 insertions(+), 27 deletions(-) diff --git a/drivers/platform/chrome/cros_ec.c b/drivers/platform/chrome/cros_ec.c index da049068b6e9..61bcef8741db 100644 --- a/drivers/platform/chrome/cros_ec.c +++ b/drivers/platform/chrome/cros_ec.c @@ -30,6 +30,14 @@ static struct cros_ec_platform pd_p = { .cmd_offset = EC_CMD_PASSTHRU_OFFSET(CROS_EC_DEV_PD_INDEX), }; +static void cros_ec_device_free(void *data) +{ + struct cros_ec_device *ec_dev = data; + + mutex_destroy(&ec_dev->lock); + lockdep_unregister_key(&ec_dev->lockdep_key); +} + struct cros_ec_device *cros_ec_device_alloc(struct device *dev) { struct cros_ec_device *ec_dev; @@ -45,7 +53,28 @@ struct cros_ec_device *cros_ec_device_alloc(struct device *dev) sizeof(struct ec_params_rwsig_action) + EC_MAX_REQUEST_OVERHEAD; + ec_dev->din = devm_kzalloc(dev, ec_dev->din_size, GFP_KERNEL); + if (!ec_dev->din) + return NULL; + + ec_dev->dout = devm_kzalloc(dev, ec_dev->dout_size, GFP_KERNEL); + if (!ec_dev->dout) + return NULL; + ec_dev->dev = dev; + ec_dev->max_response = sizeof(struct ec_response_get_protocol_info); + ec_dev->max_request = sizeof(struct ec_params_rwsig_action); + ec_dev->suspend_timeout_ms = EC_HOST_SLEEP_TIMEOUT_DEFAULT; + + BLOCKING_INIT_NOTIFIER_HEAD(&ec_dev->event_notifier); + BLOCKING_INIT_NOTIFIER_HEAD(&ec_dev->panic_notifier); + + lockdep_register_key(&ec_dev->lockdep_key); + mutex_init(&ec_dev->lock); + lockdep_set_class(&ec_dev->lock, &ec_dev->lockdep_key); + + if (devm_add_action_or_reset(dev, cros_ec_device_free, ec_dev)) + return NULL; return ec_dev; } @@ -200,29 +229,7 @@ static int cros_ec_ready_event(struct notifier_block *nb, int cros_ec_register(struct cros_ec_device *ec_dev) { struct device *dev = ec_dev->dev; - int err = 0; - - BLOCKING_INIT_NOTIFIER_HEAD(&ec_dev->event_notifier); - BLOCKING_INIT_NOTIFIER_HEAD(&ec_dev->panic_notifier); - - 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->ec = NULL; - ec_dev->pd = NULL; - ec_dev->suspend_timeout_ms = EC_HOST_SLEEP_TIMEOUT_DEFAULT; - - ec_dev->din = devm_kzalloc(dev, ec_dev->din_size, GFP_KERNEL); - if (!ec_dev->din) - return -ENOMEM; - - ec_dev->dout = devm_kzalloc(dev, ec_dev->dout_size, GFP_KERNEL); - if (!ec_dev->dout) - return -ENOMEM; - - lockdep_register_key(&ec_dev->lockdep_key); - mutex_init(&ec_dev->lock); - lockdep_set_class(&ec_dev->lock, &ec_dev->lockdep_key); + int err; /* Send RWSIG continue to jump to RW for devices using RWSIG. */ err = cros_ec_rwsig_continue(ec_dev); @@ -322,8 +329,6 @@ int cros_ec_register(struct cros_ec_device *ec_dev) exit: platform_device_unregister(ec_dev->ec); platform_device_unregister(ec_dev->pd); - mutex_destroy(&ec_dev->lock); - lockdep_unregister_key(&ec_dev->lockdep_key); return err; } EXPORT_SYMBOL(cros_ec_register); @@ -343,8 +348,6 @@ void cros_ec_unregister(struct cros_ec_device *ec_dev) &ec_dev->notifier_ready); platform_device_unregister(ec_dev->pd); platform_device_unregister(ec_dev->ec); - mutex_destroy(&ec_dev->lock); - lockdep_unregister_key(&ec_dev->lockdep_key); } EXPORT_SYMBOL(cros_ec_unregister); From 56cb557279d70397cefb497e0f06bdd6fd685f8e Mon Sep 17 00:00:00 2001 From: Tzung-Bi Shih Date: Thu, 28 Aug 2025 08:36:00 +0000 Subject: [PATCH 8/9] platform/chrome: cros_ec: Add a flag to track registration state Introduce a `registered` flag to the `struct cros_ec_device` to allow callers to determine if the device has been fully registered and is ready for use. This is a preparatory step to prevent race conditions where other drivers might try to access the device before it is fully registered or after it has been unregistered. Link: https://lore.kernel.org/r/20250828083601.856083-5-tzungbi@kernel.org Signed-off-by: Tzung-Bi Shih --- drivers/platform/chrome/cros_ec.c | 7 +++++++ drivers/platform/chrome/cros_ec_proto.c | 15 +++++++++++++++ include/linux/platform_data/cros_ec_proto.h | 4 ++++ 3 files changed, 26 insertions(+) diff --git a/drivers/platform/chrome/cros_ec.c b/drivers/platform/chrome/cros_ec.c index 61bcef8741db..1da79e3d215b 100644 --- a/drivers/platform/chrome/cros_ec.c +++ b/drivers/platform/chrome/cros_ec.c @@ -9,6 +9,7 @@ * battery charging and regulator control, firmware update. */ +#include #include #include #include @@ -316,6 +317,9 @@ int cros_ec_register(struct cros_ec_device *ec_dev) goto exit; } + scoped_guard(mutex, &ec_dev->lock) + ec_dev->registered = true; + dev_info(dev, "Chrome EC device registered\n"); /* @@ -343,6 +347,9 @@ EXPORT_SYMBOL(cros_ec_register); */ void cros_ec_unregister(struct cros_ec_device *ec_dev) { + scoped_guard(mutex, &ec_dev->lock) + ec_dev->registered = false; + if (ec_dev->mkbp_event_supported) blocking_notifier_chain_unregister(&ec_dev->event_notifier, &ec_dev->notifier_ready); diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c index 3e94a0a82173..1d8d9168ec1a 100644 --- a/drivers/platform/chrome/cros_ec_proto.c +++ b/drivers/platform/chrome/cros_ec_proto.c @@ -3,6 +3,7 @@ // // Copyright (C) 2015 Google, Inc +#include #include #include #include @@ -1153,5 +1154,19 @@ int cros_ec_get_cmd_versions(struct cros_ec_device *ec_dev, u16 cmd) } EXPORT_SYMBOL_GPL(cros_ec_get_cmd_versions); +/** + * cros_ec_device_registered - Return if the ec_dev is registered. + * + * @ec_dev: EC device + * + * Return: true if registered. Otherwise, false. + */ +bool cros_ec_device_registered(struct cros_ec_device *ec_dev) +{ + guard(mutex)(&ec_dev->lock); + return ec_dev->registered; +} +EXPORT_SYMBOL_GPL(cros_ec_device_registered); + MODULE_LICENSE("GPL"); MODULE_DESCRIPTION("ChromeOS EC communication protocol helpers"); diff --git a/include/linux/platform_data/cros_ec_proto.h b/include/linux/platform_data/cros_ec_proto.h index 4d96cffbb9e3..de14923720a5 100644 --- a/include/linux/platform_data/cros_ec_proto.h +++ b/include/linux/platform_data/cros_ec_proto.h @@ -128,6 +128,7 @@ struct cros_ec_command { * @dout_size: Size of dout buffer to allocate (zero to use static dout). * @wake_enabled: True if this device can wake the system from sleep. * @suspended: True if this device had been suspended. + * @registered: True if this device had been registered. * @cmd_xfer: Send command to EC and get response. * Returns the number of bytes received if the communication * succeeded, but that doesn't mean the EC was happy with the @@ -186,6 +187,7 @@ struct cros_ec_device { int dout_size; bool wake_enabled; bool suspended; + bool registered; int (*cmd_xfer)(struct cros_ec_device *ec, struct cros_ec_command *msg); int (*pkt_xfer)(struct cros_ec_device *ec, @@ -278,6 +280,8 @@ int cros_ec_cmd_readmem(struct cros_ec_device *ec_dev, u8 offset, u8 size, void int cros_ec_get_cmd_versions(struct cros_ec_device *ec_dev, u16 cmd); +bool cros_ec_device_registered(struct cros_ec_device *ec_dev); + /** * cros_ec_get_time_ns() - Return time in ns. * From 48633acccf38d706d7b368400647bb9db9caf1ae Mon Sep 17 00:00:00 2001 From: Tzung-Bi Shih Date: Thu, 28 Aug 2025 08:36:01 +0000 Subject: [PATCH 9/9] Input: cros_ec_keyb - Defer probe until parent EC device is registered The `cros_ec_keyb` driver can be probed before the cros_ec_device has completed the registration. This creates a race condition where `cros_ec_keyb` might access uninitialized data. Fix this by calling `cros_ec_device_registered()` to check the parent's status. If the device is not yet ready, return -EPROBE_DEFER to ensure the probe is retried later. Acked-by: Dmitry Torokhov Link: https://lore.kernel.org/r/20250828083601.856083-6-tzungbi@kernel.org Signed-off-by: Tzung-Bi Shih --- drivers/input/keyboard/cros_ec_keyb.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c index c1e53d87c8a7..f7209c8ebbcc 100644 --- a/drivers/input/keyboard/cros_ec_keyb.c +++ b/drivers/input/keyboard/cros_ec_keyb.c @@ -705,6 +705,12 @@ static int cros_ec_keyb_probe(struct platform_device *pdev) ec = dev_get_drvdata(pdev->dev.parent); if (!ec) return -EPROBE_DEFER; + /* + * Even if the cros_ec_device pointer is available, still need to check + * if the device is fully registered before using it. + */ + if (!cros_ec_device_registered(ec)) + return -EPROBE_DEFER; ckdev = devm_kzalloc(dev, sizeof(*ckdev), GFP_KERNEL); if (!ckdev)