Linux Input/HID development
 help / color / mirror / Atom feed
* [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

* [PATCH v2 6/9] Input: cap11xx - add reset gpio support
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>

Some CAP11xx devices (CAP1126/CAP1188) have a dedicated RESET pin.
Add hardware reset operation to improve device reliability and
ensure proper initialization on probe.

Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
---
 drivers/input/keyboard/cap11xx.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/input/keyboard/cap11xx.c b/drivers/input/keyboard/cap11xx.c
index 686174722204..75746a8a2233 100644
--- a/drivers/input/keyboard/cap11xx.c
+++ b/drivers/input/keyboard/cap11xx.c
@@ -5,6 +5,7 @@
  * (c) 2014 Daniel Mack <linux@zonque.org>
  */
 
+#include <linux/delay.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/interrupt.h>
@@ -43,6 +44,9 @@
 
 #define CAP11XX_MANUFACTURER_ID	0x5d
 
+#define CAP11XX_T_RST_FILT_MIN_US	10000
+#define CAP11XX_T_RST_ON_MIN_MS	400
+
 #ifdef CONFIG_LEDS_CLASS
 struct cap11xx_led {
 	struct cap11xx_priv *priv;
@@ -55,6 +59,7 @@ struct cap11xx_priv {
 	struct regmap *regmap;
 	struct device *dev;
 	struct input_dev *idev;
+	struct gpio_desc *reset_gpio;
 	const struct cap11xx_hw_model *model;
 
 	struct cap11xx_led *leds;
@@ -452,6 +457,17 @@ static int cap11xx_i2c_probe(struct i2c_client *i2c_client)
 	if (IS_ERR(priv->regmap))
 		return PTR_ERR(priv->regmap);
 
+	priv->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
+	if (IS_ERR(priv->reset_gpio))
+		return dev_err_probe(dev, PTR_ERR(priv->reset_gpio),
+				     "Failed to get 'reset' GPIO\n");
+	if (priv->reset_gpio) {
+		gpiod_set_value_cansleep(priv->reset_gpio, 1);
+		usleep_range(CAP11XX_T_RST_FILT_MIN_US, CAP11XX_T_RST_FILT_MIN_US * 2);
+		gpiod_set_value_cansleep(priv->reset_gpio, 0);
+		msleep(CAP11XX_T_RST_ON_MIN_MS);
+	}
+
 	error = regmap_read(priv->regmap, CAP11XX_REG_PRODUCT_ID, &val);
 	if (error)
 		return dev_err_probe(dev, error, "Failed to read product ID\n");
-- 
2.54.0


^ permalink raw reply related

* [PATCH v2 7/9] Input: cap11xx - refactor code for better CAP1114 support.
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>

Extend cap11xx_hw_model structure to support CAP1114 with
different register offsets and hardware characteristics:

- led_output_control_reg_base: different address on CAP1114
- sensor_input_reg_base: different address on CAP1114
- num_sensor_thresholds: separate value from num_channels for CAP1114
- has_repeat_en: repeat enable support, disabled by default on CAP1114

Include linux/bits.h, update the register operations related to LEDs.

Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
---
 drivers/input/keyboard/cap11xx.c | 73 +++++++++++++++++++++++---------
 1 file changed, 53 insertions(+), 20 deletions(-)

diff --git a/drivers/input/keyboard/cap11xx.c b/drivers/input/keyboard/cap11xx.c
index 75746a8a2233..d45bb231d7a1 100644
--- a/drivers/input/keyboard/cap11xx.c
+++ b/drivers/input/keyboard/cap11xx.c
@@ -5,6 +5,7 @@
  * (c) 2014 Daniel Mack <linux@zonque.org>
  */
 
+#include <linux/bits.h>
 #include <linux/delay.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
@@ -35,7 +36,6 @@
 #define CAP11XX_REG_LED_DUTY_CYCLE_4	0x93
 
 #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_PRODUCT_ID		0xfd
@@ -76,10 +76,14 @@ struct cap11xx_priv {
 
 struct cap11xx_hw_model {
 	u8 product_id;
+	u8 led_output_control_reg_base;
+	u8 sensor_input_reg_base;
 	unsigned int num_channels;
 	unsigned int num_leds;
+	unsigned int num_sensor_thresholds;
 	bool has_gain;
 	bool has_irq_config;
+	bool has_repeat_en;
 	bool has_sensitivity_control;
 	bool has_signal_guard;
 };
@@ -204,8 +208,8 @@ static int cap11xx_init_keys(struct cap11xx_priv *priv)
 	}
 
 	if (!of_property_read_u32_array(node, "microchip,input-threshold",
-					priv->thresholds, priv->model->num_channels)) {
-		for (i = 0; i < priv->model->num_channels; i++) {
+					priv->thresholds, priv->model->num_sensor_thresholds)) {
+		for (i = 0; i < priv->model->num_sensor_thresholds; i++) {
 			if (priv->thresholds[i] > 127) {
 				dev_err(dev, "Invalid input-threshold value %u\n",
 					priv->thresholds[i]);
@@ -279,10 +283,12 @@ static int cap11xx_init_keys(struct cap11xx_priv *priv)
 	of_property_read_u32_array(node, "linux,keycodes",
 				   priv->keycodes, priv->model->num_channels);
 
-	/* Disable autorepeat. The Linux input system has its own handling. */
-	error = regmap_write(priv->regmap, CAP11XX_REG_REPEAT_RATE, 0);
-	if (error)
-		return error;
+	if (priv->model->has_repeat_en) {
+		/* Disable autorepeat. The Linux input system has its own handling. */
+		error = regmap_write(priv->regmap, CAP11XX_REG_REPEAT_RATE, 0);
+		if (error)
+			return error;
+	}
 
 	return 0;
 }
@@ -301,7 +307,7 @@ static irqreturn_t cap11xx_thread_func(int irq_num, void *data)
 	if (ret < 0)
 		goto out;
 
-	ret = regmap_read(priv->regmap, CAP11XX_REG_SENSOR_INPUT, &status);
+	ret = regmap_read(priv->regmap, priv->model->sensor_input_reg_base, &status);
 	if (ret < 0)
 		goto out;
 
@@ -355,7 +361,7 @@ static int cap11xx_led_set(struct led_classdev *cdev,
 	 * 0 (OFF) and 1 (ON).
 	 */
 	return regmap_update_bits(priv->regmap,
-				  CAP11XX_REG_LED_OUTPUT_CONTROL,
+				  priv->model->led_output_control_reg_base,
 				  BIT(led->reg),
 				  value ? BIT(led->reg) : 0);
 }
@@ -367,6 +373,7 @@ static int cap11xx_init_leds(struct device *dev,
 	struct cap11xx_led *led;
 	int cnt = of_get_child_count(node);
 	int error;
+	u32 duty_val;
 
 	if (!num_leds || !cnt)
 		return 0;
@@ -380,15 +387,18 @@ static int cap11xx_init_leds(struct device *dev,
 
 	priv->leds = led;
 
+	/* Set all LEDs to off */
 	error = regmap_update_bits(priv->regmap,
-				CAP11XX_REG_LED_OUTPUT_CONTROL, 0xff, 0);
+				   priv->model->led_output_control_reg_base,
+				   GENMASK(num_leds - 1, 0), 0);
 	if (error)
 		return error;
 
+	duty_val = FIELD_PREP(CAP11XX_REG_LED_DUTY_MAX_MASK,
+			      CAP11XX_REG_LED_DUTY_MAX_VALUE);
+
 	error = regmap_update_bits(priv->regmap, CAP11XX_REG_LED_DUTY_CYCLE_4,
-				CAP11XX_REG_LED_DUTY_MAX_MASK,
-				CAP11XX_REG_LED_DUTY_MAX_VALUE <<
-				CAP11XX_REG_LED_DUTY_MAX_MASK_SHIFT);
+				   CAP11XX_REG_LED_DUTY_MAX_MASK, duty_val);
 	if (error)
 		return error;
 
@@ -554,41 +564,64 @@ static int cap11xx_i2c_probe(struct i2c_client *i2c_client)
 }
 
 static const struct cap11xx_hw_model cap1106_model = {
-	.product_id = 0x55, .num_channels = 6, .num_leds = 0,
+	.product_id = 0x55,
+	.num_channels = 6, .num_leds = 0, .num_sensor_thresholds = 6,
+	.sensor_input_reg_base = CAP11XX_REG_SENSOR_INPUT,
 	.has_gain = true,
 	.has_irq_config = true,
+	.has_repeat_en = true,
 };
 
 static const struct cap11xx_hw_model cap1126_model = {
-	.product_id = 0x53, .num_channels = 6, .num_leds = 2,
+	.product_id = 0x53,
+	.num_channels = 6, .num_leds = 2, .num_sensor_thresholds = 6,
+	.led_output_control_reg_base = CAP11XX_REG_LED_OUTPUT_CONTROL,
+	.sensor_input_reg_base = CAP11XX_REG_SENSOR_INPUT,
 	.has_gain = true,
 	.has_irq_config = true,
+	.has_repeat_en = true,
 };
 
 static const struct cap11xx_hw_model cap1188_model = {
-	.product_id = 0x50, .num_channels = 8, .num_leds = 8,
+	.product_id = 0x50,
+	.num_channels = 8, .num_leds = 8, .num_sensor_thresholds = 8,
+	.led_output_control_reg_base = CAP11XX_REG_LED_OUTPUT_CONTROL,
+	.sensor_input_reg_base = CAP11XX_REG_SENSOR_INPUT,
 	.has_gain = true,
 	.has_irq_config = true,
+	.has_repeat_en = true,
 };
 
 static const struct cap11xx_hw_model cap1203_model = {
-	.product_id = 0x6d, .num_channels = 3, .num_leds = 0,
+	.product_id = 0x6d,
+	.num_channels = 3, .num_leds = 0, .num_sensor_thresholds = 3,
+	.sensor_input_reg_base = CAP11XX_REG_SENSOR_INPUT,
+	.has_repeat_en = true,
 };
 
 static const struct cap11xx_hw_model cap1206_model = {
-	.product_id = 0x67, .num_channels = 6, .num_leds = 0,
+	.product_id = 0x67,
+	.num_channels = 6, .num_leds = 0, .num_sensor_thresholds = 6,
+	.sensor_input_reg_base = CAP11XX_REG_SENSOR_INPUT,
+	.has_repeat_en = true,
 };
 
 static const struct cap11xx_hw_model cap1293_model = {
-	.product_id = 0x6f, .num_channels = 3, .num_leds = 0,
+	.product_id = 0x6f,
+	.num_channels = 3, .num_leds = 0, .num_sensor_thresholds = 3,
+	.sensor_input_reg_base = CAP11XX_REG_SENSOR_INPUT,
 	.has_gain = true,
+	.has_repeat_en = true,
 	.has_sensitivity_control = true,
 	.has_signal_guard = true,
 };
 
 static const struct cap11xx_hw_model cap1298_model = {
-	.product_id = 0x71, .num_channels = 8, .num_leds = 0,
+	.product_id = 0x71,
+	.num_channels = 8, .num_leds = 0, .num_sensor_thresholds = 8,
+	.sensor_input_reg_base = CAP11XX_REG_SENSOR_INPUT,
 	.has_gain = true,
+	.has_repeat_en = true,
 	.has_sensitivity_control = true,
 	.has_signal_guard = true,
 };
-- 
2.54.0


^ permalink raw reply related

* [PATCH v2 8/9] dt-bindings: input: microchip,cap11xx: Add CAP1114 support
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>

CAP1114 is a 14-channel capacitive touch sensor with 11 LED outputs
and hardware reset support.

Add the compatible string for CAP1114, add its datasheet URL,
update the maximum of LED channel reg, and add constraint for
linux,keycodes.

Previously, the LED reg property had a default maximum of 7 for CAP1188.
With the addition of CAP1114, the default maximum is now 11.
An if-then constraint is added to limit the LED count for CAP1188.

Update description for microchip,input-threshold: CAP1114 only provides
eight threshold entries, which does not match its total channel count.

CAP1114 does not support microchip,signal-guard and
microchip,calib-sensitivity.

Add CAP1114 to the unsupported enum list.

Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
---
 .../bindings/input/microchip,cap11xx.yaml     | 32 ++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
index 778ec6d659a8..0e9a1a8a3f3e 100644
--- a/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
+++ b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
@@ -12,6 +12,7 @@ description: |
 
   For more product information please see the links below:
     CAP1106: https://ww1.microchip.com/downloads/en/DeviceDoc/00001624B.pdf
+    CAP1114: https://ww1.microchip.com/downloads/en/DeviceDoc/00002444A.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
@@ -26,6 +27,7 @@ properties:
   compatible:
     enum:
       - microchip,cap1106
+      - microchip,cap1114
       - microchip,cap1126
       - microchip,cap1188
       - microchip,cap1203
@@ -122,6 +124,8 @@ properties:
       is required for a touch to be registered, making the touch sensor less
       sensitive.
       The number of entries must correspond to the number of channels.
+      CAP1114 is an exception where channels 8~14 reuse the eighth entry's
+      threshold, so counts differ.
 
   microchip,calib-sensitivity:
     $ref: /schemas/types.yaml#/definitions/uint32-array
@@ -149,7 +153,7 @@ patternProperties:
       reg:
         description: LED channel number
         minimum: 0
-        maximum: 7
+        maximum: 10
 
       label: true
 
@@ -178,6 +182,18 @@ allOf:
       properties:
         reset-gpios: false
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - microchip,cap1114
+    then:
+      properties:
+        linux,keycodes:
+          minItems: 14
+          maxItems: 14
+
   - if:
       properties:
         compatible:
@@ -205,12 +221,26 @@ allOf:
             reg:
               maximum: 1
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - microchip,cap1188
+    then:
+      patternProperties:
+        "^led@":
+          properties:
+            reg:
+              maximum: 7
+
   - if:
       properties:
         compatible:
           contains:
             enum:
               - microchip,cap1106
+              - microchip,cap1114
               - microchip,cap1126
               - microchip,cap1188
               - microchip,cap1203
-- 
2.54.0


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox