* [PATCH 2/3] platform: arm64: Add driver for Lenovo Yoga Slim 7x's EC [not found] <20240927185345.3680-1-maccraft123mc@gmail.com> @ 2024-09-27 18:53 ` Maya Matuszczyk 2024-09-28 10:35 ` György Kurucz ` (2 more replies) 0 siblings, 3 replies; 6+ messages in thread From: Maya Matuszczyk @ 2024-09-27 18:53 UTC (permalink / raw) To: Hans de Goede, Ilpo Järvinen, Bryan O'Donoghue, Maya Matuszczyk Cc: linux-kernel, platform-driver-x86 This patch adds a basic driver for the EC in Qualcomm Snapdragon X1 Elite-based Lenovo Yoga Slim 7x. For now it supports only reporting that the AP is going to suspend and the microphone mute button, however the EC seems to also support reading fan information, other key combinations and thermal data. Signed-off-by: Maya Matuszczyk <maccraft123mc@gmail.com> --- MAINTAINERS | 1 + drivers/platform/arm64/Kconfig | 12 ++ drivers/platform/arm64/Makefile | 1 + drivers/platform/arm64/lenovo-yoga-slim7x.c | 202 ++++++++++++++++++++ 4 files changed, 216 insertions(+) create mode 100644 drivers/platform/arm64/lenovo-yoga-slim7x.c diff --git a/MAINTAINERS b/MAINTAINERS index 0d4bfdde166d..f689cba80fa3 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -12906,6 +12906,7 @@ LENOVO YOGA SLIM 7X EC DRIVER M: Maya Matuszczyk <maccraft123mc@gmail.com> S: Maintained F: Documentation/devicetree/bindings/platform/lenovo,yoga-slim7x-ec.yaml +F: drivers/platform/arm64/lenovo-yoga-slim7x.c LETSKETCH HID TABLET DRIVER M: Hans de Goede <hdegoede@redhat.com> diff --git a/drivers/platform/arm64/Kconfig b/drivers/platform/arm64/Kconfig index f88395ea3376..9ceae50f8b4e 100644 --- a/drivers/platform/arm64/Kconfig +++ b/drivers/platform/arm64/Kconfig @@ -49,4 +49,16 @@ config EC_LENOVO_YOGA_C630 Say M or Y here to include this support. +config EC_LENOVO_YOGA_SLIM7X + tristate "Lenovo Yoga Slim 7x Embedded Controller driver" + depends on ARCH_QCOM || COMPILE_TEST + depends on I2C + help + Select this option to enable driver for the EC found in the Lenovo + Yoga Slim 7x. + + This driver currently supports reporting input event for microphone + mute button, and 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..70dfc1fb979d 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_LENOVO_YOGA_SLIM7X) += lenovo-yoga-slim7x.o diff --git a/drivers/platform/arm64/lenovo-yoga-slim7x.c b/drivers/platform/arm64/lenovo-yoga-slim7x.c new file mode 100644 index 000000000000..8f6d523395bc --- /dev/null +++ b/drivers/platform/arm64/lenovo-yoga-slim7x.c @@ -0,0 +1,202 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (c) 2024 Maya Matuszczyk <maya.matuszczyk@gmail.com> + */ +#include <linux/auxiliary_bus.h> +#include <linux/cleanup.h> +#include <linux/device.h> +#include <linux/err.h> +#include <linux/i2c.h> +#include <linux/irqreturn.h> +#include <linux/lockdep.h> +#include <linux/module.h> +#include <linux/mutex.h> +#include <linux/notifier.h> +#include <linux/slab.h> +#include <linux/input.h> +//#include <linux/platform_data/lenovo-yoga-slim7x.h> + +// These are the registers that i know about available from SMBUS +#define EC_IRQ_REASON_REG 0x05 +#define EC_SUSPEND_RESUME_REG 0x23 +#define EC_IRQ_ENABLE_REG 0x35 +#define EC_BACKLIGHT_STATUS_REG 0x83 +#define EC_MIC_MUTE_LED_REG 0x84 +#define EC_AC_STATUS_REG 0x90 + +// Valid values for EC_SUSPEND_RESUME_REG +#define EC_NOTIFY_SUSPEND_ENTER 0x01 +#define EC_NOTIFY_SUSPEND_EXIT 0x00 +#define EC_NOTIFY_SCREEN_OFF 0x03 +#define EC_NOTIFY_SCREEN_ON 0x04 + +// These are the values in EC_IRQ_REASON_REG that i could find in DSDT +#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 +#define EC_IRQ_LENOVO_SUPPORT_KEY 0x90 +#define EC_IRQ_FN_Q 0x91 +#define EC_IRQ_FN_M 0x92 +#define EC_IRQ_FN_SPACE 0x93 +#define EC_IRQ_FN_R 0x94 +#define EC_IRQ_FNLOCK_ON 0x95 +#define EC_IRQ_FNLOCK_OFF 0x96 +#define EC_IRQ_FN_N 0x97 +#define EC_IRQ_AI 0x9a +#define EC_IRQ_NPU 0x9b + +struct yoga_slim7x_ec { + struct i2c_client *client; + struct input_dev *idev; + struct mutex lock; +}; + +static irqreturn_t yoga_slim7x_ec_irq(int irq, void *data) +{ + struct yoga_slim7x_ec *ec = data; + struct device *dev = &ec->client->dev; + int val; + + guard(mutex)(&ec->lock); + + 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; + } + + switch (val) { + case EC_IRQ_MICMUTE_BUTTON: + input_report_key(ec->idev, KEY_MICMUTE, 1); + input_sync(ec->idev); + input_report_key(ec->idev, KEY_MICMUTE, 0); + input_sync(ec->idev); + break; + default: + dev_info(dev, "Unhandled EC IRQ reason: %d\n", val); + } + + return IRQ_HANDLED; +} + +static int yoga_slim7x_ec_probe(struct i2c_client *client) +{ + struct device *dev = &client->dev; + struct yoga_slim7x_ec *ec; + int ret; + + ec = devm_kzalloc(dev, sizeof(*ec), GFP_KERNEL); + if (!ec) + return -ENOMEM; + + mutex_init(&ec->lock); + ec->client = client; + + ec->idev = devm_input_allocate_device(dev); + if (!ec->idev) + return -ENOMEM; + ec->idev->name = "yoga-slim7x-ec"; + ec->idev->phys = "yoga-slim7x-ec/input0"; + input_set_capability(ec->idev, EV_KEY, KEY_MICMUTE); + + ret = input_register_device(ec->idev); + if (ret < 0) + return dev_err_probe(dev, ret, "Failed to register input device\n"); + + ret = devm_request_threaded_irq(dev, client->irq, + NULL, yoga_slim7x_ec_irq, + IRQF_ONESHOT, "yoga_slim7x_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 yoga_slim7x_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 yoga_slim7x_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 yoga_slim7x_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 yoga_slim7x_ec_of_match[] = { + { .compatible = "lenovo,yoga-slim7x-ec" }, + {} +}; +MODULE_DEVICE_TABLE(of, yoga_slim7x_ec_of_match); + +static const struct i2c_device_id yoga_slim7x_ec_i2c_id_table[] = { + { "yoga-slim7x-ec", }, + {} +}; +MODULE_DEVICE_TABLE(i2c, yoga_slim7x_ec_i2c_id_table); + +static DEFINE_SIMPLE_DEV_PM_OPS(yoga_slim7x_ec_pm_ops, + yoga_slim7x_ec_suspend, + yoga_slim7x_ec_resume); + +static struct i2c_driver yoga_slim7x_ec_i2c_driver = { + .driver = { + .name = "yoga-slim7x-ec", + .of_match_table = yoga_slim7x_ec_of_match, + .pm = &yoga_slim7x_ec_pm_ops + }, + .probe = yoga_slim7x_ec_probe, + .remove = yoga_slim7x_ec_remove, + .id_table = yoga_slim7x_ec_i2c_id_table, +}; +module_i2c_driver(yoga_slim7x_ec_i2c_driver); + +MODULE_DESCRIPTION("Lenovo Yoga Slim 7x Embedded Controller"); +MODULE_LICENSE("GPL"); -- 2.45.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/3] platform: arm64: Add driver for Lenovo Yoga Slim 7x's EC 2024-09-27 18:53 ` [PATCH 2/3] platform: arm64: Add driver for Lenovo Yoga Slim 7x's EC Maya Matuszczyk @ 2024-09-28 10:35 ` György Kurucz 2024-09-28 15:53 ` Hans de Goede 2024-10-01 7:12 ` kernel test robot 2 siblings, 0 replies; 6+ messages in thread From: György Kurucz @ 2024-09-28 10:35 UTC (permalink / raw) To: Maya Matuszczyk, Hans de Goede, Ilpo Järvinen, Bryan O'Donoghue Cc: linux-kernel, platform-driver-x86 Dear Maya, > For now it supports only reporting that the AP is going to suspend and > the microphone mute button, however the EC seems to also support reading > fan information, other key combinations and thermal data. I tested the following: - When I suspend the laptop, the keyboard backlight turns off, and the power button LED starts blinking. - When I press the Fn+F4 key combo, the KEY_MICMUTE event is correctly reported to userspace. > +// These are the registers that i know about available from SMBUS > +// These are the values in EC_IRQ_REASON_REG that i could find in DSDT Nitpick: capitalize "I" if you create a v2 Tested-by: György Kurucz <me@kuruczgy.com> Best regards, György ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/3] platform: arm64: Add driver for Lenovo Yoga Slim 7x's EC 2024-09-27 18:53 ` [PATCH 2/3] platform: arm64: Add driver for Lenovo Yoga Slim 7x's EC Maya Matuszczyk 2024-09-28 10:35 ` György Kurucz @ 2024-09-28 15:53 ` Hans de Goede 2024-09-28 18:04 ` Maya Matuszczyk 2024-10-01 7:12 ` kernel test robot 2 siblings, 1 reply; 6+ messages in thread From: Hans de Goede @ 2024-09-28 15:53 UTC (permalink / raw) To: Maya Matuszczyk, Ilpo Järvinen, Bryan O'Donoghue Cc: linux-kernel, platform-driver-x86 Hi Maya, Thank you for your patch. It is great to so people working on supporting more ARM64 based laptop ECs. Note not a full review just one remark from a quick scan of the driver. On 27-Sep-24 8:53 PM, Maya Matuszczyk wrote: > This patch adds a basic driver for the EC in Qualcomm Snapdragon X1 > Elite-based Lenovo Yoga Slim 7x. > > For now it supports only reporting that the AP is going to suspend and > the microphone mute button, however the EC seems to also support reading > fan information, other key combinations and thermal data. > > Signed-off-by: Maya Matuszczyk <maccraft123mc@gmail.com> > --- > MAINTAINERS | 1 + > drivers/platform/arm64/Kconfig | 12 ++ > drivers/platform/arm64/Makefile | 1 + > drivers/platform/arm64/lenovo-yoga-slim7x.c | 202 ++++++++++++++++++++ > 4 files changed, 216 insertions(+) > create mode 100644 drivers/platform/arm64/lenovo-yoga-slim7x.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index 0d4bfdde166d..f689cba80fa3 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -12906,6 +12906,7 @@ LENOVO YOGA SLIM 7X EC DRIVER > M: Maya Matuszczyk <maccraft123mc@gmail.com> > S: Maintained > F: Documentation/devicetree/bindings/platform/lenovo,yoga-slim7x-ec.yaml > +F: drivers/platform/arm64/lenovo-yoga-slim7x.c > > LETSKETCH HID TABLET DRIVER > M: Hans de Goede <hdegoede@redhat.com> > diff --git a/drivers/platform/arm64/Kconfig b/drivers/platform/arm64/Kconfig > index f88395ea3376..9ceae50f8b4e 100644 > --- a/drivers/platform/arm64/Kconfig > +++ b/drivers/platform/arm64/Kconfig > @@ -49,4 +49,16 @@ config EC_LENOVO_YOGA_C630 > > Say M or Y here to include this support. > > +config EC_LENOVO_YOGA_SLIM7X > + tristate "Lenovo Yoga Slim 7x Embedded Controller driver" > + depends on ARCH_QCOM || COMPILE_TEST > + depends on I2C > + help > + Select this option to enable driver for the EC found in the Lenovo > + Yoga Slim 7x. > + > + This driver currently supports reporting input event for microphone > + mute button, and 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..70dfc1fb979d 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_LENOVO_YOGA_SLIM7X) += lenovo-yoga-slim7x.o > diff --git a/drivers/platform/arm64/lenovo-yoga-slim7x.c b/drivers/platform/arm64/lenovo-yoga-slim7x.c > new file mode 100644 > index 000000000000..8f6d523395bc > --- /dev/null > +++ b/drivers/platform/arm64/lenovo-yoga-slim7x.c > @@ -0,0 +1,202 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (c) 2024 Maya Matuszczyk <maya.matuszczyk@gmail.com> > + */ > +#include <linux/auxiliary_bus.h> > +#include <linux/cleanup.h> > +#include <linux/device.h> > +#include <linux/err.h> > +#include <linux/i2c.h> > +#include <linux/irqreturn.h> > +#include <linux/lockdep.h> > +#include <linux/module.h> > +#include <linux/mutex.h> > +#include <linux/notifier.h> > +#include <linux/slab.h> > +#include <linux/input.h> > +//#include <linux/platform_data/lenovo-yoga-slim7x.h> > + > +// These are the registers that i know about available from SMBUS > +#define EC_IRQ_REASON_REG 0x05 > +#define EC_SUSPEND_RESUME_REG 0x23 > +#define EC_IRQ_ENABLE_REG 0x35 > +#define EC_BACKLIGHT_STATUS_REG 0x83 > +#define EC_MIC_MUTE_LED_REG 0x84 > +#define EC_AC_STATUS_REG 0x90 > + > +// Valid values for EC_SUSPEND_RESUME_REG > +#define EC_NOTIFY_SUSPEND_ENTER 0x01 > +#define EC_NOTIFY_SUSPEND_EXIT 0x00 > +#define EC_NOTIFY_SCREEN_OFF 0x03 > +#define EC_NOTIFY_SCREEN_ON 0x04 > + > +// These are the values in EC_IRQ_REASON_REG that i could find in DSDT > +#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 > +#define EC_IRQ_LENOVO_SUPPORT_KEY 0x90 > +#define EC_IRQ_FN_Q 0x91 > +#define EC_IRQ_FN_M 0x92 > +#define EC_IRQ_FN_SPACE 0x93 > +#define EC_IRQ_FN_R 0x94 > +#define EC_IRQ_FNLOCK_ON 0x95 > +#define EC_IRQ_FNLOCK_OFF 0x96 > +#define EC_IRQ_FN_N 0x97 > +#define EC_IRQ_AI 0x9a > +#define EC_IRQ_NPU 0x9b > + > +struct yoga_slim7x_ec { > + struct i2c_client *client; > + struct input_dev *idev; > + struct mutex lock; > +}; > + > +static irqreturn_t yoga_slim7x_ec_irq(int irq, void *data) > +{ > + struct yoga_slim7x_ec *ec = data; > + struct device *dev = &ec->client->dev; > + int val; > + > + guard(mutex)(&ec->lock); > + > + 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; > + } > + > + switch (val) { > + case EC_IRQ_MICMUTE_BUTTON: > + input_report_key(ec->idev, KEY_MICMUTE, 1); > + input_sync(ec->idev); > + input_report_key(ec->idev, KEY_MICMUTE, 0); > + input_sync(ec->idev); > + break; > + default: > + dev_info(dev, "Unhandled EC IRQ reason: %d\n", val); > + } I strongly suggest to use include/linux/input/sparse-keymap.h functionality here. This will make adding new keys a lot easier and will allow re-mapping codes from userspace. E.g. replace the whole switch-case with: if (!sparse_keymap_report_event(ec->idev, val, 1, true)) dev_info(dev, "Unhandled EC IRQ reason: %d\n", val); This takes care of mapping val -> KEY_MICMUTE, and doing all the reporting (after calling sparse_keymap_setup() with an appropriate keymap from probe()). > + > + return IRQ_HANDLED; > +} > + > +static int yoga_slim7x_ec_probe(struct i2c_client *client) > +{ > + struct device *dev = &client->dev; > + struct yoga_slim7x_ec *ec; > + int ret; > + > + ec = devm_kzalloc(dev, sizeof(*ec), GFP_KERNEL); > + if (!ec) > + return -ENOMEM; > + > + mutex_init(&ec->lock); > + ec->client = client; > + > + ec->idev = devm_input_allocate_device(dev); > + if (!ec->idev) > + return -ENOMEM; > + ec->idev->name = "yoga-slim7x-ec"; > + ec->idev->phys = "yoga-slim7x-ec/input0"; > + input_set_capability(ec->idev, EV_KEY, KEY_MICMUTE); Same here, please use sparse_keymap_setup() here to have it setup capabilities for all keys in your (to be defined) const struct key_entry yoga_slim7x_ec_keymap[] This way you only need to add new mappings in the keymap and then both the capability setting as well as the reporting in the irq-handler will be taken care of by the sparse-keymap helpers. Other then that the overall structure of the driver looks good (again this is not a full review, just a quick scan). Regards, Hans > + > + ret = input_register_device(ec->idev); > + if (ret < 0) > + return dev_err_probe(dev, ret, "Failed to register input device\n"); > + > + ret = devm_request_threaded_irq(dev, client->irq, > + NULL, yoga_slim7x_ec_irq, > + IRQF_ONESHOT, "yoga_slim7x_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 yoga_slim7x_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 yoga_slim7x_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 yoga_slim7x_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 yoga_slim7x_ec_of_match[] = { > + { .compatible = "lenovo,yoga-slim7x-ec" }, > + {} > +}; > +MODULE_DEVICE_TABLE(of, yoga_slim7x_ec_of_match); > + > +static const struct i2c_device_id yoga_slim7x_ec_i2c_id_table[] = { > + { "yoga-slim7x-ec", }, > + {} > +}; > +MODULE_DEVICE_TABLE(i2c, yoga_slim7x_ec_i2c_id_table); > + > +static DEFINE_SIMPLE_DEV_PM_OPS(yoga_slim7x_ec_pm_ops, > + yoga_slim7x_ec_suspend, > + yoga_slim7x_ec_resume); > + > +static struct i2c_driver yoga_slim7x_ec_i2c_driver = { > + .driver = { > + .name = "yoga-slim7x-ec", > + .of_match_table = yoga_slim7x_ec_of_match, > + .pm = &yoga_slim7x_ec_pm_ops > + }, > + .probe = yoga_slim7x_ec_probe, > + .remove = yoga_slim7x_ec_remove, > + .id_table = yoga_slim7x_ec_i2c_id_table, > +}; > +module_i2c_driver(yoga_slim7x_ec_i2c_driver); > + > +MODULE_DESCRIPTION("Lenovo Yoga Slim 7x Embedded Controller"); > +MODULE_LICENSE("GPL"); ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/3] platform: arm64: Add driver for Lenovo Yoga Slim 7x's EC 2024-09-28 15:53 ` Hans de Goede @ 2024-09-28 18:04 ` Maya Matuszczyk 2024-09-28 18:27 ` Hans de Goede 0 siblings, 1 reply; 6+ messages in thread From: Maya Matuszczyk @ 2024-09-28 18:04 UTC (permalink / raw) To: Hans de Goede Cc: Ilpo Järvinen, Bryan O'Donoghue, linux-kernel, platform-driver-x86 Hi, thanks for taking a look at this driver. sob., 28 wrz 2024 o 17:53 Hans de Goede <hdegoede@redhat.com> napisał(a): > > Hi Maya, > > Thank you for your patch. It is great to so people working on supporting > more ARM64 based laptop ECs. > > Note not a full review just one remark from a quick scan of the driver. > > On 27-Sep-24 8:53 PM, Maya Matuszczyk wrote: > > This patch adds a basic driver for the EC in Qualcomm Snapdragon X1 > > Elite-based Lenovo Yoga Slim 7x. > > > > For now it supports only reporting that the AP is going to suspend and > > the microphone mute button, however the EC seems to also support reading > > fan information, other key combinations and thermal data. > > > > Signed-off-by: Maya Matuszczyk <maccraft123mc@gmail.com> > > --- > > MAINTAINERS | 1 + > > drivers/platform/arm64/Kconfig | 12 ++ > > drivers/platform/arm64/Makefile | 1 + > > drivers/platform/arm64/lenovo-yoga-slim7x.c | 202 ++++++++++++++++++++ > > 4 files changed, 216 insertions(+) > > create mode 100644 drivers/platform/arm64/lenovo-yoga-slim7x.c > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index 0d4bfdde166d..f689cba80fa3 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -12906,6 +12906,7 @@ LENOVO YOGA SLIM 7X EC DRIVER > > M: Maya Matuszczyk <maccraft123mc@gmail.com> > > S: Maintained > > F: Documentation/devicetree/bindings/platform/lenovo,yoga-slim7x-ec.yaml > > +F: drivers/platform/arm64/lenovo-yoga-slim7x.c > > > > LETSKETCH HID TABLET DRIVER > > M: Hans de Goede <hdegoede@redhat.com> > > diff --git a/drivers/platform/arm64/Kconfig b/drivers/platform/arm64/Kconfig > > index f88395ea3376..9ceae50f8b4e 100644 > > --- a/drivers/platform/arm64/Kconfig > > +++ b/drivers/platform/arm64/Kconfig > > @@ -49,4 +49,16 @@ config EC_LENOVO_YOGA_C630 > > > > Say M or Y here to include this support. > > > > +config EC_LENOVO_YOGA_SLIM7X > > + tristate "Lenovo Yoga Slim 7x Embedded Controller driver" > > + depends on ARCH_QCOM || COMPILE_TEST > > + depends on I2C > > + help > > + Select this option to enable driver for the EC found in the Lenovo > > + Yoga Slim 7x. > > + > > + This driver currently supports reporting input event for microphone > > + mute button, and 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..70dfc1fb979d 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_LENOVO_YOGA_SLIM7X) += lenovo-yoga-slim7x.o > > diff --git a/drivers/platform/arm64/lenovo-yoga-slim7x.c b/drivers/platform/arm64/lenovo-yoga-slim7x.c > > new file mode 100644 > > index 000000000000..8f6d523395bc > > --- /dev/null > > +++ b/drivers/platform/arm64/lenovo-yoga-slim7x.c > > @@ -0,0 +1,202 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +/* > > + * Copyright (c) 2024 Maya Matuszczyk <maya.matuszczyk@gmail.com> > > + */ > > +#include <linux/auxiliary_bus.h> > > +#include <linux/cleanup.h> > > +#include <linux/device.h> > > +#include <linux/err.h> > > +#include <linux/i2c.h> > > +#include <linux/irqreturn.h> > > +#include <linux/lockdep.h> > > +#include <linux/module.h> > > +#include <linux/mutex.h> > > +#include <linux/notifier.h> > > +#include <linux/slab.h> > > +#include <linux/input.h> > > +//#include <linux/platform_data/lenovo-yoga-slim7x.h> > > + > > +// These are the registers that i know about available from SMBUS > > +#define EC_IRQ_REASON_REG 0x05 > > +#define EC_SUSPEND_RESUME_REG 0x23 > > +#define EC_IRQ_ENABLE_REG 0x35 > > +#define EC_BACKLIGHT_STATUS_REG 0x83 > > +#define EC_MIC_MUTE_LED_REG 0x84 > > +#define EC_AC_STATUS_REG 0x90 > > + > > +// Valid values for EC_SUSPEND_RESUME_REG > > +#define EC_NOTIFY_SUSPEND_ENTER 0x01 > > +#define EC_NOTIFY_SUSPEND_EXIT 0x00 > > +#define EC_NOTIFY_SCREEN_OFF 0x03 > > +#define EC_NOTIFY_SCREEN_ON 0x04 > > + > > +// These are the values in EC_IRQ_REASON_REG that i could find in DSDT > > +#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 > > +#define EC_IRQ_LENOVO_SUPPORT_KEY 0x90 > > +#define EC_IRQ_FN_Q 0x91 > > +#define EC_IRQ_FN_M 0x92 > > +#define EC_IRQ_FN_SPACE 0x93 > > +#define EC_IRQ_FN_R 0x94 > > +#define EC_IRQ_FNLOCK_ON 0x95 > > +#define EC_IRQ_FNLOCK_OFF 0x96 > > +#define EC_IRQ_FN_N 0x97 > > +#define EC_IRQ_AI 0x9a > > +#define EC_IRQ_NPU 0x9b > > + > > +struct yoga_slim7x_ec { > > + struct i2c_client *client; > > + struct input_dev *idev; > > + struct mutex lock; > > +}; > > + > > +static irqreturn_t yoga_slim7x_ec_irq(int irq, void *data) > > +{ > > + struct yoga_slim7x_ec *ec = data; > > + struct device *dev = &ec->client->dev; > > + int val; > > + > > + guard(mutex)(&ec->lock); > > + > > + 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; > > + } > > + > > + switch (val) { > > + case EC_IRQ_MICMUTE_BUTTON: > > + input_report_key(ec->idev, KEY_MICMUTE, 1); > > + input_sync(ec->idev); > > + input_report_key(ec->idev, KEY_MICMUTE, 0); > > + input_sync(ec->idev); > > + break; > > + default: > > + dev_info(dev, "Unhandled EC IRQ reason: %d\n", val); > > + } > > I strongly suggest to use include/linux/input/sparse-keymap.h functionality > here. This will make adding new keys a lot easier and will allow re-mapping > codes from userspace. > > E.g. replace the whole switch-case with: > > if (!sparse_keymap_report_event(ec->idev, val, 1, true)) > dev_info(dev, "Unhandled EC IRQ reason: %d\n", val); > > This takes care of mapping val -> KEY_MICMUTE, and doing all > the reporting (after calling sparse_keymap_setup() with an appropriate > keymap from probe()) Thank you for the suggestion. I'm not sure how to handle the non-key related IRQs, like fan status changes. Do you think they should be filtered out like this: if (val == 0x04 || (val >= 0x90 && val <= 0x97)) Best Regards, Maya Matuszczyk. > > > > > + > > + return IRQ_HANDLED; > > +} > > + > > +static int yoga_slim7x_ec_probe(struct i2c_client *client) > > +{ > > + struct device *dev = &client->dev; > > + struct yoga_slim7x_ec *ec; > > + int ret; > > + > > + ec = devm_kzalloc(dev, sizeof(*ec), GFP_KERNEL); > > + if (!ec) > > + return -ENOMEM; > > + > > + mutex_init(&ec->lock); > > + ec->client = client; > > + > > + ec->idev = devm_input_allocate_device(dev); > > + if (!ec->idev) > > + return -ENOMEM; > > + ec->idev->name = "yoga-slim7x-ec"; > > + ec->idev->phys = "yoga-slim7x-ec/input0"; > > + input_set_capability(ec->idev, EV_KEY, KEY_MICMUTE); > > Same here, please use sparse_keymap_setup() here to have it > setup capabilities for all keys in your (to be defined) > > const struct key_entry yoga_slim7x_ec_keymap[] > > This way you only need to add new mappings in the keymap > and then both the capability setting as well as the reporting > in the irq-handler will be taken care of by the sparse-keymap > helpers. > > Other then that the overall structure of the driver looks good > (again this is not a full review, just a quick scan). > > Regards, > > Hans > > > > > > > + > > + ret = input_register_device(ec->idev); > > + if (ret < 0) > > + return dev_err_probe(dev, ret, "Failed to register input device\n"); > > + > > + ret = devm_request_threaded_irq(dev, client->irq, > > + NULL, yoga_slim7x_ec_irq, > > + IRQF_ONESHOT, "yoga_slim7x_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 yoga_slim7x_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 yoga_slim7x_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 yoga_slim7x_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 yoga_slim7x_ec_of_match[] = { > > + { .compatible = "lenovo,yoga-slim7x-ec" }, > > + {} > > +}; > > +MODULE_DEVICE_TABLE(of, yoga_slim7x_ec_of_match); > > + > > +static const struct i2c_device_id yoga_slim7x_ec_i2c_id_table[] = { > > + { "yoga-slim7x-ec", }, > > + {} > > +}; > > +MODULE_DEVICE_TABLE(i2c, yoga_slim7x_ec_i2c_id_table); > > + > > +static DEFINE_SIMPLE_DEV_PM_OPS(yoga_slim7x_ec_pm_ops, > > + yoga_slim7x_ec_suspend, > > + yoga_slim7x_ec_resume); > > + > > +static struct i2c_driver yoga_slim7x_ec_i2c_driver = { > > + .driver = { > > + .name = "yoga-slim7x-ec", > > + .of_match_table = yoga_slim7x_ec_of_match, > > + .pm = &yoga_slim7x_ec_pm_ops > > + }, > > + .probe = yoga_slim7x_ec_probe, > > + .remove = yoga_slim7x_ec_remove, > > + .id_table = yoga_slim7x_ec_i2c_id_table, > > +}; > > +module_i2c_driver(yoga_slim7x_ec_i2c_driver); > > + > > +MODULE_DESCRIPTION("Lenovo Yoga Slim 7x Embedded Controller"); > > +MODULE_LICENSE("GPL"); > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/3] platform: arm64: Add driver for Lenovo Yoga Slim 7x's EC 2024-09-28 18:04 ` Maya Matuszczyk @ 2024-09-28 18:27 ` Hans de Goede 0 siblings, 0 replies; 6+ messages in thread From: Hans de Goede @ 2024-09-28 18:27 UTC (permalink / raw) To: Maya Matuszczyk Cc: Ilpo Järvinen, Bryan O'Donoghue, linux-kernel, platform-driver-x86 Hi, On 28-Sep-24 8:04 PM, Maya Matuszczyk wrote: > Hi, thanks for taking a look at this driver. > > sob., 28 wrz 2024 o 17:53 Hans de Goede <hdegoede@redhat.com> napisał(a): >> >> Hi Maya, >> >> Thank you for your patch. It is great to so people working on supporting >> more ARM64 based laptop ECs. >> >> Note not a full review just one remark from a quick scan of the driver. >> >> On 27-Sep-24 8:53 PM, Maya Matuszczyk wrote: >>> This patch adds a basic driver for the EC in Qualcomm Snapdragon X1 >>> Elite-based Lenovo Yoga Slim 7x. >>> >>> For now it supports only reporting that the AP is going to suspend and >>> the microphone mute button, however the EC seems to also support reading >>> fan information, other key combinations and thermal data. >>> >>> Signed-off-by: Maya Matuszczyk <maccraft123mc@gmail.com> >>> --- >>> MAINTAINERS | 1 + >>> drivers/platform/arm64/Kconfig | 12 ++ >>> drivers/platform/arm64/Makefile | 1 + >>> drivers/platform/arm64/lenovo-yoga-slim7x.c | 202 ++++++++++++++++++++ >>> 4 files changed, 216 insertions(+) >>> create mode 100644 drivers/platform/arm64/lenovo-yoga-slim7x.c >>> >>> diff --git a/MAINTAINERS b/MAINTAINERS >>> index 0d4bfdde166d..f689cba80fa3 100644 >>> --- a/MAINTAINERS >>> +++ b/MAINTAINERS >>> @@ -12906,6 +12906,7 @@ LENOVO YOGA SLIM 7X EC DRIVER >>> M: Maya Matuszczyk <maccraft123mc@gmail.com> >>> S: Maintained >>> F: Documentation/devicetree/bindings/platform/lenovo,yoga-slim7x-ec.yaml >>> +F: drivers/platform/arm64/lenovo-yoga-slim7x.c >>> >>> LETSKETCH HID TABLET DRIVER >>> M: Hans de Goede <hdegoede@redhat.com> >>> diff --git a/drivers/platform/arm64/Kconfig b/drivers/platform/arm64/Kconfig >>> index f88395ea3376..9ceae50f8b4e 100644 >>> --- a/drivers/platform/arm64/Kconfig >>> +++ b/drivers/platform/arm64/Kconfig >>> @@ -49,4 +49,16 @@ config EC_LENOVO_YOGA_C630 >>> >>> Say M or Y here to include this support. >>> >>> +config EC_LENOVO_YOGA_SLIM7X >>> + tristate "Lenovo Yoga Slim 7x Embedded Controller driver" >>> + depends on ARCH_QCOM || COMPILE_TEST >>> + depends on I2C >>> + help >>> + Select this option to enable driver for the EC found in the Lenovo >>> + Yoga Slim 7x. >>> + >>> + This driver currently supports reporting input event for microphone >>> + mute button, and 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..70dfc1fb979d 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_LENOVO_YOGA_SLIM7X) += lenovo-yoga-slim7x.o >>> diff --git a/drivers/platform/arm64/lenovo-yoga-slim7x.c b/drivers/platform/arm64/lenovo-yoga-slim7x.c >>> new file mode 100644 >>> index 000000000000..8f6d523395bc >>> --- /dev/null >>> +++ b/drivers/platform/arm64/lenovo-yoga-slim7x.c >>> @@ -0,0 +1,202 @@ >>> +// SPDX-License-Identifier: GPL-2.0-only >>> +/* >>> + * Copyright (c) 2024 Maya Matuszczyk <maya.matuszczyk@gmail.com> >>> + */ >>> +#include <linux/auxiliary_bus.h> >>> +#include <linux/cleanup.h> >>> +#include <linux/device.h> >>> +#include <linux/err.h> >>> +#include <linux/i2c.h> >>> +#include <linux/irqreturn.h> >>> +#include <linux/lockdep.h> >>> +#include <linux/module.h> >>> +#include <linux/mutex.h> >>> +#include <linux/notifier.h> >>> +#include <linux/slab.h> >>> +#include <linux/input.h> >>> +//#include <linux/platform_data/lenovo-yoga-slim7x.h> >>> + >>> +// These are the registers that i know about available from SMBUS >>> +#define EC_IRQ_REASON_REG 0x05 >>> +#define EC_SUSPEND_RESUME_REG 0x23 >>> +#define EC_IRQ_ENABLE_REG 0x35 >>> +#define EC_BACKLIGHT_STATUS_REG 0x83 >>> +#define EC_MIC_MUTE_LED_REG 0x84 >>> +#define EC_AC_STATUS_REG 0x90 >>> + >>> +// Valid values for EC_SUSPEND_RESUME_REG >>> +#define EC_NOTIFY_SUSPEND_ENTER 0x01 >>> +#define EC_NOTIFY_SUSPEND_EXIT 0x00 >>> +#define EC_NOTIFY_SCREEN_OFF 0x03 >>> +#define EC_NOTIFY_SCREEN_ON 0x04 >>> + >>> +// These are the values in EC_IRQ_REASON_REG that i could find in DSDT >>> +#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 >>> +#define EC_IRQ_LENOVO_SUPPORT_KEY 0x90 >>> +#define EC_IRQ_FN_Q 0x91 >>> +#define EC_IRQ_FN_M 0x92 >>> +#define EC_IRQ_FN_SPACE 0x93 >>> +#define EC_IRQ_FN_R 0x94 >>> +#define EC_IRQ_FNLOCK_ON 0x95 >>> +#define EC_IRQ_FNLOCK_OFF 0x96 >>> +#define EC_IRQ_FN_N 0x97 >>> +#define EC_IRQ_AI 0x9a >>> +#define EC_IRQ_NPU 0x9b >>> + >>> +struct yoga_slim7x_ec { >>> + struct i2c_client *client; >>> + struct input_dev *idev; >>> + struct mutex lock; >>> +}; >>> + >>> +static irqreturn_t yoga_slim7x_ec_irq(int irq, void *data) >>> +{ >>> + struct yoga_slim7x_ec *ec = data; >>> + struct device *dev = &ec->client->dev; >>> + int val; >>> + >>> + guard(mutex)(&ec->lock); >>> + >>> + 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; >>> + } >>> + >>> + switch (val) { >>> + case EC_IRQ_MICMUTE_BUTTON: >>> + input_report_key(ec->idev, KEY_MICMUTE, 1); >>> + input_sync(ec->idev); >>> + input_report_key(ec->idev, KEY_MICMUTE, 0); >>> + input_sync(ec->idev); >>> + break; >>> + default: >>> + dev_info(dev, "Unhandled EC IRQ reason: %d\n", val); >>> + } >> >> I strongly suggest to use include/linux/input/sparse-keymap.h functionality >> here. This will make adding new keys a lot easier and will allow re-mapping >> codes from userspace. >> >> E.g. replace the whole switch-case with: >> >> if (!sparse_keymap_report_event(ec->idev, val, 1, true)) >> dev_info(dev, "Unhandled EC IRQ reason: %d\n", val); >> >> This takes care of mapping val -> KEY_MICMUTE, and doing all >> the reporting (after calling sparse_keymap_setup() with an appropriate >> keymap from probe()) > > Thank you for the suggestion. I'm not sure how to handle the non-key > related IRQs, like fan status changes. > Do you think they should be filtered out like this: > if (val == 0x04 || (val >= 0x90 && val <= 0x97)) Yes drivers like this typically first check for special values like above and then either do an early "return" or have a structure like this: if (val == special) ... else if (val == other-special) ... else { if (!sparse_keymap_report_event(ec->idev, val, 1, true)) dev_info(dev, "Unhandled EC IRQ reason: %d\n", val); } The top-level if () ... else if () ... else may also be replaced with a switch () { case : ... case : ... }. Note that if you just want to ignore the values you can also do this in the keymap and generally speaking that is better, only adding special cases like above if the driver actually needs to undertake some special action in response to the IRQ. static const struct key_entry keymap[] = { { KE_KEY, EC_IRQ_MICMUTE_BUTTON, { KEY_MICMUTE } }, { KE_IGNORE, 0x04, { KEY_RESERVED } }, /* Fan status change */ { KE_IGNORE, 0x90, { KEY_RESERVED } }, /* Fan status change */ { KE_IGNORE, 0x91, { KEY_RESERVED } }, /* Fan status change */ { KE_IGNORE, 0x92, { KEY_RESERVED } }, /* Fan status change */ { KE_IGNORE, 0x93, { KEY_RESERVED } }, /* Fan status change */ { KE_IGNORE, 0x94, { KEY_RESERVED } }, /* Fan status change */ { KE_IGNORE, 0x95, { KEY_RESERVED } }, /* Fan status change */ { KE_IGNORE, 0x96, { KEY_RESERVED } }, /* Fan status change */ { KE_IGNORE, 0x97, { KEY_RESERVED } }, /* Fan status change */ { KE_END } }; In this case with the 0x90 - 0x97 range that is not really pretty, but it probably still is best to keep the code consistent. Regards, Hans ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/3] platform: arm64: Add driver for Lenovo Yoga Slim 7x's EC 2024-09-27 18:53 ` [PATCH 2/3] platform: arm64: Add driver for Lenovo Yoga Slim 7x's EC Maya Matuszczyk 2024-09-28 10:35 ` György Kurucz 2024-09-28 15:53 ` Hans de Goede @ 2024-10-01 7:12 ` kernel test robot 2 siblings, 0 replies; 6+ messages in thread From: kernel test robot @ 2024-10-01 7:12 UTC (permalink / raw) To: Maya Matuszczyk, Hans de Goede, Ilpo Järvinen, Bryan O'Donoghue Cc: oe-kbuild-all, linux-kernel, platform-driver-x86 Hi Maya, kernel test robot noticed the following build errors: [auto build test ERROR on robh/for-next] [also build test ERROR on linus/master v6.12-rc1 next-20241001] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Maya-Matuszczyk/platform-arm64-Add-driver-for-Lenovo-Yoga-Slim-7x-s-EC/20240928-025757 base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next patch link: https://lore.kernel.org/r/20240927185345.3680-2-maccraft123mc%40gmail.com patch subject: [PATCH 2/3] platform: arm64: Add driver for Lenovo Yoga Slim 7x's EC config: sh-randconfig-002-20241001 (https://download.01.org/0day-ci/archive/20241001/202410011448.NB5fJNNX-lkp@intel.com/config) compiler: sh4-linux-gcc (GCC) 14.1.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241001/202410011448.NB5fJNNX-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202410011448.NB5fJNNX-lkp@intel.com/ All errors (new ones prefixed by >>, old ones prefixed by <<): >> ERROR: modpost: "devm_input_allocate_device" [drivers/platform/arm64/lenovo-yoga-slim7x.ko] undefined! >> ERROR: modpost: "input_register_device" [drivers/platform/arm64/lenovo-yoga-slim7x.ko] undefined! >> ERROR: modpost: "input_set_capability" [drivers/platform/arm64/lenovo-yoga-slim7x.ko] undefined! >> ERROR: modpost: "input_event" [drivers/platform/arm64/lenovo-yoga-slim7x.ko] undefined! -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-10-01 7:13 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20240927185345.3680-1-maccraft123mc@gmail.com>
2024-09-27 18:53 ` [PATCH 2/3] platform: arm64: Add driver for Lenovo Yoga Slim 7x's EC Maya Matuszczyk
2024-09-28 10:35 ` György Kurucz
2024-09-28 15:53 ` Hans de Goede
2024-09-28 18:04 ` Maya Matuszczyk
2024-09-28 18:27 ` Hans de Goede
2024-10-01 7:12 ` kernel test robot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox