* [PATCH v4 2/2] Input: Add support for Wacom W9000-series penabled touchscreens
From: Hendrik Noack @ 2026-03-07 18:15 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Hendrik Noack, Ferass El Hafidi, linux-input, devicetree,
linux-kernel
In-Reply-To: <20260307181557.66927-1-hendrik-noack@gmx.de>
Add driver for Wacom W9002 and two Wacom W9007A variants. These are
penabled touchscreens supporting passive Wacom Pens and use I2C.
Co-developed-by: Ferass El Hafidi <funderscore@postmarketos.org>
Signed-off-by: Ferass El Hafidi <funderscore@postmarketos.org>
Signed-off-by: Hendrik Noack <hendrik-noack@gmx.de>
---
drivers/input/touchscreen/Kconfig | 12 +
drivers/input/touchscreen/Makefile | 1 +
drivers/input/touchscreen/wacom_w9000.c | 510 ++++++++++++++++++++++++
3 files changed, 523 insertions(+)
create mode 100644 drivers/input/touchscreen/wacom_w9000.c
diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index 7d5b72ee07fa..a28328fb7648 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -610,6 +610,18 @@ config TOUCHSCREEN_WACOM_I2C
To compile this driver as a module, choose M here: the module
will be called wacom_i2c.
+config TOUCHSCREEN_WACOM_W9000
+ tristate "Wacom W9000-series penabled touchscreen (I2C)"
+ depends on I2C
+ help
+ Say Y here if you have a Wacom W9000-series penabled I2C touchscreen.
+ This driver supports models W9002 and W9007A.
+
+ If unsure, say N.
+
+ To compile this driver as a module, choose M here: the module
+ will be called wacom_w9000.
+
config TOUCHSCREEN_LPC32XX
tristate "LPC32XX touchscreen controller"
depends on ARCH_LPC32XX
diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index ab9abd151078..aa3915df83b2 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -102,6 +102,7 @@ tsc2007-$(CONFIG_TOUCHSCREEN_TSC2007_IIO) += tsc2007_iio.o
obj-$(CONFIG_TOUCHSCREEN_TSC2007) += tsc2007.o
obj-$(CONFIG_TOUCHSCREEN_WACOM_W8001) += wacom_w8001.o
obj-$(CONFIG_TOUCHSCREEN_WACOM_I2C) += wacom_i2c.o
+obj-$(CONFIG_TOUCHSCREEN_WACOM_W9000) += wacom_w9000.o
obj-$(CONFIG_TOUCHSCREEN_WDT87XX_I2C) += wdt87xx_i2c.o
obj-$(CONFIG_TOUCHSCREEN_WM831X) += wm831x-ts.o
obj-$(CONFIG_TOUCHSCREEN_WM97XX) += wm97xx-ts.o
diff --git a/drivers/input/touchscreen/wacom_w9000.c b/drivers/input/touchscreen/wacom_w9000.c
new file mode 100644
index 000000000000..3c7959a28ccb
--- /dev/null
+++ b/drivers/input/touchscreen/wacom_w9000.c
@@ -0,0 +1,510 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Wacom W9000-series penabled I2C touchscreen driver
+ *
+ * Copyright (c) 2026 Hendrik Noack <hendrik-noack@gmx.de>
+ *
+ * Partially based on vendor driver:
+ * Copyright (C) 2012, Samsung Electronics Co. Ltd.
+ */
+
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/input.h>
+#include <linux/input/touchscreen.h>
+#include <linux/unaligned.h>
+
+/* Some chips have flaky firmware that requires many retries before responding. */
+#define CMD_QUERY_RETRIES 8
+
+/* Message length */
+#define CMD_QUERY_NUM_MAX 9
+#define MSG_COORD_NUM_MAX 12
+
+/* Commands */
+#define CMD_QUERY 0x2a
+
+struct wacom_w9000_variant {
+ const unsigned int cmd_query_num;
+ const unsigned int msg_coord_num;
+ const char *name;
+};
+
+struct wacom_w9000_data {
+ struct i2c_client *client;
+ struct input_dev *input_dev;
+ const struct wacom_w9000_variant *variant;
+ unsigned int fw_version;
+
+ struct touchscreen_properties prop;
+ unsigned int max_pressure;
+
+ struct regulator *regulator;
+
+ struct gpio_desc *flash_mode_gpio;
+ struct gpio_desc *pen_inserted_gpio;
+ struct gpio_desc *reset_gpio;
+
+ unsigned int irq;
+ unsigned int pen_insert_irq;
+
+ bool pen_inserted;
+ bool pen_proximity;
+};
+
+static int wacom_w9000_read(struct i2c_client *client, u8 command, int len, char *data)
+{
+ int error, res;
+ struct i2c_msg msg[] = {
+ {
+ .addr = client->addr,
+ .flags = 0,
+ .buf = &command,
+ .len = sizeof(command),
+ }, {
+ .addr = client->addr,
+ .flags = I2C_M_RD,
+ .buf = data,
+ .len = len,
+ }
+ };
+
+ res = i2c_transfer(client->adapter, msg, ARRAY_SIZE(msg));
+ if (res != ARRAY_SIZE(msg)) {
+ error = res < 0 ? res : -EIO;
+ dev_err(&client->dev, "%s: i2c transfer failed: %d (%d)\n", __func__, error, res);
+ return error;
+ }
+
+ return 0;
+}
+
+static int wacom_w9000_query(struct wacom_w9000_data *wacom_data)
+{
+ struct i2c_client *client = wacom_data->client;
+ struct device *dev = &wacom_data->client->dev;
+ int error;
+ int retry = 0;
+ u8 data[CMD_QUERY_NUM_MAX];
+
+ for (; retry < CMD_QUERY_RETRIES; retry++) {
+ error = wacom_w9000_read(client, CMD_QUERY, wacom_data->variant->cmd_query_num,
+ data);
+
+ if (!error && (data[0] == 0x0f))
+ break;
+ }
+
+ if (error)
+ return error;
+
+ dev_dbg(dev, "query: %*ph, %d\n", wacom_data->variant->cmd_query_num, data, retry);
+
+ wacom_data->prop.max_x = get_unaligned_be16(&data[1]);
+ wacom_data->prop.max_y = get_unaligned_be16(&data[3]);
+ wacom_data->max_pressure = get_unaligned_be16(&data[5]);
+ wacom_data->fw_version = get_unaligned_be16(&data[7]);
+
+ dev_dbg(dev, "max_x:%d, max_y:%d, max_pressure:%d, fw:%#x", wacom_data->prop.max_x,
+ wacom_data->prop.max_y, wacom_data->max_pressure,
+ wacom_data->fw_version);
+
+ return 0;
+}
+
+static int wacom_w9000_power_on(struct wacom_w9000_data *wacom_data)
+{
+ int error;
+
+ error = regulator_enable(wacom_data->regulator);
+ if (error) {
+ dev_err(&wacom_data->client->dev, "Failed to enable regulators: %d\n", error);
+ return error;
+ }
+
+ msleep(200);
+
+ gpiod_set_value_cansleep(wacom_data->reset_gpio, 0);
+ enable_irq(wacom_data->irq);
+
+ return error;
+}
+
+static void wacom_w9000_power_off(struct wacom_w9000_data *wacom_data)
+{
+ disable_irq(wacom_data->irq);
+ gpiod_set_value_cansleep(wacom_data->reset_gpio, 1);
+ regulator_disable(wacom_data->regulator);
+}
+
+static void wacom_w9000_coord(struct wacom_w9000_data *wacom_data)
+{
+ struct i2c_client *client = wacom_data->client;
+ struct device *dev = &wacom_data->client->dev;
+ int error;
+ u8 data[MSG_COORD_NUM_MAX];
+ bool touch, rubber, side_button;
+ u16 x, y, pressure;
+ u8 distance = 0;
+
+ error = i2c_master_recv(client, data, wacom_data->variant->msg_coord_num);
+ if (error != wacom_data->variant->msg_coord_num) {
+ if (error >= 0)
+ error = -EIO;
+ dev_err(dev, "%s: i2c receive failed (%d)\n", __func__, error);
+ return;
+ }
+
+ dev_dbg(dev, "data: %*ph", wacom_data->variant->msg_coord_num, data);
+
+ if (data[0] & BIT(7)) {
+ wacom_data->pen_proximity = true;
+
+ touch = !!(data[0] & BIT(4));
+ side_button = !!(data[0] & BIT(5));
+ rubber = !!(data[0] & BIT(6));
+
+ x = get_unaligned_be16(&data[1]);
+ y = get_unaligned_be16(&data[3]);
+ pressure = get_unaligned_be16(&data[5]);
+
+ if (wacom_data->variant->msg_coord_num > 7)
+ distance = data[7];
+
+ if (x > wacom_data->prop.max_x || y > wacom_data->prop.max_y) {
+ dev_warn(dev, "Coordinates out of range x=%d, y=%d", x, y);
+ return;
+ }
+
+ touchscreen_report_pos(wacom_data->input_dev, &wacom_data->prop, x, y, false);
+ input_report_abs(wacom_data->input_dev, ABS_PRESSURE, pressure);
+
+ if (wacom_data->variant->msg_coord_num > 7)
+ input_report_abs(wacom_data->input_dev, ABS_DISTANCE, distance);
+
+ input_report_key(wacom_data->input_dev, BTN_STYLUS, side_button);
+ input_report_key(wacom_data->input_dev, BTN_TOUCH, touch);
+ input_report_key(wacom_data->input_dev, BTN_TOOL_PEN, !rubber);
+ input_report_key(wacom_data->input_dev, BTN_TOOL_RUBBER, rubber);
+ input_sync(wacom_data->input_dev);
+ } else if (wacom_data->pen_proximity) {
+ input_report_abs(wacom_data->input_dev, ABS_PRESSURE, 0);
+
+ if (wacom_data->variant->msg_coord_num > 7)
+ input_report_abs(wacom_data->input_dev, ABS_DISTANCE, 255);
+
+ input_report_key(wacom_data->input_dev, BTN_STYLUS, 0);
+ input_report_key(wacom_data->input_dev, BTN_TOUCH, 0);
+ input_report_key(wacom_data->input_dev, BTN_TOOL_PEN, 0);
+ input_report_key(wacom_data->input_dev, BTN_TOOL_RUBBER, 0);
+ input_sync(wacom_data->input_dev);
+
+ wacom_data->pen_proximity = false;
+ }
+}
+
+static irqreturn_t wacom_w9000_interrupt(int irq, void *dev_id)
+{
+ struct wacom_w9000_data *wacom_data = dev_id;
+
+ wacom_w9000_coord(wacom_data);
+
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t wacom_w9000_interrupt_pen_insert(int irq, void *dev_id)
+{
+ struct wacom_w9000_data *wacom_data = dev_id;
+ struct device *dev = &wacom_data->client->dev;
+ int error;
+ bool pen_inserted;
+
+ pen_inserted = gpiod_get_value_cansleep(wacom_data->pen_inserted_gpio);
+
+ input_report_switch(wacom_data->input_dev, SW_PEN_INSERTED, pen_inserted);
+ input_sync(wacom_data->input_dev);
+
+ if (!pen_inserted && wacom_data->pen_inserted) {
+ error = wacom_w9000_power_on(wacom_data);
+ if (error)
+ return IRQ_HANDLED;
+ } else if (pen_inserted && !wacom_data->pen_inserted) {
+ wacom_w9000_power_off(wacom_data);
+ }
+
+ dev_dbg(dev, "Pen inserted changed from %d to %d", wacom_data->pen_inserted, pen_inserted);
+
+ wacom_data->pen_inserted = pen_inserted;
+
+ return IRQ_HANDLED;
+}
+
+static int wacom_w9000_open(struct input_dev *dev)
+{
+ struct wacom_w9000_data *wacom_data = input_get_drvdata(dev);
+ int error;
+
+ if (wacom_data->pen_inserted_gpio) {
+ wacom_data->pen_inserted = gpiod_get_value_cansleep(wacom_data->pen_inserted_gpio);
+
+ input_report_switch(wacom_data->input_dev, SW_PEN_INSERTED,
+ wacom_data->pen_inserted);
+ input_sync(wacom_data->input_dev);
+ }
+
+ if (!wacom_data->pen_inserted) {
+ error = wacom_w9000_power_on(wacom_data);
+ if (error)
+ return error;
+ }
+
+ if (wacom_data->pen_inserted_gpio)
+ enable_irq(wacom_data->pen_insert_irq);
+
+ return 0;
+}
+
+static void wacom_w9000_close(struct input_dev *dev)
+{
+ struct wacom_w9000_data *wacom_data = input_get_drvdata(dev);
+
+ if (wacom_data->pen_inserted_gpio)
+ disable_irq(wacom_data->pen_insert_irq);
+
+ if (!wacom_data->pen_inserted)
+ wacom_w9000_power_off(wacom_data);
+}
+
+static int wacom_w9000_probe(struct i2c_client *client)
+{
+ struct device *dev = &client->dev;
+ struct wacom_w9000_data *wacom_data;
+ struct input_dev *input_dev;
+ int error;
+ u32 val;
+
+ if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
+ dev_err(dev, "i2c_check_functionality error\n");
+ return -EIO;
+ }
+
+ wacom_data = devm_kzalloc(dev, sizeof(*wacom_data), GFP_KERNEL);
+ if (!wacom_data)
+ return -ENOMEM;
+
+ wacom_data->variant = i2c_get_match_data(client);
+
+ if (wacom_data->variant->cmd_query_num > CMD_QUERY_NUM_MAX ||
+ wacom_data->variant->msg_coord_num > MSG_COORD_NUM_MAX) {
+ dev_err(dev, "Length of message for %s exceeds the maximum\n",
+ wacom_data->variant->name);
+ return -EINVAL;
+ }
+
+ if (wacom_data->variant->msg_coord_num < 7) {
+ dev_err(dev, "Length of coordinates message for %s too short\n",
+ wacom_data->variant->name);
+ return -EINVAL;
+ }
+
+ wacom_data->client = client;
+
+ input_dev = devm_input_allocate_device(dev);
+ if (!input_dev)
+ return -ENOMEM;
+
+ wacom_data->input_dev = input_dev;
+ input_set_drvdata(input_dev, wacom_data);
+
+ wacom_data->irq = client->irq;
+ i2c_set_clientdata(client, wacom_data);
+
+ wacom_data->regulator = devm_regulator_get(dev, "vdd");
+ if (IS_ERR(wacom_data->regulator))
+ return dev_err_probe(dev, PTR_ERR(wacom_data->regulator),
+ "Failed to get regulators\n");
+
+ wacom_data->flash_mode_gpio = devm_gpiod_get_optional(dev, "flash-mode", GPIOD_OUT_LOW);
+ if (IS_ERR(wacom_data->flash_mode_gpio))
+ return dev_err_probe(dev, PTR_ERR(wacom_data->flash_mode_gpio),
+ "Failed to get flash-mode gpio\n");
+
+ wacom_data->pen_inserted_gpio = devm_gpiod_get_optional(dev, "pen-inserted", GPIOD_IN);
+ if (IS_ERR(wacom_data->pen_inserted_gpio))
+ return dev_err_probe(dev, PTR_ERR(wacom_data->pen_inserted_gpio),
+ "Failed to get pen-insert gpio\n");
+
+ wacom_data->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
+ if (IS_ERR(wacom_data->reset_gpio))
+ return dev_err_probe(dev, PTR_ERR(wacom_data->reset_gpio),
+ "Failed to get reset gpio\n");
+
+ error = regulator_enable(wacom_data->regulator);
+ if (error)
+ return dev_err_probe(dev, error, "Failed to enable regulators\n");
+
+ msleep(200);
+
+ gpiod_set_value_cansleep(wacom_data->reset_gpio, 0);
+
+ error = wacom_w9000_query(wacom_data);
+
+ gpiod_set_value_cansleep(wacom_data->reset_gpio, 1);
+ regulator_disable(wacom_data->regulator);
+
+ if (error)
+ return dev_err_probe(dev, error, "Failed to query\n");
+
+ input_dev->name = wacom_data->variant->name;
+ input_dev->id.bustype = BUS_I2C;
+ input_dev->dev.parent = dev;
+ input_dev->id.vendor = 0x56a;
+ input_dev->id.version = wacom_data->fw_version;
+ input_dev->open = wacom_w9000_open;
+ input_dev->close = wacom_w9000_close;
+
+ input_set_capability(input_dev, EV_KEY, BTN_TOUCH);
+ input_set_capability(input_dev, EV_KEY, BTN_TOOL_PEN);
+ input_set_capability(input_dev, EV_KEY, BTN_TOOL_RUBBER);
+ input_set_capability(input_dev, EV_KEY, BTN_STYLUS);
+
+ input_set_abs_params(input_dev, ABS_X, 0, wacom_data->prop.max_x, 4, 0);
+ input_set_abs_params(input_dev, ABS_Y, 0, wacom_data->prop.max_y, 4, 0);
+ input_set_abs_params(input_dev, ABS_PRESSURE, 0, wacom_data->max_pressure, 0, 0);
+
+ if (wacom_data->variant->msg_coord_num > 7)
+ input_set_abs_params(input_dev, ABS_DISTANCE, 0, 255, 0, 0);
+
+ touchscreen_parse_properties(input_dev, false, &wacom_data->prop);
+
+ dev_info(dev, "%s size X%uY%u\n", wacom_data->variant->name,
+ wacom_data->prop.max_x, wacom_data->prop.max_y);
+
+ error = device_property_read_u32(dev, "touchscreen-x-mm", &val);
+ if (!error)
+ input_abs_set_res(input_dev, ABS_X, wacom_data->prop.max_x / val);
+ error = device_property_read_u32(dev, "touchscreen-y-mm", &val);
+ if (!error)
+ input_abs_set_res(input_dev, ABS_Y, wacom_data->prop.max_y / val);
+
+ error = devm_request_threaded_irq(dev, wacom_data->irq, NULL, wacom_w9000_interrupt,
+ IRQF_ONESHOT | IRQF_NO_AUTOEN, client->name, wacom_data);
+ if (error)
+ return dev_err_probe(dev, error, "Failed to register interrupt\n");
+
+ if (wacom_data->pen_inserted_gpio) {
+ wacom_data->pen_insert_irq = gpiod_to_irq(wacom_data->pen_inserted_gpio);
+ error = devm_request_threaded_irq(dev, wacom_data->pen_insert_irq, NULL,
+ wacom_w9000_interrupt_pen_insert, IRQF_ONESHOT |
+ IRQF_NO_AUTOEN | IRQF_TRIGGER_RISING |
+ IRQF_TRIGGER_FALLING, "wacom_pen_insert",
+ wacom_data);
+ if (error)
+ return dev_err_probe(dev, error,
+ "Failed to register pen-insert interrupt\n");
+
+ input_set_capability(input_dev, EV_SW, SW_PEN_INSERTED);
+ } else {
+ wacom_data->pen_inserted = false;
+ }
+
+ error = input_register_device(wacom_data->input_dev);
+ if (error)
+ return dev_err_probe(dev, error, "Failed to register input device\n");
+
+ return 0;
+}
+
+static int wacom_w9000_suspend(struct device *dev)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct wacom_w9000_data *wacom_data = i2c_get_clientdata(client);
+
+ guard(mutex)(&wacom_data->input_dev->mutex);
+
+ if (wacom_data->pen_inserted_gpio)
+ disable_irq(wacom_data->pen_insert_irq);
+
+ if (!wacom_data->pen_inserted)
+ wacom_w9000_power_off(wacom_data);
+
+ return 0;
+}
+
+static int wacom_w9000_resume(struct device *dev)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct wacom_w9000_data *wacom_data = i2c_get_clientdata(client);
+ int error = 0;
+
+ guard(mutex)(&wacom_data->input_dev->mutex);
+
+ if (wacom_data->pen_inserted_gpio) {
+ wacom_data->pen_inserted = gpiod_get_value_cansleep(wacom_data->pen_inserted_gpio);
+
+ input_report_switch(wacom_data->input_dev, SW_PEN_INSERTED,
+ wacom_data->pen_inserted);
+ input_sync(wacom_data->input_dev);
+ }
+
+ if (!wacom_data->pen_inserted)
+ error = wacom_w9000_power_on(wacom_data);
+
+ if (wacom_data->pen_inserted_gpio)
+ enable_irq(wacom_data->pen_insert_irq);
+
+ return error;
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(wacom_w9000_pm, wacom_w9000_suspend, wacom_w9000_resume);
+
+static const struct wacom_w9000_variant w9002 = {
+ .cmd_query_num = 9,
+ .msg_coord_num = 7,
+ .name = "Wacom W9002 Digitizer",
+};
+
+static const struct wacom_w9000_variant w9007a_lt03 = {
+ .cmd_query_num = 9,
+ .msg_coord_num = 8,
+ .name = "Wacom W9007A LT03 Digitizer",
+};
+
+static const struct wacom_w9000_variant w9007a_v1 = {
+ .cmd_query_num = 9,
+ .msg_coord_num = 12,
+ .name = "Wacom W9007A V1 Digitizer",
+};
+
+static const struct of_device_id wacom_w9000_of_match[] = {
+ { .compatible = "wacom,w9002", .data = &w9002 },
+ { .compatible = "wacom,w9007a-lt03", .data = &w9007a_lt03, },
+ { .compatible = "wacom,w9007a-v1", .data = &w9007a_v1, },
+ { }
+};
+MODULE_DEVICE_TABLE(of, wacom_w9000_of_match);
+
+static const struct i2c_device_id wacom_w9000_id[] = {
+ { .name = "w9002", .driver_data = (kernel_ulong_t)&w9002 },
+ { .name = "w9007a-lt03", .driver_data = (kernel_ulong_t)&w9007a_lt03 },
+ { .name = "w9007a-v1", .driver_data = (kernel_ulong_t)&w9007a_v1 },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, wacom_w9000_id);
+
+static struct i2c_driver wacom_w9000_driver = {
+ .driver = {
+ .name = "wacom_w9000",
+ .of_match_table = wacom_w9000_of_match,
+ .pm = pm_sleep_ptr(&wacom_w9000_pm),
+ },
+ .probe = wacom_w9000_probe,
+ .id_table = wacom_w9000_id,
+};
+module_i2c_driver(wacom_w9000_driver);
+
+/* Module information */
+MODULE_AUTHOR("Hendrik Noack <hendrik-noack@gmx.de>");
+MODULE_DESCRIPTION("Wacom W9000-series penabled touchscreen driver");
+MODULE_LICENSE("GPL");
--
2.43.0
^ permalink raw reply related
* [PATCH v4 1/2] dt-bindings: Input: Add Wacom W9000-series penabled touchscreens
From: Hendrik Noack @ 2026-03-07 18:15 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Hendrik Noack, Ferass El Hafidi, linux-input, devicetree,
linux-kernel
In-Reply-To: <20260307181557.66927-1-hendrik-noack@gmx.de>
Add bindings for Wacom W9002 and two Wacom W9007 variants which can be
found in tablets.
Co-developed-by: Ferass El Hafidi <funderscore@postmarketos.org>
Signed-off-by: Ferass El Hafidi <funderscore@postmarketos.org>
Signed-off-by: Hendrik Noack <hendrik-noack@gmx.de>
---
.../input/touchscreen/wacom,w9007a-lt03.yaml | 86 +++++++++++++++++++
1 file changed, 86 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/touchscreen/wacom,w9007a-lt03.yaml
diff --git a/Documentation/devicetree/bindings/input/touchscreen/wacom,w9007a-lt03.yaml b/Documentation/devicetree/bindings/input/touchscreen/wacom,w9007a-lt03.yaml
new file mode 100644
index 000000000000..feb87f5db39d
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/wacom,w9007a-lt03.yaml
@@ -0,0 +1,86 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/touchscreen/wacom,w9007a-lt03.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Wacom W9000-series penabled I2C touchscreen
+
+maintainers:
+ - Hendrik Noack <hendrik-noack@gmx.de>
+
+description: |
+ The W9000-series are penabled touchscreen controllers by Wacom.
+
+ The firmware of chips between devices can differ and with it also
+ how the chips behaves.
+
+allOf:
+ - $ref: touchscreen.yaml#
+
+properties:
+ compatible:
+ enum:
+ - wacom,w9002
+ - wacom,w9007a-lt03
+ - wacom,w9007a-v1
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ vdd-supply:
+ description:
+ Optional regulator for the VDD digital voltage.
+
+ flash-mode-gpios:
+ maxItems: 1
+ description:
+ Optional GPIO specifier for the touchscreen's flash-mode pin.
+
+ pen-inserted-gpios:
+ maxItems: 1
+ description:
+ Optional GPIO specifier for the touchscreen's pen-insert pin.
+
+ reset-gpios:
+ maxItems: 1
+ description:
+ Optional GPIO specifier for the touchscreen's reset pin.
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - vdd-supply
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ digitizer@56 {
+ compatible = "wacom,w9007a-lt03";
+ reg = <0x56>;
+ interrupt-parent = <&gpd1>;
+ interrupts = <1 IRQ_TYPE_EDGE_RISING>;
+
+ vdd-supply = <&stylus_reg>;
+
+ flash-mode-gpios = <&gpd1 3 GPIO_ACTIVE_HIGH>;
+ pen-inserted-gpios = <&gpx0 0 GPIO_ACTIVE_LOW>;
+ reset-gpios = <&gpx0 1 GPIO_ACTIVE_LOW>;
+
+ touchscreen-x-mm = <216>;
+ touchscreen-y-mm = <135>;
+ touchscreen-inverted-x;
+ };
+ };
--
2.43.0
^ permalink raw reply related
* [PATCH v4 0/2] Add support for Wacom W9000-series penabled touchscreens
From: Hendrik Noack @ 2026-03-07 18:15 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Hendrik Noack, Ferass El Hafidi, linux-input, devicetree,
linux-kernel
Add devicetree bindings and a driver for the Wacom W9000-series penabled
touchscreens.
The driver currently only contains the information for the W9002 and
W9007A, which I or Ferass could test on devices. It should also work with
other chips, such as W9001 or W9010. However, I couldn't test it on these
and the message length would need to be added.
The pen-inserted-gpios is used to get if the pen is inserted in the device
or not. It's also used as an interrupt so that the power state of the chip
itself can be controlled depending on a change of the insertion state of
the pen.
Signed-off-by: Hendrik Noack <hendrik-noack@gmx.de>
---
Changes in v2:
- remove pdct-gpios, as it's unnecessary
- fix devicetree example
- adopt to kernel coding style
---
Changes in v3:
- fix missing include (thanks lkp@intel.com)
---
Changes in v4:
- adopt to feedback (thanks dmitry.torokhov@gmail.com)
- add W9002 support (thanks funderscore@postmarketos.org)
- add reset-gpios, necessary for some chips
---
Hendrik Noack (2):
dt-bindings: Input: Add Wacom W9000-series penabled touchscreens
Input: Add support for Wacom W9000-series penabled touchscreens
.../input/touchscreen/wacom,w9007a-lt03.yaml | 86 +++
drivers/input/touchscreen/Kconfig | 12 +
drivers/input/touchscreen/Makefile | 1 +
drivers/input/touchscreen/wacom_w9000.c | 510 ++++++++++++++++++
4 files changed, 609 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/touchscreen/wacom,w9007a-lt03.yaml
create mode 100644 drivers/input/touchscreen/wacom_w9000.c
--
2.43.0
^ permalink raw reply
* [hid:for-7.1/lenovo 10/16] drivers/hid/hid-lenovo-go-s.c:164:73: sparse: sparse: Using plain integer as NULL pointer
From: kernel test robot @ 2026-03-07 15:05 UTC (permalink / raw)
To: Derek J. Clark
Cc: oe-kbuild-all, linux-input, Jiri Kosina, Mark Pearson,
Mario Limonciello
tree: https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-7.1/lenovo
head: d2c424e80caf8237bda4c94bc2e25398967243f9
commit: 4325fdab5dbbfd467df6797e018c9dd0e5c3ae75 [10/16] HID: hid-lenovo-go-s: Add Lenovo Legion Go S Series HID Driver
config: arc-randconfig-r113-20260307 (https://download.01.org/0day-ci/archive/20260307/202603072210.hUL7ptt2-lkp@intel.com/config)
compiler: arc-linux-gcc (GCC) 13.4.0
sparse: v0.6.5-rc1
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260307/202603072210.hUL7ptt2-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/202603072210.hUL7ptt2-lkp@intel.com/
sparse warnings: (new ones prefixed by >>)
>> drivers/hid/hid-lenovo-go-s.c:164:73: sparse: sparse: Using plain integer as NULL pointer
vim +164 drivers/hid/hid-lenovo-go-s.c
159
160 static void cfg_setup(struct work_struct *work)
161 {
162 int ret;
163
> 164 ret = mcu_property_out(drvdata.hdev, GET_VERSION, FEATURE_NONE, 0, 0);
165 if (ret) {
166 dev_err(&drvdata.hdev->dev, "Failed to retrieve MCU Version: %i\n", ret);
167 return;
168 }
169 }
170
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v2 0/7] Add support for mt6392 PMIC
From: Krzysztof Kozlowski @ 2026-03-07 15:04 UTC (permalink / raw)
To: Luca Leonardo Scorcia
Cc: linux-mediatek, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Sen Chu, Sean Wang, Macpaul Lin, Lee Jones,
Matthias Brugger, AngeloGioacchino Del Regno, Liam Girdwood,
Mark Brown, Val Packett, Gary Bisson, Louis-Alexis Eyraud,
Julien Massot, Fabien Parent, Chen Zhong, linux-input, devicetree,
linux-kernel, linux-pm, linux-arm-kernel
In-Reply-To: <20260306120521.163654-1-l.scorcia@gmail.com>
On Fri, Mar 06, 2026 at 12:03:04PM +0000, Luca Leonardo Scorcia wrote:
> The MediaTek mt6392 PMIC is usually found on devices powered by
> the mt8516/mt8167 SoC, and is yet another mt6397 variant.
>
> This series is mostly based around patches submitted a couple
> years ago by Fabien Parent and not merged and from Val Packett's
> submission from Jan 2025 that included extra cleanups, fixes, and a
> new dtsi file similar to ones that exist for other PMICs. Some
> comments weren't addressed and the series was ultimately not merged.
>
> This series only enables three functions: regulators, keys, and RTC.
>
> I have added a handful of device tree improvements to fix some
> dtbs_check errors and addressed the comments from last year's
> reviews. The series has been tested on Xiaomi Mi Smart Clock x04g.
>
> v2: Review feedback - replaced explicit compatibles with fallbacks
>
> Fabien Parent (5):
> dt-bindings: mfd: mt6397: Add bindings for MT6392 PMIC
> dt-bindings: regulator: add support for MT6392
This is incomplete - where is the actual schema for the regulators?
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v2 2/7] dt-bindings: regulator: add support for MT6392
From: Krzysztof Kozlowski @ 2026-03-07 15:03 UTC (permalink / raw)
To: Luca Leonardo Scorcia
Cc: linux-mediatek, Fabien Parent, Val Packett, Rob Herring,
Dmitry Torokhov, Krzysztof Kozlowski, Conor Dooley, Sen Chu,
Sean Wang, Macpaul Lin, Lee Jones, Matthias Brugger,
AngeloGioacchino Del Regno, Liam Girdwood, Mark Brown,
Gary Bisson, Julien Massot, Louis-Alexis Eyraud, Chen Zhong,
linux-input, devicetree, linux-kernel, linux-pm, linux-arm-kernel
In-Reply-To: <20260306120521.163654-3-l.scorcia@gmail.com>
On Fri, Mar 06, 2026 at 12:03:06PM +0000, Luca Leonardo Scorcia wrote:
> From: Fabien Parent <parent.f@gmail.com>
>
> Add binding documentation of the regulator for MT6392 SoCs.
>
> Signed-off-by: Fabien Parent <parent.f@gmail.com>
> Signed-off-by: Val Packett <val@packett.cool>
> Signed-off-by: Luca Leonardo Scorcia <l.scorcia@gmail.com>
> Acked-by: Rob Herring (Arm) <robh@kernel.org>
> ---
> Documentation/devicetree/bindings/mfd/mediatek,mt6397.yaml | 1 +
> 1 file changed, 1 insertion(+)
That's a mfd patch, so should be squashed to the patch changing mfd.
Alone is incomplete thus not really correct.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v2 1/7] dt-bindings: mfd: mt6397: Add bindings for MT6392 PMIC
From: Krzysztof Kozlowski @ 2026-03-07 15:02 UTC (permalink / raw)
To: Luca Leonardo Scorcia
Cc: linux-mediatek, Fabien Parent, Val Packett, Dmitry Torokhov,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Sen Chu,
Sean Wang, Macpaul Lin, Lee Jones, Matthias Brugger,
AngeloGioacchino Del Regno, Liam Girdwood, Mark Brown,
Gary Bisson, Julien Massot, Louis-Alexis Eyraud, Chen Zhong,
linux-input, devicetree, linux-kernel, linux-pm, linux-arm-kernel
In-Reply-To: <20260306120521.163654-2-l.scorcia@gmail.com>
On Fri, Mar 06, 2026 at 12:03:05PM +0000, Luca Leonardo Scorcia wrote:
> From: Fabien Parent <parent.f@gmail.com>
>
> Add the currently supported bindings for the MT6392 PMIC.
>
> Signed-off-by: Fabien Parent <parent.f@gmail.com>
> Signed-off-by: Val Packett <val@packett.cool>
> Signed-off-by: Luca Leonardo Scorcia <l.scorcia@gmail.com>
> ---
> .../devicetree/bindings/mfd/mediatek,mt6397.yaml | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mfd/mediatek,mt6397.yaml b/Documentation/devicetree/bindings/mfd/mediatek,mt6397.yaml
> index 6a89b479d10f..c358b2f8059c 100644
> --- a/Documentation/devicetree/bindings/mfd/mediatek,mt6397.yaml
> +++ b/Documentation/devicetree/bindings/mfd/mediatek,mt6397.yaml
> @@ -40,6 +40,10 @@ properties:
> - mediatek,mt6358
> - mediatek,mt6359
> - mediatek,mt6397
> + - items:
> + - enum:
> + - mediatek,mt6392
> + - const: mediatek,mt6323
> - items:
> - enum:
> - mediatek,mt6366
> @@ -72,6 +76,10 @@ properties:
> - enum:
> - mediatek,mt6366-rtc
> - const: mediatek,mt6358-rtc
> + - items:
> + - enum:
> + - mediatek,mt6392-rtc
> + - const: mediatek,mt6397-rtc
Why not mt6323-rtc?
Mixing different fallbacks should be briefly explained in the commit
msg.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 1/2] iio: hid-sensor-gyro-3d: move iio_device_register() to end of probe()
From: Jonathan Cameron @ 2026-03-07 14:21 UTC (permalink / raw)
To: Bhargav Joshi
Cc: jikos, srinivas.pandruvada, dlechner, nuno.sa, andy, linux-iio,
linux-kernel, linux-input
In-Reply-To: <CAPC4XCRom4ri1QKw_mTnZGc9np5XFD1LskQnd=ssDAKBdOkGmQ@mail.gmail.com>
On Mon, 2 Mar 2026 01:00:06 +0530
Bhargav Joshi <rougueprince47@gmail.com> wrote:
> On Sun, Mar 1, 2026 at 5:15 PM Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > On Sun, 1 Mar 2026 00:43:59 +0530
> > Bhargav Joshi <rougueprince47@gmail.com> wrote:
> >
> > > Currently, calling iio_device_register() before
> > > sensor_hub_register_callback() may create a race condition where the
> > > device is exposed to userspace before callbacks are wired.
> >
> > We needs some more here on 'why' this is a problem.
> > Whilst this is an unusual arrangement, I couldn't immediately find
> > anything that was actually broken. It's possible data will turn up
> > after we tear down the callbacks and before the userspace interfaces
> > are removed, but I think that just results in dropping data. Given it's
> > in a race anyway I don't think we care about that. The callbacks
> > don't seem to be involved in device configuration which can go on whether
> > or not the callbacks are registered. Maybe there is something that
> > won't get acknowledged if they aren't in place in time?
> >
> > For a fix like this we'd normally want to see a clear flow that
> > leads to a bug.
> >
> > Also a fixes tag so we know how far to backport.
> >
>
> Hi Jonathan,
>
> I originally submitted the patch for the driver because calling
> iio_device_register() before the callbacks are wired up violates
> standard IIO LIFO ordering. I assumed this created a dangerous race
> condition with userspace. However, as you correctly pointed out, the
> HID sensor hub safely drops those early events without crashing, even
> if it is unusual behaviour still won't have a hard crash.
>
> My underlying goal was simply a structural cleanup to align the
> probe() and remove() sequences with standard IIO architecture.
>
> Would you like me to send a v2 of this patch with a revised commit
> message classifying it purely as a structural cleanup (and without a
> Fixes tag), or is the current driver behavior acceptable enough that I
> should just drop this patch entirely?
My gut is leave this one alone. It's a rather unusual driver in lots
of ways, so I don't really mind one more ;)
Lets see if anyone else has strong views one way or the other.
Srinivas?
>
> Thanks,
> Bhargav
>
> > >
> > > Move iio_device_register() to the end of the probe() function to prevent
> > > race condition.
> > >
> > > Consequently, update the error handling path in probe() and in remove()
> > > ensuring that iio_device_unregister() is called first to cut off
> > > userspace access before the hardware callbacks are removed.
> > >
> > > Signed-off-by: Bhargav Joshi <rougueprince47@gmail.com>
> > > ---
> > > drivers/iio/gyro/hid-sensor-gyro-3d.c | 20 ++++++++++----------
> > > 1 file changed, 10 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/drivers/iio/gyro/hid-sensor-gyro-3d.c b/drivers/iio/gyro/hid-sensor-gyro-3d.c
> > > index c43990c518f7..8e3628cd8529 100644
> > > --- a/drivers/iio/gyro/hid-sensor-gyro-3d.c
> > > +++ b/drivers/iio/gyro/hid-sensor-gyro-3d.c
> > > @@ -333,12 +333,6 @@ static int hid_gyro_3d_probe(struct platform_device *pdev)
> > > return ret;
> > > }
> > >
> > > - ret = iio_device_register(indio_dev);
> > > - if (ret) {
> > > - dev_err(&pdev->dev, "device register failed\n");
> > > - goto error_remove_trigger;
> > > - }
> > > -
> > > gyro_state->callbacks.send_event = gyro_3d_proc_event;
> > > gyro_state->callbacks.capture_sample = gyro_3d_capture_sample;
> > > gyro_state->callbacks.pdev = pdev;
> > > @@ -346,13 +340,19 @@ static int hid_gyro_3d_probe(struct platform_device *pdev)
> > > &gyro_state->callbacks);
> > > if (ret < 0) {
> > > dev_err(&pdev->dev, "callback reg failed\n");
> > > - goto error_iio_unreg;
> > > + goto error_remove_trigger;
> > > + }
> > > +
> > > + ret = iio_device_register(indio_dev);
> > > + if (ret) {
> > > + dev_err(&pdev->dev, "device register failed\n");
> > > + goto error_remove_callback;
> > > }
> > >
> > > return ret;
> > >
> > > -error_iio_unreg:
> > > - iio_device_unregister(indio_dev);
> > > +error_remove_callback:
> > > + sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_GYRO_3D);
> > > error_remove_trigger:
> > > hid_sensor_remove_trigger(indio_dev, &gyro_state->common_attributes);
> > > return ret;
> > > @@ -365,8 +365,8 @@ static void hid_gyro_3d_remove(struct platform_device *pdev)
> > > struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> > > struct gyro_3d_state *gyro_state = iio_priv(indio_dev);
> > >
> > > - sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_GYRO_3D);
> > > iio_device_unregister(indio_dev);
> > > + sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_GYRO_3D);
> > > hid_sensor_remove_trigger(indio_dev, &gyro_state->common_attributes);
> > > }
> > >
> >
^ permalink raw reply
* [PATCH v2] HID: sony: add battery status support for Rock Band 4 PS5 guitars
From: Rosalie Wanders @ 2026-03-07 9:48 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Rosalie Wanders, linux-input, linux-kernel
This commit adds battery status support for Rock Band 4 PS5 guitars.
The data is reported in the same way as the dualsense in hid-playstation
except it's located at byte 30.
Signed-off-by: Rosalie Wanders <rosalie@mailbox.org>
---
drivers/hid/hid-sony.c | 40 +++++++++++++++++++++++++++++++++++++++-
1 file changed, 39 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index a89af14e4acc..5a1d41cb3a72 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -72,7 +72,8 @@
NAVIGATION_CONTROLLER_BT)
#define SONY_LED_SUPPORT (SIXAXIS_CONTROLLER | BUZZ_CONTROLLER |\
MOTION_CONTROLLER | NAVIGATION_CONTROLLER)
-#define SONY_BATTERY_SUPPORT (SIXAXIS_CONTROLLER | MOTION_CONTROLLER_BT | NAVIGATION_CONTROLLER)
+#define SONY_BATTERY_SUPPORT (SIXAXIS_CONTROLLER | MOTION_CONTROLLER_BT | NAVIGATION_CONTROLLER |\
+ RB4_GUITAR_PS5)
#define SONY_FF_SUPPORT (SIXAXIS_CONTROLLER | MOTION_CONTROLLER)
#define SONY_BT_DEVICE (SIXAXIS_CONTROLLER_BT | MOTION_CONTROLLER_BT | NAVIGATION_CONTROLLER_BT)
#define NSG_MRXU_REMOTE (NSG_MR5U_REMOTE_BT | NSG_MR7U_REMOTE_BT)
@@ -991,6 +992,12 @@ static void rb4_ps4_guitar_parse_report(struct sony_sc *sc, u8 *rd, int size)
static void rb4_ps5_guitar_parse_report(struct sony_sc *sc, u8 *rd, int size)
{
+ u8 charging_status;
+ u8 battery_data;
+ u8 battery_capacity;
+ u8 battery_status;
+ unsigned long flags;
+
/*
* Rock Band 4 PS5 guitars have whammy and
* tilt functionality, they're located at
@@ -1003,6 +1010,37 @@ static void rb4_ps5_guitar_parse_report(struct sony_sc *sc, u8 *rd, int size)
input_report_abs(sc->input_dev, ABS_Z, rd[41]);
input_report_abs(sc->input_dev, ABS_RZ, rd[42]);
+ /*
+ * Rock Band 4 PS5 guitars also report the
+ * battery status and level at byte 30.
+ */
+ charging_status = (rd[30] >> 4) & 0x0F;
+ battery_data = rd[30] & 0x0F;
+
+ switch (charging_status) {
+ case 0x0:
+ battery_capacity = min(battery_data * 10 + 5, 100);
+ battery_status = POWER_SUPPLY_STATUS_DISCHARGING;
+ break;
+ case 0x1:
+ battery_capacity = min(battery_data * 10 + 5, 100);
+ battery_status = POWER_SUPPLY_STATUS_CHARGING;
+ break;
+ case 0x2:
+ battery_capacity = 100;
+ battery_status = POWER_SUPPLY_STATUS_FULL;
+ break;
+ default:
+ battery_capacity = 0;
+ battery_status = POWER_SUPPLY_STATUS_UNKNOWN;
+ break;
+ }
+
+ spin_lock_irqsave(&sc->lock, flags);
+ sc->battery_capacity = battery_capacity;
+ sc->battery_status = battery_status;
+ spin_unlock_irqrestore(&sc->lock, flags);
+
input_sync(sc->input_dev);
}
--
2.53.0
^ permalink raw reply related
* [PATCH] Input: atmel_mxt_ts: Allow per-board device config
From: Hendrik Noack @ 2026-03-07 9:26 UTC (permalink / raw)
To: Nick Dyer, Dmitry Torokhov; +Cc: Hendrik Noack, linux-input, linux-kernel
The device config can be device dependent.
Rewrite the code to be able to get a per-board suffixed device config as
well. If this does not exist, fall back to the standard device config.
Signed-off-by: Hendrik Noack <hendrik-noack@gmx.de>
---
drivers/input/touchscreen/atmel_mxt_ts.c | 64 ++++++++++++++++++++++--
1 file changed, 59 insertions(+), 5 deletions(-)
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index dd0544cc1bc1..0d3921482d9f 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -36,7 +36,9 @@
/* Firmware files */
#define MXT_FW_NAME "maxtouch.fw"
-#define MXT_CFG_NAME "maxtouch.cfg"
+#define MXT_CFG_FOLDER "atmel"
+#define MXT_CFG_NAME "maxtouch"
+#define MXT_CFG_EXTENSION ".cfg"
#define MXT_CFG_MAGIC "OBP_RAW V1"
/* Registers */
@@ -2248,6 +2250,60 @@ static void mxt_config_cb(const struct firmware *cfg, void *ctx)
release_firmware(cfg);
}
+static void mxt_board_config_cb(const struct firmware *cfg, void *ctx)
+{
+ if (cfg) {
+ mxt_config_cb(cfg, ctx);
+ } else {
+ struct mxt_data *data = ctx;
+ char *fw_name;
+
+ fw_name = kasprintf(GFP_KERNEL, "%s%s", MXT_CFG_NAME, MXT_CFG_EXTENSION);
+ if (!fw_name)
+ return;
+
+ request_firmware_nowait(THIS_MODULE, true, fw_name, &data->client->dev, GFP_KERNEL,
+ data, mxt_config_cb);
+ }
+}
+
+static int mxt_invoke_config_loader(struct mxt_data *data)
+{
+ struct device_node *root;
+ char *board_type = NULL;
+ char *fw_name;
+ void (*cb)(const struct firmware *fw, void *context);
+
+ root = of_find_node_by_path("/");
+ if (root) {
+ const char *tmp;
+
+ if (!of_property_read_string_index(root, "compatible", 0, &tmp)) {
+ board_type = kstrdup(tmp, GFP_KERNEL);
+
+ /* get rid of '/' in the compatible string to be able to find the FW */
+ if (board_type)
+ strreplace(board_type, '/', '-');
+ }
+ of_node_put(root);
+ }
+
+ if (board_type) {
+ fw_name = kasprintf(GFP_KERNEL, "%s/%s.%s%s", MXT_CFG_FOLDER, MXT_CFG_NAME,
+ board_type, MXT_CFG_EXTENSION);
+ cb = mxt_board_config_cb;
+ kfree(board_type);
+ } else {
+ fw_name = kasprintf(GFP_KERNEL, "%s%s", MXT_CFG_NAME, MXT_CFG_EXTENSION);
+ cb = mxt_config_cb;
+ }
+ if (!fw_name)
+ return -ENOMEM;
+
+ return request_firmware_nowait(THIS_MODULE, true, fw_name, &data->client->dev, GFP_KERNEL,
+ data, cb);
+}
+
static int mxt_initialize(struct mxt_data *data)
{
struct i2c_client *client = data->client;
@@ -2294,11 +2350,9 @@ static int mxt_initialize(struct mxt_data *data)
if (error)
return error;
- error = request_firmware_nowait(THIS_MODULE, true, MXT_CFG_NAME,
- &client->dev, GFP_KERNEL, data,
- mxt_config_cb);
+ error = mxt_invoke_config_loader(data);
if (error) {
- dev_err(&client->dev, "Failed to invoke firmware loader: %d\n",
+ dev_err(&client->dev, "Failed to invoke config loader: %d\n",
error);
return error;
}
--
2.43.0
^ permalink raw reply related
* Re: [PATCH 09/12] dt-bindings: input: Document hid-over-spi DT schema
From: Val Packett @ 2026-03-07 7:25 UTC (permalink / raw)
To: Jingyuan Liang, Jiri Kosina, Benjamin Tissoires, Jonathan Corbet,
Mark Brown, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, linux-doc, linux-kernel, linux-spi,
linux-trace-kernel, devicetree, hbarnor, Dmitry Antipov,
Jarrett Schultz
In-Reply-To: <20260303-send-upstream-v1-9-1515ba218f3d@chromium.org>
On 3/3/26 3:13 AM, Jingyuan Liang wrote:
> Documentation describes the required and optional properties for
> implementing Device Tree for a Microsoft G6 Touch Digitizer that
> supports HID over SPI Protocol 1.0 specification.
> […]
> +properties:
> + compatible:
> + oneOf:
> + - items:
> + - enum:
> + - microsoft,g6-touch-digitizer
> + - const: hid-over-spi
> + - description: Just "hid-over-spi" alone is allowed, but not recommended.
> […]
> +required:
> + - compatible
> + - interrupts
> + - reset-gpios
Why is reset required? Is it so implausible on some device implementing
the spec there wouldn't be a reset gpio?
> + - vdd-supply
Linux makes up a dummy regulator if DT doesn't provide one, so can
regulators even be required?
> […]
> + compatible = "hid-over-spi";
Not following your own recommendation from above :)
> + reg = <0x0>;
> + interrupts-extended = <&gpio 42 IRQ_TYPE_EDGE_FALLING>;
> + reset-gpios = <&gpio 27 GPIO_ACTIVE_LOW>;
> + vdd-supply = <&pm8350c_l3>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&ts_d6_reset_assert &ts_d6_int_bias>;
Heh, "reset_assert" is a name implying it would actually set the value
from the pinctrl properties, which is what had to be done before
reset-gpios were supported. But now reset-gpios are supported.
Thanks,
~val
P.S. happy to see work on this happen again!
^ permalink raw reply
* [PATCH v3] HID: winwing: Enable rumble effects
From: Ivan Gorinov @ 2026-03-07 5:22 UTC (permalink / raw)
To: Jiri Kosina; +Cc: linux-input, linux-kernel
Enable rumble motor control on TGRIP-15E and TGRIP-15EX throttle grips
by sending haptic feedback commands (EV_FF events) to the input device.
Signed-off-by: Ivan Gorinov <linux-kernel@altimeter.info>
---
Changes since v2:
- Add comments about USB requests for LED and rumble control
Changes since v1:
- Fix possible NULL pointer dereference
---
drivers/hid/hid-winwing.c | 196 +++++++++++++++++++++++++++++++++++---
1 file changed, 182 insertions(+), 14 deletions(-)
diff --git a/drivers/hid/hid-winwing.c b/drivers/hid/hid-winwing.c
index ab65dc12d1e0..9cd25a77999e 100644
--- a/drivers/hid/hid-winwing.c
+++ b/drivers/hid/hid-winwing.c
@@ -12,6 +12,7 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/mutex.h>
+#include <linux/workqueue.h>
#define MAX_REPORT 16
@@ -35,10 +36,14 @@ static const struct winwing_led_info led_info[3] = {
struct winwing_drv_data {
struct hid_device *hdev;
- __u8 *report_buf;
- struct mutex lock;
- int map_more_buttons;
- unsigned int num_leds;
+ struct mutex lights_lock;
+ __u8 *report_lights;
+ __u8 *report_rumble;
+ struct work_struct rumble_work;
+ struct ff_rumble_effect rumble;
+ int rumble_left;
+ int rumble_right;
+ int has_grip15;
struct winwing_led leds[];
};
@@ -47,11 +52,15 @@ static int winwing_led_write(struct led_classdev *cdev,
{
struct winwing_led *led = (struct winwing_led *) cdev;
struct winwing_drv_data *data = hid_get_drvdata(led->hdev);
- __u8 *buf = data->report_buf;
+ __u8 *buf = data->report_lights;
int ret;
- mutex_lock(&data->lock);
+ mutex_lock(&data->lights_lock);
+ /*
+ * Mimicking requests captured by usbmon when LEDs
+ * are controlled by the vendor's app in a VM.
+ */
buf[0] = 0x02;
buf[1] = 0x60;
buf[2] = 0xbe;
@@ -69,7 +78,7 @@ static int winwing_led_write(struct led_classdev *cdev,
ret = hid_hw_output_report(led->hdev, buf, 14);
- mutex_unlock(&data->lock);
+ mutex_unlock(&data->lights_lock);
return ret;
}
@@ -87,9 +96,9 @@ static int winwing_init_led(struct hid_device *hdev,
if (!data)
return -EINVAL;
- data->report_buf = devm_kmalloc(&hdev->dev, MAX_REPORT, GFP_KERNEL);
+ data->report_lights = devm_kzalloc(&hdev->dev, MAX_REPORT, GFP_KERNEL);
- if (!data->report_buf)
+ if (!data->report_lights)
return -ENOMEM;
for (i = 0; i < 3; i += 1) {
@@ -117,7 +126,7 @@ static int winwing_init_led(struct hid_device *hdev,
return ret;
}
-static int winwing_map_button(int button, int map_more_buttons)
+static int winwing_map_button(int button, int has_grip15)
{
if (button < 1)
return KEY_RESERVED;
@@ -141,7 +150,7 @@ static int winwing_map_button(int button, int map_more_buttons)
return (button - 65) + BTN_TRIGGER_HAPPY17;
}
- if (!map_more_buttons) {
+ if (!has_grip15) {
/*
* Not mapping numbers [33 .. 64] which
* are not assigned to any real buttons
@@ -194,13 +203,149 @@ static int winwing_input_mapping(struct hid_device *hdev,
/* Button numbers start with 1 */
button = usage->hid & HID_USAGE;
- code = winwing_map_button(button, data->map_more_buttons);
+ code = winwing_map_button(button, data->has_grip15);
hid_map_usage(hi, usage, bit, max, EV_KEY, code);
return 1;
}
+/*
+ * If x ≤ 0, return 0;
+ * if x is in [1 .. 65535], return a value in [1 .. 255]
+ */
+static inline int convert_magnitude(int x)
+{
+ if (x < 1)
+ return 0;
+
+ return ((x * 255) >> 16) + 1;
+}
+
+static int winwing_haptic_rumble(struct winwing_drv_data *data)
+{
+ __u8 *buf;
+ __u8 m;
+
+ if (!data)
+ return -EINVAL;
+
+ if (!data->hdev)
+ return -EINVAL;
+
+ buf = data->report_rumble;
+
+ if (!buf)
+ return -EINVAL;
+
+ m = convert_magnitude(data->rumble.strong_magnitude);
+ if (m != data->rumble_left) {
+ int ret;
+
+ /*
+ * Mimicking requests captured by usbmon when rumble
+ * is activated by the vendor's app in a VM.
+ */
+ buf[0] = 0x02;
+ buf[1] = 0x01;
+ buf[2] = 0xbf;
+ buf[3] = 0x00;
+ buf[4] = 0x00;
+ buf[5] = 0x03;
+ buf[6] = 0x49;
+ buf[7] = 0x00;
+ buf[8] = m;
+ buf[9] = 0x00;
+ buf[10] = 0;
+ buf[11] = 0;
+ buf[12] = 0;
+ buf[13] = 0;
+
+ ret = hid_hw_output_report(data->hdev, buf, 14);
+ if (ret < 0) {
+ hid_err(data->hdev, "error %d (%*ph)\n", ret, 14, buf);
+ return ret;
+ }
+ data->rumble_left = m;
+ }
+
+ m = convert_magnitude(data->rumble.weak_magnitude);
+ if (m != data->rumble_right) {
+ int ret;
+
+ /*
+ * Mimicking requests captured by usbmon when rumble
+ * is activated by the vendor's app in a VM.
+ */
+ buf[0] = 0x02;
+ buf[1] = 0x03;
+ buf[2] = 0xbf;
+ buf[3] = 0x00;
+ buf[4] = 0x00;
+ buf[5] = 0x03;
+ buf[6] = 0x49;
+ buf[7] = 0x00;
+ buf[8] = m;
+ buf[9] = 0x00;
+ buf[10] = 0;
+ buf[11] = 0;
+ buf[12] = 0;
+ buf[13] = 0;
+
+ ret = hid_hw_output_report(data->hdev, buf, 14);
+ if (ret < 0) {
+ hid_err(data->hdev, "error %d (%*ph)\n", ret, 14, buf);
+ return ret;
+ }
+ data->rumble_right = m;
+ }
+
+ return 0;
+}
+
+
+static void winwing_haptic_rumble_cb(struct work_struct *work)
+{
+ struct winwing_drv_data *data;
+
+ data = container_of(work, struct winwing_drv_data, rumble_work);
+ winwing_haptic_rumble(data);
+}
+
+static int winwing_play_effect(struct input_dev *dev, void *context,
+ struct ff_effect *effect)
+{
+ struct winwing_drv_data *data = (struct winwing_drv_data *) context;
+
+ if (effect->type != FF_RUMBLE)
+ return 0;
+
+ if (!data)
+ return -EINVAL;
+
+ data->rumble = effect->u.rumble;
+
+ return schedule_work(&data->rumble_work);
+}
+
+static int winwing_init_ff(struct hid_device *hdev, struct hid_input *hidinput)
+{
+ struct winwing_drv_data *data;
+
+ data = (struct winwing_drv_data *) hid_get_drvdata(hdev);
+ if (!data)
+ return -EINVAL;
+
+ data->report_rumble = devm_kzalloc(&hdev->dev, MAX_REPORT, GFP_KERNEL);
+ data->rumble_left = -1;
+ data->rumble_right = -1;
+
+ input_set_capability(hidinput->input, EV_FF, FF_RUMBLE);
+
+ return input_ff_create_memless(hidinput->input, data,
+ winwing_play_effect);
+}
+
static int winwing_probe(struct hid_device *hdev,
const struct hid_device_id *id)
{
@@ -219,10 +364,12 @@ static int winwing_probe(struct hid_device *hdev,
if (!data)
return -ENOMEM;
- data->map_more_buttons = id->driver_data;
-
+ data->hdev = hdev;
+ data->has_grip15 = id->driver_data;
hid_set_drvdata(hdev, data);
+ INIT_WORK(&data->rumble_work, winwing_haptic_rumble_cb);
+
ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
if (ret) {
hid_err(hdev, "hw start failed\n");
@@ -232,19 +379,39 @@ static int winwing_probe(struct hid_device *hdev,
return 0;
}
+static void winwing_remove(struct hid_device *hdev)
+{
+ struct winwing_drv_data *data;
+
+ data = (struct winwing_drv_data *) hid_get_drvdata(hdev);
+
+ if (data)
+ cancel_work_sync(&data->rumble_work);
+
+ hid_hw_close(hdev);
+ hid_hw_stop(hdev);
+}
+
static int winwing_input_configured(struct hid_device *hdev,
struct hid_input *hidinput)
{
+ struct winwing_drv_data *data;
int ret;
+ data = (struct winwing_drv_data *) hid_get_drvdata(hdev);
+
ret = winwing_init_led(hdev, hidinput->input);
if (ret)
hid_err(hdev, "led init failed\n");
+ if (data->has_grip15)
+ winwing_init_ff(hdev, hidinput);
+
return ret;
}
+/* Set driver_data to 1 for grips with rumble motor and more than 32 buttons */
static const struct hid_device_id winwing_devices[] = {
{ HID_USB_DEVICE(0x4098, 0xbd65), .driver_data = 1 }, /* TGRIP-15E */
{ HID_USB_DEVICE(0x4098, 0xbd64), .driver_data = 1 }, /* TGRIP-15EX */
@@ -261,6 +428,7 @@ static struct hid_driver winwing_driver = {
.input_configured = winwing_input_configured,
.input_mapping = winwing_input_mapping,
.probe = winwing_probe,
+ .remove = winwing_remove,
};
module_hid_driver(winwing_driver);
--
2.43.0
^ permalink raw reply related
* [PATCH] HID: core: use flex array allocation
From: Rosen Penev @ 2026-03-07 3:29 UTC (permalink / raw)
To: linux-input
Cc: Jiri Kosina, linux-hardening, gustavoars, Benjamin Tissoires,
open list
Instead of embedding a pointer in the struct, use a flexible array
member to avoid the + 1 trick to point to allocation after the struct.
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
drivers/hid/hid-core.c | 6 ++----
include/linux/hid.h | 2 +-
2 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 840a60113868..6bd6cbf26c6d 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -127,15 +127,13 @@ static struct hid_field *hid_register_field(struct hid_report *report, unsigned
return NULL;
}
- field = kvzalloc((sizeof(struct hid_field) +
- usages * sizeof(struct hid_usage) +
- 3 * usages * sizeof(unsigned int)), GFP_KERNEL);
+ field = kvzalloc(struct_size(field, usage, usages) +
+ 3 * usages * sizeof(unsigned int), GFP_KERNEL);
if (!field)
return NULL;
field->index = report->maxfield++;
report->field[field->index] = field;
- field->usage = (struct hid_usage *)(field + 1);
field->value = (s32 *)(field->usage + usages);
field->new_value = (s32 *)(field->value + usages);
field->usages_priorities = (s32 *)(field->new_value + usages);
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 2990b9f94cb5..e2c3e2528582 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -524,7 +524,6 @@ struct hid_field {
unsigned physical; /* physical usage for this field */
unsigned logical; /* logical usage for this field */
unsigned application; /* application usage for this field */
- struct hid_usage *usage; /* usage table for this function */
unsigned maxusage; /* maximum usage index */
unsigned flags; /* main-item flags (i.e. volatile,array,constant) */
unsigned report_offset; /* bit offset in the report */
@@ -549,6 +548,7 @@ struct hid_field {
struct hid_input *hidinput; /* associated input structure */
__u16 dpad; /* dpad input code */
unsigned int slot_idx; /* slot index in a report */
+ struct hid_usage usage[]; /* usage table for this function */
};
#define HID_MAX_FIELDS 256
--
2.53.0
^ permalink raw reply related
* Re: [PATCH v3 0/3] Input: st1232 - add system wakeup support
From: phucduc.bui @ 2026-03-07 2:53 UTC (permalink / raw)
To: wsa+renesas
Cc: conor+dt, devicetree, dmitry.torokhov, geert+renesas, hechtb,
javier.carrasco, jeff, krzk+dt, linux-input, linux-kernel,
linux-renesas-soc, magnus.damm, phucduc.bui, robh
In-Reply-To: <aaq_Rft0gvVqxmMD@shikoro>
Hi Wolfram,
> Krzysztof already adviced you to not attach new series to old threads.
> Please follow this suggestion:
>
> Do not attach (thread) your patchsets to some other threads (unrelated
> or older versions). This buries them deep in the mailbox and might
> interfere with applying entire sets. See also:
>
> https://elixir.bootlin.com/linux/v6.16-rc2/source/Documentation/process/submitting-patches.rst#L830
>
You are right, and I apologize for the duplication of the mistake.
I missed Krzysztof's earlier reply while I was preparing v3, which led to
this incorrect threading again. I have already replied to Krzysztof's
thread to acknowledge the error.
I will follow the proper process by starting a fresh, un-threaded series
for v4.
Thank you for the reminder.
Best regards,
Phuc
^ permalink raw reply
* Re: [PATCH v2 3/3] input: touchscreen: st1232: add system wakeup support
From: phucduc.bui @ 2026-03-07 2:50 UTC (permalink / raw)
To: krzk
Cc: conor+dt, devicetree, dmitry.torokhov, geert+renesas, hechtb,
javier.carrasco, jeff, krzk+dt, linux-input, linux-kernel,
linux-renesas-soc, magnus.damm, phucduc.bui, robh, wsa+renesas
In-Reply-To: <ff7a9a31-2dfb-4588-83bd-1a3aa7809972@kernel.org>
Hi Krzysztof,
> > + dev_info(dev, "st1232: suspend called\n");
> > + dev_info(dev, "st1232: irq=%d wakeup=%d\n", client->irq,
> device_may_wakeup(dev));
>
> No, there is no need to add success messages.
>
> >
> > - if (!device_may_wakeup(&client->dev))
> > + if (device_may_wakeup(dev)) {
> > + ret = enable_irq_wake(client->irq);
> > + dev_info(dev, "st1232: Supend use wakeup\n");
> > + dev_info(dev, "enable_irq_wake ret=%d\n", ret);
>
> Drop both
>
>
> > + } else {
> > + dev_info(dev, "st1232: Suspend Don't use wakeup\n");
>
> Drop
My apologies. You are absolutely right. I realized these debug messages
were unnecessary and already removed them in the v3 I sent (though I
unfortunately messed up the threading for that version).
I will ensure they stay removed in v4, which will be sent as a fresh
thread.
Best regards,
Phuc
^ permalink raw reply
* Re: [PATCH v2 1/3] dt-bindings: input: touchscreen: sitronix,st1232: Add wakeup-source
From: phucduc.bui @ 2026-03-07 2:46 UTC (permalink / raw)
To: krzk
Cc: conor+dt, devicetree, dmitry.torokhov, geert+renesas, hechtb,
javier.carrasco, jeff, krzk+dt, krzysztof.kozlowski, linux-input,
linux-kernel, linux-renesas-soc, magnus.damm, phucduc.bui, robh,
wsa+renesas
In-Reply-To: <45fc7e39-3174-432a-9994-9de528759348@kernel.org>
Hi Krzysztof,
> Do not attach (thread) your patchsets to some other threads (unrelated
> or older versions). This buries them deep in the mailbox and might
> interfere with applying entire sets. See also:
> https://elixir.bootlin.com/linux/v6.16-rc2/source/Documentation/process/submitting-patches.rst#L830
>
Thank you for the guidance.
I would like to apologize for the confusion. While I was reviewing v2
myself, I realized there were technical issues and immediately worked on
v3 to fix them. Ironically, the fixes I made in v3 were exactly what you
suggested in your feedback for v2.
However, because I was so focused on the code, I sent out v3 before
checking my inbox and seeing your comments. This led me to unintentionally
repeat the same threading mistake you had just warned me about.
I have now carefully read the documentation you provided. I will ensure
that v4 is sent as a fresh, un-threaded series.
Thank you for your patience with a newcomer.
Best regards,
Phuc
^ permalink raw reply
* Re: [PATCH v5 2/7] mfd: Add driver for ASUS Transformer embedded controller
From: kernel test robot @ 2026-03-07 1:56 UTC (permalink / raw)
To: Svyatoslav Ryhel, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Dmitry Torokhov, Lee Jones, Pavel Machek, Sebastian Reichel,
Ion Agorria, Michał Mirosław
Cc: oe-kbuild-all, devicetree, linux-kernel, linux-input, linux-leds,
linux-pm
In-Reply-To: <20260304185751.83494-3-clamor95@gmail.com>
Hi Svyatoslav,
kernel test robot noticed the following build warnings:
[auto build test WARNING on sre-power-supply/for-next]
[also build test WARNING on robh/for-next linus/master v7.0-rc2 next-20260305]
[cannot apply to dtor-input/next dtor-input/for-linus]
[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/Svyatoslav-Ryhel/dt-bindings-embedded-controller-document-ASUS-Transformer-EC/20260305-030907
base: https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git for-next
patch link: https://lore.kernel.org/r/20260304185751.83494-3-clamor95%40gmail.com
patch subject: [PATCH v5 2/7] mfd: Add driver for ASUS Transformer embedded controller
config: openrisc-randconfig-r134-20260305 (https://download.01.org/0day-ci/archive/20260307/202603070925.iN0ySWQA-lkp@intel.com/config)
compiler: or1k-linux-gcc (GCC) 12.5.0
sparse: v0.6.5-rc1
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260307/202603070925.iN0ySWQA-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/202603070925.iN0ySWQA-lkp@intel.com/
sparse warnings: (new ones prefixed by >>)
>> drivers/mfd/asus-transformer-ec.c:482:36: sparse: sparse: incorrect type in argument 2 (different address spaces) @@ expected void const *src @@ got char const [noderef] __user *buf @@
drivers/mfd/asus-transformer-ec.c:482:36: sparse: expected void const *src
drivers/mfd/asus-transformer-ec.c:482:36: sparse: got char const [noderef] __user *buf
>> drivers/mfd/asus-transformer-ec.c:476:16: sparse: sparse: dereference of noderef expression
vim +482 drivers/mfd/asus-transformer-ec.c
467
468 static int dockram_write_one(struct i2c_client *client, int reg,
469 const char __user *buf, size_t count)
470 {
471 struct dockram_ec_data *priv = i2c_get_clientdata(client);
472 int ret;
473
474 if (!count || count > DOCKRAM_ENTRY_SIZE)
475 return -EINVAL;
> 476 if (buf[0] != count - 1)
477 return -EINVAL;
478
479 guard(mutex)(&priv->ctl_lock);
480
481 priv->ctl_data[0] = (u8)count;
> 482 memcpy(priv->ctl_data + 1, buf, count);
483 ret = asus_dockram_write(client, reg, priv->ctl_data);
484
485 return ret;
486 }
487
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v5 2/7] mfd: Add driver for ASUS Transformer embedded controller
From: kernel test robot @ 2026-03-07 0:51 UTC (permalink / raw)
To: Svyatoslav Ryhel, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Dmitry Torokhov, Lee Jones, Pavel Machek, Sebastian Reichel,
Ion Agorria, Michał Mirosław
Cc: oe-kbuild-all, devicetree, linux-kernel, linux-input, linux-leds,
linux-pm
In-Reply-To: <20260304185751.83494-3-clamor95@gmail.com>
Hi Svyatoslav,
kernel test robot noticed the following build warnings:
[auto build test WARNING on sre-power-supply/for-next]
[also build test WARNING on robh/for-next linus/master v7.0-rc2 next-20260305]
[cannot apply to dtor-input/next dtor-input/for-linus]
[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/Svyatoslav-Ryhel/dt-bindings-embedded-controller-document-ASUS-Transformer-EC/20260305-030907
base: https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git for-next
patch link: https://lore.kernel.org/r/20260304185751.83494-3-clamor95%40gmail.com
patch subject: [PATCH v5 2/7] mfd: Add driver for ASUS Transformer embedded controller
config: powerpc64-randconfig-r133-20260305 (https://download.01.org/0day-ci/archive/20260307/202603070848.ib570eG8-lkp@intel.com/config)
compiler: clang version 23.0.0git (https://github.com/llvm/llvm-project 9a109fbb6e184ec9bcce10615949f598f4c974a9)
sparse: v0.6.5-rc1
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260307/202603070848.ib570eG8-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/202603070848.ib570eG8-lkp@intel.com/
sparse warnings: (new ones prefixed by >>)
>> drivers/mfd/asus-transformer-ec.c:482:9: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const * @@ got char const [noderef] __user *buf @@
drivers/mfd/asus-transformer-ec.c:482:9: sparse: expected void const *
drivers/mfd/asus-transformer-ec.c:482:9: sparse: got char const [noderef] __user *buf
>> drivers/mfd/asus-transformer-ec.c:482:9: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const * @@ got char const [noderef] __user *buf @@
drivers/mfd/asus-transformer-ec.c:482:9: sparse: expected void const *
drivers/mfd/asus-transformer-ec.c:482:9: sparse: got char const [noderef] __user *buf
drivers/mfd/asus-transformer-ec.c:482:9: sparse: sparse: incorrect type in argument 2 (different address spaces) @@ expected void const * @@ got char const [noderef] __user *buf @@
drivers/mfd/asus-transformer-ec.c:482:9: sparse: expected void const *
drivers/mfd/asus-transformer-ec.c:482:9: sparse: got char const [noderef] __user *buf
drivers/mfd/asus-transformer-ec.c:476:16: sparse: sparse: dereference of noderef expression
vim +482 drivers/mfd/asus-transformer-ec.c
467
468 static int dockram_write_one(struct i2c_client *client, int reg,
469 const char __user *buf, size_t count)
470 {
471 struct dockram_ec_data *priv = i2c_get_clientdata(client);
472 int ret;
473
474 if (!count || count > DOCKRAM_ENTRY_SIZE)
475 return -EINVAL;
476 if (buf[0] != count - 1)
477 return -EINVAL;
478
479 guard(mutex)(&priv->ctl_lock);
480
481 priv->ctl_data[0] = (u8)count;
> 482 memcpy(priv->ctl_data + 1, buf, count);
483 ret = asus_dockram_write(client, reg, priv->ctl_data);
484
485 return ret;
486 }
487
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v2 2/7] dt-bindings: regulator: add support for MT6392
From: Mark Brown @ 2026-03-06 19:45 UTC (permalink / raw)
To: Luca Leonardo Scorcia
Cc: linux-mediatek, Fabien Parent, Val Packett, Rob Herring,
Dmitry Torokhov, Krzysztof Kozlowski, Conor Dooley, Sen Chu,
Sean Wang, Macpaul Lin, Lee Jones, Matthias Brugger,
AngeloGioacchino Del Regno, Liam Girdwood, Gary Bisson,
Julien Massot, Louis-Alexis Eyraud, Chen Zhong, linux-input,
devicetree, linux-kernel, linux-pm, linux-arm-kernel
In-Reply-To: <20260306120521.163654-3-l.scorcia@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 565 bytes --]
On Fri, Mar 06, 2026 at 12:03:06PM +0000, Luca Leonardo Scorcia wrote:
> From: Fabien Parent <parent.f@gmail.com>
>
> Add binding documentation of the regulator for MT6392 SoCs.
Please submit patches using subject lines reflecting the style for the
subsystem, this makes it easier for people to identify relevant patches.
Look at what existing commits in the area you're changing are doing and
make sure your subject lines visually resemble what they're doing.
There's no need to resubmit to fix this alone.
Acked-by: Mark Brown <broonie@kernel.org>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH] HID: wacom: fix out-of-bounds read in wacom_intuos_bt_irq
From: Gerecke, Jason @ 2026-03-06 19:32 UTC (permalink / raw)
To: bsevens
Cc: bentiss, jason.gerecke, jikos, linux-input, linux-kernel,
ping.cheng
In-Reply-To: <20260303135828.2374069-1-bsevens@google.com>
On Mar 3 2026, Benoît Sevens wrote:
> The wacom_intuos_bt_irq() function processes Bluetooth HID reports
> without sufficient bounds checking. A maliciously crafted short report
> can trigger an out-of-bounds read when copying data into the wacom
> structure.
>
> Specifically, report 0x03 requires at least 22 bytes to safely read
> the processed data and battery status, while report 0x04 (which
> falls through to 0x03) requires 32 bytes.
>
> Add explicit length checks for these report IDs and log a warning if
> a short report is received.
>
> Signed-off-by: Benoît Sevens <bsevens@google.com>
> ---
> drivers/hid/wacom_wac.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> index 9b2c710f8da1..da1f0ea85625 100644
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c
> @@ -1208,10 +1208,20 @@ static int wacom_intuos_bt_irq(struct wacom_wac *wacom, size_t len)
>
> switch (data[0]) {
> case 0x04:
> + if (len < 32) {
> + dev_warn(wacom->pen_input->dev.parent,
> + "Report 0x04 too short: %zu bytes\n", len);
> + break;
> + }
> wacom_intuos_bt_process_data(wacom, data + i);
> i += 10;
> fallthrough;
> case 0x03:
> + if (i == 1 && len < 22) {
> + dev_warn(wacom->pen_input->dev.parent,
> + "Report 0x03 too short: %zu bytes\n", len);
> + break;
> + }
> wacom_intuos_bt_process_data(wacom, data + i);
> i += 10;
> wacom_intuos_bt_process_data(wacom, data + i);
> --
> 2.53.0.473.g4a7958ca14-goog
Seems reasonable enough...
Reviewed-by: Jason Gerecke <jason.gerecke@wacom.com>
Note: this file is chock-full of functions that blindly process the
buffer in a way that could lead to out-of-bounds reads. I don't mind
fixing these two specific cases as an improvement, but we should
consider working on the other cases as well.
For reference, the potential OOB issues were introduced by commit
5e013ad20689 ("HID: wacom: Remove static WACOM_PKGLEN_MAX limit").
Prior to that point, all processing was done from a local statically-
sized buffer. Short packets might have led to unintentional behavior
but not an OOB read. We now work directly from the pointer provided
by HID and (usually) do no bounds checking to make sure the lengths
are reasonable.
^ permalink raw reply
* Re: [PATCH 1/1] dt-bindings: input: touchscreen: edt-ft5x06: add edt,edt-ft5x06 for legacy platforms
From: Frank Li @ 2026-03-06 19:08 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Yedaya Katsman, Joel Selvaraj, Jens Reidel,
open list:INPUT (KEYBOARD, MOUSE, JOYSTICK, TOUCHSCREEN)...,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list, imx
In-Reply-To: <20260306-wolf-of-striking-fragrance-0dee0e@quoll>
On Fri, Mar 06, 2026 at 09:41:03AM +0100, Krzysztof Kozlowski wrote:
> On Thu, Mar 05, 2026 at 05:37:29PM -0500, Frank Li wrote:
> > The compatible string "edt,edt-ft5x06" has been used for more than a decade
> > on older platforms such as i.MX6 and OMAP. However, it is currently missing
> > from the binding documentation.
> >
> > Add it to the binding to document existing usage and fix the following
> > CHECK_DTBS warnings.
> > arch/arm/boot/dts/nxp/imx/imx6dl-nit6xlite.dtb: /soc/bus@2100000/i2c@21a8000/touchscreen@38: failed to match any schema with compatible: ['edt,edt-ft5x06'
> >
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> > .../devicetree/bindings/input/touchscreen/edt-ft5x06.yaml | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml
> > index 6f90522de8c0a..213451f823369 100644
> > --- a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml
> > +++ b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml
> > @@ -38,6 +38,7 @@ properties:
> > - edt,edt-ft5306
> > - edt,edt-ft5406
> > - edt,edt-ft5506
> > + - edt,edt-ft5x06 # Deprecated, not use for new platform.
>
> Then document it as deprecated one.
>
> OTOH, this above is clearly not correct because we also have
> "edt,edt-ft5506", "edt,edt-ft5x06". Just use git grep.
>
> I don't understand, though, what is the point of documenting it if there
> is no ABI implemented (nothing in the driver) and it cannot work.
binding only define ABI. Many compatible string is not existed at driver
code, especially legacy platform.
Go back this case, edt,edt-ft5(2,3,4,5)06 to use the same drvdata linux
kernel. Touchscreen is off board periphial, no spec said use which one.
Two option,
- change binding to allow edt,edt-ft5x06
- change dts to use edt,edt-ft5206 (I guess), which broken ABI. like
previous fsl,imx28-lcdif case.
Which one do you like?
Frank
>
> Best regards,
> Krzysztof
>
^ permalink raw reply
* Re: [PATCH v4 4/4] Input: charlieplex_keypad: add GPIO charlieplex keypad
From: Andy Shevchenko @ 2026-03-06 14:21 UTC (permalink / raw)
To: Hugo Villeneuve
Cc: robin, andy, geert, robh, krzk+dt, conor+dt, dmitry.torokhov,
hvilleneuve, mkorpershoek, matthias.bgg,
angelogioacchino.delregno, lee, alexander.sverdlin, marek.vasut,
akurz, devicetree, linux-kernel, linux-input, linux-arm-kernel,
linux-mediatek
In-Reply-To: <20260305192101.2125660-5-hugo@hugovil.com>
On Thu, Mar 05, 2026 at 02:20:50PM -0500, Hugo Villeneuve wrote:
> Add support for GPIO-based charlieplex keypad, allowing to control
> N^2-N keys using N GPIO lines.
>
> Reuse matrix keypad keymap to simplify, even if there is no concept
> of rows and columns in this type of keyboard.
...
> --- a/drivers/input/keyboard/Makefile
> +++ b/drivers/input/keyboard/Makefile
> @@ -40,6 +40,7 @@ obj-$(CONFIG_KEYBOARD_LOCOMO) += locomokbd.o
> obj-$(CONFIG_KEYBOARD_LPC32XX) += lpc32xx-keys.o
> obj-$(CONFIG_KEYBOARD_MAPLE) += maple_keyb.o
> obj-$(CONFIG_KEYBOARD_MATRIX) += matrix_keypad.o
> +obj-$(CONFIG_KEYBOARD_CHARLIEPLEX) += charlieplex_keypad.o
Seem unordered. At least the all around it is ordered AFAICS.
> obj-$(CONFIG_KEYBOARD_MAX7359) += max7359_keypad.o
> obj-$(CONFIG_KEYBOARD_MAX7360) += max7360-keypad.o
> obj-$(CONFIG_KEYBOARD_MPR121) += mpr121_touchkey.o
...
> +/*
> + * GPIO driven charlieplex keypad driver
> + *
> + * Copyright (c) 2025 Hugo Villeneuve <hvilleneuve@dimonoff.com>
2026?
> + *
> + * Based on matrix_keyboard.c
> + */
...
> +#include <linux/bitops.h>
> +#include <linux/delay.h>
> +#include <linux/dev_printk.h>
> +#include <linux/device/devres.h>
> +#include <linux/err.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/input.h>
> +#include <linux/input/matrix_keypad.h>
> +#include <linux/math.h>
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/platform_device.h>
> +#include <linux/property.h>
> +#include <linux/types.h>
> +static void charlieplex_keypad_poll(struct input_dev *input)
> +{
> + struct charlieplex_keypad *keypad = input_get_drvdata(input);
> + int oline;
Why signed?
> + int code;
> +
> + for (code = 0, oline = 0; oline < keypad->nlines; oline++) {
Can be like
code = 0;
for (unsigned int oline = 0; oline < keypad->nlines; oline++) {
as iterator is not used outside the loop.
> + DECLARE_BITMAP(values, MATRIX_MAX_ROWS);
> + int iline;
Why signed?
> + int err;
> +
> + /* Activate only one line as output at a time. */
> + gpiod_direction_output(keypad->line_gpios->desc[oline], 1);
> +
> + if (keypad->settling_time_us)
> + fsleep(keypad->settling_time_us);
> +
> + /* Read input on all other lines. */
> + err = gpiod_get_array_value_cansleep(keypad->line_gpios->ndescs,
> + keypad->line_gpios->desc,
> + keypad->line_gpios->info, values);
> + if (err)
> + return;
> + for (iline = 0; iline < keypad->nlines; iline++) {
Can be just
for (unsigned int iline = 0; iline < keypad->nlines; iline++) {
as iterator is not used outside the loop.
> + if (iline == oline)
> + continue; /* Do not read active output line. */
> +
> + /* Check if GPIO is asserted. */
> + if (test_bit(iline, values)) {
> + code = MATRIX_SCAN_CODE(oline, iline,
> + get_count_order(keypad->nlines));
> + /*
> + * Exit loop immediately since we cannot detect
> + * more than one key press at a time.
> + */
> + break;
> + }
> + }
> +
> + gpiod_direction_input(keypad->line_gpios->desc[oline]);
> +
> + if (code)
> + break;
> + }
> +
> + charlieplex_keypad_check_switch_change(input, code);
> +}
...
> + for (unsigned int i = 0; i < keypad->nlines; i++)
> + gpiod_set_consumer_name(keypad->line_gpios->desc[i], "charlieplex_kbd_line");
Hmm... Don't you want to give it an index?
(In case you go this direction, see the kasprintf_strarray() or
its managed variant.)
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v7 0/3] TrackPoint doubletap enablement and user control
From: Mark Pearson @ 2026-03-06 13:51 UTC (permalink / raw)
To: Vishnu Sankar, Dmitry Torokhov, Henrique de Moraes Holschuh,
Hans de Goede, Jonathan Corbet, Derek J . Clark,
Ilpo Järvinen
Cc: linux-input, linux-kernel, ibm-acpi-devel, linux-doc,
platform-driver-x86@vger.kernel.org, Vishnu Sankar
In-Reply-To: <CABxCQKvNO78SeVp9pAnBOSAF=x0eFn03F33ftW-x-rZCG0-84Q@mail.gmail.com>
On Mon, Feb 23, 2026, at 6:28 PM, Vishnu Sankar wrote:
> Hi,
>
> Gentle ping on this series.
>
> This is v7 addressing all previous review comments.
> Please let me know if any further changes are needed.
>
> Thanks,
> Vishnu
>
>
> On Mon, Feb 9, 2026 at 3:34 PM Vishnu Sankar <vishnuocv@gmail.com> wrote:
>>
>> This patch series adds support for TrackPoint doubletap with a clear and
>> simple separation of responsibilities between drivers:
>>
>> 1. Firmware enablement (trackpoint.c):
>> Automatically enables doubletap on capable hardware during device
>> detection.
>>
>> 2. User control (thinkpad_acpi.c):
>> Provides a sysfs interface to enable or disable delivery of doubletap
>> events to userspace.
>>
>> The approach follows the KISS principle:
>> - The TrackPoint driver enables hardware functionality by default.
>> - The thinkpad_acpi driver controls whether ACPI doubletap events are
>> delivered, using existing hotkey filtering infrastructure.
>> - No cross-driver APIs or dual filtering paths are introduced.
>>
>> Changes in v7:
>> - Removed unwanted comments and logs
>>
>> Changes in v6:
>> - Documentation: fix formatting of the doubletap_enable sysfs attribute
>> description (separate "Values" list)
>>
>> Changes in v5:
>> - Rename sysfs attribute from doubletap_filter to doubletap_enable to
>> reflect actual behavior.
>> - Fix inverted logic so events are delivered only when doubletap is
>> enabled.
>> - Suppress ACPI hotkey delivery instead of injecting or filtering input
>> events.
>> - Register the sysfs attribute via hotkey_attributes[] instead of
>> device_create_file().
>> - Drop unnecessary helper wrappers and debug logging.
>> - Update Documentation to reflect the new naming and semantics.
>>
>> Changes in v4:
>> - Complete redesign based on reviewer feedback.
>> - trackpoint.c: Simplified to only enable doubletap by default.
>> - trackpoint.c: Removed all sysfs attributes and global variables.
>> - trackpoint.c: Uses firmware ID detection with deny list.
>> - thinkpad_acpi.c: Added sysfs interface for kernel-level event control.
>> - thinkpad_acpi.c: No cross-driver dependencies.
>> - Documentation: Updated to reflect simplified sysfs approach.
>>
>> Changes in v3:
>> - No changes.
>>
>> Changes in v2:
>> - Improved commit messages.
>> - Removed unnecessary comments and debug messages.
>> - Switched to strstarts() usage.
>> - Simplified firmware capability detection logic.
>>
>> This version addresses the remaining review feedback by correcting the
>> naming and logic inversion, aligning sysfs semantics with behavior, and
>> fully integrating with existing thinkpad_acpi hotkey handling.
>>
>> Vishnu Sankar (3):
>> input: trackpoint - Enable doubletap by default on capable devices
>> platform/x86: thinkpad_acpi: Add sysfs control for TrackPoint
>> double-tap
>> Documentation: thinkpad-acpi - Document doubletap_enable attribute
>>
>> .../admin-guide/laptops/thinkpad-acpi.rst | 21 +++++++++
>> drivers/input/mouse/trackpoint.c | 45 +++++++++++++++++++
>> drivers/input/mouse/trackpoint.h | 5 +++
>> drivers/platform/x86/lenovo/thinkpad_acpi.c | 42 ++++++++++++++---
>> 4 files changed, 106 insertions(+), 7 deletions(-)
>>
>> --
>> 2.51.0
>>
>
>
> --
>
> Regards,
>
> Vishnu Sankar
Hi Ilpo,
I was just discussing this with Vishnu and wanted to check if anything else was needed from your perspective for this patch?
I assume at this point we're waiting for the 7.1 merge window to open. Please do let us know if there's anything you need from us in the meantime.
We can't get this pulled into the distro's until it's accepted upstream - and while it's not a critical feature, we'd love it to be in place for the Linux preloads for the 2026 platforms (coming up soon)
Thanks
Mark
^ permalink raw reply
* Re: [PATCH v2 7/9] leds: Add driver for Asus Transformer LEDs
From: Svyatoslav Ryhel @ 2026-03-06 12:45 UTC (permalink / raw)
To: Lee Jones
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Dmitry Torokhov,
Pavel Machek, Arnd Bergmann, Greg Kroah-Hartman,
Sebastian Reichel, Michał Mirosław, Ion Agorria,
devicetree, linux-kernel, linux-input, linux-leds, linux-pm
In-Reply-To: <20260306100406.GE183676@google.com>
пт, 6 бер. 2026 р. о 12:04 Lee Jones <lee@kernel.org> пише:
>
> On Mon, 09 Feb 2026, Svyatoslav Ryhel wrote:
>
> > From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> >
> > Asus Transformer tablets have a green and an amber LED on both the Pad
> > and the Dock. If both LEDs are enabled simultaneously, the emitted light
> > will be yellow.
> >
> > Co-developed-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> > ---
> > drivers/leds/Kconfig | 11 ++++
> > drivers/leds/Makefile | 1 +
> > drivers/leds/leds-asus-ec.c | 104 ++++++++++++++++++++++++++++++++++++
> > 3 files changed, 116 insertions(+)
> > create mode 100644 drivers/leds/leds-asus-ec.c
> >
> > diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> > index 597d7a79c988..96dab210f6ca 100644
> > --- a/drivers/leds/Kconfig
> > +++ b/drivers/leds/Kconfig
> > @@ -120,6 +120,17 @@ config LEDS_OSRAM_AMS_AS3668
> > To compile this driver as a module, choose M here: the module
> > will be called leds-as3668.
> >
> > +config LEDS_ASUSEC
> > + tristate "LED Support for Asus Transformer charging LED"
> > + depends on LEDS_CLASS
> > + depends on MFD_ASUSEC
> > + help
> > + This option enables support for charging indicator on
> > + Asus Transformer's Pad and it's Dock.
> > +
> > + To compile this driver as a module, choose M here: the module
> > + will be called leds-asus-ec.
> > +
> > config LEDS_AW200XX
> > tristate "LED support for Awinic AW20036/AW20054/AW20072/AW20108"
> > depends on LEDS_CLASS
> > diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
> > index 8fdb45d5b439..1117304dfdf4 100644
> > --- a/drivers/leds/Makefile
> > +++ b/drivers/leds/Makefile
> > @@ -16,6 +16,7 @@ obj-$(CONFIG_LEDS_AN30259A) += leds-an30259a.o
> > obj-$(CONFIG_LEDS_APU) += leds-apu.o
> > obj-$(CONFIG_LEDS_ARIEL) += leds-ariel.o
> > obj-$(CONFIG_LEDS_AS3668) += leds-as3668.o
> > +obj-$(CONFIG_LEDS_ASUSEC) += leds-asus-ec.o
> > obj-$(CONFIG_LEDS_AW200XX) += leds-aw200xx.o
> > obj-$(CONFIG_LEDS_AW2013) += leds-aw2013.o
> > obj-$(CONFIG_LEDS_BCM6328) += leds-bcm6328.o
> > diff --git a/drivers/leds/leds-asus-ec.c b/drivers/leds/leds-asus-ec.c
> > new file mode 100644
> > index 000000000000..5dd76c9247ee
> > --- /dev/null
> > +++ b/drivers/leds/leds-asus-ec.c
> > @@ -0,0 +1,104 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * ASUS EC driver - battery LED
> > + */
> > +
> > +#include <linux/err.h>
> > +#include <linux/leds.h>
> > +#include <linux/mfd/asus-ec.h>
> > +#include <linux/mod_devicetable.h>
> > +#include <linux/module.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/slab.h>
> > +
> > +/*
> > + * F[5] & 0x07
> > + * auto: brightness == 0
> > + * bit 0: blink / charger on
> > + * bit 1: amber on
> > + * bit 2: green on
> > + */
> > +
> > +#define ASUSEC_CTL_LED_BLINK BIT_ULL(40)
> > +#define ASUSEC_CTL_LED_AMBER BIT_ULL(41)
> > +#define ASUSEC_CTL_LED_GREEN BIT_ULL(42)
> > +
> > +static void asus_ec_led_set_brightness_amber(struct led_classdev *led,
> > + enum led_brightness brightness)
> > +{
> > + const struct asusec_info *ec = dev_get_drvdata(led->dev->parent);
> > +
> > + if (brightness)
> > + asus_ec_set_ctl_bits(ec, ASUSEC_CTL_LED_AMBER);
> > + else
> > + asus_ec_clear_ctl_bits(ec, ASUSEC_CTL_LED_AMBER);
> > +}
> > +
> > +static void asus_ec_led_set_brightness_green(struct led_classdev *led,
> > + enum led_brightness brightness)
> > +{
> > + const struct asusec_info *ec = dev_get_drvdata(led->dev->parent);
> > +
> > + if (brightness)
> > + asus_ec_set_ctl_bits(ec, ASUSEC_CTL_LED_GREEN);
> > + else
> > + asus_ec_clear_ctl_bits(ec, ASUSEC_CTL_LED_GREEN);
> > +}
> > +
> > +static int asus_ec_led_probe(struct platform_device *pdev)
> > +{
> > + struct asusec_info *ec = cell_to_ec(pdev);
>
> Please remove all of your abstraction layers. They serve little purpose
> other than to complicate things. Just use dev_get_drvdata() here.
>
> Remove the "_info" part and change "ec" to "ddata".
>
Controller exposes only required stuff, exposing controllers internal
parts is not desired. Is there any written convention that driver
private data pointer MUST be named "ddata"? "priv" is pretty well
fitting to. You are just nitpicking.
> > + struct device *dev = &pdev->dev;
> > + struct led_classdev *amber_led, *green_led;
> > + int ret;
> > +
> > + platform_set_drvdata(pdev, ec);
>
> Wait, what?
>
> Why are you doing that?
>
This is redundant, yes.
> > + amber_led = devm_kzalloc(dev, sizeof(*amber_led), GFP_KERNEL);
> > + if (!amber_led)
> > + return -ENOMEM;
> > +
> > + amber_led->name = devm_kasprintf(dev, GFP_KERNEL, "%s::amber", ec->name);
> > + amber_led->max_brightness = 1;
> > + amber_led->flags = LED_CORE_SUSPENDRESUME | LED_RETAIN_AT_SHUTDOWN;
> > + amber_led->brightness_set = asus_ec_led_set_brightness_amber;
> > +
> > + ret = devm_led_classdev_register(dev, amber_led);
> > + if (ret)
> > + return dev_err_probe(dev, ret, "failed to register amber LED\n");
> > +
> > + green_led = devm_kzalloc(dev, sizeof(*green_led), GFP_KERNEL);
> > + if (!green_led)
> > + return -ENOMEM;
> > +
> > + green_led->name = devm_kasprintf(dev, GFP_KERNEL, "%s::green", ec->name);
> > + green_led->max_brightness = 1;
> > + green_led->flags = LED_CORE_SUSPENDRESUME | LED_RETAIN_AT_SHUTDOWN;
> > + green_led->brightness_set = asus_ec_led_set_brightness_green;
> > +
> > + ret = devm_led_classdev_register(dev, green_led);
> > + if (ret)
> > + return dev_err_probe(dev, ret, "failed to register green LED\n");
>
> Lots of repetition here.
>
> I'd make a sub-function that takes the differences.
>
> Same with the set brightness functions.
>
> Think to yourself - what if I had to support 16 different LEDs?
>
That is not of my concern what you would do. I have 2 LEDs and I see
no point in "abstraction for he sake of abstraction".
> > +
> > + return 0;
> > +}
> > +
> > +static const struct of_device_id asus_ec_led_match[] = {
> > + { .compatible = "asus,ec-led" },
> > + { }
> > +};
> > +MODULE_DEVICE_TABLE(of, asus_ec_led_match);
> > +
> > +static struct platform_driver asus_ec_led_driver = {
> > + .driver = {
> > + .name = "asus-ec-led",
> > + .of_match_table = asus_ec_led_match,
> > + },
> > + .probe = asus_ec_led_probe,
> > +};
> > +module_platform_driver(asus_ec_led_driver);
> > +
> > +MODULE_AUTHOR("Michał Mirosław <mirq-linux@rere.qmqm.pl>");
> > +MODULE_AUTHOR("Svyatoslav Ryhel <clamor95@gmail.com>");
> > +MODULE_DESCRIPTION("ASUS Transformer's charging LED driver");
> > +MODULE_LICENSE("GPL");
> > --
> > 2.51.0
> >
> >
>
> --
> Lee Jones [李琼斯]
^ permalink raw reply
* Re: [PATCH v2 4/9] mfd: Add driver for Asus Transformer embedded controller
From: Svyatoslav Ryhel @ 2026-03-06 12:36 UTC (permalink / raw)
To: Lee Jones
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Dmitry Torokhov,
Pavel Machek, Arnd Bergmann, Greg Kroah-Hartman,
Sebastian Reichel, Michał Mirosław, Ion Agorria,
devicetree, linux-kernel, linux-input, linux-leds, linux-pm
In-Reply-To: <20260306091856.GD183676@google.com>
пт, 6 бер. 2026 р. о 11:19 Lee Jones <lee@kernel.org> пише:
>
> On Mon, 09 Feb 2026, Svyatoslav Ryhel wrote:
>
> > From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> >
> > Support Nuvoton NPCE795-based ECs as used in Asus Transformer TF201,
> > TF300T, TF300TG, TF300TL and TF700T pad and dock, as well as TF101 dock
> > and TF600T, P1801-T and TF701T pad. This is a glue driver handling
> > detection and common operations for EC's functions.
> >
> > Co-developed-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> > ---
> > drivers/mfd/Kconfig | 15 ++
> > drivers/mfd/Makefile | 1 +
> > drivers/mfd/asus-ec.c | 467 ++++++++++++++++++++++++++++++++++++
> > include/linux/mfd/asus-ec.h | 138 +++++++++++
> > 4 files changed, 621 insertions(+)
> > create mode 100644 drivers/mfd/asus-ec.c
> >
> > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> > index 7192c9d1d268..312fd15eec6a 100644
> > --- a/drivers/mfd/Kconfig
> > +++ b/drivers/mfd/Kconfig
> > @@ -137,6 +137,21 @@ config MFD_AAT2870_CORE
> > additional drivers must be enabled in order to use the
> > functionality of the device.
> >
> > +config MFD_ASUSEC
>
> MFD_ASUS_EC
>
> > + tristate "ASUS Transformer's embedded controller"
> > + depends on I2C && OF
> > + select SYSFS
> > + select ASUS_DOCKRAM
> > + help
> > + Support ECs found in ASUS Transformer's Pad and Mobile Dock.
> > +
> > + This provides shared glue for functional part drivers:
> > + asus-ec-kbc, asus-ec-keys, leds-asus-ec, asus-ec-battery
> > + and asus-ec-charger.
>
> Why the additional tabbing? What example did you take that from?
>
MFD_MXS_LRADC
MFD_SL28CPLD
MFD_STMPE
They use 2 tabs, so I asume I have to switch to 2 tabs and a list too.
> > + This driver can also be built as a module. If so, the module
> > + will be called asus-ec.
> > +
> > config MFD_AT91_USART
> > tristate "AT91 USART Driver"
> > select MFD_CORE
> > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> > index e75e8045c28a..b676922601ba 100644
> > --- a/drivers/mfd/Makefile
> > +++ b/drivers/mfd/Makefile
> > @@ -10,6 +10,7 @@ obj-$(CONFIG_MFD_88PM805) += 88pm805.o 88pm80x.o
> > obj-$(CONFIG_MFD_88PM886_PMIC) += 88pm886.o
> > obj-$(CONFIG_MFD_ACT8945A) += act8945a.o
> > obj-$(CONFIG_MFD_SM501) += sm501.o
> > +obj-$(CONFIG_MFD_ASUSEC) += asus-ec.o
> > obj-$(CONFIG_ARCH_BCM2835) += bcm2835-pm.o
> > obj-$(CONFIG_MFD_BCM590XX) += bcm590xx.o
> > obj-$(CONFIG_MFD_BD9571MWV) += bd9571mwv.o
> > diff --git a/drivers/mfd/asus-ec.c b/drivers/mfd/asus-ec.c
> > new file mode 100644
> > index 000000000000..e151c1506aa2
> > --- /dev/null
> > +++ b/drivers/mfd/asus-ec.c
> > @@ -0,0 +1,467 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * ASUS EC driver
>
> Copyright? Author?
>
> > + */
> > +
> > +#include <linux/array_size.h>
> > +#include <linux/debugfs.h>
> > +#include <linux/delay.h>
> > +#include <linux/gpio/consumer.h>
> > +#include <linux/device.h>
> > +#include <linux/err.h>
> > +#include <linux/i2c.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/mfd/asus-ec.h>
> > +#include <linux/mfd/core.h>
> > +#include <linux/mod_devicetable.h>
> > +#include <linux/module.h>
> > +#include <linux/mutex.h>
> > +#include <linux/property.h>
> > +#include <linux/string.h>
> > +#include <linux/sysfs.h>
> > +#include <linux/types.h>
>
> Alphabetical.
>
> Are you sure all of these are in use?
>
Yes, but if you are willing to double check I would be happy to
include your findings.
> > +#define ASUSEC_RSP_BUFFER_SIZE 8
> > +
> > +struct asus_ec_chip_data {
> > + const char *name;
> > + const struct mfd_cell *mfd_devices;
> > + unsigned int num_devices;
> > +};
> > +
> > +struct asus_ec_data {
> > + struct asusec_info info;
> > + struct mutex ecreq_lock; /* prevent simultaneous access */
>
> We know what mutexes do.
>
> If you're going to provide a comment, state WHAT is is protecting.
>
> Or just omit the comment altogether.
>
checkpatch script requires a brief comment why mutex is used, I added
a brief comment where it is used. If you don't like it, then propose a
fix to checkpatch.
> > + struct gpio_desc *ecreq;
> > + struct i2c_client *self;
>
> "client"
>
Is there any written convention that struct i2c_client pointer MUST be
named "client"? "self" is pretty well fitting to. You are just
nitpicking.
> Why are you storing this?
>
To use later on to get device and irq.
> > + const struct asus_ec_chip_data *data;
> > + u8 ec_data[DOCKRAM_ENTRY_BUFSIZE];
> > + bool clr_fmode;
> > + bool logging_disabled;
> > +};
> > +
> > +#define to_ec_data(ec) \
> > + container_of(ec, struct asus_ec_data, info)
> > +
> > +static void asus_ec_remove_notifier(struct device *dev, void *res)
> > +{
> > + struct asusec_info *ec = dev_get_drvdata(dev->parent);
> > + struct notifier_block **nb = res;
> > +
> > + blocking_notifier_chain_unregister(&ec->notify_list, *nb);
> > +}
> > +
> > +/**
> > + * devm_asus_ec_register_notifier - Managed registration of notifier to an
> > + * ASUS EC blocking notifier chain.
> > + * @pdev: Device requesting the notifier (used for resource management).
> > + * @nb: Notifier block to be registered.
> > + *
> > + * Register a notifier to the ASUS EC blocking notifier chain. The notifier
> > + * will be automatically unregistered when the requesting device is detached.
> > + *
> > + * Return: 0 on success or a negative error code on failure.
> > + */
> > +int devm_asus_ec_register_notifier(struct platform_device *pdev,
> > + struct notifier_block *nb)
> > +{
> > + struct asusec_info *ec = dev_get_drvdata(pdev->dev.parent);
> > + struct notifier_block **res;
> > + int ret;
> > +
> > + res = devres_alloc(asus_ec_remove_notifier, sizeof(*res), GFP_KERNEL);
> > + if (!res)
> > + return -ENOMEM;
> > +
> > + *res = nb;
> > + ret = blocking_notifier_chain_register(&ec->notify_list, nb);
> > + if (ret) {
> > + devres_free(res);
> > + return ret;
> > + }
> > +
> > + devres_add(&pdev->dev, res);
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(devm_asus_ec_register_notifier);
> > +
> > +static int asus_ec_signal_request(const struct asusec_info *ec)
> > +{
> > + struct asus_ec_data *priv = to_ec_data(ec);
> > +
> > + guard(mutex)(&priv->ecreq_lock);
> > +
> > + dev_dbg(&priv->self->dev, "EC request\n");
> > +
> > + gpiod_set_value_cansleep(priv->ecreq, 1);
> > + msleep(50);
> > +
> > + gpiod_set_value_cansleep(priv->ecreq, 0);
> > + msleep(200);
> > +
> > + return 0;
> > +}
> > +
> > +static int asus_ec_write(struct asus_ec_data *priv, u16 data)
> > +{
> > + int ret = i2c_smbus_write_word_data(priv->self, ASUSEC_WRITE_BUF, data);
> > +
> > + dev_dbg(&priv->self->dev, "EC write: %04x, ret = %d\n", data, ret);
> > + return ret;
> > +}
> > +
> > +static int asus_ec_read(struct asus_ec_data *priv, bool in_irq)
> > +{
> > + int ret = i2c_smbus_read_i2c_block_data(priv->self, ASUSEC_READ_BUF,
> > + sizeof(priv->ec_data),
> > + priv->ec_data);
> > +
> > + dev_dbg(&priv->self->dev, "EC read: %*ph, ret = %d%s\n",
> > + sizeof(priv->ec_data), priv->ec_data,
> > + ret, in_irq ? "; in irq" : "");
> > +
> > + return ret;
> > +}
>
> Remove both of these functions and use the i2c_smbus_*() API instead.
>
This would not reduce size much but it will reduce readability
significantly since i2c_smbus_*() API have quite extended names.
> > +
> > +/**
> > + * asus_ec_i2c_command - Send a 16-bit command to the ASUS EC.
> > + * @ec: Pointer to the shared ASUS EC structure.
> > + * @data: The 16-bit command (word) to be sent.
> > + *
> > + * Return: 0 on success or a negative error code on failure.
> > + */
> > +int asus_ec_i2c_command(const struct asusec_info *ec, u16 data)
> > +{
> > + return asus_ec_write(to_ec_data(ec), data);
> > +}
> > +EXPORT_SYMBOL_GPL(asus_ec_i2c_command);
>
> Why is this needed? Why not share 'client' with the leave drivers and
> let them make their own calls to i2c_smbus_write_word_data()?
>
Because parent should not share its internal stuff with subdevices.
> > +static void asus_ec_clear_buffer(struct asus_ec_data *priv)
> > +{
> > + int retry = ASUSEC_RSP_BUFFER_SIZE;
> > +
> > + while (retry--) {
>
> Why is the amount of retries related to the buffer size?
>
Buffer size is 256 bytes, 1 read is 32 bytes. Calculate
> > + if (asus_ec_read(priv, false) < 0)
> > + continue;
> > +
> > + if (priv->ec_data[1] & ASUSEC_OBF_MASK)
>
> No magic numbers. Define the 1.
>
#define ONE 1
> > + continue;
> > +
> > + break;
> > + }
> > +}
> > +
> > +static int asus_ec_log_info(struct asus_ec_data *priv, unsigned int reg,
> > + const char *name, char **out)
> > +{
> > + char buf[DOCKRAM_ENTRY_BUFSIZE];
> > + int ret;
> > +
> > + ret = asus_dockram_read(priv->info.dockram, reg, buf);
> > + if (ret < 0)
> > + return ret;
> > +
> > + if (!priv->logging_disabled)
> > + dev_info(&priv->self->dev, "%-14s: %.*s\n", name, buf[0], buf + 1);
> > +
> > + if (out)
> > + *out = kstrndup(buf + 1, buf[0], GFP_KERNEL);
> > +
> > + return 0;
> > +}
>
> The driver is written now. You can remove this over-engineered debugging
> facility.
>
No, this is logs EC firmware behaior. According to your logic kernel
should have no logging whatsoever, and dmesg output is obviously
redundant.
> > +static int asus_ec_reset(struct asus_ec_data *priv)
> > +{
> > + int retry, ret;
> > +
> > + for (retry = 0; retry < 3; retry++) {
>
> Why 3?
>
> Why are you using for() here and while() above?
>
Transferred from vendor source. No datasheet.
> > + ret = asus_ec_write(priv, 0);
>
> Add a comment to explain how this works.
>
> Or, better still, define the value.
>
#define ZERO 0
> > + if (!ret)
> > + return 0;
> > +
> > + msleep(300);
>
> Why 300?
>
Transferred from vendor source. No datasheet.
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static int asus_ec_magic_debug(struct asus_ec_data *priv)
>
> What does this do? More comments throughout please.
>
Checks firmware behavior. More is not known.
> > +{
> > + u64 flag;
> > + int ret;
> > +
> > + ret = asus_ec_get_ctl(&priv->info, &flag);
> > + if (ret < 0)
> > + return ret;
> > +
> > + flag &= ASUSEC_CTL_SUSB_MODE;
> > + dev_info(&priv->self->dev, "EC FW behaviour: %s\n",
> > + flag ? "susb on when receive ec_req" : "susb on when system wakeup");
> > +
> > + return 0;
> > +}
> > +
> > +static int asus_ec_set_factory_mode(struct asus_ec_data *priv, bool on)
> > +{
> > + dev_info(&priv->self->dev, "Entering %s mode.\n", on ? "factory" : "normal");
>
> Remove all of the debugging prints now.
>
No, this is logs EC firmware behavior.
> > + return asus_ec_update_ctl(&priv->info, ASUSEC_CTL_FACTORY_MODE,
> > + on ? ASUSEC_CTL_FACTORY_MODE : 0);
> > +}
> > +
> > +static void asus_ec_handle_smi(struct asus_ec_data *priv, unsigned int code);
>
> No forward declarations.
>
> > +static irqreturn_t asus_ec_interrupt(int irq, void *dev_id)
> > +{
> > + struct asus_ec_data *priv = dev_id;
> > + unsigned long notify_action;
> > + int ret;
> > +
> > + ret = asus_ec_read(priv, true);
> > + if (ret <= 0 || !(priv->ec_data[1] & ASUSEC_OBF_MASK))
> > + return IRQ_NONE;
> > +
> > + notify_action = priv->ec_data[1];
> > + if (notify_action & ASUSEC_SMI_MASK) {
> > + unsigned int code = priv->ec_data[2];
> > +
> > + asus_ec_handle_smi(priv, code);
> > +
> > + notify_action |= code << 8;
> > + dev_dbg(&priv->self->dev, "SMI code: 0x%02x\n", code);
> > + }
> > +
> > + blocking_notifier_call_chain(&priv->info.notify_list,
> > + notify_action, priv->ec_data);
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
> > +static int asus_ec_detect(struct asus_ec_data *priv)
> > +{
> > + char *model = NULL;
> > + int ret;
> > +
> > + ret = asus_ec_reset(priv);
> > + if (ret)
> > + goto err_exit;
> > +
> > + asus_ec_clear_buffer(priv);
> > +
> > + ret = asus_ec_log_info(priv, ASUSEC_DOCKRAM_INFO_MODEL, "model", &model);
> > + if (ret)
> > + goto err_exit;
> > +
> > + ret = asus_ec_log_info(priv, ASUSEC_DOCKRAM_INFO_FW, "FW version", NULL);
> > + if (ret)
> > + goto err_exit;
> > +
> > + ret = asus_ec_log_info(priv, ASUSEC_DOCKRAM_INFO_CFGFMT, "Config format", NULL);
> > + if (ret)
> > + goto err_exit;
> > +
> > + ret = asus_ec_log_info(priv, ASUSEC_DOCKRAM_INFO_HW, "HW version", NULL);
> > + if (ret)
> > + goto err_exit;
> > +
> > + priv->logging_disabled = true;
> > +
> > + ret = asus_ec_magic_debug(priv);
> > + if (ret)
> > + goto err_exit;
> > +
> > + priv->info.model = model;
> > + priv->info.name = priv->data->name;
> > +
> > + if (priv->clr_fmode)
> > + asus_ec_set_factory_mode(priv, false);
> > +
> > +err_exit:
> > + if (ret)
> > + dev_err(&priv->self->dev, "failed to access EC: %d\n", ret);
> > +
> > + return ret;
> > +}
> > +
> > +static void asus_ec_handle_smi(struct asus_ec_data *priv, unsigned int code)
> > +{
> > + dev_dbg(&priv->self->dev, "SMI interrupt: 0x%02x\n", code);
> > +
> > + switch (code) {
> > + case ASUSEC_SMI_HANDSHAKE:
> > + case ASUSEC_SMI_RESET:
> > + asus_ec_detect(priv);
> > + break;
> > + }
> > +}
> > +
> > +static int ec_request_set(void *ec, u64 val)
> > +{
> > + if (val)
> > + asus_ec_signal_request(ec);
> > +
> > + return 0;
> > +}
> > +
> > +DEFINE_DEBUGFS_ATTRIBUTE(ec_request_fops, NULL, ec_request_set, "%llu\n");
> > +
> > +static int ec_irq_set(void *ec, u64 val)
> > +{
> > + struct asus_ec_data *priv = to_ec_data(ec);
> > +
> > + if (val)
> > + irq_wake_thread(priv->self->irq, priv);
> > +
> > + return 0;
> > +}
> > +
> > +DEFINE_DEBUGFS_ATTRIBUTE(ec_irq_fops, NULL, ec_irq_set, "%llu\n");
>
> Document these.
>
Debugfs does not require any documentation.
> > +static void asus_ec_debugfs_remove(void *debugfs_root)
> > +{
> > + debugfs_remove_recursive(debugfs_root);
> > +}
> > +
> > +static void devm_asus_ec_debugfs_init(struct device *dev)
> > +{
> > + struct asusec_info *ec = dev_get_drvdata(dev);
> > + struct asus_ec_data *priv = to_ec_data(ec);
> > + struct dentry *debugfs_root;
> > + char *name = devm_kasprintf(dev, GFP_KERNEL, "asus-ec-%s",
> > + priv->data->name);
> > +
> > + debugfs_root = debugfs_create_dir(name, NULL);
> > +
> > + debugfs_create_file("ec_irq", 0200, debugfs_root, ec, &ec_irq_fops);
> > + debugfs_create_file("ec_request", 0200, debugfs_root, ec, &ec_request_fops);
> > +
> > + asus_dockram_debugfs_init(priv->info.dockram, debugfs_root);
> > +
> > + devm_add_action_or_reset(dev, asus_ec_debugfs_remove, debugfs_root);
> > +}
> > +
> > +static int asus_ec_probe(struct i2c_client *client)
> > +{
> > + struct device *dev = &client->dev;
> > + struct asus_ec_data *priv;
>
> Call this "ddata".
>
Is there any written convention that driver private data pointer MUST
be named "ddata"? "priv" is pretty well fitting to. You are just
nitpicking.
> > + int ret;
> > +
> > + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> > + if (!priv)
> > + return -ENOMEM;
> > +
> > + priv->data = device_get_match_data(dev);
> > + if (!priv->data)
> > + return -ENODEV;
> > +
> > + i2c_set_clientdata(client, priv);
> > + priv->self = client;
> > +
> > + priv->info.dockram = devm_asus_dockram_get(dev);
> > + if (IS_ERR(priv->info.dockram))
> > + return dev_err_probe(dev, PTR_ERR(priv->info.dockram),
> > + "failed to get dockram\n");
> > +
> > + priv->ecreq = devm_gpiod_get(dev, "request", GPIOD_OUT_LOW);
> > + if (IS_ERR(priv->ecreq))
> > + return dev_err_probe(dev, PTR_ERR(priv->ecreq),
> > + "failed to get request GPIO\n");
> > +
> > + BLOCKING_INIT_NOTIFIER_HEAD(&priv->info.notify_list);
> > + mutex_init(&priv->ecreq_lock);
> > +
> > + priv->clr_fmode = device_property_read_bool(dev, "asus,clear-factory-mode");
> > +
> > + asus_ec_signal_request(&priv->info);
> > +
> > + ret = asus_ec_detect(priv);
> > + if (ret)
> > + return dev_err_probe(dev, ret, "failed to detect EC version\n");
> > +
> > + ret = devm_request_threaded_irq(dev, client->irq, NULL,
> > + &asus_ec_interrupt,
> > + IRQF_ONESHOT | IRQF_SHARED,
> > + client->name, priv);
> > + if (ret)
> > + return dev_err_probe(dev, ret, "failed to register IRQ\n");
> > +
> > + /* Parent I2C controller uses DMA, ASUS EC and child devices do not */
> > + client->dev.coherent_dma_mask = 0;
> > + client->dev.dma_mask = &client->dev.coherent_dma_mask;
> > +
> > + devm_asus_ec_debugfs_init(dev);
> > +
> > + return devm_mfd_add_devices(dev, 0, priv->data->mfd_devices,
> > + priv->data->num_devices, NULL, 0, NULL);
> > +}
> > +
> > +static const struct mfd_cell asus_ec_pad_mfd_devices[] = {
> > + {
> > + .name = "asus-ec-battery",
> > + .id = 0,
> > + .of_compatible = "asus,ec-battery",
> > + }, {
> > + .name = "asus-ec-charger",
> > + .id = 0,
> > + .of_compatible = "asus,ec-charger",
> > + }, {
> > + .name = "asus-ec-led",
> > + .id = 0,
> > + .of_compatible = "asus,ec-led",
> > + },
> > +};
> > +
> > +static const struct mfd_cell asus_ec_dock_mfd_devices[] = {
> > + {
> > + .name = "asus-ec-battery",
> > + .id = 1,
> > + .of_compatible = "asus,ec-battery",
> > + }, {
> > + .name = "asus-ec-charger",
> > + .id = 1,
> > + .of_compatible = "asus,ec-charger",
> > + }, {
> > + .name = "asus-ec-led",
> > + .id = 1,
> > + .of_compatible = "asus,ec-led",
> > + }, {
> > + .name = "asus-ec-keys",
> > + .of_compatible = "asus,ec-keys",
> > + }, {
> > + .name = "asus-ec-kbc",
> > + .of_compatible = "asus,ec-kbc",
> > + },
> > +};
> > +
> > +static const struct asus_ec_chip_data asus_ec_pad_data = {
> > + .name = "pad",
> > + .mfd_devices = asus_ec_pad_mfd_devices,
> > + .num_devices = ARRAY_SIZE(asus_ec_pad_mfd_devices),
> > +};
> > +
> > +static const struct asus_ec_chip_data asus_ec_dock_data = {
> > + .name = "dock",
> > + .mfd_devices = asus_ec_dock_mfd_devices,
> > + .num_devices = ARRAY_SIZE(asus_ec_dock_mfd_devices),
> > +};
> > +
> > +static const struct of_device_id asus_ec_match[] = {
> > + { .compatible = "asus,ec-pad", .data = &asus_ec_pad_data },
>
> Passing MFD data through a different registration mechanism is not
> allowed. Use identifiers to match in instead.
>
> git grep "\.data =.*void" -- drivers/mfd
>
Why? So for overwhelming majority of the kernel drivers this is
perfectly fine and ok, and for MFD this is no no? Point me please to
any written convention why using OF match (which is a must for any new
driver) is bad? Especially since this driver targets embedded devices
which rely mostly on OF device trees.
> > + { .compatible = "asus,ec-dock", .data = &asus_ec_dock_data },
> > + { }
> > +};
> > +MODULE_DEVICE_TABLE(of, asus_ec_match);
> > +
> > +static struct i2c_driver asus_ec_driver = {
> > + .driver = {
> > + .name = "asus-ec",
> > + .of_match_table = asus_ec_match,
> > + },
> > + .probe = asus_ec_probe,
> > +};
> > +module_i2c_driver(asus_ec_driver);
> > +
> > +MODULE_AUTHOR("Michał Mirosław <mirq-linux@rere.qmqm.pl>");
> > +MODULE_AUTHOR("Svyatoslav Ryhel <clamor95@gmail.com>");
> > +MODULE_DESCRIPTION("ASUS Transformer's EC driver");
> > +MODULE_LICENSE("GPL");
> > diff --git a/include/linux/mfd/asus-ec.h b/include/linux/mfd/asus-ec.h
> > index 6a36313b9ebd..6a06b125ba30 100644
> > --- a/include/linux/mfd/asus-ec.h
> > +++ b/include/linux/mfd/asus-ec.h
> > @@ -2,16 +2,78 @@
> > #ifndef __MISC_ASUS_EC_H
> > #define __MISC_ASUS_EC_H
> >
> > +#include <linux/notifier.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/workqueue.h>
> > +
> > struct i2c_client;
> >
> > +struct asusec_info {
> > + const char *model;
> > + const char *name;
> > + struct i2c_client *dockram;
> > + struct workqueue_struct *wq;
> > + struct blocking_notifier_head notify_list;
> > +};
> > +
> > #define DOCKRAM_ENTRIES 0x100
> > #define DOCKRAM_ENTRY_SIZE 32
> > #define DOCKRAM_ENTRY_BUFSIZE (DOCKRAM_ENTRY_SIZE + 1)
> >
> > +/* interrupt sources */
> > +#define ASUSEC_OBF_MASK BIT(0)
> > +#define ASUSEC_KEY_MASK BIT(2)
> > +#define ASUSEC_KBC_MASK BIT(3)
> > +#define ASUSEC_AUX_MASK BIT(5)
> > +#define ASUSEC_SCI_MASK BIT(6)
> > +#define ASUSEC_SMI_MASK BIT(7)
> > +
> > +/* SMI notification codes */
> > +#define ASUSEC_SMI_POWER_NOTIFY 0x31 /* [un]plugging USB cable */
> > +#define ASUSEC_SMI_HANDSHAKE 0x50 /* response to ec_req edge */
> > +#define ASUSEC_SMI_WAKE 0x53
> > +#define ASUSEC_SMI_RESET 0x5f
> > +#define ASUSEC_SMI_ADAPTER_EVENT 0x60 /* [un]plugging charger to dock */
> > +#define ASUSEC_SMI_BACKLIGHT_ON 0x63
> > +#define ASUSEC_SMI_AUDIO_DOCK_IN 0x70
> > +
> > +#define ASUSEC_SMI_ACTION(code) (ASUSEC_SMI_MASK | ASUSEC_OBF_MASK | \
> > + (ASUSEC_SMI_##code << 8))
> > +
> > /* control register [0x0A] layout */
> > #define ASUSEC_CTL_SIZE 8
> >
> > +/*
> > + * EC reports power from 40-pin connector in the LSB of the control
> > + * register. The following values have been observed (xor 0x02):
> > + *
> > + * PAD-ec no-plug 0x40 / PAD-ec DOCK 0x20 / DOCK-ec no-plug 0x40
> > + * PAD-ec AC 0x25 / PAD-ec DOCK+AC 0x24 / DOCK-ec AC 0x25
> > + * PAD-ec USB 0x45 / PAD-ec DOCK+USB 0x24 / DOCK-ec USB 0x41
> > + */
> > +
> > +#define ASUSEC_CTL_DIRECT_POWER_SOURCE BIT_ULL(0)
> > +#define ASUSEC_STAT_CHARGING BIT_ULL(2)
> > +#define ASUSEC_CTL_FULL_POWER_SOURCE BIT_ULL(5)
> > +#define ASUSEC_CTL_SUSB_MODE BIT_ULL(11)
> > +#define ASUSEC_CMD_SUSPEND_S3 BIT_ULL(41)
> > +#define ASUSEC_CTL_TEST_DISCHARGE BIT_ULL(43)
> > +#define ASUSEC_CMD_SUSPEND_INHIBIT BIT_ULL(45)
> > +#define ASUSEC_CTL_FACTORY_MODE BIT_ULL(46)
> > +#define ASUSEC_CTL_KEEP_AWAKE BIT_ULL(47)
> > +#define ASUSEC_CTL_USB_CHARGE BIT_ULL(50)
> > +#define ASUSEC_CMD_SWITCH_HDMI BIT_ULL(70)
> > +#define ASUSEC_CMD_WIN_SHUTDOWN BIT_ULL(76)
> > +
> > +#define ASUSEC_DOCKRAM_INFO_MODEL 0x01
> > +#define ASUSEC_DOCKRAM_INFO_FW 0x02
> > +#define ASUSEC_DOCKRAM_INFO_CFGFMT 0x03
> > +#define ASUSEC_DOCKRAM_INFO_HW 0x04
> > #define ASUSEC_DOCKRAM_CONTROL 0x0a
> > +#define ASUSEC_DOCKRAM_BATT_CTL 0x14
> > +
> > +#define ASUSEC_WRITE_BUF 0x64
> > +#define ASUSEC_READ_BUF 0x6A
> >
> > /* dockram comm */
> > int asus_dockram_read(struct i2c_client *client, int reg, char *buf);
> > @@ -21,4 +83,80 @@ int asus_dockram_access_ctl(struct i2c_client *client,
> > struct i2c_client *devm_asus_dockram_get(struct device *parent);
> > void asus_dockram_debugfs_init(struct i2c_client *client,
> > struct dentry *debugfs_root);
> > +
> > +/* EC public API */
> > +
> > +/**
> > + * cell_to_ec - Request the shared ASUS EC structure via a subdevice's pdev.
> > + * @pdev: EC subdevice pdev requesting access to the shared ASUS EC structure.
> > + *
> > + * Returns a pointer to the asusec_info structure.
> > + */
> > +static inline struct asusec_info *cell_to_ec(struct platform_device *pdev)
> > +{
> > + return dev_get_drvdata(pdev->dev.parent);
> > +}
> > +
> > +/**
> > + * asus_ec_get_ctl - Read from the DockRAM control register.
> > + * @ec: Pointer to the shared ASUS EC structure.
> > + * @out: Pointer to the variable where the register value will be stored.
> > + *
> > + * Performs a control register read and stores the value in @out.
> > + *
> > + * Return: 0 on success, or a negative errno code on failure.
> > + */
> > +static inline int asus_ec_get_ctl(const struct asusec_info *ec, u64 *out)
> > +{
> > + return asus_dockram_access_ctl(ec->dockram, out, 0, 0);
> > +}
> > +
> > +/**
> > + * asus_ec_update_ctl - Update the DockRAM control register.
> > + * @ec: Pointer to the shared ASUS EC structure.
> > + * @mask: Bitmask of bits to be cleared.
> > + * @xor: Bitmask of bits to be toggled or set (via XOR).
> > + *
> > + * Performs a read-modify-write update on the control register using
> > + * the provided @mask and @xor values.
> > + *
> > + * Return: 0 on success, or a negative errno code on failure.
> > + */
> > +static inline int asus_ec_update_ctl(const struct asusec_info *ec,
> > + u64 mask, u64 xor)
> > +{
> > + return asus_dockram_access_ctl(ec->dockram, NULL, mask, xor);
> > +}
> > +
> > +/**
> > + * asus_ec_set_ctl_bits - Sets bits of the DockRAM control register.
> > + * @ec: Pointer to the shared ASUS EC structure.
> > + * @mask: Bitmask of bits to be set.
> > + *
> > + * Sets bits of the control register using the provided @mask value.
> > + *
> > + * Return: 0 on success, or a negative errno code on failure.
> > + */
> > +static inline int asus_ec_set_ctl_bits(const struct asusec_info *ec, u64 mask)
> > +{
> > + return asus_dockram_access_ctl(ec->dockram, NULL, mask, mask);
> > +}
> > +
> > +/**
> > + * asus_ec_clear_ctl_bits - Clears bits of the DockRAM control register.
> > + * @ec: Pointer to the shared ASUS EC structure.
> > + * @mask: Bitmask of bits to be cleared.
> > + *
> > + * Clears bits of the control register using the provided @mask value.
> > + *
> > + * Return: 0 on success, or a negative errno code on failure.
> > + */
> > +static inline int asus_ec_clear_ctl_bits(const struct asusec_info *ec, u64 mask)
> > +{
> > + return asus_dockram_access_ctl(ec->dockram, NULL, mask, 0);
> > +}
>
> This is all abstraction for he sake of abstraction.
>
Is this nitpicking for the sake of nitpicking? All 3 are used by
subdevices and have proper descriptions. According to your logic maybe
all clear, set and setclr wrappers should be removed across kernel as
well. And all access functions can be dropped to xfer covers
everything.
> > +int asus_ec_i2c_command(const struct asusec_info *ec, u16 data);
> > +int devm_asus_ec_register_notifier(struct platform_device *dev,
> > + struct notifier_block *nb);
> > #endif /* __MISC_ASUS_EC_H */
> > --
> > 2.51.0
> >
> >
>
> --
> Lee Jones [李琼斯]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox