Linux Input/HID development
 help / color / mirror / Atom feed
* [PATCH v2 3/3] HID: tighten ioctl command parsing
From: bentiss @ 2025-08-26 12:39 UTC (permalink / raw)
  To: Jiri Kosina, Shuah Khan, Arnd Bergmann
  Cc: linux-input, linux-kselftest, linux-kernel, Benjamin Tissoires,
	Arnd Bergmann
In-Reply-To: <20250826-b4-hidraw-ioctls-v2-0-c7726b236719@kernel.org>

From: Arnd Bergmann <arnd@arndb.de>

The handling for variable-length ioctl commands in hidraw_ioctl() is
rather complex and the check for the data direction is incomplete.

Simplify this code by factoring out the various ioctls grouped by dir
and size, and using a switch() statement with the size masked out, to
ensure the rest of the command is correctly matched.

Fixes: 9188e79ec3fd ("HID: add phys and name ioctls to hidraw")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
 drivers/hid/hidraw.c        | 224 ++++++++++++++++++++++++--------------------
 include/uapi/linux/hidraw.h |   2 +
 2 files changed, 124 insertions(+), 102 deletions(-)

diff --git a/drivers/hid/hidraw.c b/drivers/hid/hidraw.c
index c887f48756f4be2a4bac03128f2885bde96c1e39..bbd6f23bce78951c7d667ff5c1c923cee3509e3f 100644
--- a/drivers/hid/hidraw.c
+++ b/drivers/hid/hidraw.c
@@ -394,27 +394,15 @@ static int hidraw_revoke(struct hidraw_list *list)
 	return 0;
 }
 
-static long hidraw_ioctl(struct file *file, unsigned int cmd,
-							unsigned long arg)
+static long hidraw_fixed_size_ioctl(struct file *file, struct hidraw *dev, unsigned int cmd,
+				    void __user *arg)
 {
-	struct inode *inode = file_inode(file);
-	unsigned int minor = iminor(inode);
-	long ret = 0;
-	struct hidraw *dev;
-	struct hidraw_list *list = file->private_data;
-	void __user *user_arg = (void __user*) arg;
-
-	down_read(&minors_rwsem);
-	dev = hidraw_table[minor];
-	if (!dev || !dev->exist || hidraw_is_revoked(list)) {
-		ret = -ENODEV;
-		goto out;
-	}
+	struct hid_device *hid = dev->hid;
 
 	switch (cmd) {
 		case HIDIOCGRDESCSIZE:
-			if (put_user(dev->hid->rsize, (int __user *)arg))
-				ret = -EFAULT;
+			if (put_user(hid->rsize, (int __user *)arg))
+				return -EFAULT;
 			break;
 
 		case HIDIOCGRDESC:
@@ -422,113 +410,145 @@ static long hidraw_ioctl(struct file *file, unsigned int cmd,
 				__u32 len;
 
 				if (get_user(len, (int __user *)arg))
-					ret = -EFAULT;
-				else if (len > HID_MAX_DESCRIPTOR_SIZE - 1)
-					ret = -EINVAL;
-				else if (copy_to_user(user_arg + offsetof(
-					struct hidraw_report_descriptor,
-					value[0]),
-					dev->hid->rdesc,
-					min(dev->hid->rsize, len)))
-					ret = -EFAULT;
+					return -EFAULT;
+
+				if (len > HID_MAX_DESCRIPTOR_SIZE - 1)
+					return -EINVAL;
+
+				if (copy_to_user(arg + offsetof(
+				    struct hidraw_report_descriptor,
+				    value[0]),
+				    hid->rdesc,
+				    min(hid->rsize, len)))
+					return -EFAULT;
+
 				break;
 			}
 		case HIDIOCGRAWINFO:
 			{
 				struct hidraw_devinfo dinfo;
 
-				dinfo.bustype = dev->hid->bus;
-				dinfo.vendor = dev->hid->vendor;
-				dinfo.product = dev->hid->product;
-				if (copy_to_user(user_arg, &dinfo, sizeof(dinfo)))
-					ret = -EFAULT;
+				dinfo.bustype = hid->bus;
+				dinfo.vendor = hid->vendor;
+				dinfo.product = hid->product;
+				if (copy_to_user(arg, &dinfo, sizeof(dinfo)))
+					return -EFAULT;
 				break;
 			}
 		case HIDIOCREVOKE:
 			{
-				if (user_arg)
-					ret = -EINVAL;
-				else
-					ret = hidraw_revoke(list);
-				break;
+				struct hidraw_list *list = file->private_data;
+
+				if (arg)
+					return -EINVAL;
+
+				return hidraw_revoke(list);
 			}
 		default:
-			{
-				struct hid_device *hid = dev->hid;
-				if (_IOC_TYPE(cmd) != 'H') {
-					ret = -EINVAL;
-					break;
-				}
+			/*
+			 * None of the above ioctls can return -EAGAIN, so
+			 * use it as a marker that we need to check variable
+			 * length ioctls.
+			 */
+			return -EAGAIN;
+	}
 
-				if (_IOC_NR(cmd) == _IOC_NR(HIDIOCSFEATURE(0))) {
-					int len = _IOC_SIZE(cmd);
-					ret = hidraw_send_report(file, user_arg, len, HID_FEATURE_REPORT);
-					break;
-				}
-				if (_IOC_NR(cmd) == _IOC_NR(HIDIOCGFEATURE(0))) {
-					int len = _IOC_SIZE(cmd);
-					ret = hidraw_get_report(file, user_arg, len, HID_FEATURE_REPORT);
-					break;
-				}
+	return 0;
+}
 
-				if (_IOC_NR(cmd) == _IOC_NR(HIDIOCSINPUT(0))) {
-					int len = _IOC_SIZE(cmd);
-					ret = hidraw_send_report(file, user_arg, len, HID_INPUT_REPORT);
-					break;
-				}
-				if (_IOC_NR(cmd) == _IOC_NR(HIDIOCGINPUT(0))) {
-					int len = _IOC_SIZE(cmd);
-					ret = hidraw_get_report(file, user_arg, len, HID_INPUT_REPORT);
-					break;
-				}
+static long hidraw_rw_variable_size_ioctl(struct file *file, struct hidraw *dev, unsigned int cmd,
+					  void __user *user_arg)
+{
+	int len = _IOC_SIZE(cmd);
+
+	switch (cmd & ~IOCSIZE_MASK) {
+	case HIDIOCSFEATURE(0):
+		return hidraw_send_report(file, user_arg, len, HID_FEATURE_REPORT);
+	case HIDIOCGFEATURE(0):
+		return hidraw_get_report(file, user_arg, len, HID_FEATURE_REPORT);
+	case HIDIOCSINPUT(0):
+		return hidraw_send_report(file, user_arg, len, HID_INPUT_REPORT);
+	case HIDIOCGINPUT(0):
+		return hidraw_get_report(file, user_arg, len, HID_INPUT_REPORT);
+	case HIDIOCSOUTPUT(0):
+		return hidraw_send_report(file, user_arg, len, HID_OUTPUT_REPORT);
+	case HIDIOCGOUTPUT(0):
+		return hidraw_get_report(file, user_arg, len, HID_OUTPUT_REPORT);
+	}
 
-				if (_IOC_NR(cmd) == _IOC_NR(HIDIOCSOUTPUT(0))) {
-					int len = _IOC_SIZE(cmd);
-					ret = hidraw_send_report(file, user_arg, len, HID_OUTPUT_REPORT);
-					break;
-				}
-				if (_IOC_NR(cmd) == _IOC_NR(HIDIOCGOUTPUT(0))) {
-					int len = _IOC_SIZE(cmd);
-					ret = hidraw_get_report(file, user_arg, len, HID_OUTPUT_REPORT);
-					break;
-				}
+	return -EINVAL;
+}
 
-				/* Begin Read-only ioctls. */
-				if (_IOC_DIR(cmd) != _IOC_READ) {
-					ret = -EINVAL;
-					break;
-				}
+static long hidraw_ro_variable_size_ioctl(struct file *file, struct hidraw *dev, unsigned int cmd,
+					  void __user *user_arg)
+{
+	struct hid_device *hid = dev->hid;
+	int len = _IOC_SIZE(cmd);
+	int field_len;
+
+	switch (cmd & ~IOCSIZE_MASK) {
+	case HIDIOCGRAWNAME(0):
+		field_len = strlen(hid->name) + 1;
+		if (len > field_len)
+			len = field_len;
+		return copy_to_user(user_arg, hid->name, len) ?  -EFAULT : len;
+	case HIDIOCGRAWPHYS(0):
+		field_len = strlen(hid->phys) + 1;
+		if (len > field_len)
+			len = field_len;
+		return copy_to_user(user_arg, hid->phys, len) ?  -EFAULT : len;
+	case HIDIOCGRAWUNIQ(0):
+		field_len = strlen(hid->uniq) + 1;
+		if (len > field_len)
+			len = field_len;
+		return copy_to_user(user_arg, hid->uniq, len) ?  -EFAULT : len;
+	}
 
-				if (_IOC_NR(cmd) == _IOC_NR(HIDIOCGRAWNAME(0))) {
-					int len = strlen(hid->name) + 1;
-					if (len > _IOC_SIZE(cmd))
-						len = _IOC_SIZE(cmd);
-					ret = copy_to_user(user_arg, hid->name, len) ?
-						-EFAULT : len;
-					break;
-				}
+	return -EINVAL;
+}
 
-				if (_IOC_NR(cmd) == _IOC_NR(HIDIOCGRAWPHYS(0))) {
-					int len = strlen(hid->phys) + 1;
-					if (len > _IOC_SIZE(cmd))
-						len = _IOC_SIZE(cmd);
-					ret = copy_to_user(user_arg, hid->phys, len) ?
-						-EFAULT : len;
-					break;
-				}
+static long hidraw_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
+{
+	struct inode *inode = file_inode(file);
+	unsigned int minor = iminor(inode);
+	struct hidraw *dev;
+	struct hidraw_list *list = file->private_data;
+	void __user *user_arg = (void __user *)arg;
+	int ret;
 
-				if (_IOC_NR(cmd) == _IOC_NR(HIDIOCGRAWUNIQ(0))) {
-					int len = strlen(hid->uniq) + 1;
-					if (len > _IOC_SIZE(cmd))
-						len = _IOC_SIZE(cmd);
-					ret = copy_to_user(user_arg, hid->uniq, len) ?
-						-EFAULT : len;
-					break;
-				}
-			}
+	down_read(&minors_rwsem);
+	dev = hidraw_table[minor];
+	if (!dev || !dev->exist || hidraw_is_revoked(list)) {
+		ret = -ENODEV;
+		goto out;
+	}
+
+	if (_IOC_TYPE(cmd) != 'H') {
+		ret = -EINVAL;
+		goto out;
+	}
 
+	if (_IOC_NR(cmd) > HIDIOCTL_LAST || _IOC_NR(cmd) == 0) {
 		ret = -ENOTTY;
+		goto out;
 	}
+
+	ret = hidraw_fixed_size_ioctl(file, dev, cmd, user_arg);
+	if (ret != -EAGAIN)
+		goto out;
+
+	switch (_IOC_DIR(cmd)) {
+	case (_IOC_READ | _IOC_WRITE):
+		ret = hidraw_rw_variable_size_ioctl(file, dev, cmd, user_arg);
+		break;
+	case _IOC_READ:
+		ret = hidraw_ro_variable_size_ioctl(file, dev, cmd, user_arg);
+		break;
+	default:
+		/* Any other IOC_DIR is wrong */
+		ret = -EINVAL;
+	}
+
 out:
 	up_read(&minors_rwsem);
 	return ret;
diff --git a/include/uapi/linux/hidraw.h b/include/uapi/linux/hidraw.h
index d5ee269864e07fcaba481fa285bacbd98739e44f..ebd701b3c18d9d7465880199091933f13f2e1128 100644
--- a/include/uapi/linux/hidraw.h
+++ b/include/uapi/linux/hidraw.h
@@ -48,6 +48,8 @@ struct hidraw_devinfo {
 #define HIDIOCGOUTPUT(len)    _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x0C, len)
 #define HIDIOCREVOKE	      _IOW('H', 0x0D, int) /* Revoke device access */
 
+#define HIDIOCTL_LAST		_IOC_NR(HIDIOCREVOKE)
+
 #define HIDRAW_FIRST_MINOR 0
 #define HIDRAW_MAX_DEVICES 64
 /* number of reports to buffer */

-- 
2.51.0


^ permalink raw reply related

* Re: [PATCH 1/2] hid: intel-thc-hid: intel-quicki2c: Add WCL Device IDs
From: srinivas pandruvada @ 2025-08-26 13:35 UTC (permalink / raw)
  To: Xinpeng Sun, jikos, bentiss; +Cc: linux-input, linux-kernel
In-Reply-To: <20250826072701.991046-1-xinpeng.sun@intel.com>

On Tue, 2025-08-26 at 15:27 +0800, Xinpeng Sun wrote:

Not even a single line of description?

Thanks,
Srinivas

> Signed-off-by: Xinpeng Sun <xinpeng.sun@intel.com>
> ---
>  drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c | 2 ++
>  drivers/hid/intel-thc-hid/intel-quicki2c/quicki2c-dev.h | 2 ++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
> b/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
> index f122fde879b9..17b1f2df8f8a 100644
> --- a/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
> +++ b/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
> @@ -1019,6 +1019,8 @@ static const struct pci_device_id
> quicki2c_pci_tbl[] = {
>  	{ PCI_DEVICE_DATA(INTEL, THC_PTL_H_DEVICE_ID_I2C_PORT2,
> &ptl_ddata) },
>  	{ PCI_DEVICE_DATA(INTEL, THC_PTL_U_DEVICE_ID_I2C_PORT1,
> &ptl_ddata) },
>  	{ PCI_DEVICE_DATA(INTEL, THC_PTL_U_DEVICE_ID_I2C_PORT2,
> &ptl_ddata) },
> +	{ PCI_DEVICE_DATA(INTEL, THC_WCL_DEVICE_ID_I2C_PORT1,
> &ptl_ddata) },
> +	{ PCI_DEVICE_DATA(INTEL, THC_WCL_DEVICE_ID_I2C_PORT2,
> &ptl_ddata) },
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(pci, quicki2c_pci_tbl);
> diff --git a/drivers/hid/intel-thc-hid/intel-quicki2c/quicki2c-dev.h
> b/drivers/hid/intel-thc-hid/intel-quicki2c/quicki2c-dev.h
> index b78c8864d39e..240492a38c24 100644
> --- a/drivers/hid/intel-thc-hid/intel-quicki2c/quicki2c-dev.h
> +++ b/drivers/hid/intel-thc-hid/intel-quicki2c/quicki2c-dev.h
> @@ -13,6 +13,8 @@
>  #define
> PCI_DEVICE_ID_INTEL_THC_PTL_H_DEVICE_ID_I2C_PORT2	0xE34A
>  #define
> PCI_DEVICE_ID_INTEL_THC_PTL_U_DEVICE_ID_I2C_PORT1	0xE448
>  #define
> PCI_DEVICE_ID_INTEL_THC_PTL_U_DEVICE_ID_I2C_PORT2	0xE44A
> +#define
> PCI_DEVICE_ID_INTEL_THC_WCL_DEVICE_ID_I2C_PORT1 	0x4D48
> +#define
> PCI_DEVICE_ID_INTEL_THC_WCL_DEVICE_ID_I2C_PORT2 	0x4D4A
>  
>  /* Packet size value, the unit is 16 bytes */
>  #define MAX_PACKET_SIZE_VALUE_LNL			256


^ permalink raw reply

* [PATCH v2 0/3] mfd: tps6594: add power button and power-off
From: Michael Walle @ 2025-08-26 13:46 UTC (permalink / raw)
  To: Dmitry Torokhov, Lee Jones
  Cc: jcormier, Job Sava, linux-kernel, linux-input, Michael Walle

I took over the series from [1] since the original developer was an
intern and is no longer with their former company.

Changelog is in the individual patches. But the most prominent
change is that the pin mux config is now read from the chip itself
instead of having a DT property.

[1] https://lore.kernel.org/all/20250520-linux-stable-tps6594-pwrbutton-v1-0-0cc5c6e0415c@criticallink.com/

Job Sava (1):
  input: tps6594-pwrbutton: Add power button functionality

Michael Walle (2):
  mfd: tps6594: add power button functionality
  mfd: tps6594: Add board power-off support

 drivers/input/misc/Kconfig             |  10 ++
 drivers/input/misc/Makefile            |   1 +
 drivers/input/misc/tps6594-pwrbutton.c | 126 +++++++++++++++++++++++++
 drivers/mfd/tps6594-core.c             |  58 +++++++++++-
 4 files changed, 193 insertions(+), 2 deletions(-)
 create mode 100644 drivers/input/misc/tps6594-pwrbutton.c

-- 
2.39.5


^ permalink raw reply

* [PATCH v2 1/3] input: tps6594-pwrbutton: Add power button functionality
From: Michael Walle @ 2025-08-26 13:46 UTC (permalink / raw)
  To: Dmitry Torokhov, Lee Jones
  Cc: jcormier, Job Sava, linux-kernel, linux-input, Michael Walle
In-Reply-To: <20250826134631.1499936-1-mwalle@kernel.org>

From: Job Sava <jsava@criticallink.com>

TPS6594 defines two interrupts for the power button one for push and
one for release.

This driver is very simple in that it maps the push interrupt to a key
input and the release interrupt to a key release.

Signed-off-by: Job Sava <jsava@criticallink.com>
Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Michael Walle <mwalle@kernel.org>
---
v2:
 - moved the mfd part into a seperate patch
---
 drivers/input/misc/Kconfig             |  10 ++
 drivers/input/misc/Makefile            |   1 +
 drivers/input/misc/tps6594-pwrbutton.c | 126 +++++++++++++++++++++++++
 3 files changed, 137 insertions(+)
 create mode 100644 drivers/input/misc/tps6594-pwrbutton.c

diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 0fb21c99a5e3..cce51358242f 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -506,6 +506,16 @@ config INPUT_TPS65219_PWRBUTTON
 	  To compile this driver as a module, choose M here. The module will
 	  be called tps65219-pwrbutton.
 
+config INPUT_TPS6594_PWRBUTTON
+	tristate "TPS6594 Power button driver"
+	depends on MFD_TPS6594
+	help
+	  Say Y here if you want to enable power button reporting for
+	  TPS6594 Power Management IC devices.
+
+	  To compile this driver as a module, choose M here. The module will
+	  be called tps6594-pwrbutton.
+
 config INPUT_AXP20X_PEK
 	tristate "X-Powers AXP20X power button driver"
 	depends on MFD_AXP20X
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index d468c8140b93..bb12f883851c 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -83,6 +83,7 @@ obj-$(CONFIG_INPUT_SPARCSPKR)		+= sparcspkr.o
 obj-$(CONFIG_INPUT_STPMIC1_ONKEY)  	+= stpmic1_onkey.o
 obj-$(CONFIG_INPUT_TPS65218_PWRBUTTON)	+= tps65218-pwrbutton.o
 obj-$(CONFIG_INPUT_TPS65219_PWRBUTTON)	+= tps65219-pwrbutton.o
+obj-$(CONFIG_INPUT_TPS6594_PWRBUTTON)	+= tps6594-pwrbutton.o
 obj-$(CONFIG_INPUT_TWL4030_PWRBUTTON)	+= twl4030-pwrbutton.o
 obj-$(CONFIG_INPUT_TWL4030_VIBRA)	+= twl4030-vibra.o
 obj-$(CONFIG_INPUT_TWL6040_VIBRA)	+= twl6040-vibra.o
diff --git a/drivers/input/misc/tps6594-pwrbutton.c b/drivers/input/misc/tps6594-pwrbutton.c
new file mode 100644
index 000000000000..cd039b3866dc
--- /dev/null
+++ b/drivers/input/misc/tps6594-pwrbutton.c
@@ -0,0 +1,126 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * power button driver for TI TPS6594 PMICs
+ *
+ * Copyright (C) 2025 Critical Link LLC - https://www.criticallink.com/
+ */
+#include <linux/init.h>
+#include <linux/input.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/mfd/tps6594.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+
+struct tps6594_pwrbutton {
+	struct device *dev;
+	struct input_dev *idev;
+	char phys[32];
+};
+
+static irqreturn_t tps6594_pb_push_irq(int irq, void *_pwr)
+{
+	struct tps6594_pwrbutton *pwr = _pwr;
+
+	input_report_key(pwr->idev, KEY_POWER, 1);
+	pm_wakeup_event(pwr->dev, 0);
+	input_sync(pwr->idev);
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t tps6594_pb_release_irq(int irq, void *_pwr)
+{
+	struct tps6594_pwrbutton *pwr = _pwr;
+
+	input_report_key(pwr->idev, KEY_POWER, 0);
+	input_sync(pwr->idev);
+
+	return IRQ_HANDLED;
+}
+
+static int tps6594_pb_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct tps6594_pwrbutton *pwr;
+	struct input_dev *idev;
+	int error;
+	int push_irq;
+	int release_irq;
+
+	pwr = devm_kzalloc(dev, sizeof(*pwr), GFP_KERNEL);
+	if (!pwr)
+		return -ENOMEM;
+
+	idev = devm_input_allocate_device(dev);
+	if (!idev)
+		return -ENOMEM;
+
+	idev->name = pdev->name;
+	snprintf(pwr->phys, sizeof(pwr->phys), "%s/input0",
+		 pdev->name);
+	idev->phys = pwr->phys;
+	idev->id.bustype = BUS_I2C;
+
+	input_set_capability(idev, EV_KEY, KEY_POWER);
+
+	pwr->dev = dev;
+	pwr->idev = idev;
+	device_init_wakeup(dev, true);
+
+	push_irq = platform_get_irq(pdev, 0);
+	if (push_irq < 0)
+		return -EINVAL;
+
+	release_irq = platform_get_irq(pdev, 1);
+	if (release_irq < 0)
+		return -EINVAL;
+
+	error = devm_request_threaded_irq(dev, push_irq, NULL,
+					  tps6594_pb_push_irq,
+					  IRQF_ONESHOT,
+					  pdev->resource[0].name, pwr);
+	if (error) {
+		dev_err(dev, "failed to request push IRQ #%d: %d\n", push_irq,
+			error);
+		return error;
+	}
+
+	error = devm_request_threaded_irq(dev, release_irq, NULL,
+					  tps6594_pb_release_irq,
+					  IRQF_ONESHOT,
+					  pdev->resource[1].name, pwr);
+	if (error) {
+		dev_err(dev, "failed to request release IRQ #%d: %d\n",
+			release_irq, error);
+		return error;
+	}
+
+	error = input_register_device(idev);
+	if (error) {
+		dev_err(dev, "Can't register power button: %d\n", error);
+		return error;
+	}
+
+	return 0;
+}
+
+static const struct platform_device_id tps6594_pwrbtn_id_table[] = {
+	{ "tps6594-pwrbutton", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(platform, tps6594_pwrbtn_id_table);
+
+static struct platform_driver tps6594_pb_driver = {
+	.probe = tps6594_pb_probe,
+	.driver = {
+		.name = "tps6594_pwrbutton",
+	},
+	.id_table = tps6594_pwrbtn_id_table,
+};
+module_platform_driver(tps6594_pb_driver);
+
+MODULE_DESCRIPTION("TPS6594 Power Button");
+MODULE_LICENSE("GPL");
-- 
2.39.5


^ permalink raw reply related

* [PATCH v2 2/3] mfd: tps6594: add power button functionality
From: Michael Walle @ 2025-08-26 13:46 UTC (permalink / raw)
  To: Dmitry Torokhov, Lee Jones
  Cc: jcormier, Job Sava, linux-kernel, linux-input, Michael Walle
In-Reply-To: <20250826134631.1499936-1-mwalle@kernel.org>

The PMIC has a multi-function pin PB/EN/VSENSE. If it is configured as
push-button (PB), add the corresponding device for it.

Co-developed-by: Job Sava <jsava@criticallink.com>
Signed-off-by: Job Sava <jsava@criticallink.com>
Signed-off-by: Michael Walle <mwalle@kernel.org>
---
v2:
 - new patch, these bits were previously part of another patch
 - don't use "ti,power-button" to decide whether to add the power-button
   device, but read the configuration register of the pin, whose default
   value is stored in the OTP memory of the chip
---
 drivers/mfd/tps6594-core.c | 38 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 36 insertions(+), 2 deletions(-)

diff --git a/drivers/mfd/tps6594-core.c b/drivers/mfd/tps6594-core.c
index c16c37e36617..9195c9059489 100644
--- a/drivers/mfd/tps6594-core.c
+++ b/drivers/mfd/tps6594-core.c
@@ -20,6 +20,8 @@
 #include <linux/mfd/tps6594.h>
 
 #define TPS6594_CRC_SYNC_TIMEOUT_MS 150
+#define TPS65224_EN_SEL_PB 1
+#define TPS65224_GPIO3_SEL_PB 3
 
 /* Completion to synchronize CRC feature enabling on all PMICs */
 static DECLARE_COMPLETION(tps6594_crc_comp);
@@ -128,6 +130,12 @@ static const struct resource tps6594_rtc_resources[] = {
 	DEFINE_RES_IRQ_NAMED(TPS6594_IRQ_POWER_UP, TPS6594_IRQ_NAME_POWERUP),
 };
 
+static const struct resource tps6594_pwrbutton_resources[] = {
+	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_PB_FALL, TPS65224_IRQ_NAME_PB_FALL),
+	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_PB_RISE, TPS65224_IRQ_NAME_PB_RISE),
+	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_PB_SHORT, TPS65224_IRQ_NAME_PB_SHORT),
+};
+
 static const struct mfd_cell tps6594_common_cells[] = {
 	MFD_CELL_RES("tps6594-regulator", tps6594_regulator_resources),
 	MFD_CELL_RES("tps6594-pinctrl", tps6594_pinctrl_resources),
@@ -318,8 +326,6 @@ static const struct resource tps65224_pfsm_resources[] = {
 	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_REG_UNLOCK, TPS65224_IRQ_NAME_REG_UNLOCK),
 	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_TWARN, TPS65224_IRQ_NAME_TWARN),
 	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_PB_LONG, TPS65224_IRQ_NAME_PB_LONG),
-	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_PB_FALL, TPS65224_IRQ_NAME_PB_FALL),
-	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_PB_RISE, TPS65224_IRQ_NAME_PB_RISE),
 	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_TSD_ORD, TPS65224_IRQ_NAME_TSD_ORD),
 	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_BIST_FAIL, TPS65224_IRQ_NAME_BIST_FAIL),
 	DEFINE_RES_IRQ_NAMED(TPS65224_IRQ_REG_CRC_ERR, TPS65224_IRQ_NAME_REG_CRC_ERR),
@@ -347,6 +353,12 @@ static const struct mfd_cell tps65224_common_cells[] = {
 	MFD_CELL_RES("tps6594-regulator", tps65224_regulator_resources),
 };
 
+static const struct mfd_cell tps6594_pwrbutton_cell = {
+	.name = "tps6594-pwrbutton",
+	.resources = tps6594_pwrbutton_resources,
+	.num_resources = ARRAY_SIZE(tps6594_pwrbutton_resources),
+};
+
 static const struct regmap_irq tps65224_irqs[] = {
 	/* INT_BUCK register */
 	REGMAP_IRQ_REG(TPS65224_IRQ_BUCK1_UVOV, 0, TPS65224_BIT_BUCK1_UVOV_INT),
@@ -681,6 +693,7 @@ int tps6594_device_init(struct tps6594 *tps, bool enable_crc)
 	struct device *dev = tps->dev;
 	int ret;
 	struct regmap_irq_chip *irq_chip;
+	unsigned int pwr_on, gpio3_cfg;
 	const struct mfd_cell *cells;
 	int n_cells;
 
@@ -727,6 +740,27 @@ int tps6594_device_init(struct tps6594 *tps, bool enable_crc)
 	if (ret)
 		return dev_err_probe(dev, ret, "Failed to add common child devices\n");
 
+	/* If either the PB/EN/VSENSE or GPIO3 is configured as PB, register a driver for it */
+	if (tps->chip_id == TPS65224 || tps->chip_id == TPS652G1) {
+		ret = regmap_read(tps->regmap, TPS6594_REG_NPWRON_CONF, &pwr_on);
+		if (ret)
+			return dev_err_probe(dev, ret, "Failed to read PB/EN/VSENSE config\n");
+
+		ret = regmap_read(tps->regmap, TPS6594_REG_GPIOX_CONF(2), &gpio3_cfg);
+		if (ret)
+			return dev_err_probe(dev, ret, "Failed to read GPIO3 config\n");
+
+		if (FIELD_GET(TPS65224_MASK_EN_PB_VSENSE_CONFIG, pwr_on) == TPS65224_EN_SEL_PB ||
+		    FIELD_GET(TPS65224_MASK_GPIO_SEL, gpio3_cfg) == TPS65224_GPIO3_SEL_PB) {
+			ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO,
+						   &tps6594_pwrbutton_cell, 1, NULL, 0,
+						   regmap_irq_get_domain(tps->irq_data));
+			if (ret)
+				return dev_err_probe(dev, ret,
+						     "Failed to add power button device.\n");
+		}
+	}
+
 	/* No RTC for LP8764, TPS65224 and TPS652G1 */
 	if (tps->chip_id != LP8764 && tps->chip_id != TPS65224 && tps->chip_id != TPS652G1) {
 		ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO, tps6594_rtc_cells,
-- 
2.39.5


^ permalink raw reply related

* [PATCH v2 3/3] mfd: tps6594: Add board power-off support
From: Michael Walle @ 2025-08-26 13:46 UTC (permalink / raw)
  To: Dmitry Torokhov, Lee Jones
  Cc: jcormier, Job Sava, linux-kernel, linux-input, Michael Walle
In-Reply-To: <20250826134631.1499936-1-mwalle@kernel.org>

Add a system level power-off handler if the "system-power-controller"
flag is set for this device in the device tree.

A power-off request is triggered by writing the TRIGGER_I2C_0 bit (which
is actually just a convention and really depends on the freely
programmable FSM).

Co-developed-by: Job Sava <jsava@criticallink.com>
Signed-off-by: Job Sava <jsava@criticallink.com>
Signed-off-by: Michael Walle <mwalle@kernel.org>
---
v2:
 - incoroprate feedback from Lee Jones.
 - use of_device_is_system_power_controller() instead of open coding it
 - handle errors in power_off handler
---
 drivers/mfd/tps6594-core.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/mfd/tps6594-core.c b/drivers/mfd/tps6594-core.c
index 9195c9059489..7127af7142f5 100644
--- a/drivers/mfd/tps6594-core.c
+++ b/drivers/mfd/tps6594-core.c
@@ -15,6 +15,7 @@
 #include <linux/interrupt.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/reboot.h>
 
 #include <linux/mfd/core.h>
 #include <linux/mfd/tps6594.h>
@@ -688,6 +689,19 @@ static int tps6594_enable_crc(struct tps6594 *tps)
 	return ret;
 }
 
+static int tps6594_power_off_handler(struct sys_off_data *data)
+{
+	struct tps6594 *tps = data->cb_data;
+	int ret;
+
+	ret = regmap_update_bits(tps->regmap, TPS6594_REG_FSM_I2C_TRIGGERS,
+				 TPS6594_BIT_TRIGGER_I2C(0), TPS6594_BIT_TRIGGER_I2C(0));
+	if (ret)
+		return notifier_from_errno(ret);
+
+	return NOTIFY_DONE;
+}
+
 int tps6594_device_init(struct tps6594 *tps, bool enable_crc)
 {
 	struct device *dev = tps->dev;
@@ -770,6 +784,12 @@ int tps6594_device_init(struct tps6594 *tps, bool enable_crc)
 			return dev_err_probe(dev, ret, "Failed to add RTC child device\n");
 	}
 
+	if (of_device_is_system_power_controller(dev->of_node)) {
+		ret = devm_register_power_off_handler(tps->dev, tps6594_power_off_handler, tps);
+		if (ret)
+			return dev_err_probe(dev, ret, "Failed to register power-off handler\n");
+	}
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(tps6594_device_init);
-- 
2.39.5


^ permalink raw reply related

* Re: [PATCH 1/3] Input: mtk-pmic-keys - MT6359 has a specific release irq
From: Julien Massot @ 2025-08-26 13:51 UTC (permalink / raw)
  To: Dmitry Torokhov, AngeloGioacchino Del Regno
  Cc: kernel, Matthias Brugger, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, linux-input, linux-kernel, linux-arm-kernel,
	linux-mediatek, devicetree
In-Reply-To: <akavphbw2wh5vga3eoe6abqdawdssko4cy2sz7oedef4jvzunn@37whg62gmfoh>

Hi Dmitry

On Wed, 2025-08-06 at 09:49 -0700, Dmitry Torokhov wrote:
> On Mon, Aug 04, 2025 at 10:05:21AM +0200, AngeloGioacchino Del Regno
> wrote:
> > Il 01/08/25 15:16, Julien Massot ha scritto:
> > > A recent commit in linux-next added support for key events.
> > > However, the key release event is not properly handled: only key
> > > press events
> > > are generated, leaving key states stuck in "pressed".
> > > 
> > > This patch ensures that both key press and key release events are
> > > properly
> > > emitted by handling the release logic correctly.
> > > 
> > > Note: the code was introduced in linux-next by commit
> > > bc25e6bf032e ("Input: mtk-pmic-keys - add support for MT6359 PMIC
> > > keys")
> > > and is not yet present in mainline.
> > > 
> > > Signed-off-by: Julien Massot <julien.massot@collabora.com>
> > 
> > Well, you are effectively fixing the commit that you pointed out, so
> > this needs
> > 
> > Fixes: bc25e6bf032e ("Input: mtk-pmic-keys - add support for MT6359
> > PMIC keys")
> > 
> > and
> > 
> > Reviewed-by: AngeloGioacchino Del Regno
> > <angelogioacchino.delregno@collabora.com>
> 
> I am really interested in how exactly this was developed, tested, and
> reviewed...
> 
> Thanks.

You are right, the issue comes from my side.
I prepared the patch against a slightly different tree, which led to the
discrepancy. I’ll respin it on top of the current linux-next and re-test
it properly to ensure the fix behaves as expected.

Thanks for catching this, and thanks for your time.
Julien

^ permalink raw reply

* [PATCH v2 0/3] Radxa NIO 12L: Add GPIO keys and LED support
From: Julien Massot @ 2025-08-26 14:01 UTC (permalink / raw)
  To: kernel, Dmitry Torokhov, Matthias Brugger,
	AngeloGioacchino Del Regno, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Louis-Alexis Eyraud
  Cc: linux-input, linux-kernel, linux-arm-kernel, linux-mediatek,
	devicetree, Julien Massot

This patchset adds support for the GPIO-connected red and blue LEDs,
as well as the various hardware buttons present on the Radxa NIO 12L
board.

It also includes a fix for the missing release (key-up) interrupt
handling for PMIC-managed GPIO keys.

Signed-off-by: Julien Massot <julien.massot@collabora.com>
---
Changes in v2:
PATCH 1/3
- Add Fixes tag
- Drop Angelo's Reviewed-By since I'm now introducing the
'key_release_irq' member that was missing in v1.
- Link to v1: https://lore.kernel.org/r/20250801-radxa-nio-12-l-gpio-v1-0-d0840f85d2c8@collabora.com

---
Julien Massot (3):
      Input: mtk-pmic-keys - MT6359 has a specific release irq
      arm64: dts: mediatek: mt8395-nio-12l: add PMIC and GPIO keys support
      arm64: dts: mediatek: mt8395-nio-12l: add support for blue and red LEDs

 .../boot/dts/mediatek/mt8395-radxa-nio-12l.dts     | 67 ++++++++++++++++++++++
 drivers/input/keyboard/mtk-pmic-keys.c             |  5 +-
 2 files changed, 71 insertions(+), 1 deletion(-)
---
base-commit: 6c68f4c0a147c025ae0b25fab688c7c47964a02f
change-id: 20250801-radxa-nio-12-l-gpio-54f208c25333

Best regards,
-- 
Julien Massot <julien.massot@collabora.com>


^ permalink raw reply

* [PATCH v2 1/3] Input: mtk-pmic-keys - MT6359 has a specific release irq
From: Julien Massot @ 2025-08-26 14:01 UTC (permalink / raw)
  To: kernel, Dmitry Torokhov, Matthias Brugger,
	AngeloGioacchino Del Regno, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Louis-Alexis Eyraud
  Cc: linux-input, linux-kernel, linux-arm-kernel, linux-mediatek,
	devicetree, Julien Massot
In-Reply-To: <20250826-radxa-nio-12-l-gpio-v2-0-7f18fa3fbfc8@collabora.com>

A recent commit in linux-next added support for key events.
However, the key release event is not properly handled:
only key press events are generated, leaving key states
stuck in "pressed".

This patch ensures that both key press and key release events
are properly emitted by handling the release logic correctly.

Introduce a 'key_release_irq' member to the 'mtk_pmic_regs',
to identify the devices that have a separate irq for the
release event.

Fixes: bc25e6bf032e ("Input: mtk-pmic-keys - add support for MT6359 PMIC keys")
Signed-off-by: Julien Massot <julien.massot@collabora.com>
---
 drivers/input/keyboard/mtk-pmic-keys.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/input/keyboard/mtk-pmic-keys.c b/drivers/input/keyboard/mtk-pmic-keys.c
index 50e2e792c91d2626d3f282d04a1db60845827ef2..c78d9f6d97c4f7a77b4c44cee3b93e4712a19aec 100644
--- a/drivers/input/keyboard/mtk-pmic-keys.c
+++ b/drivers/input/keyboard/mtk-pmic-keys.c
@@ -55,6 +55,7 @@ struct mtk_pmic_regs {
 	const struct mtk_pmic_keys_regs keys_regs[MTK_PMIC_MAX_KEY_COUNT];
 	u32 pmic_rst_reg;
 	u32 rst_lprst_mask; /* Long-press reset timeout bitmask */
+	bool key_release_irq;
 };
 
 static const struct mtk_pmic_regs mt6397_regs = {
@@ -116,6 +117,7 @@ static const struct mtk_pmic_regs mt6358_regs = {
 				   MTK_PMIC_HOMEKEY_RST),
 	.pmic_rst_reg = MT6358_TOP_RST_MISC,
 	.rst_lprst_mask = MTK_PMIC_RST_DU_MASK,
+	.key_release_irq = true,
 };
 
 static const struct mtk_pmic_regs mt6359_regs = {
@@ -129,6 +131,7 @@ static const struct mtk_pmic_regs mt6359_regs = {
 				   MTK_PMIC_HOMEKEY_RST),
 	.pmic_rst_reg = MT6359_TOP_RST_MISC,
 	.rst_lprst_mask = MTK_PMIC_RST_DU_MASK,
+	.key_release_irq = true,
 };
 
 struct mtk_pmic_keys_info {
@@ -368,7 +371,7 @@ static int mtk_pmic_keys_probe(struct platform_device *pdev)
 		if (keys->keys[index].irq < 0)
 			return keys->keys[index].irq;
 
-		if (of_device_is_compatible(node, "mediatek,mt6358-keys")) {
+		if (mtk_pmic_regs->key_release_irq) {
 			keys->keys[index].irq_r = platform_get_irq_byname(pdev,
 									  irqnames_r[index]);
 

-- 
2.50.1


^ permalink raw reply related

* [PATCH v2 2/3] arm64: dts: mediatek: mt8395-nio-12l: add PMIC and GPIO keys support
From: Julien Massot @ 2025-08-26 14:01 UTC (permalink / raw)
  To: kernel, Dmitry Torokhov, Matthias Brugger,
	AngeloGioacchino Del Regno, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Louis-Alexis Eyraud
  Cc: linux-input, linux-kernel, linux-arm-kernel, linux-mediatek,
	devicetree, Julien Massot
In-Reply-To: <20250826-radxa-nio-12-l-gpio-v2-0-7f18fa3fbfc8@collabora.com>

Add support for PMIC and GPIO keys on the Radxa NIO 12L board:
Declare a gpio-keys node for the Volume Up button using GPIO106.
Add the corresponding pin configuration in the pinctrl node.
Add a mediatek,mt6359-keys subnode under the PMIC to handle the
power and home buttons exposed by the MT6359.

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Signed-off-by: Julien Massot <julien.massot@collabora.com>
---
 .../boot/dts/mediatek/mt8395-radxa-nio-12l.dts     | 36 ++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8395-radxa-nio-12l.dts b/arch/arm64/boot/dts/mediatek/mt8395-radxa-nio-12l.dts
index 329c60cc6a6be0b4be8c0b8bb033b32d35302804..fd596e2298285361ad7c2fb828feec598d75a73e 100644
--- a/arch/arm64/boot/dts/mediatek/mt8395-radxa-nio-12l.dts
+++ b/arch/arm64/boot/dts/mediatek/mt8395-radxa-nio-12l.dts
@@ -8,6 +8,7 @@
 #include "mt8195.dtsi"
 #include "mt6359.dtsi"
 #include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/input.h>
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/pinctrl/mt8195-pinfunc.h>
 #include <dt-bindings/regulator/mediatek,mt6360-regulator.h>
@@ -60,6 +61,18 @@ backlight: backlight {
 		status = "disabled";
 	};
 
+	keys: gpio-keys {
+		compatible = "gpio-keys";
+
+		button-volume-up {
+			wakeup-source;
+			debounce-interval = <100>;
+			gpios = <&pio 106 GPIO_ACTIVE_LOW>;
+			label = "volume_up";
+			linux,code = <KEY_VOLUMEUP>;
+		};
+	};
+
 	wifi_vreg: regulator-wifi-3v3-en {
 		compatible = "regulator-fixed";
 		regulator-name = "wifi_3v3_en";
@@ -626,6 +639,14 @@ pins-txd {
 		};
 	};
 
+	gpio_key_pins: gpio-keys-pins {
+		pins {
+			pinmux = <PINMUX_GPIO106__FUNC_GPIO106>;
+			bias-pull-up;
+			input-enable;
+		};
+	};
+
 	i2c2_pins: i2c2-pins {
 		pins-bus {
 			pinmux = <PINMUX_GPIO12__FUNC_SDA2>,
@@ -880,6 +901,21 @@ &pciephy {
 
 &pmic {
 	interrupts-extended = <&pio 222 IRQ_TYPE_LEVEL_HIGH>;
+
+	mt6359keys: keys {
+		compatible = "mediatek,mt6359-keys";
+		mediatek,long-press-mode = <1>;
+		power-off-time-sec = <0>;
+
+		power-key {
+			linux,keycodes = <KEY_POWER>;
+			wakeup-source;
+		};
+
+		home {
+			linux,keycodes = <KEY_HOME>;
+		};
+	};
 };
 
 &scp {

-- 
2.50.1


^ permalink raw reply related

* [PATCH v2 3/3] arm64: dts: mediatek: mt8395-nio-12l: add support for blue and red LEDs
From: Julien Massot @ 2025-08-26 14:01 UTC (permalink / raw)
  To: kernel, Dmitry Torokhov, Matthias Brugger,
	AngeloGioacchino Del Regno, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Louis-Alexis Eyraud
  Cc: linux-input, linux-kernel, linux-arm-kernel, linux-mediatek,
	devicetree, Julien Massot
In-Reply-To: <20250826-radxa-nio-12-l-gpio-v2-0-7f18fa3fbfc8@collabora.com>

The Radxa NIO 12L board has an RGB LED, of which only red and blue
are controllable.

Red and blue LEDs: no need to choose, both are enabled.

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Signed-off-by: Julien Massot <julien.massot@collabora.com>
---
 .../boot/dts/mediatek/mt8395-radxa-nio-12l.dts     | 31 ++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8395-radxa-nio-12l.dts b/arch/arm64/boot/dts/mediatek/mt8395-radxa-nio-12l.dts
index fd596e2298285361ad7c2fb828feec598d75a73e..12288ad4d2932b7f78c96c0efe366a046721f919 100644
--- a/arch/arm64/boot/dts/mediatek/mt8395-radxa-nio-12l.dts
+++ b/arch/arm64/boot/dts/mediatek/mt8395-radxa-nio-12l.dts
@@ -10,6 +10,7 @@
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/input/input.h>
 #include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/leds/common.h>
 #include <dt-bindings/pinctrl/mt8195-pinfunc.h>
 #include <dt-bindings/regulator/mediatek,mt6360-regulator.h>
 #include <dt-bindings/spmi/spmi.h>
@@ -73,6 +74,28 @@ button-volume-up {
 		};
 	};
 
+	gpio-leds {
+		compatible = "gpio-leds";
+		pinctrl-0 = <&gpio_leds_pins>;
+		pinctrl-names = "default";
+
+		/*
+		 * This board has a RGB LED, of which only R and B
+		 * are controllable.
+		 */
+		led-0 {
+			label = "rgb-blue";
+			color = <LED_COLOR_ID_BLUE>;
+			gpios = <&pio 6 GPIO_ACTIVE_HIGH>;
+		};
+
+		led-1 {
+			label = "rgb-red";
+			color = <LED_COLOR_ID_RED>;
+			gpios = <&pio 7 GPIO_ACTIVE_HIGH>;
+		};
+	};
+
 	wifi_vreg: regulator-wifi-3v3-en {
 		compatible = "regulator-fixed";
 		regulator-name = "wifi_3v3_en";
@@ -647,6 +670,14 @@ pins {
 		};
 	};
 
+	gpio_leds_pins: gpio-leds-pins {
+		pins {
+			pinmux = <PINMUX_GPIO6__FUNC_GPIO6>,
+				 <PINMUX_GPIO7__FUNC_GPIO7>;
+			output-low;
+		};
+	};
+
 	i2c2_pins: i2c2-pins {
 		pins-bus {
 			pinmux = <PINMUX_GPIO12__FUNC_SDA2>,

-- 
2.50.1


^ permalink raw reply related

* [PATCH] Input: i8042 - add TUXEDO InfinityBook Pro Gen10 AMD to i8042 quirk table
From: Werner Sembach @ 2025-08-26 14:26 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Christoffer Sandberg, Werner Sembach, linux-input, linux-kernel

From: Christoffer Sandberg <cs@tuxedo.de>

Occasionally wakes up from suspend with missing input on the internal
keyboard. Setting the quirks appears to fix the issue for this device as
well.

Signed-off-by: Christoffer Sandberg <cs@tuxedo.de>
Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
Cc: stable@vger.kernel.org
---
 drivers/input/serio/i8042-acpipnpio.h | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/input/serio/i8042-acpipnpio.h b/drivers/input/serio/i8042-acpipnpio.h
index 6ed9fc34948cb..1caa6c4ca435c 100644
--- a/drivers/input/serio/i8042-acpipnpio.h
+++ b/drivers/input/serio/i8042-acpipnpio.h
@@ -1155,6 +1155,20 @@ static const struct dmi_system_id i8042_dmi_quirk_table[] __initconst = {
 		.driver_data = (void *)(SERIO_QUIRK_NOMUX | SERIO_QUIRK_RESET_ALWAYS |
 					SERIO_QUIRK_NOLOOP | SERIO_QUIRK_NOPNP)
 	},
+	{
+		.matches = {
+			DMI_MATCH(DMI_BOARD_NAME, "XxHP4NAx"),
+		},
+		.driver_data = (void *)(SERIO_QUIRK_NOMUX | SERIO_QUIRK_RESET_ALWAYS |
+					SERIO_QUIRK_NOLOOP | SERIO_QUIRK_NOPNP)
+	},
+	{
+		.matches = {
+			DMI_MATCH(DMI_BOARD_NAME, "XxKK4NAx_XxSP4NAx"),
+		},
+		.driver_data = (void *)(SERIO_QUIRK_NOMUX | SERIO_QUIRK_RESET_ALWAYS |
+					SERIO_QUIRK_NOLOOP | SERIO_QUIRK_NOPNP)
+	},
 	/*
 	 * A lot of modern Clevo barebones have touchpad and/or keyboard issues
 	 * after suspend fixable with the forcenorestore quirk.
-- 
2.43.0


^ permalink raw reply related

* Re: [PATCH] dt-bindings: input: touchscreen: imagis: add missing minItems
From: Conor Dooley @ 2025-08-26 17:38 UTC (permalink / raw)
  To: Duje Mihanović
  Cc: Markuss Broks, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Karel Balej, David Wronek, phone-devel,
	~postmarketos/upstreaming, linux-input, devicetree, linux-kernel
In-Reply-To: <5917367.DvuYhMxLoT@radijator>

[-- Attachment #1: Type: text/plain, Size: 650 bytes --]

On Mon, Aug 25, 2025 at 08:57:57PM +0200, Duje Mihanović wrote:
> On Monday, 25 August 2025 18:42:38 Central European Summer Time Conor Dooley wrote:
> > On Sun, Aug 24, 2025 at 06:12:05PM +0200, Duje Mihanović wrote:
> > > The binding currently expects exactly 5 keycodes, which matches the
> > > chip's theoretical maximum but probably not the number of touch keys on
> > > any phone using the IST3032C. Add a minItems value of 2 to prevent
> > > dt-validate complaints.
> > 
> > Does this mean that there are devicetrees in the wild that use < 5
> > keycodes?
> 
> Indeed.

oke, Acked-by: Conor Dooley <conor.dooley@microchip.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply

* Re: [PATCH v2 5/9] dt-bindings: mfd: fsl,mc13xxx: convert txt to DT schema
From: Rob Herring @ 2025-08-26 23:03 UTC (permalink / raw)
  To: Alexander Kurz
  Cc: Lee Jones, Krzysztof Kozlowski, Conor Dooley, Dmitry Torokhov,
	Dzmitry Sankouski, Dr. David Alan Gilbert, Heiko Stuebner,
	Uwe Kleine-König, devicetree, linux-input, linux-kernel
In-Reply-To: <20250823144441.12654-6-akurz@blala.de>

On Sat, Aug 23, 2025 at 02:44:37PM +0000, Alexander Kurz wrote:
> Convert the txt mc13xxx bindings to DT schema attempting to keep most
> information. The nodes codec and touchscreen are not part of the new
> schema since it was only briefly mentioned before.
> Following the convention, rename led-control to fsl,led-control.

If 'led-control' is already in use, then you can't do that. You could 
support both, but then the driver has to support both forever which is 
worse than not matching convention.
> 
> Signed-off-by: Alexander Kurz <akurz@blala.de>
> ---
>  .../devicetree/bindings/mfd/fsl,mc13xxx.yaml  | 214 ++++++++++++++++++
>  .../devicetree/bindings/mfd/mc13xxx.txt       | 156 -------------
>  2 files changed, 214 insertions(+), 156 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/mfd/fsl,mc13xxx.yaml
>  delete mode 100644 Documentation/devicetree/bindings/mfd/mc13xxx.txt
> 
> diff --git a/Documentation/devicetree/bindings/mfd/fsl,mc13xxx.yaml b/Documentation/devicetree/bindings/mfd/fsl,mc13xxx.yaml
> new file mode 100644
> index 000000000000..94e2f6557376
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/fsl,mc13xxx.yaml
> @@ -0,0 +1,214 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/fsl,mc13xxx.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Freescale MC13xxx Power Management Integrated Circuits (PMIC)
> +
> +maintainers:
> +  - Alexander Kurz <akurz@blala.de>
> +
> +description:

Needs a '>' modifier to preserve formatting.

> +  The MC13xxx PMIC series consists of the three models MC13783, MC13892
> +  and MC34708 and provide regulators and other features like RTC, ADC,
> +  LED, touchscreen, codec and input buttons.
> +
> +  Link to datasheets
> +    https://www.nxp.com/docs/en/data-sheet/MC13783.pdf
> +    https://www.nxp.com/docs/en/data-sheet/MC13892.pdf
> +    https://www.nxp.com/docs/en/data-sheet/MC34708.pdf
> +
> +properties:
> +  compatible:
> +    enum:
> +      - fsl,mc13783
> +      - fsl,mc13892
> +      - fsl,mc34708
> +
> +  reg:
> +    description: I2C slave address or SPI chip select number.
> +    maxItems: 1
> +
> +  spi-max-frequency: true
> +
> +  spi-cs-high: true
> +
> +  system-power-controller: true
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  leds:
> +    type: object
> +    $ref: /schemas/leds/common.yaml#
> +    description: |
> +      Leds

Drop the description

blank line

> +    properties:
> +      reg:
> +        description: |
> +          One of
> +          MC13783 LED IDs
> +            0: Main display
> +            1: AUX display
> +            2: Keypad
> +            3: Red 1
> +            4: Green 1
> +            5: Blue 1
> +            6: Red 2
> +            7: Green 2
> +            8: Blue 2
> +            9: Red 3
> +            10: Green 3
> +            11: Blue 3

blank line

> +          MC13892 LED IDs
> +            0: Main display
> +            1: AUX display
> +            2: Keypad
> +            3: Red
> +            4: Green
> +            5: Blue

blank line

> +          MC34708 LED IDs
> +            0: Charger Red
> +            1: Charger Green
> +        maxItems: 1

blank line

> +      fsl,led-control:
> +        $ref: /schemas/types.yaml#/definitions/uint32-array
> +        description: |
> +          Setting for LED-Control register array length depends on model,
> +          mc13783: 6, mc13892: 4, mc34708: 1
> +
> +  regulators:
> +    type: object
> +    $ref: /schemas/regulator/regulator.yaml#

This schema applies to the child nodes, not this node. Drop.

       additionalProperties:
         type: object


> +    description: |
> +      List of child nodes specifying the regulators, depending on chip variant:
> +      * MC13783: gpo[1-4], pwgt[12]spi, sw[12][ab], sw3, vaudio, vcam, vdig,
> +      vesim, vgen, viohi, violo, vmmc[12], vrf[12], vrfbg, vrfcp, vrfdig,
> +      vrfref, vsim and vvib.
> +      * MC13892: gpo[1-4], pwgt[12]spi, sw[1-4], swbst, vaudio, vcam, vcoincell,
> +      vdig, vgen[1-3], viohi, vpll, vsd, vusb, vusb2, vvideo.
> +      Each child node is defined using the standard binding for regulators and
> +      the optional regulator properties defined below.

Don't duplicate what the schema says below in free-form text.

> +
> +  fsl,mc13xxx-uses-adc:
> +    type: boolean
> +    description: Indicate the ADC is being used
> +
> +  fsl,mc13xxx-uses-codec:
> +    type: boolean
> +    description: Indicate the Audio Codec is being used
> +
> +  fsl,mc13xxx-uses-rtc:
> +    type: boolean
> +    description: Indicate the RTC is being used
> +
> +  fsl,mc13xxx-uses-touch:
> +    type: boolean
> +    description: Indicate the touchscreen controller is being used
> +
> +required:
> +  - compatible
> +  - reg
> +
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: fsl,mc13783
> +    then:
> +      properties:
> +        leds:
> +          properties:
> +            fsl,led-control:
> +              minItems: 6
> +              maxItems: 6
> +        regulators:
> +          patternProperties:
> +            "^gpo[1-4]|pwgt[12]spi|sw[12][ab]|sw3|vaudio|vcam|vdig|vesim|vgen|viohi|violo|vmmc[12]|vrf[12]|vrfbg|vrfcp|vrfdig|vrfref|vsim|vvib$":
> +              type: object
> +              $ref: /schemas/regulator/regulator.yaml#

                 unevaluatedProperties: false

> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: fsl,mc13892
> +    then:
> +      properties:
> +        leds:
> +          properties:
> +            fsl,led-control:
> +              minItems: 4
> +              maxItems: 4
> +        regulators:
> +          patternProperties:
> +            "^gpo[1-4]|pwgt[12]spi|sw[1-4]|swbst|vaudio|vcam|vcoincell|vdig|vgen[1-3]|viohi|vpll|vsd|vusb|vusb2|vvideo$":
> +              type: object
> +              $ref: /schemas/regulator/regulator.yaml#

                 unevaluatedProperties: false

> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: fsl,mc34708
> +    then:
> +      properties:
> +        leds:
> +          properties:
> +            fsl,led-control:
> +              minItems: 1
> +              maxItems: 1
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/leds/common.h>
> +
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        pmic: mc13892@0 {
> +            compatible = "fsl,mc13892";
> +            reg = <0>;
> +            spi-max-frequency = <1000000>;
> +            spi-cs-high;
> +            interrupt-parent = <&gpio0>;
> +            interrupts = <8 IRQ_TYPE_LEVEL_HIGH>;
> +            fsl,mc13xxx-uses-rtc;
> +            fsl,mc13xxx-uses-adc;
> +
> +            leds {
> +                #address-cells = <1>;
> +                #size-cells = <0>;
> +                fsl,led-control = <0x000 0x000 0x0e0 0x000>;
> +
> +                sysled@3 {
> +                    reg = <3>;
> +                    label = "system:red:live";
> +                    linux,default-trigger = "heartbeat";
> +                };
> +            };
> +
> +            regulators {
> +                sw1_reg: sw1 {
> +                    regulator-min-microvolt = <600000>;
> +                    regulator-max-microvolt = <1375000>;
> +                    regulator-boot-on;
> +                    regulator-always-on;
> +                };
> +
> +                sw2_reg: sw2 {
> +                    regulator-min-microvolt = <900000>;
> +                    regulator-max-microvolt = <1850000>;
> +                    regulator-boot-on;
> +                    regulator-always-on;
> +                };
> +            };
> +        };
> +    };

^ permalink raw reply

* Re: [PATCH v2 6/9] dt-bindings: mfd: fsl,mc13xxx: add buttons node
From: Rob Herring @ 2025-08-26 23:08 UTC (permalink / raw)
  To: Alexander Kurz
  Cc: Lee Jones, Krzysztof Kozlowski, Conor Dooley, Dmitry Torokhov,
	Dzmitry Sankouski, Dr. David Alan Gilbert, Heiko Stuebner,
	Uwe Kleine-König, devicetree, linux-input, linux-kernel
In-Reply-To: <20250823144441.12654-7-akurz@blala.de>

On Sat, Aug 23, 2025 at 02:44:38PM +0000, Alexander Kurz wrote:
> Add a buttons node and properties describing the "ONOFD" (MC13783) and
> "PWRON" (MC13892/MC34708) buttons available in the fsl,mc13xxx PMIC ICs.
> 
> Signed-off-by: Alexander Kurz <akurz@blala.de>
> ---
>  .../devicetree/bindings/mfd/fsl,mc13xxx.yaml  | 58 +++++++++++++++++++
>  1 file changed, 58 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/fsl,mc13xxx.yaml b/Documentation/devicetree/bindings/mfd/fsl,mc13xxx.yaml
> index 94e2f6557376..761267b42c85 100644
> --- a/Documentation/devicetree/bindings/mfd/fsl,mc13xxx.yaml
> +++ b/Documentation/devicetree/bindings/mfd/fsl,mc13xxx.yaml
> @@ -39,6 +39,41 @@ properties:
>    interrupts:
>      maxItems: 1
>  
> +  buttons:
> +    type: object
> +    $ref: /schemas/input/input.yaml#

       unevaluatedProperties: false

(And then fix the errors in the example)

> +    description: Buttons

Drop.

> +    properties:
> +      reg:
> +        description: |
> +          One of
> +          MC13783 BUTTON IDs:
> +            0: ONOFD1
> +            1: ONOFD2
> +            2: ONOFD3
> +          MC13892 BUTTON IDs:
> +            0: PWRON1
> +            1: PWRON2
> +            2: PWRON3
> +          MC34708 BUTTON IDs:
> +            0: PWRON1
> +            1: PWRON2

'maximum: 2' here and then only need 'maximum: 1' in one spot below.

> +
> +      debounce-delay-ms:
> +        enum: [0, 30, 150, 750]
> +        default: 30
> +        description: |
> +          Sets the debouncing delay in milliseconds.
> +          Valid values: 0, 30, 150 and 750ms.

Don't repeat schema constraints in free-form text.

> +
> +      active-low:
> +        description: Set active when pin is pulled low.
> +
> +      fsl,enable-reset:
> +        description: |

Don't need '|'.

> +          Setting of the global reset option.
> +        type: boolean
> +
>    leds:
>      type: object
>      $ref: /schemas/leds/common.yaml#
> @@ -119,6 +154,10 @@ allOf:
>              const: fsl,mc13783
>      then:
>        properties:
> +        buttons:
> +          properties:
> +            reg:
> +              enum: [0, 1, 2]
>          leds:
>            properties:
>              fsl,led-control:
> @@ -137,6 +176,10 @@ allOf:
>              const: fsl,mc13892
>      then:
>        properties:
> +        buttons:
> +          properties:
> +            reg:
> +              enum: [0, 1, 2]
>          leds:
>            properties:
>              fsl,led-control:
> @@ -155,6 +198,10 @@ allOf:
>              const: fsl,mc34708
>      then:
>        properties:
> +        buttons:
> +          properties:
> +            reg:
> +              enum: [0, 1]
>          leds:
>            properties:
>              fsl,led-control:
> @@ -183,6 +230,17 @@ examples:
>              fsl,mc13xxx-uses-rtc;
>              fsl,mc13xxx-uses-adc;
>  
> +            buttons {
> +                #address-cells = <1>;
> +                #size-cells = <0>;
> +                onkey1@0 {
> +                    reg = <0>;
> +                    debounce-delay-ms = <30>;
> +                    active-low;
> +                    fsl,enable-reset;
> +                };
> +            };
> +
>              leds {
>                  #address-cells = <1>;
>                  #size-cells = <0>;
> -- 
> 2.39.5
> 

^ permalink raw reply

* RE: [PATCH 1/2] hid: intel-thc-hid: intel-quicki2c: Add WCL Device IDs
From: Sun, Xinpeng @ 2025-08-27  1:24 UTC (permalink / raw)
  To: srinivas pandruvada, jikos@kernel.org, bentiss@kernel.org
  Cc: linux-input@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <e783088816bdf9ab95b1becde94b6b7bf2a2e93f.camel@linux.intel.com>


> -----Original Message-----
> From: srinivas pandruvada <srinivas.pandruvada@linux.intel.com>
> Sent: Tuesday, August 26, 2025 9:35 PM
> To: Sun, Xinpeng <xinpeng.sun@intel.com>; jikos@kernel.org; bentiss@kernel.org
> Cc: linux-input@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 1/2] hid: intel-thc-hid: intel-quicki2c: Add WCL Device IDs
> 
> On Tue, 2025-08-26 at 15:27 +0800, Xinpeng Sun wrote:
> 
> Not even a single line of description?

Will add a simple description in next version.

Thanks,
Xinpeng

> 
> Thanks,
> Srinivas
> 
> > Signed-off-by: Xinpeng Sun <xinpeng.sun@intel.com>
> > ---
> >  drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c | 2 ++
> >  drivers/hid/intel-thc-hid/intel-quicki2c/quicki2c-dev.h | 2 ++
> >  2 files changed, 4 insertions(+)
> >
> > diff --git a/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
> > b/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
> > index f122fde879b9..17b1f2df8f8a 100644
> > --- a/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
> > +++ b/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
> > @@ -1019,6 +1019,8 @@ static const struct pci_device_id
> > quicki2c_pci_tbl[] = {
> >  	{ PCI_DEVICE_DATA(INTEL, THC_PTL_H_DEVICE_ID_I2C_PORT2,
> > &ptl_ddata) },
> >  	{ PCI_DEVICE_DATA(INTEL, THC_PTL_U_DEVICE_ID_I2C_PORT1,
> > &ptl_ddata) },
> >  	{ PCI_DEVICE_DATA(INTEL, THC_PTL_U_DEVICE_ID_I2C_PORT2,
> > &ptl_ddata) },
> > +	{ PCI_DEVICE_DATA(INTEL, THC_WCL_DEVICE_ID_I2C_PORT1,
> > &ptl_ddata) },
> > +	{ PCI_DEVICE_DATA(INTEL, THC_WCL_DEVICE_ID_I2C_PORT2,
> > &ptl_ddata) },
> >  	{ }
> >  };
> >  MODULE_DEVICE_TABLE(pci, quicki2c_pci_tbl); diff --git
> > a/drivers/hid/intel-thc-hid/intel-quicki2c/quicki2c-dev.h
> > b/drivers/hid/intel-thc-hid/intel-quicki2c/quicki2c-dev.h
> > index b78c8864d39e..240492a38c24 100644
> > --- a/drivers/hid/intel-thc-hid/intel-quicki2c/quicki2c-dev.h
> > +++ b/drivers/hid/intel-thc-hid/intel-quicki2c/quicki2c-dev.h
> > @@ -13,6 +13,8 @@
> >  #define
> > PCI_DEVICE_ID_INTEL_THC_PTL_H_DEVICE_ID_I2C_PORT2	0xE34A
> >  #define
> > PCI_DEVICE_ID_INTEL_THC_PTL_U_DEVICE_ID_I2C_PORT1	0xE448
> >  #define
> > PCI_DEVICE_ID_INTEL_THC_PTL_U_DEVICE_ID_I2C_PORT2	0xE44A
> > +#define
> > PCI_DEVICE_ID_INTEL_THC_WCL_DEVICE_ID_I2C_PORT1 	0x4D48
> > +#define
> > PCI_DEVICE_ID_INTEL_THC_WCL_DEVICE_ID_I2C_PORT2 	0x4D4A
> >
> >  /* Packet size value, the unit is 16 bytes */
> >  #define MAX_PACKET_SIZE_VALUE_LNL			256


^ permalink raw reply

* Re: [PATCH v2 00/11] HID: playstation: Add support for audio jack handling on DualSense
From: Cristian Ciocaltea @ 2025-08-27  6:36 UTC (permalink / raw)
  To: Roderick Colenbrander
  Cc: Roderick Colenbrander, Jiri Kosina, Benjamin Tissoires,
	Henrik Rydberg, kernel, linux-input, linux-kernel
In-Reply-To: <8f7242f0-c217-47e4-ad88-fc1481ca936f@collabora.com>

On 7/23/25 10:17 AM, Cristian Ciocaltea wrote:
> Hi Roderick,
> 
> On 7/23/25 7:04 AM, Roderick Colenbrander wrote:
>> Hi Cristian,
>>
>> Thanks for the information on the audio patches in the sound tree. We
>> weren't familiar with that part.
> 
> I've actually mentioned that in the cover letter and changelog of this
> revision.  Couldn't do this previously because I submitted the HID series
> before the USB audio one.
> 
>> I talked a bit with my team members as well. In general audio is
>> getting some bigger attention (will see where that goes). I'm getting
>> a bit worried that the HID and usb driver need much closer coupling,
>> the current coupling not being enough.
> 
> I think we should keep things simple for now, at least until we land the
> basic support, also considering the USB part has been already merged.
> 
>> I don't know the USB audio spec too well, but it is more on the
>> digital interface and a DAC. I'm not sure on the exact circuitry on
>> the DualSense, but there is a lot of logic in the console drivers for
>> volume handling where adjustment of the volume talks to the HID layer
>> to send a new output report. I suspect they had very good reasons for
>> it (e.g. for headphone also dealing with different impedances).
>>
>> So I'm not sure how the volume control is really supposed to work, but
>> I would think to do it properly it requires some interaction between
>> the audio and HID drivers. Just letting the audio side do it right
>> now, is more about leveraging the range of the DAC I guess versus a
>> proper audio amplification stage.
> 
> Indeed, it's not possible to support hardware volume control from
> ALSA/usb-audio without involving HID.  This could be done, or at least it's
> worth investigating further, but it's not mandatory and definitely beyond
> the scope of the current work.
> 
>> Just thinking of things from the user perspective, they should have a
>> unified volume control. I don't know how other devices are doing it,
>> but I think we need to think a bit further and we need to reconsider
>> how things work....
> 
> There's a bit more complexity in here than initially anticipated, but the
> (software) volume control is not really a problem.  It's worth noting I am
> going to provide some UCM changes, part of the ALSA project:
> 
>   https://github.com/alsa-project/alsa-ucm-conf
> 
> This is to ensure proper support for audio profile switching between
> headphones/headset and internal speaker/microphone, which also addresses
> a few volume control related issues.  Those are mainly caused by the haptic
> feedback functionality, which is controlled by a pair of dedicated channels
> in a quadraphonic audio stream.  One of the UCM's main jobs is to split the
> 4.0 PCM stream into 4 mono channels or a pair of stereo (FL+FR) channels,
> depending on the active output device/profile.
> 
> The only blocker now is this HID series, which prevents us moving further.
> 
> Therefore, unless there is anything else remaining which requires urgent
> attention, could you please provide an ack for Jiri to be able to pick this
> up?

It's been over a month now since this was kind of blocked without any clear
reason, and by the end of next week I'll be on leave, which means we're
close to missing the merge window once again.

Considering the counterpart quirk in the generic USB audio driver has been
already merged since v6.17, I kindly ask for your support in getting this
into v6.18.

Thanks,
Cristian


^ permalink raw reply

* [PATCH 2/2 v2] selftests: input: Add tracepoint testing script
From: Morduan Zang @ 2025-08-27  7:21 UTC (permalink / raw)
  To: dmitry.torokhov
  Cc: linux-input, linux-kernel, liziyao, WangYuli, Morduan Zang
In-Reply-To: <20250827072125.139887-1-zhangdandan@uniontech.com>

From: WangYuli <wangyuli@uniontech.com>

Add a test script for the input subsystem tracepoints.

The script provides:
 - Automatic detection of available input tracepoints
 - Privilege and filesystem mount validation
 - Interactive testing with real-time trace monitoring
 - Support for both batch and live trace analysis

The script enables easy validation and demonstration of the input
tracepoint functionality, making it easier for developers to verify
the implementation and understand the tracepoint capabilities.

Usage:
  sudo ./tools/testing/selftests/input/test_input_tracepoints.sh

Signed-off-by: WangYuli <wangyuli@uniontech.com>
Signed-off-by: Morduan Zang <zhangdandan@uniontech.com>
---
 MAINTAINERS                                   |  2 +
 .../selftests/input/test_input_tracepoints.sh | 78 +++++++++++++++++++
 2 files changed, 80 insertions(+)
 create mode 100755 tools/testing/selftests/input/test_input_tracepoints.sh

diff --git a/MAINTAINERS b/MAINTAINERS
index 4dfa2d60faa0..f6e5da714ba9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12099,6 +12099,7 @@ F:	include/uapi/linux/input-event-codes.h
 F:	include/uapi/linux/input.h
 F:	include/uapi/linux/serio.h
 F:	include/uapi/linux/uinput.h
+F:	tools/testing/selftests/input/
 
 INPUT MULTITOUCH (MT) PROTOCOL
 M:	Henrik Rydberg <rydberg@bitmath.org>
@@ -25568,6 +25569,7 @@ F:	kernel/trace/
 F:	kernel/tracepoint.c
 F:	scripts/tracing/
 F:	tools/testing/selftests/ftrace/
+F:	tools/testing/selftests/input/test_input_tracepoints.sh
 
 TRACING MMIO ACCESSES (MMIOTRACE)
 M:	Steven Rostedt <rostedt@goodmis.org>
diff --git a/tools/testing/selftests/input/test_input_tracepoints.sh b/tools/testing/selftests/input/test_input_tracepoints.sh
new file mode 100755
index 000000000000..6ade2619b62d
--- /dev/null
+++ b/tools/testing/selftests/input/test_input_tracepoints.sh
@@ -0,0 +1,78 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+###############################################################################
+#
+# Input subsystem tracepoint testing script
+#
+# AUTHOR
+#      WangYuli <wangyuli@uniontech.com>
+#
+###############################################################################
+
+DEBUGFS_PATH="/sys/kernel/debug/tracing"
+INPUT_EVENTS_PATH="${DEBUGFS_PATH}/events/input"
+
+# Check if we have sufficient privileges
+if [ ! -w "$DEBUGFS_PATH" ]; then
+    echo "Error: Root privileges required to access tracing system"
+    echo "Please run: sudo $0"
+    exit 1
+fi
+
+# Check if debugfs is mounted
+if [ ! -d "$DEBUGFS_PATH" ]; then
+    echo "Error: debugfs is not mounted"
+    echo "Please run: mount -t debugfs none /sys/kernel/debug"
+    exit 1
+fi
+
+# Check if input tracepoints exist
+if [ ! -d "$INPUT_EVENTS_PATH" ]; then
+    echo "Error: input tracepoints not found, kernel may need to be recompiled"
+    exit 1
+fi
+
+echo "=== Input Subsystem Tracepoint Test ==="
+echo
+
+# Clear existing trace buffer
+echo > "${DEBUGFS_PATH}/trace"
+
+# List available input tracepoints
+echo "Available Input Tracepoints:"
+for event in "${INPUT_EVENTS_PATH}"/*; do
+    if [ -d "$event" ]; then
+        event_name=$(basename "$event")
+        echo "  - $event_name"
+    fi
+done
+echo
+
+# Enable all input tracepoints
+echo "Enabling all input tracepoints..."
+echo 1 > "${INPUT_EVENTS_PATH}/enable"
+
+if [ $? -eq 0 ]; then
+    echo "✓ Successfully enabled input tracepoints"
+else
+    echo "✗ Failed to enable input tracepoints"
+    exit 1
+fi
+
+echo
+echo "Please perform some operations in another terminal (keyboard input, mouse movement, etc.)"
+echo "or plug/unplug USB devices, then come back to view the results..."
+echo
+echo "Press any key to continue viewing trace output (press Ctrl+C to exit)..."
+read -n 1
+
+echo
+echo "=== Trace Output ==="
+echo "(Last 100 lines)"
+tail -n 100 "${DEBUGFS_PATH}/trace"
+
+echo
+echo "=== Real-time Trace Output ==="
+echo "(Press Ctrl+C to stop)"
+cat "${DEBUGFS_PATH}/trace_pipe"
-- 
2.50.1


^ permalink raw reply related

* [PATCH 1/2 v2] input: Add tracepoint support
From: Morduan Zang @ 2025-08-27  7:21 UTC (permalink / raw)
  To: dmitry.torokhov
  Cc: linux-input, linux-kernel, liziyao, WangYuli, Winston Wen,
	Morduan Zang

From: WangYuli <wangyuli@uniontech.com>

Introduce a set of tracepoints for the input subsystem to enable
better debugging, monitoring, and performance analysis of input
device operations.

This tracing event while be default to close.
When you open events of tracing, the input trcaing event with be
opened by "echo 1 > /sys/kernel/tracing/events/enabled", However,
the tracking status of each sub-event is independent of each other.

Suggested-by: Winston Wen <wentao@uniontech.com>
Signed-off-by: WangYuli <wangyuli@uniontech.com>
Signed-off-by: Morduan Zang <zhangdandan@uniontech.com>
---
 MAINTAINERS                  |   1 +
 drivers/input/input.c        |  18 ++-
 include/trace/events/input.h | 263 +++++++++++++++++++++++++++++++++++
 3 files changed, 281 insertions(+), 1 deletion(-)
 create mode 100644 include/trace/events/input.h

diff --git a/MAINTAINERS b/MAINTAINERS
index daf520a13bdf..4dfa2d60faa0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12092,6 +12092,7 @@ F:	include/linux/i8042.h
 F:	include/linux/input.h
 F:	include/linux/input/
 F:	include/linux/libps2.h
+F:	include/trace/events/input.h
 F:	include/linux/serio.h
 F:	include/uapi/linux/gameport.h
 F:	include/uapi/linux/input-event-codes.h
diff --git a/drivers/input/input.c b/drivers/input/input.c
index 1da41324362b..1c5c7056f040 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -29,6 +29,9 @@
 #include "input-core-private.h"
 #include "input-poller.h"
 
+#define CREATE_TRACE_POINTS
+#include <trace/events/input.h>
+
 MODULE_AUTHOR("Vojtech Pavlik <vojtech@suse.cz>");
 MODULE_DESCRIPTION("Input core");
 MODULE_LICENSE("GPL");
@@ -363,6 +366,7 @@ void input_handle_event(struct input_dev *dev,
 
 	disposition = input_get_disposition(dev, type, code, &value);
 	if (disposition != INPUT_IGNORE_EVENT) {
+		trace_input_event(dev, type, code, value);
 		if (type != EV_SYN)
 			add_input_randomness(type, code, value);
 
@@ -525,6 +529,7 @@ int input_grab_device(struct input_handle *handle)
 			return -EBUSY;
 
 		rcu_assign_pointer(dev->grab, handle);
+		trace_input_grab_device(dev, handle);
 	}
 
 	return 0;
@@ -539,6 +544,7 @@ static void __input_release_device(struct input_handle *handle)
 	grabber = rcu_dereference_protected(dev->grab,
 					    lockdep_is_held(&dev->mutex));
 	if (grabber == handle) {
+		trace_input_release_device(dev, handle);
 		rcu_assign_pointer(dev->grab, NULL);
 		/* Make sure input_pass_values() notices that grab is gone */
 		synchronize_rcu();
@@ -612,6 +618,7 @@ int input_open_device(struct input_handle *handle)
 
 		if (dev->poller)
 			input_dev_poller_start(dev->poller);
+		trace_input_open_device(dev);
 	}
 
 	return 0;
@@ -652,6 +659,7 @@ void input_close_device(struct input_handle *handle)
 				input_dev_poller_stop(dev->poller);
 			if (dev->close)
 				dev->close(dev);
+			trace_input_close_device(dev);
 		}
 	}
 
@@ -991,9 +999,13 @@ static int input_attach_handler(struct input_dev *dev, struct input_handler *han
 		return -ENODEV;
 
 	error = handler->connect(handler, dev, id);
-	if (error && error != -ENODEV)
+	if (error && error != -ENODEV) {
 		pr_err("failed to attach handler %s to device %s, error: %d\n",
 		       handler->name, kobject_name(&dev->dev.kobj), error);
+	} else if (!error) {
+		/* Connection successful, trace it */
+		trace_input_handler_connect(handler, dev, handler->minor);
+	}
 
 	return error;
 }
@@ -1791,6 +1803,7 @@ static int input_inhibit_device(struct input_dev *dev)
 	}
 
 	dev->inhibited = true;
+	trace_input_inhibit_device(dev);
 
 	return 0;
 }
@@ -1818,6 +1831,7 @@ static int input_uninhibit_device(struct input_dev *dev)
 
 	scoped_guard(spinlock_irq, &dev->event_lock)
 		input_dev_toggle(dev, true);
+	trace_input_uninhibit_device(dev);
 
 	return 0;
 }
@@ -2216,6 +2230,7 @@ static void __input_unregister_device(struct input_dev *dev)
 {
 	struct input_handle *handle, *next;
 
+	trace_input_device_unregister(dev);
 	input_disconnect_device(dev);
 
 	scoped_guard(mutex, &input_mutex) {
@@ -2414,6 +2429,7 @@ int input_register_device(struct input_dev *dev)
 		input_wakeup_procfs_readers();
 	}
 
+	trace_input_device_register(dev);
 	if (dev->devres_managed) {
 		dev_dbg(dev->dev.parent, "%s: registering %s with devres.\n",
 			__func__, dev_name(&dev->dev));
diff --git a/include/trace/events/input.h b/include/trace/events/input.h
new file mode 100644
index 000000000000..e01c8b5c61f7
--- /dev/null
+++ b/include/trace/events/input.h
@@ -0,0 +1,263 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* input tracepoints
+ *
+ * Copyright (C) 2025 WangYuli <wangyuli@uniontech.com>
+ */
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM input
+
+#if !defined(_TRACE_INPUT_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_INPUT_H
+
+#include <linux/tracepoint.h>
+#include <linux/input.h>
+
+/**
+ * input_event - called when an input event is processed
+ * @dev: input device that generated the event
+ * @type: event type (EV_KEY, EV_REL, EV_ABS, etc.)
+ * @code: event code within the type
+ * @value: event value
+ *
+ * This tracepoint fires for every input event processed by the input core.
+ * It can be used to monitor input device activity and debug input issues.
+ */
+TRACE_EVENT(
+	input_event,
+
+	TP_PROTO(struct input_dev *dev, unsigned int type, unsigned int code,
+		 int value),
+
+	TP_ARGS(dev, type, code, value),
+
+	TP_STRUCT__entry(__string(name, dev->name ?: "unknown")
+		__field(unsigned int, type)
+		__field(unsigned int, code)
+		__field(int, value)
+		__field(u16, bustype)
+		__field(u16, vendor)
+		__field(u16, product)),
+
+	TP_fast_assign(__assign_str(name); __entry->type = type;
+		__entry->code = code; __entry->value = value;
+		__entry->bustype = dev->id.bustype;
+		__entry->vendor = dev->id.vendor;
+		__entry->product = dev->id.product;),
+
+	TP_printk(
+		"dev=%s type=%u code=%u value=%d bus=%04x vendor=%04x product=%04x",
+		__get_str(name), __entry->type, __entry->code, __entry->value,
+		__entry->bustype, __entry->vendor, __entry->product));
+
+/**
+ * input_device_register - called when an input device is registered
+ * @dev: input device being registered
+ *
+ * This tracepoint fires when a new input device is registered with the
+ * input subsystem. Useful for monitoring device hotplug events.
+ */
+TRACE_EVENT(
+	input_device_register,
+
+	TP_PROTO(struct input_dev *dev),
+
+	TP_ARGS(dev),
+
+	TP_STRUCT__entry(__string(name, dev->name ?: "unknown")
+		__string(phys, dev->phys ?: "")
+		__field(u16, bustype)
+		__field(u16, vendor)
+		__field(u16, product)
+		__field(u16, version)),
+
+	TP_fast_assign(__assign_str(name); __assign_str(phys);
+		__entry->bustype = dev->id.bustype;
+		__entry->vendor = dev->id.vendor;
+		__entry->product = dev->id.product;
+		__entry->version = dev->id.version;),
+
+	TP_printk(
+		"dev=%s phys=%s bus=%04x vendor=%04x product=%04x version=%04x",
+		__get_str(name), __get_str(phys), __entry->bustype,
+		__entry->vendor, __entry->product, __entry->version));
+
+/**
+ * input_device_unregister - called when an input device is unregistered
+ * @dev: input device being unregistered
+ *
+ * This tracepoint fires when an input device is unregistered from the
+ * input subsystem. Useful for monitoring device hotplug events.
+ */
+TRACE_EVENT(input_device_unregister,
+
+	    TP_PROTO(struct input_dev *dev),
+
+	    TP_ARGS(dev),
+
+	    TP_STRUCT__entry(__string(name, dev->name ?: "unknown")
+		    __string(phys, dev->phys ?: "")
+		    __field(u16, bustype)
+		    __field(u16, vendor)
+		    __field(u16, product)),
+
+	    TP_fast_assign(__assign_str(name);
+		    __assign_str(phys);
+		    __entry->bustype = dev->id.bustype;
+		    __entry->vendor = dev->id.vendor;
+		    __entry->product = dev->id.product;),
+
+	    TP_printk("dev=%s phys=%s bus=%04x vendor=%04x product=%04",
+		    __get_str(name), __get_str(phys), __entry->bustype,
+		    __entry->vendor, __entry->product));
+
+/**
+ * input_handler_connect - called when an input handler connects to a device
+ * @handler: input handler
+ * @dev: input device
+ * @minor: assigned minor number (if applicable)
+ *
+ * This tracepoint fires when an input handler (like evdev, mousedev) connects
+ * to an input device, creating a new input handle.
+ */
+TRACE_EVENT(input_handler_connect,
+
+	    TP_PROTO(struct input_handler *handler, struct input_dev *dev,
+		     int minor),
+
+	    TP_ARGS(handler, dev, minor),
+
+	    TP_STRUCT__entry(__string(handler_name, handler->name)
+		    __string(dev_name, dev->name ?: "unknown")
+		    __field(int, minor)),
+
+	    TP_fast_assign(__assign_str(handler_name);
+		    __assign_str(dev_name);
+		    __entry->minor = minor;),
+
+	    TP_printk("handler=%s dev=%s minor=%d",
+		    __get_str(handler_name),
+		    __get_str(dev_name),
+		    __entry->minor));
+
+/**
+ * input_grab_device - called when a device is grabbed by a handler
+ * @dev: input device being grabbed
+ * @handle: input handle doing the grab
+ *
+ * This tracepoint fires when an input device is grabbed exclusively
+ * by an input handler, typically for security or special processing.
+ */
+TRACE_EVENT(input_grab_device,
+
+	    TP_PROTO(struct input_dev *dev, struct input_handle *handle),
+
+	    TP_ARGS(dev, handle),
+
+	    TP_STRUCT__entry(__string(dev_name, dev->name ?: "unknown")
+		    __string(handler_name, handle->handler->name)),
+
+	    TP_fast_assign(__assign_str(dev_name);
+		    __assign_str(handler_name);),
+
+	    TP_printk("dev=%s grabbed_by=%s",
+		    __get_str(dev_name),
+		    __get_str(handler_name)));
+
+/**
+ * input_release_device - called when a grabbed device is released
+ * @dev: input device being released
+ * @handle: input handle releasing the grab
+ *
+ * This tracepoint fires when an input device grab is released.
+ */
+TRACE_EVENT(input_release_device,
+
+	    TP_PROTO(struct input_dev *dev, struct input_handle *handle),
+
+	    TP_ARGS(dev, handle),
+
+	    TP_STRUCT__entry(__string(dev_name, dev->name ?: "unknown")
+		    __string(handler_name, handle->handler->name)),
+
+	    TP_fast_assign(__assign_str(dev_name);
+		    __assign_str(handler_name);),
+
+	    TP_printk("dev=%s released_by=%s",
+		    __get_str(dev_name),
+		    __get_str(handler_name)));
+
+DECLARE_EVENT_CLASS(input_device_state,
+		
+		TP_PROTO(struct input_dev *dev),
+		
+		TP_ARGS(dev),
+		
+		TP_STRUCT__entry(__string(name, dev->name ?: "unknown")
+			__field(unsigned int, users)
+			__field(bool, inhibited)),
+		
+		TP_fast_assign(__assign_str(name);
+			__entry->users = dev->users;
+			__entry->inhibited = dev->inhibited;),
+		
+		TP_printk("dev=%s users=%u inhibited=%s",
+			__get_str(name),
+			__entry->users,
+			__entry->inhibited ? "true" : "false"));
+
+/**
+ * input_open_device - called when an input device is opened
+ * @dev: input device being opened
+ *
+ * This tracepoint fires when the user count for an input device increases,
+ * typically when a userspace application opens the device.
+ */
+DEFINE_EVENT(input_device_state, input_open_device,
+
+	     TP_PROTO(struct input_dev *dev),
+
+	     TP_ARGS(dev));
+
+/**
+ * input_close_device - called when an input device is closed
+ * @dev: input device being closed
+ *
+ * This tracepoint fires when the user count for an input device decreases,
+ * typically when a userspace application closes the device.
+ */
+DEFINE_EVENT(input_device_state, input_close_device,
+
+	     TP_PROTO(struct input_dev *dev),
+
+	     TP_ARGS(dev));
+
+/**
+ * input_inhibit_device - called when an input device is inhibited
+ * @dev: input device being inhibited
+ *
+ * This tracepoint fires when an input device is inhibited (disabled),
+ * usually for power management or security reasons.
+ */
+DEFINE_EVENT(input_device_state, input_inhibit_device,
+
+	     TP_PROTO(struct input_dev *dev),
+
+	     TP_ARGS(dev));
+
+/**
+ * input_uninhibit_device - called when an input device is uninhibited
+ * @dev: input device being uninhibited
+ *
+ * This tracepoint fires when an input device is uninhibited (re-enabled)
+ * after being previously inhibited.
+ */
+DEFINE_EVENT(input_device_state, input_uninhibit_device,
+
+	     TP_PROTO(struct input_dev *dev),
+
+	     TP_ARGS(dev));
+
+#endif /* _TRACE_INPUT_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
-- 
2.50.1


^ permalink raw reply related

* Re: [PATCH v2 3/3] arm64: dts: mediatek: mt8395-nio-12l: add support for blue and red LEDs
From: Alexander Dahl @ 2025-08-27  7:40 UTC (permalink / raw)
  To: Julien Massot
  Cc: kernel, Dmitry Torokhov, Matthias Brugger,
	AngeloGioacchino Del Regno, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Louis-Alexis Eyraud, linux-input, linux-kernel,
	linux-arm-kernel, linux-mediatek, devicetree
In-Reply-To: <20250826-radxa-nio-12-l-gpio-v2-3-7f18fa3fbfc8@collabora.com>

Hello Julien,

Am Tue, Aug 26, 2025 at 04:01:54PM +0200 schrieb Julien Massot:
> The Radxa NIO 12L board has an RGB LED, of which only red and blue
> are controllable.
> 
> Red and blue LEDs: no need to choose, both are enabled.
> 
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> Signed-off-by: Julien Massot <julien.massot@collabora.com>
> ---
>  .../boot/dts/mediatek/mt8395-radxa-nio-12l.dts     | 31 ++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt8395-radxa-nio-12l.dts b/arch/arm64/boot/dts/mediatek/mt8395-radxa-nio-12l.dts
> index fd596e2298285361ad7c2fb828feec598d75a73e..12288ad4d2932b7f78c96c0efe366a046721f919 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8395-radxa-nio-12l.dts
> +++ b/arch/arm64/boot/dts/mediatek/mt8395-radxa-nio-12l.dts
> @@ -10,6 +10,7 @@
>  #include <dt-bindings/gpio/gpio.h>
>  #include <dt-bindings/input/input.h>
>  #include <dt-bindings/interrupt-controller/irq.h>
> +#include <dt-bindings/leds/common.h>
>  #include <dt-bindings/pinctrl/mt8195-pinfunc.h>
>  #include <dt-bindings/regulator/mediatek,mt6360-regulator.h>
>  #include <dt-bindings/spmi/spmi.h>
> @@ -73,6 +74,28 @@ button-volume-up {
>  		};
>  	};
>  
> +	gpio-leds {
> +		compatible = "gpio-leds";
> +		pinctrl-0 = <&gpio_leds_pins>;
> +		pinctrl-names = "default";
> +
> +		/*
> +		 * This board has a RGB LED, of which only R and B
> +		 * are controllable.
> +		 */
> +		led-0 {
> +			label = "rgb-blue";
> +			color = <LED_COLOR_ID_BLUE>;
> +			gpios = <&pio 6 GPIO_ACTIVE_HIGH>;
> +		};
> +
> +		led-1 {
> +			label = "rgb-red";
> +			color = <LED_COLOR_ID_RED>;
> +			gpios = <&pio 7 GPIO_ACTIVE_HIGH>;
> +		};

The label property is deprecated, and if I'm not mistaken not
recommended for new boards.  Do you have a reason to set it?
If so, it might be worth adding in the commit message.

Greets
Alex

> +	};
> +
>  	wifi_vreg: regulator-wifi-3v3-en {
>  		compatible = "regulator-fixed";
>  		regulator-name = "wifi_3v3_en";
> @@ -647,6 +670,14 @@ pins {
>  		};
>  	};
>  
> +	gpio_leds_pins: gpio-leds-pins {
> +		pins {
> +			pinmux = <PINMUX_GPIO6__FUNC_GPIO6>,
> +				 <PINMUX_GPIO7__FUNC_GPIO7>;
> +			output-low;
> +		};
> +	};
> +
>  	i2c2_pins: i2c2-pins {
>  		pins-bus {
>  			pinmux = <PINMUX_GPIO12__FUNC_SDA2>,
> 
> -- 
> 2.50.1
> 
> 

^ permalink raw reply

* Re: [PATCH v2 3/3] arm64: dts: mediatek: mt8395-nio-12l: add support for blue and red LEDs
From: Julien Massot @ 2025-08-27  9:04 UTC (permalink / raw)
  To: kernel, Dmitry Torokhov, Matthias Brugger,
	AngeloGioacchino Del Regno, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Louis-Alexis Eyraud, linux-input, linux-kernel,
	linux-arm-kernel, linux-mediatek, devicetree
In-Reply-To: <20250827-psychic-exclusive-be9758124693@thorsis.com>

Hi Alexander,

On Wed, 2025-08-27 at 09:40 +0200, Alexander Dahl wrote:
> Hello Julien,
> 
> Am Tue, Aug 26, 2025 at 04:01:54PM +0200 schrieb Julien Massot:
> > The Radxa NIO 12L board has an RGB LED, of which only red and blue
> > are controllable.
> > 
> > Red and blue LEDs: no need to choose, both are enabled.
> > 
> > Reviewed-by: AngeloGioacchino Del Regno
> > <angelogioacchino.delregno@collabora.com>
> > Signed-off-by: Julien Massot <julien.massot@collabora.com>
> > ---
> >  .../boot/dts/mediatek/mt8395-radxa-nio-12l.dts     | 31
> > ++++++++++++++++++++++
> >  1 file changed, 31 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8395-radxa-nio-12l.dts
> > b/arch/arm64/boot/dts/mediatek/mt8395-radxa-nio-12l.dts
> > index
> > fd596e2298285361ad7c2fb828feec598d75a73e..12288ad4d2932b7f78c96c0efe36
> > 6a046721f919 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt8395-radxa-nio-12l.dts
> > +++ b/arch/arm64/boot/dts/mediatek/mt8395-radxa-nio-12l.dts
> > @@ -10,6 +10,7 @@
> >  #include <dt-bindings/gpio/gpio.h>
> >  #include <dt-bindings/input/input.h>
> >  #include <dt-bindings/interrupt-controller/irq.h>
> > +#include <dt-bindings/leds/common.h>
> >  #include <dt-bindings/pinctrl/mt8195-pinfunc.h>
> >  #include <dt-bindings/regulator/mediatek,mt6360-regulator.h>
> >  #include <dt-bindings/spmi/spmi.h>
> > @@ -73,6 +74,28 @@ button-volume-up {
> >  		};
> >  	};
> >  
> > +	gpio-leds {
> > +		compatible = "gpio-leds";
> > +		pinctrl-0 = <&gpio_leds_pins>;
> > +		pinctrl-names = "default";
> > +
> > +		/*
> > +		 * This board has a RGB LED, of which only R and B
> > +		 * are controllable.
> > +		 */
> > +		led-0 {
> > +			label = "rgb-blue";
> > +			color = <LED_COLOR_ID_BLUE>;
> > +			gpios = <&pio 6 GPIO_ACTIVE_HIGH>;
> > +		};
> > +
> > +		led-1 {
> > +			label = "rgb-red";
> > +			color = <LED_COLOR_ID_RED>;
> > +			gpios = <&pio 7 GPIO_ACTIVE_HIGH>;
> > +		};
> 
> The label property is deprecated, and if I'm not mistaken not
> recommended for new boards.  Do you have a reason to set it?
> If so, it might be worth adding in the commit message.

No, I just wasn’t aware of the deprecation, but I can now see
it in the common LED binding.
I’ll wait a bit for any other potential reviews and then resend
the patch without the label. The LED will then appear as 'blue'
instead of 'rgb-blue' in sysfs.

Regards,
Julien

^ permalink raw reply

* [PATCH v2 00/11] mfd: macsmc: add rtc, hwmon and hid subdevices
From: James Calligeros @ 2025-08-27 11:22 UTC (permalink / raw)
  To: Sven Peter, Janne Grunau, Alyssa Rosenzweig, Neal Gompa,
	Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Alexandre Belloni, Jean Delvare, Guenter Roeck, Dmitry Torokhov
  Cc: asahi, linux-arm-kernel, devicetree, linux-kernel, linux-rtc,
	linux-hwmon, linux-input, James Calligeros, Mark Kettenis,
	Hector Martin

Hi all,

This series adds support for the remaining SMC subdevices. These are the
RTC, hwmon, and HID devices. They are being submitted together as the RTC
and hwmon drivers both require changes to the SMC DT schema.

The RTC driver is responsible for getting and setting the system clock,
and requires an NVMEM cell. This series replaces Sven's original RTC driver
submission [1].

The hwmon function is an interesting one. While each Apple Silicon device
exposes pretty similar sets of sensors, these all seem to be paired to
different SMC keys in the firmware interface. This is true even when the
sensors are on the SoC. For example, an M1 MacBook Pro will use different
keys to access the LITTLE core temperature sensors to an M1 Mac mini. This
necessitates describing which keys correspond to which sensors for each
device individually, and populating the hwmon structs at runtime. We do
this with a node in the device tree. This series includes only the keys
for sensors which we know to be common to all devices. The SMC is also
responsible for monitoring and controlling fan speeds on systems with fans,
which we expose via the hwmon driver.

The SMC also handles the hardware power button and lid switch. Power
button presses and lid opening/closing are emitted as HID events, so we
add a HID subdevice to handle them.

Note that this series is based on a branch with three additional commits
applied to add the parent SMC nodes to the relevant Devicetrees. This
was done to silence build errors. The series applies cleanly to 6.17-rc1.

Regards,

James

[1] https://lore.kernel.org/asahi/CAEg-Je84XxLWH7vznQmPRfjf6GxWOu75ZetwN7AdseAwfMLLrQ@mail.gmail.com/T/#t

Signed-off-by: James Calligeros <jcalligeros99@gmail.com>
---
Changes in v2:
- Added Rob's R-b tag to RTC DT binding
- Removed redundant nesting from hwmon DT binding
- Dedpulicated property definitions in hwmon DT schema
- Made label a required property for hwmon DT nodes
- Clarified semantics in hwmon DT schema definitions
- Split mfd tree changes into separate commits
- Fixed numerous style errors in hwmon driver
- Addressed Guenter's initial feedback on the hwmon driver
- Modified hwmon driver to reflect DT schema changes
- Added compatible property to hwmon node
- Link to v1: https://lore.kernel.org/r/20250819-macsmc-subdevs-v1-0-57df6c3e5f19@gmail.com

---
Hector Martin (2):
      rtc: Add new rtc-macsmc driver for Apple Silicon Macs
      input: macsmc-hid: New driver to handle the Apple Mac SMC buttons/lid

James Calligeros (7):
      dt-bindings: hwmon: Add Apple System Management Controller hwmon schema
      mfd: macsmc: Wire up Apple SMC RTC subdevice
      hwmon: Add Apple Silicon SMC hwmon driver
      mfd: macsmc: Wire up Apple SMC hwmon subdevice
      mfd: macsmc: Wire up Apple SMC HID subdevice
      arm64: dts: apple: Add common hwmon sensors and fans
      arm64: dts: apple: t8103, t600x, t8112: Add common hwmon nodes to devices

Sven Peter (2):
      dt-bindings: rtc: Add Apple SMC RTC
      arm64: dts: apple: t8103,t600x,t8112: Add SMC RTC node

 .../bindings/hwmon/apple,smc-hwmon.yaml  | 132 ++++
 .../bindings/mfd/apple,smc.yaml          |  45 ++
 .../bindings/rtc/apple,smc-rtc.yaml      |  35 +
 MAINTAINERS                              |   5 +
 .../boot/dts/apple/hwmon-common.dtsi     |  37 ++
 .../boot/dts/apple/hwmon-fan-dual.dtsi   |  24 +
 arch/arm64/boot/dts/apple/hwmon-fan.dtsi |  19 +
 .../boot/dts/apple/hwmon-laptop.dtsi     |  35 +
 .../boot/dts/apple/hwmon-mac-mini.dtsi   |  17 +
 .../arm64/boot/dts/apple/t6001-j375c.dts |   2 +
 .../arm64/boot/dts/apple/t6002-j375d.dts |   2 +
 .../arm64/boot/dts/apple/t600x-die0.dtsi |   6 +
 .../boot/dts/apple/t600x-j314-j316.dtsi  |   4 +
 .../arm64/boot/dts/apple/t600x-j375.dtsi |   2 +
 arch/arm64/boot/dts/apple/t8103-j274.dts |   2 +
 arch/arm64/boot/dts/apple/t8103-j293.dts |   3 +
 arch/arm64/boot/dts/apple/t8103-j313.dts |   2 +
 arch/arm64/boot/dts/apple/t8103-j456.dts |   2 +
 arch/arm64/boot/dts/apple/t8103-j457.dts |   2 +
 .../arm64/boot/dts/apple/t8103-jxxx.dtsi |   2 +
 arch/arm64/boot/dts/apple/t8103.dtsi     |   6 +
 arch/arm64/boot/dts/apple/t8112-j413.dts |   2 +
 arch/arm64/boot/dts/apple/t8112-j473.dts |   2 +
 arch/arm64/boot/dts/apple/t8112-j493.dts |   3 +
 .../arm64/boot/dts/apple/t8112-jxxx.dtsi |   2 +
 arch/arm64/boot/dts/apple/t8112.dtsi     |   6 +
 drivers/hwmon/Kconfig                    |  12 +
 drivers/hwmon/Makefile                   |   1 +
 drivers/hwmon/macsmc_hwmon.c             | 848 +++++++++++++++++++++++++
 drivers/input/misc/Kconfig               |  11 +
 drivers/input/misc/Makefile              |   1 +
 drivers/input/misc/macsmc-hid.c          | 209 ++++++
 drivers/mfd/macsmc.c                     |   3 +
 drivers/rtc/Kconfig                      |  11 +
 drivers/rtc/Makefile                     |   1 +
 drivers/rtc/rtc-macsmc.c                 | 141 ++++
 36 files changed, 1637 insertions(+)
---
base-commit: 876d6a70b24869f96ebc8672caf86cb4bae72927
change-id: 20250816-macsmc-subdevs-87032c017d0c

Best regards,
-- 
James Calligeros <jcalligeros99@gmail.com>


^ permalink raw reply

* [PATCH v2 01/11] dt-bindings: rtc: Add Apple SMC RTC
From: James Calligeros @ 2025-08-27 11:22 UTC (permalink / raw)
  To: Sven Peter, Janne Grunau, Alyssa Rosenzweig, Neal Gompa,
	Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Alexandre Belloni, Jean Delvare, Guenter Roeck, Dmitry Torokhov
  Cc: asahi, linux-arm-kernel, devicetree, linux-kernel, linux-rtc,
	linux-hwmon, linux-input, James Calligeros, Mark Kettenis
In-Reply-To: <20250827-macsmc-subdevs-v2-0-ce5e99d54c28@gmail.com>

From: Sven Peter <sven@kernel.org>

Apple Silicon Macs (M1, etc.) have an RTC that is part of the PMU IC,
but most of the PMU functionality is abstracted out by the SMC.
An additional RTC offset stored inside NVMEM is required to compute
the current date/time.

Reviewed-by: Mark Kettenis <kettenis@openbsd.org>
Reviewed-by: Neal Gompa <neal@gompa.dev>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Sven Peter <sven@kernel.org>
Signed-off-by: James Calligeros <jcalligeros99@gmail.com>
---
 .../bindings/mfd/apple,smc.yaml          |  9 +++++++
 .../bindings/rtc/apple,smc-rtc.yaml      | 35 +++++++++++++++++++++++++
 MAINTAINERS                              |  1 +
 3 files changed, 45 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/apple,smc.yaml b/Documentation/devicetree/bindings/mfd/apple,smc.yaml
index 8a10e270d421ecd703848f64af597de351fcfd74..38f077867bdeedba8a486a63e366e9c943a75681 100644
--- a/Documentation/devicetree/bindings/mfd/apple,smc.yaml
+++ b/Documentation/devicetree/bindings/mfd/apple,smc.yaml
@@ -41,6 +41,9 @@ properties:
   reboot:
     $ref: /schemas/power/reset/apple,smc-reboot.yaml
 
+  rtc:
+    $ref: /schemas/rtc/apple,smc-rtc.yaml
+
 additionalProperties: false
 
 required:
@@ -75,5 +78,11 @@ examples:
           nvmem-cell-names = "shutdown_flag", "boot_stage",
                              "boot_error_count", "panic_count";
         };
+
+        rtc {
+          compatible = "apple,smc-rtc";
+          nvmem-cells = <&rtc_offset>;
+          nvmem-cell-names = "rtc_offset";
+       };
       };
     };
diff --git a/Documentation/devicetree/bindings/rtc/apple,smc-rtc.yaml b/Documentation/devicetree/bindings/rtc/apple,smc-rtc.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..607b610665a28b3ea2e86bd90cb5f3f28ebac726
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/apple,smc-rtc.yaml
@@ -0,0 +1,35 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/rtc/apple,smc-rtc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Apple SMC RTC
+
+description:
+  Apple Silicon Macs (M1, etc.) have an RTC that is part of the PMU IC,
+  but most of the PMU functionality is abstracted out by the SMC.
+  An additional RTC offset stored inside NVMEM is required to compute
+  the current date/time.
+
+maintainers:
+  - Sven Peter <sven@kernel.org>
+
+properties:
+  compatible:
+    const: apple,smc-rtc
+
+  nvmem-cells:
+    items:
+      - description: 48bit RTC offset, specified in 32768 (2^15) Hz clock ticks
+
+  nvmem-cell-names:
+    items:
+      - const: rtc_offset
+
+required:
+  - compatible
+  - nvmem-cells
+  - nvmem-cell-names
+
+additionalProperties: false
diff --git a/MAINTAINERS b/MAINTAINERS
index fe168477caa45799dfe07de2f54de6d6a1ce0615..aaef8634985b35f54de1123ebb4176602066d177 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2397,6 +2397,7 @@ F:	Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
 F:	Documentation/devicetree/bindings/power/apple*
 F:	Documentation/devicetree/bindings/power/reset/apple,smc-reboot.yaml
 F:	Documentation/devicetree/bindings/pwm/apple,s5l-fpwm.yaml
+F:	Documentation/devicetree/bindings/rtc/apple,smc-rtc.yaml
 F:	Documentation/devicetree/bindings/spi/apple,spi.yaml
 F:	Documentation/devicetree/bindings/spmi/apple,spmi.yaml
 F:	Documentation/devicetree/bindings/watchdog/apple,wdt.yaml

-- 
2.51.0


^ permalink raw reply related

* [PATCH v2 02/11] dt-bindings: hwmon: Add Apple System Management Controller hwmon schema
From: James Calligeros @ 2025-08-27 11:22 UTC (permalink / raw)
  To: Sven Peter, Janne Grunau, Alyssa Rosenzweig, Neal Gompa,
	Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Alexandre Belloni, Jean Delvare, Guenter Roeck, Dmitry Torokhov
  Cc: asahi, linux-arm-kernel, devicetree, linux-kernel, linux-rtc,
	linux-hwmon, linux-input, James Calligeros
In-Reply-To: <20250827-macsmc-subdevs-v2-0-ce5e99d54c28@gmail.com>

Apple Silicon devices integrate a vast array of sensors, monitoring
current, power, temperature, and voltage across almost every part of
the system. The sensors themselves are all connected to the System
Management Controller (SMC). The SMC firmware exposes the data
reported by these sensors via its standard FourCC-based key-value
API. The SMC is also responsible for monitoring and controlling any
fans connected to the system, exposing them in the same way.

For reasons known only to Apple, each device exposes its sensors with
an almost totally unique set of keys. This is true even for devices
which share an SoC. An M1 Mac mini, for example, will report its core
temperatures on different keys to an M1 MacBook Pro. Worse still, the
SMC does not provide a way to enumerate the available keys at runtime,
nor do the keys follow any sort of reasonable or consistent naming
rules that could be used to deduce their purpose. We must therefore
know which keys are present on any given device, and which function
they serve, ahead of time.

Add a schema so that we can describe the available sensors for a given
Apple Silicon device in the Devicetree.

Signed-off-by: James Calligeros <jcalligeros99@gmail.com>
---
 .../bindings/hwmon/apple,smc-hwmon.yaml  | 132 +++++++++++++++++++++++++
 .../bindings/mfd/apple,smc.yaml          |  36 +++++++
 MAINTAINERS                              |   1 +
 3 files changed, 169 insertions(+)

diff --git a/Documentation/devicetree/bindings/hwmon/apple,smc-hwmon.yaml b/Documentation/devicetree/bindings/hwmon/apple,smc-hwmon.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..08cc4f55f3a41ca8b3b428088f96240266fa42e8
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/apple,smc-hwmon.yaml
@@ -0,0 +1,132 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/hwmon/apple,smc-hwmon.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Apple SMC Hardware Monitoring
+
+description:
+  Apple's System Management Controller (SMC) exposes a vast array of
+  hardware monitoring sensors, including temperature probes, current and
+  voltage sense, power meters, and fan speeds. It also provides endpoints
+  to manually control the speed of each fan individually. Each Apple
+  Silicon device exposes a different set of endpoints via SMC keys. This
+  is true even when two machines share an SoC. The CPU core temperature
+  sensor keys on an M1 Mac mini are different to those on an M1 MacBook
+  Pro, for example.
+
+maintainers:
+  - James Calligeros <jcalligeros99@gmail.com>
+
+definitions:
+  apple,key-id:
+    $ref: /schemas/types.yaml#/definitions/string
+    pattern: "^[A-Za-z0-9]{4}$"
+    description: The SMC FourCC key of the desired sensor.
+      Must match the node's suffix.
+
+  label:
+    description: Human-readable name for the sensor
+
+properties:
+  compatible:
+    const: apple,smc-hwmon
+
+patternProperties:
+  "^current-[A-Za-z0-9]{4}$":
+    type: object
+    additionalProperties: false
+
+    properties:
+      apple,key-id:
+        $ref: "#/definitions/apple,key-id"
+
+      label:
+        $ref: "#/definitions/label"
+
+    required:
+      - apple,key-id
+      - label
+
+  "^fan-[A-Za-z0-9]{4}$":
+    type: object
+    additionalProperties: false
+
+    properties:
+      apple,key-id:
+        $ref: "#/definitions/apple,key-id"
+
+      apple,fan-minimum:
+        $ref: /schemas/types.yaml#/definitions/string
+        pattern: "^[A-Za-z0-9]{4}$"
+        description: SMC key containing the fan's minimum speed
+
+      apple,fan-maximum:
+        $ref: /schemas/types.yaml#/definitions/string
+        pattern: "^[A-Za-z0-9]{4}$"
+        description: SMC key containing the fan's maximum speed
+
+      apple,fan-target:
+        $ref: /schemas/types.yaml#/definitions/string
+        pattern: "^[A-Za-z0-9]{4}$"
+        description: Writeable endpoint for setting desired fan speed
+
+      apple,fan-mode:
+        $ref: /schemas/types.yaml#/definitions/string
+        pattern: "^[A-Za-z0-9]{4}$"
+        description: Writeable key to enable/disable manual fan control
+
+      label:
+        $ref: "#/definitions/label"
+
+    required:
+      - apple,key-id
+      - label
+
+  "^power-[A-Za-z0-9]{4}$":
+    type: object
+    additionalProperties: false
+
+    properties:
+      apple,key-id:
+        $ref: "#/definitions/apple,key-id"
+
+      label:
+        $ref: "#/definitions/label"
+
+    required:
+      - apple,key-id
+      - label
+
+  "^temperature-[A-Za-z0-9]{4}$":
+    type: object
+    additionalProperties: false
+
+    properties:
+      apple,key-id:
+        $ref: "#/definitions/apple,key-id"
+
+      label:
+        $ref: "#/definitions/label"
+
+    required:
+      - apple,key-id
+      - label
+
+  "^voltage-[A-Za-z0-9]{4}$":
+    type: object
+    additionalProperties: false
+
+    properties:
+      apple,key-id:
+        $ref: "#/definitions/apple,key-id"
+
+      label:
+        $ref: "#/definitions/label"
+
+    required:
+      - apple,key-id
+      - label
+
+additionalProperties: false
diff --git a/Documentation/devicetree/bindings/mfd/apple,smc.yaml b/Documentation/devicetree/bindings/mfd/apple,smc.yaml
index 38f077867bdeedba8a486a63e366e9c943a75681..3fc4aa39292395f27b5694550858c6b34d18308d 100644
--- a/Documentation/devicetree/bindings/mfd/apple,smc.yaml
+++ b/Documentation/devicetree/bindings/mfd/apple,smc.yaml
@@ -44,6 +44,9 @@ properties:
   rtc:
     $ref: /schemas/rtc/apple,smc-rtc.yaml
 
+  hwmon:
+    $ref: /schemas/hwmon/apple,smc-hwmon.yaml
+
 additionalProperties: false
 
 required:
@@ -84,5 +87,38 @@ examples:
           nvmem-cells = <&rtc_offset>;
           nvmem-cell-names = "rtc_offset";
        };
+
+        hwmon {
+          compatible = "apple,smc-hwmon";
+
+          current-ID0R {
+            apple,key-id = "ID0R";
+            label = "AC Input Current";
+          };
+
+          fan-F0Ac {
+            apple,key-id = "F0Ac";
+            apple,fan-minimum = "F0Mn";
+            apple,fan-maximum = "F0Mx";
+            apple,fan-target = "F0Tg";
+            apple,fan-mode = "F0Md";
+            label = "Fan 1";
+          };
+
+          power-PSTR {
+            apple,key-id = "PSTR";
+            label = "Total System Power";
+          };
+
+          temperature-TW0P {
+            apple,key-id = "TW0P";
+            label = "WiFi/BT Module Temperature";
+          };
+
+          voltage-VD0R {
+            apple,key-id = "VD0R";
+            label = "AC Input Voltage";
+          };
+        };
       };
     };
diff --git a/MAINTAINERS b/MAINTAINERS
index aaef8634985b35f54de1123ebb4176602066d177..56aabfbc2520749beb9dba235f8e86c15e17b7b6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2380,6 +2380,7 @@ F:	Documentation/devicetree/bindings/cpufreq/apple,cluster-cpufreq.yaml
 F:	Documentation/devicetree/bindings/dma/apple,admac.yaml
 F:	Documentation/devicetree/bindings/gpio/apple,smc-gpio.yaml
 F:	Documentation/devicetree/bindings/gpu/apple,agx.yaml
+F:	Documentation/devicetree/bindings/hwmon/apple,smc-hwmon.yaml
 F:	Documentation/devicetree/bindings/i2c/apple,i2c.yaml
 F:	Documentation/devicetree/bindings/input/touchscreen/apple,z2-multitouch.yaml
 F:	Documentation/devicetree/bindings/interrupt-controller/apple,*

-- 
2.51.0


^ permalink raw reply related

* [PATCH v2 03/11] rtc: Add new rtc-macsmc driver for Apple Silicon Macs
From: James Calligeros @ 2025-08-27 11:22 UTC (permalink / raw)
  To: Sven Peter, Janne Grunau, Alyssa Rosenzweig, Neal Gompa,
	Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Alexandre Belloni, Jean Delvare, Guenter Roeck, Dmitry Torokhov
  Cc: asahi, linux-arm-kernel, devicetree, linux-kernel, linux-rtc,
	linux-hwmon, linux-input, James Calligeros, Hector Martin
In-Reply-To: <20250827-macsmc-subdevs-v2-0-ce5e99d54c28@gmail.com>

From: Hector Martin <marcan@marcan.st>

Apple Silicon Macs (M1, etc.) have an RTC that is part of the PMU IC,
but most of the PMU functionality is abstracted out by the SMC.
On T600x machines, the RTC counter must be accessed via the SMC to
get full functionality, and it seems likely that future machines
will move towards making SMC handle all RTC functionality.

The SMC RTC counter access is implemented on all current machines
as of the time of this writing, on firmware 12.x. However, the RTC
offset (needed to set the time) is still only accessible via direct
PMU access. To handle this, we expose the RTC offset as an NVMEM
cell from the SPMI PMU device node, and this driver consumes that
cell and uses it to compute/set the current time.

Reviewed-by: Neal Gompa <neal@gompa.dev>
Signed-off-by: Hector Martin <marcan@marcan.st>
Signed-off-by: Sven Peter <sven@kernel.org>
Signed-off-by: James Calligeros <jcalligeros99@gmail.com>
---
 MAINTAINERS              |   1 +
 drivers/rtc/Kconfig      |  11 ++
 drivers/rtc/Makefile     |   1 +
 drivers/rtc/rtc-macsmc.c | 141 +++++++++++++++++++++++++
 4 files changed, 154 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 56aabfbc2520749beb9dba235f8e86c15e17b7b6..5d53d243dc9abdf1db5865f8e6bcddbac3eafebe 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2422,6 +2422,7 @@ F:	drivers/nvmem/apple-spmi-nvmem.c
 F:	drivers/pinctrl/pinctrl-apple-gpio.c
 F:	drivers/power/reset/macsmc-reboot.c
 F:	drivers/pwm/pwm-apple.c
+F:	drivers/rtc/rtc-macsmc.c
 F:	drivers/soc/apple/*
 F:	drivers/spi/spi-apple.c
 F:	drivers/spmi/spmi-apple-controller.c
diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 64f6e9756aff4a1f6f6c50f9b4fc2140f66a8578..d28a46a89c85e6b30b402aec155e8972ed2aaa8e 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -2068,6 +2068,17 @@ config RTC_DRV_WILCO_EC
 	  This can also be built as a module. If so, the module will
 	  be named "rtc_wilco_ec".
 
+config RTC_DRV_MACSMC
+	tristate "Apple Mac System Management Controller RTC"
+	depends on MFD_MACSMC
+	help
+	  If you say yes here you get support for RTC functions
+	  inside Apple SPMI PMUs accessed through the SoC's
+	  System Management Controller
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called rtc-macsmc.
+
 config RTC_DRV_MSC313
 	tristate "MStar MSC313 RTC"
         depends on ARCH_MSTARV7 || COMPILE_TEST
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index 789bddfea99d8fcd024566891c37ee73e527cf93..bcb43b5878a562454986cbb9ab8cc45cec248dda 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -93,6 +93,7 @@ obj-$(CONFIG_RTC_DRV_M48T35)	+= rtc-m48t35.o
 obj-$(CONFIG_RTC_DRV_M48T59)	+= rtc-m48t59.o
 obj-$(CONFIG_RTC_DRV_M48T86)	+= rtc-m48t86.o
 obj-$(CONFIG_RTC_DRV_MA35D1)	+= rtc-ma35d1.o
+obj-$(CONFIG_RTC_DRV_MACSMC)	+= rtc-macsmc.o
 obj-$(CONFIG_RTC_DRV_MAX31335)	+= rtc-max31335.o
 obj-$(CONFIG_RTC_DRV_MAX6900)	+= rtc-max6900.o
 obj-$(CONFIG_RTC_DRV_MAX6902)	+= rtc-max6902.o
diff --git a/drivers/rtc/rtc-macsmc.c b/drivers/rtc/rtc-macsmc.c
new file mode 100644
index 0000000000000000000000000000000000000000..05e360277f630f3368b2856aadef1f2b96426c37
--- /dev/null
+++ b/drivers/rtc/rtc-macsmc.c
@@ -0,0 +1,141 @@
+// SPDX-License-Identifier: GPL-2.0-only OR MIT
+/*
+ * Apple SMC RTC driver
+ * Copyright The Asahi Linux Contributors
+ */
+
+#include <linux/bitops.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/macsmc.h>
+#include <linux/module.h>
+#include <linux/nvmem-consumer.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/rtc.h>
+#include <linux/slab.h>
+
+/* 48-bit RTC */
+#define RTC_BYTES 6
+#define RTC_BITS (8 * RTC_BYTES)
+
+/* 32768 Hz clock */
+#define RTC_SEC_SHIFT 15
+
+struct macsmc_rtc {
+	struct device *dev;
+	struct apple_smc *smc;
+	struct rtc_device *rtc_dev;
+	struct nvmem_cell *rtc_offset;
+};
+
+static int macsmc_rtc_get_time(struct device *dev, struct rtc_time *tm)
+{
+	struct macsmc_rtc *rtc = dev_get_drvdata(dev);
+	u64 ctr = 0, off = 0;
+	time64_t now;
+	void *p_off;
+	size_t len;
+	int ret;
+
+	ret = apple_smc_read(rtc->smc, SMC_KEY(CLKM), &ctr, RTC_BYTES);
+	if (ret < 0)
+		return ret;
+	if (ret != RTC_BYTES)
+		return -EIO;
+
+	p_off = nvmem_cell_read(rtc->rtc_offset, &len);
+	if (IS_ERR(p_off))
+		return PTR_ERR(p_off);
+	if (len < RTC_BYTES) {
+		kfree(p_off);
+		return -EIO;
+	}
+
+	memcpy(&off, p_off, RTC_BYTES);
+	kfree(p_off);
+
+	/* Sign extend from 48 to 64 bits, then arithmetic shift right 15 bits to get seconds */
+	now = sign_extend64(ctr + off, RTC_BITS - 1) >> RTC_SEC_SHIFT;
+	rtc_time64_to_tm(now, tm);
+
+	return ret;
+}
+
+static int macsmc_rtc_set_time(struct device *dev, struct rtc_time *tm)
+{
+	struct macsmc_rtc *rtc = dev_get_drvdata(dev);
+	u64 ctr = 0, off = 0;
+	int ret;
+
+	ret = apple_smc_read(rtc->smc, SMC_KEY(CLKM), &ctr, RTC_BYTES);
+	if (ret < 0)
+		return ret;
+	if (ret != RTC_BYTES)
+		return -EIO;
+
+	/* This sets the offset such that the set second begins now */
+	off = (rtc_tm_to_time64(tm) << RTC_SEC_SHIFT) - ctr;
+	return nvmem_cell_write(rtc->rtc_offset, &off, RTC_BYTES);
+}
+
+static const struct rtc_class_ops macsmc_rtc_ops = {
+	.read_time = macsmc_rtc_get_time,
+	.set_time = macsmc_rtc_set_time,
+};
+
+static int macsmc_rtc_probe(struct platform_device *pdev)
+{
+	struct apple_smc *smc = dev_get_drvdata(pdev->dev.parent);
+	struct macsmc_rtc *rtc;
+
+	/*
+	 * MFD will probe this device even without a node in the device tree,
+	 * thus bail out early if the SMC on the current machines does not
+	 * support RTC and has no node in the device tree.
+	 */
+	if (!pdev->dev.of_node)
+		return -ENODEV;
+
+	rtc = devm_kzalloc(&pdev->dev, sizeof(*rtc), GFP_KERNEL);
+	if (!rtc)
+		return -ENOMEM;
+
+	rtc->dev = &pdev->dev;
+	rtc->smc = smc;
+
+	rtc->rtc_offset = devm_nvmem_cell_get(&pdev->dev, "rtc_offset");
+	if (IS_ERR(rtc->rtc_offset))
+		return dev_err_probe(&pdev->dev, PTR_ERR(rtc->rtc_offset),
+				     "Failed to get rtc_offset NVMEM cell\n");
+
+	rtc->rtc_dev = devm_rtc_allocate_device(&pdev->dev);
+	if (IS_ERR(rtc->rtc_dev))
+		return PTR_ERR(rtc->rtc_dev);
+
+	rtc->rtc_dev->ops = &macsmc_rtc_ops;
+	rtc->rtc_dev->range_min = S64_MIN >> (RTC_SEC_SHIFT + (64 - RTC_BITS));
+	rtc->rtc_dev->range_max = S64_MAX >> (RTC_SEC_SHIFT + (64 - RTC_BITS));
+
+	platform_set_drvdata(pdev, rtc);
+
+	return devm_rtc_register_device(rtc->rtc_dev);
+}
+
+static const struct of_device_id macsmc_rtc_of_table[] = {
+	{ .compatible = "apple,smc-rtc", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, macsmc_rtc_of_table);
+
+static struct platform_driver macsmc_rtc_driver = {
+	.driver = {
+		.name = "macsmc-rtc",
+		.of_match_table = macsmc_rtc_of_table,
+	},
+	.probe = macsmc_rtc_probe,
+};
+module_platform_driver(macsmc_rtc_driver);
+
+MODULE_LICENSE("Dual MIT/GPL");
+MODULE_DESCRIPTION("Apple SMC RTC driver");
+MODULE_AUTHOR("Hector Martin <marcan@marcan.st>");

-- 
2.51.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