* Re: [PATCH v3 2/4] HID: wacom: Fix Use-After-Free in wacom_bamboo_pad
From: Dmitry Torokhov @ 2026-06-10 23:17 UTC (permalink / raw)
To: Lee Jones
Cc: Ping Cheng, Jason Gerecke, Jiri Kosina, Benjamin Tissoires,
Peter Hutterer, linux-input, linux-kernel
In-Reply-To: <20260609121353.3743782-2-lee@kernel.org>
On Tue, Jun 09, 2026 at 01:13:38PM +0100, Lee Jones wrote:
> @@ -3308,13 +3309,12 @@ static void wacom_bamboo_pad_pen_event(struct wacom_wac *wacom,
> prefix = data[0];
> data[0] = WACOM_REPORT_BPAD_PEN;
>
> - /*
> - * actually reroute the event.
> - * No need to check if wacom->shared->pen is valid, hid_input_report()
> - * will check for us.
> - */
> - hid_input_report(wacom->shared->pen, HID_INPUT_REPORT, data,
> - WACOM_PKGLEN_PENABLED, 1);
> + rcu_read_lock();
Since the driver already uses guard notation we can use
guard(rcu)();
> + pen = rcu_dereference(wacom->shared->pen);
> + if (pen)
> + hid_input_report(pen, HID_INPUT_REPORT, data,
> + WACOM_PKGLEN_PENABLED, 1);
> + rcu_read_unlock();
>
> data[0] = prefix;
> }
Otherwise:
Reviewed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v3 1/4] HID: wacom: Fix Use-After-Free in wacom_intuos_pad
From: Dmitry Torokhov @ 2026-06-10 23:22 UTC (permalink / raw)
To: Lee Jones
Cc: Ping Cheng, Jason Gerecke, Jiri Kosina, Benjamin Tissoires,
Peter Hutterer, linux-input, linux-kernel
In-Reply-To: <20260609121353.3743782-1-lee@kernel.org>
On Tue, Jun 09, 2026 at 01:13:37PM +0100, Lee Jones wrote:
> wacom_intuos_pad() accesses wacom->shared->touch_input locklessly
> inside the interrupt handler context. If the Touch sibling device
> is disconnected, wacom_remove_shared_data() clears 'touch_input'
> outside any lock, creating a Time-of-Check to Time-of-Use (TOCTOU)
> race condition where a preempted reader in interrupt context
> dereferences the freed pointer, leading to a Use-After-Free.
>
> Resolve this by introducing RCU protection for the touch_input
> pointer:
>
> - Annotate 'touch_input' in wacom_shared struct with __rcu
> - Wrap all lockless readers in wacom_wac.c with rcu_read_lock() and
> rcu_dereference() using a unified wacom_report_touch_mute()
> helper
> - Update writers in wacom_sys.c using rcu_assign_pointer()
> - Call synchronize_rcu() in wacom_remove_shared_data() to ensure
> all active RCU readers have finished before the input device is
> freed
>
> Also wrap wacom_set_shared_values() and touch/pen assignments in
> wacom_add_shared_data() inside the wacom_udev_list_lock to serialize
> concurrent probe assignments, and verify that 'shared->touch == hdev'
> before setting touch_input to prevent concurrent sibling probe state
> desynchronization.
>
> Finally, advertise the SW_MUTE_DEVICE capability on Touch input
> devices prior to registration in wacom_setup_touch_input_capabilities()
> to prevent invalid post-registration capability modifications.
>
> Fixes: 961794a00eab ("Input: wacom - add reporting of SW_MUTE_DEVICE events")
> Signed-off-by: Lee Jones <lee@kernel.org>
> ---
>
> v1 -> v2: Split and use RCU as per Dmitry's review
> v2 -> v3: Sashiko fixes
>
> drivers/hid/wacom_sys.c | 41 ++++++++++++++----------
> drivers/hid/wacom_wac.c | 70 ++++++++++++++++++++++-------------------
> drivers/hid/wacom_wac.h | 2 +-
> 3 files changed, 63 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index 2220168bf116..7ba589826548 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
> @@ -877,10 +877,16 @@ static void wacom_remove_shared_data(void *res)
> data = container_of(wacom_wac->shared, struct wacom_hdev_data,
> shared);
>
> - if (wacom_wac->shared->touch == wacom->hdev)
> - wacom_wac->shared->touch = NULL;
> - else if (wacom_wac->shared->pen == wacom->hdev)
> - wacom_wac->shared->pen = NULL;
> + scoped_guard(mutex, &wacom_udev_list_lock) {
> + if (wacom_wac->shared->touch == wacom->hdev) {
> + wacom_wac->shared->touch = NULL;
> + rcu_assign_pointer(wacom_wac->shared->touch_input, NULL);
> + } else if (wacom_wac->shared->pen == wacom->hdev) {
> + wacom_wac->shared->pen = NULL;
> + }
> + }
> +
> + synchronize_rcu();
>
> kref_put(&data->kref, wacom_release_shared_data);
> wacom_wac->shared = NULL;
> @@ -909,6 +915,11 @@ static int wacom_add_shared_data(struct hid_device *hdev)
> list_add_tail(&data->list, &wacom_udev_list);
> }
>
> + if (wacom_wac->features.device_type & WACOM_DEVICETYPE_TOUCH)
> + data->shared.touch = hdev;
> + else if (wacom_wac->features.device_type & WACOM_DEVICETYPE_PEN)
> + data->shared.pen = hdev;
> +
> mutex_unlock(&wacom_udev_list_lock);
>
> wacom_wac->shared = &data->shared;
> @@ -917,11 +928,6 @@ static int wacom_add_shared_data(struct hid_device *hdev)
> if (retval)
> return retval;
>
> - if (wacom_wac->features.device_type & WACOM_DEVICETYPE_TOUCH)
> - wacom_wac->shared->touch = hdev;
> - else if (wacom_wac->features.device_type & WACOM_DEVICETYPE_PEN)
> - wacom_wac->shared->pen = hdev;
> -
> return retval;
> }
>
> @@ -2345,9 +2351,15 @@ static void wacom_release_resources(struct wacom *wacom)
>
> static void wacom_set_shared_values(struct wacom_wac *wacom_wac)
> {
> + struct wacom *wacom = container_of(wacom_wac, struct wacom, wacom_wac);
> +
> + mutex_lock(&wacom_udev_list_lock);
Why not
guard(mutex)(&wacom_udev_list_lock);
> +
> if (wacom_wac->features.device_type & WACOM_DEVICETYPE_TOUCH) {
> - wacom_wac->shared->type = wacom_wac->features.type;
> - wacom_wac->shared->touch_input = wacom_wac->touch_input;
> + if (wacom_wac->shared->touch == wacom->hdev) {
> + wacom_wac->shared->type = wacom_wac->features.type;
> + rcu_assign_pointer(wacom_wac->shared->touch_input, wacom_wac->touch_input);
> + }
> }
>
> if (wacom_wac->has_mute_touch_switch) {
> @@ -2361,12 +2373,7 @@ static void wacom_set_shared_values(struct wacom_wac *wacom_wac)
> wacom_wac->shared->is_touch_on = true;
> }
>
> - if (wacom_wac->shared->has_mute_touch_switch &&
> - wacom_wac->shared->touch_input) {
> - set_bit(EV_SW, wacom_wac->shared->touch_input->evbit);
> - input_set_capability(wacom_wac->shared->touch_input, EV_SW,
> - SW_MUTE_DEVICE);
> - }
> + mutex_unlock(&wacom_udev_list_lock);
> }
>
> static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> index da1f0ea85625..495960227b8d 100644
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c
> @@ -510,6 +510,22 @@ static void wacom_intuos_schedule_prox_event(struct wacom_wac *wacom_wac)
> }
> }
>
> +static void wacom_report_touch_mute(struct wacom_wac *wacom_wac, bool mute)
> +{
> + struct input_dev *touch_input;
> +
> + if (!wacom_wac->shared)
> + return;
Can this happen? I think callers already check this or simply
dereference.
> +
> + rcu_read_lock();
guard(rcu)();
> + touch_input = rcu_dereference(wacom_wac->shared->touch_input);
> + if (touch_input) {
> + input_report_switch(touch_input, SW_MUTE_DEVICE, mute);
> + input_sync(touch_input);
> + }
> + rcu_read_unlock();
> +}
> +
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] Input: synaptics-rmi4 - unregister function handlers on physical driver registration failure
From: Dmitry Torokhov @ 2026-06-10 23:41 UTC (permalink / raw)
To: Haoxiang Li
Cc: git, Marge.Yang, kees, jiapeng.chong, linux-input, linux-kernel,
stable
In-Reply-To: <20260610064633.2837084-1-haoxiang_li2024@163.com>
Hi Haoxiang,
On Wed, Jun 10, 2026 at 02:46:33PM +0800, Haoxiang Li wrote:
> If rmi_register_physical_driver() fails, the current error path
> unregisters only the RMI bus. The function handlers registered
> earlier remain registered with the driver core.
>
> Add a separate error path to unregister the function handlers
> before unregistering the bus in this failure case.
>
> Fixes: d6e680837ec5 ("Input: synaptics-rmi4 - fix function name in kerneldoc")
This is not correct commit for fixes. I changed this to
2b6a321da9a2 ("Input: synaptics-rmi4 - add support for Synaptics RMI4 devices")
and applied, thank you.
Thanks.
--
Dmitry
^ permalink raw reply
* [hid:for-7.2/asus 1/7] drivers/hid/hid-asus.c:1399:14: warning: variable 'rsize_orig' is used uninitialized whenever 'if' condition is false
From: kernel test robot @ 2026-06-11 4:05 UTC (permalink / raw)
To: Joshua Leivenzon; +Cc: llvm, oe-kbuild-all, linux-input, Jiri Kosina
tree: https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-7.2/asus
head: 9bde6277292c8233fb24fc6e51323027b49d1cde
commit: 92f9f783f013a27a175089950b7b22c3d5a48249 [1/7] HID: asus: Fix up Zenbook Duo report descriptors
config: x86_64-rhel-9.4-rust (https://download.01.org/0day-ci/archive/20260611/202606110526.QfgiXQTQ-lkp@intel.com/config)
compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project f43d6834093b19baf79beda8c0337ab020ac5f17)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260611/202606110526.QfgiXQTQ-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/202606110526.QfgiXQTQ-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/hid/hid-asus.c:1399:14: warning: variable 'rsize_orig' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
1399 | } else if (drvdata->quirks & QUIRK_ZENBOOK_DUO_KEYBOARD) {
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/hid/hid-asus.c:1410:17: note: uninitialized use occurs here
1410 | if (*rsize == rsize_orig &&
| ^~~~~~~~~~
drivers/hid/hid-asus.c:1399:10: note: remove the 'if' if its condition is always true
1399 | } else if (drvdata->quirks & QUIRK_ZENBOOK_DUO_KEYBOARD) {
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/hid/hid-asus.c:1390:17: note: initialize the variable 'rsize_orig' to silence this warning
1390 | int rsize_orig;
| ^
| = 0
1 warning generated.
vim +1399 drivers/hid/hid-asus.c
1370
1371 static const __u8 *asus_report_fixup(struct hid_device *hdev, __u8 *rdesc,
1372 unsigned int *rsize)
1373 {
1374 struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
1375
1376 if (drvdata->quirks & QUIRK_FIX_NOTEBOOK_REPORT &&
1377 *rsize >= 56 && rdesc[54] == 0x25 && rdesc[55] == 0x65) {
1378 hid_info(hdev, "Fixing up Asus notebook report descriptor\n");
1379 rdesc[55] = 0xdd;
1380 }
1381 /* For the T100TA/T200TA keyboard dock */
1382 if (drvdata->quirks & QUIRK_T100_KEYBOARD &&
1383 (*rsize == 76 || *rsize == 101) &&
1384 rdesc[73] == 0x81 && rdesc[74] == 0x01) {
1385 hid_info(hdev, "Fixing up Asus T100 keyb report descriptor\n");
1386 rdesc[74] &= ~HID_MAIN_ITEM_CONSTANT;
1387 }
1388 /* For the T100CHI/T90CHI keyboard dock and Zenbook Duo 2024+ keyboards */
1389 if (drvdata->quirks & (QUIRK_T100CHI | QUIRK_T90CHI | QUIRK_ZENBOOK_DUO_KEYBOARD)) {
1390 int rsize_orig;
1391 int offs;
1392
1393 if (drvdata->quirks & QUIRK_T100CHI) {
1394 rsize_orig = 403;
1395 offs = 388;
1396 } else if (drvdata->quirks & QUIRK_T90CHI) {
1397 rsize_orig = 306;
1398 offs = 291;
> 1399 } else if (drvdata->quirks & QUIRK_ZENBOOK_DUO_KEYBOARD) {
1400 rsize_orig = 257;
1401 offs = 176;
1402 }
1403
1404 /*
1405 * Change Usage (76h) to Usage Minimum (00h), Usage Maximum
1406 * (FFh) and clear the flags in the Input() byte.
1407 * Note the descriptor has a bogus 0 byte at the end so we
1408 * only need 1 extra byte.
1409 */
1410 if (*rsize == rsize_orig &&
1411 rdesc[offs] == 0x09 && rdesc[offs + 1] == 0x76) {
1412 __u8 *new_rdesc;
1413
1414 new_rdesc = devm_kzalloc(&hdev->dev, rsize_orig + 1,
1415 GFP_KERNEL);
1416 if (!new_rdesc)
1417 return rdesc;
1418
1419 hid_info(hdev, "Fixing up %s keyb report descriptor\n",
1420 drvdata->quirks & QUIRK_T100CHI ?
1421 "T100CHI" : drvdata->quirks & QUIRK_T90CHI ?
1422 "T90CHI" : "ZENBOOK DUO");
1423
1424 memcpy(new_rdesc, rdesc, rsize_orig);
1425 *rsize = rsize_orig + 1;
1426 rdesc = new_rdesc;
1427
1428 memmove(rdesc + offs + 4, rdesc + offs + 2, 12);
1429 rdesc[offs] = 0x19;
1430 rdesc[offs + 1] = 0x00;
1431 rdesc[offs + 2] = 0x29;
1432 rdesc[offs + 3] = 0xff;
1433 rdesc[offs + 14] = 0x00;
1434 }
1435 }
1436
1437 if (drvdata->quirks & QUIRK_G752_KEYBOARD &&
1438 *rsize == 75 && rdesc[61] == 0x15 && rdesc[62] == 0x00) {
1439 /* report is missing usage minimum and maximum */
1440 __u8 *new_rdesc;
1441 size_t new_size = *rsize + sizeof(asus_g752_fixed_rdesc);
1442
1443 new_rdesc = devm_kzalloc(&hdev->dev, new_size, GFP_KERNEL);
1444 if (new_rdesc == NULL)
1445 return rdesc;
1446
1447 hid_info(hdev, "Fixing up Asus G752 keyb report descriptor\n");
1448 /* copy the valid part */
1449 memcpy(new_rdesc, rdesc, 61);
1450 /* insert missing part */
1451 memcpy(new_rdesc + 61, asus_g752_fixed_rdesc, sizeof(asus_g752_fixed_rdesc));
1452 /* copy remaining data */
1453 memcpy(new_rdesc + 61 + sizeof(asus_g752_fixed_rdesc), rdesc + 61, *rsize - 61);
1454
1455 *rsize = new_size;
1456 rdesc = new_rdesc;
1457 }
1458
1459 if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD &&
1460 *rsize == 331 && rdesc[190] == 0x85 && rdesc[191] == 0x5a &&
1461 rdesc[204] == 0x95 && rdesc[205] == 0x05) {
1462 hid_info(hdev, "Fixing up Asus N-KEY keyb report descriptor\n");
1463 rdesc[205] = 0x01;
1464 }
1465
1466 /* match many more n-key devices */
1467 if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD && *rsize > 15) {
1468 for (int i = 0; i < *rsize - 15; i++) {
1469 /* offset to the count from 0x5a report part always 14 */
1470 if (rdesc[i] == 0x85 && rdesc[i + 1] == 0x5a &&
1471 rdesc[i + 14] == 0x95 && rdesc[i + 15] == 0x05) {
1472 hid_info(hdev, "Fixing up Asus N-Key report descriptor\n");
1473 rdesc[i + 15] = 0x01;
1474 break;
1475 }
1476 }
1477 }
1478
1479 return rdesc;
1480 }
1481
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v8 2/7] mfd: Add driver for ASUS Transformer embedded controller
From: Lee Jones @ 2026-06-11 11:17 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Dmitry Torokhov,
Pavel Machek, Sebastian Reichel, Ion Agorria,
Michał Mirosław, devicetree, linux-kernel, linux-input,
linux-leds, linux-pm
In-Reply-To: <20260528053203.9339-3-clamor95@gmail.com>
On Thu, 28 May 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 | 16 +
> drivers/mfd/Makefile | 1 +
> drivers/mfd/asus-transformer-ec.c | 542 ++++++++++++++++++++++++
> include/linux/mfd/asus-transformer-ec.h | 92 ++++
> 4 files changed, 651 insertions(+)
> create mode 100644 drivers/mfd/asus-transformer-ec.c
> create mode 100644 include/linux/mfd/asus-transformer-ec.h
>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 7192c9d1d268..e1c32505b97a 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -137,6 +137,22 @@ config MFD_AAT2870_CORE
> additional drivers must be enabled in order to use the
> functionality of the device.
>
> +config MFD_ASUS_TRANSFORMER_EC
> + tristate "ASUS Transformer's embedded controller"
> + select MFD_CORE
> + depends on I2C && OF
> + help
> + Select this to enable support for the Embedded Controller (EC)
> + found in Tegra based ASUS Transformer series tablets and mobile
> + docks.
> +
> + This driver handles the core I2C communication with the EC and
> + provides support for its sub-devices, including battery management,
> + charger detection, LEDs and keyboard dock functions support.
> +
> + This driver can also be built as a module. If so, the module
> + will be called asus-transformer-ec.
> +
> config MFD_AT91_USART
> tristate "AT91 USART Driver"
> select MFD_CORE
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index e75e8045c28a..fd80088d8a9a 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_ASUS_TRANSFORMER_EC) += asus-transformer-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-transformer-ec.c b/drivers/mfd/asus-transformer-ec.c
> new file mode 100644
> index 000000000000..1f5900d0fdc9
> --- /dev/null
> +++ b/drivers/mfd/asus-transformer-ec.c
> @@ -0,0 +1,542 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +#include <linux/array_size.h>
> +#include <linux/debugfs.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/mfd/asus-transformer-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/slab.h>
> +#include <linux/string.h>
> +#include <linux/types.h>
> +#include <linux/unaligned.h>
> +
> +#define ASUSEC_RSP_BUFFER_SIZE (ASUSEC_ENTRIES / ASUSEC_ENTRY_SIZE)
> +
> +#define ASUSEC_RESET 0
> +#define ASUSEC_RETRY_MAX 3
> +#define ASUSEC_ACCESS_TIMEOUT 300
> +
> +enum asusec_variant {
> + ASUSEC_SL101_DOCK = 1,
> + ASUSEC_TF101_DOCK,
> + ASUSEC_TF201_PAD,
> + ASUSEC_TF600T_PAD,
> + ASUSEC_MAX
> +};
> +
> +enum asusec_mode {
> + ASUSEC_MODE_NONE,
> + ASUSEC_MODE_NORMAL,
> + ASUSEC_MODE_FACTORY,
> + ASUSEC_MODE_MAX
> +};
> +
> +/**
> + * struct asus_ec_chip_info
> + *
> + * @name: prefix associated with the EC
> + * @variant: id of programming model of EC
> + * @mode: state of Factory Mode bit in EC control register
> + */
> +struct asus_ec_chip_info {
> + const char *name;
> + enum asusec_variant variant;
> + enum asusec_mode fmode;
> +};
> +
> +/**
> + * struct asus_ec_data
> + *
> + * @ec: public part shared with all cells (must be first)
> + * @ecreq_lock: prevents simultaneous access to EC
> + * @ecreq_gpio: EC request GPIO
> + * @client: pointer to EC's i2c_client
> + * @info: pointer to EC's version description
> + * @ec_buf: buffer for EC read
> + * @logging_disabled: flag disabling logging on reset events
> + */
> +struct asus_ec_data {
> + struct asusec_core ec;
> + struct mutex ecreq_lock;
> + struct gpio_desc *ecreq_gpio;
> + struct i2c_client *client;
> + const struct asus_ec_chip_info *info;
> + u8 ec_buf[ASUSEC_ENTRY_BUFSIZE];
> + bool logging_disabled;
> +};
> +
> +/**
> + * struct dockram_ec_data
> + *
> + * @ctl_lock: prevent simultaneous access to Dockram
> + * @ctl_buf: buffer for Dockram read
> + */
> +struct dockram_ec_data {
> + struct mutex ctl_lock;
> + u8 ctl_buf[ASUSEC_ENTRY_BUFSIZE];
> +};
> +
> +/**
> + * asus_dockram_access_ctl - Read from or write to the DockRAM control register.
> + * @client: Handle to the DockRAM device.
> + * @out: Pointer to a variable where the register value will be stored.
> + * @mask: Bitmask of bits to be cleared.
> + * @xor: Bitmask of bits to be set (via XOR).
> + *
> + * This performs a control register read if @out is provided and both @mask
> + * and @xor are zero. Otherwise, it performs a control register update if
> + * @mask and @xor are provided.
> + *
> + * Returns a negative errno code else zero on success.
> + */
> +int asus_dockram_access_ctl(struct i2c_client *client, u64 *out, u64 mask,
> + u64 xor)
> +{
> + struct dockram_ec_data *ddata = i2c_get_clientdata(client);
> + u8 *buf = ddata->ctl_buf;
> + u64 val;
> + int ret = 0;
> +
> + guard(mutex)(&ddata->ctl_lock);
> +
> + memset(buf, 0, ASUSEC_ENTRY_BUFSIZE);
> + ret = i2c_smbus_read_i2c_block_data(client, ASUSEC_DOCKRAM_CONTROL,
> + ASUSEC_ENTRY_SIZE, buf);
> + if (ret < ASUSEC_ENTRY_SIZE) {
> + dev_err(&client->dev, "failed to access control buffer: %d\n",
> + ret);
> + return ret;
Should we return a negative error code here if the read is shorter than
expected, rather than propagating the positive byte count?
> + }
> +
> + if (buf[0] != ASUSEC_CTL_SIZE) {
> + dev_err(&client->dev, "buffer size exceeds %d: %d\n",
> + ASUSEC_CTL_SIZE, buf[0]);
> + return -EPROTO;
> + }
> +
> + val = get_unaligned_le64(buf + 1);
> +
> + if (out)
> + *out = val;
> +
> + if (mask || xor) {
> + put_unaligned_le64((val & ~mask) ^ xor, buf + 1);
> + ret = i2c_smbus_write_i2c_block_data(client,
> + ASUSEC_DOCKRAM_CONTROL,
> + ASUSEC_ENTRY_SIZE, buf);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(asus_dockram_access_ctl);
> +
> +static int asus_ec_signal_request(struct asus_ec_data *ddata)
> +{
> + guard(mutex)(&ddata->ecreq_lock);
> +
> + gpiod_set_value_cansleep(ddata->ecreq_gpio, 1);
> + msleep(50);
> +
> + gpiod_set_value_cansleep(ddata->ecreq_gpio, 0);
> + msleep(200);
Do these numbers come from the datasheet or were they arbitrarily chosen?
> +
> + return 0;
> +}
> +
> +static void asus_ec_clear_buffer(struct asus_ec_data *ddata)
> +{
> + int ret, retry = ASUSEC_RSP_BUFFER_SIZE;
> +
> + /*
> + * Read the buffer till we get valid data by checking ASUSEC_OBF_MASK
> + * of the status byte or till we reach end of the 256 byte buffer.
> + */
> + while (retry--) {
> + ret = i2c_smbus_read_i2c_block_data(ddata->client, ASUSEC_READ_BUF,
> + ASUSEC_ENTRY_SIZE,
> + ddata->ec_buf);
> + if (ret < ASUSEC_ENTRY_SIZE)
> + continue;
> +
> + if (ddata->ec_buf[ASUSEC_IRQ_STATUS] & ASUSEC_OBF_MASK)
> + continue;
> +
> + break;
> + }
> +}
> +
> +static int asus_ec_log_info(struct asus_ec_data *ddata, unsigned int reg,
> + const char *name, const char **out)
> +{
> + struct device *dev = &ddata->client->dev;
> + u8 buf[ASUSEC_ENTRY_BUFSIZE];
> + int ret;
> +
> + memset(buf, 0, ASUSEC_ENTRY_BUFSIZE);
> + ret = i2c_smbus_read_i2c_block_data(ddata->ec.dockram, reg,
> + ASUSEC_ENTRY_SIZE, buf);
> + if (ret < ASUSEC_ENTRY_SIZE)
> + return ret;
Same here. These should be negative.
> +
> + if (buf[0] > ASUSEC_ENTRY_SIZE) {
> + dev_err(dev, "bad data len; buffer: %*ph; ret: %d\n",
> + ASUSEC_ENTRY_BUFSIZE, buf, ret);
> + return -EPROTO;
> + }
> +
> + if (!ddata->logging_disabled) {
> + dev_info(dev, "%-14s: %.*s\n", name, buf[0], buf + 1);
> +
> + if (out) {
> + *out = devm_kasprintf(dev, GFP_KERNEL, "%.*s",
> + buf[0], buf + 1);
> + if (!*out)
> + return -ENOMEM;
> + }
> + }
FWIW, I hate this! What does it give you now that development is done?
> + return 0;
> +}
> +
> +static int asus_ec_reset(struct asus_ec_data *ddata)
> +{
> + int retry, ret;
> +
> + guard(mutex)(&ddata->ecreq_lock);
> +
> + for (retry = 0; retry < ASUSEC_RETRY_MAX; retry++) {
for (int return = ... is generally preferred for throwaway variables.
> + ret = i2c_smbus_write_word_data(ddata->client, ASUSEC_WRITE_BUF,
> + ASUSEC_RESET);
> + if (!ret)
> + return 0;
> +
> + msleep(ASUSEC_ACCESS_TIMEOUT);
I like that this is defined, can we do that with the others please?
> + }
> +
> + return ret;
> +}
> +
> +static int asus_ec_susb_on_status(struct asus_ec_data *ddata)
> +{
> + u64 flag;
> + int ret;
> +
> + ret = asus_dockram_access_ctl(ddata->ec.dockram, &flag, 0, 0);
> + if (ret)
> + return ret;
> +
> + flag &= ASUSEC_CTL_SUSB_MODE;
> + dev_info(&ddata->client->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 *ddata,
> + enum asusec_mode fmode)
> +{
> + dev_info(&ddata->client->dev, "Entering %s mode.\n",
> + fmode == ASUSEC_MODE_FACTORY ? "factory" : "normal");
> +
> + return asus_dockram_access_ctl(ddata->ec.dockram, NULL,
> + ASUSEC_CTL_FACTORY_MODE,
> + fmode == ASUSEC_MODE_FACTORY ?
> + ASUSEC_CTL_FACTORY_MODE : 0);
Why not create make:
ASUSEC_MODE_FACTORY == ASUSEC_CTL_FACTORY_MODE
What happens to NORMAL?
> +}
> +
> +static int asus_ec_detect(struct asus_ec_data *ddata)
> +{
> + int ret;
> +
> + ret = asus_ec_reset(ddata);
> + if (ret)
> + goto err_exit;
> +
> + asus_ec_clear_buffer(ddata);
> +
> + ret = asus_ec_log_info(ddata, ASUSEC_DOCKRAM_INFO_MODEL, "Model",
> + &ddata->ec.model);
You can use 100-chars and make the code look beautiful! :)
> + if (ret)
> + goto err_exit;
> +
> + ret = asus_ec_log_info(ddata, ASUSEC_DOCKRAM_INFO_FW, "FW version",
> + NULL);
> + if (ret)
> + goto err_exit;
> +
> + ret = asus_ec_log_info(ddata, ASUSEC_DOCKRAM_INFO_CFGFMT, "Config format",
> + NULL);
> + if (ret)
> + goto err_exit;
> +
> + ret = asus_ec_log_info(ddata, ASUSEC_DOCKRAM_INFO_HW, "HW version",
> + NULL);
> + if (ret)
> + goto err_exit;
> +
> + /* Disable logging on next EC request */
Why, but why?
> + ddata->logging_disabled = true;
> +
> + /* Check and inform about EC firmware behavior */
> + ret = asus_ec_susb_on_status(ddata);
> + if (ret)
> + goto err_exit;
> +
> + ddata->ec.name = ddata->info->name;
> +
> + /* Some EC require factory mode to be set normal on each request */
> + if (ddata->info->fmode)
> + ret = asus_ec_set_factory_mode(ddata, ddata->info->fmode);
> +
> +err_exit:
> + if (ret)
> + dev_err(&ddata->client->dev, "failed to access EC: %d\n", ret);
> +
> + return ret;
> +}
> +
> +static void asus_ec_handle_smi(struct asus_ec_data *ddata, unsigned int code)
> +{
> + switch (code) {
> + case ASUSEC_SMI_HANDSHAKE:
> + case ASUSEC_SMI_RESET:
> + asus_ec_detect(ddata);
> + break;
> + }
> +}
> +
> +static irqreturn_t asus_ec_interrupt(int irq, void *dev_id)
> +{
> + struct asus_ec_data *ddata = dev_id;
> + unsigned long notify_action;
> + int ret;
> +
> + ret = i2c_smbus_read_i2c_block_data(ddata->client, ASUSEC_READ_BUF,
> + ASUSEC_ENTRY_SIZE, ddata->ec_buf);
> + if (ret < ASUSEC_ENTRY_SIZE ||
> + !(ddata->ec_buf[ASUSEC_IRQ_STATUS] & ASUSEC_OBF_MASK))
Unwrap for readability.
Also, I think a comment would be helpful.
> + return IRQ_NONE;
> +
> + notify_action = ddata->ec_buf[ASUSEC_IRQ_STATUS];
> + if (notify_action & ASUSEC_SMI_MASK) {
> + unsigned int code = ddata->ec_buf[ASUSEC_SMI_CODE];
> +
> + asus_ec_handle_smi(ddata, code);
> +
> + notify_action |= code << 8;
> + }
> +
> + blocking_notifier_call_chain(&ddata->ec.notify_list,
> + notify_action, ddata->ec_buf);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static void asus_ec_release_dockram_dev(void *client)
> +{
> + i2c_unregister_device(client);
> +}
> +
> +static struct i2c_client *devm_asus_dockram_get(struct device *dev)
> +{
> + struct i2c_client *parent = to_i2c_client(dev);
> + struct i2c_client *dockram;
> + struct dockram_ec_data *ddata;
> + int ret;
> +
> + dockram = i2c_new_ancillary_device(parent, "dockram",
> + parent->addr + 2);
Could we define a macro for the address offset '2' here to avoid using a magic
number?
> + if (IS_ERR(dockram))
> + return dockram;
> +
> + ret = devm_add_action_or_reset(dev, asus_ec_release_dockram_dev,
> + dockram);
> + if (ret)
> + return ERR_PTR(ret);
> +
> + ddata = devm_kzalloc(&dockram->dev, sizeof(*ddata), GFP_KERNEL);
> + if (!ddata)
> + return ERR_PTR(-ENOMEM);
> +
> + i2c_set_clientdata(dockram, ddata);
> + mutex_init(&ddata->ctl_lock);
> +
> + return dockram;
> +}
> +
> +static const struct mfd_cell asus_ec_sl101_dock_mfd_devices[] = {
> + MFD_CELL_NAME("asus-transformer-ec-kbc"),
> +};
> +
> +static const struct mfd_cell asus_ec_tf101_dock_mfd_devices[] = {
> + MFD_CELL_BASIC("asus-transformer-ec-battery", NULL, NULL, 0, 1),
> + MFD_CELL_BASIC("asus-transformer-ec-charger", NULL, NULL, 0, 1),
> + MFD_CELL_BASIC("asus-transformer-ec-led", NULL, NULL, 0, 1),
> + MFD_CELL_NAME("asus-transformer-ec-keys"),
> + MFD_CELL_NAME("asus-transformer-ec-kbc"),
> +};
> +
> +static const struct mfd_cell asus_ec_tf201_pad_mfd_devices[] = {
> + MFD_CELL_NAME("asus-transformer-ec-battery"),
> + MFD_CELL_NAME("asus-transformer-ec-led"),
> +};
> +
> +static const struct mfd_cell asus_ec_tf600t_pad_mfd_devices[] = {
> + MFD_CELL_NAME("asus-transformer-ec-battery"),
> + MFD_CELL_NAME("asus-transformer-ec-charger"),
> + MFD_CELL_NAME("asus-transformer-ec-led"),
> +};
> +
> +static int asus_ec_probe(struct i2c_client *client)
> +{
> + struct device *dev = &client->dev;
> + struct asus_ec_data *ddata;
> + const struct mfd_cell *cells;
> + unsigned int num_cells;
> + unsigned long irqflags;
> + int ret;
> +
> + if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_I2C_BLOCK))
> + return dev_err_probe(dev, -ENXIO,
> + "I2C bus is missing required SMBus block mode support\n");
> +
> + ddata = devm_kzalloc(dev, sizeof(*ddata), GFP_KERNEL);
> + if (!ddata)
> + return -ENOMEM;
> +
> + ddata->info = device_get_match_data(dev);
> + if (!ddata->info)
> + return -ENODEV;
> +
> + switch (ddata->info->variant) {
> + case ASUSEC_SL101_DOCK:
> + cells = asus_ec_sl101_dock_mfd_devices;
> + num_cells = ARRAY_SIZE(asus_ec_sl101_dock_mfd_devices);
> + break;
> + case ASUSEC_TF101_DOCK:
> + cells = asus_ec_tf101_dock_mfd_devices;
> + num_cells = ARRAY_SIZE(asus_ec_tf101_dock_mfd_devices);
> + break;
> + case ASUSEC_TF201_PAD:
> + cells = asus_ec_tf201_pad_mfd_devices;
> + num_cells = ARRAY_SIZE(asus_ec_tf201_pad_mfd_devices);
> + break;
> + case ASUSEC_TF600T_PAD:
> + cells = asus_ec_tf600t_pad_mfd_devices;
> + num_cells = ARRAY_SIZE(asus_ec_tf600t_pad_mfd_devices);
> + break;
> + default:
> + return dev_err_probe(dev, -EINVAL,
> + "unknown device variant %d\n",
> + ddata->info->variant);
> + }
> +
> + i2c_set_clientdata(client, ddata);
> + ddata->client = client;
> +
> + ddata->ec.dockram = devm_asus_dockram_get(dev);
> + if (IS_ERR(ddata->ec.dockram))
> + return dev_err_probe(dev, PTR_ERR(ddata->ec.dockram),
> + "failed to get dockram\n");
> +
> + ddata->ecreq_gpio = devm_gpiod_get(dev, "request", GPIOD_OUT_LOW);
> + if (IS_ERR(ddata->ecreq_gpio))
> + return dev_err_probe(dev, PTR_ERR(ddata->ecreq_gpio),
> + "failed to get EC request GPIO\n");
> +
> + BLOCKING_INIT_NOTIFIER_HEAD(&ddata->ec.notify_list);
> + mutex_init(&ddata->ecreq_lock);
> +
> + asus_ec_signal_request(ddata);
> +
> + ret = asus_ec_detect(ddata);
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to detect EC version\n");
> +
> + /*
> + * Systems using device tree should set up interrupt via DTS,
> + * the rest will use the default low interrupt.
> + */
> + irqflags = dev->of_node ? 0 : IRQF_TRIGGER_LOW;
> +
> + ret = devm_request_threaded_irq(dev, client->irq, NULL,
> + &asus_ec_interrupt,
> + IRQF_ONESHOT | irqflags,
> + client->name, ddata);
> + 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;
> +
> + return devm_mfd_add_devices(dev, 0, cells, num_cells, NULL, 0, NULL);
> +}
> +
> +static const struct asus_ec_chip_info asus_ec_sl101_dock_data = {
> + .name = "dock",
> + .variant = ASUSEC_SL101_DOCK,
> + .fmode = ASUSEC_MODE_NONE,
> +};
> +
> +static const struct asus_ec_chip_info asus_ec_tf101_dock_data = {
> + .name = "dock",
> + .variant = ASUSEC_TF101_DOCK,
> + .fmode = ASUSEC_MODE_NONE,
> +};
> +
> +static const struct asus_ec_chip_info asus_ec_tf201_pad_data = {
> + .name = "pad",
> + .variant = ASUSEC_TF201_PAD,
> + .fmode = ASUSEC_MODE_NORMAL,
> +};
> +
> +static const struct asus_ec_chip_info asus_ec_tf600t_pad_data = {
> + .name = "pad",
> + .variant = ASUSEC_TF600T_PAD,
> + .fmode = ASUSEC_MODE_NORMAL,
> +};
Any reason not to just pass the identifier (variant) and add the name
and fmode attribues to the switch() above?
> +
> +static const struct of_device_id asus_ec_match[] = {
> + {
> + .compatible = "asus,sl101-ec-dock",
> + .data = &asus_ec_sl101_dock_data
> + }, {
> + .compatible = "asus,tf101-ec-dock",
> + .data = &asus_ec_tf101_dock_data
> + }, {
> + .compatible = "asus,tf201-ec-pad",
> + .data = &asus_ec_tf201_pad_data
> + }, {
> + .compatible = "asus,tf600t-ec-pad",
> + .data = &asus_ec_tf600t_pad_data
> + },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, asus_ec_match);
> +
> +static struct i2c_driver asus_ec_driver = {
> + .driver = {
> + .name = "asus-transformer-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-transformer-ec.h b/include/linux/mfd/asus-transformer-ec.h
> new file mode 100644
> index 000000000000..f085eea2193e
> --- /dev/null
> +++ b/include/linux/mfd/asus-transformer-ec.h
> @@ -0,0 +1,92 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +#ifndef __MFD_ASUS_TRANSFORMER_EC_H
> +#define __MFD_ASUS_TRANSFORMER_EC_H
> +
> +#include <linux/notifier.h>
> +#include <linux/platform_device.h>
> +
> +struct i2c_client;
> +
> +/**
> + * struct asusec_core - public part shared with all cells
> + *
> + * @model: firmware version running on the EC
> + * @name: prefix associated with the EC
> + * @dockram: pointer to Dockram's i2c_client
> + * @notify_list: notify list used by cells
> + */
> +struct asusec_core {
> + const char *model;
> + const char *name;
> + struct i2c_client *dockram;
> + struct blocking_notifier_head notify_list;
> +};
> +
> +#define ASUSEC_ENTRIES 0x100
> +#define ASUSEC_ENTRY_SIZE 32
> +#define ASUSEC_ENTRY_BUFSIZE (ASUSEC_ENTRY_SIZE + 1)
> +
> +/* interrupt sources */
> +#define ASUSEC_IRQ_STATUS 1
> +#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_CODE 2
> +#define ASUSEC_SMI_POWER_NOTIFY 0x31 /* USB cable plug event */
> +#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 /* charger to dock plug event */
> +#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(9)
> +#define ASUSEC_CMD_SUSPEND_S3 BIT_ULL(33)
> +#define ASUSEC_CTL_TEST_DISCHARGE BIT_ULL(35)
> +#define ASUSEC_CMD_SUSPEND_INHIBIT BIT_ULL(37)
> +#define ASUSEC_CTL_FACTORY_MODE BIT_ULL(38)
> +#define ASUSEC_CTL_KEEP_AWAKE BIT_ULL(39)
> +#define ASUSEC_CTL_USB_CHARGE BIT_ULL(40)
> +#define ASUSEC_CTL_LED_BLINK BIT_ULL(40)
> +#define ASUSEC_CTL_LED_AMBER BIT_ULL(41)
> +#define ASUSEC_CTL_LED_GREEN BIT_ULL(42)
> +#define ASUSEC_CMD_SWITCH_HDMI BIT_ULL(56)
> +#define ASUSEC_CMD_WIN_SHUTDOWN BIT_ULL(62)
> +
> +#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
> +
> +int asus_dockram_access_ctl(struct i2c_client *client,
> + u64 *out, u64 mask, u64 xor);
> +
> +#endif /* __MFD_ASUS_TRANSFORMER_EC_H */
> --
> 2.51.0
>
--
Lee Jones
^ permalink raw reply
* Re: [PATCH v8 5/7] leds: Add driver for ASUS Transformer LEDs
From: Lee Jones @ 2026-06-11 11:30 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Dmitry Torokhov,
Pavel Machek, Sebastian Reichel, Ion Agorria,
Michał Mirosław, devicetree, linux-kernel, linux-input,
linux-leds, linux-pm
In-Reply-To: <20260528053203.9339-6-clamor95@gmail.com>
On Thu, 28 May 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-transformer-ec.c | 125 ++++++++++++++++++++++++
> 3 files changed, 137 insertions(+)
> create mode 100644 drivers/leds/leds-asus-transformer-ec.c
>
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index f4a0a3c8c870..f637d23400a8 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_ASUS_TRANSFORMER_EC
> + tristate "LED Support for Asus Transformer charging LED"
> + depends on LEDS_CLASS
> + depends on MFD_ASUS_TRANSFORMER_EC
> + 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-transformer-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..d5395c3f1124 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_ASUS_TRANSFORMER_EC) += leds-asus-transformer-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-transformer-ec.c b/drivers/leds/leds-asus-transformer-ec.c
> new file mode 100644
> index 000000000000..09503e76331c
> --- /dev/null
> +++ b/drivers/leds/leds-asus-transformer-ec.c
> @@ -0,0 +1,125 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <linux/err.h>
> +#include <linux/leds.h>
> +#include <linux/mfd/asus-transformer-ec.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +enum {
> + ASUSEC_LED_AMBER,
> + ASUSEC_LED_GREEN,
> + ASUSEC_LED_MAX
> +};
> +
> +struct asus_ec_led_config {
> + const char *name;
> + unsigned int color;
> + unsigned long long ctrl_bit;
Should we use 'u64' here instead of 'unsigned long long' to align with standard
kernel integer types?
> +};
> +
> +struct asus_ec_led {
> + struct asus_ec_leds_data *ddata;
> + struct led_classdev cdev;
> + unsigned long long ctrl_bit;
Should we use 'u64' here as well to keep it consistent?
> +};
> +
> +struct asus_ec_leds_data {
> + const struct asusec_core *ec;
> + struct asus_ec_led leds[ASUSEC_LED_MAX];
> +};
> +
> +static const struct asus_ec_led_config asus_ec_leds[] = {
> + [ASUSEC_LED_AMBER] = {
> + .name = "amber",
> + .color = LED_COLOR_ID_AMBER,
> + .ctrl_bit = ASUSEC_CTL_LED_AMBER,
> + },
> + [ASUSEC_LED_GREEN] = {
> + .name = "green",
> + .color = LED_COLOR_ID_GREEN,
> + .ctrl_bit = ASUSEC_CTL_LED_GREEN,
> + },
> +};
> +
> +static enum led_brightness asus_ec_led_get_brightness(struct led_classdev *cdev)
> +{
> + struct asus_ec_led *led = container_of(cdev, struct asus_ec_led, cdev);
> + const struct asusec_core *ec = led->ddata->ec;
I'm getting confused here.
ddata is what I'd be calling the device data struct passed by the parent?
In fact, ddata is a little known concept in Leds. Any reason to go for
this over the standard nomenclature?
> + u64 ctl;
> + int ret;
> +
> + ret = asus_dockram_access_ctl(ec->dockram, &ctl, 0, 0);
Did we discuss preferring regmap already?
> + if (ret)
> + return LED_OFF;
> +
> + return ctl & led->ctrl_bit ? LED_ON : LED_OFF;
> +}
> +
> +static int asus_ec_led_set_brightness(struct led_classdev *cdev,
> + enum led_brightness brightness)
> +{
> + struct asus_ec_led *led = container_of(cdev, struct asus_ec_led, cdev);
> + const struct asusec_core *ec = led->ddata->ec;
> +
> + if (brightness)
> + return asus_dockram_access_ctl(ec->dockram, NULL,
> + led->ctrl_bit, led->ctrl_bit);
> +
> + return asus_dockram_access_ctl(ec->dockram, NULL, led->ctrl_bit, 0);
> +}
> +
> +static int asus_ec_led_probe(struct platform_device *pdev)
> +{
> + const struct asusec_core *ec = dev_get_drvdata(pdev->dev.parent);
> + struct asus_ec_leds_data *ddata;
> + struct device *dev = &pdev->dev;
> + int i, ret;
Could we declare the loop counter 'i' directly within the 'for' statement's
scope to keep its scope limited? For example, 'for (int i = 0; ...)'.
> +
> + ddata = devm_kzalloc(dev, sizeof(*ddata), GFP_KERNEL);
> + if (!ddata)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, ddata);
> + ddata->ec = ec;
> +
> + for (i = 0; i < ASUSEC_LED_MAX; i++) {
Nit: for (int i = ...
> + const struct asus_ec_led_config *cfg = &asus_ec_leds[i];
> + struct asus_ec_led *led = &ddata->leds[i];
> +
> + led->cdev.name = devm_kasprintf(dev, GFP_KERNEL, "%s::%s",
> + ddata->ec->name, cfg->name);
> + if (!led->cdev.name)
> + return -ENOMEM;
> +
> + led->cdev.max_brightness = 1;
> + led->cdev.color = cfg->color;
> + led->cdev.flags = LED_CORE_SUSPENDRESUME | LED_RETAIN_AT_SHUTDOWN;
> + led->cdev.brightness_get = asus_ec_led_get_brightness;
> + led->cdev.brightness_set_blocking = asus_ec_led_set_brightness;
> +
> + led->ddata = ddata;
> + led->ctrl_bit = cfg->ctrl_bit;
> +
> + ret = devm_led_classdev_register(dev, &led->cdev);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "failed to register %s LED\n",
> + cfg->name);
Should we capitalise the error message here to match our style guidelines
(e.g. 'Failed to register...')?
> + }
> +
> + return 0;
> +}
> +
> +static struct platform_driver asus_ec_led_driver = {
> + .driver.name = "asus-transformer-ec-led",
> + .probe = asus_ec_led_probe,
> +};
> +module_platform_driver(asus_ec_led_driver);
> +
> +MODULE_ALIAS("platform:asus-transformer-ec-led");
> +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 0/7] HID: iio: basic clean up for usage_id
From: Jonathan Cameron @ 2026-06-11 11:59 UTC (permalink / raw)
To: Sanjay Chitroda
Cc: Jiri Kosina, Srinivas Pandruvada, David Lechner, Nuno Sá,
Andy Shevchenko, linux-input, linux-iio, linux-kernel,
Maxwell Doose
In-Reply-To: <20260610-6-june-hid-iio-correct-usage-id-v2-0-c3c5f0720493@gmail.com>
On Wed, 10 Jun 2026 21:07:01 +0530
Sanjay Chitroda <sanjayembeddedse@gmail.com> wrote:
> Hi all,
>
> This series updates all HID IIO drivers to use 'u32' instead of
> bare 'unsigned' for the usage_id parameter.
>
> This matches expected callback API type and improves code clarity,
> as HID usage IDs are defined as 32-bit values.
>
> No functional changes are introduced.
>
> Testing:
> - Compiled with W=1 for each patch in the series
LGTM so I've applied it to the testing branch of iio.git. However, I won't
be doing another pull request this cycle so this tree will be rebased once rc1
is available. In meantime I'm happy to add tags or indeed back this out
if people have feedback.
Thanks,
Jonathan
>
> ---
> Changes in v2:
> - rectify commit message with input from Jonathan
> - added reviewed by tag in all change of series
> - Link to v1: https://patch.msgid.link/20260606-6-june-hid-iio-correct-usage-id-v1-0-dd4a6820b674@gmail.com
>
> ---
> Sanjay Chitroda (7):
> iio: gyro: hid-sensor-gyro-3d: use u32 instead of unsigned
> iio: accel: hid-sensor-accel-3d: use u32 instead of unsigned
> iio: light: hid-sensor-als: use u32 instead of unsigned
> iio: light: hid-sensor-prox: use u32 instead of unsigned
> iio: orientation: hid-sensor-incl-3d: use u32 instead of unsigned
> iio: orientation: hid-sensor-rotation: use u32 instead of unsigned
> iio: pressure: hid-sensor-press: use u32 instead of unsigned
>
> drivers/iio/accel/hid-sensor-accel-3d.c | 6 +++---
> drivers/iio/gyro/hid-sensor-gyro-3d.c | 6 +++---
> drivers/iio/light/hid-sensor-als.c | 6 +++---
> drivers/iio/light/hid-sensor-prox.c | 4 ++--
> drivers/iio/orientation/hid-sensor-incl-3d.c | 6 +++---
> drivers/iio/orientation/hid-sensor-rotation.c | 6 +++---
> drivers/iio/pressure/hid-sensor-press.c | 6 +++---
> 7 files changed, 20 insertions(+), 20 deletions(-)
> ---
> base-commit: ae696dfa47c30016cd429b9db5e70b259b8f509e
> change-id: 20260606-6-june-hid-iio-correct-usage-id-57ce92cb102b
>
> Best regards,
> --
> Sanjay Chitroda <sanjayembeddedse@gmail.com>
>
^ permalink raw reply
* Re: [PATCH v8 2/7] mfd: Add driver for ASUS Transformer embedded controller
From: Svyatoslav Ryhel @ 2026-06-11 12:16 UTC (permalink / raw)
To: Lee Jones
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Dmitry Torokhov,
Pavel Machek, Sebastian Reichel, Ion Agorria,
Michał Mirosław, devicetree, linux-kernel, linux-input,
linux-leds, linux-pm
In-Reply-To: <20260611111732.GN4151951@google.com>
чт, 11 черв. 2026 р. о 14:17 Lee Jones <lee@kernel.org> пише:
>
> On Thu, 28 May 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 | 16 +
> > drivers/mfd/Makefile | 1 +
> > drivers/mfd/asus-transformer-ec.c | 542 ++++++++++++++++++++++++
> > include/linux/mfd/asus-transformer-ec.h | 92 ++++
> > 4 files changed, 651 insertions(+)
> > create mode 100644 drivers/mfd/asus-transformer-ec.c
> > create mode 100644 include/linux/mfd/asus-transformer-ec.h
> >
...
> > +
> > + memset(buf, 0, ASUSEC_ENTRY_BUFSIZE);
> > + ret = i2c_smbus_read_i2c_block_data(client, ASUSEC_DOCKRAM_CONTROL,
> > + ASUSEC_ENTRY_SIZE, buf);
> > + if (ret < ASUSEC_ENTRY_SIZE) {
> > + dev_err(&client->dev, "failed to access control buffer: %d\n",
> > + ret);
> > + return ret;
>
> Should we return a negative error code here if the read is shorter than
> expected, rather than propagating the positive byte count?
>
Yes, I have adjusted it already locally for the next iteration. It
will return ret if negative and -EIO if ret is pos but less then
ASUSEC_ENTRY_SIZE (return ret < 0 ? ret : -EIO)
> > + }
> > +
> > + if (buf[0] != ASUSEC_CTL_SIZE) {
> > + dev_err(&client->dev, "buffer size exceeds %d: %d\n",
> > + ASUSEC_CTL_SIZE, buf[0]);
> > + return -EPROTO;
> > + }
> > +
> > + val = get_unaligned_le64(buf + 1);
> > +
> > + if (out)
> > + *out = val;
> > +
> > + if (mask || xor) {
> > + put_unaligned_le64((val & ~mask) ^ xor, buf + 1);
> > + ret = i2c_smbus_write_i2c_block_data(client,
> > + ASUSEC_DOCKRAM_CONTROL,
> > + ASUSEC_ENTRY_SIZE, buf);
> > + if (ret)
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(asus_dockram_access_ctl);
> > +
> > +static int asus_ec_signal_request(struct asus_ec_data *ddata)
> > +{
> > + guard(mutex)(&ddata->ecreq_lock);
> > +
> > + gpiod_set_value_cansleep(ddata->ecreq_gpio, 1);
> > + msleep(50);
> > +
> > + gpiod_set_value_cansleep(ddata->ecreq_gpio, 0);
> > + msleep(200);
>
> Do these numbers come from the datasheet or were they arbitrarily chosen?
>
There is no datasheet. These delays come from downstream driver and
with lower values or removed delays request fails.
> > +
> > + return 0;
> > +}
> > +
> > +static void asus_ec_clear_buffer(struct asus_ec_data *ddata)
> > +{
> > + int ret, retry = ASUSEC_RSP_BUFFER_SIZE;
> > +
> > + /*
> > + * Read the buffer till we get valid data by checking ASUSEC_OBF_MASK
> > + * of the status byte or till we reach end of the 256 byte buffer.
> > + */
> > + while (retry--) {
> > + ret = i2c_smbus_read_i2c_block_data(ddata->client, ASUSEC_READ_BUF,
> > + ASUSEC_ENTRY_SIZE,
> > + ddata->ec_buf);
> > + if (ret < ASUSEC_ENTRY_SIZE)
> > + continue;
> > +
> > + if (ddata->ec_buf[ASUSEC_IRQ_STATUS] & ASUSEC_OBF_MASK)
> > + continue;
> > +
> > + break;
> > + }
> > +}
> > +
> > +static int asus_ec_log_info(struct asus_ec_data *ddata, unsigned int reg,
> > + const char *name, const char **out)
> > +{
> > + struct device *dev = &ddata->client->dev;
> > + u8 buf[ASUSEC_ENTRY_BUFSIZE];
> > + int ret;
> > +
> > + memset(buf, 0, ASUSEC_ENTRY_BUFSIZE);
> > + ret = i2c_smbus_read_i2c_block_data(ddata->ec.dockram, reg,
> > + ASUSEC_ENTRY_SIZE, buf);
> > + if (ret < ASUSEC_ENTRY_SIZE)
> > + return ret;
>
> Same here. These should be negative.
>
return ret < 0 ? ret : -EIO same as above
> > +
> > + if (buf[0] > ASUSEC_ENTRY_SIZE) {
> > + dev_err(dev, "bad data len; buffer: %*ph; ret: %d\n",
> > + ASUSEC_ENTRY_BUFSIZE, buf, ret);
> > + return -EPROTO;
> > + }
> > +
> > + if (!ddata->logging_disabled) {
> > + dev_info(dev, "%-14s: %.*s\n", name, buf[0], buf + 1);
> > +
> > + if (out) {
> > + *out = devm_kasprintf(dev, GFP_KERNEL, "%.*s",
> > + buf[0], buf + 1);
> > + if (!*out)
> > + return -ENOMEM;
> > + }
> > + }
>
> FWIW, I hate this! What does it give you now that development is done?
>
We have already discussed this, and you agreed that EC and firmware
prints may stay! This prints EC model and firmware info as well as EC
firmware behavior. It allows identify possible new revisions of EC -
Firmware combo and address possible regressions (check if it is chip
malfunction or firmware needs a new programming model) without
rebuilding kernel and digging downstream kernel for needed bits of
code.
> > + return 0;
> > +}
> > +
> > +static int asus_ec_reset(struct asus_ec_data *ddata)
> > +{
> > + int retry, ret;
> > +
> > + guard(mutex)(&ddata->ecreq_lock);
> > +
> > + for (retry = 0; retry < ASUSEC_RETRY_MAX; retry++) {
>
> for (int return = ... is generally preferred for throwaway variables.
>
Not that I care too much, but I am defining ret anyway, why not add
retry too there?
>
> > + ret = i2c_smbus_write_word_data(ddata->client, ASUSEC_WRITE_BUF,
> > + ASUSEC_RESET);
> > + if (!ret)
> > + return 0;
> > +
> > + msleep(ASUSEC_ACCESS_TIMEOUT);
>
> I like that this is defined, can we do that with the others please?
>
I don't see any benefits of defining delays, they are all arbitrary
and derive from downstream kernel, removing or altering them caused
malfunction.
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static int asus_ec_susb_on_status(struct asus_ec_data *ddata)
> > +{
> > + u64 flag;
> > + int ret;
> > +
> > + ret = asus_dockram_access_ctl(ddata->ec.dockram, &flag, 0, 0);
> > + if (ret)
> > + return ret;
> > +
> > + flag &= ASUSEC_CTL_SUSB_MODE;
> > + dev_info(&ddata->client->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 *ddata,
> > + enum asusec_mode fmode)
> > +{
> > + dev_info(&ddata->client->dev, "Entering %s mode.\n",
> > + fmode == ASUSEC_MODE_FACTORY ? "factory" : "normal");
> > +
> > + return asus_dockram_access_ctl(ddata->ec.dockram, NULL,
> > + ASUSEC_CTL_FACTORY_MODE,
> > + fmode == ASUSEC_MODE_FACTORY ?
> > + ASUSEC_CTL_FACTORY_MODE : 0);
>
> Why not create make:
>
> ASUSEC_MODE_FACTORY == ASUSEC_CTL_FACTORY_MODE
>
> What happens to NORMAL?
>
ASUSEC_CTL_FACTORY_MODE is a bit in the ctl register. For NORMAL mode
bit is cleared,
for FACTORY bit it set, for NONE bit is ignored.
> > +}
> > +
> > +static int asus_ec_detect(struct asus_ec_data *ddata)
> > +{
> > + int ret;
> > +
> > + ret = asus_ec_reset(ddata);
> > + if (ret)
> > + goto err_exit;
> > +
> > + asus_ec_clear_buffer(ddata);
> > +
> > + ret = asus_ec_log_info(ddata, ASUSEC_DOCKRAM_INFO_MODEL, "Model",
> > + &ddata->ec.model);
>
> You can use 100-chars and make the code look beautiful! :)
>
Not every subsystem permits 100 chars, some stick to 80 as a strict
rule, so it is better be safe.
> > + if (ret)
> > + goto err_exit;
> > +
> > + ret = asus_ec_log_info(ddata, ASUSEC_DOCKRAM_INFO_FW, "FW version",
> > + NULL);
> > + if (ret)
> > + goto err_exit;
> > +
> > + ret = asus_ec_log_info(ddata, ASUSEC_DOCKRAM_INFO_CFGFMT, "Config format",
> > + NULL);
> > + if (ret)
> > + goto err_exit;
> > +
> > + ret = asus_ec_log_info(ddata, ASUSEC_DOCKRAM_INFO_HW, "HW version",
> > + NULL);
> > + if (ret)
> > + goto err_exit;
> > +
> > + /* Disable logging on next EC request */
>
> Why, but why?
>
Cause EC requests are frequent (handshake/reset) and constant logging
same data is not acceptable.
> > + ddata->logging_disabled = true;
> > +
> > + /* Check and inform about EC firmware behavior */
> > + ret = asus_ec_susb_on_status(ddata);
> > + if (ret)
> > + goto err_exit;
> > +
> > + ddata->ec.name = ddata->info->name;
> > +
> > + /* Some EC require factory mode to be set normal on each request */
> > + if (ddata->info->fmode)
> > + ret = asus_ec_set_factory_mode(ddata, ddata->info->fmode);
> > +
> > +err_exit:
> > + if (ret)
> > + dev_err(&ddata->client->dev, "failed to access EC: %d\n", ret);
> > +
> > + return ret;
> > +}
> > +
> > +static void asus_ec_handle_smi(struct asus_ec_data *ddata, unsigned int code)
> > +{
> > + switch (code) {
> > + case ASUSEC_SMI_HANDSHAKE:
> > + case ASUSEC_SMI_RESET:
> > + asus_ec_detect(ddata);
> > + break;
> > + }
> > +}
> > +
> > +static irqreturn_t asus_ec_interrupt(int irq, void *dev_id)
> > +{
> > + struct asus_ec_data *ddata = dev_id;
> > + unsigned long notify_action;
> > + int ret;
> > +
> > + ret = i2c_smbus_read_i2c_block_data(ddata->client, ASUSEC_READ_BUF,
> > + ASUSEC_ENTRY_SIZE, ddata->ec_buf);
> > + if (ret < ASUSEC_ENTRY_SIZE ||
> > + !(ddata->ec_buf[ASUSEC_IRQ_STATUS] & ASUSEC_OBF_MASK))
>
> Unwrap for readability.
>
> Also, I think a comment would be helpful.
>
if (ret < ASUSEC_ENTRY_SIZE)
return IRQ_NONE;
ret = ddata->ec_buf[ASUSEC_IRQ_STATUS] & ASUSEC_OBF_MASK;
if (!ret)
return IRQ_NONE;
This would be acceptable? (I will add comments later on)
> > + return IRQ_NONE;
> > +
> > + notify_action = ddata->ec_buf[ASUSEC_IRQ_STATUS];
> > + if (notify_action & ASUSEC_SMI_MASK) {
> > + unsigned int code = ddata->ec_buf[ASUSEC_SMI_CODE];
> > +
> > + asus_ec_handle_smi(ddata, code);
> > +
> > + notify_action |= code << 8;
> > + }
> > +
> > + blocking_notifier_call_chain(&ddata->ec.notify_list,
> > + notify_action, ddata->ec_buf);
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
> > +static void asus_ec_release_dockram_dev(void *client)
> > +{
> > + i2c_unregister_device(client);
> > +}
> > +
> > +static struct i2c_client *devm_asus_dockram_get(struct device *dev)
> > +{
> > + struct i2c_client *parent = to_i2c_client(dev);
> > + struct i2c_client *dockram;
> > + struct dockram_ec_data *ddata;
> > + int ret;
> > +
> > + dockram = i2c_new_ancillary_device(parent, "dockram",
> > + parent->addr + 2);
>
> Could we define a macro for the address offset '2' here to avoid using a magic
> number?
>
It seems that you are excessively concerned with "magic numbers".
> > + if (IS_ERR(dockram))
> > + return dockram;
> > +
> > + ret = devm_add_action_or_reset(dev, asus_ec_release_dockram_dev,
> > + dockram);
> > + if (ret)
> > + return ERR_PTR(ret);
> > +
> > + ddata = devm_kzalloc(&dockram->dev, sizeof(*ddata), GFP_KERNEL);
> > + if (!ddata)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + i2c_set_clientdata(dockram, ddata);
> > + mutex_init(&ddata->ctl_lock);
> > +
> > + return dockram;
> > +}
> > +
> > +static const struct mfd_cell asus_ec_sl101_dock_mfd_devices[] = {
> > + MFD_CELL_NAME("asus-transformer-ec-kbc"),
> > +};
> > +
> > +static const struct mfd_cell asus_ec_tf101_dock_mfd_devices[] = {
> > + MFD_CELL_BASIC("asus-transformer-ec-battery", NULL, NULL, 0, 1),
> > + MFD_CELL_BASIC("asus-transformer-ec-charger", NULL, NULL, 0, 1),
> > + MFD_CELL_BASIC("asus-transformer-ec-led", NULL, NULL, 0, 1),
> > + MFD_CELL_NAME("asus-transformer-ec-keys"),
> > + MFD_CELL_NAME("asus-transformer-ec-kbc"),
> > +};
> > +
> > +static const struct mfd_cell asus_ec_tf201_pad_mfd_devices[] = {
> > + MFD_CELL_NAME("asus-transformer-ec-battery"),
> > + MFD_CELL_NAME("asus-transformer-ec-led"),
> > +};
> > +
> > +static const struct mfd_cell asus_ec_tf600t_pad_mfd_devices[] = {
> > + MFD_CELL_NAME("asus-transformer-ec-battery"),
> > + MFD_CELL_NAME("asus-transformer-ec-charger"),
> > + MFD_CELL_NAME("asus-transformer-ec-led"),
> > +};
> > +
> > +static int asus_ec_probe(struct i2c_client *client)
> > +{
> > + struct device *dev = &client->dev;
> > + struct asus_ec_data *ddata;
> > + const struct mfd_cell *cells;
> > + unsigned int num_cells;
> > + unsigned long irqflags;
> > + int ret;
> > +
> > + if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_I2C_BLOCK))
> > + return dev_err_probe(dev, -ENXIO,
> > + "I2C bus is missing required SMBus block mode support\n");
> > +
> > + ddata = devm_kzalloc(dev, sizeof(*ddata), GFP_KERNEL);
> > + if (!ddata)
> > + return -ENOMEM;
> > +
> > + ddata->info = device_get_match_data(dev);
> > + if (!ddata->info)
> > + return -ENODEV;
> > +
> > + switch (ddata->info->variant) {
> > + case ASUSEC_SL101_DOCK:
> > + cells = asus_ec_sl101_dock_mfd_devices;
> > + num_cells = ARRAY_SIZE(asus_ec_sl101_dock_mfd_devices);
> > + break;
> > + case ASUSEC_TF101_DOCK:
> > + cells = asus_ec_tf101_dock_mfd_devices;
> > + num_cells = ARRAY_SIZE(asus_ec_tf101_dock_mfd_devices);
> > + break;
> > + case ASUSEC_TF201_PAD:
> > + cells = asus_ec_tf201_pad_mfd_devices;
> > + num_cells = ARRAY_SIZE(asus_ec_tf201_pad_mfd_devices);
> > + break;
> > + case ASUSEC_TF600T_PAD:
> > + cells = asus_ec_tf600t_pad_mfd_devices;
> > + num_cells = ARRAY_SIZE(asus_ec_tf600t_pad_mfd_devices);
> > + break;
> > + default:
> > + return dev_err_probe(dev, -EINVAL,
> > + "unknown device variant %d\n",
> > + ddata->info->variant);
> > + }
> > +
> > + i2c_set_clientdata(client, ddata);
> > + ddata->client = client;
> > +
> > + ddata->ec.dockram = devm_asus_dockram_get(dev);
> > + if (IS_ERR(ddata->ec.dockram))
> > + return dev_err_probe(dev, PTR_ERR(ddata->ec.dockram),
> > + "failed to get dockram\n");
> > +
> > + ddata->ecreq_gpio = devm_gpiod_get(dev, "request", GPIOD_OUT_LOW);
> > + if (IS_ERR(ddata->ecreq_gpio))
> > + return dev_err_probe(dev, PTR_ERR(ddata->ecreq_gpio),
> > + "failed to get EC request GPIO\n");
> > +
> > + BLOCKING_INIT_NOTIFIER_HEAD(&ddata->ec.notify_list);
> > + mutex_init(&ddata->ecreq_lock);
> > +
> > + asus_ec_signal_request(ddata);
> > +
> > + ret = asus_ec_detect(ddata);
> > + if (ret)
> > + return dev_err_probe(dev, ret, "failed to detect EC version\n");
> > +
> > + /*
> > + * Systems using device tree should set up interrupt via DTS,
> > + * the rest will use the default low interrupt.
> > + */
> > + irqflags = dev->of_node ? 0 : IRQF_TRIGGER_LOW;
> > +
> > + ret = devm_request_threaded_irq(dev, client->irq, NULL,
> > + &asus_ec_interrupt,
> > + IRQF_ONESHOT | irqflags,
> > + client->name, ddata);
> > + 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;
> > +
> > + return devm_mfd_add_devices(dev, 0, cells, num_cells, NULL, 0, NULL);
> > +}
> > +
> > +static const struct asus_ec_chip_info asus_ec_sl101_dock_data = {
> > + .name = "dock",
> > + .variant = ASUSEC_SL101_DOCK,
> > + .fmode = ASUSEC_MODE_NONE,
> > +};
> > +
> > +static const struct asus_ec_chip_info asus_ec_tf101_dock_data = {
> > + .name = "dock",
> > + .variant = ASUSEC_TF101_DOCK,
> > + .fmode = ASUSEC_MODE_NONE,
> > +};
> > +
> > +static const struct asus_ec_chip_info asus_ec_tf201_pad_data = {
> > + .name = "pad",
> > + .variant = ASUSEC_TF201_PAD,
> > + .fmode = ASUSEC_MODE_NORMAL,
> > +};
> > +
> > +static const struct asus_ec_chip_info asus_ec_tf600t_pad_data = {
> > + .name = "pad",
> > + .variant = ASUSEC_TF600T_PAD,
> > + .fmode = ASUSEC_MODE_NORMAL,
> > +};
>
> Any reason not to just pass the identifier (variant) and add the name
> and fmode attribues to the switch() above?
Why not set it here, I am not passing any mfd or any other API via of data.
> > +
> > +static const struct of_device_id asus_ec_match[] = {
> > + {
> > + .compatible = "asus,sl101-ec-dock",
> > + .data = &asus_ec_sl101_dock_data
> > + }, {
> > + .compatible = "asus,tf101-ec-dock",
> > + .data = &asus_ec_tf101_dock_data
> > + }, {
> > + .compatible = "asus,tf201-ec-pad",
> > + .data = &asus_ec_tf201_pad_data
> > + }, {
> > + .compatible = "asus,tf600t-ec-pad",
> > + .data = &asus_ec_tf600t_pad_data
> > + },
> > + { /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(of, asus_ec_match);
> > +
> > +static struct i2c_driver asus_ec_driver = {
> > + .driver = {
> > + .name = "asus-transformer-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-transformer-ec.h b/include/linux/mfd/asus-transformer-ec.h
> > new file mode 100644
> > index 000000000000..f085eea2193e
> > --- /dev/null
> > +++ b/include/linux/mfd/asus-transformer-ec.h
> > @@ -0,0 +1,92 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +#ifndef __MFD_ASUS_TRANSFORMER_EC_H
> > +#define __MFD_ASUS_TRANSFORMER_EC_H
> > +
> > +#include <linux/notifier.h>
> > +#include <linux/platform_device.h>
> > +
> > +struct i2c_client;
> > +
> > +/**
> > + * struct asusec_core - public part shared with all cells
> > + *
> > + * @model: firmware version running on the EC
> > + * @name: prefix associated with the EC
> > + * @dockram: pointer to Dockram's i2c_client
> > + * @notify_list: notify list used by cells
> > + */
> > +struct asusec_core {
> > + const char *model;
> > + const char *name;
> > + struct i2c_client *dockram;
> > + struct blocking_notifier_head notify_list;
> > +};
> > +
> > +#define ASUSEC_ENTRIES 0x100
> > +#define ASUSEC_ENTRY_SIZE 32
> > +#define ASUSEC_ENTRY_BUFSIZE (ASUSEC_ENTRY_SIZE + 1)
> > +
> > +/* interrupt sources */
> > +#define ASUSEC_IRQ_STATUS 1
> > +#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_CODE 2
> > +#define ASUSEC_SMI_POWER_NOTIFY 0x31 /* USB cable plug event */
> > +#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 /* charger to dock plug event */
> > +#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(9)
> > +#define ASUSEC_CMD_SUSPEND_S3 BIT_ULL(33)
> > +#define ASUSEC_CTL_TEST_DISCHARGE BIT_ULL(35)
> > +#define ASUSEC_CMD_SUSPEND_INHIBIT BIT_ULL(37)
> > +#define ASUSEC_CTL_FACTORY_MODE BIT_ULL(38)
> > +#define ASUSEC_CTL_KEEP_AWAKE BIT_ULL(39)
> > +#define ASUSEC_CTL_USB_CHARGE BIT_ULL(40)
> > +#define ASUSEC_CTL_LED_BLINK BIT_ULL(40)
> > +#define ASUSEC_CTL_LED_AMBER BIT_ULL(41)
> > +#define ASUSEC_CTL_LED_GREEN BIT_ULL(42)
> > +#define ASUSEC_CMD_SWITCH_HDMI BIT_ULL(56)
> > +#define ASUSEC_CMD_WIN_SHUTDOWN BIT_ULL(62)
> > +
> > +#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
> > +
> > +int asus_dockram_access_ctl(struct i2c_client *client,
> > + u64 *out, u64 mask, u64 xor);
> > +
> > +#endif /* __MFD_ASUS_TRANSFORMER_EC_H */
> > --
> > 2.51.0
> >
>
> --
> Lee Jones
^ permalink raw reply
* Re: [PATCH v8 5/7] leds: Add driver for ASUS Transformer LEDs
From: Svyatoslav Ryhel @ 2026-06-11 12:27 UTC (permalink / raw)
To: Lee Jones
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Dmitry Torokhov,
Pavel Machek, Sebastian Reichel, Ion Agorria,
Michał Mirosław, devicetree, linux-kernel, linux-input,
linux-leds, linux-pm
In-Reply-To: <20260611113037.GO4151951@google.com>
чт, 11 черв. 2026 р. о 14:30 Lee Jones <lee@kernel.org> пише:
>
> On Thu, 28 May 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-transformer-ec.c | 125 ++++++++++++++++++++++++
> > 3 files changed, 137 insertions(+)
> > create mode 100644 drivers/leds/leds-asus-transformer-ec.c
> >
> > diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> > index f4a0a3c8c870..f637d23400a8 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_ASUS_TRANSFORMER_EC
> > + tristate "LED Support for Asus Transformer charging LED"
> > + depends on LEDS_CLASS
> > + depends on MFD_ASUS_TRANSFORMER_EC
> > + 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-transformer-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..d5395c3f1124 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_ASUS_TRANSFORMER_EC) += leds-asus-transformer-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-transformer-ec.c b/drivers/leds/leds-asus-transformer-ec.c
> > new file mode 100644
> > index 000000000000..09503e76331c
> > --- /dev/null
> > +++ b/drivers/leds/leds-asus-transformer-ec.c
> > @@ -0,0 +1,125 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +
> > +#include <linux/err.h>
> > +#include <linux/leds.h>
> > +#include <linux/mfd/asus-transformer-ec.h>
> > +#include <linux/module.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/slab.h>
> > +
> > +enum {
> > + ASUSEC_LED_AMBER,
> > + ASUSEC_LED_GREEN,
> > + ASUSEC_LED_MAX
> > +};
> > +
> > +struct asus_ec_led_config {
> > + const char *name;
> > + unsigned int color;
> > + unsigned long long ctrl_bit;
>
> Should we use 'u64' here instead of 'unsigned long long' to align with standard
> kernel integer types?
>
sure
> > +};
> > +
> > +struct asus_ec_led {
> > + struct asus_ec_leds_data *ddata;
> > + struct led_classdev cdev;
> > + unsigned long long ctrl_bit;
>
> Should we use 'u64' here as well to keep it consistent?
>
sure
> > +};
> > +
> > +struct asus_ec_leds_data {
> > + const struct asusec_core *ec;
> > + struct asus_ec_led leds[ASUSEC_LED_MAX];
> > +};
> > +
> > +static const struct asus_ec_led_config asus_ec_leds[] = {
> > + [ASUSEC_LED_AMBER] = {
> > + .name = "amber",
> > + .color = LED_COLOR_ID_AMBER,
> > + .ctrl_bit = ASUSEC_CTL_LED_AMBER,
> > + },
> > + [ASUSEC_LED_GREEN] = {
> > + .name = "green",
> > + .color = LED_COLOR_ID_GREEN,
> > + .ctrl_bit = ASUSEC_CTL_LED_GREEN,
> > + },
> > +};
> > +
> > +static enum led_brightness asus_ec_led_get_brightness(struct led_classdev *cdev)
> > +{
> > + struct asus_ec_led *led = container_of(cdev, struct asus_ec_led, cdev);
> > + const struct asusec_core *ec = led->ddata->ec;
>
> I'm getting confused here.
>
> ddata is what I'd be calling the device data struct passed by the parent?
>
> In fact, ddata is a little known concept in Leds. Any reason to go for
> this over the standard nomenclature?
>
How then it should be named? It holds each LED's control bit.
> > + u64 ctl;
> > + int ret;
> > +
> > + ret = asus_dockram_access_ctl(ec->dockram, &ctl, 0, 0);
>
> Did we discuss preferring regmap already?
>
Yes, you were fine with all and even said that you will merge
everything with Dmitry Ack for input
HERE https://lore.kernel.org/lkml/20260527144619.GA671544@google.com/
Yet now I get a new pile of complaints and nits.
> > + if (ret)
> > + return LED_OFF;
> > +
> > + return ctl & led->ctrl_bit ? LED_ON : LED_OFF;
> > +}
> > +
> > +static int asus_ec_led_set_brightness(struct led_classdev *cdev,
> > + enum led_brightness brightness)
> > +{
> > + struct asus_ec_led *led = container_of(cdev, struct asus_ec_led, cdev);
> > + const struct asusec_core *ec = led->ddata->ec;
> > +
> > + if (brightness)
> > + return asus_dockram_access_ctl(ec->dockram, NULL,
> > + led->ctrl_bit, led->ctrl_bit);
> > +
> > + return asus_dockram_access_ctl(ec->dockram, NULL, led->ctrl_bit, 0);
> > +}
> > +
> > +static int asus_ec_led_probe(struct platform_device *pdev)
> > +{
> > + const struct asusec_core *ec = dev_get_drvdata(pdev->dev.parent);
> > + struct asus_ec_leds_data *ddata;
> > + struct device *dev = &pdev->dev;
> > + int i, ret;
>
> Could we declare the loop counter 'i' directly within the 'for' statement's
> scope to keep its scope limited? For example, 'for (int i = 0; ...)'.
>
> > +
> > + ddata = devm_kzalloc(dev, sizeof(*ddata), GFP_KERNEL);
> > + if (!ddata)
> > + return -ENOMEM;
> > +
> > + platform_set_drvdata(pdev, ddata);
> > + ddata->ec = ec;
> > +
> > + for (i = 0; i < ASUSEC_LED_MAX; i++) {
>
> Nit: for (int i = ...
>
> > + const struct asus_ec_led_config *cfg = &asus_ec_leds[i];
> > + struct asus_ec_led *led = &ddata->leds[i];
> > +
> > + led->cdev.name = devm_kasprintf(dev, GFP_KERNEL, "%s::%s",
> > + ddata->ec->name, cfg->name);
> > + if (!led->cdev.name)
> > + return -ENOMEM;
> > +
> > + led->cdev.max_brightness = 1;
> > + led->cdev.color = cfg->color;
> > + led->cdev.flags = LED_CORE_SUSPENDRESUME | LED_RETAIN_AT_SHUTDOWN;
> > + led->cdev.brightness_get = asus_ec_led_get_brightness;
> > + led->cdev.brightness_set_blocking = asus_ec_led_set_brightness;
> > +
> > + led->ddata = ddata;
> > + led->ctrl_bit = cfg->ctrl_bit;
> > +
> > + ret = devm_led_classdev_register(dev, &led->cdev);
> > + if (ret)
> > + return dev_err_probe(dev, ret,
> > + "failed to register %s LED\n",
> > + cfg->name);
>
> Should we capitalise the error message here to match our style guidelines
> (e.g. 'Failed to register...')?
>
dev messages end with ":" so it should continue with lower case. You
want cap, I don't mind
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static struct platform_driver asus_ec_led_driver = {
> > + .driver.name = "asus-transformer-ec-led",
> > + .probe = asus_ec_led_probe,
> > +};
> > +module_platform_driver(asus_ec_led_driver);
> > +
> > +MODULE_ALIAS("platform:asus-transformer-ec-led");
> > +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] HID: hiddev: Use kref to track struct hiddev lifetime
From: Heitor Alves de Siqueira @ 2026-06-11 12:36 UTC (permalink / raw)
To: Dmitry Torokhov, Heitor Alves de Siqueira
Cc: Jiri Kosina, Benjamin Tissoires, kernel-dev, linux-usb,
linux-input, linux-kernel, syzbot+563191a4939ddbfe73d4
In-Reply-To: <aib06cgHVLIb9vtl@google.com>
Hi Dmitry,
On Mon Jun 8, 2026 at 3:44 PM -03, Dmitry Torokhov wrote:
> Hi Heitor,
>
> On Mon, Jun 08, 2026 at 01:33:03PM -0300, Heitor Alves de Siqueira wrote:
>> If a USB HID device is disconnected while userspace still holds the
>> hiddev node open, hiddev_disconnect() and hiddev_release() can race on
>> the embedded existancelock mutex. Syzbot has triggered this with kfree()
>> happening during the mutex slow path.
>>
>> Fix by introducing a kref in struct hiddev, and moving kfree() into a
>> dedicated release callback. This way, struct hiddev will only be freed
>> after both hiddev_release() and hiddev_disconnect() are done.
>
> This looks like a common issue with usb_register_dev() that does not
> allow tying the lifetime of the created device, lifetime of user of the
> created device, and userspace accessing it. Ideally the class device
> would be embedded into struct hiddev, and tie its lifetime with lifetime
> of the chardev associated with it and userspace accessors using it. tie
> its lifetime with lifetime of the chardev associated with it and
> userspace accessors using it. See cdev_device_add() and how it is being
> used by multiple subsystems and how they handle class devices.
Thank you for your review! I see now, usb_register_dev() does not seem
to be the best if we want to tie the lifetimes together... At least, I
don't think its intended for this usecase, especially if we want to
embed the cdev into struct hiddev. I've been looking at some of the
other drivers that use cdev_device_add(), and noticed that we'd have to
manually handle some things that usb_register_dev() conveniently does
for us (although most seem quite straightforward like minor allocation
and device creation).
I'll give it a try for hiddev, and make sure we don't change any of the
class device paths or leave any open ends with the rest of the usbhid
subsystem. One of the USB gadget drivers already uses cdev_device_add(),
so hopefully it won't be too much trouble.
Thanks,
Heitor
^ permalink raw reply
* Re: [PATCH] HID: logitech-hidpp: sync wheel multiplier on wheel mode changes
From: Jiri Kosina @ 2026-06-11 13:56 UTC (permalink / raw)
To: Lauri Saurus
Cc: linux-input, Filipe Laíns, Bastien Nocera,
Benjamin Tissoires, linux-kernel
In-Reply-To: <20260518192649.245691-1-saurla@saurla.com>
On Mon, 18 May 2026, Lauri Saurus wrote:
> The hid-logitech-hidpp driver enables high resolution scrolling on
> device connect for capable HID++ 2.0 devices. Driver also reads the
> wheel capability and caches the returned high resolution wheel scroll
> multiplier, that is used for scroll scaling when handling wheel scroll
> events.
>
> Wheel mode can also be set externally through HID++ requests, which
> can leave the cached multiplier stale and cause incorrect scroll
> scaling. If external SetWheelMode HID++ request sets the mode to
> low resolution, the cached multiplier is not updated accordingly. This
> causes extremely slow scrolling since driver expects multiple wheel
> scroll events per detent but is only getting one.
>
> The fix listens for HID++ SetWheelMode request responses and updates
> the wheel scroll multiplier based on the set high resolution scroll
> mode. The fix has been tested with Logitech G502X lightspeed mouse.
>
> Signed-off-by: Lauri Saurus <saurla@saurla.com>
Applied, thank you.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* [dtor-input:next] BUILD SUCCESS 6251f7d3472c0409e30f8d6a24f10d33d12e3f9a
From: kernel test robot @ 2026-06-11 14:16 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
branch HEAD: 6251f7d3472c0409e30f8d6a24f10d33d12e3f9a Input: synaptics-rmi4 - unregister function handlers on physical driver registration failure
elapsed time: 751m
configs tested: 233
configs skipped: 2
The following configs have been built successfully.
More configs may be tested in the coming days.
tested configs:
alpha allnoconfig gcc-16.1.0
alpha allyesconfig gcc-16.1.0
alpha defconfig gcc-16.1.0
arc allmodconfig clang-23
arc allnoconfig gcc-16.1.0
arc allyesconfig clang-23
arc defconfig gcc-16.1.0
arc nsim_700_defconfig gcc-16.1.0
arc randconfig-001 gcc-14.3.0
arc randconfig-001-20260611 gcc-14.3.0
arc randconfig-002 gcc-14.3.0
arc randconfig-002-20260611 gcc-14.3.0
arm allnoconfig gcc-16.1.0
arm allyesconfig clang-23
arm defconfig gcc-16.1.0
arm omap1_defconfig gcc-16.1.0
arm pxa910_defconfig gcc-16.1.0
arm randconfig-001 gcc-14.3.0
arm randconfig-001-20260611 gcc-14.3.0
arm randconfig-002 gcc-14.3.0
arm randconfig-002-20260611 gcc-14.3.0
arm randconfig-003 gcc-14.3.0
arm randconfig-003-20260611 gcc-14.3.0
arm randconfig-004 gcc-14.3.0
arm randconfig-004-20260611 gcc-14.3.0
arm64 allmodconfig clang-23
arm64 allnoconfig gcc-16.1.0
arm64 defconfig gcc-16.1.0
arm64 randconfig-001-20260611 gcc-14.3.0
arm64 randconfig-002-20260611 gcc-14.3.0
arm64 randconfig-003-20260611 gcc-14.3.0
arm64 randconfig-004-20260611 gcc-14.3.0
csky allmodconfig gcc-16.1.0
csky allnoconfig gcc-16.1.0
csky defconfig gcc-16.1.0
csky randconfig-001-20260611 gcc-14.3.0
csky randconfig-002-20260611 gcc-14.3.0
hexagon allmodconfig gcc-16.1.0
hexagon allnoconfig gcc-16.1.0
hexagon defconfig gcc-16.1.0
hexagon randconfig-001-20260611 clang-16
hexagon randconfig-001-20260611 clang-17
hexagon randconfig-002-20260611 clang-16
hexagon randconfig-002-20260611 clang-17
i386 allmodconfig clang-22
i386 allmodconfig gcc-14
i386 allnoconfig gcc-16.1.0
i386 allyesconfig clang-22
i386 allyesconfig gcc-14
i386 buildonly-randconfig-001-20260611 clang-22
i386 buildonly-randconfig-002-20260611 clang-22
i386 buildonly-randconfig-003-20260611 clang-22
i386 buildonly-randconfig-004-20260611 clang-22
i386 buildonly-randconfig-005-20260611 clang-22
i386 buildonly-randconfig-006-20260611 clang-22
i386 defconfig gcc-16.1.0
i386 randconfig-001 gcc-14
i386 randconfig-001-20260611 gcc-14
i386 randconfig-002 gcc-14
i386 randconfig-002-20260611 gcc-14
i386 randconfig-003 gcc-14
i386 randconfig-003-20260611 gcc-14
i386 randconfig-004 gcc-14
i386 randconfig-004-20260611 gcc-14
i386 randconfig-005 gcc-14
i386 randconfig-005-20260611 gcc-14
i386 randconfig-006 gcc-14
i386 randconfig-006-20260611 gcc-14
i386 randconfig-007 gcc-14
i386 randconfig-007-20260611 gcc-14
i386 randconfig-011-20260611 gcc-14
i386 randconfig-012-20260611 gcc-14
i386 randconfig-013-20260611 gcc-14
i386 randconfig-014-20260611 gcc-14
i386 randconfig-015-20260611 gcc-14
i386 randconfig-016-20260611 gcc-14
i386 randconfig-017-20260611 gcc-14
loongarch allmodconfig clang-23
loongarch allnoconfig gcc-16.1.0
loongarch defconfig clang-23
loongarch randconfig-001-20260611 clang-16
loongarch randconfig-001-20260611 clang-17
loongarch randconfig-002-20260611 clang-16
loongarch randconfig-002-20260611 clang-17
m68k allmodconfig gcc-16.1.0
m68k allnoconfig gcc-16.1.0
m68k allyesconfig clang-23
m68k defconfig clang-23
microblaze allnoconfig gcc-16.1.0
microblaze allyesconfig gcc-16.1.0
microblaze defconfig clang-23
mips allmodconfig gcc-16.1.0
mips allnoconfig gcc-16.1.0
mips allyesconfig gcc-16.1.0
nios2 allmodconfig clang-20
nios2 allnoconfig clang-23
nios2 defconfig clang-23
nios2 randconfig-001-20260611 clang-16
nios2 randconfig-001-20260611 clang-17
nios2 randconfig-002-20260611 clang-16
nios2 randconfig-002-20260611 clang-17
openrisc allmodconfig clang-20
openrisc allnoconfig clang-23
openrisc defconfig gcc-16.1.0
parisc allmodconfig gcc-16.1.0
parisc allnoconfig clang-23
parisc allyesconfig clang-17
parisc allyesconfig clang-23
parisc defconfig gcc-16.1.0
parisc randconfig-001-20260611 gcc-13.4.0
parisc randconfig-002-20260611 gcc-13.4.0
parisc64 defconfig clang-23
powerpc akebono_defconfig clang-23
powerpc allmodconfig gcc-16.1.0
powerpc allnoconfig clang-23
powerpc randconfig-001-20260611 gcc-13.4.0
powerpc randconfig-002-20260611 gcc-13.4.0
powerpc tqm8540_defconfig gcc-16.1.0
powerpc64 randconfig-001-20260611 gcc-13.4.0
powerpc64 randconfig-002-20260611 gcc-13.4.0
riscv allmodconfig clang-23
riscv allnoconfig clang-23
riscv allyesconfig clang-23
riscv defconfig gcc-16.1.0
riscv randconfig-001 gcc-12.5.0
riscv randconfig-001-20260611 gcc-12.5.0
riscv randconfig-002 gcc-12.5.0
riscv randconfig-002-20260611 gcc-12.5.0
s390 allmodconfig clang-17
s390 allmodconfig clang-23
s390 allnoconfig clang-23
s390 allyesconfig gcc-16.1.0
s390 defconfig gcc-16.1.0
s390 randconfig-001 gcc-12.5.0
s390 randconfig-001-20260611 gcc-12.5.0
s390 randconfig-002 gcc-12.5.0
s390 randconfig-002-20260611 gcc-12.5.0
sh allmodconfig gcc-16.1.0
sh allnoconfig clang-23
sh allyesconfig clang-17
sh allyesconfig clang-23
sh defconfig gcc-14
sh randconfig-001 gcc-12.5.0
sh randconfig-001-20260611 gcc-12.5.0
sh randconfig-002 gcc-12.5.0
sh randconfig-002-20260611 gcc-12.5.0
sparc allnoconfig clang-23
sparc defconfig gcc-16.1.0
sparc randconfig-001-20260611 gcc-15.2.0
sparc randconfig-002-20260611 gcc-15.2.0
sparc64 allmodconfig clang-20
sparc64 defconfig gcc-14
sparc64 randconfig-001-20260611 gcc-15.2.0
sparc64 randconfig-002-20260611 gcc-15.2.0
um allmodconfig clang-17
um allmodconfig clang-23
um allnoconfig clang-23
um allyesconfig gcc-16.1.0
um defconfig gcc-14
um i386_defconfig gcc-14
um randconfig-001-20260611 gcc-15.2.0
um randconfig-002-20260611 gcc-15.2.0
um x86_64_defconfig gcc-14
x86_64 allmodconfig clang-22
x86_64 allnoconfig clang-23
x86_64 allyesconfig clang-22
x86_64 buildonly-randconfig-001 gcc-14
x86_64 buildonly-randconfig-001-20260611 gcc-14
x86_64 buildonly-randconfig-002 gcc-14
x86_64 buildonly-randconfig-002-20260611 gcc-14
x86_64 buildonly-randconfig-003 gcc-14
x86_64 buildonly-randconfig-003-20260611 gcc-14
x86_64 buildonly-randconfig-004 gcc-14
x86_64 buildonly-randconfig-004-20260611 gcc-14
x86_64 buildonly-randconfig-005 gcc-14
x86_64 buildonly-randconfig-005-20260611 gcc-14
x86_64 buildonly-randconfig-006 gcc-14
x86_64 buildonly-randconfig-006-20260611 gcc-14
x86_64 defconfig gcc-14
x86_64 kexec clang-22
x86_64 randconfig-001 clang-22
x86_64 randconfig-001-20260611 clang-22
x86_64 randconfig-001-20260611 gcc-14
x86_64 randconfig-002 clang-22
x86_64 randconfig-002-20260611 clang-22
x86_64 randconfig-002-20260611 gcc-14
x86_64 randconfig-003 clang-22
x86_64 randconfig-003-20260611 clang-22
x86_64 randconfig-003-20260611 gcc-14
x86_64 randconfig-004 clang-22
x86_64 randconfig-004-20260611 clang-22
x86_64 randconfig-004-20260611 gcc-14
x86_64 randconfig-005 clang-22
x86_64 randconfig-005-20260611 clang-22
x86_64 randconfig-005-20260611 gcc-14
x86_64 randconfig-006 clang-22
x86_64 randconfig-006-20260611 clang-22
x86_64 randconfig-006-20260611 gcc-14
x86_64 randconfig-011 clang-22
x86_64 randconfig-011-20260611 clang-22
x86_64 randconfig-011-20260611 gcc-14
x86_64 randconfig-012 clang-22
x86_64 randconfig-012-20260611 clang-22
x86_64 randconfig-012-20260611 gcc-14
x86_64 randconfig-013 clang-22
x86_64 randconfig-013-20260611 clang-22
x86_64 randconfig-013-20260611 gcc-14
x86_64 randconfig-014 clang-22
x86_64 randconfig-014-20260611 clang-22
x86_64 randconfig-014-20260611 gcc-14
x86_64 randconfig-015 clang-22
x86_64 randconfig-015-20260611 clang-22
x86_64 randconfig-015-20260611 gcc-14
x86_64 randconfig-016 clang-22
x86_64 randconfig-016-20260611 clang-22
x86_64 randconfig-016-20260611 gcc-14
x86_64 randconfig-071-20260611 clang-22
x86_64 randconfig-072-20260611 clang-22
x86_64 randconfig-073-20260611 clang-22
x86_64 randconfig-074-20260611 clang-22
x86_64 randconfig-075-20260611 clang-22
x86_64 randconfig-076-20260611 clang-22
x86_64 rhel-9.4 clang-22
x86_64 rhel-9.4-bpf gcc-14
x86_64 rhel-9.4-func clang-22
x86_64 rhel-9.4-kselftests clang-22
x86_64 rhel-9.4-kunit gcc-14
x86_64 rhel-9.4-ltp gcc-14
x86_64 rhel-9.4-rust clang-22
xtensa allnoconfig clang-23
xtensa allyesconfig clang-20
xtensa randconfig-001-20260611 gcc-15.2.0
xtensa randconfig-002-20260611 gcc-15.2.0
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH 2/2] iio: hid-sensor-rotation: Fix stale or zero output when reading raw values
From: Jonathan Cameron @ 2026-06-11 16:34 UTC (permalink / raw)
To: Zhang Lixu
Cc: Jiri Kosina, Srinivas Pandruvada, Benjamin Tissoires,
David Lechner, Nuno Sá, Andy Shevchenko, linux-input,
linux-iio, linux-kernel
In-Reply-To: <20260610082911.157232-3-lixu.zhang@intel.com>
On Wed, 10 Jun 2026 16:29:10 +0800
Zhang Lixu <lixu.zhang@intel.com> wrote:
> When reading the raw quaternion attribute (in_rot_quaternion_raw), the
> driver currently returns either all zeros (if the sensor was never enabled)
> or stale data (if the sensor was previously enabled) because it reads from
> the internal buffer without explicitly requesting a new sample from the
> sensor.
>
> To fix this, power up the sensor, call sensor_hub_input_attr_read_values()
> to issue a synchronous GET_REPORT and receive the full quaternion data
> directly into a local buffer, then decode the four components.
>
> Signed-off-by: Zhang Lixu <lixu.zhang@intel.com>
Hi Zhang,
Given this is clearly a fix, can you reply to this thread with an appropriate
Fixes tag. Otherwise looks fine to me.
Given timing and I guess that this bug is fairly old, this will probably only
go upstream after rc1.
Thanks,
Jonathan
> ---
> drivers/iio/orientation/hid-sensor-rotation.c | 40 ++++++++++++++++++-
> 1 file changed, 38 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/orientation/hid-sensor-rotation.c b/drivers/iio/orientation/hid-sensor-rotation.c
> index 4a11e45550991..1c6f02374f3ce 100644
> --- a/drivers/iio/orientation/hid-sensor-rotation.c
> +++ b/drivers/iio/orientation/hid-sensor-rotation.c
> @@ -86,6 +86,13 @@ static int dev_rot_read_raw(struct iio_dev *indio_dev,
> long mask)
> {
> struct dev_rot_state *rot_state = iio_priv(indio_dev);
> + struct hid_sensor_hub_device *hsdev = rot_state->common_attributes.hsdev;
> + struct hid_sensor_hub_attribute_info *info = &rot_state->quaternion;
> + u32 usage_id = HID_USAGE_SENSOR_ORIENT_QUATERNION;
> + union {
> + s16 val16[4];
> + s32 val32[4];
> + } raw_buf;
> int ret_type;
> int i;
>
> @@ -95,8 +102,37 @@ static int dev_rot_read_raw(struct iio_dev *indio_dev,
> switch (mask) {
> case IIO_CHAN_INFO_RAW:
> if (size >= 4) {
> - for (i = 0; i < 4; ++i)
> - vals[i] = rot_state->scan.sampled_vals[i];
> + if (info->size <= 0 || info->size > sizeof(raw_buf))
> + return -EINVAL;
> +
> + hid_sensor_power_state(&rot_state->common_attributes, true);
> +
> + ret_type = sensor_hub_input_attr_read_values(hsdev,
> + hsdev->usage,
> + usage_id,
> + info->report_id,
> + SENSOR_HUB_SYNC,
> + info->size,
> + (u8 *)&raw_buf);
> +
> + hid_sensor_power_state(&rot_state->common_attributes, false);
> +
> + if (ret_type < 0)
> + return ret_type;
> +
> + switch (info->size) {
> + case sizeof(raw_buf.val16):
> + for (i = 0; i < ARRAY_SIZE(raw_buf.val16); i++)
> + vals[i] = raw_buf.val16[i];
> + break;
> + case sizeof(raw_buf.val32):
> + for (i = 0; i < ARRAY_SIZE(raw_buf.val32); i++)
> + vals[i] = raw_buf.val32[i];
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> ret_type = IIO_VAL_INT_MULTIPLE;
> *val_len = 4;
> } else
^ permalink raw reply
* Re: [PATCH 1/2] HID: sensor-hub: Add sensor_hub_input_attr_read_values() for multi-byte reads
From: Jonathan Cameron @ 2026-06-11 16:36 UTC (permalink / raw)
To: Jiri Kosina
Cc: Zhang Lixu, Srinivas Pandruvada, Benjamin Tissoires,
David Lechner, Nuno Sá, Andy Shevchenko, linux-input,
linux-iio, linux-kernel
In-Reply-To: <nqr90q40-s472-o23n-nqn5-2s31qs75s2op@xreary.bet>
On Wed, 10 Jun 2026 18:41:27 +0200 (CEST)
Jiri Kosina <jikos@kernel.org> wrote:
> On Wed, 10 Jun 2026, Zhang Lixu wrote:
>
> > From: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> >
> > sensor_hub_input_attr_get_raw_value() is limited to returning a single
> > 32-bit value, which is insufficient for sensors that report data larger
> > than 32 bits, such as a quaternion with four s16 elements.
> >
> > Add sensor_hub_input_attr_read_values() that accepts a caller-provided
> > buffer and accumulates incoming data until the buffer is full. The two
> > paths are distinguished in sensor_hub_raw_event() by pending.max_raw_size
> > being non-zero, preserving backward compatibility.
> >
> > Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> > Co-developed-by: Zhang Lixu <lixu.zhang@intel.com>
> > Signed-off-by: Zhang Lixu <lixu.zhang@intel.com>
>
> Acked-by: Jiri Kosina <jkosina@suse.com>
>
> Jonathan, will you take the whole lot through your tree?
>
I can do that, but as a fix after rc1 now (and only once I have
a suitable Fixes tag).
Thanks,
Jonathan
> Thanks,
>
^ permalink raw reply
* [PATCH] HID: asus: add support for xgm led
From: Denis Benato @ 2026-06-11 22:11 UTC (permalink / raw)
To: linux-kernel
Cc: linux-input, Benjamin Tissoires, Jiri Kosina, Luke D . Jones,
Mateusz Schyboll, Denis Benato, Denis Benato
XG mobile stations have very bright leds behind the fan that can be
turned either ON or OFF: add a cled interface to allow controlling the
brightness of those red leds.
Signed-off-by: Denis Benato <denis.benato@linux.dev>
---
drivers/hid/hid-asus.c | 53 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 53 insertions(+)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index d34d74df3dc0..4298a3fe98c7 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -51,6 +51,8 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
#define FEATURE_KBD_LED_REPORT_ID1 0x5d
#define FEATURE_KBD_LED_REPORT_ID2 0x5e
+#define ROG_XGM_REPORT_SIZE 300
+
#define ROG_ALLY_REPORT_SIZE 64
#define ROG_ALLY_X_MIN_MCU 313
#define ROG_ALLY_MIN_MCU 319
@@ -118,6 +120,11 @@ struct asus_kbd_leds {
bool removed;
};
+struct asus_xgm_led {
+ struct led_classdev cdev;
+ struct hid_device *hdev;
+};
+
struct asus_touchpad_info {
int max_x;
int max_y;
@@ -143,6 +150,7 @@ struct asus_drvdata {
unsigned long battery_next_query;
struct work_struct fn_lock_sync_work;
bool fn_lock;
+ struct asus_xgm_led *xgm_led;
};
static int asus_report_battery(struct asus_drvdata *, u8 *, int);
@@ -940,6 +948,19 @@ static int asus_battery_probe(struct hid_device *hdev)
return ret;
}
+static void asus_xgm_led_set(struct led_classdev *led_cdev, enum led_brightness value)
+{
+ const u8 buf[ROG_XGM_REPORT_SIZE] = {
+ FEATURE_KBD_LED_REPORT_ID2, 0xC5, (value) ? 0x50 : 0x00
+ };
+ struct asus_xgm_led *xgm = container_of(led_cdev, struct asus_xgm_led, cdev);
+ int ret;
+
+ ret = asus_kbd_set_report(xgm->hdev, buf, ROG_XGM_REPORT_SIZE);
+ if (ret != ROG_XGM_REPORT_SIZE)
+ hid_err(xgm->hdev, "Unable to set XG mobile led state: %d\n", ret);
+}
+
static int asus_input_configured(struct hid_device *hdev, struct hid_input *hi)
{
struct input_dev *input = hi->input;
@@ -1183,6 +1204,9 @@ static int __maybe_unused asus_resume(struct hid_device *hdev)
}
}
+ if (drvdata->xgm_led)
+ asus_xgm_led_set(&drvdata->xgm_led->cdev, drvdata->xgm_led->cdev.brightness);
+
asus_resume_err:
return ret;
}
@@ -1309,6 +1333,32 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
}
}
+ if (asus_has_report_id(hdev, FEATURE_KBD_REPORT_ID) &&
+ ((hdev->product == USB_DEVICE_ID_ASUSTEK_XGM_2022) ||
+ (hdev->product == USB_DEVICE_ID_ASUSTEK_XGM_2023))) {
+ drvdata->xgm_led = devm_kzalloc(&hdev->dev, sizeof(*drvdata->xgm_led), GFP_KERNEL);
+ if (drvdata->xgm_led == NULL) {
+ ret = -ENOMEM;
+ goto err_stop_hw;
+ }
+ drvdata->xgm_led->hdev = hdev;
+ drvdata->xgm_led->cdev.name = "asus:xgm:led";
+ drvdata->xgm_led->cdev.brightness = 1;
+ drvdata->xgm_led->cdev.max_brightness = 1;
+ drvdata->xgm_led->cdev.brightness_set = asus_xgm_led_set;
+ ret = led_classdev_register(&hdev->dev, &drvdata->xgm_led->cdev);
+ if (ret) {
+ hid_err(hdev, "Asus failed to register xgm led: %d\n", ret);
+ goto err_stop_hw;
+ }
+
+ /*
+ * Sometimes after a reboot LEDs are ON, sometimes are OFF: set them to
+ * what the default brightness resets when doing a cold boot.
+ */
+ asus_xgm_led_set(&drvdata->xgm_led->cdev, drvdata->xgm_led->cdev.brightness);
+ }
+
/* Laptops keyboard backlight is always at 0x5a */
if (is_vendor && (drvdata->quirks & QUIRK_USE_KBD_BACKLIGHT) &&
(asus_has_report_id(hdev, FEATURE_KBD_REPORT_ID)) &&
@@ -1365,6 +1415,9 @@ static void asus_remove(struct hid_device *hdev)
if (drvdata->quirks & QUIRK_HID_FN_LOCK)
cancel_work_sync(&drvdata->fn_lock_sync_work);
+ if (drvdata->xgm_led)
+ led_classdev_unregister(&drvdata->xgm_led->cdev);
+
hid_hw_stop(hdev);
}
--
2.47.3
^ permalink raw reply related
* Re: [PATCH] HID: asus: add support for xgm led
From: sashiko-bot @ 2026-06-11 22:21 UTC (permalink / raw)
To: Denis Benato; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260611221141.45456-1-denis.benato@linux.dev>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The `asus_xgm_led_set` function is incorrectly assigned to the non-blocking `brightness_set` LED callback, but it performs sleeping operations.
- [Medium] The XGM LED class device name is hardcoded, which causes device probe failures due to sysfs naming collisions.
--
commit 55bc647f99f9d2c64c1e99f0ee43e81efa5a6298
Author: Denis Benato <denis.benato@linux.dev>
HID: asus: add support for xgm led
XG mobile stations have very bright leds behind the fan that can be
turned either ON or OFF: add a cled interface to allow controlling the
brightness of those red leds.
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index 3f5e96900b67a..8a5f364b7535f 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
[ ... ]
> @@ -1303,6 +1327,32 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
> }
> }
>
> + if (asus_has_report_id(hdev, FEATURE_KBD_REPORT_ID) &&
> + ((hdev->product == USB_DEVICE_ID_ASUSTEK_XGM_2022) ||
> + (hdev->product == USB_DEVICE_ID_ASUSTEK_XGM_2023))) {
> + drvdata->xgm_led = devm_kzalloc(&hdev->dev, sizeof(*drvdata->xgm_led), GFP_KERNEL);
> + if (drvdata->xgm_led == NULL) {
> + ret = -ENOMEM;
> + goto err_stop_hw;
> + }
> + drvdata->xgm_led->hdev = hdev;
> + drvdata->xgm_led->cdev.name = "asus:xgm:led";
[Severity: Medium]
If multiple XG Mobile docks are connected, or if an ungraceful disconnect
leaves a stale sysfs entry behind, will this hardcoded name cause a sysfs
naming collision? A duplicate name error here would cause
led_classdev_register() to fail and abort the entire HID device probe.
Should this use a dynamic identifier, perhaps incorporating dev_name(),
to ensure uniqueness?
> + drvdata->xgm_led->cdev.brightness = 1;
> + drvdata->xgm_led->cdev.max_brightness = 1;
> + drvdata->xgm_led->cdev.brightness_set = asus_xgm_led_set;
[Severity: High]
Can this cause a sleep-in-atomic bug if the LED is triggered by a software
blink (such as a timer or heartbeat sysfs trigger)?
When software blinking is used, the LED core invokes the brightness_set
callback directly from softirq context. However, asus_xgm_led_set() calls
asus_kbd_set_report(), which performs a GFP_KERNEL allocation and a blocking
USB transaction via hid_hw_raw_request().
Should this callback be assigned to brightness_set_blocking instead,
so the LED core knows to defer the sleeping work to a workqueue?
> + ret = led_classdev_register(&hdev->dev, &drvdata->xgm_led->cdev);
> + if (ret) {
> + hid_err(hdev, "Asus failed to register xgm led: %d\n", ret);
> + goto err_stop_hw;
> + }
> +
> + /*
> + * Sometimes after a reboot LEDs are ON, sometimes are OFF: set them to
> + * what the default brightness resets when doing a cold boot.
> + */
> + asus_xgm_led_set(&drvdata->xgm_led->cdev, drvdata->xgm_led->cdev.brightness);
> + }
> +
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260611221141.45456-1-denis.benato@linux.dev?part=1
^ permalink raw reply
* Re: [PATCH] amd-sfh-hid: tablet mode switch and asus quirk
From: Helge Bahmann @ 2026-06-12 4:22 UTC (permalink / raw)
To: Jiri Kosina, linux-input, Basavaraj Natikar; +Cc: Basavaraj Natikar, bentiss
In-Reply-To: <ecadaac8-e222-420a-ac5b-4d529fb3317e@amd.com>
Am Mittwoch, 10. Juni 2026, 19:12:37 Mitteleuropäische Sommerzeit schrieb Basavaraj Natikar:
>
> On 5/14/2026 1:29 PM, Helge Bahmann wrote:
>
> > [You don't often get email from hcb@chaoticmind.net. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> >
> > On Tue, 12 May 2026, Jiri Kosina wrote:
> >> On Mon, 27 Apr 2026, Helge Bahmann wrote:
> >>
> >>> Add an input driver that interprets the "operation mode" sensor offered
> >>> by the amd sfh on some laptop models.
> >>>
> >>> Add a quirk to make the driver work again with the Asus VivoBook
> >>> VivoBook (turn off the "disable interrupts" flag).
> >>>
> >>> Expose the intr_disable flag as a module parameter in case it turns out
> >>> to be needed on further laptop models.
> >>>
> >>> Signed-off-by: Helge Bahmann <hcb@chaoticmind.net>
> >> Basavaraj, can you please review this one?
> > Some additional context, maybe helpful for review:
> >
> > 1. The numbers and behavior were extracted from the ACPI tables
> > (WMI driver of sorts) of the notebook; I don't have access to any
> > official AMD / ASUS docs or similar.
> >
> > 2. I have an alternate version of this change that is more indirect:
> > - create a HID driver providing an "abstract table mode" message
> > - have an input driver attaching to this newly defined HID driver
> >
> > While that is keeping "more in line" with the current driver
> > architecture, I am not sure this indirection really helps. Particularly,
> > there is no "canonical" HID tablet mode switch message defined,
> > so it all remains completely bespoke. I am happy to change it if
> > you prefer, but would need your input.
> >
> > 3. Since this is based on Asus VivoBook and its ACPI tables,
> > there is a possibility that this "op sensor / tablet mode" behavior
> > is not as universal as I surmise. A point could be made to make this
> > entire behavior model-dependent (with a mod param to override
> > / activate for other models). Happy to take input / advice.
>
> Thanks Helge.
>
> I'd like to go with the dedicated input-driver approach (your option with
> a standalone input driver) rather than the HID-message indirection -- it
> keeps clean subsystem boundaries.
Hello Basavaraj!
Thanks for your architecture input, that sounds good. Let me briefly
clean up the patches and I will send you what I have. I will also
include the input-tabletmode driver that I have, but you will likely
want to change it (use correct protocol message as I am unsure
what would be "correct" here). You can then proceed as you like
(take the driver, rewrite it, ...) -- I will leave ownership decision
entirely up to you, I care more that you are happy with the
overall structure than that.
Thank you
Helge
>
> For splitting the work, either of these works for me -- whichever you
> prefer:
> Option 1: I create the new input driver
> drivers/input/misc/amd_sfh_tabletmode.c and once done we will review your ASUS VivoBook
> quirk + intr_disable module-param patches on top of it.
>
> Option 2: If you'd rather keep ownership of it, you write
> drivers/input/misc/amd_sfh_tabletmode.c consuming the mode exposed by
> amd-sfh, and I'll review and help.
>
>
> Let me know which option you'd like and I'll proceed.
>
> Thanks,
> --
> Basavaraj
>
>
>
^ permalink raw reply
* RE: [PATCH 2/2] iio: hid-sensor-rotation: Fix stale or zero output when reading raw values
From: Zhang, Lixu @ 2026-06-12 5:33 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Jiri Kosina, Srinivas Pandruvada, Benjamin Tissoires,
David Lechner, Nuno Sá, Andy Shevchenko,
linux-input@vger.kernel.org, linux-iio@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20260611173446.14b74365@jic23-huawei>
>-----Original Message-----
>From: Jonathan Cameron <jic23@kernel.org>
>Sent: Friday, June 12, 2026 12:35 AM
>To: Zhang, Lixu <lixu.zhang@intel.com>
>Cc: Jiri Kosina <jikos@kernel.org>; Srinivas Pandruvada
><srinivas.pandruvada@linux.intel.com>; Benjamin Tissoires
><bentiss@kernel.org>; David Lechner <dlechner@baylibre.com>; Nuno Sá
><nuno.sa@analog.com>; Andy Shevchenko <andy@kernel.org>; linux-
>input@vger.kernel.org; linux-iio@vger.kernel.org; linux-kernel@vger.kernel.org
>Subject: Re: [PATCH 2/2] iio: hid-sensor-rotation: Fix stale or zero output when
>reading raw values
>
>On Wed, 10 Jun 2026 16:29:10 +0800
>Zhang Lixu <lixu.zhang@intel.com> wrote:
>
>> When reading the raw quaternion attribute (in_rot_quaternion_raw), the
>> driver currently returns either all zeros (if the sensor was never
>> enabled) or stale data (if the sensor was previously enabled) because
>> it reads from the internal buffer without explicitly requesting a new
>> sample from the sensor.
>>
>> To fix this, power up the sensor, call
>> sensor_hub_input_attr_read_values()
>> to issue a synchronous GET_REPORT and receive the full quaternion data
>> directly into a local buffer, then decode the four components.
>>
>> Signed-off-by: Zhang Lixu <lixu.zhang@intel.com>
>Hi Zhang,
>
>Given this is clearly a fix, can you reply to this thread with an appropriate Fixes
>tag. Otherwise looks fine to me.
Sure, here is the Fixes tag:
Fixes: fc18dddc0625 ("iio: hid-sensors: Added device rotation support")
>
>Given timing and I guess that this bug is fairly old, this will probably only go
>upstream after rc1.
That's perfectly fine. Thanks for the review!
Best regards,
Lixu
>
>Thanks,
>
>Jonathan
>
^ permalink raw reply
* [PATCH v4] Input: elan_i2c - prevent division by zero and arithmetic underflow
From: Ranjan Kumar @ 2026-06-12 6:03 UTC (permalink / raw)
To: dmitry.torokhov; +Cc: bleung, bentiss, linux-input, linux-kernel, Ranjan Kumar
In-Reply-To: <aiefjOflOlbUiWDa@google.com>
The Elan I2C touchpad driver queries the device for its physical
dimensions and trace counts to calculate the device resolution and width.
However, if the device firmware or device tree provides invalid zero
values for x_traces or y_traces, it results in a fatal division-by-zero
exception leading to a kernel panic during device probe.
Add checks to ensure these parameters are non-zero before performing
the division. If invalid trace values are detected, fall back to a safe
default of 1.
Additionally, prevent an arithmetic underflow in the touch reporting
logic. Previously, if the calculated or fallback width was smaller than
ETP_FWIDTH_REDUCE (90), the subtraction would underflow, resulting in a
massive unsigned integer being reported to userspace. Clamp the adjusted
width to a minimum of 0 to safely handle small physical dimensions and
fallback scenarios.
Completing the probe with safe fallback values ensures the sysfs nodes
are created, keeping the firmware update path intact so a recovery
firmware can be flashed to the device.
Fixes: 6696777c6506 ("Input: add driver for Elan I2C/SMbus touchpad")
Fixes: e3a9a1290688 ("Input: elan_i2c - do not query the info if they are provided")
Signed-off-by: Ranjan Kumar <kumarranja@chromium.org>
---
Changes in v4:
- Reverted probe fallback width back to 1.
- Added check in elan_report_contact() to clamp adjusted widths to a
minimum of 0, fixing the root cause of the arithmetic underflow.
Changes in v3:
- Changed fallback width from 1 to ETP_FWIDTH_REDUCE to prevent underflow.
Changes in v2:
- Added check for invalid trace values of 0 to prevent division by zero.
drivers/input/mouse/elan_i2c_core.c | 36 ++++++++++++++++++++++++-----
1 file changed, 30 insertions(+), 6 deletions(-)
diff --git a/drivers/input/mouse/elan_i2c_core.c b/drivers/input/mouse/elan_i2c_core.c
index 5cba02a156ce..edb0a28d25aa 100644
--- a/drivers/input/mouse/elan_i2c_core.c
+++ b/drivers/input/mouse/elan_i2c_core.c
@@ -428,8 +428,17 @@ static int elan_query_device_parameters(struct elan_tp_data *data)
if (error)
return error;
}
- data->width_x = data->max_x / x_traces;
- data->width_y = data->max_y / y_traces;
+
+ if (!x_traces || !y_traces) {
+ dev_warn(&client->dev,
+ "invalid trace numbers: x=%u, y=%u\n",
+ x_traces, y_traces);
+ data->width_x = 1;
+ data->width_y = 1;
+ } else {
+ data->width_x = data->max_x / x_traces;
+ data->width_y = data->max_y / y_traces;
+ }
if (device_property_read_u32(&client->dev,
"touchscreen-x-mm", &x_mm) ||
@@ -443,8 +452,16 @@ static int elan_query_device_parameters(struct elan_tp_data *data)
data->x_res = elan_convert_resolution(hw_x_res, data->pattern);
data->y_res = elan_convert_resolution(hw_y_res, data->pattern);
} else {
- data->x_res = (data->max_x + 1) / x_mm;
- data->y_res = (data->max_y + 1) / y_mm;
+ if (unlikely(x_mm == 0 || y_mm == 0)) {
+ dev_warn(&client->dev,
+ "invalid physical dimensions: x_mm=%u, y_mm=%u\n",
+ x_mm, y_mm);
+ data->x_res = 1;
+ data->y_res = 1;
+ } else {
+ data->x_res = (data->max_x + 1) / x_mm;
+ data->y_res = (data->max_y + 1) / y_mm;
+ }
}
if (device_property_read_bool(&client->dev, "elan,clickpad"))
@@ -956,6 +973,7 @@ static void elan_report_contact(struct elan_tp_data *data, int contact_num,
if (data->report_features & ETP_FEATURE_REPORT_MK) {
unsigned int mk_x, mk_y, area_x, area_y;
+ int adj_width_x, adj_width_y;
u8 mk_data = high_precision ?
packet[ETP_MK_DATA_OFFSET + contact_num] :
finger_data[3];
@@ -967,8 +985,14 @@ static void elan_report_contact(struct elan_tp_data *data, int contact_num,
* To avoid treating large finger as palm, let's reduce
* the width x and y per trace.
*/
- area_x = mk_x * (data->width_x - ETP_FWIDTH_REDUCE);
- area_y = mk_y * (data->width_y - ETP_FWIDTH_REDUCE);
+
+ adj_width_x = data->width_x > ETP_FWIDTH_REDUCE ?
+ data->width_x - ETP_FWIDTH_REDUCE : 0;
+ adj_width_y = data->width_y > ETP_FWIDTH_REDUCE ?
+ data->width_y - ETP_FWIDTH_REDUCE : 0;
+
+ area_x = mk_x * adj_width_x;
+ area_y = mk_y * adj_width_y;
input_report_abs(input, ABS_TOOL_WIDTH, mk_x);
input_report_abs(input, ABS_MT_TOUCH_MAJOR,
--
2.54.0.1099.g489fc7bff1-goog
^ permalink raw reply related
* [PATCH v2 0/9] Input: cap11xx - Add support for CAP1114
From: Jun Yan @ 2026-06-12 7:22 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Jun Yan, linux-input, devicetree, linux-kernel
CAP1114 is a 14-channel capacitive touch sensor with 11 LED outputs
and hardware reset support.
Patches 1-4 perform driver cleanup and DT binding tweaks.
Patches 5-6 add reset-gpios support for CAP11xx.
Patches 7-9 add support for CAP1114.
Changes in v2:
- Drop LED property tweaks, keep only reg changes and node regex
update in DT bindings.
- Split microchip,cap1126 LED reg constraints into a separate patch.
- Replace usleep_range() with msleep() for 500 ms delay during
reset pin handling.
- Add missing <linux/delay.h> for usleep_range() and msleep().
- Add CAP1114 to unsupported enum for microchip,signal-guard and
microchip,calib-sensitivity
- Add constraint for linux,keycodes to support CAP1114.
- When reading CAP1114 button status, mask STATUS1 to bits 0-5
and OR with STATUS2.
- Adjust code style.
- Link to v1:
https://lore.kernel.org/all/20260606150458.250606-1-jerrysteve1101@gmail.com
Jun Yan (9):
Input: cap11xx - clean up duplicate log and add probe error logs
Input: cap11xx - remove unused register macros
dt-bindings: input: microchip,cap11xx: Update datasheet URL and LED
reg range
dt-bindings: input: microchip,cap11xx: Add microchip,cap1126 LED reg
constraints
dt-bindings: input: microchip,cap11xx: Add reset-gpios property
Input: cap11xx - add reset gpio support
Input: cap11xx - refactor code for better CAP1114 support.
dt-bindings: input: microchip,cap11xx: Add CAP1114 support
Input: cap11xx - add support for CAP1114
.../bindings/input/microchip,cap11xx.yaml | 85 ++++++-
drivers/input/keyboard/cap11xx.c | 230 +++++++++++-------
2 files changed, 224 insertions(+), 91 deletions(-)
--
2.54.0
^ permalink raw reply
* [PATCH v2 1/9] Input: cap11xx - clean up duplicate log and add probe error logs
From: Jun Yan @ 2026-06-12 7:22 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Jun Yan, linux-input, devicetree, linux-kernel
In-Reply-To: <20260612072237.1177304-1-jerrysteve1101@gmail.com>
Duplicated device detection log exists at line 537 and line 542,
which brings redundant kernel print messages. Drop one redundant
log entry to clean up dmesg output.
Meanwhile add missing error logs when I2C communication fails
during driver probe(), helping debug.
Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
---
drivers/input/keyboard/cap11xx.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/input/keyboard/cap11xx.c b/drivers/input/keyboard/cap11xx.c
index 2447c1ae2166..485d8ba97723 100644
--- a/drivers/input/keyboard/cap11xx.c
+++ b/drivers/input/keyboard/cap11xx.c
@@ -512,7 +512,7 @@ static int cap11xx_i2c_probe(struct i2c_client *i2c_client)
error = regmap_read(priv->regmap, CAP11XX_REG_PRODUCT_ID, &val);
if (error)
- return error;
+ return dev_err_probe(dev, error, "Failed to read product ID\n");
if (val != cap->product_id) {
dev_err(dev, "Product ID: Got 0x%02x, expected 0x%02x\n",
@@ -522,7 +522,7 @@ static int cap11xx_i2c_probe(struct i2c_client *i2c_client)
error = regmap_read(priv->regmap, CAP11XX_REG_MANUFACTURER_ID, &val);
if (error)
- return error;
+ return dev_err_probe(dev, error, "Failed to read manufacturer ID\n");
if (val != CAP11XX_MANUFACTURER_ID) {
dev_err(dev, "Manufacturer ID: Got 0x%02x, expected 0x%02x\n",
@@ -531,11 +531,8 @@ static int cap11xx_i2c_probe(struct i2c_client *i2c_client)
}
error = regmap_read(priv->regmap, CAP11XX_REG_REVISION, &rev);
- if (error < 0)
- return error;
-
- dev_info(dev, "CAP11XX detected, model %s, revision 0x%02x\n",
- id->name, rev);
+ if (error)
+ return dev_err_probe(dev, error, "Failed to read revision\n");
priv->model = cap;
--
2.54.0
^ permalink raw reply related
* [PATCH v2 2/9] Input: cap11xx - remove unused register macros
From: Jun Yan @ 2026-06-12 7:22 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Jun Yan, linux-input, devicetree, linux-kernel
In-Reply-To: <20260612072237.1177304-1-jerrysteve1101@gmail.com>
Remove unused register address macros and unused definitions in
the cap11xx_reg_defaults array and cap11xx_volatile_reg.
This cleanup reduces code clutter and makes the driver easier to
maintain without affecting functionality.
Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
---
drivers/input/keyboard/cap11xx.c | 58 --------------------------------
1 file changed, 58 deletions(-)
diff --git a/drivers/input/keyboard/cap11xx.c b/drivers/input/keyboard/cap11xx.c
index 485d8ba97723..686174722204 100644
--- a/drivers/input/keyboard/cap11xx.c
+++ b/drivers/input/keyboard/cap11xx.c
@@ -20,53 +20,23 @@
#define CAP11XX_REG_MAIN_CONTROL_GAIN_SHIFT (6)
#define CAP11XX_REG_MAIN_CONTROL_GAIN_MASK (0xc0)
#define CAP11XX_REG_MAIN_CONTROL_DLSEEP BIT(4)
-#define CAP11XX_REG_GENERAL_STATUS 0x02
#define CAP11XX_REG_SENSOR_INPUT 0x03
-#define CAP11XX_REG_NOISE_FLAG_STATUS 0x0a
-#define CAP11XX_REG_SENOR_DELTA(X) (0x10 + (X))
#define CAP11XX_REG_SENSITIVITY_CONTROL 0x1f
#define CAP11XX_REG_SENSITIVITY_CONTROL_DELTA_SENSE_MASK 0x70
-#define CAP11XX_REG_CONFIG 0x20
-#define CAP11XX_REG_SENSOR_ENABLE 0x21
-#define CAP11XX_REG_SENSOR_CONFIG 0x22
-#define CAP11XX_REG_SENSOR_CONFIG2 0x23
-#define CAP11XX_REG_SAMPLING_CONFIG 0x24
-#define CAP11XX_REG_CALIBRATION 0x26
-#define CAP11XX_REG_INT_ENABLE 0x27
#define CAP11XX_REG_REPEAT_RATE 0x28
#define CAP11XX_REG_SIGNAL_GUARD_ENABLE 0x29
-#define CAP11XX_REG_MT_CONFIG 0x2a
-#define CAP11XX_REG_MT_PATTERN_CONFIG 0x2b
-#define CAP11XX_REG_MT_PATTERN 0x2d
-#define CAP11XX_REG_RECALIB_CONFIG 0x2f
#define CAP11XX_REG_SENSOR_THRESH(X) (0x30 + (X))
-#define CAP11XX_REG_SENSOR_NOISE_THRESH 0x38
-#define CAP11XX_REG_STANDBY_CHANNEL 0x40
-#define CAP11XX_REG_STANDBY_CONFIG 0x41
-#define CAP11XX_REG_STANDBY_SENSITIVITY 0x42
-#define CAP11XX_REG_STANDBY_THRESH 0x43
#define CAP11XX_REG_CONFIG2 0x44
#define CAP11XX_REG_CONFIG2_ALT_POL BIT(6)
-#define CAP11XX_REG_SENSOR_BASE_CNT(X) (0x50 + (X))
-#define CAP11XX_REG_LED_POLARITY 0x73
#define CAP11XX_REG_LED_OUTPUT_CONTROL 0x74
#define CAP11XX_REG_CALIB_SENSITIVITY_CONFIG 0x80
#define CAP11XX_REG_CALIB_SENSITIVITY_CONFIG2 0x81
-
-#define CAP11XX_REG_LED_DUTY_CYCLE_1 0x90
-#define CAP11XX_REG_LED_DUTY_CYCLE_2 0x91
-#define CAP11XX_REG_LED_DUTY_CYCLE_3 0x92
#define CAP11XX_REG_LED_DUTY_CYCLE_4 0x93
-#define CAP11XX_REG_LED_DUTY_MIN_MASK (0x0f)
-#define CAP11XX_REG_LED_DUTY_MIN_MASK_SHIFT (0)
#define CAP11XX_REG_LED_DUTY_MAX_MASK (0xf0)
#define CAP11XX_REG_LED_DUTY_MAX_MASK_SHIFT (4)
#define CAP11XX_REG_LED_DUTY_MAX_VALUE (15)
-#define CAP11XX_REG_SENSOR_CALIB (0xb1 + (X))
-#define CAP11XX_REG_SENSOR_CALIB_LSB1 0xb9
-#define CAP11XX_REG_SENSOR_CALIB_LSB2 0xba
#define CAP11XX_REG_PRODUCT_ID 0xfd
#define CAP11XX_REG_MANUFACTURER_ID 0xfe
#define CAP11XX_REG_REVISION 0xff
@@ -111,37 +81,15 @@ struct cap11xx_hw_model {
static const struct reg_default cap11xx_reg_defaults[] = {
{ CAP11XX_REG_MAIN_CONTROL, 0x00 },
- { CAP11XX_REG_GENERAL_STATUS, 0x00 },
- { CAP11XX_REG_SENSOR_INPUT, 0x00 },
- { CAP11XX_REG_NOISE_FLAG_STATUS, 0x00 },
{ CAP11XX_REG_SENSITIVITY_CONTROL, 0x2f },
- { CAP11XX_REG_CONFIG, 0x20 },
- { CAP11XX_REG_SENSOR_ENABLE, 0x3f },
- { CAP11XX_REG_SENSOR_CONFIG, 0xa4 },
- { CAP11XX_REG_SENSOR_CONFIG2, 0x07 },
- { CAP11XX_REG_SAMPLING_CONFIG, 0x39 },
- { CAP11XX_REG_CALIBRATION, 0x00 },
- { CAP11XX_REG_INT_ENABLE, 0x3f },
{ CAP11XX_REG_REPEAT_RATE, 0x3f },
- { CAP11XX_REG_MT_CONFIG, 0x80 },
- { CAP11XX_REG_MT_PATTERN_CONFIG, 0x00 },
- { CAP11XX_REG_MT_PATTERN, 0x3f },
- { CAP11XX_REG_RECALIB_CONFIG, 0x8a },
{ CAP11XX_REG_SENSOR_THRESH(0), 0x40 },
{ CAP11XX_REG_SENSOR_THRESH(1), 0x40 },
{ CAP11XX_REG_SENSOR_THRESH(2), 0x40 },
{ CAP11XX_REG_SENSOR_THRESH(3), 0x40 },
{ CAP11XX_REG_SENSOR_THRESH(4), 0x40 },
{ CAP11XX_REG_SENSOR_THRESH(5), 0x40 },
- { CAP11XX_REG_SENSOR_NOISE_THRESH, 0x01 },
- { CAP11XX_REG_STANDBY_CHANNEL, 0x00 },
- { CAP11XX_REG_STANDBY_CONFIG, 0x39 },
- { CAP11XX_REG_STANDBY_SENSITIVITY, 0x02 },
- { CAP11XX_REG_STANDBY_THRESH, 0x40 },
{ CAP11XX_REG_CONFIG2, 0x40 },
- { CAP11XX_REG_LED_POLARITY, 0x00 },
- { CAP11XX_REG_SENSOR_CALIB_LSB1, 0x00 },
- { CAP11XX_REG_SENSOR_CALIB_LSB2, 0x00 },
};
static bool cap11xx_volatile_reg(struct device *dev, unsigned int reg)
@@ -149,12 +97,6 @@ static bool cap11xx_volatile_reg(struct device *dev, unsigned int reg)
switch (reg) {
case CAP11XX_REG_MAIN_CONTROL:
case CAP11XX_REG_SENSOR_INPUT:
- case CAP11XX_REG_SENOR_DELTA(0):
- case CAP11XX_REG_SENOR_DELTA(1):
- case CAP11XX_REG_SENOR_DELTA(2):
- case CAP11XX_REG_SENOR_DELTA(3):
- case CAP11XX_REG_SENOR_DELTA(4):
- case CAP11XX_REG_SENOR_DELTA(5):
return true;
}
--
2.54.0
^ permalink raw reply related
* [PATCH v2 3/9] dt-bindings: input: microchip,cap11xx: Update datasheet URL and LED reg range
From: Jun Yan @ 2026-06-12 7:22 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Jun Yan, linux-input, devicetree, linux-kernel
In-Reply-To: <20260612072237.1177304-1-jerrysteve1101@gmail.com>
- Add datasheet links for all supported CAP11xx variants.
- Update LED node regex and replace enum constraints with minimum/maximum
for LED reg ranges in preparation for CAP1114 support.
CAP1114 has 11 LED channels. minimum/maximum constraints are easier to
maintain than long enum lists when expanding channel count later.
Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
---
.../bindings/input/microchip,cap11xx.yaml | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
index 7ade03f1b32b..9578c7c206a2 100644
--- a/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
+++ b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
@@ -10,6 +10,15 @@ description: |
The Microchip CAP1xxx Family of RightTouchTM multiple-channel capacitive
touch controllers and LED drivers. The device communication via I2C only.
+ For more product information please see the links below:
+ CAP1106: https://ww1.microchip.com/downloads/en/DeviceDoc/00001624B.pdf
+ CAP1126: https://ww1.microchip.com/downloads/en/DeviceDoc/00001623B.pdf
+ CAP1188: https://ww1.microchip.com/downloads/en/DeviceDoc/00001620C.pdf
+ CAP1203: https://ww1.microchip.com/downloads/en/DeviceDoc/00001572B.pdf
+ CAP1206: https://ww1.microchip.com/downloads/en/DeviceDoc/00001567B.pdf
+ CAP1293: https://ww1.microchip.com/downloads/en/DeviceDoc/00001566B.pdf
+ CAP1298: https://ww1.microchip.com/downloads/en/DeviceDoc/00001571B.pdf
+
maintainers:
- Rob Herring <robh@kernel.org>
@@ -124,14 +133,16 @@ properties:
The number of entries must correspond to the number of channels.
patternProperties:
- "^led@[0-7]$":
+ "^led@[0-9a-f]$":
type: object
description: CAP11xx LEDs
$ref: /schemas/leds/common.yaml#
properties:
reg:
- enum: [0, 1, 2, 3, 4, 5, 6, 7]
+ description: LED channel number
+ minimum: 0
+ maximum: 7
label: true
@@ -158,7 +169,7 @@ allOf:
- microchip,cap1298
then:
patternProperties:
- "^led@[0-7]$": false
+ "^led@": false
- if:
properties:
--
2.54.0
^ permalink raw reply related
* [PATCH v2 4/9] dt-bindings: input: microchip,cap11xx: Add microchip,cap1126 LED reg constraints
From: Jun Yan @ 2026-06-12 7:22 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Jun Yan, linux-input, devicetree, linux-kernel
In-Reply-To: <20260612072237.1177304-1-jerrysteve1101@gmail.com>
Apply per-chip LED channel limits:
- CAP1126: max 2 channels (0-1)
- CAP1188: max 8 channels (0-7)
- CAP1106, CAP12xx: no LED support
Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
---
.../bindings/input/microchip,cap11xx.yaml | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
index 9578c7c206a2..22a292d4a880 100644
--- a/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
+++ b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
@@ -171,6 +171,19 @@ allOf:
patternProperties:
"^led@": false
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - microchip,cap1126
+ then:
+ patternProperties:
+ "^led@":
+ properties:
+ reg:
+ maximum: 1
+
- if:
properties:
compatible:
--
2.54.0
^ permalink raw reply related
* [PATCH v2 5/9] dt-bindings: input: microchip,cap11xx: Add reset-gpios property
From: Jun Yan @ 2026-06-12 7:22 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Jun Yan, Conor Dooley, linux-input, devicetree, linux-kernel
In-Reply-To: <20260612072237.1177304-1-jerrysteve1101@gmail.com>
Add support for the optional reset-gpios property to describe
the active-high reset pin for CAP1126/CAP1188 devices.
Driving the GPIO high asserts reset and deep sleep, while driving
it low releases reset for normal operation.
Restrict this property to be available only on CAP1126 and CAP1188
chips, as other CAP11xx variants do not have a hardware reset pin.
Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
---
.../bindings/input/microchip,cap11xx.yaml | 25 +++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
index 22a292d4a880..778ec6d659a8 100644
--- a/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
+++ b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
@@ -49,6 +49,13 @@ properties:
device's ALERT#/CM_IRQ# pin is connected to.
The device only has one interrupt source.
+ reset-gpios:
+ description: |
+ GPIO connected to the active-high RESET pin of the chip;
+ driving it high asserts reset and deep sleep, while driving
+ it low releases reset for normal operation.
+ maxItems: 1
+
autorepeat:
description: |
Enables the Linux input system's autorepeat feature on the input device.
@@ -157,6 +164,20 @@ patternProperties:
allOf:
- $ref: input.yaml
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - microchip,cap1106
+ - microchip,cap1203
+ - microchip,cap1206
+ - microchip,cap1293
+ - microchip,cap1298
+ then:
+ properties:
+ reset-gpios: false
+
- if:
properties:
compatible:
@@ -207,6 +228,8 @@ additionalProperties: false
examples:
- |
+ #include <dt-bindings/gpio/gpio.h>
+
i2c {
#address-cells = <1>;
#size-cells = <0>;
@@ -228,6 +251,8 @@ examples:
<109>, /* KEY_PAGEDOWN */
<104>; /* KEY_PAGEUP */
+ reset-gpios = <&gpio 17 GPIO_ACTIVE_HIGH>;
+
#address-cells = <1>;
#size-cells = <0>;
--
2.54.0
^ permalink raw reply related
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