| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <a2310cf8-5e00-4233-8c56-f04d3f692b13@linaro.org>
Date: Thu, 19 Dec 2024 20:43:16 +0000
From: Bryan O'Donoghue <bryan.odonoghue@...aro.org>
To: Maya Matuszczyk <maccraft123mc@...il.com>,
Hans de Goede <hdegoede@...hat.com>,
Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
Miguel Ojeda <ojeda@...nel.org>, Alex Gaynor <alex.gaynor@...il.com>,
Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>,
Björn Roy Baron <bjorn3_gh@...tonmail.com>,
Benno Lossin <benno.lossin@...ton.me>,
Andreas Hindborg <a.hindborg@...nel.org>, Alice Ryhl <aliceryhl@...gle.com>,
Trevor Gross <tmgross@...ch.edu>
Cc: linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org,
platform-driver-x86@...r.kernel.org, rust-for-linux@...r.kernel.org
Subject: Re: [PATCH v2 2/3] platform: arm64: Add driver for EC found in most
X1E laptops
On 19/12/2024 20:08, Maya Matuszczyk wrote:
> Currently it features only reporting that the AP is going to suspend,
> which results in keyboard backlight turning off and the power LED
> slowly blinking on the Lenovo Yoga Slim 7x.
>
> Honor Magicbook Art 14 and Lenovo Yoga Slim 7x are known to have
> firmware with extensions which would need appropriate handling.
> For reverse engineering the firmware on them I have written a Rust
> utility:
>
> https://github.com/Maccraft123/it8987-qcom-tool.git
>
> Signed-off-by: Maya Matuszczyk <maccraft123mc@...il.com>
> ---
> MAINTAINERS | 6 +
> drivers/platform/arm64/Kconfig | 8 ++
> drivers/platform/arm64/Makefile | 1 +
> drivers/platform/arm64/qcom-x1e-it8987.c | 158 +++++++++++++++++++++++
> 4 files changed, 173 insertions(+)
> create mode 100644 drivers/platform/arm64/qcom-x1e-it8987.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b878ddc99f94..08d170e2e1e3 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -12890,6 +12890,12 @@ S: Maintained
> W: http://legousb.sourceforge.net/
> F: drivers/usb/misc/legousbtower.c
>
> +QCOM IT8987 EC DRIVER
> +M: Maya Matuszczyk <maccraft123mc@...il.com>
> +S: Maintained
> +F: Documentation/devicetree/bindings/platform/qcom,x1e-it8987-ec.yaml
> +F: drivers/platform/arm64/qcom-x1e-it8987.c
> +
> LETSKETCH HID TABLET DRIVER
> M: Hans de Goede <hdegoede@...hat.com>
> L: linux-input@...r.kernel.org
> diff --git a/drivers/platform/arm64/Kconfig b/drivers/platform/arm64/Kconfig
> index f88395ea3376..ebb7b4f70ca0 100644
> --- a/drivers/platform/arm64/Kconfig
> +++ b/drivers/platform/arm64/Kconfig
> @@ -49,4 +49,12 @@ config EC_LENOVO_YOGA_C630
>
> Say M or Y here to include this support.
>
> +config EC_QCOM_X1E_IT8987
> + tristate "Embedded Controller driver for most X1E80100 laptops"
> + depends on ARCH_QCOM || COMPILE_TEST
> + depends on I2C
> + help
> + This driver currently supports reporting device suspend to the EC so it
> + can take appropriate actions.
> +
> endif # ARM64_PLATFORM_DEVICES
> diff --git a/drivers/platform/arm64/Makefile b/drivers/platform/arm64/Makefile
> index b2ae9114fdd8..b9aa195bc1e6 100644
> --- a/drivers/platform/arm64/Makefile
> +++ b/drivers/platform/arm64/Makefile
> @@ -7,3 +7,4 @@
>
> obj-$(CONFIG_EC_ACER_ASPIRE1) += acer-aspire1-ec.o
> obj-$(CONFIG_EC_LENOVO_YOGA_C630) += lenovo-yoga-c630.o
> +obj-$(CONFIG_EC_QCOM_X1E_IT8987) += qcom-x1e-it8987.o
> diff --git a/drivers/platform/arm64/qcom-x1e-it8987.c b/drivers/platform/arm64/qcom-x1e-it8987.c
> new file mode 100644
> index 000000000000..d27067d6326a
> --- /dev/null
> +++ b/drivers/platform/arm64/qcom-x1e-it8987.c
> @@ -0,0 +1,158 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2024 Maya Matuszczyk <maccraft123mc@...il.com>
> + */
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/input.h>
> +#include <linux/input/sparse-keymap.h>
Your includes should be alphabetised.
> +
> +#define EC_IRQ_REASON_REG 0x05
> +#define EC_SUSPEND_RESUME_REG 0x23
> +#define EC_IRQ_ENABLE_REG 0x35
> +
> +#define EC_NOTIFY_SUSPEND_ENTER 0x01
> +#define EC_NOTIFY_SUSPEND_EXIT 0x00
> +#define EC_NOTIFY_SCREEN_OFF 0x03
> +#define EC_NOTIFY_SCREEN_ON 0x04
> +
> +#define EC_IRQ_MICMUTE_BUTTON 0x04
> +#define EC_IRQ_FAN1_STATUS_CHANGE 0x30
> +#define EC_IRQ_FAN2_STATUS_CHANGE 0x31
> +#define EC_IRQ_FAN1_SPEED_CHANGE 0x32
> +#define EC_IRQ_FAN2_SPEED_CHANGE 0x33
> +#define EC_IRQ_COMPLETED_LUT_UPDATE 0x34
> +#define EC_IRQ_COMPLETED_FAN_PROFILE_SWITCH 0x35
> +#define EC_IRQ_THERMISTOR_1_TEMP_THRESHOLD_CROSS 0x36
> +#define EC_IRQ_THERMISTOR_2_TEMP_THRESHOLD_CROSS 0x37
> +#define EC_IRQ_THERMISTOR_3_TEMP_THRESHOLD_CROSS 0x38
> +#define EC_IRQ_THERMISTOR_4_TEMP_THRESHOLD_CROSS 0x39
> +#define EC_IRQ_THERMISTOR_5_TEMP_THRESHOLD_CROSS 0x3a
> +#define EC_IRQ_THERMISTOR_6_TEMP_THRESHOLD_CROSS 0x3b
> +#define EC_IRQ_THERMISTOR_7_TEMP_THRESHOLD_CROSS 0x3c
> +#define EC_IRQ_RECOVERED_FROM_RESET 0x3d
> +
> +struct qcom_x1e_it8987_ec {
> + struct i2c_client *client;
> + struct input_dev *idev;
> + struct mutex lock;
> +};
> +
> +static irqreturn_t qcom_x1e_it8987_ec_irq(int irq, void *data)
> +{
> + struct qcom_x1e_it8987_ec *ec = data;
> + struct device *dev = &ec->client->dev;
> + int val;
> +
> + guard(mutex)(&ec->lock);
What's the thing that can execute at the same time as this procedure -
there doesn't appear to be any concurrent candidate in this patch.
> +
> + val = i2c_smbus_read_byte_data(ec->client, EC_IRQ_REASON_REG);
> + if (val < 0) {
> + dev_err(dev, "Failed to get EC IRQ reason: %d\n", val);
> + return IRQ_HANDLED;
> + }
> +
> + dev_info(dev, "Unhandled EC IRQ reason: %d\n", val);
Should an unhandled IRQ be an info or an err ?
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int qcom_x1e_it8987_ec_probe(struct i2c_client *client)
> +{
> + struct device *dev = &client->dev;
> + struct qcom_x1e_it8987_ec *ec;
> + int ret;
> +
> + ec = devm_kzalloc(dev, sizeof(*ec), GFP_KERNEL);
> + if (!ec)
> + return -ENOMEM;
> +
> + mutex_init(&ec->lock);
> + ec->client = client;
> +
> + ret = devm_request_threaded_irq(dev, client->irq,
> + NULL, qcom_x1e_it8987_ec_irq,
> + IRQF_ONESHOT, "qcom_x1e_it8987_ec", ec);
> + if (ret < 0)
> + return dev_err_probe(dev, ret, "Unable to request irq\n");
> +
> + ret = i2c_smbus_write_byte_data(client, EC_IRQ_ENABLE_REG, 0x01);
> + if (ret < 0)
> + return dev_err_probe(dev, ret, "Failed to enable interrupts\n");
> +
> + return 0;
> +}
> +
> +static void qcom_x1e_it8987_ec_remove(struct i2c_client *client)
> +{
> + struct device *dev = &client->dev;
> + int ret;
> +
> + ret = i2c_smbus_write_byte_data(client, EC_IRQ_ENABLE_REG, 0x00);
> + if (ret < 0)
> + dev_err(dev, "Failed to disable interrupts: %d\n", ret);
> +}
> +
> +static int qcom_x1e_it8987_ec_suspend(struct device *dev)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + int ret;
> +
> + ret = i2c_smbus_write_byte_data(client, EC_SUSPEND_RESUME_REG, EC_NOTIFY_SCREEN_OFF);
> + if (ret)
> + return ret;
> +
> + ret = i2c_smbus_write_byte_data(client, EC_SUSPEND_RESUME_REG, EC_NOTIFY_SUSPEND_ENTER);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> +static int qcom_x1e_it8987_ec_resume(struct device *dev)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + int ret;
> +
> + ret = i2c_smbus_write_byte_data(client, EC_SUSPEND_RESUME_REG, EC_NOTIFY_SUSPEND_EXIT);
> + if (ret)
> + return ret;
> +
> + ret = i2c_smbus_write_byte_data(client, EC_SUSPEND_RESUME_REG, EC_NOTIFY_SCREEN_ON);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> +static const struct of_device_id qcom_x1e_it8987_ec_of_match[] = {
> + { .compatible = "lenovo,yoga-slim7x-ec" },
> + { .compatible = "qcom,x1e-it9897-ec" },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, qcom_x1e_it8987_ec_of_match);
> +
> +static const struct i2c_device_id qcom_x1e_it8987_ec_i2c_id_table[] = {
> + { "qcom-x1e-it8987-ec", },
> + {}
> +};
> +MODULE_DEVICE_TABLE(i2c, qcom_x1e_it8987_ec_i2c_id_table);
> +
> +static DEFINE_SIMPLE_DEV_PM_OPS(qcom_x1e_it8987_ec_pm_ops,
> + qcom_x1e_it8987_ec_suspend,
> + qcom_x1e_it8987_ec_resume);
> +
> +static struct i2c_driver qcom_x1e_it8987_ec_i2c_driver = {
> + .driver = {
> + .name = "yoga-slim7x-ec",
> + .of_match_table = qcom_x1e_it8987_ec_of_match,
> + .pm = &qcom_x1e_it8987_ec_pm_ops
> + },
> + .probe = qcom_x1e_it8987_ec_probe,
> + .remove = qcom_x1e_it8987_ec_remove,
> + .id_table = qcom_x1e_it8987_ec_i2c_id_table,
> +};
> +module_i2c_driver(qcom_x1e_it8987_ec_i2c_driver);
> +
> +MODULE_DESCRIPTION("Lenovo Yoga Slim 7x Embedded Controller");
> +MODULE_LICENSE("GPL");
Otherwise looks pretty good to me, nice hacking :)
---
bod
Powered by blists - more mailing lists