From 5f9952548d91263eaf70a2ca71f8897c2a638cf1 Mon Sep 17 00:00:00 2001 From: Dan Callaghan Date: Tue, 18 Oct 2022 15:06:23 +1100 Subject: platform/chrome: add a driver for HPS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch introduces a driver for the ChromeOS human presence sensor (aka. HPS). The driver supports a sensor connected to the I2C bus and identified as "GOOG0020" in the ACPI tables. When loaded, the driver exports the sensor to userspace through a character device. This device only supports power management, i.e., communication with the sensor must be done through regular I2C transmissions from userspace. Power management is implemented by enabling the respective power GPIO while at least one userspace process holds an open fd on the character device. By default, the device is powered down if there are no active clients. Note that the driver makes no effort to preserve the state of the sensor between power down and power up events. Userspace is responsible for reinitializing any needed state once power has been restored. The device firmware, I2C protocol and other documentation is available at https://chromium.googlesource.com/chromiumos/platform/hps-firmware. Co-developed-by: Sami Kyöstilä Signed-off-by: Sami Kyöstilä Signed-off-by: Dan Callaghan Signed-off-by: Tzung-Bi Shih Link: https://lore.kernel.org/r/20221018040623.2173441-1-dcallagh@chromium.org --- drivers/platform/chrome/Kconfig | 10 ++ drivers/platform/chrome/Makefile | 1 + drivers/platform/chrome/cros_hps_i2c.c | 162 +++++++++++++++++++++++++++++++++ 3 files changed, 173 insertions(+) create mode 100644 drivers/platform/chrome/cros_hps_i2c.c (limited to 'drivers') diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig index 6b954c5acadb..c1ca247987d2 100644 --- a/drivers/platform/chrome/Kconfig +++ b/drivers/platform/chrome/Kconfig @@ -228,6 +228,16 @@ config CROS_EC_TYPEC To compile this driver as a module, choose M here: the module will be called cros_ec_typec. +config CROS_HPS_I2C + tristate "ChromeOS HPS device" + depends on HID && I2C && PM + help + Say Y here if you want to enable support for the ChromeOS + human presence sensor (HPS), attached via I2C. The driver supports a + sensor connected to the I2C bus and exposes it as a character device. + To save power, the sensor is automatically powered down when no + clients are accessing it. + config CROS_USBPD_LOGGER tristate "Logging driver for USB PD charger" depends on CHARGER_CROS_USBPD diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile index 2950610101f1..f6068d077a40 100644 --- a/drivers/platform/chrome/Makefile +++ b/drivers/platform/chrome/Makefile @@ -27,6 +27,7 @@ obj-$(CONFIG_CROS_EC_DEBUGFS) += cros_ec_debugfs.o cros-ec-sensorhub-objs := cros_ec_sensorhub.o cros_ec_sensorhub_ring.o obj-$(CONFIG_CROS_EC_SENSORHUB) += cros-ec-sensorhub.o obj-$(CONFIG_CROS_EC_SYSFS) += cros_ec_sysfs.o +obj-$(CONFIG_CROS_HPS_I2C) += cros_hps_i2c.o obj-$(CONFIG_CROS_USBPD_LOGGER) += cros_usbpd_logger.o obj-$(CONFIG_CROS_USBPD_NOTIFY) += cros_usbpd_notify.o diff --git a/drivers/platform/chrome/cros_hps_i2c.c b/drivers/platform/chrome/cros_hps_i2c.c new file mode 100644 index 000000000000..92da59d94745 --- /dev/null +++ b/drivers/platform/chrome/cros_hps_i2c.c @@ -0,0 +1,162 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Driver for the ChromeOS human presence sensor (HPS), attached via I2C. + * + * The driver exposes HPS as a character device, although currently no read or + * write operations are supported. Instead, the driver only controls the power + * state of the sensor, keeping it on only while userspace holds an open file + * descriptor to the HPS device. + * + * Copyright 2022 Google LLC. + */ + +#include +#include +#include +#include +#include +#include +#include + +#define HPS_ACPI_ID "GOOG0020" + +struct hps_drvdata { + struct i2c_client *client; + struct miscdevice misc_device; + struct gpio_desc *enable_gpio; +}; + +static void hps_set_power(struct hps_drvdata *hps, bool state) +{ + gpiod_set_value_cansleep(hps->enable_gpio, state); +} + +static int hps_open(struct inode *inode, struct file *file) +{ + struct hps_drvdata *hps = container_of(file->private_data, + struct hps_drvdata, misc_device); + struct device *dev = &hps->client->dev; + + return pm_runtime_resume_and_get(dev); +} + +static int hps_release(struct inode *inode, struct file *file) +{ + struct hps_drvdata *hps = container_of(file->private_data, + struct hps_drvdata, misc_device); + struct device *dev = &hps->client->dev; + + return pm_runtime_put(dev); +} + +static const struct file_operations hps_fops = { + .owner = THIS_MODULE, + .open = hps_open, + .release = hps_release, +}; + +static int hps_i2c_probe(struct i2c_client *client) +{ + struct hps_drvdata *hps; + int ret; + + hps = devm_kzalloc(&client->dev, sizeof(*hps), GFP_KERNEL); + if (!hps) + return -ENOMEM; + + hps->misc_device.parent = &client->dev; + hps->misc_device.minor = MISC_DYNAMIC_MINOR; + hps->misc_device.name = "cros-hps"; + hps->misc_device.fops = &hps_fops; + + i2c_set_clientdata(client, hps); + hps->client = client; + + /* + * HPS is powered on from firmware before entering the kernel, so we + * acquire the line with GPIOD_OUT_HIGH here to preserve the existing + * state. The peripheral is powered off after successful probe below. + */ + hps->enable_gpio = devm_gpiod_get(&client->dev, "enable", GPIOD_OUT_HIGH); + if (IS_ERR(hps->enable_gpio)) { + ret = PTR_ERR(hps->enable_gpio); + dev_err(&client->dev, "failed to get enable gpio: %d\n", ret); + return ret; + } + + ret = misc_register(&hps->misc_device); + if (ret) { + dev_err(&client->dev, "failed to initialize misc device: %d\n", ret); + return ret; + } + + hps_set_power(hps, false); + pm_runtime_enable(&client->dev); + return 0; +} + +static int hps_i2c_remove(struct i2c_client *client) +{ + struct hps_drvdata *hps = i2c_get_clientdata(client); + + pm_runtime_disable(&client->dev); + misc_deregister(&hps->misc_device); + + /* + * Re-enable HPS, in order to return it to its default state + * (i.e. powered on). + */ + hps_set_power(hps, true); + + return 0; +} + +static int hps_suspend(struct device *dev) +{ + struct i2c_client *client = to_i2c_client(dev); + struct hps_drvdata *hps = i2c_get_clientdata(client); + + hps_set_power(hps, false); + return 0; +} + +static int hps_resume(struct device *dev) +{ + struct i2c_client *client = to_i2c_client(dev); + struct hps_drvdata *hps = i2c_get_clientdata(client); + + hps_set_power(hps, true); + return 0; +} +static UNIVERSAL_DEV_PM_OPS(hps_pm_ops, hps_suspend, hps_resume, NULL); + +static const struct i2c_device_id hps_i2c_id[] = { + { "cros-hps", 0 }, + { } +}; +MODULE_DEVICE_TABLE(i2c, hps_i2c_id); + +#ifdef CONFIG_ACPI +static const struct acpi_device_id hps_acpi_id[] = { + { HPS_ACPI_ID, 0 }, + { } +}; +MODULE_DEVICE_TABLE(acpi, hps_acpi_id); +#endif /* CONFIG_ACPI */ + +static struct i2c_driver hps_i2c_driver = { + .probe_new = hps_i2c_probe, + .remove = hps_i2c_remove, + .id_table = hps_i2c_id, + .driver = { + .name = "cros-hps", + .pm = &hps_pm_ops, + .acpi_match_table = ACPI_PTR(hps_acpi_id), + }, +}; +module_i2c_driver(hps_i2c_driver); + +MODULE_ALIAS("acpi:" HPS_ACPI_ID); +MODULE_AUTHOR("Sami Kyöstilä "); +MODULE_DESCRIPTION("Driver for ChromeOS HPS"); +MODULE_LICENSE("GPL"); -- cgit v1.2.3-58-ga151 From d8cb88f1541fdc3602dbc87ede78ec704c11546f Mon Sep 17 00:00:00 2001 From: Dan Callaghan Date: Wed, 19 Oct 2022 10:52:37 +1100 Subject: platform/chrome: cros_hps_i2c: make remove callback return void Commit ed5c2f5fd10d ("i2c: Make remove callback return void") changed the return type of the 'remove' callback to void, but this driver was originally written before that change landed. Update the remove callback to match. Fixes: 5f9952548d91 ("platform/chrome: add a driver for HPS") Reported-by: kernel test robot Signed-off-by: Dan Callaghan Signed-off-by: Tzung-Bi Shih Link: https://lore.kernel.org/r/20221018235237.2274969-1-dcallagh@chromium.org --- drivers/platform/chrome/cros_hps_i2c.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'drivers') diff --git a/drivers/platform/chrome/cros_hps_i2c.c b/drivers/platform/chrome/cros_hps_i2c.c index 92da59d94745..62ccb1acb5de 100644 --- a/drivers/platform/chrome/cros_hps_i2c.c +++ b/drivers/platform/chrome/cros_hps_i2c.c @@ -95,7 +95,7 @@ static int hps_i2c_probe(struct i2c_client *client) return 0; } -static int hps_i2c_remove(struct i2c_client *client) +static void hps_i2c_remove(struct i2c_client *client) { struct hps_drvdata *hps = i2c_get_clientdata(client); @@ -107,8 +107,6 @@ static int hps_i2c_remove(struct i2c_client *client) * (i.e. powered on). */ hps_set_power(hps, true); - - return 0; } static int hps_suspend(struct device *dev) -- cgit v1.2.3-58-ga151 From 9888feb9c68b799e758a654aae0a032871e493c2 Mon Sep 17 00:00:00 2001 From: Tzung-Bi Shih Date: Mon, 31 Oct 2022 13:06:57 +0800 Subject: platform/chrome: cros_ec_lpc_mec: remove cros_ec_lpc_mec_destroy() It's pointless (and invalid) to destroy a statically allocated mutex in cros_ec_lpc_mec_destroy(). Let's remove it. Signed-off-by: Tzung-Bi Shih Reviewed-by: Guenter Roeck Reviewed-by: Brian Norris Link: https://lore.kernel.org/r/20221031050657.3899359-1-tzungbi@kernel.org --- drivers/platform/chrome/cros_ec_lpc.c | 3 --- drivers/platform/chrome/cros_ec_lpc_mec.c | 6 ------ drivers/platform/chrome/cros_ec_lpc_mec.h | 7 ------- drivers/platform/chrome/wilco_ec/core.c | 5 ----- 4 files changed, 21 deletions(-) (limited to 'drivers') diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c index 7677ab3c0ead..6ef5e5d40ba4 100644 --- a/drivers/platform/chrome/cros_ec_lpc.c +++ b/drivers/platform/chrome/cros_ec_lpc.c @@ -593,7 +593,6 @@ static int __init cros_ec_lpc_init(void) ret = platform_driver_register(&cros_ec_lpc_driver); if (ret) { pr_err(DRV_NAME ": can't register driver: %d\n", ret); - cros_ec_lpc_mec_destroy(); return ret; } @@ -603,7 +602,6 @@ static int __init cros_ec_lpc_init(void) if (ret) { pr_err(DRV_NAME ": can't register device: %d\n", ret); platform_driver_unregister(&cros_ec_lpc_driver); - cros_ec_lpc_mec_destroy(); } } @@ -615,7 +613,6 @@ static void __exit cros_ec_lpc_exit(void) if (!cros_ec_lpc_acpi_device_found) platform_device_unregister(&cros_ec_lpc_device); platform_driver_unregister(&cros_ec_lpc_driver); - cros_ec_lpc_mec_destroy(); } module_init(cros_ec_lpc_init); diff --git a/drivers/platform/chrome/cros_ec_lpc_mec.c b/drivers/platform/chrome/cros_ec_lpc_mec.c index bbc2884f5e2f..0d9c79b270ce 100644 --- a/drivers/platform/chrome/cros_ec_lpc_mec.c +++ b/drivers/platform/chrome/cros_ec_lpc_mec.c @@ -146,9 +146,3 @@ void cros_ec_lpc_mec_init(unsigned int base, unsigned int end) mec_emi_end = end; } EXPORT_SYMBOL(cros_ec_lpc_mec_init); - -void cros_ec_lpc_mec_destroy(void) -{ - mutex_destroy(&io_mutex); -} -EXPORT_SYMBOL(cros_ec_lpc_mec_destroy); diff --git a/drivers/platform/chrome/cros_ec_lpc_mec.h b/drivers/platform/chrome/cros_ec_lpc_mec.h index aa1018f6b0f2..9d0521b23e8a 100644 --- a/drivers/platform/chrome/cros_ec_lpc_mec.h +++ b/drivers/platform/chrome/cros_ec_lpc_mec.h @@ -45,13 +45,6 @@ enum cros_ec_lpc_mec_io_type { */ void cros_ec_lpc_mec_init(unsigned int base, unsigned int end); -/* - * cros_ec_lpc_mec_destroy - * - * Cleanup MEC I/O. - */ -void cros_ec_lpc_mec_destroy(void); - /** * cros_ec_lpc_mec_in_range() - Determine if addresses are in MEC EMI range. * diff --git a/drivers/platform/chrome/wilco_ec/core.c b/drivers/platform/chrome/wilco_ec/core.c index 5b42992bff38..d6a994bdc182 100644 --- a/drivers/platform/chrome/wilco_ec/core.c +++ b/drivers/platform/chrome/wilco_ec/core.c @@ -129,7 +129,6 @@ unregister_rtc: unregister_debugfs: if (ec->debugfs_pdev) platform_device_unregister(ec->debugfs_pdev); - cros_ec_lpc_mec_destroy(); return ret; } @@ -143,10 +142,6 @@ static int wilco_ec_remove(struct platform_device *pdev) platform_device_unregister(ec->rtc_pdev); if (ec->debugfs_pdev) platform_device_unregister(ec->debugfs_pdev); - - /* Teardown cros_ec interface */ - cros_ec_lpc_mec_destroy(); - return 0; } -- cgit v1.2.3-58-ga151 From 58f23a6795a6c165b8c04041bacb999119f9dbc9 Mon Sep 17 00:00:00 2001 From: Christophe JAILLET Date: Tue, 1 Nov 2022 22:14:01 +0100 Subject: platform/chrome: Use kstrtobool() instead of strtobool() strtobool() is the same as kstrtobool(). However, the latter is more used within the kernel. In order to remove strtobool() and slightly simplify kstrtox.h, switch to the other function name. While at it, include the corresponding header file () Signed-off-by: Christophe JAILLET Signed-off-by: Tzung-Bi Shih Link: https://lore.kernel.org/r/8d66b4688c05a44b592a4d20e2660e9067163276.1667336095.git.christophe.jaillet@wanadoo.fr --- drivers/platform/chrome/cros_ec_lightbar.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/platform/chrome/cros_ec_lightbar.c b/drivers/platform/chrome/cros_ec_lightbar.c index 469dfc7a4a03..58beb2a047b2 100644 --- a/drivers/platform/chrome/cros_ec_lightbar.c +++ b/drivers/platform/chrome/cros_ec_lightbar.c @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -493,7 +494,7 @@ static ssize_t userspace_control_store(struct device *dev, bool enable; int ret; - ret = strtobool(buf, &enable); + ret = kstrtobool(buf, &enable); if (ret < 0) return ret; -- cgit v1.2.3-58-ga151 From fdf84f9ae30b40e3707359bcd467173b9d43454c Mon Sep 17 00:00:00 2001 From: Brian Norris Date: Tue, 1 Nov 2022 15:22:06 -0700 Subject: platform/chrome: cros_ec_lpc: Move mec_init to device probe Disregarding the weird global state hiding in this cros_ec_lpc_mec_*() stuff, it belongs in device probe. We shouldn't assume we can access hardware resources when the device isn't attached to the driver. Signed-off-by: Brian Norris Signed-off-by: Tzung-Bi Shih Link: https://lore.kernel.org/r/20221101152132.v2.1.I0728421299079b104710c202d5d7095b2674fd8c@changeid --- drivers/platform/chrome/cros_ec_lpc.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'drivers') diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c index 6ef5e5d40ba4..48302183d62e 100644 --- a/drivers/platform/chrome/cros_ec_lpc.c +++ b/drivers/platform/chrome/cros_ec_lpc.c @@ -354,6 +354,9 @@ static int cros_ec_lpc_probe(struct platform_device *pdev) return -EBUSY; } + cros_ec_lpc_mec_init(EC_HOST_CMD_REGION0, + EC_LPC_ADDR_MEMMAP + EC_MEMMAP_SIZE); + /* * Read the mapped ID twice, the first one is assuming the * EC is a Microchip Embedded Controller (MEC) variant, if the @@ -586,9 +589,6 @@ static int __init cros_ec_lpc_init(void) return -ENODEV; } - cros_ec_lpc_mec_init(EC_HOST_CMD_REGION0, - EC_LPC_ADDR_MEMMAP + EC_MEMMAP_SIZE); - /* Register the driver */ ret = platform_driver_register(&cros_ec_lpc_driver); if (ret) { -- cgit v1.2.3-58-ga151 From bd88b965ae8c5e46661812b620dad67bee892f78 Mon Sep 17 00:00:00 2001 From: Brian Norris Date: Tue, 1 Nov 2022 15:22:07 -0700 Subject: platform/chrome: cros_ec_lpc: Mark PROBE_PREFER_ASYNCHRONOUS This takes on the order of 60ms to probe on some systems, so let it probe asynchronously. It shouldn't have any dependencies that aren't handled cleanly. Signed-off-by: Brian Norris Signed-off-by: Tzung-Bi Shih Link: https://lore.kernel.org/r/20221101152132.v2.2.Ib1036816e77aba71ebc16b71f7615c55d054689c@changeid --- drivers/platform/chrome/cros_ec_lpc.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers') diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c index 48302183d62e..2e4dba724ada 100644 --- a/drivers/platform/chrome/cros_ec_lpc.c +++ b/drivers/platform/chrome/cros_ec_lpc.c @@ -557,6 +557,7 @@ static struct platform_driver cros_ec_lpc_driver = { .name = DRV_NAME, .acpi_match_table = cros_ec_lpc_acpi_device_ids, .pm = &cros_ec_lpc_pm_ops, + .probe_type = PROBE_PREFER_ASYNCHRONOUS, }, .probe = cros_ec_lpc_probe, .remove = cros_ec_lpc_remove, -- cgit v1.2.3-58-ga151 From 692a68ad7f3c568359b9f18d966628856fd34ff3 Mon Sep 17 00:00:00 2001 From: Brian Norris Date: Tue, 1 Nov 2022 15:22:08 -0700 Subject: platform/chrome: cros_ec_debugfs: Set PROBE_PREFER_ASYNCHRONOUS This driver takes on the order of 40ms to start on some systems. It shouldn't have many cross-device dependencies to race with, nor racy access to shared state with other drivers, so this should be a relatively low risk change. This driver was pinpointed as part of a survey of top slowest initcalls (i.e., are built in, and probing synchronously) on a lab of ChromeOS systems. Signed-off-by: Brian Norris Signed-off-by: Tzung-Bi Shih Link: https://lore.kernel.org/r/20221101152132.v2.3.Ic9a4f378f73319da323cd55940012fa6b1de24f4@changeid --- drivers/platform/chrome/cros_ec_debugfs.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers') diff --git a/drivers/platform/chrome/cros_ec_debugfs.c b/drivers/platform/chrome/cros_ec_debugfs.c index 4e63adf083ea..21d973fc6be2 100644 --- a/drivers/platform/chrome/cros_ec_debugfs.c +++ b/drivers/platform/chrome/cros_ec_debugfs.c @@ -521,6 +521,7 @@ static struct platform_driver cros_ec_debugfs_driver = { .driver = { .name = DRV_NAME, .pm = &cros_ec_debugfs_pm_ops, + .probe_type = PROBE_PREFER_ASYNCHRONOUS, }, .probe = cros_ec_debugfs_probe, .remove = cros_ec_debugfs_remove, -- cgit v1.2.3-58-ga151 From 873ab3e886b52ba3d6ade3c5f3e6a0cff0c8cb12 Mon Sep 17 00:00:00 2001 From: Brian Norris Date: Tue, 1 Nov 2022 15:22:09 -0700 Subject: platform/chrome: cros_ec_lightbar: Set PROBE_PREFER_ASYNCHRONOUS This driver takes on the order of 15ms to start on some systems. Even on systems where there is no lightbar support, it can take a few milliseconds just to probe the EC for support. It shouldn't have many cross-device dependencies to race with, nor racy access to shared state with other drivers, so this should be a relatively low risk change. This driver was pinpointed as part of a survey of top slowest initcalls (i.e., are built in, and probing synchronously) on a lab of ChromeOS systems. Signed-off-by: Brian Norris Signed-off-by: Tzung-Bi Shih Link: https://lore.kernel.org/r/20221101152132.v2.4.I565598102e0bfb03bdf8c090d3bfdf954d026bc5@changeid --- drivers/platform/chrome/cros_ec_lightbar.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers') diff --git a/drivers/platform/chrome/cros_ec_lightbar.c b/drivers/platform/chrome/cros_ec_lightbar.c index 58beb2a047b2..1674105decfb 100644 --- a/drivers/platform/chrome/cros_ec_lightbar.c +++ b/drivers/platform/chrome/cros_ec_lightbar.c @@ -602,6 +602,7 @@ static struct platform_driver cros_ec_lightbar_driver = { .driver = { .name = DRV_NAME, .pm = &cros_ec_lightbar_pm_ops, + .probe_type = PROBE_PREFER_ASYNCHRONOUS, }, .probe = cros_ec_lightbar_probe, .remove = cros_ec_lightbar_remove, -- cgit v1.2.3-58-ga151 From 015e4b05c377dc5e066e88737bff9990d5ac358d Mon Sep 17 00:00:00 2001 From: Brian Norris Date: Tue, 1 Nov 2022 15:22:10 -0700 Subject: platform/chrome: cros_ec_spi: Set PROBE_PREFER_ASYNCHRONOUS This driver often takes on the order of 10ms to start, but in some cases as much as 600ms [1]. It shouldn't have many cross-device dependencies to race with, nor racy access to shared state with other drivers, so this should be a relatively low risk change. This driver was pinpointed as part of a survey of top slowest initcalls (i.e., are built in, and probing synchronously) on a lab of ChromeOS systems. [1] 600ms was especially surprising to me, so I checked a little deeper. This driver is used to interface with Embedded Controllers besides just the traditional laptop power-state controller -- it also interfaces with some fingerprint readers, which may start up in parallel with the kernel, or which may not even be present on some SKUs, despite having a node for it. Thus, our time is wasted just timing out talking to it. At least we can do that without blocking everyone else. Signed-off-by: Brian Norris Signed-off-by: Tzung-Bi Shih Link: https://lore.kernel.org/r/20221101152132.v2.5.Ia458a69e1d592bfa4f04cde7018bbc7486f91a23@changeid --- drivers/platform/chrome/cros_ec_spi.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers') diff --git a/drivers/platform/chrome/cros_ec_spi.c b/drivers/platform/chrome/cros_ec_spi.c index 7360b3ff6e4f..21143dba8970 100644 --- a/drivers/platform/chrome/cros_ec_spi.c +++ b/drivers/platform/chrome/cros_ec_spi.c @@ -834,6 +834,7 @@ static struct spi_driver cros_ec_driver_spi = { .name = "cros-ec-spi", .of_match_table = cros_ec_spi_of_match, .pm = &cros_ec_spi_pm_ops, + .probe_type = PROBE_PREFER_ASYNCHRONOUS, }, .probe = cros_ec_spi_probe, .remove = cros_ec_spi_remove, -- cgit v1.2.3-58-ga151 From ca821c1f4ec11d6181da58118d158a015160106d Mon Sep 17 00:00:00 2001 From: Brian Norris Date: Fri, 11 Nov 2022 15:13:01 -0800 Subject: platform/chrome: cros_ec_lpc: Force synchronous probe This reverts commit bd88b965ae8c ("platform/chrome: cros_ec_lpc: Mark PROBE_PREFER_ASYNCHRONOUS"), and then some. It has been reported that there are issues with 'cros-ec-keyb' devices that are children of this. As noted in the initial patch for its ACPI support (commit ba0f32141bc5 ("Input: cros_ec_keyb - handle x86 detachable/convertible Chromebooks")), it's possible to probe an ACPI child device before its parent is probed -- hence the need for EPROBE_DEFER. Unfortunately, poking your parent's dev_get_drvdata() isn't safe with asynchronous probe, as there's no locking, and the ordering is all wrong anyway (drvdata is set before the device is *really* ready). Because this parent/child relationship has known issues, let's go the other direction and force synchronous probe, until we resolve the issues. Possible solutions involve adding device links, so we ensure the child doesn't probe before the parent is done; or perhaps some other larger refactoring (auxiliary bus?). But that might take a little more effort and review, as there are many other potential sub-devices of cros_ec_lpc that could need patching. Note that we don't have the same problem for non-ACPI cros-ec hosts, like cros-ec-spi (commit 015e4b05c377 ("platform/chrome: cros_ec_spi: Set PROBE_PREFER_ASYNCHRONOUS")), because its sub-devices aren't created until cros_ec_register(), or they don't exist at all (e.g., FPMCU uses). Fixes: bd88b965ae8c ("platform/chrome: cros_ec_lpc: Mark PROBE_PREFER_ASYNCHRONOUS") Signed-off-by: Brian Norris Reviewed-by: Dmitry Torokhov Reviewed-by: Guenter Roeck Signed-off-by: Tzung-Bi Shih Link: https://lore.kernel.org/r/20221111231302.3458191-1-briannorris@chromium.org --- drivers/platform/chrome/cros_ec_lpc.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c index 2e4dba724ada..7fc8f82280ac 100644 --- a/drivers/platform/chrome/cros_ec_lpc.c +++ b/drivers/platform/chrome/cros_ec_lpc.c @@ -557,7 +557,12 @@ static struct platform_driver cros_ec_lpc_driver = { .name = DRV_NAME, .acpi_match_table = cros_ec_lpc_acpi_device_ids, .pm = &cros_ec_lpc_pm_ops, - .probe_type = PROBE_PREFER_ASYNCHRONOUS, + /* + * ACPI child devices may probe before us, and they racily + * check our drvdata pointer. Force synchronous probe until + * those races are resolved. + */ + .probe_type = PROBE_FORCE_SYNCHRONOUS, }, .probe = cros_ec_lpc_probe, .remove = cros_ec_lpc_remove, -- cgit v1.2.3-58-ga151 From f9e510dc92df270756b6a42d98c738525d01e8c3 Mon Sep 17 00:00:00 2001 From: Uwe Kleine-König Date: Fri, 18 Nov 2022 23:44:06 +0100 Subject: platform/chrome: cros_ec: Convert to i2c's .probe_new() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The probe function doesn't make use of the i2c_device_id * parameter so it can be trivially converted. Signed-off-by: Uwe Kleine-König Reviewed-by: Guenter Roeck Signed-off-by: Tzung-Bi Shih Link: https://lore.kernel.org/r/20221118224540.619276-513-uwe@kleine-koenig.org --- drivers/platform/chrome/cros_ec_i2c.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'drivers') diff --git a/drivers/platform/chrome/cros_ec_i2c.c b/drivers/platform/chrome/cros_ec_i2c.c index b6823c654c3f..dbe698f33128 100644 --- a/drivers/platform/chrome/cros_ec_i2c.c +++ b/drivers/platform/chrome/cros_ec_i2c.c @@ -286,8 +286,7 @@ done: return ret; } -static int cros_ec_i2c_probe(struct i2c_client *client, - const struct i2c_device_id *dev_id) +static int cros_ec_i2c_probe(struct i2c_client *client) { struct device *dev = &client->dev; struct cros_ec_device *ec_dev = NULL; @@ -373,7 +372,7 @@ static struct i2c_driver cros_ec_driver = { .of_match_table = of_match_ptr(cros_ec_i2c_of_match), .pm = &cros_ec_i2c_pm_ops, }, - .probe = cros_ec_i2c_probe, + .probe_new = cros_ec_i2c_probe, .remove = cros_ec_i2c_remove, .id_table = cros_ec_i2c_id, }; -- cgit v1.2.3-58-ga151 From 5a2d96623670155d94aca72c320c0ac27bdc6bd2 Mon Sep 17 00:00:00 2001 From: Yuan Can Date: Thu, 17 Nov 2022 08:08:23 +0000 Subject: platform/chrome: cros_usbpd_notify: Fix error handling in cros_usbpd_notify_init() The following WARNING message was given when rmmod cros_usbpd_notify: Unexpected driver unregister! WARNING: CPU: 0 PID: 253 at drivers/base/driver.c:270 driver_unregister+0x8a/0xb0 Modules linked in: cros_usbpd_notify(-) CPU: 0 PID: 253 Comm: rmmod Not tainted 6.1.0-rc3 #24 ... Call Trace: cros_usbpd_notify_exit+0x11/0x1e [cros_usbpd_notify] __x64_sys_delete_module+0x3c7/0x570 ? __ia32_sys_delete_module+0x570/0x570 ? lock_is_held_type+0xe3/0x140 ? syscall_enter_from_user_mode+0x17/0x50 ? rcu_read_lock_sched_held+0xa0/0xd0 ? syscall_enter_from_user_mode+0x1c/0x50 do_syscall_64+0x37/0x90 entry_SYSCALL_64_after_hwframe+0x63/0xcd RIP: 0033:0x7f333fe9b1b7 The reason is that the cros_usbpd_notify_init() does not check the return value of platform_driver_register(), and the cros_usbpd_notify can install successfully even if platform_driver_register() failed. Fix by checking the return value of platform_driver_register() and unregister cros_usbpd_notify_plat_driver when it failed. Fixes: ec2daf6e33f9 ("platform: chrome: Add cros-usbpd-notify driver") Signed-off-by: Yuan Can Reviewed-by: Brian Norris Link: https://lore.kernel.org/r/20221117080823.77549-1-yuancan@huawei.com Signed-off-by: Prashant Malani --- drivers/platform/chrome/cros_usbpd_notify.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/platform/chrome/cros_usbpd_notify.c b/drivers/platform/chrome/cros_usbpd_notify.c index 4b5a81c9dc6d..10670b6588e3 100644 --- a/drivers/platform/chrome/cros_usbpd_notify.c +++ b/drivers/platform/chrome/cros_usbpd_notify.c @@ -239,7 +239,11 @@ static int __init cros_usbpd_notify_init(void) return ret; #ifdef CONFIG_ACPI - platform_driver_register(&cros_usbpd_notify_acpi_driver); + ret = platform_driver_register(&cros_usbpd_notify_acpi_driver); + if (ret) { + platform_driver_unregister(&cros_usbpd_notify_plat_driver); + return ret; + } #endif return 0; } -- cgit v1.2.3-58-ga151 From 9a8aadcf0b459c1257b9477fd6402e1d5952ae07 Mon Sep 17 00:00:00 2001 From: Victor Ding Date: Wed, 7 Dec 2022 09:39:40 +0000 Subject: platform/chrome: cros_ec_typec: zero out stale pointers `cros_typec_get_switch_handles` allocates four pointers when obtaining type-c switch handles. These pointers are all freed if failing to obtain any of them; therefore, pointers in `port` become stale. The stale pointers eventually cause use-after-free or double free in later code paths. Zeroing out all pointer fields after freeing to eliminate these stale pointers. Fixes: f28adb41dab4 ("platform/chrome: cros_ec_typec: Register Type C switches") Fixes: 1a8912caba02 ("platform/chrome: cros_ec_typec: Get retimer handle") Signed-off-by: Victor Ding Acked-by: Prashant Malani Signed-off-by: Tzung-Bi Shih Link: https://lore.kernel.org/r/20221207093924.v2.1.I1864b6a7ee98824118b93677868d22d3750f439b@changeid --- drivers/platform/chrome/cros_ec_typec.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'drivers') diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c index 2a7ff14dc37e..59de4ce01fab 100644 --- a/drivers/platform/chrome/cros_ec_typec.c +++ b/drivers/platform/chrome/cros_ec_typec.c @@ -173,10 +173,13 @@ static int cros_typec_get_switch_handles(struct cros_typec_port *port, role_sw_err: typec_switch_put(port->ori_sw); + port->ori_sw = NULL; ori_sw_err: typec_retimer_put(port->retimer); + port->retimer = NULL; retimer_sw_err: typec_mux_put(port->mux); + port->mux = NULL; mux_err: return -ENODEV; } -- cgit v1.2.3-58-ga151