* Re: [PATCH 2/3] Input: add support for Goodix GTX8 Touchscreen ICs
From: Alexander Koskovich @ 2026-02-18 0:19 UTC (permalink / raw)
To: aelin
Cc: conor+dt, devicetree, dmitry.torokhov, hansg, krzk+dt,
linux-input, linux-kernel, linux, neil.armstrong, pc1598,
phone-devel, robh, rydberg, ~postmarketos/upstreaming,
Alexander Koskovich
Validated on the ASUS ROG Phone 3 (GT9896).
Tested-by: Alexander Koskovich <AKoskovich@pm.me>
^ permalink raw reply
* [PATCH 1/3] dt-bindings: input: document Goodix GTX8 Touchscreen ICs
From: Aelin Reidel @ 2026-02-17 23:50 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Hans de Goede, Neil Armstrong, Henrik Rydberg
Cc: linux-input, devicetree, linux-kernel, linux, phone-devel,
~postmarketos/upstreaming, Aelin Reidel
In-Reply-To: <20260218-gtx8-v1-0-0d575b3dedc5@mainlining.org>
Document the Goodix GT9886 and GT9896 which are part of the GTX8 series
of Touchscreen controller ICs from Goodix.
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Aelin Reidel <aelin@mainlining.org>
---
.../bindings/input/touchscreen/goodix,gt9886.yaml | 71 ++++++++++++++++++++++
1 file changed, 71 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/touchscreen/goodix,gt9886.yaml b/Documentation/devicetree/bindings/input/touchscreen/goodix,gt9886.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..6307495c2746313cfc32cdbb701455d1596be435
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/goodix,gt9886.yaml
@@ -0,0 +1,71 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/touchscreen/goodix,gt9886.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Goodix GTX8 series touchscreen controller
+
+maintainers:
+ - Aelin Reidel <aelin@mainlining.org>
+
+allOf:
+ - $ref: touchscreen.yaml#
+
+properties:
+ compatible:
+ enum:
+ - goodix,gt9886
+ - goodix,gt9896
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ reset-gpios:
+ maxItems: 1
+
+ avdd-supply:
+ description: Analog power supply regulator on AVDD pin
+
+ vddio-supply:
+ description: power supply regulator on VDDIO pin
+
+ touchscreen-inverted-x: true
+ touchscreen-inverted-y: true
+ touchscreen-size-x: true
+ touchscreen-size-y: true
+ touchscreen-swapped-x-y: true
+
+additionalProperties: false
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - avdd-supply
+ - touchscreen-size-x
+ - touchscreen-size-y
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ #include <dt-bindings/gpio/gpio.h>
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ touchscreen@5d {
+ compatible = "goodix,gt9886";
+ reg = <0x5d>;
+ interrupt-parent = <&gpio>;
+ interrupts = <9 IRQ_TYPE_LEVEL_LOW>;
+ reset-gpios = <&gpio1 8 GPIO_ACTIVE_LOW>;
+ avdd-supply = <&ts_avdd>;
+ touchscreen-size-x = <1080>;
+ touchscreen-size-y = <2340>;
+ };
+ };
+
+...
--
2.53.0
^ permalink raw reply related
* [PATCH 0/3] Input: add initial support for Goodix GTX8 touchscreen ICs
From: Aelin Reidel @ 2026-02-17 23:50 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Hans de Goede, Neil Armstrong, Henrik Rydberg
Cc: linux-input, devicetree, linux-kernel, linux, phone-devel,
~postmarketos/upstreaming, Aelin Reidel, Piyush Raj Chouhan
These ICs support SPI and I2C interfaces, up to 10 finger touch, stylus
and gesture events.
This driver is derived from the Goodix gtx8_driver_linux available at
[1] and only supports the GT9886 and GT9896 ICs present in the Xiaomi
Mi 9T and Xiaomi Redmi Note 10 Pro smartphones.
The current implementation only supports Normandy and Yellowstone type
ICs, aka only GT9886 and GT9896. It is also limited to I2C only, since I
don't have a device with GTX8 over SPI at hand. Adding support for SPI
should be fairly easy in the future, since the code uses a regmap.
Support for advanced features like:
- Firmware updates
- Stylus events
- Gesture events
- Nanjing IC support
is not included in current version.
The current support requires a previously flashed firmware to be
present.
As I did not have access to datasheets for these ICs, I extracted the
addresses from a couple of config files using a small tool [2]. The
addresses are identical for the same IC families in all configs I
observed, however not all of them make sense and I stubbed out firmware
request support due to this.
I've taken a lot of inspiration from the goodix_berlin driver, but the
Berlin and GTX8 series of touchscreen ICs differ quite a bit. The driver
architecture is the same overall, i.e. the power-up sequence and general
concepts are the mostly same, but it is very clear that they are
different generations when looking at it in more detail.
Some of the differences:
- There is no equivalent to the bootoption reg that I can find in the
public GTX8 drivers
- Firmware version struct layout is different yet again
- GTX8 does not expose IC information at runtime as far as I can tell
- The checksum method differs yet again
- The vendor driver reads only 1 touch upfront rather than 2
- Register addresses are 16-bit on GTX8 and 32-bit on Berlin
- Firmware requests don't appear to really exist on GTX8
From what I can tell, the evolution seems to be:
Normandy -> Yellowstone -> Berlin
since Normandy and Yellowstone are already quite different (especially
with the way checksums work) and Yellowstone has a couple of things
(checksum, fw_version) that appear similar to Berlin series ICs.
I've tried to make the Berlin driver work for GTX8 ICs before, but
they're so different (and I lack documentation for registers to perhaps
make some parts work on GTX8) that I'd rather support these ICs in a new
and tiny driver. I hope that makes sense. I took heavy inspiration from
the Berlin driver, but the only parts that are really common between
them are very trivial things like e.g. the input dev config or power on,
which I don't think are worth putting in a separate header.
[1] https://github.com/goodix/gtx8_driver_linux
[2] https://github.com/sm7150-mainline/goodix-cfg-bin
Signed-off-by: Aelin Reidel <aelin@mainlining.org>
---
Changes in v1 (post-RFC):
- Drop RFC prefix, the series has been tested enough and works well
as-is
- Update my name and email address
- Add some reasoning for a new driver to the cover letter
- Add Rob's R-b on the dt-bindings patch
- Add Piyush's T-b to the driver patch
- Link to RFC: https://lore.kernel.org/r/20250918-gtx8-v1-0-cba879c84775@mainlining.org
---
Aelin Reidel (3):
dt-bindings: input: document Goodix GTX8 Touchscreen ICs
Input: add support for Goodix GTX8 Touchscreen ICs
MAINTAINERS: add an entry for Goodix GTX8 Touchscreen driver
.../bindings/input/touchscreen/goodix,gt9886.yaml | 71 +++
MAINTAINERS | 7 +
drivers/input/touchscreen/Kconfig | 15 +
drivers/input/touchscreen/Makefile | 1 +
drivers/input/touchscreen/goodix_gtx8.c | 562 +++++++++++++++++++++
drivers/input/touchscreen/goodix_gtx8.h | 137 +++++
6 files changed, 793 insertions(+)
---
base-commit: fe9e3edb6a215515d1148d32a5c445c5bdd7916f
change-id: 20250918-gtx8-59a50ccd78a5
Best regards,
--
Aelin Reidel <aelin@mainlining.org>
^ permalink raw reply
* [PATCH 2/3] Input: add support for Goodix GTX8 Touchscreen ICs
From: Aelin Reidel @ 2026-02-17 23:50 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Hans de Goede, Neil Armstrong, Henrik Rydberg
Cc: linux-input, devicetree, linux-kernel, linux, phone-devel,
~postmarketos/upstreaming, Aelin Reidel, Piyush Raj Chouhan
In-Reply-To: <20260218-gtx8-v1-0-0d575b3dedc5@mainlining.org>
Add initial support for the Goodix GTX8 touchscreen ICs.
These ICs support SPI and I2C interfaces, up to 10 finger touch, stylus
and gesture events.
This driver is derived from the Goodix gtx8_driver_linux available at
[1] and only supports the GT9886 and GT9896 ICs present in the Xiaomi
Mi 9T and Xiaomi Redmi Note 10 Pro smartphones.
The current implementation only supports Normandy and Yellowstone type
ICs, aka only GT9886 and GT9896. It is also limited to I2C only, since I
don't have a device with GTX8 over SPI at hand. Adding support for SPI
should be fairly easy in the future, since the code uses a regmap.
Support for advanced features like:
- Firmware updates
- Stylus events
- Gesture events
- Nanjing IC support
is not included in current version.
The current support requires a previously flashed firmware to be
present.
As I did not have access to datasheets for these ICs, I extracted the
addresses from a couple of config files using a small tool [2]. The
addresses are identical for the same IC families in all configs I
observed, however not all of them make sense and I stubbed out firmware
request support due to this.
[1] https://github.com/goodix/gtx8_driver_linux
[2] https://github.com/sm7150-mainline/goodix-cfg-bin
Tested-by: Piyush Raj Chouhan <pc1598@mainlining.org>
Signed-off-by: Aelin Reidel <aelin@mainlining.org>
---
drivers/input/touchscreen/Kconfig | 15 +
drivers/input/touchscreen/Makefile | 1 +
drivers/input/touchscreen/goodix_gtx8.c | 562 ++++++++++++++++++++++++++++++++
drivers/input/touchscreen/goodix_gtx8.h | 137 ++++++++
4 files changed, 715 insertions(+)
diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index 7d5b72ee07fa1313da39a625b5129a0459720865..099ccd3679383dcf037bc7c6e6a3dbf0741722b4 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -429,6 +429,21 @@ config TOUCHSCREEN_GOODIX_BERLIN_SPI
To compile this driver as a module, choose M here: the
module will be called goodix_berlin_spi.
+config TOUCHSCREEN_GOODIX_GTX8
+ tristate "Goodix GTX8 touchscreen"
+ depends on I2C
+ select REGMAP_I2C
+ help
+ Say Y here if you have a Goodix GTX8 IC connected to
+ your system via I2C. This driver supports Normandy and
+ Yellowstone ICs like the GT9886 and GT9896.
+ They are commonly found in mobile phones.
+
+ if unsure, say N.
+
+ To compile this driver as a module, choose M here: the
+ module will be called goodix_gtx8.
+
config TOUCHSCREEN_HIDEEP
tristate "HiDeep Touch IC"
depends on I2C
diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index ab9abd151078831a4b22d6998e00ef74fe01c356..9bcb8f01ea785dcbe2a22bd3293601dd4259ba1d 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -48,6 +48,7 @@ obj-$(CONFIG_TOUCHSCREEN_GOODIX) += goodix_ts.o
obj-$(CONFIG_TOUCHSCREEN_GOODIX_BERLIN_CORE) += goodix_berlin_core.o
obj-$(CONFIG_TOUCHSCREEN_GOODIX_BERLIN_I2C) += goodix_berlin_i2c.o
obj-$(CONFIG_TOUCHSCREEN_GOODIX_BERLIN_SPI) += goodix_berlin_spi.o
+obj-$(CONFIG_TOUCHSCREEN_GOODIX_GTX8) += goodix_gtx8.o
obj-$(CONFIG_TOUCHSCREEN_HIDEEP) += hideep.o
obj-$(CONFIG_TOUCHSCREEN_HIMAX_HX852X) += himax_hx852x.o
obj-$(CONFIG_TOUCHSCREEN_HYNITRON_CSTXXX) += hynitron_cstxxx.o
diff --git a/drivers/input/touchscreen/goodix_gtx8.c b/drivers/input/touchscreen/goodix_gtx8.c
new file mode 100644
index 0000000000000000000000000000000000000000..b210e866974e423b413ca1741b3b6c7df117b92f
--- /dev/null
+++ b/drivers/input/touchscreen/goodix_gtx8.c
@@ -0,0 +1,562 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Driver for Goodix GTX8 Touchscreens
+ *
+ * Copyright (c) 2019 - 2020 Goodix, Inc.
+ * Copyright (C) 2023 Linaro Ltd.
+ * Copyright (c) 2025 Aelin Reidel <aelin@mainlining.org>
+ *
+ * Based on gtx8_driver_linux vendor driver and goodix_berlin kernel driver.
+ *
+ * The driver currently relies on the pre-flashed firmware and only supports
+ * Normandy / Yellowstone ICs.
+ * Pen support is also missing.
+ */
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/input.h>
+#include <linux/input/mt.h>
+#include <linux/input/touchscreen.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/unaligned.h>
+
+#include "goodix_gtx8.h"
+
+static const struct regmap_config goodix_gtx8_regmap_conf = {
+ .reg_bits = 16,
+ .val_bits = 8,
+ .max_raw_read = I2C_MAX_TRANSFER_SIZE,
+ .max_raw_write = I2C_MAX_TRANSFER_SIZE,
+};
+
+/* vendor & product left unassigned here, should probably be updated from fw info */
+static const struct input_id goodix_gtx8_input_id = {
+ .bustype = BUS_I2C,
+};
+
+static bool goodix_gtx8_checksum_valid_normandy(const u8 *data, int size)
+{
+ u8 cal_checksum = 0;
+ int i;
+
+ if (size < GOODIX_GTX8_CHECKSUM_SIZE)
+ return false;
+
+ for (i = 0; i < size; i++)
+ cal_checksum += data[i];
+
+ return cal_checksum == 0;
+}
+
+static bool goodix_gtx8_checksum_valid_yellowstone(const u8 *data, int size)
+{
+ u16 cal_checksum = 0;
+ u16 r_checksum;
+ int i;
+
+ if (size < GOODIX_GTX8_CHECKSUM_SIZE)
+ return false;
+
+ for (i = 0; i < size - GOODIX_GTX8_CHECKSUM_SIZE; i++)
+ cal_checksum += data[i];
+
+ r_checksum = get_unaligned_be16(&data[i]);
+
+ return cal_checksum == r_checksum;
+}
+
+static int goodix_gtx8_get_remaining_contacts(struct goodix_gtx8_core *cd,
+ int n)
+{
+ size_t offset = cd->ic_data->header_size + GOODIX_GTX8_TOUCH_SIZE +
+ GOODIX_GTX8_CHECKSUM_SIZE;
+ u32 addr = cd->ic_data->touch_data_addr + offset;
+ int error;
+
+ error = regmap_raw_read(cd->regmap, addr, &cd->event_buffer[offset],
+ (n - 1) * GOODIX_GTX8_TOUCH_SIZE);
+ if (error) {
+ dev_err_ratelimited(cd->dev, "failed to get touch data, %d\n",
+ error);
+ return error;
+ }
+
+ return 0;
+}
+
+static void goodix_gtx8_report_state(struct goodix_gtx8_core *cd, u8 touch_num,
+ union goodix_gtx8_touch *touch_data)
+{
+ union goodix_gtx8_touch *t;
+ int i;
+ u8 finger_id;
+
+ for (i = 0; i < touch_num; i++) {
+ t = &touch_data[i];
+
+ if (cd->ic_data->ic_type == IC_TYPE_NORMANDY) {
+ input_mt_slot(cd->input_dev, t->normandy.finger_id);
+ input_mt_report_slot_state(cd->input_dev,
+ MT_TOOL_FINGER, true);
+
+ touchscreen_report_pos(cd->input_dev, &cd->props,
+ __le16_to_cpu(t->normandy.x),
+ __le16_to_cpu(t->normandy.y),
+ true);
+ input_report_abs(cd->input_dev, ABS_MT_TOUCH_MAJOR,
+ t->normandy.w);
+ } else {
+ finger_id = FIELD_GET(
+ GOODIX_GTX8_FINGER_ID_MASK_YELLOWSTONE,
+ t->yellowstone.finger_id);
+ input_mt_slot(cd->input_dev, finger_id);
+ input_mt_report_slot_state(cd->input_dev,
+ MT_TOOL_FINGER, true);
+
+ touchscreen_report_pos(cd->input_dev, &cd->props,
+ __be16_to_cpu(t->yellowstone.x),
+ __be16_to_cpu(t->yellowstone.y),
+ true);
+ input_report_abs(cd->input_dev, ABS_MT_TOUCH_MAJOR,
+ t->yellowstone.w);
+ }
+ }
+
+ input_mt_sync_frame(cd->input_dev);
+ input_sync(cd->input_dev);
+}
+
+static void goodix_gtx8_touch_handler(struct goodix_gtx8_core *cd, u8 touch_num,
+ union goodix_gtx8_touch *touch_data)
+{
+ int error;
+
+ touch_num = FIELD_GET(GOODIX_GTX8_TOUCH_COUNT_MASK, touch_num);
+
+ if (touch_num > GOODIX_GTX8_MAX_TOUCH) {
+ dev_warn(cd->dev, "invalid touch num %d\n", touch_num);
+ return;
+ }
+
+ if (touch_num > 1) {
+ /* read additional contact data if more than 1 touch event */
+ error = goodix_gtx8_get_remaining_contacts(cd, touch_num);
+ if (error)
+ return;
+ }
+
+ if (touch_num) {
+ /*
+ * Normandy checksum is for the entire read buffer,
+ * Yellowstone is only for the touch data (since header
+ * has a separate checksum)
+ */
+ if (cd->ic_data->ic_type == IC_TYPE_NORMANDY) {
+ int len = GOODIX_GTX8_HEADER_SIZE_NORMANDY +
+ touch_num * GOODIX_GTX8_TOUCH_SIZE +
+ GOODIX_GTX8_CHECKSUM_SIZE;
+ if (!goodix_gtx8_checksum_valid_normandy(
+ cd->event_buffer, len)) {
+ dev_err(cd->dev,
+ "touch data checksum error: %*ph\n",
+ len, cd->event_buffer);
+ return;
+ }
+ } else {
+ int len = touch_num * GOODIX_GTX8_TOUCH_SIZE +
+ GOODIX_GTX8_CHECKSUM_SIZE;
+ if (!goodix_gtx8_checksum_valid_yellowstone(
+ (u8 *)touch_data, len)) {
+ dev_err(cd->dev,
+ "touch data checksum error: %*ph\n",
+ len, (u8 *)touch_data);
+ return;
+ }
+ }
+ }
+
+ goodix_gtx8_report_state(cd, touch_num, touch_data);
+}
+
+static irqreturn_t goodix_gtx8_irq(int irq, void *data)
+{
+ struct goodix_gtx8_core *cd = data;
+ struct goodix_gtx8_event_normandy *ev_normandy;
+ struct goodix_gtx8_event_yellowstone *ev_yellowstone;
+ union goodix_gtx8_touch *touch_data;
+ int error;
+ u8 status, touch_num;
+
+ error = regmap_raw_read(
+ cd->regmap, cd->ic_data->touch_data_addr, cd->event_buffer,
+ cd->ic_data->header_size + GOODIX_GTX8_TOUCH_SIZE +
+ GOODIX_GTX8_CHECKSUM_SIZE);
+ if (error) {
+ dev_warn_ratelimited(
+ cd->dev, "failed to get event head data: %d\n", error);
+ goto out;
+ }
+
+ /*
+ * Both IC types have the same data in the header, just at different
+ * offsets
+ */
+ if (cd->ic_data->ic_type == IC_TYPE_NORMANDY) {
+ ev_normandy =
+ (struct goodix_gtx8_event_normandy *)cd->event_buffer;
+ status = ev_normandy->hdr.status;
+ touch_num = ev_normandy->hdr.touch_num;
+ touch_data = (union goodix_gtx8_touch *)ev_normandy->data;
+ } else {
+ ev_yellowstone = (struct goodix_gtx8_event_yellowstone *)
+ cd->event_buffer;
+ status = ev_yellowstone->hdr.status;
+ touch_num = ev_yellowstone->hdr.touch_num;
+ touch_data = (union goodix_gtx8_touch *)ev_yellowstone->data;
+ }
+
+ if (status == 0)
+ goto out;
+
+ /* Yellowstone ICs have a checksum for the header */
+ if (cd->ic_data->ic_type == IC_TYPE_YELLOWSTONE &&
+ !goodix_gtx8_checksum_valid_yellowstone(
+ cd->event_buffer, GOODIX_GTX8_HEADER_SIZE_YELLOWSTONE)) {
+ dev_warn_ratelimited(cd->dev,
+ "touch head checksum error: %*ph\n",
+ (int)GOODIX_GTX8_HEADER_SIZE_YELLOWSTONE,
+ cd->event_buffer);
+ goto out_clear;
+ }
+
+ if (status & GOODIX_GTX8_TOUCH_EVENT)
+ goodix_gtx8_touch_handler(cd, touch_num, touch_data);
+
+ if (status & GOODIX_GTX8_REQUEST_EVENT) {
+ /*
+ * All configs seen so far either set the firmware request
+ * address to 0 (Normandy) or have it equal the touch data
+ * address (Yellowstone). Neither seems correct, and this
+ * is not testable. Therefore it is currently omitted.
+ */
+ dev_dbg(cd->dev, "received request event, ignoring\n");
+ }
+
+out_clear:
+ /* Clear up status field */
+ regmap_write(cd->regmap, cd->ic_data->touch_data_addr, 0);
+
+out:
+ return IRQ_HANDLED;
+}
+
+static int goodix_gtx8_input_dev_config(struct goodix_gtx8_core *cd)
+{
+ struct input_dev *input_dev;
+ int error;
+
+ input_dev = devm_input_allocate_device(cd->dev);
+ if (!input_dev)
+ return -ENOMEM;
+
+ cd->input_dev = input_dev;
+ input_set_drvdata(input_dev, cd);
+
+ input_dev->name = "Goodix GTX8 Capacitive TouchScreen";
+ input_dev->phys = "input/ts";
+
+ input_dev->id = goodix_gtx8_input_id;
+
+ input_set_abs_params(cd->input_dev, ABS_MT_POSITION_X, 0, SZ_64K - 1, 0,
+ 0);
+ input_set_abs_params(cd->input_dev, ABS_MT_POSITION_Y, 0, SZ_64K - 1, 0,
+ 0);
+ input_set_abs_params(cd->input_dev, ABS_MT_TOUCH_MAJOR, 0, 255, 0, 0);
+
+ touchscreen_parse_properties(cd->input_dev, true, &cd->props);
+
+ error = input_mt_init_slots(cd->input_dev, GOODIX_GTX8_MAX_TOUCH,
+ INPUT_MT_DIRECT | INPUT_MT_DROP_UNUSED);
+ if (error)
+ return error;
+
+ error = input_register_device(cd->input_dev);
+ if (error)
+ return error;
+
+ return 0;
+}
+
+static int goodix_gtx8_read_version(struct goodix_gtx8_core *cd)
+{
+ int error;
+
+ /*
+ * The vendor driver reads a whole lot more data to calculate and
+ * verify a checksum. Without documentation, we don't know what
+ * most of that data is, so we only read the parts we know about
+ * and instead ensure their values are as expected
+ */
+ error = regmap_raw_read(cd->regmap, cd->ic_data->fw_version_addr,
+ &cd->fw_version, sizeof(cd->fw_version));
+ if (error) {
+ dev_err(cd->dev, "error reading fw version, %d\n", error);
+ return error;
+ }
+
+ /*
+ * Since we don't verify the checksum, do a basic check that the
+ * product ID meets expectations
+ */
+ if (memcmp(cd->fw_version.product_id, cd->ic_data->product_id,
+ sizeof(cd->fw_version.product_id))) {
+ dev_err(cd->dev, "unexpected product ID, got: %c%c%c%c\n",
+ cd->fw_version.product_id[0],
+ cd->fw_version.product_id[1],
+ cd->fw_version.product_id[2],
+ cd->fw_version.product_id[3]);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int goodix_gtx8_dev_confirm(struct goodix_gtx8_core *cd)
+{
+ u8 rx_buf[1];
+ int retry = 3;
+ int error;
+
+ while (retry--) {
+ /*
+ * test_addr appears to always be the touch_data_addr for
+ * Normandy, but it doesn't really matter since all we
+ * need is a valid address
+ */
+ error = regmap_raw_read(cd->regmap,
+ cd->ic_data->touch_data_addr, rx_buf,
+ sizeof(rx_buf));
+
+ if (!error)
+ return 0;
+
+ usleep_range(5000, 5100);
+ }
+
+ dev_err(cd->dev, "device confirm failed\n");
+
+ return -EINVAL;
+}
+
+static int goodix_gtx8_power_on(struct goodix_gtx8_core *cd)
+{
+ int error;
+
+ error = regulator_enable(cd->vddio);
+ if (error) {
+ dev_err(cd->dev, "Failed to enable VDDIO: %d\n", error);
+ return error;
+ }
+
+ error = regulator_enable(cd->avdd);
+ if (error) {
+ dev_err(cd->dev, "Failed to enable AVDD: %d\n", error);
+ goto err_vddio_disable;
+ }
+
+ /* Vendors usually configure the power on delay as 300ms */
+ msleep(GOODIX_GTX8_POWER_ON_DELAY_MS);
+
+ gpiod_set_value_cansleep(cd->reset_gpio, 0);
+
+ /* Vendor waits 5ms for firmware to initialize */
+ usleep_range(5000, 5100);
+
+ error = goodix_gtx8_dev_confirm(cd);
+ if (error)
+ goto err_dev_reset;
+
+ /* Vendor waits 100ms for firmware to fully boot */
+ msleep(GOODIX_GTX8_NORMAL_RESET_DELAY_MS);
+
+ return 0;
+
+err_dev_reset:
+ gpiod_set_value_cansleep(cd->reset_gpio, 1);
+ regulator_disable(cd->avdd);
+err_vddio_disable:
+ regulator_disable(cd->vddio);
+ return error;
+}
+
+static void goodix_gtx8_power_off(struct goodix_gtx8_core *cd)
+{
+ gpiod_set_value_cansleep(cd->reset_gpio, 1);
+ regulator_disable(cd->avdd);
+ regulator_disable(cd->vddio);
+}
+
+static int goodix_gtx8_suspend(struct device *dev)
+{
+ struct goodix_gtx8_core *cd = dev_get_drvdata(dev);
+
+ disable_irq(cd->irq);
+ goodix_gtx8_power_off(cd);
+
+ return 0;
+}
+
+static int goodix_gtx8_resume(struct device *dev)
+{
+ struct goodix_gtx8_core *cd = dev_get_drvdata(dev);
+ int error;
+
+ error = goodix_gtx8_power_on(cd);
+ if (error)
+ return error;
+
+ enable_irq(cd->irq);
+
+ return 0;
+}
+
+EXPORT_GPL_SIMPLE_DEV_PM_OPS(goodix_gtx8_pm_ops, goodix_gtx8_suspend,
+ goodix_gtx8_resume);
+
+static void goodix_gtx8_power_off_act(void *data)
+{
+ struct goodix_gtx8_core *cd = data;
+
+ goodix_gtx8_power_off(cd);
+}
+
+static int goodix_gtx8_probe(struct i2c_client *client)
+{
+ struct goodix_gtx8_core *cd;
+ struct regmap *regmap;
+ int error;
+
+ cd = devm_kzalloc(&client->dev, sizeof(*cd), GFP_KERNEL);
+ if (!cd)
+ return -ENOMEM;
+
+ regmap = devm_regmap_init_i2c(client, &goodix_gtx8_regmap_conf);
+ if (IS_ERR(regmap))
+ return PTR_ERR(regmap);
+
+ cd->dev = &client->dev;
+ cd->irq = client->irq;
+ cd->regmap = regmap;
+ cd->ic_data = i2c_get_match_data(client);
+
+ cd->event_buffer =
+ devm_kzalloc(cd->dev, cd->ic_data->event_size, GFP_KERNEL);
+ if (!cd->event_buffer)
+ return -ENOMEM;
+
+ cd->reset_gpio =
+ devm_gpiod_get_optional(cd->dev, "reset", GPIOD_OUT_HIGH);
+ if (IS_ERR(cd->reset_gpio))
+ return dev_err_probe(cd->dev, PTR_ERR(cd->reset_gpio),
+ "Failed to request reset GPIO\n");
+
+ cd->avdd = devm_regulator_get(cd->dev, "avdd");
+ if (IS_ERR(cd->avdd))
+ return dev_err_probe(cd->dev, PTR_ERR(cd->avdd),
+ "Failed to request AVDD regulator\n");
+
+ cd->vddio = devm_regulator_get(cd->dev, "vddio");
+ if (IS_ERR(cd->vddio))
+ return dev_err_probe(cd->dev, PTR_ERR(cd->vddio),
+ "Failed to request VDDIO regulator\n");
+
+ error = goodix_gtx8_power_on(cd);
+ if (error) {
+ dev_err(cd->dev, "failed power on");
+ return error;
+ }
+
+ error = devm_add_action_or_reset(cd->dev, goodix_gtx8_power_off_act,
+ cd);
+ if (error)
+ return error;
+
+ error = goodix_gtx8_read_version(cd);
+ if (error) {
+ dev_err(cd->dev, "failed to get version info");
+ return error;
+ }
+
+ error = goodix_gtx8_input_dev_config(cd);
+ if (error) {
+ dev_err(cd->dev, "failed to set input device");
+ return error;
+ }
+
+ error = devm_request_threaded_irq(cd->dev, cd->irq, NULL,
+ goodix_gtx8_irq, IRQF_ONESHOT,
+ "goodix-gtx8", cd);
+ if (error) {
+ dev_err(cd->dev, "request threaded IRQ failed: %d\n", error);
+ return error;
+ }
+
+ dev_set_drvdata(cd->dev, cd);
+
+ dev_dbg(cd->dev,
+ "Goodix GT%c%c%c%c Touchscreen Controller, Version %d.%d.%d.%d\n",
+ cd->fw_version.product_id[0], cd->fw_version.product_id[1],
+ cd->fw_version.product_id[2], cd->fw_version.product_id[3],
+ cd->fw_version.fw_version[0], cd->fw_version.fw_version[1],
+ cd->fw_version.fw_version[2], cd->fw_version.fw_version[3]);
+
+ return 0;
+}
+
+static const struct goodix_gtx8_ic_data gt9886_data = {
+ .event_size = GOODIX_GTX8_EVENT_SIZE_NORMANDY,
+ .fw_version_addr = GOODIX_GTX8_FW_VERSION_ADDR_NORMANDY,
+ .header_size = GOODIX_GTX8_HEADER_SIZE_NORMANDY,
+ .ic_type = IC_TYPE_NORMANDY,
+ .product_id = { '9', '8', '8', '6' },
+ .touch_data_addr = GOODIX_GTX8_TOUCH_DATA_ADDR_NORMANDY,
+};
+
+static const struct goodix_gtx8_ic_data gt9896_data = {
+ .event_size = GOODIX_GTX8_EVENT_SIZE_YELLOWSTONE,
+ .fw_version_addr = GOODIX_GTX8_FW_VERSION_ADDR_YELLOWSTONE,
+ .header_size = GOODIX_GTX8_HEADER_SIZE_YELLOWSTONE,
+ .ic_type = IC_TYPE_YELLOWSTONE,
+ .product_id = { '9', '8', '9', '6' },
+ .touch_data_addr = GOODIX_GTX8_TOUCH_DATA_ADDR_YELLOWSTONE,
+};
+
+static const struct i2c_device_id goodix_gtx8_i2c_id[] = {
+ { .name = "gt9886", .driver_data = (long)>9886_data },
+ { .name = "gt9896", .driver_data = (long)>9896_data },
+ {},
+};
+MODULE_DEVICE_TABLE(i2c, goodix_gtx8_i2c_id);
+
+static const struct of_device_id goodix_gtx8_of_match[] = {
+ { .compatible = "goodix,gt9886", .data = >9886_data },
+ { .compatible = "goodix,gt9896", .data = >9896_data },
+ {},
+};
+MODULE_DEVICE_TABLE(of, goodix_gtx8_of_match);
+
+static struct i2c_driver goodix_gtx8_driver = {
+ .probe = goodix_gtx8_probe,
+ .id_table = goodix_gtx8_i2c_id,
+ .driver = {
+ .name = "goodix-gtx8",
+ .of_match_table = of_match_ptr(goodix_gtx8_of_match),
+ .pm = pm_sleep_ptr(&goodix_gtx8_pm_ops),
+ },
+};
+module_i2c_driver(goodix_gtx8_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Goodix GTX8 Touchscreen driver");
+MODULE_AUTHOR("Aelin Reidel <aelin@mainlining.org>");
diff --git a/drivers/input/touchscreen/goodix_gtx8.h b/drivers/input/touchscreen/goodix_gtx8.h
new file mode 100644
index 0000000000000000000000000000000000000000..79e79988869b46a3fc70fa64b4698cdb1d7a0394
--- /dev/null
+++ b/drivers/input/touchscreen/goodix_gtx8.h
@@ -0,0 +1,137 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __GOODIX_GTX8_H__
+#define __GOODIX_GTX8_H__
+
+#define GOODIX_GTX8_NORMAL_RESET_DELAY_MS 100
+#define GOODIX_GTX8_POWER_ON_DELAY_MS 300
+
+#define GOODIX_GTX8_TOUCH_EVENT BIT(7)
+#define GOODIX_GTX8_REQUEST_EVENT BIT(6)
+#define GOODIX_GTX8_TOUCH_COUNT_MASK GENMASK(3, 0)
+#define GOODIX_GTX8_FINGER_ID_MASK_YELLOWSTONE GENMASK(7, 4)
+
+#define GOODIX_GTX8_MAX_TOUCH 10
+#define GOODIX_GTX8_CHECKSUM_SIZE sizeof(u16)
+
+#define GOODIX_GTX8_FW_VERSION_ADDR_NORMANDY 0x4535
+#define GOODIX_GTX8_FW_VERSION_ADDR_YELLOWSTONE 0x4022
+#define GOODIX_GTX8_TOUCH_DATA_ADDR_NORMANDY 0x4100
+#define GOODIX_GTX8_TOUCH_DATA_ADDR_YELLOWSTONE 0x4180
+
+#define I2C_MAX_TRANSFER_SIZE 256
+
+enum goodix_gtx8_ic_type {
+ IC_TYPE_NORMANDY,
+ IC_TYPE_YELLOWSTONE,
+};
+
+struct goodix_gtx8_ic_data {
+ size_t event_size;
+ /*
+ * This is technically not the firmware version address
+ * referenced in the vendor driver, but rather the
+ * address of the product ID part. The meaning of the
+ * other parts is unknown and they are therefore omitted
+ * for now.
+ */
+ int fw_version_addr;
+ size_t header_size;
+ enum goodix_gtx8_ic_type ic_type;
+ char product_id[4];
+ int touch_data_addr;
+};
+
+struct goodix_gtx8_header_normandy {
+ u8 status;
+ /* Only the lower 4 bits are actually used */
+ u8 touch_num;
+};
+#define GOODIX_GTX8_HEADER_SIZE_NORMANDY \
+ sizeof(struct goodix_gtx8_header_normandy)
+
+struct goodix_gtx8_header_yellowstone {
+ u8 status;
+ /* Most likely unused */
+ u8 __unknown1;
+ /* Only the lower 4 bits are actually used */
+ u8 touch_num;
+ /* Most likely unused */
+ u8 __unknown2[3];
+ __le16 checksum;
+} __packed __aligned(1);
+#define GOODIX_GTX8_HEADER_SIZE_YELLOWSTONE \
+ sizeof(struct goodix_gtx8_header_yellowstone)
+
+struct goodix_gtx8_touch_normandy {
+ u8 finger_id;
+ __le16 x;
+ __le16 y;
+ u8 w;
+ u8 __unknown[2];
+} __packed __aligned(1);
+
+struct goodix_gtx8_touch_yellowstone {
+ /*
+ * Only the upper 4 bits are used, lower 4 bits are
+ * probably the sensor ID.
+ */
+ u8 finger_id;
+ u8 __unknown1;
+ __be16 x;
+ __be16 y;
+ /*
+ * Vendor driver claims that this is a single __be16,
+ * but testing shows that it likely isn't.
+ */
+ u8 __unknown2;
+ u8 w;
+} __packed __aligned(1);
+
+union goodix_gtx8_touch {
+ struct goodix_gtx8_touch_normandy normandy;
+ struct goodix_gtx8_touch_yellowstone yellowstone;
+};
+#define GOODIX_GTX8_TOUCH_SIZE sizeof(union goodix_gtx8_touch)
+
+struct goodix_gtx8_event_normandy {
+ struct goodix_gtx8_header_normandy hdr;
+ /* The data below is u16 aligned */
+ u8 data[GOODIX_GTX8_TOUCH_SIZE * GOODIX_GTX8_MAX_TOUCH +
+ GOODIX_GTX8_CHECKSUM_SIZE];
+};
+#define GOODIX_GTX8_EVENT_SIZE_NORMANDY \
+ sizeof(struct goodix_gtx8_event_normandy)
+
+struct goodix_gtx8_event_yellowstone {
+ struct goodix_gtx8_header_yellowstone hdr;
+ /* The data below is u16 aligned */
+ u8 data[GOODIX_GTX8_TOUCH_SIZE * GOODIX_GTX8_MAX_TOUCH +
+ GOODIX_GTX8_CHECKSUM_SIZE];
+};
+#define GOODIX_GTX8_EVENT_SIZE_YELLOWSTONE \
+ sizeof(struct goodix_gtx8_event_yellowstone)
+
+struct goodix_gtx8_fw_version {
+ /* 4 digits IC number */
+ char product_id[4];
+ /* Most likely unused */
+ u8 __unknown[4];
+ /* Four components version number */
+ u8 fw_version[4];
+};
+
+struct goodix_gtx8_core {
+ struct device *dev;
+ struct regmap *regmap;
+ struct regulator *avdd;
+ struct regulator *vddio;
+ struct gpio_desc *reset_gpio;
+ struct touchscreen_properties props;
+ struct goodix_gtx8_fw_version fw_version;
+ struct input_dev *input_dev;
+ int irq;
+ const struct goodix_gtx8_ic_data *ic_data;
+ u8 *event_buffer;
+};
+
+#endif
--
2.53.0
^ permalink raw reply related
* [PATCH 3/3] MAINTAINERS: add an entry for Goodix GTX8 Touchscreen driver
From: Aelin Reidel @ 2026-02-17 23:50 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Hans de Goede, Neil Armstrong, Henrik Rydberg
Cc: linux-input, devicetree, linux-kernel, linux, phone-devel,
~postmarketos/upstreaming, Aelin Reidel
In-Reply-To: <20260218-gtx8-v1-0-0d575b3dedc5@mainlining.org>
Add MAINTAINERS entry for the Goodix GTX8 Touchscreen IC driver.
Signed-off-by: Aelin Reidel <aelin@mainlining.org>
---
MAINTAINERS | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index afbba2fdc0f49abb6d0d1877a5e161266715f275..cb0f19d622e25cd9a5ceb8fc1e781a1f2232c64a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10831,6 +10831,13 @@ M: Maud Spierings <maudspierings@gocontroll.com>
S: Maintained
F: Documentation/devicetree/bindings/connector/gocontroll,moduline-module-slot.yaml
+GOODIX GTX8 TOUCHSCREEN
+M: Aelin Reidel <aelin@mainlining.org>
+L: linux-input@vger.kernel.org
+S: Maintained
+F: Documentation/devicetree/bindings/input/touchscreen/goodix,gt9886.yaml
+F: drivers/input/touchscreen/goodix_gtx8*
+
GOODIX TOUCHSCREEN
M: Hans de Goede <hansg@kernel.org>
L: linux-input@vger.kernel.org
--
2.53.0
^ permalink raw reply related
* [syzbot] Monthly input report (Feb 2026)
From: syzbot @ 2026-02-17 22:04 UTC (permalink / raw)
To: linux-input, linux-kernel, syzkaller-bugs
Hello input maintainers/developers,
This is a 31-day syzbot report for the input subsystem.
All related reports/information can be found at:
https://syzkaller.appspot.com/upstream/s/input
During the period, 0 new issues were detected and 0 were fixed.
In total, 22 issues are still open and 63 have already been fixed.
Some of the still happening issues:
Ref Crashes Repro Title
<1> 3448 Yes WARNING in cm109_urb_irq_callback/usb_submit_urb
https://syzkaller.appspot.com/bug?extid=2d6d691af5ab4b7e66df
<2> 1677 No possible deadlock in evdev_pass_values (2)
https://syzkaller.appspot.com/bug?extid=13d3cb2a3dc61e6092f5
<3> 430 Yes KASAN: slab-out-of-bounds Read in mcp2221_raw_event (2)
https://syzkaller.appspot.com/bug?extid=1018672fe70298606e5f
<4> 116 Yes WARNING in cm109_input_open/usb_submit_urb (3)
https://syzkaller.appspot.com/bug?extid=ac0f9c4cc1e034160492
<5> 91 Yes possible deadlock in uinput_request_submit
https://syzkaller.appspot.com/bug?extid=159077b1355b8cd72757
<6> 59 No KASAN: slab-use-after-free Read in report_descriptor_read
https://syzkaller.appspot.com/bug?extid=bc537ca7a0efe33988eb
<7> 20 Yes INFO: task hung in __input_unregister_device (5)
https://syzkaller.appspot.com/bug?extid=78e2288f58b881ed3c45
---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
To disable reminders for individual bugs, reply with the following command:
#syz set <Ref> no-reminders
To change bug's subsystems, reply with:
#syz set <Ref> subsystems: new-subsystem
You may send multiple commands in a single email message.
^ permalink raw reply
* Re: [PATCH 0/3] HID: Fix some memory leaks in drivers/hid
From: Günther Noack @ 2026-02-17 20:08 UTC (permalink / raw)
To: Benjamin Tissoires; +Cc: Jiri Kosina, linux-input, linux-kernel
In-Reply-To: <aZS0OAaSPhX2pJ6l@plouf>
Hello!
On Tue, Feb 17, 2026 at 07:36:46PM +0100, Benjamin Tissoires wrote:
> On Feb 17 2026, Günther Noack wrote:
> > These patches fix a few memory leaks in HID report descriptor fixups.
> >
> > FWIW, a good ad-hoc way to look for usages of allocation functions in
> > these is:
> >
> > awk '/static.*report_fixup.*/,/^}/ { print FILENAME, $0 }' drivers/hid/hid-*.c \
> > | grep -E '(malloc|kzalloc|kcalloc|kmemdup)'
> >
> > The devm_* variants are safe in this context, because they tie the
> > allocated memory to the lifetime of the driver.
>
> No. Look at hid_close_report() in drivers/hid/hid-core.c.
>
> HID still hasn't fully migrated to devm, so as a rule of thumb, if you
> change a kzalloc into a devm_kzalloc, you are getting into troubles
> unless you fix the all the kfree path.
OK, I have not verified where the devm-allocated objects get freed up.
If devm_*() is not possible here, then the drivers hid-asus.c and
hid-gembird.c have two additional memory leaks, because they do that.
$ awk '/static.*report_fixup.*/,/^}/ { print FILENAME, $0 }' drivers/hid/hid-*.c | grep -E 'alloc'
drivers/hid/hid-asus.c new_rdesc = devm_kzalloc(&hdev->dev, new_size, GFP_KERNEL);
drivers/hid/hid-gembird.c new_rdesc = devm_kzalloc(&hdev->dev, new_size, GFP_KERNEL);
(That is without my threee patches)
> > For transparency, I generated these commits with Gemini-CLI,
> > starting with this prompt:
> >
> > We are working in the Linux kernel. In the HID drivers in
> > `drivers/hid/hid-*.c`, the `report_fixup` driver hook is a function
> > that gets a byte buffer (with size) as input and that may modify that
> > byte buffer, and optionally return a pointer to a new byte buffer and
> > update the size. The returned value is *not* memory-managed by the
> > caller though and will not be freed subsequently. When the
>
> If the memory is *not* managed, why would gemini converts kzalloc into
> devm variants without changing the kfree paths????
I'm not sure I understand the question, it's not clear to me what you
mean by the "kfree paths".
I have seen usages of devm in other HID drivers and I was under the
impression that devm_* allocations would work in the HID subsystem to
allocate objects which are then freed automatically at a later point
when the device gets removed. Is that inaccurate?
> > `report_fixup` implementation allocates a new buffer and returns that,
> > that will not get freed by the caller.
>
> This is wrong. See hid_close_report(): if the new rdesc (after fixup)
> differs from the one initially set, there is an explicit call to
> kfree().
>
> -> there is no memleak AFAICT, and your prompt is wrong.
See my discussion in [1]. The pointer returned by report_fixup() is
immediately discarded in the position marked with (4). This is still
in the hid_open_report() function where the leak happens.
Let me know whether this makes sense. I'm happy to be corrected, but
so far, I still have the feeling that my reasoning is sound.
—Günther
[1] https://lore.kernel.org/all/aZTEnPEHcWEkoTJR@google.com/
^ permalink raw reply
* Re: [PATCH 3/3] HID: asus: avoid memory leak in asus_report_fixup()
From: Günther Noack @ 2026-02-17 19:51 UTC (permalink / raw)
To: Benjamin Tissoires; +Cc: Jiri Kosina, linux-input, linux-kernel
In-Reply-To: <aZSzASB_TC2RyQsR@plouf>
Hello!
On Tue, Feb 17, 2026 at 07:31:23PM +0100, Benjamin Tissoires wrote:
> On Feb 17 2026, Günther Noack wrote:
> > The asus_report_fixup() function was allocating a new buffer with kmemdup()
> > when growing the report descriptor but never freeing it. Switch to
> > devm_kzalloc() to ensure the memory is managed and freed automatically when
> > the device is removed.
>
> Actually this one is even worse: you can't use devm_kzalloc because
> hid-core.c will later call kfree(dev->rdesc) if dev->rdesc is different
> from the one provided by the low level driver. So we are going to have
> a double free.
The buffer returned by report_fixup() is duplicated first before
hid-core stores it in dev->rdesc. The pointer that report_fixup()
returns is not managed by the caller.
I elaborated in the response to the other patch in [1]. You can see
it in the source code in the position marked with (4).
[1] https://lore.kernel.org/all/aZTEnPEHcWEkoTJR@google.com/
> I really wonder if this was ever tested.
I only convinced myself by staring at the code, because I do not
happen to have the matching USB devices here. What it your usual
approach to verifying such changes? raw-gadget?
—Günther
^ permalink raw reply
* Re: [PATCH 1/3] HID: apple: avoid memory leak in apple_report_fixup()
From: Günther Noack @ 2026-02-17 19:42 UTC (permalink / raw)
To: Benjamin Tissoires; +Cc: Jiri Kosina, linux-input, linux-kernel
In-Reply-To: <aZSxeusondD26uN3@plouf>
Hello Benjamin!
On Tue, Feb 17, 2026 at 07:22:20PM +0100, Benjamin Tissoires wrote:
> On Feb 17 2026, Günther Noack wrote:
> > The apple_report_fixup() function was allocating a new buffer with
> > kmemdup() but never freeing it. Since the caller of report_fixup() already
> > provides a writable buffer and allows returning a pointer within that
> > buffer, we can just modify the descriptor in-place and return the adjusted
> > pointer.
> >
> > Assisted-by: Gemini-CLI:Google Gemini 3
> > Signed-off-by: Günther Noack <gnoack@google.com>
> > ---
> > drivers/hid/hid-apple.c | 4 +---
> > 1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
> > index 233e367cce1d..894adc23367b 100644
> > --- a/drivers/hid/hid-apple.c
> > +++ b/drivers/hid/hid-apple.c
> > @@ -686,9 +686,7 @@ static const __u8 *apple_report_fixup(struct hid_device *hdev, __u8 *rdesc,
> > hid_info(hdev,
> > "fixing up Magic Keyboard battery report descriptor\n");
> > *rsize = *rsize - 1;
> > - rdesc = kmemdup(rdesc + 1, *rsize, GFP_KERNEL);
> > - if (!rdesc)
> > - return NULL;
> > + rdesc = rdesc + 1;
>
> I might be wrong, but later we call free(dev->rdesc) on device removal.
> AFAICT, incrementing the pointer is undefined behavior.
>
> What we should do instead is probably a krealloc instead of a kmemdup.
>
> Same for all 3 patches.
Thanks for the review.
Let me try to address your three responses in one reply.
I am happy to accept it if I am wrong about this, but I don't
understand your reasoning. (This should go without saying, but maybe
is worth reiterating, I would not have sent this if I had not
convinced myself independently of LLM-assisted reasoning.)
Let me explain my reasoning based on the place where .report_fixup()
is called from, which is hid_open_report() in hid-core.c:
start = device->bpf_rdesc; /* (1) */
if (WARN_ON(!start))
return -ENODEV;
size = device->bpf_rsize;
if (device->driver->report_fixup) {
/*
* device->driver->report_fixup() needs to work
* on a copy of our report descriptor so it can
* change it.
*/
__u8 *buf = kmemdup(start, size, GFP_KERNEL); /* (2) */
if (buf == NULL)
return -ENOMEM;
start = device->driver->report_fixup(device, buf, &size); /* (3) */
/*
* The second kmemdup is required in case report_fixup() returns
* a static read-only memory, but we have no idea if that memory
* needs to be cleaned up or not at the end.
*/
start = kmemdup(start, size, GFP_KERNEL); /* (4) */
kfree(buf); /* (5) */
if (start == NULL)
return -ENOMEM;
}
device->rdesc = start;
device->rsize = size;
This function uses a slightly elaborate scheme to call .report_fixup:
(1) start is assigned to the original device->bpf_rdesc
(2) buf is assigned to a copy of the 'start' buffer (deallocated in (5)).
. It is done because buf is meant to be mutated by .report_fixup()
. (3) start = ...->report_fixup(..., buf, ...)
. (4) start = kmemdup(start, ...)
(5) deallocate buf
Importantly:
(a) The buffer buf passed to report_fixup() is a copy of the report
descriptor whose lifetime spans only from (2) to (5).
(b) The result of .report_fixup(), start, is immediately discarded in
(4) and reassigned to the start variable again.
From (b), we can see that the result of .report_fixup() does *not* get
deallocated by the caller, and thus, when the driver wants to return a
newly allocated array, is must also hold a reference to it so that it
can deallocate it later.
From (a), we can see that the report_fixup hook is free to manipulate
the contents of the buffer that it receives, but if we were to
*reallocate* it within report_fixup, as you are suggesting above, it
could become a double free:
* During realloc, the allocator would potentially have to move the
buffer to a place where there is enough space, freeing up the old
place and allocating a new place. [1]
* In (5), we would then pass the original (now deallocated) buf
pointer to kfree, leading to a double free.
If I were to describe the current interface of the hook
.report_fixup(dev, rdesc, rsize), it would be:
* report_fixup may modify rdesc[0] to rdesc[rsize-1]
* report_fixup may *not* deallocate rdesc
(ownership of rdesc stays with the caller)
* specifically, it may also not reallocate rdesc
(because that may imply a deallocation)
* report_fixup returns a pointer to a buffer for which it can
guarantee that it lives long enough for the kmemdup in (4), but
which will *not* be deallocated by the caller (see (b) above). The
three techniques I have found for that are:
* returning a subsection of the rdesc that it received
* returning a pointer into a statically allocated array
(e.g. motion_fixup() and ps3remote_fixup() in hid-sony.c)
* allocating it with a devm_*() function. My understanding was
that this ties it to the lifetime of the device. (e.g. the
QUIRK_G752_KEYBOARD case in hid-asus.c)
Honestly, I still think that this reasoning holds, but I am happy to
be convinced otherwise. Please let me know what you think.
—Günther
^ permalink raw reply
* Re: [PATCH 0/3] HID: Fix some memory leaks in drivers/hid
From: Benjamin Tissoires @ 2026-02-17 18:36 UTC (permalink / raw)
To: Günther Noack; +Cc: Jiri Kosina, linux-input, linux-kernel
In-Reply-To: <20260217160125.1097578-1-gnoack@google.com>
On Feb 17 2026, Günther Noack wrote:
> Hello!
>
> These patches fix a few memory leaks in HID report descriptor fixups.
>
> FWIW, a good ad-hoc way to look for usages of allocation functions in
> these is:
>
> awk '/static.*report_fixup.*/,/^}/ { print FILENAME, $0 }' drivers/hid/hid-*.c \
> | grep -E '(malloc|kzalloc|kcalloc|kmemdup)'
>
> The devm_* variants are safe in this context, because they tie the
> allocated memory to the lifetime of the driver.
No. Look at hid_close_report() in drivers/hid/hid-core.c.
HID still hasn't fully migrated to devm, so as a rule of thumb, if you
change a kzalloc into a devm_kzalloc, you are getting into troubles
unless you fix the all the kfree path.
>
> For transparency, I generated these commits with Gemini-CLI,
> starting with this prompt:
>
> We are working in the Linux kernel. In the HID drivers in
> `drivers/hid/hid-*.c`, the `report_fixup` driver hook is a function
> that gets a byte buffer (with size) as input and that may modify that
> byte buffer, and optionally return a pointer to a new byte buffer and
> update the size. The returned value is *not* memory-managed by the
> caller though and will not be freed subsequently. When the
If the memory is *not* managed, why would gemini converts kzalloc into
devm variants without changing the kfree paths????
> `report_fixup` implementation allocates a new buffer and returns that,
> that will not get freed by the caller.
This is wrong. See hid_close_report(): if the new rdesc (after fixup)
differs from the one initially set, there is an explicit call to
kfree().
-> there is no memleak AFAICT, and your prompt is wrong.
Cheers,
Benjamin
> Validate this assessment and
> fix up all HID drivers where that mistake is made.
>
> (and then a little bit of additional nudging for the details).
>
> —Günther
>
>
> Günther Noack (3):
> HID: apple: avoid memory leak in apple_report_fixup()
> HID: magicmouse: avoid memory leak in magicmouse_report_fixup()
> HID: asus: avoid memory leak in asus_report_fixup()
>
> drivers/hid/hid-apple.c | 4 +---
> drivers/hid/hid-asus.c | 15 +++++++++++----
> drivers/hid/hid-magicmouse.c | 4 +---
> 3 files changed, 13 insertions(+), 10 deletions(-)
>
> --
> 2.53.0.335.g19a08e0c02-goog
>
^ permalink raw reply
* Re: [PATCH 3/3] HID: asus: avoid memory leak in asus_report_fixup()
From: Benjamin Tissoires @ 2026-02-17 18:31 UTC (permalink / raw)
To: Günther Noack; +Cc: Jiri Kosina, linux-input, linux-kernel
In-Reply-To: <20260217160125.1097578-4-gnoack@google.com>
On Feb 17 2026, Günther Noack wrote:
> The asus_report_fixup() function was allocating a new buffer with kmemdup()
> when growing the report descriptor but never freeing it. Switch to
> devm_kzalloc() to ensure the memory is managed and freed automatically when
> the device is removed.
Actually this one is even worse: you can't use devm_kzalloc because
hid-core.c will later call kfree(dev->rdesc) if dev->rdesc is different
from the one provided by the low level driver. So we are going to have
a double free.
I really wonder if this was ever tested.
Cheers,
Benjamin
>
> Also fix a harmless out-of-bounds read by copying only the original
> descriptor size.
>
> Assisted-by: Gemini-CLI:Google Gemini 3
> Signed-off-by: Günther Noack <gnoack@google.com>
> ---
> drivers/hid/hid-asus.c | 15 +++++++++++----
> 1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index 8ffcd12038e8..7a08e964b9cc 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -1399,14 +1399,21 @@ static const __u8 *asus_report_fixup(struct hid_device *hdev, __u8 *rdesc,
> */
> if (*rsize == rsize_orig &&
> rdesc[offs] == 0x09 && rdesc[offs + 1] == 0x76) {
> - *rsize = rsize_orig + 1;
> - rdesc = kmemdup(rdesc, *rsize, GFP_KERNEL);
> - if (!rdesc)
> - return NULL;
> + __u8 *new_rdesc;
> +
> + new_rdesc = devm_kzalloc(&hdev->dev, rsize_orig + 1,
> + GFP_KERNEL);
> + if (!new_rdesc)
> + return rdesc;
>
> hid_info(hdev, "Fixing up %s keyb report descriptor\n",
> drvdata->quirks & QUIRK_T100CHI ?
> "T100CHI" : "T90CHI");
> +
> + memcpy(new_rdesc, rdesc, rsize_orig);
> + *rsize = rsize_orig + 1;
> + rdesc = new_rdesc;
> +
> memmove(rdesc + offs + 4, rdesc + offs + 2, 12);
> rdesc[offs] = 0x19;
> rdesc[offs + 1] = 0x00;
> --
> 2.53.0.335.g19a08e0c02-goog
>
^ permalink raw reply
* Re: [PATCH 1/3] HID: apple: avoid memory leak in apple_report_fixup()
From: Benjamin Tissoires @ 2026-02-17 18:22 UTC (permalink / raw)
To: Günther Noack; +Cc: Jiri Kosina, linux-input, linux-kernel
In-Reply-To: <20260217160125.1097578-2-gnoack@google.com>
On Feb 17 2026, Günther Noack wrote:
> The apple_report_fixup() function was allocating a new buffer with
> kmemdup() but never freeing it. Since the caller of report_fixup() already
> provides a writable buffer and allows returning a pointer within that
> buffer, we can just modify the descriptor in-place and return the adjusted
> pointer.
>
> Assisted-by: Gemini-CLI:Google Gemini 3
> Signed-off-by: Günther Noack <gnoack@google.com>
> ---
> drivers/hid/hid-apple.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
> index 233e367cce1d..894adc23367b 100644
> --- a/drivers/hid/hid-apple.c
> +++ b/drivers/hid/hid-apple.c
> @@ -686,9 +686,7 @@ static const __u8 *apple_report_fixup(struct hid_device *hdev, __u8 *rdesc,
> hid_info(hdev,
> "fixing up Magic Keyboard battery report descriptor\n");
> *rsize = *rsize - 1;
> - rdesc = kmemdup(rdesc + 1, *rsize, GFP_KERNEL);
> - if (!rdesc)
> - return NULL;
> + rdesc = rdesc + 1;
I might be wrong, but later we call free(dev->rdesc) on device removal.
AFAICT, incrementing the pointer is undefined behavior.
What we should do instead is probably a krealloc instead of a kmemdup.
Same for all 3 patches.
Cheers,
Benjamin
>
> rdesc[0] = 0x05;
> rdesc[1] = 0x01;
> --
> 2.53.0.335.g19a08e0c02-goog
>
^ permalink raw reply
* Re: [PATCH] Input: libps2 - embed WARN_ON(1) macros into their enclosing if statements
From: Dmitry Torokhov @ 2026-02-17 18:07 UTC (permalink / raw)
To: Max Brener; +Cc: linux-input, linux-kernel
In-Reply-To: <20260214203725.6463-1-linmaxi@gmail.com>
On Sat, Feb 14, 2026 at 10:37:24PM +0200, Max Brener wrote:
> Make WARN_ON(1) statements embedded inside their respective 'if' expressions,
> to improve code clarity.
>
> Signed-off-by: Max Brener <linmaxi@gmail.com>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* [PATCH 3/3] HID: asus: avoid memory leak in asus_report_fixup()
From: Günther Noack @ 2026-02-17 16:01 UTC (permalink / raw)
To: Jiri Kosina
Cc: Benjamin Tissoires, linux-input, linux-kernel, Günther Noack
In-Reply-To: <20260217160125.1097578-1-gnoack@google.com>
The asus_report_fixup() function was allocating a new buffer with kmemdup()
when growing the report descriptor but never freeing it. Switch to
devm_kzalloc() to ensure the memory is managed and freed automatically when
the device is removed.
Also fix a harmless out-of-bounds read by copying only the original
descriptor size.
Assisted-by: Gemini-CLI:Google Gemini 3
Signed-off-by: Günther Noack <gnoack@google.com>
---
drivers/hid/hid-asus.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 8ffcd12038e8..7a08e964b9cc 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -1399,14 +1399,21 @@ static const __u8 *asus_report_fixup(struct hid_device *hdev, __u8 *rdesc,
*/
if (*rsize == rsize_orig &&
rdesc[offs] == 0x09 && rdesc[offs + 1] == 0x76) {
- *rsize = rsize_orig + 1;
- rdesc = kmemdup(rdesc, *rsize, GFP_KERNEL);
- if (!rdesc)
- return NULL;
+ __u8 *new_rdesc;
+
+ new_rdesc = devm_kzalloc(&hdev->dev, rsize_orig + 1,
+ GFP_KERNEL);
+ if (!new_rdesc)
+ return rdesc;
hid_info(hdev, "Fixing up %s keyb report descriptor\n",
drvdata->quirks & QUIRK_T100CHI ?
"T100CHI" : "T90CHI");
+
+ memcpy(new_rdesc, rdesc, rsize_orig);
+ *rsize = rsize_orig + 1;
+ rdesc = new_rdesc;
+
memmove(rdesc + offs + 4, rdesc + offs + 2, 12);
rdesc[offs] = 0x19;
rdesc[offs + 1] = 0x00;
--
2.53.0.335.g19a08e0c02-goog
^ permalink raw reply related
* [PATCH 2/3] HID: magicmouse: avoid memory leak in magicmouse_report_fixup()
From: Günther Noack @ 2026-02-17 16:01 UTC (permalink / raw)
To: Jiri Kosina
Cc: Benjamin Tissoires, linux-input, linux-kernel, Günther Noack
In-Reply-To: <20260217160125.1097578-1-gnoack@google.com>
The magicmouse_report_fixup() function was allocating a new buffer with
kmemdup() but never freeing it. Since the caller already provides a
writable buffer and allows returning a pointer within it, modify the
descriptor in-place and return the adjusted pointer.
Assisted-by: Gemini-CLI:Google Gemini 3
Signed-off-by: Günther Noack <gnoack@google.com>
---
drivers/hid/hid-magicmouse.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
index 91f621ceb924..17908d52c027 100644
--- a/drivers/hid/hid-magicmouse.c
+++ b/drivers/hid/hid-magicmouse.c
@@ -994,9 +994,7 @@ static const __u8 *magicmouse_report_fixup(struct hid_device *hdev, __u8 *rdesc,
hid_info(hdev,
"fixing up magicmouse battery report descriptor\n");
*rsize = *rsize - 1;
- rdesc = kmemdup(rdesc + 1, *rsize, GFP_KERNEL);
- if (!rdesc)
- return NULL;
+ rdesc = rdesc + 1;
rdesc[0] = 0x05;
rdesc[1] = 0x01;
--
2.53.0.335.g19a08e0c02-goog
^ permalink raw reply related
* [PATCH 1/3] HID: apple: avoid memory leak in apple_report_fixup()
From: Günther Noack @ 2026-02-17 16:01 UTC (permalink / raw)
To: Jiri Kosina
Cc: Benjamin Tissoires, linux-input, linux-kernel, Günther Noack
In-Reply-To: <20260217160125.1097578-1-gnoack@google.com>
The apple_report_fixup() function was allocating a new buffer with
kmemdup() but never freeing it. Since the caller of report_fixup() already
provides a writable buffer and allows returning a pointer within that
buffer, we can just modify the descriptor in-place and return the adjusted
pointer.
Assisted-by: Gemini-CLI:Google Gemini 3
Signed-off-by: Günther Noack <gnoack@google.com>
---
drivers/hid/hid-apple.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
index 233e367cce1d..894adc23367b 100644
--- a/drivers/hid/hid-apple.c
+++ b/drivers/hid/hid-apple.c
@@ -686,9 +686,7 @@ static const __u8 *apple_report_fixup(struct hid_device *hdev, __u8 *rdesc,
hid_info(hdev,
"fixing up Magic Keyboard battery report descriptor\n");
*rsize = *rsize - 1;
- rdesc = kmemdup(rdesc + 1, *rsize, GFP_KERNEL);
- if (!rdesc)
- return NULL;
+ rdesc = rdesc + 1;
rdesc[0] = 0x05;
rdesc[1] = 0x01;
--
2.53.0.335.g19a08e0c02-goog
^ permalink raw reply related
* [PATCH 0/3] HID: Fix some memory leaks in drivers/hid
From: Günther Noack @ 2026-02-17 16:01 UTC (permalink / raw)
To: Jiri Kosina
Cc: Benjamin Tissoires, linux-input, linux-kernel, Günther Noack
Hello!
These patches fix a few memory leaks in HID report descriptor fixups.
FWIW, a good ad-hoc way to look for usages of allocation functions in
these is:
awk '/static.*report_fixup.*/,/^}/ { print FILENAME, $0 }' drivers/hid/hid-*.c \
| grep -E '(malloc|kzalloc|kcalloc|kmemdup)'
The devm_* variants are safe in this context, because they tie the
allocated memory to the lifetime of the driver.
For transparency, I generated these commits with Gemini-CLI,
starting with this prompt:
We are working in the Linux kernel. In the HID drivers in
`drivers/hid/hid-*.c`, the `report_fixup` driver hook is a function
that gets a byte buffer (with size) as input and that may modify that
byte buffer, and optionally return a pointer to a new byte buffer and
update the size. The returned value is *not* memory-managed by the
caller though and will not be freed subsequently. When the
`report_fixup` implementation allocates a new buffer and returns that,
that will not get freed by the caller. Validate this assessment and
fix up all HID drivers where that mistake is made.
(and then a little bit of additional nudging for the details).
—Günther
Günther Noack (3):
HID: apple: avoid memory leak in apple_report_fixup()
HID: magicmouse: avoid memory leak in magicmouse_report_fixup()
HID: asus: avoid memory leak in asus_report_fixup()
drivers/hid/hid-apple.c | 4 +---
drivers/hid/hid-asus.c | 15 +++++++++++----
drivers/hid/hid-magicmouse.c | 4 +---
3 files changed, 13 insertions(+), 10 deletions(-)
--
2.53.0.335.g19a08e0c02-goog
^ permalink raw reply
* Re: [PATCH v3 1/7] dt-bindings: embedded-controller: document ASUS Transformer EC
From: Conor Dooley @ 2026-02-17 15:50 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: Krzysztof Kozlowski, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Dmitry Torokhov, Lee Jones, Pavel Machek,
Sebastian Reichel, Ion Agorria, Michał Mirosław,
devicetree, linux-kernel, linux-input, linux-leds, linux-pm
In-Reply-To: <CAPVz0n0u7uhL8_FQFiuB7DrnL++ecbaEKEoV7N2PgTVRBVECkw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2552 bytes --]
On Tue, Feb 17, 2026 at 04:29:59PM +0200, Svyatoslav Ryhel wrote:
> вт, 17 лют. 2026 р. о 16:03 Conor Dooley <conor@kernel.org> пише:
> > On Tue, Feb 17, 2026 at 01:34:01PM +0200, Svyatoslav Ryhel wrote:
> > > 17 лютого 2026 р. 13:32:26 GMT+02:00, Krzysztof Kozlowski <krzk@kernel.org> пише:
> > > >> properties:
> > > >> compatible:
> > > >> - items:
> > > >> - enum:
> > > >> - asus,p1801-t-ec-pad
> > > >> - asus,sl101-ec-dock
> > > >> - asus,tf101-ec-dock
> > > >> - asus,tf101g-ec-dock
> > > >> - asus,tf201-ec-dock
> > > >> - asus,tf201-ec-pad
> > > >> - asus,tf300t-ec-dock
> > > >> - asus,tf300t-ec-pad
> > > >> - asus,tf300tg-ec-dock
> > > >> - asus,tf300tg-ec-pad
> > > >> - asus,tf300tl-ec-dock
> > > >> - asus,tf300tl-ec-pad
> > > >> - asus,tf700t-ec-dock
> > > >> - asus,tf700t-ec-pad
> > > >> - asus,tf600t-ec-pad
> > > >> - asus,tf701t-ec-pad
> > > >> - const: asus,transformer-ec
> > > >>
> > > >> And them schema name will match the genetic compatible.
> > > >
> > > >Then what does the generic compatible express?
> > > >
> > >
> > > Then enum it is
> >
> >
> > Why would you do that, instead of what I posted earlier in the thread?
> > If you send a flat enum with all devices listed, I'm gonna just be there
> > telling you to consolidate into one device-specific fallback compatible
> > per programming model.
>
> There is no one device-specific fallback compatible! Schema describes
> HARDWARE not drivers no? I will not use random device compatible from
> the list as a fallback compatible for a different random unrelated
> device, that is plain wrong.
Which is why I am not mentioning drivers at all, and instead talking about
programming models. Fallback compatibles represent similarities in
programming model, even if the laptop that these devices are contained
in are different, so whatever device compatible you pick might be random
(but going chronologically is usually my recommendation) but it won't be
unrelated.
> Discuss this with Krzysztof and come up with something meaningful please.
If you don't consider my reviews to be meaningful, you're welcome to
carry a Nacked-by: Conor Dooley <conor.dooley@microchip.com> with a link
to this discussion instead.
Thanks,
Conor.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v3 1/7] dt-bindings: embedded-controller: document ASUS Transformer EC
From: Krzysztof Kozlowski @ 2026-02-17 14:44 UTC (permalink / raw)
To: Svyatoslav Ryhel, Conor Dooley
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Dmitry Torokhov,
Lee Jones, Pavel Machek, Sebastian Reichel, Ion Agorria,
Michał Mirosław, devicetree, linux-kernel, linux-input,
linux-leds, linux-pm
In-Reply-To: <CAPVz0n0u7uhL8_FQFiuB7DrnL++ecbaEKEoV7N2PgTVRBVECkw@mail.gmail.com>
On 17/02/2026 15:29, Svyatoslav Ryhel wrote:
>>>>> properties:
>>>>> compatible:
>>>>> - items:
>>>>> - enum:
>>>>> - asus,p1801-t-ec-pad
>>>>> - asus,sl101-ec-dock
>>>>> - asus,tf101-ec-dock
>>>>> - asus,tf101g-ec-dock
>>>>> - asus,tf201-ec-dock
>>>>> - asus,tf201-ec-pad
>>>>> - asus,tf300t-ec-dock
>>>>> - asus,tf300t-ec-pad
>>>>> - asus,tf300tg-ec-dock
>>>>> - asus,tf300tg-ec-pad
>>>>> - asus,tf300tl-ec-dock
>>>>> - asus,tf300tl-ec-pad
>>>>> - asus,tf700t-ec-dock
>>>>> - asus,tf700t-ec-pad
>>>>> - asus,tf600t-ec-pad
>>>>> - asus,tf701t-ec-pad
>>>>> - const: asus,transformer-ec
>>>>>
>>>>> And them schema name will match the genetic compatible.
>>>>
>>>> Then what does the generic compatible express?
>>>>
>>>
>>> Then enum it is
>>
>>
>> Why would you do that, instead of what I posted earlier in the thread?
>> If you send a flat enum with all devices listed, I'm gonna just be there
>> telling you to consolidate into one device-specific fallback compatible
>> per programming model.
>
> There is no one device-specific fallback compatible! Schema describes
> HARDWARE not drivers no? I will not use random device compatible from
You came with "asus,transformer-ec" as fallback which is same random
choice, so how one random device is okay, but other not? We asked to use
some meaningful device as fallback, to the best of current knowledge.
> the list as a fallback compatible for a different random unrelated
> device, that is plain wrong. Discuss this with Krzysztof and come up
> with something meaningful please.
It is not pleasant to review your patches. This happened in the past [1]
and recently is not getting better. In multiple threads. Receiving
half-baked responses to actual review questions means you do not value
our time. I think you somehow annoyed, offended or wasted time of all
three DT maintainers.
Well, sure, happens.
Just understand there might be a reason when your patches do not receive
attention or reviews.
[1]
https://lore.kernel.org/r/CAPVz0n09ZP1i2tasdTvnt8RvjhALvUYjv9u_EGRtnXPOYQtuqQ@mail.gmail.com/
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v3 1/7] dt-bindings: embedded-controller: document ASUS Transformer EC
From: Svyatoslav Ryhel @ 2026-02-17 14:29 UTC (permalink / raw)
To: Conor Dooley
Cc: Krzysztof Kozlowski, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Dmitry Torokhov, Lee Jones, Pavel Machek,
Sebastian Reichel, Ion Agorria, Michał Mirosław,
devicetree, linux-kernel, linux-input, linux-leds, linux-pm
In-Reply-To: <20260217-dig-husked-8a59b6a19aee@spud>
вт, 17 лют. 2026 р. о 16:03 Conor Dooley <conor@kernel.org> пише:
>
> On Tue, Feb 17, 2026 at 01:34:01PM +0200, Svyatoslav Ryhel wrote:
> >
> >
> > 17 лютого 2026 р. 13:32:26 GMT+02:00, Krzysztof Kozlowski <krzk@kernel.org> пише:
> > >On 17/02/2026 12:23, Svyatoslav Ryhel wrote:
> > >>>> in other words you propose this:
> > >>>>
> > >>>> properties:
> > >>>> compatible:
> > >>>> oneOf:
> > >>>> - items:
> > >>>> - enum:
> > >>>> - asus,sl101-ec-dock
> > >>>> - asus,tf101-ec-dock
> > >>>> - asus,tf101g-ec-dock
> > >>>> - asus,tf201-ec-dock
> > >>>> - asus,tf300t-ec-dock
> > >>>> - asus,tf300tg-ec-dock
> > >>>> - asus,tf300tl-ec-dock
> > >>>> - asus,tf700t-ec-dock
> > >>>> - const: asus,transformer-ec-dock
> > >>>>
> > >>>> - items:
> > >>>> - enum:
> > >>>> - asus,p1801-t-ec-pad
> > >>>> - asus,tf201-ec-pad
> > >>>> - asus,tf300t-ec-pad
> > >>>> - asus,tf300tg-ec-pad
> > >>>> - asus,tf300tl-ec-pad
> > >>>> - asus,tf700t-ec-pad
> > >>>> - asus,tf600t-ec-pad
> > >>>> - asus,tf701t-ec-pad
> > >>>> - const: asus,transformer-ec-pad
> > >>>>
> > >>>> And in the driver add match to every single entry of enums?
> > >>>
> > >>> No, I was talking about removing the generic compatibles entirely, since
> > >>> they are not suitably generic to cover all devices at the point of
> > >>> addition. So like:
> > >>>
> > >>
> > >> Actually, they all can be grouped under asus,transformer-ec fallback if that is needed, both pad and dock EC have the same core functions just different set of cells. And then in the driver each compatible will get a dedicated matching data. Will this work?
> > >
> > >Then what does the generic compatible express if it is not used by the SW.
> > >
> > >Wrap your emails to mailing list style.
> > >
> > >>
> > >> properties:
> > >> compatible:
> > >> - items:
> > >> - enum:
> > >> - asus,p1801-t-ec-pad
> > >> - asus,sl101-ec-dock
> > >> - asus,tf101-ec-dock
> > >> - asus,tf101g-ec-dock
> > >> - asus,tf201-ec-dock
> > >> - asus,tf201-ec-pad
> > >> - asus,tf300t-ec-dock
> > >> - asus,tf300t-ec-pad
> > >> - asus,tf300tg-ec-dock
> > >> - asus,tf300tg-ec-pad
> > >> - asus,tf300tl-ec-dock
> > >> - asus,tf300tl-ec-pad
> > >> - asus,tf700t-ec-dock
> > >> - asus,tf700t-ec-pad
> > >> - asus,tf600t-ec-pad
> > >> - asus,tf701t-ec-pad
> > >> - const: asus,transformer-ec
> > >>
> > >> And them schema name will match the genetic compatible.
> > >
> > >Then what does the generic compatible express?
> > >
> >
> > Then enum it is
>
>
> Why would you do that, instead of what I posted earlier in the thread?
> If you send a flat enum with all devices listed, I'm gonna just be there
> telling you to consolidate into one device-specific fallback compatible
> per programming model.
There is no one device-specific fallback compatible! Schema describes
HARDWARE not drivers no? I will not use random device compatible from
the list as a fallback compatible for a different random unrelated
device, that is plain wrong. Discuss this with Krzysztof and come up
with something meaningful please.
^ permalink raw reply
* Re: [PATCH v3 1/7] dt-bindings: embedded-controller: document ASUS Transformer EC
From: Conor Dooley @ 2026-02-17 14:03 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: Krzysztof Kozlowski, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Dmitry Torokhov, Lee Jones, Pavel Machek,
Sebastian Reichel, Ion Agorria, Michał Mirosław,
devicetree, linux-kernel, linux-input, linux-leds, linux-pm
In-Reply-To: <81844CC9-5355-4B1D-AEBD-6DD67FB8C81B@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3117 bytes --]
On Tue, Feb 17, 2026 at 01:34:01PM +0200, Svyatoslav Ryhel wrote:
>
>
> 17 лютого 2026 р. 13:32:26 GMT+02:00, Krzysztof Kozlowski <krzk@kernel.org> пише:
> >On 17/02/2026 12:23, Svyatoslav Ryhel wrote:
> >>>> in other words you propose this:
> >>>>
> >>>> properties:
> >>>> compatible:
> >>>> oneOf:
> >>>> - items:
> >>>> - enum:
> >>>> - asus,sl101-ec-dock
> >>>> - asus,tf101-ec-dock
> >>>> - asus,tf101g-ec-dock
> >>>> - asus,tf201-ec-dock
> >>>> - asus,tf300t-ec-dock
> >>>> - asus,tf300tg-ec-dock
> >>>> - asus,tf300tl-ec-dock
> >>>> - asus,tf700t-ec-dock
> >>>> - const: asus,transformer-ec-dock
> >>>>
> >>>> - items:
> >>>> - enum:
> >>>> - asus,p1801-t-ec-pad
> >>>> - asus,tf201-ec-pad
> >>>> - asus,tf300t-ec-pad
> >>>> - asus,tf300tg-ec-pad
> >>>> - asus,tf300tl-ec-pad
> >>>> - asus,tf700t-ec-pad
> >>>> - asus,tf600t-ec-pad
> >>>> - asus,tf701t-ec-pad
> >>>> - const: asus,transformer-ec-pad
> >>>>
> >>>> And in the driver add match to every single entry of enums?
> >>>
> >>> No, I was talking about removing the generic compatibles entirely, since
> >>> they are not suitably generic to cover all devices at the point of
> >>> addition. So like:
> >>>
> >>
> >> Actually, they all can be grouped under asus,transformer-ec fallback if that is needed, both pad and dock EC have the same core functions just different set of cells. And then in the driver each compatible will get a dedicated matching data. Will this work?
> >
> >Then what does the generic compatible express if it is not used by the SW.
> >
> >Wrap your emails to mailing list style.
> >
> >>
> >> properties:
> >> compatible:
> >> - items:
> >> - enum:
> >> - asus,p1801-t-ec-pad
> >> - asus,sl101-ec-dock
> >> - asus,tf101-ec-dock
> >> - asus,tf101g-ec-dock
> >> - asus,tf201-ec-dock
> >> - asus,tf201-ec-pad
> >> - asus,tf300t-ec-dock
> >> - asus,tf300t-ec-pad
> >> - asus,tf300tg-ec-dock
> >> - asus,tf300tg-ec-pad
> >> - asus,tf300tl-ec-dock
> >> - asus,tf300tl-ec-pad
> >> - asus,tf700t-ec-dock
> >> - asus,tf700t-ec-pad
> >> - asus,tf600t-ec-pad
> >> - asus,tf701t-ec-pad
> >> - const: asus,transformer-ec
> >>
> >> And them schema name will match the genetic compatible.
> >
> >Then what does the generic compatible express?
> >
>
> Then enum it is
Why would you do that, instead of what I posted earlier in the thread?
If you send a flat enum with all devices listed, I'm gonna just be there
telling you to consolidate into one device-specific fallback compatible
per programming model.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v3 1/7] dt-bindings: embedded-controller: document ASUS Transformer EC
From: Conor Dooley @ 2026-02-17 14:01 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Dmitry Torokhov,
Lee Jones, Pavel Machek, Sebastian Reichel, Ion Agorria,
Michał Mirosław, devicetree, linux-kernel, linux-input,
linux-leds, linux-pm
In-Reply-To: <55C30023-4175-48F2-BCB0-12EC23C48F01@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 8575 bytes --]
On Tue, Feb 17, 2026 at 01:23:06PM +0200, Svyatoslav Ryhel wrote:
>
>
> 17 лютого 2026 р. 13:05:09 GMT+02:00, Conor Dooley <conor@kernel.org> пише:
> >On Mon, Feb 16, 2026 at 09:14:40PM +0200, Svyatoslav Ryhel wrote:
> >> пн, 16 лют. 2026 р. о 20:50 Conor Dooley <conor@kernel.org> пише:
> >> >
> >> > On Mon, Feb 16, 2026 at 08:22:38PM +0200, Svyatoslav Ryhel wrote:
> >> > > пн, 16 лют. 2026 р. о 20:04 Conor Dooley <conor@kernel.org> пише:
> >> > > >
> >> > > > On Sat, Feb 14, 2026 at 08:09:53PM +0200, Svyatoslav Ryhel wrote:
> >> > > > > Document embedded controller used in ASUS Transformer device series.
> >> > > > >
> >> > > > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> >> > > > > ---
> >> > > > > .../asus,transformer-ec.yaml | 98 +++++++++++++++++++
> >> > > > > 1 file changed, 98 insertions(+)
> >> > > > > create mode 100644 Documentation/devicetree/bindings/embedded-controller/asus,transformer-ec.yaml
> >> > > > >
> >> > > > > diff --git a/Documentation/devicetree/bindings/embedded-controller/asus,transformer-ec.yaml b/Documentation/devicetree/bindings/embedded-controller/asus,transformer-ec.yaml
> >> > > > > new file mode 100644
> >> > > > > index 000000000000..670c4c2d339d
> >> > > > > --- /dev/null
> >> > > > > +++ b/Documentation/devicetree/bindings/embedded-controller/asus,transformer-ec.yaml
> >> > > > > @@ -0,0 +1,98 @@
> >> > > > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> >> > > > > +%YAML 1.2
> >> > > > > +---
> >> > > > > +$id: http://devicetree.org/schemas/embedded-controller/asus,transformer-ec.yaml#
> >> > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >> > > > > +
> >> > > > > +title: ASUS Transformer's Embedded Controller
> >> > > > > +
> >> > > > > +description:
> >> > > > > + Several Nuvoton based Embedded Controllers attached to an I2C bus,
> >> > > > > + running a custom ASUS firmware, specific to the ASUS Transformer
> >> > > > > + device series.
> >> > > > > +
> >> > > > > +maintainers:
> >> > > > > + - Svyatoslav Ryhel <clamor95@gmail.com>
> >> > > > > +
> >> > > > > +allOf:
> >> > > > > + - $ref: /schemas/power/supply/power-supply.yaml
> >> > > > > +
> >> > > > > +properties:
> >> > > > > + compatible:
> >> > > > > + oneOf:
> >> > > > > + - enum:
> >> > > > > + - asus,p1801-t-ec-pad
> >> > > > > + - asus,sl101-ec-dock
> >> > > > > + - asus,tf600t-ec-pad
> >> > > > > + - asus,tf701t-ec-pad
> >> > > > > +
> >> > > > > + - items:
> >> > > > > + - enum:
> >> > > > > + - asus,tf101-ec-dock
> >> > > > > + - asus,tf101g-ec-dock
> >> > > > > + - asus,tf201-ec-dock
> >> > > > > + - asus,tf300t-ec-dock
> >> > > > > + - asus,tf300tg-ec-dock
> >> > > > > + - asus,tf300tl-ec-dock
> >> > > > > + - asus,tf700t-ec-dock
> >> > > > > + - const: asus,transformer-ec-dock
> >> > > > > +
> >> > > > > + - items:
> >> > > > > + - enum:
> >> > > > > + - asus,tf201-ec-pad
> >> > > > > + - asus,tf300t-ec-pad
> >> > > > > + - asus,tf300tg-ec-pad
> >> > > > > + - asus,tf300tl-ec-pad
> >> > > > > + - asus,tf700t-ec-pad
> >> > > > > + - const: asus,transformer-ec-pad
> >
> >> > > > Also, why are some of the compatibles permitted standalone? That should
> >> > > > be mentioned in your commit message too. Also, other than the sl101, the
> >> > > > standalone ones seem to have the same match data in the mfd driver. Why
> >> > > > are fallbacks not made use of there?
> >> > > >
> >> > >
> >> > > Because standalone compatibles describe a unique hw configuration
> >> > > which cannot be grouped into something meaningful. asus,p1801-t-ec-pad
> >> > > is for EC of Tegra30/Intel based p1801-t AIO, asus,sl101-ec-dock is
> >> > > for EC of Tegra20 slider tablet, asus,tf600t-ec-pad is for altered EC
> >> > > in Win8 Tegra30 tablet, asus,tf701t-ec-pad is for Tegra114 tablet.
> >> > > Different generations, different form-factors.
> >> >
> >> > I don't see any reasons here that eliminate fallback compatibles.
> >> > + { .compatible = "asus,p1801-t-ec-pad", .data = &asus_ec_pad_charger_data },
> >> > + { .compatible = "asus,tf600t-ec-pad", .data = &asus_ec_pad_charger_data },
> >> > + { .compatible = "asus,tf701t-ec-pad", .data = &asus_ec_pad_charger_data },
> >> > + { }
> >> > Three of them use the same match data, so you need to explain why you've
> >> > made these three standalone when all the others that share a programming
> >> > model got a generic fallback. Fallback usage is based on programming
> >> > model, not based on whether the devices are a physically different, so
> >> > your explanation must reflect this.
> >> >
> >> > > > Since this transformer series seems to have multiple programming models
> >> > > > for "ec-pad" devices, it calls into question your use of the generic
> >> > > > fallback compatibles is appropriate and makes it seem like you should be
> >> > > > using device compatibles as a fallback.
> >> > >
> >> > > That is redundant.
> >> >
> >> > I don't understand how that is a response to what I said.
> >> >
> >>
> >> in other words you propose this:
> >>
> >> properties:
> >> compatible:
> >> oneOf:
> >> - items:
> >> - enum:
> >> - asus,sl101-ec-dock
> >> - asus,tf101-ec-dock
> >> - asus,tf101g-ec-dock
> >> - asus,tf201-ec-dock
> >> - asus,tf300t-ec-dock
> >> - asus,tf300tg-ec-dock
> >> - asus,tf300tl-ec-dock
> >> - asus,tf700t-ec-dock
> >> - const: asus,transformer-ec-dock
> >>
> >> - items:
> >> - enum:
> >> - asus,p1801-t-ec-pad
> >> - asus,tf201-ec-pad
> >> - asus,tf300t-ec-pad
> >> - asus,tf300tg-ec-pad
> >> - asus,tf300tl-ec-pad
> >> - asus,tf700t-ec-pad
> >> - asus,tf600t-ec-pad
> >> - asus,tf701t-ec-pad
> >> - const: asus,transformer-ec-pad
> >>
> >> And in the driver add match to every single entry of enums?
> >
> >No, I was talking about removing the generic compatibles entirely, since
> >they are not suitably generic to cover all devices at the point of
> >addition. So like:
> >
>
> Actually, they all can be grouped under asus,transformer-ec fallback if that is needed, both pad and dock EC have the same core functions just different set of cells. And then in the driver each compatible will get a dedicated matching data. Will this work?
>
> properties:
> compatible:
> - items:
> - enum:
> - asus,p1801-t-ec-pad
> - asus,sl101-ec-dock
> - asus,tf101-ec-dock
> - asus,tf101g-ec-dock
> - asus,tf201-ec-dock
> - asus,tf201-ec-pad
> - asus,tf300t-ec-dock
> - asus,tf300t-ec-pad
> - asus,tf300tg-ec-dock
> - asus,tf300tg-ec-pad
> - asus,tf300tl-ec-dock
> - asus,tf300tl-ec-pad
> - asus,tf700t-ec-dock
> - asus,tf700t-ec-pad
> - asus,tf600t-ec-pad
> - asus,tf701t-ec-pad
> - const: asus,transformer-ec
>
> And them schema name will match the genetic compatible.
>
> >items:
> > - enum:
> > - asus,tf101-ec-dock
> > - asus,tf101g-ec-dock
> > - asus,tf201-ec-dock
> > - asus,tf300t-ec-dock
> > - asus,tf300tg-ec-dock
> > - asus,tf300tl-ec-dock
> > - const: asus,tf700t-ec-dock
> >
> >and
> >
> >items:
> > - enum:
> > - asus,p1801-t-ec-pad
> > - asus,tf600t-ec-pad
> > - const: asus,tf701t-ec-pad
> >
> >I dunno about these particular devices, but if there's already two
> >programming models for these devices, what's to stop there being more
> >added if/when a new generation of produces arrives?
>
> There will be no new devices with this EC, last one was around 2013.
And what is to stop the next device in 2027 or w/e having a different
programming model (so a third)? That's my point, I wasn't talking about
there being more devices with two programming models evidenced here.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH] touchscreen-dmi quirk for SUPI S10
From: Hans de Goede @ 2026-02-17 13:24 UTC (permalink / raw)
To: Yajat Kumar, dmitry.torokhov, linux-input, linux-kernel
In-Reply-To: <20260217050755.16078-1-yajatapps3@gmail.com>
Hi,
On 17-Feb-26 06:07, Yajat Kumar wrote:
> Hi Hans,
>
> Sorry for the threading issue — Gmail broke my previous reply. Reposting confirmation below.
>
> I’ve tested the attached patch on the SUPI S10.
>
> The touchscreen uses ACPI HID GDIX1001, so no change is needed there.
> With the patch applied, the Y axis is no longer inverted and touch
> input now matches the display orientation correctly.
> You can add:
>
> Tested-by: Yajat Kumar <yajatapps3@gmail.com>
>
> Thanks for the quick fix and explanation.
Thank you for testing. Now that its been confirmed
this works I've posted the patch upstream:
https://lore.kernel.org/platform-driver-x86/20260217132346.34535-1-johannes.goede@oss.qualcomm.com/
Regards,
Hans
^ permalink raw reply
* Re: [PATCH v2 3/3] arm64: dts: qcom: monaco-pmics: Add PON power key and reset inputs
From: Konrad Dybcio @ 2026-02-17 12:30 UTC (permalink / raw)
To: Rakesh Kota, Sebastian Reichel, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Vinod Koul, Dmitry Torokhov, Courtney Cavin,
Bjorn Andersson, Konrad Dybcio
Cc: linux-pm, devicetree, linux-kernel, linux-arm-msm, linux-input
In-Reply-To: <20260209-add_pwrkey_and_resin-v2-3-f944d87b9a93@oss.qualcomm.com>
On 2/9/26 2:23 PM, Rakesh Kota wrote:
> Add the Power On (PON) peripheral with power key and reset input
> support for the PMM8654AU PMIC on Monaco platforms.
>
> Signed-off-by: Rakesh Kota <rakesh.kota@oss.qualcomm.com>
> ---
> arch/arm64/boot/dts/qcom/monaco-pmics.dtsi | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/monaco-pmics.dtsi b/arch/arm64/boot/dts/qcom/monaco-pmics.dtsi
> index e990d7367719beaa9e0cea87d9c183ae18c3ebc8..182c2339bb11af40275050a36c4688227e89497a 100644
> --- a/arch/arm64/boot/dts/qcom/monaco-pmics.dtsi
> +++ b/arch/arm64/boot/dts/qcom/monaco-pmics.dtsi
> @@ -13,6 +13,26 @@ pmm8620au_0: pmic@0 {
> #address-cells = <1>;
> #size-cells = <0>;
>
> + pmm8654au_0_pon: pon@1200 {
> + compatible = "qcom,pmm8654au-pon", "qcom,pmk8350-pon";
> + reg = <0x1200>, <0x800>;
> + reg-names = "hlos", "pbs";
> +
> + pmm8654au_0_pon_pwrkey: pwrkey {
> + compatible = "qcom,pmm8654au-pwrkey", "qcom,pmk8350-pwrkey";
> + interrupts-extended = <&spmi_bus 0x0 0x12 0x7 IRQ_TYPE_EDGE_BOTH>;
> + linux,code = <KEY_POWER>;
> + debounce = <15625>;
> + };
> +
> + pmm8654au_0_pon_resin: resin {
> + compatible = "qcom,pmm8654au-resin", "qcom,pmk8350-resin";
> + interrupts-extended = <&spmi_bus 0x0 0x12 0x6 IRQ_TYPE_EDGE_BOTH>;
> + linux,code = <KEY_VOLUMEDOWN>;
> + debounce = <15625>;
FWIW we tend to disable RESIN by default, as it's not as ubiquitous as
the power key and only assign the keycode in board DT, since it may
commonly be reused for either of the volume keys (or something else
entirely)
Konrad
^ permalink raw reply
* Re: [PATCH v2 1/3] dt-bindings: power: reset: qcom-pon: Add new compatible PMM8654AU
From: Konrad Dybcio @ 2026-02-17 12:29 UTC (permalink / raw)
To: Rakesh Kota, Krzysztof Kozlowski
Cc: Sebastian Reichel, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Vinod Koul, Dmitry Torokhov, Courtney Cavin, Bjorn Andersson,
Konrad Dybcio, linux-pm, devicetree, linux-kernel, linux-arm-msm,
linux-input
In-Reply-To: <20260211104153.r6xz7ya5emxa36cp@hu-kotarake-hyd.qualcomm.com>
On 2/11/26 11:41 AM, Rakesh Kota wrote:
> On Tue, Feb 10, 2026 at 09:32:18AM +0100, Krzysztof Kozlowski wrote:
>> On 10/02/2026 09:26, Rakesh Kota wrote:
>>> On Mon, Feb 09, 2026 at 02:49:24PM +0100, Krzysztof Kozlowski wrote:
>>>> On 09/02/2026 14:23, Rakesh Kota wrote:
>>>>> Add the compatible string "qcom,pmm8654au-pon" for the PMM8654AU PMIC.
>>>>> The PON peripheral on PMM8654AU is compatible with PMK8350, so it is
>>>>> documented as a fallback to "qcom,pmk8350-pon".
>>>>
>>>> Drop everything after ,. Do not explain WHAT you did. We see it.
>>>>
>>>>>
>>>>> While PMM8654AU supports additional registers compared to the baseline,
>>>>
>>>> full stop.
>>>>
>>>>> there is currently no active use case for these features. This specific
>>>>> compatible string reserves the identifier for future hardware-specific
>>>>> handling if required.
>>>>
>>>> All the rest is irrelevant or even wrong. We do not reserve identifiers.
>>>> If you want to reserve something, then I need to reject the patch.
>>>>
>>> Hi Konrad Dybcio,
>>>
>>> It appears that Krzysztof Kozlowski has concerns regarding the
>>> compatible string reservation for future use cases, noting that
>>> identifiers should not be reserved in this manner.
>>
>> So do not reserve identifiers but submit bindings reflecting REAL
>> hardware being used.
>>
> Yes, there is a real hardware difference between the PMK8350 and
> PMM865AU PON peripherals. The PMM865AU PON is leveraged from the PMK8350
> PON and includes extra features, but those features do not have any
> active use cases for now.
>
> If you are okay with the new compatible string, I will send V3 and fix
> the commit message suggestions.
I believe the only issue here is the commit message indeed, you're not
reserving an identifier, but rather describing a new version/implementation
of a hardware block that (with the 8350 fallback) just happens not to need
any specific handling to make use of the 99.9% of its features
Konrad
^ 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